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

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

[Jason A. Donenfeld]

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(+)

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,
-- 
2.24.1

Re: [PATCH] q800: implement mac rom reset function for BIOS-less mode

[no-reply]

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

Re: [PATCH] q800: implement mac rom reset function for BIOS-less mode

[no-reply]

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

Re: [PATCH] q800: implement mac rom reset function for BIOS-less mode

[Laurent Vivier]

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

Re: [PATCH] q800: implement mac rom reset function for BIOS-less mode

[Laurent Vivier]

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

[PATCH v2] q800: implement mac rom reset function for BIOS-less mode

[Jason A. Donenfeld]

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. We also correct the value of ROMBASE while we're at it.

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

diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
index 4ca8678007..f03679e24c 100644
--- a/hw/m68k/q800.c
+++ b/hw/m68k/q800.c
@@ -47,7 +47,7 @@
 #include "sysemu/runstate.h"
 #include "sysemu/reset.h"
 
-#define MACROM_ADDR     0x40000000
+#define MACROM_ADDR     0x40800000
 #define MACROM_SIZE     0x00100000
 
 #define MACROM_FILENAME "MacROM.bin"
@@ -128,6 +128,27 @@ 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 */
+
+    /* via2[vDirB] |= VIA2B_vPower */
+    0x20, 0x7C, 0x50, 0xF0, 0x24, 0x00, /* moveal VIA2_BASE+vDirB,%a0 */
+    0x10, 0x10,                         /* moveb %a0@,%d0 */
+    0x00, 0x00, 0x00, 0x04,             /* orib #4,%d0 */
+    0x10, 0x80,                         /* moveb %d0,%a0@ */
+
+    /* via2[vBufB] &= ~VIA2B_vPower */
+    0x20, 0x7C, 0x50, 0xF0, 0x20, 0x00, /* moveal VIA2_BASE+vBufB,%a0 */
+    0x10, 0x10,                         /* moveb %a0@,%d0 */
+    0x02, 0x00, 0xFF, 0xFB,             /* andib #-5,%d0 */
+    0x10, 0x80,                         /* moveb %d0,%a0@ */
+
+    /* while (true) ; */
+    0x60, 0xFE                          /* bras [self] */
+};
+
 static void q800_init(MachineState *machine)
 {
     M68kCPU *cpu = NULL;
@@ -340,6 +361,12 @@ static void q800_init(MachineState *machine)
                   (graphic_width * graphic_depth + 7) / 8);
         BOOTINFO1(cs->as, parameters_base, BI_MAC_SCCBASE, SCC_BASE);
 
+        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,
                         kernel_cmdline);
-- 
2.24.1

Re: [PATCH v2] q800: implement mac rom reset function for BIOS-less mode

[Laurent Vivier]

Le 02/01/2020 à 13:01, 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. We also correct the value of ROMBASE while we're at it.
> 
> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
> ---
>  hw/m68k/q800.c | 29 ++++++++++++++++++++++++++++-
>  1 file changed, 28 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
> index 4ca8678007..f03679e24c 100644
> --- a/hw/m68k/q800.c
> +++ b/hw/m68k/q800.c
> @@ -47,7 +47,7 @@
>  #include "sysemu/runstate.h"
>  #include "sysemu/reset.h"
>  
> -#define MACROM_ADDR     0x40000000
> +#define MACROM_ADDR     0x40800000
>  #define MACROM_SIZE     0x00100000
>  
>  #define MACROM_FILENAME "MacROM.bin"
> @@ -128,6 +128,27 @@ 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 */
> +
> +    /* via2[vDirB] |= VIA2B_vPower */
> +    0x20, 0x7C, 0x50, 0xF0, 0x24, 0x00, /* moveal VIA2_BASE+vDirB,%a0 */
> +    0x10, 0x10,                         /* moveb %a0@,%d0 */
> +    0x00, 0x00, 0x00, 0x04,             /* orib #4,%d0 */
> +    0x10, 0x80,                         /* moveb %d0,%a0@ */
> +
> +    /* via2[vBufB] &= ~VIA2B_vPower */
> +    0x20, 0x7C, 0x50, 0xF0, 0x20, 0x00, /* moveal VIA2_BASE+vBufB,%a0 */
> +    0x10, 0x10,                         /* moveb %a0@,%d0 */
> +    0x02, 0x00, 0xFF, 0xFB,             /* andib #-5,%d0 */
> +    0x10, 0x80,                         /* moveb %d0,%a0@ */
> +
> +    /* while (true) ; */
> +    0x60, 0xFE                          /* bras [self] */
> +};
> +
>  static void q800_init(MachineState *machine)
>  {
>      M68kCPU *cpu = NULL;
> @@ -340,6 +361,12 @@ static void q800_init(MachineState *machine)
>                    (graphic_width * graphic_depth + 7) / 8);
>          BOOTINFO1(cs->as, parameters_base, BI_MAC_SCCBASE, SCC_BASE);
>  
> +        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,
>                          kernel_cmdline);
> 

Reviewed-by: Laurent Vivier <laurent@vivier.eu>

Re: [PATCH] q800: implement mac rom reset function for BIOS-less mode

[Jason A. Donenfeld]

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.