Re: [Qemu-devel] [PATCH v8 11/12] vfio: register aer resume notification handler for aer resume

[Alex Williamson]

On Wed, 29 Jun 2016 16:54:05 +0800
Zhou Jie <address@hidden> wrote:

> Hi Alex,
> 
> > And yet we have struct pci_dev.broken_intx_masking and we test for
> > working DisINTx via pci_intx_mask_supported() rather than simply
> > looking for a PCIe device.  Some devices are broken and some simply
> > don't follow the spec, so you're going to need to deal with that or
> > exclude those devices.  
> For those devices I have no way to disable the INTx.

disable_irq()?  Clearly vfio-pci already manages these types of devices
and can disable INTx.  This is why I keep suggesting that maybe tearing
the interrupt setup down completely is a more complete and reliable
approach than masking in the command register.  Unless we're going to
exclude such devices from supporting this mode (which I don't condone),
we must deal with them.
 
> > How does that happen, aren't we notifying the user at the point the
> > error occurs, while the device is still in the process or being reset?
> > My question is how does the user know that the host reset is complete
> > in order to begin their own re-initialization?  
> I will add a state in "struct vfio_pci_device".
> The state is set when the device can not work such as a aer error
>   occured.
> And the state is clear when the device can work such as resume
>   received.
> Return the state when user get info by vfio_pci_ioctl.
> 
> >> The interrupt status will be cleared by hardware.
> >> So the hardware is the same as the state when the
> >> vfio device fd is opened.  
> >
> > The PCI-core in Linux will save and restore the device state around
> > reset, how do we know that vfio-pci itself is not racing that reset and
> > whether PCI-core will restore the state including our interrupt masking
> > or a state without it?  Do we need to restore the state to the one we
> > saved when we originally opened the device?  Shouldn't that mean we
> > teardown the interrupt setup the user had prior to the error event?  
> For above you said.
> Maybe disable the interrupt is not a good idea.
> Think about what will happend in the interrupt handler.
> Maybe read/write configure space and region bar.
> I will make the configure space read only.
> Do nothing for region bar which used by userd.

I'm thinking that vfio-pci will be attempting to mask the interrupts
via the PCI command register, which is potentially in a state of flux
due to the host reset and yet we're somehow expecting that our write to
the command register sticks.  We certainly have the ability to a)
discard interrupts received between AER error and resume, and b) if we
want to be consistent with requiring the user to reinitialize the
device, then the user interrupt setup should likely be torn down.
Thanks,

Alex