Re: [PATCH] x86: rework hypercall argument count table instantiation & use

[Juergen Gross <jgross@xxxxxxxx>]

On 27.07.22 18:00, Jan Beulich wrote:
> The initial observation were duplicate symbols that our checking warns
> about. Instead of merely renaming one or both pair(s) of symbols,
> reduce #ifdef-ary at the same time by moving the instantiation of the
> arrays into macros (a native and a 32-bit one each, where likely more
> redundancy could be eliminated, if we really wanted to). While doing the
> conversion also stop open-coding array_access_nospec().
>
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
Reviewed-by: Juergen Gross <jgross@xxxxxxxx>

With one small nit ...

> --- a/xen/arch/x86/include/asm/hypercall.h
> +++ b/xen/arch/x86/include/asm/hypercall.h
> @@ -43,16 +43,6 @@ compat_common_vcpu_op(
>   
>   #endif /* CONFIG_COMPAT */
>   
> -#ifndef NDEBUG
> -static inline unsigned int _get_nargs(const unsigned char *tbl, unsigned int c)
> -{
> -    return tbl[c];
> -}
> -#define get_nargs(t, c) _get_nargs(t, array_index_nospec(c, ARRAY_SIZE(t)))
> -#else
> -#define get_nargs(tbl, c) 0
> -#endif
> -
>   static inline void clobber_regs(struct cpu_user_regs *regs,
>                                   unsigned int nargs)
>   {
> @@ -69,6 +59,11 @@ static inline void clobber_regs(struct c
>   #endif
>   }
>   
> +#define clobber_regs(r, n, t) ({ \
> +    static const unsigned char t[] = hypercall_args_ ## t; \
> +    clobber_regs(r, array_access_nospec(t, n)); \
> +})
> +
... could I talk you into not overloading the names of the inline
functions with macros? You are changing all the call sites anyway.


Juergen

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature
Description: OpenPGP digital signature