Re: [Xen-devel] [PATCH] x86/hvm: fix domain crash when CR3 has the noflush bit set

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 29.01.18 at 13:44, <rcojocaru@xxxxxxxxxxxxxxx> wrote:
> On 01/26/2018 12:27 PM, Jan Beulich wrote:
>>>>> On 26.01.18 at 10:39, <rcojocaru@xxxxxxxxxxxxxxx> wrote:
>>> --- a/xen/arch/x86/hvm/hvm.c
>>> +++ b/xen/arch/x86/hvm/hvm.c
>>> @@ -2324,6 +2324,9 @@ int hvm_set_cr3(unsigned long value, bool_t may_defer)
>>>          }
>>>      }
>>>  
>>> +    if ( hvm_pcid_enabled(v) ) /* Clear the noflush bit. */
>>> +        value &= ((1ull << 63) - 1);
>> 
>> This needs a proper X86_CR3_* constant defined.
>> 
>> Did you investigate what it would take to actually _act_ upon the
>> bit being set (besides clearing it for the value to be stored to be
>> sane)? Afaict it would require handing a boolean through
>> paging_update_cr3() -> ... -> {svm,vmx}_update_guest_cr() to
>> allow suppressing the hvm_asid_flush_vcpu() etc there.
> 
> I'm now propagating the bool to {svm,vmx}_update_guest_cr() in a
> quick-test patch (attached), however my Windows 10 KPTI-enabled guest is
> now BSODing with "IRQL NOT LESS OR EQUAL" soon after I attach to it with
> xen-access.
> 
> Clearly I'm touching code I'm not familiar with, so if this is something
> worth pursuing any suggestions are appreciated. Otherwise, I can stick
> to the original plan and just clear the noflush bit with no other
> processing until things become clearer with the new code.

I think once you omit the unnecessary !! from

    bool flush = !!(value & X86_CR3_NOFLUSH);

you'll recognize yourself that you've got this inverted (besides
the whole construct needing to sit in the PCID conditional). I
didn't look further, because I think with such an issue things
can't possibly work. I'd anyway suggest to make the new
parameters "noflush", as the override is the exception, and
flushing is the default.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel