Re: [PATCH v3 7/8] linux-user: Simplify host <-> target errno conversion using macros

[Laurent Vivier]

Le 08/07/2021 à 19:05, Philippe Mathieu-Daudé a écrit :
> Convert the host_to_target_errno_table[] array to a switch
> case to allow compiler optimizations (such noticing the identity
> function when host and guest errnos match). Extract the errnos
> list as to a new includible unit, using a generic macro. Remove
> the code related to target_to_host_errno_table[] initialization.
> 
> Suggested-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  linux-user/syscall.c    | 159 ++++------------------------------------
>  linux-user/errnos.c.inc | 140 +++++++++++++++++++++++++++++++++++
>  2 files changed, 154 insertions(+), 145 deletions(-)
>  create mode 100644 linux-user/errnos.c.inc
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 4842a1987b7..94ec6f730b3 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -509,150 +509,26 @@ static inline int next_free_host_timer(void)
>  
>  #define ERRNO_TABLE_SIZE 1200
>  
> -/* target_to_host_errno_table[] is initialized from
> - * host_to_target_errno_table[] in syscall_init(). */
> -static uint16_t target_to_host_errno_table[ERRNO_TABLE_SIZE] = {
> -};
> -
> -/*
> - * This list is the union of errno values overridden in asm-<arch>/errno.h
> - * minus the errnos that are not actually generic to all archs.
> - */
> -static uint16_t host_to_target_errno_table[ERRNO_TABLE_SIZE] = {
> -    [EAGAIN]            = TARGET_EAGAIN,
> -    [EIDRM]             = TARGET_EIDRM,
> -    [ECHRNG]            = TARGET_ECHRNG,
> -    [EL2NSYNC]          = TARGET_EL2NSYNC,
> -    [EL3HLT]            = TARGET_EL3HLT,
> -    [EL3RST]            = TARGET_EL3RST,
> -    [ELNRNG]            = TARGET_ELNRNG,
> -    [EUNATCH]           = TARGET_EUNATCH,
> -    [ENOCSI]            = TARGET_ENOCSI,
> -    [EL2HLT]            = TARGET_EL2HLT,
> -    [EDEADLK]           = TARGET_EDEADLK,
> -    [ENOLCK]            = TARGET_ENOLCK,
> -    [EBADE]             = TARGET_EBADE,
> -    [EBADR]             = TARGET_EBADR,
> -    [EXFULL]            = TARGET_EXFULL,
> -    [ENOANO]            = TARGET_ENOANO,
> -    [EBADRQC]           = TARGET_EBADRQC,
> -    [EBADSLT]           = TARGET_EBADSLT,
> -    [EBFONT]            = TARGET_EBFONT,
> -    [ENOSTR]            = TARGET_ENOSTR,
> -    [ENODATA]           = TARGET_ENODATA,
> -    [ETIME]             = TARGET_ETIME,
> -    [ENOSR]             = TARGET_ENOSR,
> -    [ENONET]            = TARGET_ENONET,
> -    [ENOPKG]            = TARGET_ENOPKG,
> -    [EREMOTE]           = TARGET_EREMOTE,
> -    [ENOLINK]           = TARGET_ENOLINK,
> -    [EADV]              = TARGET_EADV,
> -    [ESRMNT]            = TARGET_ESRMNT,
> -    [ECOMM]             = TARGET_ECOMM,
> -    [EPROTO]            = TARGET_EPROTO,
> -    [EDOTDOT]           = TARGET_EDOTDOT,
> -    [EMULTIHOP]         = TARGET_EMULTIHOP,
> -    [EBADMSG]           = TARGET_EBADMSG,
> -    [ENAMETOOLONG]      = TARGET_ENAMETOOLONG,
> -    [EOVERFLOW]         = TARGET_EOVERFLOW,
> -    [ENOTUNIQ]          = TARGET_ENOTUNIQ,
> -    [EBADFD]            = TARGET_EBADFD,
> -    [EREMCHG]           = TARGET_EREMCHG,
> -    [ELIBACC]           = TARGET_ELIBACC,
> -    [ELIBBAD]           = TARGET_ELIBBAD,
> -    [ELIBSCN]           = TARGET_ELIBSCN,
> -    [ELIBMAX]           = TARGET_ELIBMAX,
> -    [ELIBEXEC]          = TARGET_ELIBEXEC,
> -    [EILSEQ]            = TARGET_EILSEQ,
> -    [ENOSYS]            = TARGET_ENOSYS,
> -    [ELOOP]             = TARGET_ELOOP,
> -    [ERESTART]          = TARGET_ERESTART,
> -    [ESTRPIPE]          = TARGET_ESTRPIPE,
> -    [ENOTEMPTY]         = TARGET_ENOTEMPTY,
> -    [EUSERS]            = TARGET_EUSERS,
> -    [ENOTSOCK]          = TARGET_ENOTSOCK,
> -    [EDESTADDRREQ]      = TARGET_EDESTADDRREQ,
> -    [EMSGSIZE]          = TARGET_EMSGSIZE,
> -    [EPROTOTYPE]        = TARGET_EPROTOTYPE,
> -    [ENOPROTOOPT]       = TARGET_ENOPROTOOPT,
> -    [EPROTONOSUPPORT]   = TARGET_EPROTONOSUPPORT,
> -    [ESOCKTNOSUPPORT]   = TARGET_ESOCKTNOSUPPORT,
> -    [EOPNOTSUPP]        = TARGET_EOPNOTSUPP,
> -    [EPFNOSUPPORT]      = TARGET_EPFNOSUPPORT,
> -    [EAFNOSUPPORT]      = TARGET_EAFNOSUPPORT,
> -    [EADDRINUSE]        = TARGET_EADDRINUSE,
> -    [EADDRNOTAVAIL]     = TARGET_EADDRNOTAVAIL,
> -    [ENETDOWN]          = TARGET_ENETDOWN,
> -    [ENETUNREACH]       = TARGET_ENETUNREACH,
> -    [ENETRESET]         = TARGET_ENETRESET,
> -    [ECONNABORTED]      = TARGET_ECONNABORTED,
> -    [ECONNRESET]        = TARGET_ECONNRESET,
> -    [ENOBUFS]           = TARGET_ENOBUFS,
> -    [EISCONN]           = TARGET_EISCONN,
> -    [ENOTCONN]          = TARGET_ENOTCONN,
> -    [EUCLEAN]           = TARGET_EUCLEAN,
> -    [ENOTNAM]           = TARGET_ENOTNAM,
> -    [ENAVAIL]           = TARGET_ENAVAIL,
> -    [EISNAM]            = TARGET_EISNAM,
> -    [EREMOTEIO]         = TARGET_EREMOTEIO,
> -    [EDQUOT]            = TARGET_EDQUOT,
> -    [ESHUTDOWN]         = TARGET_ESHUTDOWN,
> -    [ETOOMANYREFS]      = TARGET_ETOOMANYREFS,
> -    [ETIMEDOUT]         = TARGET_ETIMEDOUT,
> -    [ECONNREFUSED]      = TARGET_ECONNREFUSED,
> -    [EHOSTDOWN]         = TARGET_EHOSTDOWN,
> -    [EHOSTUNREACH]      = TARGET_EHOSTUNREACH,
> -    [EALREADY]          = TARGET_EALREADY,
> -    [EINPROGRESS]       = TARGET_EINPROGRESS,
> -    [ESTALE]            = TARGET_ESTALE,
> -    [ECANCELED]         = TARGET_ECANCELED,
> -    [ENOMEDIUM]         = TARGET_ENOMEDIUM,
> -    [EMEDIUMTYPE]       = TARGET_EMEDIUMTYPE,
> -#ifdef ENOKEY
> -    [ENOKEY]            = TARGET_ENOKEY,
> -#endif
> -#ifdef EKEYEXPIRED
> -    [EKEYEXPIRED]       = TARGET_EKEYEXPIRED,
> -#endif
> -#ifdef EKEYREVOKED
> -    [EKEYREVOKED]       = TARGET_EKEYREVOKED,
> -#endif
> -#ifdef EKEYREJECTED
> -    [EKEYREJECTED]      = TARGET_EKEYREJECTED,
> -#endif
> -#ifdef EOWNERDEAD
> -    [EOWNERDEAD]        = TARGET_EOWNERDEAD,
> -#endif
> -#ifdef ENOTRECOVERABLE
> -    [ENOTRECOVERABLE]   = TARGET_ENOTRECOVERABLE,
> -#endif
> -#ifdef ENOMSG
> -    [ENOMSG]            = TARGET_ENOMSG,
> -#endif
> -#ifdef ERFKILL
> -    [ERFKILL]           = TARGET_ERFKILL,
> -#endif
> -#ifdef EHWPOISON
> -    [EHWPOISON]         = TARGET_EHWPOISON,
> -#endif
> -};
> -
> -static inline int host_to_target_errno(int err)
> +static inline int host_to_target_errno(int host_errno)
>  {
> -    if (err >= 0 && err < ERRNO_TABLE_SIZE &&
> -        host_to_target_errno_table[err]) {
> -        return host_to_target_errno_table[err];
> +    switch (host_errno) {
> +#define E(X)  case X: return TARGET_##X;
> +#include "errnos.c.inc"
> +#undef E
> +    default:
> +        return host_errno;
>      }
> -    return err;
>  }
>  
> -static inline int target_to_host_errno(int err)
> +static inline int target_to_host_errno(int target_errno)
>  {
> -    if (err >= 0 && err < ERRNO_TABLE_SIZE &&
> -        target_to_host_errno_table[err]) {
> -        return target_to_host_errno_table[err];
> +    switch (target_errno) {
> +#define E(X)  case TARGET_##X: return X;
> +#include "errnos.c.inc"
> +#undef E
> +    default:
> +        return target_errno;
>      }
> -    return err;
>  }
>  
>  static inline abi_long get_errno(abi_long ret)
> @@ -7102,7 +6978,6 @@ void syscall_init(void)
>      IOCTLEntry *ie;
>      const argtype *arg_type;
>      int size;
> -    int i;
>  
>      thunk_init(STRUCT_MAX);
>  
> @@ -7112,12 +6987,6 @@ void syscall_init(void)
>  #undef STRUCT
>  #undef STRUCT_SPECIAL
>  
> -    /* Build target_to_host_errno_table[] table from
> -     * host_to_target_errno_table[]. */
> -    for (i = 0; i < ERRNO_TABLE_SIZE; i++) {
> -        target_to_host_errno_table[host_to_target_errno_table[i]] = i;
> -    }
> -
>      /* we patch the ioctl size if necessary. We rely on the fact that
>         no ioctl has all the bits at '1' in the size field */
>      ie = ioctl_entries;
> diff --git a/linux-user/errnos.c.inc b/linux-user/errnos.c.inc
> new file mode 100644
> index 00000000000..963ba1ce9d1
> --- /dev/null
> +++ b/linux-user/errnos.c.inc
> @@ -0,0 +1,140 @@
> +/*
> + * This list is the union of errno values overridden in asm-<arch>/errno.h
> + * minus the errnos that are not actually generic to all archs.
> + *
> + * Please keep this list sorted alphabetically.
> + *
> + *  Copyright (c) 2003 Fabrice Bellard
> + *
> + *  This program is free software; you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License as published by
> + *  the Free Software Foundation; either version 2 of the License, or
> + *  (at your option) any later version.
> + *
> + *  This program is distributed in the hope that it will be useful,
> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *  GNU General Public License for more details.
> + *
> + *  You should have received a copy of the GNU General Public License
> + *  along with this program; if not, see <http://www.gnu.org/licenses/>.
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +E(EADDRINUSE)
> +E(EADDRNOTAVAIL)
> +E(EADV)
> +E(EAFNOSUPPORT)
> +E(EAGAIN)
> +E(EALREADY)
> +E(EBADE)
> +E(EBADFD)
> +E(EBADMSG)
> +E(EBADR)
> +E(EBADRQC)
> +E(EBADSLT)
> +E(EBFONT)
> +E(ECANCELED)
> +E(ECHRNG)
> +E(ECOMM)
> +E(ECONNABORTED)
> +E(ECONNREFUSED)
> +E(ECONNRESET)
> +E(EDEADLK)
> +E(EDESTADDRREQ)
> +E(EDOTDOT)
> +E(EDQUOT)
> +E(EHOSTDOWN)
> +E(EHOSTUNREACH)
> +#ifdef EHWPOISON
> +E(EHWPOISON)
> +#endif
> +E(EIDRM)
> +E(EILSEQ)
> +E(EINPROGRESS)
> +E(EISCONN)
> +E(EISNAM)
> +#ifdef EKEYEXPIRED
> +E(EKEYEXPIRED)
> +#endif
> +#ifdef EKEYREJECTED
> +E(EKEYREJECTED)
> +#endif
> +#ifdef EKEYREVOKED
> +E(EKEYREVOKED)
> +#endif
> +E(EL2HLT)
> +E(EL2NSYNC)
> +E(EL3HLT)
> +E(EL3RST)
> +E(ELIBACC)
> +E(ELIBBAD)
> +E(ELIBEXEC)
> +E(ELIBMAX)
> +E(ELIBSCN)
> +E(ELNRNG)
> +E(ELOOP)
> +E(EMEDIUMTYPE)
> +E(EMSGSIZE)
> +E(EMULTIHOP)
> +E(ENAMETOOLONG)
> +E(ENAVAIL)
> +E(ENETDOWN)
> +E(ENETRESET)
> +E(ENETUNREACH)
> +E(ENOANO)
> +E(ENOBUFS)
> +E(ENOCSI)
> +E(ENODATA)
> +#ifdef ENOKEY
> +E(ENOKEY)
> +#endif
> +E(ENOLCK)
> +E(ENOLINK)
> +E(ENOMEDIUM)
> +#ifdef ENOMSG
> +E(ENOMSG)
> +#endif
> +E(ENONET)
> +E(ENOPKG)
> +E(ENOPROTOOPT)
> +E(ENOSR)
> +E(ENOSTR)
> +E(ENOSYS)
> +E(ENOTCONN)
> +E(ENOTEMPTY)
> +E(ENOTNAM)
> +#ifdef ENOTRECOVERABLE
> +E(ENOTRECOVERABLE)
> +#endif
> +E(ENOTSOCK)
> +E(ENOTUNIQ)
> +E(EOPNOTSUPP)
> +E(EOVERFLOW)
> +#ifdef EOWNERDEAD
> +E(EOWNERDEAD)
> +#endif
> +E(EPFNOSUPPORT)
> +E(EPROTO)
> +E(EPROTONOSUPPORT)
> +E(EPROTOTYPE)
> +E(EREMCHG)
> +E(EREMOTE)
> +E(EREMOTEIO)
> +E(ERESTART)
> +#ifdef ERFKILL
> +E(ERFKILL)
> +#endif
> +E(ESHUTDOWN)
> +E(ESOCKTNOSUPPORT)
> +E(ESRMNT)
> +E(ESTALE)
> +E(ESTRPIPE)
> +E(ETIME)
> +E(ETIMEDOUT)
> +E(ETOOMANYREFS)
> +E(EUCLEAN)
> +E(EUNATCH)
> +E(EUSERS)
> +E(EXFULL)
> 

Reviewed-by: Laurent Vivier <laurent@vivier.eu>