Re: [RFC PATCH 00/11] soc: mediatek: add a DRAM ECC error handling driver

[Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>]

On Mon, 2 Nov 2026 17:23:22 +0800
santino.wang <santino.wang@mediatek.com> wrote:

> From: Santino Wang <santino.wang@mediatek.com>
> 
> On the MediaTek mt8992 SoC the DRAM controller (DRAMC) raises an interrupt for
> every corrected (CE) and uncorrectable (UCE) ECC error. The interrupt is
> owned by the secure firmware (Arm Trusted Firmware at EL3). It reads the
> DRAMC error registers, applies the error containment policy and the CE
> storm protection, and hands the error to the OS firmware-first as an ACPI
> GHES record in a buffer that UEFI describes in the HEST. The OS is
> notified with an edge interrupt that the DSDT delivers through a Generic
> Event Device and a Hardware Error Device (GSIV notification).

So far this all sounds standard for asynchronous error reporting.
I'm assuming you have a something else for synchronous consumption.
From a Linux side of things we just care that error record shows up
and is signaled via GSIV (with normal Generic Event Device and Hardware
Error Device in the DSDT I guess)

> 
> The record carries a standard Platform Memory Error section and a
> MediaTek vendor section with the raw DRAMC error registers.

I'd like some details here on what those raw registers are bringing
to the OS wrt to handling these errors. 

What extra info?  I see you have a subchannel in there, but Memory
Error Section 2 brings that.  So why isn't this being reported
as a standard CPER?

> This series
> adds the driver that consumes the vendor section through the existing
> ghes_register_vendor_record_notifier() interface. No change to the GHES
> core or to arm64 code is needed.

Given a lot here doesn't sound mediatek specific it feels like it
shouldn't necessarily be in the vendor defined path.  More discussion
of why you have gone that way would be help.

> 
> The driver
>  - decodes the most severe error of a record and keeps counters, also
>    across boots in EFI variables,

Interesting.  How does this compare with just making it a userspace problem?
Perhaps you can't do that for some reason but it would take a lot to
convince me that it makes sense to have an upstream solution that relies
on custom EFI variable path.

>  - isolates pages with memory_failure(): immediately for a recoverable
>    UCE on memory that Linux manages, and for a page whose corrected
>    error count reaches a threshold (leaky bucket); memory owned by a
>    device is only reported to its owner,

This sounds standard, so I'm missing why you need a vendor driver for it.
Please provides some more info.

>  - lets in-kernel clients and user space (generic netlink, tracepoints)
>    follow the events,

Why your own netlink for this?  These are memory errors so I'd expect
existing paths to be sufficient, or perhaps there to be a need to
extend those as Pierre is doing with the Memory Error Section 2 series.

>  - exposes the containment policy, the storm protection and per range
>    severity overrides in sysfs and pushes them to the secure firmware
>    through a vendor SMC call,

The pushing of policy down to firmware is possibly something that needs
a custom driver.  I need to think a bit more on that and check a few things.
Input from Borislav or Tony might help here.

>  - after a fatal UCE (GHES panic and reset), finds the error in the BERT
>    of the next boot and reports it through the mc_event tracepoint.

If this were using the standard records that should just work. What
is missing that you need?

> 
> The driver depends on firmware that is not part of this series: the
> secure firmware provides the SMC interface used by patch 2 and writes the
> vendor section decoded by patch 4. The firmware side (the TF-A and
> edk2-platforms changes for the mt8992 SoC, plus the preloader that keeps
> the error across the reset) is being developed in the vendor's BSP trees
> and is not published yet.
> 
> Open question for this RFC: is drivers/soc/mediatek the right place for a
> driver that is built on GHES vendor records and a vendor SMC interface, or
> would the maintainers prefer another location?

You already got a reply on that. Almost nothing should ever go in drivers/soc
and definitely not something RAS related. +CC linux-edac, Borislav and Tony.

Anyhow, always interesting to discuss expanding what we do for
memory error handling, so I'll be interested to see what other feedback
this series gets.

Thanks,

Jonathan

