Re: [PATCH -tip] x86/asm: Use %a instead of %c(%%rip) in rip_rel_ptr()

[Ard Biesheuvel]

On Mon, 5 May 2025 at 04:59, H. Peter Anvin <hpa@xxxxxxxxx> wrote:
>
> On 5/4/25 11:43, Uros Bizjak wrote:
> > The "a" asm operand modifier substitutes a memory reference, with the
> > actual operand treated as address.  For x86_64, when a symbol is
> > provided, the "a" modifier emits "sym(%rip)" instead of "sym".
> >

Clang does not

https://godbolt.org/z/5Y58T45f5

> > No functional changes intended.
> >

NAK. There is a functional change with Clang, which does not emit
%(rip), and this is the whole point of this thing.

> > Signed-off-by: Uros Bizjak <ubizjak@xxxxxxxxx>
> > Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> > Cc: Ingo Molnar <mingo@xxxxxxxxxx>
> > Cc: Borislav Petkov <bp@xxxxxxxxx>
> > Cc: Dave Hansen <dave.hansen@xxxxxxxxxxxxxxx>
> > Cc: "H. Peter Anvin" <hpa@xxxxxxxxx>
> > Cc: Ard Biesheuvel <ardb@xxxxxxxxxx>
> > ---
> >   arch/x86/include/asm/asm.h | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/x86/include/asm/asm.h b/arch/x86/include/asm/asm.h
> > index f963848024a5..d7610b99b8d8 100644
> > --- a/arch/x86/include/asm/asm.h
> > +++ b/arch/x86/include/asm/asm.h
> > @@ -116,7 +116,7 @@
> >   #ifndef __ASSEMBLER__
> >   static __always_inline __pure void *rip_rel_ptr(void *p)
> >   {
> > -     asm("leaq %c1(%%rip), %0" : "=r"(p) : "i"(p));
> > +     asm("leaq %a1, %0" : "=r"(p) : "i"(p));
> >
> >       return p;
> >   }
>
> Also, this function really should be __attribute_const__ rather than __pure.
>

No it should really not.

rip_rel_ptr() will yield different results depending on whether it is
called [by the same code] from the 1:1 mapping or the ordinary kernel
virtual mapping of memory, and this is basically the entire reason we
need it in the first place.

Lying to the compiler about this is not a good idea imo.