Re: [RFC PATCH 11/21] xen/arm: vsmmuv3: Attach Stage-1 configuration to SMMUv3 hardware

[Stewart Hildebrand <stewart.hildebrand@xxxxxxx>]

On 12/1/22 11:02, Rahul Singh wrote:
> Attach the Stage-1 configuration to device STE to support nested
> translation for the guests.
> 
> Signed-off-by: Rahul Singh <rahul.singh@xxxxxxx>
> ---
>  xen/drivers/passthrough/arm/smmu-v3.c  | 79 ++++++++++++++++++++++++++
>  xen/drivers/passthrough/arm/smmu-v3.h  |  1 +
>  xen/drivers/passthrough/arm/vsmmu-v3.c | 18 ++++++
>  xen/include/xen/iommu.h                | 14 +++++
>  4 files changed, 112 insertions(+)
> 
> diff --git a/xen/drivers/passthrough/arm/smmu-v3.c 
> b/xen/drivers/passthrough/arm/smmu-v3.c
> index 4f96fdb92f..c4b4a5d86d 100644
> --- a/xen/drivers/passthrough/arm/smmu-v3.c
> +++ b/xen/drivers/passthrough/arm/smmu-v3.c
> @@ -2754,6 +2754,37 @@ static struct arm_smmu_device 
> *arm_smmu_get_by_dev(struct device *dev)
>         return NULL;
>  }
> 
> +static struct iommu_domain *arm_smmu_get_domain_by_sid(struct domain *d,
> +                               u32 sid)
> +{
> +       int i;
> +       unsigned long flags;
> +       struct iommu_domain *io_domain;
> +       struct arm_smmu_domain *smmu_domain;
> +       struct arm_smmu_master *master;
> +       struct arm_smmu_xen_domain *xen_domain = dom_iommu(d)->arch.priv;
> +
> +       /*
> +        * Loop through the &xen_domain->contexts to locate a context
> +        * assigned to this SMMU
> +        */
> +       list_for_each_entry(io_domain, &xen_domain->contexts, list) {
> +               smmu_domain = to_smmu_domain(io_domain);
> +
> +               spin_lock_irqsave(&smmu_domain->devices_lock, flags);
> +               list_for_each_entry(master, &smmu_domain->devices, 
> domain_head) {
> +                       for (i = 0; i < master->num_streams; i++) {
> +                               if (sid != master->streams[i].id)
> +                                       continue;
> +                               
> spin_unlock_irqrestore(&smmu_domain->devices_lock, flags);
> +                               return io_domain;
> +                       }
> +               }
> +               spin_unlock_irqrestore(&smmu_domain->devices_lock, flags);
> +       }
> +       return NULL;
> +}
> +
>  static struct iommu_domain *arm_smmu_get_domain(struct domain *d,
>                                 struct device *dev)
>  {
> @@ -2909,6 +2940,53 @@ static void arm_smmu_iommu_xen_domain_teardown(struct 
> domain *d)
>         xfree(xen_domain);
>  }
> 
> +static int arm_smmu_attach_guest_config(struct domain *d, u32 sid,
> +               struct iommu_guest_config *cfg)
> +{
> +       int ret = -EINVAL;
> +       unsigned long flags;
> +       struct arm_smmu_master *master;
> +       struct arm_smmu_domain *smmu_domain;
> +       struct arm_smmu_xen_domain *xen_domain = dom_iommu(d)->arch.priv;
> +       struct iommu_domain *io_domain = arm_smmu_get_domain_by_sid(d, sid);
> +
> +       if (!io_domain)
> +               return -ENODEV;
> +
> +       smmu_domain = to_smmu_domain(io_domain);
> +
> +       spin_lock(&xen_domain->lock);
> +
> +       switch (cfg->config) {
> +       case ARM_SMMU_DOMAIN_ABORT:
> +               smmu_domain->abort = true;
> +               break;
> +       case ARM_SMMU_DOMAIN_BYPASS:
> +               smmu_domain->abort = false;
> +               break;
> +       case ARM_SMMU_DOMAIN_NESTED:
> +               /* Enable Nested stage translation. */
> +               smmu_domain->stage = ARM_SMMU_DOMAIN_NESTED;
> +               smmu_domain->s1_cfg.s1ctxptr = cfg->s1ctxptr;
> +               smmu_domain->s1_cfg.s1fmt = cfg->s1fmt;
> +               smmu_domain->s1_cfg.s1cdmax = cfg->s1cdmax;
> +               smmu_domain->abort = false;
> +               break;
> +       default:
> +               goto out;
> +       }
> +
> +       spin_lock_irqsave(&smmu_domain->devices_lock, flags);
> +       list_for_each_entry(master, &smmu_domain->devices, domain_head)
> +               arm_smmu_install_ste_for_dev(master);
> +       spin_unlock_irqrestore(&smmu_domain->devices_lock, flags);
> +
> +       ret = 0;
> +out:
> +       spin_unlock(&xen_domain->lock);
> +       return ret;
> +}
> +
>  static const struct iommu_ops arm_smmu_iommu_ops = {
>         .page_sizes             = PAGE_SIZE_4K,
>         .init                   = arm_smmu_iommu_xen_domain_init,
> @@ -2921,6 +2999,7 @@ static const struct iommu_ops arm_smmu_iommu_ops = {
>         .unmap_page             = arm_iommu_unmap_page,
>         .dt_xlate               = arm_smmu_dt_xlate,
>         .add_device             = arm_smmu_add_device,
> +       .attach_guest_config = arm_smmu_attach_guest_config

Please append a trailing comma here, even if it is the last element

>  };
> 
>  static __init int arm_smmu_dt_init(struct dt_device_node *dev,
> diff --git a/xen/drivers/passthrough/arm/smmu-v3.h 
> b/xen/drivers/passthrough/arm/smmu-v3.h
> index e270fe05e0..50a050408b 100644
> --- a/xen/drivers/passthrough/arm/smmu-v3.h
> +++ b/xen/drivers/passthrough/arm/smmu-v3.h
> @@ -393,6 +393,7 @@ enum arm_smmu_domain_stage {
>         ARM_SMMU_DOMAIN_S2,
>         ARM_SMMU_DOMAIN_NESTED,
>         ARM_SMMU_DOMAIN_BYPASS,
> +       ARM_SMMU_DOMAIN_ABORT,
>  };
> 
>  /* Xen specific code. */
> diff --git a/xen/drivers/passthrough/arm/vsmmu-v3.c 
> b/xen/drivers/passthrough/arm/vsmmu-v3.c
> index 916b97b8a2..5188181929 100644
> --- a/xen/drivers/passthrough/arm/vsmmu-v3.c
> +++ b/xen/drivers/passthrough/arm/vsmmu-v3.c
> @@ -223,8 +223,11 @@ static int arm_vsmmu_handle_cfgi_ste(struct virt_smmu 
> *smmu, uint64_t *cmdptr)
>  {
>      int ret;
>      uint64_t ste[STRTAB_STE_DWORDS];
> +    struct domain *d = smmu->d;
> +    struct domain_iommu *hd = dom_iommu(d);
>      struct arm_vsmmu_s1_trans_cfg s1_cfg = {0};
>      uint32_t sid = smmu_cmd_get_sid(cmdptr[0]);
> +    struct iommu_guest_config guest_cfg = {0};
> 
>      ret = arm_vsmmu_find_ste(smmu, sid, ste);
>      if ( ret )
> @@ -234,6 +237,21 @@ static int arm_vsmmu_handle_cfgi_ste(struct virt_smmu 
> *smmu, uint64_t *cmdptr)
>      if ( ret )
>          return (ret == -EAGAIN ) ? 0 : ret;
> 
> +    guest_cfg.s1ctxptr = s1_cfg.s1ctxptr;
> +    guest_cfg.s1fmt = s1_cfg.s1fmt;
> +    guest_cfg.s1cdmax = s1_cfg.s1cdmax;
> +
> +    if ( s1_cfg.bypassed )
> +        guest_cfg.config = ARM_SMMU_DOMAIN_BYPASS;
> +    else if ( s1_cfg.aborted )
> +        guest_cfg.config = ARM_SMMU_DOMAIN_ABORT;
> +    else
> +        guest_cfg.config = ARM_SMMU_DOMAIN_NESTED;
> +
> +    ret = hd->platform_ops->attach_guest_config(d, sid, &guest_cfg);
> +    if ( ret )
> +        return ret;
> +
>      return 0;
>  }
> 
> diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h
> index 4f22fc1bed..b2fc027e5e 100644
> --- a/xen/include/xen/iommu.h
> +++ b/xen/include/xen/iommu.h
> @@ -230,6 +230,15 @@ int iommu_do_dt_domctl(struct xen_domctl *, struct 
> domain *,
> 
>  #endif /* HAS_DEVICE_TREE */
> 
> +#ifdef CONFIG_ARM
> +struct iommu_guest_config {
> +    paddr_t     s1ctxptr;
> +    uint8_t     config;
> +    uint8_t     s1fmt;
> +    uint8_t     s1cdmax;
> +};
> +#endif /* CONFIG_ARM */
> +
>  struct page_info;
> 
>  /*
> @@ -302,6 +311,11 @@ struct iommu_ops {
>       */
>      int (*dt_xlate)(device_t *dev, const struct dt_phandle_args *args);
>  #endif
> +
> +#ifdef CONFIG_ARM
> +    int (*attach_guest_config)(struct domain *d, u32 sid,
> +                               struct iommu_guest_config *cfg);
> +#endif
>  };
> 
>  /*
> --
> 2.25.1
> 
>