[v16,07/14] vfio: add aer support for vfio device

From: Chen Fan <chen.fan.fnst@cn.fujitsu.com>

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 | 81 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++---
 hw/vfio/pci.h |  3 +++
 2 files changed, 81 insertions(+), 3 deletions(-)

On 01/12/2016 04:43 AM, Cao jin wrote:
> From: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
>
> 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 | 81 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++---
>   hw/vfio/pci.h |  3 +++
>   2 files changed, 81 insertions(+), 3 deletions(-)
>
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index 64b0867..38b0aa5 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -1832,6 +1832,62 @@ static int vfio_add_std_cap(VFIOPCIDevice *vdev, uint8_t pos)
>       return 0;
>   }
>
> +static int vfio_setup_aer(VFIOPCIDevice *vdev, uint8_t cap_ver,
> +                          int pos, uint16_t size)
> +{
> +    PCIDevice *pdev = &vdev->pdev;
> +    PCIDevice *dev_iter;
> +    uint8_t type;
> +    uint32_t errcap;
> +
> +    if (!(vdev->features & VFIO_FEATURE_ENABLE_AER)) {
> +        pcie_add_capability(pdev, PCI_EXT_CAP_ID_ERR,
> +                            cap_ver, pos, size);
> +        return 0;
> +    }
> +
> +    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, pos + 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);
> +    return pcie_aer_init(pdev, pos, size);
> +
> +error:
> +    error_report("vfio: Unable to enable AER for device %s, parent bus "
> +                 "does not support AER signaling", vdev->vbasedev.name);
> +    return -1;
> +}
> +
>   static int vfio_add_ext_cap(VFIOPCIDevice *vdev)
>   {
>       PCIDevice *pdev = &vdev->pdev;
> @@ -1839,6 +1895,7 @@ static int vfio_add_ext_cap(VFIOPCIDevice *vdev)
>       uint16_t cap_id, next, size;
>       uint8_t cap_ver;
>       uint8_t *config;
> +    int ret = 0;
>
>       /*
>        * In order to avoid breaking config space, create a copy to
> @@ -1860,16 +1917,29 @@ static int vfio_add_ext_cap(VFIOPCIDevice *vdev)
>            */
>           size = vfio_ext_cap_max_size(config, next);
>
> -        pcie_add_capability(pdev, cap_id, cap_ver, next, size);
> -        pci_set_long(dev->config + next, PCI_EXT_CAP(cap_id, cap_ver, 0));
> +        switch (cap_id) {
> +        case PCI_EXT_CAP_ID_ERR:
> +            ret = vfio_setup_aer(vdev, cap_ver, next, size);
> +            break;
> +        default:
> +            pcie_add_capability(pdev, cap_id, cap_ver, next, size);
> +            break;
> +        }
> +
> +        if (ret) {
> +            goto out;
> +        }
> +
> +        pci_set_long(pdev->config + next, PCI_EXT_CAP(cap_id, cap_ver, 0));
>
>           /* Use emulated next pointer to allow dropping extended caps */
>           pci_long_test_and_set_mask(vdev->emulated_config_bits + next,
>                                      PCI_EXT_CAP_NEXT_MASK);
>       }
>
> +out:
>       g_free(config);
> -    return 0;
> +    return ret;
>   }
>
>   static int vfio_add_capabilities(VFIOPCIDevice *vdev)
> @@ -2624,6 +2694,11 @@ static int vfio_initfn(PCIDevice *pdev)
>           goto out_teardown;
>       }
>
> +    if ((vdev->features & VFIO_FEATURE_ENABLE_AER) &&
> +        !pdev->exp.aer_cap) {

Hi,

I think we need an error_report here, otherwise the init
will fail without knowing the reason.


Thanks,
Marcel


> +        goto out_teardown;
> +    }
> +
>       /* QEMU emulates all of MSI & MSIX */
>       if (pdev->cap_present & QEMU_PCI_CAP_MSIX) {
>           memset(vdev->emulated_config_bits + pdev->msix_cap, 0xff,
> diff --git a/hw/vfio/pci.h b/hw/vfio/pci.h
> index f004d52..48c1f69 100644
> --- a/hw/vfio/pci.h
> +++ b/hw/vfio/pci.h
> @@ -15,6 +15,7 @@
>   #include "qemu-common.h"
>   #include "exec/memory.h"
>   #include "hw/pci/pci.h"
> +#include "hw/pci/pci_bridge.h"
>   #include "hw/vfio/vfio-common.h"
>   #include "qemu/event_notifier.h"
>   #include "qemu/queue.h"
> @@ -127,6 +128,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;
>

On 01/18/2016 05:12 PM, Marcel Apfelbaum wrote:
> On 01/12/2016 04:43 AM, Cao jin wrote:
>> From: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
>>
>> 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 | 81 
>> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++---
>>   hw/vfio/pci.h |  3 +++
>>   2 files changed, 81 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
>> index 64b0867..38b0aa5 100644
>> --- a/hw/vfio/pci.c
>> +++ b/hw/vfio/pci.c
>> @@ -1832,6 +1832,62 @@ static int vfio_add_std_cap(VFIOPCIDevice 
>> *vdev, uint8_t pos)
>>       return 0;
>>   }
>>
>> +static int vfio_setup_aer(VFIOPCIDevice *vdev, uint8_t cap_ver,
>> +                          int pos, uint16_t size)
>> +{
>> +    PCIDevice *pdev = &vdev->pdev;
>> +    PCIDevice *dev_iter;
>> +    uint8_t type;
>> +    uint32_t errcap;
>> +
>> +    if (!(vdev->features & VFIO_FEATURE_ENABLE_AER)) {
>> +        pcie_add_capability(pdev, PCI_EXT_CAP_ID_ERR,
>> +                            cap_ver, pos, size);
>> +        return 0;
>> +    }
>> +
>> +    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, pos + 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);
>> +    return pcie_aer_init(pdev, pos, size);
>> +
>> +error:
>> +    error_report("vfio: Unable to enable AER for device %s, parent 
>> bus "
>> +                 "does not support AER signaling", 
>> vdev->vbasedev.name);
>> +    return -1;
>> +}
>> +
>>   static int vfio_add_ext_cap(VFIOPCIDevice *vdev)
>>   {
>>       PCIDevice *pdev = &vdev->pdev;
>> @@ -1839,6 +1895,7 @@ static int vfio_add_ext_cap(VFIOPCIDevice *vdev)
>>       uint16_t cap_id, next, size;
>>       uint8_t cap_ver;
>>       uint8_t *config;
>> +    int ret = 0;
>>
>>       /*
>>        * In order to avoid breaking config space, create a copy to
>> @@ -1860,16 +1917,29 @@ static int vfio_add_ext_cap(VFIOPCIDevice *vdev)
>>            */
>>           size = vfio_ext_cap_max_size(config, next);
>>
>> -        pcie_add_capability(pdev, cap_id, cap_ver, next, size);
>> -        pci_set_long(dev->config + next, PCI_EXT_CAP(cap_id, 
>> cap_ver, 0));
>> +        switch (cap_id) {
>> +        case PCI_EXT_CAP_ID_ERR:
>> +            ret = vfio_setup_aer(vdev, cap_ver, next, size);
>> +            break;
>> +        default:
>> +            pcie_add_capability(pdev, cap_id, cap_ver, next, size);
>> +            break;
>> +        }
>> +
>> +        if (ret) {
>> +            goto out;
>> +        }
>> +
>> +        pci_set_long(pdev->config + next, PCI_EXT_CAP(cap_id, 
>> cap_ver, 0));
>>
>>           /* Use emulated next pointer to allow dropping extended 
>> caps */
>> pci_long_test_and_set_mask(vdev->emulated_config_bits + next,
>>                                      PCI_EXT_CAP_NEXT_MASK);
>>       }
>>
>> +out:
>>       g_free(config);
>> -    return 0;
>> +    return ret;
>>   }
>>
>>   static int vfio_add_capabilities(VFIOPCIDevice *vdev)
>> @@ -2624,6 +2694,11 @@ static int vfio_initfn(PCIDevice *pdev)
>>           goto out_teardown;
>>       }
>>
>> +    if ((vdev->features & VFIO_FEATURE_ENABLE_AER) &&
>> +        !pdev->exp.aer_cap) {
>
> Hi,
>
> I think we need an error_report here, otherwise the init
> will fail without knowing the reason.
maybe we need to exclude this case. if the device hasn't the aer cap,
the ENABLE_AER would be not affected.

Thanks,
Chen

>
>
> Thanks,
> Marcel
>
>
>> +        goto out_teardown;
>> +    }
>> +
>>       /* QEMU emulates all of MSI & MSIX */
>>       if (pdev->cap_present & QEMU_PCI_CAP_MSIX) {
>>           memset(vdev->emulated_config_bits + pdev->msix_cap, 0xff,
>> diff --git a/hw/vfio/pci.h b/hw/vfio/pci.h
>> index f004d52..48c1f69 100644
>> --- a/hw/vfio/pci.h
>> +++ b/hw/vfio/pci.h
>> @@ -15,6 +15,7 @@
>>   #include "qemu-common.h"
>>   #include "exec/memory.h"
>>   #include "hw/pci/pci.h"
>> +#include "hw/pci/pci_bridge.h"
>>   #include "hw/vfio/vfio-common.h"
>>   #include "qemu/event_notifier.h"
>>   #include "qemu/queue.h"
>> @@ -127,6 +128,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;
>>
>
>
>
> .
>