Jan Kiszka <jan.kiszka@xxxxxxxxxxx> writes: > On 2014-04-11 20:35, Bandan Das wrote: >> Jan Kiszka <jan.kiszka@xxxxxxxxxxx> writes: >> >>> On 2014-04-11 19:26, Bandan Das wrote: >>>> Jan Kiszka <jan.kiszka@xxxxxxxxxxx> writes: >>>> >>>>> 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, >>>>>> 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. >>>> >>>> Well, but we could make life a little bit easier for the unfortunate user >>>> using the broken hypervisor :) And advertising single context inavalidation >>>> doesn't really seem to have any downsides. >>> >>> Ok, let's try it this way: single-context invalidation is inherently >>> tied to VPID support (that's how you address a context). However, KVM >>> does not expose VPID to its guest. So this discussion is mood: no >>> hypervisor will make use of this feature as it has no means to fill in >>> the required parameter. >> >> I thought (from the spec) invept single context invalidation >> takes the EP4TA as the second argument. invvpid single context >> however takes the VPID as its descriptor. > > Oops, invept/invvpid mess-up while re-reading the spec - sorry. > >> >> The Xen L1 hypervisor was actually calling single context invept >> multiple times. That's how I hit this bug. > > ...and it's no longer doing it now, I suppose. The question remains, Yes. > which hypervisor we want to cater with a > "single-context-that-is-current-context" invalidation (that is my > understanding of Marcelo's proposal). On the other hand, if some > hypervisor actually uses invept to invalidate a non-current mapping, we > would regress compared to not exposing single context invept. Hope I got > this conclusion right. ;) Yep, not sure if this holds true for any hypervisor. I traced this change down to http://www.spinics.net/lists/kvm/msg94802.html but the conversation doesn't mention the reasoning > Jan -- 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
