Re: [Qemu-devel] [RFC 00/13] hw/m68k: add Apple Machintosh Quadra 800 machine

2018-06-08 Thread no-reply
Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20180608200558.386-1-laur...@vivier.eu
Subject: [Qemu-devel] [RFC 00/13] hw/m68k: add Apple Machintosh Quadra 800 
machine

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
failed=1
echo
fi
n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
30b6069da2 dp8393x: fix receiving buffer exhaustion
a9294684d2 dp8393x: put DMA temp buffer in the state, not in the stack
8f1c639ffd dp8393x: manage big endian bus
7b43df7a43 dp8393x: fix dp8393x_receive
c87eb03bf7 hw/m68k: define Macintosh Quadra 800
742c9c5459 hw/m68k: add a dummy SWIM floppy controller
2d712f9df2 hw/m68k: add Nubus support
6fc4d6d265 ESP: add pseudo-DMA as used by Macintosh
105bc32850 hw/m68k: Apple Sound Chip (ASC) emulation
030037ad0b hw/m68k: add video card
8a99fff7de escc: introduce a selector for the register bit
20944305a6 ADB: VIA probes ADB bus when it is idle
60cce9bbe9 hw/m68k: add via support

=== OUTPUT BEGIN ===
Checking PATCH 1/13: hw/m68k: add via support...
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#145: 
new file mode 100644

