RE: [PATCH v10 03/38] x86/msr: Add the WRMSRNS instruction support

2023-09-22 Thread Li, Xin3
> > I notice there are several call sites using the safe version w/o > > checking the return value, should the unsafe version be a better > > choice in such cases? > > Depends. The safe version does not emit a warning on fail. So if the > callsite truly does not care about the error it's fine. Ri

RE: [PATCH v10 03/38] x86/msr: Add the WRMSRNS instruction support

2023-09-22 Thread Thomas Gleixner
On Fri, Sep 22 2023 at 08:16, Xin3 Li wrote: >> > > +static __always_inline void __wrmsrns(u32 msr, u32 low, u32 high) >> > >> > Shouldn't this be named wrmsrns_safe since it has exception handling, >> > similar >> to >> > the current wrmsrl_safe. >> > >> >> Both safe and unsafe versions have exc

RE: [PATCH v10 03/38] x86/msr: Add the WRMSRNS instruction support

2023-09-22 Thread Li, Xin3
> > > +static __always_inline void __wrmsrns(u32 msr, u32 low, u32 high) > > > > Shouldn't this be named wrmsrns_safe since it has exception handling, > > similar > to > > the current wrmsrl_safe. > > > > Both safe and unsafe versions have exception handling, while the safe > version returns an i

Re: [PATCH v10 03/38] x86/msr: Add the WRMSRNS instruction support

2023-09-20 Thread Juergen Gross
On 20.09.23 17:00, Peter Zijlstra wrote: On Fri, Sep 15, 2023 at 02:16:50AM +0100, andrew.coop...@citrix.com wrote: Juergen has already done the work to delete one of these two patching mechanisms and replace it with the other. https://lore.kernel.org/lkml/a32e211f-4add-4fb2-9e5a-480ae9b9b...@

Re: [PATCH v10 03/38] x86/msr: Add the WRMSRNS instruction support

2023-09-20 Thread Peter Zijlstra
On Fri, Sep 15, 2023 at 02:16:50AM +0100, andrew.coop...@citrix.com wrote: > Juergen has already done the work to delete one of these two patching > mechanisms and replace it with the other. > > https://lore.kernel.org/lkml/a32e211f-4add-4fb2-9e5a-480ae9b9b...@suse.com/ > > Unfortunately, it's o

Re: [PATCH v10 03/38] x86/msr: Add the WRMSRNS instruction support

2023-09-20 Thread Nikolay Borisov
On 14.09.23 г. 7:47 ч., Xin Li wrote: Add an always inline API __wrmsrns() to embed the WRMSRNS instruction into the code. Tested-by: Shan Kang Signed-off-by: Xin Li --- arch/x86/include/asm/msr.h | 18 ++ 1 file changed, 18 insertions(+) diff --git a/arch/x86/include/as

RE: [PATCH v10 03/38] x86/msr: Add the WRMSRNS instruction support

2023-09-20 Thread Li, Xin3
> > +static __always_inline void __wrmsrns(u32 msr, u32 low, u32 high) > > Shouldn't this be named wrmsrns_safe since it has exception handling, similar > to > the current wrmsrl_safe. > Both safe and unsafe versions have exception handling, while the safe version returns an integer to its call

Re: [PATCH v10 03/38] x86/msr: Add the WRMSRNS instruction support

2023-09-14 Thread Juergen Gross
On 15.09.23 03:16, andrew.coop...@citrix.com wrote: On 15/09/2023 2:01 am, H. Peter Anvin wrote: The whole bit with alternatives and pvops being separate is a major maintainability problem, and honestly it never made any sense in the first place. Never have two mechanisms to do one job; it makes

Re: [PATCH v10 03/38] x86/msr: Add the WRMSRNS instruction support

2023-09-14 Thread H. Peter Anvin
On 9/14/23 18:46, andrew.coop...@citrix.com wrote: On 15/09/2023 1:38 am, H. Peter Anvin wrote: On 9/14/23 17:33, andrew.coop...@citrix.com wrote: It's an assumption about what "definitely won't" be paravirt in the future. XenPV stack handling is almost-FRED-like and has been for the better p

Re: [PATCH v10 03/38] x86/msr: Add the WRMSRNS instruction support

2023-09-14 Thread andrew . cooper3
On 15/09/2023 1:38 am, H. Peter Anvin wrote: > On 9/14/23 17:33, andrew.coop...@citrix.com wrote: >> >> It's an assumption about what "definitely won't" be paravirt in the >> future. >> >> XenPV stack handling is almost-FRED-like and has been for the better >> part of two decades. >> >> You frequen

Re: [PATCH v10 03/38] x86/msr: Add the WRMSRNS instruction support

2023-09-14 Thread andrew . cooper3
On 15/09/2023 2:01 am, H. Peter Anvin wrote: > The whole bit with alternatives and pvops being separate is a major > maintainability problem, and honestly it never made any sense in the > first place. Never have two mechanisms to do one job; it makes it > harder to grok their interactions. This bi

Re: [PATCH v10 03/38] x86/msr: Add the WRMSRNS instruction support

