Re: [PATCH 11/12] ras: add DeviceTree estatus provider driver

[Ahmed Tiba <ahmed.tiba@arm.com>]

On Thu, 18 Dec 2025 03:19:17PM +0000, Will Deacon wrote:
> On Thu, Dec 18, 2025 at 01:42:47PM +0000, Ahmed Tiba wrote:
>> On Thu, 18 Dec 2025 12:13:25PM +0000, Will Deacon wrote:
>> >> Introduce a platform driver that maps the CPER status block described
>> >> in DeviceTree, feeds it into the estatus core and handles either IRQ- or
>> >> poll-driven notifications. Arm64 gains a FIX_ESTATUS_IRQ slot so the
>> >> driver can safely map the shared buffer while copying records.
>> >>
>> >> Signed-off-by: Ahmed Tiba <ahmed.tiba@arm.com>
>> >> ---
>> >>  MAINTAINERS                     |   1 +
>> >>  arch/arm64/include/asm/fixmap.h |   5 +
>> >>  drivers/ras/Kconfig             |  14 ++
>> >>  drivers/ras/Makefile            |   1 +
>> >>  drivers/ras/estatus-dt.c        | 318 ++++++++++++++++++++++++++++++++
>> >>  include/linux/estatus.h         |   3 +-
>> >>  6 files changed, 341 insertions(+), 1 deletion(-)
>> >>  create mode 100644 drivers/ras/estatus-dt.c
>> >>
>> >> diff --git a/MAINTAINERS b/MAINTAINERS
>> >> index 6b2ef2ddc0c7..5567d5e82053 100644
>> >> --- a/MAINTAINERS
>> >> +++ b/MAINTAINERS
>> >> @@ -21761,6 +21761,7 @@ RAS ERROR STATUS
>> >>  M:   Ahmed Tiba <ahmed.tiba@arm.com>
>> >>  S:   Maintained
>> >>  F:   Documentation/devicetree/bindings/ras/arm,ras-ffh.yaml
>> >> +F:   drivers/ras/estatus-dt.c
>> >>  F:   drivers/firmware/efi/estatus.c
>> >>  F:   include/linux/estatus.h
>> >>
>> >> diff --git a/arch/arm64/include/asm/fixmap.h b/arch/arm64/include/asm/fixmap.h
>> >> index 65555284446e..85ffba87bab9 100644
>> >> --- a/arch/arm64/include/asm/fixmap.h
>> >> +++ b/arch/arm64/include/asm/fixmap.h
>> >> @@ -64,6 +64,11 @@ enum fixed_addresses {
>> >>  #endif
>> >>  #endif /* CONFIG_ACPI_APEI_GHES */
>> >>
>> >> +#ifdef CONFIG_RAS_ESTATUS_DT
>> >> +     /* Used for ESTATUS mapping from assorted contexts */
>> >> +     FIX_ESTATUS_IRQ,
>> >> +#endif /* CONFIG_RAS_ESTATUS_DT */
>> >
>> > Why do we need this in addition to the four existing GHES slots? The DT
>> > code doesn't use it and I was assuming that the ACPI code would continue
>> > to use the existing irq; is that not the case?
>>
>>
>> We still need a dedicated slot when only the DT provider is built.
>> All four GHES slots are defined as part of the ACPI implementation,
>> so they are not present in a DT-only configuration.
>>
>> The estatus core always requests a fixmap index from each provider
>> before copying a CPER record. As a result, the DT driver must supply
>> its own slot to return a valid enum value to satisfy the common code.
>
> Sorry, but I still don't follow this. The DT code doesn't use the fixmap,
> does it? It looks like it maps the buffer ahead of time using
> devm_ioremap_resource() and then the accessors don't use the fixmap
> index at all, hence the horrible '(void)fixmap_idx;' cast which presumably
> stops the compiler from complaining about an unused variable.

Correct. The current DT driver keeps the CPER buffer permanently mapped with
devm_ioremap_resource() and that (void)fixmap_idx; line is just silencing
the warning. I’ll fix that by dropping the permanent mapping and copying the
status block via the fixmap entry, so the DT implementation mirrors GHES. That
gets rid of the cast and makes FIX_ESTATUS_IRQ do real work.


Thanks,
Ahmed