linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Steve Wahl <steve.wahl@hpe.com>
To: "Kirill A. Shutemov" <kirill@shutemov.name>
Cc: Steve Wahl <steve.wahl@hpe.com>,
	Dave Hansen <dave.hansen@linux.intel.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
	"H. Peter Anvin" <hpa@zytor.com>,
	x86@kernel.org, Juergen Gross <jgross@suse.com>,
	Brijesh Singh <brijesh.singh@amd.com>,
	Jordan Borgner <mail@jordan-borgner.de>,
	Feng Tang <feng.tang@intel.com>,
	linux-kernel@vger.kernel.org, Baoquan He <bhe@redhat.com>,
	russ.anderson@hpe.com, dimitri.sivanich@hpe.com,
	mike.travis@hpe.com
Subject: Re: [PATCH] x86/boot/64: Make level2_kernel_pgt pages invalid outside kernel area.
Date: Mon, 16 Sep 2019 11:17:01 -0500	[thread overview]
Message-ID: <20190916161701.GI7834@swahl-linux> (raw)
In-Reply-To: <20190916090058.mteofmkkl37ob47k@box.shutemov.name>

On Mon, Sep 16, 2019 at 12:00:58PM +0300, Kirill A. Shutemov wrote:
> On Fri, Sep 13, 2019 at 10:14:15AM -0500, Steve Wahl wrote:
> > On Thu, Sep 12, 2019 at 01:19:17PM +0300, Kirill A. Shutemov wrote:
> > > On Wed, Sep 11, 2019 at 03:08:35PM -0500, Steve Wahl wrote:
> > > > Thank you for your time looking into this with me!
> > > 
> > > With all this explanation the original patch looks sane to me.
> > > 
> > > But I would like to see more information from this thread in the commit
> > > message and some comments in the code on why it's crucial not to map more
> > > than needed.
> > 
> > I am working on this.
> > 
> > > I think we also need to make it clear that this is workaround for a broken
> > > hardware: speculative execution must not trigger a halt.
> > 
> > I think the word broken is a bit loaded here.  According to the UEFI
> > spec (version 2.8, page 167), "Regions that are backed by the physical
> > hardware, but are not supposed to be accessed by the OS, must be
> > returned as EfiReservedMemoryType."  Our interpretation is that
> > includes speculative accesses.
> 
> +Dave.
> 
> I don't think it is. Speculative access is done by hardware, not OS.

But the OS controls speculative access to physical addresses by their
presence or absence in the page tables.  Speculation is done with
calculations based on virtual addresses, without a valid translation
to physical addresses it doesn't progress to an externally visible
action.

> BTW, isn't it a BIOS issue?
> 
> I believe it should have a way to hide a range of physical address space
> from OS or force a caching mode on to exclude it from speculative
> execution. Like setup MTRRs or something.

This is their intent in marking it as reserved in the memory tables.
They have explored many other avenues (I've even suggested a couple
since pinning down this problem) and for each one there is a reason it
doesn't work.

> > I'd lean more toward "tightening of adherence to the spec required by
> > some particular hardware."  Does that work for you?
> 
> Not really, no. I still believe it's issue of the platform, not OS.

My current wording for a comment to go above the code is:

/*
 * Only the region occupied by the kernel has so far been checked against
 * the table of usable memory regions provided by the firmware, so
 * invalidate pages outside that region.  A page table entry that maps to
 * a reserved area of memory would allow processor speculation into that
 * area, and on some hardware (particularly the UV platform) speculation
 * into reserved areas can cause a system halt.
 */

How close does that come to working for you?  (I'm going to dive
specifically into the phrase "region occupied by the kernel" in a
reply to Dave in another message; I understand that phrase may need
work.  I'm more asking about the remainder of it here.)

--> Steve

> -- 
>  Kirill A. Shutemov

-- 
Steve Wahl, Hewlett Packard Enterprise

      parent reply	other threads:[~2019-09-16 16:18 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-06 21:29 [PATCH] x86/boot/64: Make level2_kernel_pgt pages invalid outside kernel area Steve Wahl
2019-09-09  8:14 ` Kirill A. Shutemov
2019-09-10  6:18   ` Ingo Molnar
2019-09-10 14:38     ` Steve Wahl
2019-09-10 14:28   ` Steve Wahl
2019-09-11  0:28     ` Kirill A. Shutemov
2019-09-11 20:08       ` Steve Wahl
2019-09-12 10:19         ` Kirill A. Shutemov
2019-09-13 15:14           ` Steve Wahl
2019-09-16  9:00             ` Kirill A. Shutemov
2019-09-16 14:25               ` Dave Hansen
2019-09-16 17:14                 ` Steve Wahl
2019-09-16 22:19                   ` Dave Hansen
2019-09-16 16:17               ` Steve Wahl [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190916161701.GI7834@swahl-linux \
    --to=steve.wahl@hpe.com \
    --cc=bhe@redhat.com \
    --cc=bp@alien8.de \
    --cc=brijesh.singh@amd.com \
    --cc=dave.hansen@linux.intel.com \
    --cc=dimitri.sivanich@hpe.com \
    --cc=feng.tang@intel.com \
    --cc=hpa@zytor.com \
    --cc=jgross@suse.com \
    --cc=kirill@shutemov.name \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mail@jordan-borgner.de \
    --cc=mike.travis@hpe.com \
    --cc=mingo@redhat.com \
    --cc=russ.anderson@hpe.com \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).