qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [EXTERNAL] [PATCH] hw/watchdog: Implement generic watchdog device mo


From: Graeme Gregory
Subject: Re: [EXTERNAL] [PATCH] hw/watchdog: Implement generic watchdog device model as per ARM BSAv0.9 hw/arm/sbsa-ref : Include SBSA watchdog device in sbsa-ref platform
Date: Thu, 17 Sep 2020 21:37:02 +0000

Something still seems to have gone wrong with subject, and description
of patch is missing? Missed blank line between 1st line and description?

Do you have the link to updated ACPI tables for testing out of interest?

On Thu, Sep 17, 2020 at 02:12:45PM -0400, Shashi Mallela wrote:
> Signed-off-by: Shashi Mallela <shashi.mallela@linaro.org>
> ---
>  hw/arm/Kconfig                      |   1 +
>  hw/arm/sbsa-ref.c                   |  23 ++
>  hw/watchdog/Kconfig                 |   4 +
>  hw/watchdog/meson.build             |   1 +
>  hw/watchdog/wdt_sbsa_gwdt.c         | 343 ++++++++++++++++++++++++++++
>  include/hw/watchdog/wdt_sbsa_gwdt.h |  68 ++++++
>  6 files changed, 440 insertions(+)
>  create mode 100644 hw/watchdog/wdt_sbsa_gwdt.c
>  create mode 100644 include/hw/watchdog/wdt_sbsa_gwdt.h
>
> diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
> index f303c6bead25..6b97e64595d3 100644
> --- a/hw/arm/Kconfig
> +++ b/hw/arm/Kconfig
> @@ -210,6 +210,7 @@ config SBSA_REF
>      select PL031 # RTC
>      select PL061 # GPIO
>      select USB_EHCI_SYSBUS
> +    select WDT_SBSA_GWDT
>
>  config SABRELITE
>      bool
> diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c
> index ac68b4640d0d..652079a12e37 100644
> --- a/hw/arm/sbsa-ref.c
> +++ b/hw/arm/sbsa-ref.c
> @@ -40,6 +40,7 @@
>  #include "hw/qdev-properties.h"
>  #include "hw/usb.h"
>  #include "hw/char/pl011.h"
> +#include "hw/watchdog/wdt_sbsa_gwdt.h"
>  #include "net/net.h"
>  #include "qom/object.h"
>
> @@ -64,6 +65,9 @@ enum {
>      SBSA_GIC_DIST,
>      SBSA_GIC_REDIST,
>      SBSA_SECURE_EC,
> +    SBSA_GWDT,
> +    SBSA_GWDT_REFRESH,
> +    SBSA_GWDT_CONTROL,
>      SBSA_SMMU,
>      SBSA_UART,
>      SBSA_RTC,
> @@ -111,6 +115,8 @@ static const MemMapEntry sbsa_ref_memmap[] = {
>      [SBSA_GIC_DIST] =           { 0x40060000, 0x00010000 },
>      [SBSA_GIC_REDIST] =         { 0x40080000, 0x04000000 },
>      [SBSA_SECURE_EC] =          { 0x50000000, 0x00001000 },
> +    [SBSA_GWDT_REFRESH] =       { 0x50010000, 0x00001000 },
> +    [SBSA_GWDT_CONTROL] =       { 0x50011000, 0x00001000 },
>      [SBSA_UART] =               { 0x60000000, 0x00001000 },
>      [SBSA_RTC] =                { 0x60010000, 0x00001000 },
>      [SBSA_GPIO] =               { 0x60020000, 0x00001000 },
> @@ -140,6 +146,7 @@ static const int sbsa_ref_irqmap[] = {
>      [SBSA_SECURE_UART_MM] = 9,
>      [SBSA_AHCI] = 10,
>      [SBSA_EHCI] = 11,
> +    [SBSA_GWDT] = 12,
>  };
>
>  static uint64_t sbsa_ref_cpu_mp_affinity(SBSAMachineState *sms, int idx)
> @@ -454,6 +461,20 @@ static void create_rtc(const SBSAMachineState *sms)
>      sysbus_create_simple("pl031", base, qdev_get_gpio_in(sms->gic, irq));
>  }
>
> +static void create_wdt(const SBSAMachineState *sms)
> +{
> +    hwaddr rbase = sbsa_ref_memmap[SBSA_GWDT_REFRESH].base;
> +    hwaddr cbase = sbsa_ref_memmap[SBSA_GWDT_CONTROL].base;
> +    DeviceState *dev = qdev_new(TYPE_WDT_SBSA_GWDT);
> +    SysBusDevice *s = SYS_BUS_DEVICE(dev);
> +    int irq = sbsa_ref_irqmap[SBSA_GWDT];
> +
> +    sysbus_realize_and_unref(s, &error_fatal);
> +    sysbus_mmio_map(s, 0, rbase);
> +    sysbus_mmio_map(s, 1, cbase);
> +    sysbus_connect_irq(s, 0, qdev_get_gpio_in(sms->gic, irq));
> +}
> +
>  static DeviceState *gpio_key_dev;
>  static void sbsa_ref_powerdown_req(Notifier *n, void *opaque)
>  {
> @@ -737,6 +758,8 @@ static void sbsa_ref_init(MachineState *machine)
>
>      create_rtc(sms);
>
> +    create_wdt(sms);
> +
>      create_gpio(sms);
>
>      create_ahci(sms);
> diff --git a/hw/watchdog/Kconfig b/hw/watchdog/Kconfig
> index 293209b291d6..94ac0d5b76b1 100644
> --- a/hw/watchdog/Kconfig
> +++ b/hw/watchdog/Kconfig
> @@ -17,3 +17,7 @@ config WDT_DIAG288
>
>  config WDT_IMX2
>      bool
> +
> +config WDT_SBSA_GWDT
> +    bool
> +    default y if SBSA_REF
> \ No newline at end of file
> diff --git a/hw/watchdog/meson.build b/hw/watchdog/meson.build
> index 9b8725e64288..a9a23307acfe 100644
> --- a/hw/watchdog/meson.build
> +++ b/hw/watchdog/meson.build
> @@ -5,3 +5,4 @@ softmmu_ss.add(when: 'CONFIG_WDT_IB700', if_true: 
> files('wdt_ib700.c'))
>  softmmu_ss.add(when: 'CONFIG_WDT_DIAG288', if_true: files('wdt_diag288.c'))
>  softmmu_ss.add(when: 'CONFIG_ASPEED_SOC', if_true: files('wdt_aspeed.c'))
>  softmmu_ss.add(when: 'CONFIG_WDT_IMX2', if_true: files('wdt_imx2.c'))
> +softmmu_ss.add(when: 'CONFIG_WDT_SBSA_GWDT', if_true: 
> files('wdt_sbsa_gwdt.c'))
> diff --git a/hw/watchdog/wdt_sbsa_gwdt.c b/hw/watchdog/wdt_sbsa_gwdt.c
> new file mode 100644
> index 000000000000..eb3be0862cf1
> --- /dev/null
> +++ b/hw/watchdog/wdt_sbsa_gwdt.c
> @@ -0,0 +1,343 @@
> +/*
> + * Generic watchdog device model for SBSA
> + *
> + * Copyright Linaro.org 2020
> + *
> + * Authors:
> + *  Shashi Mallela <shashi.mallela@linaro.org>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or (at 
> your
> + * option) any later version.  See the COPYING file in the top-level 
> directory.
> + *
> + */
> +
> +#include "qemu/osdep.h"
> +#include "sysemu/reset.h"
> +#include "sysemu/watchdog.h"
> +#include "hw/watchdog/wdt_sbsa_gwdt.h"
> +#include "qemu/timer.h"
> +#include "migration/vmstate.h"
> +#include "qemu/log.h"
> +#include "qemu/module.h"
> +
> +static WatchdogTimerModel model = {
> +    .wdt_name = TYPE_WDT_SBSA_GWDT,
> +    .wdt_description = "sbsa_gwdt device for sbsa_ref platform",
> +};
> +
> +static const VMStateDescription vmstate_sbsa_gwdt = {
> +    .name = "vmstate_sbsa_gwdt",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_TIMER_PTR(timer, SBSA_GWDTState),
> +        VMSTATE_BOOL(enabled, SBSA_GWDTState),
> +        VMSTATE_BOOL(ws0, SBSA_GWDTState),
> +        VMSTATE_BOOL(ws1, SBSA_GWDTState),
> +        VMSTATE_UINT32(wrr, SBSA_GWDTState),
> +        VMSTATE_UINT32(wcs, SBSA_GWDTState),
> +        VMSTATE_UINT32(worl, SBSA_GWDTState),
> +        VMSTATE_UINT32(woru, SBSA_GWDTState),
> +        VMSTATE_UINT32(wcvl, SBSA_GWDTState),
> +        VMSTATE_UINT32(wcvu, SBSA_GWDTState),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
> +static uint64_t sbsa_gwdt_rread(void *opaque, hwaddr addr, unsigned int size)
> +{
> +    uint32_t ret;
> +
> +    if (addr == SBSA_GWDT_WRR) {
> +        /* watch refresh read has no effect and returns 0 */
> +        ret = 0;
> +    } else {
> +        fprintf(stderr, "bad address in refresh frame read : 0x%x\n",
> +                      (int)addr);

I suspect this and the other fprintf's should be using the
qemu_log_mask(LOG_GUEST_ERROR, ... API

Graeme

> +    }
> +    return ret;
> +}
> +
> +static uint64_t sbsa_gwdt_read(void *opaque, hwaddr addr, unsigned int size)
> +{
> +    SBSA_GWDTState *s = SBSA_GWDT(opaque);
> +    uint32_t ret = 0;
> +
> +    switch (addr) {
> +    case SBSA_GWDT_WCS:
> +        ret = s->wcs;
> +        break;
> +    case SBSA_GWDT_WOR:
> +        ret = s->worl;
> +        break;
> +    case SBSA_GWDT_WORU:
> +         ret = s->woru;
> +         break;
> +    case SBSA_GWDT_WCV:
> +        ret = s->wcvl;
> +        break;
> +    case SBSA_GWDT_WCVU:
> +        ret = s->wcvu;
> +        break;
> +    case SBSA_GWDT_W_IIDR:
> +        ret = s->id;
> +        break;
> +    default:
> +        fprintf(stderr, "bad address in control frame read : 0x%x\n",
> +                         (int)addr);
> +    }
> +    return ret;
> +}
> +
> +static void sbsa_gwdt_update_timer(SBSA_GWDTState *s, bool refresh,
> +                                   bool expired)
> +{
> +    uint64_t timeout = 0;
> +
> +    if (s->enabled) {
> +        if ((refresh) || (expired && !s->ws0)) {
> +            /*
> +             * Extract the upper 16 bits from woru & 32 bits from worl
> +             * registers to construct the 48 bit offset value
> +             */
> +            timeout = s->woru & SBSA_GWDT_WOR_MASK;
> +            timeout <<= 32;
> +            timeout |= s->worl;
> +            timeout += qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
> +
> +            /* store the current timeout value into compare registers */
> +            s->wcvu = timeout >> 32;
> +            s->wcvl = timeout;
> +        } else {
> +            timeout = s->wcvu;
> +            timeout <<= 32;
> +            timeout |= s->wcvl;
> +        }
> +
> +        if (!s->ws0) {
> +            timer_del(s->timer);
> +            timer_mod(s->ptimer,
> +                       timeout * NANOSECONDS_PER_SECOND);
> +        } else {
> +            timer_del(s->ptimer);
> +            timer_mod(s->timer,
> +                       timeout * NANOSECONDS_PER_SECOND);
> +        }
> +    } else {
> +        timer_del(s->ptimer);
> +        timer_del(s->timer);
> +    }
> +}
> +
> +static void sbsa_gwdt_rwrite(void *opaque, hwaddr offset, uint64_t data,
> +                             unsigned size) {
> +    SBSA_GWDTState *s = SBSA_GWDT(opaque);
> +
> +    if (offset == SBSA_GWDT_WRR) {
> +        s->wrr = data;
> +        s->wcs &= ~SBSA_GWDT_WCS_WS0;
> +        s->wcs &= ~SBSA_GWDT_WCS_WS1;
> +        s->ws0 = false;
> +        s->ws1 = false;
> +        sbsa_gwdt_update_timer(s, true , false);
> +    } else {
> +        fprintf(stderr, "bad address in refresh frame write : 0x%x\n",
> +                        (int)offset);
> +    }
> +}
> +
> +static void sbsa_gwdt_write(void *opaque, hwaddr offset, uint64_t data,
> +                             unsigned size) {
> +    SBSA_GWDTState *s = SBSA_GWDT(opaque);
> +    bool enable;
> +
> +    switch (offset) {
> +    case SBSA_GWDT_WCS:
> +        enable = data & SBSA_GWDT_WCS_EN;
> +        if (enable) {
> +            s->wcs |= SBSA_GWDT_WCS_EN;
> +            s->enabled = true;
> +        } else {
> +            s->wcs &= ~SBSA_GWDT_WCS_EN;
> +            s->enabled = false;
> +            s->ws0 = false;
> +            s->ws1 = false;
> +        }
> +        sbsa_gwdt_update_timer(s, false, false);
> +        break;
> +
> +    case SBSA_GWDT_WOR:
> +        s->worl = data;
> +        /*
> +         * TODO:- setting woru to 0 and triggering update timer(below) is a
> +         * temporary workaround to handle current linux driver which is
> +         * based on earlier version of BSA specification.Once the linux
> +         * driver is updated to BSA v0.9 will remove these next 2 lines.
> +         */
> +        s->woru = 0;
> +        sbsa_gwdt_update_timer(s, true , false);
> +        break;
> +
> +    case SBSA_GWDT_WORU:
> +        s->woru = data;
> +        sbsa_gwdt_update_timer(s, true , false);
> +        break;
> +
> +    case SBSA_GWDT_WCV:
> +        s->wcvl = data;
> +        break;
> +
> +    case SBSA_GWDT_WCVU:
> +        s->wcvu = data;
> +        break;
> +
> +    default:
> +        fprintf(stderr, "bad address in control frame write : 0x%x\n",
> +                (int)offset);
> +    }
> +    return;
> +}
> +
> +static void wdt_sbsa_gwdt_reset(DeviceState *dev)
> +{
> +    SBSA_GWDTState *s = SBSA_GWDT(dev);
> +
> +    timer_del(s->ptimer);
> +    timer_del(s->timer);
> +
> +    s->enabled = false;
> +    s->ws0 = false;
> +    s->ws1 = false;
> +    s->wcs &= ~SBSA_GWDT_WCS_EN;
> +    s->wcs &= ~SBSA_GWDT_WCS_WS0;
> +    s->wcs &= ~SBSA_GWDT_WCS_WS1;
> +    s->wcvl = 0;
> +    s->wcvu = 0;
> +    s->worl = 0;
> +    s->woru = 0;
> +    s->id = SBSA_GWDT_ID;
> +}
> +
> +static void sbsa_gwdt_reset(void *opaque)
> +{
> +    DeviceState *sbsa_gwdt = opaque;
> +
> +    wdt_sbsa_gwdt_reset(sbsa_gwdt);
> +}
> +
> +static void sbsa_gwdt_timer_sysinterrupt(void *opaque)
> +{
> +    SBSA_GWDTState *s = SBSA_GWDT(opaque);
> +
> +    s->wcs |= SBSA_GWDT_WCS_WS0;
> +    s->ws0 = true;
> +    qemu_set_irq(s->irq, 1);
> +    sbsa_gwdt_update_timer(s, false, true);
> +}
> +
> +static void sbsa_gwdt_timer_sysreset(void *dev)
> +{
> +    SBSA_GWDTState *s = SBSA_GWDT(dev);
> +    s->wcs |= SBSA_GWDT_WCS_WS1;
> +    s->ws1 = true;
> +    qemu_log_mask(CPU_LOG_RESET, "Watchdog timer expired.\n");
> +    /*
> +     * Reset the watchdog only if the guest gets notified about
> +     * expiry. watchdog_perform_action() may temporarily relinquish
> +     * the BQL; reset before triggering the action to avoid races with
> +     * sbsa_gwdt instructions.
> +     */
> +    switch (get_watchdog_action()) {
> +    case WATCHDOG_ACTION_DEBUG:
> +    case WATCHDOG_ACTION_NONE:
> +    case WATCHDOG_ACTION_PAUSE:
> +        break;
> +    default:
> +        wdt_sbsa_gwdt_reset(dev);
> +    }
> +    watchdog_perform_action();
> +}
> +
> +static const MemoryRegionOps sbsa_gwdt_rops = {
> +    .read = sbsa_gwdt_rread,
> +    .write = sbsa_gwdt_rwrite,
> +    .endianness = DEVICE_LITTLE_ENDIAN,
> +    .valid.min_access_size = 4,
> +    .valid.max_access_size = 4,
> +    .valid.unaligned = false,
> +};
> +
> +static const MemoryRegionOps sbsa_gwdt_ops = {
> +    .read = sbsa_gwdt_read,
> +    .write = sbsa_gwdt_write,
> +    .endianness = DEVICE_LITTLE_ENDIAN,
> +    .valid.min_access_size = 4,
> +    .valid.max_access_size = 4,
> +    .valid.unaligned = false,
> +};
> +
> +static void wdt_sbsa_gwdt_realize(DeviceState *dev, Error **errp)
> +{
> +    SBSA_GWDTState *s = SBSA_GWDT(dev);
> +    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
> +
> +    memory_region_init_io(&s->rmmio, OBJECT(dev),
> +                          &sbsa_gwdt_rops, s,
> +                          "sbsa_gwdt.refresh",
> +                          SBSA_GWDT_RMMIO_SIZE);
> +
> +    memory_region_init_io(&s->cmmio, OBJECT(dev),
> +                          &sbsa_gwdt_ops, s,
> +                          "sbsa_gwdt.control",
> +                          SBSA_GWDT_CMMIO_SIZE);
> +
> +    sysbus_init_mmio(sbd, &s->rmmio);
> +    sysbus_init_mmio(sbd, &s->cmmio);
> +
> +    sysbus_init_irq(sbd, &s->irq);
> +
> +    qemu_register_reset(sbsa_gwdt_reset, s);
> +
> +    s->ptimer = timer_new_ns(QEMU_CLOCK_VIRTUAL, 
> sbsa_gwdt_timer_sysinterrupt,
> +            dev);
> +    s->timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, sbsa_gwdt_timer_sysreset,
> +                dev);
> +}
> +
> +static void wdt_sbsa_gwdt_unrealize(DeviceState *dev)
> +{
> +    SBSA_GWDTState *s = SBSA_GWDT(dev);
> +
> +    timer_del(s->ptimer);
> +    timer_free(s->ptimer);
> +
> +    timer_del(s->timer);
> +    timer_free(s->timer);
> +}
> +
> +static void wdt_sbsa_gwdt_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +
> +    dc->realize = wdt_sbsa_gwdt_realize;
> +    dc->unrealize = wdt_sbsa_gwdt_unrealize;
> +    dc->reset = wdt_sbsa_gwdt_reset;
> +    dc->hotpluggable = false;
> +    set_bit(DEVICE_CATEGORY_MISC, dc->categories);
> +    dc->vmsd = &vmstate_sbsa_gwdt;
> +}
> +
> +static const TypeInfo wdt_sbsa_gwdt_info = {
> +    .class_init = wdt_sbsa_gwdt_class_init,
> +    .parent = TYPE_SYS_BUS_DEVICE,
> +    .name  = TYPE_WDT_SBSA_GWDT,
> +    .instance_size  = sizeof(SBSA_GWDTState),
> +};
> +
> +static void wdt_sbsa_gwdt_register_types(void)
> +{
> +    watchdog_add_model(&model);
> +    type_register_static(&wdt_sbsa_gwdt_info);
> +}
> +
> +type_init(wdt_sbsa_gwdt_register_types)
> diff --git a/include/hw/watchdog/wdt_sbsa_gwdt.h 
> b/include/hw/watchdog/wdt_sbsa_gwdt.h
> new file mode 100644
> index 000000000000..8044646fa62d
> --- /dev/null
> +++ b/include/hw/watchdog/wdt_sbsa_gwdt.h
> @@ -0,0 +1,68 @@
> +#ifndef WDT_SBSA_GWDT_H
> +#define WDT_SBSA_GWDT_H
> +
> +#include "qemu/bitops.h"
> +#include "hw/sysbus.h"
> +#include "hw/irq.h"
> +
> +#define TYPE_WDT_SBSA_GWDT "sbsa_gwdt"
> +#define SBSA_GWDT(obj) \
> +    OBJECT_CHECK(SBSA_GWDTState, (obj), TYPE_WDT_SBSA_GWDT)
> +#define SBSA_GWDT_CLASS(klass) \
> +    OBJECT_CLASS_CHECK(SBSA_GWDTClass, (klass), TYPE_WDT_SBSA_GWDT)
> +#define SBSA_GWDT_GET_CLASS(obj) \
> +    OBJECT_GET_CLASS(SBSA_GWDTClass, (obj), TYPE_WDT_SBSA_GWDT)
> +
> +/* SBSA Generic Watchdog register definitions */
> +/* refresh frame */
> +#define SBSA_GWDT_WRR       0x000
> +
> +/* control frame */
> +#define SBSA_GWDT_WCS       0x000
> +#define SBSA_GWDT_WOR       0x008
> +#define SBSA_GWDT_WORU      0x00C
> +#define SBSA_GWDT_WCV       0x010
> +#define SBSA_GWDT_WCVU      0x014
> +
> +/* Watchdog Interface Identification Register */
> +#define SBSA_GWDT_W_IIDR    0xFCC
> +
> +/* Watchdog Control and Status Register Bits */
> +#define SBSA_GWDT_WCS_EN    BIT(0)
> +#define SBSA_GWDT_WCS_WS0   BIT(1)
> +#define SBSA_GWDT_WCS_WS1   BIT(2)
> +
> +#define SBSA_GWDT_WOR_MASK  0x0000FFFF
> +
> +/* Watchdog Interface Identification Register definition*/
> +#define SBSA_GWDT_ID        0x1043B
> +
> +/* 2 Separate memory regions for each of refresh & control register frames */
> +#define SBSA_GWDT_RMMIO_SIZE 0x1000
> +#define SBSA_GWDT_CMMIO_SIZE 0x1000
> +
> +typedef struct SBSA_GWDTState {
> +    /* <private> */
> +    SysBusDevice parent_obj;
> +
> +    /*< public >*/
> +    MemoryRegion rmmio;
> +    MemoryRegion cmmio;
> +    qemu_irq irq;
> +
> +    QEMUTimer *ptimer, *timer;
> +
> +    uint32_t id;
> +    uint32_t wrr;
> +    uint32_t wcs;
> +    uint32_t worl;
> +    uint32_t woru;
> +    uint32_t wcvl;
> +    uint32_t wcvu;
> +    bool enabled;
> +    bool ws0, ws1;
> +
> +    /*< public >*/
> +} SBSA_GWDTState;
> +
> +#endif /* WDT_SBSA_GWDT_H */
> --
> 2.18.4
> 
> 



reply via email to

[Prev in Thread] Current Thread [Next in Thread]