Am 25.06.2015 um 19:02 schrieb Paolo Bonzini:
> On 25/06/2015 18:10, Andreas Färber wrote:
>> Am 25.06.2015 um 18:02 schrieb Paolo Bonzini:
>>> On 25/06/2015 18:00, Andreas Färber wrote:
>>>>> - if (!mmio_registered) {
>>>>> - ICCBus *b = ICC_BUS(qdev_get_parent_bus(dev));
>>>>> - memory_region_add_subregion(b->apic_address_space, 0,
>>>>> &s->io_memory);
>>>>> - mmio_registered = true;
>>>>> - }
>>>>> +
>>>>> + root = address_space_root_memory_region(cpu->as);
>>>>> + memory_region_add_subregion_overlap(root,
>>>>> + s->apicbase &
>>>>> MSR_IA32_APICBASE_BASE,
>>>>> + &s->io_memory,
>>>>> + 0x1000);
>>>>>
>>>
>>> I had already remarked that this patch is wrong. cpu->as is completely
>>> unused on KVM, for example.
>>
>> Ah, then I don't understand this [RESEND]. ;)
>> Either way, not on my plate ATM, it seems.
>>
>> Did you also outline how it is supposed to be done instead?
>
> I said "I think this patch is incorrect, because you do not install a
> separate address space for each CPU. Also, the CPU address space is
> only used with TCG so it should be guarded by "if (tcg_enabled())"."
>
> By the way, now TCG _is_ installing a separate address space per CPU
> already, so the patch can simply guard the code with "if (tcg_enabled())".
Is the APIC MemoryRegion not used by KVM? Otherwise if we still need the
ugly code path for KVM, that's not much of an improvement here.
And is installing a separate address space per CPU for KVM difficult due
to kernel limitations, or is this just a few lines of QEMU code that Zhu
or someone would need to write? :)
Regards,
Andreas
--
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Dilip Upmanyu, Graham Norton; HRB
21284 (AG Nürnberg)
