Re: [Qemu-devel] [PATCH v16 00/14] vfio-pci: pass the aer error to guest

[Chen Fan]

On 01/17/2016 02:34 AM, Michael S. Tsirkin wrote:
> On Tue, Jan 12, 2016 at 10:43:01AM +0800, Cao jin wrote:
> > From: Chen Fan <address@hidden>
> >
> > For now, for vfio pci passthough devices when qemu receives
> > an error from host aer report, currentlly just terminate the guest,
> > but usually user want to know what error occurred but stopping the
> > guest, so this patches add aer capability support for vfio device,
> > and pass the error to guest, and have guest driver to recover
> > from the error.
> I would like to see a version of this patchset that doesn't
> depend on pci core changes.
> I think that if you make this simplifying assumption:
>
> - all devices on same bus in guest are on same bus in host
>
> then you can handle both reset and hotplug simply in function 0
> since it will belong to vfio.
>
> So we can have a version without pci core changes that simply assumes
> this, and things will just work.
>
>
> Now, if we wanted to enforce this limitation, I think the
> cleanest way would be to add a callback in struct PCIDevice:
>
>         bool is_valid_function(PCIDevice *newfunction)
>
> and call it as each function is added.
> This way aer function can validate that each function
> added shares the same bus.
> And this way issues will be detected directly and not when
> function 0 is added.
>
> I would prefer this validation code to be a patch on top so we can merge
> the functionality directly and avoid blocking it while we figure out the
> best api to validate things.
>
> I don't see why making guest topology match host would
> ever be a problem, but if it's required to support
> configurations where these differ, I'd like to see
> an attempt to address that be split out, after aer
> is supported.
Hi Michael,

   it's a good idea. we should simplify the implementation of the aer 
function first
without more affect on pci core code.

Thanks,
Chen

> > v15-v16:
> >     10/14, 11/14 are new to introduce a reset sequence id to specify the
> >     vfio devices has been reset for that reset. other patches aren't modified.
> >
> > v14-v15:
> >     1. add device hot reset callback
> >     2. add bus_in_reset for vfio device to avoid multi do host bus reset
> >
> > v13-v14:
> >     1. for multifunction device, requiring all functions enable AER.(9/13)
> >     2. due to all affected functions receive error signal, ignore no
> >        error occurred function. (12/13)
> >
> > v12-v13:
> >     1. since support multifuncion hotplug, here add callback to enable aer.
> >     2. add pci device pre+post reset for aer host reset.
> >
> > Chen Fan (14):
> >    vfio: extract vfio_get_hot_reset_info as a single function
> >    vfio: squeeze out vfio_pci_do_hot_reset for support bus reset
> >    pcie: modify the capability size assert
> >    vfio: make the 4 bytes aligned for capability size
> >    vfio: add pcie extanded capability support
> >    aer: impove pcie_aer_init to support vfio device
> >    vfio: add aer support for vfio device
> >    vfio: add check host bus reset is support or not
> >    add check reset mechanism when hotplug vfio device
> >    pci: introduce pci bus pre reset
> >    vfio: introduce last reset sequence id
> >    pcie_aer: expose pcie_aer_msg() interface
> >    vfio-pci: pass the aer error to guest
> >    vfio: add 'aer' property to expose aercap
> >
> >   hw/pci-bridge/ioh3420.c            |   2 +-
> >   hw/pci-bridge/xio3130_downstream.c |   2 +-
> >   hw/pci-bridge/xio3130_upstream.c   |   2 +-
> >   hw/pci/pci.c                       |  42 +++
> >   hw/pci/pci_bridge.c                |   3 +
> >   hw/pci/pcie.c                      |   2 +-
> >   hw/pci/pcie_aer.c                  |   6 +-
> >   hw/vfio/pci.c                      | 616 +++++++++++++++++++++++++++++++++----
> >   hw/vfio/pci.h                      |   9 +
> >   include/hw/pci/pci.h               |   1 +
> >   include/hw/pci/pci_bus.h           |   8 +
> >   include/hw/pci/pcie_aer.h          |   3 +-
> >   12 files changed, 624 insertions(+), 72 deletions(-)
> >
> > --
> > 1.9.3
>
> .