Re: [Qemu-devel] [RFC 00/13] hw/m68k: add Apple Machintosh Quadra 800 machine
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
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
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
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
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