Re: [Qemu-devel] [RFC v3 04/10] vfio: add pcie extanded capability support

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

On Tue, 2015-02-10 at 15:03 +0800, Chen Fan wrote:
> when we detect extanded capability in vfio device, then
> we should initialize the vfio device corresponding feature
> register bits.
> so guest OS can find it and set those bits as needed.
> and initialize aer capability.
> 
> Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
> ---
>  hw/vfio/pci.c | 85 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 84 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index 014a92c..75c932b 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -2435,6 +2435,20 @@ static uint8_t vfio_std_cap_max_size(PCIDevice *pdev, uint8_t pos)
>      return next - pos;
>  }
>  
> +static uint16_t vfio_ext_cap_max_size(PCIDevice *pdev, uint16_t pos)
> +{
> +    uint16_t tmp, next = PCIE_CONFIG_SPACE_SIZE - 1;
> +
> +    for (tmp = PCI_CONFIG_SPACE_SIZE; tmp;
> +        tmp = PCI_EXT_CAP_NEXT(pci_get_long(pdev->config + tmp))) {
> +        if (tmp > pos && tmp < next) {
> +            next = tmp;
> +	}
> +    }
> +
> +    return next - pos;
> +}
> +
>  static void vfio_set_word_bits(uint8_t *buf, uint16_t val, uint16_t mask)
>  {
>      pci_set_word(buf, (pci_get_word(buf) & ~mask) | val);
> @@ -2658,16 +2672,85 @@ static int vfio_add_std_cap(VFIOPCIDevice *vdev, uint8_t pos)
>      return 0;
>  }
>  
> +static void vfio_setup_pcie_aer(VFIOPCIDevice *vdev, uint16_t pos)
> +{
> +    PCIDevice *pdev = &vdev->pdev;
> +    uint32_t err_cap;
> +
> +    err_cap = pci_get_long(pdev->config + pos + PCI_ERR_CAP);
> +
> +    if (err_cap & PCI_ERR_CAP_MHRC) {
> +        pdev->exp.aer_log.log_max = PCIE_AER_LOG_MAX_DEFAULT;
> +    } else {
> +        pdev->exp.aer_log.log_max = 1;
> +    }
> +
> +    pdev->exp.aer_log.log = g_malloc0(sizeof pdev->exp.aer_log.log[0] *
> +                                      pdev->exp.aer_log.log_max);
> +
> +    pcie_aer_setup(pdev, pos, pdev->exp.aer_log.log_max);
> +}
> +
> +static int vfio_add_ext_cap(VFIOPCIDevice *vdev, uint16_t pos)
> +{
> +    PCIDevice *pdev = &vdev->pdev;
> +    uint32_t header;
> +    uint16_t cap_id, next, size;
> +    uint8_t cap_ver;
> +    int ret;
> +
> +    header = pci_get_long(pdev->config + pos);
> +    cap_id = PCI_EXT_CAP_ID(header);
> +    cap_ver = PCI_EXT_CAP_VER(header);
> +    next = PCI_EXT_CAP_NEXT(header);
> +
> +    size = vfio_ext_cap_max_size(pdev, pos);
> +

There's a big comment in the standard capability version of this that
indicates that pci_add_capability() always adds capabilities to the head
of the chain and therefore we use this recursion to keep the same
ordering as hardware.  pcie_add_capability() seems to add at the tail of
the list, so I think this actually reverses the list versus hardware,
which is undesirable.  Code comments would be nice too.

> +    if (next) {
> +        ret = vfio_add_ext_cap(vdev, next);
> +        if (ret) {
> +            return ret;
> +        }
> +    }
> +
> +    pci_set_long(vdev->emulated_config_bits + pos, 0xffff);
> +
> +    switch (cap_id) {
> +    case PCI_EXT_CAP_ID_ERR:
> +        pcie_add_capability(pdev, cap_id, cap_ver, pos, size);
> +        vfio_setup_pcie_aer(vdev, pos);
> +        break;
> +    default:
> +        pcie_add_capability(pdev, cap_id, cap_ver, pos, size);
> +        break;
> +    }
> +
> +    return 0;
> +}
> +
>  static int vfio_add_capabilities(VFIOPCIDevice *vdev)
>  {
>      PCIDevice *pdev = &vdev->pdev;
> +    int ret;
>  
>      if (!(pdev->config[PCI_STATUS] & PCI_STATUS_CAP_LIST) ||
>          !pdev->config[PCI_CAPABILITY_LIST]) {
>          return 0; /* Nothing to add */
>      }
>  
> -    return vfio_add_std_cap(vdev, pdev->config[PCI_CAPABILITY_LIST]);
> +    ret = vfio_add_std_cap(vdev, pdev->config[PCI_CAPABILITY_LIST]);
> +    if (ret) {
> +        goto out;
> +    }
> +
> +    if (!pci_get_long(pdev->config + PCI_CONFIG_SPACE_SIZE)) {
> +        goto out;
> +    }
> +
> +    ret = vfio_add_ext_cap(vdev, PCI_CONFIG_SPACE_SIZE);
> +
> +out:
> +    return ret;
>  }
>  
>  static void vfio_pci_pre_reset(VFIOPCIDevice *vdev)