lkml.org 
[lkml]   [2021]   [Feb]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH] KVM: x86: dump_vmcs should not assume GUEST_IA32_EFER is valid
From
Date
On 18/02/21 13:56, David Edmondson wrote:
> On Thursday, 2021-02-18 at 12:54:52 +01, Paolo Bonzini wrote:
>
>> On 18/02/21 11:04, David Edmondson wrote:
>>> When dumping the VMCS, retrieve the current guest value of EFER from
>>> the kvm_vcpu structure if neither VM_EXIT_SAVE_IA32_EFER or
>>> VM_ENTRY_LOAD_IA32_EFER is set, which can occur if the processor does
>>> not support the relevant VM-exit/entry controls.
>>
>> Printing vcpu->arch.efer is not the best choice however. Could we dump
>> the whole MSR load/store area instead?
>
> I'm happy to do that, and think that it would be useful, but it won't
> help with the original problem (which I should have explained more).
>
> If the guest has EFER_LMA set but we aren't using the entry/exit
> controls, vm_read64(GUEST_IA32_EFER) returns 0, causing dump_vmcs() to
> erroneously dump the PDPTRs.

Got it now. It would sort of help, because while dumping the MSR
load/store area you could get hold of the real EFER, and use it to
decide whether to dump the PDPTRs.

Thanks,

Paolo


>> Paolo
>>
>>> Fixes: 4eb64dce8d0a ("KVM: x86: dump VMCS on invalid entry")
>>> Signed-off-by: David Edmondson <david.edmondson@oracle.com>
>>> ---
>>> arch/x86/kvm/vmx/vmx.c | 14 +++++++++-----
>>> arch/x86/kvm/vmx/vmx.h | 2 +-
>>> 2 files changed, 10 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
>>> index eb69fef57485..74ea4fe6f35e 100644
>>> --- a/arch/x86/kvm/vmx/vmx.c
>>> +++ b/arch/x86/kvm/vmx/vmx.c
>>> @@ -5754,7 +5754,7 @@ static void vmx_dump_dtsel(char *name, uint32_t limit)
>>> vmcs_readl(limit + GUEST_GDTR_BASE - GUEST_GDTR_LIMIT));
>>> }
>>>
>>> -void dump_vmcs(void)
>>> +void dump_vmcs(struct kvm_vcpu *vcpu)
>>> {
>>> u32 vmentry_ctl, vmexit_ctl;
>>> u32 cpu_based_exec_ctrl, pin_based_exec_ctrl, secondary_exec_control;
>>> @@ -5771,7 +5771,11 @@ void dump_vmcs(void)
>>> cpu_based_exec_ctrl = vmcs_read32(CPU_BASED_VM_EXEC_CONTROL);
>>> pin_based_exec_ctrl = vmcs_read32(PIN_BASED_VM_EXEC_CONTROL);
>>> cr4 = vmcs_readl(GUEST_CR4);
>>> - efer = vmcs_read64(GUEST_IA32_EFER);
>>> + if ((vmexit_ctl & VM_EXIT_SAVE_IA32_EFER) ||
>>> + (vmentry_ctl & VM_ENTRY_LOAD_IA32_EFER))
>>> + efer = vmcs_read64(GUEST_IA32_EFER);
>>> + else
>>> + efer = vcpu->arch.efer;
>>> secondary_exec_control = 0;
>>> if (cpu_has_secondary_exec_ctrls())
>>> secondary_exec_control = vmcs_read32(SECONDARY_VM_EXEC_CONTROL);
>>> @@ -5955,7 +5959,7 @@ static int vmx_handle_exit(struct kvm_vcpu *vcpu, fastpath_t exit_fastpath)
>>> }
>>>
>>> if (exit_reason & VMX_EXIT_REASONS_FAILED_VMENTRY) {
>>> - dump_vmcs();
>>> + dump_vmcs(vcpu);
>>> vcpu->run->exit_reason = KVM_EXIT_FAIL_ENTRY;
>>> vcpu->run->fail_entry.hardware_entry_failure_reason
>>> = exit_reason;
>>> @@ -5964,7 +5968,7 @@ static int vmx_handle_exit(struct kvm_vcpu *vcpu, fastpath_t exit_fastpath)
>>> }
>>>
>>> if (unlikely(vmx->fail)) {
>>> - dump_vmcs();
>>> + dump_vmcs(vcpu);
>>> vcpu->run->exit_reason = KVM_EXIT_FAIL_ENTRY;
>>> vcpu->run->fail_entry.hardware_entry_failure_reason
>>> = vmcs_read32(VM_INSTRUCTION_ERROR);
>>> @@ -6049,7 +6053,7 @@ static int vmx_handle_exit(struct kvm_vcpu *vcpu, fastpath_t exit_fastpath)
>>>
>>> unexpected_vmexit:
>>> vcpu_unimpl(vcpu, "vmx: unexpected exit reason 0x%x\n", exit_reason);
>>> - dump_vmcs();
>>> + dump_vmcs(vcpu);
>>> vcpu->run->exit_reason = KVM_EXIT_INTERNAL_ERROR;
>>> vcpu->run->internal.suberror =
>>> KVM_INTERNAL_ERROR_UNEXPECTED_EXIT_REASON;
>>> diff --git a/arch/x86/kvm/vmx/vmx.h b/arch/x86/kvm/vmx/vmx.h
>>> index 9d3a557949ac..f8a0ce74798e 100644
>>> --- a/arch/x86/kvm/vmx/vmx.h
>>> +++ b/arch/x86/kvm/vmx/vmx.h
>>> @@ -489,6 +489,6 @@ static inline bool vmx_guest_state_valid(struct kvm_vcpu *vcpu)
>>> return is_unrestricted_guest(vcpu) || __vmx_guest_state_valid(vcpu);
>>> }
>>>
>>> -void dump_vmcs(void);
>>> +void dump_vmcs(struct kvm_vcpu *vcpu);
>>>
>>> #endif /* __KVM_X86_VMX_H */
>>>
>
> dme.
>

\
 
 \ /
  Last update: 2021-02-18 16:25    [W:0.541 / U:0.184 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site