Re: [PATCH 1/3] util/cpuinfo-riscv: Support host/cpuinfo.h for riscv

[Philippe Mathieu-Daudé]

On 27/6/24 20:03, Richard Henderson wrote:
> Move detection code out of tcg, similar to other hosts.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   host/include/riscv/host/cpuinfo.h | 23 +++++++++
>   tcg/riscv/tcg-target.h            | 46 ++++++++---------
>   util/cpuinfo-riscv.c              | 85 +++++++++++++++++++++++++++++++
>   tcg/riscv/tcg-target.c.inc        | 84 +++---------------------------
>   util/meson.build                  |  2 +
>   5 files changed, 139 insertions(+), 101 deletions(-)
>   create mode 100644 host/include/riscv/host/cpuinfo.h
>   create mode 100644 util/cpuinfo-riscv.c


> +/* Called both as constructor and (possibly) via other constructors. */
> +unsigned __attribute__((constructor)) cpuinfo_init(void)
> +{
> +    unsigned left = CPUINFO_ZBA | CPUINFO_ZBB | CPUINFO_ZICOND;
> +    unsigned info = cpuinfo;
> +
> +    if (info) {
> +        return info;
> +    }
> +
> +    /* Test for compile-time settings. */
> +#if defined(__riscv_arch_test) && defined(__riscv_zba)
> +    info |= CPUINFO_ZBA;
> +#endif
> +#if defined(__riscv_arch_test) && defined(__riscv_zbb)
> +    info |= CPUINFO_ZBB;
> +#endif
> +#if defined(__riscv_arch_test) && defined(__riscv_zicond)
> +    info |= CPUINFO_ZICOND;
> +#endif
> +    left &= ~info;
> +
> +    if (left) {
> +        struct sigaction sa_old, sa_new;
> +
> +        memset(&sa_new, 0, sizeof(sa_new));
> +        sa_new.sa_flags = SA_SIGINFO;
> +        sa_new.sa_sigaction = sigill_handler;
> +        sigaction(SIGILL, &sa_new, &sa_old);
> +
> +        if (left & CPUINFO_ZBA) {
> +            /* Probe for Zba: add.uw zero,zero,zero. */
> +            got_sigill = 0;
> +            asm volatile(".insn r 0x3b, 0, 0x04, zero, zero, zero"
> +                         : : : "memory");
> +            info |= !got_sigill * CPUINFO_ZBA;

A bit too optimized to my taste, 'if (sigill) info|=ZBA' would be 
simpler to follow. Otherwise,

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

> +            left &= ~CPUINFO_ZBA;
> +        }