diff mbox series

[3/7] hw/ide: Move IDE device related definitions to ide-dev.h

Message ID 20240219104912.378211-4-thuth@redhat.com (mailing list archive)
State New, archived
Headers show
Series hw/ide: Clean up hw/ide/qdev.c and include/hw/ide/internal.h | expand

Commit Message

Thomas Huth Feb. 19, 2024, 10:49 a.m. UTC
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(-)

Comments

Philippe Mathieu-Daudé Feb. 19, 2024, 11:32 a.m. UTC | #1
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.
Thomas Huth Feb. 19, 2024, 7:17 p.m. UTC | #2
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
Philippe Mathieu-Daudé Feb. 20, 2024, 7:18 a.m. UTC | #3
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
> 
>
diff mbox series

Patch

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"