[Qemu-devel] [PATCH 1/3] Synchronize the linux-headers

[Qemu-devel] [PATCH 1/3] Synchronize the linux-headers

[Bharat Bhushan]

This also includes the changes for watchdog emulation

Signed-off-by: Bharat Bhushan <bharat.bhushan@freescale.com>
---
 linux-headers/asm-powerpc/kvm_para.h |    7 ++++---
 linux-headers/asm-s390/kvm.h         |    2 +-
 linux-headers/asm-s390/kvm_para.h    |    2 +-
 linux-headers/asm-x86/kvm.h          |    1 +
 linux-headers/asm-x86/kvm_para.h     |    7 +++++++
 linux-headers/linux/kvm.h            |    4 ++++
 6 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/linux-headers/asm-powerpc/kvm_para.h b/linux-headers/asm-powerpc/kvm_para.h
index c047a84..7e64f57 100644
--- a/linux-headers/asm-powerpc/kvm_para.h
+++ b/linux-headers/asm-powerpc/kvm_para.h
@@ -75,9 +75,10 @@ struct kvm_vcpu_arch_shared {
 };
 
 #define KVM_SC_MAGIC_R0		0x4b564d21 /* "KVM!" */
-#define HC_VENDOR_KVM		(42 << 16)
-#define HC_EV_SUCCESS		0
-#define HC_EV_UNIMPLEMENTED	12
+
+#define KVM_HCALL_TOKEN(num)     _EV_HCALL_TOKEN(EV_KVM_VENDOR_ID, num)
+
+#include <asm/epapr_hcalls.h>
 
 #define KVM_FEATURE_MAGIC_PAGE	1
 
diff --git a/linux-headers/asm-s390/kvm.h b/linux-headers/asm-s390/kvm.h
index bdcbe0f..d25da59 100644
--- a/linux-headers/asm-s390/kvm.h
+++ b/linux-headers/asm-s390/kvm.h
@@ -1,7 +1,7 @@
 #ifndef __LINUX_KVM_S390_H
 #define __LINUX_KVM_S390_H
 /*
- * asm-s390/kvm.h - KVM s390 specific structures and definitions
+ * KVM s390 specific structures and definitions
  *
  * Copyright IBM Corp. 2008
  *
diff --git a/linux-headers/asm-s390/kvm_para.h b/linux-headers/asm-s390/kvm_para.h
index 8e2dd67..870051f 100644
--- a/linux-headers/asm-s390/kvm_para.h
+++ b/linux-headers/asm-s390/kvm_para.h
@@ -1,5 +1,5 @@
 /*
- * asm-s390/kvm_para.h - definition for paravirtual devices on s390
+ * definition for paravirtual devices on s390
  *
  * Copyright IBM Corp. 2008
  *
diff --git a/linux-headers/asm-x86/kvm.h b/linux-headers/asm-x86/kvm.h
index e7d1c19..246617e 100644
--- a/linux-headers/asm-x86/kvm.h
+++ b/linux-headers/asm-x86/kvm.h
@@ -12,6 +12,7 @@
 /* Select x86 specific features in <linux/kvm.h> */
 #define __KVM_HAVE_PIT
 #define __KVM_HAVE_IOAPIC
+#define __KVM_HAVE_IRQ_LINE
 #define __KVM_HAVE_DEVICE_ASSIGNMENT
 #define __KVM_HAVE_MSI
 #define __KVM_HAVE_USER_NMI
diff --git a/linux-headers/asm-x86/kvm_para.h b/linux-headers/asm-x86/kvm_para.h
index f2ac46a..a1c3d72 100644
--- a/linux-headers/asm-x86/kvm_para.h
+++ b/linux-headers/asm-x86/kvm_para.h
@@ -22,6 +22,7 @@
 #define KVM_FEATURE_CLOCKSOURCE2        3
 #define KVM_FEATURE_ASYNC_PF		4
 #define KVM_FEATURE_STEAL_TIME		5
+#define KVM_FEATURE_PV_EOI		6
 
 /* The last 8 bits are used to indicate how to interpret the flags field
  * in pvclock structure. If no bits are set, all flags are ignored.
@@ -37,6 +38,7 @@
 #define MSR_KVM_SYSTEM_TIME_NEW 0x4b564d01
 #define MSR_KVM_ASYNC_PF_EN 0x4b564d02
 #define MSR_KVM_STEAL_TIME  0x4b564d03
+#define MSR_KVM_PV_EOI_EN      0x4b564d04
 
 struct kvm_steal_time {
 	__u64 steal;
@@ -89,5 +91,10 @@ struct kvm_vcpu_pv_apf_data {
 	__u32 enabled;
 };
 
+#define KVM_PV_EOI_BIT 0
+#define KVM_PV_EOI_MASK (0x1 << KVM_PV_EOI_BIT)
+#define KVM_PV_EOI_ENABLED KVM_PV_EOI_MASK
+#define KVM_PV_EOI_DISABLED 0x0
+
 
 #endif /* _ASM_X86_KVM_PARA_H */
