Re: [PATCH 5/5] KVM: nVMX: Allow VMREAD when Enlightened VMCS is in use

[Sean Christopherson <seanjc@xxxxxxxxxx>]

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 :-)

> 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.

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);
	}