2024-03-26 21:27:12

by John Allen

[permalink] [raw]
Subject: [PATCH 0/2] PRM handler direct call interface

Platform Runtime Mechanism (PRM) introduces a means for the AML
interpreter and OS drivers to invoke runtime handlers from platform
firmware in order to remove the need for certain classes of SMIs.
Further details can be seen in the PRM specification[1].

Future AMD platforms will implement a PRM module in firmware that will
include handlers for performing various types of address translation.
The address translation PRM module is documented in chapter 22 of the
publicly available "AMD Family 1Ah Models 00h–0Fh and Models 10h–1Fh
ACPI v6.5 Porting Guide"[2].

While the kernel currently has support for calling PRM handlers from the
AML interpreter, it does not support calling PRM handlers directly from
OS drivers. This series implements the direct call interface and uses it
for translating normalized addresses to system physical addresses.

Thanks,
John

[1]: https://uefi.org/sites/default/files/resources/Platform%20Runtime%20Mechanism%20-%20with%20legal%20notice.pdf
[2]: https://www.amd.com/content/dam/amd/en/documents/epyc-technical-docs/programmer-references/58088-0.75-pub.pdf

Tree: git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm
Base commit: 4cece764965020c22cff7665b18a012006359095

John Allen (2):
ACPI: PRM: Add PRM handler direct call support
ras/amd/atl: Translate normalized to system physical addresses using
PRM

drivers/acpi/prmt.c | 24 ++++++++++++++
drivers/ras/amd/atl/Makefile | 1 +
drivers/ras/amd/atl/internal.h | 2 ++
drivers/ras/amd/atl/prm.c | 58 ++++++++++++++++++++++++++++++++++
drivers/ras/amd/atl/umc.c | 5 +++
include/linux/prmt.h | 5 +++
6 files changed, 95 insertions(+)
create mode 100644 drivers/ras/amd/atl/prm.c

--
2.34.1



2024-03-26 21:27:45

by John Allen

[permalink] [raw]
Subject: [PATCH 2/2] RAS/AMD/ATL: Translate normalized to system physical addresses using PRM

Future AMD platforms will provide a UEFI PRM module that implements a
number of address translation PRM handlers. This will provide an
interface for the OS to call platform specific code without requiring
the use of SMM or other heavy firmware operations.

AMD Zen-based systems report memory error addresses through Machine
Check banks representing Unified Memory Controllers (UMCs) in the form
of UMC relative "normalized" addresses. A normalized address must be
converted to a system physical address to be usable by the OS.

Add support for the normalized to system physical address translation
PRM handler in the AMD Address Translation Library and prefer it over
native code if available. The GUID and parameter buffer structure are
specific to the normalized to system physical address handler provided
by the address translation PRM module included in future AMD systems.

The address translation PRM module is documented in chapter 22 of the
publicly available "AMD Family 1Ah Models 00h–0Fh and Models 10h–1Fh
ACPI v6.5 Porting Guide":
https://www.amd.com/content/dam/amd/en/documents/epyc-technical-docs/programmer-references/58088-0.75-pub.pdf

Signed-off-by: John Allen <[email protected]>
---
drivers/ras/amd/atl/Makefile | 1 +
drivers/ras/amd/atl/internal.h | 2 ++
drivers/ras/amd/atl/prm.c | 61 ++++++++++++++++++++++++++++++++++
drivers/ras/amd/atl/umc.c | 5 +++
4 files changed, 69 insertions(+)
create mode 100644 drivers/ras/amd/atl/prm.c

diff --git a/drivers/ras/amd/atl/Makefile b/drivers/ras/amd/atl/Makefile
index 4acd5f05bd9c..8f1afa793e3b 100644
--- a/drivers/ras/amd/atl/Makefile
+++ b/drivers/ras/amd/atl/Makefile
@@ -14,5 +14,6 @@ amd_atl-y += denormalize.o
amd_atl-y += map.o
amd_atl-y += system.o
amd_atl-y += umc.o
+amd_atl-y += prm.o

obj-$(CONFIG_AMD_ATL) += amd_atl.o
diff --git a/drivers/ras/amd/atl/internal.h b/drivers/ras/amd/atl/internal.h
index 5de69e0bb0f9..f739dcada126 100644
--- a/drivers/ras/amd/atl/internal.h
+++ b/drivers/ras/amd/atl/internal.h
@@ -234,6 +234,8 @@ int dehash_address(struct addr_ctx *ctx);
unsigned long norm_to_sys_addr(u8 socket_id, u8 die_id, u8 coh_st_inst_id, unsigned long addr);
unsigned long convert_umc_mca_addr_to_sys_addr(struct atl_err *err);

