* [Qemu-devel] [RFC][PATCH] Add HPET emulation to qemu
@ 2008-07-10 3:48 Beth Kon
2008-07-10 6:36 ` [Qemu-devel] " Dor Laor
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Beth Kon @ 2008-07-10 3:48 UTC (permalink / raw)
To: qemu-devel; +Cc: agraf, kvm
This patch, based on an earlier patch by Alexander Graf, adds HPET
emulation to qemu. I am sending out a separate patch to kvm with the
required bios changes.
This work is incomplete.
Currently working (at least generally):
- linux 2.6.25.9 guest
Todo:
- other guest support (i.e. adding whatever may be missing for
support of other modes of operation used by other OS's).
- level-triggered interrupts
- non-legacy routing
- 64-bit operation
- ...
Basically what I've done so far is make it work for linux.
The one area that feels ugly/wrong at the moment is handling the
disabling of 8254 and RTC timer interrupts when operating in legacy
mode. The HPET spec says "in this case the 8254/RTC timer will not cause
any interrupts". I'm not sure if I should disable the RTC/8254 in some
more general way, or just disable interrupts. Comments appreciated.
Signed-off-by: Beth Kon <bkon@us.ibm.com>
diffstat output:
Makefile.target | 2
hw/hpet.c | 393
+++++++++++++++++++++++++++++++++++++++++++++++++++++++
hw/i8254.c | 8 -
hw/mc146818rtc.c | 25 ++-
hw/pc.c | 5
5 files changed, 427 insertions(+), 6 deletions(-)
diff --git a/Makefile.target b/Makefile.target
index 73adbb1..05829ea 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -530,7 +530,7 @@ ifeq ($(TARGET_BASE_ARCH), i386)
OBJS+= ide.o pckbd.o ps2.o vga.o $(SOUND_HW) dma.o
OBJS+= fdc.o mc146818rtc.o serial.o i8259.o i8254.o pcspk.o pc.o
OBJS+= cirrus_vga.o apic.o parallel.o acpi.o piix_pci.o
-OBJS+= usb-uhci.o vmmouse.o vmport.o vmware_vga.o
+OBJS+= usb-uhci.o vmmouse.o vmport.o vmware_vga.o hpet.o
CPPFLAGS += -DHAS_AUDIO -DHAS_AUDIO_CHOICE
endif
ifeq ($(TARGET_BASE_ARCH), ppc)
diff --git a/hw/hpet.c b/hw/hpet.c
new file mode 100644
index 0000000..e74de08
--- /dev/null
+++ b/hw/hpet.c
@@ -0,0 +1,393 @@
+/*
+ * High Precisition Event Timer emulation
+ *
+ * Copyright (c) 2007 Alexander Graf
+ * Copyright (c) 2008 IBM Corporation
+ *
+ * Authors: Beth Kon <bkon@us.ibm.com>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307
USA
+ *
+ * *****************************************************************
+ *
+ * This driver attempts to emulate an HPET device in software. It is by
no
+ * means complete and is prone to break on certain conditions.
+ *
+ */
+#include "hw.h"
+#include "console.h"
+#include "qemu-timer.h"
+
+//#define HPET_DEBUG
+
+#define HPET_BASE 0xfed00000
+#define HPET_PERIOD 0x00989680 /* 10000000 femptoseconds,
10ns*/
+
+#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)
+
+struct HPETState;
+typedef struct HPETTimer {
+ QEMUTimer *timer;
+ struct HPETState *state;
+ uint8_t type;
+ uint8_t active;
+ uint8_t delivery;
+ uint8_t apic_port;
+ uint8_t periodic;
+ uint8_t enabled;
+ uint32_t comparator; // if(hpet_counter == comparator) IRQ();
+ uint32_t delta;
+ qemu_irq irq;
+} HPETTimer;
+
+typedef struct HPETState {
+ uint64_t hpet_counter;
+ uint64_t hpet_offset;
+ int64_t next_periodic_time;
+ uint8_t legacy_route;
+ uint8_t enabled;
+ uint8_t updated;
+ qemu_irq *irqs;
+ HPETTimer timer[HPET_NUM_TIMERS];
+} HPETState;
+
+
+int hpet_legacy;
+
+static void update_irq(struct HPETTimer *timer)
+{
+ if(timer->enabled && timer->state->enabled) {
+ qemu_irq_pulse(timer->irq);
+ }
+}
+
+static void update_irq_all(struct HPETState *s)
+{
+ int i;
+ for(i=0; i<HPET_NUM_TIMERS; i++)
+ update_irq(&s->timer[i]);
+}
+
+static inline int64_t ticks_to_ns(int64_t value)
+{
+ return (value * HPET_PERIOD / FS_PER_NS);
+}
+
+static inline int64_t ns_to_ticks(int64_t value)
+{
+ return (value * FS_PER_NS / HPET_PERIOD);
+}
+
+static void hpet_timer(void *opaque)
+{
+ HPETTimer *s = (HPETTimer*)opaque;
+ if(s->periodic) {
+ s->comparator += s->delta;
+ qemu_mod_timer(s->timer, qemu_get_clock(vm_clock)
+ + ticks_to_ns(s->delta));
+ }
+ update_irq(s);
+}
+
+static void hpet_check(HPETTimer *s)
+{
+ qemu_mod_timer(s->timer, qemu_get_clock(vm_clock)
+ + ticks_to_ns(s->delta));
+}
+
+static uint32_t hpet_ram_readb(void *opaque, target_phys_addr_t addr)
+{
+#ifdef HPET_DEBUG
+ fprintf(stderr, "qemu: hpet_read b at %#lx\n", addr);
+#endif
+ return 10;
+}
+
+static uint32_t hpet_ram_readw(void *opaque, target_phys_addr_t addr)
+{
+#ifdef HPET_DEBUG
+ fprintf(stderr, "qemu: hpet_read w at %#lx\n", addr);
+#endif
+ return 10;
+}
+
+static uint32_t hpet_ram_readl(void *opaque, target_phys_addr_t addr)
+{
+ HPETState *s = (HPETState *)opaque;
+#ifdef HPET_DEBUG
+ fprintf(stderr, "qemu: hpet_read l at %#lx\n", addr);
+#endif
+ switch(addr - HPET_BASE) {
+ case 0x00:
+ return 0x8086a201;
+ case 0x04:
+ return HPET_PERIOD;
+ case 0x10:
+ return ((s->legacy_route << 1) | s->enabled);
+ case 0x14:
+#ifdef HPET_DEBUG
+ fprintf(stderr, "qemu: invalid hpet_read l at %#lx\n",
addr);
+#endif
+ return 0;
+ case 0xf0:
+ s->hpet_counter = ns_to_ticks(qemu_get_clock(vm_clock)
+ - s->hpet_offset) ;
+#ifdef HPET_DEBUG
+ fprintf(stderr, "qemu: reading counter %#lx\n",
s->hpet_counter);
+#endif
+ return s->hpet_counter;
+ case 0xf4:
+ return 0;
+ case 0x20:
+ {
+ uint32_t retval = 0;
+ int i;
+ for(i=0; i<HPET_NUM_TIMERS; i++) {
+ if(s->timer[i].type == HPET_TIMER_TYPE_LEVEL)
+ retval |= s->timer[i].active << i;
+ }
+ return retval;
+ }
+ case 0x100 ... 0x3ff:
+ {
+ uint8_t timer_id = (addr - HPET_BASE - 0x100) / 0x20;
+ HPETTimer *timer = &s->timer[timer_id];
+
+ switch((addr - HPET_BASE - 0x100) % 0x20) {
+ case 0x0:
+ return ((timer->delivery ==
HPET_TIMER_DELIVERY_FSB) << 14)
+ | (timer->apic_port << 9)
+ | HPET_TIMER_CAP_PER_INT
+ | (timer->periodic << 3)
+ | (timer->enabled << 2)
+ | (timer->type << 1);
+ case 0x4: // Interrupt capabilities
+ return 0x00ff;
+ case 0x8: // comparator register
+ return timer->comparator;
+ case 0xc:
+ return 0x0;
+ }
+ }
+ break;
+ }
+
+#ifdef HPET_DEBUG
+ fprintf(stderr, "qemu: invalid hpet_read l at %#lx\n", addr);
+#endif
+ return 10;
+}
+
+static void hpet_ram_writeb(void *opaque, target_phys_addr_t addr,
+ uint32_t value)
+{
+#ifdef HPET_DEBUG
+ fprintf(stderr, "qemu: invalid hpet_write b at %#lx = %#x\n", addr,
value);
+#endif
+}
+
+static void hpet_ram_writew(void *opaque, target_phys_addr_t addr,
+ uint32_t value)
+{
+#ifdef HPET_DEBUG
+ fprintf(stderr, "qemu: invalid hpet_write w at %#lx = %#x\n", addr,
value);
+#endif
+}
+
+static void hpet_ram_writel(void *opaque, target_phys_addr_t addr,
+ uint32_t value)
+{
+ HPETState *s = (HPETState *)opaque;
+#ifdef HPET_DEBUG
+ fprintf(stderr, "qemu: hpet_write l at %#lx = %#x\n", addr, value);
+#endif
+ switch(addr - HPET_BASE) {
+ case 0x00:
+ return;
+ case 0x10:
+ if(value <= 3) {
+ s->enabled = value & 1;
+ if (s->enabled && s->updated) {
+ /* subtract hpet_counter in case it was set to a
non-zero
+ value */
+ s->hpet_offset = qemu_get_clock(vm_clock)
+ - ticks_to_ns(s->hpet_counter);
+ s->updated = 0;
+ }
+ s->legacy_route = (value >> 1) & 1;
+ hpet_legacy = s->legacy_route;
+ update_irq_all(s);
+ } else {
+#ifdef HPET_DEBUG
+ fprintf(stderr, "qemu: invalid hpet_write l at %#lx = %
#x\n",
+ addr, value);
+#endif
+ }
+ break;
+ case 0x14:
+#ifdef HPET_DEBUG
+ fprintf(stderr, "qemu: invalid hpet_write l at %#lx = %#x
\n", addr,
+ value);
+#endif
+ break;
+ case 0x20:
+ {
+ int i;
+ for(i=0; i<HPET_NUM_TIMERS; i++) {
+ if(s->timer[i].type == HPET_TIMER_TYPE_LEVEL) {
+ if(value & (1 << i)) {
+ s->timer[i].active = 0;
+ update_irq(&s->timer[i]);
+ }
+ }
+ }
+ }
+ break;
+ case 0xf0:
+ if (!s->enabled) {
+ s->hpet_counter = (s->hpet_counter & (0xffffffffULL <<
32))
+ | value;
+ s->updated = 1;
+#ifdef HPET_DEBUG
+ fprintf(stderr, "qemu: HPET counter 0xf0 set to %#x -> %#lx
\n",
+ value, s->hpet_counter);
+#endif
+ } else {
+ fprintf(stderr, "qemu: Invalid write while HPET enabled
\n");
+ }
+ break;
+ case 0xf4:
+ s->hpet_counter = (s->hpet_counter & 0xffffffffULL)
+ | (((uint64_t)value) << 32);
+#ifdef HPET_DEBUG
+ fprintf(stderr, "qemu: HPET counter 0xf4 set to %#x -> %#lx
\n",
+ value, s->hpet_counter);
+#endif
+ break;
+ case 0x100 ... 0x3ff:
+ {
+ uint8_t timer_id = (addr - HPET_BASE - 0x100) / 0x20;
+#ifdef HPET_DEBUG
+ fprintf(stderr, "qemu: hpet_write l timer_id = %#x
+ addr = %#lx\n", timer_id);
+#endif
+ HPETTimer *timer = &s->timer[timer_id];
+
+ switch((addr - HPET_BASE - 0x100) % 0x20) {
+ case 0x0:
+#ifdef HPET_DEBUG
+ fprintf(stderr, "qemu: hpet_write l timer reg =
%#x\n",
+ (addr - HPET_BASE - 0x100) % 0x20 );
+#endif
+ if(value & 1) break; // reserved
+ timer->delivery = (value >> 14) & 1;
+ timer->apic_port = (value >> 9) & 16;
+ timer->irq = s->irqs[timer->apic_port];
+ timer->periodic = (value >> 3) & 1;
+ timer->enabled = (value >> 2) & 1;
+ timer->type = (value >> 1) & 1;
+#ifdef HPET_DEBUG
+ fprintf(stderr, "qemu: hpet_write l at %#lx = %
#x\n",
+ addr, value);
+#endif
+ hpet_check(timer);
+ break;
+#ifdef HPET_DEBUG
+ case 0x4: // Interrupt capabilities
+ fprintf(stderr, "qemu: qemu case 0x4 invalid
+ hpet_write l at %#lx = %#x\n", addr,
value);
+ break;
+#endif
+ case 0x8: // comparator register
+#ifdef HPET_DEBUG
+ fprintf(stderr, "qemu: hpet_write l comparator
+ value %#x\n", value);
+#endif
+ timer->comparator = value;
+ timer->delta = value;
+ hpet_check(timer);
+ break;
+ case 0xc:
+#ifdef HPET_DEBUG
+ fprintf(stderr, "qemu: case 0xc invalid
hpet_write l
+ at %#lx = %#x\n", addr, value);
+#endif
+ break;
+ }
+ }
+ break;
+ default:
+ fprintf(stderr, "qemu: invalid hpet_write l at %#lx = %#x
\n",
+ addr, value);
+ }
+
+}
+
+static CPUReadMemoryFunc *hpet_ram_read[] = {
+ hpet_ram_readb,
+ hpet_ram_readw,
+ hpet_ram_readl,
+};
+
+static CPUWriteMemoryFunc *hpet_ram_write[] = {
+ hpet_ram_writeb,
+ hpet_ram_writew,
+ hpet_ram_writel,
+};
+
+
+void hpet_init(qemu_irq *irq) {
+ int iomemtype, i;
+ HPETState *s;
+
+ /* XXX this is a dirty hack for HPET support w/o LPC
+ Actually this is a config descriptor for the RCBA */
+ fprintf (stderr, "hpet_init\n");
+ s = qemu_mallocz(sizeof(HPETState));
+ s->irqs = irq;
+
+ for(i=0; i<HPET_NUM_TIMERS; i++) {
+ HPETTimer *timer = &s->timer[i];
+ timer->comparator = 0xffffffff;
+ timer->state = s;
+ timer->timer = qemu_new_timer(vm_clock, hpet_timer, s->timer
+i);
+ switch(i) {
+ case 0:
+ timer->apic_port = 0;
+ break;
+ case 1:
+ timer->apic_port = 8;
+ break;
+ default:
+ timer->apic_port = 0;
+ break;
+ }
+ s->timer[i].irq = irq[timer->apic_port];
+ }
+
+ /* 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..29dd599 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
@@ -369,7 +372,10 @@ static void pit_irq_timer_update(PITChannelState
*s, int64_t current_time)
return;
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);
+#if defined TARGET_I386 || defined TARGET_X86_64
+ if (!hpet_legacy)
+#endif
+ qemu_set_irq(s->irq, irq_level);
#ifdef DEBUG_PIT
printf("irq_level=%d next_delay=%f\n",
irq_level,
diff --git a/hw/mc146818rtc.c b/hw/mc146818rtc.c
index 30bb044..bb8c6a8 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
@@ -101,7 +105,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)
@@ -320,14 +327,20 @@ static void rtc_update_second2(void *opaque)
s->cmos_data[RTC_HOURS_ALARM] == s->current_tm.tm_hour)) {
s->cmos_data[RTC_REG_C] |= 0xa0;
- qemu_irq_raise(s->irq);
+#if defined TARGET_I386 || defined TARGET_X86_64
+ if (!hpet_legacy)
+#endif
+ qemu_irq_raise(s->irq);
}
}
/* update ended interrupt */
if (s->cmos_data[RTC_REG_B] & REG_B_UIE) {
s->cmos_data[RTC_REG_C] |= 0x90;
- qemu_irq_raise(s->irq);
+#if defined TARGET_I386 || defined TARGET_X86_64
+ if (!hpet_legacy)
+#endif
+ qemu_irq_raise(s->irq);
}
/* clear update in progress bit */
@@ -359,7 +372,11 @@ 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 99df09d..deefd2c 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -47,6 +47,8 @@
#define MAX_IDE_BUS 2
+void hpet_init(qemu_irq irq);
+
static fdctrl_t *floppy_controller;
static RTCState *rtc_state;
static PITState *pit;
@@ -923,6 +925,9 @@ static void pc_init1(ram_addr_t ram_size, int
vga_ram_size,
ioapic = ioapic_init();
}
pit = pit_init(0x40, i8259[0]);
+
+ hpet_init(i8259);
+
pcspk_init(pit);
if (pci_enabled) {
pic_set_alt_irq_func(isa_pic, ioapic_set_irq, ioapic);
--
Elizabeth Kon (Beth)
IBM Linux Technology Center
Open Hypervisor Team
email: eak@us.ibm.com
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [Qemu-devel] Re: [RFC][PATCH] Add HPET emulation to qemu
2008-07-10 3:48 [Qemu-devel] [RFC][PATCH] Add HPET emulation to qemu Beth Kon
@ 2008-07-10 6:36 ` Dor Laor
2008-07-10 9:18 ` [Qemu-devel] " Samuel Thibault
2008-07-12 15:42 ` [Qemu-devel] " Alexander Graf
2 siblings, 0 replies; 6+ messages in thread
From: Dor Laor @ 2008-07-10 6:36 UTC (permalink / raw)
To: Beth Kon; +Cc: qemu-devel, kvm, agraf
Beth Kon wrote:
> This patch, based on an earlier patch by Alexander Graf, adds HPET
> emulation to qemu. I am sending out a separate patch to kvm with the
> required bios changes.
>
> This work is incomplete.
>
> Currently working (at least generally):
> - linux 2.6.25.9 guest
>
>
> Todo:
> - other guest support (i.e. adding whatever may be missing for
> support of other modes of operation used by other OS's).
> - level-triggered interrupts
> - non-legacy routing
> - 64-bit operation
> - ...
>
>
Blessed addition :)
Just adding to your todo list:
- save/restore support
- guest reset handler
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [RFC][PATCH] Add HPET emulation to qemu
2008-07-10 3:48 [Qemu-devel] [RFC][PATCH] Add HPET emulation to qemu Beth Kon
2008-07-10 6:36 ` [Qemu-devel] " Dor Laor
@ 2008-07-10 9:18 ` Samuel Thibault
2008-07-10 19:14 ` Beth Kon
2008-07-12 15:42 ` [Qemu-devel] " Alexander Graf
2 siblings, 1 reply; 6+ messages in thread
From: Samuel Thibault @ 2008-07-10 9:18 UTC (permalink / raw)
To: qemu-devel; +Cc: agraf, kvm
Cool!
Does it now happen that qemu no longer wakes up every 10ms? If not,
please try to make sure it happens, that would eventually fix that power
leak :)
Samuel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [RFC][PATCH] Add HPET emulation to qemu
2008-07-10 9:18 ` [Qemu-devel] " Samuel Thibault
@ 2008-07-10 19:14 ` Beth Kon
0 siblings, 0 replies; 6+ messages in thread
From: Beth Kon @ 2008-07-10 19:14 UTC (permalink / raw)
To: qemu-devel; +Cc: agraf, kvm
On Thu, 2008-07-10 at 10:18 +0100, Samuel Thibault wrote:
> Cool!
> Does it now happen that qemu no longer wakes up every 10ms? If not,
> please try to make sure it happens, that would eventually fix that power
> leak :)
>
> Samuel
>
I will look into CONFIG_NO_HZ operation next. Haven't tried that yet.
>
--
Elizabeth Kon (Beth)
IBM Linux Technology Center
Open Hypervisor Team
email: eak@us.ibm.com
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Qemu-devel] Re: [RFC][PATCH] Add HPET emulation to qemu
2008-07-10 3:48 [Qemu-devel] [RFC][PATCH] Add HPET emulation to qemu Beth Kon
2008-07-10 6:36 ` [Qemu-devel] " Dor Laor
2008-07-10 9:18 ` [Qemu-devel] " Samuel Thibault
@ 2008-07-12 15:42 ` Alexander Graf
2008-07-22 17:17 ` Beth Kon
2 siblings, 1 reply; 6+ messages in thread
From: Alexander Graf @ 2008-07-12 15:42 UTC (permalink / raw)
To: Beth Kon; +Cc: qemu-devel, kvm
Hi Beth,
On Jul 10, 2008, at 5:48 AM, Beth Kon wrote:
> This patch, based on an earlier patch by Alexander Graf, adds HPET
> emulation to qemu. I am sending out a separate patch to kvm with the
> required bios changes.
>
> This work is incomplete.
Wow it's good to see that someone's working on it. I am pretty sure
that you're basing on an older version of my HPET emulation, so you
might also want to take a look at the current patch file residing in http://alex.csgraf.de/qemu/osxpatches.tar.bz2
> Currently working (at least generally):
> - linux 2.6.25.9 guest
>
>
> Todo:
> - other guest support (i.e. adding whatever may be missing for
> support of other modes of operation used by other OS's).
> - level-triggered interrupts
Look at my current version for that. I only have Level support in
there for now though.
>
> - non-legacy routing
> - 64-bit operation
> - ...
While reading through the code I realized how badly commented it is.
At least the functions should have some comments on them what their
purpose is.
Furthermore there still are a lot of magic numbers in the code. While
that is "normal qemu code style" and I wrote it this way, I'm not too
fond of it. So it might be a good idea to at least make the switch
numbers defines.
>
>
> Basically what I've done so far is make it work for linux.
>
> The one area that feels ugly/wrong at the moment is handling the
> disabling of 8254 and RTC timer interrupts when operating in legacy
> mode. The HPET spec says "in this case the 8254/RTC timer will not
> cause
> any interrupts". I'm not sure if I should disable the RTC/8254 in some
> more general way, or just disable interrupts. Comments appreciated.
IIRC the spec defines that the HPET _can_ replace the 8254, but does
not have to. So you should be mostly fine on that part.
>
>
> Signed-off-by: Beth Kon <bkon@us.ibm.com>
>
> diffstat output:
> Makefile.target | 2
> hw/hpet.c | 393
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> hw/i8254.c | 8 -
> hw/mc146818rtc.c | 25 ++-
> hw/pc.c | 5
> 5 files changed, 427 insertions(+), 6 deletions(-)
>
>
> diff --git a/Makefile.target b/Makefile.target
> index 73adbb1..05829ea 100644
> --- a/Makefile.target
> +++ b/Makefile.target
> @@ -530,7 +530,7 @@ ifeq ($(TARGET_BASE_ARCH), i386)
> OBJS+= ide.o pckbd.o ps2.o vga.o $(SOUND_HW) dma.o
> OBJS+= fdc.o mc146818rtc.o serial.o i8259.o i8254.o pcspk.o pc.o
> OBJS+= cirrus_vga.o apic.o parallel.o acpi.o piix_pci.o
> -OBJS+= usb-uhci.o vmmouse.o vmport.o vmware_vga.o
> +OBJS+= usb-uhci.o vmmouse.o vmport.o vmware_vga.o hpet.o
> CPPFLAGS += -DHAS_AUDIO -DHAS_AUDIO_CHOICE
> endif
> ifeq ($(TARGET_BASE_ARCH), ppc)
> diff --git a/hw/hpet.c b/hw/hpet.c
> new file mode 100644
> index 0000000..e74de08
> --- /dev/null
> +++ b/hw/hpet.c
> @@ -0,0 +1,393 @@
> +/*
> + * High Precisition Event Timer emulation
> + *
> + * Copyright (c) 2007 Alexander Graf
> + * Copyright (c) 2008 IBM Corporation
> + *
> + * Authors: Beth Kon <bkon@us.ibm.com>
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, write to the Free
> Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA
> 02111-1307
> USA
> + *
> + * *****************************************************************
> + *
> + * This driver attempts to emulate an HPET device in software. It
> is by
> no
> + * means complete and is prone to break on certain conditions.
> + *
> + */
> +#include "hw.h"
> +#include "console.h"
> +#include "qemu-timer.h"
> +
> +//#define HPET_DEBUG
> +
> +#define HPET_BASE 0xfed00000
This is a dirty hack that I did to make Mac OS X happy. Actually the
HPET base address gets specified in the RCBA on the LPC and is
configured by the BIOS to point to a valid address, with 0xfed00000
being the default (IIRC if you write 0 to the fields you end up with
that address).
>
> +#define HPET_PERIOD 0x00989680 /* 10000000 femptoseconds,
> 10ns*/
Any reason why this is a hex value? I find 10000000 a lot easier to
read :-)
>
> +
> +#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)
> +
> +struct HPETState;
> +typedef struct HPETTimer {
> + QEMUTimer *timer;
> + struct HPETState *state;
> + uint8_t type;
> + uint8_t active;
> + uint8_t delivery;
> + uint8_t apic_port;
> + uint8_t periodic;
> + uint8_t enabled;
> + uint32_t comparator; // if(hpet_counter == comparator) IRQ();
> + uint32_t delta;
> + qemu_irq irq;
> +} HPETTimer;
> +
> +typedef struct HPETState {
> + uint64_t hpet_counter;
> + uint64_t hpet_offset;
> + int64_t next_periodic_time;
> + uint8_t legacy_route;
> + uint8_t enabled;
> + uint8_t updated;
> + qemu_irq *irqs;
> + HPETTimer timer[HPET_NUM_TIMERS];
> +} HPETState;
> +
> +
> +int hpet_legacy;
> +
> +static void update_irq(struct HPETTimer *timer)
> +{
> + if(timer->enabled && timer->state->enabled) {
> + qemu_irq_pulse(timer->irq);
> + }
> +}
> +
> +static void update_irq_all(struct HPETState *s)
> +{
> + int i;
> + for(i=0; i<HPET_NUM_TIMERS; i++)
> + update_irq(&s->timer[i]);
> +}
> +
> +static inline int64_t ticks_to_ns(int64_t value)
> +{
> + return (value * HPET_PERIOD / FS_PER_NS);
> +}
> +
> +static inline int64_t ns_to_ticks(int64_t value)
> +{
> + return (value * FS_PER_NS / HPET_PERIOD);
> +}
> +
> +static void hpet_timer(void *opaque)
> +{
> + HPETTimer *s = (HPETTimer*)opaque;
> + if(s->periodic) {
> + s->comparator += s->delta;
> + qemu_mod_timer(s->timer, qemu_get_clock(vm_clock)
> + + ticks_to_ns(s->delta));
> + }
> + update_irq(s);
> +}
> +
> +static void hpet_check(HPETTimer *s)
> +{
> + qemu_mod_timer(s->timer, qemu_get_clock(vm_clock)
> + + ticks_to_ns(s->delta));
> +}
> +
> +static uint32_t hpet_ram_readb(void *opaque, target_phys_addr_t addr)
> +{
> +#ifdef HPET_DEBUG
> + fprintf(stderr, "qemu: hpet_read b at %#lx\n", addr);
> +#endif
> + return 10;
> +}
> +
> +static uint32_t hpet_ram_readw(void *opaque, target_phys_addr_t addr)
> +{
> +#ifdef HPET_DEBUG
> + fprintf(stderr, "qemu: hpet_read w at %#lx\n", addr);
> +#endif
> + return 10;
> +}
If I'm not completely mistaken, all reads and writes need to be in 32-
or 64-bit mode. So it's pretty safe to remove these. I only added them
to see if Mac OS X actually would access them. To still enable other
people to do the same you might as well ifdef them out.
>
> +
> +static uint32_t hpet_ram_readl(void *opaque, target_phys_addr_t addr)
> +{
> + HPETState *s = (HPETState *)opaque;
> +#ifdef HPET_DEBUG
> + fprintf(stderr, "qemu: hpet_read l at %#lx\n", addr);
> +#endif
> + switch(addr - HPET_BASE) {
> + case 0x00:
> + return 0x8086a201;
> + case 0x04:
> + return HPET_PERIOD;
> + case 0x10:
> + return ((s->legacy_route << 1) | s->enabled);
> + case 0x14:
> +#ifdef HPET_DEBUG
> + fprintf(stderr, "qemu: invalid hpet_read l at %#lx\n",
> addr);
> +#endif
> + return 0;
> + case 0xf0:
> + s->hpet_counter = ns_to_ticks(qemu_get_clock(vm_clock)
> + - s->hpet_offset) ;
I'm having trouble understanding this part. The hpet_counter is
actually the ticks of the internal main clock of the HPET. This value
is actually supposed to constantly change wrt to the current time. The
"timers" in the HPET can now compare themselves to the "current value"
of the hpet_counter at all times, rising an interrupt if something
matches.
So far for the theory. I thought that it'd be a lot more convenient to
simply write down an internal offset (hpet_counter) to the actual
clock and base all calculations on that, so we can actually trigger a
timer based on event_time - offset. I don't see any reason to set
hpet_counter again, but maybe it's been too long that I've looked at
that code :).
Hum ... having looked further, is that what hpet_offset is supposed to
be and the reason you're setting s->updated?
>
> +#ifdef HPET_DEBUG
> + fprintf(stderr, "qemu: reading counter %#lx\n",
> s->hpet_counter);
> +#endif
> + return s->hpet_counter;
> + case 0xf4:
> + return 0;
> + case 0x20:
> + {
> + uint32_t retval = 0;
> + int i;
> + for(i=0; i<HPET_NUM_TIMERS; i++) {
> + if(s->timer[i].type == HPET_TIMER_TYPE_LEVEL)
> + retval |= s->timer[i].active << i;
> + }
> + return retval;
> + }
> + case 0x100 ... 0x3ff:
> + {
> + uint8_t timer_id = (addr - HPET_BASE - 0x100) / 0x20;
> + HPETTimer *timer = &s->timer[timer_id];
> +
> + switch((addr - HPET_BASE - 0x100) % 0x20) {
> + case 0x0:
> + return ((timer->delivery ==
> HPET_TIMER_DELIVERY_FSB) << 14)
> + | (timer->apic_port << 9)
> + | HPET_TIMER_CAP_PER_INT
> + | (timer->periodic << 3)
> + | (timer->enabled << 2)
> + | (timer->type << 1);
> + case 0x4: // Interrupt capabilities
> + return 0x00ff;
> + case 0x8: // comparator register
> + return timer->comparator;
> + case 0xc:
> + return 0x0;
> + }
> + }
> + break;
> + }
> +
> +#ifdef HPET_DEBUG
> + fprintf(stderr, "qemu: invalid hpet_read l at %#lx\n", addr);
> +#endif
> + return 10;
> +}
> +
> +static void hpet_ram_writeb(void *opaque, target_phys_addr_t addr,
> + uint32_t value)
> +{
> +#ifdef HPET_DEBUG
> + fprintf(stderr, "qemu: invalid hpet_write b at %#lx = %#x\n",
> addr,
> value);
> +#endif
> +}
> +
> +static void hpet_ram_writew(void *opaque, target_phys_addr_t addr,
> + uint32_t value)
> +{
> +#ifdef HPET_DEBUG
> + fprintf(stderr, "qemu: invalid hpet_write w at %#lx = %#x\n",
> addr,
> value);
> +#endif
> +}
> +
> +static void hpet_ram_writel(void *opaque, target_phys_addr_t addr,
> + uint32_t value)
> +{
> + HPETState *s = (HPETState *)opaque;
> +#ifdef HPET_DEBUG
> + fprintf(stderr, "qemu: hpet_write l at %#lx = %#x\n", addr,
> value);
> +#endif
> + switch(addr - HPET_BASE) {
> + case 0x00:
> + return;
> + case 0x10:
> + if(value <= 3) {
If I may cite Avi:Algorithmic and Bit-operations don't mix well. I
don't really see why we shouldn't interpret the first two bits when
any bit > 1 is set. If I read the spec correctly, the values are
reserved and should not be used, don't really make anything fail for
now though. This check would make the operation a noop though! A
warning would be nice though.
Looking at my code, it seems I did the same - so blame me if anyone
asks :-).
>
> + s->enabled = value & 1;
> + if (s->enabled && s->updated) {
> + /* subtract hpet_counter in case it was set to a
> non-zero
> + value */
> + s->hpet_offset = qemu_get_clock(vm_clock)
> + - ticks_to_ns(s->hpet_counter);
> + s->updated = 0;
> + }
> + s->legacy_route = (value >> 1) & 1;
> + hpet_legacy = s->legacy_route;
Is this what real hardware does? As soon as the HPET is there and
someone sets a random bit on it suddenly other completely unrelated
devices cease to work? I kinda doubt that.
>
> + update_irq_all(s);
> + } else {
> +#ifdef HPET_DEBUG
> + fprintf(stderr, "qemu: invalid hpet_write l at %#lx
> = %
> #x\n",
> + addr, value);
> +#endif
> + }
> + break;
> + case 0x14:
> +#ifdef HPET_DEBUG
> + fprintf(stderr, "qemu: invalid hpet_write l at %#lx = %#x
> \n", addr,
> + value);
> +#endif
> + break;
> + case 0x20:
> + {
> + int i;
> + for(i=0; i<HPET_NUM_TIMERS; i++) {
> + if(s->timer[i].type == HPET_TIMER_TYPE_LEVEL) {
> + if(value & (1 << i)) {
> + s->timer[i].active = 0;
> + update_irq(&s->timer[i]);
> + }
> + }
> + }
> + }
> + break;
> + case 0xf0:
> + if (!s->enabled) {
> + s->hpet_counter = (s->hpet_counter & (0xffffffffULL <<
> 32))
> + | value;
> + s->updated = 1;
> +#ifdef HPET_DEBUG
> + fprintf(stderr, "qemu: HPET counter 0xf0 set to %#x ->
> %#lx
> \n",
> + value, s->hpet_counter);
> +#endif
> + } else {
> + fprintf(stderr, "qemu: Invalid write while HPET
> enabled
> \n");
> + }
> + break;
> + case 0xf4:
> + s->hpet_counter = (s->hpet_counter & 0xffffffffULL)
> + | (((uint64_t)value) << 32);
> +#ifdef HPET_DEBUG
> + fprintf(stderr, "qemu: HPET counter 0xf4 set to %#x ->
> %#lx
> \n",
> + value, s->hpet_counter);
> +#endif
> + break;
> + case 0x100 ... 0x3ff:
> + {
> + uint8_t timer_id = (addr - HPET_BASE - 0x100) / 0x20;
> +#ifdef HPET_DEBUG
> + fprintf(stderr, "qemu: hpet_write l timer_id = %#x
> + addr = %#lx\n", timer_id);
> +#endif
> + HPETTimer *timer = &s->timer[timer_id];
> +
> + switch((addr - HPET_BASE - 0x100) % 0x20) {
> + case 0x0:
> +#ifdef HPET_DEBUG
> + fprintf(stderr, "qemu: hpet_write l timer
> reg =
> %#x\n",
> + (addr - HPET_BASE - 0x100) % 0x20 );
> +#endif
> + if(value & 1) break; // reserved
> + timer->delivery = (value >> 14) & 1;
> + timer->apic_port = (value >> 9) & 16;
> + timer->irq = s->irqs[timer->apic_port];
> + timer->periodic = (value >> 3) & 1;
> + timer->enabled = (value >> 2) & 1;
> + timer->type = (value >> 1) & 1;
> +#ifdef HPET_DEBUG
> + fprintf(stderr, "qemu: hpet_write l at %#lx
> = %
> #x\n",
> + addr, value);
> +#endif
> + hpet_check(timer);
> + break;
> +#ifdef HPET_DEBUG
> + case 0x4: // Interrupt capabilities
> + fprintf(stderr, "qemu: qemu case 0x4 invalid
> + hpet_write l at %#lx = %#x\n", addr,
> value);
> + break;
> +#endif
> + case 0x8: // comparator register
> +#ifdef HPET_DEBUG
> + fprintf(stderr, "qemu: hpet_write l
> comparator
> + value %#x\n", value);
> +#endif
> + timer->comparator = value;
> + timer->delta = value;
> + hpet_check(timer);
> + break;
> + case 0xc:
> +#ifdef HPET_DEBUG
> + fprintf(stderr, "qemu: case 0xc invalid
> hpet_write l
> + at %#lx = %#x\n", addr, value);
> +#endif
> + break;
> + }
> + }
> + break;
> + default:
> + fprintf(stderr, "qemu: invalid hpet_write l at %#lx = %#x
> \n",
> + addr, value);
> + }
> +
> +}
> +
> +static CPUReadMemoryFunc *hpet_ram_read[] = {
> + hpet_ram_readb,
> + hpet_ram_readw,
> + hpet_ram_readl,
> +};
> +
> +static CPUWriteMemoryFunc *hpet_ram_write[] = {
> + hpet_ram_writeb,
> + hpet_ram_writew,
> + hpet_ram_writel,
> +};
> +
> +
> +void hpet_init(qemu_irq *irq) {
> + int iomemtype, i;
> + HPETState *s;
> +
> + /* XXX this is a dirty hack for HPET support w/o LPC
> + Actually this is a config descriptor for the RCBA */
> + fprintf (stderr, "hpet_init\n");
> + s = qemu_mallocz(sizeof(HPETState));
> + s->irqs = irq;
> +
> + for(i=0; i<HPET_NUM_TIMERS; i++) {
> + HPETTimer *timer = &s->timer[i];
> + timer->comparator = 0xffffffff;
> + timer->state = s;
> + timer->timer = qemu_new_timer(vm_clock, hpet_timer, s->timer
> +i);
> + switch(i) {
> + case 0:
> + timer->apic_port = 0;
Isn't this IRQ2?
[snip]
Alex
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Qemu-devel] Re: [RFC][PATCH] Add HPET emulation to qemu
2008-07-12 15:42 ` [Qemu-devel] " Alexander Graf
@ 2008-07-22 17:17 ` Beth Kon
0 siblings, 0 replies; 6+ messages in thread
From: Beth Kon @ 2008-07-22 17:17 UTC (permalink / raw)
To: Alexander Graf; +Cc: qemu-devel, kvm
On Sat, 2008-07-12 at 17:42 +0200, Alexander Graf wrote:
> Hi Beth,
>
> On Jul 10, 2008, at 5:48 AM, Beth Kon wrote:
>
> > This patch, based on an earlier patch by Alexander Graf, adds HPET
> > emulation to qemu. I am sending out a separate patch to kvm with the
> > required bios changes.
> >
> > This work is incomplete.
>
> Wow it's good to see that someone's working on it. I am pretty sure
> that you're basing on an older version of my HPET emulation, so you
> might also want to take a look at the current patch file residing in http://alex.csgraf.de/qemu/osxpatches.tar.bz2
<snip>
Hi Alex. Thanks for the feedback! Sorry for the delayed response, I've
been on vacation. I did check the patch you pointed me to and it is
actually the same one that I started with.
>
> While reading through the code I realized how badly commented it is.
> At least the functions should have some comments on them what their
> purpose is.
> Furthermore there still are a lot of magic numbers in the code. While
> that is "normal qemu code style" and I wrote it this way, I'm not too
> fond of it. So it might be a good idea to at least make the switch
> numbers defines.
>
Ok... added those things to my todo list :-)
> >
> > The one area that feels ugly/wrong at the moment is handling the
> > disabling of 8254 and RTC timer interrupts when operating in legacy
> > mode. The HPET spec says "in this case the 8254/RTC timer will not
> > cause
> > any interrupts". I'm not sure if I should disable the RTC/8254 in some
> > more general way, or just disable interrupts. Comments appreciated.
>
> IIRC the spec defines that the HPET _can_ replace the 8254, but does
> not have to. So you should be mostly fine on that part.
>
> >
> > +
> > +//#define HPET_DEBUG
> > +
> > +#define HPET_BASE 0xfed00000
>
> This is a dirty hack that I did to make Mac OS X happy. Actually the
> HPET base address gets specified in the RCBA on the LPC and is
> configured by the BIOS to point to a valid address, with 0xfed00000
> being the default (IIRC if you write 0 to the fields you end up with
> that address).
Yes, Ryan Harper's BIOS patch that was submitted with this patch
specified the HPET address in ACPI. I am not familiar with this stuff,
so not sure how that relates to the RCBA and whether more needs to be
done here. For the time being I'll add it to the todo list.
>
> >
> > +#define HPET_PERIOD 0x00989680 /* 10000000 femptoseconds,
> > 10ns*/
>
> Any reason why this is a hex value? I find 10000000 a lot easier to
> read :-)
>
Well that's a VERY good question! Job security? :-)
> >
> >
> > +static uint32_t hpet_ram_readw(void *opaque, target_phys_addr_t addr)
> > +{
> > +#ifdef HPET_DEBUG
> > + fprintf(stderr, "qemu: hpet_read w at %#lx\n", addr);
> > +#endif
> > + return 10;
> > +}
>
> If I'm not completely mistaken, all reads and writes need to be in 32-
> or 64-bit mode. So it's pretty safe to remove these. I only added them
> to see if Mac OS X actually would access them. To still enable other
> people to do the same you might as well ifdef them out.
>
Yep, you're right. I'll do that.
<snip>
> >
> > +
> > +static uint32_t hpet_ram_readl(void *opaque, target_phys_addr_t addr)
> > +{
> > + HPETState *s = (HPETState *)opaque;
> > +#ifdef HPET_DEBUG
> > + fprintf(stderr, "qemu: hpet_read l at %#lx\n", addr);
> > +#endif
> > + switch(addr - HPET_BASE) {
> > + case 0x00:
> > + return 0x8086a201;
> > + case 0x04:
> > + return HPET_PERIOD;
> > + case 0x10:
> > + return ((s->legacy_route << 1) | s->enabled);
> > + case 0x14:
> > +#ifdef HPET_DEBUG
> > + fprintf(stderr, "qemu: invalid hpet_read l at %#lx\n",
> > addr);
> > +#endif
> > + return 0;
> > + case 0xf0:
> > + s->hpet_counter = ns_to_ticks(qemu_get_clock(vm_clock)
> > + - s->hpet_offset) ;
>
> I'm having trouble understanding this part. The hpet_counter is
> actually the ticks of the internal main clock of the HPET. This value
> is actually supposed to constantly change wrt to the current time. The
> "timers" in the HPET can now compare themselves to the "current value"
> of the hpet_counter at all times, rising an interrupt if something
> matches.
>
> So far for the theory. I thought that it'd be a lot more convenient to
> simply write down an internal offset (hpet_counter) to the actual
> clock and base all calculations on that, so we can actually trigger a
> timer based on event_time - offset. I don't see any reason to set
> hpet_counter again, but maybe it's been too long that I've looked at
> that code :).
>
> Hum ... having looked further, is that what hpet_offset is supposed to
> be and the reason you're setting s->updated?
Ok, let me explain my thinking. The hpet_counter tracks elapsed ticks,
but these ticks are emulated based on the value of the vm_clock. So to
find the elapsed ticks, I have to determine how many nanoseconds have
elapsed since the hpet_counter was enabled by software, then translate
nanoseconds to ticks. The number of elapsed nanoseconds equals the
current value of the vm_clock minus the value of the vm_clock when the
hpet_counter was last updated and enabled by software
(qemu_get_clock(vm_clock)-s->hpet_offset). s->updated marks when
software has updated the value of the counter, so that when the counter
is next enabled, hpet_offset (current value of vm_clock) can be saved. I
need to add code to handle the HPET being disabled then enabled without
the counter value being updated. This should just stop the clock
temporarily, so will require different modification to hpet_offset than
the case where the value of hpet_counter has been changed.
Whenever the hpet counter is read, it should return the current value of
elapsed ticks, which must be derived, as described above, from
qemu_get_clock. This is the only way for the guest to perceive that the
clock ticks are occurring.
Looking over the code though, I realize I oversimplified the comparator
stuff a little too much. The comparator doesn't necessarily equal a
delta from the current time. I'll need to translate the comparator value
to ticks and compare to the latest hpet_counter value to find the
"delta". A bit messy but I see no way around it.
>
> >
<snip>
> > + switch(addr - HPET_BASE) {
> > + case 0x00:
> > + return;
> > + case 0x10:
> > + if(value <= 3) {
>
> If I may cite Avi:Algorithmic and Bit-operations don't mix well. I
> don't really see why we shouldn't interpret the first two bits when
> any bit > 1 is set. If I read the spec correctly, the values are
> reserved and should not be used, don't really make anything fail for
> now though. This check would make the operation a noop though! A
> warning would be nice though.
>
> Looking at my code, it seems I did the same - so blame me if anyone
> asks :-).
>
Yeah, I just took that from you :-) But I agree. I'll change it.
> >
> > + s->enabled = value & 1;
> > + if (s->enabled && s->updated) {
> > + /* subtract hpet_counter in case it was set to a
> > non-zero
> > + value */
> > + s->hpet_offset = qemu_get_clock(vm_clock)
> > + - ticks_to_ns(s->hpet_counter);
> > + s->updated = 0;
> > + }
> > + s->legacy_route = (value >> 1) & 1;
> > + hpet_legacy = s->legacy_route;
>
> Is this what real hardware does? As soon as the HPET is there and
> someone sets a random bit on it suddenly other completely unrelated
> devices cease to work? I kinda doubt that.
I don't know what real hardware does. But the spec says if legacy
replacement option is set, "In this case, the 8254 timer will not cause
any interrupts." and "In this case the RTC will not cause any
interrupts". The spec also has a figure (figure 7) with the LEG_RT_EN
bit controlling a multiplexer between the hpet and the 8254. So that
would appear immediate to me. Any other ideas on how this should be
handled?
>
<snip>
> > + for(i=0; i<HPET_NUM_TIMERS; i++) {
> > + HPETTimer *timer = &s->timer[i];
> > + timer->comparator = 0xffffffff;
> > + timer->state = s;
> > + timer->timer = qemu_new_timer(vm_clock, hpet_timer, s->timer
> > +i);
> > + switch(i) {
> > + case 0:
> > + timer->apic_port = 0;
>
> Isn't this IRQ2?
Well, the weird thing is that both 0 and 2 work. I see in the qemu code
that the i8259 interrupts get routed through the apic (alt_irq_func) but
haven't dug into the details and don't understand why both work. But
since the "initial" mapping is to the 8259, 0 seemed appropriate. I'll
add a note to look into this further.
>
> [snip]
>
> Alex
>
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Elizabeth Kon (Beth)
IBM Linux Technology Center
Open Hypervisor Team
email: eak@us.ibm.com
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2008-07-22 17:22 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-07-10 3:48 [Qemu-devel] [RFC][PATCH] Add HPET emulation to qemu Beth Kon
2008-07-10 6:36 ` [Qemu-devel] " Dor Laor
2008-07-10 9:18 ` [Qemu-devel] " Samuel Thibault
2008-07-10 19:14 ` Beth Kon
2008-07-12 15:42 ` [Qemu-devel] " Alexander Graf
2008-07-22 17:17 ` Beth Kon
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).