diff mbox series

[6/8] hw/ide: Do ide_drive_get() within pci_ide_create_devs()

Message ID 49bf646a9419c3b20125187a26f8a4fd5f35f399.1584134074.git.balaton@eik.bme.hu
State New
Headers show
Series Misc hw/ide legacy clean up | expand

Commit Message

BALATON Zoltan March 13, 2020, 9:14 p.m. UTC
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>
---
 hw/alpha/dp264.c              | 13 +++----------
 hw/i386/pc_piix.c             |  9 +++++----
 hw/ide/pci.c                  |  5 +++--
 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(+), 36 deletions(-)

Comments

BALATON Zoltan March 13, 2020, 10:16 p.m. UTC | #1
On Fri, 13 Mar 2020, BALATON Zoltan wrote:
> 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>
> ---
> hw/alpha/dp264.c              | 13 +++----------
> hw/i386/pc_piix.c             |  9 +++++----
> hw/ide/pci.c                  |  5 +++--
> 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(+), 36 deletions(-)
>
> diff --git a/hw/alpha/dp264.c b/hw/alpha/dp264.c
> index 0f58b1b668..e23172f177 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"
> @@ -56,6 +55,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;
> @@ -98,15 +98,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);

Additionally, I think it may also make sense to move pci_ide_create_devs 
call into the realize methods of these IDE controllers so boards do not 
need to do it explicitely. These calls always follow the creation of the 
device immediately so could just be done internally in IDE device and 
simplify it further. I can attempt to prepare additional patches for that 
but first I'd like to hear if anyone has anything against that to avoid 
doing useless work.

Regards,
BALATON Zoltan

