Re: [PATCH v3 1/3] ACPI: APEI: GHES: Refactor Grace decoder helpers

On Fri Aug 7, 2026 at 8:38 AM CST, Ashok Raj wrote:
> External email: Use caution opening links or attachments
>
>
> On Tue, Aug 04, 2026 at 08:23:15PM +0800, Kai-Heng Feng wrote:
>> Split the Grace CPER processing into a separate decode step and a
>> print step so the parser can be exercised by KUnit without a live
>> ACPI device. Introduce ghes-nvidia.h to hold shared types that the
>> Vera decoder added in the next commit will also reference.
>>
>> Parse the Grace wire payload with __packed structs and
>> get_unaligned_le*() for multi-byte fields, matching the Vera
>> decoder style. Reject nvidia_ghes_grace_reg_pair() calls with a
>> non-zero register count and a NULL grace_regs pointer, and
>> rate-limit Grace decoder diagnostics.
>>
>> Signed-off-by: Kai-Heng Feng <[email protected]>
>
> Hi Kai,
>
>> ---
>> v3:
>>  - Use __packed + get_unaligned_le*() for Grace multi-byte fields,
>>    including register pairs (Shuai Xue).
>>  - Keep __counted_by / struct_size; reject NULL grace_regs when
>>    number_regs != 0; document the helper contract.
>>  - Rate-limit Grace decoder and notify error messages.
>> v2:
>>  - No change.
>> ---
>>  MAINTAINERS                     |   2 +-
>>  drivers/acpi/apei/ghes-nvidia.c | 160 ++++++++++++++++++++++----------
>>  drivers/acpi/apei/ghes-nvidia.h |  50 ++++++++++
>>  3 files changed, 160 insertions(+), 52 deletions(-)
>>  create mode 100644 drivers/acpi/apei/ghes-nvidia.h
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 5114e6db7307..aa9c2ae58ac5 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -19348,7 +19348,7 @@ NVIDIA GHES VENDOR CPER RECORD HANDLER
>>  M:   Kai-Heng Feng <[email protected]>
>>  L:   [email protected]
>>  S:   Maintained
>> -F:   drivers/acpi/apei/ghes-nvidia.c
>> +F:   drivers/acpi/apei/ghes-nvidia*
>>
>>  NVIDIA VRS RTC DRIVER
>>  M:   Shubhi Garg <[email protected]>
>> diff --git a/drivers/acpi/apei/ghes-nvidia.c b/drivers/acpi/apei/ghes-nvidia.c
>> index 597275d81de8..e7cc912344fd 100644
>> --- a/drivers/acpi/apei/ghes-nvidia.c
>> +++ b/drivers/acpi/apei/ghes-nvidia.c
>> @@ -9,13 +9,18 @@
>>  #include <linux/module.h>
>>  #include <linux/platform_device.h>
>>  #include <linux/types.h>
>> +#include <linux/unaligned.h>
>>  #include <linux/uuid.h>
>>  #include <acpi/ghes.h>
>
> One naive question, do these CPER decoding for vendor specific pieces
> belong in the kernel? Or you would manage them from user space?

I think so, since pcie-hisi-controller is already doing that. And I
think helpers like devm_ghes_register_vendor_record_notifier() is
designed for this kind of work.

Kai-Heng

>
>
>>
>> -static const guid_t nvidia_sec_guid =
>> +#include <kunit/visibility.h>
>> +#include "ghes-nvidia.h"
>> +
>> +static const guid_t nvidia_grace_sec_guid =
>>       GUID_INIT(0x6d5244f2, 0x2712, 0x11ec,
>>                 0xbe, 0xa7, 0xcb, 0x3f, 0xdb, 0x95, 0xc7, 0x86);
>>
>> +/* Grace CPER section wire layout (header without flexible register array). */
>>  struct cper_sec_nvidia {
>>       char    signature[16];
>>       __le16  error_type;
>> @@ -25,84 +30,137 @@ struct cper_sec_nvidia {
>>       u8      number_regs;
>>       u8      reserved;
>>       __le64  instance_base;
>> -     struct {
>> -             __le64  addr;
>> -             __le64  val;
>> -     } regs[] __counted_by(number_regs);
>> -};
>> +     struct nvidia_ghes_grace_reg regs[] __counted_by(number_regs);
>> +} __packed;
>> +
>
> [snip]
>
> Cheers,
> Ashok