| Message ID | 1350160539-22293-1-git-send-email-imammedo@redhat.com |
|---|---|
| State | New |
| Headers | show |
Am 13.10.2012 22:35, schrieb Igor Mammedov: > (L)APIC is a part of cpu [1] so move APIC initialization inside of > x86_cpu object. Since cpu_model and override flags currently specify > whether APIC should be created or not, APIC creation&initialization is > moved into x86_cpu_apic_init() which is called from x86_cpu_realize(). > > [1] - all x86 cpus have integrated APIC if we overlook existence of i486, > and it's more convenient to model after majority of them. > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > --- > v5: fix *-user target build, smp_cpus is defined for softmmu only I do not run into any build issue with v4. Is it a runtime issue? Andreas
On Sun, 14 Oct 2012 06:09:56 +0200 Andreas Färber <afaerber@suse.de> wrote: > Am 13.10.2012 22:35, schrieb Igor Mammedov: > > (L)APIC is a part of cpu [1] so move APIC initialization inside of > > x86_cpu object. Since cpu_model and override flags currently specify > > whether APIC should be created or not, APIC creation&initialization is > > moved into x86_cpu_apic_init() which is called from x86_cpu_realize(). > > > > [1] - all x86 cpus have integrated APIC if we overlook existence of i486, > > and it's more convenient to model after majority of them. > > > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > > --- > > v5: fix *-user target build, smp_cpus is defined for softmmu only > > I do not run into any build issue with v4. Is it a runtime issue? > > Andreas > I've run into this trying to build your latest CPUstate series with following configure options: './configure' '--enable-debug' '--target-list=x86_64-softmmu,x86_64-linux-user' Anyway it's better not to build in any APIC checks in user target since it doesn't need it at all. I'm sorry for not noticing error earlier at v4 build time. Could you re-apply it, please?
Am 15.10.2012 13:54, schrieb Igor Mammedov: > On Sun, 14 Oct 2012 06:09:56 +0200 > Andreas Färber <afaerber@suse.de> wrote: > >> Am 13.10.2012 22:35, schrieb Igor Mammedov: >>> (L)APIC is a part of cpu [1] so move APIC initialization inside of >>> x86_cpu object. Since cpu_model and override flags currently specify >>> whether APIC should be created or not, APIC creation&initialization is >>> moved into x86_cpu_apic_init() which is called from x86_cpu_realize(). >>> >>> [1] - all x86 cpus have integrated APIC if we overlook existence of i486, >>> and it's more convenient to model after majority of them. >>> >>> Signed-off-by: Igor Mammedov <imammedo@redhat.com> >>> --- >>> v5: fix *-user target build, smp_cpus is defined for softmmu only >> >> I do not run into any build issue with v4. Is it a runtime issue? >> >> Andreas >> > I've run into this trying to build your latest CPUstate series with > following configure options: > './configure' '--enable-debug' '--target-list=x86_64-softmmu,x86_64-linux-user' --enable-debug was the missing puzzle piece, now I get the undefined reference warning, too. :) > Anyway it's better not to build in any APIC checks in user target since it > doesn't need it at all. > > I'm sorry for not noticing error earlier at v4 build time. > Could you re-apply it, please? Sure, the patch itself is indeed better. Now that I've been able to reproduce, I've exchanged v4 against v5 in on qom-cpu queue: https://github.com/afaerber/qemu-cpu/commits/qom-cpu Thanks for noticing, Andreas
On Mon, 15 Oct 2012 14:58:40 +0200 Andreas Färber <afaerber@suse.de> wrote: > Am 15.10.2012 13:54, schrieb Igor Mammedov: > > On Sun, 14 Oct 2012 06:09:56 +0200 > > Andreas Färber <afaerber@suse.de> wrote: > > > >> Am 13.10.2012 22:35, schrieb Igor Mammedov: > >>> (L)APIC is a part of cpu [1] so move APIC initialization inside of > >>> x86_cpu object. Since cpu_model and override flags currently specify > >>> whether APIC should be created or not, APIC creation&initialization is > >>> moved into x86_cpu_apic_init() which is called from x86_cpu_realize(). > >>> > >>> [1] - all x86 cpus have integrated APIC if we overlook existence of > >>> i486, and it's more convenient to model after majority of them. > >>> > >>> Signed-off-by: Igor Mammedov <imammedo@redhat.com> > >>> --- > >>> v5: fix *-user target build, smp_cpus is defined for softmmu only > >> > >> I do not run into any build issue with v4. Is it a runtime issue? > >> > >> Andreas > >> > > I've run into this trying to build your latest CPUstate series with > > following configure options: > > './configure' '--enable-debug' > > '--target-list=x86_64-softmmu,x86_64-linux-user' > > --enable-debug was the missing puzzle piece, now I get the undefined > reference warning, too. :) > > > Anyway it's better not to build in any APIC checks in user target since it > > doesn't need it at all. > > > > I'm sorry for not noticing error earlier at v4 build time. > > Could you re-apply it, please? > > Sure, the patch itself is indeed better. Now that I've been able to > reproduce, I've exchanged v4 against v5 in on qom-cpu queue: > https://github.com/afaerber/qemu-cpu/commits/qom-cpu Thanks. > > Thanks for noticing, > Andreas >
diff --git a/hw/pc.c b/hw/pc.c index 6c0722d..aa5abe0 100644 --- a/hw/pc.c +++ b/hw/pc.c @@ -70,8 +70,6 @@ #define FW_CFG_E820_TABLE (FW_CFG_ARCH_LOCAL + 3) #define FW_CFG_HPET (FW_CFG_ARCH_LOCAL + 4) -#define MSI_ADDR_BASE 0xfee00000 - #define E820_NR_ENTRIES 16 struct e820_entry { @@ -845,35 +843,6 @@ DeviceState *cpu_get_current_apic(void) } } -static DeviceState *apic_init(void *env, uint8_t apic_id) -{ - DeviceState *dev; - static int apic_mapped; - - if (kvm_irqchip_in_kernel()) { - dev = qdev_create(NULL, "kvm-apic"); - } else if (xen_enabled()) { - dev = qdev_create(NULL, "xen-apic"); - } else { - dev = qdev_create(NULL, "apic"); - } - - qdev_prop_set_uint8(dev, "id", apic_id); - qdev_prop_set_ptr(dev, "cpu_env", env); - qdev_init_nofail(dev); - - /* XXX: mapping more APICs at the same memory location */ - if (apic_mapped == 0) { - /* NOTE: the APIC is directly connected to the CPU - it is not - on the global memory bus. */ - /* XXX: what if the base changes? */ - sysbus_mmio_map(sysbus_from_qdev(dev), 0, MSI_ADDR_BASE); - apic_mapped = 1; - } - - return dev; -} - void pc_acpi_smi_interrupt(void *opaque, int irq, int level) { CPUX86State *s = opaque; @@ -883,24 +852,6 @@ void pc_acpi_smi_interrupt(void *opaque, int irq, int level) } } -static X86CPU *pc_new_cpu(const char *cpu_model) -{ - X86CPU *cpu; - CPUX86State *env; - - cpu = cpu_x86_init(cpu_model); - if (cpu == NULL) { - fprintf(stderr, "Unable to find x86 CPU definition\n"); - exit(1); - } - env = &cpu->env; - if ((env->cpuid_features & CPUID_APIC) || smp_cpus > 1) { - env->apic_state = apic_init(env, env->cpuid_apic_id); - } - cpu_reset(CPU(cpu)); - return cpu; -} - void pc_cpus_init(const char *cpu_model) { int i; @@ -914,8 +865,11 @@ void pc_cpus_init(const char *cpu_model) #endif } - for(i = 0; i < smp_cpus; i++) { - pc_new_cpu(cpu_model); + for (i = 0; i < smp_cpus; i++) { + if (!cpu_x86_init(cpu_model)) { + fprintf(stderr, "Unable to find x86 CPU definition\n"); + exit(1); + } } } diff --git a/target-i386/cpu.c b/target-i386/cpu.c index f3708e6..b5cf37d 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -37,6 +37,12 @@ #include <linux/kvm_para.h> #endif +#include "sysemu.h" +#ifndef CONFIG_USER_ONLY +#include "hw/xen.h" +#include "hw/sysbus.h" +#endif + /* feature flags taken from "Intel Processor Identification and the CPUID * Instruction" and AMD's "CPUID Specification". In cases of disagreement * between feature naming conventions, aliases may be added. @@ -1870,12 +1876,63 @@ static void mce_init(X86CPU *cpu) } } +#define MSI_ADDR_BASE 0xfee00000 + +#ifndef CONFIG_USER_ONLY +static void x86_cpu_apic_init(X86CPU *cpu, Error **errp) +{ + static int apic_mapped; + CPUX86State *env = &cpu->env; + const char *apic_type = "apic"; + + if (kvm_irqchip_in_kernel()) { + apic_type = "kvm-apic"; + } else if (xen_enabled()) { + apic_type = "xen-apic"; + } + + env->apic_state = qdev_try_create(NULL, apic_type); + if (env->apic_state == NULL) { + error_setg(errp, "APIC device '%s' could not be created", apic_type); + return; + } + + object_property_add_child(OBJECT(cpu), "apic", + OBJECT(env->apic_state), NULL); + qdev_prop_set_uint8(env->apic_state, "id", env->cpuid_apic_id); + /* TODO: convert to link<> */ + qdev_prop_set_ptr(env->apic_state, "cpu_env", env); + + if (qdev_init(env->apic_state)) { + error_setg(errp, "APIC device '%s' could not be initialized", + object_get_typename(OBJECT(env->apic_state))); + return; + } + + /* XXX: mapping more APICs at the same memory location */ + if (apic_mapped == 0) { + /* NOTE: the APIC is directly connected to the CPU - it is not + on the global memory bus. */ + /* XXX: what if the base changes? */ + sysbus_mmio_map(sysbus_from_qdev(env->apic_state), 0, MSI_ADDR_BASE); + apic_mapped = 1; + } +} +#endif + void x86_cpu_realize(Object *obj, Error **errp) { X86CPU *cpu = X86_CPU(obj); #ifndef CONFIG_USER_ONLY qemu_register_reset(x86_cpu_machine_reset_cb, cpu); + + if (cpu->env.cpuid_features & CPUID_APIC || smp_cpus > 1) { + x86_cpu_apic_init(cpu, errp); + if (error_is_set(errp)) { + return; + } + } #endif mce_init(cpu);
(L)APIC is a part of cpu [1] so move APIC initialization inside of x86_cpu object. Since cpu_model and override flags currently specify whether APIC should be created or not, APIC creation&initialization is moved into x86_cpu_apic_init() which is called from x86_cpu_realize(). [1] - all x86 cpus have integrated APIC if we overlook existence of i486, and it's more convenient to model after majority of them. Signed-off-by: Igor Mammedov <imammedo@redhat.com> --- v5: fix *-user target build, smp_cpus is defined for softmmu only git tree for testing: https://github.com/imammedo/qemu/tree/apic_in_cpu.v5 --- hw/pc.c | 56 +++++------------------------------------------------- target-i386/cpu.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+), 51 deletions(-)