On Fri, 26 Apr 2013 17:35:41 +0000 Blue Swirl <blauwir...@gmail.com> wrote:
> On Fri, Apr 26, 2013 at 2:17 PM, Igor Mammedov <imamm...@redhat.com> wrote: > > On Thu, 25 Apr 2013 18:37:19 +0000 > > Blue Swirl <blauwir...@gmail.com> wrote: > > > >> On Tue, Apr 23, 2013 at 8:29 AM, Igor Mammedov <imamm...@redhat.com> wrote: > >> > kvm/ioapic is relying on the fact that SysBus device > >> > maps mmio regions with offset counted from start of system memory. > >> > But if ioapic's region is moved to another sub-region which doesn't > >> > start at the beginning of system memory then using offset isn't correct. > >> > >> But base_address in only initialized once, never changed. Does this > >> patch matter now? > > this code path is used on only at machine start-up but also on resets and > > post migration to initialize IO-APIC. And unfortunately KVM API takes not > > only base address but a bunch of other parameters in that ioctl, so > > splitting > > it doesn't look feasible for 1.5. Patch is useful in aspect that it hides > > direct access to parent's internals and without dangling SysBusDevice > > internals it allows to convert kvm/ioapic to ICCDevice [next patch in > > series], > > leaving code a bit cleaner. > > 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? > > 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. > > > > > BTW: > > there is an improved patch: > > http://permalink.gmane.org/gmane.comp.emulators.qemu/208512 > > > > that instead of introducing a new function improves current > > memory_region_find() API. > > > >> The correct solution would be to track changes to APICBASE register at > >> PIIX3 chipset level and adjust mapping and KVM base also from there. > > That we would probably need a change in KVM API first to allow set > > IO-APIC's base separately. > > -- Regards, Igor