diff mbox

[RFC,3/4] vfio-pci: add aer capability support

Message ID 1e75bc42628775555cabe140c8438af0093d4ead.1421028274.git.chen.fan.fnst@cn.fujitsu.com
State New
Headers show

Commit Message

chenfan Jan. 12, 2015, 3:04 a.m. UTC 
Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
---
 hw/vfio/pci.c | 40 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

Comments

Paolo Bonzini Jan. 12, 2015, 1:14 p.m. UTC | #1 
On 12/01/2015 04:04, Chen Fan wrote:
> +static int vfio_add_ext_capabilities(VFIOPCIDevice *vdev)
> +{
> +    PCIDevice *pdev = &vdev->pdev;
> +    PCIExpressDevice *exp;
> +    uint32_t header;
> +    uint16_t next = PCI_CONFIG_SPACE_SIZE;
> +
> +    if (pci_config_size(pdev) <= PCI_CONFIG_SPACE_SIZE) {
> +        return 0;
> +    }
> +
> +    header = pci_get_long(pdev->config + next);
> +    while (header) {
> +        switch (PCI_EXT_CAP_ID(header)) {
> +        case PCI_EXT_CAP_ID_ERR:
> +             exp = &pdev->exp;
> +             exp->aer_cap = next;
> +
> +             /* enable the error report */
> +             vfio_add_emulated_long(vdev, exp->exp_cap + PCI_EXP_DEVCTL,
> +                 PCI_EXP_DEVCTL_CERE | PCI_EXP_DEVCTL_NFERE |
> +                 PCI_EXP_DEVCTL_FERE | PCI_EXP_DEVCTL_URRE, ~0);
> +             break;
> +        };
> +
> +        next = PCI_EXT_CAP_NEXT(header);
> +        if (!next) {
> +            return 0;
> +        }
> +        header = pci_get_long(pdev->config + next);
> +    }
> +
> +    return 0;
> +}
> +

Please add a property to the VFIO device, defaulting to true, and
disable it for older machine types.

Paolo
Alex Williamson Jan. 12, 2015, 3:26 p.m. UTC | #2 
On Mon, 2015-01-12 at 11:04 +0800, Chen Fan wrote:

This patch isn't trivial enough for a blank commit log.  Why do we need
to make those bits emulated?  Do we only care about AER for now?

> Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
> ---
>  hw/vfio/pci.c | 40 ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 40 insertions(+)
> 
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index b4e73d1..0ee6326 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -2667,6 +2667,41 @@ static int vfio_add_capabilities(VFIOPCIDevice *vdev)
>      return vfio_add_std_cap(vdev, pdev->config[PCI_CAPABILITY_LIST]);
>  }
>  
> +static int vfio_add_ext_capabilities(VFIOPCIDevice *vdev)
> +{
> +    PCIDevice *pdev = &vdev->pdev;
> +    PCIExpressDevice *exp;
> +    uint32_t header;
> +    uint16_t next = PCI_CONFIG_SPACE_SIZE;
> +
> +    if (pci_config_size(pdev) <= PCI_CONFIG_SPACE_SIZE) {
> +        return 0;
> +    }
> +
> +    header = pci_get_long(pdev->config + next);
> +    while (header) {
> +        switch (PCI_EXT_CAP_ID(header)) {
> +        case PCI_EXT_CAP_ID_ERR:
> +             exp = &pdev->exp;
> +             exp->aer_cap = next;
> +
> +             /* enable the error report */
> +             vfio_add_emulated_long(vdev, exp->exp_cap + PCI_EXP_DEVCTL,
> +                 PCI_EXP_DEVCTL_CERE | PCI_EXP_DEVCTL_NFERE |
> +                 PCI_EXP_DEVCTL_FERE | PCI_EXP_DEVCTL_URRE, ~0);
> +             break;
> +        };
> +
> +        next = PCI_EXT_CAP_NEXT(header);
> +        if (!next) {
> +            return 0;
> +        }
> +        header = pci_get_long(pdev->config + next);
> +    }
> +
> +    return 0;
> +}
> +
>  static void vfio_pci_pre_reset(VFIOPCIDevice *vdev)
>  {
>      PCIDevice *pdev = &vdev->pdev;
> @@ -3293,6 +3328,11 @@ static int vfio_initfn(PCIDevice *pdev)
>          goto out_teardown;
>      }
>  
> +    ret = vfio_add_ext_capabilities(vdev);
> +    if (ret) {
> +        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,
chenfan Jan. 15, 2015, 8:40 a.m. UTC | #3 
On 01/12/2015 11:26 PM, Alex Williamson wrote:
> On Mon, 2015-01-12 at 11:04 +0800, Chen Fan wrote:
>
> This patch isn't trivial enough for a blank commit log.  Why do we need
> to make those bits emulated?  Do we only care about AER for now?
I think the vfio extend capabilities control registers should be 
manipulated by qemu self.
BTW, it is guest driver's responsibility to set PCI_EXP_DEVCTL bits, 
right? so here
only need to initialize configration sapces and set corresponding bits 
attributes.
I will change it in my next version patches.

I think AER is a good start.

Thanks,
Chen
>
>> Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
>> ---
>>   hw/vfio/pci.c | 40 ++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 40 insertions(+)
>>
>> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
>> index b4e73d1..0ee6326 100644
>> --- a/hw/vfio/pci.c
>> +++ b/hw/vfio/pci.c
>> @@ -2667,6 +2667,41 @@ static int vfio_add_capabilities(VFIOPCIDevice *vdev)
>>       return vfio_add_std_cap(vdev, pdev->config[PCI_CAPABILITY_LIST]);
>>   }
>>   
>> +static int vfio_add_ext_capabilities(VFIOPCIDevice *vdev)
>> +{
>> +    PCIDevice *pdev = &vdev->pdev;
>> +    PCIExpressDevice *exp;
>> +    uint32_t header;
>> +    uint16_t next = PCI_CONFIG_SPACE_SIZE;
>> +
>> +    if (pci_config_size(pdev) <= PCI_CONFIG_SPACE_SIZE) {
>> +        return 0;
>> +    }
>> +
>> +    header = pci_get_long(pdev->config + next);
>> +    while (header) {
>> +        switch (PCI_EXT_CAP_ID(header)) {
>> +        case PCI_EXT_CAP_ID_ERR:
>> +             exp = &pdev->exp;
>> +             exp->aer_cap = next;
>> +
>> +             /* enable the error report */
>> +             vfio_add_emulated_long(vdev, exp->exp_cap + PCI_EXP_DEVCTL,
>> +                 PCI_EXP_DEVCTL_CERE | PCI_EXP_DEVCTL_NFERE |
>> +                 PCI_EXP_DEVCTL_FERE | PCI_EXP_DEVCTL_URRE, ~0);
>> +             break;
>> +        };
>> +
>> +        next = PCI_EXT_CAP_NEXT(header);
>> +        if (!next) {
>> +            return 0;
>> +        }
>> +        header = pci_get_long(pdev->config + next);
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>>   static void vfio_pci_pre_reset(VFIOPCIDevice *vdev)
>>   {
>>       PCIDevice *pdev = &vdev->pdev;
>> @@ -3293,6 +3328,11 @@ static int vfio_initfn(PCIDevice *pdev)
>>           goto out_teardown;
>>       }
>>   
>> +    ret = vfio_add_ext_capabilities(vdev);
>> +    if (ret) {
>> +        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,
>
>
> .
>
chenfan Jan. 15, 2015, 8:45 a.m. UTC | #4 
On 01/12/2015 09:14 PM, Paolo Bonzini wrote:
>
> On 12/01/2015 04:04, Chen Fan wrote:
>> +static int vfio_add_ext_capabilities(VFIOPCIDevice *vdev)
>> +{
>> +    PCIDevice *pdev = &vdev->pdev;
>> +    PCIExpressDevice *exp;
>> +    uint32_t header;
>> +    uint16_t next = PCI_CONFIG_SPACE_SIZE;
>> +
>> +    if (pci_config_size(pdev) <= PCI_CONFIG_SPACE_SIZE) {
>> +        return 0;
>> +    }
>> +
>> +    header = pci_get_long(pdev->config + next);
>> +    while (header) {
>> +        switch (PCI_EXT_CAP_ID(header)) {
>> +        case PCI_EXT_CAP_ID_ERR:
>> +             exp = &pdev->exp;
>> +             exp->aer_cap = next;
>> +
>> +             /* enable the error report */
>> +             vfio_add_emulated_long(vdev, exp->exp_cap + PCI_EXP_DEVCTL,
>> +                 PCI_EXP_DEVCTL_CERE | PCI_EXP_DEVCTL_NFERE |
>> +                 PCI_EXP_DEVCTL_FERE | PCI_EXP_DEVCTL_URRE, ~0);
>> +             break;
>> +        };
>> +
>> +        next = PCI_EXT_CAP_NEXT(header);
>> +        if (!next) {
>> +            return 0;
>> +        }
>> +        header = pci_get_long(pdev->config + next);
>> +    }
>> +
>> +    return 0;
>> +}
>> +
> Please add a property to the VFIO device, defaulting to true, and
> disable it for older machine types.
Thanks for your suggestion.

Chen
>
> Paolo
>
diff mbox

Patch

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index b4e73d1..0ee6326 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -2667,6 +2667,41 @@  static int vfio_add_capabilities(VFIOPCIDevice *vdev)
     return vfio_add_std_cap(vdev, pdev->config[PCI_CAPABILITY_LIST]);
 }
 
+static int vfio_add_ext_capabilities(VFIOPCIDevice *vdev)
+{
+    PCIDevice *pdev = &vdev->pdev;
+    PCIExpressDevice *exp;
+    uint32_t header;
+    uint16_t next = PCI_CONFIG_SPACE_SIZE;
+
+    if (pci_config_size(pdev) <= PCI_CONFIG_SPACE_SIZE) {
+        return 0;
+    }
+
+    header = pci_get_long(pdev->config + next);
+    while (header) {
+        switch (PCI_EXT_CAP_ID(header)) {
+        case PCI_EXT_CAP_ID_ERR:
+             exp = &pdev->exp;
+             exp->aer_cap = next;
+
+             /* enable the error report */
+             vfio_add_emulated_long(vdev, exp->exp_cap + PCI_EXP_DEVCTL,
+                 PCI_EXP_DEVCTL_CERE | PCI_EXP_DEVCTL_NFERE |
+                 PCI_EXP_DEVCTL_FERE | PCI_EXP_DEVCTL_URRE, ~0);
+             break;
+        };
+
+        next = PCI_EXT_CAP_NEXT(header);
+        if (!next) {
+            return 0;
+        }
+        header = pci_get_long(pdev->config + next);
+    }
+
+    return 0;
+}
+
 static void vfio_pci_pre_reset(VFIOPCIDevice *vdev)
 {
     PCIDevice *pdev = &vdev->pdev;
@@ -3293,6 +3328,11 @@  static int vfio_initfn(PCIDevice *pdev)
         goto out_teardown;
     }
 
+    ret = vfio_add_ext_capabilities(vdev);
+    if (ret) {
+        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,