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.
So what does happen if the guest attempts to access the VI CMDQ before
its mmapping.
At the moment MMIO accesses are valid as VI_VCMDQ0 MMIOs are handled in
the same MMIO region as the Config page.
Is it the expected behavior? I guess you should signal an error and
maaybe signal an interrupt if it is not masked for this VI?
Spec says:"While the SW can program the virtual CMDQ(s) directly using
the direct VCMDQ aperture (and not though the Virtual Interface), it is
required hat the VCMDQ be allocated to a VI before it is used to send
commands to the SMMU
Note the terminology used here: direct VCMDQ aparture (not Global)
Thanks
Eric
>
> 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.
>
> 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)
