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". 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. 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. Linus