Messages in this thread | | | Date | Fri, 02 Jul 2010 15:38:02 +0800 | From | Xiao Guangrong <> | Subject | Re: [PATCH] KVM: IOAPIC: only access APIC registers one dword at a time |
| |
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!
| |