qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH v3 2/2] hw/ide/piix: Ignore writes of hardwired PCI command regis


From: Lev Kujawski
Subject: [PATCH v3 2/2] hw/ide/piix: Ignore writes of hardwired PCI command register bits
Date: Sun, 25 Sep 2022 09:37:59 +0000

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 PIIX3 specification nor actual hardware (a Tyan S1686D system)
permit modification of the Memory Space Enable (MSE) bit, 1, and thus
the command register would be left in an unspecified state without
this patch.

* hw/ide/pci.c
  Call post_load if provided by derived IDE controller.
* hw/ide/piix.c
  a) Add references to the PIIX data sheets.
  b) Mask the MSE bit using the QEMU PCI device wmask field.
  c) Add a post_load function to mask bits from saved machine states.
  d) Specify post_load for both the PIIX3/4 IDE controllers.
* include/hw/ide/pci.h
  Switch from SIMPLE_TYPE to TYPE, explicitly create a PCIIDEClass
  that includes the post_load function pointer.
* tests/qtest/ide-test.c
  Use the command_disabled field of the QPCIDevice testing model to
  indicate that PCI_COMMAND_MEMORY is hardwired in the PIIX3/4 IDE
  controller.

Signed-off-by: Lev Kujawski <lkujaw@mailbox.org>
---
(v2) Use QEMU's built-in PCI bit-masking support rather than attempting
     to manually filter writes.  Thanks to Philippe Mathieu-Daude and
     Michael S. Tsirkin for review and the pointer.
(v3) Handle migration of older machine states, which may have set bits
     masked by this patch, via a new post_load method of PCIIDEClass.
     Thanks to Michael S. Tsirkin for catching this via review.

 hw/ide/pci.c           |  5 +++++
 hw/ide/piix.c          | 39 +++++++++++++++++++++++++++++++++++++++
 include/hw/ide/pci.h   |  7 ++++++-
 tests/qtest/ide-test.c |  1 +
 4 files changed, 51 insertions(+), 1 deletion(-)

diff --git a/hw/ide/pci.c b/hw/ide/pci.c
index 84ba733548..e42c7b9415 100644
--- a/hw/ide/pci.c
+++ b/hw/ide/pci.c
@@ -447,6 +447,7 @@ static const VMStateDescription vmstate_bmdma = {
 
 static int ide_pci_post_load(void *opaque, int version_id)
 {
+    PCIIDEClass *dc = PCI_IDE_GET_CLASS(opaque);
     PCIIDEState *d = opaque;
     int i;
 
@@ -457,6 +458,10 @@ static int ide_pci_post_load(void *opaque, int version_id)
         ide_bmdma_post_load(&d->bmdma[i], -1);
     }
 
+    if (dc->post_load) {
+        dc->post_load(d, version_id);
+    }
+
     return 0;
 }
 
diff --git a/hw/ide/piix.c b/hw/ide/piix.c
index 9a9b28078e..fd55ecbd36 100644
--- a/hw/ide/piix.c
+++ b/hw/ide/piix.c
@@ -21,6 +21,12 @@
  * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
  * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
  * THE SOFTWARE.
+ *
+ * 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"
@@ -159,6 +165,19 @@ 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].
+     */
+    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);
@@ -184,11 +203,28 @@ static void pci_piix_ide_exitfn(PCIDevice *dev)
     }
 }
 
+static int pci_piix_ide_post_load(PCIIDEState *s, int version_id)
+{
+    PCIDevice *dev = PCI_DEVICE(s);
+    uint8_t *pci_conf = dev->config;
+
+    /*
+     * To preserve backward compatibility, handle saved machine states
+     * with reserved bits set (see comment in pci_piix_ide_realize()).
+     */
+    pci_set_word(pci_conf + PCI_COMMAND,
+                 pci_get_word(pci_conf + PCI_COMMAND) &
+                 (PCI_COMMAND_MASTER | PCI_COMMAND_IO));
+
+    return 0;
+}
+
 /* NOTE: for the PIIX3, the IRQs and IOports are hardcoded */
 static void piix3_ide_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
+    PCIIDEClass *ic = PCI_IDE_CLASS(klass);
 
     dc->reset = piix_ide_reset;
     k->realize = pci_piix_ide_realize;
@@ -196,6 +232,7 @@ static void piix3_ide_class_init(ObjectClass *klass, void 
*data)
     k->vendor_id = PCI_VENDOR_ID_INTEL;
     k->device_id = PCI_DEVICE_ID_INTEL_82371SB_1;
     k->class_id = PCI_CLASS_STORAGE_IDE;
+    ic->post_load = pci_piix_ide_post_load;
     set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
     dc->hotpluggable = false;
 }
@@ -211,6 +248,7 @@ static void piix4_ide_class_init(ObjectClass *klass, void 
*data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
+    PCIIDEClass *ic = PCI_IDE_CLASS(klass);
 
     dc->reset = piix_ide_reset;
     k->realize = pci_piix_ide_realize;
@@ -218,6 +256,7 @@ static void piix4_ide_class_init(ObjectClass *klass, void 
*data)
     k->vendor_id = PCI_VENDOR_ID_INTEL;
     k->device_id = PCI_DEVICE_ID_INTEL_82371AB;
     k->class_id = PCI_CLASS_STORAGE_IDE;
+    ic->post_load = pci_piix_ide_post_load;
     set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
     dc->hotpluggable = false;
 }
diff --git a/include/hw/ide/pci.h b/include/hw/ide/pci.h
index d8384e1c42..727c748a0f 100644
--- a/include/hw/ide/pci.h
+++ b/include/hw/ide/pci.h
@@ -40,7 +40,12 @@ typedef struct BMDMAState {
 } BMDMAState;
 
 #define TYPE_PCI_IDE "pci-ide"
-OBJECT_DECLARE_SIMPLE_TYPE(PCIIDEState, PCI_IDE)
+OBJECT_DECLARE_TYPE(PCIIDEState, PCIIDEClass, PCI_IDE)
+
+struct PCIIDEClass {
+    IDEDeviceClass parent_class;
+    int (*post_load)(PCIIDEState *s, int version_id);
+};
 
 struct PCIIDEState {
     /*< private >*/
diff --git a/tests/qtest/ide-test.c b/tests/qtest/ide-test.c
index 5bcb75a7e5..85a3967063 100644
--- a/tests/qtest/ide-test.c
+++ b/tests/qtest/ide-test.c
@@ -173,6 +173,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;
-- 
2.34.1




reply via email to

[Prev in Thread] Current Thread [Next in Thread]