| Message ID | 20200221065015.337915-1-jasper.lowell@bt.com |
|---|---|
| State | New |
| Headers | show |
| Series | hw/ide: Remove status register read side effect | expand |
Patchew URL: https://patchew.org/QEMU/20200221065015.337915-1-jasper.lowell@bt.com/
Hi,
This series failed the docker-quick@centos7 build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.
=== TEST SCRIPT BEGIN ===
#!/bin/bash
make docker-image-centos7 V=1 NETWORK=1
time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1
=== TEST SCRIPT END ===
TEST check-qtest-x86_64: tests/qtest/ide-test
TEST check-unit: tests/test-iov
**
ERROR:/tmp/qemu-test/src/tests/qtest/ide-test.c:294:send_dma_request: assertion failed: (!qtest_get_irq(qts, IDE_PRIMARY_IRQ))
ERROR - Bail out! ERROR:/tmp/qemu-test/src/tests/qtest/ide-test.c:294:send_dma_request: assertion failed: (!qtest_get_irq(qts, IDE_PRIMARY_IRQ))
make: *** [check-qtest-x86_64] Error 1
make: *** Waiting for unfinished jobs....
TEST check-unit: tests/test-bitmap
TEST check-unit: tests/test-aio
---
TEST iotest-qcow2: 283
Failures: 161
Failed 1 of 116 iotests
make: *** [check-tests/check-block.sh] Error 1
Traceback (most recent call last):
File "./tests/docker/docker.py", line 664, in <module>
sys.exit(main())
---
raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=3349986ab83849d5b475dd101bed4f05', '-u', '1003', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=1', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew2/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-gw5__giy/src/docker-src.2020-02-21-02.54.48.27828:/var/tmp/qemu:z,ro', 'qemu:centos7', '/var/tmp/qemu/run', 'test-quick']' returned non-zero exit status 2.
filter=--filter=label=com.qemu.instance.uuid=3349986ab83849d5b475dd101bed4f05
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-gw5__giy/src'
make: *** [docker-run-test-quick@centos7] Error 2
real 13m27.186s
user 0m9.054s
The full log is available at
http://patchew.org/logs/20200221065015.337915-1-jasper.lowell@bt.com/testing.docker-quick@centos7/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
On Fri, 21 Feb 2020, jasper.lowell@bt.com wrote: > The Linux libATA API documentation mentions that on some hardware, > reading the status register has the side effect of clearing the > interrupt condition. When emulating the generic Sun4u machine running > Solaris 10, the Solaris 10 CMD646 driver exits fatally because of this > emulated side effect. This side effect is likely to not exist on real > CMD646 hardware. The chip docs don't mention any side effect, they only say that the DMA IRQ and Error bits in the bus master IDE status reg (bits 2 and 1) are write 1 to clear so this might be OK but this change seems to break something else according to a CI test report from patchew. Unfortunately this does not change my problems with other BMDMA devices on PPC. Regards, BALATON Zoltan > Signed-off-by: Jasper Lowell <jasper.lowell@bt.com> > --- > hw/ide/core.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/hw/ide/core.c b/hw/ide/core.c > index 80000eb766..82fd0632ac 100644 > --- a/hw/ide/core.c > +++ b/hw/ide/core.c > @@ -2210,7 +2210,6 @@ uint32_t ide_ioport_read(void *opaque, uint32_t addr) > } else { > ret = s->status; > } > - qemu_irq_lower(bus->irq); > break; > } > >
> The chip docs don't mention any side effect, they only say that the > DMA > IRQ and Error bits in the bus master IDE status reg (bits 2 and 1) > are > write 1 to clear I haven't found any documentation that mention that side effect either. As you say, it only mentions that you should set the bit to clear. While the side effect of clearing interrupts by reading the status register doesn't appear to be in documentation anywhere (to my knowledge), I do see this side effect referenced in the source code of drivers occasionally. In /drivers/ide/ide-io.c of the Linux kernel: /* * Whack the status register, just in case * we have a leftover pending IRQ. */ (void)hwif->tp_ops->read_status(hwif); Along with: * There's nothing really useful we can do with an unexpected interrupt, * other than reading the status register (to clear it), and logging it. The CMD64x specific code in the Linux kernel does explicitly set the IRQ bits in the bus master IDE status register to clear it. I'm not sure why, so maybe someone else can chime in explaining why Linux sometimes clears interrupts by reading the status register and other times follows the documentation and sets the required bits. The OpenBSD driver also appears to set the bits explicitly. I think the reason why the Solaris 10 driver crashes fatally whereas Linux and OpenBSD ignore the side effect is because when clearing interrupts, Solaris 10 expects the interrupt bit to be set and checks this. Linux and OpenBSD appear to clear it regardless of its state. With the patch, I didn't notice any regression for OpenBSD under Sun4u and there were improvements to the Solaris 10 boot under Sun4u. > Unfortunately > this does not change my problems with other BMDMA devices on PPC. This patch doesn't solve all the problems for Solaris 10. It gets further in the boot process but it is still unable to mount the file system. I suspect that there are more bugs in the IDE/CMD646 emulation. I'm going to continue looking into it. It's still possible we are being affected by the same bugs. Right now, I'm considering that the unresponsive serial console under Sun4u on Solaris 10 is due to not being able to mount the file system and pull the required assets for the installation menu. > this change seems to break > something else according to a CI test report from patchew. The test appears to break here in /tests/qtest/ide-test.c for obvious reasons: /* Reading the status register clears the IRQ */ g_assert(!qtest_get_irq(qts, IDE_PRIMARY_IRQ)); Should the patch I've suggested be correct, this test would need to be updated. This is my first patch attempt for QEMU so I'm not sure what the process is. Should I be submitting a new V2 patch with these changes? I won't have the opportunity to update the patch for a few days but will continue watching the thread for reviews. Thanks, Jasper Lowell. On Fri, 2020-02-21 at 16:43 +0100, BALATON Zoltan wrote: > On Fri, 21 Feb 2020, jasper.lowell@bt.com wrote: > > The Linux libATA API documentation mentions that on some hardware, > > reading the status register has the side effect of clearing the > > interrupt condition. When emulating the generic Sun4u machine > > running > > Solaris 10, the Solaris 10 CMD646 driver exits fatally because of > > this > > emulated side effect. This side effect is likely to not exist on > > real > > CMD646 hardware. > > The chip docs don't mention any side effect, they only say that the > DMA > IRQ and Error bits in the bus master IDE status reg (bits 2 and 1) > are > write 1 to clear so this might be OK but this change seems to break > something else according to a CI test report from patchew. > Unfortunately > this does not change my problems with other BMDMA devices on PPC. > > Regards, > BALATON Zoltan > > > Signed-off-by: Jasper Lowell <jasper.lowell@bt.com> > > --- > > hw/ide/core.c | 1 - > > 1 file changed, 1 deletion(-) > > > > diff --git a/hw/ide/core.c b/hw/ide/core.c > > index 80000eb766..82fd0632ac 100644 > > --- a/hw/ide/core.c > > +++ b/hw/ide/core.c > > @@ -2210,7 +2210,6 @@ uint32_t ide_ioport_read(void *opaque, > > uint32_t addr) > > } else { > > ret = s->status; > > } > > - qemu_irq_lower(bus->irq); > > break; > > } > > > >
On Sat, 22 Feb 2020, jasper.lowell@bt.com wrote: > I think the reason why the Solaris 10 driver crashes fatally whereas > Linux and OpenBSD ignore the side effect is because when clearing > interrupts, Solaris 10 expects the interrupt bit to be set and checks > this. Linux and OpenBSD appear to clear it regardless of its state. I've also found this thread: https://lucky.openbsd.misc.narkive.com/hA6XG7Fu/bus-master-dma-error-missing-interrupt which seems to talk about missing IRQ in UDMA mode similar to our problem and it suggests OpenBSD detects this and downgrades to PIO mode so it would still work. Did you check if this is why it works with OpenBSD or it really uses UDMA mode? > This patch doesn't solve all the problems for Solaris 10. It gets > further in the boot process but it is still unable to mount the file > system. I suspect that there are more bugs in the IDE/CMD646 emulation. > I'm going to continue looking into it. It's still possible we are being > affected by the same bugs. Right now, I'm considering that the > unresponsive serial console under Sun4u on Solaris 10 is due to not > being able to mount the file system and pull the required assets for > the installation menu. This is possible. The hang I get during boot with PPC OSes I've tried is also becuase not being able to read CD drive after switching to UDMA mode. This would suggest bug may be in either common ide code or ide-cd emulation but could as well be in irq routing (in which case it's separate but similar bug in different machine emulations). Is there a way to disable UDMA mode in Solaris to check if it would work better when only using PIO? That might help locate the bug further. In my case I've tested with both on board IDE and adding an sii3112 PCI card emulation, these both use common bmdma code but route IRQs differently. I see some irqs arriving up to the interrupt controller but CPU irq is not raised for some reason so I'm not sure it's a bug in common code or somewhere else. >> this change seems to break >> something else according to a CI test report from patchew. > > The test appears to break here in /tests/qtest/ide-test.c for obvious > reasons: > /* Reading the status register clears the IRQ */ > g_assert(!qtest_get_irq(qts, IDE_PRIMARY_IRQ)); > > Should the patch I've suggested be correct, this test would need to be > updated. This is my first patch attempt for QEMU so I'm not sure what OK, I haven't checked the test just noticed the failure. > the process is. Should I be submitting a new V2 patch with these > changes? I won't have the opportunity to update the patch for a few > days but will continue watching the thread for reviews. I'd suggest to wait a few days to give people a chance to review the patch then submit a v2 with all the requested changes if any. You can submit v2 right away but then if someone finds something you'll need more versions which is OK as well, your decision how many versions you want to submit. Since this patch is only 1 line there's not much people could ask to change about it and v2 could allow CI to run and maybe reveal problems so maybe in this case a v2 with also fixing the test might help to get it reviewed faster. I assume you're aware of the page about patch submission: https://wiki.qemu.org/Contribute/SubmitAPatch Regards, BALATON Zoltan
On Sat, 22 Feb 2020, jasper.lowell@bt.com wrote: > I haven't found any documentation that mention that side effect either. > As you say, it only mentions that you should set the bit to clear. > While the side effect of clearing interrupts by reading the status > register doesn't appear to be in documentation anywhere (to my The PCI bus master IDE spec also only says the status register is read / write clear but does not mention clearing bits on read. I've also traced back the origin of this code in QEMU and looks like it was there forever since commit fc01f7e7f90 when IDE emulation was added. Interesting that only Solaris driver broke because of this so maybe there are hardware implementations which do clear this bit so drivers are prepared to handle that. I think based on at least two docs it would be correct to remove this but wonder what other drivers would this change break. Anyway: Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu> > knowledge), I do see this side effect referenced in the source code of > drivers occasionally. > > In /drivers/ide/ide-io.c of the Linux kernel: > /* > * Whack the status register, just in case > * we have a leftover pending IRQ. > */ > (void)hwif->tp_ops->read_status(hwif); > > Along with: > * There's nothing really useful we can do with an unexpected > interrupt, > * other than reading the status register (to clear it), and > logging it. > > The CMD64x specific code in the Linux kernel does explicitly set the > IRQ bits in the bus master IDE status register to clear it. I'm not > sure why, so maybe someone else can chime in explaining why Linux It seems likely that CMD646 does not clear irq on read but some other controllers probably do. Regards, BALATON Zoltan
On Sat, 22 Feb 2020, jasper.lowell@bt.com wrote: > This patch doesn't solve all the problems for Solaris 10. It gets > further in the boot process but it is still unable to mount the file > system. I suspect that there are more bugs in the IDE/CMD646 emulation. > I'm going to continue looking into it. It's still possible we are being One more idea to check is if the irq of the IDE device is mapped at the same IRQ line and described correctly in the open firmware device tree. You may need info from a real machine for that, I've tried to search for it but could not find any device tree dumps or info on how irqs are mapped in the real hardware. Since Linux can find it it's probably OK but maybe Solaris expects the irq to be mapped somewhere else or gets the info on this from some other place than Linux and that could have a bug? Not sure if this helps but I have no better idea. Regards, BALATON Zoltan
On 21/02/2020 06:50, jasper.lowell@bt.com wrote: > The Linux libATA API documentation mentions that on some hardware, > reading the status register has the side effect of clearing the > interrupt condition. When emulating the generic Sun4u machine running > Solaris 10, the Solaris 10 CMD646 driver exits fatally because of this > emulated side effect. This side effect is likely to not exist on real > CMD646 hardware. > > Signed-off-by: Jasper Lowell <jasper.lowell@bt.com> > --- > hw/ide/core.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/hw/ide/core.c b/hw/ide/core.c > index 80000eb766..82fd0632ac 100644 > --- a/hw/ide/core.c > +++ b/hw/ide/core.c > @@ -2210,7 +2210,6 @@ uint32_t ide_ioport_read(void *opaque, uint32_t addr) > } else { > ret = s->status; > } > - qemu_irq_lower(bus->irq); > break; > } I don't think that this is correct: from memory when I last looked at this, there were 2 IDE status registers: the one from the original specification which clears the IRQ upon read, and another one in subsequent revisions which allows you to read the value without clearing any pending IRQ. My guess would be that changing this would not only cause QEMU to deviate from the specification, but causes problems in other OSs. ATB, Mark.
On Sat, 22 Feb 2020, Mark Cave-Ayland wrote: > On 21/02/2020 06:50, jasper.lowell@bt.com wrote: >> The Linux libATA API documentation mentions that on some hardware, >> reading the status register has the side effect of clearing the >> interrupt condition. When emulating the generic Sun4u machine running >> Solaris 10, the Solaris 10 CMD646 driver exits fatally because of this >> emulated side effect. This side effect is likely to not exist on real >> CMD646 hardware. >> >> Signed-off-by: Jasper Lowell <jasper.lowell@bt.com> >> --- >> hw/ide/core.c | 1 - >> 1 file changed, 1 deletion(-) >> >> diff --git a/hw/ide/core.c b/hw/ide/core.c >> index 80000eb766..82fd0632ac 100644 >> --- a/hw/ide/core.c >> +++ b/hw/ide/core.c >> @@ -2210,7 +2210,6 @@ uint32_t ide_ioport_read(void *opaque, uint32_t addr) >> } else { >> ret = s->status; >> } >> - qemu_irq_lower(bus->irq); >> break; >> } > > I don't think that this is correct: from memory when I last looked at this, there > were 2 IDE status registers: the one from the original specification which clears the > IRQ upon read, and another one in subsequent revisions which allows you to read the > value without clearing any pending IRQ. My guess would be that changing this would > not only cause QEMU to deviate from the specification, but causes problems in other OSs. You're right, legacy ide has two status registers as described here: ftp://ftp.seagate.com/pub/acrobat/reference/111-1c.pdf Now question is which of these the above is emulating? Looks like CMD646 should not clear interrupt when reading status reg so maybe instead of removing this from here another change is needed to CMD646 specific read func to read alternate status instead of status reg? Regards, BALATON Zoltan
On Sat, 22 Feb 2020, BALATON Zoltan wrote: > On Sat, 22 Feb 2020, Mark Cave-Ayland wrote: >> On 21/02/2020 06:50, jasper.lowell@bt.com wrote: >>> The Linux libATA API documentation mentions that on some hardware, >>> reading the status register has the side effect of clearing the >>> interrupt condition. When emulating the generic Sun4u machine running >>> Solaris 10, the Solaris 10 CMD646 driver exits fatally because of this >>> emulated side effect. This side effect is likely to not exist on real >>> CMD646 hardware. >>> >>> Signed-off-by: Jasper Lowell <jasper.lowell@bt.com> >>> --- >>> hw/ide/core.c | 1 - >>> 1 file changed, 1 deletion(-) >>> >>> diff --git a/hw/ide/core.c b/hw/ide/core.c >>> index 80000eb766..82fd0632ac 100644 >>> --- a/hw/ide/core.c >>> +++ b/hw/ide/core.c >>> @@ -2210,7 +2210,6 @@ uint32_t ide_ioport_read(void *opaque, uint32_t >>> addr) >>> } else { >>> ret = s->status; >>> } >>> - qemu_irq_lower(bus->irq); >>> break; >>> } >> >> I don't think that this is correct: from memory when I last looked at this, >> there >> were 2 IDE status registers: the one from the original specification which >> clears the >> IRQ upon read, and another one in subsequent revisions which allows you to >> read the >> value without clearing any pending IRQ. My guess would be that changing >> this would >> not only cause QEMU to deviate from the specification, but causes problems >> in other OSs. > > You're right, legacy ide has two status registers as described here: > > ftp://ftp.seagate.com/pub/acrobat/reference/111-1c.pdf > > Now question is which of these the above is emulating? Looks like CMD646 We have both ide_status_read() which does not clear irq and ide_ioport_read() which does. pci_ide_cmd_read() which PCI ide should use calls ide_status_read() so I wonder why did reading status cleared irq on CMD646? So maybe it's cleared from somewhere else and above change should not be needed. Regards, BALATON Zoltan
I'm having another look at the SET_FEATURE Solaris 10 error. I've enabled tracing and I see the following. The pci_cfg_read that shows up at the end continues a few thousand times(?) but I've omitted it. This appears to time out or something and then Solaris gives up on the device. ide_ioport_read 41.468 pid=147030 addr=0x7 reg=b'Status' val=0x0 bus=0x55b77f922d10 s=0x55b77f 923168 ide_ioport_write 19.677 pid=147030 addr=0x6 reg=b'Device/Head' val=0xe0 bus=0x55b77f922d10 s=0 x55b77f923168 ide_ioport_write 19.417 pid=147030 addr=0x1 reg=b'Features' val=0x3 bus=0x55b77f922d10 s=0x55b 77f922d98 ide_ioport_write 9.027 pid=147030 addr=0x2 reg=b'Sector Count' val=0x42 bus=0x55b77f922d10 s=0 x55b77f922d98 ide_ioport_write 8.025 pid=147030 addr=0x7 reg=b'Command' val=0xef bus=0x55b77f922d10 s=0x55b7 7f922d98 ide_exec_cmd 0.461 pid=147030 bus=0x55b77f922d10 state=0x55b77f922d98 cmd=0xef pci_cfg_read 53.231 pid=147030 dev=b'cmd646-ide' devid=0x3 fnid=0x0 offs=0x50 val=0x4 ide_ioport_read 35.577 pid=147030 addr=0x7 reg=b'Status' val=0x50 bus=0x55b77f922d10 s=0x55b77 f922d98 ide_ioport_read 29.095 pid=147030 addr=0x7 reg=b'Status' val=0x50 bus=0x55b77f922d10 s=0x55b77 f922d98 ide_ioport_write 19.146 pid=147030 addr=0x6 reg=b'Device/Head' val=0xe0 bus=0x55b77f922d10 s=0 x55b77f922d98 pci_cfg_read 9.468 pid=147030 dev=b'cmd646-ide' devid=0x3 fnid=0x0 offs=0x50 val=0x0 pci_cfg_read 127.712 pid=147030 dev=b'cmd646-ide' devid=0x3 fnid=0x0 offs=0x50 val=0x0 pci_cfg_read 101.942 pid=147030 dev=b'cmd646-ide' devid=0x3 fnid=0x0 offs=0x50 val=0x0 pci_cfg_read 101.793 pid=147030 dev=b'cmd646-ide' devid=0x3 fnid=0x0 offs=0x50 val=0x0 pci_cfg_read 101.852 pid=147030 dev=b'cmd646-ide' devid=0x3 fnid=0x0 offs=0x50 val=0x0 pci_cfg_read 101.803 pid=147030 dev=b'cmd646-ide' devid=0x3 fnid=0x0 offs=0x50 val=0x0 pci_cfg_read 101.762 pid=147030 dev=b'cmd646-ide' devid=0x3 fnid=0x0 offs=0x50 val=0x0 pci_cfg_read 105.219 pid=147030 dev=b'cmd646-ide' devid=0x3 fnid=0x0 offs=0x50 val=0x0 pci_cfg_read 102.634 pid=147030 dev=b'cmd646-ide' devid=0x3 fnid=0x0 offs=0x50 val=0x0 pci_cfg_read 101.943 pid=147030 dev=b'cmd646-ide' devid=0x3 fnid=0x0 offs=0x50 val=0x0 pci_cfg_read 101.883 pid=147030 dev=b'cmd646-ide' devid=0x3 fnid=0x0 offs=0x50 val=0x0 pci_cfg_read 101.792 pid=147030 dev=b'cmd646-ide' devid=0x3 fnid=0x0 offs=0x50 val=0x0 It looks like I've made mistakes in previous comments about the error and what the problem might be. Excuse my inexperience. Rather than spinning on ARTTIM23_INTR_CH1 it might be the case that Solaris 10 is spinning on CFR_INTR_CH0. I think this because of the following trace: pci_cfg_read 53.231 pid=147030 dev=b'cmd646-ide' devid=0x3 fnid=0x0 offs=0x50 val=0x4 The two reads on the io status register show that DRDY (drive ready indicator bit) and DSC (drive seek complete bit). This doesn't look unusual to me. The error bit is also not set which is reassuring. I read through some of ftp://ftp.seagate.com/pub/acrobat/reference/111-1c.pdf and I'm confused by the discussion regarding interrupts and the status register. > INTRQ is cleared when the host reads the status register. My understand is that INTRQ is the signal from pin 31 on the drive and that the status register is on the drive. I understand the quoted statement as when the host (CMD646) reads the status register of the drive, the drive will lower the interrupt on this pin. The CMD646 has CFR_INTR_CH0 and ARTTIM23_INTR_CH1 in it's PCI configuration space. This is necessary to determine the source of an interrupt when the CMD646 ports are in PCI IDE Native Mode. Are we saying that when the drive lowers the interrupt, the CMD646 sees this and then clears CFR_INTR_CH0 and ARTTIM23_INTR_CH1 automatically? If this were the case then I don't know why there is an interface to clear them by writing to them. Also, if reading the ioport status register was enough to clear CFR_INTR_CH0 and ARTTIM23_INTR_CH1 (specific to CMD646) I can't reason why Linux, Solaris, and OpenBSD would have specific routines to clear them (following the CMD646 documentation) rather than just reading the ioport status register. With the patch, the tracing output changes to this: ide_ioport_read 128.512 pid=162907 addr=0x7 reg=b'Status' val=0x0 bus=0x55909512bd10 s=0x55909512c168 ide_ioport_write 22.622 pid=162907 addr=0x6 reg=b'Device/Head' val=0xe0 bus=0x55909512bd10 s=0x55909512c168 ide_ioport_write 21.330 pid=162907 addr=0x1 reg=b'Features' val=0x3 bus=0x55909512bd10 s=0x55909512bd98 ide_ioport_write 13.926 pid=162907 addr=0x2 reg=b'Sector Count' val=0x42 bus=0x55909512bd10 s=0x55909512bd98 ide_ioport_write 9.278 pid=162907 addr=0x7 reg=b'Command' val=0xef bus=0x55909512bd10 s=0x55909512bd98 ide_exec_cmd 0.921 pid=162907 bus=0x55909512bd10 state=0x55909512bd98 cmd=0xef pci_cfg_read 40.647 pid=162907 dev=b'cmd646-ide' devid=0x3 fnid=0x0 offs=0x50 val=0x4 ide_ioport_read 40.445 pid=162907 addr=0x7 reg=b'Status' val=0x50 bus=0x55909512bd10 s=0x55909512bd98 ide_ioport_read 31.580 pid=162907 addr=0x7 reg=b'Status' val=0x50 bus=0x55909512bd10 s=0x55909512bd98 ide_ioport_write 17.923 pid=162907 addr=0x6 reg=b'Device/Head' val=0xe0 bus=0x55909512bd10 s=0x55909512bd98 pci_cfg_read 10.931 pid=162907 dev=b'cmd646-ide' devid=0x3 fnid=0x0 offs=0x50 val=0x4 pci_cfg_read 19.136 pid=162907 dev=b'cmd646-ide' devid=0x3 fnid=0x0 offs=0x50 val=0x4 pci_cfg_write 26.650 pid=162907 dev=b'cmd646-ide' devid=0x3 fnid=0x0 offs=0x50 val=0x4 Now there is no fatal error and Solaris does the expected by setting CFR_INTR_CH0 to clear it. QEMU then updates the interrupt status in cmd646_pci_config_write. I'm still unconvinced that we should be lowering interrupts when the ide ioport status register is read. I might be misunderstanding the documentation though (definitely possible). If I am misunderstanding the documentation, could you or Mark clarify what I'm getting wrong here. Thanks, Jasper Lowell. On Sat, 2020-02-22 at 21:05 +0100, BALATON Zoltan wrote: > On Sat, 22 Feb 2020, BALATON Zoltan wrote: > > On Sat, 22 Feb 2020, Mark Cave-Ayland wrote: > > > On 21/02/2020 06:50, jasper.lowell@bt.com wrote: > > > > The Linux libATA API documentation mentions that on some > > > > hardware, > > > > reading the status register has the side effect of clearing the > > > > interrupt condition. When emulating the generic Sun4u machine > > > > running > > > > Solaris 10, the Solaris 10 CMD646 driver exits fatally because > > > > of this > > > > emulated side effect. This side effect is likely to not exist > > > > on real > > > > CMD646 hardware. > > > > > > > > Signed-off-by: Jasper Lowell <jasper.lowell@bt.com> > > > > --- > > > > hw/ide/core.c | 1 - > > > > 1 file changed, 1 deletion(-) > > > > > > > > diff --git a/hw/ide/core.c b/hw/ide/core.c > > > > index 80000eb766..82fd0632ac 100644 > > > > --- a/hw/ide/core.c > > > > +++ b/hw/ide/core.c > > > > @@ -2210,7 +2210,6 @@ uint32_t ide_ioport_read(void *opaque, > > > > uint32_t > > > > addr) > > > > } else { > > > > ret = s->status; > > > > } > > > > - qemu_irq_lower(bus->irq); > > > > break; > > > > } > > > > > > I don't think that this is correct: from memory when I last > > > looked at this, > > > there > > > were 2 IDE status registers: the one from the original > > > specification which > > > clears the > > > IRQ upon read, and another one in subsequent revisions which > > > allows you to > > > read the > > > value without clearing any pending IRQ. My guess would be that > > > changing > > > this would > > > not only cause QEMU to deviate from the specification, but causes > > > problems > > > in other OSs. > > > > You're right, legacy ide has two status registers as described > > here: > > > > ftp://ftp.seagate.com/pub/acrobat/reference/111-1c.pdf > > > > Now question is which of these the above is emulating? Looks like > > CMD646 > > We have both ide_status_read() which does not clear irq and > ide_ioport_read() which does. pci_ide_cmd_read() which PCI ide should > use > calls ide_status_read() so I wonder why did reading status cleared > irq on > CMD646? So maybe it's cleared from somewhere else and above change > should > not be needed. > > Regards, > BALATON Zoltan
On Sun, 23 Feb 2020, jasper.lowell@bt.com wrote: > ide_exec_cmd 0.461 pid=147030 bus=0x55b77f922d10 state=0x55b77f922d98 cmd=0xef The command is run here if I'm not mistaken Does this set the irq right away on QEMU where on real hadware this may take some time? Not sure if that's a problem but trying to understand what's happening. > pci_cfg_read 53.231 pid=147030 dev=b'cmd646-ide' devid=0x3 fnid=0x0 offs=0x50 val=0x4 > ide_ioport_read 35.577 pid=147030 addr=0x7 reg=b'Status' val=0x50 bus=0x55b77f922d10 s=0x55b77f922d98 > ide_ioport_read 29.095 pid=147030 addr=0x7 reg=b'Status' val=0x50 bus=0x55b77f922d10 s=0x55b77f922d98 So these ide_ioport_read calls clear the irq bit... > ide_ioport_write 19.146 pid=147030 addr=0x6 reg=b'Device/Head' val=0xe0 bus=0x55b77f922d10 s=0x55b77f922d98 > pci_cfg_read 9.468 pid=147030 dev=b'cmd646-ide' devid=0x3 fnid=0x0 offs=0x50 val=0x0 > pci_cfg_read 127.712 pid=147030 dev=b'cmd646-ide' devid=0x3 fnid=0x0 offs=0x50 val=0x0 > pci_cfg_read 101.942 pid=147030 dev=b'cmd646-ide' devid=0x3 fnid=0x0 offs=0x50 val=0x0 ...that would be checked here? > It looks like I've made mistakes in previous comments about the error > and what the problem might be. Excuse my inexperience. Rather than > spinning on ARTTIM23_INTR_CH1 it might be the case that Solaris 10 is > spinning on CFR_INTR_CH0. I think this because of the following trace: > > pci_cfg_read 53.231 pid=147030 dev=b'cmd646-ide' devid=0x3 fnid=0x0 offs=0x50 val=0x4 > > The two reads on the io status register show that DRDY (drive ready > indicator bit) and DSC (drive seek complete bit). This doesn't look > unusual to me. The error bit is also not set which is reassuring. What I don't get is why ide_ioport_read is called at all and from where if that's meant to emulate legacy ide ISA ioport reads and we have a PCI device accessed via PCI regs? Should the device behave differently in legacy and native mode with respect of clearing irq bit on register reads? > I read through some of > ftp://ftp.seagate.com/pub/acrobat/reference/111-1c.pdf and I'm confused > by the discussion regarding interrupts and the status register. > >> INTRQ is cleared when the host reads the status register. > > My understand is that INTRQ is the signal from pin 31 on the drive and > that the status register is on the drive. I understand the quoted > statement as when the host (CMD646) reads the status register of the > drive, the drive will lower the interrupt on this pin. > > The CMD646 has CFR_INTR_CH0 and ARTTIM23_INTR_CH1 in it's PCI > configuration space. This is necessary to determine the source of an > interrupt when the CMD646 ports are in PCI IDE Native Mode. Are we > saying that when the drive lowers the interrupt, the CMD646 sees this > and then clears CFR_INTR_CH0 and ARTTIM23_INTR_CH1 automatically? If > this were the case then I don't know why there is an interface to clear > them by writing to them. There's a possibility that software may want to clear bits without reading the current value so having a way to do that can be explained. > Also, if reading the ioport status register was enough to clear > CFR_INTR_CH0 and ARTTIM23_INTR_CH1 (specific to CMD646) I can't reason > why Linux, Solaris, and OpenBSD would have specific routines to clear > them (following the CMD646 documentation) rather than just reading the > ioport status register. But the doc not mentioning irq bits should be cleared on read and drivers do it by writing after reading is sufficient evidence that CMD646 likely does not clear bits on read. > With the patch, the tracing output changes to this: > ide_ioport_read 128.512 pid=162907 addr=0x7 reg=b'Status' val=0x0 bus=0x55909512bd10 s=0x55909512c168 > ide_ioport_write 22.622 pid=162907 addr=0x6 reg=b'Device/Head' val=0xe0 bus=0x55909512bd10 s=0x55909512c168 > ide_ioport_write 21.330 pid=162907 addr=0x1 reg=b'Features' val=0x3 bus=0x55909512bd10 s=0x55909512bd98 > ide_ioport_write 13.926 pid=162907 addr=0x2 reg=b'Sector Count' val=0x42 bus=0x55909512bd10 s=0x55909512bd98 > ide_ioport_write 9.278 pid=162907 addr=0x7 reg=b'Command' val=0xef bus=0x55909512bd10 s=0x55909512bd98 > ide_exec_cmd 0.921 pid=162907 bus=0x55909512bd10 state=0x55909512bd98 cmd=0xef > pci_cfg_read 40.647 pid=162907 dev=b'cmd646-ide' devid=0x3 fnid=0x0 offs=0x50 val=0x4 > ide_ioport_read 40.445 pid=162907 addr=0x7 reg=b'Status' val=0x50 bus=0x55909512bd10 s=0x55909512bd98 > ide_ioport_read 31.580 pid=162907 addr=0x7 reg=b'Status' val=0x50 bus=0x55909512bd10 s=0x55909512bd98 > ide_ioport_write 17.923 pid=162907 addr=0x6 reg=b'Device/Head' val=0xe0 bus=0x55909512bd10 s=0x55909512bd98 > pci_cfg_read 10.931 pid=162907 dev=b'cmd646-ide' devid=0x3 fnid=0x0 offs=0x50 val=0x4 > pci_cfg_read 19.136 pid=162907 dev=b'cmd646-ide' devid=0x3 fnid=0x0 offs=0x50 val=0x4 > pci_cfg_write 26.650 pid=162907 dev=b'cmd646-ide' devid=0x3 fnid=0x0 offs=0x50 val=0x4 The difference here is that status bits still there after ide_ioport_read when it gets it via pci_cfg_read than writes to that reg to clear it. > Now there is no fatal error and Solaris does the expected by setting > CFR_INTR_CH0 to clear it. QEMU then updates the interrupt status in > cmd646_pci_config_write. > > I'm still unconvinced that we should be lowering interrupts when the > ide ioport status register is read. I might be misunderstanding the > documentation though (definitely possible). If I am misunderstanding > the documentation, could you or Mark clarify what I'm getting wrong > here. I'm afraid I don't understand the problem enough either to be able to help. Maybe you could try to find out where is ide_ioport_read called in the above and if that's correct to call it there. Also the CMD646U docs mention irq in a lot of regs (all say write to clear) but I don't understand their relation to each other and irq raised by the drive. I've found these: 0x3c INTLINE R/W Interrupt line default=14 0x50 CFR R Configuration register bit2 1=interrupt pending, write 1 will clear this bit (in read only reg?) 0x57 ARTTIM23 R/W Drive2/3 Control/Status Register bit4 IDE drive 2/3 interrupt status (write 1 to clear) 0x71 or BAR4+1 MRDMODE DMA Master Read Mode Select bit2 Primary channel IDE interrupt (write 1 to clear) bit3 Secondary channel IDE interrupt (write 1 to clear) bit4 Primary channel interrupt enable 0 = propagate primary channel IDE interrupts to IRQ14 or INTA# pin (default) 1 = block primary channel IDE interrupts (NOTE: this does not affect the function of bit 2) bit5 same as bit4 for but for Secondary channel and IRQ15 0x72 or BAR4+2 R/W BMIDESR0 Bus Master IDE Status Reg 0 for primary channel bit2 interrupt generated on IDE bus for DMA transfer (write 1 to clear) 0x7a or BAR4+0xa R/W BMIDESR1 Bus Master IDE Status Reg 0 for secondary channel bit2 like BMIDESR0 but for channel 1 I think So maybe in DMA mode the BM* regs should be used and in legacy mode these interrupts would go to ISA IRQ14 and 15 and cleared on read as per the IDE spec while in native mode PCI INTA is raised and not cleared but the chip docs don't say anything about this so it's only guessing. Does anyone have more info on this? Regards, BALATON Zoltan
> > ide_exec_cmd 0.461 pid=147030 bus=0x55b77f922d10 > > state=0x55b77f922d98 cmd=0xef > The command is run here if I'm not mistaken Does this set the irq > right > away on QEMU where on real hadware this may take some time? Not sure > if > that's a problem but trying to understand what's happening. Yes. QEMU raises an IRQ on the completion of the command. complete = ide_cmd_table[val].handler(s, val); if (complete) { s->status &= ~BUSY_STAT; assert(!!s->error == !!(s->status & ERR_STAT)); if ((ide_cmd_table[val].flags & SET_DSC) && !s->error) { s->status |= SEEK_STAT; } ide_cmd_done(s); ide_set_irq(s->bus); } This code from /qemu/hw/ide/core.c is executed when the SET_FEATURE command request is made. I have tested that if this interrupt is not made, Solaris 10 will complain accordingly with a unique error message. I don't believe the quick interrupt here is the problem. Solaris 10 will spin for a short time while waiting for the interrupt bit to be set before continuing with its routine. If it doesn't see the interrupt bit is set before some timeout, it will print an error about the missing interrupt and give up loading the driver. > > pci_cfg_read 53.231 pid=147030 dev=b'cmd646-ide' devid=0x3 fnid=0x0 > > offs=0x50 val=0x4 > > ide_ioport_read 35.577 pid=147030 addr=0x7 reg=b'Status' val=0x50 > > bus=0x55b77f922d10 s=0x55b77f922d98 > > ide_ioport_read 29.095 pid=147030 addr=0x7 reg=b'Status' val=0x50 > > bus=0x55b77f922d10 s=0x55b77f922d98 > > So these ide_ioport_read calls clear the irq bit... That's right. The line that I proposed removing in the patch clears CFG_INTR_CH0 on ide_ioport_read. > > ide_ioport_write 19.146 pid=147030 addr=0x6 reg=b'Device/Head' > > val=0xe0 bus=0x55b77f922d10 s=0x55b77f922d98 > > pci_cfg_read 9.468 pid=147030 dev=b'cmd646-ide' devid=0x3 fnid=0x0 > > offs=0x50 val=0x0 > > pci_cfg_read 127.712 pid=147030 dev=b'cmd646-ide' devid=0x3 > > fnid=0x0 offs=0x50 val=0x0 > > pci_cfg_read 101.942 pid=147030 dev=b'cmd646-ide' devid=0x3 > > fnid=0x0 offs=0x50 val=0x0 > > ...that would be checked here? That's right. Solaris is performing pci_cfg_read on offs=0x50 until it either sees the interrupt bit set or times out. If it times out, you get a fatal error for the driver. The behaviour is not expected and aggressively checked against by the Solaris kernel. From what I can tell, Linux and OpenBSD don't check if the bit is set before clearing it. > What I don't get is why ide_ioport_read is called at all and from > where if > that's meant to emulate legacy ide ISA ioport reads and we have a > PCI > device accessed via PCI regs? Taken from the ATA specification: All commands that do not include read- or write-data transfers generate a single interrupt when the command completes. Resets do not generate an interrupt. There will be an interrupt whether the command is successful or not. If the host wants to know if an error occured it needs to inspect the status register. Solaris might be doing this. As the trace shows, there is no error and nothing is out of the ordinary. There are two devices. The PCI/IDE controller (CMD646) and the ATA compliant drive. The command, feature, and status registers belong to the drive. If you want to configure the drive in some way or interact with it you will use the ioport_read/write interface. CFR_INTR_CH0 and ARTTIM23_INTR_CH1 are PCI registers in the PCI configuration space that belongs to the PCI/IDE controller (CMD646). It makes sense to me that both are used. > There's a possibility that software may want to clear bits without > reading > the current value so having a way to do that can be explained. I agree that this might be a possibility. I also think its very normal for kernel drivers to drop the return value from operations when they are only interested in the side-effect. > I'm afraid I don't understand the problem enough either to be able > to > help. Maybe you could try to find out where is ide_ioport_read called > in > the above and if that's correct to call it there. Also the CMD646U > docs > mention irq in a lot of regs (all say write to clear) but I don't > understand their relation to each other and irq raised by the drive. I agree and I think that's part of the problem. The documentation does not explicitly mention their relation to each other. I can't see anything that suggests that reading the status register on the drive will unset bits in the pci configuration space of the controller. They are seperate devices. > So maybe in DMA mode the BM* regs should be used and in legacy mode > these > interrupts would go to ISA IRQ14 and 15 and cleared on read as per > the IDE > spec while in native mode PCI INTA is raised and not cleared but the > chip > docs don't say anything about this so it's only guessing. This might be true but I'm suspicious. In native mode the host should be checking the PCI registers to identify what device was responsible for the interrupt because it's multiplexed. This should be done before inspecting the status register of the device that caused the interrupt. > Does anyone > have > more info on this? Thanks, Jasper Lowell. On Sun, 2020-02-23 at 16:16 +0100, BALATON Zoltan wrote: > On Sun, 23 Feb 2020, jasper.lowell@bt.com wrote: > > ide_exec_cmd 0.461 pid=147030 bus=0x55b77f922d10 > > state=0x55b77f922d98 cmd=0xef > > The command is run here if I'm not mistaken Does this set the irq > right > away on QEMU where on real hadware this may take some time? Not sure > if > that's a problem but trying to understand what's happening. > > > pci_cfg_read 53.231 pid=147030 dev=b'cmd646-ide' devid=0x3 fnid=0x0 > > offs=0x50 val=0x4 > > ide_ioport_read 35.577 pid=147030 addr=0x7 reg=b'Status' val=0x50 > > bus=0x55b77f922d10 s=0x55b77f922d98 > > ide_ioport_read 29.095 pid=147030 addr=0x7 reg=b'Status' val=0x50 > > bus=0x55b77f922d10 s=0x55b77f922d98 > > So these ide_ioport_read calls clear the irq bit... > > > ide_ioport_write 19.146 pid=147030 addr=0x6 reg=b'Device/Head' > > val=0xe0 bus=0x55b77f922d10 s=0x55b77f922d98 > > pci_cfg_read 9.468 pid=147030 dev=b'cmd646-ide' devid=0x3 fnid=0x0 > > offs=0x50 val=0x0 > > pci_cfg_read 127.712 pid=147030 dev=b'cmd646-ide' devid=0x3 > > fnid=0x0 offs=0x50 val=0x0 > > pci_cfg_read 101.942 pid=147030 dev=b'cmd646-ide' devid=0x3 > > fnid=0x0 offs=0x50 val=0x0 > > ...that would be checked here? > > > It looks like I've made mistakes in previous comments about the > > error > > and what the problem might be. Excuse my inexperience. Rather than > > spinning on ARTTIM23_INTR_CH1 it might be the case that Solaris 10 > > is > > spinning on CFR_INTR_CH0. I think this because of the following > > trace: > > > > pci_cfg_read 53.231 pid=147030 dev=b'cmd646-ide' devid=0x3 fnid=0x0 > > offs=0x50 val=0x4 > > > > The two reads on the io status register show that DRDY (drive ready > > indicator bit) and DSC (drive seek complete bit). This doesn't look > > unusual to me. The error bit is also not set which is reassuring. > > What I don't get is why ide_ioport_read is called at all and from > where if > that's meant to emulate legacy ide ISA ioport reads and we have a > PCI > device accessed via PCI regs? Should the device behave differently > in > legacy and native mode with respect of clearing irq bit on register > reads? > > > I read through some of > > ftp://ftp.seagate.com/pub/acrobat/reference/111-1c.pdf and I'm > > confused > > by the discussion regarding interrupts and the status register. > > > > > INTRQ is cleared when the host reads the status register. > > > > My understand is that INTRQ is the signal from pin 31 on the drive > > and > > that the status register is on the drive. I understand the quoted > > statement as when the host (CMD646) reads the status register of > > the > > drive, the drive will lower the interrupt on this pin. > > > > The CMD646 has CFR_INTR_CH0 and ARTTIM23_INTR_CH1 in it's PCI > > configuration space. This is necessary to determine the source of > > an > > interrupt when the CMD646 ports are in PCI IDE Native Mode. Are we > > saying that when the drive lowers the interrupt, the CMD646 sees > > this > > and then clears CFR_INTR_CH0 and ARTTIM23_INTR_CH1 automatically? > > If > > this were the case then I don't know why there is an interface to > > clear > > them by writing to them. > > There's a possibility that software may want to clear bits without > reading > the current value so having a way to do that can be explained. > > > Also, if reading the ioport status register was enough to clear > > CFR_INTR_CH0 and ARTTIM23_INTR_CH1 (specific to CMD646) I can't > > reason > > why Linux, Solaris, and OpenBSD would have specific routines to > > clear > > them (following the CMD646 documentation) rather than just reading > > the > > ioport status register. > > But the doc not mentioning irq bits should be cleared on read and > drivers > do it by writing after reading is sufficient evidence that CMD646 > likely > does not clear bits on read. > > > With the patch, the tracing output changes to this: > > ide_ioport_read 128.512 pid=162907 addr=0x7 reg=b'Status' val=0x0 > > bus=0x55909512bd10 s=0x55909512c168 > > ide_ioport_write 22.622 pid=162907 addr=0x6 reg=b'Device/Head' > > val=0xe0 bus=0x55909512bd10 s=0x55909512c168 > > ide_ioport_write 21.330 pid=162907 addr=0x1 reg=b'Features' val=0x3 > > bus=0x55909512bd10 s=0x55909512bd98 > > ide_ioport_write 13.926 pid=162907 addr=0x2 reg=b'Sector Count' > > val=0x42 bus=0x55909512bd10 s=0x55909512bd98 > > ide_ioport_write 9.278 pid=162907 addr=0x7 reg=b'Command' val=0xef > > bus=0x55909512bd10 s=0x55909512bd98 > > ide_exec_cmd 0.921 pid=162907 bus=0x55909512bd10 > > state=0x55909512bd98 cmd=0xef > > pci_cfg_read 40.647 pid=162907 dev=b'cmd646-ide' devid=0x3 fnid=0x0 > > offs=0x50 val=0x4 > > ide_ioport_read 40.445 pid=162907 addr=0x7 reg=b'Status' val=0x50 > > bus=0x55909512bd10 s=0x55909512bd98 > > ide_ioport_read 31.580 pid=162907 addr=0x7 reg=b'Status' val=0x50 > > bus=0x55909512bd10 s=0x55909512bd98 > > ide_ioport_write 17.923 pid=162907 addr=0x6 reg=b'Device/Head' > > val=0xe0 bus=0x55909512bd10 s=0x55909512bd98 > > pci_cfg_read 10.931 pid=162907 dev=b'cmd646-ide' devid=0x3 fnid=0x0 > > offs=0x50 val=0x4 > > pci_cfg_read 19.136 pid=162907 dev=b'cmd646-ide' devid=0x3 fnid=0x0 > > offs=0x50 val=0x4 > > pci_cfg_write 26.650 pid=162907 dev=b'cmd646-ide' devid=0x3 > > fnid=0x0 offs=0x50 val=0x4 > > The difference here is that status bits still there after > ide_ioport_read > when it gets it via pci_cfg_read than writes to that reg to clear it. > > > Now there is no fatal error and Solaris does the expected by > > setting > > CFR_INTR_CH0 to clear it. QEMU then updates the interrupt status in > > cmd646_pci_config_write. > > > > I'm still unconvinced that we should be lowering interrupts when > > the > > ide ioport status register is read. I might be misunderstanding the > > documentation though (definitely possible). If I am > > misunderstanding > > the documentation, could you or Mark clarify what I'm getting wrong > > here. > > I'm afraid I don't understand the problem enough either to be able > to > help. Maybe you could try to find out where is ide_ioport_read called > in > the above and if that's correct to call it there. Also the CMD646U > docs > mention irq in a lot of regs (all say write to clear) but I don't > understand their relation to each other and irq raised by the drive. > I've > found these: > > 0x3c INTLINE R/W Interrupt line default=14 > > 0x50 CFR R Configuration register > bit2 1=interrupt pending, write 1 will clear this bit (in read > only reg?) > > 0x57 ARTTIM23 R/W Drive2/3 Control/Status Register > bit4 IDE drive 2/3 interrupt status (write 1 to clear) > > 0x71 or BAR4+1 MRDMODE DMA Master Read Mode Select > bit2 Primary channel IDE interrupt (write 1 to clear) > bit3 Secondary channel IDE interrupt (write 1 to clear) > bit4 Primary channel interrupt enable > 0 = propagate primary channel IDE interrupts to IRQ14 or > INTA# pin (default) > 1 = block primary channel IDE interrupts (NOTE: this does > not affect the function of bit 2) > bit5 same as bit4 for but for Secondary channel and IRQ15 > > 0x72 or BAR4+2 R/W BMIDESR0 Bus Master IDE Status Reg 0 for primary > channel > bit2 interrupt generated on IDE bus for DMA transfer (write 1 > to clear) > > 0x7a or BAR4+0xa R/W BMIDESR1 Bus Master IDE Status Reg 0 for > secondary channel > bit2 like BMIDESR0 but for channel 1 I think > > So maybe in DMA mode the BM* regs should be used and in legacy mode > these > interrupts would go to ISA IRQ14 and 15 and cleared on read as per > the IDE > spec while in native mode PCI INTA is raised and not cleared but the > chip > docs don't say anything about this so it's only guessing. Does anyone > have > more info on this? > > Regards, > BALATON Zoltan
On Tue, 25 Feb 2020, jasper.lowell@bt.com wrote: > I don't believe the quick interrupt here is the problem. Solaris 10 > will spin for a short time while waiting for the interrupt bit to be > set before continuing with its routine. If it doesn't see the interrupt > bit is set before some timeout, it will print an error about the > missing interrupt and give up loading the driver. I don't think missing delay should cause any problem either just pointed this out as a difference from real controller which may have an effect but I agree this is probably not a problem. >>> pci_cfg_read 53.231 pid=147030 dev=b'cmd646-ide' devid=0x3 fnid=0x0 >>> offs=0x50 val=0x4 >>> ide_ioport_read 35.577 pid=147030 addr=0x7 reg=b'Status' val=0x50 >>> bus=0x55b77f922d10 s=0x55b77f922d98 >>> ide_ioport_read 29.095 pid=147030 addr=0x7 reg=b'Status' val=0x50 >>> bus=0x55b77f922d10 s=0x55b77f922d98 >> >> So these ide_ioport_read calls clear the irq bit... > > That's right. The line that I proposed removing in the patch clears > CFG_INTR_CH0 on ide_ioport_read. Problem with that patch is that it removes this clearing from the func that's also used to emulate ISA IDE ioports which according to their spec should clear irq on read so that function should be OK but maybe should not be called by PCI IDE code? >>> ide_ioport_write 19.146 pid=147030 addr=0x6 reg=b'Device/Head' >>> val=0xe0 bus=0x55b77f922d10 s=0x55b77f922d98 >>> pci_cfg_read 9.468 pid=147030 dev=b'cmd646-ide' devid=0x3 fnid=0x0 >>> offs=0x50 val=0x0 >>> pci_cfg_read 127.712 pid=147030 dev=b'cmd646-ide' devid=0x3 >>> fnid=0x0 offs=0x50 val=0x0 >>> pci_cfg_read 101.942 pid=147030 dev=b'cmd646-ide' devid=0x3 >>> fnid=0x0 offs=0x50 val=0x0 >> >> ...that would be checked here? > > That's right. > > Solaris is performing pci_cfg_read on offs=0x50 until it either sees > the interrupt bit set or times out. If it times out, you get a fatal > error for the driver. The behaviour is not expected and aggressively > checked against by the Solaris kernel. From what I can tell, Linux and > OpenBSD don't check if the bit is set before clearing it. > >> What I don't get is why ide_ioport_read is called at all and from where >> if that's meant to emulate legacy ide ISA ioport reads and we have a >> PCI device accessed via PCI regs? What I meant was where is ide_ioport_read() is called in this case when we have a PCI IDE controller? Searching for it I think it may come from pci_ide_data_read() in hw/ide/pci.c. This document: http://www.bswd.com/pciide.pdf has some info on this and there are mentions of status using Alternate Status (which does not clear interrupt bit) but I think that corresponds to pci_ide_cmd_read() which already uses ide_status_read() so that seems correct. I did not find anything about contents of the Command Block in this doc which the function with ide_ioport_read call implements so not sure if that's expected to clear interrupt in PCI native mode or should we change pci_ide_data_read() to avoid that. >> mention irq in a lot of regs (all say write to clear) but I don't >> understand their relation to each other and irq raised by the drive. > > I agree and I think that's part of the problem. The documentation does > not explicitly mention their relation to each other. I can't see > anything that suggests that reading the status register on the drive > will unset bits in the pci configuration space of the controller. They > are seperate devices. Except the legacy IDE spec that does say reading status is clearing IRQ but not sure PCI native mode should do the same but it seems to use the same function in QEMU so it will clear IRQ as in legacy IDE mode. But this Linux driver says IRQ is cleared on read for PCI as well: https://github.com/torvalds/linux/blob/master/drivers/ata/libata-sff.c as does the CMD646 driver: https://github.com/torvalds/linux/blob/master/drivers/ata/pata_cmd64x.c in cmd64x_sff_irq_check() although for different chip revisions it uses cmd648_sff_irq_* functions which does this differently and avoids reading status reg and clears irq explicitely. It also has a warning at the beginning that UDMA mode is broken on most of these chips so it won't try to use it on anything below CMD646U2 so this suggests maybe there's a problem with clearing IRQs on at least some CMD646 chip revisions. I think the Sun Ultra 10 used CMD646U but not sure what the Solaris driver expects and if it can work with later chip revisions. Maybe we should either emulate the chip bugs or change something to identify as CMD646U2 which should behave more like stadard PCI IDE controllers? Although if I got that correctly Linux thinks revisions over 5 are OK and QEMU has 7. I think we need advice from someone more knowledgeable about real hardware on this. Regards, BALATON Zoltan
> Problem with that patch is that it removes this clearing from the > func > that's also used to emulate ISA IDE ioports which according to their > spec > should clear irq on read so that function should be OK but maybe > should > not be called by PCI IDE code? This might be it. The patch I provided is definitely incorrect and deviates from the specification as Mark mentioned earlier. I misunderstood what ide_ioport_read/write were for and haven't been thinking about legacy mode. The bug that I believe exists is present when the CMD646 is operating in PCI native mode. Yeah, I think a possible solution might be to avoid using the ioport_read/write functions from the PCI code if they have side effects that assume the device is in legacy mode. I'll have to spend more time reading through the code and documentation. > Except the legacy IDE spec that does say reading status is clearing > IRQ > but not sure PCI native mode should do the same but it seems to use > the > same function in QEMU so it will clear IRQ as in legacy IDE mode. According to the CMD646U2 specification: "When an IDE port is in PCI IDE Legacy Mode, the PCI646U2 is compatible with standard ISA IDE. The IDE task file registers are mapped to the standard ISA port addresses, and IDE drive interrupts occur at IRQ14 (primary) or IRQ15 (secondary)." In legacy mode, IRQ14 and IRQ15 mirror the state of INTRQ on each of the selected IDE devices. QEMU appears to emulate this correctly. In PCI native mode, INTRQ is not mirrored or given a single IRQ. Interrupts are provided by the PCI IDE controller depending on the controller's logic. For instance, an IDE device can raise an interrupt but the CMD646 may not propagate that interrupt if MRDMODE has certain bits set. I'm thinking that maybe the controller does not have logic to unset the interrupt bits in CFR and ARTTIM23 when the IDE device lowers INTRQ. This might mean that the controller will continue to assert an interrupt while bits in CFR and ARTTIM23 remain set, even if the IDE device lowers INTRQ. This would explain why the CMD646 documentation instructs developers to lower them explicitly. > Except the legacy IDE spec that does say reading status is clearing > IRQ > but not sure PCI native mode should do the same but it seems to use > the > same function in QEMU so it will clear IRQ as in legacy IDE mode. But > this > Linux driver says IRQ is cleared on read for PCI as well: > > https://github.com/torvalds/linux/blob/master/drivers/ata/libata-sff.c > > as does the CMD646 driver: > > https://github.com/torvalds/linux/blob/master/drivers/ata/pata_cmd64x.c > > in cmd64x_sff_irq_check() although for different chip revisions it > uses > cmd648_sff_irq_* functions which does this differently and avoids > reading > status reg and clears irq explicitely. It also has a warning at the > beginning that UDMA mode is broken on most of these chips so it won't > try > to use it on anything below CMD646U2 so this suggests maybe there's > a > problem with clearing IRQs on at least some CMD646 chip revisions. I > think > the Sun Ultra 10 used CMD646U but not sure what the Solaris driver > expects > and if it can work with later chip revisions. Maybe we should either > emulate the chip bugs or change something to identify as CMD646U2 > which > should behave more like stadard PCI IDE controllers? Although if I > got > that correctly Linux thinks revisions over 5 are OK and QEMU has 7. I'm not sure what it expects. If the Sun Ultra 10 shipped with the CMD646U, I reason that Solaris 10 either expects it or has support for it. The Linux driver code appears to be consistent with the behaviour that I'm seeing from Solaris 10. The following appears to be used to initialise the CMD646U. { /* CMD 646U with broken UDMA */ .flags = ATA_FLAG_SLAVE_POSS, .pio_mask = ATA_PIO4, .mwdma_mask = ATA_MWDMA2, .port_ops = &cmd646r3_port_ops }, The port operations it uses are defined as so: static struct ata_port_operations cmd646r3_port_ops = { .inherits = &cmd64x_base_ops, .sff_irq_check = cmd648_sff_irq_check, .sff_irq_clear = cmd648_sff_irq_clear, .cable_detect = ata_cable_40wire, } As you mention, cmd648_sff_irq_clear clears interrupts explicitly by setting bits in MRDMODE - consistent with the CMD646U2 documentation. This behaviour is very similar to Solaris 10. > Although if I got > that correctly Linux thinks revisions over 5 are OK and QEMU has 7. I'm not sure how revision numbers work with these chips. Do CMD646 and CMD646U2 refer to different revisions of the CMD646 chip? Thanks, Jasper Lowell. On Tue, 2020-02-25 at 16:08 +0100, BALATON Zoltan wrote: > On Tue, 25 Feb 2020, jasper.lowell@bt.com wrote: I don't believe the > quick interrupt here is the problem. Solaris 10 will spin for a short > time while waiting for the interrupt bit to be set before continuing > with its routine. If it doesn't see the interrupt bit is set before > some timeout, it will print an error about the missing interrupt and > give up loading the driver. I don't think missing delay should cause any problem either just pointed this out as a difference from real controller which may have an effect but I agree this is probably not a problem. pci_cfg_read 53.231 pid=147030 dev=b'cmd646-ide' devid=0x3 fnid=0x0 offs=0x50 val=0x4 ide_ioport_read 35.577 pid=147030 addr=0x7 reg=b'Status' val=0x50 bus=0x55b77f922d10 s=0x55b77f922d98 ide_ioport_read 29.095 pid=147030 addr=0x7 reg=b'Status' val=0x50 bus=0x55b77f922d10 s=0x55b77f922d98 So these ide_ioport_read calls clear the irq bit... That's right. The line that I proposed removing in the patch clears CFG_INTR_CH0 on ide_ioport_read. Problem with that patch is that it removes this clearing from the func that's also used to emulate ISA IDE ioports which according to their spec should clear irq on read so that function should be OK but maybe should not be called by PCI IDE code? ide_ioport_write 19.146 pid=147030 addr=0x6 reg=b'Device/Head' val=0xe0 bus=0x55b77f922d10 s=0x55b77f922d98 pci_cfg_read 9.468 pid=147030 dev=b'cmd646-ide' devid=0x3 fnid=0x0 offs=0x50 val=0x0 pci_cfg_read 127.712 pid=147030 dev=b'cmd646-ide' devid=0x3 fnid=0x0 offs=0x50 val=0x0 pci_cfg_read 101.942 pid=147030 dev=b'cmd646-ide' devid=0x3 fnid=0x0 offs=0x50 val=0x0 ...that would be checked here? That's right. Solaris is performing pci_cfg_read on offs=0x50 until it either sees the interrupt bit set or times out. If it times out, you get a fatal error for the driver. The behaviour is not expected and aggressively checked against by the Solaris kernel. From what I can tell, Linux and OpenBSD don't check if the bit is set before clearing it. What I don't get is why ide_ioport_read is called at all and from where if that's meant to emulate legacy ide ISA ioport reads and we have a PCI device accessed via PCI regs? What I meant was where is ide_ioport_read() is called in this case when we have a PCI IDE controller? Searching for it I think it may come from pci_ide_data_read() in hw/ide/pci.c. This document: http://www.bswd.com/pciide.pdf has some info on this and there are mentions of status using Alternate Status (which does not clear interrupt bit) but I think that corresponds to pci_ide_cmd_read() which already uses ide_status_read() so that seems correct. I did not find anything about contents of the Command Block in this doc which the function with ide_ioport_read call implements so not sure if that's expected to clear interrupt in PCI native mode or should we change pci_ide_data_read() to avoid that. mention irq in a lot of regs (all say write to clear) but I don't understand their relation to each other and irq raised by the drive. I agree and I think that's part of the problem. The documentation does not explicitly mention their relation to each other. I can't see anything that suggests that reading the status register on the drive will unset bits in the pci configuration space of the controller. They are seperate devices. Except the legacy IDE spec that does say reading status is clearing IRQ but not sure PCI native mode should do the same but it seems to use the same function in QEMU so it will clear IRQ as in legacy IDE mode. But this Linux driver says IRQ is cleared on read for PCI as well: https://github.com/torvalds/linux/blob/master/drivers/ata/libata-sff.c as does the CMD646 driver: https://github.com/torvalds/linux/blob/master/drivers/ata/pata_cmd64x.c in cmd64x_sff_irq_check() although for different chip revisions it uses cmd648_sff_irq_* functions which does this differently and avoids reading status reg and clears irq explicitely. It also has a warning at the beginning that UDMA mode is broken on most of these chips so it won't try to use it on anything below CMD646U2 so this suggests maybe there's a problem with clearing IRQs on at least some CMD646 chip revisions. I think the Sun Ultra 10 used CMD646U but not sure what the Solaris driver expects and if it can work with later chip revisions. Maybe we should either emulate the chip bugs or change something to identify as CMD646U2 which should behave more like stadard PCI IDE controllers? Although if I got that correctly Linux thinks revisions over 5 are OK and QEMU has 7. I think we need advice from someone more knowledgeable about real hardware on this. Regards, BALATON Zoltan
On Wed, 26 Feb 2020, jasper.lowell@bt.com wrote: > > Problem with that patch is that it removes this clearing from the func > > that's also used to emulate ISA IDE ioports which according to their > > spec should clear irq on read so that function should be OK but maybe > > should not be called by PCI IDE code? > > This might be it. > > The patch I provided is definitely incorrect and deviates from the > specification as Mark mentioned earlier. I misunderstood what > ide_ioport_read/write were for and haven't been thinking about legacy > mode. > > The bug that I believe exists is present when the CMD646 is operating > in PCI native mode. Yeah, I think a possible solution might be to avoid > using the ioport_read/write functions from the PCI code if they have > side effects that assume the device is in legacy mode. I'll have to > spend more time reading through the code and documentation. Since both the generic PCI IDE and CMD646 Linux drivers mention irq is cleared on reading status I think using ide_ioport_read in hw/ide/pci.c may be correct for the generic case. Not sure if the CMD646 has some pecularity but maybe the difference in drivers is to avoid bugs not because of CMD646 not clearing irq. The wikipedia page of CMD640: https://en.wikipedia.org/wiki/CMD640 mentions some versions of it has a bug similar to RZ-1000 for which there's a doc referenced that says the problem is that it forgets last data word after raising (or clearing?) IRQ and a workaround is to avoid checking status until all data transferred. This may explain why Linux checks alt status and clears interrupt instead of reading status register. > According to the CMD646U2 specification: > "When an IDE port is in PCI IDE Legacy Mode, the PCI646U2 is compatible > with standard ISA IDE. The IDE task file registers are mapped to the > standard ISA port addresses, and IDE drive interrupts occur at IRQ14 > (primary) or IRQ15 (secondary)." > > In legacy mode, IRQ14 and IRQ15 mirror the state of INTRQ on each of > the selected IDE devices. QEMU appears to emulate this correctly. > > In PCI native mode, INTRQ is not mirrored or given a single IRQ. > Interrupts are provided by the PCI IDE controller depending on the > controller's logic. For instance, an IDE device can raise an interrupt > but the CMD646 may not propagate that interrupt if MRDMODE has certain > bits set. I'm thinking that maybe the controller does not have logic to > unset the interrupt bits in CFR and ARTTIM23 when the IDE device lowers > INTRQ. This might mean that the controller will continue to assert an > interrupt while bits in CFR and ARTTIM23 remain set, even if the IDE > device lowers INTRQ. This would explain why the CMD646 documentation > instructs developers to lower them explicitly. I don't know but sounds plausible that reading the status reg clears irq but reading the pci config words that mirrors it won't clear it. But the traces you had show that ide_ioport_read was called so driver was likely reading status and not the config regs? I've found some further logs: https://forums.gentoo.org/viewtopic-t-270357.html https://www.redhat.com/archives/axp-list/2000-October/msg00070.html https://www.linuxtopia.org/online_books/linux_beginner_books/debian_linux_desktop_survival_guide/Docking_Station.shtml These show Linux messages for early CMD646 revisions that had bugs but what I've noticed is that they say something about not 100% native mode which seems to be similar to what I had with via-ide when it uses IRQ14-15 even in native mode. Could your problem be similar? Maybe you could search for more such logs for Linux booting on Sun Ultra machines and see what those say and check how it determines which IRQ number it should use. This may depend on some setting that's not emulated correctly. > The Linux driver code appears to be consistent with the behaviour that > I'm seeing from Solaris 10. > > The following appears to be used to initialise the CMD646U. > > { /* CMD 646U with broken UDMA */ > .flags = ATA_FLAG_SLAVE_POSS, > .pio_mask = ATA_PIO4, > .mwdma_mask = ATA_MWDMA2, > .port_ops = &cmd646r3_port_ops > }, > > The port operations it uses are defined as so: > > static struct ata_port_operations cmd646r3_port_ops = { > .inherits = &cmd64x_base_ops, > .sff_irq_check = cmd648_sff_irq_check, > .sff_irq_clear = cmd648_sff_irq_clear, > .cable_detect = ata_cable_40wire, > } > > As you mention, cmd648_sff_irq_clear clears interrupts explicitly by > setting bits in MRDMODE - consistent with the CMD646U2 documentation. > This behaviour is very similar to Solaris 10. I think this may be to avoid bug with CMD646U. > > Although if I got > > that correctly Linux thinks revisions over 5 are OK and QEMU has 7. > > I'm not sure how revision numbers work with these chips. Do CMD646 and > CMD646U2 refer to different revisions of the CMD646 chip? I'm not sure either but from what I've seen so far I think CMD646 either refers to the whole family (i.e. all versions) or early versions depending on context while there are at least two more newer versions referred to as CMD646U and CMD646U2 but probably there are more revisions within these as U2 seems to be rev5. QEMU sets the revision field to 7 but I'm not sure that's the same Linux checks for. There's some more info on this here: https://ata.wiki.kernel.org/index.php/Pata_cmd64x Regards, BALATON Zoltan
I've since looked at a Ultra 5 board and can confirm that it shipped with a CMD646U chip like the Ultra 10. > Since both the generic PCI IDE and CMD646 Linux drivers mention irq > is > cleared on reading status I think using ide_ioport_read in > hw/ide/pci.c > may be correct for the generic case. For clarity, the Linux drivers mention that for some chips reading CFR or ARTTIM23 will clear interrupts. Here in /linux/drivers/ata/pata_cmd64x.c, for instance: static bool cmd64x_sff_irq_check(struct ata_port *ap) { struct pci_dev *pdev = to_pci_dev(ap->host->dev); int irq_mask = ap->port_no ? ARTTIM23_INTR_CH1 : CFR_INTR_CH0; int irq_reg = ap->port_no ? ARTTIM23 : CFR; u8 irq_stat; /* NOTE: reading the register should clear the interrupt */ pci_read_config_byte(pdev, irq_reg, &irq_stat); return irq_stat & irq_mask; } I might be misunderstanding but isn't this a different side effect than clearing interrupts from the IDE device when the IDE device status register is read? This is saying that reading ARTTIM23 or CFR will clear INTA#. This code is also only used for the CMD643, CMD646, and CMD646 rev 1 - none of which I believe QEMU is attempting to emulate. This doesn't look relevant to us. > I'm not sure either but from what I've seen so far I think CMD646 > either > refers to the whole family (i.e. all versions) or early versions > depending > on context while there are at least two more newer versions referred > to as > CMD646U and CMD646U2 but probably there are more revisions within > these as > U2 seems to be rev5. QEMU sets the revision field to 7 but I'm not > sure > that's the same Linux checks for. There's some more info on this > here: > > https://ata.wiki.kernel.org/index.php/Pata_cmd64x I've seen CMD646 used to refer to the family as well. > QEMU sets the revision field to 7 In /linux/drivers/ide/cmd64x.c I found the following comment: * UltraDMA only supported on PCI646U and PCI646U2, which * correspond to revisions 0x03, 0x05 and 0x07 respectively. I guess the PCI646U2 is both revisions 0x05 and 0x07. According to /linux/drivers/ata/pata_cmd64x.c, interrupt behaviour doesn't change across the CMD646U, CMD646U2, CMD648, and the CMD649. These chips set the interrupt bit explicitly and do not have the comment you highlighted earlier about clearing interrupts when reading ARTTIM23 or CFR. > This may explain why > Linux > checks alt status and clears interrupt instead of reading status > register. I think Linux does this when there is no PCI IDE controller. The code in /linux/drivers/ide/ide-io.c acts directly on IDE devices. > I don't know but sounds plausible that reading the status reg clears > irq > but reading the pci config words that mirrors it won't clear it. But > the > traces you had show that ide_ioport_read was called so driver was > likely > reading status and not the config regs? When in PCI native mode, ide_ioport_read is called because of the following code in /qemu/hw/ide/pci.c. static uint64_t pci_ide_data_read(void *opaque, hwaddr addr, unsigned size) { IDEBus *bus = opaque; if (size == 1) { return ide_ioport_read(bus, addr); } else if (addr == 0) { if (size == 2) { return ide_data_readw(bus, addr); } else { return ide_data_readl(bus, addr); } } return ((uint64_t)1 << (size * 8)) - 1; } ide_ioport_read calls qemu_irq_lower when the IDE device status register is read. This will propagate all the way to the root bus. I think that when there is a PCI IDE controller inbetween and in PCI native mode, this is not always propagated past the PCI IDE controller. In the case of the CMD646U2, I think the lowering of interrupts is only propagated when the controller is in legacy/compatibility mode. From what I can tell cmd646_set_irq is only ever called from IDE device code, ie. ide_ioport_read. If the controller is not supposed to propagate the lowering of interrupts, I think a possible fix would be changing cmd646_set_irq to do nothing when the provided level is 0. Interrupts can still be cleared by writing to CFR, ARTTIM23, and MRDMODE which leverage cmd646_update_irq instead. This fix is not suitable if the emulated CMD646 can be used in compatibility mode but I don't think we have support for that anyways. I'll submit a RFC V2 patch with a proposed fix. Thanks, Jasper Lowell. On Wed, 2020-02-26 at 12:07 +0100, BALATON Zoltan wrote: > On Wed, 26 Feb 2020, jasper.lowell@bt.com wrote: > > > Problem with that patch is that it removes this clearing from the > > > func > > > that's also used to emulate ISA IDE ioports which according to > > > their > > > spec should clear irq on read so that function should be OK but > > > maybe > > > should not be called by PCI IDE code? > > > > This might be it. > > > > The patch I provided is definitely incorrect and deviates from the > > specification as Mark mentioned earlier. I misunderstood what > > ide_ioport_read/write were for and haven't been thinking about > > legacy > > mode. > > > > The bug that I believe exists is present when the CMD646 is > > operating > > in PCI native mode. Yeah, I think a possible solution might be to > > avoid > > using the ioport_read/write functions from the PCI code if they > > have > > side effects that assume the device is in legacy mode. I'll have to > > spend more time reading through the code and documentation. > > Since both the generic PCI IDE and CMD646 Linux drivers mention irq > is > cleared on reading status I think using ide_ioport_read in > hw/ide/pci.c > may be correct for the generic case. Not sure if the CMD646 has some > pecularity but maybe the difference in drivers is to avoid bugs not > because of CMD646 not clearing irq. The wikipedia page of CMD640: > > https://en.wikipedia.org/wiki/CMD640 > > mentions some versions of it has a bug similar to RZ-1000 for which > there's a doc referenced that says the problem is that it forgets > last > data word after raising (or clearing?) IRQ and a workaround is to > avoid > checking status until all data transferred. This may explain why > Linux > checks alt status and clears interrupt instead of reading status > register. > > > According to the CMD646U2 specification: > > "When an IDE port is in PCI IDE Legacy Mode, the PCI646U2 is > > compatible > > with standard ISA IDE. The IDE task file registers are mapped to > > the > > standard ISA port addresses, and IDE drive interrupts occur at > > IRQ14 > > (primary) or IRQ15 (secondary)." > > > > In legacy mode, IRQ14 and IRQ15 mirror the state of INTRQ on each > > of > > the selected IDE devices. QEMU appears to emulate this correctly. > > > > In PCI native mode, INTRQ is not mirrored or given a single IRQ. > > Interrupts are provided by the PCI IDE controller depending on the > > controller's logic. For instance, an IDE device can raise an > > interrupt > > but the CMD646 may not propagate that interrupt if MRDMODE has > > certain > > bits set. I'm thinking that maybe the controller does not have > > logic to > > unset the interrupt bits in CFR and ARTTIM23 when the IDE device > > lowers > > INTRQ. This might mean that the controller will continue to assert > > an > > interrupt while bits in CFR and ARTTIM23 remain set, even if the > > IDE > > device lowers INTRQ. This would explain why the CMD646 > > documentation > > instructs developers to lower them explicitly. > > I don't know but sounds plausible that reading the status reg clears > irq > but reading the pci config words that mirrors it won't clear it. But > the > traces you had show that ide_ioport_read was called so driver was > likely > reading status and not the config regs? > > I've found some further logs: > > https://forums.gentoo.org/viewtopic-t-270357.html > https://www.redhat.com/archives/axp-list/2000-October/msg00070.html > https://www.linuxtopia.org/online_books/linux_beginner_books/debian_linux_desktop_survival_guide/Docking_Station.shtml > > These show Linux messages for early CMD646 revisions that had bugs > but > what I've noticed is that they say something about not 100% native > mode > which seems to be similar to what I had with via-ide when it uses > IRQ14-15 > even in native mode. Could your problem be similar? Maybe you could > search > for more such logs for Linux booting on Sun Ultra machines and see > what > those say and check how it determines which IRQ number it should use. > This > may depend on some setting that's not emulated correctly. > > > The Linux driver code appears to be consistent with the behaviour > > that > > I'm seeing from Solaris 10. > > > > The following appears to be used to initialise the CMD646U. > > > > { /* CMD 646U with broken UDMA */ > > .flags = ATA_FLAG_SLAVE_POSS, > > .pio_mask = ATA_PIO4, > > .mwdma_mask = ATA_MWDMA2, > > .port_ops = &cmd646r3_port_ops > > }, > > > > The port operations it uses are defined as so: > > > > static struct ata_port_operations cmd646r3_port_ops = { > > .inherits = &cmd64x_base_ops, > > .sff_irq_check = cmd648_sff_irq_check, > > .sff_irq_clear = cmd648_sff_irq_clear, > > .cable_detect = ata_cable_40wire, > > } > > > > As you mention, cmd648_sff_irq_clear clears interrupts explicitly > > by > > setting bits in MRDMODE - consistent with the CMD646U2 > > documentation. > > This behaviour is very similar to Solaris 10. > > I think this may be to avoid bug with CMD646U. > > > > Although if I got > > > that correctly Linux thinks revisions over 5 are OK and QEMU has > > > 7. > > > > I'm not sure how revision numbers work with these chips. Do CMD646 > > and > > CMD646U2 refer to different revisions of the CMD646 chip? > > I'm not sure either but from what I've seen so far I think CMD646 > either > refers to the whole family (i.e. all versions) or early versions > depending > on context while there are at least two more newer versions referred > to as > CMD646U and CMD646U2 but probably there are more revisions within > these as > U2 seems to be rev5. QEMU sets the revision field to 7 but I'm not > sure > that's the same Linux checks for. There's some more info on this > here: > > https://ata.wiki.kernel.org/index.php/Pata_cmd64x > > Regards, > BALATON Zoltan >
> I'll submit a RFC V2 patch with a proposed fix. This will have to wait. Recent commits have caused Solaris 10 to error out of booting much earlier than previously. Jasper Lowell. On Thu, 2020-02-27 at 05:10 +0000, jasper.lowell@bt.com wrote: > I've since looked at a Ultra 5 board and can confirm that it shipped > with a CMD646U chip like the Ultra 10. > > > Since both the generic PCI IDE and CMD646 Linux drivers mention irq > > is > > cleared on reading status I think using ide_ioport_read in > > hw/ide/pci.c > > may be correct for the generic case. > > For clarity, the Linux drivers mention that for some chips reading > CFR > or ARTTIM23 will clear interrupts. Here in > /linux/drivers/ata/pata_cmd64x.c, for instance: > > static bool cmd64x_sff_irq_check(struct ata_port *ap) > { > struct pci_dev *pdev = to_pci_dev(ap->host->dev); > int irq_mask = ap->port_no ? ARTTIM23_INTR_CH1 : CFR_INTR_CH0; > int irq_reg = ap->port_no ? ARTTIM23 : CFR; > u8 irq_stat; > > /* NOTE: reading the register should clear the interrupt */ > pci_read_config_byte(pdev, irq_reg, &irq_stat); > > return irq_stat & irq_mask; > } > > I might be misunderstanding but isn't this a different side effect > than > clearing interrupts from the IDE device when the IDE device status > register is read? This is saying that reading ARTTIM23 or CFR will > clear INTA#. > > This code is also only used for the CMD643, CMD646, and CMD646 rev 1 > - > none of which I believe QEMU is attempting to emulate. This doesn't > look relevant to us. > > > I'm not sure either but from what I've seen so far I think CMD646 > > either > > refers to the whole family (i.e. all versions) or early versions > > depending > > on context while there are at least two more newer versions > > referred > > to as > > CMD646U and CMD646U2 but probably there are more revisions within > > these as > > U2 seems to be rev5. QEMU sets the revision field to 7 but I'm not > > sure > > that's the same Linux checks for. There's some more info on this > > here: > > > > https://ata.wiki.kernel.org/index.php/Pata_cmd64x > > I've seen CMD646 used to refer to the family as well. > > > QEMU sets the revision field to 7 > > In /linux/drivers/ide/cmd64x.c I found the following comment: > > * UltraDMA only supported on PCI646U and PCI646U2, which > * correspond to revisions 0x03, 0x05 and 0x07 respectively. > > I guess the PCI646U2 is both revisions 0x05 and 0x07. > > According to /linux/drivers/ata/pata_cmd64x.c, interrupt behaviour > doesn't change across the CMD646U, CMD646U2, CMD648, and the CMD649. > These chips set the interrupt bit explicitly and do not have the > comment you highlighted earlier about clearing interrupts when > reading > ARTTIM23 or CFR. > > > This may explain why > > Linux > > checks alt status and clears interrupt instead of reading status > > register. > > I think Linux does this when there is no PCI IDE controller. The code > in /linux/drivers/ide/ide-io.c acts directly on IDE devices. > > > I don't know but sounds plausible that reading the status reg > > clears > > irq > > but reading the pci config words that mirrors it won't clear it. > > But > > the > > traces you had show that ide_ioport_read was called so driver was > > likely > > reading status and not the config regs? > > When in PCI native mode, ide_ioport_read is called because of the > following code in /qemu/hw/ide/pci.c. > > static uint64_t pci_ide_data_read(void *opaque, hwaddr addr, unsigned > size) > { > IDEBus *bus = opaque; > > if (size == 1) { > return ide_ioport_read(bus, addr); > } else if (addr == 0) { > if (size == 2) { > return ide_data_readw(bus, addr); > } else { > return ide_data_readl(bus, addr); > } > } > return ((uint64_t)1 << (size * 8)) - 1; > } > > ide_ioport_read calls qemu_irq_lower when the IDE device status > register is read. This will propagate all the way to the root bus. I > think that when there is a PCI IDE controller inbetween and in PCI > native mode, this is not always propagated past the PCI IDE > controller. > In the case of the CMD646U2, I think the lowering of interrupts is > only > propagated when the controller is in legacy/compatibility mode. > > From what I can tell cmd646_set_irq is only ever called from IDE > device > code, ie. ide_ioport_read. If the controller is not supposed to > propagate the lowering of interrupts, I think a possible fix would be > changing cmd646_set_irq to do nothing when the provided level is 0. > Interrupts can still be cleared by writing to CFR, ARTTIM23, and > MRDMODE which leverage cmd646_update_irq instead. This fix is not > suitable if the emulated CMD646 can be used in compatibility mode but > I > don't think we have support for that anyways. > > I'll submit a RFC V2 patch with a proposed fix. > > Thanks, > Jasper Lowell. > > On Wed, 2020-02-26 at 12:07 +0100, BALATON Zoltan wrote: > > On Wed, 26 Feb 2020, jasper.lowell@bt.com wrote: > > > > Problem with that patch is that it removes this clearing from > > > > the > > > > func > > > > that's also used to emulate ISA IDE ioports which according to > > > > their > > > > spec should clear irq on read so that function should be OK but > > > > maybe > > > > should not be called by PCI IDE code? > > > > > > This might be it. > > > > > > The patch I provided is definitely incorrect and deviates from > > > the > > > specification as Mark mentioned earlier. I misunderstood what > > > ide_ioport_read/write were for and haven't been thinking about > > > legacy > > > mode. > > > > > > The bug that I believe exists is present when the CMD646 is > > > operating > > > in PCI native mode. Yeah, I think a possible solution might be to > > > avoid > > > using the ioport_read/write functions from the PCI code if they > > > have > > > side effects that assume the device is in legacy mode. I'll have > > > to > > > spend more time reading through the code and documentation. > > > > Since both the generic PCI IDE and CMD646 Linux drivers mention irq > > is > > cleared on reading status I think using ide_ioport_read in > > hw/ide/pci.c > > may be correct for the generic case. Not sure if the CMD646 has > > some > > pecularity but maybe the difference in drivers is to avoid bugs > > not > > because of CMD646 not clearing irq. The wikipedia page of CMD640: > > > > https://en.wikipedia.org/wiki/CMD640 > > > > mentions some versions of it has a bug similar to RZ-1000 for > > which > > there's a doc referenced that says the problem is that it forgets > > last > > data word after raising (or clearing?) IRQ and a workaround is to > > avoid > > checking status until all data transferred. This may explain why > > Linux > > checks alt status and clears interrupt instead of reading status > > register. > > > > > According to the CMD646U2 specification: > > > "When an IDE port is in PCI IDE Legacy Mode, the PCI646U2 is > > > compatible > > > with standard ISA IDE. The IDE task file registers are mapped to > > > the > > > standard ISA port addresses, and IDE drive interrupts occur at > > > IRQ14 > > > (primary) or IRQ15 (secondary)." > > > > > > In legacy mode, IRQ14 and IRQ15 mirror the state of INTRQ on each > > > of > > > the selected IDE devices. QEMU appears to emulate this correctly. > > > > > > In PCI native mode, INTRQ is not mirrored or given a single IRQ. > > > Interrupts are provided by the PCI IDE controller depending on > > > the > > > controller's logic. For instance, an IDE device can raise an > > > interrupt > > > but the CMD646 may not propagate that interrupt if MRDMODE has > > > certain > > > bits set. I'm thinking that maybe the controller does not have > > > logic to > > > unset the interrupt bits in CFR and ARTTIM23 when the IDE device > > > lowers > > > INTRQ. This might mean that the controller will continue to > > > assert > > > an > > > interrupt while bits in CFR and ARTTIM23 remain set, even if the > > > IDE > > > device lowers INTRQ. This would explain why the CMD646 > > > documentation > > > instructs developers to lower them explicitly. > > > > I don't know but sounds plausible that reading the status reg > > clears > > irq > > but reading the pci config words that mirrors it won't clear it. > > But > > the > > traces you had show that ide_ioport_read was called so driver was > > likely > > reading status and not the config regs? > > > > I've found some further logs: > > > > https://forums.gentoo.org/viewtopic-t-270357.html > > https://www.redhat.com/archives/axp-list/2000-October/msg00070.html > > https://www.linuxtopia.org/online_books/linux_beginner_books/debian_linux_desktop_survival_guide/Docking_Station.shtml > > > > These show Linux messages for early CMD646 revisions that had bugs > > but > > what I've noticed is that they say something about not 100% native > > mode > > which seems to be similar to what I had with via-ide when it uses > > IRQ14-15 > > even in native mode. Could your problem be similar? Maybe you could > > search > > for more such logs for Linux booting on Sun Ultra machines and see > > what > > those say and check how it determines which IRQ number it should > > use. > > This > > may depend on some setting that's not emulated correctly. > > > > > The Linux driver code appears to be consistent with the behaviour > > > that > > > I'm seeing from Solaris 10. > > > > > > The following appears to be used to initialise the CMD646U. > > > > > > { /* CMD 646U with broken UDMA */ > > > .flags = ATA_FLAG_SLAVE_POSS, > > > .pio_mask = ATA_PIO4, > > > .mwdma_mask = ATA_MWDMA2, > > > .port_ops = &cmd646r3_port_ops > > > }, > > > > > > The port operations it uses are defined as so: > > > > > > static struct ata_port_operations cmd646r3_port_ops = { > > > .inherits = &cmd64x_base_ops, > > > .sff_irq_check = cmd648_sff_irq_check, > > > .sff_irq_clear = cmd648_sff_irq_clear, > > > .cable_detect = ata_cable_40wire, > > > } > > > > > > As you mention, cmd648_sff_irq_clear clears interrupts explicitly > > > by > > > setting bits in MRDMODE - consistent with the CMD646U2 > > > documentation. > > > This behaviour is very similar to Solaris 10. > > > > I think this may be to avoid bug with CMD646U. > > > > > > Although if I got > > > > that correctly Linux thinks revisions over 5 are OK and QEMU > > > > has > > > > 7. > > > > > > I'm not sure how revision numbers work with these chips. Do > > > CMD646 > > > and > > > CMD646U2 refer to different revisions of the CMD646 chip? > > > > I'm not sure either but from what I've seen so far I think CMD646 > > either > > refers to the whole family (i.e. all versions) or early versions > > depending > > on context while there are at least two more newer versions > > referred > > to as > > CMD646U and CMD646U2 but probably there are more revisions within > > these as > > U2 seems to be rev5. QEMU sets the revision field to 7 but I'm not > > sure > > that's the same Linux checks for. There's some more info on this > > here: > > > > https://ata.wiki.kernel.org/index.php/Pata_cmd64x > > > > Regards, > > BALATON Zoltan > >
On Thu, 27 Feb 2020, jasper.lowell@bt.com wrote: >> I'll submit a RFC V2 patch with a proposed fix. > > This will have to wait. > > Recent commits have caused Solaris 10 to error out of booting much > earlier than previously. Can you bisect which commit broke it? Is it the same that caused slowness for arm and ppc? For that one there are patches that should fix it on the list. Regards, BALATON Zoltan
On Thu, 27 Feb 2020, jasper.lowell@bt.com wrote: > I've since looked at a Ultra 5 board and can confirm that it shipped > with a CMD646U chip like the Ultra 10. If you have access to an Ultra 5 maybe you could try testing what it does with irqs. That should give the ultimate answer to our guessing. It may need patching a Linux driver to log more info and recompile the kernel so not sure you have time for that but maybe it would help if you can do it. Regards, BALATON Zoltan
Hello, On Wed, 26 Feb 2020, jasper.lowell@bt.com wrote: > According to the CMD646U2 specification: > "When an IDE port is in PCI IDE Legacy Mode, the PCI646U2 is compatible > with standard ISA IDE. The IDE task file registers are mapped to the > standard ISA port addresses, and IDE drive interrupts occur at IRQ14 > (primary) or IRQ15 (secondary)." > > In legacy mode, IRQ14 and IRQ15 mirror the state of INTRQ on each of > the selected IDE devices. QEMU appears to emulate this correctly. So CMD646 also seems to have a legacy mode. I've also seen a CMD PCI0640B spec which is proabably a similar chip which says for interrupt handling: "When DSA1 is pulled low during reset, both IDE ports are in PCI IDE Legacy Mode. When DSA1 has no pull-down during reset, each IDE port may independently be set to PCI IDE Legacy Mode or Native Mode via the Programming Interface Byte (configuration register PROGIF, Index 9h). When an IDE port is in PCI IDE Legacy Mode, the PCI-0640B is compatible with standard ISA IDE. The IDE task file registers are mapped to the standard ISA port addresses, and IDE drive interrupts occur at IRQ14 (primary) or IRQ15 (secondary). When an IDE port is in PCI IDE Native Mode, the IDE task file registers may be mapped to non-standard port addresses, and IDE drive interrupts occur at PCI INTA. Therefore, if both IDE ports are in PCI IDE Native Mode, drive interrupts from both IDE ports are multiplexed into PCI INTA. In this case, the interrupt status bits must be polled to determine which IDE port generated the interrupt, or whether the interrupt was generated by another PCI device sharing INTA on the bus." This same explanation also appears in CMD646 doc. So what mode is the PROG_IF config reg set to and do the interrupts raised match that? cmd646_update_irq() only seems to raise PCI interrupt, should it also have an option to use INT 14 and 15 in legacy mode similar to what my patch does for via-ide? Additionally Solaris may also get info from the OF device tree so that may also have to match the device config. I'm not sure this helps but I don't have any better idea. Regards, BALATON Zoltan
I haven't. I did a pull this morning on master and everything seems to be working again. The problem was likely the same. Thanks, Jasper Lowell. On Thu, 2020-02-27 at 12:35 +0100, BALATON Zoltan wrote: > On Thu, 27 Feb 2020, jasper.lowell@bt.com wrote: > > > I'll submit a RFC V2 patch with a proposed fix. > > > > This will have to wait. > > > > Recent commits have caused Solaris 10 to error out of booting much > > earlier than previously. > > Can you bisect which commit broke it? Is it the same that caused > slowness > for arm and ppc? For that one there are patches that should fix it on > the > list. > > Regards, > BALATON Zoltan >
I'm happy to do this. It will take a while for me to collect the results though. I'll chime in once I have the results. > That should give the ultimate answer to our guessing. Agreed. Thanks, Jasper Lowell. On Thu, 2020-02-27 at 12:38 +0100, BALATON Zoltan wrote: > On Thu, 27 Feb 2020, jasper.lowell@bt.com wrote: > > I've since looked at a Ultra 5 board and can confirm that it > > shipped > > with a CMD646U chip like the Ultra 10. > > If you have access to an Ultra 5 maybe you could try testing what it > does > with irqs. That should give the ultimate answer to our guessing. It > may > need patching a Linux driver to log more info and recompile the > kernel so > not sure you have time for that but maybe it would help if you can do > it. > > Regards, > BALATON Zoltan
> cmd646_update_irq() only seems to raise PCI interrupt, should it also > have > an option to use INT 14 and 15 in legacy mode similar to what my > patch > does for via-ide? Looking through /qemu/hw/ide/cmd646.c it doesn't look like QEMU has support for legacy mode. At the very least, it looks like we default to PCI native mode: static void pci_cmd646_ide_realize(PCIDevice *dev, Error **errp) ... pci_conf[PCI_CLASS_PROG] = 0x8f; ... To add support for legacy mode it would require changing cmd646_update_irq() and maybe cmd646_set_irq() so that interrupts are conditionally raised on IRQ14 and/or IRQ15 when the ports are in legacy mode. Thanks, Jasper Lowell. On Sun, 2020-03-01 at 19:02 +0100, BALATON Zoltan wrote: > Hello, > > On Wed, 26 Feb 2020, jasper.lowell@bt.com wrote: > > According to the CMD646U2 specification: > > "When an IDE port is in PCI IDE Legacy Mode, the PCI646U2 is > > compatible > > with standard ISA IDE. The IDE task file registers are mapped to > > the > > standard ISA port addresses, and IDE drive interrupts occur at > > IRQ14 > > (primary) or IRQ15 (secondary)." > > > > In legacy mode, IRQ14 and IRQ15 mirror the state of INTRQ on each > > of > > the selected IDE devices. QEMU appears to emulate this correctly. > > So CMD646 also seems to have a legacy mode. I've also seen a CMD > PCI0640B > spec which is proabably a similar chip which says for interrupt > handling: > > "When DSA1 is pulled low during reset, both IDE ports are in PCI IDE > Legacy Mode. When DSA1 has no pull-down during reset, each IDE port > may > independently be set to PCI IDE Legacy Mode or Native Mode via the > Programming Interface Byte (configuration register PROGIF, Index 9h). > When > an IDE port is in PCI IDE Legacy Mode, the PCI-0640B is compatible > with > standard ISA IDE. The IDE task file registers are mapped to the > standard > ISA port addresses, and IDE drive interrupts occur at IRQ14 (primary) > or > IRQ15 (secondary). > > When an IDE port is in PCI IDE Native Mode, the IDE task file > registers > may be mapped to non-standard port addresses, and IDE drive > interrupts > occur at PCI INTA. Therefore, if both IDE ports are in PCI IDE > Native > Mode, drive interrupts from both IDE ports are multiplexed into PCI > INTA. > In this case, the interrupt status bits must be polled to determine > which > IDE port generated the interrupt, or whether the interrupt was > generated > by another PCI device sharing INTA on the bus." > > This same explanation also appears in CMD646 doc. So what mode is > the > PROG_IF config reg set to and do the interrupts raised match that? > cmd646_update_irq() only seems to raise PCI interrupt, should it also > have > an option to use INT 14 and 15 in legacy mode similar to what my > patch > does for via-ide? > > Additionally Solaris may also get info from the OF device tree so > that may > also have to match the device config. > > I'm not sure this helps but I don't have any better idea. > > Regards, > BALATON Zoltan >
On Wed, 4 Mar 2020, jasper.lowell@bt.com wrote: >> cmd646_update_irq() only seems to raise PCI interrupt, should it also >> have >> an option to use INT 14 and 15 in legacy mode similar to what my >> patch >> does for via-ide? > > Looking through /qemu/hw/ide/cmd646.c it doesn't look like QEMU has > support for legacy mode. At the very least, it looks like we default to > PCI native mode: > > static void pci_cmd646_ide_realize(PCIDevice *dev, Error **errp) > ... > pci_conf[PCI_CLASS_PROG] = 0x8f; > ... > > To add support for legacy mode it would require changing > cmd646_update_irq() and maybe cmd646_set_irq() so that interrupts are > conditionally raised on IRQ14 and/or IRQ15 when the ports are in legacy > mode. Ah yes, same problem as with via-ide. With current ISA IDE and PCI device emulation code in QEMU it's not easy to emulate both modes but maybe we don't need that either. (So we can avoid changing ISA and PCI QEMU parts that are widely used and risk breaking something else by that.) The Solaris driver does seem to use native mode because it could find io addresses in PCI BARs to talk to the controller just did not get interrupts if I got that right. Then maybe PCI interrupts are not routed as expected by the driver which could be due to - difference in PCI bridge emulation, - where the controller is connected - or how firmware describes it in device tree - or how it configures it compared to real hardware. Unless it also has some pecularity similar to pegasos2 with native mode and ISA interrupts. Checking on real hardware should reveal these differences so maybe that's the best way to find out. As for seeing what the firmware does Sun's OpenBoot is open sourced so maybe what that does could be checked but probably not easy to find out in Forth. Here are a few Linux logs I've found that suggest on a Sun Ultra 5/10 IDE is on irq 4 for both channels, but not sure how PCI interrupt ends up there though. Does that match what QEMU does? http://gavinduley.org/interests/computing/sundmesg.html https://lists.debian.org/debian-boot/2004/10/msg00864.html https://forums.gentoo.org/viewtopic-t-473614-start-0.html Regards, BALATON Zoltan
On 04/03/2020 03:11, jasper.lowell@bt.com wrote: >> cmd646_update_irq() only seems to raise PCI interrupt, should it also >> have >> an option to use INT 14 and 15 in legacy mode similar to what my >> patch >> does for via-ide? > > Looking through /qemu/hw/ide/cmd646.c it doesn't look like QEMU has > support for legacy mode. At the very least, it looks like we default to > PCI native mode: > > static void pci_cmd646_ide_realize(PCIDevice *dev, Error **errp) > ... > pci_conf[PCI_CLASS_PROG] = 0x8f; > ... > > To add support for legacy mode it would require changing > cmd646_update_irq() and maybe cmd646_set_irq() so that interrupts are > conditionally raised on IRQ14 and/or IRQ15 when the ports are in legacy > mode. Yes, that's correct. However I'm quite confident from booting other non-Solaris OSs under qemu-system-sparc64 that PCI native mode is being used, particularly as it is possible to see the related PCI sabre IRQ routing configuration changes. ATB, Mark.
> Yes, that's correct. However I'm quite confident from booting other > non-Solaris OSs > under qemu-system-sparc64 that PCI native mode is being used, > particularly as it is > possible to see the related PCI sabre IRQ routing configuration > changes. Considering that Solaris 10 is accessing CFR and ARTTIM23 I don't think there is any doubt that it is using the chip in PCI native mode. I don't think Solaris 10 even has support for legacy mode. Thanks, Jasper Lowell. On Wed, 2020-03-04 at 21:07 +0000, Mark Cave-Ayland wrote: > On 04/03/2020 03:11, jasper.lowell@bt.com wrote: > > > > cmd646_update_irq() only seems to raise PCI interrupt, should it > > > also > > > have > > > an option to use INT 14 and 15 in legacy mode similar to what my > > > patch > > > does for via-ide? > > > > Looking through /qemu/hw/ide/cmd646.c it doesn't look like QEMU has > > support for legacy mode. At the very least, it looks like we > > default to > > PCI native mode: > > > > static void pci_cmd646_ide_realize(PCIDevice *dev, Error **errp) > > ... > > pci_conf[PCI_CLASS_PROG] = 0x8f; > > ... > > > > To add support for legacy mode it would require changing > > cmd646_update_irq() and maybe cmd646_set_irq() so that interrupts > > are > > conditionally raised on IRQ14 and/or IRQ15 when the ports are in > > legacy > > mode. > > Yes, that's correct. However I'm quite confident from booting other > non-Solaris OSs > under qemu-system-sparc64 that PCI native mode is being used, > particularly as it is > possible to see the related PCI sabre IRQ routing configuration > changes. > > > ATB, > > Mark.
diff --git a/hw/ide/core.c b/hw/ide/core.c index 80000eb766..82fd0632ac 100644 --- a/hw/ide/core.c +++ b/hw/ide/core.c @@ -2210,7 +2210,6 @@ uint32_t ide_ioport_read(void *opaque, uint32_t addr) } else { ret = s->status; } - qemu_irq_lower(bus->irq); break; }
The Linux libATA API documentation mentions that on some hardware, reading the status register has the side effect of clearing the interrupt condition. When emulating the generic Sun4u machine running Solaris 10, the Solaris 10 CMD646 driver exits fatally because of this emulated side effect. This side effect is likely to not exist on real CMD646 hardware. Signed-off-by: Jasper Lowell <jasper.lowell@bt.com> --- hw/ide/core.c | 1 - 1 file changed, 1 deletion(-)