Hi Linus,
Am 08.07.2021 um 15:01 schrieb Linus Torvalds:
On Wed, Jul 7, 2021 at 7:51 PM Linus Torvalds
<torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:
Ok, strange. If this works for you, then the _concept_ is fine, and
there's something odd going on with the "macros with 'moves' vs 'move'
as a parameter" thing.
Oh, I think I see the problem.
Or rather, I see it in Christoph's version of the patch, I don't think
I've seen Michael's version, but that one was apparently very similar,
so maybe it has the same bug.
Christoph does:
#define __put_user_asm(inst, res, x, ptr, bwl, reg) \
asm volatile ("\n" \
"1: #inst."#bwl" %2,%1\n"
...
and then uses it with code like
__put_user_asm(MOVES, __pu_err, __pu_val, ptr, b, d);
and
__get_user_asm("move", __gk_err, __gk_dst, __gk_src, u8, b, d);
and the issue is that there's a '#' too many.
The '#' turns the argument into a string, but it was already
_supposed_ to be a string. But no, the problem is that it turns the
macro name MOVES into the _string_ "MOVES".
Which happens to compile just fine, because "moves" is a real
instruction. But it's actually _meant_ to be a macro that expands to
either the string "moves" or "move".
Yes, that's where I got to (trying to implement what you suggested a few
days ago, after the version I just sent) and hit a wall. What I meant by
'getting overly smart with the preprocessor'.
So what happens is that at least in Christoph's version, I think the
code _always_ uses "MOVES", even in configurations where the macro
MOVES should have just become "move".
So it all builds fine, looks fine to the assembler, but it uses the
wrong instruction.
Yep - that was easy enough to test - never mind what you define MOVES
as, that does not really matter. It always turns out 'moves'.
Macro expansion with the '#' character and other macros used as
arguments is something people need to be very careful with. It's why
we have a whole header file for just the "stringify" operation, see
<linux/stringify.h>
But in this case, it shouldn't have used '#' at all, since the
argument was already a string, and never needed to be turned into a
string by the pre-processor.
Testing that option next... and works!. Though I still think Christoph's
version is much cleaner...
Cheers,
Michael
Linus
