Re: [RFT PATCH] x86/pat: Fix set_mce_nospec() for pmem

[Dan Williams <dan.j.williams@intel.com>]

On Fri, Oct 1, 2021 at 3:50 AM Borislav Petkov <bp@alien8.de> wrote:
>
> On Thu, Sep 30, 2021 at 07:02:13PM -0700, Dan Williams wrote:
> > If I understand the proposal correctly Boris is basically saying
> > "figure out how to do your special PMEM stuff in the driver directly
> > and make it so MCE code has no knowledge of the PMEM case".
>
> I don't mind if it has to because of a good reason X - I'm just trying
> to simplify things and not do stuff which is practically unnecessary.
>
> > The flow
> > is:
> >
> > memory_failure(pfn, flags)
> > nfit_handle_mce(...) <--- right now this on mce notifier chain
> > set_mce_nospec(pfn) <--- drop the "whole page" concept
> >
> > This poses a problem because not all memory_failure() paths trigger
> > set_mce_nospec() or the mce notifier chain.
>
> Hmm, so this sounds like more is needed than this small patch.
>
> > If that disconnect happens, attempts to read PMEM pages that have been
> > signalled to memory_failure() will now crash in the driver without
> > workarounds for NP pages.
> >
> > So memory_failure() needs to ensure that it communicates with the
> > driver before any possible NP page attribute changes. I.e. the driver
> > needs to know that regardless of how many cachelines are poisoned the
> > entire page is always unmapped in the direct map.
>
> Ok, so those errors do get reported through MCE so MCE code does need to
> know about them.
>
> So the question is, how should MCE mark them so that the driver can deal
> with them properly.
>
> > Then, when the driver is called with the new RWF_RECOVER_DATA flag, it
> > can set up a new UC alias mapping for the pfn and access the good data
> > in the page while being careful to read around the poisoned cache
> > lines.
>
> See my other mail - that sounds good.
>
> > In my mind this moves the RWF_RECOVER_DATA flag proposal from "nice to
> > have" to "critical for properly coordinating with memory_failure() and
> > mce expectations"
>
> Right, so to reiterate: I don't mind if the MCE code knows about this -
> in the end of the day, it is that code which does the error reporting.
> My goal is to make that reporting and marking optimal so that the driver
> can work with that marking and simplify and document *why* we're doing
> that so that future MCE changes can keep that in mind.
>
> So to sum up: it sounds to me like it should simply mark *whole* pages
> as NC:
>
> 1. this should prevent the spec. access for DRAM.
> 2. PMEM can do UC alias mapping and do sub-page access to salvage data.
>
> How's that?

I think that puts us back in the situation that Tony fixed in:

17fae1294ad9 x86/{mce,mm}: Unmap the entire page if the whole page is
affected and poisoned

...where the clflush in _set_memory_uc() causes more unwanted virtual
#MC injection.

I think that means that we have no choice but to mark the page NP
unconditionally and do the work to ensure that the driver has updated
its poisoned cacheline tracking for data recovery requests.

Right?