[ 9.347342] scsi host0: pata_cmd64x
[ 9.369055] scsi host1: pata_cmd64x
[ 9.371622] ata1: PATA max UDMA/33 cmd 0x1fe02008000 ctl 0x1fe02008080 bmdma
0x1fe02008200 irq 8
[ 9.372805] ata2: PATA max UDMA/33 cmd 0x1fe02008100 ctl 0x1fe02008180 bmdma
0x1fe02008208 irq 8
[ 9.711740] ata2.00: ATAPI: QEMU DVD-ROM, 2.5+, max UDMA/100
[ 9.712591] ata2.00: Drive reports diagnostics failure. This may indicate a
drive
[ 9.713256] ata2.00: fault or invalid emulation. Contact drive vendor for
information.
[ 9.737677] ata2.00: configured for UDMA/33
[ 9.790179] scsi 1:0:0:0: CD-ROM QEMU QEMU DVD-ROM 2.5+ PQ: 0
ANSI: 5
[ 10.381172] hme 0000:01:01.1 enp1s1f1: renamed from eth0
[ 10.508819] sr 1:0:0:0: [sr0] scsi3-mmc drive: 4x/4x cd/rw xa/form2 tray
[ 10.509805] cdrom: Uniform CD-ROM driver Revision: 3.20
A session with git bisect points to the following commit:
55adb3c45620c31f29978f209e2a44a08d34e2da is the first bad commit
commit 55adb3c45620c31f29978f209e2a44a08d34e2da
Author: John Snow <jsnow@redhat.com>
Date: Fri Jul 24 01:23:00 2020 -0400
ide: cancel pending callbacks on SRST
The SRST implementation did not keep up with the rest of IDE; it is
possible to perform a weak reset on an IDE device to remove the BSY/DRQ
bits, and then issue writes to the control/device registers which can
cause chaos with the state machine.
Fix that by actually performing a real reset.
Reported-by: Alexander Bulekov <alxndr@bu.edu>
Fixes: https://bugs.launchpad.net/qemu/+bug/1878253
Fixes: https://bugs.launchpad.net/qemu/+bug/1887303
Fixes: https://bugs.launchpad.net/qemu/+bug/1887309
Signed-off-by: John Snow <jsnow@redhat.com>
:040000 040000 70a7c1cfbafb22fa815d3ae4d7ed075ac3918188
3f37762f20e9ca9d2874eaf819d7175a1dca1dd9 M hw
John/Alexander: any chance you could take a look at this? The message above is
really simple to reproduce under qemu-system-sparc64 using
http://cdimage.debian.org/cdimage/ports/9.0/sparc64/iso-cd/debian-9.0-sparc64-NETINST-1.iso
and the following command line:
./qemu-system-sparc64 \
-cdrom debian-9.0-sparc64-NETINST-1.iso \
-m 256 \
-nographic \
-boot d
ATB,
Mark.
Shucks.
This patch happened because the old SRST code reset the IDE state machine (cleared
the status register) but then didn't cancel any of the pending callbacks, so it was
possible to shuffle the state machine off the rails onto junk data. Obviously bad.
Now, SRST actually cancels the callbacks which I thought would have been safe, but
it's possible that doing a "real" reset here is touching more registers than it ought
to.
Let's take a look at the linux source code ...
/* Let the user know. We don't want to disallow opens for
rescue purposes, or in case the vendor is just a blithering
idiot. Do this after the dev_config call as some controllers
with buggy firmware may want to avoid reporting false device
bugs */
Ah, always a nice day to be called an idiot. Thank you for your service, Alan
Cox.
This message gets printed when ATA_HORKAGE_DIAGNOSTIC is set. libata-sff.c suggests
this happens when the error register* comes back 0x00 after an SRST.
(*I think that tf.feature is only feature on writes, but error on reads. Same
address.)
Now, ide_reset -- which we use for initialization and resets both always sets the
error register to 0x00. libata thinks that 0x00 means a failed diagnostics test, though.
This ought to be covered by ATA8-APT. Section 9.2, Software reset protocol.
Cliff notes:
- Host writes to SRST and waits for 5 μs.
- Both devices obey the SRST write, regardless of drive selection.
- Each device clears their registers and sets BSY (within 400ns.)
- Host clears SRST and waits for at least 2ms.
- Host polls devices, waiting for BSY to be 0.
device0 can set bit7 in the error register to 0 or 1, depending on the presence or
absence of device1 and how it behaves following a diagnostic test.
Device 1 is absent: bit7 is cleared.
Device 1 is present:
- If PDIAG- is asserted, bit7 is cleared.
- If PDIAG- is not asserted within 31 seconds, bit7 is set.
Then, ah:
The EXECUTE DEVICE DIAGNOSTICS diagnostic code shall be placed in bits (6:0) of the
Error register (See Clause 0). The device shall set the signature values (See Clause
0). The content of the Features register is undefined.
I got this pretty wrong. I'm seeing a few problems:
1. I thought SRST was triggered on the falling edge, but that's not entirely true.
BSY should be set immediately and the SRST can begin as soon as possible. The device
does not seem to have any interaction with the SRST bit being cleared from what I can
tell.
2. We aren't running the diagnostic command, actually. That should fix this
particular case. The old version of the code had an open-coded version of this, but
it wasn't clear at the time this is what it was doing.
3. There are likely other things to handle relating to the presence/absence of
device1 that we have never done for either version of the code.
Try this patch:
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 693b352d5e..98cea7ad45 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -2254,10 +2254,8 @@ static void ide_perform_srst(IDEState *s)
/* Cancel PIO callback, reset registers/signature, etc */
ide_reset(s);
- if (s->drive_kind == IDE_CD) {
- /* ATAPI drives do not set READY or SEEK */
- s->status = 0x00;
- }
+ /* perform diagnostic */
+ cmd_exec_dev_diagnostic(s, WIN_DIAGNOSE);
}
static void ide_bus_perform_srst(void *opaque)
@@ -2282,9 +2280,7 @@ void ide_ctrl_write(void *opaque, uint32_t addr, uint32_t
val)
/* Device0 and Device1 each have their own control register,
* but QEMU models it as just one register in the controller. */
- if ((bus->cmd & IDE_CTRL_RESET) &&
- !(val & IDE_CTRL_RESET)) {
- /* SRST triggers on falling edge */
+ if (!(bus->cmd & IDE_CTRL_RESET) && (val & IDE_CTRL_RESET)) {
for (i = 0; i < 2; i++) {
s = &bus->ifs[i];
s->status |= BUSY_STAT;