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@amazon.com>

Thanks,


Juergen