From: Jan Kiszka <jan.kiszka@web.de>
To: Blue Swirl <blauwirbel@gmail.com>
Cc: qemu-devel <qemu-devel@nongnu.org>
Subject: [Qemu-devel] Re: [PATCH, RFC 2/4] hpet: don't use any static state
Date: Sun, 23 May 2010 17:40:39 +0200 [thread overview]
Message-ID: <4BF94C77.8060801@web.de> (raw)
In-Reply-To: <AANLkTikJsUKGDA_TNeg1AUGvojkGbkg8UIJDe9Ho1qoP@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 11817 bytes --]
Blue Swirl wrote:
> Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
> ---
> hw/hpet.c | 68 +++++++++++++++++++++++++++++--------------------------
> hw/hpet_emul.h | 4 +-
> hw/pc.c | 8 ++++--
> hw/pc.h | 3 +-
> hw/pc_piix.c | 3 +-
> 5 files changed, 47 insertions(+), 39 deletions(-)
>
> diff --git a/hw/hpet.c b/hw/hpet.c
> index 8729fb2..f24e054 100644
> --- a/hw/hpet.c
> +++ b/hw/hpet.c
> @@ -37,14 +37,11 @@
> #define DPRINTF(...)
> #endif
>
> -static HPETState *hpet_statep;
> -
> -uint32_t hpet_in_legacy_mode(void)
> +uint32_t hpet_in_legacy_mode(void *opaque)
uint32_t hpet_in_legacy_mode(HPETState *s)
please (will become DeviceState with my patches, but it should not be
void in any case).
> {
> - if (hpet_statep)
> - return hpet_statep->config & HPET_CFG_LEGACY;
> - else
> - return 0;
> + HPETState *s = opaque;
> +
> + return s->config & HPET_CFG_LEGACY;
> }
>
> static uint32_t timer_int_route(struct HPETTimer *timer)
> @@ -54,9 +51,9 @@ static uint32_t timer_int_route(struct HPETTimer *timer)
> return route;
> }
>
> -static uint32_t hpet_enabled(void)
> +static uint32_t hpet_enabled(HPETState *s)
> {
> - return hpet_statep->config & HPET_CFG_ENABLE;
> + return s->config & HPET_CFG_ENABLE;
> }
>
> static uint32_t timer_is_periodic(HPETTimer *t)
> @@ -106,10 +103,10 @@ static int deactivating_bit(uint64_t old,
> uint64_t new, uint64_t mask)
> return ((old & mask) && !(new & mask));
> }
>
> -static uint64_t hpet_get_ticks(void)
> +static uint64_t hpet_get_ticks(HPETState *s)
> {
> uint64_t ticks;
> - ticks = ns_to_ticks(qemu_get_clock(vm_clock) + hpet_statep->hpet_offset);
> + ticks = ns_to_ticks(qemu_get_clock(vm_clock) + s->hpet_offset);
> return ticks;
> }
>
> @@ -139,7 +136,7 @@ static void update_irq(struct HPETTimer *timer)
> qemu_irq irq;
> int route;
>
> - if (timer->tn <= 1 && hpet_in_legacy_mode()) {
> + if (timer->tn <= 1 && hpet_in_legacy_mode(timer->state)) {
> /* 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.
> @@ -152,7 +149,7 @@ static void update_irq(struct HPETTimer *timer)
> route=timer_int_route(timer);
> irq=timer->state->irqs[route];
> }
> - if (timer_enabled(timer) && hpet_enabled()) {
> + if (timer_enabled(timer) && hpet_enabled(timer->state)) {
> qemu_irq_pulse(irq);
> }
> }
> @@ -161,7 +158,7 @@ static void hpet_pre_save(void *opaque)
> {
> HPETState *s = opaque;
> /* save current counter value */
> - s->hpet_counter = hpet_get_ticks();
> + s->hpet_counter = hpet_get_ticks(s);
> }
>
> static int hpet_post_load(void *opaque, int version_id)
> @@ -216,7 +213,7 @@ static void hpet_timer(void *opaque)
> uint64_t diff;
>
> uint64_t period = t->period;
> - uint64_t cur_tick = hpet_get_ticks();
> + uint64_t cur_tick = hpet_get_ticks(t->state);
>
> if (timer_is_periodic(t) && period != 0) {
> if (t->config & HPET_TN_32BIT) {
> @@ -244,7 +241,7 @@ static void hpet_set_timer(HPETTimer *t)
> {
> uint64_t diff;
> uint32_t wrap_diff; /* how many ticks until we wrap? */
> - uint64_t cur_tick = hpet_get_ticks();
> + uint64_t cur_tick = hpet_get_ticks(t->state);
>
> /* whenever new timer is being set up, make sure wrap_flag is 0 */
> t->wrap_flag = 0;
> @@ -326,17 +323,19 @@ static uint32_t hpet_ram_readl(void *opaque,
> target_phys_addr_t addr)
> DPRINTF("qemu: invalid HPET_CFG + 4 hpet_ram_readl \n");
> return 0;
> case HPET_COUNTER:
> - if (hpet_enabled())
> - cur_tick = hpet_get_ticks();
> - else
> + if (hpet_enabled(s)) {
> + cur_tick = hpet_get_ticks(s);
> + } else {
> cur_tick = s->hpet_counter;
> + }
> DPRINTF("qemu: reading counter = %" PRIx64 "\n", cur_tick);
> return cur_tick;
> case HPET_COUNTER + 4:
> - if (hpet_enabled())
> - cur_tick = hpet_get_ticks();
> - else
> + if (hpet_enabled(s)) {
> + cur_tick = hpet_get_ticks(s);
> + } else {
> cur_tick = s->hpet_counter;
> + }
> DPRINTF("qemu: reading counter + 4 = %" PRIx64 "\n",
> cur_tick);
> return cur_tick >> 32;
> case HPET_STATUS:
> @@ -419,8 +418,9 @@ static void hpet_ram_writel(void *opaque,
> target_phys_addr_t addr,
> | new_val;
> }
> timer->config &= ~HPET_TN_SETVAL;
> - if (hpet_enabled())
> + if (hpet_enabled(s)) {
> hpet_set_timer(timer);
> + }
> break;
> case HPET_TN_CMP + 4: // comparator register high order
> DPRINTF("qemu: hpet_ram_writel HPET_TN_CMP + 4\n");
> @@ -439,8 +439,9 @@ static void hpet_ram_writel(void *opaque,
> target_phys_addr_t addr,
> | new_val << 32;
> }
> timer->config &= ~HPET_TN_SETVAL;
> - if (hpet_enabled())
> + if (hpet_enabled(s)) {
> hpet_set_timer(timer);
> + }
> break;
> case HPET_TN_ROUTE + 4:
> DPRINTF("qemu: hpet_ram_writel HPET_TN_ROUTE + 4\n");
> @@ -467,7 +468,7 @@ static void hpet_ram_writel(void *opaque,
> target_phys_addr_t addr,
> }
> else if (deactivating_bit(old_val, new_val, HPET_CFG_ENABLE)) {
> /* Halt main counter and disable interrupt generation. */
> - s->hpet_counter = hpet_get_ticks();
> + s->hpet_counter = hpet_get_ticks(s);
> for (i = 0; i < HPET_NUM_TIMERS; i++)
> hpet_del_timer(&s->timer[i]);
> }
> @@ -485,16 +486,18 @@ static void hpet_ram_writel(void *opaque,
> target_phys_addr_t addr,
> /* FIXME: need to handle level-triggered interrupts */
> break;
> case HPET_COUNTER:
> - if (hpet_enabled())
> - printf("qemu: Writing counter while HPET enabled!\n");
> + if (hpet_enabled(s)) {
> + printf("qemu: Writing counter while HPET enabled!\n");
Missed it in my series as well: Should become DPRINTF at this chance.
Also below.
> + }
> s->hpet_counter = (s->hpet_counter & 0xffffffff00000000ULL)
> | value;
> DPRINTF("qemu: HPET counter written. ctr = %#x -> %"
> PRIx64 "\n",
> value, s->hpet_counter);
> break;
> case HPET_COUNTER + 4:
> - if (hpet_enabled())
> - printf("qemu: Writing counter while HPET enabled!\n");
> + if (hpet_enabled(s)) {
> + printf("qemu: Writing counter while HPET enabled!\n");
> + }
> s->hpet_counter = (s->hpet_counter & 0xffffffffULL)
> | (((uint64_t)value) << 32);
> DPRINTF("qemu: HPET counter + 4 written. ctr = %#x ->
> %" PRIx64 "\n",
> @@ -563,15 +566,14 @@ static void hpet_reset(void *opaque) {
> count = 1;
> }
>
> -
> -void hpet_init(qemu_irq *irq) {
> +HPETState *hpet_init(qemu_irq *irq)
> +{
> int i, iomemtype;
> HPETState *s;
>
> DPRINTF ("hpet_init\n");
>
> s = qemu_mallocz(sizeof(HPETState));
> - hpet_statep = s;
> s->irqs = irq;
> for (i=0; i<HPET_NUM_TIMERS; i++) {
> HPETTimer *timer = &s->timer[i];
> @@ -583,4 +585,6 @@ void hpet_init(qemu_irq *irq) {
> iomemtype = cpu_register_io_memory(hpet_ram_read,
> hpet_ram_write, s);
> cpu_register_physical_memory(HPET_BASE, 0x400, iomemtype);
> +
> + return s;
> }
> diff --git a/hw/hpet_emul.h b/hw/hpet_emul.h
> index cfd95b4..88dbf0d 100644
> --- a/hw/hpet_emul.h
> +++ b/hw/hpet_emul.h
> @@ -75,8 +75,8 @@ typedef struct HPETState {
> } HPETState;
>
> #if defined TARGET_I386
> -extern uint32_t hpet_in_legacy_mode(void);
> -extern void hpet_init(qemu_irq *irq);
> +uint32_t hpet_in_legacy_mode(void *opaque);
> +HPETState *hpet_init(qemu_irq *irq);
> #endif
>
> #endif
> diff --git a/hw/pc.c b/hw/pc.c
> index 5a703e1..9f1a9d6 100644
> --- a/hw/pc.c
> +++ b/hw/pc.c
> @@ -83,7 +83,7 @@ static void rtc_irq_handler(IsaIrqState *isa, int level)
> * mode is established while interrupt is raised. We want it to
> * be lowered in any case.
> */
> - if (!hpet_in_legacy_mode() || !level) {
> + if ((isa->hpet_state && !hpet_in_legacy_mode(isa->hpet_state)) || !level) {
> isa_set_irq(isa, RTC_ISA_IRQ, level);
> }
> }
> @@ -945,7 +945,7 @@ static void cpu_request_exit(void *opaque, int
> irq, int level)
> }
> }
>
> -void pc_basic_device_init(qemu_irq *isa_irq,
> +void pc_basic_device_init(qemu_irq *isa_irq, IsaIrqState *isa,
> FDCtrl **floppy_controller,
> ISADevice **rtc_state)
> {
> @@ -966,8 +966,10 @@ void pc_basic_device_init(qemu_irq *isa_irq,
>
> pit = pit_init(0x40, isa_reserve_irq(0));
> pcspk_init(pit);
> +
> + isa->hpet_state = NULL;
> if (!no_hpet) {
> - hpet_init(isa_irq);
> + isa->hpet_state = hpet_init(isa_irq);
> }
>
> for(i = 0; i < MAX_SERIAL_PORTS; i++) {
> diff --git a/hw/pc.h b/hw/pc.h
> index 73cccef..3e085b9 100644
> --- a/hw/pc.h
> +++ b/hw/pc.h
> @@ -42,6 +42,7 @@ void irq_info(Monitor *mon);
> typedef struct isa_irq_state {
> qemu_irq *i8259;
> qemu_irq *ioapic;
> + void *hpet_state;
> } IsaIrqState;
>
> void isa_irq_handler(void *opaque, int n, int level);
> @@ -94,7 +95,7 @@ void pc_memory_init(ram_addr_t ram_size,
> ram_addr_t *above_4g_mem_size_p);
> qemu_irq *pc_allocate_cpu_irq(void);
> void pc_vga_init(PCIBus *pci_bus);
> -void pc_basic_device_init(qemu_irq *isa_irq,
> +void pc_basic_device_init(qemu_irq *isa_irq, IsaIrqState *isa,
> FDCtrl **floppy_controller,
> ISADevice **rtc_state);
> void pc_init_ne2k_isa(NICInfo *nd);
> diff --git a/hw/pc_piix.c b/hw/pc_piix.c
> index 70f563a..1479a28 100644
> --- a/hw/pc_piix.c
> +++ b/hw/pc_piix.c
> @@ -93,7 +93,8 @@ static void pc_init1(ram_addr_t ram_size,
> pc_vga_init(pci_enabled? pci_bus: NULL);
>
> /* init basic PC hardware */
> - pc_basic_device_init(isa_irq, &floppy_controller, &rtc_state);
> + pc_basic_device_init(isa_irq, isa_irq_state, &floppy_controller,
> + &rtc_state);
>
> for(i = 0; i < nb_nics; i++) {
> NICInfo *nd = &nd_table[i];
I have a longer hpet fix/qdev'ify/enhance series queued [1], held back
until the device_show thing is through. Your patch break it, but it
makes sense. Will rebase on top.
Jan
[1] git://git.kiszka.org/qemu.git queues/hpet
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]
next prev parent reply other threads:[~2010-05-23 15:40 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-05-23 12:39 [Qemu-devel] [PATCH, RFC 2/4] hpet: don't use any static state Blue Swirl
2010-05-23 15:40 ` Jan Kiszka [this message]
2010-05-23 16:20 ` [Qemu-devel] " Blue Swirl
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4BF94C77.8060801@web.de \
--to=jan.kiszka@web.de \
--cc=blauwirbel@gmail.com \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.