Re: [Qemu-devel] [PATCH v6 1/4] apic: map APIC's MMIO region at each CPU's address space

[Igor Mammedov <imammedo@redhat.com>]

On Wed, 20 May 2015 10:40:46 +0800
Zhu Guihua <zhugh.fnst@cn.fujitsu.com> wrote:

> From: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
> 
> Replace mapping APIC at global system address space with
> mapping it at per-CPU address spaces.
> 
> Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
> Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com>
patch doesn't apply due to a small conflict in hw/i386/pc.c

otherwise
Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> ---
>  exec.c                |  5 +++++
>  hw/i386/pc.c          |  7 -------
>  hw/intc/apic_common.c | 14 ++++++++------
>  include/exec/memory.h |  5 +++++
>  target-i386/cpu.c     |  2 ++
>  5 files changed, 20 insertions(+), 13 deletions(-)
> 
> diff --git a/exec.c b/exec.c
> index ae37b98..623069b 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -2702,6 +2702,11 @@ void address_space_unmap(AddressSpace *as, void *buffer, hwaddr len,
>      cpu_notify_map_clients();
>  }
>  
> +MemoryRegion *address_space_root_memory_region(AddressSpace *as)
> +{
> +    return as->root;
> +}
> +
>  void *cpu_physical_memory_map(hwaddr addr,
>                                hwaddr *plen,
>                                int is_write)
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 578ad63..bc06bb4 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1096,13 +1096,6 @@ void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge)
>          object_unref(OBJECT(cpu));
>      }
>  
> -    /* 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 d38d24b..1c83753 100644
> --- a/hw/intc/apic_common.c
> +++ b/hw/intc/apic_common.c
> @@ -297,7 +297,8 @@ static void apic_common_realize(DeviceState *dev, Error **errp)
>      APICCommonClass *info;
>      static DeviceState *vapic;
>      static int apic_no;
> -    static bool mmio_registered;
> +    CPUState *cpu = CPU(s->cpu);
> +    MemoryRegion *root;
>  
>      if (apic_no >= MAX_APICS) {
>          error_setg(errp, "%s initialization failed.",
> @@ -308,11 +309,12 @@ 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;
> -    }
> +
> +    root = address_space_root_memory_region(cpu->as);
> +    memory_region_add_subregion_overlap(root,
> +                                        s->apicbase & MSR_IA32_APICBASE_BASE,
> +                                        &s->io_memory,
> +                                        0x1000);
>  
>      /* Note: We need at least 1M to map the VAPIC option ROM */
>      if (!vapic && s->vapic_control & VAPIC_ENABLE_MASK &&
> diff --git a/include/exec/memory.h b/include/exec/memory.h
> index 0ccfd3b..9735923 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/memory.h
> @@ -1293,6 +1293,11 @@ void *address_space_map(AddressSpace *as, hwaddr addr,
>  void address_space_unmap(AddressSpace *as, void *buffer, hwaddr len,
>                           int is_write, hwaddr access_len);
>  
> +/* address_space_root_memory_region: get root memory region
> + *
> + * @as: #AddressSpace to be accessed
> + */
> +MemoryRegion *address_space_root_memory_region(AddressSpace *as);
>  
>  #endif
>  
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 3305e09..f83e526 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -2740,6 +2740,8 @@ static void x86_cpu_apic_create(X86CPU *cpu, Error **errp)
>      /* TODO: convert to link<> */
>      apic = APIC_COMMON(cpu->apic_state);
>      apic->cpu = cpu;
> +    cpu_set_apic_base(cpu->apic_state,
> +                      APIC_DEFAULT_ADDRESS | MSR_IA32_APICBASE_ENABLE);
>  }
>  
>  static void x86_cpu_apic_realize(X86CPU *cpu, Error **errp)