RE: objtool clac/stac handling change..

2020-07-13 Thread David Laight
From: Linus Torvalds > Sent: 10 July 2020 23:37 > On Tue, Jul 7, 2020 at 5:35 AM David Laight wrote: > > > > > > So separate copy and checksum passes should easily exceed 4 bytes/clock, > > but I suspect that doing them together never does. > > (Unless the buffer is too big for the L1 cache.) >

Re: objtool clac/stac handling change..

2020-07-10 Thread Linus Torvalds
On Tue, Jul 7, 2020 at 5:35 AM David Laight wrote: > > > So separate copy and checksum passes should easily exceed 4 bytes/clock, > but I suspect that doing them together never does. > (Unless the buffer is too big for the L1 cache.) Its' the "touch the caches twice" that is the problem". And

RE: objtool clac/stac handling change..

2020-07-07 Thread David Laight
From: Al Viro > Sent: 04 July 2020 03:12 ... > BTW, looking at csum_and_copy_{to,from}_user() callers (all 3 of them, > all in lib/iov_iter.c) we have this: > 1) len is never 0 > 2) sum (initial value of csum) is always 0 > 3) failure (reported via *err_ptr) is always treateds as

Re: objtool clac/stac handling change..

2020-07-03 Thread Linus Torvalds
On Fri, Jul 3, 2020 at 7:30 PM Al Viro wrote: > > Lovely... So basically this is the rare place where we might use those > insns on userland addresses? Honestly, I think the code you quote is just confused. First off, we have special "is this page fault due to a prefetch" logic in the x86 page

Re: objtool clac/stac handling change..

2020-07-03 Thread Al Viro
On Fri, Jul 03, 2020 at 06:54:15PM -0700, Linus Torvalds wrote: > "Software Prefetches May Report A Page Fault > > Description Software prefetch instructions are defined to ignore > page faults. Under highly specific and detailed internal > circumstances, a prefetch instruction may report a

Re: objtool clac/stac handling change..

2020-07-03 Thread Al Viro
On Sat, Jul 04, 2020 at 01:49:59AM +0100, Al Viro wrote: > On Fri, Jul 03, 2020 at 10:02:37PM +0100, Al Viro wrote: > > > PS: I'm still going through the _ASM_EXTABLE... users on x86, so there > > might be more fun. Will post when I'm done... > > Lovely... Not directly related to that, but...

Re: objtool clac/stac handling change..

2020-07-03 Thread Linus Torvalds
On Fri, Jul 3, 2020 at 5:50 PM Al Viro wrote: > > How could prefetcht0 possibly > raise an exception? Intel manual says that the only exception is #UD if > LOCK PREFETCHT0 is encountered; not here, obviously. AMD manual simply > says "no exceptions". Confused... Several CPU bugs in this

Re: objtool clac/stac handling change..

