From 84597ff39484ec171567c7c80061100eb4a6c331 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Thu, 12 May 2022 16:14:55 +0100 Subject: hw/intc/arm_gicv3: Support configurable number of physical priority bits The GICv3 code has always supported a configurable number of virtual priority and preemption bits, but our implementation currently hardcodes the number of physical priority bits at 8. This is not what most hardware implementations provide; for instance the Cortex-A53 provides only 5 bits of physical priority. Make the number of physical priority/preemption bits driven by fields in the GICv3CPUState, the way that we already do for virtual priority/preemption bits. We set cs->pribits to 8, so there is no behavioural change in this commit. A following commit will add the machinery for CPUs to set this to the correct value for their implementation. Note that changing the number of priority bits would be a migration compatibility break, because the semantics of the icc_apr[][] array changes. Signed-off-by: Peter Maydell Reviewed-by: Richard Henderson Message-id: 20220512151457.3899052-5-peter.maydell@linaro.org Message-id: 20220506162129.2896966-4-peter.maydell@linaro.org --- include/hw/intc/arm_gicv3_common.h | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) (limited to 'include') diff --git a/include/hw/intc/arm_gicv3_common.h b/include/hw/intc/arm_gicv3_common.h index 4e41610055..46677ec345 100644 --- a/include/hw/intc/arm_gicv3_common.h +++ b/include/hw/intc/arm_gicv3_common.h @@ -51,11 +51,6 @@ /* Maximum number of list registers (architectural limit) */ #define GICV3_LR_MAX 16 -/* Minimum BPR for Secure, or when security not enabled */ -#define GIC_MIN_BPR 0 -/* Minimum BPR for Nonsecure when security is enabled */ -#define GIC_MIN_BPR_NS (GIC_MIN_BPR + 1) - /* For some distributor fields we want to model the array of 32-bit * register values which hold various bitmaps corresponding to enabled, * pending, etc bits. These macros and functions facilitate that; the @@ -206,6 +201,8 @@ struct GICv3CPUState { int num_list_regs; int vpribits; /* number of virtual priority bits */ int vprebits; /* number of virtual preemption bits */ + int pribits; /* number of physical priority bits */ + int prebits; /* number of physical preemption bits */ /* Current highest priority pending interrupt for this CPU. * This is cached information that can be recalculated from the -- cgit v1.2.3-55-g7522 From 39f29e599355f9512482b67624e7a6c9000c5ddd Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Thu, 12 May 2022 16:14:56 +0100 Subject: hw/intc/arm_gicv3: Use correct number of priority bits for the CPU Make the GICv3 set its number of bits of physical priority from the implementation-specific value provided in the CPU state struct, in the same way we already do for virtual priority bits. Because this would be a migration compatibility break, we provide a property force-8-bit-prio which is enabled for 7.0 and earlier versioned board models to retain the legacy "always use 8 bits" behaviour. Signed-off-by: Peter Maydell Reviewed-by: Richard Henderson Message-id: 20220512151457.3899052-6-peter.maydell@linaro.org Message-id: 20220506162129.2896966-5-peter.maydell@linaro.org --- hw/core/machine.c | 4 +++- hw/intc/arm_gicv3_common.c | 5 +++++ hw/intc/arm_gicv3_cpuif.c | 15 +++++++++++---- include/hw/intc/arm_gicv3_common.h | 1 + target/arm/cpu.h | 1 + target/arm/cpu64.c | 6 ++++++ 6 files changed, 27 insertions(+), 5 deletions(-) (limited to 'include') diff --git a/hw/core/machine.c b/hw/core/machine.c index b03d9192ba..bb0dc8f6a9 100644 --- a/hw/core/machine.c +++ b/hw/core/machine.c @@ -41,7 +41,9 @@ #include "hw/virtio/virtio-pci.h" #include "qom/object_interfaces.h" -GlobalProperty hw_compat_7_0[] = {}; +GlobalProperty hw_compat_7_0[] = { + { "arm-gicv3-common", "force-8-bit-prio", "on" }, +}; const size_t hw_compat_7_0_len = G_N_ELEMENTS(hw_compat_7_0); GlobalProperty hw_compat_6_2[] = { diff --git a/hw/intc/arm_gicv3_common.c b/hw/intc/arm_gicv3_common.c index 5634c6fc78..351843db4a 100644 --- a/hw/intc/arm_gicv3_common.c +++ b/hw/intc/arm_gicv3_common.c @@ -563,6 +563,11 @@ static Property arm_gicv3_common_properties[] = { DEFINE_PROP_UINT32("revision", GICv3State, revision, 3), DEFINE_PROP_BOOL("has-lpi", GICv3State, lpi_enable, 0), DEFINE_PROP_BOOL("has-security-extensions", GICv3State, security_extn, 0), + /* + * Compatibility property: force 8 bits of physical priority, even + * if the CPU being emulated should have fewer. + */ + DEFINE_PROP_BOOL("force-8-bit-prio", GICv3State, force_8bit_prio, 0), DEFINE_PROP_ARRAY("redist-region-count", GICv3State, nb_redist_regions, redist_region_count, qdev_prop_uint32, uint32_t), DEFINE_PROP_LINK("sysmem", GICv3State, dma, TYPE_MEMORY_REGION, diff --git a/hw/intc/arm_gicv3_cpuif.c b/hw/intc/arm_gicv3_cpuif.c index 69a15f7a44..66e06b787c 100644 --- a/hw/intc/arm_gicv3_cpuif.c +++ b/hw/intc/arm_gicv3_cpuif.c @@ -2798,6 +2798,7 @@ void gicv3_init_cpuif(GICv3State *s) * cpu->gic_num_lrs * cpu->gic_vpribits * cpu->gic_vprebits + * cpu->gic_pribits */ /* Note that we can't just use the GICv3CPUState as an opaque pointer @@ -2810,11 +2811,17 @@ void gicv3_init_cpuif(GICv3State *s) define_arm_cp_regs(cpu, gicv3_cpuif_reginfo); /* - * For the moment, retain the existing behaviour of 8 priority bits; - * in a following commit we will take this from the CPU state, - * as we do for the virtual priority bits. + * The CPU implementation specifies the number of supported + * bits of physical priority. For backwards compatibility + * of migration, we have a compat property that forces use + * of 8 priority bits regardless of what the CPU really has. */ - cs->pribits = 8; + if (s->force_8bit_prio) { + cs->pribits = 8; + } else { + cs->pribits = cpu->gic_pribits ?: 5; + } + /* * The GICv3 has separate ID register fields for virtual priority * and preemption bit values, but only a single ID register field diff --git a/include/hw/intc/arm_gicv3_common.h b/include/hw/intc/arm_gicv3_common.h index 46677ec345..ab5182a28a 100644 --- a/include/hw/intc/arm_gicv3_common.h +++ b/include/hw/intc/arm_gicv3_common.h @@ -248,6 +248,7 @@ struct GICv3State { uint32_t revision; bool lpi_enable; bool security_extn; + bool force_8bit_prio; bool irq_reset_nonsecure; bool gicd_no_migration_shift_bug; diff --git a/target/arm/cpu.h b/target/arm/cpu.h index a99b430e54..a42464eb57 100644 --- a/target/arm/cpu.h +++ b/target/arm/cpu.h @@ -1002,6 +1002,7 @@ struct ArchCPU { int gic_num_lrs; /* number of list registers */ int gic_vpribits; /* number of virtual priority bits */ int gic_vprebits; /* number of virtual preemption bits */ + int gic_pribits; /* number of physical priority bits */ /* Whether the cfgend input is high (i.e. this CPU should reset into * big-endian mode). This setting isn't used directly: instead it modifies diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c index 804a54922c..7628f4fa39 100644 --- a/target/arm/cpu64.c +++ b/target/arm/cpu64.c @@ -87,6 +87,7 @@ static void aarch64_a57_initfn(Object *obj) cpu->gic_num_lrs = 4; cpu->gic_vpribits = 5; cpu->gic_vprebits = 5; + cpu->gic_pribits = 5; define_cortex_a72_a57_a53_cp_reginfo(cpu); } @@ -140,6 +141,7 @@ static void aarch64_a53_initfn(Object *obj) cpu->gic_num_lrs = 4; cpu->gic_vpribits = 5; cpu->gic_vprebits = 5; + cpu->gic_pribits = 5; define_cortex_a72_a57_a53_cp_reginfo(cpu); } @@ -191,6 +193,7 @@ static void aarch64_a72_initfn(Object *obj) cpu->gic_num_lrs = 4; cpu->gic_vpribits = 5; cpu->gic_vprebits = 5; + cpu->gic_pribits = 5; define_cortex_a72_a57_a53_cp_reginfo(cpu); } @@ -252,6 +255,7 @@ static void aarch64_a76_initfn(Object *obj) cpu->gic_num_lrs = 4; cpu->gic_vpribits = 5; cpu->gic_vprebits = 5; + cpu->gic_pribits = 5; /* From B5.1 AdvSIMD AArch64 register summary */ cpu->isar.mvfr0 = 0x10110222; @@ -317,6 +321,7 @@ static void aarch64_neoverse_n1_initfn(Object *obj) cpu->gic_num_lrs = 4; cpu->gic_vpribits = 5; cpu->gic_vprebits = 5; + cpu->gic_pribits = 5; /* From B5.1 AdvSIMD AArch64 register summary */ cpu->isar.mvfr0 = 0x10110222; @@ -1008,6 +1013,7 @@ static void aarch64_a64fx_initfn(Object *obj) cpu->gic_num_lrs = 4; cpu->gic_vpribits = 5; cpu->gic_vprebits = 5; + cpu->gic_pribits = 5; /* Suppport of A64FX's vector length are 128,256 and 512bit only */ aarch64_add_sve_properties(obj); -- cgit v1.2.3-55-g7522 From 6e76d35f2375c3ef58aaaccbe5cee54b20a1f74a Mon Sep 17 00:00:00 2001 From: Philippe Mathieu-Daudé Date: Mon, 9 May 2022 22:20:35 +0200 Subject: hw/adc/zynq-xadc: Use qemu_irq typedef Except hw/core/irq.c which implements the forward-declared opaque qemu_irq structure, hw/adc/zynq-xadc.{c,h} are the only files not using the typedef. Fix this single exception. Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Bernhard Beschow Message-id: 20220509202035.50335-1-philippe.mathieu.daude@gmail.com Signed-off-by: Peter Maydell --- hw/adc/zynq-xadc.c | 4 ++-- include/hw/adc/zynq-xadc.h | 3 +-- 2 files changed, 3 insertions(+), 4 deletions(-) (limited to 'include') diff --git a/hw/adc/zynq-xadc.c b/hw/adc/zynq-xadc.c index cfc7bab065..032e19cbd0 100644 --- a/hw/adc/zynq-xadc.c +++ b/hw/adc/zynq-xadc.c @@ -86,7 +86,7 @@ static void zynq_xadc_update_ints(ZynqXADCState *s) s->regs[INT_STS] |= INT_DFIFO_GTH; } - qemu_set_irq(s->qemu_irq, !!(s->regs[INT_STS] & ~s->regs[INT_MASK])); + qemu_set_irq(s->irq, !!(s->regs[INT_STS] & ~s->regs[INT_MASK])); } static void zynq_xadc_reset(DeviceState *d) @@ -262,7 +262,7 @@ static void zynq_xadc_init(Object *obj) memory_region_init_io(&s->iomem, obj, &xadc_ops, s, "zynq-xadc", ZYNQ_XADC_MMIO_SIZE); sysbus_init_mmio(sbd, &s->iomem); - sysbus_init_irq(sbd, &s->qemu_irq); + sysbus_init_irq(sbd, &s->irq); } static const VMStateDescription vmstate_zynq_xadc = { diff --git a/include/hw/adc/zynq-xadc.h b/include/hw/adc/zynq-xadc.h index 2017b7a803..c10cc4c379 100644 --- a/include/hw/adc/zynq-xadc.h +++ b/include/hw/adc/zynq-xadc.h @@ -39,8 +39,7 @@ struct ZynqXADCState { uint16_t xadc_dfifo[ZYNQ_XADC_FIFO_DEPTH]; uint16_t xadc_dfifo_entries; - struct IRQState *qemu_irq; - + qemu_irq irq; }; #endif /* ZYNQ_XADC_H */ -- cgit v1.2.3-55-g7522 From 9598c1bb39b2d4f0d3a55072cc70251c452132cd Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Mon, 16 May 2022 11:30:58 +0100 Subject: ptimer: Rename PTIMER_POLICY_DEFAULT to PTIMER_POLICY_LEGACY The traditional ptimer behaviour includes a collection of weird edge case behaviours. In 2016 we improved the ptimer implementation to fix these and generally make the behaviour more flexible, with ptimers opting in to the new behaviour by passing an appropriate set of policy flags to ptimer_init(). For backwards-compatibility, we defined PTIMER_POLICY_DEFAULT (which sets no flags) to give the old weird behaviour. This turns out to be a poor choice of name, because people writing new devices which use ptimers are misled into thinking that the default is probably a sensible choice of flags, when in fact it is almost always not what you want. Rename PTIMER_POLICY_DEFAULT to PTIMER_POLICY_LEGACY and beef up the comment to more clearly say that new devices should not be using it. The code-change part of this commit was produced by sed -i -e 's/PTIMER_POLICY_DEFAULT/PTIMER_POLICY_LEGACY/g' $(git grep -l PTIMER_POLICY_DEFAULT) with the exception of a test name string change in tests/unit/ptimer-test.c which was added manually. Signed-off-by: Peter Maydell Reviewed-by: Francisco Iglesias Reviewed-by: Richard Henderson Message-id: 20220516103058.162280-1-peter.maydell@linaro.org --- hw/arm/musicpal.c | 2 +- hw/dma/xilinx_axidma.c | 2 +- hw/dma/xlnx_csu_dma.c | 2 +- hw/m68k/mcf5206.c | 2 +- hw/m68k/mcf5208.c | 2 +- hw/net/can/xlnx-zynqmp-can.c | 2 +- hw/net/fsl_etsec/etsec.c | 2 +- hw/net/lan9118.c | 2 +- hw/rtc/exynos4210_rtc.c | 4 ++-- hw/timer/allwinner-a10-pit.c | 2 +- hw/timer/altera_timer.c | 2 +- hw/timer/arm_timer.c | 2 +- hw/timer/digic-timer.c | 2 +- hw/timer/etraxfs_timer.c | 6 +++--- hw/timer/exynos4210_mct.c | 6 +++--- hw/timer/exynos4210_pwm.c | 2 +- hw/timer/grlib_gptimer.c | 2 +- hw/timer/imx_epit.c | 4 ++-- hw/timer/imx_gpt.c | 2 +- hw/timer/mss-timer.c | 2 +- hw/timer/sh_timer.c | 2 +- hw/timer/slavio_timer.c | 2 +- hw/timer/xilinx_timer.c | 2 +- include/hw/ptimer.h | 16 ++++++++++++---- tests/unit/ptimer-test.c | 6 +++--- 25 files changed, 44 insertions(+), 36 deletions(-) (limited to 'include') diff --git a/hw/arm/musicpal.c b/hw/arm/musicpal.c index 7c840fb428..b65c020115 100644 --- a/hw/arm/musicpal.c +++ b/hw/arm/musicpal.c @@ -464,7 +464,7 @@ static void mv88w8618_timer_init(SysBusDevice *dev, mv88w8618_timer_state *s, sysbus_init_irq(dev, &s->irq); s->freq = freq; - s->ptimer = ptimer_init(mv88w8618_timer_tick, s, PTIMER_POLICY_DEFAULT); + s->ptimer = ptimer_init(mv88w8618_timer_tick, s, PTIMER_POLICY_LEGACY); } static uint64_t mv88w8618_pit_read(void *opaque, hwaddr offset, diff --git a/hw/dma/xilinx_axidma.c b/hw/dma/xilinx_axidma.c index bc383f53cc..cbb8f0f169 100644 --- a/hw/dma/xilinx_axidma.c +++ b/hw/dma/xilinx_axidma.c @@ -552,7 +552,7 @@ static void xilinx_axidma_realize(DeviceState *dev, Error **errp) st->dma = s; st->nr = i; - st->ptimer = ptimer_init(timer_hit, st, PTIMER_POLICY_DEFAULT); + st->ptimer = ptimer_init(timer_hit, st, PTIMER_POLICY_LEGACY); ptimer_transaction_begin(st->ptimer); ptimer_set_freq(st->ptimer, s->freqhz); ptimer_transaction_commit(st->ptimer); diff --git a/hw/dma/xlnx_csu_dma.c b/hw/dma/xlnx_csu_dma.c index 60ada3286b..1ce52ea5a2 100644 --- a/hw/dma/xlnx_csu_dma.c +++ b/hw/dma/xlnx_csu_dma.c @@ -666,7 +666,7 @@ static void xlnx_csu_dma_realize(DeviceState *dev, Error **errp) sysbus_init_irq(SYS_BUS_DEVICE(dev), &s->irq); s->src_timer = ptimer_init(xlnx_csu_dma_src_timeout_hit, - s, PTIMER_POLICY_DEFAULT); + s, PTIMER_POLICY_LEGACY); s->attr = MEMTXATTRS_UNSPECIFIED; diff --git a/hw/m68k/mcf5206.c b/hw/m68k/mcf5206.c index 6d93d761a5..2ab1b4f059 100644 --- a/hw/m68k/mcf5206.c +++ b/hw/m68k/mcf5206.c @@ -152,7 +152,7 @@ static m5206_timer_state *m5206_timer_init(qemu_irq irq) m5206_timer_state *s; s = g_new0(m5206_timer_state, 1); - s->timer = ptimer_init(m5206_timer_trigger, s, PTIMER_POLICY_DEFAULT); + s->timer = ptimer_init(m5206_timer_trigger, s, PTIMER_POLICY_LEGACY); s->irq = irq; m5206_timer_reset(s); return s; diff --git a/hw/m68k/mcf5208.c b/hw/m68k/mcf5208.c index 655207e393..be1033f84f 100644 --- a/hw/m68k/mcf5208.c +++ b/hw/m68k/mcf5208.c @@ -197,7 +197,7 @@ static void mcf5208_sys_init(MemoryRegion *address_space, qemu_irq *pic) /* Timers. */ for (i = 0; i < 2; i++) { s = g_new0(m5208_timer_state, 1); - s->timer = ptimer_init(m5208_timer_trigger, s, PTIMER_POLICY_DEFAULT); + s->timer = ptimer_init(m5208_timer_trigger, s, PTIMER_POLICY_LEGACY); memory_region_init_io(&s->iomem, NULL, &m5208_timer_ops, s, "m5208-timer", 0x00004000); memory_region_add_subregion(address_space, 0xfc080000 + 0x4000 * i, diff --git a/hw/net/can/xlnx-zynqmp-can.c b/hw/net/can/xlnx-zynqmp-can.c index 22bb8910fa..82ac48cee2 100644 --- a/hw/net/can/xlnx-zynqmp-can.c +++ b/hw/net/can/xlnx-zynqmp-can.c @@ -1079,7 +1079,7 @@ static void xlnx_zynqmp_can_realize(DeviceState *dev, Error **errp) /* Allocate a new timer. */ s->can_timer = ptimer_init(xlnx_zynqmp_can_ptimer_cb, s, - PTIMER_POLICY_DEFAULT); + PTIMER_POLICY_LEGACY); ptimer_transaction_begin(s->can_timer); diff --git a/hw/net/fsl_etsec/etsec.c b/hw/net/fsl_etsec/etsec.c index 6d50c39543..4e6cc708de 100644 --- a/hw/net/fsl_etsec/etsec.c +++ b/hw/net/fsl_etsec/etsec.c @@ -393,7 +393,7 @@ static void etsec_realize(DeviceState *dev, Error **errp) object_get_typename(OBJECT(dev)), dev->id, etsec); qemu_format_nic_info_str(qemu_get_queue(etsec->nic), etsec->conf.macaddr.a); - etsec->ptimer = ptimer_init(etsec_timer_hit, etsec, PTIMER_POLICY_DEFAULT); + etsec->ptimer = ptimer_init(etsec_timer_hit, etsec, PTIMER_POLICY_LEGACY); ptimer_transaction_begin(etsec->ptimer); ptimer_set_freq(etsec->ptimer, 100); ptimer_transaction_commit(etsec->ptimer); diff --git a/hw/net/lan9118.c b/hw/net/lan9118.c index 6aff424cbe..456ae38107 100644 --- a/hw/net/lan9118.c +++ b/hw/net/lan9118.c @@ -1363,7 +1363,7 @@ static void lan9118_realize(DeviceState *dev, Error **errp) s->pmt_ctrl = 1; s->txp = &s->tx_packet; - s->timer = ptimer_init(lan9118_tick, s, PTIMER_POLICY_DEFAULT); + s->timer = ptimer_init(lan9118_tick, s, PTIMER_POLICY_LEGACY); ptimer_transaction_begin(s->timer); ptimer_set_freq(s->timer, 10000); ptimer_set_limit(s->timer, 0xffff, 1); diff --git a/hw/rtc/exynos4210_rtc.c b/hw/rtc/exynos4210_rtc.c index ae67641de6..d1620c7a2a 100644 --- a/hw/rtc/exynos4210_rtc.c +++ b/hw/rtc/exynos4210_rtc.c @@ -564,14 +564,14 @@ static void exynos4210_rtc_init(Object *obj) Exynos4210RTCState *s = EXYNOS4210_RTC(obj); SysBusDevice *dev = SYS_BUS_DEVICE(obj); - s->ptimer = ptimer_init(exynos4210_rtc_tick, s, PTIMER_POLICY_DEFAULT); + s->ptimer = ptimer_init(exynos4210_rtc_tick, s, PTIMER_POLICY_LEGACY); ptimer_transaction_begin(s->ptimer); ptimer_set_freq(s->ptimer, RTC_BASE_FREQ); exynos4210_rtc_update_freq(s, 0); ptimer_transaction_commit(s->ptimer); s->ptimer_1Hz = ptimer_init(exynos4210_rtc_1Hz_tick, - s, PTIMER_POLICY_DEFAULT); + s, PTIMER_POLICY_LEGACY); ptimer_transaction_begin(s->ptimer_1Hz); ptimer_set_freq(s->ptimer_1Hz, RTC_BASE_FREQ); ptimer_transaction_commit(s->ptimer_1Hz); diff --git a/hw/timer/allwinner-a10-pit.c b/hw/timer/allwinner-a10-pit.c index c3fc2a4daa..971f78462a 100644 --- a/hw/timer/allwinner-a10-pit.c +++ b/hw/timer/allwinner-a10-pit.c @@ -275,7 +275,7 @@ static void a10_pit_init(Object *obj) tc->container = s; tc->index = i; - s->timer[i] = ptimer_init(a10_pit_timer_cb, tc, PTIMER_POLICY_DEFAULT); + s->timer[i] = ptimer_init(a10_pit_timer_cb, tc, PTIMER_POLICY_LEGACY); } } diff --git a/hw/timer/altera_timer.c b/hw/timer/altera_timer.c index c6e02d2b5a..0f1f54206a 100644 --- a/hw/timer/altera_timer.c +++ b/hw/timer/altera_timer.c @@ -185,7 +185,7 @@ static void altera_timer_realize(DeviceState *dev, Error **errp) return; } - t->ptimer = ptimer_init(timer_hit, t, PTIMER_POLICY_DEFAULT); + t->ptimer = ptimer_init(timer_hit, t, PTIMER_POLICY_LEGACY); ptimer_transaction_begin(t->ptimer); ptimer_set_freq(t->ptimer, t->freq_hz); ptimer_transaction_commit(t->ptimer); diff --git a/hw/timer/arm_timer.c b/hw/timer/arm_timer.c index 84cf2726bb..69c8863472 100644 --- a/hw/timer/arm_timer.c +++ b/hw/timer/arm_timer.c @@ -180,7 +180,7 @@ static arm_timer_state *arm_timer_init(uint32_t freq) s->freq = freq; s->control = TIMER_CTRL_IE; - s->timer = ptimer_init(arm_timer_tick, s, PTIMER_POLICY_DEFAULT); + s->timer = ptimer_init(arm_timer_tick, s, PTIMER_POLICY_LEGACY); vmstate_register(NULL, VMSTATE_INSTANCE_ID_ANY, &vmstate_arm_timer, s); return s; } diff --git a/hw/timer/digic-timer.c b/hw/timer/digic-timer.c index e3aae4a45a..d5186f4454 100644 --- a/hw/timer/digic-timer.c +++ b/hw/timer/digic-timer.c @@ -139,7 +139,7 @@ static void digic_timer_init(Object *obj) { DigicTimerState *s = DIGIC_TIMER(obj); - s->ptimer = ptimer_init(digic_timer_tick, NULL, PTIMER_POLICY_DEFAULT); + s->ptimer = ptimer_init(digic_timer_tick, NULL, PTIMER_POLICY_LEGACY); /* * FIXME: there is no documentation on Digic timer diff --git a/hw/timer/etraxfs_timer.c b/hw/timer/etraxfs_timer.c index 139e5b86a4..ecc2831baf 100644 --- a/hw/timer/etraxfs_timer.c +++ b/hw/timer/etraxfs_timer.c @@ -370,9 +370,9 @@ static void etraxfs_timer_realize(DeviceState *dev, Error **errp) ETRAXTimerState *t = ETRAX_TIMER(dev); SysBusDevice *sbd = SYS_BUS_DEVICE(dev); - t->ptimer_t0 = ptimer_init(timer0_hit, t, PTIMER_POLICY_DEFAULT); - t->ptimer_t1 = ptimer_init(timer1_hit, t, PTIMER_POLICY_DEFAULT); - t->ptimer_wd = ptimer_init(watchdog_hit, t, PTIMER_POLICY_DEFAULT); + t->ptimer_t0 = ptimer_init(timer0_hit, t, PTIMER_POLICY_LEGACY); + t->ptimer_t1 = ptimer_init(timer1_hit, t, PTIMER_POLICY_LEGACY); + t->ptimer_wd = ptimer_init(watchdog_hit, t, PTIMER_POLICY_LEGACY); sysbus_init_irq(sbd, &t->irq); sysbus_init_irq(sbd, &t->nmi); diff --git a/hw/timer/exynos4210_mct.c b/hw/timer/exynos4210_mct.c index d0e5343996..e175a9f5b9 100644 --- a/hw/timer/exynos4210_mct.c +++ b/hw/timer/exynos4210_mct.c @@ -1503,17 +1503,17 @@ static void exynos4210_mct_init(Object *obj) /* Global timer */ s->g_timer.ptimer_frc = ptimer_init(exynos4210_gfrc_event, s, - PTIMER_POLICY_DEFAULT); + PTIMER_POLICY_LEGACY); memset(&s->g_timer.reg, 0, sizeof(struct gregs)); /* Local timers */ for (i = 0; i < 2; i++) { s->l_timer[i].tick_timer.ptimer_tick = ptimer_init(exynos4210_ltick_event, &s->l_timer[i], - PTIMER_POLICY_DEFAULT); + PTIMER_POLICY_LEGACY); s->l_timer[i].ptimer_frc = ptimer_init(exynos4210_lfrc_event, &s->l_timer[i], - PTIMER_POLICY_DEFAULT); + PTIMER_POLICY_LEGACY); s->l_timer[i].id = i; } diff --git a/hw/timer/exynos4210_pwm.c b/hw/timer/exynos4210_pwm.c index 220088120e..02924a9e5b 100644 --- a/hw/timer/exynos4210_pwm.c +++ b/hw/timer/exynos4210_pwm.c @@ -400,7 +400,7 @@ static void exynos4210_pwm_init(Object *obj) sysbus_init_irq(dev, &s->timer[i].irq); s->timer[i].ptimer = ptimer_init(exynos4210_pwm_tick, &s->timer[i], - PTIMER_POLICY_DEFAULT); + PTIMER_POLICY_LEGACY); s->timer[i].id = i; s->timer[i].parent = s; } diff --git a/hw/timer/grlib_gptimer.c b/hw/timer/grlib_gptimer.c index d511890109..5c4923c1e0 100644 --- a/hw/timer/grlib_gptimer.c +++ b/hw/timer/grlib_gptimer.c @@ -383,7 +383,7 @@ static void grlib_gptimer_realize(DeviceState *dev, Error **errp) timer->unit = unit; timer->ptimer = ptimer_init(grlib_gptimer_hit, timer, - PTIMER_POLICY_DEFAULT); + PTIMER_POLICY_LEGACY); timer->id = i; /* One IRQ line for each timer */ diff --git a/hw/timer/imx_epit.c b/hw/timer/imx_epit.c index ebd58254d1..2bf8c754b2 100644 --- a/hw/timer/imx_epit.c +++ b/hw/timer/imx_epit.c @@ -347,9 +347,9 @@ static void imx_epit_realize(DeviceState *dev, Error **errp) 0x00001000); sysbus_init_mmio(sbd, &s->iomem); - s->timer_reload = ptimer_init(imx_epit_reload, s, PTIMER_POLICY_DEFAULT); + s->timer_reload = ptimer_init(imx_epit_reload, s, PTIMER_POLICY_LEGACY); - s->timer_cmp = ptimer_init(imx_epit_cmp, s, PTIMER_POLICY_DEFAULT); + s->timer_cmp = ptimer_init(imx_epit_cmp, s, PTIMER_POLICY_LEGACY); } static void imx_epit_class_init(ObjectClass *klass, void *data) diff --git a/hw/timer/imx_gpt.c b/hw/timer/imx_gpt.c index 5c0d9a269c..80b8302639 100644 --- a/hw/timer/imx_gpt.c +++ b/hw/timer/imx_gpt.c @@ -505,7 +505,7 @@ static void imx_gpt_realize(DeviceState *dev, Error **errp) 0x00001000); sysbus_init_mmio(sbd, &s->iomem); - s->timer = ptimer_init(imx_gpt_timeout, s, PTIMER_POLICY_DEFAULT); + s->timer = ptimer_init(imx_gpt_timeout, s, PTIMER_POLICY_LEGACY); } static void imx_gpt_class_init(ObjectClass *klass, void *data) diff --git a/hw/timer/mss-timer.c b/hw/timer/mss-timer.c index fe0ca905f3..ee7438f168 100644 --- a/hw/timer/mss-timer.c +++ b/hw/timer/mss-timer.c @@ -232,7 +232,7 @@ static void mss_timer_init(Object *obj) for (i = 0; i < NUM_TIMERS; i++) { struct Msf2Timer *st = &t->timers[i]; - st->ptimer = ptimer_init(timer_hit, st, PTIMER_POLICY_DEFAULT); + st->ptimer = ptimer_init(timer_hit, st, PTIMER_POLICY_LEGACY); ptimer_transaction_begin(st->ptimer); ptimer_set_freq(st->ptimer, t->freq_hz); ptimer_transaction_commit(st->ptimer); diff --git a/hw/timer/sh_timer.c b/hw/timer/sh_timer.c index c72c327bfa..7788939766 100644 --- a/hw/timer/sh_timer.c +++ b/hw/timer/sh_timer.c @@ -239,7 +239,7 @@ static void *sh_timer_init(uint32_t freq, int feat, qemu_irq irq) s->enabled = 0; s->irq = irq; - s->timer = ptimer_init(sh_timer_tick, s, PTIMER_POLICY_DEFAULT); + s->timer = ptimer_init(sh_timer_tick, s, PTIMER_POLICY_LEGACY); sh_timer_write(s, OFFSET_TCOR >> 2, s->tcor); sh_timer_write(s, OFFSET_TCNT >> 2, s->tcnt); diff --git a/hw/timer/slavio_timer.c b/hw/timer/slavio_timer.c index 90fdce4c44..8c4f6eb06b 100644 --- a/hw/timer/slavio_timer.c +++ b/hw/timer/slavio_timer.c @@ -405,7 +405,7 @@ static void slavio_timer_init(Object *obj) tc->timer_index = i; s->cputimer[i].timer = ptimer_init(slavio_timer_irq, tc, - PTIMER_POLICY_DEFAULT); + PTIMER_POLICY_LEGACY); ptimer_transaction_begin(s->cputimer[i].timer); ptimer_set_period(s->cputimer[i].timer, TIMER_PERIOD); ptimer_transaction_commit(s->cputimer[i].timer); diff --git a/hw/timer/xilinx_timer.c b/hw/timer/xilinx_timer.c index 1eb927eb84..c7f17cd646 100644 --- a/hw/timer/xilinx_timer.c +++ b/hw/timer/xilinx_timer.c @@ -223,7 +223,7 @@ static void xilinx_timer_realize(DeviceState *dev, Error **errp) xt->parent = t; xt->nr = i; - xt->ptimer = ptimer_init(timer_hit, xt, PTIMER_POLICY_DEFAULT); + xt->ptimer = ptimer_init(timer_hit, xt, PTIMER_POLICY_LEGACY); ptimer_transaction_begin(xt->ptimer); ptimer_set_freq(xt->ptimer, t->freq_hz); ptimer_transaction_commit(xt->ptimer); diff --git a/include/hw/ptimer.h b/include/hw/ptimer.h index c443218475..4dc02b0de4 100644 --- a/include/hw/ptimer.h +++ b/include/hw/ptimer.h @@ -33,9 +33,17 @@ * to stderr when the guest attempts to enable the timer. */ -/* The default ptimer policy retains backward compatibility with the legacy - * timers. Custom policies are adjusting the default one. Consider providing - * a correct policy for your timer. +/* + * The 'legacy' ptimer policy retains backward compatibility with the + * traditional ptimer behaviour from before policy flags were introduced. + * It has several weird behaviours which don't match typical hardware + * timer behaviour. For a new device using ptimers, you should not + * use PTIMER_POLICY_LEGACY, but instead check the actual behaviour + * that you need and specify the right set of policy flags to get that. + * + * If you are overhauling an existing device that uses PTIMER_POLICY_LEGACY + * and are in a position to check or test the real hardware behaviour, + * consider updating it to specify the right policy flags. * * The rough edges of the default policy: * - Starting to run with a period = 0 emits error message and stops the @@ -54,7 +62,7 @@ * since the last period, effectively restarting the timer with a * counter = counter value at the moment of change (.i.e. one less). */ -#define PTIMER_POLICY_DEFAULT 0 +#define PTIMER_POLICY_LEGACY 0 /* Periodic timer counter stays with "0" for a one period before wrapping * around. */ diff --git a/tests/unit/ptimer-test.c b/tests/unit/ptimer-test.c index 9176b96c1c..a80ef5aff4 100644 --- a/tests/unit/ptimer-test.c +++ b/tests/unit/ptimer-test.c @@ -768,8 +768,8 @@ static void add_ptimer_tests(uint8_t policy) char policy_name[256] = ""; char *tmp; - if (policy == PTIMER_POLICY_DEFAULT) { - g_sprintf(policy_name, "default"); + if (policy == PTIMER_POLICY_LEGACY) { + g_sprintf(policy_name, "legacy"); } if (policy & PTIMER_POLICY_WRAP_AFTER_ONE_PERIOD) { @@ -862,7 +862,7 @@ static void add_ptimer_tests(uint8_t policy) static void add_all_ptimer_policies_comb_tests(void) { int last_policy = PTIMER_POLICY_TRIGGER_ONLY_ON_DECREMENT; - int policy = PTIMER_POLICY_DEFAULT; + int policy = PTIMER_POLICY_LEGACY; for (; policy < (last_policy << 1); policy++) { if ((policy & PTIMER_POLICY_TRIGGER_ONLY_ON_DECREMENT) && -- cgit v1.2.3-55-g7522