Re: [Qemu-devel] [PATCH] Update i440FX/PIIX3 emulation

2007-10-31 Thread Avi Kivity

Michael Hanselmann wrote:



This does not yet remove the workaround introduced by Igor Lvovsky's
patch. However, I'm working on that since it, despite my earlier mail,
seems to help with my ACPI shutdown problem.



So, I found the bug causing this behaviour. It turned out to be a
wrongly named variable in the ACPI DSDT from Bochs. See the patch for
Bochs below. I already sent it to the bochs-developers list[1].
qemu/pc-bios/bios.bin needs to be rebuilt from Bochs' code,
qemu/pc-bios/bios.diff and my patch.

The second patch below reverts the changes made by Igor Lvovsky. After
applying the patch to the BIOS, ACPI IRQs finally reach the system.

Finding this bug took me about the free time of four weeks. However, I
learned a lot about the internals of a PC. :-)

Thanks,
Michael

[1] 
http://sourceforge.net/mailarchive/forum.php?thread_name=20071031000835.GA20915%40hansmi.chforum_name=bochs-developers

---
Index: bios/acpi-dsdt.dsl
===
RCS file: /cvsroot/bochs/bochs/bios/acpi-dsdt.dsl,v
retrieving revision 1.1
diff -u -p -u -p -r1.1 acpi-dsdt.dsl
--- bios/acpi-dsdt.dsl  28 Sep 2006 18:56:20 -  1.1
+++ bios/acpi-dsdt.dsl  30 Oct 2007 23:52:22 -
@@ -369,7 +369,7 @@ DefinitionBlock (
 Method (_STA, 0, NotSerialized)
 {
 Store (0x0B, Local0)
-If (And (0x80, PRQ0, Local1))
+If (And (0x80, PRQ0, Local0))
 {
  Store (0x09, Local0)
 }
  


Can you explain this?

The original code seems to return either 0xb (present, enabled, 
functional) or 0x9 (present, functional).  The new code seems to return 
either 0x9 (present, functional) or 0 (if PRQ0 had its seventh bit clear).


Am I reading the code incorrectly?

--
Any sufficiently difficult bug is indistinguishable from a feature.





Re: [Qemu-devel] [PATCH] Update i440FX/PIIX3 emulation

2007-10-31 Thread Michael Hanselmann
Hi Avi

On Wed, Oct 31, 2007 at 03:17:04PM +0200, Avi Kivity wrote:
 --- bios/acpi-dsdt.dsl   28 Sep 2006 18:56:20 -  1.1
 +++ bios/acpi-dsdt.dsl   30 Oct 2007 23:52:22 -
 @@ -369,7 +369,7 @@ DefinitionBlock (
  Method (_STA, 0, NotSerialized)
  {
  Store (0x0B, Local0)
 -If (And (0x80, PRQ0, Local1))
 +If (And (0x80, PRQ0, Local0))
  {
   Store (0x09, Local0)
  }

 Can you explain this?

 The original code seems to return either 0xb (present, enabled, functional) 
 or 0x9 (present, functional).  The new code seems to return either 0x9 
 (present, functional) or 0 (if PRQ0 had its seventh bit clear).

 Am I reading the code incorrectly?

It looks like you're correct. Here's the same function from a HP
Pavilion laptop:

Method (_STA, 0, NotSerialized)
{
If (And (PIRH, 0x80))
{
Return (0x09)
}
Return (0x0B)
}

Basically this means I have to go back to debug this stuff.

Sorry for the false alarm,
Michael

-- 
http://hansmi.ch/




Re: [Qemu-devel] [PATCH] Update i440FX/PIIX3 emulation

2007-10-30 Thread Michael Hanselmann
On Thu, Oct 25, 2007 at 12:42:22AM +0200, Michael Hanselmann wrote:
 The patch below updates the i440FX/PIIX3 emulation. It does:

I never got any reaction to that patch. Is it still awaiting review?

 This does not yet remove the workaround introduced by Igor Lvovsky's
 patch. However, I'm working on that since it, despite my earlier mail,
 seems to help with my ACPI shutdown problem.

So, I found the bug causing this behaviour. It turned out to be a
wrongly named variable in the ACPI DSDT from Bochs. See the patch for
Bochs below. I already sent it to the bochs-developers list[1].
qemu/pc-bios/bios.bin needs to be rebuilt from Bochs' code,
qemu/pc-bios/bios.diff and my patch.

The second patch below reverts the changes made by Igor Lvovsky. After
applying the patch to the BIOS, ACPI IRQs finally reach the system.

Finding this bug took me about the free time of four weeks. However, I
learned a lot about the internals of a PC. :-)

Thanks,
Michael

[1] 
http://sourceforge.net/mailarchive/forum.php?thread_name=20071031000835.GA20915%40hansmi.chforum_name=bochs-developers

---
Index: bios/acpi-dsdt.dsl
===
RCS file: /cvsroot/bochs/bochs/bios/acpi-dsdt.dsl,v
retrieving revision 1.1
diff -u -p -u -p -r1.1 acpi-dsdt.dsl
--- bios/acpi-dsdt.dsl  28 Sep 2006 18:56:20 -  1.1
+++ bios/acpi-dsdt.dsl  30 Oct 2007 23:52:22 -
@@ -369,7 +369,7 @@ DefinitionBlock (
 Method (_STA, 0, NotSerialized)
 {
 Store (0x0B, Local0)
-If (And (0x80, PRQ0, Local1))
+If (And (0x80, PRQ0, Local0))
 {
  Store (0x09, Local0)
 }
@@ -416,7 +416,7 @@ DefinitionBlock (
 Method (_STA, 0, NotSerialized)
 {
 Store (0x0B, Local0)
-If (And (0x80, PRQ1, Local1))
+If (And (0x80, PRQ1, Local0))
 {
  Store (0x09, Local0)
 }
@@ -463,7 +463,7 @@ DefinitionBlock (
 Method (_STA, 0, NotSerialized)
 {
 Store (0x0B, Local0)
-If (And (0x80, PRQ2, Local1))
+If (And (0x80, PRQ2, Local0))
 {
  Store (0x09, Local0)
 }
@@ -510,7 +510,7 @@ DefinitionBlock (
 Method (_STA, 0, NotSerialized)
 {
 Store (0x0B, Local0)
-If (And (0x80, PRQ3, Local1))
+If (And (0x80, PRQ3, Local0))
 {
  Store (0x09, Local0)
 }

---
diff --git a/hw/piix_pci.c b/hw/piix_pci.c
index 7c7d0f3..eabff8e 100644
--- a/hw/piix_pci.c
+++ b/hw/piix_pci.c
@@ -269,7 +269,6 @@ static void piix3_set_irq(qemu_irq *pic, int irq_num, int 
level)
 {
 int i, pic_irq, pic_level;
 
-piix3_dev-config[0x60 + irq_num] = ~0x80;
 pci_irq_levels[irq_num] = level;
 
 /* now we change the pic irq level according to the piix irq mappings */




[Qemu-devel] [PATCH] Update i440FX/PIIX3 emulation

2007-10-24 Thread Michael Hanselmann
The patch below updates the i440FX/PIIX3 emulation. It does:
- Add links to all datasheets containing the specifications
- Combine initialization functions for the PIIX, PIIX3 and PIIX4 chips
- Break apart long lists of magic values and name them
- Set more registers to their default values from the specs

Signed-off-by: Michael Hanselmann [EMAIL PROTECTED]

---
This does not yet remove the workaround introduced by Igor Lvovsky's
patch. However, I'm working on that since it, despite my earlier mail,
seems to help with my ACPI shutdown problem.

Greets,
Michael

Index: hw/piix_pci.c
===
RCS file: /sources/qemu/qemu/hw/piix_pci.c,v
retrieving revision 1.12
diff -u -p -r1.12 piix_pci.c
--- hw/piix_pci.c   20 Oct 2007 20:36:52 -  1.12
+++ hw/piix_pci.c   24 Oct 2007 22:39:41 -
@@ -22,12 +22,37 @@
  * THE SOFTWARE.
  */
 
+/*
+ * Datasheets:
+ * - 82371FB (PIIX) and 82371SB (PIIX3) PCI ISA IDE Xcelerator
+ *   http://www.intel.com/design/intarch/datashts/290550.htm
+ * - 82371AB PCI-TO-ISA/IDE Xcelerator (PIIX4)
+ *   http://www.intel.com/design/intarch/datashts/290562.htm
+ * - 82371SB (PIIX3) PCIset Specification Update
+ *   http://www.intel.com/design/chipsets/specupdt/297658.htm
+ * - 82371EB (PIIX4E) Specification Update
+ *   http://developer.intel.com/design/chipsets/specupdt/290635.htm
+ * - 82371AB PIIX4, 82371EB PIIX4E and 82371MB PIIX4M Specification Update
+ *   http://www.intel.com/design/chipsets/specupdt/297738.htm
+ * - 440FX PCIset - 82441FX PCI and Memory Controller (PMC) and 82442FX Data
+ *   Bus Accelerator (DBX) Datasheet
+ *   http://developer.intel.com/design/chipsets/datashts/290549.htm
+ * - 440FX PCIset 82441FX (PMC) and 82442FX (DBX) Specification Update
+ *   http://developer.intel.com/design/chipsets/specupdt/297654.htm
+ */
+
 #include vl.h
 typedef uint32_t pci_addr_t;
 #include pci_host.h
 
 typedef PCIHostState I440FXState;
 
+enum PCIChip {
+PIIX,
+PIIX3,
+PIIX4
+};
+
 static void i440fx_addr_writel(void* opaque, uint32_t addr, uint32_t val)
 {
 I440FXState *s = opaque;
@@ -178,16 +203,52 @@ PCIBus *i440fx_init(PCIDevice **pi440fx_
 d = pci_register_device(b, i440FX, sizeof(PCIDevice), 0,
 NULL, i440fx_write_config);
 
-d-config[0x00] = 0x86; // vendor_id
+/* Vendor Identification */
+d-config[0x00] = 0x86; /* Intel */
 d-config[0x01] = 0x80;
-d-config[0x02] = 0x37; // device_id
+
+/* Device Identification */
+d-config[0x02] = 0x37;
 d-config[0x03] = 0x12;
-d-config[0x08] = 0x02; // revision
+
+/* PCI Command Register */
+d-config[0x04] = 0x06;
+d-config[0x05] = 0x00;
+
+/* PCI Status Register */
+d-config[0x06] = 0x00; /* Default would be 0x80, but we don't support
+   fast back-to-back transactions as described
+   in the 440FX datasheet. */
+d-config[0x07] = 0x02;
+
+/* Revision Identification */
+d-config[0x08] = 0x02;
+
+/* Class Code */
 d-config[0x0a] = 0x00; // class_sub = host2pci
 d-config[0x0b] = 0x06; // class_base = PCI_bridge
-d-config[0x0e] = 0x00; // header_type
 
-d-config[0x72] = 0x02; /* SMRAM */
+/* Header Type */
+d-config[0x0e] = 0x00;
+
+/* PMC Configuration */
+d-config[0x50] = 0x02;
+d-config[0x51] = 0x00;
+
+/* DBX Buffer Control */
+d-config[0x53] = 0x80;
+
+/* DRAM Control */
+d-config[0x57] = 0x01;
+
+/* DRAM Timing */
+d-config[0x58] = 0x10;
+
+/* CPU Latency Timer */
+d-config[0x71] = 0x10;
+
+/* System Management RAM Control */
+d-config[0x72] = 0x02;
 
 register_savevm(I440FX, 0, 1, i440fx_save, i440fx_load, d);
 *pi440fx_state = d;
@@ -208,7 +269,7 @@ static void piix3_set_irq(qemu_irq *pic,
 {
 int i, pic_irq, pic_level;
 
-piix3_dev-config[0x60 + irq_num] = ~0x80;   // enable bit
+piix3_dev-config[0x60 + irq_num] = ~0x80;
 pci_irq_levels[irq_num] = level;
 
 /* now we change the pic irq level according to the piix irq mappings */
@@ -226,160 +287,195 @@ static void piix3_set_irq(qemu_irq *pic,
 }
 }
 
-static void piix3_reset(PCIDevice *d)
+static void piix_save(QEMUFile* f, void *opaque)
 {
-uint8_t *pci_conf = d-config;
+PCIDevice *d = opaque;
+pci_device_save(d, f);
+}
 
-pci_conf[0x04] = 0x07; // master, memory and I/O
-pci_conf[0x05] = 0x00;
-pci_conf[0x06] = 0x00;
-pci_conf[0x07] = 0x02; // PCI_status_devsel_medium
-pci_conf[0x4c] = 0x4d;
-pci_conf[0x4e] = 0x03;
-pci_conf[0x4f] = 0x00;
-pci_conf[0x60] = 0x80;
-pci_conf[0x69] = 0x02;
-pci_conf[0x70] = 0x80;
-pci_conf[0x76] = 0x0c;
-pci_conf[0x77] = 0x0c;
-pci_conf[0x78] = 0x02;
-pci_conf[0x79] = 0x00;
-pci_conf[0x80] = 0x00;
-pci_conf[0x82] = 0x00;
-pci_conf[0xa0] = 0x08;
-pci_conf[0xa2] = 0x00;
-pci_conf[0xa3] = 0x00;
-