Re: [Qemu-devel] [PATCH] Sun4m : SCSI ESP controller

[Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>]

On 20/02/14 08:58, Mark Cave-Ayland wrote:

> On 16/02/14 23:13, Olivier Danet wrote:
>
>> Two small fixes for the ESP (AM53C94) SCSI controller
>>
>> * Signal the end of the DMA transfer after a SCSI command.
>>
>> * The status register (RSTAT) is cleared after reading the interrupt
>> status register (RINTR), except for the TC bit (=Count To Zero) and the
>> scsi phase bits, which mirror SCSI signals levels.
>>
>> Fixes the bug "esp0: !TC on DATA XFER" with NetBSD
>> https://bugs.launchpad.net/qemu/+bug/1055090
>>
>> Signed-off-by: Olivier Danet <odanet@caramail.com>
>> ---
>> hw/scsi/esp.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
>> index 2d150bf..5e91077 100644
>> --- a/hw/scsi/esp.c
>> +++ b/hw/scsi/esp.c
>> @@ -246,6 +246,7 @@ static void esp_do_dma(ESPState *s)
>> s->cmdlen = 0;
>> s->do_cmd = 0;
>> do_cmd(s, s->cmdbuf);
>> + esp_dma_done(s);
>> return;
>> }
>> if (s->async_len == 0) {
>> @@ -417,10 +418,9 @@ uint64_t esp_reg_read(ESPState *s, uint32_t saddr)
>> except TC */
>> old_val = s->rregs[ESP_RINTR];
>> s->rregs[ESP_RINTR] = 0;
>> - s->rregs[ESP_RSTAT] &= ~STAT_TC;
>> s->rregs[ESP_RSEQ] = SEQ_CD;
>> esp_lower_irq(s);
>> -
>> + s->rregs[ESP_RSTAT] &= STAT_TC | STAT_MI;
>> return old_val;
>> default:
>> break;
>
> I've applied the non-whitespace damaged patch (attached) but
> unfortunately I still see the "!TC on DATA XFER" bug with my NetBSD 5
> ISO under qemu-system-sparc :/ Note that the second part of the patch
> showed some fuzz so I'm wondering if this patch is complete or whether
> part of it is still missing?
>
> Also ESP patches should have an esp or scsi/esp prefix in the subject
> line and should CC the current SCSI maintainer (see MAINTAINERS file)
> which is currently Paolo.

Hi Olivier,

I've done some more testing on this patch today, and what I've found is 
that there are 2 types of error emitted from the NetBSD ESP driver: the 
majority of the "!TC on DATA XFER" messages go away under NetBSD 6 with 
the patch, but I still get the occasional one on the console, 
particularly with NetBSD 5. There are still messages about DMA 
interrupts appearing as can be seen at 
http://www.ilande.co.uk/tmp/netbsd5-esp.png.

Both Linux and NetBSD seem to install fine with the patch applied, so no 
regressions there - I haven't had a chance to test Solaris yet. If 
Artyom could give this a quick test on his collection of Solaris images 
then I'd say the patch is okay to apply.

More interesting is that with or without the patch, if you boot a 
standard Debian etch .qcow2 HD image and then shut it down then you get 
the following kernel panic: http://www.ilande.co.uk/tmp/debian-esp.png.

So my current conclusion is that this patch doesn't appear to cause any 
noticeable regressions outside of Solaris, however there is a still a 
problem somewhere with the ESP DMA interrupts on SPARC32.


ATB,

Mark.