[SeaBIOS] [PATCH] pci: enable SERR# for error forwording in bridge control register

[Kevin OConnor]

The patch looks fine to me, but not for the next release. So, I'll look to
commit after the release in mid February.

-Kevin
On Feb 9, 2015 10:22 PM, "Chen Fan" <chen.fan.fnst at cn.fujitsu.com> wrote:

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