[Qemu-devel] [PATCH v2 0/8] x86: correctly implement soft reset

[Qemu-devel] [PATCH v2 0/8] x86: correctly implement soft reset

[Paolo Bonzini]

A repost of an old patch series, rebased and retested.  Patches 3 to 5
are new, everything else already carries a Reviewed-by.

v1->v2: compile-tested on ARM (I also have had a /dev/kvm on my ARM board
        for two hours now, but still no guest to try this on).

Paolo Bonzini (8):
  kvm: reset state from the CPU's reset method
  kvm: forward INIT signals coming from the chipset
  target-i386: fix set of registers zeroed on reset
  target-i386: preserve FPU and MSR state on INIT
  apic: do not accept SIPI on the bootstrap processor
  cpu: make CPU_INTERRUPT_RESET available on all targets
  pc: port 92 reset requires a low->high transition
  x86: correctly implement soft reset

 cpu-exec.c             | 23 ++++++++++---------
 cpus.c                 |  9 ++++++++
 hw/i386/pc.c           |  6 +++--
 hw/input/pckbd.c       |  5 +++--
 hw/intc/apic_common.c  |  2 +-
 hw/isa/lpc_ich9.c      | 12 ++++++++--
 hw/pci-host/piix.c     |  8 +++++--
 include/exec/cpu-all.h |  8 ++++---
 include/sysemu/cpus.h  |  1 +
 include/sysemu/kvm.h   |  2 --
 kvm-all.c              | 11 ---------
 target-arm/cpu.c       |  7 ++++++
 target-arm/kvm32.c     |  4 +---
 target-arm/kvm64.c     |  2 +-
 target-arm/kvm_arm.h   |  8 +++++++
 target-i386/cpu.c      | 11 +++++----
 target-i386/cpu.h      | 60 ++++++++++++++++++++++++++++++--------------------
 target-i386/helper.c   | 14 ++++++++++--
 target-i386/kvm.c      | 39 +++++++++++++++++++++-----------
 target-i386/kvm_i386.h |  2 ++
 target-ppc/cpu.h       |  3 ---
 target-ppc/kvm.c       |  4 ----
 target-s390x/cpu.c     |  4 ++++
 target-s390x/cpu.h     |  5 +++++
 target-s390x/kvm.c     |  6 +++--
 25 files changed, 165 insertions(+), 91 deletions(-)

-- 
1.8.3.1

[Qemu-devel] [PATCH v2 1/8] kvm: reset state from the CPU's reset method

[Paolo Bonzini]

Now that we have a CPU object with a reset method, it is better to
keep the KVM reset close to the CPU reset.  Using qemu_register_reset
as we do now keeps them far apart.

With this patch, PPC no longer calls the kvm_arch_ function, so
it can get removed there.  Other arches call it from their CPU
reset handler, and the function gets an ARMCPU/X86CPU/S390CPU.

Note that ARM- and s390-specific functions are called kvm_arm_*
and kvm_s390_*, while x86-specific functions are called kvm_arch_*.
That follows the convention used by the different architectures.
Changing that is the topic of a separate patch.

Reviewed-by: Gleb Natapov <gnatapov@redhat.com>
Reviewed-by: Andreas Färber <afaerber@suse.de>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 include/sysemu/kvm.h   |  2 --
 kvm-all.c              | 11 -----------
 target-arm/cpu.c       |  7 +++++++
 target-arm/kvm32.c     |  4 +---
 target-arm/kvm64.c     |  2 +-
 target-arm/kvm_arm.h   |  8 ++++++++
 target-i386/cpu.c      |  5 +++++
 target-i386/kvm.c      |  3 +--
 target-i386/kvm_i386.h |  1 +
 target-ppc/kvm.c       |  4 ----
 target-s390x/cpu.c     |  4 ++++
 target-s390x/cpu.h     |  5 +++++
 target-s390x/kvm.c     |  6 ++++--
 13 files changed, 37 insertions(+), 25 deletions(-)

diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
index 0bee1e8..cab306d 100644
--- a/include/sysemu/kvm.h
+++ b/include/sysemu/kvm.h
@@ -245,8 +245,6 @@ int kvm_arch_init_vcpu(CPUState *cpu);
 /* Returns VCPU ID to be used on KVM_CREATE_VCPU ioctl() */
 unsigned long kvm_arch_vcpu_id(CPUState *cpu);
 
-void kvm_arch_reset_vcpu(CPUState *cpu);
-
 int kvm_arch_on_sigbus_vcpu(CPUState *cpu, int code, void *addr);
 int kvm_arch_on_sigbus(int code, void *addr);
 
diff --git a/kvm-all.c b/kvm-all.c
index 82a9119..1285cf0 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -223,13 +223,6 @@ static int kvm_set_user_memory_region(KVMState *s, KVMSlot *slot)
     return kvm_vm_ioctl(s, KVM_SET_USER_MEMORY_REGION, &mem);
 }
 
-static void kvm_reset_vcpu(void *opaque)
-{
-    CPUState *cpu = opaque;
-
-    kvm_arch_reset_vcpu(cpu);
-}
-
 int kvm_init_vcpu(CPUState *cpu)
 {
     KVMState *s = kvm_state;
@@ -269,10 +262,6 @@ int kvm_init_vcpu(CPUState *cpu)
     }
 
     ret = kvm_arch_init_vcpu(cpu);
-    if (ret == 0) {
-        qemu_register_reset(kvm_reset_vcpu, cpu);
-        kvm_arch_reset_vcpu(cpu);
-    }
 err:
     return ret;
 }
diff --git a/target-arm/cpu.c b/target-arm/cpu.c
index c0ddc3e..6c6f2b3 100644
--- a/target-arm/cpu.c
+++ b/target-arm/cpu.c
@@ -29,6 +29,7 @@
 #include "hw/arm/arm.h"
 #include "sysemu/sysemu.h"
 #include "sysemu/kvm.h"
+#include "kvm_arm.h"
 
 static void arm_cpu_set_pc(CPUState *cs, vaddr value)
 {
@@ -165,6 +166,12 @@ static void arm_cpu_reset(CPUState *s)
      * tb_flush().
      */
     tb_flush(env);
+
+#ifndef CONFIG_USER_ONLY
+    if (kvm_enabled()) {
+        kvm_arm_reset_vcpu(cpu);
+    }
+#endif
 }
 
 #ifndef CONFIG_USER_ONLY
diff --git a/target-arm/kvm32.c b/target-arm/kvm32.c
index a690d99..b79750c 100644
--- a/target-arm/kvm32.c
+++ b/target-arm/kvm32.c
@@ -510,11 +510,9 @@ int kvm_arch_get_registers(CPUState *cs)
     return 0;
 }
 
-void kvm_arch_reset_vcpu(CPUState *cs)
+void kvm_arm_reset_vcpu(ARMCPU *cpu)
 {
     /* Feed the kernel back its initial register state */
-    ARMCPU *cpu = ARM_CPU(cs);
-
     memmove(cpu->cpreg_values, cpu->cpreg_reset_values,
             cpu->cpreg_array_len * sizeof(cpu->cpreg_values[0]));
 
diff --git a/target-arm/kvm64.c b/target-arm/kvm64.c
index e115879..c729b9e 100644
--- a/target-arm/kvm64.c
+++ b/target-arm/kvm64.c
@@ -260,6 +260,6 @@ int kvm_arch_get_registers(CPUState *cs)
     return ret;
 }
 
-void kvm_arch_reset_vcpu(CPUState *cs)
+void kvm_arm_reset_vcpu(ARMCPU *cpu)
 {
 }
diff --git a/target-arm/kvm_arm.h b/target-arm/kvm_arm.h
index 137c567..dc4e233 100644
--- a/target-arm/kvm_arm.h
+++ b/target-arm/kvm_arm.h
@@ -67,6 +67,14 @@ bool write_list_to_kvmstate(ARMCPU *cpu);
  */
 bool write_kvmstate_to_list(ARMCPU *cpu);
 
