Re: [Qemu-devel] [PATCH] Revert "target-sparc: Make cpu_dst local to OP=2 insns"

[Aurelien Jarno]

On Mon, Oct 22, 2012 at 07:29:19AM +1000, Richard Henderson wrote:
> On 2012-10-21 09:17, Aurelien Jarno wrote:
> > On Sun, Oct 21, 2012 at 08:48:52AM +1000, Richard Henderson wrote:
> >> On 2012-10-21 00:48, Aurelien Jarno wrote:
> >>> I am not sure it is the real problem, but at least the optimization of
> >>> using the destination register as a temporary is wrong when the
> >>> instruction might trigger an exception. In that case the result is
> >>> written to the destination register while it should have not.
> >>>
> >>> This reverts commit 5793f2a47e201d251856c7956d6f7907ec0d9f1f.
> >>
> >> Which insn might trigger an exception?  Most OP=2 insns don't.  There's
> >> divide, but that's done out-of-line, so the assignment to dst does not
> >> happen before the exception...
> > 
> > Indeed there a are a few one triggering exception, but I looked too
> > quickly and indeed they do the assignment before. There should be
> > another problem elsewhere as reverting this patch fixes the issue.
> > 
> >> Is this sparc64?  I assume so, since I did test sparc32...
> >>
> > 
> > Yes it's with a sparc64 kernel. I can reproduce the problem with both a 
> > 32 and 64-bit userland, though it happens earlier with a 32-bit
> > userland.
> > 
> 
> Sadly, I don't seem to be able to boot a "real" sparc64 system at all,
> even with a qemu version before the patch set in question went in.
> 
> (1) The recent "vga cleanup" has totally broken video for all sparc.
>     This is bisectable to f2898771435701df33145cacabeb42c6aa3a9a16,
>     your patch that adjusts sun4u.  One can see this problem just by
>     booting OpenBIOS; no kernel needed.

Oops, it's something I haven't realized, because my scripts are either
passing -vga none or -vga std. I'll try to come with a patch, but in the
meantime the problem can be workarounded by passing -vga std.

> (2) Switching to serial console, the kernel gets stuck talking to the
>     disks.  It gets so far, and then reports lost interrupts forever:
> 
> $ ../bld/sparc64-softmmu/qemu-system-sparc64 -kernel sparc64 -initrd 
> initrd.gz -append "root=/dev/ram console=ttyS0" -hda ./hda.img -cdrom 
> ./debian-6.0.6-sparc-netinst.iso -nographic
> ...
> [   14.488838] ata1.00: ATA-7: QEMU HARDDISK, 1.2.50, max UDMA/100
> [   14.489893] ata1.00: 25165824 sectors, multi 16: LBA48 
> [   14.504620] ata1.00: configured for UDMA/33
> [   14.549962] scsi 0:0:0:0: Direct-Access     ATA      QEMU HARDDISK    1.2. 
> PQ: 0 ANSI: 5
> [   14.732381] ata2.00: ATAPI: QEMU DVD-ROM, 1.2.50, max UDMA/100
> [   14.734341] ata2.00: configured for UDMA/33
> [   35.596592] ata2: lost interrupt (Status 0x50)
> [   56.594942] ata2: lost interrupt (Status 0x50)

Yes, the emulated IDE controller is currently buggy and causes the
kernel to go in a dead loop. Instead of using Debian Squeeze, I am using
Debian Wheezy which provides virtio support. Then the system can boot
with virtio-disk and virtio-net.

> I re-examined the opcodes in OP=2.  The only one left that throws an inline 
> exception
> is Tcc, which of course doesn't write to a destination register at all.  If, 
> somehow,
> there is some exception (or other write-before-use) that I've missed, then 
> the following
> patch should fix it, without moving the cpu_dst variable back to global 
> scope.  If this
> patch does not fix the problem, then there's some erroneous use of cpu_dst 
> that I've
> missed, and the original patch is question is broken for some other reason.

The attached patch fixes the problem.

> Given my experience above, I wonder what you're testing differently?  I'm 
> trying the
> stock standard debian 6.0.6 (current?) distribution media images.  The 
> kernel/initrd
> images above are pulled out of that iso, as I couldn't make OpenBIOS boot the 
> cdrom
> image directly.


I'll try to make an image so that people can test sparc64 more easily.

 

-- 
Aurelien Jarno                          GPG: 1024D/F1BCDB73
address@hidden                 http://www.aurel32.net