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 majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html