All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrew Cooper <andrew.cooper3@citrix.com>
To: Jan Beulich <JBeulich@suse.com>,
	Sergey Dyasli <sergey.dyasli@citrix.com>
Cc: Kevin Tian <kevin.tian@intel.com>,
	Jun Nakajima <jun.nakajima@intel.com>,
	xen-devel@lists.xen.org
Subject: Re: [PATCH v1 1/2] x86/vvmx: check vmcs address in vmread/vmwrite
Date: Wed, 1 Mar 2017 13:22:17 +0000	[thread overview]
Message-ID: <6c3e61df-2d0e-e1fb-bf51-03494b150f65@citrix.com> (raw)
In-Reply-To: <58B6D2E9020000780013EB47@prv-mh.provo.novell.com>

On 01/03/17 12:55, Jan Beulich wrote:
>>>> On 01.03.17 at 10:13, <sergey.dyasli@citrix.com> wrote:
>> If nested vmcs's address is invalid, virtual_vmcs_enter() will fail
>> during vmread/vmwrite:
>>
>> (XEN) Xen BUG at .../git/upstream/xen/xen/include/asm/hvm/vmx/vmx.h:333
>> (XEN) ----[ Xen-4.9-unstable  x86_64  debug=y   Tainted:    H ]----
>> (XEN) Xen call trace:
>> (XEN)    [<ffff82d0801f925e>] vmcs.c#arch/x86/hvm/vmx/vmcs.o.unlikely+0x28/0x19a
>> (XEN)    [<ffff82d0801f60e3>] virtual_vmcs_vmwrite_safe+0x16/0x52
>> (XEN)    [<ffff82d080202cb2>] nvmx_handle_vmwrite+0x70/0xfe
>> (XEN)    [<ffff82d0801fe98a>] vmx_vmexit_handler+0x1379/0x1c49
>> (XEN)    [<ffff82d08020427c>] vmx_asm_vmexit_handler+0x3c/0x120
>>
>> Fix this by emulating VMfailInvalid if the address is invalid.
> So just like in patch 2 this is __vmptrld() not properly dealing with
> errors. Instead of doing checks in software which hardware does
> anyway, wouldn't it be better to introduce (and use here and
> there) vmptrld_safe()?

Lonterm, I'd like to see us move to a model where the base version is
safe, and error handling is along the lines of:

if ( on_behalf_of_nested )
    propagate_to_guest();
else
{
    dump_relevent_info();
    domain_crash(current->domain);
}


This is far more useful for error isolation (not taking the host down
even if it is Xen's fault the failure occured), and diagnosing what
actually went wrong, than simply having hit a BUG().

~Andrew

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

  reply	other threads:[~2017-03-01 13:22 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-01  9:13 [PATCH v1 0/2] x86/vvmx: a couple of fixes (vmptrld + vmread/vmwrite) Sergey Dyasli
2017-03-01  9:13 ` [PATCH v1 1/2] x86/vvmx: check vmcs address in vmread/vmwrite Sergey Dyasli
2017-03-01 10:58   ` Andrew Cooper
2017-03-01 12:55   ` Jan Beulich
2017-03-01 13:22     ` Andrew Cooper [this message]
2017-03-01 13:40       ` Jan Beulich
2017-03-01 13:44     ` Sergey Dyasli
2017-03-01 14:04       ` Jan Beulich
2017-03-01 14:22         ` Sergey Dyasli
2017-03-01 14:28           ` Jan Beulich
2017-03-01 15:23             ` Sergey Dyasli
2017-03-01 15:39               ` Jan Beulich
2017-03-03  8:21                 ` Tian, Kevin
2017-03-01  9:13 ` [PATCH v1 2/2] x86/vvmx: add vmcs id check into vmptrld emulation Sergey Dyasli
2017-03-01 11:01   ` Andrew Cooper
2017-03-03 10:54     ` Jan Beulich
2017-03-03  8:21   ` Tian, Kevin

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=6c3e61df-2d0e-e1fb-bf51-03494b150f65@citrix.com \
    --to=andrew.cooper3@citrix.com \
    --cc=JBeulich@suse.com \
    --cc=jun.nakajima@intel.com \
    --cc=kevin.tian@intel.com \
    --cc=sergey.dyasli@citrix.com \
    --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 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.