RE: [PATCH v13 1/2] ACPI / APEI: Add a notifier chain for unknown (vendor) CPER records

From: Shiju Jose
Date: Fri Jul 24 2020 - 05:00:45 EST


Hi Bjorn,

Thanks for reviewing.

>-----Original Message-----
>From: Bjorn Helgaas [mailto:helgaas@xxxxxxxxxx]
>Sent: 24 July 2020 00:21
>To: Shiju Jose <shiju.jose@xxxxxxxxxx>
>Cc: linux-acpi@xxxxxxxxxxxxxxx; linux-pci@xxxxxxxxxxxxxxx; linux-
>kernel@xxxxxxxxxxxxxxx; rjw@xxxxxxxxxxxxx; bp@xxxxxxxxx;
>james.morse@xxxxxxx; lenb@xxxxxxxxxx; tony.luck@xxxxxxxxx;
>dan.carpenter@xxxxxxxxxx; zhangliguang@xxxxxxxxxxxxxxxxx;
>andriy.shevchenko@xxxxxxxxxxxxxxx; Wangkefeng (OS Kernel Lab)
><wangkefeng.wang@xxxxxxxxxx>; jroedel@xxxxxxx; Linuxarm
><linuxarm@xxxxxxxxxx>; yangyicong <yangyicong@xxxxxxxxxx>; Jonathan
>Cameron <jonathan.cameron@xxxxxxxxxx>; tanxiaofei
><tanxiaofei@xxxxxxxxxx>
>Subject: Re: [PATCH v13 1/2] ACPI / APEI: Add a notifier chain for unknown
>(vendor) CPER records
>
>On Wed, Jul 22, 2020 at 11:39:51AM +0100, Shiju Jose wrote:
>> CPER records describing a firmware-first error are identified by GUID.
>> The ghes driver currently logs, but ignores any unknown CPER records.
>> This prevents describing errors that can't be represented by a
>> standard entry, that would otherwise allow a driver to recover from an
>error.
>> The UEFI spec calls these 'Non-standard Section Body' (N.2.3 of
>> version 2.8).
>
>> +#ifdef CONFIG_ACPI_APEI_GHES
>> +/**
>> + * ghes_register_vendor_record_notifier - register a notifier for
>> +vendor
>> + * records that the kernel would otherwise ignore.
>> + * @nb: pointer to the notifier_block structure of the event handler.
>> + *
>> + * return 0 : SUCCESS, non-zero : FAIL */ int
>> +ghes_register_vendor_record_notifier(struct notifier_block *nb);
>> +
>> +/**
>> + * ghes_unregister_vendor_record_notifier - unregister the previously
>> + * registered vendor record notifier.
>> + * @nb: pointer to the notifier_block structure of the vendor record
>handler.
>> + */
>> +void ghes_unregister_vendor_record_notifier(struct notifier_block
>> +*nb); #else static inline int
>> +ghes_register_vendor_record_notifier(struct notifier_block *nb) {
>> + return -ENODEV;
>> +}
>> +
>> +static inline void ghes_unregister_vendor_record_notifier(struct
>> +notifier_block *nb) { }
>
>If you made CONFIG_PCIE_HISI_ERR depend on CONFIG_ACPI_APEI_GHES,
>you'd be able to get rid of these stubs, wouldn't you? It doesn't look like
>there's any point in building pcie-hisi-error.c at all unless
>CONFIG_ACPI_APEI_GHES is enabled.
The stub is added because this interface is expected to use by the other drivers as well.
Some drivers may not want add the build depend on the CONFIG_ACPI_APEI_GHES
if the error reporting has less priority in the driver.
However we can add dependency on CONFIG_ACPI_APEI_GHES for building pcie-hisi-error.c.
>
>> +#endif
>> +
>> int ghes_estatus_pool_init(int num_ghes);
>>
>> /* From drivers/edac/ghes_edac.c */
>> --
>> 2.17.1
>>
>>

Thanks,
Shiju