[Qemu-devel] Re: [PATCH v3 06/13] pcie/aer: helper functions for pcie aer capability.

[Isaku Yamahata]

On Wed, Sep 22, 2010 at 01:50:16PM +0200, Michael S. Tsirkin wrote:

> > +    }
> > +
> > +    /* capability & control */
> > +    if (ranges_overlap(addr, len, pos + PCI_ERR_CAP, 4)) {
> > +        uint32_t err_cap = pci_get_long(dev->config + pos + PCI_ERR_CAP);
> > +        if (!(err_cap & PCI_ERR_CAP_MHRE)) {
> > +            pcie_aer_log_clear_all_err(&dev->exp.aer_log);
> > +            pci_set_long(dev->w1cmask + pos + PCI_ERR_UNCOR_STATUS,
> > +                         PCI_ERR_UNC_SUPPORTED);
> > +        } else {
> > +            /* When multiple header recording is enabled, only the bit that
> > +             * first error pointer indicates is cleared.
> > +             * that is handled specifically.
> > +             */
> > +            pci_set_long(dev->w1cmask + pos + PCI_ERR_UNCOR_STATUS, 0);
> 
> This is wrong I think: you do not change mask on each write.
> Do it on setup.

The register behaves quite differently depending on whether
multiple header recording(MHR) is enabled or not.
With MHR disabled, the register is w1c. It indicates which errors
have occurred.
With MHR enabled, it behaves quite complexly. It reports errors in order
which had occurred.
Fro details, please refer to
6.2.4.2. Multiple Error Handling (Advanced Error Reporting Capability).


> > +static inline void pcie_aer_errmsg(PCIDevice *dev, const PCIE_AERErrMsg 
> > *msg)
> > +{
> > +    assert(pci_is_express(dev));
> > +    assert(dev->exp.aer_errmsg);
> > +    dev->exp.aer_errmsg(dev, msg);
> 
> Why do we want the indirection? Why not have users just call the function?

To handle error signaling uniformly.
Please see 
6.2.5. Sequence of Device Error Signaling and Logging Operations
and figure 6-2 and 6-3.
-- 
yamahata