Re: [Qemu-devel] [RFC v8.1 05/13] vfio: add aer support for vfio device

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

On Wed, 2015-05-27 at 10:46 +0800, Chen Fan wrote:
> Calling pcie_aer_init to initilize aer related registers for
> vfio device, then reload physical related registers to expose
> device capability.
> 
> Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
> ---
>  hw/vfio/pci.c | 83 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 82 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index 81a4a9a..f4e7855 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -32,6 +32,7 @@
>  #include "hw/pci/msi.h"
>  #include "hw/pci/msix.h"
>  #include "hw/pci/pci.h"
> +#include "hw/pci/pci_bridge.h"
>  #include "qemu-common.h"
>  #include "qemu/error-report.h"
>  #include "qemu/event_notifier.h"
> @@ -160,6 +161,8 @@ typedef struct VFIOPCIDevice {
>  #define VFIO_FEATURE_ENABLE_VGA (1 << VFIO_FEATURE_ENABLE_VGA_BIT)
>  #define VFIO_FEATURE_ENABLE_REQ_BIT 1
>  #define VFIO_FEATURE_ENABLE_REQ (1 << VFIO_FEATURE_ENABLE_REQ_BIT)
> +#define VFIO_FEATURE_ENABLE_AER_BIT 2
> +#define VFIO_FEATURE_ENABLE_AER (1 << VFIO_FEATURE_ENABLE_AER_BIT)
>      int32_t bootindex;
>      uint8_t pm_cap;
>      bool has_vga;
> @@ -2817,12 +2820,78 @@ static int vfio_add_std_cap(VFIOPCIDevice *vdev, uint8_t pos)
>      return 0;
>  }
>  
> +static int vfio_setup_aer(VFIOPCIDevice *vdev, int pos, uint16_t size)
> +{
> +    PCIDevice *pdev = &vdev->pdev;
> +    uint8_t *exp_cap = pdev->config + pdev->exp.exp_cap;
> +    PCIDevice *dev_iter;
> +    uint8_t type;
> +    uint32_t severity, errcap;
> +    int ret;
> +
> +    if (!(vdev->features & VFIO_FEATURE_ENABLE_AER)) {
> +        return 0;

There's a subtle behavioral change here that we not only do not enable
AER through QEMU, but we hide the AER capability if the AER feature bit
is not enabled.  Do we want to use pcie_add_capability() here to
continue exposing the dummy AER capability?

> +    }
> +
> +    dev_iter = pci_bridge_get_device(pdev->bus);
> +    if (!dev_iter) {
> +        goto error;
> +    }
> +
> +    while (dev_iter) {
> +        type = pcie_cap_get_type(dev_iter);
> +        if ((type != PCI_EXP_TYPE_ROOT_PORT &&
> +             type != PCI_EXP_TYPE_UPSTREAM &&
> +             type != PCI_EXP_TYPE_DOWNSTREAM)) {
> +            goto error;
> +        }
> +
> +        if (!dev_iter->exp.aer_cap) {
> +            goto error;
> +        }
> +
> +        dev_iter = pci_bridge_get_device(dev_iter->bus);
> +    }
> +
> +    errcap = vfio_pci_read_config(pdev, pdev->exp.aer_cap + PCI_ERR_CAP, 4);
> +    /*
> +     * The ability to record multiple headers is depending on
> +     * the state of the Multiple Header Recording Capable bit and
> +     * enabled by the Multiple Header Recording Enable bit.
> +     */
> +    if ((errcap & PCI_ERR_CAP_MHRC) &&
> +        (errcap & PCI_ERR_CAP_MHRE)) {
> +        pdev->exp.aer_log.log_max = PCIE_AER_LOG_MAX_DEFAULT;
> +    } else {
> +        pdev->exp.aer_log.log_max = 0;
> +    }
> +
> +    pcie_cap_deverr_init(pdev);
> +    ret = pcie_aer_init(pdev, pos, size);
> +    if (ret) {
> +        return ret;
> +    }
> +
> +    /* load physical registers */
> +    severity = vfio_pci_read_config(pdev,
> +                   pdev->exp.aer_cap + PCI_ERR_UNCOR_SEVER, 4);
> +    pci_long_test_and_clear_mask(exp_cap + PCI_ERR_UNCOR_SEVER, ~severity);
> +
> +    return 0;
> +
> +error:
> +    error_report("vfio: Unable to enable AER for device %s, parent bus "
> +                 "do not support signal AER", vdev->vbasedev.name);

"does not support AER signaling"

> +    return -1;
> +}
> +
>  static int vfio_add_ext_cap(VFIOPCIDevice *vdev, const uint8_t *config)
>  {
>      PCIDevice *pdev = &vdev->pdev;
>      uint32_t header;
>      uint16_t cap_id, next, size;
>      uint8_t cap_ver;
> +    int ret = 0;
>  
>      for (next = PCI_CONFIG_SPACE_SIZE; next;
>           next = PCI_EXT_CAP_NEXT(pci_get_long(config + next))) {
> @@ -2838,7 +2907,19 @@ static int vfio_add_ext_cap(VFIOPCIDevice *vdev, const uint8_t *config)
>           */
>          size = vfio_ext_cap_max_size(config, next);
>  
> -        pcie_add_capability(pdev, cap_id, cap_ver, next, size);
> +        switch (cap_id) {
> +        case PCI_EXT_CAP_ID_ERR:
> +            ret = vfio_setup_aer(vdev, next, size);
> +            break;
> +        default:
> +            pcie_add_capability(pdev, cap_id, cap_ver, next, size);
> +            break;
> +        }
> +
> +        if (ret) {
> +            return ret;
> +        }
> +
>          if (next == PCI_CONFIG_SPACE_SIZE) {
>              /* Begin the rebuild, we should set the next offset zero. */
>              pci_set_long(pdev->config + next, PCI_EXT_CAP(cap_id, cap_ver, 0));