Re: [PATCH v3 17/32] hw/arm/tegra241-cmdqv: mmap VINTF Page0 for CMDQV

[Nicolin Chen <nicolinc@nvidia.com>]

On Thu, Mar 12, 2026 at 11:05:36AM -0700, Shameer Kolothum Thodi wrote:
> > 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.

Well, looks like we exposed a kernel bug...

It seems that the sequence of setting hyp_own and supports_cmd got
inverted during a rework when I got the driver upstream.

It turned out that both host and guest kernels sets supports_cmd to
tegra241_guest_vcmdq_supports_cmd().. IOW, the host is sending non-
invalidation commands to the SMMU CMDQ rather than the HYP_OWNed
VCMDQ, although this is completely harmless and probably does not
affect host performance at all since non-invalidation commands are
not that frequently issued.

This explains why Eric's experiment didn't fail. I'll submit a fix.

With that being said, in QEMU we do need to wire HYP_OWN bit to 0.
This is not decided by the spec, but by the host SW implementation:

 550         /*
 551          * Note that HYP_OWN bit is wired to zero when running in guest kernel,
 552          * whether enabling it here or not, as !HYP_OWN cmdq HWs only support a
 553          * restricted set of supported commands.
 554          */
 555         regval = FIELD_PREP(VINTF_HYP_OWN, hyp_own) | 
 556                  FIELD_PREP(VINTF_VMID, vintf->vsmmu.vmid);
 557         writel(regval, REG_VINTF(vintf, CONFIG));
 558
 559         ret = vintf_write_config(vintf, regval | VINTF_EN);
 560         if (ret)
 561                 return ret;
 562         /*
 563          * As being mentioned above, HYP_OWN bit is wired to zero for a guest
 564          * kernel, so read it back from HW to ensure that reflects in hyp_own
 565          */
 566         vintf->hyp_own = !!(VINTF_HYP_OWN & readl(REG_VINTF(vintf, CONFIG)));

This should have been mentioned in the uAPI header. I'll submit a
patch to add this line near IOMMU_VIOMMU_TYPE_TEGRA241_CMDQV.

One thing we need to get our head around is that a guest hardware
is different than the host hardware:
 - Host HW is backed by a VINTF (HYP_OWN=1)
 - Guest HW is backed by a VINTF (HYP_OWN=0)
This is a fixed configuration, decided by the hardware and the spec
also.

Thus, the guest version of the VINTF must align with its underlying
hardware, so QEMU cannot allow setting HYP_OWN=1.

Otherwise, guest OS would have no idea this is a guest version of
hardware and would issue unsupported commands that will fail.

Nicolin