Re: [Qemu-devel] [RFC PATCH 2/4] pcie-aer: Fix command pcie_aer_inject_error is invalid

[Chen Fan]

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.
>
>
>  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 <address@hidden>
> > ---
> >   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,
after sweep the code again,  I think you are right.

And for pcie spec  6.2.5 chapter. I think we should add a check for
validating the "Fatal Error Reporting Enable" bit in Device Control register
or whether #SERR is enabled either.

Thanks,
Chen



> Thanks,
> Marcel
>
>
> >       if (pcie_aer_msg_is_uncor(msg)) {
> >           /* Received System Error */
>
> .