Jin Dongming wrote:

>>  
>>      switch (len) {
>> -    case 8:
>> -            *(u64 *) val = result;
>> -            break;
>> -    case 1:
>> -    case 2:
>>      case 4:
>> -            memcpy(val, (char *)&result, len);
> 
> Here the parameter len is not used for reading data from ioapic register, 
> before switch case
> the value of ioapic register has been read by ioapic_read_indirect().
> 

Yeah, it's right, but it's rarely operation that guest using incorrect width to 
access
the registers, so i don't think it's worthy to move the width judgment before 
the real
registers read.

> 
>> +            *(u32 *) val = result;
>>              break;
>> +
>>      default:
>>              printk(KERN_WARNING "ioapic: wrong length %d\n", len);
> 
> And I think here is not good to print warning message. Maybe it is better to 
> do
> such kind of checking at the first of this function before 
> ioapic_read_indirect().
> 

ditto

Thanks for your review, Jin!
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to