Am 14.04.2015 um 14:41 schrieb Igor Mammedov:
> On Fri, 10 Apr 2015 16:18:05 +0800
> Chen Fan <chen.fan.f...@cn.fujitsu.com> wrote:
>
>> Due to local apic address is in view of CPU's address space,
>> so able to move apic mapping to each apic realizefn.
>>
>> Signed-off-by: Chen Fan <chen.fan.f...@cn.fujitsu.com>
>> ---
>> hw/i386/pc.c | 7 -------
>> hw/intc/apic_common.c | 11 +++++------
>> 2 files changed, 5 insertions(+), 13 deletions(-)
>>
>> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
>> index a8e6be1..a5e2a27 100644
>> --- a/hw/i386/pc.c
>> +++ b/hw/i386/pc.c
>> @@ -1089,13 +1089,6 @@ void pc_cpus_init(const char *cpu_model, DeviceState
>> *icc_bridge)
>> }
>> }
>>
>> - /* map APIC MMIO area if CPU has APIC */
>> - if (cpu && cpu->apic_state) {
>> - /* XXX: what if the base changes? */
>> - sysbus_mmio_map_overlap(SYS_BUS_DEVICE(icc_bridge), 0,
>> - APIC_DEFAULT_ADDRESS, 0x1000);
>> - }
>> -
>> /* tell smbios about cpuid version and features */
>> smbios_set_cpuid(cpu->env.cpuid_version, cpu->env.features[FEAT_1_EDX]);
>> }
>> diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
>> index 042e960..004821a 100644
>> --- a/hw/intc/apic_common.c
>> +++ b/hw/intc/apic_common.c
>> @@ -306,7 +306,6 @@ static void apic_common_realize(DeviceState *dev, Error
>> **errp)
>> APICCommonClass *info;
>> static DeviceState *vapic;
>> static int apic_no;
>> - static bool mmio_registered;
>>
>> if (apic_no >= MAX_APICS) {
>> error_setg(errp, "%s initialization failed.",
>> @@ -317,11 +316,11 @@ static void apic_common_realize(DeviceState *dev,
>> Error **errp)
>>
>> info = APIC_COMMON_GET_CLASS(s);
>> info->realize(dev, errp);
>> - 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;
>> - }
>> +
>> + memory_region_add_subregion_overlap(CPU(s->cpu)->as->root,
> if you look at AddressSpace declaration, root field is marked as private
> so you can't access it.
> Perhaps you need to add wrapper to get root, something like this:
> address_space_root_memory_region()
Also, by convention please use a local CPUState *cpu or *cs variable
rather than calling CPU() inline.
Regards,
Andreas
>> + APIC_DEFAULT_ADDRESS,
> use s->apicbase here and set its default value from CPU using
> cpu_set_apic_base()
> for example in: x86_cpu_apic_create()
>
> PS:
> we also probably need to teach kvm_apic_set_base()/apic_set_base() to
> remap APIC in case base changes but it's out of scope of this series.
>
>> + &s->io_memory,
>> + 0x1000);
>>
>> /* Note: We need at least 1M to map the VAPIC option ROM */
>> if (!vapic && s->vapic_control & VAPIC_ENABLE_MASK &&
--
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Jennifer Guild, Dilip Upmanyu,
Graham Norton; HRB 21284 (AG Nürnberg)
