On 4/12/2022 3:07 AM, Borislav Petkov wrote: > On Tue, Apr 05, 2022 at 01:47:44PM -0600, Jane Chu wrote: >> The set_memory_uc() approach doesn't work well in all cases. >> For example, when "The VMM unmapped the bad page from guest >> physical space and passed the machine check to the guest." >> "The guest gets virtual #MC on an access to that page. >> When the guest tries to do set_memory_uc() and instructs >> cpa_flush() to do clean caches that results in taking another >> fault / exception perhaps because the VMM unmapped the page >> from the guest." > > I presume this is quoting someone... Yes, will mention Dan's name next time. > >> Since the driver has special knowledge to handle NP or UC, >> let's mark the poisoned page with NP and let driver handle it > > s/let's mark/mark/ > Okay. >> when it comes down to repair. >> >> Please refer to discussions here for more details. >> https://lore.kernel.org/all/CAPcyv4hrXPb1tASBZUg-GgdVs0OOFKXMXLiHmktg_kFi7YBMyQ@xxxxxxxxxxxxxx/ >> >> Now since poisoned page is marked as not-present, in order to >> avoid writing to a 'np' page and trigger kernel Oops, also fix > > You can write it out: "non-present page..." Okay. > >> pmem_do_write(). >> >> Fixes: 284ce4011ba6 ("x86/memory_failure: Introduce {set, clear}_mce_nospec()") >> Signed-off-by: Jane Chu <jane.chu@xxxxxxxxxx> >> --- >> arch/x86/kernel/cpu/mce/core.c | 6 +++--- >> arch/x86/mm/pat/set_memory.c | 18 ++++++------------ >> drivers/nvdimm/pmem.c | 31 +++++++------------------------ >> include/linux/set_memory.h | 4 ++-- >> 4 files changed, 18 insertions(+), 41 deletions(-) > > For such mixed subsystem patches we probably should talk about who picks > them up, eventually... Hmm, maintainers' action item? > >> diff --git a/arch/x86/kernel/cpu/mce/core.c b/arch/x86/kernel/cpu/mce/core.c >> index 981496e6bc0e..fa67bb9d1afe 100644 >> --- a/arch/x86/kernel/cpu/mce/core.c >> +++ b/arch/x86/kernel/cpu/mce/core.c >> @@ -579,7 +579,7 @@ static int uc_decode_notifier(struct notifier_block *nb, unsigned long val, >> >> pfn = mce->addr >> PAGE_SHIFT; >> if (!memory_failure(pfn, 0)) { >> - set_mce_nospec(pfn, whole_page(mce)); >> + set_mce_nospec(pfn); >> mce->kflags |= MCE_HANDLED_UC; >> } >> >> @@ -1316,7 +1316,7 @@ static void kill_me_maybe(struct callback_head *cb) >> >> ret = memory_failure(p->mce_addr >> PAGE_SHIFT, flags); >> if (!ret) { >> - set_mce_nospec(p->mce_addr >> PAGE_SHIFT, p->mce_whole_page); >> + set_mce_nospec(p->mce_addr >> PAGE_SHIFT); >> sync_core(); >> return; >> } >> @@ -1342,7 +1342,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); >> if (!memory_failure(p->mce_addr >> PAGE_SHIFT, 0)) >> - set_mce_nospec(p->mce_addr >> PAGE_SHIFT, p->mce_whole_page); >> + set_mce_nospec(p->mce_addr >> PAGE_SHIFT); >> } > > Both that ->mce_whole_page and whole_page() look unused after this. > Yes, indeed, Dan also noticed, will remove while_page() in a follow-on patch. thanks, -jane
