[PATCH v2] x86: correct asm() constraints when dealing with immediate selector values

[PATCH v2] x86: correct asm() constraints when dealing with immediate selector values

[Jan Beulich]

asm() constraints need to fit both the intended insn(s) which the
respective operands are going to be used with as well as the actual kind
of value specified. "m" (alone) together with a constant, however, leads
to gcc saying

error: memory input <N> is not directly addressable

while clang complains

error: invalid lvalue in asm input for constraint 'm'

And rightly so - in order to access a memory operand, an address needs
to be specified to the insn. In some cases it might be possible for a
compiler to synthesize a memory operand holding the requested constant,
but I think any solution there would have sharp edges.

If "m" alone doesn't work with constants, it is at best pointless (and
perhaps misleading or even risky - the compiler might decide to actually
pick "m" and not try very hard to find a suitable register) to specify
it alongside "r". And indeed clang does, oddly enough despite its
objection to "m" alone. Which means there the change also improves the
generated code.

While there also switch the two operand case to using named operands.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
v2: Use named operands in do_double_fault().

--- a/xen/arch/x86/cpu/amd.c
+++ b/xen/arch/x86/cpu/amd.c
@@ -736,7 +736,7 @@ void __init detect_zen2_null_seg_behavio
 	uint64_t base;
 
 	wrmsrl(MSR_FS_BASE, 1);
-	asm volatile ( "mov %0, %%fs" :: "rm" (0) );
+	asm volatile ( "mov %0, %%fs" :: "r" (0) );
 	rdmsrl(MSR_FS_BASE, base);
 
 	if (base == 0)
--- a/xen/arch/x86/x86_64/traps.c
+++ b/xen/arch/x86/x86_64/traps.c
@@ -248,7 +248,8 @@ void do_double_fault(struct cpu_user_reg
 
     console_force_unlock();
 
-    asm ( "lsll %1, %0" : "=r" (cpu) : "rm" (PER_CPU_SELECTOR) );
+    asm ( "lsll %[sel], %[limit]" : [limit] "=r" (cpu)
+                                  : [sel] "r" (PER_CPU_SELECTOR) );
 
     /* Find information saved during fault and dump it to the console. */
     printk("*** DOUBLE FAULT ***\n");

Ping: [PATCH v2] x86: correct asm() constraints when dealing with immediate selector values

[Jan Beulich]

On 13.09.2021 08:26, Jan Beulich wrote:
> asm() constraints need to fit both the intended insn(s) which the
> respective operands are going to be used with as well as the actual kind
> of value specified. "m" (alone) together with a constant, however, leads
> to gcc saying
> 
> error: memory input <N> is not directly addressable
> 
> while clang complains
> 
> error: invalid lvalue in asm input for constraint 'm'
> 
> And rightly so - in order to access a memory operand, an address needs
> to be specified to the insn. In some cases it might be possible for a
> compiler to synthesize a memory operand holding the requested constant,
> but I think any solution there would have sharp edges.
> 
> If "m" alone doesn't work with constants, it is at best pointless (and
> perhaps misleading or even risky - the compiler might decide to actually
> pick "m" and not try very hard to find a suitable register) to specify
> it alongside "r". And indeed clang does, oddly enough despite its
> objection to "m" alone. Which means there the change also improves the
> generated code.
> 
> While there also switch the two operand case to using named operands.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> ---
> v2: Use named operands in do_double_fault().

This has been pending for over 9 months. May I ask for feedback?

Thanks, Jan

> --- a/xen/arch/x86/cpu/amd.c
> +++ b/xen/arch/x86/cpu/amd.c
> @@ -736,7 +736,7 @@ void __init detect_zen2_null_seg_behavio
>  	uint64_t base;
>  
>  	wrmsrl(MSR_FS_BASE, 1);
> -	asm volatile ( "mov %0, %%fs" :: "rm" (0) );
> +	asm volatile ( "mov %0, %%fs" :: "r" (0) );
>  	rdmsrl(MSR_FS_BASE, base);
>  
>  	if (base == 0)
> --- a/xen/arch/x86/x86_64/traps.c
> +++ b/xen/arch/x86/x86_64/traps.c
> @@ -248,7 +248,8 @@ void do_double_fault(struct cpu_user_reg
>  
>      console_force_unlock();
>  
> -    asm ( "lsll %1, %0" : "=r" (cpu) : "rm" (PER_CPU_SELECTOR) );
> +    asm ( "lsll %[sel], %[limit]" : [limit] "=r" (cpu)
> +                                  : [sel] "r" (PER_CPU_SELECTOR) );
>  
>      /* Find information saved during fault and dump it to the console. */
>      printk("*** DOUBLE FAULT ***\n");

Re: [PATCH v2] x86: correct asm() constraints when dealing with immediate selector values

[Roger Pau Monné]

On Mon, Sep 13, 2021 at 08:26:21AM +0200, Jan Beulich wrote:
> asm() constraints need to fit both the intended insn(s) which the
> respective operands are going to be used with as well as the actual kind
> of value specified. "m" (alone) together with a constant, however, leads
> to gcc saying
> 
> error: memory input <N> is not directly addressable
> 
> while clang complains
> 
> error: invalid lvalue in asm input for constraint 'm'
> 
> And rightly so - in order to access a memory operand, an address needs
> to be specified to the insn. In some cases it might be possible for a
> compiler to synthesize a memory operand holding the requested constant,
> but I think any solution there would have sharp edges.
> 
> If "m" alone doesn't work with constants, it is at best pointless (and
> perhaps misleading or even risky - the compiler might decide to actually
> pick "m" and not try very hard to find a suitable register) to specify
> it alongside "r". And indeed clang does, oddly enough despite its
> objection to "m" alone. Which means there the change also improves the
> generated code.
> 
> While there also switch the two operand case to using named operands.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Acked-by: Roger Pau Monné <roger.pau@citrix.com>

Thanks, and sorry for the delay.

Roger.