On Wed, 24 Oct 2012 15:50:01 -0200
Eduardo Habkost <ehabk...@redhat.com> wrote:
> This keeps compatibility on machine-types pc-1.2 and older, and prints a
> warning in case the requested configuration won't get the correct
> topology.
>
> Signed-off-by: Eduardo Habkost <ehabk...@redhat.com>
> ---
> Changes v1 -> v2:
> - Move code to cpu.c
> - keep using cpu_index on *-user
> - Use SMP.contiguous_apic_ids global property
> - Prints warning in case the compatibility mode will expose incorrect
> topology
>
> Changes v2 -> v3:
> - Now all code is inside hw/pc.c
> - Use a real "PC" class and a "contiguous_apic_ids" property
>
> Changes v3 -> v4:
> - Instead of using a global property, use a separate machine init
> function and a PCInitArgs field, to implement compatibility mode
> - Use error_report() instead of fprintf(stderr) for the warning
> - Use a field on PCInitArgs instead of a static variable to check
> if warning was already printed
> ---
> hw/pc.c | 19 +++++++++++++++----
> hw/pc.h | 6 ++++++
> hw/pc_piix.c | 1 +
> 3 files changed, 22 insertions(+), 4 deletions(-)
>
> diff --git a/hw/pc.c b/hw/pc.c
> index 8895087..3d08271 100644
> --- a/hw/pc.c
> +++ b/hw/pc.c
> @@ -51,6 +51,8 @@
> #include "exec-memory.h"
> #include "arch_init.h"
> #include "bitmap.h"
> +#include "topology.h"
> +#include "cpus.h"
>
> /* debug PC/ISA interrupts */
> //#define DEBUG_IRQ
> @@ -562,10 +564,19 @@ static void bochs_bios_write(void *opaque, uint32_t
> addr, uint32_t val) */
> static uint32_t apic_id_for_cpu(PCInitArgs *args, int cpu_index)
> {
> - /* right now APIC ID == CPU index. this will eventually change to use
> - * the CPU topology configuration properly
> - */
> - return cpu_index;
> + uint32_t correct_id;
> +
> + correct_id = topo_apicid_for_cpu(smp_cores, smp_threads, cpu_index);
> + if (args->compat_contiguous_apic_ids) {
> + if (cpu_index != correct_id && !args->apic_id_warned) {
> + error_report("APIC IDs set in compatibility mode, "
> + "CPU topology won't match the configuration");
> + args->apic_id_warned = true;
> + }
> + return cpu_index;
> + } else {
> + return correct_id;
> + }
> }
>
> int e820_add_entry(uint64_t address, uint64_t length, uint32_t type)
> diff --git a/hw/pc.h b/hw/pc.h
> index 53883f5..a14944a 100644
> --- a/hw/pc.h
> +++ b/hw/pc.h
> @@ -81,6 +81,12 @@ typedef struct PCInitArgs {
> QEMUMachineInitArgs *qemu_args;
> bool pci_enabled;
> bool kvmclock_enabled;
> + /* Enable compatibility (buggy) APIC ID generation, that keep APIC IDs
> + * contiguous
> + */
> + bool compat_contiguous_apic_ids;
> + /* Warning message about incorrect APIC ID was shown */
> + bool apic_id_warned;
>
> /* Memory regions & sizes: */
> MemoryRegion *system_memory;
> diff --git a/hw/pc_piix.c b/hw/pc_piix.c
> index 57a3228..79a54e6 100644
> --- a/hw/pc_piix.c
> +++ b/hw/pc_piix.c
> @@ -298,6 +298,7 @@ static void pc_init_pci_v1_2(QEMUMachineInitArgs *args)
> .qemu_args = args,
> .pci_enabled = true,
> .kvmclock_enabled = true,
> + .compat_contiguous_apic_ids = true,
I suppose libvirt/whatever would know that it's starting v1_2 machine, so it
would be able to provide valid APIC ID at hotplug time. But it looks fragile.
As Anthony once suggested, perhaps we could model sockets as links.
/machine/cpu[apic_id..]
So machine at level we would have a pre-defined topology with apic_id in link
name serving as initial apic_id selector pins in socket.
Pre-allocate them (sockets) in pc_cpus_init() for all possible CPUs and when
creating CPUs, apic_id property setter could accept link path and get
appropriate apic_id from link name and assign itself to a specified link.
It would be friendly with hot-plug and
-device X86CPU,id='/machine/cpu[apic_id]' cmdline if we ever decide in future
to create CPUs this way.
> };
> pc_init1(&pc_args);
> }
