Re: [Qemu-devel] [PATCH 17/21] introduce memory_region_get_address() and use it in kvm/ioapic

[Paolo Bonzini]

Il 27/04/2013 12:09, Blue Swirl ha scritto:
> On Fri, Apr 26, 2013 at 10:13 PM, Paolo Bonzini <address@hidden> 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