| Message ID | 20200312184521.24579-7-sean.j.christopherson@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: VMX: Unionize vcpu_vmx.exit_reason | expand |
Sean Christopherson <sean.j.christopherson@intel.com> writes: > Use a u16 for nested_vmx_enter_non_root_mode()'s local "exit_reason" to > make it clear the intermediate code is only responsible for setting the > basic exit reason, e.g. FAILED_VMENTRY is unconditionally OR'd in when > emulating a failed VM-Entry. > > No functional change intended. > > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com> > --- > arch/x86/kvm/vmx/nested.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c > index 1848ca0116c0..8fbbe2152ab7 100644 > --- a/arch/x86/kvm/vmx/nested.c > +++ b/arch/x86/kvm/vmx/nested.c > @@ -3182,7 +3182,7 @@ enum nvmx_vmentry_status nested_vmx_enter_non_root_mode(struct kvm_vcpu *vcpu, > struct vcpu_vmx *vmx = to_vmx(vcpu); > struct vmcs12 *vmcs12 = get_vmcs12(vcpu); > bool evaluate_pending_interrupts; > - u32 exit_reason = EXIT_REASON_INVALID_STATE; > + u16 exit_reason = EXIT_REASON_INVALID_STATE; > u32 exit_qual; > > evaluate_pending_interrupts = exec_controls_get(vmx) & > @@ -3308,7 +3308,7 @@ enum nvmx_vmentry_status nested_vmx_enter_non_root_mode(struct kvm_vcpu *vcpu, > return NVMX_VMENTRY_VMEXIT; > > load_vmcs12_host_state(vcpu, vmcs12); > - vmcs12->vm_exit_reason = exit_reason | VMX_EXIT_REASONS_FAILED_VMENTRY; > + vmcs12->vm_exit_reason = VMX_EXIT_REASONS_FAILED_VMENTRY | exit_reason; My personal preference would be to do (u32)exit_reason | VMX_EXIT_REASONS_FAILED_VMENTRY instead but maybe I'm just not in love with implicit type convertion in C. > vmcs12->exit_qualification = exit_qual; > if (enable_shadow_vmcs || vmx->nested.hv_evmcs) > vmx->nested.need_vmcs12_to_shadow_sync = true; Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
From: Vitaly Kuznetsov > Sent: 13 March 2020 13:56 ... > > load_vmcs12_host_state(vcpu, vmcs12); > > - vmcs12->vm_exit_reason = exit_reason | VMX_EXIT_REASONS_FAILED_VMENTRY; > > + vmcs12->vm_exit_reason = VMX_EXIT_REASONS_FAILED_VMENTRY | exit_reason; > > My personal preference would be to do > (u32)exit_reason | VMX_EXIT_REASONS_FAILED_VMENTRY > instead but maybe I'm just not in love with implicit type convertion in C. I look at the cast and wonder what is going on :-) David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
On Fri, Mar 13, 2020 at 02:55:38PM +0100, Vitaly Kuznetsov wrote: > Sean Christopherson <sean.j.christopherson@intel.com> writes: > > > Use a u16 for nested_vmx_enter_non_root_mode()'s local "exit_reason" to > > make it clear the intermediate code is only responsible for setting the > > basic exit reason, e.g. FAILED_VMENTRY is unconditionally OR'd in when > > emulating a failed VM-Entry. > > > > No functional change intended. > > > > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com> > > --- > > arch/x86/kvm/vmx/nested.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c > > index 1848ca0116c0..8fbbe2152ab7 100644 > > --- a/arch/x86/kvm/vmx/nested.c > > +++ b/arch/x86/kvm/vmx/nested.c > > @@ -3182,7 +3182,7 @@ enum nvmx_vmentry_status nested_vmx_enter_non_root_mode(struct kvm_vcpu *vcpu, > > struct vcpu_vmx *vmx = to_vmx(vcpu); > > struct vmcs12 *vmcs12 = get_vmcs12(vcpu); > > bool evaluate_pending_interrupts; > > - u32 exit_reason = EXIT_REASON_INVALID_STATE; > > + u16 exit_reason = EXIT_REASON_INVALID_STATE; > > u32 exit_qual; > > > > evaluate_pending_interrupts = exec_controls_get(vmx) & > > @@ -3308,7 +3308,7 @@ enum nvmx_vmentry_status nested_vmx_enter_non_root_mode(struct kvm_vcpu *vcpu, > > return NVMX_VMENTRY_VMEXIT; > > > > load_vmcs12_host_state(vcpu, vmcs12); > > - vmcs12->vm_exit_reason = exit_reason | VMX_EXIT_REASONS_FAILED_VMENTRY; > > + vmcs12->vm_exit_reason = VMX_EXIT_REASONS_FAILED_VMENTRY | exit_reason; > > My personal preference would be to do > (u32)exit_reason | VMX_EXIT_REASONS_FAILED_VMENTRY > instead but maybe I'm just not in love with implicit type convertion in C. Either way works for me. Paolo?
On 17/03/20 06:29, Sean Christopherson wrote: >>> >>> load_vmcs12_host_state(vcpu, vmcs12); >>> - vmcs12->vm_exit_reason = exit_reason | VMX_EXIT_REASONS_FAILED_VMENTRY; >>> + vmcs12->vm_exit_reason = VMX_EXIT_REASONS_FAILED_VMENTRY | exit_reason; >> My personal preference would be to do >> (u32)exit_reason | VMX_EXIT_REASONS_FAILED_VMENTRY >> instead but maybe I'm just not in love with implicit type convertion in C. > Either way works for me. Paolo? > Flip a coin? :) I think your version is fine. Paolo
diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c index 1848ca0116c0..8fbbe2152ab7 100644 --- a/arch/x86/kvm/vmx/nested.c +++ b/arch/x86/kvm/vmx/nested.c @@ -3182,7 +3182,7 @@ enum nvmx_vmentry_status nested_vmx_enter_non_root_mode(struct kvm_vcpu *vcpu, struct vcpu_vmx *vmx = to_vmx(vcpu); struct vmcs12 *vmcs12 = get_vmcs12(vcpu); bool evaluate_pending_interrupts; - u32 exit_reason = EXIT_REASON_INVALID_STATE; + u16 exit_reason = EXIT_REASON_INVALID_STATE; u32 exit_qual; evaluate_pending_interrupts = exec_controls_get(vmx) & @@ -3308,7 +3308,7 @@ enum nvmx_vmentry_status nested_vmx_enter_non_root_mode(struct kvm_vcpu *vcpu, return NVMX_VMENTRY_VMEXIT; load_vmcs12_host_state(vcpu, vmcs12); - vmcs12->vm_exit_reason = exit_reason | VMX_EXIT_REASONS_FAILED_VMENTRY; + vmcs12->vm_exit_reason = VMX_EXIT_REASONS_FAILED_VMENTRY | exit_reason; vmcs12->exit_qualification = exit_qual; if (enable_shadow_vmcs || vmx->nested.hv_evmcs) vmx->nested.need_vmcs12_to_shadow_sync = true;
Use a u16 for nested_vmx_enter_non_root_mode()'s local "exit_reason" to make it clear the intermediate code is only responsible for setting the basic exit reason, e.g. FAILED_VMENTRY is unconditionally OR'd in when emulating a failed VM-Entry. No functional change intended. Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com> --- arch/x86/kvm/vmx/nested.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)