Re: [PATCH 2/2] ACPI, APEI, PCIE AER, use general HEST table parsing in AER firmware_first setup

[Huang Ying <ying.huang@intel.com>]

On Tue, 2010-03-02 at 19:04 +0800, Hidetoshi Seto wrote:
> (2010/03/02 18:13), Huang Ying wrote:
> > On Tue, 2010-03-02 at 16:09 +0800, Hidetoshi Seto wrote:
> >> The aer_init() will be called for root ports, but not for end point
> >> devices or so on.  So please remain the firmware_first setup code in
> >> PCI core.  Otherwise endpoint drivers will get success on call of
> >> pci_enable_pcie_error_reporting() regardless of the firmware first.
> > 
> > Or we can call firmware_first setup code in
> > pci_enable_pcie_error_reporting(), because
> > 
> > 1. I think AER related code should be put in drivers/pci/pcie/aer
> > instead of PCI core or drivers/acpi, if it is possible.
> > 
> > 2. pci_setup_device is called so early, so that it is hard to do some
> > HEST related initialization (such as checking bad format) before it.
> 
> I understands the feeling, but before agreeing with your
> proposal, I'd like to have an answer of a question:
> 
>  - Is it necessary to setup the firmware_first flag
>    for an endpoint even if the endpoint's driver never
>    call pci_enable_pcie_error_reporting()?
> 
> According to the current implementation, there are no
> driver referring the firmware_first flag other than that
> it owns.  However I guess that the flag will be necessary
> for AER driver (i.e. aerdrv_core) in near future, because
> we can use the flag to determine whether the AER driver
> can check the device or not, when it is required to walk
> pci bus hierarchy to find an erroneous device.
> 
> For example, assume that there are 2 endpoints under a same
> root port.  One is (likely on-board) "firmware first" endpoint,
> with driver which does not call pci_enable_pcie_error_reporting()
> (because of no interest in AER, or just not implemented yet,
> anyway).  The other is (likely card seated on a slot) not
> firmware first, with better driver which can handle it's AER.
> If my understanding is correct and if everything goes well,
> errors on one should be reported via APEI while the other should
> be reported via AER driver.

Yes. I think this should be supported. How about something as follow?

struct pci_dev {
	...
	unsigned int __firmware_first:2;
	...
};

int pcie_aer_get_firmware_first(struct pci_dev *dev)
{
	if (!dev->__firmware_first)
		aer_set_firmware_first(dev);
	return dev->__firmware_first & 0x1;
}

Then we use pcie_aer_get_firmware_first() instead of dev->firmware_first
directly.

Best Regards,
Huang Ying