On 5/8/26 11:03 AM, Shameer Kolothum Thodi wrote:
> Hi Eric,
>
>> -----Original Message-----
>> From: Eric Auger <[email protected]>
>> Sent: 07 May 2026 17: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 22/31] hw/arm/tegra241-cmdqv: Map VINTF page0
>> into guest MMIO space
>>
>> External email: Use caution opening links or attachments
>>
>>
>> On 5/6/26 4:24 PM, Shameer Kolothum Thodi wrote:
>>> Hi Eric,
>>>
>>>> -----Original Message-----
>>>> From: Eric Auger <[email protected]>
>>>> Sent: 06 May 2026 13:45
>>>> 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 22/31] hw/arm/tegra241-cmdqv: Map VINTF page0
>>>> into guest MMIO space
>>>>
>>>> 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]>
>>>>>
>>>>> Once a VCMDQ is allocated, map the mmap'd vintf_page0 region directly
>>>>> into the guest-visible MMIO space at offset 0x30000 as a RAM-backed
>>>>> MemoryRegion. This eliminates QEMU trapping for hot-path CONS/PROD
>>>>> index updates.
>>>>>
>>>>> After this patch, the two VCMDQ apertures use different access paths:
>>>>> the direct aperture (0x10000) remains QEMU-trapped and writes via
>>>>> vintf_ptr, while the VI aperture (0x30000) is a direct guest RAM
>>>>> mapping. Both paths write to the same underlying vintf_page0 memory,
>>>>> so no synchronisation between the apertures is needed.
>>>> I fail to understand when the previous trapped path using ptr in
>>>> tegra241_cmdqv_read/write_vcmdq gets used versus that path. Is it
>>>> eventually used?
>>> The spec does not prevent a guest from using the 0x10000 path for
>> allocated
>>> VCMDQs, so the trap path remains valid and QEMU forwards those accesses
>> to
>>> the mmap'd vintf_page0 via vintf_ptr.
>>>
>>> We cannot map 0x10000 directly to the guest as RAM because the kernel
>>> mmap only backs VCMDQs actually allocated via HW_QUEUE ioctl. If the
>>> guest allocates only 1 of 2 VCMDQs, exposing the full direct aperture page0
>>> as RAM would give the guest access to unallocated VCMDQ slots. Hence, it
>>> remains trapped and QEMU only fwds to vintf page0 for allocated queues.
>> Actually my question rather was why don't we use a subregion_overlap for
>> global vcmdq page0, mapping to vintf_page0 once cmdqv->vintf_page0 is
>> set, just as we do for VINTF0 page0? I understand
>> tegra241_cmdqv_read/write_vcmdq would return the same content anyway.
> This is my understanding why we cannot map direct VCMDQ aperture(0x10000)
> to vintf_page0:
>
> - Unallocated VCMDQs accessed via vintf_page0 get "access dropped with no
> Fault/Interrupt" behaviour per spec((p.172)
>
> - Mapping the direct VCMDQ aperture (0x10000) to vintf_page0 would replicate
> this behaviour for unallocated VCMDQ accesses via the direct aperture too.
>
> - However, per spec p.176, software can program VCMDQs via the direct
> VCMDQ aperture without going through the VINTF. Mapping vintf_page0
> at 0x10000 violates this.
>
> - Partial allocation case (VCMDQ0 allocated, VCMDQ1 not): the trap correctly
> serves VCMDQ1 accesses from QEMU cache while forwarding VCMDQ0 accesses
> to vintf_page0. A direct RAM mapping cannot make this distinction.
I agree that normally we shouldn't use VINTF view for the global view.
When you say "the trap correctly serves VCMDQ1 accesses from QEMU cache
while forwarding VCMDQ0 accesses to vintf_page0", the filtering is done by
tegra241_cmdqv_vintf_ptr. Whatever the queue being enabled,
cmdqv->vintf_page0 is allocated. So only the test on cmdqv->vcmdq[index]
does the filtering in that case, correct?
If so I now share your pov
Eric
>
>> Besides, I would recommend that all over the code, page0 and page1 are
>> clearly differentiated in terms of accessors and comments and we use the
>> same terminology through the code for global vcmdq and vintf vcmdqs.
> Sure. I will address this.
>
> Thanks,
> Shameer
>
