* [PATCH v2] riscv: enable multi-range memory layout @ 2023-09-14 5:30 Fei Wu 2023-09-14 6:05 ` Heinrich Schuchardt 0 siblings, 1 reply; 14+ messages in thread From: Fei Wu @ 2023-09-14 5:30 UTC (permalink / raw) To: xypron.glpk, rick, ycliang, bmeng.cn, sjg, pali, u-boot, andrei.warkentin, sunilvl Cc: Fei Wu 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 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@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(); } 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 -- 2.34.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v2] riscv: enable multi-range memory layout 2023-09-14 5:30 [PATCH v2] riscv: enable multi-range memory layout Fei Wu @ 2023-09-14 6:05 ` Heinrich Schuchardt 2023-09-14 6:48 ` Wu, Fei 2023-10-09 10:04 ` Wu, Fei 0 siblings, 2 replies; 14+ messages in thread From: Heinrich Schuchardt @ 2023-09-14 6:05 UTC (permalink / raw) To: Fei Wu, rick, ycliang, bmeng.cn, sjg, pali, u-boot, andrei.warkentin, sunilvl Am 14. September 2023 07:30:55 MESZ schrieb Fei Wu <fei2.wu@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. >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@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. 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2] riscv: enable multi-range memory layout 2023-09-14 6:05 ` Heinrich Schuchardt @ 2023-09-14 6:48 ` Wu, Fei 2023-09-14 7:17 ` Wu, Fei 2023-09-14 7:20 ` Heinrich Schuchardt 2023-10-09 10:04 ` Wu, Fei 1 sibling, 2 replies; 14+ messages in thread From: Wu, Fei @ 2023-09-14 6:48 UTC (permalink / raw) To: Heinrich Schuchardt, rick, ycliang, bmeng.cn, sjg, pali, u-boot, andrei.warkentin, sunilvl On 9/14/2023 2:05 PM, Heinrich Schuchardt wrote: > > > Am 14. September 2023 07:30:55 MESZ schrieb Fei Wu <fei2.wu@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; } >> 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@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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2] riscv: enable multi-range memory layout 2023-09-14 6:48 ` Wu, Fei @ 2023-09-14 7:17 ` Wu, Fei 2023-09-14 7:20 ` Heinrich Schuchardt 1 sibling, 0 replies; 14+ messages in thread From: Wu, Fei @ 2023-09-14 7:17 UTC (permalink / raw) To: Heinrich Schuchardt, rick, ycliang, bmeng.cn, sjg, pali, u-boot, andrei.warkentin, sunilvl On 9/14/2023 2:48 PM, 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@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; > } > >>> 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@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. > The whole idea of this change is to find an address with backed ram, because of the current implementation (or limitation) of board_get_usable_ram_top(), the lowest range is returned as a solution. If the Linaro work can remove that limitation, likely we are both fine. The next question is when will this Linaro work be upstreamed. I'm changing the memory layout for PCIe passthrough on QEMU, it's better to see u-boot is ready before QEMU change, or the system is not able to boot up. Thanks, Fei. > 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 > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2] riscv: enable multi-range memory layout 2023-09-14 6:48 ` Wu, Fei 2023-09-14 7:17 ` Wu, Fei @ 2023-09-14 7:20 ` Heinrich Schuchardt 2023-09-14 7:42 ` Wu, Fei 2023-09-26 5:07 ` Anup Patel 1 sibling, 2 replies; 14+ messages in thread From: Heinrich Schuchardt @ 2023-09-14 7:20 UTC (permalink / raw) To: Wu, Fei Cc: rick, ycliang, bmeng.cn, sjg, u-boot, pali, andrei.warkentin, sunilvl 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@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? 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@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 > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2] riscv: enable multi-range memory layout 2023-09-14 7:20 ` Heinrich Schuchardt @ 2023-09-14 7:42 ` Wu, Fei 2023-09-14 10:21 ` Heinrich Schuchardt 2023-09-26 5:07 ` Anup Patel 1 sibling, 1 reply; 14+ messages in thread From: Wu, Fei @ 2023-09-14 7:42 UTC (permalink / raw) To: Heinrich Schuchardt Cc: rick, ycliang, bmeng.cn, sjg, u-boot, pali, andrei.warkentin, sunilvl On 9/14/2023 3:20 PM, Heinrich Schuchardt 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@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? > Please try this: base: ccb86f079a9e patch: https://lore.kernel.org/all/CAKmqyKMtAzt5saCUMd4vXYfgAQibpzQJAhtTSuSb+yeKhcYpfw@mail.gmail.com/T/ Thanks, Fei. > 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@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 >> > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2] riscv: enable multi-range memory layout 2023-09-14 7:42 ` Wu, Fei @ 2023-09-14 10:21 ` Heinrich Schuchardt 2023-09-14 11:19 ` Heinrich Schuchardt ` (2 more replies) 0 siblings, 3 replies; 14+ messages in thread From: Heinrich Schuchardt @ 2023-09-14 10:21 UTC (permalink / raw) To: Wu, Fei, Anup Patel Cc: rick, ycliang, bmeng.cn, sjg, u-boot, pali, andrei.warkentin, sunilvl On 9/14/23 09:42, Wu, Fei wrote: > On 9/14/2023 3:20 PM, Heinrich Schuchardt 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@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 capablehe >>> * 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. @Anup: The generic code was introduced by your patch 26f4fd1cb4f6 ("riscv: generic: Ensure that U-Boot runs within 4GB for 64bit systems"). Which devices have that 32bit DMA issue? fu540, fu740, jh7110 already have a cpu specific implementation and don't need the generic one. >> >> 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? >> > Please try this: > > base: ccb86f079a9e > patch: > https://lore.kernel.org/all/CAKmqyKMtAzt5saCUMd4vXYfgAQibpzQJAhtTSuSb+yeKhcYpfw@mail.gmail.com/T/ @Fei With that patch I see the two memory banks in QEMU but when I attach a vfio-pci device my whole system crashes and becomes unresponsive but that is not a U-Boot issue. Best regards Heinrich > > Thanks, > Fei. > >> 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@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 >>> >> > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2] riscv: enable multi-range memory layout 2023-09-14 10:21 ` Heinrich Schuchardt @ 2023-09-14 11:19 ` Heinrich Schuchardt 2023-09-22 6:17 ` Wu, Fei 2023-09-25 5:52 ` Wu, Fei 2 siblings, 0 replies; 14+ messages in thread From: Heinrich Schuchardt @ 2023-09-14 11:19 UTC (permalink / raw) To: Wu, Fei Cc: rick, ycliang, bmeng.cn, sjg, u-boot, pali, andrei.warkentin, sunilvl, Anup Patel On 9/14/23 12:21, Heinrich Schuchardt wrote: > On 9/14/23 09:42, Wu, Fei wrote: >> On 9/14/2023 3:20 PM, Heinrich Schuchardt wrote: <snip /> >>> >>> With which version of QEMU were you able to use PCI pass through? >>> >> Please try this: >> >> base: ccb86f079a9e >> patch: >> https://lore.kernel.org/all/CAKmqyKMtAzt5saCUMd4vXYfgAQibpzQJAhtTSuSb+yeKhcYpfw@mail.gmail.com/T/ > > @Fei > > With that patch I see the two memory banks in QEMU but when I attach a > vfio-pci device my whole system crashes and becomes unresponsive but > that is not a U-Boot issue. This problem seems to be specific to my SATA controller. Passing through USB or Ethernet works fine. Best regards Heinrich ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2] riscv: enable multi-range memory layout 2023-09-14 10:21 ` Heinrich Schuchardt 2023-09-14 11:19 ` Heinrich Schuchardt @ 2023-09-22 6:17 ` Wu, Fei 2023-09-25 5:52 ` Wu, Fei 2 siblings, 0 replies; 14+ messages in thread From: Wu, Fei @ 2023-09-22 6:17 UTC (permalink / raw) To: Heinrich Schuchardt, Anup Patel Cc: rick, ycliang, bmeng.cn, sjg, u-boot, pali, andrei.warkentin, sunilvl On 9/14/2023 6:21 PM, Heinrich Schuchardt wrote: > On 9/14/23 09:42, Wu, Fei wrote: >> On 9/14/2023 3:20 PM, Heinrich Schuchardt 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@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 capablehe >>>> * 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. > > @Anup: > > The generic code was introduced by your patch 26f4fd1cb4f6 ("riscv: > generic: Ensure that U-Boot runs within 4GB for 64bit systems"). > > Which devices have that 32bit DMA issue? > > fu540, fu740, jh7110 already have a cpu specific implementation and > don't need the generic one. > @Anup, any comments? In case you missed this email. Thanks, Fei. >>> >>> 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? >>> >> Please try this: >> >> base: ccb86f079a9e >> patch: >> https://lore.kernel.org/all/CAKmqyKMtAzt5saCUMd4vXYfgAQibpzQJAhtTSuSb+yeKhcYpfw@mail.gmail.com/T/ > > @Fei > > With that patch I see the two memory banks in QEMU but when I attach a > vfio-pci device my whole system crashes and becomes unresponsive but > that is not a U-Boot issue. > > Best regards > > Heinrich > >> >> Thanks, >> Fei. >> >>> 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@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 >>>> >>> >> > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2] riscv: enable multi-range memory layout 2023-09-14 10:21 ` Heinrich Schuchardt 2023-09-14 11:19 ` Heinrich Schuchardt 2023-09-22 6:17 ` Wu, Fei @ 2023-09-25 5:52 ` Wu, Fei 2 siblings, 0 replies; 14+ messages in thread From: Wu, Fei @ 2023-09-25 5:52 UTC (permalink / raw) To: Heinrich Schuchardt, Anup Patel Cc: rick, ycliang, bmeng.cn, sjg, u-boot, pali, andrei.warkentin, sunilvl On 9/14/2023 6:21 PM, Heinrich Schuchardt wrote: > On 9/14/23 09:42, Wu, Fei wrote: >> On 9/14/2023 3:20 PM, Heinrich Schuchardt 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@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 capablehe >>>> * 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. > > @Anup: > > The generic code was introduced by your patch 26f4fd1cb4f6 ("riscv: > generic: Ensure that U-Boot runs within 4GB for 64bit systems"). > > Which devices have that 32bit DMA issue? > > fu540, fu740, jh7110 already have a cpu specific implementation and > don't need the generic one. > Hi Heinrich, Besides ram_top, ram_base is another one we need to consider. setup_dest_addr() has: #ifdef CFG_SYS_SDRAM_BASE gd->ram_base = CFG_SYS_SDRAM_BASE; #endif gd->ram_top = gd->ram_base + get_effective_memsize(); gd->ram_top = board_get_usable_ram_top(gd->mon_len); gd->relocaddr = gd->ram_top; If ram_base is modified in #ifdef, ram_top is still not ensured to be backed by ram. I tried another workaround w/o this patch, it boots up successfully: * return gd->ram_top in board_get_usable_ram_top() that's >4GB * comment out #ifdef here. This is necessary. Thanks, Fei. >>> >>> 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? >>> >> Please try this: >> >> base: ccb86f079a9e >> patch: >> https://lore.kernel.org/all/CAKmqyKMtAzt5saCUMd4vXYfgAQibpzQJAhtTSuSb+yeKhcYpfw@mail.gmail.com/T/ > > @Fei > > With that patch I see the two memory banks in QEMU but when I attach a > vfio-pci device my whole system crashes and becomes unresponsive but > that is not a U-Boot issue. > > Best regards > > Heinrich > >> >> Thanks, >> Fei. >> >>> 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@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 >>>> >>> >> > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2] riscv: enable multi-range memory layout 2023-09-14 7:20 ` Heinrich Schuchardt 2023-09-14 7:42 ` Wu, Fei @ 2023-09-26 5:07 ` Anup Patel 2023-09-26 7:04 ` Heinrich Schuchardt 1 sibling, 1 reply; 14+ messages in thread From: Anup Patel @ 2023-09-26 5:07 UTC (permalink / raw) To: Heinrich Schuchardt Cc: Wu, Fei, rick, ycliang, bmeng.cn, sjg, u-boot, pali, andrei.warkentin, sunilvl On Thu, Sep 14, 2023 at 12:49 PM Heinrich Schuchardt <xypron.glpk@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@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@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 > > > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2] riscv: enable multi-range memory layout 2023-09-26 5:07 ` Anup Patel @ 2023-09-26 7:04 ` Heinrich Schuchardt 0 siblings, 0 replies; 14+ messages in thread From: Heinrich Schuchardt @ 2023-09-26 7:04 UTC (permalink / raw) To: Anup Patel Cc: Wu, Fei, rick, ycliang, bmeng.cn, sjg, u-boot, pali, andrei.warkentin, sunilvl On 9/26/23 07:07, Anup Patel wrote: > On Thu, Sep 14, 2023 at 12:49 PM Heinrich Schuchardt <xypron.glpk@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@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 Thanks Anup for clarification. As of today sifive_unleashed_defconfig builds arch/riscv/cpu/fu540/dram.c which contains the cpu specific implementation of board_get_usable_ram_top(). We already have a weak implementation common/board_f.c:327: __weak phys_addr_t board_get_usable_ram_top(phys_size_t total_size) So we don't need a Kconfig option. We just can drop the implementation in arch/riscv/cpu/generic/dram.c. QEMU should not need an implementation of the function but provide the implemented RAM banks via its device-tree. Best regards Heinrich ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2] riscv: enable multi-range memory layout 2023-09-14 6:05 ` Heinrich Schuchardt 2023-09-14 6:48 ` Wu, Fei @ 2023-10-09 10:04 ` Wu, Fei 2023-10-11 10:05 ` Wu, Fei 1 sibling, 1 reply; 14+ messages in thread From: Wu, Fei @ 2023-10-09 10:04 UTC (permalink / raw) To: Heinrich Schuchardt, rick, ycliang, bmeng.cn, sjg, pali, u-boot, andrei.warkentin, sunilvl On 9/14/2023 2:05 PM, Heinrich Schuchardt wrote: > > > Am 14. September 2023 07:30:55 MESZ schrieb Fei Wu <fei2.wu@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. > >> 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@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. > It's okay if we don't do this installation and pcie passthrough together on risc-v, nothing is changed if there is single-range memory, 1GB is large enough for other cases. > 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. > You mentioned a generic solution for all architectures in another thread, it looks not trivial for me with little background on u-boot, e.g. something is confusing to me, when a board should define CFG_SYS_SDRAM_BASE, what happens if gd->ram_base != CFG_SYS_SDRAM_BASE, I'm not sure if you or anyone can help drive this together. btw, the good part of this patch is that it doesn't change the semantics of ram_base, and the best choice is the largest range, the upper range might not. 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2] riscv: enable multi-range memory layout 2023-10-09 10:04 ` Wu, Fei @ 2023-10-11 10:05 ` Wu, Fei 0 siblings, 0 replies; 14+ messages in thread From: Wu, Fei @ 2023-10-11 10:05 UTC (permalink / raw) To: Heinrich Schuchardt, rick, ycliang, bmeng.cn, sjg, pali, u-boot, andrei.warkentin, sunilvl On 10/9/2023 6:04 PM, 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@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. >> >>> 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@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. >> > It's okay if we don't do this installation and pcie passthrough together > on risc-v, nothing is changed if there is single-range memory, 1GB is > large enough for other cases. > So far there are the following methods: 1. use fdtdec_setup_mem_size_base_lowest as this one. This is a change to generic riscv iff the multi-range memory is applied, which is not the case for all the current riscv boards. If there are such memory layout riscv boards in the future, it can have its specialized version. 2. specialize dram_init() for qemu-riscv, it limits the impact only to qemu-riscv, even if there is a need to download image, it doesn't need to enable pcie-passthru at the same time, so no impact from this perspective, the drawback is that qemu cannot reuse the generic logic. 3. a complete solution for multi-range memory layout to select the largest range for all architectures. Are #1 or #2 acceptable at this stage? If we go #3, can someone help me drive this together to reduce the email ping-pongs? Thanks, Fei. >> 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. >> > You mentioned a generic solution for all architectures in another > thread, it looks not trivial for me with little background on u-boot, > e.g. something is confusing to me, when a board should define > CFG_SYS_SDRAM_BASE, what happens if gd->ram_base != CFG_SYS_SDRAM_BASE, > I'm not sure if you or anyone can help drive this together. btw, the > good part of this patch is that it doesn't change the semantics of > ram_base, and the best choice is the largest range, the upper range > might not. > > 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 > ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2023-10-11 10:05 UTC | newest] Thread overview: 14+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-09-14 5:30 [PATCH v2] riscv: enable multi-range memory layout Fei Wu 2023-09-14 6:05 ` Heinrich Schuchardt 2023-09-14 6:48 ` Wu, Fei 2023-09-14 7:17 ` Wu, Fei 2023-09-14 7:20 ` Heinrich Schuchardt 2023-09-14 7:42 ` Wu, Fei 2023-09-14 10:21 ` Heinrich Schuchardt 2023-09-14 11:19 ` Heinrich Schuchardt 2023-09-22 6:17 ` Wu, Fei 2023-09-25 5:52 ` Wu, Fei 2023-09-26 5:07 ` Anup Patel 2023-09-26 7:04 ` Heinrich Schuchardt 2023-10-09 10:04 ` Wu, Fei 2023-10-11 10:05 ` Wu, Fei
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox