diff mbox series

[2/2] hw/ide/piix: Ignore writes of hardwired PCI command register bits

Message ID 20221024094621.512806-3-lkujaw@mailbox.org
State New
Headers show
Series [1/2] qpci_device_enable: Allow for command bits hardwired to 0 | expand

Commit Message

Lev Kujawski Oct. 24, 2022, 9:46 a.m. UTC 
One method to enable PCI bus mastering for IDE controllers, often used
by x86 firmware, is to write 0x7 to the PCI command register.  Neither
the PIIX 3/4 specifications nor actual PIIX 3 hardware (a Tyan S1686D
system) permit setting the Memory Space Enable (MSE) bit, thus the
command register would be left in an unspecified state without this
patch.

* hw/core/machine.c
  Facilitate migration by not masking writes to the PIIX 3/4 PCICMD
  register for machine states of QEMU versions prior to 7.2.
* hw/ide/piix.c
  a) Add a reference to the PIIX 4 data sheet.
  b) Mask the MSE bit using the QEMU PCI device wmask field.
  c) Define a new boolean property, x-filter-pcicmd, to control
     whether the write mask is enabled and enable it by default
     for both the PIIX 3 and PIIX 4 IDE controllers.
* include/hw/ide/pci.h
  Add the boolean filter_pcicmd field to the PCIIDEState structure,
  because the PIIX IDE controllers do not define their own state.
* tests/qtest/ide-test.c
  Use the command_disabled field of the QPCIDevice testing model to
  indicate that PCI_COMMAND_MEMORY is hardwired within PIIX 3/4 IDE
  controllers.

Signed-off-by: Lev Kujawski <lkujaw@mailbox.org>
---
 hw/core/machine.c      |  5 ++++-
 hw/ide/piix.c          | 24 ++++++++++++++++++++++++
 include/hw/ide/pci.h   |  1 +
 tests/qtest/ide-test.c |  1 +
 4 files changed, 30 insertions(+), 1 deletion(-)
diff mbox series

Patch

diff --git a/hw/core/machine.c b/hw/core/machine.c
index aa520e74a8..8e8e69c081 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -40,7 +40,10 @@ 
 #include "hw/virtio/virtio-pci.h"
 #include "qom/object_interfaces.h"
 
-GlobalProperty hw_compat_7_1[] = {};
+GlobalProperty hw_compat_7_1[] = {
+    { "piix3-ide", "x-filter-pcicmd", "false" },
+    { "piix4-ide", "x-filter-pcicmd", "false" },
+};
 const size_t hw_compat_7_1_len = G_N_ELEMENTS(hw_compat_7_1);
 
 GlobalProperty hw_compat_7_0[] = {
diff --git a/hw/ide/piix.c b/hw/ide/piix.c
index de1f4f0efb..a3af32e126 100644
--- a/hw/ide/piix.c
+++ b/hw/ide/piix.c
@@ -25,6 +25,8 @@ 
  * References:
  *  [1] 82371FB (PIIX) AND 82371SB (PIIX3) PCI ISA IDE XCELERATOR,
  *      290550-002, Intel Corporation, April 1997.
+ *  [2] 82371AB PCI-TO-ISA / IDE XCELERATOR (PIIX4), 290562-001,
+ *      Intel Corporation, April 1997.
  */
 
 #include "qemu/osdep.h"
@@ -160,6 +162,21 @@  static void pci_piix_ide_realize(PCIDevice *dev, Error **errp)
     uint8_t *pci_conf = dev->config;
     int rc;
 
+    /*
+     * Mask all IDE PCI command register bits except for Bus Master
+     * Function Enable (bit 2) and I/O Space Enable (bit 0), as the
+     * remainder are hardwired to 0 [1, p.48] [2, p.89-90].
+     *
+     * NOTE: According to the PIIX3 datasheet [1], the Memory Space
+     * Enable (MSE, bit 1) is hardwired to 1, but this is contradicted
+     * by actual PIIX3 hardware, the datasheet itself (viz., Default
+     * Value: 0000h), and the PIIX4 datasheet [2].
+     */
+    if (d->filter_pcicmd) {
+        pci_set_word(dev->wmask + PCI_COMMAND,
+                     PCI_COMMAND_MASTER | PCI_COMMAND_IO);
+    }
+
     pci_conf[PCI_CLASS_PROG] = 0x80; // legacy ATA mode
 
     bmdma_setup_bar(d);
@@ -185,6 +202,11 @@  static void pci_piix_ide_exitfn(PCIDevice *dev)
     }
 }
 
+static Property pci_piix_ide_properties[] = {
+    DEFINE_PROP_BOOL("x-filter-pcicmd", PCIIDEState, filter_pcicmd, TRUE),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
 /* NOTE: for the PIIX3, the IRQs and IOports are hardcoded */
 static void piix3_ide_class_init(ObjectClass *klass, void *data)
 {
@@ -198,6 +220,7 @@  static void piix3_ide_class_init(ObjectClass *klass, void *data)
     k->device_id = PCI_DEVICE_ID_INTEL_82371SB_1;
     k->class_id = PCI_CLASS_STORAGE_IDE;
     set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
+    device_class_set_props(dc, pci_piix_ide_properties);
     dc->hotpluggable = false;
 }
 
@@ -220,6 +243,7 @@  static void piix4_ide_class_init(ObjectClass *klass, void *data)
     k->device_id = PCI_DEVICE_ID_INTEL_82371AB;
     k->class_id = PCI_CLASS_STORAGE_IDE;
     set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
+    device_class_set_props(dc, pci_piix_ide_properties);
     dc->hotpluggable = false;
 }
 
diff --git a/include/hw/ide/pci.h b/include/hw/ide/pci.h
index d8384e1c42..5424b00a9e 100644
--- a/include/hw/ide/pci.h
+++ b/include/hw/ide/pci.h
@@ -53,6 +53,7 @@  struct PCIIDEState {
     MemoryRegion bmdma_bar;
     MemoryRegion cmd_bar[2];
     MemoryRegion data_bar[2];
+    bool filter_pcicmd; /* used only for piix3/4 */
 };
 
 static inline IDEState *bmdma_active_if(BMDMAState *bmdma)
diff --git a/tests/qtest/ide-test.c b/tests/qtest/ide-test.c
index dbe1563b23..d5cec7c14c 100644
--- a/tests/qtest/ide-test.c
+++ b/tests/qtest/ide-test.c
@@ -174,6 +174,7 @@  static QPCIDevice *get_pci_device(QTestState *qts, QPCIBar *bmdma_bar,
 
     *ide_bar = qpci_legacy_iomap(dev, IDE_BASE);
 
+    dev->command_disabled = PCI_COMMAND_MEMORY;
     qpci_device_enable(dev);
 
     return dev;