On 5/8/26 3:38 PM, Shameer Kolothum Thodi wrote:
>
>> -----Original Message-----
>> From: Eric Auger <[email protected]>
>> Sent: 07 May 2026 18:24
>> To: Shameer Kolothum Thodi <[email protected]>; qemu-
>> [email protected]; [email protected]
>> Cc: [email protected]; [email protected]; [email protected]; Nicolin
>> Chen <[email protected]>; Nathan Chen <[email protected]>; Matt
>> Ochs <[email protected]>; Jiandi An <[email protected]>; Jason Gunthorpe
>> <[email protected]>; [email protected];
>> [email protected]; Krishnakant Jaju <[email protected]>;
>> [email protected]
>> Subject: Re: [PATCH v4 26/31] hw/arm/tegra241-cmdqv: Limit queue size
>> based on backend page size
>>
>> External email: Use caution opening links or attachments
>>
>>
>> Hi Shameer,
>>
>> On 4/15/26 12:55 PM, Shameer Kolothum wrote:
>>> From: Nicolin Chen <[email protected]>
>>>
>>> CMDQV HW reads guest queue memory in its host physical address setup via
>>> IOMMUFD. This requires the guest queue memory is not only contiguous in
>>> guest PA space but also in host PA space. With Tegra241 CMDQV enabled,
>> we
>>> must only advertise a CMDQS that the host can safely back with physically
>> s/a CMDQS/ a command queue size (CMDQS)
>>> contiguous memory. Allowing a queue larger than the host page size could
>> a queue size
>>> cause the hardware to DMA across page boundaries, leading to faults.
>>>
>>> Walk the RAMBlock list to find the smallest memory-backend page size, then
>>> limit IDR1.CMDQS so the guest cannot configure a command queue that
>> exceeds
>> what is the minimal CMDQS required?
> AFAICS, spec doesn't specify a minimum. But in practical terms I think,
> it will be 4K page size(CMDQS=8).
OK
>
>>> that contiguous backing. Fall back to the real host page size if no
>>> memory-backend RAM blocks are found.
>>>
>>> Signed-off-by: Nicolin Chen <[email protected]>
>>> Signed-off-by: Shameer Kolothum <[email protected]>
>>> ---
>>> hw/arm/tegra241-cmdqv.c | 41
>> +++++++++++++++++++++++++++++++++++++++++
>>> 1 file changed, 41 insertions(+)
>>>
>>> diff --git a/hw/arm/tegra241-cmdqv.c b/hw/arm/tegra241-cmdqv.c
>>> index af68add2f0..2870886783 100644
>>> --- a/hw/arm/tegra241-cmdqv.c
>>> +++ b/hw/arm/tegra241-cmdqv.c
>>> @@ -14,6 +14,9 @@
>>> #include "hw/arm/smmuv3.h"
>>> #include "hw/core/irq.h"
>>> #include "smmuv3-accel.h"
>>> +#include "smmuv3-internal.h"
>>> +#include "system/ramblock.h"
>>> +#include "system/ramlist.h"
>>> #include "tegra241-cmdqv.h"
>>> #include "trace.h"
>>>
>>> @@ -646,9 +649,38 @@ free_viommu:
>>> return false;
>>> }
>>>
>>> +static size_t tegra241_cmdqv_min_ram_pagesize(void)
>> shouldn't we put that rather in system/physmem.c
>> there we also have qemu_ram_pagesize_largest() for instance
> Could do. But we may have to rename it differently compared to
> qemu_ram_pagesize_largest() as we are checking the size among
> memory-backend RAM only.
>
> May be qemu_ram_backend_pagesize_min() ?
Yes sounds reasonable to me. Let's add the physmem.c maintainers in cc.
Eric
>
> Thanks,
> Shameer
