[ 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;