Hi Denis,
On 17/8/26 22:52, Denis V. Lunev wrote:
From: Denis V. Lunev <[email protected]>
A power on or hardware reset returns the device parameters to their
power-on defaults (ATA-5 9.1). A software reset keeps them unless the
guest asked with SET FEATURES 0xCC for the next reset to revert (ATA-5 9.2
and 8.16.6). ide_reset() applied the second rule to every reset, so a
translation a guest selected outlived the reset of the machine it selected
it on, and the guest that came up next addressed the disk through a
geometry it never asked for.
Neither ide_reset() nor, for AHCI, ide_bus_reset() could tell the two
apart: a guest clearing SRST in the second host to device FIS of the
software reset protocol lands in the same ahci_reset_port() as a COMRESET
or a reset of the host adapter. Pass the kind down from the callers, which
do know.
ide_drive_pre_load() stays necessary: it restores the same fields, but a
vmstate cannot depend on its device having been reset first.
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/ahci.c | 12 ++++++------
hw/ide/cmd646.c | 2 +-
hw/ide/core.c | 18 +++++++++---------
hw/ide/ide-internal.h | 7 ++++++-
hw/ide/isa.c | 2 +-
hw/ide/macio.c | 2 +-
hw/ide/mmio.c | 2 +-
hw/ide/piix.c | 2 +-
hw/ide/sii3112.c | 6 +++---
hw/ide/via.c | 2 +-
10 files changed, 30 insertions(+), 25 deletions(-)
diff --git a/hw/ide/core.c b/hw/ide/core.c
index c330508128..bd155f8d3a 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -1347,7 +1347,7 @@ void ide_ioport_write(void *opaque, uint32_t addr,
uint32_t val)
}
}
-static void ide_reset(IDEState *s)
+static void ide_reset(IDEState *s, IDEResetKind kind)
{
trace_ide_reset(s);
@@ -1356,7 +1356,7 @@ static void ide_reset(IDEState *s)
s->pio_aiocb = NULL;
}
- if (s->reset_reverts) {
+ if (kind == IDE_RESET_HARDWARE || s->reset_reverts) {
s->reset_reverts = false;
s->heads = s->drive_heads;
s->sectors = s->drive_sectors;
The change is good, so:
Reviewed-by: Philippe Mathieu-Daudé <[email protected]>
Still I think it could be logically better to rework ide_reset() and
use IDEResetKind as a IDEResetLevel instead, having IDE_RESET_HARDWARE
the higher level. Anyway just thinking about it, feel free to ignore me.
diff --git a/hw/ide/ide-internal.h b/hw/ide/ide-internal.h
index 281d07c9d5..094772209d 100644
--- a/hw/ide/ide-internal.h
+++ b/hw/ide/ide-internal.h
@@ -393,7 +393,12 @@ extern const VMStateDescription vmstate_ide_drive;
#define VMSTATE_IDE_DRIVE(_field, _state) \
VMSTATE_STRUCT(_field, _state, 1, vmstate_ide_drive, IDEState)
-void ide_bus_reset(IDEBus *bus);
+typedef enum {
+ IDE_RESET_HARDWARE, /* power on, hardware reset or COMRESET, ATA-5 9.1 */
+ IDE_RESET_SOFTWARE, /* SRST or DEVICE RESET, ATA-5 9.2 */
+} IDEResetKind;
