Re: [PATCH] KVM: x86: Expose the split lock detection feature to guest VM

["Liu, Jingqi" <jingqi.liu@xxxxxxxxx>]

On 5/21/2018 10:32 PM, Paolo Bonzini wrote:
> On 22/05/2018 04:51, Jingqi Liu wrote:
> > A new control bit(bit 29) in the TEST_CTL MSR will be introduced
> > to enable detection of split locks.
> >
> > When bit 29 of the TEST_CTL(33H) MSR is set, the processor
> > causes an #AC exception to be issued instead of suppressing LOCK on
> > bus(during split lock access). A previous control bit (bit 31)
> > in this MSR causes the processor to disable LOCK# assertion for
> > split locked accesses when set. When bits 29 and 31 are both set,
> > bit 29 takes precedence.
> >
> > The release document ref below link:
> > https://software.intel.com/sites/default/files/managed/c5/15/\
> > architecture-instruction-set-extensions-programming-reference.pdf
> > This patch has a dependency on https://lkml.org/lkml/2018/5/14/1157
> Please follow the way spec_ctrl is implemented,
Hi Paolo,
It could not follow the way spec_ctrl is implemented.
The guest will use the setting in TEST_CTL MSR which is set by host.
If the bit is set in the host,  an #AC exception will be caused when the 
guest starts.
Actually, the bit is disabled when guest starts. And it can be enabled 
in guest kernel.
Thanks
Jingqi
> so that there is no
> wrmsr/rdmsr unless the guest is using the feature (and also no wrmsr
> unless the guest and host settings don't match).
>
> How should the guest detect that the bits are available?  Is there a
> CPUID bit?
>
> Paolo
>
> > Signed-off-by: Jingqi Liu <jingqi.liu@xxxxxxxxx>
> > ---
> >   arch/x86/kvm/vmx.c | 13 +++++++++++++
> >   1 file changed, 13 insertions(+)
> >
> > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> > index 3f16965..07986e0 100644
> > --- a/arch/x86/kvm/vmx.c
> > +++ b/arch/x86/kvm/vmx.c
> > @@ -610,6 +610,8 @@ struct vcpu_vmx {
> >   
> >   	u64 		      arch_capabilities;
> >   	u64 		      spec_ctrl;
> > +	u64		      guest_split_lock_ctrl;
> > +	u64		      host_split_lock_ctrl;
> >   
> >   	u32 vm_entry_controls_shadow;
> >   	u32 vm_exit_controls_shadow;
> > @@ -6013,6 +6015,8 @@ static void vmx_vcpu_setup(struct vcpu_vmx *vmx)
> >   	vmcs_write32(VM_ENTRY_MSR_LOAD_COUNT, 0);
> >   	vmcs_write64(VM_ENTRY_MSR_LOAD_ADDR, __pa(vmx->msr_autoload.guest));
> >   
> > +	vmx->guest_split_lock_ctrl = 0;
> > +
> >   	if (vmcs_config.vmentry_ctrl & VM_ENTRY_LOAD_IA32_PAT)
> >   		vmcs_write64(GUEST_IA32_PAT, vmx->vcpu.arch.pat);
> >   
> > @@ -6062,6 +6066,7 @@ static void vmx_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event)
> >   
> >   	vmx->rmode.vm86_active = 0;
> >   	vmx->spec_ctrl = 0;
> > +	vmx->guest_split_lock_ctrl = 0;
> >   
> >   	vcpu->arch.microcode_version = 0x100000000ULL;
> >   	vmx->vcpu.arch.regs[VCPU_REGS_RDX] = get_rdx_init_val();
> > @@ -9725,6 +9730,9 @@ static void __noclone vmx_vcpu_run(struct kvm_vcpu *vcpu)
> >   	if (vmx->spec_ctrl)
> >   		native_wrmsrl(MSR_IA32_SPEC_CTRL, vmx->spec_ctrl);
> >   
> > +	vmx->host_split_lock_ctrl = native_read_msr(MSR_TEST_CTL);
> > +	native_wrmsrl(MSR_TEST_CTL, vmx->guest_split_lock_ctrl);
> > +
> >   	vmx->__launched = vmx->loaded_vmcs->launched;
> >   
> >   	evmcs_rsp = static_branch_unlikely(&enable_evmcs) ?
> > @@ -9874,6 +9882,9 @@ static void __noclone vmx_vcpu_run(struct kvm_vcpu *vcpu)
> >   	if (vmx->spec_ctrl)
> >   		native_wrmsrl(MSR_IA32_SPEC_CTRL, 0);
> >   
> > +	vmx->guest_split_lock_ctrl = native_read_msr(MSR_TEST_CTL);
> > +	native_wrmsrl(MSR_TEST_CTL, vmx->host_split_lock_ctrl);
> > +
> >   	/* Eliminate branch target predictions from guest mode */
> >   	vmexit_fill_RSB();
> >   
> > @@ -10037,6 +10048,8 @@ static struct kvm_vcpu *vmx_create_vcpu(struct kvm *kvm, unsigned int id)
> >   	vmx_disable_intercept_for_msr(msr_bitmap, MSR_IA32_SYSENTER_CS, MSR_TYPE_RW);
> >   	vmx_disable_intercept_for_msr(msr_bitmap, MSR_IA32_SYSENTER_ESP, MSR_TYPE_RW);
> >   	vmx_disable_intercept_for_msr(msr_bitmap, MSR_IA32_SYSENTER_EIP, MSR_TYPE_RW);
> > +	vmx_disable_intercept_for_msr(msr_bitmap, MSR_TEST_CTL, MSR_TYPE_RW);
> > +
> >   	vmx->msr_bitmap_mode = 0;
> >   
> >   	vmx->loaded_vmcs = &vmx->vmcs01;