From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52861) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bjpPd-0007iP-1v for qemu-devel@nongnu.org; Tue, 13 Sep 2016 11:14:22 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bjpPY-0004SS-2G for qemu-devel@nongnu.org; Tue, 13 Sep 2016 11:14:15 -0400 Received: from mx1.redhat.com ([209.132.183.28]:45940) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bjpPX-0004R3-QP for qemu-devel@nongnu.org; Tue, 13 Sep 2016 11:14:11 -0400 Date: Tue, 13 Sep 2016 09:14:09 -0600 From: Alex Williamson Message-ID: <20160913091409.4d607d6c@t450s.home> In-Reply-To: <722a1249-fde8-d2c4-50db-56938ca719bf@cn.fujitsu.com> References: <1468913909-21811-1-git-send-email-zhoujie2011@cn.fujitsu.com> <1468913909-21811-4-git-send-email-zhoujie2011@cn.fujitsu.com> <20160831134430.52e39567@t450s.home> <722a1249-fde8-d2c4-50db-56938ca719bf@cn.fujitsu.com> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v9 03/11] vfio: add aer support for vfio device List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Dou Liyang Cc: Zhou Jie , Chen Fan , izumi.taku@jp.fujitsu.com, fan.chen@easystack.cn, qemu-devel@nongnu.org, mst@redhat.com Hi Dou, On Tue, 13 Sep 2016 14:56:15 +0800 Dou Liyang wrote: > Hi Alex, >=20 > I am Dou. > I am testing these patches. >=20 > At 09/01/2016 03:44 AM, Alex Williamson wrote: >=20 > [...] >=20 > >> + > >> + pcie_cap_deverr_init(pdev); > >> + return pcie_aer_init(pdev, pos, size); =20 > > > > pcie_aer_init() adds a v2 AER capability regardless of the version of > > the AER capability on the device. Is this expected? =20 >=20 > I think it is not good. >=20 > v2 defines a lot > > more bits in various registers than v1, so are we simply hoping that > > devices have reserved bits as zero like they're supposed to? =20 >=20 > I read the Capability Version in PCIe Spec. it says: >=20 > Capability Version =E2=80=93 This field is a PCI-SIG defined version numb= er > that indicates the version of the Capability structure present. > OR > This field must be 2h if the End-End TLP Prefix Supported bit (see > Section 7.8.15) is Set and must be 1h or 2h otherwise. >=20 > In my option, I guess that the spec may mean that v1/v2 is used for the > End-End TLP Prefix Supported bit supported. >=20 > And I find that Kernel and QEmu don't do some special work for it. this > feature may not affect the registers in AER. >=20 > can you give me some AER bits that are defined in v2 ,but not in v1? There are quite a lot. In the uncorrectable error status register bits 21-25 are only defined for v2, same for the mask and severity for this register. Bits 14 & 15 are new for v2 in the correctable error status registers, status and mask. Bits 9-11 are new in the control register. > It's a > > bit strange that for an Intel 82576 NIC I get a v1 AER capability w/o > > aer=3Don and a v2 with. Thanks, > > =20 >=20 > Yes, I do the test, and get the same result for me. >=20 > I use the following device to test: >=20 > 06:00.0 Ethernet controller: Intel Corporation 82576 Gigabit Network > Connection (rev 01) >=20 > Firstly, In host, I use the command "lspci -vvv -s 06:00.0"=EF=BC=9A >=20 > [...] > Capabilities: [100 v1] Advanced Error Reporting > UESta: DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt- RxOF- > MalfTLP- ECRC- UnsupReq- ACSViol- > UEMsk: DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt- RxOF- > MalfTLP- ECRC- UnsupReq+ ACSViol- > UESvrt: DLP+ SDES- TLP- FCP+ CmpltTO- CmpltAbrt- UnxCmplt- RxOF+ > MalfTLP+ ECRC- UnsupReq- ACSViol- > CESta: RxErr- BadTLP- BadDLLP- Rollover- Timeout- NonFatalErr- > CEMsk: RxErr- BadTLP+ BadDLLP- Rollover- Timeout- NonFatalErr+ > AERCap: First Error Pointer: 12, GenCap- CGenEn- ChkCap- ChkEn- > [...] >=20 > Secondly, I pass through the device to the guest. In the guest: >=20 > [...] > Capabilities: [100 v2] Advanced Error Reporting > UESta: DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt- RxOF- > MalfTLP- ECRC- UnsupReq- ACSViol- > UEMsk: DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt- RxOF- > MalfTLP- ECRC- UnsupReq+ ACSViol- > UESvrt: DLP+ SDES- TLP- FCP+ CmpltTO- CmpltAbrt- UnxCmplt- RxOF+ > MalfTLP+ ECRC- UnsupReq- ACSViol- > CESta: RxErr- BadTLP- BadDLLP- Rollover- Timeout- NonFatalErr- > CEMsk: RxErr- BadTLP+ BadDLLP- Rollover- Timeout- NonFatalErr+ > AERCap: First Error Pointer: 12, GenCap- CGenEn- ChkCap- ChkEn- > [...] >=20 > They look the same, except the capabilities version. >=20 > Thirdly, I drop the patches and do the test. After I launch QEmu, > In guest: >=20 > [...] > Capabilities: [100 v1] Advanced Error Reporting > [...] >=20 > Fourthly, I modify the PCI_ERR_VER: #define PCI_ERR_VER 1. > In guest: > [...] > Capabilities: [100 v1] Advanced Error Reporting > [...] >=20 > They also have the same features. >=20 > At last, I think the v1/v2 is not influence on our AER function. > But, it is actually strange that we can get a v1 AER capability > w/o aer=3Don and a v2 with. > I suggest that we add a capability version parameter to extend the > pcie_aer_init(),or add a new pcie_aer_v1_init() for the devices with > v1 AER cap. Agreed. Thanks, Alex