Re: [Qemu-devel] [RFC v3 08/10] vfio-pci: add VFIO_FEATURE_ENABLE_AER_CAP feature

[Chen Fan <chen.fan.fnst@cn.fujitsu.com>]

On 02/11/2015 12:39 AM, Alex Williamson wrote:
> On Tue, 2015-02-10 at 15:03 +0800, Chen Fan wrote:
>> add a new "aercap" feature in vfio device, for controlling
>> whether expose aer capability.
>>
>> Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
>> ---
>>   hw/vfio/pci.c | 10 ++++++++--
>>   1 file changed, 8 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
>> index bf314a1..c21b40b 100644
>> --- a/hw/vfio/pci.c
>> +++ b/hw/vfio/pci.c
>> @@ -138,6 +138,8 @@ typedef struct VFIOMSIXInfo {
>>   enum {
>>   #define VFIO_FEATURE_ENABLE_VGA_BIT 0
>>       VFIO_FEATURE_ENABLE_VGA = (1 << VFIO_FEATURE_ENABLE_VGA_BIT),
>> +#define VFIO_FEATURE_ENABLE_AER_CAP_BIT 1
>> +    VFIO_FEATURE_ENABLE_AER_CAP = (1 << VFIO_FEATURE_ENABLE_AER_CAP_BIT),
>>   };
>>   
>>   typedef struct VFIOPCIDevice {
>> @@ -2721,8 +2723,10 @@ static int vfio_add_ext_cap(VFIOPCIDevice *vdev, uint16_t pos)
>>   
>>       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);
>> +        if (vdev->features & VFIO_FEATURE_ENABLE_AER_CAP) {
>> +            pcie_add_capability(pdev, cap_id, cap_ver, pos, size);
>> +            vfio_setup_pcie_aer(vdev, pos);
>> +        }
Hi Alex,
     sorry for replaying so late. I am just back from holiday. :)

> Maybe the question should be why we're adding extended capabilities at
> all if the chipset doesn't expose them.  If we boot on 440fx, all
> extended capability parsing could be disabled.  We could then add an
> x-aer=off option to vfio-pci to allow the user to disable aer
> specifically if they want.
your meaning is adding two option:

1) one controls exposing extended caps to device. in particular on
440fx chipset we must disable it.

2) the other one controls exposing aer capability to user specifically.

right?

Thanks,
Chen





>>           break;
>>       default:
>>           pcie_add_capability(pdev, cap_id, cap_ver, pos, size);
>> @@ -3487,6 +3491,8 @@ static Property vfio_pci_dev_properties[] = {
>>       DEFINE_PROP_BIT("x-vga", VFIOPCIDevice, features,
>>                       VFIO_FEATURE_ENABLE_VGA_BIT, false),
>>       DEFINE_PROP_INT32("bootindex", VFIOPCIDevice, bootindex, -1),
>> +    DEFINE_PROP_BIT("aercap", VFIOPCIDevice, features,
>> +                    VFIO_FEATURE_ENABLE_AER_CAP_BIT, true),
>>       /*
>>        * TODO - support passed fds... is this necessary?
>>        * DEFINE_PROP_STRING("vfiofd", VFIOPCIDevice, vfiofd_name),
>
>
> .
>