Hi Shameer,
On 6/1/26 1:42 PM, Shameer Kolothum wrote:
> From: Nicolin Chen <[email protected]>
>
> The kernel currently exposes a single VINTF per emulated SMMUv3
> instance. IOMMU_VIOMMU_ALLOC returns an mmap offset for the host
> VINTF Page0 allocated for this SMMU. However, VCMDQs only become
> bound to that VINTF after IOMMU_HW_QUEUE_ALLOC, so until then the
> mapped Page0 does not back any real VCMDQ state.
>
> When VINTF is enabled, mmap the kernel-provided Page0 region and set
> STATUS.ENABLE_OK only if the mmap succeeds; unmap it when VINTF is
> disabled. This prepares the VINTF mapping in advance of subsequent
> patches that add VCMDQ allocation and the guest MMIO install.
>
> 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 | 3 +++
> hw/arm/tegra241-cmdqv.c | 50 ++++++++++++++++++++++++++++++++++++++---
> 2 files changed, 50 insertions(+), 3 deletions(-)
>
> diff --git a/hw/arm/tegra241-cmdqv.h b/hw/arm/tegra241-cmdqv.h
> index ad7fb8725f..c4d327a9a5 100644
> --- a/hw/arm/tegra241-cmdqv.h
> +++ b/hw/arm/tegra241-cmdqv.h
> @@ -37,6 +37,8 @@
> #define CMDQV_VINTF_PAGE1_BASE 0x40000
> #define CMDQV_VCMDQ_STRIDE 0x80
>
> +#define VINTF_PAGE_SIZE 0x10000
> +
> struct iommu_viommu_tegra241_cmdqv;
>
> typedef struct Tegra241CMDQV {
> @@ -45,6 +47,7 @@ typedef struct Tegra241CMDQV {
> MemoryRegion mmio_cmdqv;
> qemu_irq irq;
> IOMMUFDVeventq *veventq;
> + void *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 3cb1b45575..a52e153501 100644
> --- a/hw/arm/tegra241-cmdqv.c
> +++ b/hw/arm/tegra241-cmdqv.c
> @@ -9,6 +9,8 @@
>
> #include "qemu/osdep.h"
> #include "qemu/log.h"
> +#include "qemu/error-report.h"
> +#include "qapi/error.h"
>
> #include "hw/arm/smmuv3.h"
> #include "hw/arm/smmuv3-common.h"
> @@ -218,8 +220,42 @@ static void
> tegra241_cmdqv_write_vcmdq_page1_64(Tegra241CMDQV *cmdqv,
> offset0, value);
> }
>
> +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;
> +}
> +
> static void tegra241_cmdqv_config_vintf_write(Tegra241CMDQV *cmdqv,
> - hwaddr offset, uint64_t value)
> + hwaddr offset, uint64_t value,
> + Error **errp)
> {
> int i;
>
> @@ -234,8 +270,11 @@ static void
> tegra241_cmdqv_config_vintf_write(Tegra241CMDQV *cmdqv,
>
> cmdqv->vintf_config = value;
> if (value & R_VINTF0_CONFIG_ENABLE_MASK) {
> - cmdqv->vintf_status |= R_VINTF0_STATUS_ENABLE_OK_MASK;
> + if (tegra241_cmdqv_mmap_vintf_page0(cmdqv, errp)) {
> + cmdqv->vintf_status |= R_VINTF0_STATUS_ENABLE_OK_MASK;
> + }
> } else {
> + tegra241_cmdqv_munmap_vintf_page0(cmdqv, errp);
the munmap is only done upon guest action in this patch. Later on, it is
also done on reset (in smmuv3_accel_reset called on smmu_reset_exit
called in exit phase). So I think we are good overall but it is a bit
annoying to leave it done only upon guest action in this patch. What do
you think?
Thanks
Eric
> cmdqv->vintf_status &= ~R_VINTF0_STATUS_ENABLE_OK_MASK;
> }
> break;
> @@ -341,6 +380,7 @@ out:
> static void tegra241_cmdqv_writel_mmio(Tegra241CMDQV *cmdqv, hwaddr offset,
> uint32_t value)
> {
> + Error *local_err = NULL;
> int index;
>
> switch (offset) {
> @@ -359,7 +399,7 @@ static void tegra241_cmdqv_writel_mmio(Tegra241CMDQV
> *cmdqv, hwaddr offset,
> 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_config_vintf_write(cmdqv, offset, value);
> + tegra241_cmdqv_config_vintf_write(cmdqv, offset, value, &local_err);
> break;
> case A_VI_VCMDQ0_CONS_INDX ... A_VI_VCMDQ1_GERRORN:
> /*
> @@ -398,6 +438,10 @@ static void tegra241_cmdqv_writel_mmio(Tegra241CMDQV
> *cmdqv, hwaddr offset,
> qemu_log_mask(LOG_UNIMP, "%s unhandled write access at 0x%" PRIx64
> "\n",
> __func__, offset);
> }
> +
> + if (local_err) {
> + error_report_err(local_err);
> + }
> }
>
> /*
