Re: [PATCH v6 13/25] iommu/arm-smmu-v3-kvm: Probe SMMU HW

Mostafa Saleh
2026-05-04T12:30:40Z

On Fri, May 01, 2026 at 09:51:48AM -0300, Jason Gunthorpe wrote:
> On Fri, May 01, 2026 at 11:19:15AM +0000, Mostafa Saleh wrote:
> > diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> > index 61e6ab364086..157acde0436d 100644
> > --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> > +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> > @@ -4738,12 +4738,6 @@ static int arm_smmu_device_reset(struct arm_smmu_device *smmu)
> > return 0;
> > }
> >
> > -#define IIDR_IMPLEMENTER_ARM 0x43b
> > -#define IIDR_PRODUCTID_ARM_MMU_600 0x483
> > -#define IIDR_PRODUCTID_ARM_MMU_700 0x487
> > -#define IIDR_PRODUCTID_ARM_MMU_L1 0x48a
> > -#define IIDR_PRODUCTID_ARM_MMU_S3 0x498
> > -
> > static void arm_smmu_device_iidr_probe(struct arm_smmu_device *smmu)
> > {
> > u32 reg;
> > diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
> > index 64618299d03a..f904f4d19609 100644
> > --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
> > +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
> > @@ -84,6 +84,12 @@ struct arm_vsmmu;
> > #define IIDR_REVISION GENMASK(15, 12)
> > #define IIDR_IMPLEMENTER GENMASK(11, 0)
> >
> > +#define IIDR_IMPLEMENTER_ARM 0x43b
> > +#define IIDR_PRODUCTID_ARM_MMU_600 0x483
> > +#define IIDR_PRODUCTID_ARM_MMU_700 0x487
> > +#define IIDR_PRODUCTID_ARM_MMU_L1 0x48a
> > +#define IIDR_PRODUCTID_ARM_MMU_S3 0x498
> > +
> > #define ARM_SMMU_AIDR 0x1C
>
> Lets put these hunks in some earlier patch to migrate out the
> functions/etc
>
> I think all these pkvm/arm-smmu-v3.c should just be building up the
> driver.

Will do.

>
> > +static bool smmu_nesting_supported(struct hyp_arm_smmu_v3_device *smmu)
> > +{
> > + unsigned int implementer, productid, variant, revision;
> > + u32 reg;
> > +
> > + if (!(smmu->features & ARM_SMMU_FEAT_TRANS_S1) ||
> > + !(smmu->features & ARM_SMMU_FEAT_TRANS_S2))
> > + return false;
> > +
> > + reg = readl_relaxed(smmu->base + ARM_SMMU_IIDR);
> > + implementer = FIELD_GET(IIDR_IMPLEMENTER, reg);
> > + productid = FIELD_GET(IIDR_PRODUCTID, reg);
> > + variant = FIELD_GET(IIDR_VARIANT, reg);
> > + revision = FIELD_GET(IIDR_REVISION, reg);
> > +
> > + if (implementer != IIDR_IMPLEMENTER_ARM)
> > + return true;
> > +
> > + if (productid == IIDR_PRODUCTID_ARM_MMU_600)
> > + return variant >= 2;
> > + else if (productid == IIDR_PRODUCTID_ARM_MMU_700)
> > + return !(variant < 1 || revision < 1);
> > +
> > + return true;
> > +}
>
> Why not share all this errata stuff with the idr parsing code too?
>
> We already have ARM_SMMU_FEAT_NESTING that has the above calculation.
>
> The two drivers use the same ARM_SMMU_FEAT system, I would expect one
> chunk of shared code to compute the FEATs, who cares if pkvm doesn't
> use all of them?
>
> Use the same errata logic and so on to get to the feat bitmap.

Makes sense.

Thanks,
Mostafa

>
> Jason