[Qemu-devel] [PATCH v2 5/9] ide: Drop redundant IDEState geometry members

[armbru@redhat.com]

From: Markus Armbruster <armbru@redhat.com>

Members cylinders, heads, sectors, chs_trans are copies of
dev->conf.cyls, dev->conf.heads, dev->conf.secs, dev->chs_trans.
Copies were needed for non-qdevified controllers, which lacked dev.

Note that pci_piix3_xen_ide_unplug() did not clear the copies (it only
cleared the copy of bs).  Begs the question whether stale data could
have been used after unplug.  As far as I can tell, the copies were
used only when the copy of bs was non-null, thus no bug there.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/ide/core.c     | 52 ++++++++++++++++++++++++----------------------------
 hw/ide/internal.h |  5 +----
 hw/ide/qdev.c     | 12 +++++-------
 3 files changed, 30 insertions(+), 39 deletions(-)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index 448dcbd..275df4b 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -89,11 +89,11 @@ static void ide_identify(IDEState *s)
     memset(s->io_buffer, 0, 512);
     p = (uint16_t *)s->io_buffer;
     put_le16(p + 0, 0x0040);
-    put_le16(p + 1, s->cylinders);
-    put_le16(p + 3, s->heads);
-    put_le16(p + 4, 512 * s->sectors); /* XXX: retired, remove ? */
+    put_le16(p + 1, s->dev->conf.cyls);
+    put_le16(p + 3, s->dev->conf.heads);
+    put_le16(p + 4, 512 * s->dev->conf.secs); /* XXX: retired, remove ? */
     put_le16(p + 5, 512); /* XXX: retired, remove ? */
-    put_le16(p + 6, s->sectors);
+    put_le16(p + 6, s->dev->conf.secs);
     padstr((char *)(p + 10), s->drive_serial_str, 20); /* serial number */
     put_le16(p + 20, 3); /* XXX: retired, remove ? */
     put_le16(p + 21, 512); /* cache size in sectors */
@@ -108,10 +108,10 @@ static void ide_identify(IDEState *s)
     put_le16(p + 51, 0x200); /* PIO transfer cycle */
     put_le16(p + 52, 0x200); /* DMA transfer cycle */
     put_le16(p + 53, 1 | (1 << 1) | (1 << 2)); /* words 54-58,64-70,88 are valid */
-    put_le16(p + 54, s->cylinders);
-    put_le16(p + 55, s->heads);
-    put_le16(p + 56, s->sectors);
-    oldsize = s->cylinders * s->heads * s->sectors;
+    put_le16(p + 54, s->dev->conf.cyls);
+    put_le16(p + 55, s->dev->conf.heads);
+    put_le16(p + 56, s->dev->conf.secs);
+    oldsize = s->dev->conf.cyls * s->dev->conf.heads * s->dev->conf.secs;
     put_le16(p + 57, oldsize);
     put_le16(p + 58, oldsize >> 16);
     if (s->mult_sectors)
@@ -249,12 +249,12 @@ static void ide_cfata_identify(IDEState *s)
 
     memset(p, 0, sizeof(s->identify_data));
 
-    cur_sec = s->cylinders * s->heads * s->sectors;
+    cur_sec = s->dev->conf.cyls * s->dev->conf.heads * s->dev->conf.secs;
 
     put_le16(p + 0, 0x848a);			/* CF Storage Card signature */
-    put_le16(p + 1, s->cylinders);		/* Default cylinders */
-    put_le16(p + 3, s->heads);			/* Default heads */
-    put_le16(p + 6, s->sectors);		/* Default sectors per track */
+    put_le16(p + 1, s->dev->conf.cyls);         /* Default cylinders */
+    put_le16(p + 3, s->dev->conf.heads);        /* Default heads */
+    put_le16(p + 6, s->dev->conf.secs);         /* Default sectors per track */
     put_le16(p + 7, s->nb_sectors >> 16);	/* Sectors per card */
     put_le16(p + 8, s->nb_sectors);		/* Sectors per card */
     padstr((char *)(p + 10), s->drive_serial_str, 20); /* serial number */
@@ -270,9 +270,9 @@ static void ide_cfata_identify(IDEState *s)
     put_le16(p + 51, 0x0002);			/* PIO cycle timing mode */
     put_le16(p + 52, 0x0001);			/* DMA cycle timing mode */
     put_le16(p + 53, 0x0003);			/* Translation params valid */
-    put_le16(p + 54, s->cylinders);		/* Current cylinders */
-    put_le16(p + 55, s->heads);			/* Current heads */
-    put_le16(p + 56, s->sectors);		/* Current sectors */
+    put_le16(p + 54, s->dev->conf.cyls);        /* Current cylinders */
+    put_le16(p + 55, s->dev->conf.heads);       /* Current heads */
+    put_le16(p + 56, s->dev->conf.secs);        /* Current sectors */
     put_le16(p + 57, cur_sec);			/* Current capacity */
     put_le16(p + 58, cur_sec >> 16);		/* Current capacity */
     if (s->mult_sectors)			/* Multiple sector setting */
