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

[Blue Swirl]

On Tue, Oct 13, 2009 at 11:10 PM, Artyom Tarasenko
<address@hidden> wrote:
> 2009/10/12 Blue Swirl <address@hidden>:
>> On Thu, Oct 8, 2009 at 7:14 PM, Artyom Tarasenko
>> <address@hidden> wrote:
>>> 2009/9/23 Artyom Tarasenko <address@hidden>:
>>>> 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?
>>>
>>>
>>> Actually, scsi-disk.c doesn't implement block descriptor for mode
>>> pages. The SCSI-2 documentation suggests, that although the block
>>> descriptor is optional for an arbitrary SCSI-2 device (chapter 8.2.10,
>>> http://ldkelley.com/SCSI2/SCSI2/SCSI2/SCSI2/SCSI2-08.html ) it is
>>> mandatory for a disk: chapters 9.1.2, 9.3.3 (
>>> http://ldkelley.com/SCSI2/SCSI2/SCSI2/SCSI2-09.html ) don't say
>>> "optional" any more, just "The block descriptor in the MODE SENSE data
>>> describes the block lengths that are used on the medium."
>>
>> I agree.
>>
>>> NetBSD expects that the block descriptor is always there:
>>> sd.c:
>>>
>>> struct scsi_mode_sense_data {
>>>        struct scsi_mode_header header;
>>>        struct scsi_blk_desc blk_desc;
>>>        union scsi_disk_pages pages;
>>> };
>>>
>>> Shall we implement the block descriptor? We can start with the
>>> following, which fixes NetBSD geometry detection. Shall I post it as a
>>> patch?
>>
>> Yes, please. I did not see any difference with NetBSD (2.1, 3.0 or
>> 4.0) Sparc32 guest, though.
>
> Just tested with 4.0.1 and it worked.
>
> Did you specify -hdachs? Although the docu says "Usually QEMU can
> guess all those parameters.", I don't observe this actually happening.
> What is probably meant here "Usually a guest OS can guess all those
> parameters."
>
>>> And there is one more problem regarding the disk geometry. The
>>> "-hdachs" command line switch's sanity check seems to be IDE-specific:
>>> for instance it doesn't accept "-hdachs 6,64,32". Is there an
>>> alternative way to specify the SCSI disk geometry?
>>
>> I haven't tried, but does -drive handle cyls= etc?
>
> Yes, but it seems to have the same limitation:
>
> qemu-system-sparc -nographic  -drive
> file=NetBSD-4.0.1-miniroot,cyls=8,heads=64,secs=32,media=disk -m 64 -M
> SS-20
> qemu: '(null)' invalid physical heads number

This patch should help. I think only IDE needs the CHS limits.

>> Extra space after 4.
>
> Fixed.

Thanks, applied.

0001-Only-IDE-needs-limits-for-CHS-configuration.patch
Description: Source code patch