On 06/09/2011 21:33, Alexander Graf wrote:
>
>
> Am 01.09.2011 um 10:20 schrieb Fabien Chouteau <chout...@adacore.com>:
>
>> While working on the emulation of the freescale p2010 (e500v2) I realized
>> that
>> there's no implementation of booke's timers features. Currently mpc8544 uses
>> ppc_emb (ppc_emb_timers_init) which is close but not exactly like booke (for
>> example booke uses different SPR).
>>
>> Signed-off-by: Fabien Chouteau <chout...@adacore.com>
>> ---
>> Makefile.target | 2 +-
>> hw/ppc.c | 133 ++++++++--------------
>> hw/ppc.h | 25 ++++-
>> hw/ppc4xx_devs.c | 2 +-
>> hw/ppc_booke.c | 263
>> +++++++++++++++++++++++++++++++++++++++++++
>> hw/ppce500_mpc8544ds.c | 4 +-
>> hw/virtex_ml507.c | 11 +--
>> target-ppc/cpu.h | 29 +++++
>> target-ppc/translate_init.c | 43 +++++++-
>> 9 files changed, 412 insertions(+), 100 deletions(-)
>> create mode 100644 hw/ppc_booke.c
>>
>> diff --git a/Makefile.target b/Makefile.target
>> index 07af4d4..c8704cd 100644
>> --- a/Makefile.target
>> +++ b/Makefile.target
>> @@ -234,7 +234,7 @@ obj-i386-$(CONFIG_KVM) += kvmclock.o
>> obj-i386-$(CONFIG_SPICE) += qxl.o qxl-logger.o qxl-render.o
>>
>> # shared objects
>> -obj-ppc-y = ppc.o
>> +obj-ppc-y = ppc.o ppc_booke.o
>> obj-ppc-y += vga.o
>> # PREP target
>> obj-ppc-y += i8259.o mc146818rtc.o
>> diff --git a/hw/ppc.c b/hw/ppc.c
>> index 8870748..bcb1e91 100644
>> --- a/hw/ppc.c
>> +++ b/hw/ppc.c
>> @@ -50,7 +50,7 @@
>> static void cpu_ppc_tb_stop (CPUState *env);
>> static void cpu_ppc_tb_start (CPUState *env);
>>
>> -static void ppc_set_irq (CPUState *env, int n_IRQ, int level)
>> +void ppc_set_irq(CPUState *env, int n_IRQ, int level)
>> {
>> unsigned int old_pending = env->pending_interrupts;
>>
>> @@ -423,25 +423,8 @@ void ppce500_irq_init (CPUState *env)
>> }
>> /*****************************************************************************/
>> /* PowerPC time base and decrementer emulation */
>> -struct ppc_tb_t {
>> - /* Time base management */
>> - int64_t tb_offset; /* Compensation */
>> - int64_t atb_offset; /* Compensation */
>> - uint32_t tb_freq; /* TB frequency */
>> - /* Decrementer management */
>> - uint64_t decr_next; /* Tick for next decr interrupt */
>> - uint32_t decr_freq; /* decrementer frequency */
>> - struct QEMUTimer *decr_timer;
>> - /* Hypervisor decrementer management */
>> - uint64_t hdecr_next; /* Tick for next hdecr interrupt */
>> - struct QEMUTimer *hdecr_timer;
>> - uint64_t purr_load;
>> - uint64_t purr_start;
>> - void *opaque;
>> -};
>>
>> -static inline uint64_t cpu_ppc_get_tb(ppc_tb_t *tb_env, uint64_t vmclk,
>> - int64_t tb_offset)
>> +uint64_t cpu_ppc_get_tb(ppc_tb_t *tb_env, uint64_t vmclk, int64_t tb_offset)
>> {
>> /* TB time in tb periods */
>> return muldiv64(vmclk, tb_env->tb_freq, get_ticks_per_sec()) + tb_offset;
>> @@ -611,10 +594,13 @@ static inline uint32_t _cpu_ppc_load_decr(CPUState
>> *env, uint64_t next)
>> int64_t diff;
>>
>> diff = next - qemu_get_clock_ns(vm_clock);
>> - if (diff >= 0)
>> + if (diff >= 0) {
>> decr = muldiv64(diff, tb_env->decr_freq, get_ticks_per_sec());
>> - else
>> + } else if (env->insns_flags & PPC_BOOKE) {
>> + decr = 0;
>
> I don't think we should expose instruction interpreter details into the
> timing code. It'd be better to have a separate flag that gets set on the booke
> timer init function which would be stored in tb_env. Keeps things better
> separated :)
Good idea, but how do we set the flags? ppc_booke_timers_init doesn't know
which processor is emulated.
>
>> + } else {
>> decr = -muldiv64(-diff, tb_env->decr_freq, get_ticks_per_sec());
>> + }
>> LOG_TB("%s: %08" PRIx32 "\n", __func__, decr);
>>
>> return decr;
>> @@ -678,18 +664,23 @@ static void __cpu_ppc_store_decr (CPUState *env,
>> uint64_t *nextp,
>> decr, value);
>> now = qemu_get_clock_ns(vm_clock);
>> next = now + muldiv64(value, get_ticks_per_sec(), tb_env->decr_freq);
>> - if (is_excp)
>> + if (is_excp) {
>> next += *nextp - now;
>> - if (next == now)
>> + }
>> + if (next == now) {
>> next++;
>> + }
>> *nextp = next;
>> /* Adjust timer */
>> qemu_mod_timer(timer, next);
>> /* If we set a negative value and the decrementer was positive,
>> - * raise an exception.
>> + * raise an exception (not for booke).
>> */
>> - if ((value & 0x80000000) && !(decr & 0x80000000))
>> + if (!(env->insns_flags & PPC_BOOKE)
>> + && (value & 0x80000000)
>> + && !(decr & 0x80000000)) {
>> (*raise_excp)(env);
>
> Please make this a separate flag too. IIRC this is not unified behavior on
> all ppc CPUs, not even all server type ones.
>
This come from a comment by Scott (CC'd), I don't know much about it. Can you
help me to find a good name for this feature?
>> + }
>> }
>>
>> static inline void _cpu_ppc_store_decr(CPUState *env, uint32_t decr,
>> @@ -806,11 +797,11 @@ uint32_t cpu_ppc601_load_rtcl (CPUState *env)
>> }
>>
>> /*****************************************************************************/
>> -/* Embedded PowerPC timers */
>> +/* PowerPC 40x timers */
>>
>> /* PIT, FIT & WDT */
>> -typedef struct ppcemb_timer_t ppcemb_timer_t;
>> -struct ppcemb_timer_t {
>> +typedef struct ppc40x_timer_t ppc40x_timer_t;
>> +struct ppc40x_timer_t {
>> uint64_t pit_reload; /* PIT auto-reload value */
>> uint64_t fit_next; /* Tick for next FIT interrupt */
>> struct QEMUTimer *fit_timer;
>> @@ -826,12 +817,12 @@ static void cpu_4xx_fit_cb (void *opaque)
>> {
>> CPUState *env;
>> ppc_tb_t *tb_env;
>> - ppcemb_timer_t *ppcemb_timer;
>> + ppc40x_timer_t *ppc40x_timer;
>> uint64_t now, next;
>>
>> env = opaque;
>> tb_env = env->tb_env;
>> - ppcemb_timer = tb_env->opaque;
>> + ppc40x_timer = tb_env->opaque;
>> now = qemu_get_clock_ns(vm_clock);
>> switch ((env->spr[SPR_40x_TCR] >> 24) & 0x3) {
>> case 0:
>> @@ -853,7 +844,7 @@ static void cpu_4xx_fit_cb (void *opaque)
>> next = now + muldiv64(next, get_ticks_per_sec(), tb_env->tb_freq);
>> if (next == now)
>> next++;
>> - qemu_mod_timer(ppcemb_timer->fit_timer, next);
>> + qemu_mod_timer(ppc40x_timer->fit_timer, next);
>> env->spr[SPR_40x_TSR] |= 1 << 26;
>> if ((env->spr[SPR_40x_TCR] >> 23) & 0x1)
>> ppc_set_irq(env, PPC_INTERRUPT_FIT, 1);
>> @@ -865,11 +856,11 @@ static void cpu_4xx_fit_cb (void *opaque)
>> /* Programmable interval timer */
>> static void start_stop_pit (CPUState *env, ppc_tb_t *tb_env, int is_excp)
>> {
>> - ppcemb_timer_t *ppcemb_timer;
>> + ppc40x_timer_t *ppc40x_timer;
>> uint64_t now, next;
>>
>> - ppcemb_timer = tb_env->opaque;
>> - if (ppcemb_timer->pit_reload <= 1 ||
>> + ppc40x_timer = tb_env->opaque;
>> + if (ppc40x_timer->pit_reload <= 1 ||
>> !((env->spr[SPR_40x_TCR] >> 26) & 0x1) ||
>> (is_excp && !((env->spr[SPR_40x_TCR] >> 22) & 0x1))) {
>> /* Stop PIT */
>> @@ -877,9 +868,9 @@ static void start_stop_pit (CPUState *env, ppc_tb_t
>> *tb_env, int is_excp)
>> qemu_del_timer(tb_env->decr_timer);
>> } else {
>> LOG_TB("%s: start PIT %016" PRIx64 "\n",
>> - __func__, ppcemb_timer->pit_reload);
>> + __func__, ppc40x_timer->pit_reload);
>> now = qemu_get_clock_ns(vm_clock);
>> - next = now + muldiv64(ppcemb_timer->pit_reload,
>> + next = now + muldiv64(ppc40x_timer->pit_reload,
>> get_ticks_per_sec(), tb_env->decr_freq);
>> if (is_excp)
>> next += tb_env->decr_next - now;
>> @@ -894,21 +885,21 @@ static void cpu_4xx_pit_cb (void *opaque)
>> {
>> CPUState *env;
>> ppc_tb_t *tb_env;
>> - ppcemb_timer_t *ppcemb_timer;
>> + ppc40x_timer_t *ppc40x_timer;
>>
>> env = opaque;
>> tb_env = env->tb_env;
>> - ppcemb_timer = tb_env->opaque;
>> + ppc40x_timer = tb_env->opaque;
>> env->spr[SPR_40x_TSR] |= 1 << 27;
>> if ((env->spr[SPR_40x_TCR] >> 26) & 0x1)
>> - ppc_set_irq(env, ppcemb_timer->decr_excp, 1);
>> + ppc_set_irq(env, ppc40x_timer->decr_excp, 1);
>> start_stop_pit(env, tb_env, 1);
>> LOG_TB("%s: ar %d ir %d TCR " TARGET_FMT_lx " TSR " TARGET_FMT_lx " "
>> "%016" PRIx64 "\n", __func__,
>> (int)((env->spr[SPR_40x_TCR] >> 22) & 0x1),
>> (int)((env->spr[SPR_40x_TCR] >> 26) & 0x1),
>> env->spr[SPR_40x_TCR], env->spr[SPR_40x_TSR],
>> - ppcemb_timer->pit_reload);
>> + ppc40x_timer->pit_reload);
>> }
>>
>> /* Watchdog timer */
>> @@ -916,12 +907,12 @@ static void cpu_4xx_wdt_cb (void *opaque)
>> {
>> CPUState *env;
>> ppc_tb_t *tb_env;
>> - ppcemb_timer_t *ppcemb_timer;
>> + ppc40x_timer_t *ppc40x_timer;
>> uint64_t now, next;
>>
>> env = opaque;
>> tb_env = env->tb_env;
>> - ppcemb_timer = tb_env->opaque;
>> + ppc40x_timer = tb_env->opaque;
>> now = qemu_get_clock_ns(vm_clock);
>> switch ((env->spr[SPR_40x_TCR] >> 30) & 0x3) {
>> case 0:
>> @@ -948,13 +939,13 @@ static void cpu_4xx_wdt_cb (void *opaque)
>> switch ((env->spr[SPR_40x_TSR] >> 30) & 0x3) {
>> case 0x0:
>> case 0x1:
>> - qemu_mod_timer(ppcemb_timer->wdt_timer, next);
>> - ppcemb_timer->wdt_next = next;
>> + qemu_mod_timer(ppc40x_timer->wdt_timer, next);
>> + ppc40x_timer->wdt_next = next;
>> env->spr[SPR_40x_TSR] |= 1 << 31;
>> break;
>> case 0x2:
>> - qemu_mod_timer(ppcemb_timer->wdt_timer, next);
>> - ppcemb_timer->wdt_next = next;
>> + qemu_mod_timer(ppc40x_timer->wdt_timer, next);
>> + ppc40x_timer->wdt_next = next;
>> env->spr[SPR_40x_TSR] |= 1 << 30;
>> if ((env->spr[SPR_40x_TCR] >> 27) & 0x1)
>> ppc_set_irq(env, PPC_INTERRUPT_WDT, 1);
>> @@ -982,12 +973,12 @@ static void cpu_4xx_wdt_cb (void *opaque)
>> void store_40x_pit (CPUState *env, target_ulong val)
>> {
>> ppc_tb_t *tb_env;
>> - ppcemb_timer_t *ppcemb_timer;
>> + ppc40x_timer_t *ppc40x_timer;
>>
>> tb_env = env->tb_env;
>> - ppcemb_timer = tb_env->opaque;
>> + ppc40x_timer = tb_env->opaque;
>> LOG_TB("%s val" TARGET_FMT_lx "\n", __func__, val);
>> - ppcemb_timer->pit_reload = val;
>> + ppc40x_timer->pit_reload = val;
>> start_stop_pit(env, tb_env, 0);
>> }
>>
>> @@ -996,31 +987,7 @@ target_ulong load_40x_pit (CPUState *env)
>> return cpu_ppc_load_decr(env);
>> }
>>
>> -void store_booke_tsr (CPUState *env, target_ulong val)
>> -{
>> - ppc_tb_t *tb_env = env->tb_env;
>> - ppcemb_timer_t *ppcemb_timer;
>> -
>> - ppcemb_timer = tb_env->opaque;
>> -
>> - LOG_TB("%s: val " TARGET_FMT_lx "\n", __func__, val);
>> - env->spr[SPR_40x_TSR] &= ~(val & 0xFC000000);
>> - if (val & 0x80000000)
>> - ppc_set_irq(env, ppcemb_timer->decr_excp, 0);
>> -}
>> -
>> -void store_booke_tcr (CPUState *env, target_ulong val)
>> -{
>> - ppc_tb_t *tb_env;
>> -
>> - tb_env = env->tb_env;
>> - LOG_TB("%s: val " TARGET_FMT_lx "\n", __func__, val);
>> - env->spr[SPR_40x_TCR] = val & 0xFFC00000;
>> - start_stop_pit(env, tb_env, 1);
>> - cpu_4xx_wdt_cb(env);
>> -}
>> -
>> -static void ppc_emb_set_tb_clk (void *opaque, uint32_t freq)
>> +static void ppc_40x_set_tb_clk (void *opaque, uint32_t freq)
>> {
>> CPUState *env = opaque;
>> ppc_tb_t *tb_env = env->tb_env;
>> @@ -1032,30 +999,30 @@ static void ppc_emb_set_tb_clk (void *opaque,
>> uint32_t freq)
>> /* XXX: we should also update all timers */
>> }
>>
>> -clk_setup_cb ppc_emb_timers_init (CPUState *env, uint32_t freq,
>> +clk_setup_cb ppc_40x_timers_init (CPUState *env, uint32_t freq,
>> unsigned int decr_excp)
>> {
>> ppc_tb_t *tb_env;
>> - ppcemb_timer_t *ppcemb_timer;
>> + ppc40x_timer_t *ppc40x_timer;
>>
>> tb_env = g_malloc0(sizeof(ppc_tb_t));
>> env->tb_env = tb_env;
>> - ppcemb_timer = g_malloc0(sizeof(ppcemb_timer_t));
>> + ppc40x_timer = g_malloc0(sizeof(ppc40x_timer_t));
>> tb_env->tb_freq = freq;
>> tb_env->decr_freq = freq;
>> - tb_env->opaque = ppcemb_timer;
>> + tb_env->opaque = ppc40x_timer;
>> LOG_TB("%s freq %" PRIu32 "\n", __func__, freq);
>> - if (ppcemb_timer != NULL) {
>> + if (ppc40x_timer != NULL) {
>> /* We use decr timer for PIT */
>> tb_env->decr_timer = qemu_new_timer_ns(vm_clock, &cpu_4xx_pit_cb,
>> env);
>> - ppcemb_timer->fit_timer =
>> + ppc40x_timer->fit_timer =
>> qemu_new_timer_ns(vm_clock, &cpu_4xx_fit_cb, env);
>> - ppcemb_timer->wdt_timer =
>> + ppc40x_timer->wdt_timer =
>> qemu_new_timer_ns(vm_clock, &cpu_4xx_wdt_cb, env);
>> - ppcemb_timer->decr_excp = decr_excp;
>> + ppc40x_timer->decr_excp = decr_excp;
>> }
>>
>> - return &ppc_emb_set_tb_clk;
>> + return &ppc_40x_set_tb_clk;
>> }
>>
>> /*****************************************************************************/
>> diff --git a/hw/ppc.h b/hw/ppc.h
>> index 3ccf134..16df16a 100644
>> --- a/hw/ppc.h
>> +++ b/hw/ppc.h
>> @@ -1,3 +1,5 @@
>> +void ppc_set_irq (CPUState *env, int n_IRQ, int level);
>> +
>> /* PowerPC hardware exceptions management helpers */
>> typedef void (*clk_setup_cb)(void *opaque, uint32_t freq);
>> typedef struct clk_setup_t clk_setup_t;
>> @@ -11,6 +13,24 @@ static inline void clk_setup (clk_setup_t *clk, uint32_t
>> freq)
>> (*clk->cb)(clk->opaque, freq);
>> }
>>
>> +struct ppc_tb_t {
>> + /* Time base management */
>> + int64_t tb_offset; /* Compensation */
>> + int64_t atb_offset; /* Compensation */
>> + uint32_t tb_freq; /* TB frequency */
>> + /* Decrementer management */
>> + uint64_t decr_next; /* Tick for next decr interrupt */
>> + uint32_t decr_freq; /* decrementer frequency */
>> + struct QEMUTimer *decr_timer;
>> + /* Hypervisor decrementer management */
>> + uint64_t hdecr_next; /* Tick for next hdecr interrupt */
>> + struct QEMUTimer *hdecr_timer;
>> + uint64_t purr_load;
>> + uint64_t purr_start;
>> + void *opaque;
>> +};
>> +
>> +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 (CPUState *env, uint32_t freq);
>> /* Embedded PowerPC DCR management */
>> typedef uint32_t (*dcr_read_cb)(void *opaque, int dcrn);
>> @@ -19,7 +39,7 @@ int ppc_dcr_init (CPUState *env, int (*dcr_read_error)(int
>> dcrn),
>> int (*dcr_write_error)(int dcrn));
>> int ppc_dcr_register (CPUState *env, int dcrn, void *opaque,
>> dcr_read_cb drc_read, dcr_write_cb dcr_write);
>> -clk_setup_cb ppc_emb_timers_init (CPUState *env, uint32_t freq,
>> +clk_setup_cb ppc_40x_timers_init (CPUState *env, uint32_t freq,
>> unsigned int decr_excp);
>>
>> /* Embedded PowerPC reset */
>> @@ -55,3 +75,6 @@ enum {
>> #define FW_CFG_PPC_KVM_PID (FW_CFG_ARCH_LOCAL + 0x07)
>>
>> #define PPC_SERIAL_MM_BAUDBASE 399193
>> +
>> +/* ppc_booke.c */
>> +void ppc_booke_timers_init (CPUState *env, uint32_t freq);
>> diff --git a/hw/ppc4xx_devs.c b/hw/ppc4xx_devs.c
>> index 349f046..d18caa4 100644
>> --- a/hw/ppc4xx_devs.c
>> +++ b/hw/ppc4xx_devs.c
>> @@ -56,7 +56,7 @@ CPUState *ppc4xx_init (const char *cpu_model,
>> cpu_clk->cb = NULL; /* We don't care about CPU clock frequency changes */
>> cpu_clk->opaque = env;
>> /* Set time-base frequency to sysclk */
>> - tb_clk->cb = ppc_emb_timers_init(env, sysclk, PPC_INTERRUPT_PIT);
>> + tb_clk->cb = ppc_40x_timers_init(env, sysclk, PPC_INTERRUPT_PIT);
>> tb_clk->opaque = env;
>> ppc_dcr_init(env, NULL, NULL);
>> /* Register qemu callbacks */
>> diff --git a/hw/ppc_booke.c b/hw/ppc_booke.c
>> new file mode 100644
>> index 0000000..35f11ca
>> --- /dev/null
>> +++ b/hw/ppc_booke.c
>> @@ -0,0 +1,263 @@
>> +/*
>> + * QEMU PowerPC Booke hardware System Emulator
>> + *
>> + * Copyright (c) 2011 AdaCore
>> + *
>> + * Permission is hereby granted, free of charge, to any person obtaining a
>> copy
>> + * of this software and associated documentation files (the "Software"), to
>> deal
>> + * in the Software without restriction, including without limitation the
>> rights
>> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
>> + * copies of the Software, and to permit persons to whom the Software is
>> + * furnished to do so, subject to the following conditions:
>> + *
>> + * The above copyright notice and this permission notice shall be included
>> in
>> + * all copies or substantial portions of the Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
>> OR
>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
>> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
>> OTHER
>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
>> FROM,
>> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
>> + * THE SOFTWARE.
>> + */
>> +#include "hw.h"
>> +#include "ppc.h"
>> +#include "qemu-timer.h"
>> +#include "sysemu.h"
>> +#include "nvram.h"
>> +#include "qemu-log.h"
>> +#include "loader.h"
>> +
>> +
>> +/* Timer Control Register */
>> +
>> +#define TCR_WP_SHIFT 30 /* Watchdog Timer Period */
>> +#define TCR_WP_MASK (0x3 << TCR_WP_SHIFT)
>> +#define TCR_WRC_SHIFT 28 /* Watchdog Timer Reset Control */
>> +#define TCR_WRC_MASK (0x3 << TCR_WRC_SHIFT)
>> +#define TCR_WIE (1 << 27) /* Watchdog Timer Interrupt Enable */
>> +#define TCR_DIE (1 << 26) /* Decrementer Interrupt Enable */
>> +#define TCR_FP_SHIFT 24 /* Fixed-Interval Timer Period */
>> +#define TCR_FP_MASK (0x3 << TCR_FP_SHIFT)
>> +#define TCR_FIE (1 << 23) /* Fixed-Interval Timer Interrupt Enable */
>> +#define TCR_ARE (1 << 22) /* Auto-Reload Enable */
>> +
>> +/* Timer Control Register (e500 specific fields) */
>> +
>> +#define TCR_E500_FPEXT_SHIFT 13 /* Fixed-Interval Timer Period Extension */
>> +#define TCR_E500_FPEXT_MASK (0xf << TCR_E500_FPEXT_SHIFT)
>> +#define TCR_E500_WPEXT_SHIFT 17 /* Watchdog Timer Period Extension */
>> +#define TCR_E500_WPEXT_MASK (0xf << TCR_E500_WPEXT_SHIFT)
>> +
>> +/* Timer Status Register */
>> +
>> +#define TSR_FIS (1 << 26) /* Fixed-Interval Timer Interrupt Status */
>> +#define TSR_DIS (1 << 27) /* Decrementer Interrupt Status */
>> +#define TSR_WRS_SHIFT 28 /* Watchdog Timer Reset Status */
>> +#define TSR_WRS_MASK (0x3 << TSR_WRS_SHIFT)
>> +#define TSR_WIS (1 << 30) /* Watchdog Timer Interrupt Status */
>> +#define TSR_ENW (1 << 31) /* Enable Next Watchdog Timer */
>> +
>> +typedef struct booke_timer_t booke_timer_t;
>> +struct booke_timer_t {
>> +
>> + uint64_t fit_next;
>> + struct QEMUTimer *fit_timer;
>> +
>> + uint64_t wdt_next;
>> + struct QEMUTimer *wdt_timer;
>> +};
>> +
>> +/* Return the location of the bit of time base at which the FIT will raise
>> an
>> + interrupt */
>> +static uint8_t booke_get_fit_target(CPUState *env)
>> +{
>> + uint8_t fp = (env->spr[SPR_BOOKE_TCR] & TCR_FP_MASK) >> TCR_FP_SHIFT;
>> +
>> + /* Only for e500 */
>> + if (env->insns_flags2 & PPC2_E500) {
>
> Same here again :). Do you have test cases for fit? IIRC Linux doesn't use it.
VxWorks uses it.
>
>> + uint32_t fpext = (env->spr[SPR_BOOKE_TCR] & TCR_E500_FPEXT_MASK)
>> + >> TCR_E500_FPEXT_SHIFT;
>> + fp = 63 - (fp | fpext << 2);
>> + } else {
>> + fp = env->fit_period[fp];
>> + }
>> +
>> + return fp;
>> +}
>> +
>> +/* Return the location of the bit of time base at which the WDT will raise
>> an
>> + interrupt */
>> +static uint8_t booke_get_wdt_target(CPUState *env)
>> +{
>> + uint8_t wp = (env->spr[SPR_BOOKE_TCR] & TCR_WP_MASK) >> TCR_WP_SHIFT;
>> +
>> + /* Only for e500 */
>> + if (env->insns_flags2 & PPC2_E500) {
>> + uint32_t wpext = (env->spr[SPR_BOOKE_TCR] & TCR_E500_WPEXT_MASK)
>> + >> TCR_E500_WPEXT_SHIFT;
>> + wp = 63 - (wp | wpext << 2);
>> + } else {
>> + wp = env->wdt_period[wp];
>> + }
>> +
>> + return wp;
>> +}
>> +
>> +static void booke_update_fixed_timer(CPUState *env,
>> + uint8_t target_bit,
>> + uint64_t *next,
>> + struct QEMUTimer *timer)
>> +{
>> + ppc_tb_t *tb_env = env->tb_env;
>> + uint64_t lapse;
>> + uint64_t tb;
>> + uint64_t period = 1 << (target_bit + 1);
>> + uint64_t now;
>> +
>> + now = qemu_get_clock_ns(vm_clock);
>> + tb = cpu_ppc_get_tb(tb_env, now, tb_env->tb_offset);
>> +
>> + lapse = period - ((tb - (1 << target_bit)) & (period - 1));
>> +
>> + *next = now + muldiv64(lapse, get_ticks_per_sec(), tb_env->tb_freq);
>> +
>> + if (*next == now) {
>> + (*next)++;
>
> Huh? If we hit the timer we don't fire it?
Yes we do, but one nanosecond later :)
>
>> + }
>> +
>> + qemu_mod_timer(timer, *next);
>> +}
>> +
>> +static void booke_decr_cb(void *opaque)
>> +{
>> + CPUState *env;
>> + ppc_tb_t *tb_env;
>> + booke_timer_t *booke_timer;
>> +
>> + env = opaque;
>> + tb_env = env->tb_env;
>> + booke_timer = tb_env->opaque;
>> + env->spr[SPR_BOOKE_TSR] |= TSR_DIS;
>> + if (env->spr[SPR_BOOKE_TCR] & TCR_DIE) {
>> + ppc_set_irq(env, PPC_INTERRUPT_DECR, 1);
>> + }
>
> You will need this more often - also for the TCR write case - so please put
> the 3 lines above into a separate function and just call that here :)
Actually the checks are never exactly the same, here we test DIE in TCR...
>> +
>> + if (env->spr[SPR_BOOKE_TCR] & TCR_ARE) {
>> + /* Auto Reload */
>> + cpu_ppc_store_decr(env, env->spr[SPR_BOOKE_DECAR]);
>> + }
>
> Ah nice - never seen this one used. Do you have a test case?
>
VxWorks :)
>> +}
>> +
>> +static void booke_fit_cb(void *opaque)
>> +{
>> + CPUState *env;
>> + ppc_tb_t *tb_env;
>> + booke_timer_t *booke_timer;
>> +
>> + env = opaque;
>> + tb_env = env->tb_env;
>> + booke_timer = tb_env->opaque;
>> + env->spr[SPR_BOOKE_TSR] |= TSR_FIS;
>> + if (env->spr[SPR_BOOKE_TCR] & TCR_FIE) {
>> + ppc_set_irq(env, PPC_INTERRUPT_FIT, 1);
>> + }
>
> Same as above :)
>
>> +
>> + booke_update_fixed_timer(env,
>> + booke_get_fit_target(env),
>> + &booke_timer->fit_next,
>> + booke_timer->fit_timer);
>> +}
>> +
>> +static void booke_wdt_cb(void *opaque)
>> +{
>> + CPUState *env;
>> + ppc_tb_t *tb_env;
>> + booke_timer_t *booke_timer;
>> +
>> + env = opaque;
>> + tb_env = env->tb_env;
>> + booke_timer = tb_env->opaque;
>> +
>> + /* TODO: There's lots of complicated stuff to do here */
>> +
>> + booke_update_fixed_timer(env,
>> + booke_get_wdt_target(env),
>> + &booke_timer->wdt_next,
>> + booke_timer->wdt_timer);
>> +}
>> +
>> +void store_booke_tsr(CPUState *env, target_ulong val)
>> +{
>> + ppc_tb_t *tb_env = env->tb_env;
>> + booke_timer_t *booke_timer;
>> +
>> + booke_timer = tb_env->opaque;
>> +
>> + env->spr[SPR_BOOKE_TSR] &= ~val;
>> +
>> + if (val & TSR_DIS) {
>> + ppc_set_irq(env, PPC_INTERRUPT_DECR, 0);
>> + }
>> +
>> + if (val & TSR_FIS) {
>> + ppc_set_irq(env, PPC_INTERRUPT_FIT, 0);
>> + }
>> +
>> + if (val & TSR_WIS) {
>> + ppc_set_irq(env, PPC_INTERRUPT_WDT, 0);
>> + }
>
> Why do you need the above? Shouldn't they still be active if the guest didn't
> reset the bit? This is probably hiding a bug in the interrupt delivery
> mechanism automatically unmasking an irq bit when it's delivered, right?
They are active until the bit is cleared by user, and TSR is a write-1-to-clear
register.
>> +}
>> +
>> +void store_booke_tcr(CPUState *env, target_ulong val)
>> +{
>> + ppc_tb_t *tb_env = env->tb_env;
>> + booke_timer_t *booke_timer = tb_env->opaque;
>> +
>> + tb_env = env->tb_env;
>> + env->spr[SPR_BOOKE_TCR] = val;
>> +
>> + booke_update_fixed_timer(env,
>> + booke_get_fit_target(env),
>> + &booke_timer->fit_next,
>> + booke_timer->fit_timer);
>> +
>> + booke_update_fixed_timer(env,
>> + booke_get_wdt_target(env),
>> + &booke_timer->wdt_next,
>> + booke_timer->wdt_timer);
>> +
>> + if (val & TCR_DIE && env->spr[SPR_BOOKE_TSR] & TSR_DIS) {
>> + ppc_set_irq(env, PPC_INTERRUPT_DECR, 1);
>> + }
>> +
>> + if (val & TCR_FIE && env->spr[SPR_BOOKE_TSR] & TSR_FIS) {
>> + ppc_set_irq(env, PPC_INTERRUPT_FIT, 1);
>> + }
>> +
>> + if (val & TCR_WIE && env->spr[SPR_BOOKE_TSR] & TSR_WIS) {
>> + ppc_set_irq(env, PPC_INTERRUPT_WDT, 1);
>> + }
>
> Here is the second user of the checks. It really does make sense to only have
> them in a single spot, so that ever ppc_set_irq(DECR) always goes through the
> TSR and TCR checks for example :).
...here we test DIE in TCR and DIS in TSR.
>> +}
>> +
>> +void ppc_booke_timers_init(CPUState *env, uint32_t freq)
>> +{
>> + ppc_tb_t *tb_env;
>> + booke_timer_t *booke_timer;
>> +
>> + tb_env = g_malloc0(sizeof(ppc_tb_t));
>> + booke_timer = g_malloc0(sizeof(booke_timer_t));
>> +
>> + env->tb_env = tb_env;
>> +
>> + tb_env->tb_freq = freq;
>> + tb_env->decr_freq = freq;
>> + tb_env->opaque = booke_timer;
>> + tb_env->decr_timer = qemu_new_timer_ns(vm_clock, &booke_decr_cb, env);
>> +
>> + booke_timer->fit_timer =
>> + qemu_new_timer_ns(vm_clock, &booke_fit_cb, env);
>> + booke_timer->wdt_timer =
>> + qemu_new_timer_ns(vm_clock, &booke_wdt_cb, env);
>> +}
>> diff --git a/hw/ppce500_mpc8544ds.c b/hw/ppce500_mpc8544ds.c
>> index 1274a3e..b8aa0d0 100644
>> --- a/hw/ppce500_mpc8544ds.c
>> +++ b/hw/ppce500_mpc8544ds.c
>> @@ -252,9 +252,7 @@ static void mpc8544ds_init(ram_addr_t ram_size,
>> exit(1);
>> }
>>
>> - /* XXX register timer? */
>> - ppc_emb_timers_init(env, 400000000, PPC_INTERRUPT_DECR);
>> - ppc_dcr_init(env, NULL, NULL);
>> + ppc_booke_timers_init(env, 400000000);
>>
>> /* Register reset handler */
>> qemu_register_reset(mpc8544ds_cpu_reset, env);
>> diff --git a/hw/virtex_ml507.c b/hw/virtex_ml507.c
>> index 333050c..dccaea3 100644
>> --- a/hw/virtex_ml507.c
>> +++ b/hw/virtex_ml507.c
>> @@ -81,7 +81,6 @@ static void mmubooke_create_initial_mapping(CPUState *env,
>> static CPUState *ppc440_init_xilinx(ram_addr_t *ram_size,
>> int do_init,
>> const char *cpu_model,
>> - clk_setup_t *cpu_clk, clk_setup_t
>> *tb_clk,
>> uint32_t sysclk)
>> {
>> CPUState *env;
>> @@ -93,11 +92,7 @@ static CPUState *ppc440_init_xilinx(ram_addr_t *ram_size,
>> exit(1);
>> }
>>
>> - cpu_clk->cb = NULL; /* We don't care about CPU clock frequency changes
>> */
>> - cpu_clk->opaque = env;
>> - /* Set time-base frequency to sysclk */
>> - tb_clk->cb = ppc_emb_timers_init(env, sysclk, PPC_INTERRUPT_DECR);
>> - tb_clk->opaque = env;
>> + ppc_booke_timers_init(env, sysclk);
>>
>> ppc_dcr_init(env, NULL, NULL);
>>
>> @@ -198,7 +193,6 @@ static void virtex_init(ram_addr_t ram_size,
>> ram_addr_t phys_ram;
>> ram_addr_t phys_flash;
>> qemu_irq irq[32], *cpu_irq;
>> - clk_setup_t clk_setup[7];
>> int kernel_size;
>> int i;
>>
>> @@ -208,8 +202,7 @@ static void virtex_init(ram_addr_t ram_size,
>> }
>>
>> memset(clk_setup, 0, sizeof(clk_setup));
>> - env = ppc440_init_xilinx(&ram_size, 1, cpu_model, &clk_setup[0],
>> - &clk_setup[1], 400000000);
>> + env = ppc440_init_xilinx(&ram_size, 1, cpu_model, 400000000);
>> qemu_register_reset(main_cpu_reset, env);
>>
>> phys_ram = qemu_ram_alloc(NULL, "ram", ram_size);
>> diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
>> index b8d42e0..be0d79c 100644
>> --- a/target-ppc/cpu.h
>> +++ b/target-ppc/cpu.h
>> @@ -1010,8 +1010,35 @@ struct CPUPPCState {
>> #if !defined(CONFIG_USER_ONLY)
>> void *load_info; /* Holds boot loading state. */
>> #endif
>> +
>> + /* booke timers */
>> +
>> + /* Specifies bit locations of the Time Base used to signal a fixed timer
>> + * exception on a transition from 0 to 1. (watchdog or fixed-interval
>> timer)
>> + *
>> + * 0 selects the least significant bit.
>> + * 63 selects the most significant bit.
>> + */
>> + uint8_t fit_period[4];
>> + uint8_t wdt_period[4];
>> };
>>
>> +#define SET_FIT_PERIOD(a_, b_, c_, d_) \
>> +do { \
>> + env->fit_period[0] = (a_); \
>> + env->fit_period[1] = (b_); \
>> + env->fit_period[2] = (c_); \
>> + env->fit_period[3] = (d_); \
>> + } while (0)
>> +
>> +#define SET_WDT_PERIOD(a_, b_, c_, d_) \
>> +do { \
>> + env->wdt_period[0] = (a_); \
>> + env->wdt_period[1] = (b_); \
>> + env->wdt_period[2] = (c_); \
>> + env->wdt_period[3] = (d_); \
>> + } while (0)
>> +
>> #if !defined(CONFIG_USER_ONLY)
>> /* Context used internally during MMU translations */
>> typedef struct mmu_ctx_t mmu_ctx_t;
>> @@ -1806,6 +1833,8 @@ enum {
>>
>> /* BookE 2.06 PowerPC specification
>> */
>> PPC2_BOOKE206 = 0x0000000000000001ULL,
>> + /* e500 support
>> */
>> + PPC2_E500 = 0x0000000000000002ULL,
>
> I don't think we should have an e500 inst target. It should be enough to have
> an SPE inst target and specific SPR init functions for e500, no? Keep in mind
> that these flags are only meant to be used by the instruction interpreter.
Yes sure, it was the easy way to implement e500 features in the timers, but now
I will remove it.
> Very nice patch however! Thanks a lot for sitting down and fixing the timer
> mess on booke that we currently have.
You are welcome, It's my thank you gift for the MMU ;)
> Since you definitely know your way around booke timekeeping code, could you
> please also take a look at the decr patches on kvm-ppc@vger that Liu posted
> recently?
I don't know anything about kvm but I can take a look. Do you have the name of
this patch?
Regards,
--
Fabien Chouteau
