Re: [Intel-gfx] [PATCH 04/10] vfio: Move storage of allow_unsafe_interrupts to vfio_main.c

Tian, Kevin
2022-11-09T03:21:38Z

> From: Jason Gunthorpe <jgg@nvidia.com>
> Sent: Wednesday, November 9, 2022 9:05 AM
>
> On Tue, Nov 08, 2022 at 03:55:20PM -0700, Alex Williamson wrote:
>
> > > > So why exactly isn't this an issue for VDPA? Are we just burying our
> > > > head in the sand that such platforms exists and can still be useful
> > > > given the appropriate risk vs reward trade-off?
> > >
> > > Simply that nobody has asked for it, and might never ask for it. This
> > > is all support for old platforms, and there just doesn't seem to be a
> > > "real" use case for very new (and actually rare) NIC hardware stuck
> > > into ancient platforms with this security problem.
> >
> > vIOMMU support for interrupt remapping is relatively new, the nesting
> > case is important as well.
>
> This is where we got hit. In the end we fixed the qemu..

but the point is that old qemu could have a much longer lifespan than
old platforms then when running newer kernel which supports vdpa
on old qemu the same tradeoff consideration is then not vfio specific...

> > It's certainly not acceptable in the latest proposal, iommufd consumes
> > an option set by another module and when that module goes away, so
> does
> > any claim of compatibility. The code becomes dead and the feature not
> > present. The option doesn't belong on the vfio module. Do we need a
> > vfio-iommufd module to host it? Thanks,
>
> I don't know, as I said in the other email, these little things need
> work and discussion to resolve. We need to recheck the security stuff
> against the 2022 kernel where things have changed. We don't need to do
> it all right now.
>
> People who want allow_unsafe_interrupts to work will simply not set
> VFIO_CONTAINER=n at this time. Same with P2P, vfio-no-iommu and any
> other gaps we haven't discovered.
>

If all agree that VFIO_CONTAINER=n is a process to evolve, does it make
more sense to remove this patch from this series i.e. let it buried in
VFIO_CONTAINER=y for now? Then resolve it in a follow up patch if
no consensus can be made quickly at this point.