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.
>
> 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()?
>                  /*
>                   * VCMDQs whose BASE was programmed before VINTF was
>                   * enabled need their hw_queue allocated now.
> @@ -452,6 +487,7 @@ static void 
> tegra241_cmdqv_config_vintf_write(Tegra241CMDQV *cmdqv,
>                  tegra241_cmdqv_setup_all_vcmdq(cmdqv, errp);
>              }
>          } 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;
Thanks

Eric


Reply via email to