On Sat, Dec 10, 2011 at 12:28, Jan Kiszka <jan.kiszka@xxxxxx> wrote: > From: Jan Kiszka <jan.kiszka@xxxxxxxxxxx> > > When the HPET enters legacy mode, the IRQ output of the PIT is > suppressed and replaced by the HPET timer 0. But the current code to > emulate this was broken in many ways. It reset the PIT state after > re-enabling, it worked against a stale static PIT structure, and it did > not properly saved/restored the IRQ output mask in the PIT vmstate. > > This patch solves the PIT IRQ control in a different way. On x86, it > both redirects the PIT IRQ to the HPET, just like the RTC. But it also > keeps the control line from the HPET to the PIT. This allows to disable > the PIT QEMU timer when it is not needed. The PIT's view on the control > line state is now saved in the same format that qemu-kvm is already > using. > > Note that, in contrast to the suppressed RTC IRQ line, we do not need to > save/restore the PIT line state in the HPET. As we trigger a PIT IRQ > update via the control line, the line state is reconstructed on mode > switch. > > Signed-off-by: Jan Kiszka <jan.kiszka@xxxxxxxxxxx> > --- > hw/alpha_dp264.c | 2 +- > hw/hpet.c | 38 +++++++++++++++++--------------- > hw/hpet_emul.h | 3 ++ > hw/i8254.c | 60 +++++++++++++++++++++++++++++---------------------- > hw/mips_fulong2e.c | 2 +- > hw/mips_jazz.c | 2 +- > hw/mips_malta.c | 2 +- > hw/mips_r4k.c | 2 +- > hw/pc.c | 13 ++++++++-- > hw/pc.h | 13 +---------- > hw/ppc_prep.c | 2 +- > 11 files changed, 74 insertions(+), 65 deletions(-) > > diff --git a/hw/alpha_dp264.c b/hw/alpha_dp264.c > index fcc20e9..412ccf0 100644 > --- a/hw/alpha_dp264.c > +++ b/hw/alpha_dp264.c > @@ -70,7 +70,7 @@ static void clipper_init(ram_addr_t ram_size, > pci_bus = typhoon_init(ram_size, &rtc_irq, cpus, clipper_pci_map_irq); > > rtc_init(1980, rtc_irq); > - pit_init(0x40, 0); > + pit_init(0x40, isa_get_irq(0)); > isa_create_simple("i8042"); > > /* VGA setup. Don't bother loading the bios. */ > diff --git a/hw/hpet.c b/hw/hpet.c > index 1b64e6a..ace0b1d 100644 > --- a/hw/hpet.c > +++ b/hw/hpet.c > @@ -64,6 +64,7 @@ typedef struct HPETState { > qemu_irq irqs[HPET_NUM_IRQ_ROUTES]; > uint32_t flags; > uint8_t rtc_irq_level; > + qemu_irq pit_enabled; > uint8_t num_timers; > HPETTimer timer[HPET_MAX_TIMERS]; > > @@ -572,12 +573,15 @@ static void hpet_ram_write(void *opaque, target_phys_addr_t addr, > hpet_del_timer(&s->timer[i]); > } > } > - /* i8254 and RTC are disabled when HPET is in legacy mode */ > + /* i8254 and RTC output pins are disabled > + * when HPET is in legacy mode */ > if (activating_bit(old_val, new_val, HPET_CFG_LEGACY)) { > - hpet_pit_disable(); > + qemu_set_irq(s->pit_enabled, 0); > + qemu_irq_lower(s->irqs[0]); > qemu_irq_lower(s->irqs[RTC_ISA_IRQ]); > } else if (deactivating_bit(old_val, new_val, HPET_CFG_LEGACY)) { > - hpet_pit_enable(); > + qemu_irq_lower(s->irqs[0]); > + qemu_set_irq(s->pit_enabled, 1); > qemu_set_irq(s->irqs[RTC_ISA_IRQ], s->rtc_irq_level); > } > break; > @@ -631,7 +635,6 @@ static void hpet_reset(DeviceState *d) > { > HPETState *s = FROM_SYSBUS(HPETState, sysbus_from_qdev(d)); > int i; > - static int count = 0; > > for (i = 0; i < s->num_timers; i++) { > HPETTimer *timer = &s->timer[i]; > @@ -648,29 +651,27 @@ static void hpet_reset(DeviceState *d) > timer->wrap_flag = 0; > } > > + qemu_set_irq(s->pit_enabled, 1); > s->hpet_counter = 0ULL; > s->hpet_offset = 0ULL; > s->config = 0ULL; > - if (count > 0) { > - /* we don't enable pit when hpet_reset is first called (by hpet_init) > - * because hpet is taking over for pit here. On subsequent invocations, > - * hpet_reset is called due to system reset. At this point control must > - * be returned to pit until SW reenables hpet. > - */ > - hpet_pit_enable(); > - } > hpet_cfg.hpet[s->hpet_id].event_timer_block_id = (uint32_t)s->capability; > hpet_cfg.hpet[s->hpet_id].address = sysbus_from_qdev(d)->mmio[0].addr; > - count = 1; > } > > -static void hpet_handle_rtc_irq(void *opaque, int n, int level) > +static void hpet_handle_legacy_irq(void *opaque, int n, int level) > { > HPETState *s = FROM_SYSBUS(HPETState, opaque); > > - s->rtc_irq_level = level; > - if (!hpet_in_legacy_mode(s)) { > - qemu_set_irq(s->irqs[RTC_ISA_IRQ], level); > + if (n == HPET_LEGACY_PIT_INT) { > + if (!hpet_in_legacy_mode(s)) { > + qemu_set_irq(s->irqs[0], level); > + } > + } else { > + s->rtc_irq_level = level; > + if (!hpet_in_legacy_mode(s)) { > + qemu_set_irq(s->irqs[RTC_ISA_IRQ], level); > + } > } > } > > @@ -713,7 +714,8 @@ static int hpet_init(SysBusDevice *dev) > s->capability |= (s->num_timers - 1) << HPET_ID_NUM_TIM_SHIFT; > s->capability |= ((HPET_CLK_PERIOD) << 32); > > - qdev_init_gpio_in(&dev->qdev, hpet_handle_rtc_irq, 1); > + qdev_init_gpio_in(&dev->qdev, hpet_handle_legacy_irq, 2); > + qdev_init_gpio_out(&dev->qdev, &s->pit_enabled, 1); > > /* HPET Area */ > memory_region_init_io(&s->iomem, &hpet_ram_ops, s, "hpet", 0x400); > diff --git a/hw/hpet_emul.h b/hw/hpet_emul.h > index 6128702..757f79f 100644 > --- a/hw/hpet_emul.h > +++ b/hw/hpet_emul.h > @@ -22,6 +22,9 @@ > > #define HPET_NUM_IRQ_ROUTES 32 > > +#define HPET_LEGACY_PIT_INT 0 > +#define HPET_LEGACY_RTC_INT 1 > + > #define HPET_CFG_ENABLE 0x001 > #define HPET_CFG_LEGACY 0x002 > > diff --git a/hw/i8254.c b/hw/i8254.c > index 12571ef..b3d9624 100644 > --- a/hw/i8254.c > +++ b/hw/i8254.c > @@ -51,18 +51,16 @@ typedef struct PITChannelState { > int64_t next_transition_time; > QEMUTimer *irq_timer; > qemu_irq irq; > + uint32_t irq_disabled; > } PITChannelState; > > typedef struct PITState { > ISADevice dev; > MemoryRegion ioports; > - uint32_t irq; > uint32_t iobase; > PITChannelState channels[3]; > } PITState; > > -static PITState pit_state; > - > static void pit_irq_timer_update(PITChannelState *s, int64_t current_time); > > static int pit_get_count(PITChannelState *s) > @@ -378,8 +376,9 @@ static void pit_irq_timer_update(PITChannelState *s, int64_t current_time) > int64_t expire_time; > int irq_level; > > - if (!s->irq_timer) > + if (!s->irq_timer || s->irq_disabled) { > 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); > @@ -450,6 +449,7 @@ static int pit_load_old(QEMUFile *f, void *opaque, int version_id) > qemu_get_8s(f, &s->bcd); > qemu_get_8s(f, &s->gate); > s->count_load_time=qemu_get_be64(f); > + s->irq_disabled = 0; > if (s->irq_timer) { > s->next_transition_time=qemu_get_be64(f); > qemu_get_timer(f, s->irq_timer); > @@ -460,11 +460,12 @@ static int pit_load_old(QEMUFile *f, void *opaque, int version_id) > > static const VMStateDescription vmstate_pit = { > .name = "i8254", > - .version_id = 2, > + .version_id = 3, > .minimum_version_id = 2, > .minimum_version_id_old = 1, > .load_state_old = pit_load_old, > .fields = (VMStateField []) { > + VMSTATE_UINT32_V(channels[0].irq_disabled, PITState, 3), > VMSTATE_STRUCT_ARRAY(channels, PITState, 3, 2, vmstate_pit_channel, PITChannelState), > VMSTATE_TIMER(channels[0].irq_timer, PITState), > VMSTATE_END_OF_LIST() > @@ -485,26 +486,20 @@ static void pit_reset(DeviceState *dev) > } > } > > -/* When HPET is operating in legacy mode, i8254 timer0 is disabled */ > -void hpet_pit_disable(void) { > - PITChannelState *s; > - s = &pit_state.channels[0]; > - if (s->irq_timer) > - qemu_del_timer(s->irq_timer); > -} > - > -/* When HPET is reset or leaving legacy mode, it must reenable i8254 > - * timer 0 > - */ > - > -void hpet_pit_enable(void) > +/* When HPET is operating in legacy mode, suppress the ignored timer IRQ, > + * reenable it when legacy mode is left again. */ > +static void pit_irq_control(void *opaque, int n, int enable) > { > - PITState *pit = &pit_state; > - PITChannelState *s; > - s = &pit->channels[0]; > - s->mode = 3; > - s->gate = 1; > - pit_load_count(s, 0); > + PITState *pit = opaque; > + PITChannelState *s = &pit->channels[0]; > + > + if (enable) { > + s->irq_disabled = 0; > + pit_irq_timer_update(s, qemu_get_clock_ns(vm_clock)); > + } else { > + s->irq_disabled = 1; > + qemu_del_timer(s->irq_timer); > + } > } > > static const MemoryRegionPortio pit_portio[] = { > @@ -525,16 +520,30 @@ static int pit_initfn(ISADevice *dev) > s = &pit->channels[0]; > /* the timer 0 is connected to an IRQ */ > s->irq_timer = qemu_new_timer_ns(vm_clock, pit_irq_timer, s); > - s->irq = isa_get_irq(pit->irq); > + qdev_init_gpio_out(&dev->qdev, &s->irq, 1); > > memory_region_init_io(&pit->ioports, &pit_ioport_ops, pit, "pit", 4); > isa_register_ioport(dev, &pit->ioports, pit->iobase); > > + qdev_init_gpio_in(&dev->qdev, pit_irq_control, 1); > + > qdev_set_legacy_instance_id(&dev->qdev, pit->iobase, 2); > > return 0; > } > > +ISADevice *pit_init(int base, qemu_irq irq) Please retain this function in pc.h, or even better, introduce i8254.h. > +{ > + ISADevice *dev; > + > + dev = isa_create("isa-pit"); > + qdev_prop_set_uint32(&dev->qdev, "iobase", base); > + qdev_init_nofail(&dev->qdev); > + qdev_connect_gpio_out(&dev->qdev, 0, irq); > + > + return dev; > +} > + > static ISADeviceInfo pit_info = { > .qdev.name = "isa-pit", > .qdev.size = sizeof(PITState), > @@ -543,7 +552,6 @@ static ISADeviceInfo pit_info = { > .qdev.no_user = 1, > .init = pit_initfn, > .qdev.props = (Property[]) { > - DEFINE_PROP_UINT32("irq", PITState, irq, -1), > DEFINE_PROP_HEX32("iobase", PITState, iobase, -1), > DEFINE_PROP_END_OF_LIST(), > }, > diff --git a/hw/mips_fulong2e.c b/hw/mips_fulong2e.c > index 04921c1..8a9c006 100644 > --- a/hw/mips_fulong2e.c > +++ b/hw/mips_fulong2e.c > @@ -358,7 +358,7 @@ static void mips_fulong2e_init(ram_addr_t ram_size, const char *boot_device, > smbus_eeprom_init(smbus, 1, eeprom_spd, sizeof(eeprom_spd)); > > /* init other devices */ > - pit = pit_init(0x40, 0); > + pit = pit_init(0x40, isa_get_irq(0)); > cpu_exit_irq = qemu_allocate_irqs(cpu_request_exit, NULL, 1); > DMA_init(0, cpu_exit_irq); > > diff --git a/hw/mips_jazz.c b/hw/mips_jazz.c > index 358de59..a076e1d 100644 > --- a/hw/mips_jazz.c > +++ b/hw/mips_jazz.c > @@ -188,7 +188,7 @@ static void mips_jazz_init(MemoryRegion *address_space, > isa_bus_irqs(i8259); > cpu_exit_irq = qemu_allocate_irqs(cpu_request_exit, NULL, 1); > DMA_init(0, cpu_exit_irq); > - pit = pit_init(0x40, 0); > + pit = pit_init(0x40, isa_get_irq(0)); > pcspk_init(pit); > > /* ISA IO space at 0x90000000 */ > diff --git a/hw/mips_malta.c b/hw/mips_malta.c > index bb49749..d09a48a 100644 > --- a/hw/mips_malta.c > +++ b/hw/mips_malta.c > @@ -953,7 +953,7 @@ void mips_malta_init (ram_addr_t ram_size, > NULL, NULL, 0); > /* TODO: Populate SPD eeprom data. */ > smbus_eeprom_init(smbus, 8, NULL, 0); > - pit = pit_init(0x40, 0); > + pit = pit_init(0x40, isa_get_irq(0)); > cpu_exit_irq = qemu_allocate_irqs(cpu_request_exit, NULL, 1); > DMA_init(0, cpu_exit_irq); > > diff --git a/hw/mips_r4k.c b/hw/mips_r4k.c > index d0564d4..9fef3ba 100644 > --- a/hw/mips_r4k.c > +++ b/hw/mips_r4k.c > @@ -266,7 +266,7 @@ void mips_r4k_init (ram_addr_t ram_size, > isa_mmio_init(0x14000000, 0x00010000); > isa_mem_base = 0x10000000; > > - pit = pit_init(0x40, 0); > + pit = pit_init(0x40, isa_get_irq(0)); > > for(i = 0; i < MAX_SERIAL_PORTS; i++) { > if (serial_hds[i]) { > diff --git a/hw/pc.c b/hw/pc.c > index 33778fe..dcf87af 100644 > --- a/hw/pc.c > +++ b/hw/pc.c > @@ -1119,6 +1119,8 @@ void pc_basic_device_init(qemu_irq *gsi, > { > int i; > DriveInfo *fd[MAX_FD]; > + DeviceState *hpet = NULL; > + qemu_irq pit_irq = isa_get_irq(0); > qemu_irq rtc_irq = NULL; > qemu_irq *a20_line; > ISADevice *i8042, *port92, *vmmouse, *pit; > @@ -1129,20 +1131,25 @@ void pc_basic_device_init(qemu_irq *gsi, > register_ioport_write(0xf0, 1, 1, ioportF0_write, NULL); > > if (!no_hpet) { > - DeviceState *hpet = sysbus_try_create_simple("hpet", HPET_BASE, NULL); > + hpet = sysbus_try_create_simple("hpet", HPET_BASE, NULL); > > if (hpet) { > for (i = 0; i < GSI_NUM_PINS; i++) { > sysbus_connect_irq(sysbus_from_qdev(hpet), i, gsi[i]); > } > - rtc_irq = qdev_get_gpio_in(hpet, 0); > + pit_irq = qdev_get_gpio_in(hpet, HPET_LEGACY_PIT_INT); > + rtc_irq = qdev_get_gpio_in(hpet, HPET_LEGACY_RTC_INT); > } > } > *rtc_state = rtc_init(2000, rtc_irq); > > qemu_register_boot_set(pc_boot_set, *rtc_state); > > - pit = pit_init(0x40, 0); > + pit = pit_init(0x40, pit_irq); > + if (hpet) { > + /* connect PIT to output control line of the HPET */ > + qdev_connect_gpio_out(hpet, 0, qdev_get_gpio_in(&pit->qdev, 0)); > + } > pcspk_init(pit); > > for(i = 0; i < MAX_SERIAL_PORTS; i++) { > diff --git a/hw/pc.h b/hw/pc.h > index b7b7e40..20ee543 100644 > --- a/hw/pc.h > +++ b/hw/pc.h > @@ -84,18 +84,7 @@ void gsi_handler(void *opaque, int n, int level); > > #define PIT_FREQ 1193182 > > -static inline ISADevice *pit_init(int base, int irq) > -{ > - ISADevice *dev; > - > - dev = isa_create("isa-pit"); > - qdev_prop_set_uint32(&dev->qdev, "iobase", base); > - qdev_prop_set_uint32(&dev->qdev, "irq", irq); > - qdev_init_nofail(&dev->qdev); > - > - return dev; > -} > - > +ISADevice *pit_init(int base, qemu_irq irq); > void pit_set_gate(ISADevice *dev, int channel, int val); > int pit_get_gate(ISADevice *dev, int channel); > int pit_get_initial_count(ISADevice *dev, int channel); > diff --git a/hw/ppc_prep.c b/hw/ppc_prep.c > index f22d5b9..84e2028 100644 > --- a/hw/ppc_prep.c > +++ b/hw/ppc_prep.c > @@ -641,7 +641,7 @@ static void ppc_prep_init (ram_addr_t ram_size, > /* init basic PC hardware */ > pci_vga_init(pci_bus); > // openpic = openpic_init(0x00000000, 0xF0000000, 1); > - // pit = pit_init(0x40, 0); > + // pit = pit_init(0x40, isa_get_irq(0)); > rtc_init(2000, NULL); > > if (serial_hds[0]) > -- > 1.7.3.4 > -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html