Re: [PATCH v2] kvm: Disable MSI/MSI-X in assigned device reset path

[Alex Williamson <alex.williamson@redhat.com>]

On Mon, 2012-04-09 at 11:35 +0300, Avi Kivity wrote:
> On 04/08/2012 08:37 PM, Jan Kiszka wrote:
> > The core problem is not the ordering. The problem is that the kernel is
> > susceptible to ordering mistakes of userspace. And that is because the
> > kernel panics on PCI errors of devices that are in user hands - a
> > critical kernel bug IMHO. 
> 
> Certainly.  But this userspace patch won't fix it.

No, it won't in general and I don't think it makes sense to mangle
pci-sysfs config space support to the nuances of a user space driver.
We really need a userspace driver interface that limits the config space
interactions and provides a channel to support error reporting and
userspace recovery.  This type of thing can be done with VFIO if we
could ever get off the ground and get some consensus around it.  Please
feel free to contribute to that discussion if you ever want to get away
from this clunky device assignment interface we have now.

> > Proper reset of MSI or even the whole PCI
> > config space is another issue, but one the kernel should not worry about
> > - still, it should be fixed (therefore this patch).
> 
> And I was asking what is the right way to do it.  Reset the device and
> read back the register values, or do an emulated reset and push down the
> register values.

Reading back the register values is currently a noop since the kernel
restores the config space to the incoming state after reset.  KVM does
stash away the original config space of the device to be restored prior
to releasing the device.  We could restore to that each time, but that
would mean implementing a device reset ioctl in kvm, and we'd still need
this patch for compatibility and we still have the issues Michael brings
up with the config restore updating things like MSI that we need to then
manually sync with kvm.  I fear suggesting it, but perhaps another way
to achieve this result would be to de-assign and re-assign the device in
reset.

> > But even if we disallowed userland to disable MMIO and PIO access to the
> > device, we would be be able to exclude that there are secrete channels
> > in the device's interface having the same effect. So we likely need to
> > enhance PCI error handling to catch and handle faults for certain
> > devices differently - those we cannot trust to behave properly while
> > they are under userland/guest control.
> 
> Why not all of them?

I think Jan is probably suggesting that we do need user space error
handling for all userland/guest controlled devices, but some classes of
errors on certain devices may be handled automatically by the userspace
interface layer... which we could do with VFIO (well, assuming the APEI
spec let's us nak the bios reporting a fatal error).  So do we want to
invent new solutions for each of these or do we want to move to a new
interface?  Thanks,

Alex