diff mbox series

q800: implement mac rom reset function for BIOS-less mode

Message ID 20200102103644.233370-1-Jason@zx2c4.com (mailing list archive)
State New, archived
Headers show
Series q800: implement mac rom reset function for BIOS-less mode | expand

Commit Message

Jason A. Donenfeld Jan. 2, 2020, 10:36 a.m. UTC
On Linux, calling `reboot(RB_AUTOBOOT);` will result in
arch/m68k/mac/misc.c's mac_reset function being called. That in turn
looks at the rombase (or uses 0x40800000 is there's no rombase), adds
0xa, and jumps to that address. At the moment, there's nothing there, so
the kernel just crashes when trying to reboot. So, this commit adds a
very simple implementation at that location, which just writes to via2
to power down.

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
 hw/m68k/q800.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

Comments

no-reply@patchew.org Jan. 2, 2020, 10:42 a.m. UTC | #1
Patchew URL: https://patchew.org/QEMU/20200102103644.233370-1-Jason@zx2c4.com/



Hi,

This series failed the docker-quick@centos7 build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
make docker-image-centos7 V=1 NETWORK=1
time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1
=== TEST SCRIPT END ===




The full log is available at
http://patchew.org/logs/20200102103644.233370-1-Jason@zx2c4.com/testing.docker-quick@centos7/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
no-reply@patchew.org Jan. 2, 2020, 10:43 a.m. UTC | #2
Patchew URL: https://patchew.org/QEMU/20200102103644.233370-1-Jason@zx2c4.com/



Hi,

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

Subject: [PATCH] q800: implement mac rom reset function for BIOS-less mode
Type: series
Message-id: 20200102103644.233370-1-Jason@zx2c4.com

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

fatal: unable to write new index file
warning: Clone succeeded, but checkout failed.
You can inspect what was checked out with 'git status'
and retry the checkout with 'git checkout -f HEAD'

Traceback (most recent call last):
  File "patchew-tester2/src/patchew-cli", line 531, in test_one
    git_clone_repo(clone, r["repo"], r["head"], logf, True)
  File "patchew-tester2/src/patchew-cli", line 62, in git_clone_repo
    subprocess.check_call(clone_cmd, stderr=logf, stdout=logf)
  File "/opt/rh/rh-python36/root/usr/lib64/python3.6/subprocess.py", line 291, in check_call
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['git', 'clone', '-q', '/home/patchew2/.cache/patchew-git-cache/httpsgithubcompatchewprojectqemu-3c8cf5a9c21ff8782164d1def7f44bd888713384', '/var/tmp/patchew-tester-tmp-h4dvlwzu/src']' returned non-zero exit status 128.



The full log is available at
http://patchew.org/logs/20200102103644.233370-1-Jason@zx2c4.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
Laurent Vivier Jan. 2, 2020, 11:10 a.m. UTC | #3
Le 02/01/2020 à 11:36, Jason A. Donenfeld a écrit :
> On Linux, calling `reboot(RB_AUTOBOOT);` will result in
> arch/m68k/mac/misc.c's mac_reset function being called. That in turn
> looks at the rombase (or uses 0x40800000 is there's no rombase), adds
> 0xa, and jumps to that address. At the moment, there's nothing there, so
> the kernel just crashes when trying to reboot. So, this commit adds a
> very simple implementation at that location, which just writes to via2
> to power down.
> 
> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
> ---

There are two cleaners solution to do that:
1- catch the jump to the ROM address in QEMU and shutdown the machine, see

https://github.com/vivier/qemu-m68k/commit/51cd57d1128059819038b9800455fbf794430c15

2- or as you do, write a fake ROM but use the VIA2 port B bit 3 to
shutdown the machine see hw/misc/mac_via.c mos6522_q800_via2_portB_write().

Thanks,
Laurent
Laurent Vivier Jan. 2, 2020, 11:41 a.m. UTC | #4
Le 02/01/2020 à 12:10, Laurent Vivier a écrit :
> Le 02/01/2020 à 11:36, Jason A. Donenfeld a écrit :
>> On Linux, calling `reboot(RB_AUTOBOOT);` will result in
>> arch/m68k/mac/misc.c's mac_reset function being called. That in turn
>> looks at the rombase (or uses 0x40800000 is there's no rombase), adds
>> 0xa, and jumps to that address. At the moment, there's nothing there, so
>> the kernel just crashes when trying to reboot. So, this commit adds a
>> very simple implementation at that location, which just writes to via2
>> to power down.
>>
>> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
>> ---
> 
> There are two cleaners solution to do that:
> 1- catch the jump to the ROM address in QEMU and shutdown the machine, see
> 
> https://github.com/vivier/qemu-m68k/commit/51cd57d1128059819038b9800455fbf794430c15
> 
> 2- or as you do, write a fake ROM but use the VIA2 port B bit 3 to
> shutdown the machine see hw/misc/mac_via.c mos6522_q800_via2_portB_write().

OK, 2 is what you do, so I think we can take this.

The assembly code is correct but not easy to read, could you use defined
values (VIA_BASE) and add some comments?

I think we don't need the BI_MAC_ROMBASE: in fact MACROM_ADDR is wrong,
you can change its definition to 0x40800000.

Thanks,
Laurent
Jason A. Donenfeld Jan. 2, 2020, 1:41 p.m. UTC | #5
On Thu, Jan 2, 2020 at 12:41 PM Laurent Vivier <laurent@vivier.eu> wrote:
>
> Le 02/01/2020 à 12:10, Laurent Vivier a écrit :
> > Le 02/01/2020 à 11:36, Jason A. Donenfeld a écrit :
> >> On Linux, calling `reboot(RB_AUTOBOOT);` will result in
> >> arch/m68k/mac/misc.c's mac_reset function being called. That in turn
> >> looks at the rombase (or uses 0x40800000 is there's no rombase), adds
> >> 0xa, and jumps to that address. At the moment, there's nothing there, so
> >> the kernel just crashes when trying to reboot. So, this commit adds a
> >> very simple implementation at that location, which just writes to via2
> >> to power down.
> >>
> >> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
> >> ---
> >
> > There are two cleaners solution to do that:
> > 1- catch the jump to the ROM address in QEMU and shutdown the machine, see
> >
> > https://github.com/vivier/qemu-m68k/commit/51cd57d1128059819038b9800455fbf794430c15
> >
> > 2- or as you do, write a fake ROM but use the VIA2 port B bit 3 to
> > shutdown the machine see hw/misc/mac_via.c mos6522_q800_via2_portB_write().
>
> OK, 2 is what you do, so I think we can take this.
>
> The assembly code is correct but not easy to read, could you use defined
> values (VIA_BASE) and add some comments?
>
> I think we don't need the BI_MAC_ROMBASE: in fact MACROM_ADDR is wrong,
> you can change its definition to 0x40800000.
>
> Thanks,
> Laurent

I've addressed your comments and submitted v2.
diff mbox series

Patch

diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
index 4ca8678007..491ba11200 100644
--- a/hw/m68k/q800.c
+++ b/hw/m68k/q800.c
@@ -128,6 +128,20 @@  static void main_cpu_reset(void *opaque)
     cpu->env.pc = ldl_phys(cs->as, 4);
 }
 
+static uint8_t fake_mac_rom[] = {
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    /* offset: 0xa - mac_reset */
+    0x20, 0x7C, 0x50, 0xF0, 0x24, 0x00,	/* moveal #1357915136,%a0 */
+    0x10, 0x10,				/* moveb %a0@,%d0 */
+    0x00, 0x00, 0x00, 0x04,		/* orib #4,%d0 */
+    0x10, 0x80,				/* moveb %d0,%a0@ */
+    0x20, 0x7C, 0x50, 0xF0, 0x20, 0x00,	/* moveal #1357914112,%a0 */
+    0x10, 0x10,				/* moveb %a0@,%d0 */
+    0x02, 0x00, 0xFF, 0xFB,		/* andib #-5,%d0 */
+    0x10, 0x80,				/* moveb %d0,%a0@ */
+    0x60, 0xFE				/* bras [self] */
+};
+
 static void q800_init(MachineState *machine)
 {
     M68kCPU *cpu = NULL;
@@ -339,6 +353,13 @@  static void q800_init(MachineState *machine)
         BOOTINFO1(cs->as, parameters_base, BI_MAC_VROW,
                   (graphic_width * graphic_depth + 7) / 8);
         BOOTINFO1(cs->as, parameters_base, BI_MAC_SCCBASE, SCC_BASE);
+        BOOTINFO1(cs->as, parameters_base, BI_MAC_ROMBASE, MACROM_ADDR);
+
+        rom = g_malloc(sizeof(*rom));
+        memory_region_init_ram_ptr(rom, NULL, "m68k_fake_mac.rom",
+                                   sizeof(fake_mac_rom), fake_mac_rom);
+        memory_region_set_readonly(rom, true);
+        memory_region_add_subregion(get_system_memory(), MACROM_ADDR, rom);
 
         if (kernel_cmdline) {
             BOOTINFOSTR(cs->as, parameters_base, BI_COMMAND_LINE,