| Message ID | 1530709593-87702-1-git-send-email-jingqi.liu@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 04/07/2018 15:06, Jingqi Liu wrote: > A new control bit(bit 29) in the TEST_CTRL MSR will be introduced > to enable detection of split locks. > > When bit 29 of the TEST_CTRL(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/27/78. > > Signed-off-by: Jingqi Liu <jingqi.liu@intel.com> > --- > arch/x86/include/asm/kvm_host.h | 1 + > arch/x86/kvm/vmx.c | 77 +++++++++++++++++++++++++++++++++++++++++ > arch/x86/kvm/x86.c | 10 ++++++ > arch/x86/kvm/x86.h | 5 +++ > include/uapi/linux/kvm.h | 1 + > 5 files changed, 94 insertions(+) Checking for split lock is done with the MSR_TEST_CTL too, so you should not use a capability to expose the availability to KVM userspace. Instead you should expose the contents of MSR_TEST_CTL on the host, in a similar way to https://marc.info/?l=kvm&m=152998661713547&w=2. Please coordinate with Robert Hu on the QEMU patches too, because he's working on the infrastructure to use KVM_GET_MSR_FEATURE_INDEX_LIST in QEMU. Thanks, Paolo
On Wed, 4 Jul 2018, Paolo Bonzini wrote: > On 04/07/2018 15:06, Jingqi Liu wrote: > > A new control bit(bit 29) in the TEST_CTRL MSR will be introduced > > to enable detection of split locks. > > > > When bit 29 of the TEST_CTRL(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/27/78. That dependency is useless, because that series is going nowhere. > > Signed-off-by: Jingqi Liu <jingqi.liu@intel.com> > > --- > > arch/x86/include/asm/kvm_host.h | 1 + > > arch/x86/kvm/vmx.c | 77 +++++++++++++++++++++++++++++++++++++++++ > > arch/x86/kvm/x86.c | 10 ++++++ > > arch/x86/kvm/x86.h | 5 +++ > > include/uapi/linux/kvm.h | 1 + > > 5 files changed, 94 insertions(+) > > Checking for split lock is done with the MSR_TEST_CTL too, so you should > not use a capability to expose the availability to KVM userspace. > Instead you should expose the contents of MSR_TEST_CTL on the host, in a > similar way to https://marc.info/?l=kvm&m=152998661713547&w=2. > > Please coordinate with Robert Hu on the QEMU patches too, because he's > working on the infrastructure to use KVM_GET_MSR_FEATURE_INDEX_LIST in QEMU. Can we please sort out the whole AC mess on the host first including the detection stuff? There is no rush for this to be in KVM/QEMU now because all what exists for this new split lock thing is 'silicon' running on an emulator. And w/o support in the kernel proper this is completely useless. So this needs the following things: 1) Proper enumeration via CPUID or MISC_FEATURES. The current detection hack is just broken. 2) A proper host side implementation, which then automatically makes the stuff usable in a guest once it is exposed. 3) A proper way how to expose MSR_TEST_CTL to the guest, but surely not with extra split_lock_ctrl voodoo. It's an MSR nothing else. KVM/QEMU have standartized ways to deal with MSRs and the required selective bitwise access control. Thanks, tglx
On 04/07/2018 16:51, Thomas Gleixner wrote: > There is no rush for this to be in KVM/QEMU now because all what exists for > this new split lock thing is 'silicon' running on an emulator. And w/o > support in the kernel proper this is completely useless. That's good. I assumed it was IceLake, in which case the feature would block the definition of a standard IceLake CPU model in QEMU. > So this needs the following things: > > 1) Proper enumeration via CPUID or MISC_FEATURES. The current detection > hack is just broken. Yes please. > 2) A proper host side implementation, which then automatically makes the > stuff usable in a guest once it is exposed. If the CPUID bit or MISC_FEATURES is added, you don't even need the host side for the guests to use it. It's only needed now because of the ugly MSR-based detection. > 3) A proper way how to expose MSR_TEST_CTL to the guest, but surely not > with extra split_lock_ctrl voodoo. It's an MSR nothing else. KVM/QEMU > have standartized ways to deal with MSRs and the required selective > bitwise access control. That part is pretty much standard, I'm not worried about it. We have one variable in struct kvm_vcpu_arch for each MSR (or set of MSRs) that we expose, so that's the split_lock_ctrl voodoo. :) Once the detection is sorted out, KVM is easy. Paolo
On Wed, 4 Jul 2018, Paolo Bonzini wrote: > On 04/07/2018 16:51, Thomas Gleixner wrote: > > There is no rush for this to be in KVM/QEMU now because all what exists for > > this new split lock thing is 'silicon' running on an emulator. And w/o > > support in the kernel proper this is completely useless. > > That's good. I assumed it was IceLake, in which case the feature would > block the definition of a standard IceLake CPU model in QEMU. > > > So this needs the following things: > > > > 1) Proper enumeration via CPUID or MISC_FEATURES. The current detection > > hack is just broken. > > Yes please. > > > 2) A proper host side implementation, which then automatically makes the > > stuff usable in a guest once it is exposed. > > If the CPUID bit or MISC_FEATURES is added, you don't even need the host > side for the guests to use it. It's only needed now because of the ugly > MSR-based detection. > > > 3) A proper way how to expose MSR_TEST_CTL to the guest, but surely not > > with extra split_lock_ctrl voodoo. It's an MSR nothing else. KVM/QEMU > > have standartized ways to deal with MSRs and the required selective > > bitwise access control. > > That part is pretty much standard, I'm not worried about it. We have > one variable in struct kvm_vcpu_arch for each MSR (or set of MSRs) that > we expose, so that's the split_lock_ctrl voodoo. :) > > Once the detection is sorted out, KVM is easy. That's what I thought and even if it was IceLeak then they still can flip a CPUID/MISC FEATURE bit in their binary BIOS blob or ucode. We really need to push back hard on these half baken features which need voodoo programming to detect. Thanks, tglx
Hi Jingqi,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on kvm/linux-next]
[also build test ERROR on v4.18-rc3 next-20180704]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Jingqi-Liu/KVM-Expose-the-split-lock-detection-feature-to-guest-VM/20180705-041612
base: https://git.kernel.org/pub/scm/virt/kvm/kvm.git linux-next
config: i386-allmodconfig (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All error/warnings (new ones prefixed by >>):
arch/x86/kvm/vmx.c: In function 'vmx_get_msr':
>> arch/x86/kvm/vmx.c:3757:7: error: 'MSR_TEST_CTL' undeclared (first use in this function); did you mean 'MSR_THERM2_CTL'?
case MSR_TEST_CTL:
^~~~~~~~~~~~
MSR_THERM2_CTL
arch/x86/kvm/vmx.c:3757:7: note: each undeclared identifier is reported only once for each function it appears in
arch/x86/kvm/vmx.c: In function 'vmx_set_msr':
arch/x86/kvm/vmx.c:3881:7: error: 'MSR_TEST_CTL' undeclared (first use in this function); did you mean 'MSR_THERM2_CTL'?
case MSR_TEST_CTL:
^~~~~~~~~~~~
MSR_THERM2_CTL
In file included from arch/x86/include/asm/thread_info.h:53:0,
from include/linux/thread_info.h:38,
from arch/x86/include/asm/preempt.h:7,
from include/linux/preempt.h:81,
from include/linux/spinlock.h:51,
from include/linux/mm_types.h:9,
from arch/x86/kvm/irq.h:25,
from arch/x86/kvm/vmx.c:19:
arch/x86/kvm/vmx.c: In function 'x86_split_lock_ctrl_init':
>> arch/x86/kvm/vmx.c:9982:19: error: 'X86_FEATURE_AC_SPLIT_LOCK' undeclared (first use in this function); did you mean 'X86_FEATURE_CAT_L2'?
if (boot_cpu_has(X86_FEATURE_AC_SPLIT_LOCK)) {
^
arch/x86/include/asm/cpufeature.h:111:24: note: in definition of macro 'cpu_has'
(__builtin_constant_p(bit) && REQUIRED_MASK_BIT_SET(bit) ? 1 : \
^~~
>> arch/x86/kvm/vmx.c:9982:6: note: in expansion of macro 'boot_cpu_has'
if (boot_cpu_has(X86_FEATURE_AC_SPLIT_LOCK)) {
^~~~~~~~~~~~
In file included from arch/x86/include/asm/msr.h:246:0,
from arch/x86/include/asm/processor.h:21,
from arch/x86/include/asm/cpufeature.h:5,
from arch/x86/include/asm/thread_info.h:53,
from include/linux/thread_info.h:38,
from arch/x86/include/asm/preempt.h:7,
from include/linux/preempt.h:81,
from include/linux/spinlock.h:51,
from include/linux/mm_types.h:9,
from arch/x86/kvm/irq.h:25,
from arch/x86/kvm/vmx.c:19:
arch/x86/kvm/vmx.c:9983:10: error: 'MSR_TEST_CTL' undeclared (first use in this function); did you mean 'MSR_THERM2_CTL'?
rdmsrl(MSR_TEST_CTL, x86_split_lock_ctrl_base);
^
arch/x86/include/asm/paravirt.h:145:26: note: in definition of macro 'rdmsrl'
val = paravirt_read_msr(msr); \
^~~
>> arch/x86/kvm/vmx.c:9984:30: error: 'MSR_TEST_CTL_ENABLE_AC_SPLIT_LOCK' undeclared (first use in this function); did you mean 'X86_FEATURE_AC_SPLIT_LOCK'?
x86_split_lock_ctrl_mask = MSR_TEST_CTL_ENABLE_AC_SPLIT_LOCK;
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
X86_FEATURE_AC_SPLIT_LOCK
In file included from arch/x86/include/asm/thread_info.h:53:0,
from include/linux/thread_info.h:38,
from arch/x86/include/asm/preempt.h:7,
from include/linux/preempt.h:81,
from include/linux/spinlock.h:51,
from include/linux/mm_types.h:9,
from arch/x86/kvm/irq.h:25,
from arch/x86/kvm/vmx.c:19:
arch/x86/kvm/vmx.c: In function 'x86_set_split_lock_ctrl':
arch/x86/kvm/vmx.c:9995:19: error: 'X86_FEATURE_AC_SPLIT_LOCK' undeclared (first use in this function); did you mean 'X86_FEATURE_CAT_L2'?
if (boot_cpu_has(X86_FEATURE_AC_SPLIT_LOCK)) {
^
arch/x86/include/asm/cpufeature.h:111:24: note: in definition of macro 'cpu_has'
(__builtin_constant_p(bit) && REQUIRED_MASK_BIT_SET(bit) ? 1 : \
^~~
arch/x86/kvm/vmx.c:9995:6: note: in expansion of macro 'boot_cpu_has'
if (boot_cpu_has(X86_FEATURE_AC_SPLIT_LOCK)) {
^~~~~~~~~~~~
arch/x86/kvm/vmx.c:10003:11: error: 'MSR_TEST_CTL' undeclared (first use in this function); did you mean 'MSR_THERM2_CTL'?
wrmsrl(MSR_TEST_CTL, msrval);
^~~~~~~~~~~~
MSR_THERM2_CTL
arch/x86/kvm/vmx.c: In function 'vmx_vcpu_run':
arch/x86/kvm/vmx.c:10230:35: error: 'MSR_TEST_CTL' undeclared (first use in this function); did you mean 'MSR_THERM2_CTL'?
!msr_write_intercepted(vcpu, MSR_TEST_CTL)) {
^~~~~~~~~~~~
MSR_THERM2_CTL
vim +3757 arch/x86/kvm/vmx.c
3731
3732 /*
3733 * Reads an msr value (of 'msr_index') into 'pdata'.
3734 * Returns 0 on success, non-0 otherwise.
3735 * Assumes vcpu_load() was already called.
3736 */
3737 static int vmx_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
3738 {
3739 struct vcpu_vmx *vmx = to_vmx(vcpu);
3740 struct shared_msr_entry *msr;
3741
3742 switch (msr_info->index) {
3743 #ifdef CONFIG_X86_64
3744 case MSR_FS_BASE:
3745 msr_info->data = vmcs_readl(GUEST_FS_BASE);
3746 break;
3747 case MSR_GS_BASE:
3748 msr_info->data = vmcs_readl(GUEST_GS_BASE);
3749 break;
3750 case MSR_KERNEL_GS_BASE:
3751 vmx_load_host_state(vmx);
3752 msr_info->data = vmx->msr_guest_kernel_gs_base;
3753 break;
3754 #endif
3755 case MSR_EFER:
3756 return kvm_get_msr_common(vcpu, msr_info);
> 3757 case MSR_TEST_CTL:
3758 if (!msr_info->host_initiated &&
3759 !kvm_split_lock_ac_in_guest(vcpu->kvm))
3760 return 1;
3761 msr_info->data = to_vmx(vcpu)->split_lock_ctrl;
3762 break;
3763 case MSR_IA32_SPEC_CTRL:
3764 if (!msr_info->host_initiated &&
3765 !guest_cpuid_has(vcpu, X86_FEATURE_SPEC_CTRL))
3766 return 1;
3767
3768 msr_info->data = to_vmx(vcpu)->spec_ctrl;
3769 break;
3770 case MSR_IA32_ARCH_CAPABILITIES:
3771 if (!msr_info->host_initiated &&
3772 !guest_cpuid_has(vcpu, X86_FEATURE_ARCH_CAPABILITIES))
3773 return 1;
3774 msr_info->data = to_vmx(vcpu)->arch_capabilities;
3775 break;
3776 case MSR_IA32_SYSENTER_CS:
3777 msr_info->data = vmcs_read32(GUEST_SYSENTER_CS);
3778 break;
3779 case MSR_IA32_SYSENTER_EIP:
3780 msr_info->data = vmcs_readl(GUEST_SYSENTER_EIP);
3781 break;
3782 case MSR_IA32_SYSENTER_ESP:
3783 msr_info->data = vmcs_readl(GUEST_SYSENTER_ESP);
3784 break;
3785 case MSR_IA32_BNDCFGS:
3786 if (!kvm_mpx_supported() ||
3787 (!msr_info->host_initiated &&
3788 !guest_cpuid_has(vcpu, X86_FEATURE_MPX)))
3789 return 1;
3790 msr_info->data = vmcs_read64(GUEST_BNDCFGS);
3791 break;
3792 case MSR_IA32_MCG_EXT_CTL:
3793 if (!msr_info->host_initiated &&
3794 !(vmx->msr_ia32_feature_control &
3795 FEATURE_CONTROL_LMCE))
3796 return 1;
3797 msr_info->data = vcpu->arch.mcg_ext_ctl;
3798 break;
3799 case MSR_IA32_FEATURE_CONTROL:
3800 msr_info->data = vmx->msr_ia32_feature_control;
3801 break;
3802 case MSR_IA32_VMX_BASIC ... MSR_IA32_VMX_VMFUNC:
3803 if (!nested_vmx_allowed(vcpu))
3804 return 1;
3805 return vmx_get_vmx_msr(&vmx->nested.msrs, msr_info->index,
3806 &msr_info->data);
3807 case MSR_IA32_XSS:
3808 if (!vmx_xsaves_supported())
3809 return 1;
3810 msr_info->data = vcpu->arch.ia32_xss;
3811 break;
3812 case MSR_TSC_AUX:
3813 if (!msr_info->host_initiated &&
3814 !guest_cpuid_has(vcpu, X86_FEATURE_RDTSCP))
3815 return 1;
3816 /* Otherwise falls through */
3817 default:
3818 msr = find_msr_entry(vmx, msr_info->index);
3819 if (msr) {
3820 msr_info->data = msr->data;
3821 break;
3822 }
3823 return kvm_get_msr_common(vcpu, msr_info);
3824 }
3825
3826 return 0;
3827 }
3828
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
Hi Jingqi,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on kvm/linux-next]
[also build test ERROR on v4.18-rc3 next-20180704]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Jingqi-Liu/KVM-Expose-the-split-lock-detection-feature-to-guest-VM/20180705-041612
base: https://git.kernel.org/pub/scm/virt/kvm/kvm.git linux-next
config: x86_64-randconfig-s4-07050438 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All error/warnings (new ones prefixed by >>):
In file included from arch/x86/include/asm/thread_info.h:53:0,
from include/linux/thread_info.h:38,
from arch/x86/include/asm/preempt.h:7,
from include/linux/preempt.h:81,
from include/linux/hardirq.h:5,
from include/linux/kvm_host.h:10,
from arch/x86/kvm/x86.c:22:
arch/x86/kvm/x86.c: In function 'kvm_vm_ioctl_check_extension':
>> arch/x86/kvm/x86.c:2946:20: error: 'X86_FEATURE_AC_SPLIT_LOCK' undeclared (first use in this function); did you mean 'X86_FEATURE_CAT_L2'?
if (boot_cpu_has(X86_FEATURE_AC_SPLIT_LOCK))
^
arch/x86/include/asm/cpufeature.h:111:24: note: in definition of macro 'cpu_has'
(__builtin_constant_p(bit) && REQUIRED_MASK_BIT_SET(bit) ? 1 : \
^~~
>> arch/x86/kvm/x86.c:2946:7: note: in expansion of macro 'boot_cpu_has'
if (boot_cpu_has(X86_FEATURE_AC_SPLIT_LOCK))
^~~~~~~~~~~~
arch/x86/kvm/x86.c:2946:20: note: each undeclared identifier is reported only once for each function it appears in
if (boot_cpu_has(X86_FEATURE_AC_SPLIT_LOCK))
^
arch/x86/include/asm/cpufeature.h:111:24: note: in definition of macro 'cpu_has'
(__builtin_constant_p(bit) && REQUIRED_MASK_BIT_SET(bit) ? 1 : \
^~~
>> arch/x86/kvm/x86.c:2946:7: note: in expansion of macro 'boot_cpu_has'
if (boot_cpu_has(X86_FEATURE_AC_SPLIT_LOCK))
^~~~~~~~~~~~
vim +2946 arch/x86/kvm/x86.c
2842
2843 int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
2844 {
2845 int r = 0;
2846
2847 switch (ext) {
2848 case KVM_CAP_IRQCHIP:
2849 case KVM_CAP_HLT:
2850 case KVM_CAP_MMU_SHADOW_CACHE_CONTROL:
2851 case KVM_CAP_SET_TSS_ADDR:
2852 case KVM_CAP_EXT_CPUID:
2853 case KVM_CAP_EXT_EMUL_CPUID:
2854 case KVM_CAP_CLOCKSOURCE:
2855 case KVM_CAP_PIT:
2856 case KVM_CAP_NOP_IO_DELAY:
2857 case KVM_CAP_MP_STATE:
2858 case KVM_CAP_SYNC_MMU:
2859 case KVM_CAP_USER_NMI:
2860 case KVM_CAP_REINJECT_CONTROL:
2861 case KVM_CAP_IRQ_INJECT_STATUS:
2862 case KVM_CAP_IOEVENTFD:
2863 case KVM_CAP_IOEVENTFD_NO_LENGTH:
2864 case KVM_CAP_PIT2:
2865 case KVM_CAP_PIT_STATE2:
2866 case KVM_CAP_SET_IDENTITY_MAP_ADDR:
2867 case KVM_CAP_XEN_HVM:
2868 case KVM_CAP_VCPU_EVENTS:
2869 case KVM_CAP_HYPERV:
2870 case KVM_CAP_HYPERV_VAPIC:
2871 case KVM_CAP_HYPERV_SPIN:
2872 case KVM_CAP_HYPERV_SYNIC:
2873 case KVM_CAP_HYPERV_SYNIC2:
2874 case KVM_CAP_HYPERV_VP_INDEX:
2875 case KVM_CAP_HYPERV_EVENTFD:
2876 case KVM_CAP_HYPERV_TLBFLUSH:
2877 case KVM_CAP_PCI_SEGMENT:
2878 case KVM_CAP_DEBUGREGS:
2879 case KVM_CAP_X86_ROBUST_SINGLESTEP:
2880 case KVM_CAP_XSAVE:
2881 case KVM_CAP_ASYNC_PF:
2882 case KVM_CAP_GET_TSC_KHZ:
2883 case KVM_CAP_KVMCLOCK_CTRL:
2884 case KVM_CAP_READONLY_MEM:
2885 case KVM_CAP_HYPERV_TIME:
2886 case KVM_CAP_IOAPIC_POLARITY_IGNORED:
2887 case KVM_CAP_TSC_DEADLINE_TIMER:
2888 case KVM_CAP_ENABLE_CAP_VM:
2889 case KVM_CAP_DISABLE_QUIRKS:
2890 case KVM_CAP_SET_BOOT_CPU_ID:
2891 case KVM_CAP_SPLIT_IRQCHIP:
2892 case KVM_CAP_IMMEDIATE_EXIT:
2893 case KVM_CAP_GET_MSR_FEATURES:
2894 r = 1;
2895 break;
2896 case KVM_CAP_SYNC_REGS:
2897 r = KVM_SYNC_X86_VALID_FIELDS;
2898 break;
2899 case KVM_CAP_ADJUST_CLOCK:
2900 r = KVM_CLOCK_TSC_STABLE;
2901 break;
2902 case KVM_CAP_X86_DISABLE_EXITS:
2903 r |= KVM_X86_DISABLE_EXITS_HLT | KVM_X86_DISABLE_EXITS_PAUSE;
2904 if(kvm_can_mwait_in_guest())
2905 r |= KVM_X86_DISABLE_EXITS_MWAIT;
2906 break;
2907 case KVM_CAP_X86_SMM:
2908 /* SMBASE is usually relocated above 1M on modern chipsets,
2909 * and SMM handlers might indeed rely on 4G segment limits,
2910 * so do not report SMM to be available if real mode is
2911 * emulated via vm86 mode. Still, do not go to great lengths
2912 * to avoid userspace's usage of the feature, because it is a
2913 * fringe case that is not enabled except via specific settings
2914 * of the module parameters.
2915 */
2916 r = kvm_x86_ops->has_emulated_msr(MSR_IA32_SMBASE);
2917 break;
2918 case KVM_CAP_VAPIC:
2919 r = !kvm_x86_ops->cpu_has_accelerated_tpr();
2920 break;
2921 case KVM_CAP_NR_VCPUS:
2922 r = KVM_SOFT_MAX_VCPUS;
2923 break;
2924 case KVM_CAP_MAX_VCPUS:
2925 r = KVM_MAX_VCPUS;
2926 break;
2927 case KVM_CAP_NR_MEMSLOTS:
2928 r = KVM_USER_MEM_SLOTS;
2929 break;
2930 case KVM_CAP_PV_MMU: /* obsolete */
2931 r = 0;
2932 break;
2933 case KVM_CAP_MCE:
2934 r = KVM_MAX_MCE_BANKS;
2935 break;
2936 case KVM_CAP_XCRS:
2937 r = boot_cpu_has(X86_FEATURE_XSAVE);
2938 break;
2939 case KVM_CAP_TSC_CONTROL:
2940 r = kvm_has_tsc_control;
2941 break;
2942 case KVM_CAP_X2APIC_API:
2943 r = KVM_X2APIC_API_VALID_FLAGS;
2944 break;
2945 case KVM_CAP_X86_SPLIT_LOCK_AC:
> 2946 if (boot_cpu_has(X86_FEATURE_AC_SPLIT_LOCK))
2947 r = 1;
2948 else
2949 r = 0;
2950 break;
2951 default:
2952 break;
2953 }
2954 return r;
2955
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
On 04/07/2018 21:37, Paolo Bonzini wrote: > On 04/07/2018 15:06, Jingqi Liu wrote: > > A new control bit(bit 29) in the TEST_CTRL MSR will be introduced to > > enable detection of split locks. > > > > When bit 29 of the TEST_CTRL(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/27/78. > > > > Signed-off-by: Jingqi Liu <jingqi.liu@intel.com> > > --- > > arch/x86/include/asm/kvm_host.h | 1 + > > arch/x86/kvm/vmx.c | 77 > +++++++++++++++++++++++++++++++++++++++++ > > arch/x86/kvm/x86.c | 10 ++++++ > > arch/x86/kvm/x86.h | 5 +++ > > include/uapi/linux/kvm.h | 1 + > > 5 files changed, 94 insertions(+) > > Checking for split lock is done with the MSR_TEST_CTL too, so you should not > use a capability to expose the availability to KVM userspace. > Instead you should expose the contents of MSR_TEST_CTL on the host, in a > similar way to https://marc.info/?l=kvm&m=152998661713547&w=2. > > Please coordinate with Robert Hu on the QEMU patches too, because he's > working on the infrastructure to use KVM_GET_MSR_FEATURE_INDEX_LIST in > QEMU. > Hi Paolo, Thanks for your review, had synced with Robert. Need to work out a way, not impact on the old infrastructure. Thanks Jingqi > Thanks, > > Paolo
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h index c13cd28..adf4c8e 100644 --- a/arch/x86/include/asm/kvm_host.h +++ b/arch/x86/include/asm/kvm_host.h @@ -809,6 +809,7 @@ struct kvm_arch { bool mwait_in_guest; bool hlt_in_guest; bool pause_in_guest; + bool split_lock_ac_in_guest; unsigned long irq_sources_bitmap; s64 kvmclock_offset; diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c index 1689f43..d380764 100644 --- a/arch/x86/kvm/vmx.c +++ b/arch/x86/kvm/vmx.c @@ -58,6 +58,9 @@ #include "pmu.h" #include "vmx_evmcs.h" +static u64 x86_split_lock_ctrl_base; +static u64 x86_split_lock_ctrl_mask; + #define __ex(x) __kvm_handle_fault_on_reboot(x) #define __ex_clear(x, reg) \ ____kvm_handle_fault_on_reboot(x, "xor " reg " , " reg) @@ -776,6 +779,7 @@ struct vcpu_vmx { u64 arch_capabilities; u64 spec_ctrl; + u64 split_lock_ctrl; u32 vm_entry_controls_shadow; u32 vm_exit_controls_shadow; @@ -3750,6 +3754,12 @@ static int vmx_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info) #endif case MSR_EFER: return kvm_get_msr_common(vcpu, msr_info); + case MSR_TEST_CTL: + if (!msr_info->host_initiated && + !kvm_split_lock_ac_in_guest(vcpu->kvm)) + return 1; + msr_info->data = to_vmx(vcpu)->split_lock_ctrl; + break; case MSR_IA32_SPEC_CTRL: if (!msr_info->host_initiated && !guest_cpuid_has(vcpu, X86_FEATURE_SPEC_CTRL)) @@ -3868,6 +3878,19 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info) return 1; vmcs_write64(GUEST_BNDCFGS, data); break; + case MSR_TEST_CTL: + if (!msr_info->host_initiated && + !kvm_split_lock_ac_in_guest(vcpu->kvm)) + return 1; + + vmx->split_lock_ctrl = data; + + if (!data) + break; + vmx_disable_intercept_for_msr(vmx->vmcs01.msr_bitmap, + MSR_TEST_CTL, + MSR_TYPE_RW); + break; case MSR_IA32_SPEC_CTRL: if (!msr_info->host_initiated && !guest_cpuid_has(vcpu, X86_FEATURE_SPEC_CTRL)) @@ -6293,6 +6316,8 @@ static void vmx_vcpu_setup(struct vcpu_vmx *vmx) vmcs_write64(PML_ADDRESS, page_to_phys(vmx->pml_pg)); vmcs_write16(GUEST_PML_INDEX, PML_ENTITY_NUM - 1); } + + vmx->split_lock_ctrl = 0; } static void vmx_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event) @@ -6303,6 +6328,7 @@ static void vmx_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event) vmx->rmode.vm86_active = 0; vmx->spec_ctrl = 0; + vmx->split_lock_ctrl = 0; vcpu->arch.microcode_version = 0x100000000ULL; vmx->vcpu.arch.regs[VCPU_REGS_RDX] = get_rdx_init_val(); @@ -9947,6 +9973,38 @@ static void vmx_arm_hv_timer(struct kvm_vcpu *vcpu) vmcs_write32(VMX_PREEMPTION_TIMER_VALUE, delta_tsc); } +static void x86_split_lock_ctrl_init(void) +{ + /* + * Read the MSR_TEST_CTL MSR to account for reserved bits which may + * have unknown values. + */ + if (boot_cpu_has(X86_FEATURE_AC_SPLIT_LOCK)) { + rdmsrl(MSR_TEST_CTL, x86_split_lock_ctrl_base); + x86_split_lock_ctrl_mask = MSR_TEST_CTL_ENABLE_AC_SPLIT_LOCK; + } +} + +static void x86_set_split_lock_ctrl(struct kvm_vcpu *vcpu, + u64 guest_split_lock_ctrl, bool setguest) +{ + /* + * Check if the feature of #AC exception + * for split locked access is supported. + */ + if (boot_cpu_has(X86_FEATURE_AC_SPLIT_LOCK)) { + u64 msrval, guestval; + u64 hostval = x86_split_lock_ctrl_base; + + guestval = hostval & ~x86_split_lock_ctrl_mask; + guestval |= guest_split_lock_ctrl & x86_split_lock_ctrl_mask; + if (hostval != guestval) { + msrval = setguest ? guestval : hostval; + wrmsrl(MSR_TEST_CTL, msrval); + } + } +} + static void __noclone vmx_vcpu_run(struct kvm_vcpu *vcpu) { struct vcpu_vmx *vmx = to_vmx(vcpu); @@ -10014,6 +10072,12 @@ static void __noclone vmx_vcpu_run(struct kvm_vcpu *vcpu) */ x86_spec_ctrl_set_guest(vmx->spec_ctrl, 0); + /* + * Restore the guest's value of TEST_CTL MSR + * if it's different with the host's value. + */ + x86_set_split_lock_ctrl(vcpu, vmx->split_lock_ctrl, true); + vmx->__launched = vmx->loaded_vmcs->launched; evmcs_rsp = static_branch_unlikely(&enable_evmcs) ? @@ -10162,6 +10226,17 @@ static void __noclone vmx_vcpu_run(struct kvm_vcpu *vcpu) x86_spec_ctrl_restore_host(vmx->spec_ctrl, 0); + if (kvm_split_lock_ac_in_guest(vcpu->kvm) && + !msr_write_intercepted(vcpu, MSR_TEST_CTL)) { + vmx->split_lock_ctrl = native_read_msr(MSR_TEST_CTL); + } + + /* + * Restore the host's value of TEST_CTL MSR + * if it's different with the guest's value. + */ + x86_set_split_lock_ctrl(vcpu, vmx->split_lock_ctrl, false); + /* Eliminate branch target predictions from guest mode */ vmexit_fill_RSB(); @@ -13120,6 +13195,8 @@ static int __init vmx_init(void) { int r; + x86_split_lock_ctrl_init(); + #if IS_ENABLED(CONFIG_HYPERV) /* * Enlightened VMCS usage should be recommended and the host needs diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 0046aa7..2611022 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -2942,6 +2942,12 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext) case KVM_CAP_X2APIC_API: r = KVM_X2APIC_API_VALID_FLAGS; break; + case KVM_CAP_X86_SPLIT_LOCK_AC: + if (boot_cpu_has(X86_FEATURE_AC_SPLIT_LOCK)) + r = 1; + else + r = 0; + break; default: break; } @@ -4260,6 +4266,10 @@ static int kvm_vm_ioctl_enable_cap(struct kvm *kvm, kvm->arch.pause_in_guest = true; r = 0; break; + case KVM_CAP_X86_SPLIT_LOCK_AC: + kvm->arch.split_lock_ac_in_guest = true; + r = 0; + break; default: r = -EINVAL; break; diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h index 257f276..aa4daeb 100644 --- a/arch/x86/kvm/x86.h +++ b/arch/x86/kvm/x86.h @@ -326,6 +326,11 @@ static inline bool kvm_pause_in_guest(struct kvm *kvm) return kvm->arch.pause_in_guest; } +static inline bool kvm_split_lock_ac_in_guest(struct kvm *kvm) +{ + return kvm->arch.split_lock_ac_in_guest; +} + DECLARE_PER_CPU(struct kvm_vcpu *, current_vcpu); static inline void kvm_before_interrupt(struct kvm_vcpu *vcpu) diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h index b6270a3..219f5fd 100644 --- a/include/uapi/linux/kvm.h +++ b/include/uapi/linux/kvm.h @@ -949,6 +949,7 @@ struct kvm_ppc_resize_hpt { #define KVM_CAP_GET_MSR_FEATURES 153 #define KVM_CAP_HYPERV_EVENTFD 154 #define KVM_CAP_HYPERV_TLBFLUSH 155 +#define KVM_CAP_X86_SPLIT_LOCK_AC 156 #ifdef KVM_CAP_IRQ_ROUTING
A new control bit(bit 29) in the TEST_CTRL MSR will be introduced to enable detection of split locks. When bit 29 of the TEST_CTRL(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/27/78. Signed-off-by: Jingqi Liu <jingqi.liu@intel.com> --- arch/x86/include/asm/kvm_host.h | 1 + arch/x86/kvm/vmx.c | 77 +++++++++++++++++++++++++++++++++++++++++ arch/x86/kvm/x86.c | 10 ++++++ arch/x86/kvm/x86.h | 5 +++ include/uapi/linux/kvm.h | 1 + 5 files changed, 94 insertions(+)