+unsigned long prm_umc_norm_to_sys_addr(u8 socket_id, u64 umc_bank_inst_id, unsigned long addr);
+
/*
* Make a gap in @data that is @num_bits long starting at @bit_num.
* e.g. data = 11111111'b
diff --git a/drivers/ras/amd/atl/prm.c b/drivers/ras/amd/atl/prm.c
new file mode 100644
index 000000000000..54a69e660eb5
--- /dev/null
+++ b/drivers/ras/amd/atl/prm.c
@@ -0,0 +1,61 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * AMD Address Translation Library
+ *
+ * prm.c : Plumbing code to UEFI Platform Runtime Mechanism (PRM)
+ *
+ * Copyright (c) 2024, Advanced Micro Devices, Inc.
+ * All Rights Reserved.
+ *
+ * Author: John Allen <[email protected]>
+ */
+
+#include "internal.h"
+
+#if defined(CONFIG_ACPI_PRMT)
+
+#include <linux/prmt.h>
+
+struct prm_umc_param_buffer_norm {
+ u64 norm_addr;
+ u8 socket;
+ u64 umc_bank_inst_id;
+ void *output_buffer;
+} __packed;
+
+const guid_t norm_to_sys_prm_handler_guid = GUID_INIT(0xE7180659, 0xA65D,
+ 0x451D, 0x92, 0xCD,
+ 0x2B, 0x56, 0xF1, 0x2B,
+ 0xEB, 0xA6);
+
+unsigned long prm_umc_norm_to_sys_addr(u8 socket_id, u64 umc_bank_inst_id, unsigned long addr)
+{
+ struct prm_umc_param_buffer_norm param_buffer;
+ unsigned long ret_addr;
+ int ret;
+
+ param_buffer.norm_addr = addr;
+ param_buffer.socket = socket_id;
+ param_buffer.umc_bank_inst_id = umc_bank_inst_id;
+ param_buffer.output_buffer = &ret_addr;
+
+ ret = acpi_call_prm_handler(norm_to_sys_prm_handler_guid, &param_buffer);
+ if (!ret)
+ return ret_addr;
+
+ if (ret == -ENODEV)
+ pr_info("PRM module/handler not available\n");
+ else
+ pr_info("PRM address translation failed\n");
+
+ return ret;
+}
+
+#else /* ACPI_PRMT */
+
+unsigned long prm_umc_norm_to_sys_addr(u8 socket_id, u64 umc_bank_inst_id, unsigned long addr)
+{
+ return -ENODEV;
+}
+
+#endif
diff --git a/drivers/ras/amd/atl/umc.c b/drivers/ras/amd/atl/umc.c
index 59b6169093f7..954cbe6bf465 100644
--- a/drivers/ras/amd/atl/umc.c
+++ b/drivers/ras/amd/atl/umc.c
@@ -333,9 +333,14 @@ unsigned long convert_umc_mca_addr_to_sys_addr(struct atl_err *err)
u8 coh_st_inst_id = get_coh_st_inst_id(err);
unsigned long addr = get_addr(err->addr);
u8 die_id = get_die_id(err);
+ unsigned long ret_addr;

pr_debug("socket_id=0x%x die_id=0x%x coh_st_inst_id=0x%x addr=0x%016lx",
socket_id, die_id, coh_st_inst_id, addr);

+ ret_addr = prm_umc_norm_to_sys_addr(socket_id, err->ipid, addr);
+ if (!IS_ERR_VALUE(ret_addr))
+ return ret_addr;
+
return norm_to_sys_addr(socket_id, die_id, coh_st_inst_id, addr);
}
--
2.34.1


2024-03-26 21:51:57

by John Allen

[permalink] [raw]
Subject: [PATCH 1/2] ACPI: PRM: Add PRM handler direct call support

Platform Runtime Mechanism (PRM) handlers can be invoked from either the
AML interpreter or directly by an OS driver. Implement the direct call
method.

Export the symbol as this will be used by modules such as the AMD
Address Translation Library and likely others in the future.

Signed-off-by: John Allen <[email protected]>
---
drivers/acpi/prmt.c | 24 ++++++++++++++++++++++++
include/linux/prmt.h | 5 +++++
2 files changed, 29 insertions(+)