+/**
+ * kvm_arm_reset_vcpu:
+ * @cpu: ARMCPU
+ *
+ * Called at reset time to kernel registers to their initial values.
+ */
+void kvm_arm_reset_vcpu(ARMCPU *cpu);
+
 #ifdef CONFIG_KVM
 /**
  * kvm_arm_create_scratch_host_vcpu:
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 8fd1497..05c3005 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -24,6 +24,7 @@
 #include "cpu.h"
 #include "sysemu/kvm.h"
 #include "sysemu/cpus.h"
+#include "kvm_i386.h"
 #include "topology.h"
 
 #include "qemu/option.h"
@@ -2486,6 +2487,10 @@ static void x86_cpu_reset(CPUState *s)
     }
 
     s->halted = !cpu_is_bsp(cpu);
+
+    if (kvm_enabled()) {
+        kvm_arch_reset_vcpu(cpu);
+    }
 #endif
 }
 
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 4389959..2319d78 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -724,9 +724,8 @@ int kvm_arch_init_vcpu(CPUState *cs)
     return 0;
 }
 
-void kvm_arch_reset_vcpu(CPUState *cs)
+void kvm_arch_reset_vcpu(X86CPU *cpu)
 {
-    X86CPU *cpu = X86_CPU(cs);
     CPUX86State *env = &cpu->env;
 
     env->exception_injected = -1;
diff --git a/target-i386/kvm_i386.h b/target-i386/kvm_i386.h
index 4392ab4..b0b2193 100644
--- a/target-i386/kvm_i386.h
+++ b/target-i386/kvm_i386.h
@@ -14,6 +14,7 @@
 #include "sysemu/kvm.h"
 
 bool kvm_allows_irq0_override(void);
+void kvm_arch_reset_vcpu(X86CPU *cs);
 
 int kvm_device_pci_assign(KVMState *s, PCIHostDeviceAddress *dev_addr,
                           uint32_t flags, uint32_t *dev_id);
diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
index 9974b10..ff15820 100644
--- a/target-ppc/kvm.c
+++ b/target-ppc/kvm.c
@@ -434,10 +434,6 @@ int kvm_arch_init_vcpu(CPUState *cs)
     return ret;
 }
 
-void kvm_arch_reset_vcpu(CPUState *cpu)
-{
-}
-
 static void kvm_sw_tlb_put(PowerPCCPU *cpu)
 {
     CPUPPCState *env = &cpu->env;
diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
index dfd83e8..c3082b7 100644
--- a/target-s390x/cpu.c
+++ b/target-s390x/cpu.c
@@ -152,6 +152,10 @@ static void s390_cpu_full_reset(CPUState *s)
      * after incrementing the cpu counter */
 #if !defined(CONFIG_USER_ONLY)
     s->halted = 1;
+
+    if (kvm_enabled()) {
+        kvm_s390_reset_vcpu(cpu);
+    }
 #endif
     tlb_flush(s, 1);
 }
diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
index f332d41..e40382e 100644
--- a/target-s390x/cpu.h
+++ b/target-s390x/cpu.h
@@ -353,11 +353,16 @@ void s390x_cpu_timer(void *opaque);
 int s390_virtio_hypercall(CPUS390XState *env);
 
 #ifdef CONFIG_KVM
+void kvm_s390_reset_vcpu(S390CPU *cpu);
 void kvm_s390_interrupt(S390CPU *cpu, int type, uint32_t code);
 void kvm_s390_virtio_irq(S390CPU *cpu, int config_change, uint64_t token);
 void kvm_s390_interrupt_internal(S390CPU *cpu, int type, uint32_t parm,
                                  uint64_t parm64, int vm);
 #else
+static inline void kvm_s390_reset_vcpu(S390CPU *cpu)
+{
+}
+
 static inline void kvm_s390_interrupt(S390CPU *cpu, int type, uint32_t code)
 {
 }
diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
index 56b9af7..8c37a81 100644
--- a/target-s390x/kvm.c
+++ b/target-s390x/kvm.c
@@ -116,14 +116,16 @@ int kvm_arch_init_vcpu(CPUState *cpu)
     return 0;
 }
 
-void kvm_arch_reset_vcpu(CPUState *cpu)
+void kvm_s390_reset_vcpu(S390CPU *cpu)
 {
+    CPUState *cs = CPU(cpu);
+
     /* The initial reset call is needed here to reset in-kernel
      * vcpu data that we can't access directly from QEMU
      * (i.e. with older kernels which don't support sync_regs/ONE_REG).
      * Before this ioctl cpu_synchronize_state() is called in common kvm
      * code (kvm-all) */
-    if (kvm_vcpu_ioctl(cpu, KVM_S390_INITIAL_RESET, NULL)) {
+    if (kvm_vcpu_ioctl(cs, KVM_S390_INITIAL_RESET, NULL)) {
         perror("Can't reset vcpu\n");
     }
 }
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v2 1/8] kvm: reset state from the CPU's reset method

[Andreas Färber]

Am 02.05.2014 16:33, schrieb Paolo Bonzini:
> Now that we have a CPU object with a reset method, it is better to
> keep the KVM reset close to the CPU reset.  Using qemu_register_reset
> as we do now keeps them far apart.
> 
> With this patch, PPC no longer calls the kvm_arch_ function, so
> it can get removed there.  Other arches call it from their CPU
> reset handler, and the function gets an ARMCPU/X86CPU/S390CPU.
> 
> Note that ARM- and s390-specific functions are called kvm_arm_*
> and kvm_s390_*, while x86-specific functions are called kvm_arch_*.
> That follows the convention used by the different architectures.
> Changing that is the topic of a separate patch.
> 
> Reviewed-by: Gleb Natapov <gnatapov@redhat.com>
> Reviewed-by: Andreas Färber <afaerber@suse.de>

Re-reviewed, looks good now, please take it through the KVM tree.

Thanks,
Andreas

> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  include/sysemu/kvm.h   |  2 --
>  kvm-all.c              | 11 -----------
>  target-arm/cpu.c       |  7 +++++++
>  target-arm/kvm32.c     |  4 +---
>  target-arm/kvm64.c     |  2 +-
>  target-arm/kvm_arm.h   |  8 ++++++++
>  target-i386/cpu.c      |  5 +++++
>  target-i386/kvm.c      |  3 +--
>  target-i386/kvm_i386.h |  1 +
>  target-ppc/kvm.c       |  4 ----
>  target-s390x/cpu.c     |  4 ++++
>  target-s390x/cpu.h     |  5 +++++
>  target-s390x/kvm.c     |  6 ++++--
>  13 files changed, 37 insertions(+), 25 deletions(-)

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

[Qemu-devel] [PATCH v2 2/8] kvm: forward INIT signals coming from the chipset

[Paolo Bonzini]

Reviewed-by: Gleb Natapov <gnatapov@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 target-i386/helper.c   |  4 ++++
 target-i386/kvm.c      | 36 +++++++++++++++++++++++++-----------
 target-i386/kvm_i386.h |  1 +
 3 files changed, 30 insertions(+), 11 deletions(-)

diff --git a/target-i386/helper.c b/target-i386/helper.c
index 372f0e3..27b3582 100644
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -19,6 +19,7 @@
 
 #include "cpu.h"
 #include "sysemu/kvm.h"
+#include "kvm_i386.h"
 #ifndef CONFIG_USER_ONLY
 #include "sysemu/sysemu.h"
 #include "monitor/monitor.h"
@@ -1335,6 +1336,9 @@ void do_cpu_init(X86CPU *cpu)
     cpu_reset(cs);
     cs->interrupt_request = sipi;
     env->pat = pat;
+    if (kvm_enabled()) {
+        kvm_arch_do_init_vcpu(cpu);
+    }
     apic_init_reset(cpu->apic_state);
 }
 
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 2319d78..1c0565f 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -30,6 +30,8 @@
 #include "qemu/config-file.h"
 #include "hw/i386/pc.h"
 #include "hw/i386/apic.h"
+#include "hw/i386/apic_internal.h"
+#include "hw/i386/apic-msidef.h"
 #include "exec/ioport.h"
 #include <asm/hyperv.h>
 #include "hw/pci/pci.h"
@@ -739,6 +741,16 @@ void kvm_arch_reset_vcpu(X86CPU *cpu)
     }
 }
 
+void kvm_arch_do_init_vcpu(X86CPU *cpu)
+{
+    CPUX86State *env = &cpu->env;
+
+    /* APs get directly into wait-for-SIPI state.  */
+    if (env->mp_state == KVM_MP_STATE_UNINITIALIZED) {
+        env->mp_state = KVM_MP_STATE_INIT_RECEIVED;
+    }
+}
+
 static int kvm_get_supported_msrs(KVMState *s)
 {
     static int kvm_supported_msrs;
@@ -2004,14 +2016,15 @@ void kvm_arch_pre_run(CPUState *cpu, struct kvm_run *run)
         }
     }
 
