| Message ID | 1356671812-7634-4-git-send-email-bharat.bhushan@freescale.com |
|---|---|
| State | New |
| Headers | show |
On 28.12.2012, at 06:16, 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> > --- > v2: > - access cap_* from target_ppc/kvm.c only. > - Added wrapper functions in target_ppc/kvm.c for > enable_watchdog and tsr_sregs synchronization. > - Incorporated other Review comments > > hw/ppc.h | 1 + > hw/ppc_booke.c | 36 +++++++++++++++++++++++++++++++- > target-ppc/kvm.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++ > target-ppc/kvm_ppc.h | 11 +++++++++ > 4 files changed, 103 insertions(+), 1 deletions(-) > > diff --git a/hw/ppc.h b/hw/ppc.h > index 2f3ea27..6ad9e1f 100644 > --- a/hw/ppc.h > +++ b/hw/ppc.h > @@ -90,3 +90,4 @@ enum { > > /* ppc_booke.c */ > void ppc_booke_timers_init(CPUPPCState *env, uint32_t freq, uint32_t flags); > +void ppc_booke_watchdog_clear_tsr(CPUPPCState *env); > diff --git a/hw/ppc_booke.c b/hw/ppc_booke.c > index 837a5b6..7273259 100644 > --- a/hw/ppc_booke.c > +++ b/hw/ppc_booke.c > @@ -28,7 +28,7 @@ > #include "nvram.h" > #include "qemu-log.h" > #include "loader.h" > - > +#include "kvm_ppc.h" > > /* Timer Control Register */ > > @@ -203,6 +203,11 @@ static void booke_wdt_cb(void *opaque) > booke_timer->wdt_timer); > } > > +void ppc_booke_watchdog_clear_tsr(CPUPPCState *env) > +{ > + env->spr[SPR_BOOKE_TSR] &= ~(TSR_ENW | TSR_WIS | TSR_WRS_MASK); > +} > + > void store_booke_tsr(CPUPPCState *env, target_ulong val) > { > env->spr[SPR_BOOKE_TSR] &= ~val; > @@ -241,10 +246,27 @@ 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) > +{ Needs a comment when this happens > + CPUPPCState *env = opaque; > + > + if (!running) { > + return; > + } > + > + /* > + * Clear watchdog interrupt condition by clearing TSR. > + */ > + ppc_booke_watchdog_clear_tsr(env); > + > + kvmppc_synch_sregs_tsr(env); kvmppc_sync_tsr. Also please add the sync to store_booke_tsr(). Then here, you can just do store_booke_tsr(TSR_ENW | TSR_WIS | TSR_WRS_MASK); > +} > + > void ppc_booke_timers_init(CPUPPCState *env, uint32_t freq, uint32_t flags) > { > ppc_tb_t *tb_env; > booke_timer_t *booke_timer; > + int ret = 0; > > tb_env = g_malloc0(sizeof(ppc_tb_t)); > booke_timer = g_malloc0(sizeof(booke_timer_t)); > @@ -262,5 +284,17 @@ 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); > > + ret = kvmppc_booke_watchdog_enable(env); > + > + if (ret) { > + /* TODO: Start the QEMU emulated watchdog if not running on KVM. > + * Also start the QEMU emulated watchdog if KVM does not support > + * emulated watchdog or somehow it is not enabled (supported but > + * not enabled is though some bug and requires debugging :)). > + */ > + } > + > + 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..6828afa 100644 > --- a/target-ppc/kvm.c > +++ b/target-ppc/kvm.c > @@ -32,10 +32,12 @@ > #include "device_tree.h" > #include "hw/sysbus.h" > #include "hw/spapr.h" > +#include "hw/watchdog.h" > > #include "hw/sysbus.h" > #include "hw/spapr.h" > #include "hw/spapr_vio.h" > +#include "hw/ppc.h" > > //#define DEBUG_KVM > > @@ -61,6 +63,7 @@ static int cap_ppc_smt; > static int cap_ppc_rma; > static int cap_spapr_tce; > static int cap_hior; > +static 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 +93,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 +827,12 @@ int kvm_arch_handle_exit(CPUPPCState *env, struct kvm_run *run) > ret = 0; > break; > #endif > + case KVM_EXIT_WATCHDOG: > + dprintf("handle watchdog expiry\n"); > + watchdog_perform_action(); > + ret = 0; > + break; > + > default: > fprintf(stderr, "KVM: unknown exit reason %d\n", run->exit_reason); > ret = -1; > @@ -1239,3 +1249,49 @@ int kvm_arch_on_sigbus(int code, void *addr) > { > return 1; > } > + > +int kvmppc_synch_sregs_tsr(CPUPPCState *env) > +{ > + struct kvm_sregs sregs; > + int ret; > + > + if (!kvm_enabled() || !cap_booke_sregs || !cap_ppc_watchdog) { > + return -1; > + } > + > + ret = kvm_vcpu_ioctl(env, KVM_GET_SREGS, &sregs); > + > + env->spr[SPR_BOOKE_TSR] = sregs.u.e.tsr; > + ppc_booke_watchdog_clear_tsr(env); You're getting _and_ setting it in the same function? This is nonsense. Please split it into a get and a set function if you need it. > + sregs.u.e.tsr = env->spr[SPR_BOOKE_TSR]; > + sregs.u.e.update_special = KVM_SREGS_E_UPDATE_TSR; > + > + ret = kvm_vcpu_ioctl(env, KVM_SET_SREGS, &sregs); > + > + return ret; > +} > + > +int kvmppc_booke_watchdog_enable(CPUPPCState *env) > +{ > + int ret; > + struct kvm_enable_cap encap = {}; > + > + if (!kvm_enabled()) { > + return -1; > + } > + > + if (!cap_ppc_watchdog) { > + printf("warning: KVM does not support watchdog"); > + return -1; > + } > + > + 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; > +} > diff --git a/target-ppc/kvm_ppc.h b/target-ppc/kvm_ppc.h > index baad6eb..4ccc9fa 100644 > --- a/target-ppc/kvm_ppc.h > +++ b/target-ppc/kvm_ppc.h > @@ -23,6 +23,8 @@ int kvmppc_get_hypercall(CPUPPCState *env, uint8_t *buf, int buf_len); > int kvmppc_set_interrupt(CPUPPCState *env, int irq, int level); > void kvmppc_set_papr(CPUPPCState *env); > int kvmppc_smt_threads(void); > +int kvmppc_synch_sregs_tsr(CPUPPCState *cenv); > +int kvmppc_booke_watchdog_enable(CPUPPCState *env); > #ifndef CONFIG_USER_ONLY Missing code for !CONFIG_KVM. Please always check compile your code on x86. Alex > off_t kvmppc_alloc_rma(const char *name, MemoryRegion *sysmem); > void *kvmppc_create_spapr_tce(uint32_t liobn, uint32_t window_size, int *pfd); > @@ -79,6 +81,15 @@ static inline int kvmppc_smt_threads(void) > return 1; > } > > +static inline int kvmppc_synch_sregs_tsr(CPUPPCState *env) > +{ > + return -1; > +} > + > +static inline int kvmppc_booke_watchdog_enable(CPUPPCState *env) > +{ > + return -1; > +} > #ifndef CONFIG_USER_ONLY > static inline off_t kvmppc_alloc_rma(const char *name, MemoryRegion *sysmem) > { > -- > 1.7.0.4 > >
> -----Original Message----- > From: Alexander Graf [mailto:agraf@suse.de] > Sent: Thursday, January 10, 2013 9:07 PM > To: Bhushan Bharat-R65777 > Cc: qemu-devel@nongnu.org; qemu-ppc@nongnu.org; Bhushan Bharat-R65777 > Subject: Re: [PATCH 3/3 v2] Enable kvm emulated watchdog > > > On 28.12.2012, at 06:16, 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> > > --- > > v2: > > - access cap_* from target_ppc/kvm.c only. > > - Added wrapper functions in target_ppc/kvm.c for > > enable_watchdog and tsr_sregs synchronization. > > - Incorporated other Review comments > > > > hw/ppc.h | 1 + > > hw/ppc_booke.c | 36 +++++++++++++++++++++++++++++++- > > target-ppc/kvm.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++ > > target-ppc/kvm_ppc.h | 11 +++++++++ > > 4 files changed, 103 insertions(+), 1 deletions(-) > > > > diff --git a/hw/ppc.h b/hw/ppc.h > > index 2f3ea27..6ad9e1f 100644 > > --- a/hw/ppc.h > > +++ b/hw/ppc.h > > @@ -90,3 +90,4 @@ enum { > > > > /* ppc_booke.c */ > > void ppc_booke_timers_init(CPUPPCState *env, uint32_t freq, uint32_t > > flags); > > +void ppc_booke_watchdog_clear_tsr(CPUPPCState *env); > > diff --git a/hw/ppc_booke.c b/hw/ppc_booke.c index 837a5b6..7273259 > > 100644 > > --- a/hw/ppc_booke.c > > +++ b/hw/ppc_booke.c > > @@ -28,7 +28,7 @@ > > #include "nvram.h" > > #include "qemu-log.h" > > #include "loader.h" > > - > > +#include "kvm_ppc.h" > > > > /* Timer Control Register */ > > > > @@ -203,6 +203,11 @@ static void booke_wdt_cb(void *opaque) > > booke_timer->wdt_timer); } > > > > +void ppc_booke_watchdog_clear_tsr(CPUPPCState *env) { > > + env->spr[SPR_BOOKE_TSR] &= ~(TSR_ENW | TSR_WIS | TSR_WRS_MASK); } > > + > > void store_booke_tsr(CPUPPCState *env, target_ulong val) { > > env->spr[SPR_BOOKE_TSR] &= ~val; > > @@ -241,10 +246,27 @@ 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) { > > Needs a comment when this happens > > > + CPUPPCState *env = opaque; > > + > > + if (!running) { > > + return; > > + } > > + > > + /* > > + * Clear watchdog interrupt condition by clearing TSR. > > + */ > > + ppc_booke_watchdog_clear_tsr(env); > > + > > + kvmppc_synch_sregs_tsr(env); > > kvmppc_sync_tsr. Also please add the sync to store_booke_tsr(). Then here, you > can just do > > store_booke_tsr(TSR_ENW | TSR_WIS | TSR_WRS_MASK); > > > +} > > + > > void ppc_booke_timers_init(CPUPPCState *env, uint32_t freq, uint32_t > > flags) { > > ppc_tb_t *tb_env; > > booke_timer_t *booke_timer; > > + int ret = 0; > > > > tb_env = g_malloc0(sizeof(ppc_tb_t)); > > booke_timer = g_malloc0(sizeof(booke_timer_t)); @@ -262,5 +284,17 > > @@ 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); > > > > + ret = kvmppc_booke_watchdog_enable(env); > > + > > + if (ret) { > > + /* TODO: Start the QEMU emulated watchdog if not running on KVM. > > + * Also start the QEMU emulated watchdog if KVM does not support > > + * emulated watchdog or somehow it is not enabled (supported but > > + * not enabled is though some bug and requires debugging :)). > > + */ > > + } > > + > > + 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..6828afa > > 100644 > > --- a/target-ppc/kvm.c > > +++ b/target-ppc/kvm.c > > @@ -32,10 +32,12 @@ > > #include "device_tree.h" > > #include "hw/sysbus.h" > > #include "hw/spapr.h" > > +#include "hw/watchdog.h" > > > > #include "hw/sysbus.h" > > #include "hw/spapr.h" > > #include "hw/spapr_vio.h" > > +#include "hw/ppc.h" > > > > //#define DEBUG_KVM > > > > @@ -61,6 +63,7 @@ static int cap_ppc_smt; static int cap_ppc_rma; > > static int cap_spapr_tce; static int cap_hior; > > +static 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 +93,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 +827,12 @@ int kvm_arch_handle_exit(CPUPPCState *env, struct kvm_run > *run) > > ret = 0; > > break; > > #endif > > + case KVM_EXIT_WATCHDOG: > > + dprintf("handle watchdog expiry\n"); > > + watchdog_perform_action(); > > + ret = 0; > > + break; > > + > > default: > > fprintf(stderr, "KVM: unknown exit reason %d\n", run->exit_reason); > > ret = -1; > > @@ -1239,3 +1249,49 @@ int kvm_arch_on_sigbus(int code, void *addr) > > { > > return 1; > > } > > + > > +int kvmppc_synch_sregs_tsr(CPUPPCState *env) > > +{ > > + struct kvm_sregs sregs; > > + int ret; > > + > > + if (!kvm_enabled() || !cap_booke_sregs || !cap_ppc_watchdog) { > > + return -1; > > + } > > + > > + ret = kvm_vcpu_ioctl(env, KVM_GET_SREGS, &sregs); > > + > > + env->spr[SPR_BOOKE_TSR] = sregs.u.e.tsr; > > + ppc_booke_watchdog_clear_tsr(env); > > You're getting _and_ setting it in the same function? This is nonsense. Please > split it into a get and a set function if you need it. > > > + sregs.u.e.tsr = env->spr[SPR_BOOKE_TSR]; > > + sregs.u.e.update_special = KVM_SREGS_E_UPDATE_TSR; > > + > > + ret = kvm_vcpu_ioctl(env, KVM_SET_SREGS, &sregs); > > + > > + return ret; > > +} > > + > > +int kvmppc_booke_watchdog_enable(CPUPPCState *env) > > +{ > > + int ret; > > + struct kvm_enable_cap encap = {}; > > + > > + if (!kvm_enabled()) { > > + return -1; > > + } > > + > > + if (!cap_ppc_watchdog) { > > + printf("warning: KVM does not support watchdog"); > > + return -1; > > + } > > + > > + 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; > > +} > > diff --git a/target-ppc/kvm_ppc.h b/target-ppc/kvm_ppc.h > > index baad6eb..4ccc9fa 100644 > > --- a/target-ppc/kvm_ppc.h > > +++ b/target-ppc/kvm_ppc.h > > @@ -23,6 +23,8 @@ int kvmppc_get_hypercall(CPUPPCState *env, uint8_t *buf, int > buf_len); > > int kvmppc_set_interrupt(CPUPPCState *env, int irq, int level); > > void kvmppc_set_papr(CPUPPCState *env); > > int kvmppc_smt_threads(void); > > +int kvmppc_synch_sregs_tsr(CPUPPCState *cenv); > > +int kvmppc_booke_watchdog_enable(CPUPPCState *env); > > #ifndef CONFIG_USER_ONLY > > Missing code for !CONFIG_KVM. Please always check compile your code on x86. Alex is not the below code covers !CONFIG_KVM ... > > > Alex > > > off_t kvmppc_alloc_rma(const char *name, MemoryRegion *sysmem); > > void *kvmppc_create_spapr_tce(uint32_t liobn, uint32_t window_size, int *pfd); > > @@ -79,6 +81,15 @@ static inline int kvmppc_smt_threads(void) > > return 1; > > } > > > > +static inline int kvmppc_synch_sregs_tsr(CPUPPCState *env) > > +{ > > + return -1; > > +} > > + > > +static inline int kvmppc_booke_watchdog_enable(CPUPPCState *env) > > +{ > > + return -1; > > +} I mean this one Thanks -Bharat > > #ifndef CONFIG_USER_ONLY > > static inline off_t kvmppc_alloc_rma(const char *name, MemoryRegion *sysmem) > > { > > -- > > 1.7.0.4 > > > > >
Am 11.01.2013 um 07:42 schrieb Bhushan Bharat-R65777 <R65777@freescale.com>: > > >> -----Original Message----- >> From: Alexander Graf [mailto:agraf@suse.de] >> Sent: Thursday, January 10, 2013 9:07 PM >> To: Bhushan Bharat-R65777 >> Cc: qemu-devel@nongnu.org; qemu-ppc@nongnu.org; Bhushan Bharat-R65777 >> Subject: Re: [PATCH 3/3 v2] Enable kvm emulated watchdog >> >> >> On 28.12.2012, at 06:16, 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> >>> --- >>> v2: >>> - access cap_* from target_ppc/kvm.c only. >>> - Added wrapper functions in target_ppc/kvm.c for >>> enable_watchdog and tsr_sregs synchronization. >>> - Incorporated other Review comments >>> >>> hw/ppc.h | 1 + >>> hw/ppc_booke.c | 36 +++++++++++++++++++++++++++++++- >>> target-ppc/kvm.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++ >>> target-ppc/kvm_ppc.h | 11 +++++++++ >>> 4 files changed, 103 insertions(+), 1 deletions(-) >>> >>> diff --git a/hw/ppc.h b/hw/ppc.h >>> index 2f3ea27..6ad9e1f 100644 >>> --- a/hw/ppc.h >>> +++ b/hw/ppc.h >>> @@ -90,3 +90,4 @@ enum { >>> >>> /* ppc_booke.c */ >>> void ppc_booke_timers_init(CPUPPCState *env, uint32_t freq, uint32_t >>> flags); >>> +void ppc_booke_watchdog_clear_tsr(CPUPPCState *env); >>> diff --git a/hw/ppc_booke.c b/hw/ppc_booke.c index 837a5b6..7273259 >>> 100644 >>> --- a/hw/ppc_booke.c >>> +++ b/hw/ppc_booke.c >>> @@ -28,7 +28,7 @@ >>> #include "nvram.h" >>> #include "qemu-log.h" >>> #include "loader.h" >>> - >>> +#include "kvm_ppc.h" >>> >>> /* Timer Control Register */ >>> >>> @@ -203,6 +203,11 @@ static void booke_wdt_cb(void *opaque) >>> booke_timer->wdt_timer); } >>> >>> +void ppc_booke_watchdog_clear_tsr(CPUPPCState *env) { >>> + env->spr[SPR_BOOKE_TSR] &= ~(TSR_ENW | TSR_WIS | TSR_WRS_MASK); } >>> + >>> void store_booke_tsr(CPUPPCState *env, target_ulong val) { >>> env->spr[SPR_BOOKE_TSR] &= ~val; >>> @@ -241,10 +246,27 @@ 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) { >> >> Needs a comment when this happens >> >>> + CPUPPCState *env = opaque; >>> + >>> + if (!running) { >>> + return; >>> + } >>> + >>> + /* >>> + * Clear watchdog interrupt condition by clearing TSR. >>> + */ >>> + ppc_booke_watchdog_clear_tsr(env); >>> + >>> + kvmppc_synch_sregs_tsr(env); >> >> kvmppc_sync_tsr. Also please add the sync to store_booke_tsr(). Then here, you >> can just do >> >> store_booke_tsr(TSR_ENW | TSR_WIS | TSR_WRS_MASK); >> >>> +} >>> + >>> void ppc_booke_timers_init(CPUPPCState *env, uint32_t freq, uint32_t >>> flags) { >>> ppc_tb_t *tb_env; >>> booke_timer_t *booke_timer; >>> + int ret = 0; >>> >>> tb_env = g_malloc0(sizeof(ppc_tb_t)); >>> booke_timer = g_malloc0(sizeof(booke_timer_t)); @@ -262,5 +284,17 >>> @@ 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); >>> >>> + ret = kvmppc_booke_watchdog_enable(env); >>> + >>> + if (ret) { >>> + /* TODO: Start the QEMU emulated watchdog if not running on KVM. >>> + * Also start the QEMU emulated watchdog if KVM does not support >>> + * emulated watchdog or somehow it is not enabled (supported but >>> + * not enabled is though some bug and requires debugging :)). >>> + */ >>> + } >>> + >>> + 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..6828afa >>> 100644 >>> --- a/target-ppc/kvm.c >>> +++ b/target-ppc/kvm.c >>> @@ -32,10 +32,12 @@ >>> #include "device_tree.h" >>> #include "hw/sysbus.h" >>> #include "hw/spapr.h" >>> +#include "hw/watchdog.h" >>> >>> #include "hw/sysbus.h" >>> #include "hw/spapr.h" >>> #include "hw/spapr_vio.h" >>> +#include "hw/ppc.h" >>> >>> //#define DEBUG_KVM >>> >>> @@ -61,6 +63,7 @@ static int cap_ppc_smt; static int cap_ppc_rma; >>> static int cap_spapr_tce; static int cap_hior; >>> +static 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 +93,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 +827,12 @@ int kvm_arch_handle_exit(CPUPPCState *env, struct kvm_run >> *run) >>> ret = 0; >>> break; >>> #endif >>> + case KVM_EXIT_WATCHDOG: >>> + dprintf("handle watchdog expiry\n"); >>> + watchdog_perform_action(); >>> + ret = 0; >>> + break; >>> + >>> default: >>> fprintf(stderr, "KVM: unknown exit reason %d\n", run->exit_reason); >>> ret = -1; >>> @@ -1239,3 +1249,49 @@ int kvm_arch_on_sigbus(int code, void *addr) >>> { >>> return 1; >>> } >>> + >>> +int kvmppc_synch_sregs_tsr(CPUPPCState *env) >>> +{ >>> + struct kvm_sregs sregs; >>> + int ret; >>> + >>> + if (!kvm_enabled() || !cap_booke_sregs || !cap_ppc_watchdog) { >>> + return -1; >>> + } >>> + >>> + ret = kvm_vcpu_ioctl(env, KVM_GET_SREGS, &sregs); >>> + >>> + env->spr[SPR_BOOKE_TSR] = sregs.u.e.tsr; >>> + ppc_booke_watchdog_clear_tsr(env); >> >> You're getting _and_ setting it in the same function? This is nonsense. Please >> split it into a get and a set function if you need it. >> >>> + sregs.u.e.tsr = env->spr[SPR_BOOKE_TSR]; >>> + sregs.u.e.update_special = KVM_SREGS_E_UPDATE_TSR; >>> + >>> + ret = kvm_vcpu_ioctl(env, KVM_SET_SREGS, &sregs); >>> + >>> + return ret; >>> +} >>> + >>> +int kvmppc_booke_watchdog_enable(CPUPPCState *env) >>> +{ >>> + int ret; >>> + struct kvm_enable_cap encap = {}; >>> + >>> + if (!kvm_enabled()) { >>> + return -1; >>> + } >>> + >>> + if (!cap_ppc_watchdog) { >>> + printf("warning: KVM does not support watchdog"); >>> + return -1; >>> + } >>> + >>> + 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; >>> +} >>> diff --git a/target-ppc/kvm_ppc.h b/target-ppc/kvm_ppc.h >>> index baad6eb..4ccc9fa 100644 >>> --- a/target-ppc/kvm_ppc.h >>> +++ b/target-ppc/kvm_ppc.h >>> @@ -23,6 +23,8 @@ int kvmppc_get_hypercall(CPUPPCState *env, uint8_t *buf, int >> buf_len); >>> int kvmppc_set_interrupt(CPUPPCState *env, int irq, int level); >>> void kvmppc_set_papr(CPUPPCState *env); >>> int kvmppc_smt_threads(void); >>> +int kvmppc_synch_sregs_tsr(CPUPPCState *cenv); >>> +int kvmppc_booke_watchdog_enable(CPUPPCState *env); >>> #ifndef CONFIG_USER_ONLY >> >> Missing code for !CONFIG_KVM. Please always check compile your code on x86. > > Alex is not the below code covers !CONFIG_KVM ... > >> >> >> Alex >> >>> off_t kvmppc_alloc_rma(const char *name, MemoryRegion *sysmem); >>> void *kvmppc_create_spapr_tce(uint32_t liobn, uint32_t window_size, int *pfd); >>> @@ -79,6 +81,15 @@ static inline int kvmppc_smt_threads(void) >>> return 1; >>> } >>> >>> +static inline int kvmppc_synch_sregs_tsr(CPUPPCState *env) >>> +{ >>> + return -1; >>> +} >>> + >>> +static inline int kvmppc_booke_watchdog_enable(CPUPPCState *env) >>> +{ >>> + return -1; >>> +} > > I mean this one Hrm. Somehow I didn't see those lines during review :). Yes, it's fine - sorry. Alex > > Thanks > -Bharat > >>> #ifndef CONFIG_USER_ONLY >>> static inline off_t kvmppc_alloc_rma(const char *name, MemoryRegion *sysmem) >>> { >>> -- >>> 1.7.0.4 >>> >>> >> > >
diff --git a/hw/ppc.h b/hw/ppc.h index 2f3ea27..6ad9e1f 100644 --- a/hw/ppc.h +++ b/hw/ppc.h @@ -90,3 +90,4 @@ enum { /* ppc_booke.c */ void ppc_booke_timers_init(CPUPPCState *env, uint32_t freq, uint32_t flags); +void ppc_booke_watchdog_clear_tsr(CPUPPCState *env); diff --git a/hw/ppc_booke.c b/hw/ppc_booke.c index 837a5b6..7273259 100644 --- a/hw/ppc_booke.c +++ b/hw/ppc_booke.c @@ -28,7 +28,7 @@ #include "nvram.h" #include "qemu-log.h" #include "loader.h" - +#include "kvm_ppc.h" /* Timer Control Register */ @@ -203,6 +203,11 @@ static void booke_wdt_cb(void *opaque) booke_timer->wdt_timer); } +void ppc_booke_watchdog_clear_tsr(CPUPPCState *env) +{ + env->spr[SPR_BOOKE_TSR] &= ~(TSR_ENW | TSR_WIS | TSR_WRS_MASK); +} + void store_booke_tsr(CPUPPCState *env, target_ulong val) { env->spr[SPR_BOOKE_TSR] &= ~val; @@ -241,10 +246,27 @@ 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; + + if (!running) { + return; + } + + /* + * Clear watchdog interrupt condition by clearing TSR. + */ + ppc_booke_watchdog_clear_tsr(env); + + kvmppc_synch_sregs_tsr(env); +} + void ppc_booke_timers_init(CPUPPCState *env, uint32_t freq, uint32_t flags) { ppc_tb_t *tb_env; booke_timer_t *booke_timer; + int ret = 0; tb_env = g_malloc0(sizeof(ppc_tb_t)); booke_timer = g_malloc0(sizeof(booke_timer_t)); @@ -262,5 +284,17 @@ 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); + ret = kvmppc_booke_watchdog_enable(env); + + if (ret) { + /* TODO: Start the QEMU emulated watchdog if not running on KVM. + * Also start the QEMU emulated watchdog if KVM does not support + * emulated watchdog or somehow it is not enabled (supported but + * not enabled is though some bug and requires debugging :)). + */ + } + + 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..6828afa 100644 --- a/target-ppc/kvm.c +++ b/target-ppc/kvm.c @@ -32,10 +32,12 @@ #include "device_tree.h" #include "hw/sysbus.h" #include "hw/spapr.h" +#include "hw/watchdog.h" #include "hw/sysbus.h" #include "hw/spapr.h" #include "hw/spapr_vio.h" +#include "hw/ppc.h" //#define DEBUG_KVM @@ -61,6 +63,7 @@ static int cap_ppc_smt; static int cap_ppc_rma; static int cap_spapr_tce; static int cap_hior; +static 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 +93,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 +827,12 @@ int kvm_arch_handle_exit(CPUPPCState *env, struct kvm_run *run) ret = 0; break; #endif + case KVM_EXIT_WATCHDOG: + dprintf("handle watchdog expiry\n"); + watchdog_perform_action(); + ret = 0; + break; + default: fprintf(stderr, "KVM: unknown exit reason %d\n", run->exit_reason); ret = -1; @@ -1239,3 +1249,49 @@ int kvm_arch_on_sigbus(int code, void *addr) { return 1; } + +int kvmppc_synch_sregs_tsr(CPUPPCState *env) +{ + struct kvm_sregs sregs; + int ret; + + if (!kvm_enabled() || !cap_booke_sregs || !cap_ppc_watchdog) { + return -1; + } + + ret = kvm_vcpu_ioctl(env, KVM_GET_SREGS, &sregs); + + env->spr[SPR_BOOKE_TSR] = sregs.u.e.tsr; + ppc_booke_watchdog_clear_tsr(env); + sregs.u.e.tsr = env->spr[SPR_BOOKE_TSR]; + sregs.u.e.update_special = KVM_SREGS_E_UPDATE_TSR; + + ret = kvm_vcpu_ioctl(env, KVM_SET_SREGS, &sregs); + + return ret; +} + +int kvmppc_booke_watchdog_enable(CPUPPCState *env) +{ + int ret; + struct kvm_enable_cap encap = {}; + + if (!kvm_enabled()) { + return -1; + } + + if (!cap_ppc_watchdog) { + printf("warning: KVM does not support watchdog"); + return -1; + } + + 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; +} diff --git a/target-ppc/kvm_ppc.h b/target-ppc/kvm_ppc.h index baad6eb..4ccc9fa 100644 --- a/target-ppc/kvm_ppc.h +++ b/target-ppc/kvm_ppc.h @@ -23,6 +23,8 @@ int kvmppc_get_hypercall(CPUPPCState *env, uint8_t *buf, int buf_len); int kvmppc_set_interrupt(CPUPPCState *env, int irq, int level); void kvmppc_set_papr(CPUPPCState *env); int kvmppc_smt_threads(void); +int kvmppc_synch_sregs_tsr(CPUPPCState *cenv); +int kvmppc_booke_watchdog_enable(CPUPPCState *env); #ifndef CONFIG_USER_ONLY off_t kvmppc_alloc_rma(const char *name, MemoryRegion *sysmem); void *kvmppc_create_spapr_tce(uint32_t liobn, uint32_t window_size, int *pfd); @@ -79,6 +81,15 @@ static inline int kvmppc_smt_threads(void) return 1; } +static inline int kvmppc_synch_sregs_tsr(CPUPPCState *env) +{ + return -1; +} + +static inline int kvmppc_booke_watchdog_enable(CPUPPCState *env) +{ + return -1; +} #ifndef CONFIG_USER_ONLY static inline off_t kvmppc_alloc_rma(const char *name, MemoryRegion *sysmem) {
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> --- v2: - access cap_* from target_ppc/kvm.c only. - Added wrapper functions in target_ppc/kvm.c for enable_watchdog and tsr_sregs synchronization. - Incorporated other Review comments hw/ppc.h | 1 + hw/ppc_booke.c | 36 +++++++++++++++++++++++++++++++- target-ppc/kvm.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++ target-ppc/kvm_ppc.h | 11 +++++++++ 4 files changed, 103 insertions(+), 1 deletions(-)