Re: [PATCH v5 14/32] hw/arm/smmuv3-accel: Install SMMUv3 GBPA based hwpt

[Eric Auger <eric.auger@redhat.com>]

Hi Shameer,

On 10/31/25 11:49 AM, Shameer Kolothum wrote:
> When the Guest reboots or updates the GBPA we need to attach a nested HWPT
> based on the GBPA register values.
In practice you only take into account GPBA.ABORT bit. Also reminds what
this latter does, ie.
"attach a nested HWPT based on the new GPBA.ABORT bit which either
aborts all incoming transactions or bypass them"
>
> Signed-off-by: Shameer Kolothum <skolothumtho@nvidia.com>
> ---
>  hw/arm/smmuv3-accel.c | 42 ++++++++++++++++++++++++++++++++++++++++++
>  hw/arm/smmuv3-accel.h |  8 ++++++++
>  hw/arm/smmuv3.c       |  2 ++
>  3 files changed, 52 insertions(+)
>
> diff --git a/hw/arm/smmuv3-accel.c b/hw/arm/smmuv3-accel.c
> index c74e95a0ea..0573ae3772 100644
> --- a/hw/arm/smmuv3-accel.c
> +++ b/hw/arm/smmuv3-accel.c
> @@ -479,6 +479,48 @@ static const PCIIOMMUOps smmuv3_accel_ops = {
>      .unset_iommu_device = smmuv3_accel_unset_iommu_device,
>  };
>  
> +
> +/* Based on SMUUv3 GBPA configuration, attach a corresponding HWPT */
> +void smmuv3_accel_gbpa_update(SMMUv3State *s)
> +{
> +    SMMUv3AccelDevice *accel_dev;
> +    Error *local_err = NULL;
> +    SMMUViommu *vsmmu;
> +    uint32_t hwpt_id;
> +
> +    if (!s->accel || !s->s_accel->vsmmu) {
> +        return;
> +    }
> +
> +    vsmmu = s->s_accel->vsmmu;
> +    /*
> +     * The Linux kernel does not allow configuring GBPA MemAttr, MTCFG,
> +     * ALLOCCFG, SHCFG, PRIVCFG, or INSTCFG fields for a vSTE. Host kernel
I think in general we shall avoid doing any assumptions about linux
kernel capability.
> +     * has final control over these parameters. Hence, use one of the
It seems to be contradictory to the above statement.
> +     * pre-allocated HWPTs depending on GBPA.ABORT value.
I would remove the comment
> +     */
> +    if (s->gbpa & SMMU_GBPA_ABORT) {
> +        hwpt_id = vsmmu->abort_hwpt_id;
> +    } else {
> +        hwpt_id = vsmmu->bypass_hwpt_id;
> +    }
> +
> +    QLIST_FOREACH(accel_dev, &vsmmu->device_list, next) {
> +        if (!host_iommu_device_iommufd_attach_hwpt(accel_dev->idev, hwpt_id,
> +                                                   &local_err)) {
> +            error_append_hint(&local_err, "Failed to attach GBPA hwpt id %u "
> +                              "for dev id %u", hwpt_id, accel_dev->idev->devid);
> +            error_report_err(local_err);
> +        }
> +    }
> +}
> +
> +void smmuv3_accel_reset(SMMUv3State *s)
> +{
> +     /* Attach a HWPT based on GBPA reset value */
> +     smmuv3_accel_gbpa_update(s);
> +}
> +
>  static void smmuv3_accel_as_init(SMMUv3State *s)
>  {
>  
> diff --git a/hw/arm/smmuv3-accel.h b/hw/arm/smmuv3-accel.h
> index 73b44cd7be..8931e83dc5 100644
> --- a/hw/arm/smmuv3-accel.h
> +++ b/hw/arm/smmuv3-accel.h
> @@ -51,6 +51,8 @@ bool smmuv3_accel_install_nested_ste(SMMUv3State *s, SMMUDevice *sdev, int sid,
>                                       Error **errp);
>  bool smmuv3_accel_install_nested_ste_range(SMMUv3State *s, SMMUSIDRange *range,
>                                             Error **errp);
> +void smmuv3_accel_gbpa_update(SMMUv3State *s);
> +void smmuv3_accel_reset(SMMUv3State *s);
>  #else
>  static inline void smmuv3_accel_init(SMMUv3State *s)
>  {
> @@ -67,6 +69,12 @@ smmuv3_accel_install_nested_ste_range(SMMUv3State *s, SMMUSIDRange *range,
>  {
>      return true;
>  }
> +static inline void smmuv3_accel_gbpa_update(SMMUv3State *s)
> +{
> +}
> +static inline void smmuv3_accel_reset(SMMUv3State *s)
> +{
> +}
>  #endif
>  
>  #endif /* HW_ARM_SMMUV3_ACCEL_H */
> diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
> index 1fd8aaa0c7..cc32b618ed 100644
> --- a/hw/arm/smmuv3.c
> +++ b/hw/arm/smmuv3.c
> @@ -1603,6 +1603,7 @@ static MemTxResult smmu_writel(SMMUv3State *s, hwaddr offset,
>          if (data & R_GBPA_UPDATE_MASK) {
>              /* Ignore update bit as write is synchronous. */
>              s->gbpa = data & ~R_GBPA_UPDATE_MASK;
> +            smmuv3_accel_gbpa_update(s);
>          }
>          return MEMTX_OK;
>      case A_STRTAB_BASE: /* 64b */
> @@ -1885,6 +1886,7 @@ static void smmu_reset_exit(Object *obj, ResetType type)
>      }
>  
>      smmuv3_init_regs(s);
> +    smmuv3_accel_reset(s);
>  }
>  
>  static void smmu_realize(DeviceState *d, Error **errp)
Besides

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

Eric