From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56399) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YDs83-0002fP-KZ for qemu-devel@nongnu.org; Wed, 21 Jan 2015 05:03:17 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YDs80-0004Ll-1k for qemu-devel@nongnu.org; Wed, 21 Jan 2015 05:03:15 -0500 Received: from [59.151.112.132] (port=15234 helo=heian.cn.fujitsu.com) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YDs7y-0004E2-PW for qemu-devel@nongnu.org; Wed, 21 Jan 2015 05:03:11 -0500 Message-ID: <54BF77DF.9030307@cn.fujitsu.com> Date: Wed, 21 Jan 2015 17:56:47 +0800 From: Chen Fan MIME-Version: 1.0 References: <54B3D280.90205@gmail.com> <54B8C416.5050701@cn.fujitsu.com> In-Reply-To: <54B8C416.5050701@cn.fujitsu.com> Content-Type: multipart/alternative; boundary="------------050102000305090400090001" Subject: Re: [Qemu-devel] [RFC PATCH 2/4] pcie-aer: Fix command pcie_aer_inject_error is invalid List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: marcel@redhat.com, qemu-devel@nongnu.org Cc: Alex Williamson --------------050102000305090400090001 Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit On 01/16/2015 03:56 PM, Chen Fan wrote: > > On 01/12/2015 09:56 PM, Marcel Apfelbaum wrote: >> On 01/12/2015 05:04 AM, Chen Fan wrote: >>> in spec "PCI Express 3.0" section 6.2.6 Figure 6-3 virtual bridge part, >>> the flowchart showing tell us SERR# enable at Bridge Control register >>> associate with system error at Secondary Status register can send error >>> message. but bridge_control from dev->config is NULL, and SERR# was set >>> in dev->wmask in pcie_aer_init() >> wmask denotes the register bits that can be written by the guest. >> >> If you are referring to: >> pci_word_test_and_set_mask(dev->wmask + PCI_BRIDGE_CONTROL, >> PCI_BRIDGE_CTL_SERR); >> that means that the OS *is able* to turn on/off SERR forwarding. > Hi marcel, > > I saw the OS that turn on SERR# is to via evaluate _HPP (Hot Plug > Parameters) method in ACPI. > is it the only way to turn on SERR#? I saw there was one option called*//*PCI SERR# Generation **searched from web pages in firmware on hardware****. Do it turn on the SERR#? if so, we can enable it in seabios. Thanks, Chen > > Thanks, > Chen > >> >> >> which was implemented by root port and >>> swith devices, so we should add wmask (for w/r) bit set for bridge >>> control. >>> we can user command like: >>> qemu_system_x86_64: >>> -device ioh3420,bus=pcie.0,addr=1c.0,multifunction=on,port=1,id=bridge1 >>> -device x3130-upstream,bus=bridge1,id=up.1,addr=00.0 >>> -device >>> xio3130-downstream,bus=up.1,id=down.1,port=1,addr=00.0,chassis=5 >>> >>> (qemu) pcie_aer_inject_error net0 POISON_TLP >>> >>> after that, >>> guest can output the error message. >>> >>> Signed-off-by: Chen Fan >>> --- >>> hw/pci/pcie_aer.c | 3 ++- >>> 1 file changed, 2 insertions(+), 1 deletion(-) >>> >>> diff --git a/hw/pci/pcie_aer.c b/hw/pci/pcie_aer.c >>> index 7ca077a..571dc92 100644 >>> --- a/hw/pci/pcie_aer.c >>> +++ b/hw/pci/pcie_aer.c >>> @@ -231,7 +231,8 @@ pcie_aer_msg_alldev(PCIDevice *dev, const >>> PCIEAERMsg *msg) >>> */ >>> static bool pcie_aer_msg_vbridge(PCIDevice *dev, const PCIEAERMsg >>> *msg) >>> { >>> - uint16_t bridge_control = pci_get_word(dev->config + >>> PCI_BRIDGE_CONTROL); >> Here we check if the Guest OS/firmware actually turned the #SERR >> forwarding on. >> >>> + uint16_t bridge_control = pci_get_word(dev->config + >>> PCI_BRIDGE_CONTROL) | >>> + pci_get_word(dev->wmask + >>> PCI_BRIDGE_CONTROL); >> I don't think that this check is correct given the above comments. >> Please correct me if I mislead you, >> Thanks, >> Marcel >> >> >>> >>> if (pcie_aer_msg_is_uncor(msg)) { >>> /* Received System Error */ >>> >> >> . >> > > > --------------050102000305090400090001 Content-Type: text/html; charset="windows-1252" Content-Transfer-Encoding: 8bit
On 01/16/2015 03:56 PM, Chen Fan wrote:

On 01/12/2015 09:56 PM, Marcel Apfelbaum wrote:
On 01/12/2015 05:04 AM, Chen Fan wrote:
in spec "PCI Express 3.0" section 6.2.6 Figure 6-3 virtual bridge part,
the flowchart showing tell us SERR# enable at Bridge Control register
associate with system error at Secondary Status register can send error
message. but bridge_control from dev->config is NULL, and SERR# was set
in dev->wmask in pcie_aer_init()
wmask denotes the register bits that can be written by the guest.

If you are referring to:
       pci_word_test_and_set_mask(dev->wmask + PCI_BRIDGE_CONTROL,
                                  PCI_BRIDGE_CTL_SERR);
that means that the OS *is able* to turn on/off SERR forwarding.
Hi marcel,

I saw the OS that turn on SERR# is to via evaluate _HPP (Hot Plug Parameters) method in ACPI.
is it the only way to turn on SERR#?

I saw there was one option called PCI SERR# Generation searched from web pages in firmware on hardware.
Do it turn on the SERR#? if so, we can enable it in seabios.

Thanks,
Chen


Thanks,
Chen



 which was implemented by root port and
swith devices, so we should add wmask (for w/r) bit set for bridge control.
we can user command like:
qemu_system_x86_64:
-device ioh3420,bus=pcie.0,addr=1c.0,multifunction=on,port=1,id=bridge1
-device x3130-upstream,bus=bridge1,id=up.1,addr=00.0
-device xio3130-downstream,bus=up.1,id=down.1,port=1,addr=00.0,chassis=5

(qemu) pcie_aer_inject_error net0 POISON_TLP

after that,
guest can output the error message.

Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
---
  hw/pci/pcie_aer.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/pci/pcie_aer.c b/hw/pci/pcie_aer.c
index 7ca077a..571dc92 100644
--- a/hw/pci/pcie_aer.c
+++ b/hw/pci/pcie_aer.c
@@ -231,7 +231,8 @@ pcie_aer_msg_alldev(PCIDevice *dev, const PCIEAERMsg *msg)
   */
  static bool pcie_aer_msg_vbridge(PCIDevice *dev, const PCIEAERMsg *msg)
  {
-    uint16_t bridge_control = pci_get_word(dev->config + PCI_BRIDGE_CONTROL);
Here we check if the Guest OS/firmware actually turned the #SERR forwarding on.

+    uint16_t bridge_control = pci_get_word(dev->config + PCI_BRIDGE_CONTROL) |
+                              pci_get_word(dev->wmask + PCI_BRIDGE_CONTROL);
I don't think that this check is correct given the above comments.
Please correct me if I mislead you,
Thanks,
Marcel



      if (pcie_aer_msg_is_uncor(msg)) {
          /* Received System Error */


.





--------------050102000305090400090001--