From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40835) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aL5rJ-0002V2-W4 for qemu-devel@nongnu.org; Mon, 18 Jan 2016 04:12:22 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aL5rF-0001an-Sl for qemu-devel@nongnu.org; Mon, 18 Jan 2016 04:12:21 -0500 Received: from mail-wm0-x244.google.com ([2a00:1450:400c:c09::244]:36638) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aL5rF-0001ad-JH for qemu-devel@nongnu.org; Mon, 18 Jan 2016 04:12:17 -0500 Received: by mail-wm0-x244.google.com with SMTP id l65so15263321wmf.3 for ; Mon, 18 Jan 2016 01:12:17 -0800 (PST) References: From: Marcel Apfelbaum Message-ID: <569CAC6E.8080802@gmail.com> Date: Mon, 18 Jan 2016 11:12:14 +0200 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v16 07/14] vfio: add aer support for vfio device Reply-To: marcel@redhat.com List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Cao jin , qemu-devel@nongnu.org Cc: chen.fan.fnst@cn.fujitsu.com, izumi.taku@jp.fujitsu.com, alex.williamson@redhat.com, mst@redhat.com On 01/12/2016 04:43 AM, Cao jin wrote: > From: Chen Fan > > Calling pcie_aer_init to initilize aer related registers for > vfio device, then reload physical related registers to expose > device capability. > > Signed-off-by: Chen Fan > --- > hw/vfio/pci.c | 81 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++--- > hw/vfio/pci.h | 3 +++ > 2 files changed, 81 insertions(+), 3 deletions(-) > > diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c > index 64b0867..38b0aa5 100644 > --- a/hw/vfio/pci.c > +++ b/hw/vfio/pci.c > @@ -1832,6 +1832,62 @@ static int vfio_add_std_cap(VFIOPCIDevice *vdev, uint8_t pos) > return 0; > } > > +static int vfio_setup_aer(VFIOPCIDevice *vdev, uint8_t cap_ver, > + int pos, uint16_t size) > +{ > + PCIDevice *pdev = &vdev->pdev; > + PCIDevice *dev_iter; > + uint8_t type; > + uint32_t errcap; > + > + if (!(vdev->features & VFIO_FEATURE_ENABLE_AER)) { > + pcie_add_capability(pdev, PCI_EXT_CAP_ID_ERR, > + cap_ver, pos, size); > + return 0; > + } > + > + dev_iter = pci_bridge_get_device(pdev->bus); > + if (!dev_iter) { > + goto error; > + } > + > + while (dev_iter) { > + type = pcie_cap_get_type(dev_iter); > + if ((type != PCI_EXP_TYPE_ROOT_PORT && > + type != PCI_EXP_TYPE_UPSTREAM && > + type != PCI_EXP_TYPE_DOWNSTREAM)) { > + goto error; > + } > + > + if (!dev_iter->exp.aer_cap) { > + goto error; > + } > + > + dev_iter = pci_bridge_get_device(dev_iter->bus); > + } > + > + errcap = vfio_pci_read_config(pdev, pos + PCI_ERR_CAP, 4); > + /* > + * The ability to record multiple headers is depending on > + * the state of the Multiple Header Recording Capable bit and > + * enabled by the Multiple Header Recording Enable bit. > + */ > + if ((errcap & PCI_ERR_CAP_MHRC) && > + (errcap & PCI_ERR_CAP_MHRE)) { > + pdev->exp.aer_log.log_max = PCIE_AER_LOG_MAX_DEFAULT; > + } else { > + pdev->exp.aer_log.log_max = 0; > + } > + > + pcie_cap_deverr_init(pdev); > + return pcie_aer_init(pdev, pos, size); > + > +error: > + error_report("vfio: Unable to enable AER for device %s, parent bus " > + "does not support AER signaling", vdev->vbasedev.name); > + return -1; > +} > + > static int vfio_add_ext_cap(VFIOPCIDevice *vdev) > { > PCIDevice *pdev = &vdev->pdev; > @@ -1839,6 +1895,7 @@ static int vfio_add_ext_cap(VFIOPCIDevice *vdev) > uint16_t cap_id, next, size; > uint8_t cap_ver; > uint8_t *config; > + int ret = 0; > > /* > * In order to avoid breaking config space, create a copy to > @@ -1860,16 +1917,29 @@ static int vfio_add_ext_cap(VFIOPCIDevice *vdev) > */ > size = vfio_ext_cap_max_size(config, next); > > - pcie_add_capability(pdev, cap_id, cap_ver, next, size); > - pci_set_long(dev->config + next, PCI_EXT_CAP(cap_id, cap_ver, 0)); > + switch (cap_id) { > + case PCI_EXT_CAP_ID_ERR: > + ret = vfio_setup_aer(vdev, cap_ver, next, size); > + break; > + default: > + pcie_add_capability(pdev, cap_id, cap_ver, next, size); > + break; > + } > + > + if (ret) { > + goto out; > + } > + > + pci_set_long(pdev->config + next, PCI_EXT_CAP(cap_id, cap_ver, 0)); > > /* Use emulated next pointer to allow dropping extended caps */ > pci_long_test_and_set_mask(vdev->emulated_config_bits + next, > PCI_EXT_CAP_NEXT_MASK); > } > > +out: > g_free(config); > - return 0; > + return ret; > } > > static int vfio_add_capabilities(VFIOPCIDevice *vdev) > @@ -2624,6 +2694,11 @@ static int vfio_initfn(PCIDevice *pdev) > goto out_teardown; > } > > + if ((vdev->features & VFIO_FEATURE_ENABLE_AER) && > + !pdev->exp.aer_cap) { Hi, I think we need an error_report here, otherwise the init will fail without knowing the reason. Thanks, Marcel > + 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, > diff --git a/hw/vfio/pci.h b/hw/vfio/pci.h > index f004d52..48c1f69 100644 > --- a/hw/vfio/pci.h > +++ b/hw/vfio/pci.h > @@ -15,6 +15,7 @@ > #include "qemu-common.h" > #include "exec/memory.h" > #include "hw/pci/pci.h" > +#include "hw/pci/pci_bridge.h" > #include "hw/vfio/vfio-common.h" > #include "qemu/event_notifier.h" > #include "qemu/queue.h" > @@ -127,6 +128,8 @@ typedef struct VFIOPCIDevice { > #define VFIO_FEATURE_ENABLE_VGA (1 << VFIO_FEATURE_ENABLE_VGA_BIT) > #define VFIO_FEATURE_ENABLE_REQ_BIT 1 > #define VFIO_FEATURE_ENABLE_REQ (1 << VFIO_FEATURE_ENABLE_REQ_BIT) > +#define VFIO_FEATURE_ENABLE_AER_BIT 2 > +#define VFIO_FEATURE_ENABLE_AER (1 << VFIO_FEATURE_ENABLE_AER_BIT) > int32_t bootindex; > uint8_t pm_cap; > bool has_vga; >