Hi Kees, On Tue, Apr 23, 2019 at 04:26:22PM -0700, Kees Cook wrote: > Clang's integrated assembler does not allow assembly macros defined > in one inline asm block using the .macro directive to be used across > separate asm blocks. LLVM developers consider this a feature and not a > bug, recommending code refactoring: > > https://bugs.llvm.org/show_bug.cgi?id=19749 > > As binutils doesn't allow macros to be redefined, this change uses > UNDEFINE_MRS_S and UNDEFINE_MSR_S to define corresponding macros > in-place and workaround gcc and clang limitations on redefining macros > across different assembler blocks.
[...] > diff --git a/arch/arm64/include/asm/irqflags.h > b/arch/arm64/include/asm/irqflags.h > index 43d8366c1e87..d359f28765cb 100644 > --- a/arch/arm64/include/asm/irqflags.h > +++ b/arch/arm64/include/asm/irqflags.h > @@ -43,7 +43,7 @@ static inline void arch_local_irq_enable(void) > asm volatile(ALTERNATIVE( > "msr daifclr, #2 // arch_local_irq_enable\n" > "nop", > - "msr_s " __stringify(SYS_ICC_PMR_EL1) ",%0\n" > + __msr_s(SYS_ICC_PMR_EL1) Seeing the __stringify() go is nice! Our assembly happens to use argument 0 by coincidence rather than by deliberate design, so please have the macro take the template, e.g. __msr_s(SYS_ICC_PMR_EL1, "%x0") ... that ways it's obvious as to which asm parameter is used, and this won't complicate refactoring of the assembly (which may shuffle or rename the parameters). Likewise for __mrs_s(), e.g. __mrs_s("%x[va]", SYS_WHATEVER); [...] > +#define __mrs_s(r) \ > + DEFINE_MRS_S \ > +" mrs_s %0, " __stringify(r) "\n" \ > + UNDEFINE_MRS_S > + > +#define __msr_s(r) \ > + DEFINE_MSR_S \ > +" msr_s " __stringify(r) ", %0\n" \ > + UNDEFINE_MSR_S These can be: #define __mrs_s(v, r) \ DEFINE_MRS_S \ " mrs_s " v ", " __stringify(r) "\n" \ UNDEFINE_MRS_S #define __msr_s(r, v) \ DEFINE_MSR_S \ " msr_s " __stringify(r) ", " v "\n" \ UNDEFINE_MSR_S > + > +/* For use with "rZ" constraints. */ > +#define __msr_s_x0(r) \ > + DEFINE_MSR_S \ > +" msr_s " __stringify(r) ", %x0\n" \ > + UNDEFINE_MSR_S I don't think we need this. If we pass the template in, this is solved by the caller, and we could consistently use the x form regardless. Otherwise, I think this is as clean as we can make this short of bumping our minimum binutils version and using the S<Op0>_<...> names. Thanks, Mark.