diff --git a/drivers/acpi/prmt.c b/drivers/acpi/prmt.c
index c78453c74ef5..9e548426cc22 100644
--- a/drivers/acpi/prmt.c
+++ b/drivers/acpi/prmt.c
@@ -214,6 +214,30 @@ static struct prm_handler_info *find_prm_handler(const guid_t *guid)
#define UPDATE_LOCK_ALREADY_HELD 4
#define UPDATE_UNLOCK_WITHOUT_LOCK 5

+int acpi_call_prm_handler(guid_t handler_guid, void *param_buffer)
+{
+ struct prm_handler_info *handler = find_prm_handler(&handler_guid);
+ struct prm_module_info *module = find_prm_module(&handler_guid);
+ struct prm_context_buffer context;
+ efi_status_t status;
+
+ if (!module || !handler)
+ return -ENODEV;
+
+ memset(&context, 0, sizeof(context));
+ ACPI_COPY_NAMESEG(context.signature, "PRMC");
+ context.identifier = handler->guid;
+ context.static_data_buffer = handler->static_data_buffer_addr;
+ context.mmio_ranges = module->mmio_info;
+
+ status = efi_call_acpi_prm_handler(handler->handler_addr,
+ (u64)param_buffer,
+ &context);
+
+ return efi_status_to_err(status);
+}
+EXPORT_SYMBOL_GPL(acpi_call_prm_handler);
+
/*
* This is the PlatformRtMechanism opregion space handler.
* @function: indicates the read/write. In fact as the PlatformRtMechanism
diff --git a/include/linux/prmt.h b/include/linux/prmt.h
index 24da8364b919..9c094294403f 100644
--- a/include/linux/prmt.h
+++ b/include/linux/prmt.h
@@ -2,6 +2,11 @@

#ifdef CONFIG_ACPI_PRMT
void init_prmt(void);
+int acpi_call_prm_handler(guid_t handler_guid, void *param_buffer);
#else
static inline void init_prmt(void) { }
+static inline int acpi_call_prm_handler(guid_t handler_guid, void *param_buffer)
+{
+ return -EOPNOTSUPP;
+}
#endif
--
2.34.1


2024-04-07 13:43:39

by Yazen Ghannam

[permalink] [raw]
Subject: Re: [PATCH 1/2] ACPI: PRM: Add PRM handler direct call support



On 3/26/24 17:26, John Allen wrote:
> Platform Runtime Mechanism (PRM) handlers can be invoked from either the
> AML interpreter or directly by an OS driver. Implement the direct call
> method.
>
> Export the symbol as this will be used by modules such as the AMD
> Address Translation Library and likely others in the future.
>
> Signed-off-by: John Allen <[email protected]>
> ---
> drivers/acpi/prmt.c | 24 ++++++++++++++++++++++++
> include/linux/prmt.h | 5 +++++
> 2 files changed, 29 insertions(+)
>
> diff --git a/drivers/acpi/prmt.c b/drivers/acpi/prmt.c
> index c78453c74ef5..9e548426cc22 100644
> --- a/drivers/acpi/prmt.c
> +++ b/drivers/acpi/prmt.c
> @@ -214,6 +214,30 @@ static struct prm_handler_info *find_prm_handler(const guid_t *guid)
> #define UPDATE_LOCK_ALREADY_HELD 4
> #define UPDATE_UNLOCK_WITHOUT_LOCK 5
>
> +int acpi_call_prm_handler(guid_t handler_guid, void *param_buffer)
> +{
> + struct prm_handler_info *handler = find_prm_handler(&handler_guid);
> + struct prm_module_info *module = find_prm_module(&handler_guid);

I wonder if the module revision should be checked. Maybe this is a
future problem to address if/when the need arises?

It seems like versioning can be done a few ways for a semi-stable
interface.

1) Keep the module GUID the same and change the module major/minor
revisions as needed.
2) Give the module a new GUID every time there's a change.
3) Keep the module GUID the same, but change a handler GUID if the
handler's inputs/outputs change.

I think #3 would be the way to go for most cases. So the onus is on the
caller to have the correct handler GUID. And no changes needed here.

Just wanted to write out some thoughts in case others have feedback.

> + struct prm_context_buffer context;
> + efi_status_t status;
> +
> + if (!module || !handler)
> + return -ENODEV;
> +
> + memset(&context, 0, sizeof(context));
> + ACPI_COPY_NAMESEG(context.signature, "PRMC");
> + context.identifier = handler->guid;
> + context.static_data_buffer = handler->static_data_buffer_addr;
> + context.mmio_ranges = module->mmio_info;

Minor nit: I suggest aligning these lines on the '=' for easier reading.

> +
> + status = efi_call_acpi_prm_handler(handler->handler_addr,
> + (u64)param_buffer,
> + &context);
> +
> + return efi_status_to_err(status);
> +}
> +EXPORT_SYMBOL_GPL(acpi_call_prm_handler);
> +
> /*
> * This is the PlatformRtMechanism opregion space handler.
> * @function: indicates the read/write. In fact as the PlatformRtMechanism
> diff --git a/include/linux/prmt.h b/include/linux/prmt.h
> index 24da8364b919..9c094294403f 100644
> --- a/include/linux/prmt.h
> +++ b/include/linux/prmt.h
> @@ -2,6 +2,11 @@
>
> #ifdef CONFIG_ACPI_PRMT
> void init_prmt(void);
> +int acpi_call_prm_handler(guid_t handler_guid, void *param_buffer);
> #else
> static inline void init_prmt(void) { }
> +static inline int acpi_call_prm_handler(guid_t handler_guid, void *param_buffer)
> +{
> + return -EOPNOTSUPP;
> +}
> #endif

Overall, looks good to me.

Reviewed-by: Yazen Ghannam <[email protected]>

Thanks,
Yazen

2024-04-07 14:17:41

by Yazen Ghannam

[permalink] [raw]
Subject: Re: [PATCH 2/2] RAS/AMD/ATL: Translate normalized to system physical addresses using PRM



On 3/26/24 17:26, John Allen wrote:
> Future AMD platforms will provide a UEFI PRM module that implements a
> number of address translation PRM handlers. This will provide an
> interface for the OS to call platform specific code without requiring
> the use of SMM or other heavy firmware operations.
>
> AMD Zen-based systems report memory error addresses through Machine
> Check banks representing Unified Memory Controllers (UMCs) in the form
> of UMC relative "normalized" addresses. A normalized address must be
> converted to a system physical address to be usable by the OS.
>
> Add support for the normalized to system physical address translation
> PRM handler in the AMD Address Translation Library and prefer it over
> native code if available. The GUID and parameter buffer structure are
> specific to the normalized to system physical address handler provided
> by the address translation PRM module included in future AMD systems.
>
> The address translation PRM module is documented in chapter 22 of the
> publicly available "AMD Family 1Ah Models 00h–0Fh and Models 10h–1Fh
> ACPI v6.5 Porting Guide":
> https://www.amd.com/content/dam/amd/en/documents/epyc-technical-docs/programmer-references/58088-0.75-pub.pdf
>
> Signed-off-by: John Allen <[email protected]>
> ---
> drivers/ras/amd/atl/Makefile | 1 +
> drivers/ras/amd/atl/internal.h | 2 ++
> drivers/ras/amd/atl/prm.c | 61 ++++++++++++++++++++++++++++++++++
> drivers/ras/amd/atl/umc.c | 5 +++
> 4 files changed, 69 insertions(+)
> create mode 100644 drivers/ras/amd/atl/prm.c
>
> diff --git a/drivers/ras/amd/atl/Makefile b/drivers/ras/amd/atl/Makefile
> index 4acd5f05bd9c..8f1afa793e3b 100644
> --- a/drivers/ras/amd/atl/Makefile
> +++ b/drivers/ras/amd/atl/Makefile
> @@ -14,5 +14,6 @@ amd_atl-y += denormalize.o
> amd_atl-y += map.o
> amd_atl-y += system.o
> amd_atl-y += umc.o
> +amd_atl-y += prm.o
>
> obj-$(CONFIG_AMD_ATL) += amd_atl.o
> diff --git a/drivers/ras/amd/atl/internal.h b/drivers/ras/amd/atl/internal.h
> index 5de69e0bb0f9..f739dcada126 100644
> --- a/drivers/ras/amd/atl/internal.h
> +++ b/drivers/ras/amd/atl/internal.h
> @@ -234,6 +234,8 @@ int dehash_address(struct addr_ctx *ctx);
> unsigned long norm_to_sys_addr(u8 socket_id, u8 die_id, u8 coh_st_inst_id, unsigned long addr);
> unsigned long convert_umc_mca_addr_to_sys_addr(struct atl_err *err);
>
> +unsigned long prm_umc_norm_to_sys_addr(u8 socket_id, u64 umc_bank_inst_id, unsigned long addr);
> +
> /*
> * Make a gap in @data that is @num_bits long starting at @bit_num.
> * e.g. data = 11111111'b
> diff --git a/drivers/ras/amd/atl/prm.c b/drivers/ras/amd/atl/prm.c
> new file mode 100644
> index 000000000000..54a69e660eb5
> --- /dev/null
> +++ b/drivers/ras/amd/atl/prm.c
> @@ -0,0 +1,61 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * AMD Address Translation Library
> + *
> + * prm.c : Plumbing code to UEFI Platform Runtime Mechanism (PRM)
> + *
> + * Copyright (c) 2024, Advanced Micro Devices, Inc.
> + * All Rights Reserved.
> + *
> + * Author: John Allen <[email protected]>
> + */
> +
> +#include "internal.h"
> +
> +#if defined(CONFIG_ACPI_PRMT)
> +
> +#include <linux/prmt.h>
> +
> +struct prm_umc_param_buffer_norm {
> + u64 norm_addr;
> + u8 socket;
> + u64 umc_bank_inst_id;
> + void *output_buffer;
> +} __packed;
> +
> +const guid_t norm_to_sys_prm_handler_guid = GUID_INIT(0xE7180659, 0xA65D,

Use the static keyword since this is only used in the current file.

> + 0x451D, 0x92, 0xCD,
> + 0x2B, 0x56, 0xF1, 0x2B,
> + 0xEB, 0xA6);
> +
> +unsigned long prm_umc_norm_to_sys_addr(u8 socket_id, u64 umc_bank_inst_id, unsigned long addr)
> +{
> + struct prm_umc_param_buffer_norm param_buffer;
> + unsigned long ret_addr;
> + int ret;
> +
> + param_buffer.norm_addr = addr;
> + param_buffer.socket = socket_id;
> + param_buffer.umc_bank_inst_id = umc_bank_inst_id;
> + param_buffer.output_buffer = &ret_addr;
> +
> + ret = acpi_call_prm_handler(norm_to_sys_prm_handler_guid, &param_buffer);
> + if (!ret)
> + return ret_addr;
> +
> + if (ret == -ENODEV)
> + pr_info("PRM module/handler not available\n");

Make this a pr_debug(). I don't think this is something a user could do
anything about. And one goal of this library to abstract how the
functions work. So "trying different backends" is a library developer
concern.

> + else
> + pr_info("PRM address translation failed\n");

Make this a pr_notice_once().

If the handler is available and fails, then this is likely a bug. It
should be reported to the system vendor. And it may be possible for the
user to update the PRM handler. This could be through a BIOS update or
the runtime update option for PRM.

Aside: is the runtime update option implemented?

"Notice" is between info and warning. I think we'd want the user to
notice, but this isn't so severe to need a warning.

Also, *_once() will prevent duplicate messages in the case of multiple
memory errors in the system. The handler shouldn't fail on any valid
input, so a single notice is enough. Especially if the message doesn't
have any error/context-specific details.

Another aside: it's possible to have invalid input. This can happen in
"software/simulated" MCA errors, i.e. the user provides an arbitrary
value for MCA_ADDR. But this would be a user error. I don't think it's
worth trying to filter out this case. An expert user could provide valid
inputs, and they may want to test the full flow. And this isn't an issue
just for PRM but the ATL overall. I hit this myself while testing
another feature. I used a signature for MCA_ADDR (0xC001C0DE01ABCDEF ?)
and the translation failed. But I was more interested in the signature
than the real value. :)

