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

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

On Thu, Sep 30, 2021 at 2:21 PM Borislav Petkov <bp@alien8.de> wrote:
>
> On Thu, Sep 30, 2021 at 02:05:49PM -0700, Dan Williams wrote:
> > I.e. it's fine
>
> A lot of things are fine - question is, what makes sense and what makes
> this thing as simple as required to be.
>
> > if a DRAM page with a single cacheline error only gets marked UC.
> > Speculation is disabled and the page allocator will still throw it
> > away and never use it again.
>
> Normal DRAM is poisoned as a whole page, as we established.

It is not in all cases, as I read:

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

...it is MSi_MISC that determines what happens and MSi_MISC is not
memory-type aware.

> So whatever
> it is marked with - UC or NP - it probably doesn't matter. But since
> the page won't be ever used, then that page is practically not present.
> So I say, let's mark normal DRAM pages which have been poisoned as not
> present and be done with it. Period.

Where would this routine learn the memory type? Pass it in via
memory_failure(), to what end?

> > Similarly NP is fine for PMEM when the machine-check-registers
> > indicate that the entire page is full of poison. The driver will
> > record that and block any attempt to recover any data in that page.
>
> So this is still kinda weird. We will mark it with either NP or UC but
> the driver has some special knowledge how to tiptoe around poison. So
> for simplicity, let's mark it with whatever fits best and be done with
> it - driver can handle it just fine.
>
> I hope you're cathing my drift: it doesn't really matter what's possible
> wrt marking - it matters what the practical side of the whole thing
> is wrt further page handling and recovery action. And we should do
> here whatever does not impede that further page handling even if other
> markings are possible.
>
> Ok?

I'm sorry, but I'm not tracking.

set_mce_nospec() just wants to prevent speculative consumption of
poison going forward. It has 2 options NP and UC. UC is suitable for
DRAM and desired for PMEM as long as MSi_MISC indicates a sub-page
size poison event.

If you want set_mce_nospec() to specify NP for the page even when
MSi_MISC indicates sub-page blast radius it would require extra
plumbing to convey the memory type to set_mce_nospec().

I fail to see the point of that extra plumbing when MSi_MISC
indicating "whole_page", or not is sufficient. What am I missing?