From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1MxQhT-0004TL-N8 for qemu-devel@nongnu.org; Mon, 12 Oct 2009 15:36:55 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1MxQhP-0004Pw-RI for qemu-devel@nongnu.org; Mon, 12 Oct 2009 15:36:55 -0400 Received: from [199.232.76.173] (port=55858 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1MxQhP-0004Pt-Ko for qemu-devel@nongnu.org; Mon, 12 Oct 2009 15:36:51 -0400 Received: from mail-bw0-f211.google.com ([209.85.218.211]:56151) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1MxQhO-00008H-Sr for qemu-devel@nongnu.org; Mon, 12 Oct 2009 15:36:51 -0400 Received: by bwz7 with SMTP id 7so3503963bwz.34 for ; Mon, 12 Oct 2009 12:36:48 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: References: From: Blue Swirl Date: Mon, 12 Oct 2009 22:36:27 +0300 Message-ID: Subject: Re: [Qemu-devel] Re: sparc esp NetBSD-guest "sd3: mode sense (4) returned nonsense" Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Artyom Tarasenko Cc: qemu-devel On Thu, Oct 8, 2009 at 7:14 PM, Artyom Tarasenko wrote: > 2009/9/23 Artyom Tarasenko : >> 2009/9/19 Blue Swirl : >>> On Fri, Sep 18, 2009 at 8:26 PM, Artyom Tarasenko >>> wrote: >>>> 2009/9/14 Blue Swirl : >>>>> On Mon, Sep 14, 2009 at 7:47 PM, Artyom Tarasenko >>>>> wrote: >>>>>> 2009/9/14 Blue Swirl : >>>>>>> On Mon, Sep 14, 2009 at 7:29 PM, Artyom Tarasenko >>>>>>> wrote: >>>>>>>> 2009/9/14 Blue Swirl : >>>>>>>>> On Mon, Sep 14, 2009 at 12:32 AM, Artyom Tarasenko >>>>>>>>> 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 wo= rk >>>>>>>>>> though, so NetBSD has to fabricate a disk geometry. >>>>>>>>>> >>>>>>>>>> To make debugging easier I tried to boot an older version - NetB= SD >>>>>>>>>> 1.3.3. And put some extra debugging in esp.c: >>>>>>>>>> >>>>>>>>>> static uint32_t get_cmd(ESPState *s, uint8_t *buf) >>>>>>>>>> { >>>>>>>>>> =C2=A0 =C2=A0uint32_t dmalen; >>>>>>>>>> =C2=A0 =C2=A0int target; >>>>>>>>>> >>>>>>>>>> =C2=A0 =C2=A0target =3D s->wregs[ESP_WBUSID] & BUSID_DID; >>>>>>>>>> =C2=A0 =C2=A0if (s->dma) { >>>>>>>>>> =C2=A0 =C2=A0 =C2=A0 =C2=A0dmalen =3D s->rregs[ESP_TCLO] | (s->r= regs[ESP_TCMID] << 8); >>>>>>>>>> =C2=A0 =C2=A0 =C2=A0 =C2=A0s->dma_memory_read(s->dma_opaque, buf= , dmalen); >>>>>>>>>> =C2=A0 =C2=A0} else { >>>>>>>>>> =C2=A0 =C2=A0 =C2=A0 =C2=A0dmalen =3D s->ti_size; >>>>>>>>>> =C2=A0 =C2=A0 =C2=A0 =C2=A0memcpy(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]); >>>>>>>>>> =C2=A0 =C2=A0 =C2=A0 =C2=A0buf[0] =3D 0; >>>>>>>>>> =C2=A0 =C2=A0} >>>>>>>>>> >>>>>>>>>> qemu-system-sparc -M SS-20 -nographic =C2=A0-hda ~/sparc/miniroo= t-133.fs -m 64 >>>>>>>>>> ... >>>>>>>>>> NON-DMA rptr 0, wptr 1 c0 (0) =C2=A00 =C2=A00 1a =C2=A00 >>>>>>>>>> Set ATN & Stop: cmdlen 3 >>>>>>>>>> scsi-disk: Command: lun=3D0 tag=3D0x0 data=3D0x00 0x00 0x1a 0x00= 0x04 0x00 >>>>>>>>>> scsi-disk: Test Unit Ready >>>>>>>>>> scsi-disk: Command complete tag=3D0x0 status=3D0 sense=3D0 >>>>>>>>>> sd3: mode sense (4) returned nonsense; using fictitious geometry >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> NetBSD sent command "0x1a" via Set ATN & Stop, but it for some r= eason >>>>>>>>>> the command got padded and disk got "0x0 0x0 0x1a", no wonder th= at 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 comman= d >>>>>>>> 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 ju= st >>>>>> doesn't get the command. >>>>> >>>>> Ah, I see. What about FIFO state then, perhaps there are some leftove= r >>>>> bytes (0, 0 could be status + sense?) from the previous command in th= e >>>>> 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 g= eometry: >>> >>> 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 { > =C2=A0 =C2=A0 =C2=A0 =C2=A0struct scsi_mode_header header; > =C2=A0 =C2=A0 =C2=A0 =C2=A0struct scsi_blk_desc blk_desc; > =C2=A0 =C2=A0 =C2=A0 =C2=A0union 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. > 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=3D etc? > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (nb_sectors >= UINT32_MAX) > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 nb_secto= rs =3D UINT32_MAX; Here the indentation was off by one. > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0nb_sectors--; > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0outbuf[3] =3D 8;= /* Block descriptor length. =C2=A0*/ > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0p[0] =3D 0; This is density code (add comment?), but it looks like only some weird optical devices could have nonzero values. > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0p[1] =3D (nb_sec= tors >> 16) & 0xff; > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0p[2] =3D (nb_sec= tors >> 8) & 0xff; > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0p[3] =3D nb_sect= ors & 0xff; > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0p[4] =3D 0; =C2= =A0/* reserved */ > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0p[5] =3D 0; /* b= ytes 5-7 are the sector size in bytes */ > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0p[6] =3D s->clus= ter_size * 2; > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0p[7] =3D 0; > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0p +=3D 8; > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0} > + > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (page =3D=3D 4 ) { Extra space after 4.