Am 26.07.2013 05:08, schrieb Chen Fan:
> This regression is caused by commit c643bed99.
>
> When using x86_64-softmmu --enable-kvm boot qemu, cpu-add command fails to
> add a vcpu,
> there show (KVM: setting VAPIC address failed), but at boot, there is no
> problem.
>
> The reason for this is:
> in hotplug case it uses an uninitialized vcpu to set apic into kvm, then
> calls qemu_init_vcpu below.
> ->x86_cpu_realizefn
> ->x86_cpu_apic_realize
> ->qdev_init
> ->device_set_realized
> ->device_reset (hotplugged == 1)
> ->apic_reset_common
> ->vapic_base_update
> ->kvm_apic_vapic_base_update (at here, vcpu is uninitialized, kvmfd is
> zero, fail to ioctl).
>
> But in startup case system wide reset after vcpu was initialized.
> ->machine->init -> qdev_machine_creation_done -> qemu_system_reset ->
> cpu_synchronize_all_post_reset -> cpu_synchronize_post_reset.
>
> v1-v2: Change cpu_apic_realize to post_vcpu_init.
>
> Signed-off-by: Chen Fan <chen.fan.f...@cn.fujitsu.com>
I have started to overhaul this patch (e.g., error handling and naming)
but am not feeling comfortable to include that in tonight's pull - will
prepare a v3 for Monday's pull.
Reset looks broken? Would need to move that after APIC realization -
needs testing.
Andreas
> ---
> include/qom/cpu.h | 2 ++
> qom/cpu.c | 13 +++++++++++++
> target-i386/cpu.c | 10 ++++------
> 3 files changed, 19 insertions(+), 6 deletions(-)
>
> diff --git a/include/qom/cpu.h b/include/qom/cpu.h
> index daf1835..4b16385 100644
> --- a/include/qom/cpu.h
> +++ b/include/qom/cpu.h
> @@ -80,6 +80,7 @@ struct TranslationBlock;
> * @synchronize_from_tb: Callback for synchronizing state from a TCG
> * #TranslationBlock.
> * @get_phys_page_debug: Callback for obtaining a physical address.
> + * @post_vcpu_init: Callback for doing some extra initialization.
> * @vmsd: State description for migration.
> *
> * Represents a CPU family or model.
> @@ -108,6 +109,7 @@ typedef struct CPUClass {
> void (*set_pc)(CPUState *cpu, vaddr value);
> void (*synchronize_from_tb)(CPUState *cpu, struct TranslationBlock *tb);
> hwaddr (*get_phys_page_debug)(CPUState *cpu, vaddr addr);
> + void (*post_vcpu_init)(CPUState *cpu, Error **errp);
>
> const struct VMStateDescription *vmsd;
> int (*write_elf64_note)(WriteCoreDumpFunction f, CPUState *cpu,
> diff --git a/qom/cpu.c b/qom/cpu.c
> index 5c45ab5..28f63b7 100644
> --- a/qom/cpu.c
> +++ b/qom/cpu.c
> @@ -213,12 +213,25 @@ static ObjectClass *cpu_common_class_by_name(const char
> *cpu_model)
> return NULL;
> }
>
> +static void post_vcpu_init(CPUState *cpu, Error **errp)
> +{
> + CPUClass *cc = CPU_GET_CLASS(cpu);
> + if (cc->post_vcpu_init != NULL) {
> + (*cc->post_vcpu_init)(cpu, errp);
> + }
> +}
> +
> static void cpu_common_realizefn(DeviceState *dev, Error **errp)
> {
> CPUState *cpu = CPU(dev);
>
> qemu_init_vcpu(cpu);
>
> + post_vcpu_init(cpu, errp);
> + if (error_is_set(errp)) {
> + return;
> + }
> +
> if (dev->hotplugged) {
> cpu_synchronize_post_init(cpu);
> notifier_list_notify(&cpu_added_notifiers, dev);
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index cd350cb..d51ab8b 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -2311,8 +2311,9 @@ static void x86_cpu_apic_create(X86CPU *cpu, Error
> **errp)
> apic->cpu = cpu;
> }
>
> -static void x86_cpu_apic_realize(X86CPU *cpu, Error **errp)
> +static void x86_cpu_apic_realize(CPUState *s, Error **errp)
> {
> + X86CPU *cpu = X86_CPU(s);
> CPUX86State *env = &cpu->env;
>
> if (env->apic_state == NULL) {
> @@ -2326,7 +2327,7 @@ static void x86_cpu_apic_realize(X86CPU *cpu, Error
> **errp)
> }
> }
> #else
> -static void x86_cpu_apic_realize(X86CPU *cpu, Error **errp)
> +static void x86_cpu_apic_realize(CPUState *s, Error **errp)
> {
> }
> #endif
> @@ -2388,10 +2389,6 @@ static void x86_cpu_realizefn(DeviceState *dev, Error
> **errp)
>
> mce_init(cpu);
>
> - x86_cpu_apic_realize(cpu, &local_err);
> - if (local_err != NULL) {
> - goto out;
> - }
> cpu_reset(CPU(cpu));
>
> xcc->parent_realize(dev, &local_err);
> @@ -2540,6 +2537,7 @@ static void x86_cpu_common_class_init(ObjectClass *oc,
> void *data)
> cc->synchronize_from_tb = x86_cpu_synchronize_from_tb;
> cc->get_arch_id = x86_cpu_get_arch_id;
> cc->get_paging_enabled = x86_cpu_get_paging_enabled;
> + cc->post_vcpu_init = x86_cpu_apic_realize;
> #ifndef CONFIG_USER_ONLY
> cc->get_memory_mapping = x86_cpu_get_memory_mapping;
> cc->get_phys_page_debug = x86_cpu_get_phys_page_debug;
>
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
