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