On 17/8/26 22:52, Denis V. Lunev wrote:
From: Denis V. Lunev <[email protected]>
IDENTIFY DEVICE words 1, 3 and 6 describe the default CHS translation,
and ATA-5 8.16.8 requires INITIALIZE DEVICE PARAMETERS to leave them
alone; the translation in effect is described by words 54 to 56 instead.
Words 3 and 6 were filled from s->heads and s->sectors, which the command
replaces, so a guest that selected a translation of its own was told that
its choice was what the drive came with, and could no longer find out the
default. Word 1 is already right, as no command changes s->cylinders.
Report s->drive_heads and s->drive_sectors, which ide_init_drive() keeps
for exactly this, along with the retired word 4 derived from them. The
CompactFlash data labels those words as the default geometry too, and
INITIALIZE DEVICE PARAMETERS is accepted for CFA drives, so fix both.
Cc: [email protected]
Cc: John Snow <[email protected]>
Cc: Peter Maydell <[email protected]>
Fixes: 176e4961bb33 ("hw/ide/core.c: Implement ATA INITIALIZE_DEVICE_PARAMETERS
command")
Signed-off-by: Denis V. Lunev <[email protected]>
---
hw/ide/core.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 747fa71677..befbab9486 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -138,11 +138,12 @@ static void ide_identify(IDEState *s)
memset(p, 0, sizeof(s->identify_data));
put_le16(p + 0, 0x0040);
+ /* Words 1, 3 and 6 describe the default translation (ATA-5 8.16.8) */
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 + 3, s->drive_heads);
+ put_le16(p + 4, 512 * s->drive_sectors); /* XXX: retired, remove ? */
If we are deliberately keeping this, can we update the XXX comment?
Otherwise LGTM:
Reviewed-by: Philippe Mathieu-Daudé <[email protected]>
put_le16(p + 5, 512); /* XXX: retired, remove ? */
- put_le16(p + 6, s->sectors);
+ put_le16(p + 6, s->drive_sectors);
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 */
@@ -332,8 +333,8 @@ static void ide_cfata_identify(IDEState *s)
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 + 3, s->drive_heads); /* Default heads */
+ put_le16(p + 6, s->drive_sectors); /* Default sectors per track */
/* *(p + 7) := nb_sectors >> 16 -- see ide_cfata_identify_size */
/* *(p + 8) := nb_sectors -- see ide_cfata_identify_size */
padstr((char *)(p + 10), s->drive_serial_str, 20); /* serial number */
