Re: [PATCH v4 10/11] xen/arm: call hypercall handlers via generated macro

[Juergen Gross <jgross@xxxxxxxx>]

On 23.03.22 10:40, Julien Grall wrote:
> Hi,
>
> On 10/03/2022 07:34, Juergen Gross wrote:
> > @@ -1520,7 +1460,10 @@ static bool check_multicall_32bit_clean(struct 
> > multicall_entry *multi)
> >   {
> >       int i;
> > -    for ( i = 0; i < arm_hypercall_table[multi->op].nr_args; i++ )
> > +    if ( multi->op >= ARRAY_SIZE(hypercall_args) )
> > +        return true;
> NIT: This change reads odd to me. So I would prefer...
>
> > +
> > +    for ( i = 0; i < hypercall_args[multi->op]; i++ )
> >       {
> >           if ( unlikely(multi->args[i] & 0xffffffff00000000ULL) )
> >           {
> > @@ -1537,28 +1480,13 @@ static bool check_multicall_32bit_clean(struct 
> > multicall_entry *multi)
> >   enum mc_disposition arch_do_multicall_call(struct mc_state *state)
> >   {
> >       struct multicall_entry *multi = &state->call;
> > -    arm_hypercall_fn_t call = NULL;
> > -
> > -    if ( multi->op >= ARRAY_SIZE(arm_hypercall_table) )
> ... if we keep this checks. So we don't return true in 
> check_multicall_32bit_clean() when the hypercall doesn't exist.
The idea was to spare the not necessary check in case of a 64-bit guest.

If you prefer to keep the check here, I'm fine to do it this way.

> The code still do the right thing, so either way:
>
> Reviewed-by: Julien Grall <jgrall@xxxxxxxxxx>
Thanks,


Juergen

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature
Description: OpenPGP digital signature