2020-07-03 Thread Al Viro
On Fri, Jul 03, 2020 at 10:02:37PM +0100, Al Viro wrote: > PS: I'm still going through the _ASM_EXTABLE... users on x86, so there > might be more fun. Will post when I'm done... Lovely... Not directly related to that, but... WTF? arch/x86/lib/csum-copy_64.S: /* * No

Re: objtool clac/stac handling change..

2020-07-03 Thread Al Viro
On Fri, Jul 03, 2020 at 02:41:43PM -0700, Andy Lutomirski wrote: > I still feel like the ex_handler-automatically-does-CLAC thing is an > optimization that isn't worth it. Once we pull our heads out of the > giant pile of macros and inlined functions, we're talking about > changing: > clac;

Re: objtool clac/stac handling change..

2020-07-03 Thread Al Viro
On Fri, Jul 03, 2020 at 02:10:08PM -0700, Linus Torvalds wrote: > Yeah, the "stac" instruction isn't hugely fast, and serializes the > pipeline, so it's a nasty 20 cycles or something. > > But for chissake, this > (a) happens approximately never > (b) is after a fault that took a thousand

Re: objtool clac/stac handling change..

2020-07-03 Thread Al Viro
On Fri, Jul 03, 2020 at 10:59:22PM +0100, Al Viro wrote: > On Fri, Jul 03, 2020 at 02:10:08PM -0700, Linus Torvalds wrote: > > On Fri, Jul 3, 2020 at 2:02 PM Al Viro wrote: > > > > > > Actually, for more serious problem consider arch/x86/lib/copy_user_64.S > > > > What? No. > > > > > In case of

Re: objtool clac/stac handling change..

2020-07-03 Thread Al Viro
On Fri, Jul 03, 2020 at 02:10:08PM -0700, Linus Torvalds wrote: > On Fri, Jul 3, 2020 at 2:02 PM Al Viro wrote: > > > > Actually, for more serious problem consider arch/x86/lib/copy_user_64.S > > What? No. > > > In case of an unhandled fault on attempt to read an (unaligned) word, > > the damn

Re: objtool clac/stac handling change..

2020-07-03 Thread Andy Lutomirski
On Fri, Jul 3, 2020 at 2:10 PM Linus Torvalds wrote: > > On Fri, Jul 3, 2020 at 2:02 PM Al Viro wrote: > > > > Actually, for more serious problem consider arch/x86/lib/copy_user_64.S > > What? No. > > > In case of an unhandled fault on attempt to read an (unaligned) word, > > the damn thing

Re: objtool clac/stac handling change..

2020-07-03 Thread Linus Torvalds
On Fri, Jul 3, 2020 at 2:02 PM Al Viro wrote: > > Actually, for more serious problem consider arch/x86/lib/copy_user_64.S What? No. > In case of an unhandled fault on attempt to read an (unaligned) word, > the damn thing falls back to this: > SYM_CODE_START_LOCAL(.Lcopy_user_handle_tail) >

Re: objtool clac/stac handling change..

2020-07-03 Thread Al Viro
On Fri, Jul 03, 2020 at 02:33:28AM +0100, Al Viro wrote: > On Thu, Jul 02, 2020 at 02:55:19PM -0700, Linus Torvalds wrote: > > > And while XSTATE_OP() is still disgusting, it's > > > > (a) slightly less disgusting than it used to be > > > > (b) now easily fixable if we do the "exceptions

Re: objtool clac/stac handling change..

2020-07-02 Thread Christophe Leroy
Le 03/07/2020 à 05:17, Michael Ellerman a écrit : Christophe Leroy writes: Le 02/07/2020 à 15:34, Michael Ellerman a écrit : Linus Torvalds writes: On Wed, Jul 1, 2020 at 12:59 PM Al Viro wrote: On Wed, Jul 01, 2020 at 12:04:36PM -0700, Linus Torvalds wrote: That's actually for the

Re: objtool clac/stac handling change..

2020-07-02 Thread Michael Ellerman
Linus Torvalds writes: > On Thu, Jul 2, 2020 at 8:13 AM Christophe Leroy > wrote: >> >> Isn't it something easy to do in bad_page_fault() ? > > Can't the user access functions take any other faults on ppc? Yes they definitely can. I think I can enumerate all the possibilities on 64-bit, but I

Re: objtool clac/stac handling change..

2020-07-02 Thread Linus Torvalds
On Thu, Jul 2, 2020 at 6:33 PM Al Viro wrote: > > What about load_unaligned_zeropad()? What about it? It doesn't care. It's for kernel addresses, clearing AC on exception does nothing. There's no user_access_begin/end anywhere around that thing that I can see. Linus

Re: objtool clac/stac handling change..

2020-07-02 Thread Michael Ellerman
Christophe Leroy writes: > Le 02/07/2020 à 15:34, Michael Ellerman a écrit : >> Linus Torvalds writes: >>> On Wed, Jul 1, 2020 at 12:59 PM Al Viro wrote: On Wed, Jul 01, 2020 at 12:04:36PM -0700, Linus Torvalds wrote: > > That's actually for the access granting. Shutting the access

Re: objtool clac/stac handling change..

2020-07-02 Thread Al Viro
On Thu, Jul 02, 2020 at 02:55:19PM -0700, Linus Torvalds wrote: > And while XSTATE_OP() is still disgusting, it's > > (a) slightly less disgusting than it used to be > > (b) now easily fixable if we do the "exceptions clear AC" thing. > > so it's an improvement all around. > > If it works,

Re: objtool clac/stac handling change..

2020-07-02 Thread Linus Torvalds
On Thu, Jul 2, 2020 at 1:59 PM Al Viro wrote: > > I'm not sure it's the right solution in this case. Look at the call chain > and the stuff done nearby (that __clear_user(), for example)... > > I'm not saying that this code is not awful - it certainly is. But it's > not that simple,

Re: objtool clac/stac handling change..

2020-07-02 Thread Al Viro
On Thu, Jul 02, 2020 at 01:32:34PM -0700, Linus Torvalds wrote: > Ugh, the above is bad anyway. > > It doesn't use _ASM_EXTABLE_UA, so it won't warn about the noncanonical cases. FWIW, the address is inside a sigframe we decided to build, so noncanonical addresses shouldn't occur in practice.

Re: objtool clac/stac handling change..

2020-07-02 Thread Linus Torvalds
On Thu, Jul 2, 2020 at 1:18 PM Al Viro wrote: > > stac(); > XSTATE_OP(XSAVE, buf, -1, -1, err); > clac(); > > Rely upon objtool not noticing that we have, in effect, clac() in a state > where AC is already cleared? We could massage that thing to take a label, > but it

Re: objtool clac/stac handling change..

2020-07-02 Thread Al Viro
On Thu, Jul 02, 2020 at 12:52:27PM -0700, Linus Torvalds wrote: > On Thu, Jul 2, 2020 at 6:32 AM Michael Ellerman wrote: > > > > Probably the simplest option for us is to just handle it in our > > unsafe_op_wrap(). I'll try and come up with something tomorrow. > > IMy suggestion was to basically

Re: objtool clac/stac handling change..

2020-07-02 Thread Linus Torvalds
On Thu, Jul 2, 2020 at 8:13 AM Christophe Leroy wrote: > > Isn't it something easy to do in bad_page_fault() ? Can't the user access functions take any other faults on ppc? On x86-64, we have the "address is non-canonical" case which doesn't take a page fault at all, but takes a general

Re: objtool clac/stac handling change..

2020-07-02 Thread Linus Torvalds
On Thu, Jul 2, 2020 at 6:32 AM Michael Ellerman wrote: > > Probably the simplest option for us is to just handle it in our > unsafe_op_wrap(). I'll try and come up with something tomorrow. IMy suggestion was to basically just always handle it in all exception cases. And note that IU don't mean

Re: objtool clac/stac handling change..

2020-07-02 Thread Christophe Leroy
Le 02/07/2020 à 15:34, Michael Ellerman a écrit : Linus Torvalds writes: On Wed, Jul 1, 2020 at 12:59 PM Al Viro wrote: On Wed, Jul 01, 2020 at 12:04:36PM -0700, Linus Torvalds wrote: That's actually for the access granting. Shutting the access down ends up always doing the same thing

Re: objtool clac/stac handling change..

2020-07-02 Thread Al Viro
On Thu, Jul 02, 2020 at 03:01:59PM +0100, Al Viro wrote: > On Thu, Jul 02, 2020 at 11:34:31PM +1000, Michael Ellerman wrote: > > > I think we can do something to make it work. > > > > We don't have an equivalent of x86's ex_handler_uaccess(), so it's not > > quite as easy as whacking a

Re: objtool clac/stac handling change..

2020-07-02 Thread Al Viro
On Thu, Jul 02, 2020 at 11:34:31PM +1000, Michael Ellerman wrote: > I think we can do something to make it work. > > We don't have an equivalent of x86's ex_handler_uaccess(), so it's not > quite as easy as whacking a user_access_end() in there. > > Probably the simplest option for us is to

Re: objtool clac/stac handling change..

2020-07-02 Thread Michael Ellerman
Linus Torvalds writes: > On Wed, Jul 1, 2020 at 12:59 PM Al Viro wrote: >> On Wed, Jul 01, 2020 at 12:04:36PM -0700, Linus Torvalds wrote: >> > >> > That's actually for the access granting. Shutting the access down ends >> > up always doing the same thing anyway.. >> >> #define

Re: objtool clac/stac handling change..

2020-07-02 Thread Peter Zijlstra
On Wed, Jul 01, 2020 at 07:00:41PM -0500, Josh Poimboeuf wrote: > On Wed, Jul 01, 2020 at 02:02:42PM -0700, Linus Torvalds wrote: > > So the objtool rule might be: > > > > - in a STAC region, no exception handlers at all except for that > > ex_handler_uaccess case > > > > - and that case will

Re: objtool clac/stac handling change..

2020-07-01 Thread Andy Lutomirski
> On Jul 1, 2020, at 7:30 PM, Linus Torvalds > wrote: > > On Wed, Jul 1, 2020 at 5:48 PM Andy Lutomirski wrote: >> >> You inspired me to mock it up. > > Ahh, you want to just use the jump folding of gcc to avoid the problem. > > I guess we could do that. Are there cases where this

Re: objtool clac/stac handling change..

2020-07-01 Thread Linus Torvalds
On Wed, Jul 1, 2020 at 7:30 PM Linus Torvalds wrote: > > I guess we could do that. Are there cases where this actually helps? Hmm. Thinking about it., using that approach might make the "CONFIG_CC_HAS_ASM_GOTO_OUTPUT" choices simpler to handle. With ASM_GOTO_OUTPUT it generates the perfect

Re: objtool clac/stac handling change..

2020-07-01 Thread Linus Torvalds
On Wed, Jul 1, 2020 at 5:48 PM Andy Lutomirski wrote: > > You inspired me to mock it up. Ahh, you want to just use the jump folding of gcc to avoid the problem. I guess we could do that. Are there cases where this actually helps? Linus

Re: objtool clac/stac handling change..

2020-07-01 Thread Andy Lutomirski
On Wed, Jul 1, 2020 at 1:51 PM Linus Torvalds wrote: > > On Wed, Jul 1, 2020 at 1:36 PM Andy Lutomirski wrote: > > > > We ought to be able to do it the way I described and get decent code > > generation too. > > No, we really can't. > > Each access really needs to jump to an exception label.

Re: objtool clac/stac handling change..

2020-07-01 Thread Josh Poimboeuf
On Wed, Jul 01, 2020 at 02:02:42PM -0700, Linus Torvalds wrote: > So the objtool rule might be: > > - in a STAC region, no exception handlers at all except for that > ex_handler_uaccess case > > - and that case will clear AC if it triggers. > > and maybe such an objtool check would show some

Re: objtool clac/stac handling change..

2020-07-01 Thread Linus Torvalds
On Wed, Jul 1, 2020 at 1:51 PM Josh Poimboeuf wrote: > > Yeah. Peter's more of the expert here, but I think we'd at least need > to annotate the code which expects an implicit CLAC so objtool knows > what to expect. It's not trivial, but it might be doable. In both C and asm code, it's the

Re: objtool clac/stac handling change..

2020-07-01 Thread Josh Poimboeuf
On Wed, Jul 01, 2020 at 01:36:22PM -0700, Andy Lutomirski wrote: > If we do this extable change, we end up with a different mess: some > exception handlers will clear AC and some won’t. I’m sure objtool can > deal with this with some effort, but I’m not convinced it’s worth it. Yeah. Peter's

Re: objtool clac/stac handling change..

2020-07-01 Thread Linus Torvalds
On Wed, Jul 1, 2020 at 1:36 PM Andy Lutomirski wrote: > > We ought to be able to do it the way I described and get decent code > generation too. No, we really can't. Each access really needs to jump to an exception label. Otherwise any time you have multiple operations (think "strncpy()" and

Re: objtool clac/stac handling change..

2020-07-01 Thread Andy Lutomirski
> On Jul 1, 2020, at 12:35 PM, Linus Torvalds > wrote: > > On Wed, Jul 1, 2020 at 11:29 AM Andy Lutomirski wrote: >> >> Do we really want the exception handling to do the CLAC? Having >> unsafe_get_user() do CLAC seems surprising to me, and it will break >> use cases like: >> >> if

Re: objtool clac/stac handling change..

2020-07-01 Thread Linus Torvalds
On Wed, Jul 1, 2020 at 12:59 PM Al Viro wrote: > > On Wed, Jul 01, 2020 at 12:04:36PM -0700, Linus Torvalds wrote: > > > > That's actually for the access granting. Shutting the access down ends > > up always doing the same thing anyway.. > > #define user_read_access_end

Re: objtool clac/stac handling change..

2020-07-01 Thread Al Viro
On Wed, Jul 01, 2020 at 12:04:36PM -0700, Linus Torvalds wrote: > On Wed, Jul 1, 2020 at 11:41 AM Al Viro wrote: > > > > Rather nasty for ppc; they have separate user_read_access_end() and > > user_write_access_end(). > > That's actually for the access granting. Shutting the access down ends >

Re: objtool clac/stac handling change..

2020-07-01 Thread Linus Torvalds
On Wed, Jul 1, 2020 at 11:29 AM Andy Lutomirski wrote: > > Do we really want the exception handling to do the CLAC? Having > unsafe_get_user() do CLAC seems surprising to me, and it will break > use cases like: > > if (!user_access_begin(...) > goto out; > > ret = unsafe_get_user(...); > >

Re: objtool clac/stac handling change..

2020-07-01 Thread Linus Torvalds
On Wed, Jul 1, 2020 at 11:41 AM Al Viro wrote: > > Rather nasty for ppc; they have separate user_read_access_end() and > user_write_access_end(). That's actually for the access granting. Shutting the access down ends up always doing the same thing anyway.. Linus

Re: objtool clac/stac handling change..

2020-07-01 Thread Al Viro
On Wed, Jul 01, 2020 at 11:22:01AM -0700, Linus Torvalds wrote: > Josh / PeterZ, > it turns out that clang seems to now have fixed the last known > nagging details with "asm goto" with outputs, so I'm looking at > actually trying to merge the support for that in the kernel. > > The main

Re: objtool clac/stac handling change..

2020-07-01 Thread Andy Lutomirski
On Wed, Jul 1, 2020 at 11:22 AM Linus Torvalds wrote: > > Josh / PeterZ, > it turns out that clang seems to now have fixed the last known > nagging details with "asm goto" with outputs, so I'm looking at > actually trying to merge the support for that in the kernel. > > The main annoyance isn't