-    if (!kvm_irqchip_in_kernel()) {
-        /* Force the VCPU out of its inner loop to process any INIT requests
-         * or pending TPR access reports. */
-        if (cpu->interrupt_request &
-            (CPU_INTERRUPT_INIT | CPU_INTERRUPT_TPR)) {
-            cpu->exit_request = 1;
-        }
+    /* Force the VCPU out of its inner loop to process any INIT requests
+     * or (for userspace APIC, but it is cheap to combine the checks here)
+     * pending TPR access reports.
+     */
+    if (cpu->interrupt_request & (CPU_INTERRUPT_INIT | CPU_INTERRUPT_TPR)) {
+        cpu->exit_request = 1;
+    }
 
+    if (!kvm_irqchip_in_kernel()) {
         /* Try to inject an interrupt if the guest can accept it */
         if (run->ready_for_interrupt_injection &&
             (cpu->interrupt_request & CPU_INTERRUPT_HARD) &&
@@ -2091,6 +2104,11 @@ int kvm_arch_process_async_events(CPUState *cs)
         }
     }
 
+    if (cs->interrupt_request & CPU_INTERRUPT_INIT) {
+        kvm_cpu_synchronize_state(cs);
+        do_cpu_init(cpu);
+    }
+
     if (kvm_irqchip_in_kernel()) {
         return 0;
     }
@@ -2104,10 +2122,6 @@ int kvm_arch_process_async_events(CPUState *cs)
         (cs->interrupt_request & CPU_INTERRUPT_NMI)) {
         cs->halted = 0;
     }
-    if (cs->interrupt_request & CPU_INTERRUPT_INIT) {
-        kvm_cpu_synchronize_state(cs);
-        do_cpu_init(cpu);
-    }
     if (cs->interrupt_request & CPU_INTERRUPT_SIPI) {
         kvm_cpu_synchronize_state(cs);
         do_cpu_sipi(cpu);
diff --git a/target-i386/kvm_i386.h b/target-i386/kvm_i386.h
index b0b2193..cac30fd 100644
--- a/target-i386/kvm_i386.h
+++ b/target-i386/kvm_i386.h
@@ -15,6 +15,7 @@
 
 bool kvm_allows_irq0_override(void);
 void kvm_arch_reset_vcpu(X86CPU *cs);
+void kvm_arch_do_init_vcpu(X86CPU *cs);
 
 int kvm_device_pci_assign(KVMState *s, PCIHostDeviceAddress *dev_addr,
                           uint32_t flags, uint32_t *dev_id);
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v2 2/8] kvm: forward INIT signals coming from the chipset

[Andreas Färber]

Am 02.05.2014 16:33, schrieb Paolo Bonzini:
> Reviewed-by: Gleb Natapov <gnatapov@redhat.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  target-i386/helper.c   |  4 ++++
>  target-i386/kvm.c      | 36 +++++++++++++++++++++++++-----------
>  target-i386/kvm_i386.h |  1 +
>  3 files changed, 30 insertions(+), 11 deletions(-)

Reviewed-by: Andreas Färber <afaerber@suse.de>

[...]
> diff --git a/target-i386/kvm_i386.h b/target-i386/kvm_i386.h
> index b0b2193..cac30fd 100644
> --- a/target-i386/kvm_i386.h
> +++ b/target-i386/kvm_i386.h
> @@ -15,6 +15,7 @@
>  
>  bool kvm_allows_irq0_override(void);
>  void kvm_arch_reset_vcpu(X86CPU *cs);
> +void kvm_arch_do_init_vcpu(X86CPU *cs);

Maybe tweak both patches to read "cpu" in the header?

Regards,
Andreas

>  
>  int kvm_device_pci_assign(KVMState *s, PCIHostDeviceAddress *dev_addr,
>                            uint32_t flags, uint32_t *dev_id);


-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

[Qemu-devel] [PATCH v2 3/8] target-i386: fix set of registers zeroed on reset

[Paolo Bonzini]

BND0-3, BNDCFGU, BNDCFGS, BNDSTATUS were not zeroed on reset, but they
should be (Intel Instruction Set Extensions Programming Reference
319433-015, pages 9-4 and 9-6).  Same for YMM.

XCR0 should be reset to 1.

TSC and TSC_RESET were zeroed already by the memset, remove the explicit
assignments.

Cc: Andreas Färber <afaerber@suse.de>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 target-i386/cpu.c |  3 +--
 target-i386/cpu.h | 11 ++++++-----
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 05c3005..78c1573 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -2477,8 +2477,7 @@ static void x86_cpu_reset(CPUState *s)
     cpu_breakpoint_remove_all(s, BP_CPU);
     cpu_watchpoint_remove_all(s, BP_CPU);
 
