Re: [PATCH v9 09/17] x86/split_lock: Handle #AC exception for split lock

2019-10-21 Thread Paolo Bonzini
On 17/10/19 03:23, Xiaoyao Li wrote: > However, without force_emulation_prefix enabled, I'm not sure whether > malicious guest can create the case causing the emulation with a lock > prefix and going to the emulator_cmpxchg_emulated(). > I found it impossible without force_emulation_prefix enabled

Re: [PATCH v9 09/17] x86/split_lock: Handle #AC exception for split lock

2019-10-21 Thread Paolo Bonzini
On 16/10/19 19:42, Sean Christopherson wrote: > KVM uses a locked cmpxchg in emulator_cmpxchg_emulated() and the address > is guest controlled, e.g. a guest could coerce the host into disabling > split-lock detection via the host's #AC handler by triggering emulation > and inducing an #AC in the

Re: [PATCH v9 09/17] x86/split_lock: Handle #AC exception for split lock

2019-10-21 Thread Paolo Bonzini
On 16/10/19 18:23, Sean Christopherson wrote: >> Yes we can get fancy, but remember that KVM is not yet supporting >> emulation of locked instructions. Adding it is possible but shouldn't >> be in the critical path for the whole feature. > Ah, didn't realize that. I'm surprised emulating all

Re: [PATCH v9 09/17] x86/split_lock: Handle #AC exception for split lock

2019-10-16 Thread Xiaoyao Li
On 10/17/2019 1:42 AM, Sean Christopherson wrote: On Wed, Oct 16, 2019 at 09:23:37AM -0700, Sean Christopherson wrote: On Wed, Oct 16, 2019 at 05:43:53PM +0200, Paolo Bonzini wrote: On 16/10/19 17:41, Sean Christopherson wrote: On Wed, Oct 16, 2019 at 04:08:14PM +0200, Paolo Bonzini wrote:

Re: [PATCH v9 09/17] x86/split_lock: Handle #AC exception for split lock

2019-10-16 Thread Sean Christopherson
On Wed, Oct 16, 2019 at 09:23:37AM -0700, Sean Christopherson wrote: > On Wed, Oct 16, 2019 at 05:43:53PM +0200, Paolo Bonzini wrote: > > On 16/10/19 17:41, Sean Christopherson wrote: > > > On Wed, Oct 16, 2019 at 04:08:14PM +0200, Paolo Bonzini wrote: > > >> SIGBUS (actually a new

Re: [PATCH v9 09/17] x86/split_lock: Handle #AC exception for split lock

2019-10-16 Thread Paolo Bonzini
On 16/10/19 18:25, Xiaoyao Li wrote: >> >>    3 | Y | Y   |  N  |   Y |   x   | Switch >> MSR_TEST_CTRL on >> |   | | | |   | enter/exit, >> plus: >> |   | | | |   | A) #AC >> forwarded to guest.

Re: [PATCH v9 09/17] x86/split_lock: Handle #AC exception for split lock

2019-10-16 Thread Xiaoyao Li
On 10/16/2019 11:37 PM, Paolo Bonzini wrote: On 16/10/19 16:43, Thomas Gleixner wrote: N | #AC | #AC enabled | SMT | Ctrl| Guest | Action R | available | on host | | exposed | #AC | --|---|-|-|-|---|- | |

Re: [PATCH v9 09/17] x86/split_lock: Handle #AC exception for split lock

