Re: [PATCH v4 1/8] KVM: nVMX: Keep KVM updates to BNDCFGS ctrl bits across MSR write

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Mar 02, 2022 at 10:22:43PM +0100, Paolo Bonzini wrote:
> On 3/2/22 21:51, Oliver Upton wrote:
> > On Wed, Mar 02, 2022 at 01:21:23PM +0100, Paolo Bonzini wrote:
> > > On 3/1/22 19:43, Oliver Upton wrote:
> > > > Right, a 1-setting of '{load,clear} IA32_BNDCFGS' should really be the
> > > > responsibility of userspace. My issue is that the commit message in
> > > > commit 5f76f6f5ff96 ("KVM: nVMX: Do not expose MPX VMX controls when
> > > > guest MPX disabled") suggests that userspace can expect these bits to be
> > > > configured based on guest CPUID. Furthermore, before commit aedbaf4f6afd
> > > > ("KVM: x86: Extract kvm_update_cpuid_runtime() from
> > > > kvm_update_cpuid()"), if userspace clears these bits, KVM will continue
> > > > to set them based on CPUID.
> > > > 
> > > > What is the userspace expectation here? If we are saying that changes to
> > > > IA32_VMX_TRUE_{ENTRY,EXIT}_CTLS after userspace writes these MSRs is a
> > > > bug, then I agree aedbaf4f6afd is in fact a bugfix. But, the commit
> > > > message in 5f76f6f5ff96 seems to indicate that userspace wants KVM to
> > > > configure these bits based on guest CPUID.
> > > 
> > > Yes, but I think it's reasonable that userspace wants to override them.  It
> > > has to do that after KVM_SET_CPUID2, but that's okay too.
> > > 
> > 
> > In that case, I can rework the tests at the end of this series to ensure
> > userspace's ability to override w/o a quirk. Sorry for the toil,
> > aedbaf4f6afd caused some breakage for us internally, but really is just
> > a userspace bug.
> 
> How did vanadium break?

Maybe I can redirect you to a test case to highlight a possible
regression in KVM, as seen by userspace ;-)

from Patch 7/8:

> +	/*
> +	 * Test that KVM will set these bits regardless of userspace if the
> +	 * guest CPUID exposes a supporting vPMU.
> +	 */
> +	test_vmx_control_msr(vm, MSR_IA32_VMX_TRUE_ENTRY_CTLS,
> +			     0,						/* set */
> +			     VM_ENTRY_LOAD_IA32_PERF_GLOBAL_CTRL,	/* clear */
> +			     VM_ENTRY_LOAD_IA32_PERF_GLOBAL_CTRL,	/* exp_set */
> +			     0);					/* exp_clear */
> +	test_vmx_control_msr(vm, MSR_IA32_VMX_TRUE_EXIT_CTLS,
> +			     0,						/* set */
> +			     VM_EXIT_LOAD_IA32_PERF_GLOBAL_CTRL,	/* clear */
> +			     VM_EXIT_LOAD_IA32_PERF_GLOBAL_CTRL,	/* exp_set */
> +			     0);					/* exp_clear */

Same goes for the "{load,clear} IA32_BNDCFGS" bits too.

--
Thanks,
Oliver



[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux