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

Suzuki K Poulose posted 3 patches 5 months ago
There is a newer version of this series
[PATCH v2 3/3] arm64: acpi: Enable ACPI CCEL support
Posted by Suzuki K Poulose 5 months ago
Add support for ACPI CCEL by handling the EfiACPIMemoryNVS type memory.
As per UEFI specifications NVS memory is reserved for Firmware use even
after exiting boot services. Thus map the region as read-only.

Cc: Sami Mujawar <sami.mujawar@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Aneesh Kumar K.V <aneesh.kumar@kernel.org>
Cc: Steven Price <steven.price@arm.com>
Cc: Sudeep Holla <sudeep.holla@arm.com>
Cc: Gavin Shan <gshan@redhat.com>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
---
Changes since v1
 - Map NVS region as read-only, update comment to clarify that the region
   is reserved for firmware use.

---
 arch/arm64/kernel/acpi.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
index 4d529ff7ba51..93b70f48a51f 100644
--- a/arch/arm64/kernel/acpi.c
+++ b/arch/arm64/kernel/acpi.c
@@ -360,6 +360,17 @@ void __iomem *acpi_os_ioremap(acpi_physical_address phys, acpi_size size)
 			prot = PAGE_KERNEL_RO;
 			break;
 
+		case EFI_ACPI_MEMORY_NVS:
+			/*
+			 * ACPI NVS marks an area reserved for use by the
+			 * firmware, even after exiting the boot service.
+			 * This may be used by the firmware for sharing dynamic
+			 * tables/data (e.g., ACPI CCEL) with the OS. Map it
+			 * as read-only.
+			 */
+			prot = PAGE_KERNEL_RO;
+			break;
+
 		case EFI_ACPI_RECLAIM_MEMORY:
 			/*
 			 * ACPI reclaim memory is used to pass firmware tables
-- 
2.43.0
Re: [PATCH v2 3/3] arm64: acpi: Enable ACPI CCEL support
Posted by Will Deacon 4 months, 3 weeks ago
On Mon, Sep 08, 2025 at 11:35:19PM +0100, Suzuki K Poulose wrote:
> Add support for ACPI CCEL by handling the EfiACPIMemoryNVS type memory.
> As per UEFI specifications NVS memory is reserved for Firmware use even
> after exiting boot services. Thus map the region as read-only.
> 
> Cc: Sami Mujawar <sami.mujawar@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Aneesh Kumar K.V <aneesh.kumar@kernel.org>
> Cc: Steven Price <steven.price@arm.com>
> Cc: Sudeep Holla <sudeep.holla@arm.com>
> Cc: Gavin Shan <gshan@redhat.com>
> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> ---
> Changes since v1
>  - Map NVS region as read-only, update comment to clarify that the region
>    is reserved for firmware use.
> 
> ---
>  arch/arm64/kernel/acpi.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
> index 4d529ff7ba51..93b70f48a51f 100644
> --- a/arch/arm64/kernel/acpi.c
> +++ b/arch/arm64/kernel/acpi.c
> @@ -360,6 +360,17 @@ void __iomem *acpi_os_ioremap(acpi_physical_address phys, acpi_size size)
>  			prot = PAGE_KERNEL_RO;
>  			break;
>  
> +		case EFI_ACPI_MEMORY_NVS:
> +			/*
> +			 * ACPI NVS marks an area reserved for use by the
> +			 * firmware, even after exiting the boot service.
> +			 * This may be used by the firmware for sharing dynamic
> +			 * tables/data (e.g., ACPI CCEL) with the OS. Map it
> +			 * as read-only.
> +			 */
> +			prot = PAGE_KERNEL_RO;
> +			break;
> +

Shouldn't this be merged with the other case handling read-only mappings?
e.g. something like:

		switch (region->type) {
		case EFI_LOADER_CODE:
		case EFI_LOADER_DATA:
		case EFI_BOOT_SERVICES_CODE:
		case EFI_BOOT_SERVICES_DATA:
		case EFI_CONVENTIONAL_MEMORY:
		case EFI_PERSISTENT_MEMORY:
			if (memblock_is_map_memory(phys) ||
			    !memblock_is_region_memory(phys, size)) {
				pr_warn(FW_BUG "requested region covers kernel memory @ %pa\n", &phys);
				return NULL;
			}
			/*
			 * Mapping kernel memory is permitted if the region in
			 * question is covered by a single memblock with the
			 * NOMAP attribute set: this enables the use of ACPI
			 * table overrides passed via initramfs, which are
			 * reserved in memory using arch_reserve_mem_area()
			 * below. As this particular use case only requires
			 * read access, fall through to the R/O mapping case.
			 */
			fallthrough;

		case EFI_RUNTIME_SERVICES_CODE:
			/*
			 * This would be unusual, but not problematic per se,
			 * as long as we take care not to create a writable
			 * mapping for executable code.
			 */
			fallthrough;

		case EFI_ACPI_MEMORY_NVS:
			/*
			 * ACPI NVS marks an area reserved for use by the
			 * firmware, even after exiting the boot service.
			 * This may be used by the firmware for sharing dynamic
			 * tables/data (e.g., ACPI CCEL) with the OS. Map it
			 * as read-only.
			 */
			prot = PAGE_KERNEL_RO;
			break;


With that, I'm happy to pick up the series (let me know if you want me
to make the change above locally to save you a resend).

Will
Re: [PATCH v2 3/3] arm64: acpi: Enable ACPI CCEL support
Posted by Suzuki K Poulose 4 months, 3 weeks ago
Hi Will

On 18/09/2025 13:31, Will Deacon wrote:
> On Mon, Sep 08, 2025 at 11:35:19PM +0100, Suzuki K Poulose wrote:
>> Add support for ACPI CCEL by handling the EfiACPIMemoryNVS type memory.
>> As per UEFI specifications NVS memory is reserved for Firmware use even
>> after exiting boot services. Thus map the region as read-only.
>>
>> Cc: Sami Mujawar <sami.mujawar@arm.com>
>> Cc: Will Deacon <will@kernel.org>
>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>> Cc: Aneesh Kumar K.V <aneesh.kumar@kernel.org>
>> Cc: Steven Price <steven.price@arm.com>
>> Cc: Sudeep Holla <sudeep.holla@arm.com>
>> Cc: Gavin Shan <gshan@redhat.com>
>> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>> ---
>> Changes since v1
>>   - Map NVS region as read-only, update comment to clarify that the region
>>     is reserved for firmware use.
>>
>> ---
>>   arch/arm64/kernel/acpi.c | 11 +++++++++++
>>   1 file changed, 11 insertions(+)
>>
>> diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
>> index 4d529ff7ba51..93b70f48a51f 100644
>> --- a/arch/arm64/kernel/acpi.c
>> +++ b/arch/arm64/kernel/acpi.c
>> @@ -360,6 +360,17 @@ void __iomem *acpi_os_ioremap(acpi_physical_address phys, acpi_size size)
>>   			prot = PAGE_KERNEL_RO;
>>   			break;
>>   
>> +		case EFI_ACPI_MEMORY_NVS:
>> +			/*
>> +			 * ACPI NVS marks an area reserved for use by the
>> +			 * firmware, even after exiting the boot service.
>> +			 * This may be used by the firmware for sharing dynamic
>> +			 * tables/data (e.g., ACPI CCEL) with the OS. Map it
>> +			 * as read-only.
>> +			 */
>> +			prot = PAGE_KERNEL_RO;
>> +			break;
>> +
> 
> Shouldn't this be merged with the other case handling read-only mappings?
> e.g. something like:
> 

I thought about it, but went against it, to keep the code separate. But
surely this is fine. I will resend the series with the proposed change.

Suzuki

> 		switch (region->type) {
> 		case EFI_LOADER_CODE:
> 		case EFI_LOADER_DATA:
> 		case EFI_BOOT_SERVICES_CODE:
> 		case EFI_BOOT_SERVICES_DATA:
> 		case EFI_CONVENTIONAL_MEMORY:
> 		case EFI_PERSISTENT_MEMORY:
> 			if (memblock_is_map_memory(phys) ||
> 			    !memblock_is_region_memory(phys, size)) {
> 				pr_warn(FW_BUG "requested region covers kernel memory @ %pa\n", &phys);
> 				return NULL;
> 			}
> 			/*
> 			 * Mapping kernel memory is permitted if the region in
> 			 * question is covered by a single memblock with the
> 			 * NOMAP attribute set: this enables the use of ACPI
> 			 * table overrides passed via initramfs, which are
> 			 * reserved in memory using arch_reserve_mem_area()
> 			 * below. As this particular use case only requires
> 			 * read access, fall through to the R/O mapping case.
> 			 */
> 			fallthrough;
> 
> 		case EFI_RUNTIME_SERVICES_CODE:
> 			/*
> 			 * This would be unusual, but not problematic per se,
> 			 * as long as we take care not to create a writable
> 			 * mapping for executable code.
> 			 */
> 			fallthrough;
> 
> 		case EFI_ACPI_MEMORY_NVS:
> 			/*
> 			 * ACPI NVS marks an area reserved for use by the
> 			 * firmware, even after exiting the boot service.
> 			 * This may be used by the firmware for sharing dynamic
> 			 * tables/data (e.g., ACPI CCEL) with the OS. Map it
> 			 * as read-only.
> 			 */
> 			prot = PAGE_KERNEL_RO;
> 			break;
> 
> 
> With that, I'm happy to pick up the series (let me know if you want me
> to make the change above locally to save you a resend).
> 
> Will
Re: [PATCH v2 3/3] arm64: acpi: Enable ACPI CCEL support
Posted by Gavin Shan 4 months, 3 weeks ago
On 9/9/25 8:35 AM, Suzuki K Poulose wrote:
> Add support for ACPI CCEL by handling the EfiACPIMemoryNVS type memory.
> As per UEFI specifications NVS memory is reserved for Firmware use even
> after exiting boot services. Thus map the region as read-only.
> 
> Cc: Sami Mujawar <sami.mujawar@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Aneesh Kumar K.V <aneesh.kumar@kernel.org>
> Cc: Steven Price <steven.price@arm.com>
> Cc: Sudeep Holla <sudeep.holla@arm.com>
> Cc: Gavin Shan <gshan@redhat.com>
> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> ---
> Changes since v1
>   - Map NVS region as read-only, update comment to clarify that the region
>     is reserved for firmware use.
> 
> ---
>   arch/arm64/kernel/acpi.c | 11 +++++++++++
>   1 file changed, 11 insertions(+)
> 

Reviewed-by: Gavin Shan <gshan@redhat.com>