On 19/06/2025 14:57, Adrian Hunter wrote:
> On 18/06/2025 17:55, Dave Hansen wrote:
>> On 6/18/25 05:08, Adrian Hunter wrote:
>>> --- a/arch/x86/kernel/cpu/mce/core.c
>>> +++ b/arch/x86/kernel/cpu/mce/core.c
>>> @@ -1665,7 +1665,8 @@ noinstr void do_machine_check(struct pt_regs *regs)
>>> * be added to free list when the guest is terminated.
>>> */
>>> if (mce_usable_address(m)) {
>>> - struct page *p = pfn_to_online_page(m->addr >> PAGE_SHIFT);
>>> + unsigned long pfn = (m->addr & MCI_ADDR_PHYSADDR) >> PAGE_SHIFT;
>>> + struct page *p = pfn_to_online_page(pfn);
>>
>> If ->addr isn't really an address that software can do much with,
>> shouldn't we mask MCI_ADDR_PHYSADDR off up front, like in mce_read_aux()?
>
> Would that mean no one would know if the mce addr had KeyID bits or not?
Current design, to keep the bits in mce addr, is from Tony's patch:
x86/mce: Mask out non-address bits from machine check bank
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=8a01ec97dc066009dd89e43bfcf55644f2dd6d19
Assuming that is not altered, a tidy-up is still possible like:
diff --git a/arch/x86/include/asm/mce.h b/arch/x86/include/asm/mce.h
index 6c77c03139f7..b469b7a7ecfa 100644
--- a/arch/x86/include/asm/mce.h
+++ b/arch/x86/include/asm/mce.h
@@ -386,4 +386,14 @@ static inline void mce_amd_feature_init(struct cpuinfo_x86 *c) { }
unsigned long copy_mc_fragile_handle_tail(char *to, char *from, unsigned len);
+static inline unsigned long mce_addr_to_phys(u64 mce_addr)
+{
+ return mce_addr & MCI_ADDR_PHYSADDR;
+}
+
+static inline unsigned long mce_addr_to_pfn(u64 mce_addr)
+{
+ return mce_addr_to_phys(mce_addr) >> PAGE_SHIFT;
+}
+
#endif /* _ASM_X86_MCE_H */
diff --git a/arch/x86/kernel/cpu/mce/core.c b/arch/x86/kernel/cpu/mce/core.c
index 76c4634c6a5f..e9e8c377790f 100644
--- a/arch/x86/kernel/cpu/mce/core.c
+++ b/arch/x86/kernel/cpu/mce/core.c
@@ -642,7 +642,7 @@ static int uc_decode_notifier(struct notifier_block *nb, unsigned long val,
mce->severity != MCE_DEFERRED_SEVERITY)
return NOTIFY_DONE;
- pfn = (mce->addr & MCI_ADDR_PHYSADDR) >> PAGE_SHIFT;
+ pfn = mce_addr_to_pfn(mce->addr);
if (!memory_failure(pfn, 0)) {
set_mce_nospec(pfn);
mce->kflags |= MCE_HANDLED_UC;
@@ -1412,7 +1412,7 @@ static void kill_me_maybe(struct callback_head *cb)
if (!p->mce_ripv)
flags |= MF_MUST_KILL;
- pfn = (p->mce_addr & MCI_ADDR_PHYSADDR) >> PAGE_SHIFT;
+ pfn = mce_addr_to_pfn(p->mce_addr);
ret = memory_failure(pfn, flags);
if (!ret) {
set_mce_nospec(pfn);
@@ -1441,7 +1441,7 @@ static void kill_me_never(struct callback_head *cb)
p->mce_count = 0;
pr_err("Kernel accessed poison in user space at %llx\n", p->mce_addr);
- pfn = (p->mce_addr & MCI_ADDR_PHYSADDR) >> PAGE_SHIFT;
+ pfn = mce_addr_to_pfn(p->mce_addr);
if (!memory_failure(pfn, 0))
set_mce_nospec(pfn);
}
@@ -1665,7 +1665,7 @@ noinstr void do_machine_check(struct pt_regs *regs)
* be added to free list when the guest is terminated.
*/
if (mce_usable_address(m)) {
- unsigned long pfn = (m->addr & MCI_ADDR_PHYSADDR) >> PAGE_SHIFT;
+ unsigned long pfn = mce_addr_to_pfn(m->addr);
struct page *p = pfn_to_online_page(pfn);
if (p)
diff --git a/drivers/cxl/core/mce.c b/drivers/cxl/core/mce.c
index ff8d078c6ca1..f3c4d6a5f159 100644
--- a/drivers/cxl/core/mce.c
+++ b/drivers/cxl/core/mce.c
@@ -24,7 +24,7 @@ static int cxl_handle_mce(struct notifier_block *nb, unsigned long val,
if (!endpoint)
return NOTIFY_DONE;
- spa = mce->addr & MCI_ADDR_PHYSADDR;
+ spa = mce_addr_to_phys(mce->addr);
pfn = spa >> PAGE_SHIFT;
if (!pfn_valid(pfn))
diff --git a/drivers/edac/skx_common.c b/drivers/edac/skx_common.c
index c9ade45c1a99..83fcec743ea7 100644
--- a/drivers/edac/skx_common.c
+++ b/drivers/edac/skx_common.c
@@ -732,7 +732,7 @@ int skx_mce_check_error(struct notifier_block *nb, unsigned long val,
memset(&res, 0, sizeof(res));
res.mce = mce;
- res.addr = mce->addr & MCI_ADDR_PHYSADDR;
+ res.addr = mce_addr_to_phys(mce->addr);
if (!pfn_to_online_page(res.addr >> PAGE_SHIFT) && !arch_is_platform_page(res.addr)) {
pr_err("Invalid address 0x%llx in IA32_MC%d_ADDR\n", mce->addr, mce->bank);
return NOTIFY_DONE;