diff --git a/linux-headers/linux/kvm.h b/linux-headers/linux/kvm.h
index 4b9e575..8fda35b 100644
--- a/linux-headers/linux/kvm.h
+++ b/linux-headers/linux/kvm.h
@@ -163,6 +163,7 @@ struct kvm_pit_config {
 #define KVM_EXIT_OSI              18
 #define KVM_EXIT_PAPR_HCALL	  19
 #define KVM_EXIT_S390_UCONTROL	  20
+#define KVM_EXIT_WATCHDOG         21
 
 /* For KVM_EXIT_INTERNAL_ERROR */
 #define KVM_INTERNAL_ERROR_EMULATION 1
@@ -473,6 +474,8 @@ struct kvm_ppc_smmu_info {
 	struct kvm_ppc_one_seg_page_size sps[KVM_PPC_PAGE_SIZES_MAX_SZ];
 };
 
+#define KVM_PPC_PVINFO_FLAGS_EV_IDLE   (1<<0)
+
 #define KVMIO 0xAE
 
 /* machine type bits, to be used as argument to KVM_CREATE_VM */
@@ -618,6 +621,7 @@ struct kvm_ppc_smmu_info {
 #define KVM_CAP_PPC_GET_SMMU_INFO 78
 #define KVM_CAP_S390_COW 79
 #define KVM_CAP_PPC_ALLOC_HTAB 80
+#define KVM_CAP_PPC_BOOKE_WATCHDOG 81
 
 #ifdef KVM_CAP_IRQ_ROUTING
 
-- 
1.7.0.4

[Qemu-devel] [PATCH 2/3] Registered timer reset handler

[Bharat Bhushan]

Reset qemu timers when guest reset.

Signed-off-by: Bharat Bhushan <bharat.bhushan@freescale.com>
---
 hw/ppc_booke.c |   12 ++++++++++++
 1 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/hw/ppc_booke.c b/hw/ppc_booke.c
index d51e7fa..837a5b6 100644
--- a/hw/ppc_booke.c
+++ b/hw/ppc_booke.c
@@ -231,6 +231,16 @@ void store_booke_tcr(CPUPPCState *env, target_ulong val)
 
 }
 
+static void ppc_booke_timer_reset_handle(void *opaque)
+{
+    CPUPPCState *env = opaque;
+
+    env->spr[SPR_BOOKE_TSR] = 0;
+    env->spr[SPR_BOOKE_TCR] = 0;
+
+    booke_update_irq(env);
+}
+
 void ppc_booke_timers_init(CPUPPCState *env, uint32_t freq, uint32_t flags)
 {
     ppc_tb_t *tb_env;
@@ -251,4 +261,6 @@ void ppc_booke_timers_init(CPUPPCState *env, uint32_t freq, uint32_t flags)
         qemu_new_timer_ns(vm_clock, &booke_fit_cb, env);
     booke_timer->wdt_timer =
         qemu_new_timer_ns(vm_clock, &booke_wdt_cb, env);
+
+    qemu_register_reset(ppc_booke_timer_reset_handle, env);
 }
-- 
1.7.0.4

[Qemu-devel] [PATCH 3/3] Enable kvm emulated watchdog

[Bharat Bhushan]

Enable the KVM emulated watchdog if KVM supports (use the
capability enablement in watchdog handler). Also watchdog exit
(KVM_EXIT_WATCHDOG) handling is added.
Watchdog state machine is cleared whenever VM state changes to running.
This is to handle the cases like return from debug halt etc.

Signed-off-by: Bharat Bhushan <bharat.bhushan@freescale.com>
---
v5:
 - Revert " Enbale watchdog support only when user specifies watchdog-action"
 - Moved the logic into hw/ppc_booke.c

v4: Enbale watchdog support only when user specifies watchdog-action
    Earlier this was [3/3] of the patch series. Now because i separated
    linux-header updation in separate patch, so this become [4/4]

v3:
 - TSR clearing is removed in whatchdog exit handling as this is
   no more needed.

v2:
 - Merged ([PATCH 3/4] Watchdog exit handling support) and
   ([PATCH 4/4] Enable to use kvm emulated watchdog)
 - Clear watchdog state machine when VM state changes to running.


 hw/ppc.h         |    2 +
 hw/ppc_booke.c   |   71 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 target-ppc/kvm.c |   12 ++++++++-
 3 files changed, 84 insertions(+), 1 deletions(-)

diff --git a/hw/ppc.h b/hw/ppc.h
index 2f3ea27..3672fe8 100644
--- a/hw/ppc.h
+++ b/hw/ppc.h
@@ -44,6 +44,8 @@ struct ppc_tb_t {
 
 uint64_t cpu_ppc_get_tb(ppc_tb_t *tb_env, uint64_t vmclk, int64_t tb_offset);
 clk_setup_cb cpu_ppc_tb_init (CPUPPCState *env, uint32_t freq);
+extern int cap_ppc_watchdog;
+extern int cap_booke_sregs;
 /* Embedded PowerPC DCR management */
 typedef uint32_t (*dcr_read_cb)(void *opaque, int dcrn);
 typedef void (*dcr_write_cb)(void *opaque, int dcrn, uint32_t val);
diff --git a/hw/ppc_booke.c b/hw/ppc_booke.c
index 837a5b6..f18df74 100644
--- a/hw/ppc_booke.c
+++ b/hw/ppc_booke.c
@@ -21,6 +21,8 @@
  * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
  * THE SOFTWARE.
  */
+#include "sysemu.h"
+#include "kvm.h"
 #include "hw.h"
 #include "ppc.h"
 #include "qemu-timer.h"
@@ -203,6 +205,11 @@ static void booke_wdt_cb(void *opaque)
                              booke_timer->wdt_timer);
 }
 
+static void ppc_booke_watchdog_clear_tsr(CPUPPCState *env, target_ulong tsr)
+{
+    env->spr[SPR_BOOKE_TSR] = tsr & ~(TSR_ENW | TSR_WIS | TSR_WRS_MASK);
+}
+
 void store_booke_tsr(CPUPPCState *env, target_ulong val)
 {
     env->spr[SPR_BOOKE_TSR] &= ~val;
@@ -241,6 +248,64 @@ static void ppc_booke_timer_reset_handle(void *opaque)
     booke_update_irq(env);
 }
 
