Re: [PATCH] PCI AER: Handle non-AER HEST error sources inaer_hest_parse()

From: Bjorn Helgaas
Date: Fri Dec 13 2013 - 17:16:30 EST


On Fri, Dec 13, 2013 at 08:23:16AM -0700, Betty Dall wrote:
> aer_hest_parse() could pass a non-AER HEST error source to the function
> hest_match_pci(). hest_match_pci() assumes that the HEST error source is
> type ACPI_HEST_TYPE_AER_ROOT_PORT, ACPI_HEST_TYPE_AER_ENDPOINT, or
> ACPI_HEST_TYPE_AER_BRIDGE. I have a platform that has some
> ACPI_HEST_TYPE_GENERIC error sources where hest_match_pci() was trying to
> access the structure as if it had the acpi_hest_aer_common fields.
>
> aer_hest_parse() is only ever interested in the AER type HEST error
> sources. Add a check on the type of the error souce at the beginning of
> aer_hest_parse().
>
> Signed-off-by: Betty Dall <betty.dall@xxxxxx>
> ---
>
> drivers/pci/pcie/aer/aerdrv_acpi.c | 4 ++++
> 1 files changed, 4 insertions(+), 0 deletions(-)
>
>
> diff --git a/drivers/pci/pcie/aer/aerdrv_acpi.c b/drivers/pci/pcie/aer/aerdrv_acpi.c
> index cf611ab..39186e3 100644
> --- a/drivers/pci/pcie/aer/aerdrv_acpi.c
> +++ b/drivers/pci/pcie/aer/aerdrv_acpi.c
> @@ -56,6 +56,10 @@ static int aer_hest_parse(struct acpi_hest_header *hest_hdr, void *data)
> struct acpi_hest_aer_common *p;
> int ff;
>
> + 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 0;
> p = (struct acpi_hest_aer_common *)(hest_hdr + 1);
> ff = !!(p->flags & ACPI_HEST_FIRMWARE_FIRST);
> if (p->flags & ACPI_HEST_GLOBAL) {

Hi Betty,

I propose the following tweaked version. Will this work for you?


PCI/AER: Ingore non-PCIe AER error sources in aer_hest_parse()

From: Betty Dall <betty.dall@xxxxxx>

aer_set_firmware_first() searches the HEST for an error source descriptor
matching the specified PCI device. It uses the apei_hest_parse() iterator
to call aer_hest_parse() for every descriptor in the HEST.

Previously, aer_hest_parse() incorrectly assumed every descriptor was for a
PCIe error source. This patch adds a check to avoid that error.

[bhelgaas: factor check into helper function, changelog]
Signed-off-by: Betty Dall <betty.dall@xxxxxx>
Signed-off-by: Bjorn Helgaas <bhelgaas@xxxxxxxxxx>
---
drivers/pci/pcie/aer/aerdrv_acpi.c | 12 ++++++++++++
1 file changed, 12 insertions(+)

diff --git a/drivers/pci/pcie/aer/aerdrv_acpi.c b/drivers/pci/pcie/aer/aerdrv_acpi.c
index cf611ab2193a..f670313479ee 100644
--- a/drivers/pci/pcie/aer/aerdrv_acpi.c
+++ b/drivers/pci/pcie/aer/aerdrv_acpi.c
@@ -50,12 +50,24 @@ struct aer_hest_parse_info {
int firmware_first;
};

+static int 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 1;
+ return 0;
+}
+
static int aer_hest_parse(struct acpi_hest_header *hest_hdr, void *data)
{
struct aer_hest_parse_info *info = data;
struct acpi_hest_aer_common *p;
int ff;

+ if (!hest_source_is_pcie_aer(hest_hdr))
+ return 0;
+
p = (struct acpi_hest_aer_common *)(hest_hdr + 1);
ff = !!(p->flags & ACPI_HEST_FIRMWARE_FIRST);
if (p->flags & ACPI_HEST_GLOBAL) {
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/