[PATCH v2] riscv: enable multi-range memory layout

Tue Sep 26 07:07:17 CEST 2023

On Thu, Sep 14, 2023 at 12:49 PM Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
>
> On 9/14/23 08:48, Wu, Fei wrote:
> > On 9/14/2023 2:05 PM, Heinrich Schuchardt wrote:
> >>
> >>
> >> Am 14. September 2023 07:30:55 MESZ schrieb Fei Wu <fei2.wu at intel.com>:
> >>> In order to enable PCIe passthrough on qemu riscv, the physical memory
> >>> range between 3GB and 4GB is reserved. Therefore if guest has 4GB ram,
> >>> two ranges are created as [2G, 3G) and [4G, 7G), currently u-boot sets
> >>> ram_top to 4G - 1 if the gd->ram_top is above 4G in
> >>
> >> This should move to 7GiB - 1 in your example on riscv64.
> >>
> > I'm describing the current implementation of board_get_usable_ram_top()
> > in ./arch/riscv/cpu/generic/dram.c. Do you mean this function should be
> > changed? Is the comment about 32bit DMA device still valid?
> >
> > phys_size_t board_get_usable_ram_top(phys_size_t total_size)
> > {
> >          /*
> >           * Ensure that we run from first 4GB so that all
> >           * addresses used by U-Boot are 32bit addresses.
> >           *
> >           * This in-turn ensures that 32bit DMA capable
> >           * devices work fine because DMA mapping APIs will
> >           * provide 32bit DMA addresses only.
> >           */
> >          if (gd->ram_top >= SZ_4G)
> >                  return SZ_4G - 1;
> >
> >          return gd->ram_top;
> > }
>
> The comment above says 32bit DMA is board specific and not architecture
> specific. So it is wrong to have this board_get_usable_ram_top()
> function on architecture level. It makes usage of devices with all
> memory above 4 GiB impossible.
>
> It tried to pass through a SATA controller but received an error:
>
> # modprobe vfio-pci
> # echo 0000:06:00.0 > /sys/bus/pci/drivers/ahci/unbind
> # echo 1022 7091 > /sys/bus/pci/drivers/vfio-pci/new_id
> # qemu-system-riscv64 -kernel u-boot.bin -nographic -M virt -m 4G
> -device vfio-pci,host=0000:06:00.0
> qemu-system-riscv64: -device vfio-pci,host=0000:06:00.0: VFIO_MAP_DMA
> failed: Invalid argument
> qemu-system-riscv64: -device vfio-pci,host=0000:06:00.0: vfio
> 0000:06:00.0: failed to setup container for group 5: memory listener
> initialization failed: Region riscv_virt_board.ram:
> vfio_dma_map(0x55adbde66f70, 0x80000000, 0x100000000, 0x7fcd6fe00000) =
> -22 (Invalid argument)
>
> With which version of QEMU were you able to use PCI pass through?

The original S-mode SiFive unleashed board support was using the
generic CPU under arch/riscv. We have Cadance MACB ethernet
device on this board which is a 32-bit device and can't access memory
beyond 4GB.

For more details, refer "git show 26f4fd1cb4f2"

I think we can wrap the board_get_usable_ram_top() implementation
in generic RISC-V CPU with a #ifdef Kconfig option which allows
certain boards (such as QEMU-virt) to implement their own
board_get_usable_ram_top().

Regards,
Anup