+static void cpu_state_change_handler(void *opaque, int running, RunState state)
+{
+    CPUPPCState *env = opaque;
+
+    struct kvm_sregs sregs;
+
+    if (!running) {
+        return;
+    }
+
+    /*
+     * Clear watchdog interrupt condition by clearing TSR.
+     * Similar logic needed to be implemented for watchdog
+     * emulation in qemu.
+     */
+
+    if (!kvm_enabled()) {
+        /* FIXME: add handling for qemu emulated case */
+        return;
+    }
+
+    if (cap_booke_sregs && cap_ppc_watchdog) {
+        kvm_vcpu_ioctl(env, KVM_GET_SREGS, &sregs);
+
+        /* Clear TSR.ENW, TSR.WIS and TSR.WRS */
+        ppc_booke_watchdog_clear_tsr(env, sregs.u.e.tsr);
+        sregs.u.e.tsr = env->spr[SPR_BOOKE_TSR];
+        sregs.u.e.update_special = KVM_SREGS_E_UPDATE_TSR;
+
+        kvm_vcpu_ioctl(env, KVM_SET_SREGS, &sregs);
+    }
+}
+
+static int kvm_booke_watchdog_enable(CPUPPCState *env)
+{
+    int ret;
+    struct kvm_enable_cap encap = {};
+
+    if (!kvm_enabled()) {
+        return 0;
+    }
+
+    if (!cap_ppc_watchdog) {
+        printf("warning: KVM does not support watchdog");
+        return 0;
+    }
+
+    encap.cap = KVM_CAP_PPC_BOOKE_WATCHDOG;
+    ret = kvm_vcpu_ioctl(env, KVM_ENABLE_CAP, &encap);
+    if (ret < 0) {
+        fprintf(stderr, "%s: couldn't enable KVM_CAP_PPC_BOOKE_WATCHDOG: %s\n",
+                __func__, strerror(-ret));
+        return ret;
+    }
+
+    return ret;
+}
+
 void ppc_booke_timers_init(CPUPPCState *env, uint32_t freq, uint32_t flags)
 {
     ppc_tb_t *tb_env;
@@ -262,5 +327,11 @@ void ppc_booke_timers_init(CPUPPCState *env, uint32_t freq, uint32_t flags)
     booke_timer->wdt_timer =
         qemu_new_timer_ns(vm_clock, &booke_wdt_cb, env);
 
+    if (kvm_enabled()) {
+        kvm_booke_watchdog_enable(env);
+    }
+
+    qemu_add_vm_change_state_handler(cpu_state_change_handler, env);
+
     qemu_register_reset(ppc_booke_timer_reset_handle, env);
 }
diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
index 829e180..b1c23fd 100644
--- a/target-ppc/kvm.c
+++ b/target-ppc/kvm.c
@@ -32,6 +32,7 @@
 #include "device_tree.h"
 #include "hw/sysbus.h"
 #include "hw/spapr.h"
+#include "hw/watchdog.h"
 
 #include "hw/sysbus.h"
 #include "hw/spapr.h"
@@ -56,10 +57,11 @@ const KVMCapabilityInfo kvm_arch_required_capabilities[] = {
 static int cap_interrupt_unset = false;
 static int cap_interrupt_level = false;
 static int cap_segstate;
-static int cap_booke_sregs;
+int cap_booke_sregs;
 static int cap_ppc_smt;
 static int cap_ppc_rma;
 static int cap_spapr_tce;
+int cap_ppc_watchdog;
 
 /* XXX We have a race condition where we actually have a level triggered
  *     interrupt, but the infrastructure can't expose that yet, so the guest
@@ -86,6 +88,7 @@ int kvm_arch_init(KVMState *s)
     cap_ppc_smt = kvm_check_extension(s, KVM_CAP_PPC_SMT);
     cap_ppc_rma = kvm_check_extension(s, KVM_CAP_PPC_RMA);
     cap_spapr_tce = kvm_check_extension(s, KVM_CAP_SPAPR_TCE);
+    cap_ppc_watchdog = kvm_check_extension(s, KVM_CAP_PPC_BOOKE_WATCHDOG);
 
     if (!cap_interrupt_level) {
         fprintf(stderr, "KVM: Couldn't find level irq capability. Expect the "
@@ -769,6 +772,13 @@ int kvm_arch_handle_exit(CPUPPCState *env, struct kvm_run *run)
         ret = 1;
         break;
 #endif
+#ifdef KVM_EXIT_WATCHDOG
+    case KVM_EXIT_WATCHDOG:
+        dprintf("handle watchdog expiry\n");
+        watchdog_perform_action();
+        ret = 0;
+        break;
+#endif
     default:
         fprintf(stderr, "KVM: unknown exit reason %d\n", run->exit_reason);
         ret = -1;
-- 
1.7.0.4

[Qemu-devel] [PATCH 3/3] Enable kvm emulated watchdog

[Bharat Bhushan]

Enable the KVM emulated watchdog if KVM supports (use the
capability enablement in watchdog handler). Also watchdog exit
(KVM_EXIT_WATCHDOG) handling is added.
Watchdog state machine is cleared whenever VM state changes to running.
This is to handle the cases like return from debug halt etc.

Signed-off-by: Bharat Bhushan <bharat.bhushan@freescale.com>
---
 hw/ppc.h         |    2 +
 hw/ppc_booke.c   |   71 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 target-ppc/kvm.c |   13 +++++++++-
 3 files changed, 85 insertions(+), 1 deletions(-)

diff --git a/hw/ppc.h b/hw/ppc.h
index 2f3ea27..3672fe8 100644
--- a/hw/ppc.h
+++ b/hw/ppc.h
@@ -44,6 +44,8 @@ struct ppc_tb_t {
 
 uint64_t cpu_ppc_get_tb(ppc_tb_t *tb_env, uint64_t vmclk, int64_t tb_offset);
 clk_setup_cb cpu_ppc_tb_init (CPUPPCState *env, uint32_t freq);
+extern int cap_ppc_watchdog;
+extern int cap_booke_sregs;
 /* Embedded PowerPC DCR management */
 typedef uint32_t (*dcr_read_cb)(void *opaque, int dcrn);
 typedef void (*dcr_write_cb)(void *opaque, int dcrn, uint32_t val);
diff --git a/hw/ppc_booke.c b/hw/ppc_booke.c
index 837a5b6..f18df74 100644
--- a/hw/ppc_booke.c
+++ b/hw/ppc_booke.c
@@ -21,6 +21,8 @@
  * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
  * THE SOFTWARE.
  */
+#include "sysemu.h"
+#include "kvm.h"
 #include "hw.h"
 #include "ppc.h"
 #include "qemu-timer.h"
@@ -203,6 +205,11 @@ static void booke_wdt_cb(void *opaque)
                              booke_timer->wdt_timer);
 }
 
