All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrew Cooper <andrew.cooper3@citrix.com>
To: Jan Beulich <JBeulich@suse.com>,
	xen-devel <xen-devel@lists.xenproject.org>
Cc: Julien Grall <julien.grall@arm.com>
Subject: Re: [PATCH] x86/HVM: correct notion of new CPL in task switch emulation
Date: Fri, 2 Jun 2017 21:02:19 +0100	[thread overview]
Message-ID: <4397a7ed-a6c5-375a-1abf-9a3b0212e4ec@citrix.com> (raw)
In-Reply-To: <59302098020000780015EB40@prv-mh.provo.novell.com>

On 01/06/17 13:11, Jan Beulich wrote:
> Commit aac1df3d03 ("x86/HVM: introduce hvm_get_cpl() and respective
> hook") went too far in one aspect: When emulating a task switch we
> really shouldn't be looking at what hvm_get_cpl() returns, as we're
> switching all segment registers.
>
> However, instead of reverting the relevant parts of that commit, have
> the caller tell the segment loading function what the new CPL is. This
> at once fixes ES being loaded before CS so far having had its checks
> done against the old CPL.

I'd have an extra paragraph describing the symptoms in practice.  e.g.

This bug manifests as a vmentry failure for 32bit VMs which use task
gates to service interrupts/exceptions, in situations where delivering
the event interrupts user code, and a privilege increase is required.

?

>
> Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

I have finally managed to reproduce the original vmentry failure with an
XTF test.  This patch resolves the issue, so

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Tested-by: Andrew Cooper <andrew.cooper3@citrix.com>

Julien: This really should be taken in 4.9, otherwise 32bit VMs will
sporadically crash, especially windows which uses this mechanism to
handle NMIs.

> ---
> An alternative to adding yet another parameter to the function would
> be to have "cpl" have a special case value (e.g. negative) to indicate
> VM86 mode. That would allow replacing the current "eflags" parameter.

Keeping the parameters separate is clearer.  It is not like this is a
hotpath we need to optimise.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

  reply	other threads:[~2017-06-02 20:02 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-06-01 12:11 [PATCH] x86/HVM: correct notion of new CPL in task switch emulation Jan Beulich
2017-06-02 20:02 ` Andrew Cooper [this message]
2017-06-02 20:33   ` Andrew Cooper
2017-06-06  7:06     ` Jan Beulich
2017-06-06 10:21       ` Andrew Cooper
2017-06-06 17:14   ` Julien Grall
2017-06-05 13:06 ` Andrew Cooper
2017-06-06  6:42   ` Jan Beulich
2017-06-06 10:21     ` 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=4397a7ed-a6c5-375a-1abf-9a3b0212e4ec@citrix.com \
    --to=andrew.cooper3@citrix.com \
    --cc=JBeulich@suse.com \
    --cc=julien.grall@arm.com \
    --cc=xen-devel@lists.xenproject.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.