-    env->tsc_adjust = 0;
-    env->tsc = 0;
+    env->xcr0 = 1;
 
 #if !defined(CONFIG_USER_ONLY)
     /* We hard-wire the BSP to the first CPU. */
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 2a22a7d..e2244e9 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -797,6 +797,10 @@ typedef struct CPUX86State {
     target_ulong cr[5]; /* NOTE: cr1 is unused */
     int32_t a20_mask;
 
+    BNDReg bnd_regs[4];
+    BNDCSReg bndcs_regs;
+    uint64_t msr_bndcfgs;
+
     /* FPU state */
     unsigned int fpstt; /* top of stack index */
     uint16_t fpus;
@@ -819,6 +823,8 @@ typedef struct CPUX86State {
     XMMReg xmm_t0;
     MMXReg mmx_t0;
 
+    XMMReg ymmh_regs[CPU_NB_REGS];
+
     /* sysenter registers */
     uint32_t sysenter_cs;
     target_ulong sysenter_esp;
@@ -928,12 +934,7 @@ typedef struct CPUX86State {
     uint16_t fpus_vmstate;
     uint16_t fptag_vmstate;
     uint16_t fpregs_format_vmstate;
-
     uint64_t xstate_bv;
-    XMMReg ymmh_regs[CPU_NB_REGS];
-    BNDReg bnd_regs[4];
-    BNDCSReg bndcs_regs;
-    uint64_t msr_bndcfgs;
 
     uint64_t xcr0;
 
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v2 3/8] target-i386: fix set of registers zeroed on reset

[Andreas Färber]

Am 02.05.2014 16:33, schrieb Paolo Bonzini:
> BND0-3, BNDCFGU, BNDCFGS, BNDSTATUS were not zeroed on reset, but they
> should be (Intel Instruction Set Extensions Programming Reference
> 319433-015, pages 9-4 and 9-6).  Same for YMM.
> 
> XCR0 should be reset to 1.
> 
> TSC and TSC_RESET were zeroed already by the memset, remove the explicit
> assignments.
> 
> Cc: Andreas Färber <afaerber@suse.de>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Reviewed-by: Andreas Färber <afaerber@suse.de>

In general I'm happy if trivial target-specific tweaks like this can go
through someone else's tree. :)

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

[Qemu-devel] [PATCH v2 4/8] target-i386: preserve FPU and MSR state on INIT

[Paolo Bonzini]

Most MSRs, plus the FPU, MMX, MXCSR, XMM and YMM registers should not
be zeroed on INIT (Table 9-1 in the Intel SDM).  Copy them out of
CPUX86State and back in, instead of special casing env->pat.

The relevant fields are already consecutive except PAT and SMBASE.
However:

- KVM and Hyper-V MSRs should be reset because they include memory
locations written by the hypervisor.  These MSRs are moved together
at the end of the preserved area.

- SVM state can be moved out of the way since it is written by VMRUN.

Cc: Andreas Färber <afaerber@suse.de>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 target-i386/cpu.c    |  3 +--
 target-i386/cpu.h    | 42 ++++++++++++++++++++++++++----------------
 target-i386/helper.c | 10 ++++++++--
 3 files changed, 35 insertions(+), 20 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 78c1573..c14bd12 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -2410,8 +2410,7 @@ static void x86_cpu_reset(CPUState *s)
 
     xcc->parent_reset(s);
 
-
-    memset(env, 0, offsetof(CPUX86State, pat));
+    memset(env, 0, offsetof(CPUX86State, cpuid_level));
 
     tlb_flush(s, 1);
 
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index e2244e9..c205058 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -801,6 +801,9 @@ typedef struct CPUX86State {
     BNDCSReg bndcs_regs;
     uint64_t msr_bndcfgs;
 
+    /* Beginning of state preserved by INIT (dummy marker).  */
+    struct {} start_init_save;
+
     /* FPU state */
     unsigned int fpstt; /* top of stack index */
     uint16_t fpus;
@@ -833,15 +836,6 @@ typedef struct CPUX86State {
     uint64_t star;
 
     uint64_t vm_hsave;
-    uint64_t vm_vmcb;
-    uint64_t tsc_offset;
-    uint64_t intercept;
-    uint16_t intercept_cr_read;
-    uint16_t intercept_cr_write;
-    uint16_t intercept_dr_read;
-    uint16_t intercept_dr_write;
-    uint32_t intercept_exceptions;
-    uint8_t v_tpr;
 
 #ifdef TARGET_X86_64
     target_ulong lstar;
@@ -849,11 +843,6 @@ typedef struct CPUX86State {
     target_ulong fmask;
     target_ulong kernelgsbase;
 #endif
-    uint64_t system_time_msr;
-    uint64_t wall_clock_msr;
-    uint64_t steal_time_msr;
-    uint64_t async_pf_en_msr;
-    uint64_t pv_eoi_en_msr;
 
     uint64_t tsc;
     uint64_t tsc_adjust;
@@ -870,6 +859,19 @@ typedef struct CPUX86State {
     uint64_t msr_fixed_counters[MAX_FIXED_COUNTERS];
     uint64_t msr_gp_counters[MAX_GP_COUNTERS];
     uint64_t msr_gp_evtsel[MAX_GP_COUNTERS];
+
+    uint64_t pat;
+    uint32_t smbase;
+
+    /* End of state preserved by INIT (dummy marker).  */
+    struct {} end_init_save;
+
+    uint64_t system_time_msr;
+    uint64_t wall_clock_msr;
+    uint64_t steal_time_msr;
+    uint64_t async_pf_en_msr;
+    uint64_t pv_eoi_en_msr;
+
     uint64_t msr_hv_hypercall;
     uint64_t msr_hv_guest_os_id;
     uint64_t msr_hv_vapic;
@@ -884,9 +886,18 @@ typedef struct CPUX86State {
         struct CPUBreakpoint *cpu_breakpoint[4];
         struct CPUWatchpoint *cpu_watchpoint[4];
     }; /* break/watchpoints for dr[0..3] */
-    uint32_t smbase;
     int old_exception;  /* exception in flight */
 
+    uint64_t vm_vmcb;
+    uint64_t tsc_offset;
+    uint64_t intercept;
+    uint16_t intercept_cr_read;
+    uint16_t intercept_cr_write;
+    uint16_t intercept_dr_read;
+    uint16_t intercept_dr_write;
+    uint32_t intercept_exceptions;
+    uint8_t v_tpr;
+
     /* KVM states, automatically cleared on reset */
     uint8_t nmi_injected;
     uint8_t nmi_pending;
@@ -894,7 +905,6 @@ typedef struct CPUX86State {
     CPU_COMMON
 
     /* Fields from here on are preserved across CPU reset. */
-    uint64_t pat;
 
     /* processor features (e.g. for CPUID insn) */
     uint32_t cpuid_level;
diff --git a/target-i386/helper.c b/target-i386/helper.c
index 27b3582..46d20e4 100644
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -1330,12 +1330,18 @@ void do_cpu_init(X86CPU *cpu)
 {
     CPUState *cs = CPU(cpu);
     CPUX86State *env = &cpu->env;
+    CPUX86State *save = g_new(CPUX86State, 1);
     int sipi = cs->interrupt_request & CPU_INTERRUPT_SIPI;
-    uint64_t pat = env->pat;
+
+    *save = *env;
 
     cpu_reset(cs);
     cs->interrupt_request = sipi;
-    env->pat = pat;
+    memcpy(&env->start_init_save, &save->start_init_save,
+           offsetof(CPUX86State, end_init_save) -
+           offsetof(CPUX86State, start_init_save));
+    g_free(save);
+
     if (kvm_enabled()) {
         kvm_arch_do_init_vcpu(cpu);
     }
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v2 4/8] target-i386: preserve FPU and MSR state on INIT

[Andreas Färber]

Am 02.05.2014 16:33, schrieb Paolo Bonzini:
> Most MSRs, plus the FPU, MMX, MXCSR, XMM and YMM registers should not
> be zeroed on INIT (Table 9-1 in the Intel SDM).  Copy them out of
> CPUX86State and back in, instead of special casing env->pat.
> 
> The relevant fields are already consecutive except PAT and SMBASE.
> However:
> 
> - KVM and Hyper-V MSRs should be reset because they include memory
> locations written by the hypervisor.  These MSRs are moved together
> at the end of the preserved area.
> 
> - SVM state can be moved out of the way since it is written by VMRUN.
> 
> Cc: Andreas Färber <afaerber@suse.de>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  target-i386/cpu.c    |  3 +--
>  target-i386/cpu.h    | 42 ++++++++++++++++++++++++++----------------
>  target-i386/helper.c | 10 ++++++++--
>  3 files changed, 35 insertions(+), 20 deletions(-)

Fine with me. You might as well use a third marker for zeroed-on-reset
to avoid the pat -> cpuid_level change.

If we want to widen this pattern, a macro might make sense.

Regards,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

[Qemu-devel] [PATCH v2 5/8] apic: do not accept SIPI on the bootstrap processor

[Paolo Bonzini]

SIPI interrupts are ignored on the bootstrap.  Never accept one.

Cc: Andreas Färber <afaerber@suse.de>
Cc: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/intc/apic_common.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
index 7ecce2d..05c0e08 100644
--- a/hw/intc/apic_common.c
+++ b/hw/intc/apic_common.c
@@ -200,7 +200,7 @@ void apic_init_reset(DeviceState *dev)
     s->initial_count = 0;
     s->initial_count_load_time = 0;
     s->next_time = 0;
-    s->wait_for_sipi = 1;
+    s->wait_for_sipi = !cpu_is_bsp(s->cpu);
 
     if (s->timer) {
         timer_del(s->timer);
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v2 5/8] apic: do not accept SIPI on the bootstrap processor

[Andreas Färber]

Am 02.05.2014 16:33, schrieb Paolo Bonzini:
> SIPI interrupts are ignored on the bootstrap.  Never accept one.
> 
> Cc: Andreas Färber <afaerber@suse.de>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  hw/intc/apic_common.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
> index 7ecce2d..05c0e08 100644
> --- a/hw/intc/apic_common.c
> +++ b/hw/intc/apic_common.c
> @@ -200,7 +200,7 @@ void apic_init_reset(DeviceState *dev)
>      s->initial_count = 0;
>      s->initial_count_load_time = 0;
>      s->next_time = 0;
> -    s->wait_for_sipi = 1;
> +    s->wait_for_sipi = !cpu_is_bsp(s->cpu);

I wondered whether this will work on hot-add. I believe so, since the
device is reset on realized=true after executing the realize hook, and
the APIC gets realized as part of the CPU (but before it's reset).

Reviewed-by: Andreas Färber <afaerber@suse.de>

Andreas

>  
>      if (s->timer) {
>          timer_del(s->timer);

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

[Qemu-devel] [PATCH v2 6/8] cpu: make CPU_INTERRUPT_RESET available on all targets

[Paolo Bonzini]

On the x86, some devices need access to the CPU reset pin (INIT#).
Provide a generic service to do this, using one of the internal
cpu_interrupt targets.  Generalize the PPC-specific code for
CPU_INTERRUPT_RESET to other targets.

Since PPC does not support migration across QEMU versions (its
machine types are not versioned yet), I picked the value that
is used on x86, CPU_INTERRUPT_TGT_INT_1.  Consequently, TGT_INT_2
and TGT_INT_3 are shifted down by one while keeping their value.

Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 cpu-exec.c             | 23 +++++++++++++----------
 cpus.c                 |  9 +++++++++
 include/exec/cpu-all.h |  8 +++++---
 include/sysemu/cpus.h  |  1 +
 target-i386/cpu.h      |  7 ++++---
 target-ppc/cpu.h       |  3 ---
 6 files changed, 32 insertions(+), 19 deletions(-)

diff --git a/cpu-exec.c b/cpu-exec.c
index 2f54054..38e5f02 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -336,19 +336,25 @@ int cpu_exec(CPUArchState *env)
                     }
 #endif
 #if defined(TARGET_I386)
+                    if (interrupt_request & CPU_INTERRUPT_INIT) {
+                        cpu_svm_check_intercept_param(env, SVM_EXIT_INIT, 0);
+                        do_cpu_init(x86_cpu);
+                        cpu->exception_index = EXCP_HALTED;
+                        cpu_loop_exit(cpu);
+                    }
+#else
+                    if (interrupt_request & CPU_INTERRUPT_RESET) {
+                        cpu_reset(cpu);
+                    }
+#endif
+#if defined(TARGET_I386)
 #if !defined(CONFIG_USER_ONLY)
                     if (interrupt_request & CPU_INTERRUPT_POLL) {
                         cpu->interrupt_request &= ~CPU_INTERRUPT_POLL;
                         apic_poll_irq(x86_cpu->apic_state);
                     }
 #endif
-                    if (interrupt_request & CPU_INTERRUPT_INIT) {
-                            cpu_svm_check_intercept_param(env, SVM_EXIT_INIT,
-                                                          0);
-                            do_cpu_init(x86_cpu);
-                            cpu->exception_index = EXCP_HALTED;
-                            cpu_loop_exit(cpu);
-                    } else if (interrupt_request & CPU_INTERRUPT_SIPI) {
+                    if (interrupt_request & CPU_INTERRUPT_SIPI) {
                             do_cpu_sipi(x86_cpu);
                     } else if (env->hflags2 & HF2_GIF_MASK) {
                         if ((interrupt_request & CPU_INTERRUPT_SMI) &&
@@ -405,9 +411,6 @@ int cpu_exec(CPUArchState *env)
                         }
                     }
 #elif defined(TARGET_PPC)
-                    if ((interrupt_request & CPU_INTERRUPT_RESET)) {
-                        cpu_reset(cpu);
-                    }
                     if (interrupt_request & CPU_INTERRUPT_HARD) {
                         ppc_hw_interrupt(env);
                         if (env->pending_interrupts == 0) {
diff --git a/cpus.c b/cpus.c
index 1104d61..daf3ffa 100644
--- a/cpus.c
+++ b/cpus.c
@@ -495,6 +495,15 @@ void hw_error(const char *fmt, ...)
     abort();
 }
 
+void cpu_soft_reset(void)
+{
+    CPUState *cpu;
+
+    CPU_FOREACH(cpu) {
+        cpu_interrupt(cpu, CPU_INTERRUPT_RESET);
+    }
+}
+
 void cpu_synchronize_all_states(void)
 {
     CPUState *cpu;
diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
index fb649a4..9cab592 100644
--- a/include/exec/cpu-all.h
+++ b/include/exec/cpu-all.h
@@ -381,6 +381,9 @@ CPUArchState *cpu_copy(CPUArchState *env);
 /* Debug event pending.  */
 #define CPU_INTERRUPT_DEBUG       0x0080
 
+/* Reset signal.  */
+#define CPU_INTERRUPT_RESET       0x0400
+
 /* Several target-specific external hardware interrupts.  Each target/cpu.h
    should define proper names based on these defines.  */
 #define CPU_INTERRUPT_TGT_EXT_0   0x0008
@@ -395,9 +398,8 @@ CPUArchState *cpu_copy(CPUArchState *env);
    instruction being executed.  These, therefore, are not masked while
    single-stepping within the debugger.  */
 #define CPU_INTERRUPT_TGT_INT_0   0x0100
-#define CPU_INTERRUPT_TGT_INT_1   0x0400
-#define CPU_INTERRUPT_TGT_INT_2   0x0800
-#define CPU_INTERRUPT_TGT_INT_3   0x2000
+#define CPU_INTERRUPT_TGT_INT_1   0x0800
+#define CPU_INTERRUPT_TGT_INT_2   0x2000
 
 /* First unused bit: 0x4000.  */
 
diff --git a/include/sysemu/cpus.h b/include/sysemu/cpus.h
index 6502488..87b9829 100644
--- a/include/sysemu/cpus.h
+++ b/include/sysemu/cpus.h
@@ -7,6 +7,7 @@ void resume_all_vcpus(void);
 void pause_all_vcpus(void);
 void cpu_stop_current(void);
 
+void cpu_soft_reset(void);
 void cpu_synchronize_all_states(void);
 void cpu_synchronize_all_post_reset(void);
 void cpu_synchronize_all_post_init(void);
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index c205058..61ba6eb 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -606,10 +606,11 @@ typedef uint32_t FeatureWordArray[FEATURE_WORDS];
 #define CPU_INTERRUPT_NMI       CPU_INTERRUPT_TGT_EXT_3
 #define CPU_INTERRUPT_MCE       CPU_INTERRUPT_TGT_EXT_4
 #define CPU_INTERRUPT_VIRQ      CPU_INTERRUPT_TGT_INT_0
-#define CPU_INTERRUPT_INIT      CPU_INTERRUPT_TGT_INT_1
-#define CPU_INTERRUPT_SIPI      CPU_INTERRUPT_TGT_INT_2
-#define CPU_INTERRUPT_TPR       CPU_INTERRUPT_TGT_INT_3
+#define CPU_INTERRUPT_SIPI      CPU_INTERRUPT_TGT_INT_1
+#define CPU_INTERRUPT_TPR       CPU_INTERRUPT_TGT_INT_2
 
+/* Use a clearer name for this.  */
+#define CPU_INTERRUPT_INIT      CPU_INTERRUPT_RESET
 
 typedef enum {
     CC_OP_DYNAMIC, /* must use dynamic code to get cc_op */
diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
index d498340..75ed5fa 100644
--- a/target-ppc/cpu.h
+++ b/target-ppc/cpu.h
@@ -2042,9 +2042,6 @@ enum {
     PPC_INTERRUPT_PERFM,          /* Performance monitor interrupt        */
 };
 
-/* CPU should be reset next, restart from scratch afterwards */
-#define CPU_INTERRUPT_RESET       CPU_INTERRUPT_TGT_INT_0
-
 /*****************************************************************************/
 
 static inline target_ulong cpu_read_xer(CPUPPCState *env)
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v2 6/8] cpu: make CPU_INTERRUPT_RESET available on all targets

[Andreas Färber]

Am 02.05.2014 16:33, schrieb Paolo Bonzini:
> On the x86, some devices need access to the CPU reset pin (INIT#).
> Provide a generic service to do this, using one of the internal
> cpu_interrupt targets.  Generalize the PPC-specific code for
> CPU_INTERRUPT_RESET to other targets.
> 
> Since PPC does not support migration across QEMU versions (its
> machine types are not versioned yet), I picked the value that
> is used on x86, CPU_INTERRUPT_TGT_INT_1.  Consequently, TGT_INT_2
> and TGT_INT_3 are shifted down by one while keeping their value.
> 
> Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  cpu-exec.c             | 23 +++++++++++++----------
>  cpus.c                 |  9 +++++++++
>  include/exec/cpu-all.h |  8 +++++---
>  include/sysemu/cpus.h  |  1 +
>  target-i386/cpu.h      |  7 ++++---
>  target-ppc/cpu.h       |  3 ---
>  6 files changed, 32 insertions(+), 19 deletions(-)

No objection from my side, but I thought there had been agreement among
Anthony, Peter and others that soft-reset is nothing generic that can be
implemented as API?

s390x has multiple ways to do resets, same for ppc, and I thought the
suggested way to implement them was a qemu_irq in the particular piece
of hardware together with custom reset functions as done for s390x?

CC'ing some more maintainers.

IIRC Richard was against exposing target interrupt codes to generic code
when I tried to clean up some header by moving things to qom/cpu.h.

Regards,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH v2 6/8] cpu: make CPU_INTERRUPT_RESET available on all targets

[Peter Maydell]

On 12 May 2014 08:47, Andreas Färber <afaerber@suse.de> wrote:
> Am 02.05.2014 16:33, schrieb Paolo Bonzini:
>> On the x86, some devices need access to the CPU reset pin (INIT#).
>> Provide a generic service to do this, using one of the internal
>> cpu_interrupt targets.  Generalize the PPC-specific code for
>> CPU_INTERRUPT_RESET to other targets.
>>
>> Since PPC does not support migration across QEMU versions (its
>> machine types are not versioned yet), I picked the value that
>> is used on x86, CPU_INTERRUPT_TGT_INT_1.  Consequently, TGT_INT_2
>> and TGT_INT_3 are shifted down by one while keeping their value.

> No objection from my side, but I thought there had been agreement among
> Anthony, Peter and others that soft-reset is nothing generic that can be
> implemented as API?
>
> s390x has multiple ways to do resets, same for ppc, and I thought the
> suggested way to implement them was a qemu_irq in the particular piece
> of hardware together with custom reset functions as done for s390x?

I think the right way to expose reset to the outside world is via
a qemu_irq line, yes, but possibly the implementation inside the
CPU object might use a CPU_INTERRUPT_* bit (compare the way
that ARM IRQ and FIQ are qemu_irq lines to the outside world
but operate by just setting bits for the mainloop to check).

This patch also seems to be eliding the difference between
"reset signal asserted, stop doing stuff" and "reset signal
deasserted, start executing code again".

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 6/8] cpu: make CPU_INTERRUPT_RESET available on all targets

[Paolo Bonzini]

Il 12/05/2014 11:41, Peter Maydell ha scritto:
> On 12 May 2014 08:47, Andreas Färber <afaerber@suse.de> wrote:
>> Am 02.05.2014 16:33, schrieb Paolo Bonzini:
>>> On the x86, some devices need access to the CPU reset pin (INIT#).
>>> Provide a generic service to do this, using one of the internal
>>> cpu_interrupt targets.  Generalize the PPC-specific code for
>>> CPU_INTERRUPT_RESET to other targets.
>>>
>>> Since PPC does not support migration across QEMU versions (its
>>> machine types are not versioned yet), I picked the value that
>>> is used on x86, CPU_INTERRUPT_TGT_INT_1.  Consequently, TGT_INT_2
>>> and TGT_INT_3 are shifted down by one while keeping their value.
>
>> No objection from my side, but I thought there had been agreement among
>> Anthony, Peter and others that soft-reset is nothing generic that can be
>> implemented as API?
>>
>> s390x has multiple ways to do resets, same for ppc, and I thought the
>> suggested way to implement them was a qemu_irq in the particular piece
>> of hardware together with custom reset functions as done for s390x?
>
> I think the right way to expose reset to the outside world is via
> a qemu_irq line, yes, but possibly the implementation inside the
> CPU object might use a CPU_INTERRUPT_* bit (compare the way
> that ARM IRQ and FIQ are qemu_irq lines to the outside world
> but operate by just setting bits for the mainloop to check).

Ok, I'll queue patches 1-7 and rewrite patch 8 to use a qemu_irq.

> This patch also seems to be eliding the difference between
> "reset signal asserted, stop doing stuff" and "reset signal
> deasserted, start executing code again".

In the x86 architecture it is actually not defined whether the reset is 
edge-triggered or level-triggered, so that's okay I think.

Paolo

Re: [Qemu-devel] [PATCH v2 6/8] cpu: make CPU_INTERRUPT_RESET available on all targets

[Peter Maydell]

On 2 May 2014 15:33, Paolo Bonzini <pbonzini@redhat.com> wrote:
> On the x86, some devices need access to the CPU reset pin (INIT#).
> Provide a generic service to do this, using one of the internal
> cpu_interrupt targets.  Generalize the PPC-specific code for
> CPU_INTERRUPT_RESET to other targets.
>
> Since PPC does not support migration across QEMU versions (its
> machine types are not versioned yet), I picked the value that
> is used on x86, CPU_INTERRUPT_TGT_INT_1.  Consequently, TGT_INT_2
> and TGT_INT_3 are shifted down by one while keeping their value.
>
> Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  cpu-exec.c             | 23 +++++++++++++----------
>  cpus.c                 |  9 +++++++++
>  include/exec/cpu-all.h |  8 +++++---
>  include/sysemu/cpus.h  |  1 +
>  target-i386/cpu.h      |  7 ++++---
>  target-ppc/cpu.h       |  3 ---
>  6 files changed, 32 insertions(+), 19 deletions(-)
>
> diff --git a/cpu-exec.c b/cpu-exec.c
> index 2f54054..38e5f02 100644
> --- a/cpu-exec.c
> +++ b/cpu-exec.c
> @@ -336,19 +336,25 @@ int cpu_exec(CPUArchState *env)
>                      }
>  #endif
>  #if defined(TARGET_I386)
> +                    if (interrupt_request & CPU_INTERRUPT_INIT) {
> +                        cpu_svm_check_intercept_param(env, SVM_EXIT_INIT, 0);
> +                        do_cpu_init(x86_cpu);
> +                        cpu->exception_index = EXCP_HALTED;
> +                        cpu_loop_exit(cpu);
> +                    }
> +#else
> +                    if (interrupt_request & CPU_INTERRUPT_RESET) {
> +                        cpu_reset(cpu);
> +                    }
> +#endif

I was looking at cleaning up the horrible ifdef ladder a little
lower in this function, and I noticed this code had been
added recently. Why is TARGET_I386 a special case here?
New #ifdef TARGET_* here are pretty bogus and we should
try to avoid them.

Could we have the CPU_INTERRUPT_RESET check be all-targets
and move the INTERRUPT_INIT check down below it to
be with all the other x86 specific interrupt test code ?

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 6/8] cpu: make CPU_INTERRUPT_RESET available on all targets

[Paolo Bonzini]

Il 23/05/2014 19:59, Peter Maydell ha scritto:
> On 2 May 2014 15:33, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> On the x86, some devices need access to the CPU reset pin (INIT#).
>> Provide a generic service to do this, using one of the internal
>> cpu_interrupt targets.  Generalize the PPC-specific code for
>> CPU_INTERRUPT_RESET to other targets.
>>
>> Since PPC does not support migration across QEMU versions (its
>> machine types are not versioned yet), I picked the value that
>> is used on x86, CPU_INTERRUPT_TGT_INT_1.  Consequently, TGT_INT_2
>> and TGT_INT_3 are shifted down by one while keeping their value.
>>
>> Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>> ---
>>  cpu-exec.c             | 23 +++++++++++++----------
>>  cpus.c                 |  9 +++++++++
>>  include/exec/cpu-all.h |  8 +++++---
>>  include/sysemu/cpus.h  |  1 +
>>  target-i386/cpu.h      |  7 ++++---
>>  target-ppc/cpu.h       |  3 ---
>>  6 files changed, 32 insertions(+), 19 deletions(-)
>>
>> diff --git a/cpu-exec.c b/cpu-exec.c
>> index 2f54054..38e5f02 100644
>> --- a/cpu-exec.c
>> +++ b/cpu-exec.c
>> @@ -336,19 +336,25 @@ int cpu_exec(CPUArchState *env)
>>                      }
>>  #endif
>>  #if defined(TARGET_I386)
>> +                    if (interrupt_request & CPU_INTERRUPT_INIT) {
>> +                        cpu_svm_check_intercept_param(env, SVM_EXIT_INIT, 0);
>> +                        do_cpu_init(x86_cpu);
>> +                        cpu->exception_index = EXCP_HALTED;
>> +                        cpu_loop_exit(cpu);
>> +                    }
>> +#else
>> +                    if (interrupt_request & CPU_INTERRUPT_RESET) {
>> +                        cpu_reset(cpu);
>> +                    }
>> +#endif
>
> I was looking at cleaning up the horrible ifdef ladder a little
> lower in this function, and I noticed this code had been
> added recently. Why is TARGET_I386 a special case here?

Because a hypervisor (cpu_svm_check_intercept_param) can block the 
interrupt.  Note that CPU_INTERRUPT_INIT is actually the same bit as 
CPU_INTERRUPT_RESET.

The whole #ifdef mess should probably be changed to a function in cpu.c, 
now that we don't have AREG0 constraints anymore.

Paolo

Re: [Qemu-devel] [PATCH v2 6/8] cpu: make CPU_INTERRUPT_RESET available on all targets

[Peter Maydell]

On 23 May 2014 19:10, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 23/05/2014 19:59, Peter Maydell ha scritto:
>
>> On 2 May 2014 15:33, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> I was looking at cleaning up the horrible ifdef ladder a little
>> lower in this function, and I noticed this code had been
>> added recently. Why is TARGET_I386 a special case here?
>
>
> Because a hypervisor (cpu_svm_check_intercept_param) can block the
> interrupt.  Note that CPU_INTERRUPT_INIT is actually the same bit as
> CPU_INTERRUPT_RESET.

Ugh. This suggests reset is probably not really generic...

> The whole #ifdef mess should probably be changed to a function in cpu.c, now
> that we don't have AREG0 constraints anymore.

Well, I'm planning to move the bodies of all the ifdefs into
a cpu_check_interrupts() provided by the target's cpu.h[*].
This x86 bit is just awkward because it means there's
x86 stuff both before and after the generic reset code.

[*] not a cpu method since it seemed like it would be
a bad idea to have a function pointer call every
time round the main loop when there's a blocked
interrupt...

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 6/8] cpu: make CPU_INTERRUPT_RESET available on all targets

[Paolo Bonzini]

Il 24/05/2014 10:30, Peter Maydell ha scritto:
> Well, I'm planning to move the bodies of all the ifdefs into
> a cpu_check_interrupts() provided by the target's cpu.h[*].
> This x86 bit is just awkward because it means there's
> x86 stuff both before and after the generic reset code.

What about

        if (interrupt_request & CPU_INTERRUPT_DEBUG) {
            cpu->interrupt_request &= ~CPU_INTERRUPT_DEBUG;
            cpu->exception_index = EXCP_DEBUG;
            cpu_loop_exit(cpu);
        }
	if (!cpu_check_interrupts(...)) {
            if (interrupt_request & CPU_INTERRUPT_HALT) {
                cpu->interrupt_request &= ~CPU_INTERRUPT_HALT;
                cpu->halted = 1;
                cpu->exception_index = EXCP_HLT;
                cpu_loop_exit(cpu);
            }
            if (interrupt_request & CPU_INTERRUPT_RESET) {
                cpu_reset(cpu);
            }
	}

Then:
- only X86 returns 1 for CPU_INTERRUPT_RESET 
- all except ARM/SPARC/MIPS/PPC/Alpha/cris/MicroBlaze/LM32/Unicore32
return 1 for CPU_INTERRUPT_HALT

> [*] not a cpu method since it seemed like it would be
> a bad idea to have a function pointer call every
> time round the main loop when there's a blocked
> interrupt...

We have that already for cc->do_interrupt, which could be
"devirtualized" if you add a check_interrupts method...  In
the end you'd be adding a function pointer call for all
interrupt requests but removing one for CPU_INTERRUPT_HARD
(and FIQ too on ARM).  That should be a wash.

Paolo

Re: [Qemu-devel] [PATCH v2 6/8] cpu: make CPU_INTERRUPT_RESET available on all targets

[Peter Maydell]

On 24 May 2014 13:59, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 24/05/2014 10:30, Peter Maydell ha scritto:
>> Well, I'm planning to move the bodies of all the ifdefs into
>> a cpu_check_interrupts() provided by the target's cpu.h[*].
>> This x86 bit is just awkward because it means there's
>> x86 stuff both before and after the generic reset code.
>
> What about
>
>         if (interrupt_request & CPU_INTERRUPT_DEBUG) {
>             cpu->interrupt_request &= ~CPU_INTERRUPT_DEBUG;
>             cpu->exception_index = EXCP_DEBUG;
>             cpu_loop_exit(cpu);
>         }
>         if (!cpu_check_interrupts(...)) {
>             if (interrupt_request & CPU_INTERRUPT_HALT) {
>                 cpu->interrupt_request &= ~CPU_INTERRUPT_HALT;
>                 cpu->halted = 1;
>                 cpu->exception_index = EXCP_HLT;
>                 cpu_loop_exit(cpu);
>             }
>             if (interrupt_request & CPU_INTERRUPT_RESET) {
>                 cpu_reset(cpu);
>             }
>         }
>
> Then:
> - only X86 returns 1 for CPU_INTERRUPT_RESET
> - all except ARM/SPARC/MIPS/PPC/Alpha/cris/MicroBlaze/LM32/Unicore32
> return 1 for CPU_INTERRUPT_HALT

That last point sounds wrong, at least -- halt should work
the same way for everything. If the target doesn't want
to halt it should never set the HALT bit in interrupt_request.

>> [*] not a cpu method since it seemed like it would be
>> a bad idea to have a function pointer call every
>> time round the main loop when there's a blocked
>> interrupt...
>
> We have that already for cc->do_interrupt, which could be
> "devirtualized" if you add a check_interrupts method...  In
> the end you'd be adding a function pointer call for all
> interrupt requests but removing one for CPU_INTERRUPT_HARD
> (and FIQ too on ARM).  That should be a wash.

But we only call cc->do_interrupt if we're going to actually
*take* an interrupt, in which case the bulk of the cost is
actually doing the work. I don't want to call via a pointer
just for the other end to say "actually PSTATE_I is set
because the guest has interrupts blocked, so don't do
anything".

thanks
-- PMM

[Qemu-devel] [PATCH v2 7/8] pc: port 92 reset requires a low->high transition

[Paolo Bonzini]

The PIIX datasheet says that "before another INIT pulse can be
generated via [port 92h], [bit 0] must be written back to a
zero.

This bug is masked right now because a full reset will clear the
value of port 92h.  But once we implement soft reset correctly,
the next attempt to enable the A20 line by setting bit 1 (and
leaving the others untouched) will cause another reset.

Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/i386/pc.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 14f0d91..a59e958 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -471,11 +471,12 @@ static void port92_write(void *opaque, hwaddr addr, uint64_t val,
                          unsigned size)
 {
     Port92State *s = opaque;
+    int oldval = s->outport;
 
     DPRINTF("port92: write 0x%02x\n", val);
     s->outport = val;
     qemu_set_irq(*s->a20_out, (val >> 1) & 1);
-    if (val & 1) {
+    if ((val & 1) && !(oldval & 1)) {
         qemu_system_reset_request();
     }
 }
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v2 7/8] pc: port 92 reset requires a low->high transition

[Andreas Färber]

Am 02.05.2014 16:33, schrieb Paolo Bonzini:
> The PIIX datasheet says that "before another INIT pulse can be
> generated via [port 92h], [bit 0] must be written back to a
> zero.
> 
> This bug is masked right now because a full reset will clear the
> value of port 92h.  But once we implement soft reset correctly,
> the next attempt to enable the A20 line by setting bit 1 (and
> leaving the others untouched) will cause another reset.
> 
> Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Reviewed-by: Andreas Färber <afaerber@suse.de>

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

[Qemu-devel] [PATCH v2 8/8] x86: correctly implement soft reset

[Paolo Bonzini]

Do not do a hard reset for port 92h, keyboard controller, or cf9h soft reset.
These only reset the CPU.

Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/i386/pc.c       |  3 ++-
 hw/input/pckbd.c   |  5 +++--
 hw/isa/lpc_ich9.c  | 12 ++++++++++--
 hw/pci-host/piix.c |  8 ++++++--
 4 files changed, 21 insertions(+), 7 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index a59e958..8716864 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -45,6 +45,7 @@
 #include "kvm_i386.h"
 #include "hw/xen/xen.h"
 #include "sysemu/blockdev.h"
+#include "sysemu/cpus.h"
 #include "hw/block/block.h"
 #include "ui/qemu-spice.h"
 #include "exec/memory.h"
@@ -477,7 +478,7 @@ static void port92_write(void *opaque, hwaddr addr, uint64_t val,
     s->outport = val;
     qemu_set_irq(*s->a20_out, (val >> 1) & 1);
     if ((val & 1) && !(oldval & 1)) {
-        qemu_system_reset_request();
+        cpu_soft_reset();
     }
 }
 
diff --git a/hw/input/pckbd.c b/hw/input/pckbd.c
index 29af3d7..fd87776 100644
--- a/hw/input/pckbd.c
+++ b/hw/input/pckbd.c
@@ -26,6 +26,7 @@
 #include "hw/i386/pc.h"
 #include "hw/input/ps2.h"
 #include "sysemu/sysemu.h"
+#include "sysemu/cpus.h"
 
 /* debug PC keyboard */
 //#define DEBUG_KBD
@@ -220,7 +221,7 @@ static void outport_write(KBDState *s, uint32_t val)
         qemu_set_irq(*s->a20_out, (val >> 1) & 1);
     }
     if (!(val & 1)) {
-        qemu_system_reset_request();
+        cpu_soft_reset();
     }
 }
 
@@ -299,7 +300,7 @@ static void kbd_write_command(void *opaque, hwaddr addr,
         s->outport &= ~KBD_OUT_A20;
         break;
     case KBD_CCMD_RESET:
-        qemu_system_reset_request();
+        cpu_soft_reset();
         break;
     case KBD_CCMD_NO_OP:
         /* ignore that */
diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c
index 51ce12d..e9fde85 100644
--- a/hw/isa/lpc_ich9.c
+++ b/hw/isa/lpc_ich9.c
@@ -45,6 +45,7 @@
 #include "hw/pci/pci_bus.h"
 #include "exec/address-spaces.h"
 #include "sysemu/sysemu.h"
+#include "sysemu/cpus.h"
 
 static int ich9_lpc_sci_irq(ICH9LPCState *lpc);
 
@@ -507,8 +508,15 @@ static void ich9_rst_cnt_write(void *opaque, hwaddr addr, uint64_t val,
     ICH9LPCState *lpc = opaque;
 
     if (val & 4) {
-        qemu_system_reset_request();
-        return;
+        /* In a real ICH9, FULL_RST affects whether the hardware goes to S5
+         * for 3-5 seconds, but is not enough alone; you need to set SYS_RST
+         * too.
+         */
+        if (val & 2) {
+            qemu_system_reset_request();
+        } else {
+            cpu_soft_reset();
+        }
     }
     lpc->rst_cnt = val & 0xA; /* keep FULL_RST (bit 3) and SYS_RST (bit 1) */
 }
diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
index ffdc853..1e60172 100644
--- a/hw/pci-host/piix.c
+++ b/hw/pci-host/piix.c
@@ -34,6 +34,7 @@
 #include "sysemu/sysemu.h"
 #include "hw/i386/ioapic.h"
 #include "qapi/visitor.h"
+#include "sysemu/cpus.h"
 
 /*
  * I440FX chipset data sheet.
@@ -587,8 +588,11 @@ static void rcr_write(void *opaque, hwaddr addr, uint64_t val, unsigned len)
     PIIX3State *d = opaque;
 
     if (val & 4) {
-        qemu_system_reset_request();
-        return;
+        if (val & 2) {
+            qemu_system_reset_request();
+        } else {
+            cpu_soft_reset();
+        }
     }
     d->rcr = val & 2; /* keep System Reset type only */
 }
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v2 8/8] x86: correctly implement soft reset

[Michael S. Tsirkin]

On Fri, May 02, 2014 at 04:33:22PM +0200, Paolo Bonzini wrote:
> Do not do a hard reset for port 92h, keyboard controller, or cf9h soft reset.
> These only reset the CPU.
> 
> Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

I note that we allow setting reset type in the same write
with reset itself.
i440fx spec says
    2
    Reset CPU (RCPU). RCPU is used to initiate a hard or soft reset to the
    CPU. During hard reset, the
    PMC asserts CRESET# for 2 msec and PCIRST# for 1 msec. During soft
    reset, the PMC asserts INIT#.
    BISTE and SHRE must be set up prior to writing a 1 to this bit. Two
    operations are required to initiate a
    reset using this register. The first write operation programs BISTE and
    SHRE to the appropriate state
    while setting RCPU to 0. The second write operation keeps the BISTE and
    SHRE at their programmed
    state while setting RCPU to 1.

it does not actually say what happens if you try to change reset type
in the same access as the reset so I think we are ok
with this simple logic that you implemented.



> ---
>  hw/i386/pc.c       |  3 ++-
>  hw/input/pckbd.c   |  5 +++--
>  hw/isa/lpc_ich9.c  | 12 ++++++++++--
>  hw/pci-host/piix.c |  8 ++++++--
>  4 files changed, 21 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index a59e958..8716864 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -45,6 +45,7 @@
>  #include "kvm_i386.h"
>  #include "hw/xen/xen.h"
>  #include "sysemu/blockdev.h"
> +#include "sysemu/cpus.h"
>  #include "hw/block/block.h"
>  #include "ui/qemu-spice.h"
>  #include "exec/memory.h"
> @@ -477,7 +478,7 @@ static void port92_write(void *opaque, hwaddr addr, uint64_t val,
>      s->outport = val;
>      qemu_set_irq(*s->a20_out, (val >> 1) & 1);
>      if ((val & 1) && !(oldval & 1)) {
> -        qemu_system_reset_request();
> +        cpu_soft_reset();
>      }
>  }
>  
> diff --git a/hw/input/pckbd.c b/hw/input/pckbd.c
> index 29af3d7..fd87776 100644
> --- a/hw/input/pckbd.c
> +++ b/hw/input/pckbd.c
> @@ -26,6 +26,7 @@
>  #include "hw/i386/pc.h"
>  #include "hw/input/ps2.h"
>  #include "sysemu/sysemu.h"
> +#include "sysemu/cpus.h"
>  
>  /* debug PC keyboard */
>  //#define DEBUG_KBD
> @@ -220,7 +221,7 @@ static void outport_write(KBDState *s, uint32_t val)
>          qemu_set_irq(*s->a20_out, (val >> 1) & 1);
>      }
>      if (!(val & 1)) {
> -        qemu_system_reset_request();
> +        cpu_soft_reset();
>      }
>  }
>  
> @@ -299,7 +300,7 @@ static void kbd_write_command(void *opaque, hwaddr addr,
>          s->outport &= ~KBD_OUT_A20;
>          break;
>      case KBD_CCMD_RESET:
> -        qemu_system_reset_request();
> +        cpu_soft_reset();
>          break;
>      case KBD_CCMD_NO_OP:
>          /* ignore that */
> diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c
> index 51ce12d..e9fde85 100644
> --- a/hw/isa/lpc_ich9.c
> +++ b/hw/isa/lpc_ich9.c
> @@ -45,6 +45,7 @@
>  #include "hw/pci/pci_bus.h"
>  #include "exec/address-spaces.h"
>  #include "sysemu/sysemu.h"
> +#include "sysemu/cpus.h"
>  
>  static int ich9_lpc_sci_irq(ICH9LPCState *lpc);
>  
> @@ -507,8 +508,15 @@ static void ich9_rst_cnt_write(void *opaque, hwaddr addr, uint64_t val,
>      ICH9LPCState *lpc = opaque;
>  
>      if (val & 4) {
> -        qemu_system_reset_request();
> -        return;
> +        /* In a real ICH9, FULL_RST affects whether the hardware goes to S5
> +         * for 3-5 seconds, but is not enough alone; you need to set SYS_RST
> +         * too.
> +         */
> +        if (val & 2) {
> +            qemu_system_reset_request();
> +        } else {
> +            cpu_soft_reset();
> +        }
>      }
>      lpc->rst_cnt = val & 0xA; /* keep FULL_RST (bit 3) and SYS_RST (bit 1) */
>  }
> diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
> index ffdc853..1e60172 100644
> --- a/hw/pci-host/piix.c
> +++ b/hw/pci-host/piix.c
> @@ -34,6 +34,7 @@
>  #include "sysemu/sysemu.h"
>  #include "hw/i386/ioapic.h"
>  #include "qapi/visitor.h"
> +#include "sysemu/cpus.h"
>  
>  /*
>   * I440FX chipset data sheet.
> @@ -587,8 +588,11 @@ static void rcr_write(void *opaque, hwaddr addr, uint64_t val, unsigned len)
>      PIIX3State *d = opaque;
>  
>      if (val & 4) {
> -        qemu_system_reset_request();
> -        return;
> +        if (val & 2) {
> +            qemu_system_reset_request();
> +        } else {
> +            cpu_soft_reset();
> +        }
>      }
>      d->rcr = val & 2; /* keep System Reset type only */
>  }
> -- 
> 1.8.3.1
> 

Re: [Qemu-devel] [PATCH v2 8/8] x86: correctly implement soft reset

[Andreas Färber]

Am 02.05.2014 16:33, schrieb Paolo Bonzini:
> Do not do a hard reset for port 92h, keyboard controller, or cf9h soft reset.
> These only reset the CPU.
> 
> Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Apart from the question of whether cpu_soft_reset() is the right API for
this: Does this result in guest-visible changes that we need to suppress
for previous machine types?

Regards,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH v2 8/8] x86: correctly implement soft reset

[Paolo Bonzini]

Il 12/05/2014 09:53, Andreas Färber ha scritto:
>> > Do not do a hard reset for port 92h, keyboard controller, or cf9h soft reset.
>> > These only reset the CPU.
>> >
>> > Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>
>> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> Apart from the question of whether cpu_soft_reset() is the right API for
> this: Does this result in guest-visible changes that we need to suppress
> for previous machine types?

The changes can be considered bug fixes.  If anything did rely on INIT 
signals resetting nothing but the CPU (for example old 16-bit code that 
wanted to get out of protected mode), it would be broken without these 
changes.

Paolo

Re: [Qemu-devel] [PATCH v2 0/8] x86: correctly implement soft reset

[Michael S. Tsirkin]

On Fri, May 02, 2014 at 04:33:14PM +0200, Paolo Bonzini wrote:
> A repost of an old patch series, rebased and retested.  Patches 3 to 5
> are new, everything else already carries a Reviewed-by.
> 
> v1->v2: compile-tested on ARM (I also have had a /dev/kvm on my ARM board
>         for two hours now, but still no guest to try this on).

Went over all of the series, it all looks good to me.

Acked-by: Michael S. Tsirkin <mst@redhat.com>

mostly kvm stuff so I guess your tree makes more sense,
but if you want me to take it instead let me know,
won't be a problem.


> Paolo Bonzini (8):
>   kvm: reset state from the CPU's reset method
>   kvm: forward INIT signals coming from the chipset
>   target-i386: fix set of registers zeroed on reset
>   target-i386: preserve FPU and MSR state on INIT
>   apic: do not accept SIPI on the bootstrap processor
>   cpu: make CPU_INTERRUPT_RESET available on all targets
>   pc: port 92 reset requires a low->high transition
>   x86: correctly implement soft reset
> 
>  cpu-exec.c             | 23 ++++++++++---------
>  cpus.c                 |  9 ++++++++
>  hw/i386/pc.c           |  6 +++--
>  hw/input/pckbd.c       |  5 +++--
>  hw/intc/apic_common.c  |  2 +-
>  hw/isa/lpc_ich9.c      | 12 ++++++++--
>  hw/pci-host/piix.c     |  8 +++++--
>  include/exec/cpu-all.h |  8 ++++---
>  include/sysemu/cpus.h  |  1 +
>  include/sysemu/kvm.h   |  2 --
>  kvm-all.c              | 11 ---------
>  target-arm/cpu.c       |  7 ++++++
>  target-arm/kvm32.c     |  4 +---
>  target-arm/kvm64.c     |  2 +-
>  target-arm/kvm_arm.h   |  8 +++++++
>  target-i386/cpu.c      | 11 +++++----
>  target-i386/cpu.h      | 60 ++++++++++++++++++++++++++++++--------------------
>  target-i386/helper.c   | 14 ++++++++++--
>  target-i386/kvm.c      | 39 +++++++++++++++++++++-----------
>  target-i386/kvm_i386.h |  2 ++
>  target-ppc/cpu.h       |  3 ---
>  target-ppc/kvm.c       |  4 ----
>  target-s390x/cpu.c     |  4 ++++
>  target-s390x/cpu.h     |  5 +++++
>  target-s390x/kvm.c     |  6 +++--
>  25 files changed, 165 insertions(+), 91 deletions(-)
> 
> -- 
> 1.8.3.1
>