Il 27/04/2013 12:09, Blue Swirl ha scritto: > On Fri, Apr 26, 2013 at 10:13 PM, Paolo Bonzini <pbonz...@redhat.com> wrote: >> Il 26/04/2013 19:46, Igor Mammedov ha scritto: >>>>> But as the address can't be changed (yet), the entire patch could be >>>>> simply: >>>>> - kioapic->base_address = s->busdev.mmio[0].addr; >>>>> + kioapic->base_address = IO_APIC_DEFAULT_ADDRESS; >>> It's a bit fragile, but that for sure simpler and can work. >>> >>> Jan, Paolo, >>> Are you ok with this approach? >>> >> >> I think extending memory_region_find is a good idea anyway, and at this >> point I don't see a reason to do the above change... > > The reasoning was in the part that Igor cut off: > > "Later, when it's possible to change the address via PIIX3 registers, > we can adjust the base and pass that properly to kioapic and on to > KVM. > > Resolving the base address every time when kvm_ioapic_put() is called > is also less efficient, assuming of course that the base address > changes less often than the KVM ioctl is used." > > I think the patch is a bit flawed. If the guest maps something else on > top of IOAPIC, like LAPIC (which should be in CPU specific address > spaces, but for now it lives in the global system memory space), the > guest could trigger the abort() by resetting the system.
The questions are, in order of importance: (1) what privileges would this require in the guest? Answer: a lot. (2) is this likely to happen by chance? Answer: no, not at all. (3) is there a workaround? Answer: yes, disable in-kernel irqchip. Simply setting IO_APIC_DEFAULT_ADDRESS is also flawed in my opinion. I'm not sure the in-kernel irqchip handles correctly an overlap between the IOAPIC and LAPIC regions, maybe an abort is predictable after all. Paolo
