| Message ID | 20200315050517.127446-4-xiaoyao.li@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | x86/split_lock: Add feature split lock detection | expand |
On Sun, Mar 15, 2020 at 01:05:11PM +0800, Xiaoyao Li wrote: > --- a/Documentation/admin-guide/kernel-parameters.txt > +++ b/Documentation/admin-guide/kernel-parameters.txt > @@ -4666,7 +4666,10 @@ > instructions that access data across cache line > boundaries will result in an alignment check exception. > > - off - not enabled > + disable - disabled, neither kernel nor kvm can use it. Are command line arguments "ABI"? The "=off" variant isn't upstream yet, but it is in TIP. I'm ok with this change, but perhaps this patch (or at least this part of this patch) needs to catch up with the older one within the 5.7 merge window (or sometime before v5.7). Reviewed-by: Tony Luck <tony.luck@intel.com>
Xiaoyao Li <xiaoyao.li@intel.com> writes: > Change sld_off to sld_disable, which means disabling feature split lock > detection and it cannot be used in kernel nor can kvm expose it guest. > Of course, the X86_FEATURE_SPLIT_LOCK_DETECT is not set. > > Add a new optioin sld_kvm_only, which means kernel turns split lock > detection off, but kvm can expose it to guest. What's the point of this? If the host is not clean, then you better fix the host first before trying to expose it to guests. Thanks, tglx
On 3/24/2020 1:10 AM, Thomas Gleixner wrote: > Xiaoyao Li <xiaoyao.li@intel.com> writes: > >> Change sld_off to sld_disable, which means disabling feature split lock >> detection and it cannot be used in kernel nor can kvm expose it guest. >> Of course, the X86_FEATURE_SPLIT_LOCK_DETECT is not set. >> >> Add a new optioin sld_kvm_only, which means kernel turns split lock >> detection off, but kvm can expose it to guest. > > What's the point of this? If the host is not clean, then you better fix > the host first before trying to expose it to guests. It's not about whether or not host is clean. It's for the cases that users just don't want it enabled on host, to not break the applications or drivers that do have split lock issue. > Thanks, > > tglx >
Xiaoyao Li <xiaoyao.li@intel.com> writes: > On 3/24/2020 1:10 AM, Thomas Gleixner wrote: >> Xiaoyao Li <xiaoyao.li@intel.com> writes: >> >>> Change sld_off to sld_disable, which means disabling feature split lock >>> detection and it cannot be used in kernel nor can kvm expose it guest. >>> Of course, the X86_FEATURE_SPLIT_LOCK_DETECT is not set. >>> >>> Add a new optioin sld_kvm_only, which means kernel turns split lock >>> detection off, but kvm can expose it to guest. >> >> What's the point of this? If the host is not clean, then you better fix >> the host first before trying to expose it to guests. > > It's not about whether or not host is clean. It's for the cases that > users just don't want it enabled on host, to not break the applications > or drivers that do have split lock issue. It's very much about whether the host is split lock clean. If your host kernel is not, then this wants to be fixed first. If your host application is broken, then either fix it or use "warn". Stop proliferating crap. Thanks, tglx
On Tue, Mar 24, 2020 at 11:40:18AM +0100, Thomas Gleixner wrote: > Xiaoyao Li <xiaoyao.li@intel.com> writes: > > On 3/24/2020 1:10 AM, Thomas Gleixner wrote: > >> Xiaoyao Li <xiaoyao.li@intel.com> writes: > >> > >>> Change sld_off to sld_disable, which means disabling feature split lock > >>> detection and it cannot be used in kernel nor can kvm expose it guest. > >>> Of course, the X86_FEATURE_SPLIT_LOCK_DETECT is not set. > >>> > >>> Add a new optioin sld_kvm_only, which means kernel turns split lock > >>> detection off, but kvm can expose it to guest. > >> > >> What's the point of this? If the host is not clean, then you better fix > >> the host first before trying to expose it to guests. > > > > It's not about whether or not host is clean. It's for the cases that > > users just don't want it enabled on host, to not break the applications > > or drivers that do have split lock issue. > > It's very much about whether the host is split lock clean. > > If your host kernel is not, then this wants to be fixed first. If your > host application is broken, then either fix it or use "warn". The "kvm only" option was my suggestion. The thought was to provide a way for users to leverage KVM to debug/test kernels without having to have a known good kernel and/or to minimize the risk of crashing their physical system. E.g. debug a misbehaving driver by assigning its associated device to a guest.
Sean Christopherson <sean.j.christopherson@intel.com> writes: > On Tue, Mar 24, 2020 at 11:40:18AM +0100, Thomas Gleixner wrote: >> >> It's very much about whether the host is split lock clean. >> >> If your host kernel is not, then this wants to be fixed first. If your >> host application is broken, then either fix it or use "warn". > > The "kvm only" option was my suggestion. The thought was to provide a way > for users to leverage KVM to debug/test kernels without having to have a > known good kernel and/or to minimize the risk of crashing their physical > system. E.g. debug a misbehaving driver by assigning its associated device > to a guest. warn is giving you that, right? I won't crash the host because the #AC triggers in guest context. Thanks, tglx
On 3/24/2020 6:40 PM, Thomas Gleixner wrote: > Xiaoyao Li <xiaoyao.li@intel.com> writes: >> On 3/24/2020 1:10 AM, Thomas Gleixner wrote: >>> Xiaoyao Li <xiaoyao.li@intel.com> writes: >>> >>>> Change sld_off to sld_disable, which means disabling feature split lock >>>> detection and it cannot be used in kernel nor can kvm expose it guest. >>>> Of course, the X86_FEATURE_SPLIT_LOCK_DETECT is not set. >>>> >>>> Add a new optioin sld_kvm_only, which means kernel turns split lock >>>> detection off, but kvm can expose it to guest. >>> >>> What's the point of this? If the host is not clean, then you better fix >>> the host first before trying to expose it to guests. >> >> It's not about whether or not host is clean. It's for the cases that >> users just don't want it enabled on host, to not break the applications >> or drivers that do have split lock issue. > > It's very much about whether the host is split lock clean. > > If your host kernel is not, then this wants to be fixed first. If your > host application is broken, then either fix it or use "warn". > My thought is for CSPs that they might not turn on SLD on their product environment. Any split lock in kernel or drivers may break their service for tenants.
Xiaoyao Li <xiaoyao.li@intel.com> writes: > On 3/24/2020 6:40 PM, Thomas Gleixner wrote: >> Xiaoyao Li <xiaoyao.li@intel.com> writes: >>> On 3/24/2020 1:10 AM, Thomas Gleixner wrote: >>>> Xiaoyao Li <xiaoyao.li@intel.com> writes: >>>> >>>>> Change sld_off to sld_disable, which means disabling feature split lock >>>>> detection and it cannot be used in kernel nor can kvm expose it guest. >>>>> Of course, the X86_FEATURE_SPLIT_LOCK_DETECT is not set. >>>>> >>>>> Add a new optioin sld_kvm_only, which means kernel turns split lock >>>>> detection off, but kvm can expose it to guest. >>>> >>>> What's the point of this? If the host is not clean, then you better fix >>>> the host first before trying to expose it to guests. >>> >>> It's not about whether or not host is clean. It's for the cases that >>> users just don't want it enabled on host, to not break the applications >>> or drivers that do have split lock issue. >> >> It's very much about whether the host is split lock clean. >> >> If your host kernel is not, then this wants to be fixed first. If your >> host application is broken, then either fix it or use "warn". >> > > My thought is for CSPs that they might not turn on SLD on their product > environment. Any split lock in kernel or drivers may break their service > for tenants. Again you are proliferating crap and making excuses for Common Sense violating Purposes (CSP). Thanks, tglx
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt index 1ee2d1e6d89a..2b922061ff08 100644 --- a/Documentation/admin-guide/kernel-parameters.txt +++ b/Documentation/admin-guide/kernel-parameters.txt @@ -4666,7 +4666,10 @@ instructions that access data across cache line boundaries will result in an alignment check exception. - off - not enabled + disable - disabled, neither kernel nor kvm can use it. + + kvm_only - off in kernel but kvm can expose it to + guest for debug/testing scenario. warn - the kernel will emit rate limited warnings about applications triggering the #AC diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c index 4b3245035b5a..3eeab717a0d0 100644 --- a/arch/x86/kernel/cpu/intel.c +++ b/arch/x86/kernel/cpu/intel.c @@ -35,7 +35,8 @@ enum split_lock_detect_state { sld_not_exist = 0, - sld_off, + sld_disable, + sld_kvm_only, sld_warn, sld_fatal, }; @@ -973,7 +974,8 @@ static const struct { const char *option; enum split_lock_detect_state state; } sld_options[] __initconst = { - { "off", sld_off }, + { "disable", sld_disable }, + { "kvm_only", sld_kvm_only }, { "warn", sld_warn }, { "fatal", sld_fatal }, }; @@ -1004,10 +1006,14 @@ static void __init split_lock_setup(void) } switch (sld_state) { - case sld_off: + case sld_disable: pr_info("disabled\n"); break; + case sld_kvm_only: + pr_info("off in kernel, but kvm can expose it to guest\n"); + break; + case sld_warn: pr_info("warning about user-space split_locks\n"); break; @@ -1062,7 +1068,13 @@ static void split_lock_init(struct cpuinfo_x86 *c) test_ctrl_val = val; switch (sld_state) { - case sld_off: + case sld_disable: + if (wrmsrl_safe(MSR_TEST_CTRL, test_ctrl_val & ~MSR_TEST_CTRL_SPLIT_LOCK_DETECT)) + goto msr_broken; + return; + case sld_kvm_only: + if (wrmsrl_safe(MSR_TEST_CTRL, test_ctrl_val | MSR_TEST_CTRL_SPLIT_LOCK_DETECT)) + goto msr_broken; if (wrmsrl_safe(MSR_TEST_CTRL, test_ctrl_val & ~MSR_TEST_CTRL_SPLIT_LOCK_DETECT)) goto msr_broken; break; @@ -1087,7 +1099,7 @@ static void split_lock_init(struct cpuinfo_x86 *c) * funny things and you get to keep whatever pieces. */ pr_warn_once("MSR fail -- disabled\n"); - sld_state = sld_off; + sld_state = sld_disable; } bool handle_user_split_lock(struct pt_regs *regs, long error_code)
Change sld_off to sld_disable, which means disabling feature split lock detection and it cannot be used in kernel nor can kvm expose it guest. Of course, the X86_FEATURE_SPLIT_LOCK_DETECT is not set. Add a new optioin sld_kvm_only, which means kernel turns split lock detection off, but kvm can expose it to guest. Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com> --- .../admin-guide/kernel-parameters.txt | 5 ++++- arch/x86/kernel/cpu/intel.c | 22 ++++++++++++++----- 2 files changed, 21 insertions(+), 6 deletions(-)