diff mbox series

[v1,2/5] KVM: x86: nVMX: Update VMCS12 fields existence when nVMX MSRs are set

Message ID 1629192673-9911-3-git-send-email-robert.hu@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series KVM/x86/nVMX: Add field existence support in VMCS12 | expand

Commit Message

Robert Hoo Aug. 17, 2021, 9:31 a.m. UTC 
Signed-off-by: Robert Hoo <robert.hu@linux.intel.com>
Signed-off-by: Yu Zhang <yu.c.zhang@linux.intel.com>
---
 arch/x86/kvm/vmx/nested.c | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

Comments

Paolo Bonzini Oct. 20, 2021, 3:11 p.m. UTC | #1 
On 17/08/21 11:31, Robert Hoo wrote:
> +		vmcs12_field_update_by_vmexit_ctrl(vmx->nested.msrs.entry_ctls_high,
> +				*highp, data >> 32,
> +				vmx->nested.vmcs12_field_existence_bitmap);
> +		break;
> +	case MSR_IA32_VMX_TRUE_ENTRY_CTLS:
> +		vmcs12_field_update_by_vmentry_ctrl(vmx->nested.msrs.exit_ctls_high,
> +				*highp, data >> 32,
> +				vmx->nested.vmcs12_field_existence_bitmap);

These two functions maybe could be merged into just one, since there are 
going to be duplicate checks.

Paolo
Robert Hoo Oct. 21, 2021, 1:08 p.m. UTC | #2 
On Wed, 2021-10-20 at 17:11 +0200, Paolo Bonzini wrote:
> On 17/08/21 11:31, Robert Hoo wrote:
> > +		vmcs12_field_update_by_vmexit_ctrl(vmx-
> > >nested.msrs.entry_ctls_high,
> > +				*highp, data >> 32,
> > +				vmx-
> > >nested.vmcs12_field_existence_bitmap);
> > +		break;
> > +	case MSR_IA32_VMX_TRUE_ENTRY_CTLS:
> > +		vmcs12_field_update_by_vmentry_ctrl(vmx-
> > >nested.msrs.exit_ctls_high,
> > +				*highp, data >> 32,
> > +				vmx-
> > >nested.vmcs12_field_existence_bitmap);
> 
> These two functions maybe could be merged into just one, since there
> are 
> going to be duplicate checks.

Can I keep them? I think this is trivial, and separating them looks
more clear, from logical perspective.:-)

A summary question:
am I going to send v2? since I'm not sure about Sean and Jim's decision
on whether to implement the interaction with shadow VMCS (which will
have to consume 8KiB more memory for each vmx).
And, Jim mentioned they have some virtualizing shadow vmcs patches
which is going to be sent to community. Should I wait for their
patches?
> 
> Paolo
>
diff mbox series

Patch

diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
index 125b94dc3cf1..b8121f8f6d96 100644
--- a/arch/x86/kvm/vmx/nested.c
+++ b/arch/x86/kvm/vmx/nested.c
@@ -1262,6 +1262,34 @@  vmx_restore_control_msr(struct vcpu_vmx *vmx, u32 msr_index, u64 data)
 
 	*lowp = data;
 	*highp = data >> 32;
+
+	switch (msr_index) {
+	case MSR_IA32_VMX_TRUE_PINBASED_CTLS:
+		vmcs12_field_update_by_pinbased_ctrl(*highp,
+				data >> 32,
+				vmx->nested.vmcs12_field_existence_bitmap);
+		break;
+	case MSR_IA32_VMX_TRUE_PROCBASED_CTLS:
+		vmcs12_field_update_by_procbased_ctrl(*highp,
+				data >> 32,
+				vmx->nested.vmcs12_field_existence_bitmap);
+		break;
+	case MSR_IA32_VMX_PROCBASED_CTLS2:
+		vmcs12_field_update_by_procbased_ctrl2(*highp,
+				data >> 32,
+				vmx->nested.vmcs12_field_existence_bitmap);
+		break;
+	case MSR_IA32_VMX_TRUE_EXIT_CTLS:
+		vmcs12_field_update_by_vmexit_ctrl(vmx->nested.msrs.entry_ctls_high,
+				*highp, data >> 32,
+				vmx->nested.vmcs12_field_existence_bitmap);
+		break;
+	case MSR_IA32_VMX_TRUE_ENTRY_CTLS:
+		vmcs12_field_update_by_vmentry_ctrl(vmx->nested.msrs.exit_ctls_high,
+				*highp, data >> 32,
+				vmx->nested.vmcs12_field_existence_bitmap);
+		break;
+	}
 	return 0;
 }
 
@@ -1403,6 +1431,8 @@  int vmx_set_vmx_msr(struct kvm_vcpu *vcpu, u32 msr_index, u64 data)
 	case MSR_IA32_VMX_VMFUNC:
 		if (data & ~vmx->nested.msrs.vmfunc_controls)
 			return -EINVAL;
+		vmcs12_field_update_by_vm_func(vmx->nested.msrs.vmfunc_controls,
+				data, vmx->nested.vmcs12_field_existence_bitmap);
 		vmx->nested.msrs.vmfunc_controls = data;
 		return 0;
 	default: