On 2026-03-20 10:13, Jonathan Cameron wrote:
> External email: Use caution opening links or attachments
>
>
> On Thu, 19 Mar 2026 19:13:09 +0800
> Kai-Heng Feng <[email protected]> wrote:
>
> > Add support for decoding NVIDIA-specific CPER sections delivered via
> > the APEI GHES vendor record notifier chain. NVIDIA hardware generates
> > vendor-specific CPER sections containing error signatures and diagnostic
> > register dumps. This implementation registers a notifier_block with the
> > GHES vendor record notifier and decodes these sections, printing error
> > details via dev_info().
> >
> > The driver binds to ACPI device NVDA2012, present on NVIDIA server
> > platforms. The NVIDIA CPER section contains a fixed header with error
> > metadata (signature, error type, severity, socket) followed by
> > variable-length register address-value pairs for hardware diagnostics.
> >
> > This work is based on libcper [0].
> >
> > Example output:
> > nvidia-ghes NVDA2012:00: NVIDIA CPER section, error_data_length: 544
> > nvidia-ghes NVDA2012:00: signature: CMET-INFO
> > nvidia-ghes NVDA2012:00: error_type: 0
> > nvidia-ghes NVDA2012:00: error_instance: 0
> > nvidia-ghes NVDA2012:00: severity: 3
> > nvidia-ghes NVDA2012:00: socket: 0
> > nvidia-ghes NVDA2012:00: number_regs: 32
> > nvidia-ghes NVDA2012:00: instance_base: 0x0000000000000000
> > nvidia-ghes NVDA2012:00: register[0]: address=0x8000000100000000
> > value=0x0000000100000000
> >
> > [0] https://github.com/openbmc/libcper/commit/683e055061ce
> > Cc: Jonathan Cameron <[email protected]>
> > Cc: Shiju Jose <[email protected]>
> > Signed-off-by: Kai-Heng Feng <[email protected]>
> Only significant thing is around use of dev_err_probe().
>
> I'm surprised that didn't give you error messages in the log even on success.
>
> With that fixed (other stuff is all up to you).
> Reviewed-by: Jonathan Cameron <[email protected]>
>
>
> > apei-y := apei-base.o hest.o erst.o bert.o
> > diff --git a/drivers/acpi/apei/nvidia-ghes.c
> > b/drivers/acpi/apei/nvidia-ghes.c
> > new file mode 100644
> > index 000000000000..aa2e3a387b49
> > --- /dev/null
> > +++ b/drivers/acpi/apei/nvidia-ghes.c
>
> > +static void nvidia_ghes_print_error(struct device *dev,
> > + const struct cper_sec_nvidia *nvidia_err,
> > + size_t error_data_length, bool fatal)
> > +{
> > + const char *level = fatal ? KERN_ERR : KERN_INFO;
> > + size_t min_size;
> > + int i;
> ...
>
>
> > + * Validate that all registers fit within error_data_length.
> > + * Each register pair is two little-endian u64s.
> > + */
> > + min_size = struct_size(nvidia_err, regs, nvidia_err->number_regs);
> > + if (error_data_length < min_size) {
> > + dev_err(dev, "Invalid number_regs %u (section size %zu, need
> > %zu)\n",
> > + nvidia_err->number_regs, error_data_length, min_size);
> > + return;
> > + }
> > +
> > + for (i = 0; i < nvidia_err->number_regs; i++)
>
> Trivial but I'd take advantage of it now being acceptable (in general) to do
> for (int i = 0; i < ....)
Didn't know it's acceptable now. Will change.
>
> > + dev_printk(level, dev, "register[%d]: address=0x%016llx
> > value=0x%016llx\n",
> > + i, le64_to_cpu(nvidia_err->regs[i].addr),
> > + le64_to_cpu(nvidia_err->regs[i].val));
> > +}
>
> > +static int nvidia_ghes_probe(struct platform_device *pdev)
> > +{
> > + struct nvidia_ghes_private *priv;
> > +
> > + priv = devm_kmalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> > + if (!priv)
> > + return -ENOMEM;
> > +
> > + *priv = (struct nvidia_ghes_private) {
> > + .nb.notifier_call = nvidia_ghes_notify,
> > + .dev = &pdev->dev,
> > + };
> > +
> > + return dev_err_probe(&pdev->dev,
> > +
> > devm_ghes_register_vendor_record_notifier(&pdev->dev, &priv->nb),
> That's too not great for readability and dev_err_probe() should only be
> called on errors
> I'm fairly sure it doesn't have special handling for 0 so will call dev_err()
> or dev_warn()
> and print some stuff before saying 'no error'.
>
> int ret;
> ...
>
> ret = devm_ghes_register_vendor_record_notifier(&pdev->dev,
> &priv->nb);
> if (ret)
> return dev_err_probe(&pdev->dev,
> "Failed to register NVIDIA GHES vendor
> record notifier\n");
>
> return 0;
OK, will change.
>
>
>
> > + "Failed to register NVIDIA GHES vendor record
> > notifier\n");
> > +}
> > +
> > +static const struct acpi_device_id nvidia_ghes_acpi_match[] = {
> > + { "NVDA2012" },
>
> London Olympics :)
Michael Phelps did great :)
>
> > + { }
> > +};
> > +MODULE_DEVICE_TABLE(acpi, nvidia_ghes_acpi_match);
> > +
> > +static struct platform_driver nvidia_ghes_driver = {
> > + .driver = {
> > + .name = "nvidia-ghes",
> > + .acpi_match_table = nvidia_ghes_acpi_match,
> > + },
> > + .probe = nvidia_ghes_probe,
>
> I'd just not attempt to align the =
> static struct platform_driver nvidia_ghes_driver = {
> .driver = {
> .name = "nvidia-ghes",
> .acpi_match_table = nvidia_ghes_acpi_match,
> },
> .probe = nvidia_ghes_probe,
>
> There aren't enough of them to make it much of a readability improvement
> and doing this often results in unnecessary churn as a driver evolves.
> Also it's already broken!
OK, will change too.
>
> > +};
> > +module_platform_driver(nvidia_ghes_driver);
> > +
> > +MODULE_AUTHOR("Kai-Heng Feng <[email protected]>");
> > +MODULE_DESCRIPTION("NVIDIA GHES vendor CPER record handler");
> > +MODULE_LICENSE("GPL");
>
