xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: Andrew Cooper <andrew.cooper3@citrix.com>
To: Xen-devel List <xen-devel@lists.xen.org>,
	Jan Beulich <JBeulich@suse.com>, Keir Fraser <keir@xen.org>,
	Tim Deegan <tim@xen.org>
Subject: Boot time trap handling
Date: Mon, 12 May 2014 11:40:39 +0100	[thread overview]
Message-ID: <5370A527.3000406@citrix.com> (raw)

Hello,

When reviewing the SMAP series, Jan noticed an issue I had overlooked
while developing 7e510a7b874fa "x86/boot: move some __high_start code
and data into init sections".

All interrupts we expect to receive are indeed replaced with real
handlers, but the reserved exceptions are left as-are pointing at
ignore_int().  With ignore_int() in the init section, it will point
somewhere arbitrary after boot.  This is obviously a bad thing, but the
previous behaviour wasn't much more useful in this regard, by locking up
a pcpu without bringing the system down.

While attempting to fix this issue, I discovered some more concerning ones.

On the BSP, we:
* load the empty 'idt_table'
* patch ignore_int() into every entry (and indeed, bottom first which is
rather unsafe)
* enter __start_xen()
* patch in early_page_fault()
* perform large mounts of setup
* enter trap_init()
* patch the real trap handlers into 'idt_table'
* Set up ISTs for #DF, #NMI, #MCE in 'idt_table'
* Set up IST stacks in our local TSS
* Load our local TSS

On APs, we:
* Load mmu_cr4_features, including CR4.MCE
* Load the BSPs 'idt_table', complete with IST information
* Set up IST stacks in our local TSS
* Load our local TSS
* Switch onto our own local memcpy() of the BSPs idt.

In both cases, until we have loaded the TSS, we risk trying to take an
MCE or NMI without a TSS loaded.  I cant spot which contributory
exception would be generated, but I suspect #NP, or possibly #TS.  With
#DF set up in the same way, we will triple fault.


Looking at the real trap handlers, they appear to be safe from almost
the start of __start_xen().  Therefore, I propose:

* The boot critical region with an empty idtr gets extended slightly
into the top of __start_xen() and start_secondary()
* Inside this critical region, set set up and load the TSS.
* Load ourselves onto our local idt.
* Load cr4, after the MCE entry path is valid.

This has the added advantage that we gain full bugframe and extable
support for the earlier parts of setup.

Is there anything I have overlooked, or does this plan look plausible?

~Andrew

             reply	other threads:[~2014-05-12 10:40 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-05-12 10:40 Andrew Cooper [this message]
2014-05-12 12:09 ` Boot time trap handling Jan Beulich
2014-05-12 12:53   ` Andrew Cooper
2014-05-12 12:59     ` Jan Beulich
2014-05-12 13:01       ` Andrew Cooper

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=5370A527.3000406@citrix.com \
    --to=andrew.cooper3@citrix.com \
    --cc=JBeulich@suse.com \
    --cc=keir@xen.org \
    --cc=tim@xen.org \
    --cc=xen-devel@lists.xen.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).