On Mon, May 06, 2024 at 05:47:21PM +0000, 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. This should be your first paragraph. > 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 Those URLs are flaky and become invalid over time. When you quote a document, quote it in such a way so that searching for it on the web, can find it. The name above works for me so that's good. > +#include "internal.h" > + > +#if defined(CONFIG_ACPI_PRMT) Instead of that ifdeffery you could do: config AMD_ATL_PRM depends on AMD_ATL && ACPI_PRMT and it'll get enabled automatically and then you don't need the empty stub either. > +#include <linux/prmt.h> > + > +struct prm_umc_param_buffer_norm { What's a prm_umc_param_buffer_norm? > + u64 norm_addr; > + u8 socket; > + u64 umc_bank_inst_id; > + void *output_buffer; Use the usual short versions for such standard names: "out_buf" > +} __packed; > + > +static const guid_t norm_to_sys_prm_handler_guid = GUID_INIT(0xE7180659, 0xA65D, > + 0x451D, 0x92, 0xCD, > + 0x2B, 0x56, 0xF1, 0x2B, > + 0xEB, 0xA6); When you define such long variable names, your lines stick out unnecessarily. Shorten pls. > +unsigned long prm_umc_norm_to_sys_addr(u8 socket_id, u64 umc_bank_inst_id, unsigned long addr) bank_id is fine. > +{ > + struct prm_umc_param_buffer_norm param_buffer; ... p_buf; > + 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, ¶m_buffer); > + if (!ret) > + return ret_addr; > + > + if (ret == -ENODEV) > + pr_debug("PRM module/handler not available\n"); > + else > + pr_notice_once("PRM address translation failed\n"); > + > + return ret; > +} -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
