On Thu, Apr 11, 2013 at 04:51:57PM +0200, Igor Mammedov wrote:
> ... and leave links for not present CPUs empty.
>
> It will allow users to query for possible APIC IDs and use them
> with cpu-add QMP command.
>
> Signed-off-by: Igor Mammedov <imamm...@redhat.com>
I don't see anything wrong with having icc-bridge links as well, but I
would really like to have a target-independent namespace with links,
that could be used to query for the available/valid CPU IDs for cpu-add
commands instead of icc-bridge. The IDs on that namespace could be
considered completely opaque.
> ---
> v2:
> * s/get_firmware_id/get_arch_id/ due to rebase
> * rename cpu_add_notifier to cpu_added_notifier &
> icc_bridge_cpu_add_req -> icc_bridge_cpued_add_req
> ---
> hw/cpu/icc_bus.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
> hw/i386/pc.c | 9 +++++++--
> include/hw/i386/icc_bus.h | 2 ++
> 3 files changed, 55 insertions(+), 2 deletions(-)
>
> diff --git a/hw/cpu/icc_bus.c b/hw/cpu/icc_bus.c
> index ab9623d..5c0b9d4 100644
> --- a/hw/cpu/icc_bus.c
> +++ b/hw/cpu/icc_bus.c
> @@ -18,6 +18,7 @@
> */
> #include "hw/i386/icc_bus.h"
> #include "hw/sysbus.h"
> +#include "sysemu/sysemu.h"
>
> static void icc_bus_initfn(Object *obj)
> {
> @@ -61,15 +62,39 @@ typedef struct ICCBridgeState {
> SysBusDevice busdev;
> MemoryRegion apic_container;
> MemoryRegion ioapic_container;
> + Notifier cpu_added_notifier;
> + Object **links;
> } ICCBridgeState;
> #define ICC_BRIGDE(obj) OBJECT_CHECK(ICCBridgeState, (obj), TYPE_ICC_BRIDGE)
>
>
> +void icc_bridge_set_cpu_link(Object *bridge, Object *cpu_obj)
> +{
> + gchar *name;
> + Error *error = NULL;
> + CPUState *cpu = CPU(cpu_obj);
> + int64_t id = CPU_GET_CLASS(cpu)->get_arch_id(cpu);
> +
> + name = g_strdup_printf("cpu[%" PRIu32 "]",
> x86_cpu_apic_id_from_index(id));
> + object_property_set_link(bridge, cpu_obj, name, &error);
> + g_free(name);
> +
> + g_assert(error == NULL);
> +}
> +
> +static void icc_bridge_cpu_added_req(Notifier *n, void *opaque)
> +{
> + ICCBridgeState *s = container_of(n, ICCBridgeState, cpu_added_notifier);
> +
> + icc_bridge_set_cpu_link(OBJECT(s), OBJECT(opaque));
> +}
> +
> static void icc_bridge_initfn(Object *obj)
> {
> ICCBridgeState *s = ICC_BRIGDE(obj);
> SysBusDevice *sb = SYS_BUS_DEVICE(obj);
> ICCBus *ibus;
> + int i;
>
> ibus = ICC_BUS(qbus_create(TYPE_ICC_BUS, DEVICE(obj), "icc-bus"));
>
> @@ -85,12 +110,33 @@ static void icc_bridge_initfn(Object *obj)
> memory_region_init(&s->ioapic_container, "icc-ioapic-container", 0x1000);
> sysbus_init_mmio(sb, &s->ioapic_container);
> ibus->ioapic_address_space = &s->ioapic_container;
> +
> + s->links = g_malloc0(sizeof(Object *) * max_cpus);
> + for (i = 0; i < max_cpus; i++) {
> + gchar *cpu_name;
> +
> + cpu_name = g_strdup_printf("cpu[%" PRIu32 "]",
> + x86_cpu_apic_id_from_index(i));
> + object_property_add_link(obj, cpu_name, TYPE_CPU, &s->links[i],
> NULL);
> + g_free(cpu_name);
> + }
> +
> + s->cpu_added_notifier.notify = icc_bridge_cpu_added_req;
> + qemu_register_cpu_added_notifier(&s->cpu_added_notifier);
> +}
> +
> +static void icc_bridge_fini(Object *obj)
> +{
> + ICCBridgeState *s = ICC_BRIGDE(obj);
> +
> + g_free(s->links);
> }
>
> static const TypeInfo icc_bridge_info = {
> .name = "icc-bridge",
> .parent = TYPE_SYS_BUS_DEVICE,
> .instance_init = icc_bridge_initfn,
> + .instance_finalize = icc_bridge_fini,
> .instance_size = sizeof(ICCBridgeState),
> };
>
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 6d5e164..ada235c 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -870,7 +870,8 @@ void pc_acpi_smi_interrupt(void *opaque, int irq, int
> level)
> }
> }
>
> -static X86CPU *pc_new_cpu(const char *cpu_model, int64_t apic_id, Error
> **errp)
> +static X86CPU *pc_new_cpu(const char *cpu_model, int64_t apic_id,
> + SysBusDevice *icc_bridge, Error **errp)
> {
> X86CPU *cpu;
>
> @@ -882,6 +883,10 @@ static X86CPU *pc_new_cpu(const char *cpu_model, int64_t
> apic_id, Error **errp)
> object_property_set_int(OBJECT(cpu), apic_id, "apic-id", errp);
> object_property_set_bool(OBJECT(cpu), true, "realized", errp);
>
> + if (icc_bridge != NULL) {
> + icc_bridge_set_cpu_link(OBJECT(icc_bridge), OBJECT(cpu));
> + }
> +
> if (error_is_set(errp)) {
> if (cpu != NULL) {
> object_unref(OBJECT(cpu));
> @@ -911,7 +916,7 @@ void pc_cpus_init(const char *cpu_model)
> TYPE_ICC_BRIDGE, NULL));
>
> for (i = 0; i < smp_cpus; i++) {
> - cpu = pc_new_cpu(cpu_model, x86_cpu_apic_id_from_index(i), &error);
> + cpu = pc_new_cpu(cpu_model, x86_cpu_apic_id_from_index(i), ib,
> &error);
> if (error) {
> fprintf(stderr, "%s\n", error_get_pretty(error));
> error_free(error);
> diff --git a/include/hw/i386/icc_bus.h b/include/hw/i386/icc_bus.h
> index 69a0278..bc31cd9 100644
> --- a/include/hw/i386/icc_bus.h
> +++ b/include/hw/i386/icc_bus.h
> @@ -49,5 +49,7 @@ typedef struct ICCDeviceClass {
>
> #define TYPE_ICC_BRIDGE "icc-bridge"
>
> +void icc_bridge_set_cpu_link(Object *bridge, Object *cpu);
> +
> #endif /* CONFIG_USER_ONLY */
> #endif
> --
> 1.8.2
>
--
Eduardo
