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 < ....) > + 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; > + "Failed to register NVIDIA GHES vendor record > notifier\n"); > +} > + > +static const struct acpi_device_id nvidia_ghes_acpi_match[] = { > + { "NVDA2012" }, London Olympics :) > + { } > +}; > +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! > +}; > +module_platform_driver(nvidia_ghes_driver); > + > +MODULE_AUTHOR("Kai-Heng Feng <[email protected]>"); > +MODULE_DESCRIPTION("NVIDIA GHES vendor CPER record handler"); > +MODULE_LICENSE("GPL");