+static void ppc_booke_watchdog_clear_tsr(CPUPPCState *env, target_ulong tsr)
+{
+    env->spr[SPR_BOOKE_TSR] = tsr & ~(TSR_ENW | TSR_WIS | TSR_WRS_MASK);
+}
+
 void store_booke_tsr(CPUPPCState *env, target_ulong val)
 {
     env->spr[SPR_BOOKE_TSR] &= ~val;
@@ -241,6 +248,64 @@ static void ppc_booke_timer_reset_handle(void *opaque)
     booke_update_irq(env);
 }
 
+static void cpu_state_change_handler(void *opaque, int running, RunState state)
+{
+    CPUPPCState *env = opaque;
+
+    struct kvm_sregs sregs;
+
+    if (!running) {
+        return;
+    }
+
+    /*
+     * Clear watchdog interrupt condition by clearing TSR.
+     * Similar logic needed to be implemented for watchdog
+     * emulation in qemu.
+     */
+
+    if (!kvm_enabled()) {
+        /* FIXME: add handling for qemu emulated case */
+        return;
+    }
+
+    if (cap_booke_sregs && cap_ppc_watchdog) {
+        kvm_vcpu_ioctl(env, KVM_GET_SREGS, &sregs);
+
+        /* Clear TSR.ENW, TSR.WIS and TSR.WRS */
+        ppc_booke_watchdog_clear_tsr(env, sregs.u.e.tsr);
+        sregs.u.e.tsr = env->spr[SPR_BOOKE_TSR];
+        sregs.u.e.update_special = KVM_SREGS_E_UPDATE_TSR;
+
+        kvm_vcpu_ioctl(env, KVM_SET_SREGS, &sregs);
+    }
+}
+
+static int kvm_booke_watchdog_enable(CPUPPCState *env)
+{
+    int ret;
+    struct kvm_enable_cap encap = {};
+
+    if (!kvm_enabled()) {
+        return 0;
+    }
+
+    if (!cap_ppc_watchdog) {
+        printf("warning: KVM does not support watchdog");
+        return 0;
+    }
+
+    encap.cap = KVM_CAP_PPC_BOOKE_WATCHDOG;
+    ret = kvm_vcpu_ioctl(env, KVM_ENABLE_CAP, &encap);
+    if (ret < 0) {
+        fprintf(stderr, "%s: couldn't enable KVM_CAP_PPC_BOOKE_WATCHDOG: %s\n",
+                __func__, strerror(-ret));
+        return ret;
+    }
+
+    return ret;
+}
+
 void ppc_booke_timers_init(CPUPPCState *env, uint32_t freq, uint32_t flags)
 {
     ppc_tb_t *tb_env;
@@ -262,5 +327,11 @@ void ppc_booke_timers_init(CPUPPCState *env, uint32_t freq, uint32_t flags)
     booke_timer->wdt_timer =
         qemu_new_timer_ns(vm_clock, &booke_wdt_cb, env);
 
+    if (kvm_enabled()) {
+        kvm_booke_watchdog_enable(env);
+    }
+
+    qemu_add_vm_change_state_handler(cpu_state_change_handler, env);
+
     qemu_register_reset(ppc_booke_timer_reset_handle, env);
 }
diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
index 3f5df57..3d5f86a 100644
--- a/target-ppc/kvm.c
+++ b/target-ppc/kvm.c
@@ -32,6 +32,7 @@
 #include "device_tree.h"
 #include "hw/sysbus.h"
 #include "hw/spapr.h"
+#include "hw/watchdog.h"
 
 #include "hw/sysbus.h"
 #include "hw/spapr.h"