>     /* 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 b363a69e2e..a7a696d0c8 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -84,7 +84,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;
> @@ -238,20 +237,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..03d6eef592 100644
> --- a/hw/ide/pci.c
> +++ b/hw/ide/pci.c
> @@ -476,14 +476,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[MAX_IDE_BUS * 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 3690b76061..508dc57737 100644
> --- a/hw/mips/mips_fulong2e.c
> +++ b/hw/mips/mips_fulong2e.c
> @@ -238,7 +238,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));
> @@ -258,8 +257,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 6f51e33e7b..1da5271922 100644
> --- a/hw/mips/mips_malta.c
> +++ b/hw/mips/mips_malta.c
> @@ -1401,7 +1401,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 74acfd39b3..329e82c0f0 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"
> @@ -562,7 +561,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;
> @@ -662,12 +660,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
>
Paolo Bonzini March 14, 2020, 10:01 a.m. UTC | #2
On 13/03/20 23:16, BALATON Zoltan wrote:
>>
>> +    pci_dev = pci_create_simple(pci_bus, -1, "cmd646-ide");
>> +    pci_ide_create_devs(pci_dev);
> 
> Additionally, I think it may also make sense to move pci_ide_create_devs
> call into the realize methods of these IDE controllers so boards do not
> need to do it explicitely. These calls always follow the creation of the
> device immediately so could just be done internally in IDE device and
> simplify it further. I can attempt to prepare additional patches for
> that but first I'd like to hear if anyone has anything against that to
> avoid doing useless work.

No, it's better to do it separately.  I think that otherwise you could
add another IDE controller with -device, and both controllers would try
to add the drives.

Basically, separating the call means that only automatically added
controllers obey "if=ide".

Paolo
Markus Armbruster March 16, 2020, 6:23 a.m. UTC | #3
Paolo Bonzini <pbonzini@redhat.com> writes:

> On 13/03/20 23:16, BALATON Zoltan wrote:
>>>
>>> +    pci_dev = pci_create_simple(pci_bus, -1, "cmd646-ide");
>>> +    pci_ide_create_devs(pci_dev);
>> 
>> Additionally, I think it may also make sense to move pci_ide_create_devs
>> call into the realize methods of these IDE controllers so boards do not
>> need to do it explicitely. These calls always follow the creation of the
>> device immediately so could just be done internally in IDE device and
>> simplify it further. I can attempt to prepare additional patches for
>> that but first I'd like to hear if anyone has anything against that to
>> avoid doing useless work.
>
> No, it's better to do it separately.  I think that otherwise you could
> add another IDE controller with -device, and both controllers would try
> to add the drives.

Correct.

Creating device frontends for -drive if=ide is the board's job.  Boards
may delegate to suitable helpers.  I'd very much prefer these helpers
not to live with device model code.  Board and device model code should
be cleanly separated to to reduce the temptation to muddle their
responsibilities.  It's separation of concerns.

I actually wish we had separate sub-trees for boards and devices instead
of keeping both in hw/.

> Basically, separating the call means that only automatically added
> controllers obey "if=ide".
Philippe Mathieu-Daudé March 16, 2020, 8:30 a.m. UTC | #4
On 3/16/20 7:23 AM, Markus Armbruster wrote:
> Paolo Bonzini <pbonzini@redhat.com> writes:
> 
>> On 13/03/20 23:16, BALATON Zoltan wrote:
>>>>
>>>> +    pci_dev = pci_create_simple(pci_bus, -1, "cmd646-ide");
>>>> +    pci_ide_create_devs(pci_dev);
>>>
>>> Additionally, I think it may also make sense to move pci_ide_create_devs
>>> call into the realize methods of these IDE controllers so boards do not
>>> need to do it explicitely. These calls always follow the creation of the
>>> device immediately so could just be done internally in IDE device and
>>> simplify it further. I can attempt to prepare additional patches for
>>> that but first I'd like to hear if anyone has anything against that to
>>> avoid doing useless work.
>>
>> No, it's better to do it separately.  I think that otherwise you could
>> add another IDE controller with -device, and both controllers would try
>> to add the drives.
> 
> Correct.
> 
> Creating device frontends for -drive if=ide is the board's job.  Boards
> may delegate to suitable helpers.  I'd very much prefer these helpers
> not to live with device model code.  Board and device model code should
> be cleanly separated to to reduce the temptation to muddle their
> responsibilities.  It's separation of concerns.
> 
> I actually wish we had separate sub-trees for boards and devices instead
> of keeping both in hw/.

Never too late!

To be clear, you suggest:

- one dir with machines, boards, system-on-module
- one dir with devices, cpu, system-on-chips

Correct?

> 
>> Basically, separating the call means that only automatically added
>> controllers obey "if=ide".
>
Markus Armbruster March 16, 2020, 2:03 p.m. UTC | #5
Philippe Mathieu-Daudé <philmd@redhat.com> writes:

> On 3/16/20 7:23 AM, Markus Armbruster wrote:
>> Paolo Bonzini <pbonzini@redhat.com> writes:
>>
>>> On 13/03/20 23:16, BALATON Zoltan wrote:
>>>>>
>>>>> +    pci_dev = pci_create_simple(pci_bus, -1, "cmd646-ide");
>>>>> +    pci_ide_create_devs(pci_dev);
>>>>
>>>> Additionally, I think it may also make sense to move pci_ide_create_devs
>>>> call into the realize methods of these IDE controllers so boards do not
>>>> need to do it explicitely. These calls always follow the creation of the
>>>> device immediately so could just be done internally in IDE device and
>>>> simplify it further. I can attempt to prepare additional patches for
>>>> that but first I'd like to hear if anyone has anything against that to
>>>> avoid doing useless work.
>>>
>>> No, it's better to do it separately.  I think that otherwise you could
>>> add another IDE controller with -device, and both controllers would try
>>> to add the drives.
>>
>> Correct.
>>
>> Creating device frontends for -drive if=ide is the board's job.  Boards
>> may delegate to suitable helpers.  I'd very much prefer these helpers
>> not to live with device model code.  Board and device model code should
>> be cleanly separated to to reduce the temptation to muddle their
>> responsibilities.  It's separation of concerns.
>>
>> I actually wish we had separate sub-trees for boards and devices instead
>> of keeping both in hw/.
>
> Never too late!
>
> To be clear, you suggest:
>
> - one dir with machines, boards, system-on-module
> - one dir with devices, cpu, system-on-chips
>
> Correct?

In QOM terms:

* One sub-tree with descendants of TYPE_DEVICE
* One sub-tree with descendants of TYPE_MACHINE
diff mbox series

Patch

diff --git a/hw/alpha/dp264.c b/hw/alpha/dp264.c
index 0f58b1b668..e23172f177 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"
@@ -56,6 +55,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;
@@ -98,15 +98,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 b363a69e2e..a7a696d0c8 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -84,7 +84,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;
@@ -238,20 +237,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..03d6eef592 100644
--- a/hw/ide/pci.c
+++ b/hw/ide/pci.c
@@ -476,14 +476,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[MAX_IDE_BUS * 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 3690b76061..508dc57737 100644
--- a/hw/mips/mips_fulong2e.c
+++ b/hw/mips/mips_fulong2e.c
@@ -238,7 +238,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));
@@ -258,8 +257,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 6f51e33e7b..1da5271922 100644
--- a/hw/mips/mips_malta.c
+++ b/hw/mips/mips_malta.c
@@ -1401,7 +1401,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 74acfd39b3..329e82c0f0 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"
@@ -562,7 +561,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;
@@ -662,12 +660,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