> -----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. So when the mapping between a global VCMDQ and a logical VCMDQ is created through the HW_QUEUE ioctl, we should sync the state between the Global VCMDQ Page0 and the VINTF0 Logical VCMDQ Page0. I will double check this. Thanks, Shameer > Thanks > > Eric > > > > Signed-off-by: Nicolin Chen <[email protected]> > > Signed-off-by: Shameer Kolothum <[email protected]> > > --- > > hw/arm/tegra241-cmdqv.h | 3 +++ > > hw/arm/tegra241-cmdqv.c | 44 > +++++++++++++++++++++++++++++++++++++++-- > > 2 files changed, 45 insertions(+), 2 deletions(-) > > > > diff --git a/hw/arm/tegra241-cmdqv.h b/hw/arm/tegra241-cmdqv.h > > index d379b8860c..3ce9f539ae 100644 > > --- a/hw/arm/tegra241-cmdqv.h > > +++ b/hw/arm/tegra241-cmdqv.h > > @@ -18,6 +18,8 @@ > > #define TEGRA241_CMDQV_MAX_CMDQ (1U << > TEGRA241_CMDQV_NUM_CMDQ_LOG2) > > #define TEGRA241_CMDQV_NUM_SID_PER_VM_LOG2 4 > > > > +#define VINTF_PAGE_SIZE 0x10000 > > + > > /* > > * Tegra241 CMDQV MMIO layout (64KB pages) > > * > > @@ -34,6 +36,7 @@ typedef struct Tegra241CMDQV { > > SMMUv3AccelState *s_accel; > > MemoryRegion mmio_cmdqv; > > qemu_irq irq; > > + void *vintf_page0; > > > > /* Register Cache */ > > uint32_t config; > > diff --git a/hw/arm/tegra241-cmdqv.c b/hw/arm/tegra241-cmdqv.c > > index e1f1562c44..a3767a85a3 100644 > > --- a/hw/arm/tegra241-cmdqv.c > > +++ b/hw/arm/tegra241-cmdqv.c > > @@ -151,6 +151,39 @@ static uint64_t tegra241_cmdqv_read(void > *opaque, hwaddr offset, unsigned size) > > } > > } > > > > +static bool > > +tegra241_cmdqv_munmap_vintf_page0(Tegra241CMDQV *cmdqv, Error > **errp) > > +{ > > + if (!cmdqv->vintf_page0) { > > + return true; > > + } > > + > > + if (munmap(cmdqv->vintf_page0, VINTF_PAGE_SIZE) < 0) { > > + error_setg_errno(errp, errno, "Failed to unmap VINTF page0"); > > + return false; > > + } > > + cmdqv->vintf_page0 = NULL; > > + return true; > > +} > > + > > +static bool tegra241_cmdqv_mmap_vintf_page0(Tegra241CMDQV > *cmdqv, Error **errp) > > +{ > > + IOMMUFDViommu *viommu = cmdqv->s_accel->viommu; > > + > > + if (cmdqv->vintf_page0) { > > + return true; > > + } > > + > > + if (!iommufd_backend_viommu_mmap(viommu->iommufd, viommu- > >viommu_id, > > + VINTF_PAGE_SIZE, > > + > > cmdqv->cmdqv_data.out_vintf_mmap_offset, > > + &cmdqv->vintf_page0, errp)) { > > + return false; > > + } > > + > > + return true; > > +} > > + > > /* > > * Write a VCMDQ register using VCMDQ0_* offsets. > > * > > @@ -216,7 +249,7 @@ tegra241_cmdqv_write_vcmdq(Tegra241CMDQV > *cmdqv, hwaddr offset0, int index, > > } > > > > static void tegra241_cmdqv_write_vintf(Tegra241CMDQV *cmdqv, hwaddr > offset, > > - uint64_t value) > > + uint64_t value, Error **errp) > > { > > int i; > > > > @@ -227,8 +260,10 @@ static void > tegra241_cmdqv_write_vintf(Tegra241CMDQV *cmdqv, hwaddr offset, > > > > cmdqv->vintf_config = value; > > if (value & R_VINTF0_CONFIG_ENABLE_MASK) { > > + tegra241_cmdqv_mmap_vintf_page0(cmdqv, errp); > > cmdqv->vintf_status |= R_VINTF0_STATUS_ENABLE_OK_MASK; > > } else { > > + tegra241_cmdqv_munmap_vintf_page0(cmdqv, errp); > > cmdqv->vintf_status &= ~R_VINTF0_STATUS_ENABLE_OK_MASK; > > } > > break; > > @@ -251,6 +286,7 @@ static void tegra241_cmdqv_write(void *opaque, > hwaddr offset, uint64_t value, > > unsigned size) > > { > > Tegra241CMDQV *cmdqv = (Tegra241CMDQV *)opaque; > > + Error *local_err = NULL; > > int index; > > > > if (offset >= TEGRA241_CMDQV_IO_LEN) { > > @@ -276,7 +312,7 @@ static void tegra241_cmdqv_write(void *opaque, > hwaddr offset, uint64_t value, > > cmdqv->cmdq_alloc_map[(offset - A_CMDQ_ALLOC_MAP_0) / 4] = > value; > > break; > > case A_VINTF0_CONFIG ... A_VINTF0_LVCMDQ_ERR_MAP_3: > > - tegra241_cmdqv_write_vintf(cmdqv, offset, value); > > + tegra241_cmdqv_write_vintf(cmdqv, offset, value, &local_err); > > break; > > case A_VI_VCMDQ0_CONS_INDX ... A_VI_VCMDQ1_GERRORN: > > /* Same decoding as read() case: See comments above */ > > @@ -300,6 +336,10 @@ static void tegra241_cmdqv_write(void *opaque, > hwaddr offset, uint64_t value, > > qemu_log_mask(LOG_UNIMP, "%s unhandled write access at 0x%" > PRIx64 "\n", > > __func__, offset); > > } > > + > > + if (local_err) { > > + error_report_err(local_err); > > + } > > } > > > > static void tegra241_cmdqv_free_viommu(SMMUv3State *s)
