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

[Mostafa Saleh <smostafa@google.com>]

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