[Qemu-devel] [PATCH v8 0/2] bugs fix for hpet

[Qemu-devel] [PATCH v8 0/2] bugs fix for hpet

[Liu Ping Fan]

v8: 
  make piix/q35 compat diverge
  simplify the code, use hpet_irqs to pass "intcap" value

v7:
  use macro to define "intcap" in pc.h
  (as to 3/4 and 4/4, I am not sure about whether to merge them or not, so keep them separate")

v6:
  move the setting of intcap to board, and keep the init value as zero. (thanks for the discussion from Paolo and Michael)
  introduce an extra hpet property "compat" to tell PC version

v5:
  use stand compat property to fix hpet intcap on pc-q35, while on pc-piix, hard code intcap as IRQ2

v4:
  use stand compat property to fix hpet intcap

v3:
  change hpet interrupt capablity on board's demand


Liu Ping Fan (2):
  hpet: inverse polarity when pin above ISA_NUM_IRQS
  hpet: enable to entitle more irq pins for hpet

 hw/i386/pc.c         | 19 ++++++++++++++++---
 hw/i386/pc_piix.c    |  3 ++-
 hw/i386/pc_q35.c     | 21 +++++++++++++++++----
 hw/timer/hpet.c      | 23 +++++++++++++++++++----
 include/hw/i386/pc.h | 24 +++++++++++++++++++++++-
 5 files changed, 77 insertions(+), 13 deletions(-)

-- 
1.8.1.4

[Qemu-devel] [PATCH v8 1/2] hpet: inverse polarity when pin above ISA_NUM_IRQS

[Liu Ping Fan]

According to hpet spec, hpet irq is high active. But according to
ICH spec, there is inversion before the input of ioapic. So the OS
will expect low active on this IRQ line. (On bare metal, if OS driver
claims high active on this line, spurious irq is generated)

We fold the emulation of this inversion inside the hpet logic.

Signed-off-by: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
---
 hw/timer/hpet.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/hw/timer/hpet.c b/hw/timer/hpet.c
index fcd22ae..8429eb3 100644
--- a/hw/timer/hpet.c
+++ b/hw/timer/hpet.c
@@ -198,13 +198,23 @@ static void update_irq(struct HPETTimer *timer, int set)
     if (!set || !timer_enabled(timer) || !hpet_enabled(timer->state)) {
         s->isr &= ~mask;
         if (!timer_fsb_route(timer)) {
-            qemu_irq_lower(s->irqs[route]);
+            /* fold the ICH PIRQ# pin's internal inversion logic into hpet */
+            if (route >= ISA_NUM_IRQS) {
+                qemu_irq_raise(s->irqs[route]);
+            } else {
+                qemu_irq_lower(s->irqs[route]);
+            }
         }
     } else if (timer_fsb_route(timer)) {
         stl_le_phys(timer->fsb >> 32, timer->fsb & 0xffffffff);
     } else if (timer->config & HPET_TN_TYPE_LEVEL) {
         s->isr |= mask;
-        qemu_irq_raise(s->irqs[route]);
+        /* fold the ICH PIRQ# pin's internal inversion logic into hpet */
+        if (route >= ISA_NUM_IRQS) {
+            qemu_irq_lower(s->irqs[route]);
+        } else {
+            qemu_irq_raise(s->irqs[route]);
+        }
     } else {
         s->isr &= ~mask;
         qemu_irq_pulse(s->irqs[route]);
-- 
1.8.1.4

[Qemu-devel] [PATCH v8 2/2] hpet: enable to entitle more irq pins for hpet

[Liu Ping Fan]

Owning to some different hardware design, piix and q35 need
different compat. So making them diverge.

On q35, IRQ2/8 can be reserved for hpet timer 0/1. And pin 16~23
can be assigned to hpet as guest chooses. So we introduce intcap
property to do that.

Consider the compat and piix/q35, we finally have the following
value for intcap: For piix, hpet's intcap is hard coded as IRQ2.
For pc-q35-1.7 and earlier, we use IRQ2 for compat reason. Otherwise
IRQ2, IRQ8, and IRQ16~23 are allowed.

Signed-off-by: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
---
 hw/i386/pc.c         | 19 ++++++++++++++++---
 hw/i386/pc_piix.c    |  3 ++-
 hw/i386/pc_q35.c     | 21 +++++++++++++++++----
 hw/timer/hpet.c      |  9 +++++++--
 include/hw/i386/pc.h | 24 +++++++++++++++++++++++-
 5 files changed, 65 insertions(+), 11 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 0c313fe..bb92465 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1219,7 +1219,8 @@ static const MemoryRegionOps ioportF0_io_ops = {
 void pc_basic_device_init(ISABus *isa_bus, qemu_irq *gsi,
                           ISADevice **rtc_state,
                           ISADevice **floppy,
-                          bool no_vmport)
+                          bool no_vmport,
+                          uint32 hpet_irqs)
 {
     int i;
     DriveInfo *fd[MAX_FD];
@@ -1246,9 +1247,21 @@ void pc_basic_device_init(ISABus *isa_bus, qemu_irq *gsi,
      * when the HPET wants to take over. Thus we have to disable the latter.
      */
     if (!no_hpet && (!kvm_irqchip_in_kernel() || kvm_has_pit_state2())) {
-        hpet = sysbus_try_create_simple("hpet", HPET_BASE, NULL);
-
+        /* In order to set property, here not using sysbus_try_create_simple */
+        hpet = qdev_try_create(NULL, "hpet");
         if (hpet) {
+            /* For pc-piix-*, hpet's intcap is always IRQ2. For pc-q35-1.7
+             * and earlier, use IRQ2 for compat. Otherwise, use IRQ16~23,
+             * IRQ8 and IRQ2.
+             */
+            uint8_t compat = object_property_get_int(OBJECT(hpet),
+                    HPET_INTCAP, NULL);
+            if (!compat) {
+                qdev_prop_set_uint32(hpet, HPET_INTCAP, hpet_irqs);
+            }
+            qdev_init_nofail(hpet);
+            sysbus_mmio_map(SYS_BUS_DEVICE(hpet), 0, HPET_BASE);
+
             for (i = 0; i < GSI_NUM_PINS; i++) {
                 sysbus_connect_irq(SYS_BUS_DEVICE(hpet), i, gsi[i]);
             }
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index c6042c7..506f026 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -180,7 +180,8 @@ static void pc_init1(QEMUMachineInitArgs *args,
     pc_vga_init(isa_bus, pci_enabled ? pci_bus : NULL);
 
     /* init basic PC hardware */
-    pc_basic_device_init(isa_bus, gsi, &rtc_state, &floppy, xen_enabled());
+    pc_basic_device_init(isa_bus, gsi, &rtc_state, &floppy, xen_enabled(),
+        0x4);
 
     pc_nic_init(isa_bus, pci_bus);
 
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index ca84e1c..d12d3f0 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -181,7 +181,7 @@ static void pc_q35_init(QEMUMachineInitArgs *args)
     pc_register_ferr_irq(gsi[13]);
 
     /* init basic PC hardware */
-    pc_basic_device_init(isa_bus, gsi, &rtc_state, &floppy, false);
+    pc_basic_device_init(isa_bus, gsi, &rtc_state, &floppy, false, 0xff0104);
 
     /* connect pm stuff to lpc */
     ich9_lpc_pm_init(lpc);
@@ -263,6 +263,15 @@ static void pc_q35_init_1_4(QEMUMachineInitArgs *args)
     .desc = "Standard PC (Q35 + ICH9, 2009)", \
     .hot_add_cpu = pc_hot_add_cpu
 
+#define PC_Q35_1_8_MACHINE_OPTIONS PC_Q35_MACHINE_OPTIONS
+
+static QEMUMachine pc_q35_machine_v1_8 = {
+    PC_Q35_1_7_MACHINE_OPTIONS,
+    .name = "pc-q35-1.8",
+    .alias = "q35",
+    .init = pc_q35_init,
+};
+
 #define PC_Q35_1_7_MACHINE_OPTIONS PC_Q35_MACHINE_OPTIONS
 
 static QEMUMachine pc_q35_machine_v1_7 = {
@@ -270,6 +279,10 @@ static QEMUMachine pc_q35_machine_v1_7 = {
     .name = "pc-q35-1.7",
     .alias = "q35",
     .init = pc_q35_init,
+    .compat_props = (GlobalProperty[]) {
+        PC_Q35_COMPAT_1_7,
+        { /* end of list */ }
+    },
 };
 
 #define PC_Q35_1_6_MACHINE_OPTIONS PC_Q35_MACHINE_OPTIONS
@@ -279,7 +292,7 @@ static QEMUMachine pc_q35_machine_v1_6 = {
     .name = "pc-q35-1.6",
     .init = pc_q35_init_1_6,
     .compat_props = (GlobalProperty[]) {
-        PC_COMPAT_1_6,
+        PC_Q35_COMPAT_1_6,
         { /* end of list */ }
     },
 };
@@ -289,7 +302,7 @@ static QEMUMachine pc_q35_machine_v1_5 = {
     .name = "pc-q35-1.5",
     .init = pc_q35_init_1_5,
     .compat_props = (GlobalProperty[]) {
-        PC_COMPAT_1_5,
+        PC_Q35_COMPAT_1_5,
         { /* end of list */ }
     },
 };
@@ -303,7 +316,7 @@ static QEMUMachine pc_q35_machine_v1_4 = {
     .name = "pc-q35-1.4",
     .init = pc_q35_init_1_4,
     .compat_props = (GlobalProperty[]) {
-        PC_COMPAT_1_4,
+        PC_Q35_COMPAT_1_4,
         { /* end of list */ }
     },
 };
diff --git a/hw/timer/hpet.c b/hw/timer/hpet.c
index 8429eb3..2ce52ae 100644
--- a/hw/timer/hpet.c
+++ b/hw/timer/hpet.c
@@ -73,6 +73,7 @@ typedef struct HPETState {
     uint8_t rtc_irq_level;
     qemu_irq pit_enabled;
     uint8_t num_timers;
+    uint32_t intcap;
     HPETTimer timer[HPET_MAX_TIMERS];
 
     /* Memory-mapped, software visible registers */
@@ -663,8 +664,8 @@ static void hpet_reset(DeviceState *d)
         if (s->flags & (1 << HPET_MSI_SUPPORT)) {
             timer->config |= HPET_TN_FSB_CAP;
         }
-        /* advertise availability of ioapic inti2 */
-        timer->config |=  0x00000004ULL << 32;
+        /* advertise availability of ioapic int */
+        timer->config |=  (uint64_t)s->intcap << 32;
         timer->period = 0ULL;
         timer->wrap_flag = 0;
     }
@@ -713,6 +714,9 @@ static void hpet_realize(DeviceState *dev, Error **errp)
     int i;
     HPETTimer *timer;
 
+    if (!s->intcap) {
+        error_printf("Hpet's intcap not initialized.\n");
+    }
     if (hpet_cfg.count == UINT8_MAX) {
         /* first instance */
         hpet_cfg.count = 0;
@@ -753,6 +757,7 @@ static void hpet_realize(DeviceState *dev, Error **errp)
 static Property hpet_device_properties[] = {
     DEFINE_PROP_UINT8("timers", HPETState, num_timers, HPET_MIN_TIMERS),
     DEFINE_PROP_BIT("msi", HPETState, flags, HPET_MSI_SUPPORT, false),
+    DEFINE_PROP_UINT32(HPET_INTCAP, HPETState, intcap, 0),
     DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 9b2ddc4..a1cdfb08 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -10,6 +10,8 @@
 
 #include "qemu/range.h"
 
+#define HPET_INTCAP "hpet-intcap"
+
 /* PC-style peripherals (also used by other machines).  */
 
 typedef struct PcPciInfo {
@@ -134,7 +136,8 @@ DeviceState *pc_vga_init(ISABus *isa_bus, PCIBus *pci_bus);
 void pc_basic_device_init(ISABus *isa_bus, qemu_irq *gsi,
                           ISADevice **rtc_state,
                           ISADevice **floppy,
-                          bool no_vmport);
+                          bool no_vmport,
+                          uint32 hpet_irqs);
 void pc_init_ne2k_isa(ISABus *bus, NICInfo *nd);
 void pc_cmos_init(ram_addr_t ram_size, ram_addr_t above_4g_mem_size,
                   const char *boot_device,
@@ -225,6 +228,25 @@ void pvpanic_init(ISABus *bus);
 
 int e820_add_entry(uint64_t, uint64_t, uint32_t);
 
+#define PC_Q35_COMPAT_1_7 \
+        {\
+            .driver   = "hpet",\
+            .property = HPET_INTCAP,\
+            .value    = stringify(4),\
+        }
+
+#define PC_Q35_COMPAT_1_6 \
+        PC_COMPAT_1_6, \
+        PC_Q35_COMPAT_1_7
+
+#define PC_Q35_COMPAT_1_5 \
+        PC_COMPAT_1_5, \
+        PC_Q35_COMPAT_1_6
+
+#define PC_Q35_COMPAT_1_4 \
+        PC_COMPAT_1_4, \
+        PC_Q35_COMPAT_1_5
+
 #define PC_COMPAT_1_6 \
         {\
             .driver   = "e1000",\
-- 
1.8.1.4

Re: [Qemu-devel] [PATCH v8 0/2] bugs fix for hpet

[liu ping fan]

ping? Any further comment?

Thanks

On Fri, Oct 18, 2013 at 12:00 PM, Liu Ping Fan <qemulist@gmail.com> wrote:
> v8:
>   make piix/q35 compat diverge
>   simplify the code, use hpet_irqs to pass "intcap" value
>
> v7:
>   use macro to define "intcap" in pc.h
>   (as to 3/4 and 4/4, I am not sure about whether to merge them or not, so keep them separate")
>
> v6:
>   move the setting of intcap to board, and keep the init value as zero. (thanks for the discussion from Paolo and Michael)
>   introduce an extra hpet property "compat" to tell PC version
>
> v5:
>   use stand compat property to fix hpet intcap on pc-q35, while on pc-piix, hard code intcap as IRQ2
>
> v4:
>   use stand compat property to fix hpet intcap
>
> v3:
>   change hpet interrupt capablity on board's demand
>
>
> Liu Ping Fan (2):
>   hpet: inverse polarity when pin above ISA_NUM_IRQS
>   hpet: enable to entitle more irq pins for hpet
>
>  hw/i386/pc.c         | 19 ++++++++++++++++---
>  hw/i386/pc_piix.c    |  3 ++-
>  hw/i386/pc_q35.c     | 21 +++++++++++++++++----
>  hw/timer/hpet.c      | 23 +++++++++++++++++++----
>  include/hw/i386/pc.h | 24 +++++++++++++++++++++++-
>  5 files changed, 77 insertions(+), 13 deletions(-)
>
> --
> 1.8.1.4
>