RE: [PATCH v1 02/14] iommufd: Add nesting related data structures for ARM SMMUv3

[Shameerali Kolothum Thodi]

> -----Original Message-----
> From: Jean-Philippe Brucker [mailto:jean-philippe@xxxxxxxxxx]
> Sent: 09 March 2023 13:42
> To: Nicolin Chen <nicolinc@xxxxxxxxxx>
> Cc: jgg@xxxxxxxxxx; robin.murphy@xxxxxxx; will@xxxxxxxxxx;
> eric.auger@xxxxxxxxxx; kevin.tian@xxxxxxxxx; baolu.lu@xxxxxxxxxxxxxxx;
> joro@xxxxxxxxxx; Shameerali Kolothum Thodi
> <shameerali.kolothum.thodi@xxxxxxxxxx>;
> linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; iommu@xxxxxxxxxxxxxxx;
> linux-kernel@xxxxxxxxxxxxxxx; yi.l.liu@xxxxxxxxx
> Subject: Re: [PATCH v1 02/14] iommufd: Add nesting related data structures
> for ARM SMMUv3
> 
> Hi Nicolin,
> 
> On Thu, Mar 09, 2023 at 02:53:38AM -0800, Nicolin Chen wrote:
> > Add the following data structures for corresponding ioctls:
> >                iommu_hwpt_arm_smmuv3 =>
> IOMMUFD_CMD_HWPT_ALLOC
> >     iommu_hwpt_invalidate_arm_smmuv3 =>
> IOMMUFD_CMD_HWPT_INVALIDATE
> >
> > Also, add IOMMU_HW_INFO_TYPE_ARM_SMMUV3 and
> IOMMU_PGTBL_TYPE_ARM_SMMUV3_S1
> > to the header and corresponding type/size arrays.
> >
> > Signed-off-by: Nicolin Chen <nicolinc@xxxxxxxxxx>
> 
> > +/**
> > + * struct iommu_hwpt_arm_smmuv3 - ARM SMMUv3 specific page table
> data
> > + *
> > + * @flags: page table entry attributes
> > + * @s2vmid: Virtual machine identifier
> > + * @s1ctxptr: Stage-1 context descriptor pointer
> > + * @s1cdmax: Number of CDs pointed to by s1ContextPtr
> > + * @s1fmt: Stage-1 Format
> > + * @s1dss: Default substream
> > + */
> > +struct iommu_hwpt_arm_smmuv3 {
> > +#define IOMMU_SMMUV3_FLAG_S2	(1 << 0) /* if unset, stage-1 */
> 
> I don't understand the purpose of this flag, since the structure only
> provides stage-1 configuration fields
> 
> > +#define IOMMU_SMMUV3_FLAG_VMID	(1 << 1) /* vmid override */
> 
> Doesn't this break isolation?  The VMID space is global for the SMMU, so
> the guest could access cached mappings of another device

On platforms that supports BTM [1], we may need the VMID allocated by KVM.
But again getting that from user pace doesn't look safe. I have attempted to revise
the earlier RFC to pin and use the KVM VMID from SMMUv3 here[2].

But the problem is getting the KVM instance associated with the device. Currently I am 
going through the VFIO layer to retrieve the KVM instance(vfio_device->kvm).

On the previous RFC discussion thread[3], Jean has proposed,

" In the new design we can require from the start that creating a nesting IOMMU
container through /dev/iommu *must* come with a KVM context, that way
we're sure to reuse the existing VMID. "

Is that something we can still do or there is a better way to handle this now?

Thanks,
Shameer


1. https://lore.kernel.org/linux-arm-kernel/YEEUocRn3IfIDpLj@myrica/T/#m478f7e7d5dcb729e02721beda35efa12c1d20707
2. https://github.com/hisilicon/kernel-dev/commits/iommufd-v6.2-rc4-nesting-arm-btm-v2
3. https://lore.kernel.org/linux-arm-kernel/YEEUocRn3IfIDpLj@myrica/T/#m11cde7534943ea7cf35f534cb809a023eabd9da3