<p dir="ltr">The patch looks fine to me, but not for the next release. So, I'll look to commit after the release in mid February.</p>
<p dir="ltr">-Kevin</p>
<div class="gmail_quote">On Feb 9, 2015 10:22 PM, "Chen Fan" <<a href="mailto:chen.fan.fnst@cn.fujitsu.com">chen.fan.fnst@cn.fujitsu.com</a>> wrote:<br type="attribution"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On 02/02/2015 10:44 PM, Bjorn Helgaas wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
On Sun, Feb 1, 2015 at 10:08 PM, Nishank Trivedi<br>
<<a href="mailto:nishank.trivedi@netapp.com" target="_blank">nishank.trivedi@netapp.com</a>> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
On 2/1/15 6:13 PM, Chen Fan wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
On 02/01/2015 07:01 AM, Kevin O'Connor wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
On Wed, Jan 28, 2015 at 04:05:13PM +0800, Chen Fan wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
For PCIe device support AER(Advanced Error Reporting), from the<br>
pcie spec 3.0 chapter 6.2.5, ERR_COR, ERR_NONFATAL, and ERR_FATAL<br>
can be forwarded from the secondary interface to the primary interface,<br>
only require the SERR# Enable bit in the Bridge Control register is set.<br>
<br>
and at the kernel side, we found only _HPP() method can enable<br>
SERR#, So here we want to turn on this bit.<br>
<br>
Signed-off-by: Chen Fan <<a href="mailto:chen.fan.fnst@cn.fujitsu.com" target="_blank">chen.fan.fnst@cn.fujitsu.com</a>><br>
---<br>
   src/fw/pciinit.c | 4 ++++<br>
   1 file changed, 4 insertions(+)<br>
<br>
diff --git a/src/fw/pciinit.c b/src/fw/pciinit.c<br>
index 34279a4..28ed1af 100644<br>
--- a/src/fw/pciinit.c<br>
+++ b/src/fw/pciinit.c<br>
@@ -310,6 +310,10 @@ static void pci_bios_init_device(struct<br>
pci_device *pci)<br>
       /* enable memory mappings */<br>
       pci_config_maskw(bdf, PCI_COMMAND, 0,<br>
                        PCI_COMMAND_IO | PCI_COMMAND_MEMORY |<br>
PCI_COMMAND_SERR);<br>
+    /* enable SERR# for forwording */<br>
+    if (pci->header_type & PCI_HEADER_TYPE_BRIDGE)<br>
+        pci_config_maskw(bdf, PCI_BRIDGE_CONTROL, 0,<br>
+                         PCI_BRIDGE_CTL_SERR);<br>
   }<br>
</blockquote>
Thanks for submitting your patch.<br>
<br>
I'm not that familiar with all the details of the PCI specification.<br>
Can you elaborate on what doesn't work if this patch is not present,<br>
or what does work or work better when present?  Is this associated<br>
with a QEMU feature or change?<br>
</blockquote>
With this patch,  the aer error inject feature for pcie device in QEMU<br>
will work . the hmp command line is called pcie_aer_inject_error.<br>
a endpoint device propagates error messages to guest that need the<br>
parent bridge type devices enable the SERR# in bridge control reigster<br>
from pcie spec 3.0 chapter 6.2.5, and currently, the register bit is<br>
not turned on, so the command pcie_aer_inject_error is not work.<br>
and if the patch is ok, could you check in this patch?<br>
</blockquote>
<br>
Patch looks fine to me.<br>
Certain PCI switches, like PEX87xx, provide mechanism to explicitly inject<br>
errors (unlike aer_inject tool where just interrupt is raised). However, if<br>
SERR bit is not enabled in the bridges, error is not percolated to root<br>
port, probably because kernel expects BIOS to enable this bit. CC'ing Bjorn<br>
who can correct if this incorrect understanding.<br>
</blockquote>
I think your understanding of PCI_BRIDGE_CTL_SERR is correct.  Linux<br>
doesn't really have formal expectations about how platforms should set<br>
PCI_BRIDGE_CTL_SERR.  It's possible that Linux should be more<br>
proactive about enabling it.  I think it's just an area that nobody<br>
has paid much attention to.<br>
</blockquote>
Hi Kevin,<br>
<br>
How do you think of this patch?<br>
<br>
Thanks,<br>
Chen<br>
<br>
<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Bjorn<br>
.<br>
<br>
</blockquote>
<br>
</blockquote></div>