> -----Original Message----- > From: Eric Auger <[email protected]> > Sent: 04 June 2026 11:07 > 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]; Krishnakant Jaju > <[email protected]>; [email protected] > Subject: Re: [PATCH v6 21/31] hw/arm/tegra241-cmdqv: Map VINTF page0 > into guest MMIO space > > External email: Use caution opening links or attachments > > > Hi Shammer, > > On 6/1/26 1:42 PM, Shameer Kolothum wrote: > > From: Nicolin Chen <[email protected]> > I would disambiguate the title a little bit: > Use mmap'd host VINTF page0 for virtual VINTF page0 > > Also in previous patch titles you could emphase the mmap'd page is the host > one in a similar way.
Ok. > > Install the mmap'd vintf_page0 region as a RAM-device MemoryRegion at > > guest MMIO offset 0x30000 (the VINTF Page 0 aperture) when VINTF is > > enabled, and remove it on VINTF disable. This eliminates QEMU trapping > > for hot-path CONS/PROD index updates via the VINTF aperture. > > > > After this patch, the two VCMDQ Page 0 apertures use different access > > paths: the direct aperture (0x10000) remains QEMU-trapped, while the > > VINTF aperture (0x30000) is a guest-direct RAM mapping. > > > > The direct aperture is intentionally kept trapped (not aliased to the > > VINTF mmap) so that writes to an unallocated VCMDQ remain > > well-defined. The CMDQV architecture allows software to program a > > VCMDQ through the direct aperture without first allocating it to a > > VINTF; aliasing would route those writes to unallocated logical slots > > in the VINTF page, where the hardware silently drops them. > > > > Signed-off-by: Nicolin Chen <[email protected]> > > Co-developed-by: Shameer Kolothum <[email protected]> > > Signed-off-by: Shameer Kolothum <[email protected]> > > --- > > hw/arm/tegra241-cmdqv.h | 1 + > > hw/arm/tegra241-cmdqv.c | 36 > ++++++++++++++++++++++++++++++++++++ > > 2 files changed, 37 insertions(+) > > > > diff --git a/hw/arm/tegra241-cmdqv.h b/hw/arm/tegra241-cmdqv.h index > > 84499b840d..01cd6af97d 100644 > > --- a/hw/arm/tegra241-cmdqv.h > > +++ b/hw/arm/tegra241-cmdqv.h > > @@ -49,6 +49,7 @@ typedef struct Tegra241CMDQV { > > IOMMUFDVeventq *veventq; > > IOMMUFDHWqueue *vcmdq[TEGRA241_CMDQV_MAX_CMDQ]; > > void *vintf_page0; > > + MemoryRegion *mr_vintf_page0; > > > > /* CMDQ-V Config page register cache */ > > uint32_t config; > > diff --git a/hw/arm/tegra241-cmdqv.c b/hw/arm/tegra241-cmdqv.c index > > dce7089697..cdd4fa4f1a 100644 > > --- a/hw/arm/tegra241-cmdqv.c > > +++ b/hw/arm/tegra241-cmdqv.c > > @@ -18,6 +18,40 @@ > > #include "tegra241-cmdqv.h" > > #include "trace.h" > > > > +static void tegra241_cmdqv_guest_unmap_vintf_page0(Tegra241CMDQV > > +*cmdqv) { > > + if (!cmdqv->mr_vintf_page0) { > > + return; > > + } > > + > > + memory_region_del_subregion(&cmdqv->mmio_cmdqv, cmdqv- > >mr_vintf_page0); > > + object_unparent(OBJECT(cmdqv->mr_vintf_page0)); > > + g_free(cmdqv->mr_vintf_page0); > > + cmdqv->mr_vintf_page0 = NULL; > > +} > > + > > +static void tegra241_cmdqv_guest_map_vintf_page0(Tegra241CMDQV > > +*cmdqv) { > > + char *name; > > + > > + if (cmdqv->mr_vintf_page0) { > > + return; > > + } > > + > > + name = g_strdup_printf("%s vintf-page0", > > + memory_region_name(&cmdqv->mmio_cmdqv)); > > + cmdqv->mr_vintf_page0 = g_malloc0(sizeof(*cmdqv->mr_vintf_page0)); > > + memory_region_init_ram_device_ptr(cmdqv->mr_vintf_page0, > > + > > memory_region_owner(&cmdqv->mmio_cmdqv), > > + name, VINTF_PAGE_SIZE, > > + cmdqv->vintf_page0); > > + memory_region_set_skip_iommu_map(cmdqv->mr_vintf_page0, true); > > + memory_region_add_subregion_overlap(&cmdqv->mmio_cmdqv, > > + CMDQV_VINTF_PAGE0_BASE, > > + cmdqv->mr_vintf_page0, 1); > > + g_free(name); > > +} > > + > > static void tegra241_cmdqv_free_vcmdq(Tegra241CMDQV *cmdqv, int > > index) { > > IOMMUFDViommu *viommu = cmdqv->s_accel->viommu; @@ -445,6 > +479,7 > > @@ static void tegra241_cmdqv_config_vintf_write(Tegra241CMDQV > *cmdqv, > > if (value & R_VINTF0_CONFIG_ENABLE_MASK) { > > if (tegra241_cmdqv_mmap_vintf_page0(cmdqv, errp)) { > > cmdqv->vintf_status |= > > R_VINTF0_STATUS_ENABLE_OK_MASK; > > + tegra241_cmdqv_guest_map_vintf_page0(cmdqv); > don't you want to do that only after the tegra241_cmdqv_setup_all_vcmdq()? Functionally same. Could do, reads better, I think. Thanks, Shameer
