> -----Original Message-----
> From: Eric Auger <[email protected]>
> Sent: 12 March 2026 15:11
> 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]; [email protected]; Krishnakant Jaju
> <[email protected]>; [email protected]
> Subject: Re: [PATCH v3 17/32] hw/arm/tegra241-cmdqv: mmap VINTF Page0
> for CMDQV
> 
> External email: Use caution opening links or attachments
> 
> 
> On 3/11/26 2:59 PM, Shameer Kolothum Thodi wrote:
> >
> >> -----Original Message-----
> >> From: Eric Auger <[email protected]>
> >> Sent: 11 March 2026 13:19
> >> 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]; [email protected]; Krishnakant Jaju
> >> <[email protected]>; [email protected]
> >> Subject: Re: [PATCH v3 17/32] hw/arm/tegra241-cmdqv: mmap VINTF
> Page0
> >> for CMDQV
> >>
> >> External email: Use caution opening links or attachments
> >>
> >>
> >> On 3/11/26 1:34 PM, Shameer Kolothum Thodi wrote:
> >>>> -----Original Message-----
> >>>> From: Eric Auger <[email protected]>
> >>>> Sent: 11 March 2026 10:05
> >>>> 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]; [email protected]; Krishnakant Jaju
> >>>> <[email protected]>; [email protected]
> >>>> Subject: Re: [PATCH v3 17/32] hw/arm/tegra241-cmdqv: mmap VINTF
> >> Page0
> >>>> for CMDQV
> >>>>
> >>>> External email: Use caution opening links or attachments
> >>>>
> >>>>
> >>>> On 3/11/26 10:26 AM, Shameer Kolothum Thodi wrote:
> >>>>>> -----Original Message-----
> >>>>>> From: Eric Auger <[email protected]>
> >>>>>> Sent: 11 March 2026 07:55
> >>>>>> 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]; [email protected]; Krishnakant
> Jaju
> >>>>>> <[email protected]>; [email protected]
> >>>>>> Subject: Re: [PATCH v3 17/32] hw/arm/tegra241-cmdqv: mmap
> VINTF
> >>>> Page0
> >>>>>> for CMDQV
> >>>>>>
> >>>>>> External email: Use caution opening links or attachments
> >>>>>>
> >>>>>>
> >>>>>> On 2/26/26 11:50 AM, Shameer Kolothum wrote:
> >>>>>>> From: Nicolin Chen <[email protected]>
> >>>>>>>
> >>>>>>> Global VCMDQ pages provide a VM wide view of all VCMDQs, while
> the
> >>>>>>> VINTF pages expose a logical view local to a given VINTF. Although 
> >>>>>>> real
> >>>>>>> hardware may support multiple VINTFs, the kernel currently exposes
> a
> >>>>>>> single VINTF per VM.
> >>>>>>>
> >>>>>>> The kernel provides an mmap offset for the VINTF Page0 region
> during
> >>>>>>> vIOMMU allocation. However, the logical-to-physical association
> >> between
> >>>>>>> VCMDQs and a VINTF is only established after HW_QUEUE allocation.
> >> Prior
> >>>>>>> to that, the mapped Page0 does not back any real VCMDQ state.
> >>>>>>>
> >>>>>>> When VINTF is enabled, mmap the kernel provided Page0 region and
> >>>>>>> unmap it when VINTF is disabled. This prepares the VINTF mapping
> >>>>>>> in advance of subsequent patches that add VCMDQ allocation
> support.
> >>>>>> So at some point we transition from something that is purely emulated
> >>>>>> (page 1 global cmdq) to something that is mmapped on a host page.
> >> How
> >>>> do
> >>>>>> we transfer the state of the cmdq from one to the other?
> >>>>> Right. If a guest uses both the "Global VCMDQ registers Page0" and the
> >>>>> "VINTF0 Logical VCMDQ registers Page0" interchangeably (and I see
> >>>>> nothing in the spec that forbids this), then we need to keep the two
> >>>>> views in sync.
> >>>> Also assuming the global VCMDQs are accessible and thus used, I guess
> we
> >>>> shall properly handle the actual commands (equivalent of
> >>>> smmuv3_cmdq_consume()), no?
> >>>> I don't see it done at the moment.
> >>>>
> >>>> By the way, do we want support of VCMDQs in fully emulated mode. I
> >> guess
> >>>> it would be sensible?
> >>> Yes, command handling is not implemented now.
> >>>
> >>> Since this is part of the accelerated SMMUv3 feature, I am not sure it
> >>> makes sense to implement full command handling in QEMU and then
> >> forward
> >>> the supported invalidation commands back to the host again.
> >>>
> >>> The idea here is to accelerate the CMDQ processing through the hardware.
> >>> I am not sure a guest would have a real use case for VCMDQ in a fully
> >>> emulated mode.
> >> OK but really the whole user model is really confusing. One starts using
> >> a "global" vcmdq and then switches to a logical one.
> >> On real HW I guess they are synced. But on emulation, currently they are
> >> not. Also when the guest is using the global vcmdq, it can
> >> access the registers but they actually do not trigger any commands. This
> >> really needs to be clarified.
> > Yes, this could use better documentation. I can clarify the model in the
> > commit message and code comments.
> >
> > One option we discussed earlier to avoid the confusion was to restrict
> > usage to the "VINTF logical VCMDQ" interface and return an error if the
> > guest accesses the "Global VCMDQ registers".
> >
> > However, since this is still MMIO space, the thinking was that it may be
> > better to allow the access even if it is effectively non-functional, rather
> > than making the behaviour unpredictable.
> >
> > See the discussion:
> > https://lore.kernel.org/qemu-devel/aa8cJszkOKGLUYdD@Asurada-Nvidia/
> >
> >> Then I understand we want to focus on accel mode but above points need
> >> to be clatified. I just wanted to mention that vcmdq should logically
> >> also work in fully emulated mode (and you seemed to implement a mix
> >> actually when you have emulated access to vi vcmdq) as it replaces the
> >> SMMU standard cmdq, if I understand correctly
> > IIUC, in the current Linux implementation VCMDQ does not replace the
> > standard SMMU CMDQ from the guest perspective. This is also why we do
> > not expose the HYP_OWN bit to the guest.
> > When HYP_OWN is not set, guest still uses the normal SMMU CMDQ.
> > CMDQ is only used as a fast-path for a small set of invalidation commands
> > that can be sent directly to the host hardware.
> >
> > See:
> >
> > static bool tegra241_guest_vcmdq_supports_cmd(struct
> arm_smmu_cmdq_ent *ent)
> > {
> >         switch (ent->opcode) {
> >         case CMDQ_OP_TLBI_NH_ASID:
> >         case CMDQ_OP_TLBI_NH_VA:
> >         case CMDQ_OP_ATC_INV:
> >                 return true;
> >         default:
> >                 return false;
> >         }
> > }
> >
> > So VCMDQ is not a replacement for the SMMU CMDQ in the guest. It is
> > only used to accelerate these invalidation commands.
> 
> I understand. But guest works in HYP_OWN mode as it sets the HYP_OWN
> bit. So resetting it looks like a hack to me, not documented in any way
> in the spec. Besides, as tegra241_guest_vcmdq_supports_cmd() forces to
> send on the cmdqv cmdq only legal cmds I don't see any problem leaving
> the bit to 1. What is important is that the host does the check (and the
> host sets HYP_OWN to 0 for that cmdq). I did the trial to comment
>         //value &= ~R_VINTF0_CONFIG_HYP_OWN_MASK;
> 
> and it seems to work properly

Ah.. that’s interesting. .I was under the impression that we are toggling
the HYP_OWN so that guest does sets the tegra241_guest_vcmdq_supports_cmd()
correctly and delegates the supported cmds through that queue.

See, 
    tegra241_cmdqv_init_structures()
        tegra241_vintf_alloc_lvcmdq()
          tegra241_vcmdq_alloc_smmu_cmdq()
          {
               .... 
                if (!vcmdq->vintf->hyp_own)
                      cmdq->supports_cmd = tegra241_guest_vcmdq_supports_cmd;
            
However, it looks like the above happens before tegra241_vintf_hw_init(),
which sets HYP_OWN. That may explain why toggling the bit here does not
make any difference.

I need to double check why we did the toggling in the QEMU then.

@Nicolin, do you remember?

Thanks,
Shameer

Reply via email to