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

[Chen Fan]

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 <address@hidden>
> > ---
> >   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),
>
>
> .