Re: [PATCH] x86/AMD: Convert rdmsr_amd_safe() to use asm goto()

[Jan Beulich <jbeulich@xxxxxxxx>]

On 07.04.2025 17:35, Andrew Cooper wrote:
> Unlike the WRMSR side, we can't use asm goto() unconditionally, because our
> toolchain baseline doesn't support asm goto with outputs.

Is there actually a benefit we gain from now needing to maintain two different
pieces of logic fulfilling the same purpose?

> Also, there's a different errata workaround we'll need if we want to use asm
> goto() with "+" constraints:
> 
> config CC_HAS_ASM_GOTO_TIED_OUTPUT
>       depends on CC_HAS_ASM_GOTO_OUTPUT
>       # Detect buggy gcc and clang, fixed in gcc-11 clang-14.
>       def_bool $(success,echo 'int foo(int *x) { asm goto (".long (%l[bar]) - 
> .": "+m"(*x) ::: bar); return *x; bar: return 0; }' | $CC -x c - -c -o 
> /dev/null)
> 
> I'm tempted to put it in straight away, lest we forget about it.

Perhaps best if we really want to go this route. Yet then - why "TIED"? Isn't
"tied" the term they use when referring to an earlier operand by using a
digit (or the operand's name in square brackets)?

> --- a/xen/Kconfig
> +++ b/xen/Kconfig
> @@ -41,6 +41,20 @@ config CC_SPLIT_SECTIONS
>  config CC_HAS_UBSAN
>       def_bool $(cc-option,-fsanitize=undefined)
>  
> +# Fixed in GCC 14, 13.3, 12.4 and 11.5
> +# https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113921
> +config GCC_ASM_GOTO_OUTPUT_BROKEN
> +     bool
> +     depends on CC_IS_GCC
> +     default y if GCC_VERSION < 110500
> +     default y if GCC_VERSION >= 120000 && GCC_VERSION < 120400
> +     default y if GCC_VERSION >= 130000 && GCC_VERSION < 130300

Unlike for pre-release versions (x.0.y) I view this as problematic. Distros
are likely to have backported the fix before the minor releases took place.
Or they may have backported without ever meaning to follow later minor
releases. We'd needlessly exclude them here. Imo ...

> +config CC_HAS_ASM_GOTO_OUTPUT
> +     def_bool y
> +     depends on !GCC_ASM_GOTO_OUTPUT_BROKEN
> +     depends on $(success,echo 'int foo(int x) { asm goto ("": "=r"(x) ::: 
> bar); return x; bar: return 0; }' | $(CC) -x c - -c -o /dev/null)

... the only option is to actually probe for support as well as the (non-)
buggy-ness.

Jan