* [PATCH v2 1/7] hw/ide: Add the possibility to disable the CompactFlash device in the build
2024-02-20 8:54 [PATCH v2 0/7] hw/ide: Clean up hw/ide/qdev.c and include/hw/ide/internal.h Thomas Huth
@ 2024-02-20 8:54 ` Thomas Huth
2024-02-20 8:55 ` [PATCH v2 2/7] hw/ide: Split qdev.c into ide-bus.c and ide-dev.c Thomas Huth
` (6 subsequent siblings)
7 siblings, 0 replies; 14+ messages in thread
From: Thomas Huth @ 2024-02-20 8:54 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.
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
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] 14+ messages in thread* [PATCH v2 2/7] hw/ide: Split qdev.c into ide-bus.c and ide-dev.c
2024-02-20 8:54 [PATCH v2 0/7] hw/ide: Clean up hw/ide/qdev.c and include/hw/ide/internal.h Thomas Huth
2024-02-20 8:54 ` [PATCH v2 1/7] hw/ide: Add the possibility to disable the CompactFlash device in the build Thomas Huth
@ 2024-02-20 8:55 ` Thomas Huth
2024-02-20 8:55 ` [PATCH v2 3/7] hw/ide: Move IDE DMA related definitions to a separate header ide-dma.h Thomas Huth
` (5 subsequent siblings)
7 siblings, 0 replies; 14+ messages in thread
From: Thomas Huth @ 2024-02-20 8:55 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.
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
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] 14+ messages in thread* [PATCH v2 3/7] hw/ide: Move IDE DMA related definitions to a separate header ide-dma.h
2024-02-20 8:54 [PATCH v2 0/7] hw/ide: Clean up hw/ide/qdev.c and include/hw/ide/internal.h Thomas Huth
2024-02-20 8:54 ` [PATCH v2 1/7] hw/ide: Add the possibility to disable the CompactFlash device in the build Thomas Huth
2024-02-20 8:55 ` [PATCH v2 2/7] hw/ide: Split qdev.c into ide-bus.c and ide-dev.c Thomas Huth
@ 2024-02-20 8:55 ` Thomas Huth
2024-02-21 18:37 ` Philippe Mathieu-Daudé
2024-02-20 8:55 ` [PATCH v2 4/7] hw/ide: Move IDE device related definitions to ide-dev.h Thomas Huth
` (4 subsequent siblings)
7 siblings, 1 reply; 14+ messages in thread
From: Thomas Huth @ 2024-02-20 8:55 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 | 37 +++++++++++++++++++++++++++++++++++++
include/hw/ide/internal.h | 29 +----------------------------
2 files changed, 38 insertions(+), 28 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..3ac4c3c9fa
--- /dev/null
+++ b/include/hw/ide/ide-dma.h
@@ -0,0 +1,37 @@
+#ifndef HW_IDE_DMA_H
+#define HW_IDE_DMA_H
+
+#include "block/aio.h"
+#include "qemu/iov.h"
+
+typedef struct IDEState IDEState;
+typedef struct IDEDMAOps IDEDMAOps;
+typedef struct IDEDMA IDEDMA;
+
+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 3bdcc75597..a3a6702eec 100644
--- a/include/hw/ide/internal.h
+++ b/include/hw/ide/internal.h
@@ -11,6 +11,7 @@
#include "sysemu/dma.h"
#include "hw/block/block.h"
#include "exec/ioport.h"
+#include "hw/ide/ide-dma.h"
/* debug IDE devices */
#define USE_DMA_CDROM
@@ -18,8 +19,6 @@
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)
@@ -332,13 +331,6 @@ 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);
-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;
@@ -460,25 +452,6 @@ struct IDEState {
int ncq_queues;
};
-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;
-};
-
struct IDEBus {
BusState qbus;
IDEDevice *master;
--
2.43.2
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v2 3/7] hw/ide: Move IDE DMA related definitions to a separate header ide-dma.h
2024-02-20 8:55 ` [PATCH v2 3/7] hw/ide: Move IDE DMA related definitions to a separate header ide-dma.h Thomas Huth
@ 2024-02-21 18:37 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 14+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-02-21 18:37 UTC (permalink / raw)
To: Thomas Huth, John Snow, qemu-devel; +Cc: BALATON Zoltan, qemu-block
On 20/2/24 09:55, 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 | 37 +++++++++++++++++++++++++++++++++++++
> include/hw/ide/internal.h | 29 +----------------------------
> 2 files changed, 38 insertions(+), 28 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..3ac4c3c9fa
> --- /dev/null
> +++ b/include/hw/ide/ide-dma.h
> @@ -0,0 +1,37 @@
> +#ifndef HW_IDE_DMA_H
> +#define HW_IDE_DMA_H
> +
> +#include "block/aio.h"
> +#include "qemu/iov.h"
> +
> +typedef struct IDEState IDEState;
> +typedef struct IDEDMAOps IDEDMAOps;
> +typedef struct IDEDMA IDEDMA;
> +
> +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;
While here, let's use the typedef, otherwise:
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> + QEMUIOVector qiov;
> + BlockAIOCB *aiocb;
> +};
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2 4/7] hw/ide: Move IDE device related definitions to ide-dev.h
2024-02-20 8:54 [PATCH v2 0/7] hw/ide: Clean up hw/ide/qdev.c and include/hw/ide/internal.h Thomas Huth
` (2 preceding siblings ...)
2024-02-20 8:55 ` [PATCH v2 3/7] hw/ide: Move IDE DMA related definitions to a separate header ide-dma.h Thomas Huth
@ 2024-02-20 8:55 ` Thomas Huth
2024-02-21 18:43 ` Philippe Mathieu-Daudé
2024-02-20 8:55 ` [PATCH v2 5/7] hw/ide: Move IDE bus related definitions to a new header ide-bus.h Thomas Huth
` (3 subsequent siblings)
7 siblings, 1 reply; 14+ messages in thread
From: Thomas Huth @ 2024-02-20 8:55 UTC (permalink / raw)
To: John Snow, qemu-devel
Cc: BALATON Zoltan, Philippe Mathieu-Daudé, qemu-block
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 | 143 +++++++++++++++++++++++++++++++++++++-
include/hw/ide/internal.h | 143 +-------------------------------------
hw/ide/ide-dev.c | 1 +
3 files changed, 144 insertions(+), 143 deletions(-)
diff --git a/include/hw/ide/ide-dev.h b/include/hw/ide/ide-dev.h
index 7e9663cda9..1f62e58ebc 100644
--- a/include/hw/ide/ide-dev.h
+++ b/include/hw/ide/ide-dev.h
@@ -20,9 +20,150 @@
#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 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 a3a6702eec..1aab89b27b 100644
--- a/include/hw/ide/internal.h
+++ b/include/hw/ide/internal.h
@@ -8,23 +8,17 @@
*/
#include "hw/ide.h"
-#include "sysemu/dma.h"
-#include "hw/block/block.h"
#include "exec/ioport.h"
#include "hw/ide/ide-dma.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;
-
#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'.
@@ -327,23 +321,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 *);
-
-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[];
@@ -361,97 +338,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 IDEBus {
BusState qbus;
IDEDevice *master;
@@ -475,33 +361,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] 14+ messages in thread* Re: [PATCH v2 4/7] hw/ide: Move IDE device related definitions to ide-dev.h
2024-02-20 8:55 ` [PATCH v2 4/7] hw/ide: Move IDE device related definitions to ide-dev.h Thomas Huth
@ 2024-02-21 18:43 ` Philippe Mathieu-Daudé
2024-02-21 19:13 ` Thomas Huth
0 siblings, 1 reply; 14+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-02-21 18:43 UTC (permalink / raw)
To: Thomas Huth, John Snow, qemu-devel; +Cc: BALATON Zoltan, qemu-block
On 20/2/24 09:55, Thomas Huth wrote:
> Unentangle internal.h by moving public IDE device related
Disentangle ... from? Untangle? TIL Unentangle.
Anyway I'm not a native English speaker, so:
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> definitions to ide-dev.h.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
> include/hw/ide/ide-dev.h | 143 +++++++++++++++++++++++++++++++++++++-
> include/hw/ide/internal.h | 143 +-------------------------------------
> hw/ide/ide-dev.c | 1 +
> 3 files changed, 144 insertions(+), 143 deletions(-)
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 4/7] hw/ide: Move IDE device related definitions to ide-dev.h
2024-02-21 18:43 ` Philippe Mathieu-Daudé
@ 2024-02-21 19:13 ` Thomas Huth
2024-02-21 20:52 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 14+ messages in thread
From: Thomas Huth @ 2024-02-21 19:13 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, John Snow, qemu-devel
Cc: BALATON Zoltan, qemu-block
On 21/02/2024 19.43, Philippe Mathieu-Daudé wrote:
> On 20/2/24 09:55, Thomas Huth wrote:
>> Unentangle internal.h by moving public IDE device related
>
> Disentangle ... from? Untangle? TIL Unentangle.
You're right, "disentangle" seems to be the more appropriate word.
I'll fix it up.
> Anyway I'm not a native English speaker, so:
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Thanks!
Thomas
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 4/7] hw/ide: Move IDE device related definitions to ide-dev.h
2024-02-21 19:13 ` Thomas Huth
@ 2024-02-21 20:52 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 14+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-02-21 20:52 UTC (permalink / raw)
To: Thomas Huth, John Snow, qemu-devel; +Cc: BALATON Zoltan, qemu-block
On 21/2/24 20:13, Thomas Huth wrote:
> On 21/02/2024 19.43, Philippe Mathieu-Daudé wrote:
>> On 20/2/24 09:55, Thomas Huth wrote:
>>> Unentangle internal.h by moving public IDE device related
>>
>> Disentangle ... from? Untangle? TIL Unentangle.
>
> You're right, "disentangle" seems to be the more appropriate word.
> I'll fix it up.
No need, series queued and fixed!
>> Anyway I'm not a native English speaker, so:
>> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>
> Thanks!
>
> Thomas
>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2 5/7] hw/ide: Move IDE bus related definitions to a new header ide-bus.h
2024-02-20 8:54 [PATCH v2 0/7] hw/ide: Clean up hw/ide/qdev.c and include/hw/ide/internal.h Thomas Huth
` (3 preceding siblings ...)
2024-02-20 8:55 ` [PATCH v2 4/7] hw/ide: Move IDE device related definitions to ide-dev.h Thomas Huth
@ 2024-02-20 8:55 ` Thomas Huth
2024-02-21 18:46 ` Philippe Mathieu-Daudé
2024-02-20 8:55 ` [PATCH v2 6/7] hw/ide: Remove the include/hw/ide.h legacy file Thomas Huth
` (2 subsequent siblings)
7 siblings, 1 reply; 14+ messages in thread
From: Thomas Huth @ 2024-02-20 8:55 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 | 42 +++++++++++++++++++++++++++++++++++++++
include/hw/ide/internal.h | 40 +------------------------------------
2 files changed, 43 insertions(+), 39 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..4841a7dcd6
--- /dev/null
+++ b/include/hw/ide/ide-bus.h
@@ -0,0 +1,42 @@
+#ifndef HW_IDE_BUS_H
+#define HW_IDE_BUS_H
+
+#include "exec/ioport.h"
+#include "hw/ide/ide-dev.h"
+#include "hw/ide/ide-dma.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 1aab89b27b..d3ec16a945 100644
--- a/include/hw/ide/internal.h
+++ b/include/hw/ide/internal.h
@@ -8,16 +8,10 @@
*/
#include "hw/ide.h"
-#include "exec/ioport.h"
-#include "hw/ide/ide-dma.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
@@ -338,29 +332,6 @@ typedef struct IDEBufferedRequest {
bool orphaned;
} IDEBufferedRequest;
-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
@@ -477,15 +448,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] 14+ messages in thread* [PATCH v2 6/7] hw/ide: Remove the include/hw/ide.h legacy file
2024-02-20 8:54 [PATCH v2 0/7] hw/ide: Clean up hw/ide/qdev.c and include/hw/ide/internal.h Thomas Huth
` (4 preceding siblings ...)
2024-02-20 8:55 ` [PATCH v2 5/7] hw/ide: Move IDE bus related definitions to a new header ide-bus.h Thomas Huth
@ 2024-02-20 8:55 ` Thomas Huth
2024-02-20 8:55 ` [PATCH v2 7/7] hw/ide: Stop exposing internal.h to non-IDE files Thomas Huth
2024-02-21 11:54 ` [PATCH v2 0/7] hw/ide: Clean up hw/ide/qdev.c and include/hw/ide/internal.h Mark Cave-Ayland
7 siblings, 0 replies; 14+ messages in thread
From: Thomas Huth @ 2024-02-20 8:55 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.
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
MAINTAINERS | 1 -
include/hw/ide.h | 9 ---------
include/hw/ide/ide-dev.h | 2 ++
include/hw/ide/internal.h | 3 +--
4 files changed, 3 insertions(+), 12 deletions(-)
delete mode 100644 include/hw/ide.h
diff --git a/MAINTAINERS b/MAINTAINERS
index 7d61fb9319..f3cbfb8818 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1936,7 +1936,6 @@ IDE
M: John Snow <jsnow@redhat.com>
L: qemu-block@nongnu.org
S: Odd Fixes
-F: include/hw/ide.h
F: include/hw/ide/
F: hw/ide/
F: hw/block/block.c
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 1f62e58ebc..708cc0fda3 100644
--- a/include/hw/ide/ide-dev.h
+++ b/include/hw/ide/ide-dev.h
@@ -179,4 +179,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 d3ec16a945..20dde37f45 100644
--- a/include/hw/ide/internal.h
+++ b/include/hw/ide/internal.h
@@ -4,10 +4,9 @@
/*
* QEMU IDE Emulation -- internal header file
* only files in hw/ide/ are supposed to include this file.
- * non-internal declarations are in hw/ide.h
+ * non-internal declarations are in hw/include/ide-*.h
*/
-#include "hw/ide.h"
#include "hw/ide/ide-bus.h"
/* debug IDE devices */
--
2.43.2
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v2 7/7] hw/ide: Stop exposing internal.h to non-IDE files
2024-02-20 8:54 [PATCH v2 0/7] hw/ide: Clean up hw/ide/qdev.c and include/hw/ide/internal.h Thomas Huth
` (5 preceding siblings ...)
2024-02-20 8:55 ` [PATCH v2 6/7] hw/ide: Remove the include/hw/ide.h legacy file Thomas Huth
@ 2024-02-20 8:55 ` Thomas Huth
2024-02-21 11:54 ` [PATCH v2 0/7] hw/ide: Clean up hw/ide/qdev.c and include/hw/ide/internal.h Mark Cave-Ayland
7 siblings, 0 replies; 14+ messages in thread
From: Thomas Huth @ 2024-02-20 8:55 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.
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
include/hw/ide/pci.h | 2 +-
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, 7 insertions(+), 2 deletions(-)
diff --git a/include/hw/ide/pci.h b/include/hw/ide/pci.h
index a814a0a7c3..ef03764caa 100644
--- a/include/hw/ide/pci.h
+++ b/include/hw/ide/pci.h
@@ -1,7 +1,7 @@
#ifndef HW_IDE_PCI_H
#define HW_IDE_PCI_H
-#include "hw/ide/internal.h"
+#include "hw/ide/ide-bus.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] 14+ messages in thread* Re: [PATCH v2 0/7] hw/ide: Clean up hw/ide/qdev.c and include/hw/ide/internal.h
2024-02-20 8:54 [PATCH v2 0/7] hw/ide: Clean up hw/ide/qdev.c and include/hw/ide/internal.h Thomas Huth
` (6 preceding siblings ...)
2024-02-20 8:55 ` [PATCH v2 7/7] hw/ide: Stop exposing internal.h to non-IDE files Thomas Huth
@ 2024-02-21 11:54 ` Mark Cave-Ayland
7 siblings, 0 replies; 14+ messages in thread
From: Mark Cave-Ayland @ 2024-02-21 11:54 UTC (permalink / raw)
To: Thomas Huth, John Snow, qemu-devel
Cc: BALATON Zoltan, Philippe Mathieu-Daudé, qemu-block
On 20/02/2024 08:54, 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.
>
> v2:
> - Change the order of the DMA patch and move typedef struct IDEDMAOps
> and typedef struct IDEDMA IDEDMA into ide-dma.h, too
> - Make sure that the headers are self-contained (i.e. #include the
> right other headers)
> - Remove some more remnants of include/hw/ide.h
>
> 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 DMA related definitions to a separate header
> ide-dma.h
> hw/ide: Move IDE device related definitions to ide-dev.h
> hw/ide: Move IDE bus related definitions to a new header ide-bus.h
> hw/ide: Remove the include/hw/ide.h legacy file
> hw/ide: Stop exposing internal.h to non-IDE files
>
> MAINTAINERS | 1 -
> include/hw/ide.h | 9 --
> include/hw/ide/ide-bus.h | 42 +++++++
> include/hw/ide/ide-dev.h | 184 ++++++++++++++++++++++++++++++
> include/hw/ide/ide-dma.h | 37 ++++++
> include/hw/ide/internal.h | 211 +----------------------------------
> include/hw/ide/pci.h | 2 +-
> hw/i386/pc.c | 2 +-
> hw/ide/cf.c | 58 ++++++++++
> hw/ide/cmd646.c | 1 +
> hw/ide/ide-bus.c | 111 ++++++++++++++++++
> hw/ide/{qdev.c => ide-dev.c} | 137 +----------------------
> hw/ide/pci.c | 1 +
> hw/ide/piix.c | 1 +
> hw/ide/sii3112.c | 1 +
> hw/ide/via.c | 1 +
> hw/arm/Kconfig | 2 +
> hw/ide/Kconfig | 32 ++++--
> hw/ide/meson.build | 4 +-
> 19 files changed, 470 insertions(+), 367 deletions(-)
> delete mode 100644 include/hw/ide.h
> create mode 100644 include/hw/ide/ide-bus.h
> create mode 100644 include/hw/ide/ide-dev.h
> create mode 100644 include/hw/ide/ide-dma.h
> create mode 100644 hw/ide/cf.c
> create mode 100644 hw/ide/ide-bus.c
> rename hw/ide/{qdev.c => ide-dev.c} (67%)
I've had a quick skim of this series, and it looks like a good tidy-up to me so:
Acked-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
ATB,
Mark.
^ permalink raw reply [flat|nested] 14+ messages in thread