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

[Qemu-devel] [PATCH v2 1/3] 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.(And this is observed on
bare metal).

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 648b383..1139448 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 v2 2/3] vl: add func to check the machine type

[Liu Ping Fan]

On different machines, the device can has different properties.
Export machine type's check interface to devices, so the device
can decide its behavior at runtime.

Signed-off-by: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
---
 include/hw/boards.h | 1 +
 vl.c                | 8 ++++++++
 2 files changed, 9 insertions(+)

diff --git a/include/hw/boards.h b/include/hw/boards.h
index fb7c6f1..020edfb 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -49,6 +49,7 @@ typedef struct QEMUMachine {
 } QEMUMachine;
 
 int qemu_register_machine(QEMUMachine *m);
+bool qemu_check_machine(const char *idstr);
 QEMUMachine *find_default_machine(void);
 
 extern QEMUMachine *current_machine;
diff --git a/vl.c b/vl.c
index f422a1c..1f37489 100644
--- a/vl.c
+++ b/vl.c
@@ -1667,6 +1667,14 @@ int qemu_register_machine(QEMUMachine *m)
     return 0;
 }
 
+bool qemu_check_machine(const char *idstr)
+{
+    size_t n = strlen(idstr);
+
+    assert(current_machine);
+    return !strncmp(current_machine->name, idstr, n);
+}
+
 static QEMUMachine *find_machine(const char *name)
 {
     QEMUMachine *m;
-- 
1.8.1.4

[Qemu-devel] [PATCH v2 3/3] hpet: entitle more irq pins for hpet

[Liu Ping Fan]

On q35 machine, IRQ2/8 can be reserved for hpet timer 0/1. And pin 16~23
of ioapic can be dynamically assigned to hpet as guest chooses.

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 1139448..92cd4fa 100644
--- a/hw/timer/hpet.c
+++ b/hw/timer/hpet.c
@@ -25,6 +25,7 @@
  */
 
 #include "hw/hw.h"
+#include "hw/boards.h"
 #include "hw/i386/pc.h"
 #include "ui/console.h"
 #include "qemu/timer.h"
@@ -42,6 +43,11 @@
 
 #define HPET_MSI_SUPPORT        0
 
+/* only IRQ2 allowed for pc-1.6 and former */
+#define HPET_TN_INT_CAP_PC (0x4ULL << 32)
+/* Hpet can use non-legacy IRQ16~23, and an IRQ2 ,IRQ8 */
+#define HPET_TN_INT_CAP_Q35 (0xff0104ULL << 32)
+
 #define TYPE_HPET "hpet"
 #define HPET(obj) OBJECT_CHECK(HPETState, (obj), TYPE_HPET)
 
@@ -663,8 +669,12 @@ 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 */
+        if (qemu_check_machine("pc-q35")) {
+            timer->config |=  HPET_TN_INT_CAP_Q35;
+        } else {
+            timer->config |=  HPET_TN_INT_CAP_PC;
+        }
         timer->period = 0ULL;
         timer->wrap_flag = 0;
     }
-- 
1.8.1.4

Re: [Qemu-devel] [PATCH v2 3/3] hpet: entitle more irq pins for hpet

[Paolo Bonzini]

Il 27/08/2013 10:10, Liu Ping Fan ha scritto:
> On q35 machine, IRQ2/8 can be reserved for hpet timer 0/1. And pin 16~23
> of ioapic can be dynamically assigned to hpet as guest chooses.

First of all, the backwards-compatible q35 machines should also use the
old value.

Second, the HPET should _not_ know the machine types.  You need to add a
qdev property as I suggested in my reply to v1.  The default value
should be 0xFF0104.  Then you can add the compatibility value in
hw/i386/pc_piix.c and hw/i386/pc_q35.c.

Paolo

> 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 1139448..92cd4fa 100644
> --- a/hw/timer/hpet.c
> +++ b/hw/timer/hpet.c
> @@ -25,6 +25,7 @@
>   */
>  
>  #include "hw/hw.h"
> +#include "hw/boards.h"
>  #include "hw/i386/pc.h"
>  #include "ui/console.h"
>  #include "qemu/timer.h"
> @@ -42,6 +43,11 @@
>  
>  #define HPET_MSI_SUPPORT        0
>  
> +/* only IRQ2 allowed for pc-1.6 and former */
> +#define HPET_TN_INT_CAP_PC (0x4ULL << 32)
> +/* Hpet can use non-legacy IRQ16~23, and an IRQ2 ,IRQ8 */
> +#define HPET_TN_INT_CAP_Q35 (0xff0104ULL << 32)
> +
>  #define TYPE_HPET "hpet"
>  #define HPET(obj) OBJECT_CHECK(HPETState, (obj), TYPE_HPET)
>  
> @@ -663,8 +669,12 @@ 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 */
> +        if (qemu_check_machine("pc-q35")) {
> +            timer->config |=  HPET_TN_INT_CAP_Q35;
> +        } else {
> +            timer->config |=  HPET_TN_INT_CAP_PC;
> +        }
>          timer->period = 0ULL;
>          timer->wrap_flag = 0;
>      }
> 

