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

[Michael S. Tsirkin]

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.



> 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
> 
>