2023-09-14 Thread H. Peter Anvin
WRMSR has one complexity that most other PV-ops don't, and that's the exception table reference for the instruction itself. In a theoretical future ought to look like: mov$msr, %ecx mov$lo, %eax mov$hi, %edx 1: {call paravirt_blah(%rip) | cs...cs wrmsr | cs...cs wrmsr

Re: [PATCH v10 03/38] x86/msr: Add the WRMSRNS instruction support

2023-09-14 Thread Thomas Gleixner
On Fri, Sep 15 2023 at 00:46, andrew wrote: > On 15/09/2023 12:00 am, Thomas Gleixner wrote: > What I meant was "there should be the two top-level APIs, and under the > covers they DTRT".  Part of doing the right thing is to wire up paravirt > for configs where that is specified. > > Anything else

Re: [PATCH v10 03/38] x86/msr: Add the WRMSRNS instruction support

2023-09-14 Thread H. Peter Anvin
On 9/14/23 17:33, andrew.coop...@citrix.com wrote: It's an assumption about what "definitely won't" be paravirt in the future. XenPV stack handling is almost-FRED-like and has been for the better part of two decades. You frequently complain that there's too much black magic holding XenPV toget

Re: [PATCH v10 03/38] x86/msr: Add the WRMSRNS instruction support

2023-09-14 Thread andrew . cooper3
On 15/09/2023 1:12 am, Thomas Gleixner wrote: > On Fri, Sep 15 2023 at 00:46, andrew wrote: >> On 15/09/2023 12:00 am, Thomas Gleixner wrote: >>> So no. I'm fundamentally disagreeing with your recommendation. The way >>> forward is: >>> >>> 1) Provide the native variant for wrmsrns(), i.e. rename

Re: [PATCH v10 03/38] x86/msr: Add the WRMSRNS instruction support

2023-09-14 Thread Thomas Gleixner
On Fri, Sep 15 2023 at 00:46, andrew wrote: > On 15/09/2023 12:00 am, Thomas Gleixner wrote: >> So no. I'm fundamentally disagreeing with your recommendation. The way >> forward is: >> >> 1) Provide the native variant for wrmsrns(), i.e. rename the proposed >> wrmsrns() to native_wrmsr_ns()

Re: [PATCH v10 03/38] x86/msr: Add the WRMSRNS instruction support

2023-09-14 Thread andrew . cooper3
On 15/09/2023 12:00 am, Thomas Gleixner wrote: >> and despite what Juergen said, I'm going to recommend that you do wire >> this through the paravirt infrastructure, for the benefit of regular >> users having a nice API, not because XenPV is expecting to do something >> wildly different here. > I f

Re: [PATCH v10 03/38] x86/msr: Add the WRMSRNS instruction support

2023-09-14 Thread H. Peter Anvin
On September 14, 2023 4:00:39 PM PDT, Thomas Gleixner wrote: >Andrew! > >On Thu, Sep 14 2023 at 15:05, andrew wrote: >> On 14/09/2023 5:47 am, Xin Li wrote: >>> +static __always_inline void wrmsrns(u32 msr, u64 val) >>> +{ >>> + __wrmsrns(msr, val, val >> 32); >>> +} >> >> This API works in ter

Re: [PATCH v10 03/38] x86/msr: Add the WRMSRNS instruction support

2023-09-14 Thread Thomas Gleixner
Andrew! On Thu, Sep 14 2023 at 15:05, andrew wrote: > On 14/09/2023 5:47 am, Xin Li wrote: >> +static __always_inline void wrmsrns(u32 msr, u64 val) >> +{ >> +__wrmsrns(msr, val, val >> 32); >> +} > > This API works in terms of this series where every WRMSRNS is hidden > behind a FRED check, b

Re: [PATCH v10 03/38] x86/msr: Add the WRMSRNS instruction support

2023-09-14 Thread andrew . cooper3
On 14/09/2023 5:47 am, Xin Li wrote: > Add an always inline API __wrmsrns() to embed the WRMSRNS instruction > into the code. > > Tested-by: Shan Kang > Signed-off-by: Xin Li > --- > arch/x86/include/asm/msr.h | 18 ++ > 1 file changed, 18 insertions(+) > > diff --git a/arch/x86/

Re: [PATCH v10 03/38] x86/msr: Add the WRMSRNS instruction support

2023-09-14 Thread andrew . cooper3
On 14/09/2023 7:02 am, Juergen Gross wrote: > On 14.09.23 06:47, Xin Li wrote: >> Add an always inline API __wrmsrns() to embed the WRMSRNS instruction >> into the code. >> >> Tested-by: Shan Kang >> Signed-off-by: Xin Li > > In order to avoid having to add paravirt support for WRMSRNS I think >

Re: [PATCH v10 03/38] x86/msr: Add the WRMSRNS instruction support

2023-09-13 Thread Juergen Gross
On 14.09.23 06:47, Xin Li wrote: Add an always inline API __wrmsrns() to embed the WRMSRNS instruction into the code. Tested-by: Shan Kang Signed-off-by: Xin Li In order to avoid having to add paravirt support for WRMSRNS I think xen_init_capabilities() should gain: + setup_clear_cpu_