Marcelo Tosatti <mtosatti@xxxxxxxxxx> writes:
> On Fri, Apr 11, 2014 at 08:22:13AM +0200, Jan Kiszka wrote:
>> On 2014-04-11 02:27, Bandan Das wrote:
>> > Marcelo Tosatti <mtosatti@xxxxxxxxxx> writes:
>> >
>> >> On Mon, Mar 31, 2014 at 05:00:23PM -0400, Bandan Das wrote:
>> >>> For single context invalidation, we fall through to global
>> >>> invalidation in handle_invept() except for one case - when
>> >>> the operand supplied by L1 is different from what we have in
>> >>> vmcs12. However, typically hypervisors will only call invept
>> >>> for the currently loaded eptp, so the condition will
>> >>> never be true.
>> >>>
>> >>> Signed-off-by: Bandan Das <bsd@xxxxxxxxxx>
>> >>
>>> Bandan,
>> >>
>> >> Why not fix INVEPT single-context rather than removing it entirely?
>> >>
>> >> "Single-context. If the INVEPT type is 1, the logical processor
>> >> invalidates all guest-physical mappings and combined mappings associated
>> >> with the EP4TA specified in the INVEPT descriptor. Combined mappings for
>> >> that EP4TA are invalidated for all VPIDs and all PCIDs. (The instruction
>> >> may invalidate mappings associated with other EP4TAs.)"
>> >>
>> >> So just removing the "if (EPTP != CURRENT.EPTP) BREAK" should be enough.
>> >
>> > The single context invalidation in handle_invept() doesn't do
>> > anything different. It just falls down to the global case.
>> > And the invept code in Xen and KVM both seemed to fall back
>> > to global invalidation if support for single context wasn't found.
>> > So, it was proposed not to advertise it at all.
>> >
>> > But rethinking this again, I agree with you. If there's a hypervisor
>> > with a single context invept implmentation that does not fallback,
>
> What do you mean "does not fallback" ? The hypervisor cannot detect
> fallback because:
>
> "(The instruction may invalidate mappings associated with other EP4TAs.)"
>
> So the spec says single context can behave as global context (similar
> with TLB entries and INVLPG).
>
> So it is valid to implement single context as global context.
I meant if single context invalidation isn't supported,
the hypervisor falls back to global invalidation like in kvm -
static inline void ept_sync_context(u64 eptp)
{
...
if (cpu_has_vmx_invept_context())
__invept(VMX_EPT_EXTENT_CONTEXT, eptp, 0);
else
ept_sync_global();
...
>> > this will unfortunately not work. Jan, do you agree with this ?
>>
>> A hypervisor that doesn't properly check the HW caps is just broken. And
>> one that mandates single context invalidation support is silly.
>>
>> Jan
>
> I imagined Xen broke because broken KVM's implementation of INVEPT
> single context (so that should be fixed).
It's failing because of this check in handle_invept -
if ((operand.eptp & eptp_mask) !=
(nested_ept_get_cr3(vcpu) & eptp_mask))
break;
Problem is invept can get called even after a vmclear and Jan
pointed out that there's probably no case where this if will
evaluate to true (atleast not for kvm/xen).
> If with the proper implementation of INVEPT single context in KVM Xen
> still fails for some reason, would have to understand why it is failing.
The argument was that since kvm doesn't do anything different
for single context invalidation, does it make sense to not advertise
it at all assuming that the above snippet of invept code is used
by all hypervisors ?
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
