On 10/27/2022 11:15 AM, 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) > +{ > + if (hest_hdr->type == ACPI_HEST_TYPE_AER_ROOT_PORT || > + hest_hdr->type == ACPI_HEST_TYPE_AER_ENDPOINT || > + hest_hdr->type == ACPI_HEST_TYPE_AER_BRIDGE) > + return true; > + return false; > +} > + > +static inline bool hest_match_type(struct acpi_hest_header *hest_hdr, > + struct pci_dev *dev) > +{ > + u16 hest_type = hest_hdr->type; > + u8 pcie_type = pci_pcie_type(dev); > + > + if ((hest_type == ACPI_HEST_TYPE_AER_ROOT_PORT && > + pcie_type == PCI_EXP_TYPE_ROOT_PORT) || > + (hest_type == ACPI_HEST_TYPE_AER_ENDPOINT && > + pcie_type == PCI_EXP_TYPE_ENDPOINT) || > + (hest_type == ACPI_HEST_TYPE_AER_BRIDGE && > + (pcie_type == PCI_EXP_TYPE_PCI_BRIDGE || pcie_type == PCI_EXP_TYPE_PCIE_BRIDGE))) > + return true; > + return false; > +} > + > +static inline bool hest_match_pci_devfn(struct acpi_hest_aer_common *p, > + struct pci_dev *pci) > +{ > + return ACPI_HEST_SEGMENT(p->bus) == pci_domain_nr(pci->bus) && > + ACPI_HEST_BUS(p->bus) == pci->bus->number && > + p->device == PCI_SLOT(pci->devfn) && > + p->function == PCI_FUNC(pci->devfn); > +} > + > +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)); > + else > + return false; > +} > > static int apei_hest_parse(apei_hest_func_t func, void *data) > { > @@ -124,6 +166,81 @@ static int apei_hest_parse(apei_hest_func_t func, void *data) > return 0; > } > > +/* > + * 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. > + */ > +int apei_hest_parse_aer(struct acpi_hest_header *hest_hdr, void *data) > +{ > + struct acpi_hest_parse_aer_info *info = data; > + struct acpi_hest_aer_endpoint *acpi_hest_aer_endpoint = NULL; > + struct acpi_hest_aer_root_port *acpi_hest_aer_root_port = NULL; > + struct acpi_hest_aer_for_bridge *acpi_hest_aer_for_bridge = NULL; > + > + if (!hest_source_is_pcie_aer(hest_hdr)) > + return 0; > + > + if (hest_hdr->type == ACPI_HEST_TYPE_AER_ROOT_PORT) { > + acpi_hest_aer_root_port = (struct acpi_hest_aer_root_port *)(hest_hdr + 1); > + if (acpi_hest_aer_root_port->flags & ACPI_HEST_GLOBAL) { > + if (hest_match_type(hest_hdr, info->pci_dev)) { > + info->acpi_hest_aer_root_port = acpi_hest_aer_root_port; > + info->hest_matched_with_dev = 1; > + } else > + info->hest_matched_with_dev = 0; > + } else { > + if (hest_match_pci(hest_hdr, > + (struct acpi_hest_aer_common *)acpi_hest_aer_root_port, > + info->pci_dev)) { > + info->acpi_hest_aer_root_port = acpi_hest_aer_root_port; > + info->hest_matched_with_dev = 1; > + } else > + info->hest_matched_with_dev = 0; > + } > + } else if (hest_hdr->type == ACPI_HEST_TYPE_AER_ENDPOINT) { > + acpi_hest_aer_endpoint = (struct acpi_hest_aer_endpoint *)(hest_hdr + 1); > + if (acpi_hest_aer_endpoint->flags & ACPI_HEST_GLOBAL) { > + if (hest_match_type(hest_hdr, info->pci_dev)) { > + info->acpi_hest_aer_endpoint = acpi_hest_aer_endpoint; > + info->hest_matched_with_dev = 1; > + } else > + info->hest_matched_with_dev = 0; > + } else { > + if (hest_match_pci(hest_hdr, > + (struct acpi_hest_aer_common *)acpi_hest_aer_endpoint, > + info->pci_dev)) { > + info->acpi_hest_aer_endpoint = acpi_hest_aer_endpoint; > + info->hest_matched_with_dev = 1; > + } else > + info->hest_matched_with_dev = 0; > + } > + } else if (hest_hdr->type == ACPI_HEST_TYPE_AER_BRIDGE) { > + acpi_hest_aer_for_bridge = > + (struct acpi_hest_aer_for_bridge *)(hest_hdr + 1); > + if (acpi_hest_aer_for_bridge->flags & ACPI_HEST_GLOBAL) { > + if (hest_match_type(hest_hdr, info->pci_dev)) { > + info->acpi_hest_aer_for_bridge = acpi_hest_aer_for_bridge; > + info->hest_matched_with_dev = 1; > + } else > + info->hest_matched_with_dev = 0; > + } else { > + if (hest_match_pci(hest_hdr, > + (struct acpi_hest_aer_common *)acpi_hest_aer_for_bridge, > + info->pci_dev)) { > + info->acpi_hest_aer_for_bridge = acpi_hest_aer_for_bridge; > + info->hest_matched_with_dev = 1; > + } else > + info->hest_matched_with_dev = 0; > + } > + } > + return info->hest_matched_with_dev; > +} Hi Leo, What do you think of that we could use a switch structure to instead these hest_source_is_pcie_aer() and if-else. thanks Ming > + > /* > * Check if firmware advertises firmware first mode. We need FF bit to be set > * along with a set of MC banks which work in FF mode. > diff --git a/include/acpi/actbl1.h b/include/acpi/actbl1.h > index 15c78678c5d3..7f52035512b2 100644 > --- a/include/acpi/actbl1.h > +++ b/include/acpi/actbl1.h > @@ -1385,6 +1385,75 @@ struct acpi_hest_aer_bridge { > u32 advanced_capabilities2; > }; > > +struct acpi_hest_parse_aer_info { > + struct pci_dev *pci_dev; > + int hest_matched_with_dev; > + struct acpi_hest_aer_endpoint *acpi_hest_aer_endpoint; > + struct acpi_hest_aer_root_port *acpi_hest_aer_root_port; > + struct acpi_hest_aer_for_bridge *acpi_hest_aer_for_bridge; > +}; > + > +/* HEST Sub-structure for PCIE EndPoint Structure (6) */ > + > +struct acpi_hest_aer_root_port { > + u16 reserved1; > + u8 flags; > + u8 enabled; > + u32 records_to_preallocate; > + u32 max_sections_per_record; > + u32 bus; /* Bus and Segment numbers */ > + u16 device; > + u16 function; > + u16 device_control; > + u16 reserved2; > + u32 uncorrectable_mask; > + u32 uncorrectable_severity; > + u32 correctable_mask; > + u32 advanced_capabilities; > + u32 root_error_command; > +}; > + > +/* HEST Sub-structure for PCIE EndPoint Structure (7) */ > + > +struct acpi_hest_aer_endpoint { > + u16 reserved1; > + u8 flags; > + u8 enabled; > + u32 records_to_preallocate; > + u32 max_sections_per_record; > + u32 bus; /* Bus and Segment numbers */ > + u16 device; > + u16 function; > + u16 device_control; > + u16 reserved2; > + u32 uncorrectable_mask; > + u32 uncorrectable_severity; > + u32 correctable_mask; > + u32 advanced_capabilities; > +}; > + > +/* HEST Sub-structure for PCIE/PCI Bridge Structure (8) */ > + > +struct acpi_hest_aer_for_bridge { > + u16 reserved1; > + u8 flags; > + u8 enabled; > + u32 records_to_preallocate; > + u32 max_sections_per_record; > + u32 bus; > + u16 device; > + u16 function; > + u16 device_control; > + u16 reserved2; > + u32 uncorrectable_mask; > + u32 uncorrectable_severity; > + u32 correctable_mask; > + u32 advanced_capabilities; > + u32 uncorrectable_mask2; > + u32 uncorrectable_severity2; > + u32 advanced_capabilities2; > +}; > + > /* 9: Generic Hardware Error Source */ > > struct acpi_hest_generic { > diff --git a/include/acpi/apei.h b/include/acpi/apei.h > index dc60f7db5524..8a0b2b9edbaf 100644 > --- a/include/acpi/apei.h > +++ b/include/acpi/apei.h > @@ -33,10 +33,17 @@ void __init acpi_ghes_init(void); > static inline void acpi_ghes_init(void) { } > #endif > > +typedef int (*apei_hest_func_t)(struct acpi_hest_header *hest_hdr, void *data); > + > #ifdef CONFIG_ACPI_APEI > void __init acpi_hest_init(void); > +int apei_hest_parse_aer(struct acpi_hest_header *hest_hdr, void *data); > #else > static inline void acpi_hest_init(void) { } > +static inline int apei_hest_parse_aer(struct acpi_hest_header *hest_hdr, void *data) > +{ > + return -EINVAL; > +} > #endif > > int erst_write(const struct cper_record_header *record);