Patchwork [1/6] kvm: bios: advertise pci irqs as active high

login
register
mail settings
Submitter Gerd Hoffmann
Date Aug. 14, 2009, 12:21 p.m.
Message ID <1250252518-29300-2-git-send-email-kraxel@redhat.com>
Download mbox | patch
Permalink /patch/31397/
State Superseded
Headers show

Comments

Gerd Hoffmann - Aug. 14, 2009, 12:21 p.m.
From: Avi Kivity <avi@qumranet.com>

now that kvm emulates the ioapic polarity correctly, we must describe
the polarity correctly in the acpi tables.  otherwise pci interrupts won't
be delivered correctly.

Signed-off-by: Avi Kivity <avi@qumranet.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 acpi-dsdt.dsl |   16 ++++++++--------
 1 files changed, 8 insertions(+), 8 deletions(-)
Gleb Natapov - Aug. 14, 2009, 1:19 p.m.
On Fri, Aug 14, 2009 at 02:21:53PM +0200, Gerd Hoffmann wrote:
> From: Avi Kivity <avi@qumranet.com>
> 
> now that kvm emulates the ioapic polarity correctly, we must describe
kvm yes, but qemu doesn't. It make sense to fix qemu polarity handling
in the same series. Otherwise this patch does nothing.

> the polarity correctly in the acpi tables.  otherwise pci interrupts won't
> be delivered correctly.
> 
> Signed-off-by: Avi Kivity <avi@qumranet.com>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  acpi-dsdt.dsl |   16 ++++++++--------
>  1 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/acpi-dsdt.dsl b/acpi-dsdt.dsl
> index 7bff30a..76ff100 100644
> --- a/acpi-dsdt.dsl
> +++ b/acpi-dsdt.dsl
> @@ -441,7 +441,7 @@ DefinitionBlock (
>                  Name(_HID, EISAID("PNP0C0F"))     // PCI interrupt link
>                  Name(_UID, 1)
>                  Name(_PRS, ResourceTemplate(){
> -                    IRQ (Level, ActiveLow, Shared)
> +                    IRQ (Level, ActiveHigh, Shared)
>                          {3,4,5,6,7,9,10,11,12}
>                  })
>                  Method (_STA, 0, NotSerialized)
> @@ -461,7 +461,7 @@ DefinitionBlock (
>                  {
>                      Name (PRR0, ResourceTemplate ()
>                      {
> -                        IRQ (Level, ActiveLow, Shared)
> +                        IRQ (Level, ActiveHigh, Shared)
>                              {1}
>                      })
>                      CreateWordField (PRR0, 0x01, TMP)
> @@ -488,7 +488,7 @@ DefinitionBlock (
>                  Name(_HID, EISAID("PNP0C0F"))     // PCI interrupt link
>                  Name(_UID, 2)
>                  Name(_PRS, ResourceTemplate(){
> -                    IRQ (Level, ActiveLow, Shared)
> +                    IRQ (Level, ActiveHigh, Shared)
>                          {3,4,5,6,7,9,10,11,12}
>                  })
>                  Method (_STA, 0, NotSerialized)
> @@ -508,7 +508,7 @@ DefinitionBlock (
>                  {
>                      Name (PRR0, ResourceTemplate ()
>                      {
> -                        IRQ (Level, ActiveLow, Shared)
> +                        IRQ (Level, ActiveHigh, Shared)
>                              {1}
>                      })
>                      CreateWordField (PRR0, 0x01, TMP)
> @@ -535,7 +535,7 @@ DefinitionBlock (
>                  Name(_HID, EISAID("PNP0C0F"))     // PCI interrupt link
>                  Name(_UID, 3)
>                  Name(_PRS, ResourceTemplate(){
> -                    IRQ (Level, ActiveLow, Shared)
> +                    IRQ (Level, ActiveHigh, Shared)
>                          {3,4,5,6,7,9,10,11,12}
>                  })
>                  Method (_STA, 0, NotSerialized)
> @@ -555,7 +555,7 @@ DefinitionBlock (
>                  {
>                      Name (PRR0, ResourceTemplate ()
>                      {
> -                        IRQ (Level, ActiveLow, Shared)
> +                        IRQ (Level, ActiveHigh, Shared)
>                              {1}
>                      })
>                      CreateWordField (PRR0, 0x01, TMP)
> @@ -582,7 +582,7 @@ DefinitionBlock (
>                  Name(_HID, EISAID("PNP0C0F"))     // PCI interrupt link
>                  Name(_UID, 4)
>                  Name(_PRS, ResourceTemplate(){
> -                    IRQ (Level, ActiveLow, Shared)
> +                    IRQ (Level, ActiveHigh, Shared)
>                          {3,4,5,6,7,9,10,11,12}
>                  })
>                  Method (_STA, 0, NotSerialized)
> @@ -602,7 +602,7 @@ DefinitionBlock (
>                  {
>                      Name (PRR0, ResourceTemplate ()
>                      {
> -                        IRQ (Level, ActiveLow, Shared)
> +                        IRQ (Level, ActiveHigh, Shared)
>                              {1}
>                      })
>                      CreateWordField (PRR0, 0x01, TMP)
> -- 
> 1.6.2.5
> 
> 

--
			Gleb.
Gerd Hoffmann - Aug. 14, 2009, 1:59 p.m.
On 08/14/09 15:19, Gleb Natapov wrote:
> On Fri, Aug 14, 2009 at 02:21:53PM +0200, Gerd Hoffmann wrote:
>> From: Avi Kivity<avi@qumranet.com>
>>
>> now that kvm emulates the ioapic polarity correctly, we must describe
> kvm yes, but qemu doesn't. It make sense to fix qemu polarity handling
> in the same series.

Same series doesn't work.  These patches are for the pcbios git tree.

Of course it makes sense to get more features and fixes from kvm merged 
upstream.  Feel free to join the party to speed this up ;)

> Otherwise this patch does nothing.

It doesn't hurt though and reduces the differences between qemu and kvm, 
which is a good thing.  Also note that the other patches which fix real 
bugs depend on this one.

cheers,
   Gerd
Gleb Natapov - Aug. 14, 2009, 4:50 p.m.
On Fri, Aug 14, 2009 at 03:59:41PM +0200, Gerd Hoffmann wrote:
> On 08/14/09 15:19, Gleb Natapov wrote:
> >On Fri, Aug 14, 2009 at 02:21:53PM +0200, Gerd Hoffmann wrote:
> >>From: Avi Kivity<avi@qumranet.com>
> >>
> >>now that kvm emulates the ioapic polarity correctly, we must describe
> >kvm yes, but qemu doesn't. It make sense to fix qemu polarity handling
> >in the same series.
> 
> Same series doesn't work.  These patches are for the pcbios git tree.
> 
We already have pcbios git tree. Cool.

> Of course it makes sense to get more features and fixes from kvm
> merged upstream.  Feel free to join the party to speed this up ;)
> 
kvm implements polarity in in-kernel ioapic, not in qemu one. It should
be very simple though. I though may be while you are at it... :)

> >Otherwise this patch does nothing.
> 
> It doesn't hurt though and reduces the differences between qemu and
> kvm, which is a good thing.  Also note that the other patches which
> fix real bugs depend on this one.
> 
Can you elaborate which one fixes what bugs? I am not at all against
those patches going into qemu, just curious. AFAIK current bios works
with qemu as is.

--
			Gleb.
Gerd Hoffmann - Aug. 17, 2009, 7:28 a.m.
On 08/14/09 18:50, Gleb Natapov wrote:
> On Fri, Aug 14, 2009 at 03:59:41PM +0200, Gerd Hoffmann wrote:
>> On 08/14/09 15:19, Gleb Natapov wrote:
>>> On Fri, Aug 14, 2009 at 02:21:53PM +0200, Gerd Hoffmann wrote:
>>>> From: Avi Kivity<avi@qumranet.com>
>>>>
>>>> now that kvm emulates the ioapic polarity correctly, we must describe
>>> kvm yes, but qemu doesn't. It make sense to fix qemu polarity handling
>>> in the same series.
>> Same series doesn't work.  These patches are for the pcbios git tree.
>>
> We already have pcbios git tree. Cool.

Have a look at http://git.qemu.org/.  Not yet live.  As far I know the 
plan is to (a) switch from savannah to qemu.org as master tree, (b) 
import the bios trees as git submodules and (c) hook them into the qemu 
build process.

>> Of course it makes sense to get more features and fixes from kvm
>> merged upstream.  Feel free to join the party to speed this up ;)
>>
> kvm implements polarity in in-kernel ioapic, not in qemu one.

Ah, ok.  upstream qemu probably wants to use the in-kernel ioapic too 
some day, then we'll need the fixes anyway.  But even without that it is 
saner to have correct ACPI entries IMHO.

>> kvm, which is a good thing.  Also note that the other patches which
>> fix real bugs depend on this one.
>>
> Can you elaborate which one fixes what bugs? I am not at all against
> those patches going into qemu, just curious. AFAIK current bios works
> with qemu as is.

The patch descriptions are pretty clear ...

Patch 4/6 (disallow sharing acpi irq #9) actually fixes some acpi 
hickups.  I've noticed the 'system_powerdown' monitor command started 
working correctly recently for me, and I think this is this patch 
(didn't double-check though).

cheers,
   Gerd

Patch

diff --git a/acpi-dsdt.dsl b/acpi-dsdt.dsl
index 7bff30a..76ff100 100644
--- a/acpi-dsdt.dsl
+++ b/acpi-dsdt.dsl
@@ -441,7 +441,7 @@  DefinitionBlock (
                 Name(_HID, EISAID("PNP0C0F"))     // PCI interrupt link
                 Name(_UID, 1)
                 Name(_PRS, ResourceTemplate(){
-                    IRQ (Level, ActiveLow, Shared)
+                    IRQ (Level, ActiveHigh, Shared)
                         {3,4,5,6,7,9,10,11,12}
                 })
                 Method (_STA, 0, NotSerialized)
@@ -461,7 +461,7 @@  DefinitionBlock (
                 {
                     Name (PRR0, ResourceTemplate ()
                     {
-                        IRQ (Level, ActiveLow, Shared)
+                        IRQ (Level, ActiveHigh, Shared)
                             {1}
                     })
                     CreateWordField (PRR0, 0x01, TMP)
@@ -488,7 +488,7 @@  DefinitionBlock (
                 Name(_HID, EISAID("PNP0C0F"))     // PCI interrupt link
                 Name(_UID, 2)
                 Name(_PRS, ResourceTemplate(){
-                    IRQ (Level, ActiveLow, Shared)
+                    IRQ (Level, ActiveHigh, Shared)
                         {3,4,5,6,7,9,10,11,12}
                 })
                 Method (_STA, 0, NotSerialized)
@@ -508,7 +508,7 @@  DefinitionBlock (
                 {
                     Name (PRR0, ResourceTemplate ()
                     {
-                        IRQ (Level, ActiveLow, Shared)
+                        IRQ (Level, ActiveHigh, Shared)
                             {1}
                     })
                     CreateWordField (PRR0, 0x01, TMP)
@@ -535,7 +535,7 @@  DefinitionBlock (
                 Name(_HID, EISAID("PNP0C0F"))     // PCI interrupt link
                 Name(_UID, 3)
                 Name(_PRS, ResourceTemplate(){
-                    IRQ (Level, ActiveLow, Shared)
+                    IRQ (Level, ActiveHigh, Shared)
                         {3,4,5,6,7,9,10,11,12}
                 })
                 Method (_STA, 0, NotSerialized)
@@ -555,7 +555,7 @@  DefinitionBlock (
                 {
                     Name (PRR0, ResourceTemplate ()
                     {
-                        IRQ (Level, ActiveLow, Shared)
+                        IRQ (Level, ActiveHigh, Shared)
                             {1}
                     })
                     CreateWordField (PRR0, 0x01, TMP)
@@ -582,7 +582,7 @@  DefinitionBlock (
                 Name(_HID, EISAID("PNP0C0F"))     // PCI interrupt link
                 Name(_UID, 4)
                 Name(_PRS, ResourceTemplate(){
-                    IRQ (Level, ActiveLow, Shared)
+                    IRQ (Level, ActiveHigh, Shared)
                         {3,4,5,6,7,9,10,11,12}
                 })
                 Method (_STA, 0, NotSerialized)
@@ -602,7 +602,7 @@  DefinitionBlock (
                 {
                     Name (PRR0, ResourceTemplate ()
                     {
-                        IRQ (Level, ActiveLow, Shared)
+                        IRQ (Level, ActiveHigh, Shared)
                             {1}
                     })
                     CreateWordField (PRR0, 0x01, TMP)