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

Heinrich Schuchardt
2023-09-26T07:04:00Z

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