* [PATCH v2 7/7] hw/ide: Remove unneeded inclusion of hw/ide.h
2020-03-17 9:39 [PATCH v2 0/7] Misc hw/ide legacy clean up BALATON Zoltan
@ 2020-03-17 9:39 ` BALATON Zoltan
2020-03-17 9:39 ` [PATCH v2 3/7] hw/ide: Remove now unneded #include "hw/pci/pci.h" from hw/ide.h BALATON Zoltan
` (5 subsequent siblings)
6 siblings, 0 replies; 20+ messages in thread
From: BALATON Zoltan @ 2020-03-17 9:39 UTC (permalink / raw)
To: qemu-devel, qemu-block
Cc: Eduardo Habkost, Michael S. Tsirkin, John Snow, Mark Cave-Ayland,
Markus Armbruster, hpoussin, Aleksandar Markovic, Paolo Bonzini,
philmd, Artyom Tarasenko, Richard Henderson
After previous clean ups we can drop direct inclusion of hw/ide.h from
several places.
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
---
hw/hppa/hppa_sys.h | 1 -
hw/hppa/machine.c | 1 -
hw/i386/pc_piix.c | 1 -
hw/isa/piix4.c | 1 -
hw/mips/mips_fulong2e.c | 1 -
hw/ppc/mac_newworld.c | 1 -
hw/ppc/mac_oldworld.c | 1 -
hw/ppc/prep.c | 1 -
8 files changed, 8 deletions(-)
diff --git a/hw/hppa/hppa_sys.h b/hw/hppa/hppa_sys.h
index 4d08501464..0b18271cc9 100644
--- a/hw/hppa/hppa_sys.h
+++ b/hw/hppa/hppa_sys.h
@@ -5,7 +5,6 @@
#include "hw/pci/pci.h"
#include "hw/pci/pci_host.h"
-#include "hw/ide.h"
#include "hw/boards.h"
#include "hw/intc/i8259.h"
diff --git a/hw/hppa/machine.c b/hw/hppa/machine.c
index 9175f4b790..00dd9f58d6 100644
--- a/hw/hppa/machine.c
+++ b/hw/hppa/machine.c
@@ -13,7 +13,6 @@
#include "sysemu/reset.h"
#include "sysemu/sysemu.h"
#include "hw/rtc/mc146818rtc.h"
-#include "hw/ide.h"
#include "hw/timer/i8254.h"
#include "hw/char/serial.h"
#include "hw/net/lasi_82596.h"
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 9216596ec6..e6756216f9 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -38,7 +38,6 @@
#include "hw/pci/pci_ids.h"
#include "hw/usb.h"
#include "net/net.h"
-#include "hw/ide.h"
#include "hw/ide/pci.h"
#include "hw/irq.h"
#include "sysemu/kvm.h"
diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
index 13fa1660c3..136a763e3f 100644
--- a/hw/isa/piix4.c
+++ b/hw/isa/piix4.c
@@ -34,7 +34,6 @@
#include "hw/dma/i8257.h"
#include "hw/timer/i8254.h"
#include "hw/rtc/mc146818rtc.h"
-#include "hw/ide.h"
#include "hw/ide/pci.h"
#include "migration/vmstate.h"
#include "sysemu/reset.h"
diff --git a/hw/mips/mips_fulong2e.c b/hw/mips/mips_fulong2e.c
index 0f312b5a35..5040afd581 100644
--- a/hw/mips/mips_fulong2e.c
+++ b/hw/mips/mips_fulong2e.c
@@ -36,7 +36,6 @@
#include "audio/audio.h"
#include "qemu/log.h"
#include "hw/loader.h"
-#include "hw/ide.h"
#include "hw/ide/pci.h"
#include "elf.h"
#include "hw/isa/vt82c686.h"
diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
index b8189bf7a4..13164ee9d7 100644
--- a/hw/ppc/mac_newworld.c
+++ b/hw/ppc/mac_newworld.c
@@ -62,7 +62,6 @@
#include "hw/char/escc.h"
#include "hw/misc/macio/macio.h"
#include "hw/ppc/openpic.h"
-#include "hw/ide.h"
#include "hw/loader.h"
#include "hw/fw-path-provider.h"
#include "elf.h"
diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
index 440c406eb4..2d419d82fa 100644
--- a/hw/ppc/mac_oldworld.c
+++ b/hw/ppc/mac_oldworld.c
@@ -41,7 +41,6 @@
#include "hw/nvram/fw_cfg.h"
#include "hw/char/escc.h"
#include "hw/misc/macio/macio.h"
-#include "hw/ide.h"
#include "hw/loader.h"
#include "hw/fw-path-provider.h"
#include "elf.h"
diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
index 111cc80867..44be9d25a2 100644
--- a/hw/ppc/prep.c
+++ b/hw/ppc/prep.c
@@ -37,7 +37,6 @@
#include "hw/boards.h"
#include "qemu/error-report.h"
#include "qemu/log.h"
-#include "hw/ide.h"
#include "hw/irq.h"
#include "hw/loader.h"
#include "hw/rtc/mc146818rtc.h"
--
2.21.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH v2 3/7] hw/ide: Remove now unneded #include "hw/pci/pci.h" from hw/ide.h
2020-03-17 9:39 [PATCH v2 0/7] Misc hw/ide legacy clean up BALATON Zoltan
2020-03-17 9:39 ` [PATCH v2 7/7] hw/ide: Remove unneeded inclusion of hw/ide.h BALATON Zoltan
@ 2020-03-17 9:39 ` BALATON Zoltan
2020-03-17 9:39 ` [PATCH v2 5/7] hw/ide: Do ide_drive_get() within pci_ide_create_devs() BALATON Zoltan
` (4 subsequent siblings)
6 siblings, 0 replies; 20+ messages in thread
From: BALATON Zoltan @ 2020-03-17 9:39 UTC (permalink / raw)
To: qemu-devel, qemu-block
Cc: Eduardo Habkost, Michael S. Tsirkin, John Snow, Mark Cave-Ayland,
Markus Armbruster, hpoussin, Aleksandar Markovic, Paolo Bonzini,
philmd, Artyom Tarasenko, Richard Henderson
After previous patches we don't need hw/pci/pci.h any more in
hw/ide.h. Some files depended on implicit inclusion by this header
which are also fixed up here.
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
---
hw/ide/ahci_internal.h | 1 +
include/hw/ide.h | 1 -
include/hw/ide/pci.h | 1 +
include/hw/misc/macio/macio.h | 1 +
4 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/hw/ide/ahci_internal.h b/hw/ide/ahci_internal.h
index 73424516da..bab0459774 100644
--- a/hw/ide/ahci_internal.h
+++ b/hw/ide/ahci_internal.h
@@ -27,6 +27,7 @@
#include "hw/ide/ahci.h"
#include "hw/ide/internal.h"
#include "hw/sysbus.h"
+#include "hw/pci/pci.h"
#define AHCI_MEM_BAR_SIZE 0x1000
#define AHCI_MAX_PORTS 32
diff --git a/include/hw/ide.h b/include/hw/ide.h
index 21bd8f23f1..d52c211f32 100644
--- a/include/hw/ide.h
+++ b/include/hw/ide.h
@@ -2,7 +2,6 @@
#define HW_IDE_H
#include "hw/isa/isa.h"
-#include "hw/pci/pci.h"
#include "exec/memory.h"
#define MAX_IDE_DEVS 2
diff --git a/include/hw/ide/pci.h b/include/hw/ide/pci.h
index a9f2c33e68..98ffa7dfcd 100644
--- a/include/hw/ide/pci.h
+++ b/include/hw/ide/pci.h
@@ -2,6 +2,7 @@
#define HW_IDE_PCI_H
#include "hw/ide/internal.h"
+#include "hw/pci/pci.h"
#define BM_STATUS_DMAING 0x01
#define BM_STATUS_ERROR 0x02
diff --git a/include/hw/misc/macio/macio.h b/include/hw/misc/macio/macio.h
index 070a694eb5..87335a991c 100644
--- a/include/hw/misc/macio/macio.h
+++ b/include/hw/misc/macio/macio.h
@@ -27,6 +27,7 @@
#define MACIO_H
#include "hw/char/escc.h"
+#include "hw/pci/pci.h"
#include "hw/ide/internal.h"
#include "hw/intc/heathrow_pic.h"
#include "hw/misc/macio/cuda.h"
--
2.21.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH v2 5/7] hw/ide: Do ide_drive_get() within pci_ide_create_devs()
2020-03-17 9:39 [PATCH v2 0/7] Misc hw/ide legacy clean up BALATON Zoltan
2020-03-17 9:39 ` [PATCH v2 7/7] hw/ide: Remove unneeded inclusion of hw/ide.h BALATON Zoltan
2020-03-17 9:39 ` [PATCH v2 3/7] hw/ide: Remove now unneded #include "hw/pci/pci.h" from hw/ide.h BALATON Zoltan
@ 2020-03-17 9:39 ` BALATON Zoltan
2020-03-17 9:39 ` [PATCH v2 2/7] hw/ide: Get rid of piix4_init function BALATON Zoltan
` (3 subsequent siblings)
6 siblings, 0 replies; 20+ messages in thread
From: BALATON Zoltan @ 2020-03-17 9:39 UTC (permalink / raw)
To: qemu-devel, qemu-block
Cc: Eduardo Habkost, Michael S. Tsirkin, John Snow, Mark Cave-Ayland,
Markus Armbruster, hpoussin, Aleksandar Markovic, Paolo Bonzini,
philmd, Artyom Tarasenko, Richard Henderson
The pci_ide_create_devs() function takes a hd_table parameter but all
callers just pass what ide_drive_get() returns so we can do it locally
simplifying callers and removing hd_table parameter.
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
---
hw/alpha/dp264.c | 13 +++----------
hw/i386/pc_piix.c | 9 +++++----
hw/ide/pci.c | 4 +++-
hw/isa/piix4.c | 10 ++--------
hw/mips/mips_fulong2e.c | 4 +---
hw/mips/mips_malta.c | 2 +-
hw/sparc64/sun4u.c | 6 +-----
include/hw/ide/pci.h | 2 +-
include/hw/southbridge/piix.h | 3 +--
9 files changed, 18 insertions(+), 35 deletions(-)
diff --git a/hw/alpha/dp264.c b/hw/alpha/dp264.c
index 27595767e5..f7751b18f6 100644
--- a/hw/alpha/dp264.c
+++ b/hw/alpha/dp264.c
@@ -15,7 +15,6 @@
#include "qemu/error-report.h"
#include "sysemu/sysemu.h"
#include "hw/rtc/mc146818rtc.h"
-#include "hw/ide.h"
#include "hw/ide/pci.h"
#include "hw/timer/i8254.h"
#include "hw/isa/superio.h"
@@ -58,6 +57,7 @@ static void clipper_init(MachineState *machine)
const char *initrd_filename = machine->initrd_filename;
AlphaCPU *cpus[4];
PCIBus *pci_bus;
+ PCIDevice *pci_dev;
ISABus *isa_bus;
qemu_irq rtc_irq;
long size, i;
@@ -100,15 +100,8 @@ static void clipper_init(MachineState *machine)
isa_create_simple(isa_bus, TYPE_SMC37C669_SUPERIO);
/* IDE disk setup. */
- {
- DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
- PCIDevice *pci_dev;
-
- ide_drive_get(hd, ARRAY_SIZE(hd));
-
- pci_dev = pci_create_simple(pci_bus, -1, "cmd646-ide");
- pci_ide_create_devs(pci_dev, hd);
- }
+ pci_dev = pci_create_simple(pci_bus, -1, "cmd646-ide");
+ pci_ide_create_devs(pci_dev);
/* Load PALcode. Given that this is not "real" cpu palcode,
but one explicitly written for the emulation, we might as
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index c399398739..9216596ec6 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -86,7 +86,6 @@ static void pc_init1(MachineState *machine,
int piix3_devfn = -1;
qemu_irq smi_irq;
GSIState *gsi_state;
- DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
BusState *idebus[MAX_IDE_BUS];
ISADevice *rtc_state;
MemoryRegion *ram_memory;
@@ -240,20 +239,22 @@ static void pc_init1(MachineState *machine,
pc_nic_init(pcmc, isa_bus, pci_bus);
- ide_drive_get(hd, ARRAY_SIZE(hd));
if (pcmc->pci_enabled) {
PCIDevice *dev;
dev = pci_create_simple(pci_bus, piix3_devfn + 1,
xen_enabled() ? "piix3-ide-xen" : "piix3-ide");
- pci_ide_create_devs(dev, hd);
+ pci_ide_create_devs(dev);
idebus[0] = qdev_get_child_bus(&dev->qdev, "ide.0");
idebus[1] = qdev_get_child_bus(&dev->qdev, "ide.1");
pc_cmos_init(pcms, idebus[0], idebus[1], rtc_state);
}
#ifdef CONFIG_IDE_ISA
-else {
+ else {
+ DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
int i;
+
+ ide_drive_get(hd, ARRAY_SIZE(hd));
for (i = 0; i < MAX_IDE_BUS; i++) {
ISADevice *dev;
char busname[] = "ide.0";
diff --git a/hw/ide/pci.c b/hw/ide/pci.c
index e0c84392e2..97347f07f1 100644
--- a/hw/ide/pci.c
+++ b/hw/ide/pci.c
@@ -477,13 +477,15 @@ const VMStateDescription vmstate_ide_pci = {
};
/* hd_table must contain 4 block drivers */
-void pci_ide_create_devs(PCIDevice *dev, DriveInfo **hd_table)
+void pci_ide_create_devs(PCIDevice *dev)
{
PCIIDEState *d = PCI_IDE(dev);
+ DriveInfo *hd_table[2 * MAX_IDE_DEVS];
static const int bus[4] = { 0, 0, 1, 1 };
static const int unit[4] = { 0, 1, 0, 1 };
int i;
+ ide_drive_get(hd_table, ARRAY_SIZE(hd_table));
for (i = 0; i < 4; i++) {
if (hd_table[i]) {
ide_create_drive(d->bus + bus[i], unit[i], hd_table[i]);
diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
index 0ab4787658..13fa1660c3 100644
--- a/hw/isa/piix4.c
+++ b/hw/isa/piix4.c
@@ -241,11 +241,8 @@ static void piix4_register_types(void)
type_init(piix4_register_types)
-DeviceState *piix4_create(PCIBus *pci_bus, ISABus **isa_bus,
- I2CBus **smbus, size_t ide_buses)
+DeviceState *piix4_create(PCIBus *pci_bus, ISABus **isa_bus, I2CBus **smbus)
{
- size_t ide_drives = ide_buses * MAX_IDE_DEVS;
- DriveInfo **hd;
PCIDevice *pci;
DeviceState *dev;
@@ -257,10 +254,7 @@ DeviceState *piix4_create(PCIBus *pci_bus, ISABus **isa_bus,
}
pci = pci_create_simple(pci_bus, pci->devfn + 1, "piix4-ide");
- hd = g_new(DriveInfo *, ide_drives);
- ide_drive_get(hd, ide_drives);
- pci_ide_create_devs(pci, hd);
- g_free(hd);
+ pci_ide_create_devs(pci);
pci_create_simple(pci_bus, pci->devfn + 2, "piix4-usb-uhci");
if (smbus) {
diff --git a/hw/mips/mips_fulong2e.c b/hw/mips/mips_fulong2e.c
index 639ba2a091..0f312b5a35 100644
--- a/hw/mips/mips_fulong2e.c
+++ b/hw/mips/mips_fulong2e.c
@@ -239,7 +239,6 @@ static void vt82c686b_southbridge_init(PCIBus *pci_bus, int slot, qemu_irq intc,
{
qemu_irq *i8259;
ISABus *isa_bus;
- DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
PCIDevice *dev;
isa_bus = vt82c686b_isa_init(pci_bus, PCI_DEVFN(slot, 0));
@@ -259,8 +258,7 @@ static void vt82c686b_southbridge_init(PCIBus *pci_bus, int slot, qemu_irq intc,
isa_create_simple(isa_bus, TYPE_VT82C686B_SUPERIO);
dev = pci_create_simple(pci_bus, PCI_DEVFN(slot, 1), "via-ide");
- ide_drive_get(hd, ARRAY_SIZE(hd));
- pci_ide_create_devs(dev, hd);
+ pci_ide_create_devs(dev);
pci_create_simple(pci_bus, PCI_DEVFN(slot, 2), "vt82c686b-usb-uhci");
pci_create_simple(pci_bus, PCI_DEVFN(slot, 3), "vt82c686b-usb-uhci");
diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
index d380f73d7b..e4c4de1b4e 100644
--- a/hw/mips/mips_malta.c
+++ b/hw/mips/mips_malta.c
@@ -1403,7 +1403,7 @@ void mips_malta_init(MachineState *machine)
pci_bus = gt64120_register(s->i8259);
/* Southbridge */
- dev = piix4_create(pci_bus, &isa_bus, &smbus, MAX_IDE_BUS);
+ dev = piix4_create(pci_bus, &isa_bus, &smbus);
/* Interrupt controller */
qdev_connect_gpio_out_named(dev, "intr", 0, i8259_irq);
diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
index d33e84f831..6abfcb30f8 100644
--- a/hw/sparc64/sun4u.c
+++ b/hw/sparc64/sun4u.c
@@ -50,7 +50,6 @@
#include "hw/sparc/sparc64.h"
#include "hw/nvram/fw_cfg.h"
#include "hw/sysbus.h"
-#include "hw/ide.h"
#include "hw/ide/pci.h"
#include "hw/loader.h"
#include "hw/fw-path-provider.h"
@@ -563,7 +562,6 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
PCIBus *pci_bus, *pci_busA, *pci_busB;
PCIDevice *ebus, *pci_dev;
SysBusDevice *s;
- DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
DeviceState *iommu, *dev;
FWCfgState *fw_cfg;
NICInfo *nd;
@@ -663,12 +661,10 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
qemu_macaddr_default_if_unset(&macaddr);
}
- ide_drive_get(hd, ARRAY_SIZE(hd));
-
pci_dev = pci_create(pci_busA, PCI_DEVFN(3, 0), "cmd646-ide");
qdev_prop_set_uint32(&pci_dev->qdev, "secondary", 1);
qdev_init_nofail(&pci_dev->qdev);
- pci_ide_create_devs(pci_dev, hd);
+ pci_ide_create_devs(pci_dev);
/* Map NVRAM into I/O (ebus) space */
nvram = m48t59_init(NULL, 0, 0, NVRAM_SIZE, 1968, 59);
diff --git a/include/hw/ide/pci.h b/include/hw/ide/pci.h
index 98ffa7dfcd..dd504e5a0b 100644
--- a/include/hw/ide/pci.h
+++ b/include/hw/ide/pci.h
@@ -63,7 +63,7 @@ static inline IDEState *bmdma_active_if(BMDMAState *bmdma)
void bmdma_init(IDEBus *bus, BMDMAState *bm, PCIIDEState *d);
void bmdma_cmd_writeb(BMDMAState *bm, uint32_t val);
extern MemoryRegionOps bmdma_addr_ioport_ops;
-void pci_ide_create_devs(PCIDevice *dev, DriveInfo **hd_table);
+void pci_ide_create_devs(PCIDevice *dev);
extern const VMStateDescription vmstate_ide_pci;
extern const MemoryRegionOps pci_ide_cmd_le_ops;
diff --git a/include/hw/southbridge/piix.h b/include/hw/southbridge/piix.h
index 152628c6d9..02bd741209 100644
--- a/include/hw/southbridge/piix.h
+++ b/include/hw/southbridge/piix.h
@@ -68,7 +68,6 @@ extern PCIDevice *piix4_dev;
PIIX3State *piix3_create(PCIBus *pci_bus, ISABus **isa_bus);
-DeviceState *piix4_create(PCIBus *pci_bus, ISABus **isa_bus,
- I2CBus **smbus, size_t ide_buses);
+DeviceState *piix4_create(PCIBus *pci_bus, ISABus **isa_bus, I2CBus **smbus);
#endif
--
2.21.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH v2 2/7] hw/ide: Get rid of piix4_init function
2020-03-17 9:39 [PATCH v2 0/7] Misc hw/ide legacy clean up BALATON Zoltan
` (2 preceding siblings ...)
2020-03-17 9:39 ` [PATCH v2 5/7] hw/ide: Do ide_drive_get() within pci_ide_create_devs() BALATON Zoltan
@ 2020-03-17 9:39 ` BALATON Zoltan
2020-03-17 10:41 ` Philippe Mathieu-Daudé
2020-03-17 9:39 ` [PATCH v2 4/7] hw/ide/pci.c: Coding style update to fix checkpatch errors BALATON Zoltan
` (2 subsequent siblings)
6 siblings, 1 reply; 20+ messages in thread
From: BALATON Zoltan @ 2020-03-17 9:39 UTC (permalink / raw)
To: qemu-devel, qemu-block
Cc: Eduardo Habkost, Michael S. Tsirkin, John Snow, Mark Cave-Ayland,
Markus Armbruster, hpoussin, Aleksandar Markovic, Paolo Bonzini,
philmd, Artyom Tarasenko, Richard Henderson
This removes pci_piix4_ide_init() function similar to clean up done to
other ide devices.
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
---
hw/ide/piix.c | 12 +-----------
hw/isa/piix4.c | 5 ++++-
include/hw/ide.h | 1 -
3 files changed, 5 insertions(+), 13 deletions(-)
diff --git a/hw/ide/piix.c b/hw/ide/piix.c
index 8bcd6b72c2..3b2de4c312 100644
--- a/hw/ide/piix.c
+++ b/hw/ide/piix.c
@@ -208,17 +208,6 @@ static void pci_piix_ide_exitfn(PCIDevice *dev)
}
}
-/* hd_table must contain 4 block drivers */
-/* NOTE: for the PIIX4, the IRQs and IOports are hardcoded */
-PCIDevice *pci_piix4_ide_init(PCIBus *bus, DriveInfo **hd_table, int devfn)
-{
- PCIDevice *dev;
-
- dev = pci_create_simple(bus, devfn, "piix4-ide");
- pci_ide_create_devs(dev, hd_table);
- return dev;
-}
-
/* NOTE: for the PIIX3, the IRQs and IOports are hardcoded */
static void piix3_ide_class_init(ObjectClass *klass, void *data)
{
@@ -247,6 +236,7 @@ static const TypeInfo piix3_ide_xen_info = {
.class_init = piix3_ide_class_init,
};
+/* NOTE: for the PIIX4, the IRQs and IOports are hardcoded */
static void piix4_ide_class_init(ObjectClass *klass, void *data)
{
DeviceClass *dc = DEVICE_CLASS(klass);
diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
index 7edec5e149..0ab4787658 100644
--- a/hw/isa/piix4.c
+++ b/hw/isa/piix4.c
@@ -35,6 +35,7 @@
#include "hw/timer/i8254.h"
#include "hw/rtc/mc146818rtc.h"
#include "hw/ide.h"
+#include "hw/ide/pci.h"
#include "migration/vmstate.h"
#include "sysemu/reset.h"
#include "sysemu/runstate.h"
@@ -255,10 +256,12 @@ DeviceState *piix4_create(PCIBus *pci_bus, ISABus **isa_bus,
*isa_bus = ISA_BUS(qdev_get_child_bus(dev, "isa.0"));
}
+ pci = pci_create_simple(pci_bus, pci->devfn + 1, "piix4-ide");
hd = g_new(DriveInfo *, ide_drives);
ide_drive_get(hd, ide_drives);
- pci_piix4_ide_init(pci_bus, hd, pci->devfn + 1);
+ pci_ide_create_devs(pci, hd);
g_free(hd);
+
pci_create_simple(pci_bus, pci->devfn + 2, "piix4-usb-uhci");
if (smbus) {
*smbus = piix4_pm_init(pci_bus, pci->devfn + 3, 0x1100,
diff --git a/include/hw/ide.h b/include/hw/ide.h
index 883bbaeb9b..21bd8f23f1 100644
--- a/include/hw/ide.h
+++ b/include/hw/ide.h
@@ -12,7 +12,6 @@ ISADevice *isa_ide_init(ISABus *bus, int iobase, int iobase2, int isairq,
DriveInfo *hd0, DriveInfo *hd1);
/* ide-pci.c */
-PCIDevice *pci_piix4_ide_init(PCIBus *bus, DriveInfo **hd_table, int devfn);
int pci_piix3_xen_ide_unplug(DeviceState *dev, bool aux);
/* ide-mmio.c */
--
2.21.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v2 2/7] hw/ide: Get rid of piix4_init function
2020-03-17 9:39 ` [PATCH v2 2/7] hw/ide: Get rid of piix4_init function BALATON Zoltan
@ 2020-03-17 10:41 ` Philippe Mathieu-Daudé
2020-03-17 10:49 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 20+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-03-17 10:41 UTC (permalink / raw)
To: BALATON Zoltan, qemu-devel, qemu-block
Cc: Eduardo Habkost, Michael S. Tsirkin, Mark Cave-Ayland,
Markus Armbruster, hpoussin, Aleksandar Markovic, Paolo Bonzini,
John Snow, Artyom Tarasenko, Richard Henderson
On 3/17/20 10:39 AM, BALATON Zoltan wrote:
> This removes pci_piix4_ide_init() function similar to clean up done to
> other ide devices.
>
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> Reviewed-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> hw/ide/piix.c | 12 +-----------
> hw/isa/piix4.c | 5 ++++-
> include/hw/ide.h | 1 -
> 3 files changed, 5 insertions(+), 13 deletions(-)
>
> diff --git a/hw/ide/piix.c b/hw/ide/piix.c
> index 8bcd6b72c2..3b2de4c312 100644
> --- a/hw/ide/piix.c
> +++ b/hw/ide/piix.c
> @@ -208,17 +208,6 @@ static void pci_piix_ide_exitfn(PCIDevice *dev)
> }
> }
>
> -/* hd_table must contain 4 block drivers */
> -/* NOTE: for the PIIX4, the IRQs and IOports are hardcoded */
> -PCIDevice *pci_piix4_ide_init(PCIBus *bus, DriveInfo **hd_table, int devfn)
> -{
> - PCIDevice *dev;
> -
> - dev = pci_create_simple(bus, devfn, "piix4-ide");
> - pci_ide_create_devs(dev, hd_table);
> - return dev;
> -}
> -
> /* NOTE: for the PIIX3, the IRQs and IOports are hardcoded */
> static void piix3_ide_class_init(ObjectClass *klass, void *data)
> {
> @@ -247,6 +236,7 @@ static const TypeInfo piix3_ide_xen_info = {
> .class_init = piix3_ide_class_init,
> };
>
> +/* NOTE: for the PIIX4, the IRQs and IOports are hardcoded */
> static void piix4_ide_class_init(ObjectClass *klass, void *data)
> {
> DeviceClass *dc = DEVICE_CLASS(klass);
> diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
> index 7edec5e149..0ab4787658 100644
> --- a/hw/isa/piix4.c
> +++ b/hw/isa/piix4.c
> @@ -35,6 +35,7 @@
> #include "hw/timer/i8254.h"
> #include "hw/rtc/mc146818rtc.h"
> #include "hw/ide.h"
> +#include "hw/ide/pci.h"
> #include "migration/vmstate.h"
> #include "sysemu/reset.h"
> #include "sysemu/runstate.h"
> @@ -255,10 +256,12 @@ DeviceState *piix4_create(PCIBus *pci_bus, ISABus **isa_bus,
> *isa_bus = ISA_BUS(qdev_get_child_bus(dev, "isa.0"));
> }
>
> + pci = pci_create_simple(pci_bus, pci->devfn + 1, "piix4-ide");
> hd = g_new(DriveInfo *, ide_drives);
> ide_drive_get(hd, ide_drives);
> - pci_piix4_ide_init(pci_bus, hd, pci->devfn + 1);
> + pci_ide_create_devs(pci, hd);
> g_free(hd);
> +
> pci_create_simple(pci_bus, pci->devfn + 2, "piix4-usb-uhci");
> if (smbus) {
> *smbus = piix4_pm_init(pci_bus, pci->devfn + 3, 0x1100,
> diff --git a/include/hw/ide.h b/include/hw/ide.h
> index 883bbaeb9b..21bd8f23f1 100644
> --- a/include/hw/ide.h
> +++ b/include/hw/ide.h
> @@ -12,7 +12,6 @@ ISADevice *isa_ide_init(ISABus *bus, int iobase, int iobase2, int isairq,
> DriveInfo *hd0, DriveInfo *hd1);
>
> /* ide-pci.c */
> -PCIDevice *pci_piix4_ide_init(PCIBus *bus, DriveInfo **hd_table, int devfn);
> int pci_piix3_xen_ide_unplug(DeviceState *dev, bool aux);
>
> /* ide-mmio.c */
>
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v2 2/7] hw/ide: Get rid of piix4_init function
2020-03-17 10:41 ` Philippe Mathieu-Daudé
@ 2020-03-17 10:49 ` Philippe Mathieu-Daudé
2020-03-17 13:50 ` John Snow
0 siblings, 1 reply; 20+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-03-17 10:49 UTC (permalink / raw)
To: BALATON Zoltan, qemu-devel, qemu-block
Cc: Eduardo Habkost, Michael S. Tsirkin, Mark Cave-Ayland,
Markus Armbruster, hpoussin, Aleksandar Markovic, Paolo Bonzini,
John Snow, Artyom Tarasenko, Richard Henderson
On 3/17/20 11:41 AM, Philippe Mathieu-Daudé wrote:
> On 3/17/20 10:39 AM, BALATON Zoltan wrote:
>> This removes pci_piix4_ide_init() function similar to clean up done to
>> other ide devices.
>>
>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>> Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>> Reviewed-by: Markus Armbruster <armbru@redhat.com>
>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Please disregard this tag (I withdraw it), I mis-read the pci variable
was not assigned.
>
>> ---
>> hw/ide/piix.c | 12 +-----------
>> hw/isa/piix4.c | 5 ++++-
>> include/hw/ide.h | 1 -
>> 3 files changed, 5 insertions(+), 13 deletions(-)
>>
>> diff --git a/hw/ide/piix.c b/hw/ide/piix.c
>> index 8bcd6b72c2..3b2de4c312 100644
>> --- a/hw/ide/piix.c
>> +++ b/hw/ide/piix.c
>> @@ -208,17 +208,6 @@ static void pci_piix_ide_exitfn(PCIDevice *dev)
>> }
>> }
>> -/* hd_table must contain 4 block drivers */
>> -/* NOTE: for the PIIX4, the IRQs and IOports are hardcoded */
>> -PCIDevice *pci_piix4_ide_init(PCIBus *bus, DriveInfo **hd_table, int
>> devfn)
>> -{
>> - PCIDevice *dev;
>> -
>> - dev = pci_create_simple(bus, devfn, "piix4-ide");
>> - pci_ide_create_devs(dev, hd_table);
>> - return dev;
>> -}
>> -
>> /* NOTE: for the PIIX3, the IRQs and IOports are hardcoded */
>> static void piix3_ide_class_init(ObjectClass *klass, void *data)
>> {
>> @@ -247,6 +236,7 @@ static const TypeInfo piix3_ide_xen_info = {
>> .class_init = piix3_ide_class_init,
>> };
>> +/* NOTE: for the PIIX4, the IRQs and IOports are hardcoded */
>> static void piix4_ide_class_init(ObjectClass *klass, void *data)
>> {
>> DeviceClass *dc = DEVICE_CLASS(klass);
>> diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
>> index 7edec5e149..0ab4787658 100644
>> --- a/hw/isa/piix4.c
>> +++ b/hw/isa/piix4.c
>> @@ -35,6 +35,7 @@
>> #include "hw/timer/i8254.h"
>> #include "hw/rtc/mc146818rtc.h"
>> #include "hw/ide.h"
>> +#include "hw/ide/pci.h"
>> #include "migration/vmstate.h"
>> #include "sysemu/reset.h"
>> #include "sysemu/runstate.h"
>> @@ -255,10 +256,12 @@ DeviceState *piix4_create(PCIBus *pci_bus,
>> ISABus **isa_bus,
>> *isa_bus = ISA_BUS(qdev_get_child_bus(dev, "isa.0"));
>> }
>> + pci = pci_create_simple(pci_bus, pci->devfn + 1, "piix4-ide");
>> hd = g_new(DriveInfo *, ide_drives);
>> ide_drive_get(hd, ide_drives);
>> - pci_piix4_ide_init(pci_bus, hd, pci->devfn + 1);
>> + pci_ide_create_devs(pci, hd);
>> g_free(hd);
>> +
>> pci_create_simple(pci_bus, pci->devfn + 2, "piix4-usb-uhci");
>> if (smbus) {
>> *smbus = piix4_pm_init(pci_bus, pci->devfn + 3, 0x1100,
>> diff --git a/include/hw/ide.h b/include/hw/ide.h
>> index 883bbaeb9b..21bd8f23f1 100644
>> --- a/include/hw/ide.h
>> +++ b/include/hw/ide.h
>> @@ -12,7 +12,6 @@ ISADevice *isa_ide_init(ISABus *bus, int iobase, int
>> iobase2, int isairq,
>> DriveInfo *hd0, DriveInfo *hd1);
>> /* ide-pci.c */
>> -PCIDevice *pci_piix4_ide_init(PCIBus *bus, DriveInfo **hd_table, int
>> devfn);
>> int pci_piix3_xen_ide_unplug(DeviceState *dev, bool aux);
>> /* ide-mmio.c */
>>
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v2 2/7] hw/ide: Get rid of piix4_init function
2020-03-17 10:49 ` Philippe Mathieu-Daudé
@ 2020-03-17 13:50 ` John Snow
2020-03-17 14:01 ` Philippe Mathieu-Daudé
2020-03-17 14:05 ` BALATON Zoltan
0 siblings, 2 replies; 20+ messages in thread
From: John Snow @ 2020-03-17 13:50 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, BALATON Zoltan, qemu-devel,
qemu-block
Cc: Eduardo Habkost, Michael S. Tsirkin, Mark Cave-Ayland,
Markus Armbruster, hpoussin, Aleksandar Markovic, Paolo Bonzini,
Artyom Tarasenko, Richard Henderson
On 3/17/20 6:49 AM, Philippe Mathieu-Daudé wrote:
> On 3/17/20 11:41 AM, Philippe Mathieu-Daudé wrote:
>> On 3/17/20 10:39 AM, BALATON Zoltan wrote:
>>> This removes pci_piix4_ide_init() function similar to clean up done to
>>> other ide devices.
>>>
>>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>>> Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>>> Reviewed-by: Markus Armbruster <armbru@redhat.com>
>>
>> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>
> Please disregard this tag (I withdraw it), I mis-read the pci variable
> was not assigned.
>
Is there an issue you've noticed, or you are just no longer certain
enough to give an RB?
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 2/7] hw/ide: Get rid of piix4_init function
2020-03-17 13:50 ` John Snow
@ 2020-03-17 14:01 ` Philippe Mathieu-Daudé
2020-03-17 14:07 ` BALATON Zoltan
2020-03-17 14:05 ` BALATON Zoltan
1 sibling, 1 reply; 20+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-03-17 14:01 UTC (permalink / raw)
To: John Snow, BALATON Zoltan, qemu-devel, qemu-block
Cc: Eduardo Habkost, Michael S. Tsirkin, Mark Cave-Ayland,
Markus Armbruster, hpoussin, Aleksandar Markovic, Paolo Bonzini,
Artyom Tarasenko, Richard Henderson
On 3/17/20 2:50 PM, John Snow wrote:
> On 3/17/20 6:49 AM, Philippe Mathieu-Daudé wrote:
>> On 3/17/20 11:41 AM, Philippe Mathieu-Daudé wrote:
>>> On 3/17/20 10:39 AM, BALATON Zoltan wrote:
>>>> This removes pci_piix4_ide_init() function similar to clean up done to
>>>> other ide devices.
>>>>
>>>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>>>> Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>>>> Reviewed-by: Markus Armbruster <armbru@redhat.com>
>>>
>>> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>>
>> Please disregard this tag (I withdraw it), I mis-read the pci variable
>> was not assigned.
>>
>
> Is there an issue you've noticed, or you are just no longer certain
> enough to give an RB?
I asked Zoltan there why he was reassigning 'pci' and he replied here:
https://www.mail-archive.com/qemu-block@nongnu.org/msg63324.html
I don't know enough the PCI API (and don't have time this week to dig
into it) to check how pci->devfn is used (is it populated by a
pci_create() call?).
pci = pci_create_simple_multifunction(pci_bus, PCI_DEVFN(10, 0),
true, TYPE_PIIX4_PCI_DEVICE);
...
+ pci = pci_create_simple(pci_bus, pci->devfn + 1, "piix4-ide");
...
pci_create_simple(pci_bus, pci->devfn + 2, "piix4-usb-uhci");
What annoys me is here -------^^^^^^ I don't know if reassigning the pci
variable can have an impact, so as I am not confident I prefer to
withdraw my review tag.
Since the patch already has 2 R-b I'm not going to NAck it neither.
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v2 2/7] hw/ide: Get rid of piix4_init function
2020-03-17 14:01 ` Philippe Mathieu-Daudé
@ 2020-03-17 14:07 ` BALATON Zoltan
2020-03-17 14:10 ` Philippe Mathieu-Daudé
2020-03-17 14:41 ` Mark Cave-Ayland
0 siblings, 2 replies; 20+ messages in thread
From: BALATON Zoltan @ 2020-03-17 14:07 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Eduardo Habkost, qemu-block, Michael S. Tsirkin,
Markus Armbruster, Mark Cave-Ayland, qemu-devel, hpoussin,
Aleksandar Markovic, Paolo Bonzini, John Snow, Artyom Tarasenko,
Richard Henderson
[-- Attachment #1: Type: text/plain, Size: 1827 bytes --]
On Tue, 17 Mar 2020, Philippe Mathieu-Daudé wrote:
> On 3/17/20 2:50 PM, John Snow wrote:
>> On 3/17/20 6:49 AM, Philippe Mathieu-Daudé wrote:
>>> On 3/17/20 11:41 AM, Philippe Mathieu-Daudé wrote:
>>>> On 3/17/20 10:39 AM, BALATON Zoltan wrote:
>>>>> This removes pci_piix4_ide_init() function similar to clean up done to
>>>>> other ide devices.
>>>>>
>>>>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>>>>> Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>>>>> Reviewed-by: Markus Armbruster <armbru@redhat.com>
>>>>
>>>> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>>>
>>> Please disregard this tag (I withdraw it), I mis-read the pci variable
>>> was not assigned.
>>>
>>
>> Is there an issue you've noticed, or you are just no longer certain
>> enough to give an RB?
>
> I asked Zoltan there why he was reassigning 'pci' and he replied here:
> https://www.mail-archive.com/qemu-block@nongnu.org/msg63324.html
>
> I don't know enough the PCI API (and don't have time this week to dig into
> it) to check how pci->devfn is used (is it populated by a pci_create()
> call?).
>
> pci = pci_create_simple_multifunction(pci_bus, PCI_DEVFN(10, 0),
> true, TYPE_PIIX4_PCI_DEVICE);
> ...
> + pci = pci_create_simple(pci_bus, pci->devfn + 1, "piix4-ide");
> ...
> pci_create_simple(pci_bus, pci->devfn + 2, "piix4-usb-uhci");
>
> What annoys me is here -------^^^^^^ I don't know if reassigning the pci
> variable can have an impact, so as I am not confident I prefer to withdraw my
> review tag.
OK, I did not know that's what you were asking about and did not notice
this could be a problem. To avoid any doubt I'll send a new version to
avoid this shortly.
Regards,
BALATON Zoltan
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 2/7] hw/ide: Get rid of piix4_init function
2020-03-17 14:07 ` BALATON Zoltan
@ 2020-03-17 14:10 ` Philippe Mathieu-Daudé
2020-03-17 14:41 ` Mark Cave-Ayland
1 sibling, 0 replies; 20+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-03-17 14:10 UTC (permalink / raw)
To: BALATON Zoltan
Cc: Eduardo Habkost, Qemu-block, Michael S. Tsirkin,
Markus Armbruster, Mark Cave-Ayland, QEMU Developers,
Hervé Poussineau, Aleksandar Markovic, Paolo Bonzini,
John Snow, Artyom Tarasenko, Richard Henderson
On Tue, Mar 17, 2020 at 3:07 PM BALATON Zoltan <balaton@eik.bme.hu> wrote:
> On Tue, 17 Mar 2020, Philippe Mathieu-Daudé wrote:
> > On 3/17/20 2:50 PM, John Snow wrote:
> >> On 3/17/20 6:49 AM, Philippe Mathieu-Daudé wrote:
> >>> On 3/17/20 11:41 AM, Philippe Mathieu-Daudé wrote:
> >>>> On 3/17/20 10:39 AM, BALATON Zoltan wrote:
> >>>>> This removes pci_piix4_ide_init() function similar to clean up done to
> >>>>> other ide devices.
> >>>>>
> >>>>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> >>>>> Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> >>>>> Reviewed-by: Markus Armbruster <armbru@redhat.com>
> >>>>
> >>>> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> >>>
> >>> Please disregard this tag (I withdraw it), I mis-read the pci variable
> >>> was not assigned.
> >>>
> >>
> >> Is there an issue you've noticed, or you are just no longer certain
> >> enough to give an RB?
> >
> > I asked Zoltan there why he was reassigning 'pci' and he replied here:
> > https://www.mail-archive.com/qemu-block@nongnu.org/msg63324.html
> >
> > I don't know enough the PCI API (and don't have time this week to dig into
> > it) to check how pci->devfn is used (is it populated by a pci_create()
> > call?).
> >
> > pci = pci_create_simple_multifunction(pci_bus, PCI_DEVFN(10, 0),
> > true, TYPE_PIIX4_PCI_DEVICE);
> > ...
> > + pci = pci_create_simple(pci_bus, pci->devfn + 1, "piix4-ide");
> > ...
> > pci_create_simple(pci_bus, pci->devfn + 2, "piix4-usb-uhci");
> >
> > What annoys me is here -------^^^^^^ I don't know if reassigning the pci
> > variable can have an impact, so as I am not confident I prefer to withdraw my
> > review tag.
>
> OK, I did not know that's what you were asking about and did not notice
> this could be a problem. To avoid any doubt I'll send a new version to
> avoid this shortly.
Sorry I was a bit rushed for soft-freeze, I should have spent more
time in my comment then, this would have saved everybody a bit of
email traffic.
I'll try to remember next time.
Thanks for sending an updated version.
> Regards,
> BALATON Zoltan
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 2/7] hw/ide: Get rid of piix4_init function
2020-03-17 14:07 ` BALATON Zoltan
2020-03-17 14:10 ` Philippe Mathieu-Daudé
@ 2020-03-17 14:41 ` Mark Cave-Ayland
1 sibling, 0 replies; 20+ messages in thread
From: Mark Cave-Ayland @ 2020-03-17 14:41 UTC (permalink / raw)
To: BALATON Zoltan, Philippe Mathieu-Daudé
Cc: Eduardo Habkost, qemu-block, Michael S. Tsirkin, qemu-devel,
Markus Armbruster, hpoussin, Aleksandar Markovic, Paolo Bonzini,
John Snow, Artyom Tarasenko, Richard Henderson
On 17/03/2020 14:07, BALATON Zoltan wrote:
> On Tue, 17 Mar 2020, Philippe Mathieu-Daudé wrote:
>> On 3/17/20 2:50 PM, John Snow wrote:
>>> On 3/17/20 6:49 AM, Philippe Mathieu-Daudé wrote:
>>>> On 3/17/20 11:41 AM, Philippe Mathieu-Daudé wrote:
>>>>> On 3/17/20 10:39 AM, BALATON Zoltan wrote:
>>>>>> This removes pci_piix4_ide_init() function similar to clean up done to
>>>>>> other ide devices.
>>>>>>
>>>>>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>>>>>> Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>>>>>> Reviewed-by: Markus Armbruster <armbru@redhat.com>
>>>>>
>>>>> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>>>>
>>>> Please disregard this tag (I withdraw it), I mis-read the pci variable
>>>> was not assigned.
>>>>
>>>
>>> Is there an issue you've noticed, or you are just no longer certain
>>> enough to give an RB?
>>
>> I asked Zoltan there why he was reassigning 'pci' and he replied here:
>> https://www.mail-archive.com/qemu-block@nongnu.org/msg63324.html
>>
>> I don't know enough the PCI API (and don't have time this week to dig into it) to
>> check how pci->devfn is used (is it populated by a pci_create() call?).
>>
>> pci = pci_create_simple_multifunction(pci_bus, PCI_DEVFN(10, 0),
>> true, TYPE_PIIX4_PCI_DEVICE);
>> ...
>> + pci = pci_create_simple(pci_bus, pci->devfn + 1, "piix4-ide");
>> ...
>> pci_create_simple(pci_bus, pci->devfn + 2, "piix4-usb-uhci");
>>
>> What annoys me is here -------^^^^^^ I don't know if reassigning the pci variable
>> can have an impact, so as I am not confident I prefer to withdraw my review tag.
>
> OK, I did not know that's what you were asking about and did not notice this could be
> a problem. To avoid any doubt I'll send a new version to avoid this shortly.
Hmmm actually yes I think isn't quite right. Here you have PCI_DEVFN(10, 0) for the
PIIX4_PCI_DEVICE, and PCI_DEVFN(10, 0) + 1 for "piix4-ide". But since you've
reassigned pci then "piix4-usb-uhci" now ends up as (PCI_DEVFN(10, 0) + 1) + 2 =
PCI_DEVFN(10, 0) + 3 rather than PCI_DEVFN(10, 0) + 2 as it was before.
ATB,
Mark.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 2/7] hw/ide: Get rid of piix4_init function
2020-03-17 13:50 ` John Snow
2020-03-17 14:01 ` Philippe Mathieu-Daudé
@ 2020-03-17 14:05 ` BALATON Zoltan
2020-03-17 14:09 ` John Snow
1 sibling, 1 reply; 20+ messages in thread
From: BALATON Zoltan @ 2020-03-17 14:05 UTC (permalink / raw)
To: John Snow
Cc: Eduardo Habkost, qemu-block, Michael S. Tsirkin,
Markus Armbruster, Mark Cave-Ayland, qemu-devel, hpoussin,
Aleksandar Markovic, Paolo Bonzini, Philippe Mathieu-Daudé,
Artyom Tarasenko, Richard Henderson
[-- Attachment #1: Type: text/plain, Size: 2131 bytes --]
On Tue, 17 Mar 2020, John Snow wrote:
> On 3/17/20 6:49 AM, Philippe Mathieu-Daudé wrote:
>> On 3/17/20 11:41 AM, Philippe Mathieu-Daudé wrote:
>>> On 3/17/20 10:39 AM, BALATON Zoltan wrote:
>>>> This removes pci_piix4_ide_init() function similar to clean up done to
>>>> other ide devices.
>>>>
>>>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>>>> Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>>>> Reviewed-by: Markus Armbruster <armbru@redhat.com>
>>>
>>> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>>
>> Please disregard this tag (I withdraw it), I mis-read the pci variable
>> was not assigned.
>>
>
> Is there an issue you've noticed, or you are just no longer certain
> enough to give an RB?
My previous replies to this question:
https://lists.nongnu.org/archive/html/qemu-devel/2020-03/msg04356.html
https://lists.nongnu.org/archive/html/qemu-devel/2020-03/msg04381.html
End result after all patches in the series looks like this:
DeviceState *piix4_create(PCIBus *pci_bus, ISABus **isa_bus, I2CBus **smbus)
{
PCIDevice *pci;
DeviceState *dev;
pci = pci_create_simple_multifunction(pci_bus, PCI_DEVFN(10, 0),
true, TYPE_PIIX4_PCI_DEVICE);
dev = DEVICE(pci);
if (isa_bus) {
*isa_bus = ISA_BUS(qdev_get_child_bus(dev, "isa.0"));
}
pci = pci_create_simple(pci_bus, pci->devfn + 1, "piix4-ide");
pci_ide_create_devs(pci);
pci_create_simple(pci_bus, pci->devfn + 2, "piix4-usb-uhci");
if (smbus) {
*smbus = piix4_pm_init(pci_bus, pci->devfn + 3, 0x1100,
isa_get_irq(NULL, 9), NULL, 0, NULL);
}
return dev;
}
I think it's clear enough what the pci variable is used for here and there
was no further reply from Philippe and Michael and others were OK with
this so there was no definitive decision here. Mayby Philippe still does
not like reusal of this variable enough to give Reviewed tag but there
should be no other problem with it otherwise.
Regards,
BALATON Zoltan
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v2 2/7] hw/ide: Get rid of piix4_init function
2020-03-17 14:05 ` BALATON Zoltan
@ 2020-03-17 14:09 ` John Snow
0 siblings, 0 replies; 20+ messages in thread
From: John Snow @ 2020-03-17 14:09 UTC (permalink / raw)
To: BALATON Zoltan
Cc: Eduardo Habkost, qemu-block, Michael S. Tsirkin,
Markus Armbruster, Mark Cave-Ayland, qemu-devel, hpoussin,
Aleksandar Markovic, Paolo Bonzini, Philippe Mathieu-Daudé,
Artyom Tarasenko, Richard Henderson
On 3/17/20 10:05 AM, BALATON Zoltan wrote:
> On Tue, 17 Mar 2020, John Snow wrote:
>> On 3/17/20 6:49 AM, Philippe Mathieu-Daudé wrote:
>>> On 3/17/20 11:41 AM, Philippe Mathieu-Daudé wrote:
>>>> On 3/17/20 10:39 AM, BALATON Zoltan wrote:
>>>>> This removes pci_piix4_ide_init() function similar to clean up done to
>>>>> other ide devices.
>>>>>
>>>>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>>>>> Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>>>>> Reviewed-by: Markus Armbruster <armbru@redhat.com>
>>>>
>>>> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>>>
>>> Please disregard this tag (I withdraw it), I mis-read the pci variable
>>> was not assigned.
>>>
>>
>> Is there an issue you've noticed, or you are just no longer certain
>> enough to give an RB?
>
> My previous replies to this question:
>
> https://lists.nongnu.org/archive/html/qemu-devel/2020-03/msg04356.html
> https://lists.nongnu.org/archive/html/qemu-devel/2020-03/msg04381.html
>
> End result after all patches in the series looks like this:
>
> DeviceState *piix4_create(PCIBus *pci_bus, ISABus **isa_bus, I2CBus
> **smbus)
> {
> PCIDevice *pci;
> DeviceState *dev;
>
> pci = pci_create_simple_multifunction(pci_bus, PCI_DEVFN(10, 0),
> true, TYPE_PIIX4_PCI_DEVICE);
> dev = DEVICE(pci);
> if (isa_bus) {
> *isa_bus = ISA_BUS(qdev_get_child_bus(dev, "isa.0"));
> }
>
> pci = pci_create_simple(pci_bus, pci->devfn + 1, "piix4-ide");
> pci_ide_create_devs(pci);
>
> pci_create_simple(pci_bus, pci->devfn + 2, "piix4-usb-uhci");
> if (smbus) {
> *smbus = piix4_pm_init(pci_bus, pci->devfn + 3, 0x1100,
> isa_get_irq(NULL, 9), NULL, 0, NULL);
> }
>
> return dev;
> }
>
> I think it's clear enough what the pci variable is used for here and
> there was no further reply from Philippe and Michael and others were OK
> with this so there was no definitive decision here. Mayby Philippe still
> does not like reusal of this variable enough to give Reviewed tag but
> there should be no other problem with it otherwise.
>
> Regards,
> BALATON Zoltan
Thanks for the pointers -- my attention was focused elsewhere, and the
discussion was so vigorous I felt comfortable letting you handle it :)
I'm seeing a compilation error -- I cleared my cache and am looking for
the commit that introduced it. Stay tuned, please.
--js
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 4/7] hw/ide/pci.c: Coding style update to fix checkpatch errors
2020-03-17 9:39 [PATCH v2 0/7] Misc hw/ide legacy clean up BALATON Zoltan
` (3 preceding siblings ...)
2020-03-17 9:39 ` [PATCH v2 2/7] hw/ide: Get rid of piix4_init function BALATON Zoltan
@ 2020-03-17 9:39 ` BALATON Zoltan
2020-03-17 9:39 ` [PATCH v2 6/7] hw/ide: Move MAX_IDE_DEVS define to hw/ide/internal.h BALATON Zoltan
2020-03-17 9:39 ` [PATCH v2 1/7] hw/ide: Get rid of piix3_init functions BALATON Zoltan
6 siblings, 0 replies; 20+ messages in thread
From: BALATON Zoltan @ 2020-03-17 9:39 UTC (permalink / raw)
To: qemu-devel, qemu-block
Cc: Eduardo Habkost, Michael S. Tsirkin, John Snow, Mark Cave-Ayland,
Markus Armbruster, hpoussin, Aleksandar Markovic, Paolo Bonzini,
philmd, Artyom Tarasenko, Richard Henderson
Spaces are required around a + operator and if statements should have
braces even for single line. Also make it simpler by reversing the
condition instead of breaking the loop.
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
---
hw/ide/pci.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/hw/ide/pci.c b/hw/ide/pci.c
index 4fc76c5225..e0c84392e2 100644
--- a/hw/ide/pci.c
+++ b/hw/ide/pci.c
@@ -485,9 +485,9 @@ void pci_ide_create_devs(PCIDevice *dev, DriveInfo **hd_table)
int i;
for (i = 0; i < 4; i++) {
- if (hd_table[i] == NULL)
- continue;
- ide_create_drive(d->bus+bus[i], unit[i], hd_table[i]);
+ if (hd_table[i]) {
+ ide_create_drive(d->bus + bus[i], unit[i], hd_table[i]);
+ }
}
}
--
2.21.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH v2 6/7] hw/ide: Move MAX_IDE_DEVS define to hw/ide/internal.h
2020-03-17 9:39 [PATCH v2 0/7] Misc hw/ide legacy clean up BALATON Zoltan
` (4 preceding siblings ...)
2020-03-17 9:39 ` [PATCH v2 4/7] hw/ide/pci.c: Coding style update to fix checkpatch errors BALATON Zoltan
@ 2020-03-17 9:39 ` BALATON Zoltan
2020-03-17 14:22 ` John Snow
2020-03-17 9:39 ` [PATCH v2 1/7] hw/ide: Get rid of piix3_init functions BALATON Zoltan
6 siblings, 1 reply; 20+ messages in thread
From: BALATON Zoltan @ 2020-03-17 9:39 UTC (permalink / raw)
To: qemu-devel, qemu-block
Cc: Eduardo Habkost, Michael S. Tsirkin, John Snow, Mark Cave-Ayland,
Markus Armbruster, hpoussin, Aleksandar Markovic, Paolo Bonzini,
philmd, Artyom Tarasenko, Richard Henderson
We can move this define now that less files use it to internal.h to
further reduce dependency on hw/ide.h.
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
---
include/hw/ide.h | 2 --
include/hw/ide/internal.h | 2 ++
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/hw/ide.h b/include/hw/ide.h
index d52c211f32..c5ce5da4f4 100644
--- a/include/hw/ide.h
+++ b/include/hw/ide.h
@@ -4,8 +4,6 @@
#include "hw/isa/isa.h"
#include "exec/memory.h"
-#define MAX_IDE_DEVS 2
-
/* ide-isa.c */
ISADevice *isa_ide_init(ISABus *bus, int iobase, int iobase2, int isairq,
DriveInfo *hd0, DriveInfo *hd1);
diff --git a/include/hw/ide/internal.h b/include/hw/ide/internal.h
index 1bc1fc73e5..55da35d768 100644
--- a/include/hw/ide/internal.h
+++ b/include/hw/ide/internal.h
@@ -27,6 +27,8 @@ typedef struct IDEDMAOps IDEDMAOps;
#define TYPE_IDE_BUS "IDE"
#define IDE_BUS(obj) OBJECT_CHECK(IDEBus, (obj), TYPE_IDE_BUS)
+#define MAX_IDE_DEVS 2
+
/* Bits of HD_STATUS */
#define ERR_STAT 0x01
#define INDEX_STAT 0x02
--
2.21.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v2 6/7] hw/ide: Move MAX_IDE_DEVS define to hw/ide/internal.h
2020-03-17 9:39 ` [PATCH v2 6/7] hw/ide: Move MAX_IDE_DEVS define to hw/ide/internal.h BALATON Zoltan
@ 2020-03-17 14:22 ` John Snow
2020-03-17 14:24 ` BALATON Zoltan
0 siblings, 1 reply; 20+ messages in thread
From: John Snow @ 2020-03-17 14:22 UTC (permalink / raw)
To: BALATON Zoltan, qemu-devel, qemu-block
Cc: Eduardo Habkost, Michael S. Tsirkin, Mark Cave-Ayland,
Markus Armbruster, hpoussin, Aleksandar Markovic, Paolo Bonzini,
philmd, Artyom Tarasenko, Richard Henderson
On 3/17/20 5:39 AM, BALATON Zoltan wrote:
> We can move this define now that less files use it to internal.h to
> further reduce dependency on hw/ide.h.
>
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> Reviewed-by: Markus Armbruster <armbru@redhat.com>
> ---
> include/hw/ide.h | 2 --
> include/hw/ide/internal.h | 2 ++
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/include/hw/ide.h b/include/hw/ide.h
> index d52c211f32..c5ce5da4f4 100644
> --- a/include/hw/ide.h
> +++ b/include/hw/ide.h
> @@ -4,8 +4,6 @@
> #include "hw/isa/isa.h"
> #include "exec/memory.h"
>
> -#define MAX_IDE_DEVS 2
> -
> /* ide-isa.c */
> ISADevice *isa_ide_init(ISABus *bus, int iobase, int iobase2, int isairq,
> DriveInfo *hd0, DriveInfo *hd1);
> diff --git a/include/hw/ide/internal.h b/include/hw/ide/internal.h
> index 1bc1fc73e5..55da35d768 100644
> --- a/include/hw/ide/internal.h
> +++ b/include/hw/ide/internal.h
> @@ -27,6 +27,8 @@ typedef struct IDEDMAOps IDEDMAOps;
> #define TYPE_IDE_BUS "IDE"
> #define IDE_BUS(obj) OBJECT_CHECK(IDEBus, (obj), TYPE_IDE_BUS)
>
> +#define MAX_IDE_DEVS 2
> +
> /* Bits of HD_STATUS */
> #define ERR_STAT 0x01
> #define INDEX_STAT 0x02
>
/home/jsnow/src/qemu.git/ide/hw/mips/mips_r4k.c: In function
‘mips_r4k_init’:
/home/jsnow/src/qemu.git/ide/hw/mips/mips_r4k.c:190:33: error:
‘MAX_IDE_DEVS’ undeclared (first use in this function); did you mean
‘MAX_IDE_BUS’?
190 | DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
| ^~~~~~~~~~~~
| MAX_IDE_BUS
Missed a spot.
--js
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v2 6/7] hw/ide: Move MAX_IDE_DEVS define to hw/ide/internal.h
2020-03-17 14:22 ` John Snow
@ 2020-03-17 14:24 ` BALATON Zoltan
2020-03-17 14:27 ` John Snow
0 siblings, 1 reply; 20+ messages in thread
From: BALATON Zoltan @ 2020-03-17 14:24 UTC (permalink / raw)
To: John Snow
Cc: Eduardo Habkost, qemu-block, Michael S. Tsirkin,
Markus Armbruster, Mark Cave-Ayland, qemu-devel, hpoussin,
Aleksandar Markovic, Paolo Bonzini, philmd, Artyom Tarasenko,
Richard Henderson
[-- Attachment #1: Type: text/plain, Size: 2010 bytes --]
On Tue, 17 Mar 2020, John Snow wrote:
> On 3/17/20 5:39 AM, BALATON Zoltan wrote:
>> We can move this define now that less files use it to internal.h to
>> further reduce dependency on hw/ide.h.
>>
>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>> Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>> Reviewed-by: Markus Armbruster <armbru@redhat.com>
>> ---
>> include/hw/ide.h | 2 --
>> include/hw/ide/internal.h | 2 ++
>> 2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/include/hw/ide.h b/include/hw/ide.h
>> index d52c211f32..c5ce5da4f4 100644
>> --- a/include/hw/ide.h
>> +++ b/include/hw/ide.h
>> @@ -4,8 +4,6 @@
>> #include "hw/isa/isa.h"
>> #include "exec/memory.h"
>>
>> -#define MAX_IDE_DEVS 2
>> -
>> /* ide-isa.c */
>> ISADevice *isa_ide_init(ISABus *bus, int iobase, int iobase2, int isairq,
>> DriveInfo *hd0, DriveInfo *hd1);
>> diff --git a/include/hw/ide/internal.h b/include/hw/ide/internal.h
>> index 1bc1fc73e5..55da35d768 100644
>> --- a/include/hw/ide/internal.h
>> +++ b/include/hw/ide/internal.h
>> @@ -27,6 +27,8 @@ typedef struct IDEDMAOps IDEDMAOps;
>> #define TYPE_IDE_BUS "IDE"
>> #define IDE_BUS(obj) OBJECT_CHECK(IDEBus, (obj), TYPE_IDE_BUS)
>>
>> +#define MAX_IDE_DEVS 2
>> +
>> /* Bits of HD_STATUS */
>> #define ERR_STAT 0x01
>> #define INDEX_STAT 0x02
>>
>
> /home/jsnow/src/qemu.git/ide/hw/mips/mips_r4k.c: In function
> ‘mips_r4k_init’:
> /home/jsnow/src/qemu.git/ide/hw/mips/mips_r4k.c:190:33: error:
> ‘MAX_IDE_DEVS’ undeclared (first use in this function); did you mean
> ‘MAX_IDE_BUS’?
> 190 | DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
> | ^~~~~~~~~~~~
> | MAX_IDE_BUS
>
>
>
> Missed a spot.
Probably due to dropping patch 4, I'll check and send a v3. Is there
anything else besides Philippe's suggestion?
Regards,
BALATON Zoltan
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 6/7] hw/ide: Move MAX_IDE_DEVS define to hw/ide/internal.h
2020-03-17 14:24 ` BALATON Zoltan
@ 2020-03-17 14:27 ` John Snow
0 siblings, 0 replies; 20+ messages in thread
From: John Snow @ 2020-03-17 14:27 UTC (permalink / raw)
To: BALATON Zoltan
Cc: Eduardo Habkost, qemu-block, Michael S. Tsirkin,
Markus Armbruster, Mark Cave-Ayland, qemu-devel, hpoussin,
Aleksandar Markovic, Paolo Bonzini, philmd, Artyom Tarasenko,
Richard Henderson
On 3/17/20 10:24 AM, BALATON Zoltan wrote:
> On Tue, 17 Mar 2020, John Snow wrote:
>> On 3/17/20 5:39 AM, BALATON Zoltan wrote:
>>> We can move this define now that less files use it to internal.h to
>>> further reduce dependency on hw/ide.h.
>>>
>>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>>> Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>>> Reviewed-by: Markus Armbruster <armbru@redhat.com>
>>> ---
>>> include/hw/ide.h | 2 --
>>> include/hw/ide/internal.h | 2 ++
>>> 2 files changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/include/hw/ide.h b/include/hw/ide.h
>>> index d52c211f32..c5ce5da4f4 100644
>>> --- a/include/hw/ide.h
>>> +++ b/include/hw/ide.h
>>> @@ -4,8 +4,6 @@
>>> #include "hw/isa/isa.h"
>>> #include "exec/memory.h"
>>>
>>> -#define MAX_IDE_DEVS 2
>>> -
>>> /* ide-isa.c */
>>> ISADevice *isa_ide_init(ISABus *bus, int iobase, int iobase2, int
>>> isairq,
>>> DriveInfo *hd0, DriveInfo *hd1);
>>> diff --git a/include/hw/ide/internal.h b/include/hw/ide/internal.h
>>> index 1bc1fc73e5..55da35d768 100644
>>> --- a/include/hw/ide/internal.h
>>> +++ b/include/hw/ide/internal.h
>>> @@ -27,6 +27,8 @@ typedef struct IDEDMAOps IDEDMAOps;
>>> #define TYPE_IDE_BUS "IDE"
>>> #define IDE_BUS(obj) OBJECT_CHECK(IDEBus, (obj), TYPE_IDE_BUS)
>>>
>>> +#define MAX_IDE_DEVS 2
>>> +
>>> /* Bits of HD_STATUS */
>>> #define ERR_STAT 0x01
>>> #define INDEX_STAT 0x02
>>>
>>
>> /home/jsnow/src/qemu.git/ide/hw/mips/mips_r4k.c: In function
>> ‘mips_r4k_init’:
>> /home/jsnow/src/qemu.git/ide/hw/mips/mips_r4k.c:190:33: error:
>> ‘MAX_IDE_DEVS’ undeclared (first use in this function); did you mean
>> ‘MAX_IDE_BUS’?
>> 190 | DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
>> | ^~~~~~~~~~~~
>> | MAX_IDE_BUS
>>
>>
>>
>> Missed a spot.
>
> Probably due to dropping patch 4, I'll check and send a v3. Is there
> anything else besides Philippe's suggestion?
>
Not that I am aware of at this very second.
--js
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 1/7] hw/ide: Get rid of piix3_init functions
2020-03-17 9:39 [PATCH v2 0/7] Misc hw/ide legacy clean up BALATON Zoltan
` (5 preceding siblings ...)
2020-03-17 9:39 ` [PATCH v2 6/7] hw/ide: Move MAX_IDE_DEVS define to hw/ide/internal.h BALATON Zoltan
@ 2020-03-17 9:39 ` BALATON Zoltan
6 siblings, 0 replies; 20+ messages in thread
From: BALATON Zoltan @ 2020-03-17 9:39 UTC (permalink / raw)
To: qemu-devel, qemu-block
Cc: Eduardo Habkost, Michael S. Tsirkin, John Snow, Mark Cave-Ayland,
Markus Armbruster, hpoussin, Aleksandar Markovic, Paolo Bonzini,
philmd, Artyom Tarasenko, Richard Henderson
This removes pci_piix3_ide_init() and pci_piix3_xen_ide_init()
functions similar to clean up done to other ide devices.
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
---
hw/i386/pc_piix.c | 10 +++++-----
hw/ide/pci.c | 1 +
hw/ide/piix.c | 21 +--------------------
include/hw/ide.h | 2 --
4 files changed, 7 insertions(+), 27 deletions(-)
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index e2d98243bc..c399398739 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -39,6 +39,7 @@
#include "hw/usb.h"
#include "net/net.h"
#include "hw/ide.h"
+#include "hw/ide/pci.h"
#include "hw/irq.h"
#include "sysemu/kvm.h"
#include "hw/kvm/clock.h"
@@ -242,11 +243,10 @@ static void pc_init1(MachineState *machine,
ide_drive_get(hd, ARRAY_SIZE(hd));
if (pcmc->pci_enabled) {
PCIDevice *dev;
- if (xen_enabled()) {
- dev = pci_piix3_xen_ide_init(pci_bus, hd, piix3_devfn + 1);
- } else {
- dev = pci_piix3_ide_init(pci_bus, hd, piix3_devfn + 1);
- }
+
+ dev = pci_create_simple(pci_bus, piix3_devfn + 1,
+ xen_enabled() ? "piix3-ide-xen" : "piix3-ide");
+ pci_ide_create_devs(dev, hd);
idebus[0] = qdev_get_child_bus(&dev->qdev, "ide.0");
idebus[1] = qdev_get_child_bus(&dev->qdev, "ide.1");
pc_cmos_init(pcms, idebus[0], idebus[1], rtc_state);
diff --git a/hw/ide/pci.c b/hw/ide/pci.c
index 1a6a287e76..4fc76c5225 100644
--- a/hw/ide/pci.c
+++ b/hw/ide/pci.c
@@ -476,6 +476,7 @@ const VMStateDescription vmstate_ide_pci = {
}
};
+/* hd_table must contain 4 block drivers */
void pci_ide_create_devs(PCIDevice *dev, DriveInfo **hd_table)
{
PCIIDEState *d = PCI_IDE(dev);
diff --git a/hw/ide/piix.c b/hw/ide/piix.c
index bc575b4d70..8bcd6b72c2 100644
--- a/hw/ide/piix.c
+++ b/hw/ide/piix.c
@@ -197,15 +197,6 @@ int pci_piix3_xen_ide_unplug(DeviceState *dev, bool aux)
return 0;
}
-PCIDevice *pci_piix3_xen_ide_init(PCIBus *bus, DriveInfo **hd_table, int devfn)
-{
- PCIDevice *dev;
-
- dev = pci_create_simple(bus, devfn, "piix3-ide-xen");
- pci_ide_create_devs(dev, hd_table);
- return dev;
-}
-
static void pci_piix_ide_exitfn(PCIDevice *dev)
{
PCIIDEState *d = PCI_IDE(dev);
@@ -217,17 +208,6 @@ static void pci_piix_ide_exitfn(PCIDevice *dev)
}
}
-/* hd_table must contain 4 block drivers */
-/* NOTE: for the PIIX3, the IRQs and IOports are hardcoded */
-PCIDevice *pci_piix3_ide_init(PCIBus *bus, DriveInfo **hd_table, int devfn)
-{
- PCIDevice *dev;
-
- dev = pci_create_simple(bus, devfn, "piix3-ide");
- pci_ide_create_devs(dev, hd_table);
- return dev;
-}
-
/* hd_table must contain 4 block drivers */
/* NOTE: for the PIIX4, the IRQs and IOports are hardcoded */
PCIDevice *pci_piix4_ide_init(PCIBus *bus, DriveInfo **hd_table, int devfn)
@@ -239,6 +219,7 @@ PCIDevice *pci_piix4_ide_init(PCIBus *bus, DriveInfo **hd_table, int devfn)
return dev;
}
+/* 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);
diff --git a/include/hw/ide.h b/include/hw/ide.h
index dea0ecf5be..883bbaeb9b 100644
--- a/include/hw/ide.h
+++ b/include/hw/ide.h
@@ -12,8 +12,6 @@ ISADevice *isa_ide_init(ISABus *bus, int iobase, int iobase2, int isairq,
DriveInfo *hd0, DriveInfo *hd1);
/* ide-pci.c */
-PCIDevice *pci_piix3_xen_ide_init(PCIBus *bus, DriveInfo **hd_table, int devfn);
-PCIDevice *pci_piix3_ide_init(PCIBus *bus, DriveInfo **hd_table, int devfn);
PCIDevice *pci_piix4_ide_init(PCIBus *bus, DriveInfo **hd_table, int devfn);
int pci_piix3_xen_ide_unplug(DeviceState *dev, bool aux);
--
2.21.1
^ permalink raw reply related [flat|nested] 20+ messages in thread