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

["Wu, Fei" <fei2.wu@intel.com>]

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
>>>>
>>>
>>
>