Re: [Xen-devel] [PATCH 6/9] xen/multicall: Rework arch multicall handling

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 18.07.16 at 11:51, <andrew.cooper3@xxxxxxxxxx> wrote:
> --- a/xen/arch/x86/hypercall.c
> +++ b/xen/arch/x86/hypercall.c
> @@ -338,6 +338,34 @@ long pv_hypercall(struct cpu_user_regs *regs)
>      return ret;
>  }
>  
> +void arch_do_multicall_call(struct mc_state *state)
> +{
> +    if ( !is_pv_32bit_vcpu(current) )
> +    {
> +        struct multicall_entry *call = &state->call;
> +
> +        if ( (call->op < NR_hypercalls) && hypercall_table[call->op] )
> +            call->result = hypercall_table[call->op](
> +                call->args[0], call->args[1], call->args[2],
> +                call->args[3], call->args[4], call->args[5]);
> +        else
> +            call->result = -ENOSYS;
> +    }
> +#ifdef CONFIG_COMPAT
> +    else
> +    {
> +        struct compat_multicall_entry *call = &state->compat_call;
> +
> +        if ( (call->op < NR_hypercalls) && compat_hypercall_table[call->op] )

Why two distinct checks here when pv_hypercall() does just one
outside the if/else? With them folded (or if there is a good reason),
Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
with one more remark:

> --- a/xen/common/multicall.c
> +++ b/xen/common/multicall.c
> @@ -63,7 +63,7 @@ do_multicall(
>  
>          trace_multicall_call(&mcs->call);
>  
> -        do_multicall_call(&mcs->call);
> +        arch_do_multicall_call(mcs);

I think do_multicall_call() as a name was fine, but otoh I also don't
really mind the name change.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel