Re: [RFC PATCH v2 09/11] hw/arm/smmuv3: Add stage-2 support in iova notifier

[Eric Auger]

Hi,

On 2/26/23 23:06, Mostafa Saleh wrote:
> In smmuv3_notify_iova, read the granule based on translation stage
> and use VMID if valid value is sent.
>
> Signed-off-by: Mostafa Saleh <smostafa@google.com>
> ---
>  hw/arm/smmuv3.c     | 39 ++++++++++++++++++++++++++-------------
>  hw/arm/trace-events |  2 +-
>  2 files changed, 27 insertions(+), 14 deletions(-)
>
> diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
> index 8c76a48c8d..7297f6adc1 100644
> --- a/hw/arm/smmuv3.c
> +++ b/hw/arm/smmuv3.c
> @@ -971,18 +971,21 @@ epilogue:
>   * @mr: IOMMU mr region handle
>   * @n: notifier to be called
>   * @asid: address space ID or negative value if we don't care
> + * @vmid: virtual machine ID or negative value if we don't care
>   * @iova: iova
>   * @tg: translation granule (if communicated through range invalidation)
>   * @num_pages: number of @granule sized pages (if tg != 0), otherwise 1
>   */
>  static void smmuv3_notify_iova(IOMMUMemoryRegion *mr,
>                                 IOMMUNotifier *n,
> -                               int asid, dma_addr_t iova,
> -                               uint8_t tg, uint64_t num_pages)
> +                               int asid, int vmid,
> +                               dma_addr_t iova, uint8_t tg,
> +                               uint64_t num_pages)
>  {
>      SMMUDevice *sdev = container_of(mr, SMMUDevice, iommu);
>      IOMMUTLBEvent event;
>      uint8_t granule;
> +    SMMUv3State *s = sdev->smmu;

I am not sure notifiers are likely to be registered in the S2 case
because it is mostly meant to integrate with vhost or VFIO. The code in
this patch would rather prepare for nested stage support I guess, I
don't think it can harm.

Reviewed-by: Eric Auger <eric.auger@redhat.com>

Eric

>  
>      if (!tg) {
>          SMMUEventInfo event = {.inval_ste_allowed = true};
> @@ -997,11 +1000,20 @@ static void smmuv3_notify_iova(IOMMUMemoryRegion *mr,
>              return;
>          }
>  
> -        tt = select_tt(cfg, iova);
> -        if (!tt) {
> +        if (vmid >= 0 && cfg->s2cfg.vmid != vmid) {
>              return;
>          }
> -        granule = tt->granule_sz;
> +
> +        if (STAGE1_SUPPORTED(s)) {
> +            tt = select_tt(cfg, iova);
> +            if (!tt) {
> +                return;
> +            }
> +            granule = tt->granule_sz;
> +        } else {
> +            granule = cfg->s2cfg.granule_sz;
> +        }
> +
>      } else {
>          granule = tg * 2 + 10;
>      }
> @@ -1015,9 +1027,10 @@ static void smmuv3_notify_iova(IOMMUMemoryRegion *mr,
>      memory_region_notify_iommu_one(n, &event);
>  }
>  
> -/* invalidate an asid/iova range tuple in all mr's */
> -static void smmuv3_inv_notifiers_iova(SMMUState *s, int asid, dma_addr_t 
> iova,
> -                                      uint8_t tg, uint64_t num_pages)
> +/* invalidate an asid/vmid/iova range tuple in all mr's */
> +static void smmuv3_inv_notifiers_iova(SMMUState *s, int asid, int vmid,
> +                                      dma_addr_t iova, uint8_t tg,
> +                                      uint64_t num_pages)
>  {
>      SMMUDevice *sdev;
>  
> @@ -1025,11 +1038,11 @@ static void smmuv3_inv_notifiers_iova(SMMUState *s, 
> int asid, dma_addr_t iova,
>          IOMMUMemoryRegion *mr = &sdev->iommu;
>          IOMMUNotifier *n;
>  
> -        trace_smmuv3_inv_notifiers_iova(mr->parent_obj.name, asid, iova,
> -                                        tg, num_pages);
> +        trace_smmuv3_inv_notifiers_iova(mr->parent_obj.name, asid, vmid,
> +                                        iova, tg, num_pages);
>  
>          IOMMU_NOTIFIER_FOREACH(n, mr) {
> -            smmuv3_notify_iova(mr, n, asid, iova, tg, num_pages);
> +            smmuv3_notify_iova(mr, n, asid, vmid, iova, tg, num_pages);
>          }
>      }
>  }
> @@ -1060,7 +1073,7 @@ static void smmuv3_range_inval(SMMUState *s, Cmd *cmd)
>  
>      if (!tg) {
>          trace_smmuv3_range_inval(vmid, asid, addr, tg, 1, ttl, leaf);
> -        smmuv3_inv_notifiers_iova(s, asid, addr, tg, 1);
> +        smmuv3_inv_notifiers_iova(s, asid, vmid, addr, tg, 1);
>          smmu_iotlb_inv_iova(s, asid, vmid, addr, tg, 1, ttl);
>          return;
>      }
> @@ -1078,7 +1091,7 @@ static void smmuv3_range_inval(SMMUState *s, Cmd *cmd)
>  
>          num_pages = (mask + 1) >> granule;
>          trace_smmuv3_range_inval(vmid, asid, addr, tg, num_pages, ttl, leaf);
> -        smmuv3_inv_notifiers_iova(s, asid, addr, tg, num_pages);
> +        smmuv3_inv_notifiers_iova(s, asid, vmid, addr, tg, num_pages);
>          smmu_iotlb_inv_iova(s, asid, vmid, addr, tg, num_pages, ttl);
>          addr += mask + 1;
>      }
> diff --git a/hw/arm/trace-events b/hw/arm/trace-events
> index f8fdf1ca9f..cdc1ea06a8 100644
> --- a/hw/arm/trace-events
> +++ b/hw/arm/trace-events
> @@ -53,5 +53,5 @@ smmuv3_cmdq_tlbi_s12_vmid(uint16_t vmid) "vmid=%d"
>  smmuv3_config_cache_inv(uint32_t sid) "Config cache INV for sid=0x%x"
>  smmuv3_notify_flag_add(const char *iommu) "ADD SMMUNotifier node for iommu 
> mr=%s"
>  smmuv3_notify_flag_del(const char *iommu) "DEL SMMUNotifier node for iommu 
> mr=%s"
> -smmuv3_inv_notifiers_iova(const char *name, uint16_t asid, uint64_t iova, 
> uint8_t tg, uint64_t num_pages) "iommu mr=%s asid=%d iova=0x%"PRIx64" tg=%d 
> num_pages=0x%"PRIx64
> +smmuv3_inv_notifiers_iova(const char *name, uint16_t asid, uint16_t vmid, 
> uint64_t iova, uint8_t tg, uint64_t num_pages) "iommu mr=%s asid=%d vmid=%d 
> iova=0x%"PRIx64" tg=%d num_pages=0x%"PRIx64
>