On Tue, May 10, 2022 at 06:52:06PM +0100, Robin Murphy wrote:
> On 2022-05-10 17:55, Jason Gunthorpe via iommu wrote:
> > This control causes the ARM SMMU drivers to choose a stage 2
> > implementation for the IO pagetable (vs the stage 1 usual default),
> > however this choice has no visible impact to the VFIO user. Further qemu
> > never implemented this and no other userspace user is known.
> >
> > The original description in commit f5c9ecebaf2a ("vfio/iommu_type1: add
> > new VFIO_TYPE1_NESTING_IOMMU IOMMU type") suggested this was to "provide
> > SMMU translation services to the guest operating system" however the rest
> > of the API to set the guest table pointer for the stage 1 was never
> > completed, or at least never upstreamed, rendering this part useless dead
> > code.
> >
> > Since the current patches to enable nested translation, aka userspace page
> > tables, rely on iommufd and will not use the enable_nesting()
> > iommu_domain_op, remove this infrastructure. However, don't cut too deep
> > into the SMMU drivers for now expecting the iommufd work to pick it up -
> > we still need to create S2 IO page tables.
> >
> > Remove VFIO_TYPE1_NESTING_IOMMU and everything under it including the
> > enable_nesting iommu_domain_op.
> >
> > Just in-case there is some userspace using this continue to treat
> > requesting it as a NOP, but do not advertise support any more.
>
> I assume the nested translation/guest SVA patches that Eric and Vivek were
> working on pre-IOMMUFD made use of this, and given that they got quite far
> along, I wouldn't be too surprised if some eager cloud vendors might have
> even deployed something based on the patches off the list.
With upstream there is no way to make use of this flag, if someone is
using it they have other out of tree kernel, vfio, kvm and qemu
patches to make it all work.
You can see how much is still needed in Eric's tree:
https://github.com/eauger/linux/commits/v5.15-rc7-nested-v16
> I can't help feeling a little wary about removing this until IOMMUFD
> can actually offer a functional replacement - is it in the way of
> anything upcoming?
>From an upstream perspective if someone has a patched kernel to
complete the feature, then they can patch this part in as well, we
should not carry dead code like this in the kernel and in the uapi.
It is not directly in the way, but this needs to get done at some
point, I'd rather just get it out of the way.
Thanks,
Jason