@@ -462,8 +462,10 @@ int64_t ide_get_sector(IDEState *s)
 		((int64_t) s->lcyl << 8) | s->sector;
 	}
     } else {
-        sector_num = ((s->hcyl << 8) | s->lcyl) * s->heads * s->sectors +
-            (s->select & 0x0f) * s->sectors + (s->sector - 1);
+        sector_num =
+            (((s->hcyl << 8) | s->lcyl) * s->dev->conf.heads
+             + (s->select & 0x0f)) * s->dev->conf.secs
+            + (s->sector - 1);
     }
     return sector_num;
 }
@@ -486,12 +488,12 @@ void ide_set_sector(IDEState *s, int64_t sector_num)
 	    s->hob_hcyl = sector_num >> 40;
 	}
     } else {
-        cyl = sector_num / (s->heads * s->sectors);
-        r = sector_num % (s->heads * s->sectors);
+        cyl = sector_num / (s->dev->conf.heads * s->dev->conf.secs);
+        r = sector_num % (s->dev->conf.heads * s->dev->conf.secs);
         s->hcyl = cyl >> 8;
         s->lcyl = cyl;
-        s->select = (s->select & 0xf0) | ((r / s->sectors) & 0x0f);
-        s->sector = (r % s->sectors) + 1;
+        s->select = (s->select & 0xf0) | ((r / s->dev->conf.secs) & 0x0f);
+        s->sector = (r % s->dev->conf.secs) + 1;
     }
 }
 
@@ -2088,9 +2090,7 @@ static const BlockDevOps ide_cd_block_ops = {
 
 int ide_init_drive(IDEState *s, IDEDriveKind kind,
                    const char *version, const char *serial, const char *model,
-                   uint64_t wwn,
-                   uint32_t cylinders, uint32_t heads, uint32_t secs,
-                   int chs_trans)
+                   uint64_t wwn)
 {
     BlockDriverState *bs = s->dev->conf.bs;
     uint64_t nb_sectors;
@@ -2098,10 +2098,6 @@ int ide_init_drive(IDEState *s, IDEDriveKind kind,
     s->drive_kind = kind;
 
     bdrv_get_geometry(bs, &nb_sectors);
-    s->cylinders = cylinders;
-    s->heads = heads;
-    s->sectors = secs;
-    s->chs_trans = chs_trans;
     s->nb_sectors = nb_sectors;
     s->wwn = wwn;
     /* The SMART values should be preserved across power cycles
diff --git a/hw/ide/internal.h b/hw/ide/internal.h
index 8f673f8..7a39d44 100644
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -345,7 +345,6 @@ struct IDEState {
     uint8_t unit;
     /* ide config */
     IDEDriveKind drive_kind;
-    int cylinders, heads, sectors, chs_trans;
     int64_t nb_sectors;
     int mult_sectors;
     int identify_set;
@@ -547,9 +546,7 @@ uint32_t ide_data_readl(void *opaque, uint32_t addr);
 
 int ide_init_drive(IDEState *s, IDEDriveKind kind,
                    const char *version, const char *serial, const char *model,
-                   uint64_t wwn,
-                   uint32_t cylinders, uint32_t heads, uint32_t secs,
-                   int chs_trans);
+                   uint64_t wwn);
 void ide_init2(IDEBus *bus, qemu_irq irq);
 void ide_init_ioport(IDEBus *bus, ISADevice *isa, int iobase, int iobase2);
 
diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index f9a4d31..c233d66 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -124,15 +124,15 @@ int ide_get_geometry(BusState *bus, int unit,
         return -1;
     }
 
-    *cyls = s->cylinders;
-    *heads = s->heads;
-    *secs = s->sectors;
+    *cyls = s->dev->conf.cyls;
+    *heads = s->dev->conf.heads;
+    *secs = s->dev->conf.secs;
     return 0;
 }
 
 int ide_get_bios_chs_trans(BusState *bus, int unit)
 {
-    return DO_UPCAST(IDEBus, qbus, bus)->ifs[unit].chs_trans;
+    return DO_UPCAST(IDEBus, qbus, bus)->ifs[unit].dev->chs_trans;
 }
 
 /* --------------------------------- */
@@ -161,9 +161,7 @@ static int ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind)
     }
 
     if (ide_init_drive(s, kind,
-                       dev->version, dev->serial, dev->model, dev->wwn,
-                       dev->conf.cyls, dev->conf.heads, dev->conf.secs,
-                       dev->chs_trans) < 0) {
+                       dev->version, dev->serial, dev->model, dev->wwn) < 0) {
         return -1;
     }
 
-- 
1.8.1.4