From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34474) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YINPd-0000fj-I0 for qemu-devel@nongnu.org; Mon, 02 Feb 2015 15:16:06 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YINPY-00014n-NX for qemu-devel@nongnu.org; Mon, 02 Feb 2015 15:16:01 -0500 Received: from mx1.redhat.com ([209.132.183.28]:41843) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YINPY-00014B-FY for qemu-devel@nongnu.org; Mon, 02 Feb 2015 15:15:56 -0500 Message-ID: <1422908147.22865.416.camel@redhat.com> From: Alex Williamson Date: Mon, 02 Feb 2015 13:15:47 -0700 In-Reply-To: <515cec0e8873b456341be9a0272f5fbea805c65f.1422433767.git.chen.fan.fnst@cn.fujitsu.com> References: <515cec0e8873b456341be9a0272f5fbea805c65f.1422433767.git.chen.fan.fnst@cn.fujitsu.com> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [RFC v2 2/8] vfio-pci: add aer capability support List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Chen Fan Cc: marcel@redhat.com, izumi.taku@jp.fujitsu.com, qemu-devel@nongnu.org On Wed, 2015-01-28 at 16:37 +0800, Chen Fan wrote: > if we detect the aer capability in vfio device, then > we should initialize the vfio device aer rigister bits. > so guest OS can set this bits as needed. s/rigister/register/ > Signed-off-by: Chen Fan > --- > hw/vfio/pci.c | 72 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 72 insertions(+) > > diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c > index 014a92c..2072261 100644 > --- a/hw/vfio/pci.c > +++ b/hw/vfio/pci.c > @@ -2670,6 +2670,73 @@ static int vfio_add_capabilities(VFIOPCIDevice *vdev) > return vfio_add_std_cap(vdev, pdev->config[PCI_CAPABILITY_LIST]); > } > > +static void vfio_pci_aer_init(VFIOPCIDevice *vdev) > +{ > + PCIDevice *pdev = &vdev->pdev; > + PCIExpressDevice *exp = &pdev->exp; > + uint16_t offset = exp->aer_cap; > + > + if (!offset) { > + return; > + } > + All of these need to be documented with comments. > + memset(pdev->wmask + offset, 0, PCI_ERR_SIZEOF); > + memset(pdev->w1cmask + offset, 0, PCI_ERR_SIZEOF); > + memset(pdev->cmask + offset, 0xFF, PCI_ERR_SIZEOF); > + > + pci_long_test_and_set_mask(pdev->wmask + exp->exp_cap + PCI_EXP_DEVCTL, > + PCI_EXP_DEVCTL_CERE | PCI_EXP_DEVCTL_NFERE | > + PCI_EXP_DEVCTL_FERE | PCI_EXP_DEVCTL_URRE); > + pci_long_test_and_set_mask(pdev->w1cmask + exp->exp_cap + PCI_EXP_DEVSTA, > + PCI_EXP_DEVSTA_CED | PCI_EXP_DEVSTA_NFED | > + PCI_EXP_DEVSTA_FED | PCI_EXP_DEVSTA_URD); > + > + pci_set_long(pdev->w1cmask + offset + PCI_ERR_UNCOR_STATUS, > + PCI_ERR_UNC_SUPPORTED); > + pci_set_long(pdev->wmask + offset + PCI_ERR_UNCOR_SEVER, > + PCI_ERR_UNC_SUPPORTED); > + pci_long_test_and_set_mask(pdev->w1cmask + offset + PCI_ERR_COR_STATUS, > + PCI_ERR_COR_STATUS); > + pci_set_long(pdev->wmask + offset + PCI_ERR_COR_MASK, > + PCI_ERR_COR_SUPPORTED); > + > + pci_set_long(pdev->wmask + offset + PCI_ERR_CAP, > + PCI_ERR_CAP_ECRC_GENE | PCI_ERR_CAP_ECRC_CHKE | > + PCI_ERR_CAP_MHRE); > +} > + > +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; Shouldn't we call pcie_aer_init() here? > + > + vfio_pci_aer_init(vdev); > + break; > + }; > + > + next = PCI_EXT_CAP_NEXT(header); > + if (!next) { > + return 0; > + } > + header = pci_get_long(pdev->config + next); I'd like to see this look more like vfio_add_std_cap(), registering every capability with the QEMU PCIe-core and setting up emulation to allow QEMU to skip capabilities that it doesn't want to expose. > + } > + > + return 0; > +} > + > static void vfio_pci_pre_reset(VFIOPCIDevice *vdev) > { > PCIDevice *pdev = &vdev->pdev; > @@ -3296,6 +3363,11 @@ static int vfio_initfn(PCIDevice *pdev) > goto out_teardown; > } > > + ret = vfio_add_ext_capabilities(vdev); > + if (ret) { > + goto out_teardown; > + } > + Why not extend vfio_add_capabilities()? It specifically calls vfio_add_std_cap() in order that there could be a vfio_add_ext_cap(). > /* QEMU emulates all of MSI & MSIX */ > if (pdev->cap_present & QEMU_PCI_CAP_MSIX) { > memset(vdev->emulated_config_bits + pdev->msix_cap, 0xff,