Re: [PATCH 1/5] ACPI/APEI: Add apei_hest_parse_aer()

[LeoLiuoc]

在 2022/10/28 6:07, Bjorn Helgaas 写道:
> On Thu, Oct 27, 2022 at 11:15:18AM +0800, LeoLiu-oc wrote:
> > From: leoliu-oc <leoliu-oc@xxxxxxxxxxx>
> >
> > apei_hest_parse_aer() is used to parse and record the PCI Express AER
> > Structure in the HEST Table.
> >
> > Signed-off-by: leoliu-oc <leoliu-oc@xxxxxxxxxxx>
> > ---
> >   drivers/acpi/apei/hest.c | 119 ++++++++++++++++++++++++++++++++++++++-
> >   include/acpi/actbl1.h    |  69 +++++++++++++++++++++++
> >   include/acpi/apei.h      |   7 +++
> >   3 files changed, 194 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/acpi/apei/hest.c b/drivers/acpi/apei/hest.c
> > index 6aef1ee5e1bd..0bfdc18758f5 100644
> > --- a/drivers/acpi/apei/hest.c
> > +++ b/drivers/acpi/apei/hest.c
> > @@ -25,6 +25,7 @@
> >   #include <linux/platform_device.h>
> >   #include <acpi/apei.h>
> >   #include <acpi/ghes.h>
> > +#include <linux/pci.h>
> >   
> >   #include "apei-internal.h"
> >   
> > @@ -86,7 +87,48 @@ static int hest_esrc_len(struct acpi_hest_header *hest_hdr)
> >   	return len;
> >   };
> >   
> > -typedef int (*apei_hest_func_t)(struct acpi_hest_header *hest_hdr, void *data);
> > +static inline bool hest_source_is_pcie_aer(struct acpi_hest_header *hest_hdr)
>
> Drop "inline" here and below.  This is not a performance path, so it's
> more clutter than it's worth.

Yes, I agree with it. Will fix it in the next version.

Thanks
leoliu-oc

> > +static inline bool hest_match_pci(struct acpi_hest_header *hest_hdr,
> > +		struct acpi_hest_aer_common *p, struct pci_dev *pci)
> > +{
> > +	if (hest_match_type(hest_hdr, pci))
> > +		return(hest_match_pci_devfn(p, pci));
>
> No need for parens around hest_match_pci_devfn().
>
> > +	else
>
> You can drop the else, too.

Yes, Agree with it. I will also fix it in the next version.

Thanks
leoliu-oc
> > +		return false;
> > +}
>
> > + * apei_hest_parse_aer - Find the AER structure in the HEST Table and
> > + * match it with the PCI device.
> > + *
> > + * @hest_hdr: To save the acpi aer error source in hest table
> > + *
> > + * Return 1 if the pci dev matched with the acpi aer error source in
> > + * hest table, else return 0.
>
> In comments and commit logs,
>
>    s/pci/PCI/
>    s/aer/AER/
>    s/acpi/ACPI/
>    s/hest/HEST/
>    s/HEST Table/HEST/ (since the "T" in "HEST" stands for "Table")
>
> > +/* HEST Sub-structure for PCIE EndPoint Structure (6) */
>
> PCIe Root Port, if I'm following this correctly.
>
> > +/* HEST Sub-structure for PCIE EndPoint Structure (7) */
>
> PCIe Endpoint.
>
> > +/* HEST Sub-structure for PCIE/PCI Bridge Structure (8) */
>
> PCIe/PCI-X Bridge

Got it, will modify it in the next version.

Thanks
leoliu-oc