> 
> Testing: each patch builds with the arm64 defconfig plus
> CONFIG_MTK_DRAM_ECC=m on linux-next 20261030 (W=1, no warnings).
> The series has been tested pre-silicon on a Palladium emulation platform.
> 
> Santino Wang (11):
>   soc: mediatek: dram-ecc: add the DRAM ECC driver skeleton
>   soc: mediatek: dram-ecc: add the SMC interface to the secure firmware
>   soc: mediatek: dram-ecc: add tracepoints
>   soc: mediatek: dram-ecc: handle GHES vendor records from the firmware
>   soc: mediatek: dram-ecc: add error counters and the address lists
>   soc: mediatek: dram-ecc: keep error counters across boots
>   soc: mediatek: dram-ecc: add containment policy and storm protection
>   soc: mediatek: dram-ecc: add the corrected error leaky bucket
>   soc: mediatek: dram-ecc: add a generic netlink event interface
>   soc: mediatek: dram-ecc: report an uncorrectable error of the previous
>     boot
>   Documentation: ABI: add the sysfs attributes of the MediaTek DRAM ECC
>     driver
> 
>  .../ABI/testing/sysfs-class-mtk-dram-ecc      | 182 +++++++++
>  MAINTAINERS                                   |  10 +
>  drivers/soc/mediatek/Kconfig                  |   2 +
>  drivers/soc/mediatek/Makefile                 |   1 +
>  drivers/soc/mediatek/dram-ecc/Kconfig         |  19 +
>  drivers/soc/mediatek/dram-ecc/Makefile        |   3 +
>  drivers/soc/mediatek/dram-ecc/core/Makefile   |  20 +
>  .../soc/mediatek/dram-ecc/core/addresses.c    |  88 ++++
>  .../soc/mediatek/dram-ecc/core/addresses.h    |  13 +
>  .../soc/mediatek/dram-ecc/core/bert_check.c   | 240 +++++++++++
>  .../soc/mediatek/dram-ecc/core/bert_check.h   |  12 +
>  drivers/soc/mediatek/dram-ecc/core/counters.c | 336 ++++++++++++++++
>  drivers/soc/mediatek/dram-ecc/core/counters.h |  29 ++
>  .../mediatek/dram-ecc/core/ghes_notifier.c    | 377 ++++++++++++++++++
>  .../mediatek/dram-ecc/core/ghes_notifier.h    |  22 +
>  drivers/soc/mediatek/dram-ecc/core/main.c     | 113 ++++++
>  .../soc/mediatek/dram-ecc/core/mtk_dram_ecc.h |  27 ++
>  .../dram-ecc/core/mtk_dram_ecc_event.h        |  47 +++
>  drivers/soc/mediatek/dram-ecc/core/netlink.c  |  83 ++++
>  drivers/soc/mediatek/dram-ecc/core/netlink.h  |  14 +
>  .../dram-ecc/core/policy/address_range.c      | 120 ++++++
>  .../dram-ecc/core/policy/containment.c        | 107 +++++
>  .../dram-ecc/core/policy/leaky_bucket.c       | 277 +++++++++++++
>  .../mediatek/dram-ecc/core/policy/policy.c    |  57 +++
>  .../mediatek/dram-ecc/core/policy/policy.h    |  23 ++
>  .../dram-ecc/core/policy/storm_protection.c   | 137 +++++++
>  drivers/soc/mediatek/dram-ecc/core/smc.c      | 137 +++++++
>  drivers/soc/mediatek/dram-ecc/core/smc.h      |  55 +++
>  drivers/soc/mediatek/dram-ecc/core/trace.c    |   6 +
>  drivers/soc/mediatek/dram-ecc/core/trace.h    | 119 ++++++
>  include/uapi/linux/mtk_dram_ecc_netlink.h     |  44 ++
>  31 files changed, 2720 insertions(+)
>  create mode 100644 Documentation/ABI/testing/sysfs-class-mtk-dram-ecc
>  create mode 100644 drivers/soc/mediatek/dram-ecc/Kconfig
>  create mode 100644 drivers/soc/mediatek/dram-ecc/Makefile
>  create mode 100644 drivers/soc/mediatek/dram-ecc/core/Makefile
>  create mode 100644 drivers/soc/mediatek/dram-ecc/core/addresses.c
>  create mode 100644 drivers/soc/mediatek/dram-ecc/core/addresses.h
>  create mode 100644 drivers/soc/mediatek/dram-ecc/core/bert_check.c
>  create mode 100644 drivers/soc/mediatek/dram-ecc/core/bert_check.h
>  create mode 100644 drivers/soc/mediatek/dram-ecc/core/counters.c
>  create mode 100644 drivers/soc/mediatek/dram-ecc/core/counters.h
>  create mode 100644 drivers/soc/mediatek/dram-ecc/core/ghes_notifier.c
>  create mode 100644 drivers/soc/mediatek/dram-ecc/core/ghes_notifier.h
>  create mode 100644 drivers/soc/mediatek/dram-ecc/core/main.c
>  create mode 100644 drivers/soc/mediatek/dram-ecc/core/mtk_dram_ecc.h
>  create mode 100644 drivers/soc/mediatek/dram-ecc/core/mtk_dram_ecc_event.h
>  create mode 100644 drivers/soc/mediatek/dram-ecc/core/netlink.c
>  create mode 100644 drivers/soc/mediatek/dram-ecc/core/netlink.h
>  create mode 100644 drivers/soc/mediatek/dram-ecc/core/policy/address_range.c
>  create mode 100644 drivers/soc/mediatek/dram-ecc/core/policy/containment.c
>  create mode 100644 drivers/soc/mediatek/dram-ecc/core/policy/leaky_bucket.c
>  create mode 100644 drivers/soc/mediatek/dram-ecc/core/policy/policy.c
>  create mode 100644 drivers/soc/mediatek/dram-ecc/core/policy/policy.h
>  create mode 100644 drivers/soc/mediatek/dram-ecc/core/policy/storm_protection.c
>  create mode 100644 drivers/soc/mediatek/dram-ecc/core/smc.c
>  create mode 100644 drivers/soc/mediatek/dram-ecc/core/smc.h
>  create mode 100644 drivers/soc/mediatek/dram-ecc/core/trace.c
>  create mode 100644 drivers/soc/mediatek/dram-ecc/core/trace.h
>  create mode 100644 include/uapi/linux/mtk_dram_ecc_netlink.h
>