Re: [PATCH 3/3] arm64: acpi: Enable ACPI CCEL support

[Suzuki K Poulose]

Hi Will

On 15/07/2025 15:58, Will Deacon wrote:
> On Tue, Jul 15, 2025 at 03:38:41PM +0100, Suzuki K Poulose wrote:
> > On 15/07/2025 14:56, Will Deacon wrote:
> > > On Fri, Jun 13, 2025 at 12:11:53PM +0100, Suzuki K Poulose wrote:
> > > > ACPI CCEL memory area is reported as Non-Volatile storage area. Map it as
> > > > PAGE_KERNEL.
> > >
> > > It would be helpful to have a citation for that. I've tried digging it
> > > out of the ACPI spec, but that pointed me to the Intel TDX site and I
> > > got lost in whitepapers :/
> >
> > https://uefi.org/specs/ACPI/6.5/05_ACPI_Software_Programming_Model.html#cc-event-log-acpi-table
>
> Thanks, but I still can't spot where it specifies the memory attributes.
> I think we should be able to refer to that as justification for ignoring
> the attributes specified by EFI.

Here is the description for the Efi memory types.

https://uefi.org/specs/UEFI/2.10/07_Services_Boot_Services.html#memory-type-usage-before-exitbootservices

From that I gather that OS must map the area as read only.

> > > > Cc: Sami Mujawar <sami.mujawar@xxxxxxx>
> > > > Cc: Will Deacon <will@xxxxxxxxxx>
> > > > Cc: Catalin Marinas <catalin.marinas@xxxxxxx>
> > > > Cc: "Aneesh Kumar K.V" <aneesh.kumar@xxxxxxxxxx>
> > > > Cc: Steven Price <steven.price@xxxxxxx>
> > > > Cc: Sudeep Holla <sudeep.holla@xxxxxxx>
> > > > Signed-off-by: Suzuki K Poulose <suzuki.poulose@xxxxxxx>
> > > > ---
> > > >    arch/arm64/kernel/acpi.c | 5 +++++
> > > >    1 file changed, 5 insertions(+)
> > > >
> > > > diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
> > > > index b9a66fc146c9..f52439d411a0 100644
> > > > --- a/arch/arm64/kernel/acpi.c
> > > > +++ b/arch/arm64/kernel/acpi.c
> > > > @@ -356,6 +356,11 @@ void __iomem *acpi_os_ioremap(acpi_physical_address phys, acpi_size size)
> > > >    			prot = PAGE_KERNEL_RO;
> > > >    			break;
> > > > +		case EFI_ACPI_MEMORY_NVS:
> > > > +			/* Non-volatile storage, required for CCEL */
> > > > +			prot = PAGE_KERNEL;
> > >
> > > By "storage" you just mean memory, right?
> >
> > Yep, it is just memory. I will drop the "NV storage" and simply mention,
> > CCEL.
>
> Thanks. Also, why don't we check against memblock_is_map_memory() for
> this, like we do for all the other types apart from
> EFI_RUNTIME_SERVICES_CODE?

As per the spec, the NVS type must be reserved and we do that in the
kernel. For the other types, you mentioned, they are usable by
general OS and in Linux we only reserve them, if they are have
WB attribute (See is_usable_memory()). Thus we do extra checks to
make sure, if it is in use and mark them as RO.

So, NVS is by default reserved and doesn't have to do extra checks.

That said, I will fix the prot to PAGE_KERNEL_RO to make sure
the OS doesn't write to it.

Thanks
Suzuki

> Will