Re: [PATCH 3/3] KVM: nVMX: check for null vmcs12 when L1 does invept

[Jan Kiszka <jan.kiszka@web.de>]

[-- Attachment #1: Type: text/plain, Size: 2215 bytes --]

On 2014-03-22 17:43, Bandan Das wrote:
> Jan Kiszka <jan.kiszka@web.de> writes:
> 
>> On 2014-03-20 21:58, Bandan Das wrote:
>>> Jan Kiszka <jan.kiszka@siemens.com> writes:
>>>
>>>> On 2014-03-20 04:28, Bandan Das wrote:
>>>>> Some L1 hypervisors such as Xen seem to be calling invept after
>>>>> vmclear or before vmptrld on L2. In this case, proceed with
>>>>> falling through and syncing roots as a case where
>>>>> context wide invalidation can't be supported
>>>>
>>>> Can we also base this behaviour on a statement in the SDM? But on first
>>>> glance, I do not find anything like this over there.
>>>
>>> The SDM has nothing of this sort explicitly mentioned but 28.3.3.1 
>>> "Operations that invalidate Cached Mappings" does mention that
>>> the instruction may invalidate mappings associated with other
>>> EP4TAs (even in single context).
>>
>> Yes, "may". So we are implementing undefined behavior in order to please
>> a broken hypervisor that relies on it? Then please state this in the
>> patch and probably also inform Xen about their issue.
> 
> Why undefined behavior ? We don't do anything specific for 
> the single context invalidation case ianyway .e If the eptp matches what 
> vmcs12 has, single context invalidation does fall though to the global 
> invalidation case already. All this change does is add the "L1 calls 
> invept after vmclear and  before vmptrld" to the list of cases to fall 
> though to global invalidation since nvmx doesn't have any knowledge of 
> the current eptp for this case.

OK, I think I misunderstood what the guest expects and how we currently
achieve this: we do not track the mapping between guest and host eptp,
thus cannot properly emulate its behaviour. We therefore need to flush
everything.

> 
> Or do you think we should rethink this approach ?

Well, I wonder if we should expose single-context invept support at all.

I'm also wondering if we are returning proper flags on

    if ((operand.eptp & eptp_mask) !=
                    (nested_ept_get_cr3(vcpu) & eptp_mask))
            break;

Neither nested_vmx_succeed nor nested_vmx_fail* is called if this
condition evaluates to true.

Jan



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]