-no-kvm broken since merge a5b526135d

-no-kvm broken since merge a5b526135d

[Jan Kiszka]

JFYI: qemu-kvm commit de408d70d8 works fine with -no-kvm, a5b526135d
produces

qemu: fatal: Trying to execute code outside RAM or ROM at 0x00000001000d5f4f

EAX=00000001 EBX=00000070 ECX=0000005f EDX=00000001
ESI=00000000 EDI=00000000 EBP=00000000 ESP=00000000
EIP=000e5f4f EFL=00000002 [-------] CPL=0 II=0 A20=1 SMM=0 HLT=0
ES =0000 00000000 0000ffff 00009200
CS =f000 ffff0000 0000ffff 00009a00
SS =0000 00000000 0000ffff 00009200
DS =0000 00000000 0000ffff 00009200
FS =0000 00000000 0000ffff 00009200
GS =0000 00000000 0000ffff 00009200
LDT=0000 00000000 0000ffff 00008200
TR =0000 00000000 0000ffff 00008b00
GDT=     00000000 0000ffff
IDT=     00000000 0000ffff
CR0=60000010 CR2=00000000 CR3=00000000 CR4=00000000
DR0=00000000 DR1=00000000 DR2=00000000 DR3=00000000
DR6=ffff0ff0 DR7=00000400
CCS=00000001 CCD=00000000 CCO=SUBL
FCW=037f FSW=0000 [ST=0] FTW=00 MXCSR=00001f80
FPR0=0000000000000000 0000 FPR1=0000000000000000 0000
FPR2=0000000000000000 0000 FPR3=0000000000000000 0000
FPR4=0000000000000000 0000 FPR5=0000000000000000 0000
FPR6=0000000000000000 0000 FPR7=0000000000000000 0000
XMM00=00000000000000000000000000000000
XMM01=00000000000000000000000000000000
XMM02=00000000000000000000000000000000
XMM03=00000000000000000000000000000000
XMM04=00000000000000000000000000000000
XMM05=00000000000000000000000000000000
XMM06=00000000000000000000000000000000
XMM07=00000000000000000000000000000000

(addresses may vary)

during early BIOS boot. Probably a merge conflict, but I do not yet see
which one. Maybe someone has an immediate idea what could cause this.

Upstream does not show this regression.

Jan

-- 
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux

[PATCH] qemu-kvm: x86: Fix CPU initialization

[Jan Kiszka]

Fix regression in CPU initialization caused by merge a5b526135d and try
to avoid this in the future by dropping qemu-kvm specific pc_new_cpu. If
such refactoring is desired, it should go through upstream first.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---

 hw/pc.c |   44 ++++++++++++++++++--------------------------
 1 files changed, 18 insertions(+), 26 deletions(-)

diff --git a/hw/pc.c b/hw/pc.c
index 6c19f55..c4117e5 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -839,31 +839,6 @@ int cpu_is_bsp(CPUState *env)
 	return env->cpuid_apic_id == 0;
 }
 
-CPUState *pc_new_cpu(int cpu, const char *cpu_model, int pci_enabled)
-{
-        CPUState *env = cpu_init(cpu_model);
-        if (!env) {
-            fprintf(stderr, "Unable to find x86 CPU definition\n");
-            exit(1);
-        }
-        if (cpu != 0)
-            env->halted = 1;
-        if ((env->cpuid_features & CPUID_APIC) || smp_cpus > 1) {
-            env->cpuid_apic_id = env->cpu_index;
-            apic_init(env);
-        }
-        qemu_register_reset(main_cpu_reset, 0, env);
-        if (pci_enabled) {
-            apic_init(env);
-        }
-
-    /* kvm needs this to run after the apic is initialized. Otherwise,
-     * it can access invalid state and crash.
-     */
-    qemu_init_vcpu(env);
-	return env;
-}
-
 /* PC hardware initialisation */
 static void pc_init1(ram_addr_t ram_size,
                      const char *boot_device,
@@ -906,7 +881,24 @@ static void pc_init1(ram_addr_t ram_size,
     }
     
     for(i = 0; i < smp_cpus; i++) {
-	env = pc_new_cpu(i, cpu_model, pci_enabled);
+        env = cpu_init(cpu_model);
+        if (!env) {
+            fprintf(stderr, "Unable to find x86 CPU definition\n");
+            exit(1);
+        }
+        if (cpu != 0)
+            env->halted = 1;
+        if ((env->cpuid_features & CPUID_APIC) || smp_cpus > 1) {
+            env->cpuid_apic_id = env->cpu_index;
+            apic_init(env);
+        }
+        qemu_register_reset(main_cpu_reset, 0, env);
+
+        /*
+         * FIXME: qemu-kvm needs this after apic_init as it accesses APIC
+         * structures.
+         */
+        qemu_init_vcpu(env);
     }
 
     vmport_init();

[PATCH v2] qemu-kvm: x86: Fix CPU initialization

[Jan Kiszka]

Jan Kiszka wrote:
> Fix regression in CPU initialization caused by merge a5b526135d and try
> to avoid this in the future by dropping qemu-kvm specific pc_new_cpu. If
> such refactoring is desired, it should go through upstream first.

F...ine. I'll write a hundred times: "Don't post while still compiling."
Here is a version that only fixes the regression.

----------->

Fix regression in CPU initialization caused by merge a5b526135d.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---

 hw/pc.c |    3 ---
 1 files changed, 0 insertions(+), 3 deletions(-)

diff --git a/hw/pc.c b/hw/pc.c
index 6c19f55..cb5b4d0 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -853,9 +853,6 @@ CPUState *pc_new_cpu(int cpu, const char *cpu_model, int pci_enabled)
             apic_init(env);
         }
         qemu_register_reset(main_cpu_reset, 0, env);
-        if (pci_enabled) {
-            apic_init(env);
-        }
 
     /* kvm needs this to run after the apic is initialized. Otherwise,
      * it can access invalid state and crash.

Re: [PATCH v2] qemu-kvm: x86: Fix CPU initialization

[Avi Kivity]

On 06/19/2009 08:17 PM, Jan Kiszka wrote:
> Jan Kiszka wrote:
>    
>> Fix regression in CPU initialization caused by merge a5b526135d and try
>> to avoid this in the future by dropping qemu-kvm specific pc_new_cpu. If
>> such refactoring is desired, it should go through upstream first.
>>      
>
> F...ine. I'll write a hundred times: "Don't post while still compiling."
> Here is a version that only fixes the regression.
>    

Applied, thanks.

This has bitten us more than once.  Care to upstream pc_new_cpu()?  
we'll need it anyway when we upstream cpu hotplug, and this will make 
life easier for us here downstream.

-- 
error compiling committee.c: too many arguments to function