Re: [Xen-devel] [PATCH v1 1/2] x86/vvmx: check vmcs address in vmread/vmwrite

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 01.03.17 at 16:23, <sergey.dyasli@xxxxxxxxxx> wrote:
> On Wed, 2017-03-01 at 07:28 -0700, Jan Beulich wrote:
>> > > > On 01.03.17 at 15:22, <sergey.dyasli@xxxxxxxxxx> wrote:
>> > 
>> > On Wed, 2017-03-01 at 07:04 -0700, Jan Beulich wrote:
>> > > > > > On 01.03.17 at 14:44, <sergey.dyasli@xxxxxxxxxx> wrote:
>> > > > 
>> > > > Additionally, it would be painful to return the correct error value
>> > > > all the way back to nvmx_handle_vmptrld().
>> > > 
>> > > Surely that's at best relevant in the other patch. Here you're in
>> > > virtual_vmcs_vmwrite_safe(), which already knows how to
>> > > communicate back an error indicator.
>> > 
>> > How checking the return value of virtual_vmcs_enter() is different
>> > from checking nv_vvmcxaddr?
>> 
>> Checking the address is just one step. As the other patch shows,
>> checking the ID is necessary too. Over time more such checks may
>> be found necessary. Checking what hardware hands us is a single
>> check, and is always going to be sufficient no matter what new
>> features get added to hardware.
> 
> Conceptually, the result of __vmptrld() is irrelevant to a guest
> during nested vmread/vmwrite emulation.  The fact that Xen is doing
> __vmptrld() is a side effect of nested VMX.

True.

> All necessary checks may be found in Intel SDM.

All _currently_ necessary checks. It is precisely possible new ones
getting added which I'd like to see covered by other than adding
further checks to our software when hardware already does them.

>  And it states that
> there can be only 1 VMfail if VMCS pointer is not valid: VMfailInvalid.
> vmptrld can end up in 3 different VMfails and returning them to the
> guest as a result of vmread/vmwrite emulation is wrong.

As is crashing Xen because of such.

The implication of course would be that the insn-error may need
adjustment in such a case.

> (Even though each of them will end up being VMfailInvalid in current
> implementation)
> 
> I think that Xen's goal in nested VMX is emulating H/W as close as
> possible.

Correct. Preferably by leveraging hardware instead of re-doing the
same thing in software.

Anyway - we'll see what the VMX maintainers think.

Jan


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