[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] Re: [PATCH 1/2] Add HPET emulation to qemu (v3)
|
From: |
Anthony Liguori |
|
Subject: |
[Qemu-devel] Re: [PATCH 1/2] Add HPET emulation to qemu (v3) |
|
Date: |
Tue, 21 Oct 2008 10:21:01 -0500 |
|
User-agent: |
Thunderbird 2.0.0.17 (X11/20080925) |
Beth Kon wrote:
This version contains many miscellaneous changes, including incorporation
of comments received, addition of save/restore support and a reset handler,
and a couple of bugfixes.
I've booted Linux and Win2k832 guests on QEMU. Win2k832 still looks shaky
on QEMU even without the hpet - I've gotten intermittent blue screens. But
I did get it to boot at least once both with and without hpet.
Clock drift on Linux is in the range of .017% - .019%, loaded and unloaded. I
haven't found a straightforward way to test on Windows and would appreciate
any pointers to existing approaches.
The second patch in this series contains the needed bochs bios changes.
We need a --disable-hpet option at least to be able to disable the hpet
when doing migration from an older QEMU to a newer one. An info hpet
would also be nice to be able to probe as to whether the hpet was active.
+#define FS_PER_NS 1000000
+#define HPET_NUM_TIMERS 3
+#define HPET_TIMER_TYPE_LEVEL 1
+#define HPET_TIMER_TYPE_EDGE 0
+#define HPET_TIMER_DELIVERY_APIC 0
+#define HPET_TIMER_DELIVERY_FSB 1
+#define HPET_TIMER_CAP_FSB_INT_DEL (1 << 15)
+#define HPET_TIMER_CAP_PER_INT (1 << 4)
+
+#define HPET_CFG_ENABLE 0x001
+#define HPET_CFG_LEGACY 0x002
+
+#define HPET_ID 0x000
+#define HPET_PERIOD 0x004
+#define HPET_CFG 0x010
+#define HPET_STATUS 0x020
+#define HPET_COUNTER 0x0f0
+#define HPET_TN_REGS 0x100 ... 0x3ff /*address range of all TN regs*/
Most of this stuff could go into a header file. This particular define
uses a GCCism (range case statements). I would object to this sort of
define in principle but regardless, we shouldn't introduce this GCCism
to QEMU since it only serves cosmetic function. A simple if() before
the switch statement would work just as well.
+#define HPET_TN_CFG 0x000
+#define HPET_TN_CMP 0x008
+#define HPET_TN_ROUTE 0x010
+
+
+#define HPET_TN_ENABLE 0x004
+#define HPET_TN_PERIODIC 0x008
+#define HPET_TN_PERIODIC_CAP 0x010
+#define HPET_TN_SIZE_CAP 0x020
+#define HPET_TN_SETVAL 0x040
+#define HPET_TN_32BIT 0x100
+#define HPET_TN_INT_ROUTE_MASK 0x3e00
+#define HPET_TN_INT_ROUTE_SHIFT 9
+#define HPET_TN_INT_ROUTE_CAP_SHIFT 32
+#define HPET_TN_CFG_BITS_READONLY_OR_RESERVED 0xffff80b1U
+
+#define timer_int_route(timer) \
+ ((timer->config & HPET_TN_INT_ROUTE_MASK) >> HPET_TN_INT_ROUTE_SHIFT)
+#define hpet_enabled(s) (s->config & HPET_CFG_ENABLE)
+#define timer_is_periodic(t) (t->config & HPET_TN_PERIODIC)
+#define timer_enabled(t) (t->config & HPET_TN_ENABLE)
+
+#define hpet_time_after(a, b) ((int32_t)(b) - (int32_t)(a) < 0)
+#define hpet_time_after64(a, b) ((int64_t)(b) - (int64_t)(a) < 0)
These should all be static functions.
+
+/*indicator hpet is operating in legacy mode */
+int hpet_legacy=0;
This should be part of the HPETState, and we should use an function to
access it from other parts of the code.
+static inline uint64_t ticks_to_ns(uint64_t value)
+{
+ return (muldiv64(value, HPET_CLK_PERIOD, FS_PER_NS));
+}
inline is unnecessary for all of these.
+static inline uint64_t hpet_get_ticks(HPETState *s) {
We need to be consistent in our formatting. { should go on a new line.
+static void update_irq(struct HPETTimer *timer)
+{
+ qemu_irq irq;
+ int route, do_ioapic = 0;
+
+ if ( (timer->tn <= 1) && (timer->state->config & HPET_CFG_LEGACY) ) {
Whitespace is unnecessary here.
+
+ /* if LegacyReplacementRoute bit is set, HPET specification requires
+ * timer0 be routed to IRQ0 in NON-APIC or IRQ2 in the I/O APIC,
+ * timer1 be routed to IRQ8 in NON-APIC or IRQ8 in the I/O APIC.
+ */
+ if (timer->tn == 0) {
+ irq=timer->state->irqs[0];
But necessary here.
+ do_ioapic = 1;
+ } else
+ irq=timer->state->irqs[8];
+ }else{
And here and in lots of other places. Please try to make the whitespace
consistent in this file as with the rest of QEMU.
+ route=timer_int_route(timer);
+ irq=timer->state->irqs[route];
+ }
+
+ if(timer_enabled(timer) && hpet_enabled(timer->state)) {
+ qemu_irq_pulse(irq);
+ /* windows wants timer0 on irq2 and linux wants irq0,
+ * so we pulse both
+ */
+ if (do_ioapic)
+ qemu_irq_pulse(timer->state->irqs[2]);
This seems curious and not quite right. We should be able to detect
whether the HPET is being used in IO APIC mode and raise the appropriate
interrupt instead of generating a spurious irq0 interrupt.
+ }
+}
+
+static void hpet_save(QEMUFile *f, void *opaque)
+{
+ HPETState *s = opaque;
+ int i;
+ qemu_put_be64s(f, &s->config);
+ qemu_put_be64s(f, &s->isr);
+ /* save current counter value */
+ s->hpet_counter = hpet_get_ticks(s);
+ qemu_put_be64s(f, &s->hpet_counter);
+
+ for(i = 0; i < HPET_NUM_TIMERS; i++) {
+ qemu_put_8s(f, &s->timer[i].tn);
+ qemu_put_be64s(f, &s->timer[i].config);
+ qemu_put_be64s(f, &s->timer[i].cmp);
+ qemu_put_be64s(f, &s->timer[i].fsb);
+ qemu_put_be64s(f, &s->timer[i].period);
+ if (s->timer[i].qemu_timer) {
+ qemu_put_timer(f, s->timer[i].qemu_timer);
+ }
Would qemu_timer ever be NULL?
+/*
+ * timer expiration callback
+ */
+
+static void hpet_timer(void *opaque)
+{
+ HPETTimer *t = (HPETTimer*)opaque;
The cast is unnecessary.
+ uint64_t diff;
+
+ uint64_t period = t->period;
+ uint64_t cur_tick = hpet_get_ticks(t->state);
+
+ if (timer_is_periodic(t) && (t->period != 0))
+ {
Bad formatting.
+ if (t->config & HPET_TN_32BIT) {
+ while (hpet_time_after(cur_tick, t->cmp))
+ t->cmp = (uint32_t)(t->cmp + t->period);
+ } else
+ while (hpet_time_after64(cur_tick, t->cmp))
+ t->cmp += period;
+
+ diff = hpet_calculate_diff(t, cur_tick);
+ qemu_mod_timer(t->qemu_timer, qemu_get_clock(vm_clock)
+ + (int64_t)ticks_to_ns(diff));
May want to convert ticks_to_ns to take and return an int64_t. The
explicit casting could introduce very subtle bugs.
+ }
+ update_irq(t);
+}
+
+static void hpet_set_timer(HPETTimer *t)
+{
+ uint64_t diff;
+ uint64_t cur_tick = hpet_get_ticks(t->state);
+
+ diff = hpet_calculate_diff(t, cur_tick);
+ qemu_mod_timer(t->qemu_timer, qemu_get_clock(vm_clock) +
(int64_t)ticks_to_ns(diff));
+}
+
+static void hpet_del_timer(HPETTimer *t)
+{
+ qemu_del_timer(t->qemu_timer);
+}
+
+#ifdef HPET_DEBUG
+static uint32_t hpet_ram_readb(void *opaque, target_phys_addr_t addr)
+{
+ fprintf(stderr, "qemu: hpet_read b at %" PRIx64 "\n", addr);
+ return 10;
+}
+
+static uint32_t hpet_ram_readw(void *opaque, target_phys_addr_t addr)
+{
+ fprintf(stderr, "qemu: hpet_read w at %" PRIx64 "\n", addr);
+ return 10;
+}
+#endif
+
+static uint32_t hpet_ram_readl(void *opaque, target_phys_addr_t addr)
+{
+ HPETState *s = (HPETState *)opaque;
+ uint64_t cur_tick;
+#ifdef HPET_DEBUG
+ fprintf(stderr, "qemu: hpet_read l at %" PRIx64 "\n", addr);
+#endif
I think it would be better to introduce a dprintf() so we could avoid
having so many open coded #ifdefs.
+ switch(addr - HPET_BASE) {
+ case HPET_ID:
+ return s->capability;
+ case HPET_PERIOD:
+ return s->capability >> 32;
+ case HPET_CFG:
+ return s->config;
+ case HPET_CFG + 4:
+#ifdef HPET_DEBUG
+ fprintf(stderr, "qemu: invalid hpet_read l at %" PRIx64 "\n",
addr);
+#endif
+ return 0;
+ case HPET_COUNTER:
+ if (hpet_enabled(s))
+ cur_tick = hpet_get_ticks(s);
Any reason for hpet_get_ticks(s) to not have this check integrated into it?
+ else
+ cur_tick = s->hpet_counter;
+#ifdef HPET_DEBUG
+ fprintf(stderr, "qemu: reading counter %" PRIx64 "\n", cur_tick);
+#endif
+ return cur_tick;
+ case HPET_COUNTER + 4:
+ return 0;
+ case HPET_STATUS:
+ return s->isr;
+ case HPET_TN_REGS:
+ {
+ uint8_t timer_id = (addr - HPET_BASE - 0x100) / 0x20;
+ if (timer_id > HPET_NUM_TIMERS - 1) {
+ fprintf(stderr, "qemu: timer id out of range\n");
+ return 0;
+ }
+ HPETTimer *timer = &s->timer[timer_id];
+
+ switch((addr - HPET_BASE - 0x100) % 0x20) {
+ case HPET_TN_CFG:
+ return timer->config;
+ case HPET_TN_CFG + 4: // Interrupt capabilities
+ return timer->config >> 32;
+ case HPET_TN_CMP: // comparator register
+ return timer->cmp;
+ case HPET_TN_CMP + 4:
+ return timer->cmp >> 32;
+ case HPET_TN_ROUTE:
+ return timer->fsb >> 32;
+ }
+ }
+ break;
+ }
+
+#ifdef HPET_DEBUG
+ fprintf(stderr, "qemu: invalid hpet_read l at %" PRIx64 "\n", addr);
+#endif
+ return 10;
+}
+
+#ifdef HPET_DEBUG
+static void hpet_ram_writeb(void *opaque, target_phys_addr_t addr,
+ uint32_t value)
+{
+ fprintf(stderr, "qemu: invalid hpet_write b at %" PRIx64 " = %#x\n",
+ addr, value);
+}
+
+static void hpet_ram_writew(void *opaque, target_phys_addr_t addr,
+ uint32_t value)
+{
+ fprintf(stderr, "qemu: invalid hpet_write w at %" PRIx64 " = %#x\n",
+ addr, value);
+}
+#endif
+
+static void hpet_ram_writel(void *opaque, target_phys_addr_t addr,
+ uint32_t value)
+{
+ int i;
+ HPETState *s = (HPETState *)opaque;
+ uint64_t old_val, new_val;
+
+#ifdef HPET_DEBUG
+ fprintf(stderr, "qemu: hpet_write l at %" PRIx64 " = %#x\n", addr, value);
+#endif
+ old_val = hpet_ram_readl(opaque, addr);
+ new_val = value;
+
+
+ switch(addr - HPET_BASE) {
+ case HPET_ID:
+ return;
+ case HPET_CFG:
+ s->config = hpet_fixup_reg(new_val, old_val, 0x3);
+ if (!(old_val & HPET_CFG_ENABLE) && (new_val & HPET_CFG_ENABLE)) {
+ /* Enable main counter and interrupt generation. */
+ s->hpet_offset = ticks_to_ns(s->hpet_counter)
+ - qemu_get_clock(vm_clock);
+ for (i = 0; i < HPET_NUM_TIMERS; i++)
+ if ((&s->timer[i])->cmp != ~0ULL)
+ hpet_set_timer(&s->timer[i]);
+ }
+ else if ( (old_val & HPET_CFG_ENABLE) &&
+ !(new_val & HPET_CFG_ENABLE)) {
+ /* Halt main counter and disable interrupt generation. */
+ s->hpet_counter = hpet_get_ticks(s);
+ for (i = 0; i < HPET_NUM_TIMERS; i++)
+ hpet_del_timer(&s->timer[i]);
+ }
+ hpet_legacy = s->config & HPET_CFG_LEGACY;
+ break;
+ case HPET_CFG + 4:
+#ifdef HPET_DEBUG
+ fprintf(stderr, "qemu: invalid hpet_writel at %" PRIx64 "=
%#x\n",
+ addr, value);
+#endif
+ break;
+ case HPET_STATUS:
+ /* FIXME: need to handle level-triggered interrupts */
+ break;
+ case HPET_COUNTER:
+ if (hpet_enabled(s))
+ fprintf(stderr, "qemu: Writing counter while HPET enabled!\n");
+ s->hpet_counter = value;
+ break;
+ case HPET_COUNTER + 4:
+ s->hpet_counter = (s->hpet_counter & 0xffffffffULL)
+ | (((uint64_t)value) << 32);
+#ifdef HPET_DEBUG
+ fprintf(stderr, "qemu: HPET ctr set to %#x -> %" PRIx64 "\n",
+ value, s->hpet_counter);
+#endif
+ break;
+ case HPET_TN_REGS:
+ {
+ uint8_t timer_id = (addr - HPET_BASE - 0x100) / 0x20;
+#ifdef HPET_DEBUG
+ fprintf(stderr,
+ "qemu: hpet_write l timer_id = %#x \n", timer_id);
+#endif
+ HPETTimer *timer = &s->timer[timer_id];
+
+ switch((addr - HPET_BASE - 0x100) % 0x20) {
+ case HPET_TN_CFG:
+#ifdef HPET_DEBUG
+ fprintf(stderr,
+ "qemu: hpet_write l TN config value %#x\n",
+ value);
+#endif
+ timer->config =
+ hpet_fixup_reg(new_val, old_val, 0x3e4e);
+ if (new_val & HPET_TN_32BIT) {
+ timer->cmp = (uint32_t)timer->cmp;
+ timer->period = (uint32_t)timer->period;
+ }
+ if (new_val & HPET_TIMER_TYPE_LEVEL){
+ fprintf(stderr,
+ "qemu: level-triggered hpet not supported\n");
+ exit (-1);
+ }
+
+ break;
+#ifdef HPET_DEBUG
+ case HPET_TN_CFG + 4: // Interrupt capabilities
+ fprintf(stderr,
+ "qemu: invalid write at %" PRIx64 "=%#x\n",
+ addr, value);
+ break;
+#endif
+ case HPET_TN_CMP: // comparator register
+#ifdef HPET_DEBUG
+ fprintf(stderr,
+ "qemu: hpet_writel TN cmp value %#x\n",
+ value);
+#endif
+ if ( timer->config & HPET_TN_32BIT)
+ new_val = (uint32_t)new_val;
+ if ( !timer_is_periodic(timer) ||
+ (timer->config & HPET_TN_SETVAL) )
+ timer->cmp = new_val;
+ else {
+ /*
+ * FIXME: Clamp period to reasonable min value?
+ * Clamp period to reasonable max value
+ */
+ new_val &= (timer->config & HPET_TN_32BIT ? ~0u :
~0ull) >> 1;
+ timer->period = new_val;
+ }
+ timer->config &= ~HPET_TN_SETVAL;
+ if (hpet_enabled(s))
+ hpet_set_timer(timer);
+ break;
+
+ case HPET_TN_ROUTE + 4:
+#ifdef HPET_DEBUG
+ fprintf(stderr,
+ "qemu: invalid write at %" PRIx64 " = %#x\n",
+ addr, value);
+#endif
+ break;
+ }
+ }
+ break;
+ default:
+ fprintf(stderr, "qemu: invalid write at %" PRIx64 " = %#x\n",
+ addr, value);
+ }
+
+}
+
+static CPUReadMemoryFunc *hpet_ram_read[] = {
+#ifdef HPET_DEBUG
+ hpet_ram_readb,
+ hpet_ram_readw,
+#else
+ NULL,
+ NULL,
+#endif
+ hpet_ram_readl,
+};
+
+static CPUWriteMemoryFunc *hpet_ram_write[] = {
+#ifdef HPET_DEBUG
+ hpet_ram_writeb,
+ hpet_ram_writew,
+#else
+ NULL,
+ NULL,
+#endif
+ hpet_ram_writel,
+};
+
+static void hpet_reset(void *opaque) {
+ HPETState *s = opaque;
+ int i;
+
+ /* 32-bit main counter; 3 timers supported; LegacyReplacementRoute.
+ * 64-bit counter cannot be supported until 64 bit atomic reads are
+ * possible, since reading counter involves grabbing current qemu
+ * clock, can't be done in 2 32-bit chunks.
+ */
+ s->capability = 0x80868201ULL;
+ s->capability |= ((HPET_CLK_PERIOD) << 32);
+
+ for(i=0; i<HPET_NUM_TIMERS; i++) {
+ HPETTimer *timer = &s->timer[i];
+ timer->tn = i;
+ timer->cmp = ~0ULL;
+ timer->config = HPET_TN_PERIODIC_CAP;
+ timer->config |= 0x0000ffffULL << 32;
+ timer->state = s;
+ timer->qemu_timer = qemu_new_timer(vm_clock, hpet_timer, timer);
+ }
+}
+
+
+void hpet_init(qemu_irq *irq) {
+ int iomemtype;
+ HPETState *s;
+
+ fprintf (stderr, "hpet_init\n");
Don't unconditionally print to stderr.
+
+ /* XXX this is a dirty hack for HPET support w/o LPC
+ Actually this is a config descriptor for the RCBA */
What's the dirty hack?
+ s = qemu_mallocz(sizeof(HPETState));
+ s->irqs = irq;
+ hpet_reset(s);
+ register_savevm("hpet", -1, 1, hpet_save, hpet_load, s);
+ qemu_register_reset(hpet_reset, s);
+ /* HPET Area */
+ iomemtype = cpu_register_io_memory(0, hpet_ram_read,
+ hpet_ram_write, s);
+ cpu_register_physical_memory(HPET_BASE, 0x400, iomemtype);
+}
diff --git a/hw/i8254.c b/hw/i8254.c
index 4813b03..7ffdf61 100644
--- a/hw/i8254.c
+++ b/hw/i8254.c
@@ -26,6 +26,9 @@
#include "isa.h"
#include "qemu-timer.h"
+#if defined TARGET_I386 || defined TARGET_X86_64
+extern int hpet_legacy;
+#endif
//#define DEBUG_PIT
#define RW_STATE_LSB 1
@@ -367,6 +370,14 @@ static void pit_irq_timer_update(PITChannelState *s,
int64_t current_time)
if (!s->irq_timer)
return;
+
+#if defined TARGET_I386 || defined TARGET_X86_64
+ if (hpet_legacy) {
+ qemu_del_timer(s->irq_timer);
+ return;
+ }
+#endif
This should have a comment.
expire_time = pit_get_next_transition_time(s, current_time);
irq_level = pit_get_out1(s, current_time);
qemu_set_irq(s->irq, irq_level);
diff --git a/hw/mc146818rtc.c b/hw/mc146818rtc.c
index 30bb044..ffb7a48 100644
--- a/hw/mc146818rtc.c
+++ b/hw/mc146818rtc.c
@@ -27,6 +27,10 @@
#include "pc.h"
#include "isa.h"
+#if defined TARGET_I386 || defined TARGET_X86_64
+extern int hpet_legacy;
+#endif
+
//#define DEBUG_CMOS
#define RTC_SECONDS 0
@@ -80,7 +84,11 @@ static void rtc_timer_update(RTCState *s, int64_t
current_time)
period_code = s->cmos_data[RTC_REG_A] & 0x0f;
if (period_code != 0 &&
+#if defined TARGET_I386 || defined TARGET_X86_64
+ (s->cmos_data[RTC_REG_B] & REG_B_PIE) && !hpet_legacy) {
+#else
(s->cmos_data[RTC_REG_B] & REG_B_PIE)) {
+#endif
This needs a comment.
if (period_code <= 2)
period_code += 7;
/* period in 32 Khz cycles */
@@ -101,7 +109,10 @@ static void rtc_periodic_timer(void *opaque)
rtc_timer_update(s, s->next_periodic_time);
s->cmos_data[RTC_REG_C] |= 0xc0;
- qemu_irq_raise(s->irq);
+#if defined TARGET_I386 || defined TARGET_X86_64
+ if (!hpet_legacy)
+#endif
+ qemu_irq_raise(s->irq);
}
static void cmos_ioport_write(void *opaque, uint32_t addr, uint32_t data)
@@ -281,6 +292,12 @@ static void rtc_update_second(void *opaque)
RTCState *s = opaque;
int64_t delay;
+#if defined TARGET_I386 || defined TARGET_X86_64
+ if (hpet_legacy) {
+ qemu_del_timer(s->second_timer2);
+ return;
+ }
+#endif
/* if the oscillator is not in normal operation, we do not update */
if ((s->cmos_data[RTC_REG_A] & 0x70) != 0x20) {
s->next_second_time += ticks_per_sec;
@@ -306,6 +323,12 @@ static void rtc_update_second2(void *opaque)
{
RTCState *s = opaque;
+#if defined TARGET_I386 || defined TARGET_X86_64
+ if (hpet_legacy) {
+ qemu_del_timer(s->second_timer);
+ return;
+ }
+#endif
if (!(s->cmos_data[RTC_REG_B] & REG_B_SET)) {
rtc_copy_date(s);
}
@@ -359,7 +382,10 @@ static uint32_t cmos_ioport_read(void *opaque, uint32_t
addr)
break;
case RTC_REG_C:
ret = s->cmos_data[s->cmos_index];
- qemu_irq_lower(s->irq);
+#if defined TARGET_I386 || defined TARGET_X86_64
+ if (!hpet_legacy)
+#endif
+ qemu_irq_lower(s->irq);
s->cmos_data[RTC_REG_C] = 0x00;
break;
default:
diff --git a/hw/pc.c b/hw/pc.c
index 34683e7..d2452e0 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -48,6 +48,7 @@
#define BIOS_CFG_IOPORT 0x510
#define MAX_IDE_BUS 2
+void hpet_init(qemu_irq irq);
static fdctrl_t *floppy_controller;
static RTCState *rtc_state;
@@ -949,6 +950,7 @@ static void pc_init1(ram_addr_t ram_size, int vga_ram_size,
}
pit = pit_init(0x40, i8259[0]);
pcspk_init(pit);
+ hpet_init(i8259);
if (pci_enabled) {
pic_set_alt_irq_func(isa_pic, ioapic_set_irq, ioapic);
}
Regards,
Anthony Liguori