2019-10-16 Thread Sean Christopherson
On Wed, Oct 16, 2019 at 05:43:53PM +0200, Paolo Bonzini wrote: > On 16/10/19 17:41, Sean Christopherson wrote: > > On Wed, Oct 16, 2019 at 04:08:14PM +0200, Paolo Bonzini wrote: > >> SIGBUS (actually a new KVM_EXIT_INTERNAL_ERROR result from KVM_RUN is > >> better, but that's the idea) is for when

Re: [PATCH v9 09/17] x86/split_lock: Handle #AC exception for split lock

2019-10-16 Thread Sean Christopherson
On Wed, Oct 16, 2019 at 11:29:00AM +0200, Thomas Gleixner wrote: > > - Modify the #AC handler to test/set the same atomic variable as the > > sysfs knob. This is the "disabled by kernel" flow. > > That's the #AC in kernel handler, right? Yes. > > - Modify the debugfs/sysfs knob to only

Re: [PATCH v9 09/17] x86/split_lock: Handle #AC exception for split lock

2019-10-16 Thread Paolo Bonzini
On 16/10/19 17:41, Sean Christopherson wrote: > On Wed, Oct 16, 2019 at 04:08:14PM +0200, Paolo Bonzini wrote: >> SIGBUS (actually a new KVM_EXIT_INTERNAL_ERROR result from KVM_RUN is >> better, but that's the idea) is for when you're debugging guests. >> Global disable (or alternatively, disable

Re: [PATCH v9 09/17] x86/split_lock: Handle #AC exception for split lock

2019-10-16 Thread Sean Christopherson
On Wed, Oct 16, 2019 at 04:08:14PM +0200, Paolo Bonzini wrote: > SIGBUS (actually a new KVM_EXIT_INTERNAL_ERROR result from KVM_RUN is > better, but that's the idea) is for when you're debugging guests. > Global disable (or alternatively, disable SMT) is for production use. Alternatively, for

Re: [PATCH v9 09/17] x86/split_lock: Handle #AC exception for split lock

2019-10-16 Thread Paolo Bonzini
On 16/10/19 16:43, Thomas Gleixner wrote: > > N | #AC | #AC enabled | SMT | Ctrl| Guest | Action > R | available | on host | | exposed | #AC | > --|---|-|-|-|---|- > | | | | | | >

RE: [PATCH v9 09/17] x86/split_lock: Handle #AC exception for split lock

2019-10-16 Thread Thomas Gleixner
On Wed, 16 Oct 2019, David Laight wrote: > For the smt case, can you make #AC enable a property of the process? > Then disable it on the core if either smt process requires it be disabled? That would be feasible if the logic of the TEST_CTRL_MSR would be AND, but it's OR. Thread0 #AC-EN

Re: [PATCH v9 09/17] x86/split_lock: Handle #AC exception for split lock

2019-10-16 Thread Thomas Gleixner
On Wed, 16 Oct 2019, Xiaoyao Li wrote: > On 10/16/2019 7:58 PM, Paolo Bonzini wrote: > > > With your proposal you render #AC useless even on hosts which have SMT > > > disabled, which is just wrong. There are enough good reasons to disable > > > SMT. > > > > My lazy "solution" only applies to SMT

Re: [PATCH v9 09/17] x86/split_lock: Handle #AC exception for split lock

2019-10-16 Thread Thomas Gleixner
On Wed, 16 Oct 2019, Xiaoyao Li wrote: > On 10/16/2019 7:26 PM, Paolo Bonzini wrote: > > Old guests are prevalent enough that enabling split-lock detection by > > default would be a big usability issue. And even ignoring that, you > > would get the issue you describe below: > > Right, whether

RE: [PATCH v9 09/17] x86/split_lock: Handle #AC exception for split lock

2019-10-16 Thread David Laight
For the smt case, can you make #AC enable a property of the process? Then disable it on the core if either smt process requires it be disabled? This would mean that is a 'mixed environment' not all split accesses would actually generate #AC - but enough would to detect broken code that doesn't

Re: [PATCH v9 09/17] x86/split_lock: Handle #AC exception for split lock

2019-10-16 Thread Paolo Bonzini
On 16/10/19 15:51, Xiaoyao Li wrote: > On 10/16/2019 7:58 PM, Paolo Bonzini wrote: >> On 16/10/19 13:49, Thomas Gleixner wrote: >>> On Wed, 16 Oct 2019, Paolo Bonzini wrote: Yes it does.  But Sean's proposal, as I understand it, leads to the guest receiving #AC when it wasn't expecting

Re: [PATCH v9 09/17] x86/split_lock: Handle #AC exception for split lock

2019-10-16 Thread Xiaoyao Li
On 10/16/2019 7:58 PM, Paolo Bonzini wrote: On 16/10/19 13:49, Thomas Gleixner wrote: On Wed, 16 Oct 2019, Paolo Bonzini wrote: Yes it does. But Sean's proposal, as I understand it, leads to the guest receiving #AC when it wasn't expecting one. So for an old guest, as soon as the guest

Re: [PATCH v9 09/17] x86/split_lock: Handle #AC exception for split lock

2019-10-16 Thread Xiaoyao Li
On 10/16/2019 7:26 PM, Paolo Bonzini wrote: On 16/10/19 13:23, Xiaoyao Li wrote: KVM always traps #AC, and only advertises split-lock detection to guest when the global variable split_lock_detection_enabled in host is true. - If guest enables #AC (CPL3 alignment check or split-lock detection

Re: [PATCH v9 09/17] x86/split_lock: Handle #AC exception for split lock

2019-10-16 Thread Paolo Bonzini
On 16/10/19 13:49, Thomas Gleixner wrote: > On Wed, 16 Oct 2019, Paolo Bonzini wrote: >> Yes it does. But Sean's proposal, as I understand it, leads to the >> guest receiving #AC when it wasn't expecting one. So for an old guest, >> as soon as the guest kernel happens to do a split lock, it gets

Re: [PATCH v9 09/17] x86/split_lock: Handle #AC exception for split lock

2019-10-16 Thread Thomas Gleixner
On Wed, 16 Oct 2019, Paolo Bonzini wrote: > On 16/10/19 11:47, Thomas Gleixner wrote: > > On Wed, 16 Oct 2019, Paolo Bonzini wrote: > >> Just never advertise split-lock > >> detection to guests. If the host has enabled split-lock detection, > >> trap #AC and forward it to the host handler---which

Re: [PATCH v9 09/17] x86/split_lock: Handle #AC exception for split lock

2019-10-16 Thread Paolo Bonzini
On 16/10/19 13:23, Xiaoyao Li wrote: > KVM always traps #AC, and only advertises split-lock detection to guest > when the global variable split_lock_detection_enabled in host is true. > > - If guest enables #AC (CPL3 alignment check or split-lock detection > enabled), injecting #AC back into

Re: [PATCH v9 09/17] x86/split_lock: Handle #AC exception for split lock

2019-10-16 Thread Xiaoyao Li
On 10/16/2019 6:16 PM, Paolo Bonzini wrote: On 16/10/19 11:47, Thomas Gleixner wrote: On Wed, 16 Oct 2019, Paolo Bonzini wrote: Just never advertise split-lock detection to guests. If the host has enabled split-lock detection, trap #AC and forward it to the host handler---which would disable

Re: [PATCH v9 09/17] x86/split_lock: Handle #AC exception for split lock

2019-10-16 Thread Paolo Bonzini
On 16/10/19 11:47, Thomas Gleixner wrote: > On Wed, 16 Oct 2019, Paolo Bonzini wrote: >> Just never advertise split-lock >> detection to guests. If the host has enabled split-lock detection, >> trap #AC and forward it to the host handler---which would disable >> split lock detection globally and

Re: [PATCH v9 09/17] x86/split_lock: Handle #AC exception for split lock

2019-10-16 Thread Thomas Gleixner
On Wed, 16 Oct 2019, Paolo Bonzini wrote: > On 25/09/19 20:09, Sean Christopherson wrote: > > - Remove KVM loading of MSR_TEST_CTRL, i.e. KVM *never* writes the CPU's > > actual MSR_TEST_CTRL. KVM still emulates MSR_TEST_CTRL so that the > > guest can do WRMSR and handle its own #AC

Re: [PATCH v9 09/17] x86/split_lock: Handle #AC exception for split lock

2019-10-16 Thread Paolo Bonzini
On 25/09/19 20:09, Sean Christopherson wrote: > We're trying to sort out the trainwreck, but there's an additional wrinkle > that I'd like your input on. That's not exactly a wrinkle... > - Remove KVM loading of MSR_TEST_CTRL, i.e. KVM *never* writes the CPU's > actual MSR_TEST_CTRL. KVM

Re: [PATCH v9 09/17] x86/split_lock: Handle #AC exception for split lock

2019-10-16 Thread Thomas Gleixner
Sean, On Wed, 25 Sep 2019, Sean Christopherson wrote: sorry for the late reply. This got lost in travel/conferencing/vacation induced backlog. > On Wed, Jun 26, 2019 at 11:47:40PM +0200, Thomas Gleixner wrote: > > So only one of the CPUs will win the cmpxchg race, set te variable to 1 and > >

Re: [PATCH v9 09/17] x86/split_lock: Handle #AC exception for split lock

2019-10-16 Thread Xiaoyao Li
On 9/26/2019 2:09 AM, Sean Christopherson wrote: On Wed, Jun 26, 2019 at 11:47:40PM +0200, Thomas Gleixner wrote: So only one of the CPUs will win the cmpxchg race, set te variable to 1 and warn, the other and any subsequent AC on any other CPU will not warn either. So you don't need

Re: [PATCH v9 09/17] x86/split_lock: Handle #AC exception for split lock

2019-09-25 Thread Sean Christopherson
On Wed, Jun 26, 2019 at 11:47:40PM +0200, Thomas Gleixner wrote: > So only one of the CPUs will win the cmpxchg race, set te variable to 1 and > warn, the other and any subsequent AC on any other CPU will not warn > either. So you don't need WARN_ONCE() at all. It's redundant and confusing > along

Re: [PATCH v9 09/17] x86/split_lock: Handle #AC exception for split lock

2019-06-26 Thread Thomas Gleixner
On Wed, 26 Jun 2019, Fenghua Yu wrote: > On Wed, Jun 26, 2019 at 10:20:05PM +0200, Thomas Gleixner wrote: > > On Tue, 18 Jun 2019, Fenghua Yu wrote: > > > + > > > +static atomic_t split_lock_debug; > > > + > > > +void split_lock_disable(void) > > > +{ > > > + /* Disable split lock detection on

Re: [PATCH v9 09/17] x86/split_lock: Handle #AC exception for split lock

2019-06-26 Thread Fenghua Yu
On Wed, Jun 26, 2019 at 10:20:05PM +0200, Thomas Gleixner wrote: > On Tue, 18 Jun 2019, Fenghua Yu wrote: > > + > > +static atomic_t split_lock_debug; > > + > > +void split_lock_disable(void) > > +{ > > + /* Disable split lock detection on this CPU */ > > + this_cpu_and(msr_test_ctl_cached,

Re: [PATCH v9 09/17] x86/split_lock: Handle #AC exception for split lock

2019-06-26 Thread Thomas Gleixner
On Tue, 18 Jun 2019, Fenghua Yu wrote: > + > +static atomic_t split_lock_debug; > + > +void split_lock_disable(void) > +{ > + /* Disable split lock detection on this CPU */ > + this_cpu_and(msr_test_ctl_cached, ~MSR_TEST_CTL_SPLIT_LOCK_DETECT); > + wrmsrl(MSR_TEST_CTL,

[PATCH v9 09/17] x86/split_lock: Handle #AC exception for split lock

2019-06-18 Thread Fenghua Yu
There may be different considerations on how to handle #AC for split lock, e.g. how to handle system hang caused by split lock issue in firmware, how to emulate faulting instruction, etc. We use a simple method to handle user and kernel split lock and may extend the method in the future. When #AC