Re: [PATCH v2 11/15] xen/x86: call hypercall handlers via generated macro

[Jan Beulich <jbeulich@xxxxxxxx>]

On 01.11.2021 16:20, Juergen Gross wrote:
> Instead of using a function table use the generated macros for calling
> the appropriate hypercall handlers.
> 
> This is beneficial to performance and avoids speculation issues.
> 
> With calling the handlers using the correct number of parameters now
> it is possible to do the parameter register clobbering in the NDEBUG
> case after returning from the handler. With the additional generated
> data the hard coded hypercall_args_table[] can be replaced by tables
> using the generated number of parameters.
> 
> Note that this change modifies behavior of clobbering registers in a
> minor way: in case a hypercall is returning -ENOSYS for any reason
> the parameter registers will no longer be clobbered. This should be
> of no real concern, as those cases ought to be extremely rare and
> reuse of the registers in those cases seems rather far fetched.

Considering mem-op where certain sub-ops can return huge positive
values, may I suggest to amend -ENOSYS by "(or the unsigned equivalent
thereof)" to make clear that this case was also recognized/considered?

> @@ -55,4 +42,45 @@ 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];

While maybe not overly relevant in debug builds, it doesn't cost us
much to use array_access_nospec() here, so I'd like to ask for that
to be switched to (replacing the original array_index_nospec() that
you remove). Preferably with this adjustment
Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

Jan