On 10/11/2010 07:53 PM, Ruben Kerkhof wrote:
On Mon, Oct 11, 2010 at 18:57, Avi Kivityavi@redhat.com wrote:
On 10/11/2010 06:29 PM, Ruben Kerkhof wrote:
Hi Avi,
On Sun, Aug 15, 2010 at 13:00, Avi Kivityavi@redhat.com wrote:
I'm betting 73b48d914f9 is the cause, but let's see the full bisect.
system_powerdown with FreeBSD 8.1 works on 73b48d914f9.
I've bisected the issue to commit 76e617c.
Let me know if I should do some other tests.
It works on 73b48d914f9 but fails on 76e617c due to the seabios change.
Can you try 76e617c but with SeaBIOS 0.5.0? If that works, keep qemu-kvm on 76e617c but bisect SeaBIOS from 0.5.0 to 0.5.1 to see which commit broke freebsd.
Sure,
5c99b6c984682ddb1d4543a7e27a1f4ca633e6a6 is the first bad commit commit 5c99b6c984682ddb1d4543a7e27a1f4ca633e6a6 Author: Kevin O'Connorkevin@koconnor.net Date: Wed Dec 30 12:36:22 2009 -0500
Commit compiled dsdt file; misc comment updates. Commit new dsdt with recent changes. Add some misc comments. Also, fix uninitialized warning in mptable code.Regards, Ruben
Gleb, Kevin, any ideas?
(summary: qemu-kvm doesn't acpi shutdown freebsd 8.1 with this commit; qemu.git does. May be due to interrupt polarity which kvm implements but qemu does not)
On Tue, Oct 12, 2010 at 08:49:58AM +0200, Avi Kivity wrote:
On 10/11/2010 07:53 PM, Ruben Kerkhof wrote:
5c99b6c984682ddb1d4543a7e27a1f4ca633e6a6 is the first bad commit commit 5c99b6c984682ddb1d4543a7e27a1f4ca633e6a6 Author: Kevin O'Connorkevin@koconnor.net
Gleb, Kevin, any ideas?
(summary: qemu-kvm doesn't acpi shutdown freebsd 8.1 with this commit; qemu.git does. May be due to interrupt polarity which kvm implements but qemu does not)
The only thing in commit 5c99b6c9 that could cause an issue is that it has the compiled acpi changes actually made in commit 29f4b912, but I don't see how that would be a problem to reboots:
diff --git a/src/acpi-dsdt.dsl b/src/acpi-dsdt.dsl index cee038a..2bede25 100644 --- a/src/acpi-dsdt.dsl +++ b/src/acpi-dsdt.dsl @@ -58,7 +58,10 @@ DefinitionBlock ( #define prt_slot2(nr) prt_slot(nr, LNKB, LNKC, LNKD, LNKA) #define prt_slot3(nr) prt_slot(nr, LNKC, LNKD, LNKA, LNKB) prt_slot0(0x0000), - prt_slot1(0x0001), + Package() { 0x0001ffff, 0, 0, 9 }, + Package() { 0x0001ffff, 1, LNKB, 0 }, + Package() { 0x0001ffff, 2, LNKC, 0 }, + Package() { 0x0001ffff, 3, LNKD, 0 }, prt_slot2(0x0002), prt_slot3(0x0003), prt_slot0(0x0004),
Can you confirm that commit 4c94b7ea works reliably while commit 5c99b6c9 does not?
-Kevin
On Tue, Oct 12, 2010 at 03:11:24AM -0400, Kevin O'Connor wrote:
On Tue, Oct 12, 2010 at 08:49:58AM +0200, Avi Kivity wrote:
On 10/11/2010 07:53 PM, Ruben Kerkhof wrote:
5c99b6c984682ddb1d4543a7e27a1f4ca633e6a6 is the first bad commit commit 5c99b6c984682ddb1d4543a7e27a1f4ca633e6a6 Author: Kevin O'Connorkevin@koconnor.net
Gleb, Kevin, any ideas?
(summary: qemu-kvm doesn't acpi shutdown freebsd 8.1 with this commit; qemu.git does. May be due to interrupt polarity which kvm implements but qemu does not)
The only thing in commit 5c99b6c9 that could cause an issue is that it has the compiled acpi changes actually made in commit 29f4b912, but I don't see how that would be a problem to reboots:
It makes line 0 of device 1 to be active low instead of active high.
diff --git a/src/acpi-dsdt.dsl b/src/acpi-dsdt.dsl index cee038a..2bede25 100644 --- a/src/acpi-dsdt.dsl +++ b/src/acpi-dsdt.dsl @@ -58,7 +58,10 @@ DefinitionBlock ( #define prt_slot2(nr) prt_slot(nr, LNKB, LNKC, LNKD, LNKA) #define prt_slot3(nr) prt_slot(nr, LNKC, LNKD, LNKA, LNKB) prt_slot0(0x0000),
prt_slot1(0x0001),
Package() { 0x0001ffff, 0, 0, 9 },Package() { 0x0001ffff, 1, LNKB, 0 },Package() { 0x0001ffff, 2, LNKC, 0 },Package() { 0x0001ffff, 3, LNKD, 0 }, prt_slot2(0x0002), prt_slot3(0x0003), prt_slot0(0x0004),Can you confirm that commit 4c94b7ea works reliably while commit 5c99b6c9 does not?
-Kevin
-- Gleb.
On 10/12/2010 09:14 AM, Gleb Natapov wrote:
On Tue, Oct 12, 2010 at 03:11:24AM -0400, Kevin O'Connor wrote:
On Tue, Oct 12, 2010 at 08:49:58AM +0200, Avi Kivity wrote:
On 10/11/2010 07:53 PM, Ruben Kerkhof wrote:
5c99b6c984682ddb1d4543a7e27a1f4ca633e6a6 is the first bad commit commit 5c99b6c984682ddb1d4543a7e27a1f4ca633e6a6 Author: Kevin O'Connorkevin@koconnor.net
Gleb, Kevin, any ideas?
(summary: qemu-kvm doesn't acpi shutdown freebsd 8.1 with this commit; qemu.git does. May be due to interrupt polarity which kvm implements but qemu does not)
The only thing in commit 5c99b6c9 that could cause an issue is that it has the compiled acpi changes actually made in commit 29f4b912, but I don't see how that would be a problem to reboots:
It makes line 0 of device 1 to be active low instead of active high.
Correctly, according to the documentation (the line is marked IRQ9OUT#, which indicates it's active low, though that isn't set down elsewhere).
Qemu however emulates it as active high. Other guests work, which is wierd.
On 10/12/2010 09:25 AM, Avi Kivity wrote:
On 10/12/2010 09:14 AM, Gleb Natapov wrote:
On Tue, Oct 12, 2010 at 03:11:24AM -0400, Kevin O'Connor wrote:
On Tue, Oct 12, 2010 at 08:49:58AM +0200, Avi Kivity wrote:
On 10/11/2010 07:53 PM, Ruben Kerkhof wrote:
5c99b6c984682ddb1d4543a7e27a1f4ca633e6a6 is the first bad commit commit 5c99b6c984682ddb1d4543a7e27a1f4ca633e6a6 Author: Kevin O'Connorkevin@koconnor.net
Gleb, Kevin, any ideas?
(summary: qemu-kvm doesn't acpi shutdown freebsd 8.1 with this commit; qemu.git does. May be due to interrupt polarity which kvm implements but qemu does not)
The only thing in commit 5c99b6c9 that could cause an issue is that it has the compiled acpi changes actually made in commit 29f4b912, but I don't see how that would be a problem to reboots:
It makes line 0 of device 1 to be active low instead of active high.
Correctly, according to the documentation (the line is marked IRQ9OUT#, which indicates it's active low, though that isn't set down elsewhere).
Qemu however emulates it as active high. Other guests work, which is wierd.
Aha! The piix4 specification updates says:
• IRQ9OUT#/GPO29 should be labeled as IRQ9OUT/GPO29
So it looks like it should be active high, and seabios is wrong in listing it as active low. Perhaps other OSes have quirks to force it to active high.
On 10/12/2010 09:33 AM, Avi Kivity wrote:
Aha! The piix4 specification updates says:
• IRQ9OUT#/GPO29 should be labeled as IRQ9OUT/GPO29
Even more explicitly:
- IRQ9OUT# Is Active HI
Problem: The signal identified as IRQ9OUT#/GPO29, pin F3, is not active level LO, it is active level HI, when APIC Chip Select (XBCS[8]) is set. Implication: This signal is typically used in Dual Processor capable systems and is connected to an IOAPIC. If the IOAPIC input is programmed for level LO, and SCIs or SM Bus events in the PIIX4/PIIX4E/PIIX4M are programmed to be reported on IRQ9OUT, devices using these will not be recognized by the IOAPIC and will not work correctly. Workaround: Program the appropriate input of the IOAPIC to active level HI. Status: This will not be fixed in the PIIX4/PIIX4E/PIIX4M. This is planned to be incorporated into the PIIX4 datasheet as a specification change.
On Tue, Oct 12, 2010 at 09:33:42AM +0200, Avi Kivity wrote:
On 10/12/2010 09:25 AM, Avi Kivity wrote:
On 10/12/2010 09:14 AM, Gleb Natapov wrote:
On Tue, Oct 12, 2010 at 03:11:24AM -0400, Kevin O'Connor wrote:
On Tue, Oct 12, 2010 at 08:49:58AM +0200, Avi Kivity wrote:
On 10/11/2010 07:53 PM, Ruben Kerkhof wrote:
5c99b6c984682ddb1d4543a7e27a1f4ca633e6a6 is the first bad commit commit 5c99b6c984682ddb1d4543a7e27a1f4ca633e6a6 Author: Kevin O'Connorkevin@koconnor.net
Gleb, Kevin, any ideas?
(summary: qemu-kvm doesn't acpi shutdown freebsd 8.1 with this commit; qemu.git does. May be due to interrupt polarity which kvm implements but qemu does not)
The only thing in commit 5c99b6c9 that could cause an issue is that it has the compiled acpi changes actually made in commit 29f4b912, but I don't see how that would be a problem to reboots:
It makes line 0 of device 1 to be active low instead of active high.
Correctly, according to the documentation (the line is marked IRQ9OUT#, which indicates it's active low, though that isn't set down elsewhere).
Qemu however emulates it as active high. Other guests work, which is wierd.
Aha! The piix4 specification updates says:
• IRQ9OUT#/GPO29 should be labeled as IRQ9OUT/GPO29
So it looks like it should be active high, and seabios is wrong in listing it as active low. Perhaps other OSes have quirks to force it to active high.
It is even stranger then that. Seabios creates interrupt override entry for irq9 to be active high, level triggered, but it should be edge triggered.
-- Gleb.
On 10/12/2010 09:40 AM, Gleb Natapov wrote:
• IRQ9OUT#/GPO29 should be labeled as IRQ9OUT/GPO29
So it looks like it should be active high, and seabios is wrong in listing it as active low. Perhaps other OSes have quirks to force it to active high.
It is even stranger then that. Seabios creates interrupt override entry for irq9 to be active high, level triggered, but it should be edge triggered.
So two bugs, not one...
Hi Kevin,
On Tue, Oct 12, 2010 at 09:11, Kevin O'Connor kevin@koconnor.net wrote:
On Tue, Oct 12, 2010 at 08:49:58AM +0200, Avi Kivity wrote:
On 10/11/2010 07:53 PM, Ruben Kerkhof wrote:
5c99b6c984682ddb1d4543a7e27a1f4ca633e6a6 is the first bad commit commit 5c99b6c984682ddb1d4543a7e27a1f4ca633e6a6 Author: Kevin O'Connorkevin@koconnor.net
Gleb, Kevin, any ideas?
(summary: qemu-kvm doesn't acpi shutdown freebsd 8.1 with this commit; qemu.git does. May be due to interrupt polarity which kvm implements but qemu does not)
The only thing in commit 5c99b6c9 that could cause an issue is that it has the compiled acpi changes actually made in commit 29f4b912, but I don't see how that would be a problem to reboots:
diff --git a/src/acpi-dsdt.dsl b/src/acpi-dsdt.dsl index cee038a..2bede25 100644 --- a/src/acpi-dsdt.dsl +++ b/src/acpi-dsdt.dsl @@ -58,7 +58,10 @@ DefinitionBlock ( #define prt_slot2(nr) prt_slot(nr, LNKB, LNKC, LNKD, LNKA) #define prt_slot3(nr) prt_slot(nr, LNKC, LNKD, LNKA, LNKB) prt_slot0(0x0000),
- prt_slot1(0x0001),
- Package() { 0x0001ffff, 0, 0, 9 },
- Package() { 0x0001ffff, 1, LNKB, 0 },
- Package() { 0x0001ffff, 2, LNKC, 0 },
- Package() { 0x0001ffff, 3, LNKD, 0 },
prt_slot2(0x0002), prt_slot3(0x0003), prt_slot0(0x0004),
Can you confirm that commit 4c94b7ea works reliably while commit 5c99b6c9 does not?
-Kevin
Yes, that's correct.
Is there anything I can do to help resolve this? Would rolling back this commit have any impact on other operating systems?
Regards,
Ruben
On Fri, Oct 15, 2010 at 03:45:06AM +0200, Ruben Kerkhof wrote:
Is there anything I can do to help resolve this? Would rolling back this commit have any impact on other operating systems?
Patch below should fix the problem.
diff --git a/src/acpi-dsdt.dsl b/src/acpi-dsdt.dsl index bb0a176..dafcf45 100644 --- a/src/acpi-dsdt.dsl +++ b/src/acpi-dsdt.dsl @@ -73,7 +73,7 @@ DefinitionBlock ( #define prt_slot3(nr) prt_slot(nr, LNKC, LNKD, LNKA, LNKB) prt_slot0(0x0000), /* Device 1 is power mgmt device, and can only use irq 9 */ - Package() { 0x0001ffff, 0, 0, 9 }, + Package() { 0x0001ffff, 0, LNKS, 0 }, Package() { 0x0001ffff, 1, LNKB, 0 }, Package() { 0x0001ffff, 2, LNKC, 0 }, Package() { 0x0001ffff, 3, LNKD, 0 }, @@ -634,6 +634,46 @@ DefinitionBlock ( Store (TMP, PRQ3) } } + Device(LNKS){ + Name(_HID, EISAID("PNP0C0F")) // PCI interrupt link + Name(_UID, 5) + Name(_PRS, ResourceTemplate(){ + Interrupt (, Level, ActiveHigh, Shared) + { 9 } + }) + Method (_STA, 0, NotSerialized) + { + Store (0x0B, Local0) + If (And (0x80, PRQ0, Local1)) + { + Store (0x09, Local0) + } + Return (Local0) + } + Method (_DIS, 0, NotSerialized) + { + Or (PRQ0, 0x80, PRQ0) + } + Method (_CRS, 0, NotSerialized) + { + Name (PRR0, ResourceTemplate () + { + Interrupt (, Level, ActiveHigh, Shared) + {9} + }) + CreateDWordField (PRR0, 0x05, TMP) + Store (PRQ0, Local0) + If (LLess (Local0, 0x80)) + { + Store (Local0, TMP) + } + Else + { + Store (Zero, TMP) + } + Return (PRR0) + } + } }
/* -- Gleb.
On Fri, Oct 15, 2010 at 09:01, Gleb Natapov gleb@redhat.com wrote:
On Fri, Oct 15, 2010 at 03:45:06AM +0200, Ruben Kerkhof wrote:
Is there anything I can do to help resolve this? Would rolling back this commit have any impact on other operating systems?
Patch below should fix the problem.
diff --git a/src/acpi-dsdt.dsl b/src/acpi-dsdt.dsl index bb0a176..dafcf45 100644 --- a/src/acpi-dsdt.dsl +++ b/src/acpi-dsdt.dsl @@ -73,7 +73,7 @@ DefinitionBlock ( #define prt_slot3(nr) prt_slot(nr, LNKC, LNKD, LNKA, LNKB) prt_slot0(0x0000), /* Device 1 is power mgmt device, and can only use irq 9 */
- Package() { 0x0001ffff, 0, 0, 9 },
- Package() { 0x0001ffff, 0, LNKS, 0 },
Package() { 0x0001ffff, 1, LNKB, 0 }, Package() { 0x0001ffff, 2, LNKC, 0 }, Package() { 0x0001ffff, 3, LNKD, 0 }, @@ -634,6 +634,46 @@ DefinitionBlock ( Store (TMP, PRQ3) } }
- Device(LNKS){
- Name(_HID, EISAID("PNP0C0F")) // PCI interrupt link
- Name(_UID, 5)
- Name(_PRS, ResourceTemplate(){
- Interrupt (, Level, ActiveHigh, Shared)
- { 9 }
- })
- Method (_STA, 0, NotSerialized)
- {
- Store (0x0B, Local0)
- If (And (0x80, PRQ0, Local1))
- {
- Store (0x09, Local0)
- }
- Return (Local0)
- }
- Method (_DIS, 0, NotSerialized)
- {
- Or (PRQ0, 0x80, PRQ0)
- }
- Method (_CRS, 0, NotSerialized)
- {
- Name (PRR0, ResourceTemplate ()
- {
- Interrupt (, Level, ActiveHigh, Shared)
- {9}
- })
- CreateDWordField (PRR0, 0x05, TMP)
- Store (PRQ0, Local0)
- If (LLess (Local0, 0x80))
- {
- Store (Local0, TMP)
- }
- Else
- {
- Store (Zero, TMP)
- }
- Return (PRR0)
- }
- }
}
/*
Gleb.
It does indeed.
Thanks a lot!
Ruben
Thanks a lot for all :)
Kindest regards, Giam Teck Choon
-
Avi Kivity -
Gleb Natapov -
Kevin O'Connor -
Ruben Kerkhof -
Teck Choon Giam