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.bhus...@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
>
>
