Re: [Qemu-devel] Re: sparc esp NetBSD-guest "sd3: mode sense (4) returned nonsense"

[Artyom Tarasenko]

2009/9/19 Blue Swirl <address@hidden>:
> On Fri, Sep 18, 2009 at 8:26 PM, Artyom Tarasenko
> <address@hidden> wrote:
>> 2009/9/14 Blue Swirl <address@hidden>:
>>> On Mon, Sep 14, 2009 at 7:47 PM, Artyom Tarasenko
>>> <address@hidden> wrote:
>>>> 2009/9/14 Blue Swirl <address@hidden>:
>>>>> On Mon, Sep 14, 2009 at 7:29 PM, Artyom Tarasenko
>>>>> <address@hidden> wrote:
>>>>>> 2009/9/14 Blue Swirl <address@hidden>:
>>>>>>> On Mon, Sep 14, 2009 at 12:32 AM, Artyom Tarasenko
>>>>>>> <address@hidden> wrote:
>>>>>>>> From NetBSD source, it looks like HDD geometry detection should work
>>>>>>>> under qemu: they call "mode sense" and "read capacity", and both
>>>>>>>> commands are implemented in qemu's hw/scsi-disk.h. It doesn't work
>>>>>>>> though, so NetBSD has to fabricate a disk geometry.
>>>>>>>>
>>>>>>>> To make debugging easier I tried to boot an older version - NetBSD
>>>>>>>> 1.3.3. And put some extra debugging in esp.c:
>>>>>>>>
>>>>>>>> static uint32_t get_cmd(ESPState *s, uint8_t *buf)
>>>>>>>> {
>>>>>>>>    uint32_t dmalen;
>>>>>>>>    int target;
>>>>>>>>
>>>>>>>>    target = s->wregs[ESP_WBUSID] & BUSID_DID;
>>>>>>>>    if (s->dma) {
>>>>>>>>        dmalen = s->rregs[ESP_TCLO] | (s->rregs[ESP_TCMID] << 8);
>>>>>>>>        s->dma_memory_read(s->dma_opaque, buf, dmalen);
>>>>>>>>    } else {
>>>>>>>>        dmalen = s->ti_size;
>>>>>>>>        memcpy(buf, s->ti_buf, dmalen);
>>>>>>>> printf("NON-DMA rptr %d, wptr %d %2x (0) %2x %2x %2x %2x\n",
>>>>>>>> s->ti_rptr, s-> ti_wptr, buf[0],buf[1], buf[2],buf[3], buf[4]);
>>>>>>>>        buf[0] = 0;
>>>>>>>>    }
>>>>>>>>
>>>>>>>> qemu-system-sparc -M SS-20 -nographic  -hda ~/sparc/miniroot-133.fs -m 
>>>>>>>> 64
>>>>>>>> ...
>>>>>>>> NON-DMA rptr 0, wptr 1 c0 (0)  0  0 1a  0
>>>>>>>> Set ATN & Stop: cmdlen 3
>>>>>>>> scsi-disk: Command: lun=0 tag=0x0 data=0x00 0x00 0x1a 0x00 0x04 0x00
>>>>>>>> scsi-disk: Test Unit Ready
>>>>>>>> scsi-disk: Command complete tag=0x0 status=0 sense=0
>>>>>>>> sd3: mode sense (4) returned nonsense; using fictitious geometry
>>>>>>>>
>>>>>>>>
>>>>>>>> NetBSD sent command "0x1a" via Set ATN & Stop, but it for some reason
>>>>>>>> the command got padded and disk got "0x0 0x0 0x1a", no wonder that its
>>>>>>>> output looks like a non-sense to NetBSD.
>>>>>>>>
>>>>>>>> Any ideas why does it happen?
>>>>>>>>
>>>>>>>
>>>>>>> The problem could be in the DMA (sparc32_dma.c), or incorrect
>>>>>>> programming of DMA or IOMMU DVMA by NetBSD, (or bug in iommu.c).
>>>>>>
>>>>>> Why DMA? It hits the else branch of "if (s->dma)". Does the command
>>>>>> still get in via DMA?
>>>>>
>>>>> Sorry, I missed that. But is the response also read without DMA?
>>>>>
>>>>
>>>> You mean the disk's response? It doesn't matter, because the disk just
>>>> doesn't get the command.
>>>
>>> Ah, I see. What about FIFO state then, perhaps there are some leftover
>>> bytes (0, 0 could be status + sense?) from the previous command in the
>>> buffer before the command is written there?
>>
>> You were right, it was FIFO, but I ran the tests in a wrong qemu
>> branch. It's sort of funny, because the bug was fixed in the HEAD by
>> my own patch (the "Message accepted" patch).
>>
>> Now the disk gets commands properly, but NetBSD still complains about
>> getting nonsense.
>>
>> One of the reasons is, the disk's geometry has to be explicitly
>> specified via -hdachs , but
>>
>>> But is the response also read without DMA?
>>
>> you are right about this one too. It is read via DMA, and it seems
>> that the response gets shifted by -8 bytes:
>> the follofing hack in hw/sparc32_dma.c makes NetBSD to recognize the 
>> geometry:
>
> Could be a bug in the DMA controller. For example, the feature for
> automatic load of next address is not implemented. IIRC it's not
> available in all versions, so downgrading the controller version may
> help.

Downgrading the controller version didn't change anything. I also
tried to boot with -M LX , to downgrade other components as well, the
result was still the same.

But this brings me to another question: Is there a reason for silent
catching of errors produced by unimplemented features?

I like the way it is implenented in hw/scsi-disk.c: along with DPRINTF
for debugging there is a BADF for reporting unimplemented/unexpected
cases. DPRINTFs may be turned on by a #define, and BADFs are always
on. Shouldn't similar constructs were used for mmu, iommu and other
units with partially implemented funcionality?