ERROR: space prohibited after that '&&' (ctx:WxW)
#622: FILE: hw/misc/mac_via.c:473:
+if (!(s->last_b & VIA1B_vRTCClk) && (s->b & VIA1B_vRTCClk)) {
  ^

total: 1 errors, 1 warnings, 1126 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 2/13: ADB: VIA probes ADB bus when it is idle...
Checking PATCH 3/13: escc: introduce a selector for the register bit...
Checking PATCH 4/13: hw/m68k: add video card...
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#37: 
new file mode 100644

total: 0 errors, 1 warnings, 475 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 5/13: hw/m68k: Apple Sound Chip (ASC) emulation...
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#21: 
new file mode 100644

total: 0 errors, 1 warnings, 517 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 6/13: ESP: add pseudo-DMA as used by Macintosh...
Checking PATCH 7/13: hw/m68k: add Nubus support...
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#158: 
new file mode 100644

total: 0 errors, 1 warnings, 739 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 8/13: hw/m68k: add a dummy SWIM floppy controller...
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#22: 
new file mode 100644

total: 0 errors, 1 warnings, 332 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 9/13: hw/m68k: define Macintosh Quadra 800...
Argument "m" isn't numeric in numeric eq (==) at ./scripts/checkpatch.pl line 
2665.
Argument "m" isn't numeric in numeric eq (==) at ./scripts/checkpatch.pl line 
2665.
Use of uninitialized value $1 in concatenation (.) or string at 
./scripts/checkpatch.pl line 2666.
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#110: 
new file mode 100644

WARNING: line over 80 characters
#206: FILE: hw/m68k/bootinfo.h:92:
+stw_phys(as, base, (sizeof(struct bi_record) + strlen(string) + 2) & 
~1); \

ERROR: unnecessary cast may hide bugs, use g_new0 instead
#387: FILE: hw/m68k/mac.c:168:
+s = (q800_glue_state_t *)g_malloc0(sizeof(q800_glue_state_t));

total: 1 errors, 2 warnings, 614 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 10/13: dp8393x: fix dp8393x_receive...
Checking PATCH 11/13: dp8393x: manage big endian bus...
ERROR: do not initialise statics to 0 or NULL
#31: FILE: hw/net/dp8393x.c:181:
+static const bool host_big_endian = false;

total: 1 errors, 0 warnings, 190 lines checked

Your patch has style problems, please re

Re: [Qemu-devel] [RFC 00/13] hw/m68k: add Apple Machintosh Quadra 800 machine

2018-06-08 Thread Philippe Mathieu-Daudé
On 06/08/2018 05:05 PM, Laurent Vivier wrote:
> I'm rebasing some of these patches for seven years now,
> too many years...
> 
> It's an RFC because things have changed in QEMU in seven years,
> for instance the VIA has a new implementation (mos6522) introduced
> by Mark Cave-Ayland and I didn't rework my implementation to
> fit into this new one (any volunteers?), display has some glitches,
> ADB devices are not identified correctly.
> 
> if you want to test the machine, I'm sorry, it doesn't boot
> a MacROM, but you can boot a linux kernel from the command line.
> 
> You can install your own disk using debian-installer, with:
> 
> ...
> -M q800 \
> -serial none -serial mon:stdio \
> -m 1000M -drive file=m68k.qcow2,format=qcow2 \
> -net nic,model=dp83932,addr=09:00:07:12:34:57 \
> -append "console=ttyS0 vga=off" \
> -kernel vmlinux-4.15.0-2-m68k \
> -initrd initrd.gz \
> -drive file=debian-9.0-m68k-NETINST-1.iso \
> -drive file=m68k.qcow2,format=qcow2 \
> -nographic

qemu-system-m68k: -drive file=m68k.qcow2,format=qcow2: Failed to get
"write" lock
Is another process using the image?

(two times same file provided in cmdline arguments)

@block-team I found it funny because this is the very same process which
already locked. The error message is enough as it imo.



Re: [Qemu-devel] [RFC 00/13] hw/m68k: add Apple Machintosh Quadra 800 machine

2018-06-09 Thread Laurent Vivier
Le 09/06/2018 à 05:26, Philippe Mathieu-Daudé a écrit :
> On 06/08/2018 05:05 PM, Laurent Vivier wrote:
>> I'm rebasing some of these patches for seven years now,
>> too many years...
>>
>> It's an RFC because things have changed in QEMU in seven years,
>> for instance the VIA has a new implementation (mos6522) introduced
>> by Mark Cave-Ayland and I didn't rework my implementation to
>> fit into this new one (any volunteers?), display has some glitches,
>> ADB devices are not identified correctly.
>>
>> if you want to test the machine, I'm sorry, it doesn't boot
>> a MacROM, but you can boot a linux kernel from the command line.
>>
>> You can install your own disk using debian-installer, with:
>>
>> ...
>> -M q800 \
>> -serial none -serial mon:stdio \
>> -m 1000M -drive file=m68k.qcow2,format=qcow2 \
>> -net nic,model=dp83932,addr=09:00:07:12:34:57 \
>> -append "console=ttyS0 vga=off" \
>> -kernel vmlinux-4.15.0-2-m68k \
>> -initrd initrd.gz \
>> -drive file=debian-9.0-m68k-NETINST-1.iso \
>> -drive file=m68k.qcow2,format=qcow2 \
>> -nographic
> 
> qemu-system-m68k: -drive file=m68k.qcow2,format=qcow2: Failed to get
> "write" lock
> Is another process using the image?
> 
> (two times same file provided in cmdline arguments)

Yes, you're right. A bad cut'n'paste.

Thanks,
Laurent




Re: [Qemu-devel] [RFC 00/13] hw/m68k: add Apple Machintosh Quadra 800 machine

2018-06-09 Thread Philippe Mathieu-Daudé
Hi Laurent,

On 06/08/2018 05:05 PM, Laurent Vivier wrote:
> if you want to test the machine, I'm sorry, it doesn't boot
> a MacROM, but you can boot a linux kernel from the command line.
> 
> You can install your own disk using debian-installer, with:
> 
> ...
> -M q800 \
> -serial none -serial mon:stdio \
> -m 1000M -drive file=m68k.qcow2,format=qcow2 \
> -net nic,model=dp83932,addr=09:00:07:12:34:57 \
> -append "console=ttyS0 vga=off" \
> -kernel vmlinux-4.15.0-2-m68k \
> -initrd initrd.gz \
> -drive file=debian-9.0-m68k-NETINST-1.iso \
> -drive file=m68k.qcow2,format=qcow2 \
> -nographic
> 
> If you use a graphic adapter instead of "-nographic", you can use "-g" to set 
> the
> size of the display (I use "-g 1600x800x24").
> 
> You can get the ISO from:
> 
> https://cdimage.debian.org/mirror/cdimage/ports/9.0/m68k/iso-cd/debian-9.0-m68k-NETINST-1.iso
> 
> and extract the kernel and initrd.gz:
> 
> guestfish --add debian-9.0-m68k-NETINST-1.iso --ro \
>   --mount /dev/sda:/ <<_EOF_
> copy-out /install/cdrom/initrd.gz .
> copy-out /install/kernels/vmlinux-4.15.0-2-m68k .
> _EOF_

Running with -d in_asm,int I get:


IN: nf_get_id
0xd432:  movel %a3,%d0
0xd434:  addil #0,%d0
0xd43a:  movel %d0,%sp@-
0xd43c:  jsr 0xd404


IN:
0xd404:  071400

INT  1: Unassigned(0xf4) pc=d404 sp=00393e60 sr=2700
INT  2: Access Fault(0x8) pc= sp=00393e58 sr=2700
ssw:  0506 ea:    sfc:  5dfc: 5


IN:
0x280c:  clrl %sp@-
0x280e:  pea 0x
0x2812:  movel %d0,%sp@-
0x2814:  moveml %d1-%d5/%a0-%a2,%sp@-
0x2818:  movel %sp,%d0
0x281a:  andil #-8192,%d0
0x2820:  moveal %d0,%a2
0x2822:  moveal %a2@,%a2
0x2824:  movel %sp,%sp@-
0x2826:  bsrl 0x557c


IN: buserr_c
0x557c:  subql #4,%sp
0x557e:  moveml %d2-%d7/%a3-%fp,%sp@-
0x5582:  moveal %sp@(48),%a3
0x5586:  btst #5,%a3@(44)
0x558c:  bnes 0x5592

...


IN: panic
0x0002c956:  moveal 0x39503c,%a0
0x0002c95c:  moveq #101,%d1
0x0002c95e:  subql #1,%d1
0x0002c960:  bnes 0x2c9c6

objdump -S gives:

d404 :
d404:   7300mvsb %d0,%d1
d406:   4e75rts

Instruction which exists in the disas code, but doesn't seem
tcg-implemented:

disas/m68k.c:3654:{"mvsb", 2,   one(0070400),   one(0170700), "*bDd",
mcfisa_b },

> 
> The mirror to use is: http://ftp.ports.debian.org/debian-ports/
> when it fails, continue without boot loader.
> 
> In the same way, you can extract the kernel and the initramfs from the qcow2
> image to use it with "-kernel" and "-initrd":
> 
> guestfish --add m68k.qcow2 --mount /dev/sda2:/ <<_EOF_
> copy-out /boot/vmlinux-4.15.0-2-m68k .
> copy-out /boot/initrd.img-4.15.0-2-m68k .
> _EOF_
> 
> and boot with:
> 
>...
>-append "root=/dev/sda2 rw console=ttyS0 console=tty \
>-kernel vmlinux-4.15.0-2-m68k \
>-initrd initrd.img-4.15.0-2-m68k



Re: [Qemu-devel] [RFC 00/13] hw/m68k: add Apple Machintosh Quadra 800 machine

2018-06-09 Thread Thomas Huth
On 09.06.2018 16:25, Philippe Mathieu-Daudé wrote:
> Hi Laurent,
> 
> On 06/08/2018 05:05 PM, Laurent Vivier wrote:
>> if you want to test the machine, I'm sorry, it doesn't boot
>> a MacROM, but you can boot a linux kernel from the command line.
>>
>> You can install your own disk using debian-installer, with:
>>
>> ...
>> -M q800 \
>> -serial none -serial mon:stdio \
>> -m 1000M -drive file=m68k.qcow2,format=qcow2 \
>> -net nic,model=dp83932,addr=09:00:07:12:34:57 \
>> -append "console=ttyS0 vga=off" \
>> -kernel vmlinux-4.15.0-2-m68k \
>> -initrd initrd.gz \
>> -drive file=debian-9.0-m68k-NETINST-1.iso \
>> -drive file=m68k.qcow2,format=qcow2 \
>> -nographic
>>
>> If you use a graphic adapter instead of "-nographic", you can use "-g" to 
>> set the
>> size of the display (I use "-g 1600x800x24").
>>
>> You can get the ISO from:
>>
>> https://cdimage.debian.org/mirror/cdimage/ports/9.0/m68k/iso-cd/debian-9.0-m68k-NETINST-1.iso
>>
>> and extract the kernel and initrd.gz:
>>
>> guestfish --add debian-9.0-m68k-NETINST-1.iso --ro \
>>   --mount /dev/sda:/ <<_EOF_
>> copy-out /install/cdrom/initrd.gz .
>> copy-out /install/kernels/vmlinux-4.15.0-2-m68k .
>> _EOF_
> 
> Running with -d in_asm,int I get:
> 
> 
> IN: nf_get_id
> 0xd432:  movel %a3,%d0
> 0xd434:  addil #0,%d0
> 0xd43a:  movel %d0,%sp@-
> 0xd43c:  jsr 0xd404
> 
> 
> IN:
> 0xd404:  071400
> 
> INT  1: Unassigned(0xf4) pc=d404 sp=00393e60 sr=2700
> INT  2: Access Fault(0x8) pc= sp=00393e58 sr=2700
> ssw:  0506 ea:    sfc:  5dfc: 5
> 
> 
> IN:
> 0x280c:  clrl %sp@-
> 0x280e:  pea 0x
> 0x2812:  movel %d0,%sp@-
> 0x2814:  moveml %d1-%d5/%a0-%a2,%sp@-
> 0x2818:  movel %sp,%d0
> 0x281a:  andil #-8192,%d0
> 0x2820:  moveal %d0,%a2
> 0x2822:  moveal %a2@,%a2
> 0x2824:  movel %sp,%sp@-
> 0x2826:  bsrl 0x557c
> 
> 
> IN: buserr_c
> 0x557c:  subql #4,%sp
> 0x557e:  moveml %d2-%d7/%a3-%fp,%sp@-
> 0x5582:  moveal %sp@(48),%a3
> 0x5586:  btst #5,%a3@(44)
> 0x558c:  bnes 0x5592
> 
> ...
> 
> 
> IN: panic
> 0x0002c956:  moveal 0x39503c,%a0
> 0x0002c95c:  moveq #101,%d1
> 0x0002c95e:  subql #1,%d1
> 0x0002c960:  bnes 0x2c9c6
> 
> objdump -S gives:
> 
> d404 :
> d404:   7300mvsb %d0,%d1
> d406:   4e75rts
> 
> Instruction which exists in the disas code, but doesn't seem
> tcg-implemented:
> 
> disas/m68k.c:3654:{"mvsb", 2,   one(0070400),   one(0170700), "*bDd",
> mcfisa_b },

0x7300 is the illegal opcode that is used by the Aranym emulator for its
"Native Feature" (some kind of Hypercall) interface:

https://github.com/aranym/aranym/wiki/natfeats-proposal#special-opcodes

It's also a valid opcode in the ColdFire ISA (that's why the
disassembler detects this as valid instruction), but the name of the
function (nf_get_id_phys) clearly indicates that Linux is trying to use
the Natfeats opcode here.

So it's normal that this opcode is not implemented in QEMU 680x0 mode.
If Linux correctly catches the illegal opcode exception afterwards,
everything is fine and you don't need to worry about this anymore.
However, if Linux fails to catch it correctly, there is certainly
something wrong here...

 Thomas