| Message ID | 20170313205640.171747-1-jmattson@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Am 13.03.2017 um 21:56 schrieb Jim Mattson: > It is never correct for a VMX instruction to fail with "invalid VMCS" > if there is, in fact, a current VMCS. Reads from unbacked addresses > return all 1's, which means that an unbacked VMCS will not have the > correct VMCS revision ID (i.e. VMCS12_REVISION). I would really like a comment regarding "reading unbacked addresses returns all 1's -> invalid revision" > > Fixes: 63846663eac78 ("KVM: nVMX: Implement VMPTRLD") > Signed-off-by: Jim Mattson <jmattson@google.com> > --- > arch/x86/kvm/vmx.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c > index 98e82ee1e699..a142e529c93d 100644 > --- a/arch/x86/kvm/vmx.c > +++ b/arch/x86/kvm/vmx.c > @@ -7604,7 +7604,8 @@ static int handle_vmptrld(struct kvm_vcpu *vcpu) > struct page *page; > page = nested_get_page(vcpu, vmptr); > if (page == NULL) { > - nested_vmx_failInvalid(vcpu); > + nested_vmx_failValid(vcpu, > + VMXERR_VMPTRLD_INCORRECT_VMCS_REVISION_ID); > return kvm_skip_emulated_instruction(vcpu); > } > new_vmcs12 = kmap(page); >
On 15.03.2017 15:40, Jim Mattson wrote: > It is never correct for a VMX instruction to fail with "invalid VMCS" > if there is, in fact, a current VMCS. Reads from unbacked addresses > return all 1's, which means that an unbacked VMCS will not have the > correct VMCS revision ID (i.e. VMCS12_REVISION). > > Fixes: 63846663eac78 ("KVM: nVMX: Implement VMPTRLD") > Signed-off-by: Jim Mattson <jmattson@google.com> > --- > arch/x86/kvm/vmx.c | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c > index 98e82ee1e699..2c21181c8a70 100644 > --- a/arch/x86/kvm/vmx.c > +++ b/arch/x86/kvm/vmx.c > @@ -7604,7 +7604,14 @@ static int handle_vmptrld(struct kvm_vcpu *vcpu) > struct page *page; > page = nested_get_page(vcpu, vmptr); > if (page == NULL) { > - nested_vmx_failInvalid(vcpu); > + /* > + * Reads from an unbacked page return all 1s, > + * which means that the 32 bits located at the > + * given physical address won't match the required > + * VMCS12_REVISION identifier. > + */ > + nested_vmx_failValid(vcpu, > + VMXERR_VMPTRLD_INCORRECT_VMCS_REVISION_ID); > return kvm_skip_emulated_instruction(vcpu); > } > new_vmcs12 = kmap(page); > Reviewed-by: David Hildenbrand <david@redhat.com>
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c index 98e82ee1e699..a142e529c93d 100644 --- a/arch/x86/kvm/vmx.c +++ b/arch/x86/kvm/vmx.c @@ -7604,7 +7604,8 @@ static int handle_vmptrld(struct kvm_vcpu *vcpu) struct page *page; page = nested_get_page(vcpu, vmptr); if (page == NULL) { - nested_vmx_failInvalid(vcpu); + nested_vmx_failValid(vcpu, + VMXERR_VMPTRLD_INCORRECT_VMCS_REVISION_ID); return kvm_skip_emulated_instruction(vcpu); } new_vmcs12 = kmap(page);
It is never correct for a VMX instruction to fail with "invalid VMCS" if there is, in fact, a current VMCS. Reads from unbacked addresses return all 1's, which means that an unbacked VMCS will not have the correct VMCS revision ID (i.e. VMCS12_REVISION). Fixes: 63846663eac78 ("KVM: nVMX: Implement VMPTRLD") Signed-off-by: Jim Mattson <jmattson@google.com> --- arch/x86/kvm/vmx.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)