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

[Andreas Färber]

Am 29.04.2013 19:03, schrieb Igor Mammedov:
> 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>
> ---
> v3:
>   - fix compile error caused by mismerged hunk
> v2:
>   - use icc-bridge from args instead of resolving it

Thanks, applied to qom-cpu:
https://github.com/afaerber/qemu-cpu/commits/qom-cpu

One question though...

[...]
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 658ff6c..6b3faac 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
[...]
> @@ -929,14 +931,21 @@ 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) {

Might sysbus_mmio_get_region(SYS_BUS_DEVICE(icc_bridge), 0) != NULL be a
more straightforward guard? We're not accessing apic_state at all. :)

Andreas

> +        /* XXX: what if the base changes? */
> +        sysbus_mmio_map_overlap(SYS_BUS_DEVICE(icc_bridge), 0,
> +                                APIC_DEFAULT_ADDRESS, 0x1000);
> +    }
>  }
>  
>  void pc_acpi_init(const char *default_dsdt)
[snip]


-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg