* [PATCH 1/7] hw/ide: Add the possibility to disable the CompactFlash device in the build
2024-02-19 10:49 [PATCH 0/7] hw/ide: Clean up hw/ide/qdev.c and include/hw/ide/internal.h Thomas Huth
@ 2024-02-19 10:49 ` Thomas Huth
2024-02-19 10:49 ` [PATCH 2/7] hw/ide: Split qdev.c into ide-bus.c and ide-dev.c Thomas Huth
` (6 subsequent siblings)
7 siblings, 0 replies; 16+ messages in thread
From: Thomas Huth @ 2024-02-19 10:49 UTC (permalink / raw)
To: John Snow, qemu-devel
Cc: BALATON Zoltan, Philippe Mathieu-Daudé, qemu-block
For distros like downstream RHEL, it would be helpful to allow to disable
the CompactFlash device. For making this possible, we need a separate
Kconfig switch for this device, and the code should reside in a separate
file. Let's also introduce a new header ide-dev.h which can be used to
collect definitions related to IDE devices.
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
include/hw/ide/ide-dev.h | 41 ++++++++++++++++++++++++++++
hw/ide/cf.c | 58 ++++++++++++++++++++++++++++++++++++++++
hw/ide/qdev.c | 51 ++---------------------------------
hw/ide/Kconfig | 4 +++
hw/ide/meson.build | 1 +
5 files changed, 106 insertions(+), 49 deletions(-)
create mode 100644 include/hw/ide/ide-dev.h
create mode 100644 hw/ide/cf.c
diff --git a/include/hw/ide/ide-dev.h b/include/hw/ide/ide-dev.h
new file mode 100644
index 0000000000..7e9663cda9
--- /dev/null
+++ b/include/hw/ide/ide-dev.h
@@ -0,0 +1,41 @@
+/*
+ * ide device definitions
+ *
+ * Copyright (c) 2009 Gerd Hoffmann <kraxel@redhat.com>
+ *
+ * This code is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef IDE_DEV_H
+#define IDE_DEV_H
+
+#include "hw/qdev-properties.h"
+#include "hw/block/block.h"
+#include "hw/ide/internal.h"
+
+typedef struct IDEDrive {
+ IDEDevice dev;
+} IDEDrive;
+
+#define DEFINE_IDE_DEV_PROPERTIES() \
+ DEFINE_BLOCK_PROPERTIES(IDEDrive, dev.conf), \
+ DEFINE_BLOCK_ERROR_PROPERTIES(IDEDrive, dev.conf), \
+ DEFINE_PROP_STRING("ver", IDEDrive, dev.version), \
+ DEFINE_PROP_UINT64("wwn", IDEDrive, dev.wwn, 0), \
+ DEFINE_PROP_STRING("serial", IDEDrive, dev.serial),\
+ DEFINE_PROP_STRING("model", IDEDrive, dev.model)
+
+void ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind, Error **errp);
+
+#endif
diff --git a/hw/ide/cf.c b/hw/ide/cf.c
new file mode 100644
index 0000000000..2a425cb0f2
--- /dev/null
+++ b/hw/ide/cf.c
@@ -0,0 +1,58 @@
+/*
+ * ide CompactFlash support
+ *
+ * This code is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "qemu/osdep.h"
+#include "hw/ide/ide-dev.h"
+#include "qapi/qapi-types-block.h"
+
+static void ide_cf_realize(IDEDevice *dev, Error **errp)
+{
+ ide_dev_initfn(dev, IDE_CFATA, errp);
+}
+
+static Property ide_cf_properties[] = {
+ DEFINE_IDE_DEV_PROPERTIES(),
+ DEFINE_BLOCK_CHS_PROPERTIES(IDEDrive, dev.conf),
+ DEFINE_PROP_BIOS_CHS_TRANS("bios-chs-trans",
+ IDEDrive, dev.chs_trans, BIOS_ATA_TRANSLATION_AUTO),
+ DEFINE_PROP_END_OF_LIST(),
+};
+
+static void ide_cf_class_init(ObjectClass *klass, void *data)
+{
+ DeviceClass *dc = DEVICE_CLASS(klass);
+ IDEDeviceClass *k = IDE_DEVICE_CLASS(klass);
+
+ k->realize = ide_cf_realize;
+ dc->fw_name = "drive";
+ dc->desc = "virtual CompactFlash card";
+ device_class_set_props(dc, ide_cf_properties);
+}
+
+static const TypeInfo ide_cf_info = {
+ .name = "ide-cf",
+ .parent = TYPE_IDE_DEVICE,
+ .instance_size = sizeof(IDEDrive),
+ .class_init = ide_cf_class_init,
+};
+
+static void ide_cf_register_type(void)
+{
+ type_register_static(&ide_cf_info);
+}
+
+type_init(ide_cf_register_type)
diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index 1b3b4da01d..4189313d30 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -24,12 +24,9 @@
#include "qemu/error-report.h"
#include "qemu/main-loop.h"
#include "qemu/module.h"
-#include "hw/ide/internal.h"
-#include "hw/qdev-properties.h"
-#include "hw/qdev-properties-system.h"
+#include "hw/ide/ide-dev.h"
#include "sysemu/block-backend.h"
#include "sysemu/blockdev.h"
-#include "hw/block/block.h"
#include "sysemu/sysemu.h"
#include "sysemu/runstate.h"
#include "qapi/visitor.h"
@@ -158,11 +155,7 @@ int ide_get_bios_chs_trans(BusState *bus, int unit)
/* --------------------------------- */
-typedef struct IDEDrive {
- IDEDevice dev;
-} IDEDrive;
-
-static void ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind, Error **errp)
+void ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind, Error **errp)
{
IDEBus *bus = DO_UPCAST(IDEBus, qbus, dev->qdev.parent_bus);
IDEState *s = bus->ifs + dev->unit;
@@ -283,19 +276,6 @@ static void ide_cd_realize(IDEDevice *dev, Error **errp)
ide_dev_initfn(dev, IDE_CD, errp);
}
-static void ide_cf_realize(IDEDevice *dev, Error **errp)
-{
- ide_dev_initfn(dev, IDE_CFATA, errp);
-}
-
-#define DEFINE_IDE_DEV_PROPERTIES() \
- DEFINE_BLOCK_PROPERTIES(IDEDrive, dev.conf), \
- DEFINE_BLOCK_ERROR_PROPERTIES(IDEDrive, dev.conf), \
- DEFINE_PROP_STRING("ver", IDEDrive, dev.version), \
- DEFINE_PROP_UINT64("wwn", IDEDrive, dev.wwn, 0), \
- DEFINE_PROP_STRING("serial", IDEDrive, dev.serial),\
- DEFINE_PROP_STRING("model", IDEDrive, dev.model)
-
static Property ide_hd_properties[] = {
DEFINE_IDE_DEV_PROPERTIES(),
DEFINE_BLOCK_CHS_PROPERTIES(IDEDrive, dev.conf),
@@ -346,32 +326,6 @@ static const TypeInfo ide_cd_info = {
.class_init = ide_cd_class_init,
};
-static Property ide_cf_properties[] = {
- DEFINE_IDE_DEV_PROPERTIES(),
- DEFINE_BLOCK_CHS_PROPERTIES(IDEDrive, dev.conf),
- DEFINE_PROP_BIOS_CHS_TRANS("bios-chs-trans",
- IDEDrive, dev.chs_trans, BIOS_ATA_TRANSLATION_AUTO),
- DEFINE_PROP_END_OF_LIST(),
-};
-
-static void ide_cf_class_init(ObjectClass *klass, void *data)
-{
- DeviceClass *dc = DEVICE_CLASS(klass);
- IDEDeviceClass *k = IDE_DEVICE_CLASS(klass);
-
- k->realize = ide_cf_realize;
- dc->fw_name = "drive";
- dc->desc = "virtual CompactFlash card";
- device_class_set_props(dc, ide_cf_properties);
-}
-
-static const TypeInfo ide_cf_info = {
- .name = "ide-cf",
- .parent = TYPE_IDE_DEVICE,
- .instance_size = sizeof(IDEDrive),
- .class_init = ide_cf_class_init,
-};
-
static void ide_device_class_init(ObjectClass *klass, void *data)
{
DeviceClass *k = DEVICE_CLASS(klass);
@@ -396,7 +350,6 @@ static void ide_register_types(void)
type_register_static(&ide_bus_info);
type_register_static(&ide_hd_info);
type_register_static(&ide_cd_info);
- type_register_static(&ide_cf_info);
type_register_static(&ide_device_type_info);
}
diff --git a/hw/ide/Kconfig b/hw/ide/Kconfig
index dd85fa3619..b93d6743d5 100644
--- a/hw/ide/Kconfig
+++ b/hw/ide/Kconfig
@@ -57,3 +57,7 @@ config IDE_SII3112
bool
select IDE_PCI
select IDE_QDEV
+
+config IDE_CF
+ bool
+ default y if IDE_QDEV
diff --git a/hw/ide/meson.build b/hw/ide/meson.build
index e050eef942..d2e5b45c9e 100644
--- a/hw/ide/meson.build
+++ b/hw/ide/meson.build
@@ -1,6 +1,7 @@
system_ss.add(when: 'CONFIG_AHCI', if_true: files('ahci.c'))
system_ss.add(when: 'CONFIG_AHCI_ICH9', if_true: files('ich.c'))
system_ss.add(when: 'CONFIG_ALLWINNER_A10', if_true: files('ahci-allwinner.c'))
+system_ss.add(when: 'CONFIG_IDE_CF', if_true: files('cf.c'))
system_ss.add(when: 'CONFIG_IDE_CMD646', if_true: files('cmd646.c'))
system_ss.add(when: 'CONFIG_IDE_CORE', if_true: files('core.c', 'atapi.c'))
system_ss.add(when: 'CONFIG_IDE_ISA', if_true: files('isa.c', 'ioport.c'))
--
2.43.2
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH 2/7] hw/ide: Split qdev.c into ide-bus.c and ide-dev.c
2024-02-19 10:49 [PATCH 0/7] hw/ide: Clean up hw/ide/qdev.c and include/hw/ide/internal.h Thomas Huth
2024-02-19 10:49 ` [PATCH 1/7] hw/ide: Add the possibility to disable the CompactFlash device in the build Thomas Huth
@ 2024-02-19 10:49 ` Thomas Huth
2024-02-19 11:45 ` BALATON Zoltan
2024-02-19 10:49 ` [PATCH 3/7] hw/ide: Move IDE device related definitions to ide-dev.h Thomas Huth
` (5 subsequent siblings)
7 siblings, 1 reply; 16+ messages in thread
From: Thomas Huth @ 2024-02-19 10:49 UTC (permalink / raw)
To: John Snow, qemu-devel
Cc: BALATON Zoltan, Philippe Mathieu-Daudé, qemu-block
qdev.c is a mixture between IDE bus specific functions and IDE device
functions. Let's split it up to make it more obvious which part is
related to bus handling and which part is related to device handling.
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
hw/ide/ide-bus.c | 111 +++++++++++++++++++++++++++++++++++
hw/ide/{qdev.c => ide-dev.c} | 87 +--------------------------
hw/arm/Kconfig | 2 +
hw/ide/Kconfig | 30 ++++++----
hw/ide/meson.build | 3 +-
5 files changed, 134 insertions(+), 99 deletions(-)
create mode 100644 hw/ide/ide-bus.c
rename hw/ide/{qdev.c => ide-dev.c} (78%)
diff --git a/hw/ide/ide-bus.c b/hw/ide/ide-bus.c
new file mode 100644
index 0000000000..57fe67b29c
--- /dev/null
+++ b/hw/ide/ide-bus.c
@@ -0,0 +1,111 @@
+/*
+ * ide bus support for qdev.
+ *
+ * Copyright (c) 2009 Gerd Hoffmann <kraxel@redhat.com>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "qemu/error-report.h"
+#include "qemu/module.h"
+#include "hw/ide/internal.h"
+#include "sysemu/block-backend.h"
+#include "sysemu/blockdev.h"
+#include "sysemu/runstate.h"
+
+static char *idebus_get_fw_dev_path(DeviceState *dev);
+static void idebus_unrealize(BusState *qdev);
+
+static void ide_bus_class_init(ObjectClass *klass, void *data)
+{
+ BusClass *k = BUS_CLASS(klass);
+
+ k->get_fw_dev_path = idebus_get_fw_dev_path;
+ k->unrealize = idebus_unrealize;
+}
+
+static void idebus_unrealize(BusState *bus)
+{
+ IDEBus *ibus = IDE_BUS(bus);
+
+ if (ibus->vmstate) {
+ qemu_del_vm_change_state_handler(ibus->vmstate);
+ }
+}
+
+static const TypeInfo ide_bus_info = {
+ .name = TYPE_IDE_BUS,
+ .parent = TYPE_BUS,
+ .instance_size = sizeof(IDEBus),
+ .class_init = ide_bus_class_init,
+};
+
+void ide_bus_init(IDEBus *idebus, size_t idebus_size, DeviceState *dev,
+ int bus_id, int max_units)
+{
+ qbus_init(idebus, idebus_size, TYPE_IDE_BUS, dev, NULL);
+ idebus->bus_id = bus_id;
+ idebus->max_units = max_units;
+}
+
+static char *idebus_get_fw_dev_path(DeviceState *dev)
+{
+ char path[30];
+
+ snprintf(path, sizeof(path), "%s@%x", qdev_fw_name(dev),
+ ((IDEBus *)dev->parent_bus)->bus_id);
+
+ return g_strdup(path);
+}
+
+IDEDevice *ide_bus_create_drive(IDEBus *bus, int unit, DriveInfo *drive)
+{
+ DeviceState *dev;
+
+ dev = qdev_new(drive->media_cd ? "ide-cd" : "ide-hd");
+ qdev_prop_set_uint32(dev, "unit", unit);
+ qdev_prop_set_drive_err(dev, "drive", blk_by_legacy_dinfo(drive),
+ &error_fatal);
+ qdev_realize_and_unref(dev, &bus->qbus, &error_fatal);
+ return DO_UPCAST(IDEDevice, qdev, dev);
+}
+
+int ide_get_geometry(BusState *bus, int unit,
+ int16_t *cyls, int8_t *heads, int8_t *secs)
+{
+ IDEState *s = &DO_UPCAST(IDEBus, qbus, bus)->ifs[unit];
+
+ if (s->drive_kind != IDE_HD || !s->blk) {
+ return -1;
+ }
+
+ *cyls = s->cylinders;
+ *heads = s->heads;
+ *secs = s->sectors;
+ return 0;
+}
+
+int ide_get_bios_chs_trans(BusState *bus, int unit)
+{
+ return DO_UPCAST(IDEBus, qbus, bus)->ifs[unit].chs_trans;
+}
+
+static void ide_bus_register_type(void)
+{
+ type_register_static(&ide_bus_info);
+}
+
+type_init(ide_bus_register_type)
diff --git a/hw/ide/qdev.c b/hw/ide/ide-dev.c
similarity index 78%
rename from hw/ide/qdev.c
rename to hw/ide/ide-dev.c
index 4189313d30..15d088fd06 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/ide-dev.c
@@ -1,5 +1,5 @@
/*
- * ide bus support for qdev.
+ * IDE device functions
*
* Copyright (c) 2009 Gerd Hoffmann <kraxel@redhat.com>
*
@@ -18,71 +18,21 @@
*/
#include "qemu/osdep.h"
-#include "sysemu/dma.h"
#include "qapi/error.h"
#include "qapi/qapi-types-block.h"
#include "qemu/error-report.h"
-#include "qemu/main-loop.h"
#include "qemu/module.h"
#include "hw/ide/ide-dev.h"
#include "sysemu/block-backend.h"
#include "sysemu/blockdev.h"
#include "sysemu/sysemu.h"
-#include "sysemu/runstate.h"
#include "qapi/visitor.h"
-/* --------------------------------- */
-
-static char *idebus_get_fw_dev_path(DeviceState *dev);
-static void idebus_unrealize(BusState *qdev);
-
static Property ide_props[] = {
DEFINE_PROP_UINT32("unit", IDEDevice, unit, -1),
DEFINE_PROP_END_OF_LIST(),
};
-static void ide_bus_class_init(ObjectClass *klass, void *data)
-{
- BusClass *k = BUS_CLASS(klass);
-
- k->get_fw_dev_path = idebus_get_fw_dev_path;
- k->unrealize = idebus_unrealize;
-}
-
-static void idebus_unrealize(BusState *bus)
-{
- IDEBus *ibus = IDE_BUS(bus);
-
- if (ibus->vmstate) {
- qemu_del_vm_change_state_handler(ibus->vmstate);
- }
-}
-
-static const TypeInfo ide_bus_info = {
- .name = TYPE_IDE_BUS,
- .parent = TYPE_BUS,
- .instance_size = sizeof(IDEBus),
- .class_init = ide_bus_class_init,
-};
-
-void ide_bus_init(IDEBus *idebus, size_t idebus_size, DeviceState *dev,
- int bus_id, int max_units)
-{
- qbus_init(idebus, idebus_size, TYPE_IDE_BUS, dev, NULL);
- idebus->bus_id = bus_id;
- idebus->max_units = max_units;
-}
-
-static char *idebus_get_fw_dev_path(DeviceState *dev)
-{
- char path[30];
-
- snprintf(path, sizeof(path), "%s@%x", qdev_fw_name(dev),
- ((IDEBus*)dev->parent_bus)->bus_id);
-
- return g_strdup(path);
-}
-
static void ide_qdev_realize(DeviceState *qdev, Error **errp)
{
IDEDevice *dev = IDE_DEVICE(qdev);
@@ -121,40 +71,6 @@ static void ide_qdev_realize(DeviceState *qdev, Error **errp)
dc->realize(dev, errp);
}
-IDEDevice *ide_bus_create_drive(IDEBus *bus, int unit, DriveInfo *drive)
-{
- DeviceState *dev;
-
- dev = qdev_new(drive->media_cd ? "ide-cd" : "ide-hd");
- qdev_prop_set_uint32(dev, "unit", unit);
- qdev_prop_set_drive_err(dev, "drive", blk_by_legacy_dinfo(drive),
- &error_fatal);
- qdev_realize_and_unref(dev, &bus->qbus, &error_fatal);
- return DO_UPCAST(IDEDevice, qdev, dev);
-}
-
-int ide_get_geometry(BusState *bus, int unit,
- int16_t *cyls, int8_t *heads, int8_t *secs)
-{
- IDEState *s = &DO_UPCAST(IDEBus, qbus, bus)->ifs[unit];
-
- if (s->drive_kind != IDE_HD || !s->blk) {
- return -1;
- }
-
- *cyls = s->cylinders;
- *heads = s->heads;
- *secs = s->sectors;
- return 0;
-}
-
-int ide_get_bios_chs_trans(BusState *bus, int unit)
-{
- return DO_UPCAST(IDEBus, qbus, bus)->ifs[unit].chs_trans;
-}
-
-/* --------------------------------- */
-
void ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind, Error **errp)
{
IDEBus *bus = DO_UPCAST(IDEBus, qbus, dev->qdev.parent_bus);
@@ -347,7 +263,6 @@ static const TypeInfo ide_device_type_info = {
static void ide_register_types(void)
{
- type_register_static(&ide_bus_info);
type_register_static(&ide_hd_info);
type_register_static(&ide_cd_info);
type_register_static(&ide_device_type_info);
diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index 29abe1da29..b372b819a4 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -275,6 +275,8 @@ config SBSA_REF
select USB_XHCI_SYSBUS
select WDT_SBSA
select BOCHS_DISPLAY
+ select IDE_BUS
+ select IDE_DEV
config SABRELITE
bool
diff --git a/hw/ide/Kconfig b/hw/ide/Kconfig
index b93d6743d5..6dfc5a2129 100644
--- a/hw/ide/Kconfig
+++ b/hw/ide/Kconfig
@@ -1,51 +1,58 @@
config IDE_CORE
bool
-config IDE_QDEV
+config IDE_BUS
bool
select IDE_CORE
+config IDE_DEV
+ bool
+ depends on IDE_BUS
+
config IDE_PCI
bool
depends on PCI
- select IDE_QDEV
+ select IDE_BUS
+ select IDE_DEV
config IDE_ISA
bool
depends on ISA_BUS
- select IDE_QDEV
+ select IDE_BUS
+ select IDE_DEV
config IDE_PIIX
bool
select IDE_PCI
- select IDE_QDEV
config IDE_CMD646
bool
select IDE_PCI
- select IDE_QDEV
config IDE_MACIO
bool
- select IDE_QDEV
+ select IDE_BUS
+ select IDE_DEV
config IDE_MMIO
bool
- select IDE_QDEV
+ select IDE_BUS
+ select IDE_DEV
config IDE_VIA
bool
select IDE_PCI
- select IDE_QDEV
config MICRODRIVE
bool
- select IDE_QDEV
+ select IDE_BUS
+ select IDE_DEV
depends on PCMCIA
config AHCI
bool
- select IDE_QDEV
+ select IDE_BUS
+ select IDE_DEV
config AHCI_ICH9
bool
@@ -56,8 +63,7 @@ config AHCI_ICH9
config IDE_SII3112
bool
select IDE_PCI
- select IDE_QDEV
config IDE_CF
bool
- default y if IDE_QDEV
+ default y if IDE_BUS
diff --git a/hw/ide/meson.build b/hw/ide/meson.build
index d2e5b45c9e..d09705cac0 100644
--- a/hw/ide/meson.build
+++ b/hw/ide/meson.build
@@ -1,15 +1,16 @@
system_ss.add(when: 'CONFIG_AHCI', if_true: files('ahci.c'))
system_ss.add(when: 'CONFIG_AHCI_ICH9', if_true: files('ich.c'))
system_ss.add(when: 'CONFIG_ALLWINNER_A10', if_true: files('ahci-allwinner.c'))
+system_ss.add(when: 'CONFIG_IDE_BUS', if_true: files('ide-bus.c'))
system_ss.add(when: 'CONFIG_IDE_CF', if_true: files('cf.c'))
system_ss.add(when: 'CONFIG_IDE_CMD646', if_true: files('cmd646.c'))
system_ss.add(when: 'CONFIG_IDE_CORE', if_true: files('core.c', 'atapi.c'))
+system_ss.add(when: 'CONFIG_IDE_DEV', if_true: files('ide-dev.c'))
system_ss.add(when: 'CONFIG_IDE_ISA', if_true: files('isa.c', 'ioport.c'))
system_ss.add(when: 'CONFIG_IDE_MACIO', if_true: files('macio.c'))
system_ss.add(when: 'CONFIG_IDE_MMIO', if_true: files('mmio.c'))
system_ss.add(when: 'CONFIG_IDE_PCI', if_true: files('pci.c'))
system_ss.add(when: 'CONFIG_IDE_PIIX', if_true: files('piix.c', 'ioport.c'))
-system_ss.add(when: 'CONFIG_IDE_QDEV', if_true: files('qdev.c'))
system_ss.add(when: 'CONFIG_IDE_SII3112', if_true: files('sii3112.c'))
system_ss.add(when: 'CONFIG_IDE_VIA', if_true: files('via.c'))
system_ss.add(when: 'CONFIG_MICRODRIVE', if_true: files('microdrive.c'))
--
2.43.2
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH 2/7] hw/ide: Split qdev.c into ide-bus.c and ide-dev.c
2024-02-19 10:49 ` [PATCH 2/7] hw/ide: Split qdev.c into ide-bus.c and ide-dev.c Thomas Huth
@ 2024-02-19 11:45 ` BALATON Zoltan
2024-02-19 18:31 ` Thomas Huth
0 siblings, 1 reply; 16+ messages in thread
From: BALATON Zoltan @ 2024-02-19 11:45 UTC (permalink / raw)
To: Thomas Huth
Cc: John Snow, qemu-devel, Philippe Mathieu-Daudé, qemu-block
On Mon, 19 Feb 2024, Thomas Huth wrote:
> qdev.c is a mixture between IDE bus specific functions and IDE device
> functions. Let's split it up to make it more obvious which part is
> related to bus handling and which part is related to device handling.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
> hw/ide/ide-bus.c | 111 +++++++++++++++++++++++++++++++++++
> hw/ide/{qdev.c => ide-dev.c} | 87 +--------------------------
> hw/arm/Kconfig | 2 +
> hw/ide/Kconfig | 30 ++++++----
> hw/ide/meson.build | 3 +-
> 5 files changed, 134 insertions(+), 99 deletions(-)
> create mode 100644 hw/ide/ide-bus.c
> rename hw/ide/{qdev.c => ide-dev.c} (78%)
[...]
> diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
> index 29abe1da29..b372b819a4 100644
> --- a/hw/arm/Kconfig
> +++ b/hw/arm/Kconfig
> @@ -275,6 +275,8 @@ config SBSA_REF
> select USB_XHCI_SYSBUS
> select WDT_SBSA
> select BOCHS_DISPLAY
> + select IDE_BUS
> + select IDE_DEV
>
> config SABRELITE
> bool
> diff --git a/hw/ide/Kconfig b/hw/ide/Kconfig
> index b93d6743d5..6dfc5a2129 100644
> --- a/hw/ide/Kconfig
> +++ b/hw/ide/Kconfig
> @@ -1,51 +1,58 @@
> config IDE_CORE
> bool
>
> -config IDE_QDEV
> +config IDE_BUS
> bool
> select IDE_CORE
Maybe we can assume if something has an IDE bus it also wants to connect
IDE devices to it so just select IDE_DEV here and not at every place
IDE_BUS is selected? Or is there a place that only wants IDE_BUS?
Regards,
BALATON Zoltan
> +config IDE_DEV
> + bool
> + depends on IDE_BUS
> +
> config IDE_PCI
> bool
> depends on PCI
> - select IDE_QDEV
> + select IDE_BUS
> + select IDE_DEV
>
> config IDE_ISA
> bool
> depends on ISA_BUS
> - select IDE_QDEV
> + select IDE_BUS
> + select IDE_DEV
>
> config IDE_PIIX
> bool
> select IDE_PCI
> - select IDE_QDEV
>
> config IDE_CMD646
> bool
> select IDE_PCI
> - select IDE_QDEV
>
> config IDE_MACIO
> bool
> - select IDE_QDEV
> + select IDE_BUS
> + select IDE_DEV
>
> config IDE_MMIO
> bool
> - select IDE_QDEV
> + select IDE_BUS
> + select IDE_DEV
>
> config IDE_VIA
> bool
> select IDE_PCI
> - select IDE_QDEV
>
> config MICRODRIVE
> bool
> - select IDE_QDEV
> + select IDE_BUS
> + select IDE_DEV
> depends on PCMCIA
>
> config AHCI
> bool
> - select IDE_QDEV
> + select IDE_BUS
> + select IDE_DEV
>
> config AHCI_ICH9
> bool
> @@ -56,8 +63,7 @@ config AHCI_ICH9
> config IDE_SII3112
> bool
> select IDE_PCI
> - select IDE_QDEV
>
> config IDE_CF
> bool
> - default y if IDE_QDEV
> + default y if IDE_BUS
> diff --git a/hw/ide/meson.build b/hw/ide/meson.build
> index d2e5b45c9e..d09705cac0 100644
> --- a/hw/ide/meson.build
> +++ b/hw/ide/meson.build
> @@ -1,15 +1,16 @@
> system_ss.add(when: 'CONFIG_AHCI', if_true: files('ahci.c'))
> system_ss.add(when: 'CONFIG_AHCI_ICH9', if_true: files('ich.c'))
> system_ss.add(when: 'CONFIG_ALLWINNER_A10', if_true: files('ahci-allwinner.c'))
> +system_ss.add(when: 'CONFIG_IDE_BUS', if_true: files('ide-bus.c'))
> system_ss.add(when: 'CONFIG_IDE_CF', if_true: files('cf.c'))
> system_ss.add(when: 'CONFIG_IDE_CMD646', if_true: files('cmd646.c'))
> system_ss.add(when: 'CONFIG_IDE_CORE', if_true: files('core.c', 'atapi.c'))
> +system_ss.add(when: 'CONFIG_IDE_DEV', if_true: files('ide-dev.c'))
> system_ss.add(when: 'CONFIG_IDE_ISA', if_true: files('isa.c', 'ioport.c'))
> system_ss.add(when: 'CONFIG_IDE_MACIO', if_true: files('macio.c'))
> system_ss.add(when: 'CONFIG_IDE_MMIO', if_true: files('mmio.c'))
> system_ss.add(when: 'CONFIG_IDE_PCI', if_true: files('pci.c'))
> system_ss.add(when: 'CONFIG_IDE_PIIX', if_true: files('piix.c', 'ioport.c'))
> -system_ss.add(when: 'CONFIG_IDE_QDEV', if_true: files('qdev.c'))
> system_ss.add(when: 'CONFIG_IDE_SII3112', if_true: files('sii3112.c'))
> system_ss.add(when: 'CONFIG_IDE_VIA', if_true: files('via.c'))
> system_ss.add(when: 'CONFIG_MICRODRIVE', if_true: files('microdrive.c'))
>
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH 2/7] hw/ide: Split qdev.c into ide-bus.c and ide-dev.c
2024-02-19 11:45 ` BALATON Zoltan
@ 2024-02-19 18:31 ` Thomas Huth
0 siblings, 0 replies; 16+ messages in thread
From: Thomas Huth @ 2024-02-19 18:31 UTC (permalink / raw)
To: BALATON Zoltan
Cc: John Snow, qemu-devel, Philippe Mathieu-Daudé, qemu-block
On 19/02/2024 12.45, BALATON Zoltan wrote:
> On Mon, 19 Feb 2024, Thomas Huth wrote:
>> qdev.c is a mixture between IDE bus specific functions and IDE device
>> functions. Let's split it up to make it more obvious which part is
>> related to bus handling and which part is related to device handling.
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>> hw/ide/ide-bus.c | 111 +++++++++++++++++++++++++++++++++++
>> hw/ide/{qdev.c => ide-dev.c} | 87 +--------------------------
>> hw/arm/Kconfig | 2 +
>> hw/ide/Kconfig | 30 ++++++----
>> hw/ide/meson.build | 3 +-
>> 5 files changed, 134 insertions(+), 99 deletions(-)
>> create mode 100644 hw/ide/ide-bus.c
>> rename hw/ide/{qdev.c => ide-dev.c} (78%)
> [...]
>> diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
>> index 29abe1da29..b372b819a4 100644
>> --- a/hw/arm/Kconfig
>> +++ b/hw/arm/Kconfig
>> @@ -275,6 +275,8 @@ config SBSA_REF
>> select USB_XHCI_SYSBUS
>> select WDT_SBSA
>> select BOCHS_DISPLAY
>> + select IDE_BUS
>> + select IDE_DEV
>>
>> config SABRELITE
>> bool
>> diff --git a/hw/ide/Kconfig b/hw/ide/Kconfig
>> index b93d6743d5..6dfc5a2129 100644
>> --- a/hw/ide/Kconfig
>> +++ b/hw/ide/Kconfig
>> @@ -1,51 +1,58 @@
>> config IDE_CORE
>> bool
>>
>> -config IDE_QDEV
>> +config IDE_BUS
>> bool
>> select IDE_CORE
>
> Maybe we can assume if something has an IDE bus it also wants to connect IDE
> devices to it so just select IDE_DEV here and not at every place IDE_BUS is
> selected? Or is there a place that only wants IDE_BUS?
Currently not, but I think it's conceptually much cleaner if we are explicit
here.
Thomas
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 3/7] hw/ide: Move IDE device related definitions to ide-dev.h
2024-02-19 10:49 [PATCH 0/7] hw/ide: Clean up hw/ide/qdev.c and include/hw/ide/internal.h Thomas Huth
2024-02-19 10:49 ` [PATCH 1/7] hw/ide: Add the possibility to disable the CompactFlash device in the build Thomas Huth
2024-02-19 10:49 ` [PATCH 2/7] hw/ide: Split qdev.c into ide-bus.c and ide-dev.c Thomas Huth
@ 2024-02-19 10:49 ` Thomas Huth
2024-02-19 11:32 ` Philippe Mathieu-Daudé
2024-02-19 10:49 ` [PATCH 4/7] hw/ide: Move IDE bus related definitions to a new header ide-bus.h Thomas Huth
` (4 subsequent siblings)
7 siblings, 1 reply; 16+ messages in thread
From: Thomas Huth @ 2024-02-19 10:49 UTC (permalink / raw)
To: John Snow, qemu-devel
Cc: BALATON Zoltan, Philippe Mathieu-Daudé, qemu-block
Let's start to unentangle internal.h by moving public IDE device
related definitions to ide-dev.h.
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
include/hw/ide/ide-dev.h | 145 +++++++++++++++++++++++++++++++++++++-
include/hw/ide/internal.h | 145 +-------------------------------------
hw/ide/ide-dev.c | 1 +
3 files changed, 146 insertions(+), 145 deletions(-)
diff --git a/include/hw/ide/ide-dev.h b/include/hw/ide/ide-dev.h
index 7e9663cda9..de88784a25 100644
--- a/include/hw/ide/ide-dev.h
+++ b/include/hw/ide/ide-dev.h
@@ -20,9 +20,152 @@
#ifndef IDE_DEV_H
#define IDE_DEV_H
+#include "sysemu/dma.h"
#include "hw/qdev-properties.h"
#include "hw/block/block.h"
-#include "hw/ide/internal.h"
+
+typedef struct IDEDevice IDEDevice;
+typedef struct IDEState IDEState;
+typedef struct IDEDMA IDEDMA;
+typedef struct IDEDMAOps IDEDMAOps;
+typedef struct IDEBus IDEBus;
+
+typedef void EndTransferFunc(IDEState *);
+
+#define MAX_IDE_DEVS 2
+
+#define TYPE_IDE_DEVICE "ide-device"
+OBJECT_DECLARE_TYPE(IDEDevice, IDEDeviceClass, IDE_DEVICE)
+
+typedef enum { IDE_HD, IDE_CD, IDE_CFATA } IDEDriveKind;
+
+struct unreported_events {
+ bool eject_request;
+ bool new_media;
+};
+
+enum ide_dma_cmd {
+ IDE_DMA_READ = 0,
+ IDE_DMA_WRITE,
+ IDE_DMA_TRIM,
+ IDE_DMA_ATAPI,
+ IDE_DMA__COUNT
+};
+
+/* NOTE: IDEState represents in fact one drive */
+struct IDEState {
+ IDEBus *bus;
+ uint8_t unit;
+ /* ide config */
+ IDEDriveKind drive_kind;
+ int drive_heads, drive_sectors;
+ int cylinders, heads, sectors, chs_trans;
+ int64_t nb_sectors;
+ int mult_sectors;
+ int identify_set;
+ uint8_t identify_data[512];
+ int drive_serial;
+ char drive_serial_str[21];
+ char drive_model_str[41];
+ uint64_t wwn;
+ /* ide regs */
+ uint8_t feature;
+ uint8_t error;
+ uint32_t nsector;
+ uint8_t sector;
+ uint8_t lcyl;
+ uint8_t hcyl;
+ /* other part of tf for lba48 support */
+ uint8_t hob_feature;
+ uint8_t hob_nsector;
+ uint8_t hob_sector;
+ uint8_t hob_lcyl;
+ uint8_t hob_hcyl;
+
+ uint8_t select;
+ uint8_t status;
+
+ bool io8;
+ bool reset_reverts;
+
+ /* set for lba48 access */
+ uint8_t lba48;
+ BlockBackend *blk;
+ char version[9];
+ /* ATAPI specific */
+ struct unreported_events events;
+ uint8_t sense_key;
+ uint8_t asc;
+ bool tray_open;
+ bool tray_locked;
+ uint8_t cdrom_changed;
+ int packet_transfer_size;
+ int elementary_transfer_size;
+ int32_t io_buffer_index;
+ int lba;
+ int cd_sector_size;
+ int atapi_dma; /* true if dma is requested for the packet cmd */
+ BlockAcctCookie acct;
+ BlockAIOCB *pio_aiocb;
+ QEMUIOVector qiov;
+ QLIST_HEAD(, IDEBufferedRequest) buffered_requests;
+ /* ATA DMA state */
+ uint64_t io_buffer_offset;
+ int32_t io_buffer_size;
+ QEMUSGList sg;
+ /* PIO transfer handling */
+ int req_nb_sectors; /* number of sectors per interrupt */
+ EndTransferFunc *end_transfer_func;
+ uint8_t *data_ptr;
+ uint8_t *data_end;
+ uint8_t *io_buffer;
+ /* PIO save/restore */
+ int32_t io_buffer_total_len;
+ int32_t cur_io_buffer_offset;
+ int32_t cur_io_buffer_len;
+ uint8_t end_transfer_fn_idx;
+ QEMUTimer *sector_write_timer; /* only used for win2k install hack */
+ uint32_t irq_count; /* counts IRQs when using win2k install hack */
+ /* CF-ATA extended error */
+ uint8_t ext_error;
+ /* CF-ATA metadata storage */
+ uint32_t mdata_size;
+ uint8_t *mdata_storage;
+ int media_changed;
+ enum ide_dma_cmd dma_cmd;
+ /* SMART */
+ uint8_t smart_enabled;
+ uint8_t smart_autosave;
+ int smart_errors;
+ uint8_t smart_selftest_count;
+ uint8_t *smart_selftest_data;
+ /* AHCI */
+ int ncq_queues;
+};
+
+struct IDEDeviceClass {
+ DeviceClass parent_class;
+ void (*realize)(IDEDevice *dev, Error **errp);
+};
+
+struct IDEDevice {
+ DeviceState qdev;
+ uint32_t unit;
+ BlockConf conf;
+ int chs_trans;
+ char *version;
+ char *serial;
+ char *model;
+ uint64_t wwn;
+ /*
+ * 0x0000 - rotation rate not reported
+ * 0x0001 - non-rotating medium (SSD)
+ * 0x0002-0x0400 - reserved
+ * 0x0401-0xffe - rotations per minute
+ * 0xffff - reserved
+ */
+ uint16_t rotation_rate;
+};
typedef struct IDEDrive {
IDEDevice dev;
diff --git a/include/hw/ide/internal.h b/include/hw/ide/internal.h
index 3bdcc75597..5cc109fe82 100644
--- a/include/hw/ide/internal.h
+++ b/include/hw/ide/internal.h
@@ -8,24 +8,16 @@
*/
#include "hw/ide.h"
-#include "sysemu/dma.h"
-#include "hw/block/block.h"
#include "exec/ioport.h"
+#include "hw/ide/ide-dev.h"
/* debug IDE devices */
#define USE_DMA_CDROM
#include "qom/object.h"
-typedef struct IDEDevice IDEDevice;
-typedef struct IDEState IDEState;
-typedef struct IDEDMA IDEDMA;
-typedef struct IDEDMAOps IDEDMAOps;
-
#define TYPE_IDE_BUS "IDE"
OBJECT_DECLARE_SIMPLE_TYPE(IDEBus, IDE_BUS)
-#define MAX_IDE_DEVS 2
-
/* Device/Head ("select") Register */
#define ATA_DEV_SELECT 0x10
/* ATA1,3: Defined as '1'.
@@ -328,10 +320,6 @@ OBJECT_DECLARE_SIMPLE_TYPE(IDEBus, IDE_BUS)
#define SMART_DISABLE 0xd9
#define SMART_STATUS 0xda
-typedef enum { IDE_HD, IDE_CD, IDE_CFATA } IDEDriveKind;
-
-typedef void EndTransferFunc(IDEState *);
-
typedef void DMAStartFunc(const IDEDMA *, IDEState *, BlockCompletionFunc *);
typedef void DMAVoidFunc(const IDEDMA *);
typedef int DMAIntFunc(const IDEDMA *, bool);
@@ -339,19 +327,6 @@ typedef int32_t DMAInt32Func(const IDEDMA *, int32_t len);
typedef void DMAu32Func(const IDEDMA *, uint32_t);
typedef void DMAStopFunc(const IDEDMA *, bool);
-struct unreported_events {
- bool eject_request;
- bool new_media;
-};
-
-enum ide_dma_cmd {
- IDE_DMA_READ = 0,
- IDE_DMA_WRITE,
- IDE_DMA_TRIM,
- IDE_DMA_ATAPI,
- IDE_DMA__COUNT
-};
-
extern const char *IDE_DMA_CMD_lookup[IDE_DMA__COUNT];
extern const MemoryRegionPortio ide_portio_list[];
@@ -369,97 +344,6 @@ typedef struct IDEBufferedRequest {
bool orphaned;
} IDEBufferedRequest;
-/* NOTE: IDEState represents in fact one drive */
-struct IDEState {
- IDEBus *bus;
- uint8_t unit;
- /* ide config */
- IDEDriveKind drive_kind;
- int drive_heads, drive_sectors;
- int cylinders, heads, sectors, chs_trans;
- int64_t nb_sectors;
- int mult_sectors;
- int identify_set;
- uint8_t identify_data[512];
- int drive_serial;
- char drive_serial_str[21];
- char drive_model_str[41];
- uint64_t wwn;
- /* ide regs */
- uint8_t feature;
- uint8_t error;
- uint32_t nsector;
- uint8_t sector;
- uint8_t lcyl;
- uint8_t hcyl;
- /* other part of tf for lba48 support */
- uint8_t hob_feature;
- uint8_t hob_nsector;
- uint8_t hob_sector;
- uint8_t hob_lcyl;
- uint8_t hob_hcyl;
-
- uint8_t select;
- uint8_t status;
-
- bool io8;
- bool reset_reverts;
-
- /* set for lba48 access */
- uint8_t lba48;
- BlockBackend *blk;
- char version[9];
- /* ATAPI specific */
- struct unreported_events events;
- uint8_t sense_key;
- uint8_t asc;
- bool tray_open;
- bool tray_locked;
- uint8_t cdrom_changed;
- int packet_transfer_size;
- int elementary_transfer_size;
- int32_t io_buffer_index;
- int lba;
- int cd_sector_size;
- int atapi_dma; /* true if dma is requested for the packet cmd */
- BlockAcctCookie acct;
- BlockAIOCB *pio_aiocb;
- QEMUIOVector qiov;
- QLIST_HEAD(, IDEBufferedRequest) buffered_requests;
- /* ATA DMA state */
- uint64_t io_buffer_offset;
- int32_t io_buffer_size;
- QEMUSGList sg;
- /* PIO transfer handling */
- int req_nb_sectors; /* number of sectors per interrupt */
- EndTransferFunc *end_transfer_func;
- uint8_t *data_ptr;
- uint8_t *data_end;
- uint8_t *io_buffer;
- /* PIO save/restore */
- int32_t io_buffer_total_len;
- int32_t cur_io_buffer_offset;
- int32_t cur_io_buffer_len;
- uint8_t end_transfer_fn_idx;
- QEMUTimer *sector_write_timer; /* only used for win2k install hack */
- uint32_t irq_count; /* counts IRQs when using win2k install hack */
- /* CF-ATA extended error */
- uint8_t ext_error;
- /* CF-ATA metadata storage */
- uint32_t mdata_size;
- uint8_t *mdata_storage;
- int media_changed;
- enum ide_dma_cmd dma_cmd;
- /* SMART */
- uint8_t smart_enabled;
- uint8_t smart_autosave;
- int smart_errors;
- uint8_t smart_selftest_count;
- uint8_t *smart_selftest_data;
- /* AHCI */
- int ncq_queues;
-};
-
struct IDEDMAOps {
DMAStartFunc *start_dma;
DMAVoidFunc *pio_transfer;
@@ -502,33 +386,6 @@ struct IDEBus {
VMChangeStateEntry *vmstate;
};
-#define TYPE_IDE_DEVICE "ide-device"
-OBJECT_DECLARE_TYPE(IDEDevice, IDEDeviceClass, IDE_DEVICE)
-
-struct IDEDeviceClass {
- DeviceClass parent_class;
- void (*realize)(IDEDevice *dev, Error **errp);
-};
-
-struct IDEDevice {
- DeviceState qdev;
- uint32_t unit;
- BlockConf conf;
- int chs_trans;
- char *version;
- char *serial;
- char *model;
- uint64_t wwn;
- /*
- * 0x0000 - rotation rate not reported
- * 0x0001 - non-rotating medium (SSD)
- * 0x0002-0x0400 - reserved
- * 0x0401-0xffe - rotations per minute
- * 0xffff - reserved
- */
- uint16_t rotation_rate;
-};
-
/* These are used for the error_status field of IDEBus */
#define IDE_RETRY_MASK 0xf8
#define IDE_RETRY_DMA 0x08
diff --git a/hw/ide/ide-dev.c b/hw/ide/ide-dev.c
index 15d088fd06..c8e2033469 100644
--- a/hw/ide/ide-dev.c
+++ b/hw/ide/ide-dev.c
@@ -23,6 +23,7 @@
#include "qemu/error-report.h"
#include "qemu/module.h"
#include "hw/ide/ide-dev.h"
+#include "hw/ide/internal.h"
#include "sysemu/block-backend.h"
#include "sysemu/blockdev.h"
#include "sysemu/sysemu.h"
--
2.43.2
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH 3/7] hw/ide: Move IDE device related definitions to ide-dev.h
2024-02-19 10:49 ` [PATCH 3/7] hw/ide: Move IDE device related definitions to ide-dev.h Thomas Huth
@ 2024-02-19 11:32 ` Philippe Mathieu-Daudé
2024-02-19 19:17 ` Thomas Huth
0 siblings, 1 reply; 16+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-02-19 11:32 UTC (permalink / raw)
To: Thomas Huth, John Snow, qemu-devel; +Cc: BALATON Zoltan, qemu-block
On 19/2/24 11:49, Thomas Huth wrote:
> Let's start to unentangle internal.h by moving public IDE device
> related definitions to ide-dev.h.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
> include/hw/ide/ide-dev.h | 145 +++++++++++++++++++++++++++++++++++++-
> include/hw/ide/internal.h | 145 +-------------------------------------
> hw/ide/ide-dev.c | 1 +
> 3 files changed, 146 insertions(+), 145 deletions(-)
>
> diff --git a/include/hw/ide/ide-dev.h b/include/hw/ide/ide-dev.h
> index 7e9663cda9..de88784a25 100644
> --- a/include/hw/ide/ide-dev.h
> +++ b/include/hw/ide/ide-dev.h
> @@ -20,9 +20,152 @@
> #ifndef IDE_DEV_H
> #define IDE_DEV_H
>
> +#include "sysemu/dma.h"
Not required.
> #include "hw/qdev-properties.h"
> #include "hw/block/block.h"
> -#include "hw/ide/internal.h"
> +
> +typedef struct IDEDevice IDEDevice;
> +typedef struct IDEState IDEState;
> +typedef struct IDEDMA IDEDMA;
> +typedef struct IDEDMAOps IDEDMAOps;
> +typedef struct IDEBus IDEBus;
Looking at next patches, better forward-declare IDEBus and
IDEDMA in "qemu/typedefs.h".
IDEDMAOps and "sysemu/dma.h" belong to "hw/ide/ide-dma.h.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 3/7] hw/ide: Move IDE device related definitions to ide-dev.h
2024-02-19 11:32 ` Philippe Mathieu-Daudé
@ 2024-02-19 19:17 ` Thomas Huth
2024-02-20 7:18 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 16+ messages in thread
From: Thomas Huth @ 2024-02-19 19:17 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, John Snow, qemu-devel
Cc: BALATON Zoltan, qemu-block
On 19/02/2024 12.32, Philippe Mathieu-Daudé wrote:
> On 19/2/24 11:49, Thomas Huth wrote:
>> Let's start to unentangle internal.h by moving public IDE device
>> related definitions to ide-dev.h.
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>> include/hw/ide/ide-dev.h | 145 +++++++++++++++++++++++++++++++++++++-
>> include/hw/ide/internal.h | 145 +-------------------------------------
>> hw/ide/ide-dev.c | 1 +
>> 3 files changed, 146 insertions(+), 145 deletions(-)
>>
>> diff --git a/include/hw/ide/ide-dev.h b/include/hw/ide/ide-dev.h
>> index 7e9663cda9..de88784a25 100644
>> --- a/include/hw/ide/ide-dev.h
>> +++ b/include/hw/ide/ide-dev.h
>> @@ -20,9 +20,152 @@
>> #ifndef IDE_DEV_H
>> #define IDE_DEV_H
>> +#include "sysemu/dma.h"
>
> Not required.
It's required for QEMUSGList that is used in struct IDEState.
>> #include "hw/qdev-properties.h"
>> #include "hw/block/block.h"
>> -#include "hw/ide/internal.h"
>> +
>> +typedef struct IDEDevice IDEDevice;
>> +typedef struct IDEState IDEState;
>
>> +typedef struct IDEDMA IDEDMA;
>> +typedef struct IDEDMAOps IDEDMAOps;
>> +typedef struct IDEBus IDEBus;
>
> Looking at next patches, better forward-declare IDEBus and
> IDEDMA in "qemu/typedefs.h".
I really dislike using qemu/typedefs.h for things that are not really part
of the core framework, since it's a
touch-it-once-and-everything-gets-recompiled header. So IMHO the typedefs
here are the lesser evil.
> IDEDMAOps and "sysemu/dma.h" belong to "hw/ide/ide-dma.h.
Ok, I can move the typedef for IDEDMAOps to ide-dma.h instead.
Thomas
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 3/7] hw/ide: Move IDE device related definitions to ide-dev.h
2024-02-19 19:17 ` Thomas Huth
@ 2024-02-20 7:18 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 16+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-02-20 7:18 UTC (permalink / raw)
To: Thomas Huth, John Snow, qemu-devel; +Cc: BALATON Zoltan, qemu-block
On 19/2/24 20:17, Thomas Huth wrote:
> On 19/02/2024 12.32, Philippe Mathieu-Daudé wrote:
>> On 19/2/24 11:49, Thomas Huth wrote:
>>> Let's start to unentangle internal.h by moving public IDE device
>>> related definitions to ide-dev.h.
>>>
>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>> ---
>>> include/hw/ide/ide-dev.h | 145 +++++++++++++++++++++++++++++++++++++-
>>> include/hw/ide/internal.h | 145 +-------------------------------------
>>> hw/ide/ide-dev.c | 1 +
>>> 3 files changed, 146 insertions(+), 145 deletions(-)
>>>
>>> diff --git a/include/hw/ide/ide-dev.h b/include/hw/ide/ide-dev.h
>>> index 7e9663cda9..de88784a25 100644
>>> --- a/include/hw/ide/ide-dev.h
>>> +++ b/include/hw/ide/ide-dev.h
>>> @@ -20,9 +20,152 @@
>>> #ifndef IDE_DEV_H
>>> #define IDE_DEV_H
>>> +#include "sysemu/dma.h"
>>
>> Not required.
>
> It's required for QEMUSGList that is used in struct IDEState.
Oh, OK.
>>> #include "hw/qdev-properties.h"
>>> #include "hw/block/block.h"
>>> -#include "hw/ide/internal.h"
>>> +
>>> +typedef struct IDEDevice IDEDevice;
>>> +typedef struct IDEState IDEState;
>>
>>> +typedef struct IDEDMA IDEDMA;
>>> +typedef struct IDEDMAOps IDEDMAOps;
>>> +typedef struct IDEBus IDEBus;
>>
>> Looking at next patches, better forward-declare IDEBus and
>> IDEDMA in "qemu/typedefs.h".
>
> I really dislike using qemu/typedefs.h for things that are not really
> part of the core framework, since it's a
> touch-it-once-and-everything-gets-recompiled header. So IMHO the
> typedefs here are the lesser evil.
OK then.
>> IDEDMAOps and "sysemu/dma.h" belong to "hw/ide/ide-dma.h.
>
> Ok, I can move the typedef for IDEDMAOps to ide-dma.h instead.
>
> Thomas
>
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 4/7] hw/ide: Move IDE bus related definitions to a new header ide-bus.h
2024-02-19 10:49 [PATCH 0/7] hw/ide: Clean up hw/ide/qdev.c and include/hw/ide/internal.h Thomas Huth
` (2 preceding siblings ...)
2024-02-19 10:49 ` [PATCH 3/7] hw/ide: Move IDE device related definitions to ide-dev.h Thomas Huth
@ 2024-02-19 10:49 ` Thomas Huth
2024-02-19 10:49 ` [PATCH 5/7] hw/ide: Move IDE DMA related definitions to a separate header ide-dma.h Thomas Huth
` (3 subsequent siblings)
7 siblings, 0 replies; 16+ messages in thread
From: Thomas Huth @ 2024-02-19 10:49 UTC (permalink / raw)
To: John Snow, qemu-devel
Cc: BALATON Zoltan, Philippe Mathieu-Daudé, qemu-block
Let's consolidate the public IDE bus related functions in a separate
header.
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
include/hw/ide/ide-bus.h | 41 +++++++++++++++++++++++++++++++++++++++
include/hw/ide/internal.h | 38 +-----------------------------------
2 files changed, 42 insertions(+), 37 deletions(-)
create mode 100644 include/hw/ide/ide-bus.h
diff --git a/include/hw/ide/ide-bus.h b/include/hw/ide/ide-bus.h
new file mode 100644
index 0000000000..e0460700ed
--- /dev/null
+++ b/include/hw/ide/ide-bus.h
@@ -0,0 +1,41 @@
+#ifndef HW_IDE_BUS_H
+#define HW_IDE_BUS_H
+
+#include "exec/ioport.h"
+#include "hw/ide/ide-dev.h"
+
+struct IDEBus {
+ BusState qbus;
+ IDEDevice *master;
+ IDEDevice *slave;
+ IDEState ifs[2];
+ QEMUBH *bh;
+
+ int bus_id;
+ int max_units;
+ IDEDMA *dma;
+ uint8_t unit;
+ uint8_t cmd;
+ qemu_irq irq; /* bus output */
+
+ int error_status;
+ uint8_t retry_unit;
+ int64_t retry_sector_num;
+ uint32_t retry_nsector;
+ PortioList portio_list;
+ PortioList portio2_list;
+ VMChangeStateEntry *vmstate;
+};
+
+#define TYPE_IDE_BUS "IDE"
+OBJECT_DECLARE_SIMPLE_TYPE(IDEBus, IDE_BUS)
+
+void ide_bus_init(IDEBus *idebus, size_t idebus_size, DeviceState *dev,
+ int bus_id, int max_units);
+IDEDevice *ide_bus_create_drive(IDEBus *bus, int unit, DriveInfo *drive);
+
+int ide_get_geometry(BusState *bus, int unit,
+ int16_t *cyls, int8_t *heads, int8_t *secs);
+int ide_get_bios_chs_trans(BusState *bus, int unit);
+
+#endif
diff --git a/include/hw/ide/internal.h b/include/hw/ide/internal.h
index 5cc109fe82..642bd1a979 100644
--- a/include/hw/ide/internal.h
+++ b/include/hw/ide/internal.h
@@ -8,16 +8,12 @@
*/
#include "hw/ide.h"
-#include "exec/ioport.h"
-#include "hw/ide/ide-dev.h"
+#include "hw/ide/ide-bus.h"
/* debug IDE devices */
#define USE_DMA_CDROM
#include "qom/object.h"
-#define TYPE_IDE_BUS "IDE"
-OBJECT_DECLARE_SIMPLE_TYPE(IDEBus, IDE_BUS)
-
/* Device/Head ("select") Register */
#define ATA_DEV_SELECT 0x10
/* ATA1,3: Defined as '1'.
@@ -363,29 +359,6 @@ struct IDEDMA {
BlockAIOCB *aiocb;
};
-struct IDEBus {
- BusState qbus;
- IDEDevice *master;
- IDEDevice *slave;
- IDEState ifs[2];
- QEMUBH *bh;
-
- int bus_id;
- int max_units;
- IDEDMA *dma;
- uint8_t unit;
- uint8_t cmd;
- qemu_irq irq; /* bus output */
-
- int error_status;
- uint8_t retry_unit;
- int64_t retry_sector_num;
- uint32_t retry_nsector;
- PortioList portio_list;
- PortioList portio2_list;
- VMChangeStateEntry *vmstate;
-};
-
/* These are used for the error_status field of IDEBus */
#define IDE_RETRY_MASK 0xf8
#define IDE_RETRY_DMA 0x08
@@ -502,15 +475,6 @@ void ide_cancel_dma_sync(IDEState *s);
void ide_atapi_cmd(IDEState *s);
void ide_atapi_cmd_reply_end(IDEState *s);
-/* hw/ide/qdev.c */
-void ide_bus_init(IDEBus *idebus, size_t idebus_size, DeviceState *dev,
- int bus_id, int max_units);
-IDEDevice *ide_bus_create_drive(IDEBus *bus, int unit, DriveInfo *drive);
-
-int ide_get_geometry(BusState *bus, int unit,
- int16_t *cyls, int8_t *heads, int8_t *secs);
-int ide_get_bios_chs_trans(BusState *bus, int unit);
-
int ide_handle_rw_error(IDEState *s, int error, int op);
#endif /* HW_IDE_INTERNAL_H */
--
2.43.2
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH 5/7] hw/ide: Move IDE DMA related definitions to a separate header ide-dma.h
2024-02-19 10:49 [PATCH 0/7] hw/ide: Clean up hw/ide/qdev.c and include/hw/ide/internal.h Thomas Huth
` (3 preceding siblings ...)
2024-02-19 10:49 ` [PATCH 4/7] hw/ide: Move IDE bus related definitions to a new header ide-bus.h Thomas Huth
@ 2024-02-19 10:49 ` Thomas Huth
2024-02-19 11:53 ` BALATON Zoltan
2024-02-19 10:49 ` [PATCH 6/7] hw/ide: Remove the include/hw/ide.h legacy file Thomas Huth
` (2 subsequent siblings)
7 siblings, 1 reply; 16+ messages in thread
From: Thomas Huth @ 2024-02-19 10:49 UTC (permalink / raw)
To: John Snow, qemu-devel
Cc: BALATON Zoltan, Philippe Mathieu-Daudé, qemu-block
These definitions are required outside of the hw/ide/ code, too,
so lets's move them from internal.h to a new header called ide-dma.h.
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
include/hw/ide/ide-dma.h | 30 ++++++++++++++++++++++++++++++
include/hw/ide/internal.h | 27 +--------------------------
2 files changed, 31 insertions(+), 26 deletions(-)
create mode 100644 include/hw/ide/ide-dma.h
diff --git a/include/hw/ide/ide-dma.h b/include/hw/ide/ide-dma.h
new file mode 100644
index 0000000000..fb82966bdd
--- /dev/null
+++ b/include/hw/ide/ide-dma.h
@@ -0,0 +1,30 @@
+#ifndef HW_IDE_DMA_H
+#define HW_IDE_DMA_H
+
+typedef void DMAStartFunc(const IDEDMA *, IDEState *, BlockCompletionFunc *);
+typedef void DMAVoidFunc(const IDEDMA *);
+typedef int DMAIntFunc(const IDEDMA *, bool);
+typedef int32_t DMAInt32Func(const IDEDMA *, int32_t len);
+typedef void DMAu32Func(const IDEDMA *, uint32_t);
+typedef void DMAStopFunc(const IDEDMA *, bool);
+
+struct IDEDMAOps {
+ DMAStartFunc *start_dma;
+ DMAVoidFunc *pio_transfer;
+ DMAInt32Func *prepare_buf;
+ DMAu32Func *commit_buf;
+ DMAIntFunc *rw_buf;
+ DMAVoidFunc *restart;
+ DMAVoidFunc *restart_dma;
+ DMAStopFunc *set_inactive;
+ DMAVoidFunc *cmd_done;
+ DMAVoidFunc *reset;
+};
+
+struct IDEDMA {
+ const struct IDEDMAOps *ops;
+ QEMUIOVector qiov;
+ BlockAIOCB *aiocb;
+};
+
+#endif
diff --git a/include/hw/ide/internal.h b/include/hw/ide/internal.h
index 642bd1a979..d1d3fcd23a 100644
--- a/include/hw/ide/internal.h
+++ b/include/hw/ide/internal.h
@@ -9,6 +9,7 @@
#include "hw/ide.h"
#include "hw/ide/ide-bus.h"
+#include "hw/ide/ide-dma.h"
/* debug IDE devices */
#define USE_DMA_CDROM
@@ -316,13 +317,6 @@
#define SMART_DISABLE 0xd9
#define SMART_STATUS 0xda
-typedef void DMAStartFunc(const IDEDMA *, IDEState *, BlockCompletionFunc *);
-typedef void DMAVoidFunc(const IDEDMA *);
-typedef int DMAIntFunc(const IDEDMA *, bool);
-typedef int32_t DMAInt32Func(const IDEDMA *, int32_t len);
-typedef void DMAu32Func(const IDEDMA *, uint32_t);
-typedef void DMAStopFunc(const IDEDMA *, bool);
-
extern const char *IDE_DMA_CMD_lookup[IDE_DMA__COUNT];
extern const MemoryRegionPortio ide_portio_list[];
@@ -340,25 +334,6 @@ typedef struct IDEBufferedRequest {
bool orphaned;
} IDEBufferedRequest;
-struct IDEDMAOps {
- DMAStartFunc *start_dma;
- DMAVoidFunc *pio_transfer;
- DMAInt32Func *prepare_buf;
- DMAu32Func *commit_buf;
- DMAIntFunc *rw_buf;
- DMAVoidFunc *restart;
- DMAVoidFunc *restart_dma;
- DMAStopFunc *set_inactive;
- DMAVoidFunc *cmd_done;
- DMAVoidFunc *reset;
-};
-
-struct IDEDMA {
- const struct IDEDMAOps *ops;
- QEMUIOVector qiov;
- BlockAIOCB *aiocb;
-};
-
/* These are used for the error_status field of IDEBus */
#define IDE_RETRY_MASK 0xf8
#define IDE_RETRY_DMA 0x08
--
2.43.2
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH 5/7] hw/ide: Move IDE DMA related definitions to a separate header ide-dma.h
2024-02-19 10:49 ` [PATCH 5/7] hw/ide: Move IDE DMA related definitions to a separate header ide-dma.h Thomas Huth
@ 2024-02-19 11:53 ` BALATON Zoltan
2024-02-19 19:49 ` Thomas Huth
0 siblings, 1 reply; 16+ messages in thread
From: BALATON Zoltan @ 2024-02-19 11:53 UTC (permalink / raw)
To: Thomas Huth
Cc: John Snow, qemu-devel, Philippe Mathieu-Daudé, qemu-block
On Mon, 19 Feb 2024, Thomas Huth wrote:
> These definitions are required outside of the hw/ide/ code, too,
> so lets's move them from internal.h to a new header called ide-dma.h.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
> include/hw/ide/ide-dma.h | 30 ++++++++++++++++++++++++++++++
> include/hw/ide/internal.h | 27 +--------------------------
> 2 files changed, 31 insertions(+), 26 deletions(-)
> create mode 100644 include/hw/ide/ide-dma.h
>
> diff --git a/include/hw/ide/ide-dma.h b/include/hw/ide/ide-dma.h
> new file mode 100644
> index 0000000000..fb82966bdd
> --- /dev/null
> +++ b/include/hw/ide/ide-dma.h
> @@ -0,0 +1,30 @@
> +#ifndef HW_IDE_DMA_H
> +#define HW_IDE_DMA_H
> +
> +typedef void DMAStartFunc(const IDEDMA *, IDEState *, BlockCompletionFunc *);
> +typedef void DMAVoidFunc(const IDEDMA *);
> +typedef int DMAIntFunc(const IDEDMA *, bool);
> +typedef int32_t DMAInt32Func(const IDEDMA *, int32_t len);
> +typedef void DMAu32Func(const IDEDMA *, uint32_t);
> +typedef void DMAStopFunc(const IDEDMA *, bool);
> +
> +struct IDEDMAOps {
> + DMAStartFunc *start_dma;
> + DMAVoidFunc *pio_transfer;
> + DMAInt32Func *prepare_buf;
> + DMAu32Func *commit_buf;
> + DMAIntFunc *rw_buf;
> + DMAVoidFunc *restart;
> + DMAVoidFunc *restart_dma;
> + DMAStopFunc *set_inactive;
> + DMAVoidFunc *cmd_done;
> + DMAVoidFunc *reset;
> +};
> +
> +struct IDEDMA {
> + const struct IDEDMAOps *ops;
> + QEMUIOVector qiov;
> + BlockAIOCB *aiocb;
Doesn't this need to #include something to define QEMUIOVector and
BlockAIOCB and some of the DMA and IDE types not defined above?
Regards,
BALATON Zoltan
> +};
> +
> +#endif
> diff --git a/include/hw/ide/internal.h b/include/hw/ide/internal.h
> index 642bd1a979..d1d3fcd23a 100644
> --- a/include/hw/ide/internal.h
> +++ b/include/hw/ide/internal.h
> @@ -9,6 +9,7 @@
>
> #include "hw/ide.h"
> #include "hw/ide/ide-bus.h"
> +#include "hw/ide/ide-dma.h"
>
> /* debug IDE devices */
> #define USE_DMA_CDROM
> @@ -316,13 +317,6 @@
> #define SMART_DISABLE 0xd9
> #define SMART_STATUS 0xda
>
> -typedef void DMAStartFunc(const IDEDMA *, IDEState *, BlockCompletionFunc *);
> -typedef void DMAVoidFunc(const IDEDMA *);
> -typedef int DMAIntFunc(const IDEDMA *, bool);
> -typedef int32_t DMAInt32Func(const IDEDMA *, int32_t len);
> -typedef void DMAu32Func(const IDEDMA *, uint32_t);
> -typedef void DMAStopFunc(const IDEDMA *, bool);
> -
> extern const char *IDE_DMA_CMD_lookup[IDE_DMA__COUNT];
>
> extern const MemoryRegionPortio ide_portio_list[];
> @@ -340,25 +334,6 @@ typedef struct IDEBufferedRequest {
> bool orphaned;
> } IDEBufferedRequest;
>
> -struct IDEDMAOps {
> - DMAStartFunc *start_dma;
> - DMAVoidFunc *pio_transfer;
> - DMAInt32Func *prepare_buf;
> - DMAu32Func *commit_buf;
> - DMAIntFunc *rw_buf;
> - DMAVoidFunc *restart;
> - DMAVoidFunc *restart_dma;
> - DMAStopFunc *set_inactive;
> - DMAVoidFunc *cmd_done;
> - DMAVoidFunc *reset;
> -};
> -
> -struct IDEDMA {
> - const struct IDEDMAOps *ops;
> - QEMUIOVector qiov;
> - BlockAIOCB *aiocb;
> -};
> -
> /* These are used for the error_status field of IDEBus */
> #define IDE_RETRY_MASK 0xf8
> #define IDE_RETRY_DMA 0x08
>
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH 5/7] hw/ide: Move IDE DMA related definitions to a separate header ide-dma.h
2024-02-19 11:53 ` BALATON Zoltan
@ 2024-02-19 19:49 ` Thomas Huth
0 siblings, 0 replies; 16+ messages in thread
From: Thomas Huth @ 2024-02-19 19:49 UTC (permalink / raw)
To: BALATON Zoltan
Cc: John Snow, qemu-devel, Philippe Mathieu-Daudé, qemu-block
On 19/02/2024 12.53, BALATON Zoltan wrote:
> On Mon, 19 Feb 2024, Thomas Huth wrote:
>> These definitions are required outside of the hw/ide/ code, too,
>> so lets's move them from internal.h to a new header called ide-dma.h.
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>> include/hw/ide/ide-dma.h | 30 ++++++++++++++++++++++++++++++
>> include/hw/ide/internal.h | 27 +--------------------------
>> 2 files changed, 31 insertions(+), 26 deletions(-)
>> create mode 100644 include/hw/ide/ide-dma.h
>>
>> diff --git a/include/hw/ide/ide-dma.h b/include/hw/ide/ide-dma.h
>> new file mode 100644
>> index 0000000000..fb82966bdd
>> --- /dev/null
>> +++ b/include/hw/ide/ide-dma.h
>> @@ -0,0 +1,30 @@
>> +#ifndef HW_IDE_DMA_H
>> +#define HW_IDE_DMA_H
>> +
>> +typedef void DMAStartFunc(const IDEDMA *, IDEState *, BlockCompletionFunc
>> *);
>> +typedef void DMAVoidFunc(const IDEDMA *);
>> +typedef int DMAIntFunc(const IDEDMA *, bool);
>> +typedef int32_t DMAInt32Func(const IDEDMA *, int32_t len);
>> +typedef void DMAu32Func(const IDEDMA *, uint32_t);
>> +typedef void DMAStopFunc(const IDEDMA *, bool);
>> +
>> +struct IDEDMAOps {
>> + DMAStartFunc *start_dma;
>> + DMAVoidFunc *pio_transfer;
>> + DMAInt32Func *prepare_buf;
>> + DMAu32Func *commit_buf;
>> + DMAIntFunc *rw_buf;
>> + DMAVoidFunc *restart;
>> + DMAVoidFunc *restart_dma;
>> + DMAStopFunc *set_inactive;
>> + DMAVoidFunc *cmd_done;
>> + DMAVoidFunc *reset;
>> +};
>> +
>> +struct IDEDMA {
>> + const struct IDEDMAOps *ops;
>> + QEMUIOVector qiov;
>> + BlockAIOCB *aiocb;
>
> Doesn't this need to #include something to define QEMUIOVector and
> BlockAIOCB and some of the DMA and IDE types not defined above?
Yes, it currently works by accident since the header is only included in
spots where those things are already defined, but I'll better add some
#include statements here so that this header can also be used stand-alone in
other spots.
Thomas
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 6/7] hw/ide: Remove the include/hw/ide.h legacy file
2024-02-19 10:49 [PATCH 0/7] hw/ide: Clean up hw/ide/qdev.c and include/hw/ide/internal.h Thomas Huth
` (4 preceding siblings ...)
2024-02-19 10:49 ` [PATCH 5/7] hw/ide: Move IDE DMA related definitions to a separate header ide-dma.h Thomas Huth
@ 2024-02-19 10:49 ` Thomas Huth
2024-02-19 10:49 ` [PATCH 7/7] hw/ide: Stop exposing internal.h to non-IDE files Thomas Huth
2024-02-19 11:32 ` [PATCH 0/7] hw/ide: Clean up hw/ide/qdev.c and include/hw/ide/internal.h Philippe Mathieu-Daudé
7 siblings, 0 replies; 16+ messages in thread
From: Thomas Huth @ 2024-02-19 10:49 UTC (permalink / raw)
To: John Snow, qemu-devel
Cc: BALATON Zoltan, Philippe Mathieu-Daudé, qemu-block
There was only one prototype left in this legacy file. Move it to
ide-dev.h to finally get rid of it.
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
include/hw/ide.h | 9 ---------
include/hw/ide/ide-dev.h | 2 ++
include/hw/ide/internal.h | 1 -
3 files changed, 2 insertions(+), 10 deletions(-)
delete mode 100644 include/hw/ide.h
diff --git a/include/hw/ide.h b/include/hw/ide.h
deleted file mode 100644
index db963bdb77..0000000000
--- a/include/hw/ide.h
+++ /dev/null
@@ -1,9 +0,0 @@
-#ifndef HW_IDE_H
-#define HW_IDE_H
-
-#include "exec/memory.h"
-
-/* ide/core.c */
-void ide_drive_get(DriveInfo **hd, int max_bus);
-
-#endif /* HW_IDE_H */
diff --git a/include/hw/ide/ide-dev.h b/include/hw/ide/ide-dev.h
index de88784a25..ad55997442 100644
--- a/include/hw/ide/ide-dev.h
+++ b/include/hw/ide/ide-dev.h
@@ -181,4 +181,6 @@ typedef struct IDEDrive {
void ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind, Error **errp);
+void ide_drive_get(DriveInfo **hd, int max_bus);
+
#endif
diff --git a/include/hw/ide/internal.h b/include/hw/ide/internal.h
index d1d3fcd23a..0fc2013374 100644
--- a/include/hw/ide/internal.h
+++ b/include/hw/ide/internal.h
@@ -7,7 +7,6 @@
* non-internal declarations are in hw/ide.h
*/
-#include "hw/ide.h"
#include "hw/ide/ide-bus.h"
#include "hw/ide/ide-dma.h"
--
2.43.2
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH 7/7] hw/ide: Stop exposing internal.h to non-IDE files
2024-02-19 10:49 [PATCH 0/7] hw/ide: Clean up hw/ide/qdev.c and include/hw/ide/internal.h Thomas Huth
` (5 preceding siblings ...)
2024-02-19 10:49 ` [PATCH 6/7] hw/ide: Remove the include/hw/ide.h legacy file Thomas Huth
@ 2024-02-19 10:49 ` Thomas Huth
2024-02-19 11:32 ` [PATCH 0/7] hw/ide: Clean up hw/ide/qdev.c and include/hw/ide/internal.h Philippe Mathieu-Daudé
7 siblings, 0 replies; 16+ messages in thread
From: Thomas Huth @ 2024-02-19 10:49 UTC (permalink / raw)
To: John Snow, qemu-devel
Cc: BALATON Zoltan, Philippe Mathieu-Daudé, qemu-block
include/hw/ide/internal.h is currently included by include/hw/ide/pci.h
and thus exposed to a lot of files that are not part of the IDE subsystem.
Stop including internal.h there and use the appropriate new headers
ide-bus.h and ide-dma.h instead.
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
include/hw/ide/pci.h | 3 ++-
hw/i386/pc.c | 2 +-
hw/ide/cmd646.c | 1 +
hw/ide/pci.c | 1 +
hw/ide/piix.c | 1 +
hw/ide/sii3112.c | 1 +
hw/ide/via.c | 1 +
7 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/include/hw/ide/pci.h b/include/hw/ide/pci.h
index a814a0a7c3..e1e012c387 100644
--- a/include/hw/ide/pci.h
+++ b/include/hw/ide/pci.h
@@ -1,7 +1,8 @@
#ifndef HW_IDE_PCI_H
#define HW_IDE_PCI_H
-#include "hw/ide/internal.h"
+#include "hw/ide/ide-bus.h"
+#include "hw/ide/ide-dma.h"
#include "hw/pci/pci_device.h"
#include "qom/object.h"
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 196827531a..22d0c29575 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -31,7 +31,7 @@
#include "hw/i386/fw_cfg.h"
#include "hw/i386/vmport.h"
#include "sysemu/cpus.h"
-#include "hw/ide/internal.h"
+#include "hw/ide/ide-bus.h"
#include "hw/timer/hpet.h"
#include "hw/loader.h"
#include "hw/rtc/mc146818rtc.h"
diff --git a/hw/ide/cmd646.c b/hw/ide/cmd646.c
index c0bcfa4414..23d213ff01 100644
--- a/hw/ide/cmd646.c
+++ b/hw/ide/cmd646.c
@@ -33,6 +33,7 @@
#include "sysemu/reset.h"
#include "hw/ide/pci.h"
+#include "hw/ide/internal.h"
#include "trace.h"
/* CMD646 specific */
diff --git a/hw/ide/pci.c b/hw/ide/pci.c
index ca85d8474c..73efeec7f4 100644
--- a/hw/ide/pci.c
+++ b/hw/ide/pci.c
@@ -30,6 +30,7 @@
#include "sysemu/dma.h"
#include "qemu/error-report.h"
#include "qemu/module.h"
+#include "hw/ide/internal.h"
#include "hw/ide/pci.h"
#include "trace.h"
diff --git a/hw/ide/piix.c b/hw/ide/piix.c
index 4e5e12935f..1773a068c3 100644
--- a/hw/ide/piix.c
+++ b/hw/ide/piix.c
@@ -30,6 +30,7 @@
#include "qemu/osdep.h"
#include "qapi/error.h"
#include "hw/pci/pci.h"
+#include "hw/ide/internal.h"
#include "hw/ide/piix.h"
#include "hw/ide/pci.h"
#include "trace.h"
diff --git a/hw/ide/sii3112.c b/hw/ide/sii3112.c
index 63dc4a0494..321b9e46a1 100644
--- a/hw/ide/sii3112.c
+++ b/hw/ide/sii3112.c
@@ -13,6 +13,7 @@
*/
#include "qemu/osdep.h"
+#include "hw/ide/internal.h"
#include "hw/ide/pci.h"
#include "qemu/module.h"
#include "trace.h"
diff --git a/hw/ide/via.c b/hw/ide/via.c
index 3f3c484253..cf151e70ec 100644
--- a/hw/ide/via.c
+++ b/hw/ide/via.c
@@ -25,6 +25,7 @@
*/
#include "qemu/osdep.h"
+#include "hw/ide/internal.h"
#include "hw/pci/pci.h"
#include "migration/vmstate.h"
#include "qemu/module.h"
--
2.43.2
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH 0/7] hw/ide: Clean up hw/ide/qdev.c and include/hw/ide/internal.h
2024-02-19 10:49 [PATCH 0/7] hw/ide: Clean up hw/ide/qdev.c and include/hw/ide/internal.h Thomas Huth
` (6 preceding siblings ...)
2024-02-19 10:49 ` [PATCH 7/7] hw/ide: Stop exposing internal.h to non-IDE files Thomas Huth
@ 2024-02-19 11:32 ` Philippe Mathieu-Daudé
7 siblings, 0 replies; 16+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-02-19 11:32 UTC (permalink / raw)
To: Thomas Huth, John Snow, qemu-devel; +Cc: BALATON Zoltan, qemu-block
On 19/2/24 11:49, Thomas Huth wrote:
> While trying to make it possible to compile-out the CompactFlash IDE device
> in downstream distributions (first patch), we noticed that there are more
> things in the IDE code that could use a proper clean up:
>
> First, hw/ide/qdev.c is quite a mix between IDE BUS specific functions
> and (disk) device specific functions. Thus the second patch splits qdev.c
> into two new separate files to make it more obvious which part belongs
> to which kind of devices.
>
> The remaining patches unentangle include/hw/ide/internal.h, which is meant
> as a header that should only be used internally to the IDE subsystem, but
> which is currently exposed to the world since include/hw/ide/pci.h includes
> this header, too. Thus we move the definitions that are also required for
> non-IDE code to other new header files, so we can finally change pci.h to
> stop including internal.h. After these changes, internal.h is only included
> by files in hw/ide/ as it should be.
>
> Thomas Huth (7):
> hw/ide: Add the possibility to disable the CompactFlash device in the
> build
> hw/ide: Split qdev.c into ide-bus.c and ide-dev.c
> hw/ide: Move IDE device related definitions to ide-dev.h
Modulo comments in "hw/ide/ide-dev.h", series:
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> hw/ide: Move IDE bus related definitions to a new header ide-bus.h
> hw/ide: Move IDE DMA related definitions to a separate header
> ide-dma.h
> hw/ide: Remove the include/hw/ide.h legacy file
> hw/ide: Stop exposing internal.h to non-IDE files
^ permalink raw reply [flat|nested] 16+ messages in thread