> +
> + return ret;
> +}
> +
> +#else /* ACPI_PRMT */
> +
> +unsigned long prm_umc_norm_to_sys_addr(u8 socket_id, u64 umc_bank_inst_id, unsigned long addr)
> +{
> + return -ENODEV;
> +}
> +
> +#endif
> diff --git a/drivers/ras/amd/atl/umc.c b/drivers/ras/amd/atl/umc.c
> index 59b6169093f7..954cbe6bf465 100644
> --- a/drivers/ras/amd/atl/umc.c
> +++ b/drivers/ras/amd/atl/umc.c
> @@ -333,9 +333,14 @@ unsigned long convert_umc_mca_addr_to_sys_addr(struct atl_err *err)
> u8 coh_st_inst_id = get_coh_st_inst_id(err);
> unsigned long addr = get_addr(err->addr);
> u8 die_id = get_die_id(err);
> + unsigned long ret_addr;
>
> pr_debug("socket_id=0x%x die_id=0x%x coh_st_inst_id=0x%x addr=0x%016lx",
> socket_id, die_id, coh_st_inst_id, addr);
>
> + ret_addr = prm_umc_norm_to_sys_addr(socket_id, err->ipid, addr);
> + if (!IS_ERR_VALUE(ret_addr))
> + return ret_addr;
> +
> return norm_to_sys_addr(socket_id, die_id, coh_st_inst_id, addr);
> }

Thanks,
Yazen