| Message ID | 20221031124928.128475-69-mst@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | [PULL,01/86] bios-tables-test: do not ignore allowed diff list | expand |
On Mon, Oct 31, 2022 at 08:53:57AM -0400, Michael S. Tsirkin wrote: > From: Lev Kujawski <lkujaw@mailbox.org> > > 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> > Message-Id: <20221024094621.512806-3-lkujaw@mailbox.org> > Reviewed-by: Michael S. Tsirkin <mst@redhat.com> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> Caused test asserts reported by Stefan. Dropped for now, pls figure the issue out and resubmit. Thanks! > --- > include/hw/ide/pci.h | 1 + > hw/core/machine.c | 2 ++ > hw/ide/piix.c | 24 ++++++++++++++++++++++++ > tests/qtest/ide-test.c | 1 + > 4 files changed, 28 insertions(+) > > 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/hw/core/machine.c b/hw/core/machine.c > index 907fa78ff0..65fdfe2fed 100644 > --- a/hw/core/machine.c > +++ b/hw/core/machine.c > @@ -42,6 +42,8 @@ > > GlobalProperty hw_compat_7_1[] = { > { "virtio-device", "queue_reset", "false" }, > + { "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); > > 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/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; > -- > MST >
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/hw/core/machine.c b/hw/core/machine.c index 907fa78ff0..65fdfe2fed 100644 --- a/hw/core/machine.c +++ b/hw/core/machine.c @@ -42,6 +42,8 @@ GlobalProperty hw_compat_7_1[] = { { "virtio-device", "queue_reset", "false" }, + { "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); 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/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;