[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

From: Chen Fan
Subject: Re: [Qemu-devel] [RFC v3 08/10] vfio-pci: add VFIO_FEATURE_ENABLE_AER_CAP feature
Date: Mon, 2 Mar 2015 15:23:35 +0800
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.3.0

On 02/27/2015 06:18 AM, Alex Williamson wrote:
On Thu, 2015-02-26 at 14:46 +0800, Chen Fan wrote:
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 {
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, a flag toggled by the chipset type doesn't work because whether
we want to expose AER is dependent on the exposed bus type to the guest.
Even if we're using an express chipset like q35, the device could be
behind a PCIe-to-PCI bridge in the guest and the guest would not have
access to extended config space, including AER.  Therefore it seems like
we simply need to test pci_bus_is_express() for a device and skip all
extended capability parsing when false.

Then we've solved the bus topology problem and we only need to be
concerned about whether the user has a need to disable AER by choice.
We probably do need to allow that for compatibility with existing PCIe
guest configurations.  I'd probably name it something like "disable_aer"
because we don't want to give the user any ideas that it can be used to
toggle AER on, only off.  Thanks,
Hi Alex,

I has made a v4 patch which added a 'x-aer' to enable/disable aer capability
and remake the parsing extended caps method.
pls help to review it. :)



           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),



reply via email to

[Prev in Thread] Current Thread [Next in Thread]