+Nick
On Thu, Jan 27, 2022, Peter Zijlstra wrote:
> On Thu, Jan 27, 2022 at 06:36:19AM +0000, Sean Christopherson wrote:
> > On Thu, Jan 27, 2022, Sean Christopherson wrote:
> > > Doh, I should have specified that KVM needs 8-byte CMPXCHG on 32-bit kernels due
> > > to using it to atomically update guest PAE PTEs and LTR descriptors (yay).
> > >
> > > Also, KVM's use case isn't a tight loop, how gross would it be to add a slightly
> > > less unsafe version that does __uaccess_begin_nospec()? KVM pre-checks the address
> > > way ahead of time, so the access_ok() check can be omitted. Alternatively, KVM
> > > could add its own macro, but that seems a little silly. E.g. somethign like this,
> > > though I don't think this is correct
> >
> > *sigh*
> >
> > Finally realized I forgot to add back the page offset after converting from guest
> > page frame to host virtual address. Anyways, this is what I ended up with, will
> > test more tomorrow.
>
> Looks about right :-) (famous last words etc..)
And it was right, but clang-13 ruined the party :-/
clang barfs on asm goto with a "+m" input/output. Change the "+m" to "=m" and
clang is happy. Remove usage of the label, clang is happy.
I tried a bunch of different variants to see if anything would squeak by, but
clang found a way to die on everything I threw at it.
$ clang --version
Debian clang version 13.0.0-9+build1
Target: x86_64-pc-linux-gnu
Thread model: posix
InstalledDir: /usr/bin
As written, with a named label param, clang yields:
$ echo 'int foo(int *x) { asm goto (".long (%l[bar]) - .\n": "+m"(*x) ::: bar); return *x; bar: return 0; }' | clang -x c - -c -o /dev/null
<stdin>:1:29: error: invalid operand in inline asm: '.long (${1:l}) - .'
int foo(int *x) { asm goto (".long (%l[bar]) - .\n": "+m"(*x) ::: bar); return *x; bar: return 0; }
^
<stdin>:1:29: error: unknown token in expression
<inline asm>:1:9: note: instantiated into assembly here
.long () - .
^
2 errors generated.
While clang is perfectly happy switching "+m" to "=m":
$ echo 'int foo(int *x) { asm goto (".long (%l[bar]) - .\n": "=m"(*x) ::: bar); return *x; bar: return 0; }' | clang -x c - -c -o /dev/null
Referencing the label with a numbered param yields either the original error:
$ echo 'int foo(int *x) { asm goto (".long (%l1) - .\n": "+m"(*x) ::: bar); return *x; bar: return 0; }' | clang -x c - -c -o /dev/null
<stdin>:1:29: error: invalid operand in inline asm: '.long (${1:l}) - .'
int foo(int *x) { asm goto (".long (%l1) - .\n": "+m"(*x) ::: bar); return *x; bar: return 0; }
^
<stdin>:1:29: error: unknown token in expression
<inline asm>:1:9: note: instantiated into assembly here
.long () - .
^
2 errors generated.
Bumping the param number (more below) yields a different error (I tried defining
tmp1, that didn't work :-) ).
$ echo 'int foo(int *x) { asm goto (".long (%l2) - .\n": "+m"(*x) ::: bar); return *x; bar: return 0; }' | clang -x c - -c -o /dev/null
error: Undefined temporary symbol .Ltmp1
1 error generated.
Regarding the param number, gcc also appears to have a goof with asm goto and "+m",
but bumping the param number in that case remedies its woes.
$echo 'int foo(int *x) { asm goto (".long (%l1) - .\n": "+m"(*x) ::: bar); return *x; bar: return 0; }' | gcc -x c - -c -o /dev/null
<stdin>: In function ‘foo’:
<stdin>:1:19: error: invalid 'asm': '%l' operand isn't a label
$ echo 'int foo(int *x) { asm goto (".long (%l2) - .\n": "+m"(*x) ::: bar); return *x; bar: return 0; }' | gcc -x c - -c -o /dev/null
So my immediate question: how do we want to we deal with this in the kernel? Keeping
in mind that I'd really like to send this to stable@ to fix the KVM mess.
I can think of few options that are varying degrees of gross.
1) Use a more complex sequence for probing CC_HAS_ASM_GOTO_OUTPUT.
2) Use an output-only "=m" operand.
3) Use an input register param.
Option #1 has the obvious downside of the fancier asm goto for __get_user_asm()
and friends being collateral damage. The biggest benefit is it'd reduce the
likelihood of someone else having to debug similar errors, which was quite painful.
Options #2 and #3 are quite gross, but I _think_ would be ok since the sequence
is tagged as clobbering memory anyways?
