Sean Christopherson <seanjc@xxxxxxxxxx> writes:
> On Tue, Dec 14, 2021, Vitaly Kuznetsov wrote:
>> Hyper-V TLFS explicitly forbids VMREAD and VMWRITE instructions when
>> Enlightened VMCS interface is in use:
>>
>> "Any VMREAD or VMWRITE instructions while an enlightened VMCS is
>> active is unsupported and can result in unexpected behavior.""
>>
>> Windows 11 + WSL2 seems to ignore this, attempts to VMREAD VMCS field
>> 0x4404 ("VM-exit interruption information") are observed. Failing
>> these attempts with nested_vmx_failInvalid() makes such guests
>> unbootable.
>>
>> Microsoft confirms this is a Hyper-V bug and claims that it'll get fixed
>> eventually but for the time being we need a workaround. (Temporary) allow
>
> Temporarily. And for the record, I highly doubt this will be temporary :-)
>
I'm just trying to be optimistic, at least in commit messages, you know :-)
>> VMREAD to get data from the currently loaded Enlightened VMCS.
>
> ...
>
>> @@ -5074,27 +5075,44 @@ static int handle_vmread(struct kvm_vcpu *vcpu)
>> if (!nested_vmx_check_permission(vcpu))
>> return 1;
>>
>> + /* Normal or Enlightened VMPTRLD must be performed first */
>> + if (vmx->nested.current_vmptr == INVALID_GPA &&
>> + !evmptr_is_valid(vmx->nested.hv_evmcs_vmptr))
>> + return nested_vmx_failInvalid(vcpu);
>
> I believe this is wrong, as it allows this combination
>
> current_vmptr == INVALID_GPA && evmptr_is_valid() && is_guest_mode()
>
> which is eVMCS with VMCS shadowing exposed to L2. SECONDARY_EXEC_SHADOW_VMCS is
> listed in EVMCS1_UNSUPPORTED_2NDEXEC, so it should be impossible for VMCS shadowing
> to be enabled for L2. And if VMCS shadowing is not enabled, all VMREADs cause
> exits to L1, i.e. shouldn't reach this point. If we want to allow that behavior,
> then I think that should be a separate change.
I think you're right, there's no need to allow for that. I'll use your
suggestion from below, thanks!
>
> Assuming eVMCS really isn't compatible with shadow VMCS, I believe we can do:
>
> /*
> * Decode instruction info and find the field to read. This can be
> * done speculatively as there are no side effects
> */
> field = kvm_register_read(vcpu, (((instr_info) >> 28) & 0xf));
>
> if (!evmptr_is_valid(vmx->nested.hv_evmcs_vmptr)) {
> /*
> * In VMX non-root operation, when the VMCS-link pointer is
> * INVALID_GPA, any VMREAD sets the ALU flags for VMfailInvalid.
> */
> if (vmx->nested.current_vmptr == INVALID_GPA ||
> (is_guest_mode(vcpu) &&
> get_vmcs12(vcpu)->vmcs_link_pointer == INVALID_GPA))
> return nested_vmx_failInvalid(vcpu);
>
> offset = vmcs12_field_offset(field);
> if (offset < 0)
> return nested_vmx_fail(vcpu, VMXERR_UNSUPPORTED_VMCS_COMPONENT);
>
> if (!is_guest_mode(vcpu) && is_vmcs12_ext_field(field))
> copy_vmcs02_to_vmcs12_rare(vcpu, vmcs12);
>
> /* Read the field, zero-extended to a u64 value */
> value = vmcs12_read_any(vmcs12, field, offset);
> } else {
> /*
> * <snarky comment about Hyper-V>
> */
> if (WARN_ON_ONCE(is_guest_mode(vcpu))
> return nested_vmx_failInvalid(vcpu);
>
> offset = evmcs_field_offset(field, NULL);
> if (offset < 0)
> return nested_vmx_fail(vcpu, VMXERR_UNSUPPORTED_VMCS_COMPONENT);
>
> /* Read the field, zero-extended to a u64 value */
> value = evmcs_read_any(vmx->nested.hv_evmcs, field, offset);
> }
>
--
Vitaly
