Re: [PATCH v3 5/9] iommu/arm-smmu-v3: Support IOMMU_GET_HW_INFO via struct arm_smmu_hw_info

[Nicolin Chen <nicolinc@nvidia.com>]

On Wed, Oct 30, 2024 at 02:56:15PM -0300, Jason Gunthorpe wrote:
> On Wed, Oct 30, 2024 at 04:24:44PM +0000, Mostafa Saleh wrote:
> > > +void *arm_smmu_hw_info(struct device *dev, u32 *length, u32 *type)
> > > +{
> > > +	struct arm_smmu_master *master = dev_iommu_priv_get(dev);
> > > +	struct iommu_hw_info_arm_smmuv3 *info;
> > > +	u32 __iomem *base_idr;
> > > +	unsigned int i;
> > > +
> > > +	info = kzalloc(sizeof(*info), GFP_KERNEL);
> > > +	if (!info)
> > > +		return ERR_PTR(-ENOMEM);
> > > +
> > > +	base_idr = master->smmu->base + ARM_SMMU_IDR0;
> > > +	for (i = 0; i <= 5; i++)
> > > +		info->idr[i] = readl_relaxed(base_idr + i);
> > > +	info->iidr = readl_relaxed(master->smmu->base + ARM_SMMU_IIDR);
> > > +	info->aidr = readl_relaxed(master->smmu->base + ARM_SMMU_AIDR);
> > 
> > I wonder if passing the IDRs is enough for the VMM, for example in some
> > cases, firmware can override the coherency, also the IIDR can override
> > some features (as MMU700 and BTM), although, the VMM can deal with.
> 
> I'm confident it is not enough
> 
> BTM support requires special kernel vBTM support which will need a
> dedicated flag someday
> 
> ATS is linked to the kernel per-device enable_ats, that will have to
> flow to ACPI/etc tables on a per-device basis
> 
> PRI is linked to the ability to attach a fault capable domain..
> 
> And so on.
> 
> Nicolin, what do your qemu patches even use IIDR for today?

Not yet.

I think this is a very good point. Checking IIDR in VMM as the
kernel driver does is doable with the iommu_hw_info_arm_smmuv3
while a firmware override like IORT might not..

Thanks
Nicolin

> It wouldn't surprise me if we end up only using a few bits of the raw
> physical information.
> 
> > Maybe for those(coherency, ATS, PRI) we would need to keep the VMM view and
> > the kernel in sync?
> 
> Definately
> 
> Jason