>
> Best regards
>
> Heinrich
>
> >
> >>> board_get_usable_ram_top(), but that address is not backed by ram. This
> >>> patch selects the lowest range instead.
> >>>
> >>> Signed-off-by: Fei Wu <fei2.wu at intel.com>
> >>> ---
> >>> arch/riscv/cpu/generic/dram.c        | 2 +-
> >>> configs/qemu-riscv64_defconfig       | 2 +-
> >>> configs/qemu-riscv64_smode_defconfig | 2 +-
> >>> configs/qemu-riscv64_spl_defconfig   | 2 +-
> >>> 4 files changed, 4 insertions(+), 4 deletions(-)
> >>>
> >>> diff --git a/arch/riscv/cpu/generic/dram.c b/arch/riscv/cpu/generic/dram.c
> >>> index 44e11bd56c..fb53a57b4e 100644
> >>> --- a/arch/riscv/cpu/generic/dram.c
> >>> +++ b/arch/riscv/cpu/generic/dram.c
> >>> @@ -13,7 +13,7 @@ DECLARE_GLOBAL_DATA_PTR;
> >>>
> >>> int dram_init(void)
> >>> {
> >>> -   return fdtdec_setup_mem_size_base();
> >>> +   return fdtdec_setup_mem_size_base_lowest();
> >>
> >> Linaro is working on allowing to download a distro image via https and installing from a RAM disk.
> >>
> >> We should not artificially reduce the RAM size available for U-Boot by restricting ourselfs to 1 GiB.
> >>
> >> We must ensure that ram top is in the upper range.
> >>
> > How do they handle the case of >4GB? board_get_usable_ram_top() attached
> > above always returns <4GB. And it seems like
> > fdtdec_setup_mem_size_base() cannot ensure the upper one is picked, it
> > just picks up one, but which one is selected depending on fdt.
> >
> > Thanks,
> > Fei.
> >
> >> Best regards
> >>
> >> Heinrich
> >>
> >>> }
> >>>
> >>> int dram_init_banksize(void)
> >>> diff --git a/configs/qemu-riscv64_defconfig b/configs/qemu-riscv64_defconfig
> >>> index 9a8bbef192..aa55317d26 100644
> >>> --- a/configs/qemu-riscv64_defconfig
> >>> +++ b/configs/qemu-riscv64_defconfig
> >>> @@ -1,6 +1,6 @@
> >>> CONFIG_RISCV=y
> >>> CONFIG_SYS_MALLOC_LEN=0x800000
> >>> -CONFIG_NR_DRAM_BANKS=1
> >>> +CONFIG_NR_DRAM_BANKS=2
> >>> CONFIG_HAS_CUSTOM_SYS_INIT_SP_ADDR=y
> >>> CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0x80200000
> >>> CONFIG_ENV_SIZE=0x20000
> >>> diff --git a/configs/qemu-riscv64_smode_defconfig b/configs/qemu-riscv64_smode_defconfig
> >>> index 1d0f021ade..de08a49dab 100644
> >>> --- a/configs/qemu-riscv64_smode_defconfig
> >>> +++ b/configs/qemu-riscv64_smode_defconfig
> >>> @@ -1,6 +1,6 @@
> >>> CONFIG_RISCV=y
> >>> CONFIG_SYS_MALLOC_LEN=0x800000
> >>> -CONFIG_NR_DRAM_BANKS=1
> >>> +CONFIG_NR_DRAM_BANKS=2
> >>> CONFIG_HAS_CUSTOM_SYS_INIT_SP_ADDR=y
> >>> CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0x80200000
> >>> CONFIG_ENV_SIZE=0x20000
> >>> diff --git a/configs/qemu-riscv64_spl_defconfig b/configs/qemu-riscv64_spl_defconfig
> >>> index bb10145e6e..66dc2a1dd9 100644
> >>> --- a/configs/qemu-riscv64_spl_defconfig
> >>> +++ b/configs/qemu-riscv64_spl_defconfig
> >>> @@ -1,6 +1,6 @@
> >>> CONFIG_RISCV=y
> >>> CONFIG_SYS_MALLOC_LEN=0x800000
> >>> -CONFIG_NR_DRAM_BANKS=1
> >>> +CONFIG_NR_DRAM_BANKS=2
> >>> CONFIG_HAS_CUSTOM_SYS_INIT_SP_ADDR=y
> >>> CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0x80200000
> >>> CONFIG_ENV_SIZE=0x20000
> >
>