Re: [Qemu-devel] [PATCH v2 3/3] hpet: entitle more irq pins for hpet

[liu ping fan]

On Tue, Aug 27, 2013 at 4:45 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 27/08/2013 10:10, Liu Ping Fan ha scritto:
>> On q35 machine, IRQ2/8 can be reserved for hpet timer 0/1. And pin 16~23
>> of ioapic can be dynamically assigned to hpet as guest chooses.
>
> First of all, the backwards-compatible q35 machines should also use the
> old value.
>
Sorry but could you tell me why even on q35, we can only use IRQ2 for hpet?

> Second, the HPET should _not_ know the machine types.  You need to add a
> qdev property as I suggested in my reply to v1.  The default value
> should be 0xFF0104.  Then you can add the compatibility value in
> hw/i386/pc_piix.c and hw/i386/pc_q35.c.
>
So export the hpet's property at board-level?

Regards,
Pingfan
>> ---
>>  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 1139448..92cd4fa 100644
>> --- a/hw/timer/hpet.c
>> +++ b/hw/timer/hpet.c
>> @@ -25,6 +25,7 @@
>>   */
>>
>>  #include "hw/hw.h"
>> +#include "hw/boards.h"
>>  #include "hw/i386/pc.h"
>>  #include "ui/console.h"
>>  #include "qemu/timer.h"
>> @@ -42,6 +43,11 @@
>>
>>  #define HPET_MSI_SUPPORT        0
>>
>> +/* only IRQ2 allowed for pc-1.6 and former */
>> +#define HPET_TN_INT_CAP_PC (0x4ULL << 32)
>> +/* Hpet can use non-legacy IRQ16~23, and an IRQ2 ,IRQ8 */
>> +#define HPET_TN_INT_CAP_Q35 (0xff0104ULL << 32)
>> +
>>  #define TYPE_HPET "hpet"
>>  #define HPET(obj) OBJECT_CHECK(HPETState, (obj), TYPE_HPET)
>>
>> @@ -663,8 +669,12 @@ 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 */
>> +        if (qemu_check_machine("pc-q35")) {
>> +            timer->config |=  HPET_TN_INT_CAP_Q35;
>> +        } else {
>> +            timer->config |=  HPET_TN_INT_CAP_PC;
>> +        }
>>          timer->period = 0ULL;
>>          timer->wrap_flag = 0;
>>      }
>>
>

Re: [Qemu-devel] [PATCH v2 3/3] hpet: entitle more irq pins for hpet

[Paolo Bonzini]

Il 27/08/2013 11:01, liu ping fan ha scritto:
> On Tue, Aug 27, 2013 at 4:45 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> Il 27/08/2013 10:10, Liu Ping Fan ha scritto:
>>> On q35 machine, IRQ2/8 can be reserved for hpet timer 0/1. And pin 16~23
>>> of ioapic can be dynamically assigned to hpet as guest chooses.
>>
>> First of all, the backwards-compatible q35 machines should also use the
>> old value.
>>
> Sorry but could you tell me why even on q35, we can only use IRQ2 for hpet?

Because the versioned machine types are supposed not to change the guest
ABI.  This sometime requires bug-compatibility; for example, this
register is part of the guest ABI.

>> Second, the HPET should _not_ know the machine types.  You need to add a
>> qdev property as I suggested in my reply to v1.  The default value
>> should be 0xFF0104.  Then you can add the compatibility value in
>> hw/i386/pc_piix.c and hw/i386/pc_q35.c.
>>
> So export the hpet's property at board-level?

Yes, the compat property mechanism can be used for this.

Paolo

> Regards,
> Pingfan
>>> ---
>>>  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 1139448..92cd4fa 100644
>>> --- a/hw/timer/hpet.c
>>> +++ b/hw/timer/hpet.c
>>> @@ -25,6 +25,7 @@
>>>   */
>>>
>>>  #include "hw/hw.h"
>>> +#include "hw/boards.h"
>>>  #include "hw/i386/pc.h"
>>>  #include "ui/console.h"
>>>  #include "qemu/timer.h"
>>> @@ -42,6 +43,11 @@
>>>
>>>  #define HPET_MSI_SUPPORT        0
>>>
>>> +/* only IRQ2 allowed for pc-1.6 and former */
>>> +#define HPET_TN_INT_CAP_PC (0x4ULL << 32)
>>> +/* Hpet can use non-legacy IRQ16~23, and an IRQ2 ,IRQ8 */
>>> +#define HPET_TN_INT_CAP_Q35 (0xff0104ULL << 32)
>>> +
>>>  #define TYPE_HPET "hpet"
>>>  #define HPET(obj) OBJECT_CHECK(HPETState, (obj), TYPE_HPET)
>>>
>>> @@ -663,8 +669,12 @@ 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 */
>>> +        if (qemu_check_machine("pc-q35")) {
>>> +            timer->config |=  HPET_TN_INT_CAP_Q35;
>>> +        } else {
>>> +            timer->config |=  HPET_TN_INT_CAP_PC;
>>> +        }
>>>          timer->period = 0ULL;
>>>          timer->wrap_flag = 0;
>>>      }
>>>
>>