Re: [Qemu-devel] [PATCH 3/7] target-i386: Move APIC to ICC bus

[Igor Mammedov]

On Mon, 29 Apr 2013 17:02:52 +0200
Igor Mammedov <address@hidden> wrote:

> It allows APIC to be hotplugged.
> 
>  * map APIC's mmio at board level if it is present
>  * do not register mmio region for each APIC, since
>    only one is used/mapped
> 
> Signed-off-by: Igor Mammedov <address@hidden>
> Signed-off-by: Andreas Färber <address@hidden>
> ---
> v2:
>   - use icc-bridge from args instead of resolving it
> ---
>  hw/cpu/icc_bus.c                | 10 ++++++++++
>  hw/i386/pc.c                    | 12 ++++++++++--
>  hw/intc/apic_common.c           | 18 ++++++++++++------
>  include/hw/cpu/icc_bus.h        |  3 +++
>  include/hw/i386/apic_internal.h |  6 +++---
>  target-i386/cpu.c               | 16 +++-------------
>  6 files changed, 41 insertions(+), 24 deletions(-)
> 
> diff --git a/hw/cpu/icc_bus.c b/hw/cpu/icc_bus.c
> index 3ac8eeb..73a1dc9 100644
> --- a/hw/cpu/icc_bus.c
> +++ b/hw/cpu/icc_bus.c
> @@ -80,6 +80,7 @@ typedef struct ICCBridgeState {
>      /*< public >*/
>  
>      ICCBus icc_bus;
> +    MemoryRegion apic_container;
>  } ICCBridgeState;
>  
>  #define ICC_BRIGDE(obj) OBJECT_CHECK(ICCBridgeState, (obj), TYPE_ICC_BRIDGE)
> @@ -87,8 +88,17 @@ typedef struct ICCBridgeState {
>  static void icc_bridge_init(Object *obj)
>  {
>      ICCBridgeState *s = ICC_BRIGDE(obj);
> +    SysBusDevice *sb = SYS_BUS_DEVICE(obj);
>  
>      qbus_create_inplace(&s->icc_bus, TYPE_ICC_BUS, DEVICE(s), "icc");
> +
> +    /* Do not change order of registering regions,
> +     * APIC must be first registered region, board maps it by 0 index
> +     */
> +    memory_region_init(&s->apic_container, "icc-apic-container",
> +                       APIC_SPACE_SIZE);
> +    sysbus_init_mmio(sb, &s->apic_container);
> +    s->icc_bus.apic_address_space = &s->apic_container;
>  }
>  
>  static const TypeInfo icc_bridge_info = {
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 658ff6c..ce9357e 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -53,6 +53,7 @@
>  #include "qemu/bitmap.h"
>  #include "qemu/config-file.h"
>  #include "hw/acpi/acpi.h"
> +#include "hw/cpu/icc_bus.h"
>  
>  /* debug PC/ISA interrupts */
>  //#define DEBUG_IRQ
> @@ -917,6 +918,7 @@ static X86CPU *pc_new_cpu(const char *cpu_model, int64_t 
> apic_id,
>  void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge)
>  {
>      int i;
> +    X86CPU *cpu = NULL;
>      Error *error = NULL;
>  
>      /* init CPUs */
> @@ -929,14 +931,20 @@ void pc_cpus_init(const char *cpu_model, DeviceState 
> *icc_bridge)
>      }
>  
>      for (i = 0; i < smp_cpus; i++) {
> -        pc_new_cpu(cpu_model, x86_cpu_apic_id_from_index(i),
> -                   icc_bridge, &error);
> +        cpu = pc_new_cpu(cpu_model, x86_cpu_apic_id_from_index(i),
> +                         icc_bridge, &error);
>          if (error) {
>              fprintf(stderr, "%s\n", error_get_pretty(error));
>              error_free(error);
>              exit(1);
>          }
>      }
> +
> +    /* map APIC MMIO area if CPU has APIC */
> +    if (cpu && cpu->env.apic_state) {
> +        /* XXX: what if the base changes? */
> +        sysbus_mmio_map_overlap(icc_bridge, 0, APIC_DEFAULT_ADDRESS, 0x1000);
> +    }
hunk that makes icc_bridge SysBusDevice escaped into next patch, sorry. I'll
resend it.


-- 
Regards,
  Igor