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 mapping is installed lazily on first successful VCMDQ hardware > queue allocation and removed when CMDQV or VINTF is disabled. > > 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 | 37 +++++++++++++++++++++++++++++++++++++ > 2 files changed, 38 insertions(+) > > diff --git a/hw/arm/tegra241-cmdqv.h b/hw/arm/tegra241-cmdqv.h > index 039d86374f..2befa6205e 100644 > --- a/hw/arm/tegra241-cmdqv.h > +++ b/hw/arm/tegra241-cmdqv.h > @@ -46,6 +46,7 @@ typedef struct Tegra241CMDQV { > IOMMUFDVeventq *veventq; > IOMMUFDHWqueue *vcmdq[TEGRA241_CMDQV_MAX_CMDQ]; > void *vintf_page0; > + MemoryRegion *mr_vintf_page0; > > /* Register Cache */ > uint32_t config; > diff --git a/hw/arm/tegra241-cmdqv.c b/hw/arm/tegra241-cmdqv.c > index eb619e1134..bf989dd51f 100644 > --- a/hw/arm/tegra241-cmdqv.c > +++ b/hw/arm/tegra241-cmdqv.c > @@ -15,6 +15,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); > + cmdqv->mr_vintf_page0->ram_device_skip_iommu_map = true; I guess you need a setter, here to. > + 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; > @@ -72,6 +106,7 @@ static bool tegra241_cmdqv_setup_vcmdq(Tegra241CMDQV > *cmdqv, int index, > hw_queue->viommu = viommu; > cmdqv->vcmdq[index] = hw_queue; > > + tegra241_cmdqv_guest_map_vintf_page0(cmdqv); > return true; > } > > @@ -312,6 +347,7 @@ static void > tegra241_cmdqv_config_vintf_write(Tegra241CMDQV *cmdqv, > cmdqv->vintf_status |= R_VINTF0_STATUS_ENABLE_OK_MASK; > } > } else { > + tegra241_cmdqv_guest_unmap_vintf_page0(cmdqv); > tegra241_cmdqv_free_all_vcmdq(cmdqv); > tegra241_cmdqv_munmap_vintf_page0(cmdqv, errp); > cmdqv->vintf_status &= ~R_VINTF0_STATUS_ENABLE_OK_MASK; > @@ -438,6 +474,7 @@ static void tegra241_cmdqv_write_mmio(void *opaque, > hwaddr offset, > if (value & R_CONFIG_CMDQV_EN_MASK) { > cmdqv->status |= R_STATUS_CMDQV_ENABLED_MASK; > } else { > + tegra241_cmdqv_guest_unmap_vintf_page0(cmdqv); > tegra241_cmdqv_free_all_vcmdq(cmdqv); > cmdqv->status &= ~R_STATUS_CMDQV_ENABLED_MASK; > } Thanks Eric