@@ -56,11 +57,12 @@ const KVMCapabilityInfo kvm_arch_required_capabilities[] = {
 static int cap_interrupt_unset = false;
 static int cap_interrupt_level = false;
 static int cap_segstate;
-static int cap_booke_sregs;
+int cap_booke_sregs;
 static int cap_ppc_smt;
 static int cap_ppc_rma;
 static int cap_spapr_tce;
 static int cap_hior;
+int cap_ppc_watchdog;
 
 /* XXX We have a race condition where we actually have a level triggered
  *     interrupt, but the infrastructure can't expose that yet, so the guest
@@ -90,6 +92,7 @@ int kvm_arch_init(KVMState *s)
     cap_ppc_rma = kvm_check_extension(s, KVM_CAP_PPC_RMA);
     cap_spapr_tce = kvm_check_extension(s, KVM_CAP_SPAPR_TCE);
     cap_hior = kvm_check_extension(s, KVM_CAP_PPC_HIOR);
+    cap_ppc_watchdog = kvm_check_extension(s, KVM_CAP_PPC_BOOKE_WATCHDOG);
 
     if (!cap_interrupt_level) {
         fprintf(stderr, "KVM: Couldn't find level irq capability. Expect the "
@@ -823,6 +826,14 @@ int kvm_arch_handle_exit(CPUPPCState *env, struct kvm_run *run)
         ret = 0;
         break;
 #endif
+#ifdef KVM_EXIT_WATCHDOG
+    case KVM_EXIT_WATCHDOG:
+        dprintf("handle watchdog expiry\n");
+        watchdog_perform_action();
+        ret = 0;
+        break;
+#endif
+
     default:
         fprintf(stderr, "KVM: unknown exit reason %d\n", run->exit_reason);
         ret = -1;
-- 
1.7.0.4

Re: [Qemu-devel] [PATCH 3/3] Enable kvm emulated watchdog

[Alexander Graf]

On 17.12.2012, at 07:08, Bharat Bhushan wrote:

> Enable the KVM emulated watchdog if KVM supports (use the
> capability enablement in watchdog handler). Also watchdog exit
> (KVM_EXIT_WATCHDOG) handling is added.
> Watchdog state machine is cleared whenever VM state changes to running.
> This is to handle the cases like return from debug halt etc.
> 
> Signed-off-by: Bharat Bhushan <bharat.bhushan@freescale.com>
> ---
> hw/ppc.h         |    2 +
> hw/ppc_booke.c   |   71 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> target-ppc/kvm.c |   13 +++++++++-
> 3 files changed, 85 insertions(+), 1 deletions(-)
> 
> diff --git a/hw/ppc.h b/hw/ppc.h
> index 2f3ea27..3672fe8 100644
> --- a/hw/ppc.h
> +++ b/hw/ppc.h
> @@ -44,6 +44,8 @@ struct ppc_tb_t {
> 
> uint64_t cpu_ppc_get_tb(ppc_tb_t *tb_env, uint64_t vmclk, int64_t tb_offset);
> clk_setup_cb cpu_ppc_tb_init (CPUPPCState *env, uint32_t freq);
> +extern int cap_ppc_watchdog;
> +extern int cap_booke_sregs;

No. Never export cap_ variables. They are kvm internal.

> /* Embedded PowerPC DCR management */
> typedef uint32_t (*dcr_read_cb)(void *opaque, int dcrn);
> typedef void (*dcr_write_cb)(void *opaque, int dcrn, uint32_t val);
> diff --git a/hw/ppc_booke.c b/hw/ppc_booke.c
> index 837a5b6..f18df74 100644
> --- a/hw/ppc_booke.c
> +++ b/hw/ppc_booke.c
> @@ -21,6 +21,8 @@
>  * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
>  * THE SOFTWARE.
>  */
> +#include "sysemu.h"
> +#include "kvm.h"
> #include "hw.h"
> #include "ppc.h"
> #include "qemu-timer.h"
> @@ -203,6 +205,11 @@ static void booke_wdt_cb(void *opaque)
>                              booke_timer->wdt_timer);
> }
> 
> +static void ppc_booke_watchdog_clear_tsr(CPUPPCState *env, target_ulong tsr)
> +{
> +    env->spr[SPR_BOOKE_TSR] = tsr & ~(TSR_ENW | TSR_WIS | TSR_WRS_MASK);
> +}
> +
> void store_booke_tsr(CPUPPCState *env, target_ulong val)
> {
>     env->spr[SPR_BOOKE_TSR] &= ~val;
> @@ -241,6 +248,64 @@ static void ppc_booke_timer_reset_handle(void *opaque)
>     booke_update_irq(env);
> }
> 
> +static void cpu_state_change_handler(void *opaque, int running, RunState state)
> +{
> +    CPUPPCState *env = opaque;
> +
> +    struct kvm_sregs sregs;
> +
> +    if (!running) {
> +        return;
> +    }
> +
> +    /*
> +     * Clear watchdog interrupt condition by clearing TSR.
> +     * Similar logic needed to be implemented for watchdog
> +     * emulation in qemu.
> +     */
> +
> +    if (!kvm_enabled()) {
> +        /* FIXME: add handling for qemu emulated case */
> +        return;
> +    }
> +
> +    if (cap_booke_sregs && cap_ppc_watchdog) {
> +        kvm_vcpu_ioctl(env, KVM_GET_SREGS, &sregs);
> +
> +        /* Clear TSR.ENW, TSR.WIS and TSR.WRS */
> +        ppc_booke_watchdog_clear_tsr(env, sregs.u.e.tsr);

This should happen outside of the if (kvm_enabled()) block.

> +        sregs.u.e.tsr = env->spr[SPR_BOOKE_TSR];
> +        sregs.u.e.update_special = KVM_SREGS_E_UPDATE_TSR;
> +
> +        kvm_vcpu_ioctl(env, KVM_SET_SREGS, &sregs);

Please create a kvmppc_... wrapper for all this in target-ppc/kvm.c. Or maybe even better yet add a helper variable that tells the kvm register sync function to sync TSR as well and just use the normal cpu_synchronize_state() way of pushing register into the CPU.

> +    }
> +}
> +
> +static int kvm_booke_watchdog_enable(CPUPPCState *env)
> +{
> +    int ret;
> +    struct kvm_enable_cap encap = {};
> +
> +    if (!kvm_enabled()) {
> +        return 0;

Why return 0?

> +    }
> +
> +    if (!cap_ppc_watchdog) {
> +        printf("warning: KVM does not support watchdog");
> +        return 0;

Why return 0?

> +    }
> +
> +    encap.cap = KVM_CAP_PPC_BOOKE_WATCHDOG;
> +    ret = kvm_vcpu_ioctl(env, KVM_ENABLE_CAP, &encap);
> +    if (ret < 0) {
> +        fprintf(stderr, "%s: couldn't enable KVM_CAP_PPC_BOOKE_WATCHDOG: %s\n",
> +                __func__, strerror(-ret));
> +        return ret;
> +    }

Please wrap this in a kvmppc_... function in kvm.c.

> +
> +    return ret;
> +}
> +
> void ppc_booke_timers_init(CPUPPCState *env, uint32_t freq, uint32_t flags)
> {
>     ppc_tb_t *tb_env;
> @@ -262,5 +327,11 @@ void ppc_booke_timers_init(CPUPPCState *env, uint32_t freq, uint32_t flags)
>     booke_timer->wdt_timer =
>         qemu_new_timer_ns(vm_clock, &booke_wdt_cb, env);
> 
> +    if (kvm_enabled()) {

Why double-check kvm_enabled()? And what happens on failure? Shouldn't failure be crucial if a watchdog action is set?

> +        kvm_booke_watchdog_enable(env);
> +    }
> +
> +    qemu_add_vm_change_state_handler(cpu_state_change_handler, env);
> +
>     qemu_register_reset(ppc_booke_timer_reset_handle, env);
> }
> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
> index 3f5df57..3d5f86a 100644
> --- a/target-ppc/kvm.c
> +++ b/target-ppc/kvm.c
> @@ -32,6 +32,7 @@
> #include "device_tree.h"
> #include "hw/sysbus.h"
> #include "hw/spapr.h"
> +#include "hw/watchdog.h"
> 
> #include "hw/sysbus.h"
> #include "hw/spapr.h"
> @@ -56,11 +57,12 @@ const KVMCapabilityInfo kvm_arch_required_capabilities[] = {
> static int cap_interrupt_unset = false;
> static int cap_interrupt_level = false;
> static int cap_segstate;
> -static int cap_booke_sregs;
> +int cap_booke_sregs;

No.

> static int cap_ppc_smt;
> static int cap_ppc_rma;
> static int cap_spapr_tce;
> static int cap_hior;
> +int cap_ppc_watchdog;

static

> 
> /* XXX We have a race condition where we actually have a level triggered
>  *     interrupt, but the infrastructure can't expose that yet, so the guest
> @@ -90,6 +92,7 @@ int kvm_arch_init(KVMState *s)
>     cap_ppc_rma = kvm_check_extension(s, KVM_CAP_PPC_RMA);
>     cap_spapr_tce = kvm_check_extension(s, KVM_CAP_SPAPR_TCE);
>     cap_hior = kvm_check_extension(s, KVM_CAP_PPC_HIOR);
> +    cap_ppc_watchdog = kvm_check_extension(s, KVM_CAP_PPC_BOOKE_WATCHDOG);
> 
>     if (!cap_interrupt_level) {
>         fprintf(stderr, "KVM: Couldn't find level irq capability. Expect the "
> @@ -823,6 +826,14 @@ int kvm_arch_handle_exit(CPUPPCState *env, struct kvm_run *run)
>         ret = 0;
>         break;
> #endif
> +#ifdef KVM_EXIT_WATCHDOG

Why an #ifdef here?


Alex

> +    case KVM_EXIT_WATCHDOG:
> +        dprintf("handle watchdog expiry\n");
> +        watchdog_perform_action();
> +        ret = 0;
> +        break;
> +#endif
> +
>     default:
>         fprintf(stderr, "KVM: unknown exit reason %d\n", run->exit_reason);
>         ret = -1;
> -- 
> 1.7.0.4
> 
> 

Re: [Qemu-devel] [PATCH 3/3] Enable kvm emulated watchdog

[Bhushan Bharat-R65777]

> -----Original Message-----
> From: Alexander Graf [mailto:agraf@suse.de]
> Sent: Monday, December 17, 2012 8:09 PM
> To: Bhushan Bharat-R65777
> Cc: qemu-devel@nongnu.org; qemu-ppc@nongnu.org; Bhushan Bharat-R65777
> Subject: Re: [PATCH 3/3] Enable kvm emulated watchdog
> 
> 
> On 17.12.2012, at 07:08, Bharat Bhushan wrote:
> 
> > Enable the KVM emulated watchdog if KVM supports (use the capability
> > enablement in watchdog handler). Also watchdog exit
> > (KVM_EXIT_WATCHDOG) handling is added.
> > Watchdog state machine is cleared whenever VM state changes to running.
> > This is to handle the cases like return from debug halt etc.
> >
> > Signed-off-by: Bharat Bhushan <bharat.bhushan@freescale.com>
> > ---
> > hw/ppc.h         |    2 +
> > hw/ppc_booke.c   |   71 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > target-ppc/kvm.c |   13 +++++++++-
> > 3 files changed, 85 insertions(+), 1 deletions(-)
> >
> > diff --git a/hw/ppc.h b/hw/ppc.h
> > index 2f3ea27..3672fe8 100644
> > --- a/hw/ppc.h
> > +++ b/hw/ppc.h
> > @@ -44,6 +44,8 @@ struct ppc_tb_t {
> >
> > uint64_t cpu_ppc_get_tb(ppc_tb_t *tb_env, uint64_t vmclk, int64_t
> > tb_offset); clk_setup_cb cpu_ppc_tb_init (CPUPPCState *env, uint32_t
> > freq);
> > +extern int cap_ppc_watchdog;
> > +extern int cap_booke_sregs;
> 
> No. Never export cap_ variables. They are kvm internal.
> 
> > /* Embedded PowerPC DCR management */
> > typedef uint32_t (*dcr_read_cb)(void *opaque, int dcrn); typedef void
> > (*dcr_write_cb)(void *opaque, int dcrn, uint32_t val); diff --git
> > a/hw/ppc_booke.c b/hw/ppc_booke.c index 837a5b6..f18df74 100644
> > --- a/hw/ppc_booke.c
> > +++ b/hw/ppc_booke.c
> > @@ -21,6 +21,8 @@
> >  * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> > DEALINGS IN
> >  * THE SOFTWARE.
> >  */
> > +#include "sysemu.h"
> > +#include "kvm.h"
> > #include "hw.h"
> > #include "ppc.h"
> > #include "qemu-timer.h"
> > @@ -203,6 +205,11 @@ static void booke_wdt_cb(void *opaque)
> >                              booke_timer->wdt_timer); }
> >
> > +static void ppc_booke_watchdog_clear_tsr(CPUPPCState *env,
> > +target_ulong tsr) {
> > +    env->spr[SPR_BOOKE_TSR] = tsr & ~(TSR_ENW | TSR_WIS |
> > +TSR_WRS_MASK); }
> > +
> > void store_booke_tsr(CPUPPCState *env, target_ulong val) {
> >     env->spr[SPR_BOOKE_TSR] &= ~val;
> > @@ -241,6 +248,64 @@ static void ppc_booke_timer_reset_handle(void *opaque)
> >     booke_update_irq(env);
> > }
> >
> > +static void cpu_state_change_handler(void *opaque, int running,
> > +RunState state) {
> > +    CPUPPCState *env = opaque;
> > +
> > +    struct kvm_sregs sregs;
> > +
> > +    if (!running) {
> > +        return;
> > +    }
> > +
> > +    /*
> > +     * Clear watchdog interrupt condition by clearing TSR.
> > +     * Similar logic needed to be implemented for watchdog
> > +     * emulation in qemu.
> > +     */
> > +
> > +    if (!kvm_enabled()) {
> > +        /* FIXME: add handling for qemu emulated case */
> > +        return;
> > +    }
> > +
> > +    if (cap_booke_sregs && cap_ppc_watchdog) {
> > +        kvm_vcpu_ioctl(env, KVM_GET_SREGS, &sregs);
> > +
> > +        /* Clear TSR.ENW, TSR.WIS and TSR.WRS */
> > +        ppc_booke_watchdog_clear_tsr(env, sregs.u.e.tsr);
> 
> This should happen outside of the if (kvm_enabled()) block.
> 
> > +        sregs.u.e.tsr = env->spr[SPR_BOOKE_TSR];
> > +        sregs.u.e.update_special = KVM_SREGS_E_UPDATE_TSR;
> > +
> > +        kvm_vcpu_ioctl(env, KVM_SET_SREGS, &sregs);
> 
> Please create a kvmppc_... wrapper for all this in target-ppc/kvm.c. Or maybe
> even better yet add a helper variable that tells the kvm register sync function
> to sync TSR as well and just use the normal cpu_synchronize_state() way of
> pushing register into the CPU.

Not sure what type of helper variable you are talking about.
What came in my mine is we define a helper variable as per bitmap of SREGS update feature KVM_SREGS_E_UPDATE_* (update_special) in env. Whenever any code changes the env[spr] it will set the update_special. Env->update_special will be checked in put_registers().

Thanks
-Bharat

> 
> > +    }
> > +}
> > +
> > +static int kvm_booke_watchdog_enable(CPUPPCState *env) {
> > +    int ret;
> > +    struct kvm_enable_cap encap = {};
> > +
> > +    if (!kvm_enabled()) {
> > +        return 0;
> 
> Why return 0?
> 
> > +    }
> > +
> > +    if (!cap_ppc_watchdog) {
> > +        printf("warning: KVM does not support watchdog");
> > +        return 0;
> 
> Why return 0?
> 
> > +    }
> > +
> > +    encap.cap = KVM_CAP_PPC_BOOKE_WATCHDOG;
> > +    ret = kvm_vcpu_ioctl(env, KVM_ENABLE_CAP, &encap);
> > +    if (ret < 0) {
> > +        fprintf(stderr, "%s: couldn't enable KVM_CAP_PPC_BOOKE_WATCHDOG:
> %s\n",
> > +                __func__, strerror(-ret));
> > +        return ret;
> > +    }
> 
> Please wrap this in a kvmppc_... function in kvm.c.
> 
> > +
> > +    return ret;
> > +}
> > +
> > void ppc_booke_timers_init(CPUPPCState *env, uint32_t freq, uint32_t
> > flags) {
> >     ppc_tb_t *tb_env;
> > @@ -262,5 +327,11 @@ void ppc_booke_timers_init(CPUPPCState *env, uint32_t
> freq, uint32_t flags)
> >     booke_timer->wdt_timer =
> >         qemu_new_timer_ns(vm_clock, &booke_wdt_cb, env);
> >
> > +    if (kvm_enabled()) {
> 
> Why double-check kvm_enabled()? And what happens on failure? Shouldn't failure
> be crucial if a watchdog action is set?
> 
> > +        kvm_booke_watchdog_enable(env);
> > +    }
> > +
> > +    qemu_add_vm_change_state_handler(cpu_state_change_handler, env);
> > +
> >     qemu_register_reset(ppc_booke_timer_reset_handle, env); } diff
> > --git a/target-ppc/kvm.c b/target-ppc/kvm.c index 3f5df57..3d5f86a
> > 100644
> > --- a/target-ppc/kvm.c
> > +++ b/target-ppc/kvm.c
> > @@ -32,6 +32,7 @@
> > #include "device_tree.h"
> > #include "hw/sysbus.h"
> > #include "hw/spapr.h"
> > +#include "hw/watchdog.h"
> >
> > #include "hw/sysbus.h"
> > #include "hw/spapr.h"
> > @@ -56,11 +57,12 @@ const KVMCapabilityInfo
> > kvm_arch_required_capabilities[] = { static int cap_interrupt_unset =
> > false; static int cap_interrupt_level = false; static int
> > cap_segstate; -static int cap_booke_sregs;
> > +int cap_booke_sregs;
> 
> No.
> 
> > static int cap_ppc_smt;
> > static int cap_ppc_rma;
> > static int cap_spapr_tce;
> > static int cap_hior;
> > +int cap_ppc_watchdog;
> 
> static
> 
> >
> > /* XXX We have a race condition where we actually have a level triggered
> >  *     interrupt, but the infrastructure can't expose that yet, so the guest
> > @@ -90,6 +92,7 @@ int kvm_arch_init(KVMState *s)
> >     cap_ppc_rma = kvm_check_extension(s, KVM_CAP_PPC_RMA);
> >     cap_spapr_tce = kvm_check_extension(s, KVM_CAP_SPAPR_TCE);
> >     cap_hior = kvm_check_extension(s, KVM_CAP_PPC_HIOR);
> > +    cap_ppc_watchdog = kvm_check_extension(s,
> > + KVM_CAP_PPC_BOOKE_WATCHDOG);
> >
> >     if (!cap_interrupt_level) {
> >         fprintf(stderr, "KVM: Couldn't find level irq capability. Expect the "
> > @@ -823,6 +826,14 @@ int kvm_arch_handle_exit(CPUPPCState *env, struct kvm_run
> *run)
> >         ret = 0;
> >         break;
> > #endif
> > +#ifdef KVM_EXIT_WATCHDOG
> 
> Why an #ifdef here?
> 
> 
> Alex
> 
> > +    case KVM_EXIT_WATCHDOG:
> > +        dprintf("handle watchdog expiry\n");
> > +        watchdog_perform_action();
> > +        ret = 0;
> > +        break;
> > +#endif
> > +
> >     default:
> >         fprintf(stderr, "KVM: unknown exit reason %d\n", run->exit_reason);
> >         ret = -1;
> > --
> > 1.7.0.4
> >
> >
> 

Re: [Qemu-devel] [PATCH 3/3] Enable kvm emulated watchdog

[Alexander Graf]

On 27.12.2012, at 12:38, Bhushan Bharat-R65777 wrote:

> 
> 
>> -----Original Message-----
>> From: Alexander Graf [mailto:agraf@suse.de]
>> Sent: Monday, December 17, 2012 8:09 PM
>> To: Bhushan Bharat-R65777
>> Cc: qemu-devel@nongnu.org; qemu-ppc@nongnu.org; Bhushan Bharat-R65777
>> Subject: Re: [PATCH 3/3] Enable kvm emulated watchdog
>> 
>> 
>> On 17.12.2012, at 07:08, Bharat Bhushan wrote:
>> 
>>> Enable the KVM emulated watchdog if KVM supports (use the capability
>>> enablement in watchdog handler). Also watchdog exit
>>> (KVM_EXIT_WATCHDOG) handling is added.
>>> Watchdog state machine is cleared whenever VM state changes to running.
>>> This is to handle the cases like return from debug halt etc.
>>> 
>>> Signed-off-by: Bharat Bhushan <bharat.bhushan@freescale.com>
>>> ---
>>> hw/ppc.h         |    2 +
>>> hw/ppc_booke.c   |   71 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>> target-ppc/kvm.c |   13 +++++++++-
>>> 3 files changed, 85 insertions(+), 1 deletions(-)
>>> 
>>> diff --git a/hw/ppc.h b/hw/ppc.h
>>> index 2f3ea27..3672fe8 100644
>>> --- a/hw/ppc.h
>>> +++ b/hw/ppc.h
>>> @@ -44,6 +44,8 @@ struct ppc_tb_t {
>>> 
>>> uint64_t cpu_ppc_get_tb(ppc_tb_t *tb_env, uint64_t vmclk, int64_t
>>> tb_offset); clk_setup_cb cpu_ppc_tb_init (CPUPPCState *env, uint32_t
>>> freq);
>>> +extern int cap_ppc_watchdog;
>>> +extern int cap_booke_sregs;
>> 
>> No. Never export cap_ variables. They are kvm internal.
>> 
>>> /* Embedded PowerPC DCR management */
>>> typedef uint32_t (*dcr_read_cb)(void *opaque, int dcrn); typedef void
>>> (*dcr_write_cb)(void *opaque, int dcrn, uint32_t val); diff --git
>>> a/hw/ppc_booke.c b/hw/ppc_booke.c index 837a5b6..f18df74 100644
>>> --- a/hw/ppc_booke.c
>>> +++ b/hw/ppc_booke.c
>>> @@ -21,6 +21,8 @@
>>> * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
>>> DEALINGS IN
>>> * THE SOFTWARE.
>>> */
>>> +#include "sysemu.h"
>>> +#include "kvm.h"
>>> #include "hw.h"
>>> #include "ppc.h"
>>> #include "qemu-timer.h"
>>> @@ -203,6 +205,11 @@ static void booke_wdt_cb(void *opaque)
>>>                             booke_timer->wdt_timer); }
>>> 
>>> +static void ppc_booke_watchdog_clear_tsr(CPUPPCState *env,
>>> +target_ulong tsr) {
>>> +    env->spr[SPR_BOOKE_TSR] = tsr & ~(TSR_ENW | TSR_WIS |
>>> +TSR_WRS_MASK); }
>>> +
>>> void store_booke_tsr(CPUPPCState *env, target_ulong val) {
>>>    env->spr[SPR_BOOKE_TSR] &= ~val;
>>> @@ -241,6 +248,64 @@ static void ppc_booke_timer_reset_handle(void *opaque)
>>>    booke_update_irq(env);
>>> }
>>> 
>>> +static void cpu_state_change_handler(void *opaque, int running,
>>> +RunState state) {
>>> +    CPUPPCState *env = opaque;
>>> +
>>> +    struct kvm_sregs sregs;
>>> +
>>> +    if (!running) {
>>> +        return;
>>> +    }
>>> +
>>> +    /*
>>> +     * Clear watchdog interrupt condition by clearing TSR.
>>> +     * Similar logic needed to be implemented for watchdog
>>> +     * emulation in qemu.
>>> +     */
>>> +
>>> +    if (!kvm_enabled()) {
>>> +        /* FIXME: add handling for qemu emulated case */
>>> +        return;
>>> +    }
>>> +
>>> +    if (cap_booke_sregs && cap_ppc_watchdog) {
>>> +        kvm_vcpu_ioctl(env, KVM_GET_SREGS, &sregs);
>>> +
>>> +        /* Clear TSR.ENW, TSR.WIS and TSR.WRS */
>>> +        ppc_booke_watchdog_clear_tsr(env, sregs.u.e.tsr);
>> 
>> This should happen outside of the if (kvm_enabled()) block.
>> 
>>> +        sregs.u.e.tsr = env->spr[SPR_BOOKE_TSR];
>>> +        sregs.u.e.update_special = KVM_SREGS_E_UPDATE_TSR;
>>> +
>>> +        kvm_vcpu_ioctl(env, KVM_SET_SREGS, &sregs);
>> 
>> Please create a kvmppc_... wrapper for all this in target-ppc/kvm.c. Or maybe
>> even better yet add a helper variable that tells the kvm register sync function
>> to sync TSR as well and just use the normal cpu_synchronize_state() way of
>> pushing register into the CPU.
> 
> Not sure what type of helper variable you are talking about.
> What came in my mine is we define a helper variable as per bitmap of SREGS update feature KVM_SREGS_E_UPDATE_* (update_special) in env. Whenever any code changes the env[spr] it will set the update_special. Env->update_special will be checked in put_registers().

Yes, just that the bitmap shouldn't use KVM_SREGS bits but its own bit ids :). That way we can also support ONE_REG variables.


Alex