[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH] x86/HVM: correct notion of new CPL in task switch emulation



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@xxxxxxxxxx>
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>

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@xxxxxxxxxx>
Tested-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

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@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.