[RFC PATCH 0/3] OpenRISC Semihosting and Virt

[RFC PATCH 0/3] OpenRISC Semihosting and Virt

[Stafford Horne]

Hello,

I provide 2 options here 2 help with OpenRISC CI testing sush as the wireguard
testing that Jason has been working on.

The two are:

 1. Add semihosting to openrisc to handle l.nop based Halt and Reset
 2. Define a new virt platform, this includes widing in the sifive test device
    that provides a syscon interface to allow for shutdown and reboot.

We could upstream both of them or just one. I am leaning toward dropping the
semi-hosting work and just going ahead with virt.

The semi-hosting stuff was discussed here:

 - https://www.mail-archive.com/qemu-devel@nongnu.org/msg884560.html

Also, I started to propose architecture changes here:

 - https://github.com/openrisc/openrisc.github.io/pull/18

However, looking at how other platforms define semihosting it seems the openrisc
requirements are quite different.  We do not use the open/close/write etc
syscalls as riscv, arm etc do.

Stafford Horne (3):
  target/openrisc: Add basic support for semihosting
  hw/openrisc: Split re-usable boot time apis out to boot.c
  hw/openrisc: Add the OpenRISC virtual machine

 configs/devices/or1k-softmmu/default.mak |   3 +
 hw/openrisc/Kconfig                      |   9 +
 hw/openrisc/boot.c                       | 127 +++++++
 hw/openrisc/meson.build                  |   2 +
 hw/openrisc/openrisc_sim.c               | 106 +-----
 hw/openrisc/virt.c                       | 429 +++++++++++++++++++++++
 include/hw/openrisc/boot.h               |  34 ++
 qemu-options.hx                          |  16 +-
 target/openrisc/cpu.h                    |   2 +
 target/openrisc/helper.h                 |   1 +
 target/openrisc/meson.build              |   1 +
 target/openrisc/openrisc-semi.c          |  54 +++
 target/openrisc/sys_helper.c             |   5 +
 target/openrisc/translate.c              |   6 +
 14 files changed, 690 insertions(+), 105 deletions(-)
 create mode 100644 hw/openrisc/boot.c
 create mode 100644 hw/openrisc/virt.c
 create mode 100644 include/hw/openrisc/boot.h
 create mode 100644 target/openrisc/openrisc-semi.c

-- 
2.31.1

[RFC PATCH 0/3] OpenRISC Semihosting and Virt

[Stafford Horne]

Hello,

I provide 2 options here 2 help with OpenRISC CI testing sush as the wireguard
testing that Jason has been working on.

The two are:

 1. Add semihosting to openrisc to handle l.nop based Halt and Reset
 2. Define a new virt platform, this includes widing in the sifive test device
    that provides a syscon interface to allow for shutdown and reboot.

We could upstream both of them or just one. I am leaning toward dropping the
semi-hosting work and just going ahead with virt.

The semi-hosting stuff was discussed here:

 - https://www.mail-archive.com/qemu-devel@nongnu.org/msg884560.html

Also, I started to propose architecture changes here:

 - https://github.com/openrisc/openrisc.github.io/pull/18

However, looking at how other platforms define semihosting it seems the openrisc
requirements are quite different.  We do not use the open/close/write etc
syscalls as riscv, arm etc do.

Stafford Horne (3):
  target/openrisc: Add basic support for semihosting
  hw/openrisc: Split re-usable boot time apis out to boot.c
  hw/openrisc: Add the OpenRISC virtual machine

 configs/devices/or1k-softmmu/default.mak |   3 +
 hw/openrisc/Kconfig                      |   9 +
 hw/openrisc/boot.c                       | 127 +++++++
 hw/openrisc/meson.build                  |   2 +
 hw/openrisc/openrisc_sim.c               | 106 +-----
 hw/openrisc/virt.c                       | 429 +++++++++++++++++++++++
 include/hw/openrisc/boot.h               |  34 ++
 qemu-options.hx                          |  16 +-
 target/openrisc/cpu.h                    |   2 +
 target/openrisc/helper.h                 |   1 +
 target/openrisc/meson.build              |   1 +
 target/openrisc/openrisc-semi.c          |  54 +++
 target/openrisc/sys_helper.c             |   5 +
 target/openrisc/translate.c              |   6 +
 14 files changed, 690 insertions(+), 105 deletions(-)
 create mode 100644 hw/openrisc/boot.c
 create mode 100644 hw/openrisc/virt.c
 create mode 100644 include/hw/openrisc/boot.h
 create mode 100644 target/openrisc/openrisc-semi.c

-- 
2.31.1

[RFC PATCH 1/3] target/openrisc: Add basic support for semihosting

[Stafford Horne]

For OpenRISC we currently only use semihosting for system exit and
reset.  This patch implements that.

The implementation uses a helper to delegate to the semihosting
facility.  The helper is marked as having side effects but currently
does not have any.  I have defined it like this as our other
unimplemented semihosting calls will have side effects and return
results in register r11.

Signed-off-by: Stafford Horne <shorne@gmail.com>
---
 configs/devices/or1k-softmmu/default.mak |  2 +
 qemu-options.hx                          | 16 ++++---
 target/openrisc/cpu.h                    |  2 +
 target/openrisc/helper.h                 |  1 +
 target/openrisc/meson.build              |  1 +
 target/openrisc/openrisc-semi.c          | 54 ++++++++++++++++++++++++
 target/openrisc/sys_helper.c             |  5 +++
 target/openrisc/translate.c              |  6 +++
 8 files changed, 82 insertions(+), 5 deletions(-)
 create mode 100644 target/openrisc/openrisc-semi.c

diff --git a/configs/devices/or1k-softmmu/default.mak b/configs/devices/or1k-softmmu/default.mak
index 168101c39a..5b3ac89491 100644
--- a/configs/devices/or1k-softmmu/default.mak
+++ b/configs/devices/or1k-softmmu/default.mak
@@ -1,5 +1,7 @@
 # Default configuration for or1k-softmmu
 
+CONFIG_SEMIHOSTING=y
+
 # Boards:
 #
 CONFIG_OR1K_SIM=y
diff --git a/qemu-options.hx b/qemu-options.hx
index b484640067..312c68b065 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -4566,10 +4566,12 @@ ERST
 DEF("semihosting", 0, QEMU_OPTION_semihosting,
     "-semihosting    semihosting mode\n",
     QEMU_ARCH_ARM | QEMU_ARCH_M68K | QEMU_ARCH_XTENSA |
-    QEMU_ARCH_MIPS | QEMU_ARCH_NIOS2 | QEMU_ARCH_RISCV)
+    QEMU_ARCH_MIPS | QEMU_ARCH_NIOS2 | QEMU_ARCH_RISCV |
+    QEMU_ARCH_OPENRISC)
 SRST
 ``-semihosting``
-    Enable semihosting mode (ARM, M68K, Xtensa, MIPS, Nios II, RISC-V only).
+    Enable semihosting mode (ARM, M68K, Xtensa, MIPS, Nios II, RISC-V,
+    OpenRISC only).
 
     Note that this allows guest direct access to the host filesystem, so
     should only be used with a trusted guest OS.
@@ -4581,11 +4583,12 @@ DEF("semihosting-config", HAS_ARG, QEMU_OPTION_semihosting_config,
     "-semihosting-config [enable=on|off][,target=native|gdb|auto][,chardev=id][,arg=str[,...]]\n" \
     "                semihosting configuration\n",
 QEMU_ARCH_ARM | QEMU_ARCH_M68K | QEMU_ARCH_XTENSA |
-QEMU_ARCH_MIPS | QEMU_ARCH_NIOS2 | QEMU_ARCH_RISCV)
+QEMU_ARCH_MIPS | QEMU_ARCH_NIOS2 | QEMU_ARCH_RISCV |
+QEMU_ARCH_OPENRISC)
 SRST
 ``-semihosting-config [enable=on|off][,target=native|gdb|auto][,chardev=id][,arg=str[,...]]``
-    Enable and configure semihosting (ARM, M68K, Xtensa, MIPS, Nios II, RISC-V
-    only).
+    Enable and configure semihosting (ARM, M68K, Xtensa, MIPS, Nios II, RISC-V,
+    OpenRISC only).
 
     Note that this allows guest direct access to the host filesystem, so
     should only be used with a trusted guest OS.
@@ -4601,6 +4604,9 @@ SRST
 
     On RISC-V this implements the standard semihosting API, version 0.2.
 
+    On OpenRISC this only supports providing simulation exit and reset
+    facilities.
+
     ``target=native|gdb|auto``
         Defines where the semihosting calls will be addressed, to QEMU
         (``native``) or to GDB (``gdb``). The default is ``auto``, which
diff --git a/target/openrisc/cpu.h b/target/openrisc/cpu.h
index b9584f10d4..4617f1272b 100644
--- a/target/openrisc/cpu.h
+++ b/target/openrisc/cpu.h
@@ -407,4 +407,6 @@ void cpu_set_fpcsr(CPUOpenRISCState *env, uint32_t val);
 
 #define CPU_INTERRUPT_TIMER   CPU_INTERRUPT_TGT_INT_0
 
+void do_or1k_semihosting(CPUOpenRISCState *env, uint32_t k);
+
 #endif /* OPENRISC_CPU_H */
diff --git a/target/openrisc/helper.h b/target/openrisc/helper.h
index d847814a28..2fe3e4e4ca 100644
--- a/target/openrisc/helper.h
+++ b/target/openrisc/helper.h
@@ -64,3 +64,4 @@ DEF_HELPER_FLAGS_1(rfe, 0, void, env)
 /* sys */
 DEF_HELPER_FLAGS_3(mtspr, 0, void, env, tl, tl)
 DEF_HELPER_FLAGS_3(mfspr, TCG_CALL_NO_WG, tl, env, tl, tl)
+DEF_HELPER_FLAGS_2(nop, 0, void, env, i32)
diff --git a/target/openrisc/meson.build b/target/openrisc/meson.build
index 84322086ec..1c1758b846 100644
--- a/target/openrisc/meson.build
+++ b/target/openrisc/meson.build
@@ -10,6 +10,7 @@ openrisc_ss.add(files(
   'fpu_helper.c',
   'gdbstub.c',
   'interrupt_helper.c',
+  'openrisc-semi.c',
   'sys_helper.c',
   'translate.c',
 ))
diff --git a/target/openrisc/openrisc-semi.c b/target/openrisc/openrisc-semi.c
new file mode 100644
index 0000000000..97d6aaacdb
--- /dev/null
+++ b/target/openrisc/openrisc-semi.c
@@ -0,0 +1,54 @@
+/*
+ *  OpenRISC Semihosting syscall interface.
+ *
+ *  Copyright (c) 2022 Stafford Horne
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "qemu/osdep.h"
+
+#include "cpu.h"
+#include "exec/gdbstub.h"
+#include "sysemu/runstate.h"
+#include "qemu/log.h"
+
+#define HOSTED_EXIT 1
+#define HOSTED_RESET 13
+
+static void or1k_semi_return_u32(CPUOpenRISCState *env, uint32_t ret)
+{
+    cpu_set_gpr(env, 11, ret);
+}
+
+void do_or1k_semihosting(CPUOpenRISCState *env, uint32_t k)
+{
+    uint32_t result;
+
+    switch (k) {
+    case HOSTED_EXIT:
+        gdb_exit(cpu_get_gpr(env, 3));
+        exit(cpu_get_gpr(env, 3));
+    case HOSTED_RESET:
+#ifndef CONFIG_USER_ONLY
+        qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
+        return;
+#endif
+    default:
+        qemu_log_mask(LOG_GUEST_ERROR, "or1k-semihosting: unsupported "
+                      "semihosting syscall %d\n", k);
+        result = 0;
+    }
+    or1k_semi_return_u32(env, result);
+}
diff --git a/target/openrisc/sys_helper.c b/target/openrisc/sys_helper.c
index 48674231e7..eb698a527e 100644
--- a/target/openrisc/sys_helper.c
+++ b/target/openrisc/sys_helper.c
@@ -314,3 +314,8 @@ target_ulong HELPER(mfspr)(CPUOpenRISCState *env, target_ulong rd,
     /* for rd is passed in, if rd unchanged, just keep it back.  */
     return rd;
 }
+
+void HELPER(nop)(CPUOpenRISCState *env, target_ulong k)
+{
+    do_or1k_semihosting(env, k);
+}
diff --git a/target/openrisc/translate.c b/target/openrisc/translate.c
index 7b8ad43d5f..ec7b3b46ad 100644
--- a/target/openrisc/translate.c
+++ b/target/openrisc/translate.c
@@ -28,6 +28,7 @@
 #include "qemu/qemu-print.h"
 #include "exec/cpu_ldst.h"
 #include "exec/translator.h"
+#include "semihosting/semihost.h"
 
 #include "exec/helper-proto.h"
 #include "exec/helper-gen.h"
@@ -780,6 +781,11 @@ static bool trans_l_sh(DisasContext *dc, arg_store *a)
 
 static bool trans_l_nop(DisasContext *dc, arg_l_nop *a)
 {
+    if (semihosting_enabled() &&
+        a->k != 0) {
+        gen_helper_nop(cpu_env, tcg_constant_i32(a->k));
+    }
+
     return true;
 }
 
-- 
2.31.1

[RFC PATCH 1/3] target/openrisc: Add basic support for semihosting

[Stafford Horne]

For OpenRISC we currently only use semihosting for system exit and
reset.  This patch implements that.

The implementation uses a helper to delegate to the semihosting
facility.  The helper is marked as having side effects but currently
does not have any.  I have defined it like this as our other
unimplemented semihosting calls will have side effects and return
results in register r11.

Signed-off-by: Stafford Horne <shorne@gmail.com>
---
 configs/devices/or1k-softmmu/default.mak |  2 +
 qemu-options.hx                          | 16 ++++---
 target/openrisc/cpu.h                    |  2 +
 target/openrisc/helper.h                 |  1 +
 target/openrisc/meson.build              |  1 +
 target/openrisc/openrisc-semi.c          | 54 ++++++++++++++++++++++++
 target/openrisc/sys_helper.c             |  5 +++
 target/openrisc/translate.c              |  6 +++
 8 files changed, 82 insertions(+), 5 deletions(-)
 create mode 100644 target/openrisc/openrisc-semi.c

diff --git a/configs/devices/or1k-softmmu/default.mak b/configs/devices/or1k-softmmu/default.mak
index 168101c39a..5b3ac89491 100644
--- a/configs/devices/or1k-softmmu/default.mak
+++ b/configs/devices/or1k-softmmu/default.mak
@@ -1,5 +1,7 @@
 # Default configuration for or1k-softmmu
 
+CONFIG_SEMIHOSTING=y
+
 # Boards:
 #
 CONFIG_OR1K_SIM=y
diff --git a/qemu-options.hx b/qemu-options.hx
index b484640067..312c68b065 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -4566,10 +4566,12 @@ ERST
 DEF("semihosting", 0, QEMU_OPTION_semihosting,
     "-semihosting    semihosting mode\n",
     QEMU_ARCH_ARM | QEMU_ARCH_M68K | QEMU_ARCH_XTENSA |
-    QEMU_ARCH_MIPS | QEMU_ARCH_NIOS2 | QEMU_ARCH_RISCV)
+    QEMU_ARCH_MIPS | QEMU_ARCH_NIOS2 | QEMU_ARCH_RISCV |
+    QEMU_ARCH_OPENRISC)
 SRST
 ``-semihosting``
-    Enable semihosting mode (ARM, M68K, Xtensa, MIPS, Nios II, RISC-V only).
+    Enable semihosting mode (ARM, M68K, Xtensa, MIPS, Nios II, RISC-V,
+    OpenRISC only).
 
     Note that this allows guest direct access to the host filesystem, so
     should only be used with a trusted guest OS.
@@ -4581,11 +4583,12 @@ DEF("semihosting-config", HAS_ARG, QEMU_OPTION_semihosting_config,
     "-semihosting-config [enable=on|off][,target=native|gdb|auto][,chardev=id][,arg=str[,...]]\n" \
     "                semihosting configuration\n",
 QEMU_ARCH_ARM | QEMU_ARCH_M68K | QEMU_ARCH_XTENSA |
-QEMU_ARCH_MIPS | QEMU_ARCH_NIOS2 | QEMU_ARCH_RISCV)
+QEMU_ARCH_MIPS | QEMU_ARCH_NIOS2 | QEMU_ARCH_RISCV |
+QEMU_ARCH_OPENRISC)
 SRST
 ``-semihosting-config [enable=on|off][,target=native|gdb|auto][,chardev=id][,arg=str[,...]]``
-    Enable and configure semihosting (ARM, M68K, Xtensa, MIPS, Nios II, RISC-V
-    only).
+    Enable and configure semihosting (ARM, M68K, Xtensa, MIPS, Nios II, RISC-V,
+    OpenRISC only).
 
     Note that this allows guest direct access to the host filesystem, so
     should only be used with a trusted guest OS.
@@ -4601,6 +4604,9 @@ SRST
 
     On RISC-V this implements the standard semihosting API, version 0.2.
 
+    On OpenRISC this only supports providing simulation exit and reset
+    facilities.
+
     ``target=native|gdb|auto``
         Defines where the semihosting calls will be addressed, to QEMU
         (``native``) or to GDB (``gdb``). The default is ``auto``, which
diff --git a/target/openrisc/cpu.h b/target/openrisc/cpu.h
index b9584f10d4..4617f1272b 100644
--- a/target/openrisc/cpu.h
+++ b/target/openrisc/cpu.h
@@ -407,4 +407,6 @@ void cpu_set_fpcsr(CPUOpenRISCState *env, uint32_t val);
 
 #define CPU_INTERRUPT_TIMER   CPU_INTERRUPT_TGT_INT_0
 
+void do_or1k_semihosting(CPUOpenRISCState *env, uint32_t k);
+
 #endif /* OPENRISC_CPU_H */
diff --git a/target/openrisc/helper.h b/target/openrisc/helper.h
index d847814a28..2fe3e4e4ca 100644
--- a/target/openrisc/helper.h
+++ b/target/openrisc/helper.h
@@ -64,3 +64,4 @@ DEF_HELPER_FLAGS_1(rfe, 0, void, env)
 /* sys */
 DEF_HELPER_FLAGS_3(mtspr, 0, void, env, tl, tl)
 DEF_HELPER_FLAGS_3(mfspr, TCG_CALL_NO_WG, tl, env, tl, tl)
+DEF_HELPER_FLAGS_2(nop, 0, void, env, i32)
diff --git a/target/openrisc/meson.build b/target/openrisc/meson.build
index 84322086ec..1c1758b846 100644
--- a/target/openrisc/meson.build
+++ b/target/openrisc/meson.build
@@ -10,6 +10,7 @@ openrisc_ss.add(files(
   'fpu_helper.c',
   'gdbstub.c',
   'interrupt_helper.c',
+  'openrisc-semi.c',
   'sys_helper.c',
   'translate.c',
 ))
diff --git a/target/openrisc/openrisc-semi.c b/target/openrisc/openrisc-semi.c
new file mode 100644
index 0000000000..97d6aaacdb
--- /dev/null
+++ b/target/openrisc/openrisc-semi.c
@@ -0,0 +1,54 @@
+/*
+ *  OpenRISC Semihosting syscall interface.
+ *
+ *  Copyright (c) 2022 Stafford Horne
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "qemu/osdep.h"
+
+#include "cpu.h"
+#include "exec/gdbstub.h"
+#include "sysemu/runstate.h"
+#include "qemu/log.h"
+
+#define HOSTED_EXIT 1
+#define HOSTED_RESET 13
+
+static void or1k_semi_return_u32(CPUOpenRISCState *env, uint32_t ret)
+{
+    cpu_set_gpr(env, 11, ret);
+}
+
+void do_or1k_semihosting(CPUOpenRISCState *env, uint32_t k)
+{
+    uint32_t result;
+
+    switch (k) {
+    case HOSTED_EXIT:
+        gdb_exit(cpu_get_gpr(env, 3));
+        exit(cpu_get_gpr(env, 3));
+    case HOSTED_RESET:
+#ifndef CONFIG_USER_ONLY
+        qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
+        return;
+#endif
+    default:
+        qemu_log_mask(LOG_GUEST_ERROR, "or1k-semihosting: unsupported "
+                      "semihosting syscall %d\n", k);
+        result = 0;
+    }
+    or1k_semi_return_u32(env, result);
+}
diff --git a/target/openrisc/sys_helper.c b/target/openrisc/sys_helper.c
index 48674231e7..eb698a527e 100644
--- a/target/openrisc/sys_helper.c
+++ b/target/openrisc/sys_helper.c
@@ -314,3 +314,8 @@ target_ulong HELPER(mfspr)(CPUOpenRISCState *env, target_ulong rd,
     /* for rd is passed in, if rd unchanged, just keep it back.  */
     return rd;
 }
+
+void HELPER(nop)(CPUOpenRISCState *env, target_ulong k)
+{
+    do_or1k_semihosting(env, k);
+}
diff --git a/target/openrisc/translate.c b/target/openrisc/translate.c
index 7b8ad43d5f..ec7b3b46ad 100644
--- a/target/openrisc/translate.c
+++ b/target/openrisc/translate.c
@@ -28,6 +28,7 @@
 #include "qemu/qemu-print.h"
 #include "exec/cpu_ldst.h"
 #include "exec/translator.h"
+#include "semihosting/semihost.h"
 
 #include "exec/helper-proto.h"
 #include "exec/helper-gen.h"
@@ -780,6 +781,11 @@ static bool trans_l_sh(DisasContext *dc, arg_store *a)
 
 static bool trans_l_nop(DisasContext *dc, arg_l_nop *a)
 {
+    if (semihosting_enabled() &&
+        a->k != 0) {
+        gen_helper_nop(cpu_env, tcg_constant_i32(a->k));
+    }
+
     return true;
 }
 
-- 
2.31.1

Re: [RFC PATCH 1/3] target/openrisc: Add basic support for semihosting

[Richard Henderson]

On 5/27/22 10:27, Stafford Horne wrote:
> +void do_or1k_semihosting(CPUOpenRISCState *env, uint32_t k);
...
> +DEF_HELPER_FLAGS_2(nop, 0, void, env, i32)

Just call the helper "semihosting" and be done with it.
And the helper wants an ifdef for system mode.

> @@ -10,6 +10,7 @@ openrisc_ss.add(files(
>     'fpu_helper.c',
>     'gdbstub.c',
>     'interrupt_helper.c',
> +  'openrisc-semi.c',
>     'sys_helper.c',
>     'translate.c',
>   ))

You want to add the new file for system mode only.
Or, now that I think of it, conditional on CONFIG_SEMIHOSTING itself.

> +static void or1k_semi_return_u32(CPUOpenRISCState *env, uint32_t ret)
> +{
> +    cpu_set_gpr(env, 11, ret);
> +}

Let's drop this until you actually use it.  This appears to be attempting to mirror other, 
more complete semihosting, but missing the third "error" argument.


> +void do_or1k_semihosting(CPUOpenRISCState *env, uint32_t k)
> +{
> +    uint32_t result;
> +
> +    switch (k) {
> +    case HOSTED_EXIT:
> +        gdb_exit(cpu_get_gpr(env, 3));
> +        exit(cpu_get_gpr(env, 3));
> +    case HOSTED_RESET:
> +#ifndef CONFIG_USER_ONLY
> +        qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
> +        return;
> +#endif

Do you in fact want to exit to the main loop after asking for reset?
That's the only way that "no return value" makes sense to me...

> +    default:
> +        qemu_log_mask(LOG_GUEST_ERROR, "or1k-semihosting: unsupported "
> +                      "semihosting syscall %d\n", k);

%u.

>   static bool trans_l_nop(DisasContext *dc, arg_l_nop *a)
>   {
> +    if (semihosting_enabled() &&
> +        a->k != 0) {
> +        gen_helper_nop(cpu_env, tcg_constant_i32(a->k));
> +    }

Perhaps cleaner to move the semihosting dispatch switch here, instead of leaving it to the 
runtime?  The reason we have a runtime switch for other guests is that the semihosting 
syscall number is in a register.  This would then be

     if (semihosting_enabled()) {
         switch (a->k) {
         case 0:
             break; /* normal nop */
         case HOSTED_EXIT:
             gen_helper_semihost_exit(cpu_R(dc, 3));
             break;
         case HOSTED_RESET:
             gen_helper_semihost_reset();
             tcg_gen_movi_tl(cpu_pc, dc->base.pc_next + 4);

             dc->base.is_jmp = DISAS_EXIT;
             break;
         ...
         }
     }


r~

Re: [RFC PATCH 1/3] target/openrisc: Add basic support for semihosting

[Richard Henderson]

On 5/27/22 10:27, Stafford Horne wrote:
> +void do_or1k_semihosting(CPUOpenRISCState *env, uint32_t k);
...
> +DEF_HELPER_FLAGS_2(nop, 0, void, env, i32)

Just call the helper "semihosting" and be done with it.
And the helper wants an ifdef for system mode.

> @@ -10,6 +10,7 @@ openrisc_ss.add(files(
>     'fpu_helper.c',
>     'gdbstub.c',
>     'interrupt_helper.c',
> +  'openrisc-semi.c',
>     'sys_helper.c',
>     'translate.c',
>   ))

You want to add the new file for system mode only.
Or, now that I think of it, conditional on CONFIG_SEMIHOSTING itself.

> +static void or1k_semi_return_u32(CPUOpenRISCState *env, uint32_t ret)
> +{
> +    cpu_set_gpr(env, 11, ret);
> +}

Let's drop this until you actually use it.  This appears to be attempting to mirror other, 
more complete semihosting, but missing the third "error" argument.


> +void do_or1k_semihosting(CPUOpenRISCState *env, uint32_t k)
> +{
> +    uint32_t result;
> +
> +    switch (k) {
> +    case HOSTED_EXIT:
> +        gdb_exit(cpu_get_gpr(env, 3));
> +        exit(cpu_get_gpr(env, 3));
> +    case HOSTED_RESET:
> +#ifndef CONFIG_USER_ONLY
> +        qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
> +        return;
> +#endif

Do you in fact want to exit to the main loop after asking for reset?
That's the only way that "no return value" makes sense to me...

> +    default:
> +        qemu_log_mask(LOG_GUEST_ERROR, "or1k-semihosting: unsupported "
> +                      "semihosting syscall %d\n", k);

%u.

>   static bool trans_l_nop(DisasContext *dc, arg_l_nop *a)
>   {
> +    if (semihosting_enabled() &&
> +        a->k != 0) {
> +        gen_helper_nop(cpu_env, tcg_constant_i32(a->k));
> +    }

Perhaps cleaner to move the semihosting dispatch switch here, instead of leaving it to the 
runtime?  The reason we have a runtime switch for other guests is that the semihosting 
syscall number is in a register.  This would then be

     if (semihosting_enabled()) {
         switch (a->k) {
         case 0:
             break; /* normal nop */
         case HOSTED_EXIT:
             gen_helper_semihost_exit(cpu_R(dc, 3));
             break;
         case HOSTED_RESET:
             gen_helper_semihost_reset();
             tcg_gen_movi_tl(cpu_pc, dc->base.pc_next + 4);

             dc->base.is_jmp = DISAS_EXIT;
             break;
         ...
         }
     }


r~

Re: [RFC PATCH 1/3] target/openrisc: Add basic support for semihosting

[Stafford Horne]

On Thu, Jun 02, 2022 at 08:39:21AM -0700, Richard Henderson wrote:
> On 5/27/22 10:27, Stafford Horne wrote:
> > +void do_or1k_semihosting(CPUOpenRISCState *env, uint32_t k);
> ...
> > +DEF_HELPER_FLAGS_2(nop, 0, void, env, i32)
> 
> Just call the helper "semihosting" and be done with it.
> And the helper wants an ifdef for system mode.
> 
> > @@ -10,6 +10,7 @@ openrisc_ss.add(files(
> >     'fpu_helper.c',
> >     'gdbstub.c',
> >     'interrupt_helper.c',
> > +  'openrisc-semi.c',
> >     'sys_helper.c',
> >     'translate.c',
> >   ))
> 
> You want to add the new file for system mode only.
> Or, now that I think of it, conditional on CONFIG_SEMIHOSTING itself.

That's right, I'll update it.

> > +static void or1k_semi_return_u32(CPUOpenRISCState *env, uint32_t ret)
> > +{
> > +    cpu_set_gpr(env, 11, ret);
> > +}
> 
> Let's drop this until you actually use it.  This appears to be attempting to
> mirror other, more complete semihosting, but missing the third "error"
> argument

Sure, I did mention I kept these here for future (real) semihosting support.
But I don't think that will happen.  So I can remove.

> > +void do_or1k_semihosting(CPUOpenRISCState *env, uint32_t k)
> > +{
> > +    uint32_t result;
> > +
> > +    switch (k) {
> > +    case HOSTED_EXIT:
> > +        gdb_exit(cpu_get_gpr(env, 3));
> > +        exit(cpu_get_gpr(env, 3));
> > +    case HOSTED_RESET:
> > +#ifndef CONFIG_USER_ONLY
> > +        qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
> > +        return;
> > +#endif
> 
> Do you in fact want to exit to the main loop after asking for reset?
> That's the only way that "no return value" makes sense to me...

OK. I'll look at this more.
 
> > +    default:
> > +        qemu_log_mask(LOG_GUEST_ERROR, "or1k-semihosting: unsupported "
> > +                      "semihosting syscall %d\n", k);
> 
> %u.

OK.

> >   static bool trans_l_nop(DisasContext *dc, arg_l_nop *a)
> >   {
> > +    if (semihosting_enabled() &&
> > +        a->k != 0) {
> > +        gen_helper_nop(cpu_env, tcg_constant_i32(a->k));
> > +    }
> 
> Perhaps cleaner to move the semihosting dispatch switch here, instead of
> leaving it to the runtime?  The reason we have a runtime switch for other
> guests is that the semihosting syscall number is in a register.  This would
> then be
> 
>     if (semihosting_enabled()) {
>         switch (a->k) {
>         case 0:
>             break; /* normal nop */
>         case HOSTED_EXIT:
>             gen_helper_semihost_exit(cpu_R(dc, 3));
>             break;
>         case HOSTED_RESET:
>             gen_helper_semihost_reset();
>             tcg_gen_movi_tl(cpu_pc, dc->base.pc_next + 4);
> 
>             dc->base.is_jmp = DISAS_EXIT;
>             break;
>         ...
>         }
>     }

Yeah, that makes sense. I had written it in a way that would allow expanding for
real semi-hosting.  But I don't think we will do that with OpenRISC, so this is
good enough.

I am not sure if you saw the cover letter. I sent this RFC series to help
illustrate two options for providing OpenRISC targets that support poweroff and
reset.

One option being using these NOP's, the second is to create a virt target with
reset/poweroff hardware.

I am kind of leaning towards dropping the semi-hosting patches and only moving
forward with the virt patches.  The reason being that 1. we would not need to
expand the architecture spec to support the qemu virt platform, and we would
need to document the NOP's formally, and 2. OpenRISC doesn't really support the
full "semihosting" facilities for file open/close/write etc.

Any thoughts?  I guess this "semihosting" patch is pretty trivial.  But, maybe
it causes more confusion compared to just going with the virt route.  Also, if
we have virt I can't imagine anyone using the semihosting much.

-Stafford

Re: [RFC PATCH 1/3] target/openrisc: Add basic support for semihosting

[Stafford Horne]

On Thu, Jun 02, 2022 at 08:39:21AM -0700, Richard Henderson wrote:
> On 5/27/22 10:27, Stafford Horne wrote:
> > +void do_or1k_semihosting(CPUOpenRISCState *env, uint32_t k);
> ...
> > +DEF_HELPER_FLAGS_2(nop, 0, void, env, i32)
> 
> Just call the helper "semihosting" and be done with it.
> And the helper wants an ifdef for system mode.
> 
> > @@ -10,6 +10,7 @@ openrisc_ss.add(files(
> >     'fpu_helper.c',
> >     'gdbstub.c',
> >     'interrupt_helper.c',
> > +  'openrisc-semi.c',
> >     'sys_helper.c',
> >     'translate.c',
> >   ))
> 
> You want to add the new file for system mode only.
> Or, now that I think of it, conditional on CONFIG_SEMIHOSTING itself.

That's right, I'll update it.

> > +static void or1k_semi_return_u32(CPUOpenRISCState *env, uint32_t ret)
> > +{
> > +    cpu_set_gpr(env, 11, ret);
> > +}
> 
> Let's drop this until you actually use it.  This appears to be attempting to
> mirror other, more complete semihosting, but missing the third "error"
> argument

Sure, I did mention I kept these here for future (real) semihosting support.
But I don't think that will happen.  So I can remove.

> > +void do_or1k_semihosting(CPUOpenRISCState *env, uint32_t k)
> > +{
> > +    uint32_t result;
> > +
> > +    switch (k) {
> > +    case HOSTED_EXIT:
> > +        gdb_exit(cpu_get_gpr(env, 3));
> > +        exit(cpu_get_gpr(env, 3));
> > +    case HOSTED_RESET:
> > +#ifndef CONFIG_USER_ONLY
> > +        qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
> > +        return;
> > +#endif
> 
> Do you in fact want to exit to the main loop after asking for reset?
> That's the only way that "no return value" makes sense to me...

OK. I'll look at this more.
 
> > +    default:
> > +        qemu_log_mask(LOG_GUEST_ERROR, "or1k-semihosting: unsupported "
> > +                      "semihosting syscall %d\n", k);
> 
> %u.

OK.

> >   static bool trans_l_nop(DisasContext *dc, arg_l_nop *a)
> >   {
> > +    if (semihosting_enabled() &&
> > +        a->k != 0) {
> > +        gen_helper_nop(cpu_env, tcg_constant_i32(a->k));
> > +    }
> 
> Perhaps cleaner to move the semihosting dispatch switch here, instead of
> leaving it to the runtime?  The reason we have a runtime switch for other
> guests is that the semihosting syscall number is in a register.  This would
> then be
> 
>     if (semihosting_enabled()) {
>         switch (a->k) {
>         case 0:
>             break; /* normal nop */
>         case HOSTED_EXIT:
>             gen_helper_semihost_exit(cpu_R(dc, 3));
>             break;
>         case HOSTED_RESET:
>             gen_helper_semihost_reset();
>             tcg_gen_movi_tl(cpu_pc, dc->base.pc_next + 4);
> 
>             dc->base.is_jmp = DISAS_EXIT;
>             break;
>         ...
>         }
>     }

Yeah, that makes sense. I had written it in a way that would allow expanding for
real semi-hosting.  But I don't think we will do that with OpenRISC, so this is
good enough.

I am not sure if you saw the cover letter. I sent this RFC series to help
illustrate two options for providing OpenRISC targets that support poweroff and
reset.

One option being using these NOP's, the second is to create a virt target with
reset/poweroff hardware.

I am kind of leaning towards dropping the semi-hosting patches and only moving
forward with the virt patches.  The reason being that 1. we would not need to
expand the architecture spec to support the qemu virt platform, and we would
need to document the NOP's formally, and 2. OpenRISC doesn't really support the
full "semihosting" facilities for file open/close/write etc.

Any thoughts?  I guess this "semihosting" patch is pretty trivial.  But, maybe
it causes more confusion compared to just going with the virt route.  Also, if
we have virt I can't imagine anyone using the semihosting much.

-Stafford

Re: [RFC PATCH 1/3] target/openrisc: Add basic support for semihosting

[Richard Henderson]

On 6/4/22 17:57, Stafford Horne wrote:
> I am kind of leaning towards dropping the semi-hosting patches and only moving
> forward with the virt patches.  The reason being that 1. we would not need to
> expand the architecture spec to support the qemu virt platform, and we would
> need to document the NOP's formally, and 2. OpenRISC doesn't really support the
> full "semihosting" facilities for file open/close/write etc.

I agree that "virt" would to more for openrisc devel than these nops.

> Also, if we have virt I can't imagine anyone using the semihosting much.

IMO, semihosting is most valuable for writing regression tests and not much more.  (You 
have no control over the exit status of qemu with normal shutdown, as compared with 
semihosting exit.)


r~

Re: [RFC PATCH 1/3] target/openrisc: Add basic support for semihosting

[Richard Henderson]

On 6/4/22 17:57, Stafford Horne wrote:
> I am kind of leaning towards dropping the semi-hosting patches and only moving
> forward with the virt patches.  The reason being that 1. we would not need to
> expand the architecture spec to support the qemu virt platform, and we would
> need to document the NOP's formally, and 2. OpenRISC doesn't really support the
> full "semihosting" facilities for file open/close/write etc.

I agree that "virt" would to more for openrisc devel than these nops.

> Also, if we have virt I can't imagine anyone using the semihosting much.

IMO, semihosting is most valuable for writing regression tests and not much more.  (You 
have no control over the exit status of qemu with normal shutdown, as compared with 
semihosting exit.)


r~

[RFC PATCH 2/3] hw/openrisc: Split re-usable boot time apis out to boot.c

[Stafford Horne]

These will be shared with the virt platform.

Signed-off-by: Stafford Horne <shorne@gmail.com>
---
 hw/openrisc/boot.c         | 127 +++++++++++++++++++++++++++++++++++++
 hw/openrisc/meson.build    |   1 +
 hw/openrisc/openrisc_sim.c | 106 ++-----------------------------
 include/hw/openrisc/boot.h |  34 ++++++++++
 4 files changed, 168 insertions(+), 100 deletions(-)
 create mode 100644 hw/openrisc/boot.c
 create mode 100644 include/hw/openrisc/boot.h

diff --git a/hw/openrisc/boot.c b/hw/openrisc/boot.c
new file mode 100644
index 0000000000..32f0925a8c
--- /dev/null
+++ b/hw/openrisc/boot.c
@@ -0,0 +1,127 @@
+/*
+ * QEMU OpenRISC boot helpers.
+ *
+ * Copyright (c) 2022 Stafford Horne <shorne@gmail.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2 or later, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "qemu/osdep.h"
+#include "cpu.h"
+#include "exec/cpu-defs.h"
+#include "elf.h"
+#include "hw/loader.h"
+#include "hw/openrisc/boot.h"
+#include "sysemu/device_tree.h"
+#include "sysemu/qtest.h"
+
+#include <libfdt.h>
+
+#define KERNEL_LOAD_ADDR 0x100
+
+hwaddr openrisc_load_kernel(ram_addr_t ram_size,
+                            const char *kernel_filename,
+                            uint32_t *bootstrap_pc)
+{
+    long kernel_size;
+    uint64_t elf_entry;
+    uint64_t high_addr;
+    hwaddr entry;
+
+    if (kernel_filename && !qtest_enabled()) {
+        kernel_size = load_elf(kernel_filename, NULL, NULL, NULL,
+                               &elf_entry, NULL, &high_addr, NULL, 1,
+                               EM_OPENRISC, 1, 0);
+        entry = elf_entry;
+        if (kernel_size < 0) {
+            kernel_size = load_uimage(kernel_filename,
+                                      &entry, NULL, NULL, NULL, NULL);
+            high_addr = entry + kernel_size;
+        }
+        if (kernel_size < 0) {
+            kernel_size = load_image_targphys(kernel_filename,
+                                              KERNEL_LOAD_ADDR,
+                                              ram_size - KERNEL_LOAD_ADDR);
+            high_addr = KERNEL_LOAD_ADDR + kernel_size;
+        }
+
+        if (entry <= 0) {
+            entry = KERNEL_LOAD_ADDR;
+        }
+
+        if (kernel_size < 0) {
+            error_report("couldn't load the kernel '%s'", kernel_filename);
+            exit(1);
+        }
+        *bootstrap_pc = entry;
+
+        return high_addr;
+    }
+    return 0;
+}
+
+hwaddr openrisc_load_initrd(void *fdt, const char *filename,
+                            hwaddr load_start, uint64_t mem_size)
+{
+    int size;
+    hwaddr start;
+
+    /* We put the initrd right after the kernel; page aligned. */
+    start = TARGET_PAGE_ALIGN(load_start);
+
+    size = load_ramdisk(filename, start, mem_size - start);
+    if (size < 0) {
+        size = load_image_targphys(filename, start, mem_size - start);
+        if (size < 0) {
+            error_report("could not load ramdisk '%s'", filename);
+            exit(1);
+        }
+    }
+
+    if (fdt) {
+        qemu_fdt_setprop_cell(fdt, "/chosen",
+                              "linux,initrd-start", start);
+        qemu_fdt_setprop_cell(fdt, "/chosen",
+                              "linux,initrd-end", start + size);
+    }
+
+    return start + size;
+}
+
+uint32_t openrisc_load_fdt(void *fdt, hwaddr load_start,
+                           uint64_t mem_size)
+{
+    uint32_t fdt_addr;
+    int ret;
+    int fdtsize = fdt_totalsize(fdt);
+
+    if (fdtsize <= 0) {
+        error_report("invalid device-tree");
+        exit(1);
+    }
+
+    /* We put fdt right after the kernel and/or initrd. */
+    fdt_addr = TARGET_PAGE_ALIGN(load_start);
+
+    ret = fdt_pack(fdt);
+    /* Should only fail if we've built a corrupted tree */
+    g_assert(ret == 0);
+    /* copy in the device tree */
+    qemu_fdt_dumpdtb(fdt, fdtsize);
+
+    rom_add_blob_fixed_as("fdt", fdt, fdtsize, fdt_addr,
+                          &address_space_memory);
+
+    return fdt_addr;
+}
+
diff --git a/hw/openrisc/meson.build b/hw/openrisc/meson.build
index ec48172c9d..ab563820c5 100644
--- a/hw/openrisc/meson.build
+++ b/hw/openrisc/meson.build
@@ -1,5 +1,6 @@
 openrisc_ss = ss.source_set()
 openrisc_ss.add(files('cputimer.c'))
+openrisc_ss.add(files('boot.c'))
 openrisc_ss.add(when: 'CONFIG_OR1K_SIM', if_true: [files('openrisc_sim.c'), fdt])
 
 hw_arch += {'openrisc': openrisc_ss}
diff --git a/hw/openrisc/openrisc_sim.c b/hw/openrisc/openrisc_sim.c
index 35adce17ac..35da123aef 100644
--- a/hw/openrisc/openrisc_sim.c
+++ b/hw/openrisc/openrisc_sim.c
@@ -24,10 +24,9 @@
 #include "cpu.h"
 #include "hw/irq.h"
 #include "hw/boards.h"
-#include "elf.h"
 #include "hw/char/serial.h"
 #include "net/net.h"
-#include "hw/loader.h"
+#include "hw/openrisc/boot.h"
 #include "hw/qdev-properties.h"
 #include "exec/address-spaces.h"
 #include "sysemu/device_tree.h"
@@ -283,101 +282,6 @@ static void openrisc_sim_serial_init(Or1ksimState *state, hwaddr base,
     g_free(nodename);
 }
 
-static hwaddr openrisc_load_kernel(ram_addr_t ram_size,
-                                   const char *kernel_filename)
-{
-    long kernel_size;
-    uint64_t elf_entry;
-    uint64_t high_addr;
-    hwaddr entry;
-
-    if (kernel_filename && !qtest_enabled()) {
-        kernel_size = load_elf(kernel_filename, NULL, NULL, NULL,
-                               &elf_entry, NULL, &high_addr, NULL, 1,
-                               EM_OPENRISC, 1, 0);
-        entry = elf_entry;
-        if (kernel_size < 0) {
-            kernel_size = load_uimage(kernel_filename,
-                                      &entry, NULL, NULL, NULL, NULL);
-            high_addr = entry + kernel_size;
-        }
-        if (kernel_size < 0) {
-            kernel_size = load_image_targphys(kernel_filename,
-                                              KERNEL_LOAD_ADDR,
-                                              ram_size - KERNEL_LOAD_ADDR);
-            high_addr = KERNEL_LOAD_ADDR + kernel_size;
-        }
-
-        if (entry <= 0) {
-            entry = KERNEL_LOAD_ADDR;
-        }
-
-        if (kernel_size < 0) {
-            error_report("couldn't load the kernel '%s'", kernel_filename);
-            exit(1);
-        }
-        boot_info.bootstrap_pc = entry;
-
-        return high_addr;
-    }
-    return 0;
-}
-
-static hwaddr openrisc_load_initrd(Or1ksimState *state, const char *filename,
-                                   hwaddr load_start, uint64_t mem_size)
-{
-    void *fdt = state->fdt;
-    int size;
-    hwaddr start;
-
-    /* We put the initrd right after the kernel; page aligned. */
-    start = TARGET_PAGE_ALIGN(load_start);
-
-    size = load_ramdisk(filename, start, mem_size - start);
-    if (size < 0) {
-        size = load_image_targphys(filename, start, mem_size - start);
-        if (size < 0) {
-            error_report("could not load ramdisk '%s'", filename);
-            exit(1);
-        }
-    }
-
-    qemu_fdt_setprop_cell(fdt, "/chosen",
-                          "linux,initrd-start", start);
-    qemu_fdt_setprop_cell(fdt, "/chosen",
-                          "linux,initrd-end", start + size);
-
-    return start + size;
-}
-
-static uint32_t openrisc_load_fdt(Or1ksimState *state, hwaddr load_start,
-                                  uint64_t mem_size)
-{
-    void *fdt = state->fdt;
-    uint32_t fdt_addr;
-    int ret;
-    int fdtsize = fdt_totalsize(fdt);
-
-    if (fdtsize <= 0) {
-        error_report("invalid device-tree");
-        exit(1);
-    }
-
-    /* We put fdt right after the kernel and/or initrd. */
-    fdt_addr = TARGET_PAGE_ALIGN(load_start);
-
-    ret = fdt_pack(fdt);
-    /* Should only fail if we've built a corrupted tree */
-    g_assert(ret == 0);
-    /* copy in the device tree */
-    qemu_fdt_dumpdtb(fdt, fdtsize);
-
-    rom_add_blob_fixed_as("fdt", fdt, fdtsize, fdt_addr,
-                          &address_space_memory);
-
-    return fdt_addr;
-}
-
 static void openrisc_sim_init(MachineState *machine)
 {
     ram_addr_t ram_size = machine->ram_size;
@@ -428,13 +332,15 @@ static void openrisc_sim_init(MachineState *machine)
                                  or1ksim_memmap[OR1KSIM_UART].size,
                                  smp_cpus, cpus, OR1KSIM_UART_IRQ, n);
 
-    load_addr = openrisc_load_kernel(ram_size, kernel_filename);
+    load_addr = openrisc_load_kernel(ram_size, kernel_filename,
+                                     &boot_info.bootstrap_pc);
     if (load_addr > 0) {
         if (machine->initrd_filename) {
-            load_addr = openrisc_load_initrd(state, machine->initrd_filename,
+            load_addr = openrisc_load_initrd(state->fdt,
+                                             machine->initrd_filename,
                                              load_addr, machine->ram_size);
         }
-        boot_info.fdt_addr = openrisc_load_fdt(state, load_addr,
+        boot_info.fdt_addr = openrisc_load_fdt(state->fdt, load_addr,
                                                machine->ram_size);
     }
 }
diff --git a/include/hw/openrisc/boot.h b/include/hw/openrisc/boot.h
new file mode 100644
index 0000000000..25a313d63a
--- /dev/null
+++ b/include/hw/openrisc/boot.h
@@ -0,0 +1,34 @@
+/*
+ * QEMU OpenRISC boot helpers.
+ *
+ * Copyright (c) 2022 Stafford Horne <shorne@gmail.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2 or later, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef OPENRISC_BOOT_H
+#define OPENRISC_BOOT_H
+
+#include "exec/cpu-defs.h"
+
+hwaddr openrisc_load_kernel(ram_addr_t ram_size,
+                            const char *kernel_filename,
+                            uint32_t *bootstrap_pc);
+
+hwaddr openrisc_load_initrd(void *fdt, const char *filename,
+                            hwaddr load_start, uint64_t mem_size);
+
+uint32_t openrisc_load_fdt(void *fdt, hwaddr load_start,
+                           uint64_t mem_size);
+
+#endif /* OPENRISC_BOOT_H */
-- 
2.31.1

[RFC PATCH 2/3] hw/openrisc: Split re-usable boot time apis out to boot.c

[Stafford Horne]

These will be shared with the virt platform.

Signed-off-by: Stafford Horne <shorne@gmail.com>
---
 hw/openrisc/boot.c         | 127 +++++++++++++++++++++++++++++++++++++
 hw/openrisc/meson.build    |   1 +
 hw/openrisc/openrisc_sim.c | 106 ++-----------------------------
 include/hw/openrisc/boot.h |  34 ++++++++++
 4 files changed, 168 insertions(+), 100 deletions(-)
 create mode 100644 hw/openrisc/boot.c
 create mode 100644 include/hw/openrisc/boot.h

diff --git a/hw/openrisc/boot.c b/hw/openrisc/boot.c
new file mode 100644
index 0000000000..32f0925a8c
--- /dev/null
+++ b/hw/openrisc/boot.c
@@ -0,0 +1,127 @@
+/*
+ * QEMU OpenRISC boot helpers.
+ *
+ * Copyright (c) 2022 Stafford Horne <shorne@gmail.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2 or later, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "qemu/osdep.h"
+#include "cpu.h"
+#include "exec/cpu-defs.h"
+#include "elf.h"
+#include "hw/loader.h"
+#include "hw/openrisc/boot.h"
+#include "sysemu/device_tree.h"
+#include "sysemu/qtest.h"
+
+#include <libfdt.h>
+
+#define KERNEL_LOAD_ADDR 0x100
+
+hwaddr openrisc_load_kernel(ram_addr_t ram_size,
+                            const char *kernel_filename,
+                            uint32_t *bootstrap_pc)
+{
+    long kernel_size;
+    uint64_t elf_entry;
+    uint64_t high_addr;
+    hwaddr entry;
+
+    if (kernel_filename && !qtest_enabled()) {
+        kernel_size = load_elf(kernel_filename, NULL, NULL, NULL,
+                               &elf_entry, NULL, &high_addr, NULL, 1,
+                               EM_OPENRISC, 1, 0);
+        entry = elf_entry;
+        if (kernel_size < 0) {
+            kernel_size = load_uimage(kernel_filename,
+                                      &entry, NULL, NULL, NULL, NULL);
+            high_addr = entry + kernel_size;
+        }
+        if (kernel_size < 0) {
+            kernel_size = load_image_targphys(kernel_filename,
+                                              KERNEL_LOAD_ADDR,
+                                              ram_size - KERNEL_LOAD_ADDR);
+            high_addr = KERNEL_LOAD_ADDR + kernel_size;
+        }
+
+        if (entry <= 0) {
+            entry = KERNEL_LOAD_ADDR;
+        }
+
+        if (kernel_size < 0) {
+            error_report("couldn't load the kernel '%s'", kernel_filename);
+            exit(1);
+        }
+        *bootstrap_pc = entry;
+
+        return high_addr;
+    }
+    return 0;
+}
+
+hwaddr openrisc_load_initrd(void *fdt, const char *filename,
+                            hwaddr load_start, uint64_t mem_size)
+{
+    int size;
+    hwaddr start;
+
+    /* We put the initrd right after the kernel; page aligned. */
+    start = TARGET_PAGE_ALIGN(load_start);
+
+    size = load_ramdisk(filename, start, mem_size - start);
+    if (size < 0) {
+        size = load_image_targphys(filename, start, mem_size - start);
+        if (size < 0) {
+            error_report("could not load ramdisk '%s'", filename);
+            exit(1);
+        }
+    }
+
+    if (fdt) {
+        qemu_fdt_setprop_cell(fdt, "/chosen",
+                              "linux,initrd-start", start);
+        qemu_fdt_setprop_cell(fdt, "/chosen",
+                              "linux,initrd-end", start + size);
+    }
+
+    return start + size;
+}
+
+uint32_t openrisc_load_fdt(void *fdt, hwaddr load_start,
+                           uint64_t mem_size)
+{
+    uint32_t fdt_addr;
+    int ret;
+    int fdtsize = fdt_totalsize(fdt);
+
+    if (fdtsize <= 0) {
+        error_report("invalid device-tree");
+        exit(1);
+    }
+
+    /* We put fdt right after the kernel and/or initrd. */
+    fdt_addr = TARGET_PAGE_ALIGN(load_start);
+
+    ret = fdt_pack(fdt);
+    /* Should only fail if we've built a corrupted tree */
+    g_assert(ret == 0);
+    /* copy in the device tree */
+    qemu_fdt_dumpdtb(fdt, fdtsize);
+
+    rom_add_blob_fixed_as("fdt", fdt, fdtsize, fdt_addr,
+                          &address_space_memory);
+
+    return fdt_addr;
+}
+
diff --git a/hw/openrisc/meson.build b/hw/openrisc/meson.build
index ec48172c9d..ab563820c5 100644
--- a/hw/openrisc/meson.build
+++ b/hw/openrisc/meson.build
@@ -1,5 +1,6 @@
 openrisc_ss = ss.source_set()
 openrisc_ss.add(files('cputimer.c'))
+openrisc_ss.add(files('boot.c'))
 openrisc_ss.add(when: 'CONFIG_OR1K_SIM', if_true: [files('openrisc_sim.c'), fdt])
 
 hw_arch += {'openrisc': openrisc_ss}
diff --git a/hw/openrisc/openrisc_sim.c b/hw/openrisc/openrisc_sim.c
index 35adce17ac..35da123aef 100644
--- a/hw/openrisc/openrisc_sim.c
+++ b/hw/openrisc/openrisc_sim.c
@@ -24,10 +24,9 @@
 #include "cpu.h"
 #include "hw/irq.h"
 #include "hw/boards.h"
-#include "elf.h"
 #include "hw/char/serial.h"
 #include "net/net.h"
-#include "hw/loader.h"
+#include "hw/openrisc/boot.h"
 #include "hw/qdev-properties.h"
 #include "exec/address-spaces.h"
 #include "sysemu/device_tree.h"
@@ -283,101 +282,6 @@ static void openrisc_sim_serial_init(Or1ksimState *state, hwaddr base,
     g_free(nodename);
 }
 
-static hwaddr openrisc_load_kernel(ram_addr_t ram_size,
-                                   const char *kernel_filename)
-{
-    long kernel_size;
-    uint64_t elf_entry;
-    uint64_t high_addr;
-    hwaddr entry;
-
-    if (kernel_filename && !qtest_enabled()) {
-        kernel_size = load_elf(kernel_filename, NULL, NULL, NULL,
-                               &elf_entry, NULL, &high_addr, NULL, 1,
-                               EM_OPENRISC, 1, 0);
-        entry = elf_entry;
-        if (kernel_size < 0) {
-            kernel_size = load_uimage(kernel_filename,
-                                      &entry, NULL, NULL, NULL, NULL);
-            high_addr = entry + kernel_size;
-        }
-        if (kernel_size < 0) {
-            kernel_size = load_image_targphys(kernel_filename,
-                                              KERNEL_LOAD_ADDR,
-                                              ram_size - KERNEL_LOAD_ADDR);
-            high_addr = KERNEL_LOAD_ADDR + kernel_size;
-        }
-
-        if (entry <= 0) {
-            entry = KERNEL_LOAD_ADDR;
-        }
-
-        if (kernel_size < 0) {
-            error_report("couldn't load the kernel '%s'", kernel_filename);
-            exit(1);
-        }
-        boot_info.bootstrap_pc = entry;
-
-        return high_addr;
-    }
-    return 0;
-}
-
-static hwaddr openrisc_load_initrd(Or1ksimState *state, const char *filename,
-                                   hwaddr load_start, uint64_t mem_size)
-{
-    void *fdt = state->fdt;
-    int size;
-    hwaddr start;
-
-    /* We put the initrd right after the kernel; page aligned. */
-    start = TARGET_PAGE_ALIGN(load_start);
-
-    size = load_ramdisk(filename, start, mem_size - start);
-    if (size < 0) {
-        size = load_image_targphys(filename, start, mem_size - start);
-        if (size < 0) {
-            error_report("could not load ramdisk '%s'", filename);
-            exit(1);
-        }
-    }
-
-    qemu_fdt_setprop_cell(fdt, "/chosen",
-                          "linux,initrd-start", start);
-    qemu_fdt_setprop_cell(fdt, "/chosen",
-                          "linux,initrd-end", start + size);
-
-    return start + size;
-}
-
-static uint32_t openrisc_load_fdt(Or1ksimState *state, hwaddr load_start,
-                                  uint64_t mem_size)
-{
-    void *fdt = state->fdt;
-    uint32_t fdt_addr;
-    int ret;
-    int fdtsize = fdt_totalsize(fdt);
-
-    if (fdtsize <= 0) {
-        error_report("invalid device-tree");
-        exit(1);
-    }
-
-    /* We put fdt right after the kernel and/or initrd. */
-    fdt_addr = TARGET_PAGE_ALIGN(load_start);
-
-    ret = fdt_pack(fdt);
-    /* Should only fail if we've built a corrupted tree */
-    g_assert(ret == 0);
-    /* copy in the device tree */
-    qemu_fdt_dumpdtb(fdt, fdtsize);
-
-    rom_add_blob_fixed_as("fdt", fdt, fdtsize, fdt_addr,
-                          &address_space_memory);
-
-    return fdt_addr;
-}
-
 static void openrisc_sim_init(MachineState *machine)
 {
     ram_addr_t ram_size = machine->ram_size;
@@ -428,13 +332,15 @@ static void openrisc_sim_init(MachineState *machine)
                                  or1ksim_memmap[OR1KSIM_UART].size,
                                  smp_cpus, cpus, OR1KSIM_UART_IRQ, n);
 
-    load_addr = openrisc_load_kernel(ram_size, kernel_filename);
+    load_addr = openrisc_load_kernel(ram_size, kernel_filename,
+                                     &boot_info.bootstrap_pc);
     if (load_addr > 0) {
         if (machine->initrd_filename) {
-            load_addr = openrisc_load_initrd(state, machine->initrd_filename,
+            load_addr = openrisc_load_initrd(state->fdt,
+                                             machine->initrd_filename,
                                              load_addr, machine->ram_size);
         }
-        boot_info.fdt_addr = openrisc_load_fdt(state, load_addr,
+        boot_info.fdt_addr = openrisc_load_fdt(state->fdt, load_addr,
                                                machine->ram_size);
     }
 }
diff --git a/include/hw/openrisc/boot.h b/include/hw/openrisc/boot.h
new file mode 100644
index 0000000000..25a313d63a
--- /dev/null
+++ b/include/hw/openrisc/boot.h
@@ -0,0 +1,34 @@
+/*
+ * QEMU OpenRISC boot helpers.
+ *
+ * Copyright (c) 2022 Stafford Horne <shorne@gmail.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2 or later, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef OPENRISC_BOOT_H
+#define OPENRISC_BOOT_H
+
+#include "exec/cpu-defs.h"
+
+hwaddr openrisc_load_kernel(ram_addr_t ram_size,
+                            const char *kernel_filename,
+                            uint32_t *bootstrap_pc);
+
+hwaddr openrisc_load_initrd(void *fdt, const char *filename,
+                            hwaddr load_start, uint64_t mem_size);
+
+uint32_t openrisc_load_fdt(void *fdt, hwaddr load_start,
+                           uint64_t mem_size);
+
+#endif /* OPENRISC_BOOT_H */
-- 
2.31.1

Re: [RFC PATCH 2/3] hw/openrisc: Split re-usable boot time apis out to boot.c

[Richard Henderson]

On 5/27/22 10:27, Stafford Horne wrote:
> These will be shared with the virt platform.
> 
> Signed-off-by: Stafford Horne<shorne@gmail.com>
> ---
>   hw/openrisc/boot.c         | 127 +++++++++++++++++++++++++++++++++++++
>   hw/openrisc/meson.build    |   1 +
>   hw/openrisc/openrisc_sim.c | 106 ++-----------------------------
>   include/hw/openrisc/boot.h |  34 ++++++++++
>   4 files changed, 168 insertions(+), 100 deletions(-)
>   create mode 100644 hw/openrisc/boot.c
>   create mode 100644 include/hw/openrisc/boot.h

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [RFC PATCH 2/3] hw/openrisc: Split re-usable boot time apis out to boot.c

[Richard Henderson]

On 5/27/22 10:27, Stafford Horne wrote:
> These will be shared with the virt platform.
> 
> Signed-off-by: Stafford Horne<shorne@gmail.com>
> ---
>   hw/openrisc/boot.c         | 127 +++++++++++++++++++++++++++++++++++++
>   hw/openrisc/meson.build    |   1 +
>   hw/openrisc/openrisc_sim.c | 106 ++-----------------------------
>   include/hw/openrisc/boot.h |  34 ++++++++++
>   4 files changed, 168 insertions(+), 100 deletions(-)
>   create mode 100644 hw/openrisc/boot.c
>   create mode 100644 include/hw/openrisc/boot.h

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[RFC PATCH 3/3] hw/openrisc: Add the OpenRISC virtual machine

[Stafford Horne]

This patch add the OpenRISC virtual machine 'virt' for OpenRISC.  This
platform allows for a convenient CI platform for toolchain, software
ports and the OpenRISC linux kernel port.

Much of this has been sourced from the m68k and riscv virt platforms.

The platform provides:
 - OpenRISC SMP with up to 8 cpus
 - A virtio bus with up to 8 devices
 - Standard ns16550a serial
 - Goldfish RTC
 - SiFive TEST device for poweroff and reboot
 - Generated RTC to automatically configure the guest kernel

Signed-off-by: Stafford Horne <shorne@gmail.com>
---
 configs/devices/or1k-softmmu/default.mak |   1 +
 hw/openrisc/Kconfig                      |   9 +
 hw/openrisc/meson.build                  |   1 +
 hw/openrisc/virt.c                       | 429 +++++++++++++++++++++++
 4 files changed, 440 insertions(+)
 create mode 100644 hw/openrisc/virt.c

diff --git a/configs/devices/or1k-softmmu/default.mak b/configs/devices/or1k-softmmu/default.mak
index 5b3ac89491..f3bf816067 100644
--- a/configs/devices/or1k-softmmu/default.mak
+++ b/configs/devices/or1k-softmmu/default.mak
@@ -5,3 +5,4 @@ CONFIG_SEMIHOSTING=y
 # Boards:
 #
 CONFIG_OR1K_SIM=y
+CONFIG_OR1K_VIRT=y
diff --git a/hw/openrisc/Kconfig b/hw/openrisc/Kconfig
index 8f284f3ba0..202134668e 100644
--- a/hw/openrisc/Kconfig
+++ b/hw/openrisc/Kconfig
@@ -4,3 +4,12 @@ config OR1K_SIM
     select OPENCORES_ETH
     select OMPIC
     select SPLIT_IRQ
+
+config OR1K_VIRT
+    bool
+    imply VIRTIO_VGA
+    imply TEST_DEVICES
+    select GOLDFISH_RTC
+    select SERIAL
+    select SIFIVE_TEST
+    select VIRTIO_MMIO
diff --git a/hw/openrisc/meson.build b/hw/openrisc/meson.build
index ab563820c5..2dbc6365bb 100644
--- a/hw/openrisc/meson.build
+++ b/hw/openrisc/meson.build
@@ -2,5 +2,6 @@ openrisc_ss = ss.source_set()
 openrisc_ss.add(files('cputimer.c'))
 openrisc_ss.add(files('boot.c'))
 openrisc_ss.add(when: 'CONFIG_OR1K_SIM', if_true: [files('openrisc_sim.c'), fdt])
+openrisc_ss.add(when: 'CONFIG_OR1K_VIRT', if_true: [files('virt.c'), fdt])
 
 hw_arch += {'openrisc': openrisc_ss}
diff --git a/hw/openrisc/virt.c b/hw/openrisc/virt.c
new file mode 100644
index 0000000000..147196fda3
--- /dev/null
+++ b/hw/openrisc/virt.c
@@ -0,0 +1,429 @@
+/*
+ * OpenRISC QEMU virtual machine.
+ *
+ * Copyright (c) 2022 Stafford Horne <shorne@gmail.com>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/error-report.h"
+#include "qapi/error.h"
+#include "cpu.h"
+#include "hw/irq.h"
+#include "hw/boards.h"
+#include "hw/char/serial.h"
+#include "hw/openrisc/boot.h"
+#include "hw/misc/sifive_test.h"
+#include "hw/qdev-properties.h"
+#include "exec/address-spaces.h"
+#include "sysemu/device_tree.h"
+#include "sysemu/sysemu.h"
+#include "hw/sysbus.h"
+#include "sysemu/qtest.h"
+#include "sysemu/reset.h"
+#include "hw/core/split-irq.h"
+
+#include <libfdt.h>
+
+#define VIRT_CPUS_MAX 4
+#define VIRT_CLK_MHZ 20000000
+
+#define TYPE_VIRT_MACHINE MACHINE_TYPE_NAME("virt")
+#define VIRT_MACHINE(obj) \
+    OBJECT_CHECK(OR1KVirtState, (obj), TYPE_VIRT_MACHINE)
+
+typedef struct OR1KVirtState {
+    /*< private >*/
+    MachineState parent_obj;
+
+    /*< public >*/
+    void *fdt;
+    int fdt_size;
+
+} OR1KVirtState;
+
+enum {
+    VIRT_DRAM,
+    VIRT_TEST,
+    VIRT_RTC,
+    VIRT_VIRTIO,
+    VIRT_UART,
+    VIRT_OMPIC,
+};
+
+enum {
+    VIRT_OMPIC_IRQ = 1,
+    VIRT_UART_IRQ = 2,
+    VIRT_RTC_IRQ = 3,
+    VIRT_VIRTIO_IRQ = 4, /* to 12 */
+    VIRTIO_COUNT = 8,
+};
+
+static const struct MemmapEntry {
+    hwaddr base;
+    hwaddr size;
+} virt_memmap[] = {
+    [VIRT_DRAM] =      { 0x00000000,          0 },
+    [VIRT_UART] =      { 0x90000000,      0x100 },
+    [VIRT_TEST] =      { 0x96000000,        0x8 },
+    [VIRT_RTC] =       { 0x96005000,     0x1000 },
+    [VIRT_VIRTIO] =    { 0x97000000,     0x1000 },
+    [VIRT_OMPIC] =     { 0x98000000, VIRT_CPUS_MAX * 8 },
+};
+
+static struct openrisc_boot_info {
+    uint32_t bootstrap_pc;
+    uint32_t fdt_addr;
+} boot_info;
+
+static void main_cpu_reset(void *opaque)
+{
+    OpenRISCCPU *cpu = opaque;
+    CPUState *cs = CPU(cpu);
+
+    cpu_reset(CPU(cpu));
+
+    cpu_set_pc(cs, boot_info.bootstrap_pc);
+    cpu_set_gpr(&cpu->env, 3, boot_info.fdt_addr);
+}
+
+static qemu_irq get_cpu_irq(OpenRISCCPU *cpus[], int cpunum, int irq_pin)
+{
+    return qdev_get_gpio_in_named(DEVICE(cpus[cpunum]), "IRQ", irq_pin);
+}
+
+static qemu_irq get_per_cpu_irq(OpenRISCCPU *cpus[], int num_cpus, int irq_pin)
+{
+    int i;
+
+    if (num_cpus > 1) {
+        DeviceState *splitter = qdev_new(TYPE_SPLIT_IRQ);
+        qdev_prop_set_uint32(splitter, "num-lines", num_cpus);
+        qdev_realize_and_unref(splitter, NULL, &error_fatal);
+        for (i = 0; i < num_cpus; i++) {
+            qdev_connect_gpio_out(splitter, i, get_cpu_irq(cpus, i, irq_pin));
+        }
+        return qdev_get_gpio_in(splitter, 0);
+    } else {
+        return get_cpu_irq(cpus, 0, irq_pin);
+    }
+}
+
+static void openrisc_create_fdt(OR1KVirtState *state,
+                                const struct MemmapEntry *memmap,
+                                int num_cpus, uint64_t mem_size,
+                                const char *cmdline)
+{
+    void *fdt;
+    int cpu;
+    char *nodename;
+    int pic_ph;
+
+    fdt = state->fdt = create_device_tree(&state->fdt_size);
+    if (!fdt) {
+        error_report("create_device_tree() failed");
+        exit(1);
+    }
+
+    qemu_fdt_setprop_string(fdt, "/", "compatible", "opencores,or1ksim");
+    qemu_fdt_setprop_cell(fdt, "/", "#address-cells", 0x1);
+    qemu_fdt_setprop_cell(fdt, "/", "#size-cells", 0x1);
+
+    qemu_fdt_add_subnode(fdt, "/soc");
+    qemu_fdt_setprop(fdt, "/soc", "ranges", NULL, 0);
+    qemu_fdt_setprop_string(fdt, "/soc", "compatible", "simple-bus");
+    qemu_fdt_setprop_cell(fdt, "/soc", "#address-cells", 0x1);
+    qemu_fdt_setprop_cell(fdt, "/soc", "#size-cells", 0x1);
+
+    nodename = g_strdup_printf("/memory@%" HWADDR_PRIx,
+                               memmap[VIRT_DRAM].base);
+    qemu_fdt_add_subnode(fdt, nodename);
+    qemu_fdt_setprop_cells(fdt, nodename, "reg",
+                           memmap[VIRT_DRAM].base, mem_size);
+    qemu_fdt_setprop_string(fdt, nodename, "device_type", "memory");
+    g_free(nodename);
+
+    qemu_fdt_add_subnode(fdt, "/cpus");
+    qemu_fdt_setprop_cell(fdt, "/cpus", "#size-cells", 0x0);
+    qemu_fdt_setprop_cell(fdt, "/cpus", "#address-cells", 0x1);
+
+    for (cpu = 0; cpu < num_cpus; cpu++) {
+        nodename = g_strdup_printf("/cpus/cpu@%d", cpu);
+        qemu_fdt_add_subnode(fdt, nodename);
+        qemu_fdt_setprop_string(fdt, nodename, "compatible",
+                                "opencores,or1200-rtlsvn481");
+        qemu_fdt_setprop_cell(fdt, nodename, "reg", cpu);
+        qemu_fdt_setprop_cell(fdt, nodename, "clock-frequency",
+                              VIRT_CLK_MHZ);
+        g_free(nodename);
+    }
+
+    nodename = (char *)"/pic";
+    qemu_fdt_add_subnode(fdt, nodename);
+    pic_ph = qemu_fdt_alloc_phandle(fdt);
+    qemu_fdt_setprop_string(fdt, nodename, "compatible",
+                            "opencores,or1k-pic-level");
+    qemu_fdt_setprop_cell(fdt, nodename, "#interrupt-cells", 1);
+    qemu_fdt_setprop(fdt, nodename, "interrupt-controller", NULL, 0);
+    qemu_fdt_setprop_cell(fdt, nodename, "phandle", pic_ph);
+
+    qemu_fdt_setprop_cell(fdt, "/", "interrupt-parent", pic_ph);
+
+    qemu_fdt_add_subnode(fdt, "/chosen");
+    if (cmdline) {
+        qemu_fdt_setprop_string(fdt, "/chosen", "bootargs", cmdline);
+    }
+
+    /* Create aliases node for use by devices. */
+    qemu_fdt_add_subnode(fdt, "/aliases");
+}
+
+static void openrisc_virt_ompic_init(OR1KVirtState *state, hwaddr base,
+                                    hwaddr size, int num_cpus,
+                                    OpenRISCCPU *cpus[], int irq_pin)
+{
+    void *fdt = state->fdt;
+    DeviceState *dev;
+    SysBusDevice *s;
+    char *nodename;
+    int i;
+
+    dev = qdev_new("or1k-ompic");
+    qdev_prop_set_uint32(dev, "num-cpus", num_cpus);
+
+    s = SYS_BUS_DEVICE(dev);
+    sysbus_realize_and_unref(s, &error_fatal);
+    for (i = 0; i < num_cpus; i++) {
+        sysbus_connect_irq(s, i, get_cpu_irq(cpus, i, irq_pin));
+    }
+    sysbus_mmio_map(s, 0, base);
+
+    /* Add device tree node for ompic. */
+    nodename = g_strdup_printf("/ompic@%" HWADDR_PRIx, base);
+    qemu_fdt_add_subnode(fdt, nodename);
+    qemu_fdt_setprop_string(fdt, nodename, "compatible", "openrisc,ompic");
+    qemu_fdt_setprop_cells(fdt, nodename, "reg", base, size);
+    qemu_fdt_setprop(fdt, nodename, "interrupt-controller", NULL, 0);
+    qemu_fdt_setprop_cell(fdt, nodename, "#interrupt-cells", 0);
+    qemu_fdt_setprop_cell(fdt, nodename, "interrupts", irq_pin);
+    g_free(nodename);
+}
+
+static void openrisc_virt_serial_init(OR1KVirtState *state, hwaddr base,
+                                     hwaddr size, int num_cpus,
+                                     OpenRISCCPU *cpus[], int irq_pin)
+{
+    void *fdt = state->fdt;
+    char *nodename;
+    qemu_irq serial_irq = get_per_cpu_irq(cpus, num_cpus, irq_pin);
+
+    serial_mm_init(get_system_memory(), base, 0, serial_irq, 115200,
+                   serial_hd(0), DEVICE_NATIVE_ENDIAN);
+
+    /* Add device tree node for serial. */
+    nodename = g_strdup_printf("/serial@%" HWADDR_PRIx, base);
+    qemu_fdt_add_subnode(fdt, nodename);
+    qemu_fdt_setprop_string(fdt, nodename, "compatible", "ns16550a");
+    qemu_fdt_setprop_cells(fdt, nodename, "reg", base, size);
+    qemu_fdt_setprop_cell(fdt, nodename, "interrupts", irq_pin);
+    qemu_fdt_setprop_cell(fdt, nodename, "clock-frequency", VIRT_CLK_MHZ);
+    qemu_fdt_setprop(fdt, nodename, "big-endian", NULL, 0);
+
+    /* The /chosen node is created during fdt creation. */
+    qemu_fdt_setprop_string(fdt, "/chosen", "stdout-path", nodename);
+    qemu_fdt_setprop_string(fdt, "/aliases", "uart0", nodename);
+    g_free(nodename);
+}
+
+static void openrisc_virt_test_init(OR1KVirtState *state, hwaddr base,
+                                   hwaddr size)
+{
+    void *fdt = state->fdt;
+    int test_ph;
+    char *nodename;
+
+    /* SiFive Test MMIO device */
+    sifive_test_create(base);
+
+    /* SiFive Test MMIO Reset device FDT */
+    nodename = g_strdup_printf("/soc/test@%" HWADDR_PRIx, base);
+    qemu_fdt_add_subnode(fdt, nodename);
+    qemu_fdt_setprop_string(fdt, nodename, "compatible", "syscon");
+    test_ph = qemu_fdt_alloc_phandle(fdt);
+    qemu_fdt_setprop_cells(fdt, nodename, "reg", base, size);
+    qemu_fdt_setprop_cell(fdt, nodename, "phandle", test_ph);
+    qemu_fdt_setprop(fdt, nodename, "big-endian", NULL, 0);
+    g_free(nodename);
+
+    nodename = g_strdup_printf("/soc/reboot");
+    qemu_fdt_add_subnode(fdt, nodename);
+    qemu_fdt_setprop_string(fdt, nodename, "compatible", "syscon-reboot");
+    qemu_fdt_setprop_cell(fdt, nodename, "regmap", test_ph);
+    qemu_fdt_setprop_cell(fdt, nodename, "offset", 0x0);
+    qemu_fdt_setprop_cell(fdt, nodename, "value", FINISHER_RESET);
+    g_free(nodename);
+
+    nodename = g_strdup_printf("/soc/poweroff");
+    qemu_fdt_add_subnode(fdt, nodename);
+    qemu_fdt_setprop_string(fdt, nodename, "compatible", "syscon-poweroff");
+    qemu_fdt_setprop_cell(fdt, nodename, "regmap", test_ph);
+    qemu_fdt_setprop_cell(fdt, nodename, "offset", 0x0);
+    qemu_fdt_setprop_cell(fdt, nodename, "value", FINISHER_PASS);
+    g_free(nodename);
+
+}
+static void openrisc_virt_rtc_init(OR1KVirtState *state, hwaddr base,
+                                   hwaddr size, int num_cpus,
+                                   OpenRISCCPU *cpus[], int irq_pin)
+{
+    void *fdt = state->fdt;
+    char *nodename;
+    qemu_irq rtc_irq = get_per_cpu_irq(cpus, num_cpus, irq_pin);
+
+    /* Goldfish RTC */
+    sysbus_create_simple("goldfish_rtc", base, rtc_irq);
+
+    /* Goldfish RTC FDT */
+    nodename = g_strdup_printf("/soc/rtc@%" HWADDR_PRIx, base);
+    qemu_fdt_add_subnode(fdt, nodename);
+    qemu_fdt_setprop_string(fdt, nodename, "compatible",
+                            "google,goldfish-rtc");
+    qemu_fdt_setprop_cells(fdt, nodename, "reg", base, size);
+    qemu_fdt_setprop_cell(fdt, nodename, "interrupts", irq_pin);
+    g_free(nodename);
+
+}
+static void openrisc_virt_virtio_init(OR1KVirtState *state, hwaddr base,
+                                      hwaddr size, int num_cpus,
+                                      OpenRISCCPU *cpus[], int irq_pin,
+                                      int virtio_idx)
+{
+    void *fdt = state->fdt;
+    char *nodename;
+    DeviceState *dev;
+    SysBusDevice *sysbus;
+    qemu_irq virtio_irq = get_per_cpu_irq(cpus, num_cpus, irq_pin + virtio_idx);
+
+    /* VirtIO MMIO devices */
+    dev = qdev_new("virtio-mmio");
+    qdev_prop_set_bit(dev, "force-legacy", false);
+    sysbus = SYS_BUS_DEVICE(dev);
+    sysbus_realize_and_unref(sysbus, &error_fatal);
+    sysbus_connect_irq(sysbus, 0, virtio_irq);
+    sysbus_mmio_map(sysbus, 0, base + virtio_idx * size);
+
+    /* VirtIO MMIO devices FDT */
+    nodename = g_strdup_printf("/soc/virtio_mmio@%" HWADDR_PRIx,
+                               base + virtio_idx * size);
+    qemu_fdt_add_subnode(fdt, nodename);
+    qemu_fdt_setprop_string(fdt, nodename, "compatible", "virtio,mmio");
+    qemu_fdt_setprop_cells(fdt, nodename, "reg",
+                           base + virtio_idx * size,
+                           size);
+    qemu_fdt_setprop_cell(fdt, nodename, "interrupts", irq_pin + virtio_idx);
+    g_free(nodename);
+}
+
+static void openrisc_virt_init(MachineState *machine)
+{
+    ram_addr_t ram_size = machine->ram_size;
+    const char *kernel_filename = machine->kernel_filename;
+    OpenRISCCPU *cpus[VIRT_CPUS_MAX] = {};
+    OR1KVirtState *state = VIRT_MACHINE(machine);
+    MemoryRegion *ram;
+    hwaddr load_addr;
+    int n;
+    unsigned int smp_cpus = machine->smp.cpus;
+
+    assert(smp_cpus >= 1 && smp_cpus <= VIRT_CPUS_MAX);
+    for (n = 0; n < smp_cpus; n++) {
+        cpus[n] = OPENRISC_CPU(cpu_create(machine->cpu_type));
+        if (cpus[n] == NULL) {
+            fprintf(stderr, "Unable to find CPU definition!\n");
+            exit(1);
+        }
+
+        cpu_openrisc_clock_init(cpus[n]);
+
+        qemu_register_reset(main_cpu_reset, cpus[n]);
+    }
+
+    ram = g_malloc(sizeof(*ram));
+    memory_region_init_ram(ram, NULL, "openrisc.ram", ram_size, &error_fatal);
+    memory_region_add_subregion(get_system_memory(), 0, ram);
+
+    openrisc_create_fdt(state, virt_memmap, smp_cpus, machine->ram_size,
+                        machine->kernel_cmdline);
+
+    if (smp_cpus > 1) {
+        openrisc_virt_ompic_init(state, virt_memmap[VIRT_OMPIC].base,
+                                 virt_memmap[VIRT_OMPIC].size,
+                                 smp_cpus, cpus, VIRT_OMPIC_IRQ);
+    }
+
+    openrisc_virt_serial_init(state, virt_memmap[VIRT_UART].base,
+                              virt_memmap[VIRT_UART].size,
+                              smp_cpus, cpus, VIRT_UART_IRQ);
+
+    openrisc_virt_test_init(state, virt_memmap[VIRT_TEST].base,
+                            virt_memmap[VIRT_TEST].size);
+
+    openrisc_virt_rtc_init(state, virt_memmap[VIRT_RTC].base,
+                           virt_memmap[VIRT_RTC].size, smp_cpus, cpus,
+                           VIRT_RTC_IRQ);
+
+    for (n = 0; n < VIRTIO_COUNT; n++) {
+        openrisc_virt_virtio_init(state, virt_memmap[VIRT_VIRTIO].base,
+                                  virt_memmap[VIRT_VIRTIO].size,
+                                  smp_cpus, cpus, VIRT_VIRTIO_IRQ, n);
+    }
+
+    load_addr = openrisc_load_kernel(ram_size, kernel_filename,
+                                     &boot_info.bootstrap_pc);
+    if (load_addr > 0) {
+        if (machine->initrd_filename) {
+            load_addr = openrisc_load_initrd(state->fdt,
+                                             machine->initrd_filename,
+                                             load_addr, machine->ram_size);
+        }
+        boot_info.fdt_addr = openrisc_load_fdt(state->fdt, load_addr,
+                                               machine->ram_size);
+    }
+}
+
+static void openrisc_virt_machine_init(ObjectClass *oc, void *data)
+{
+    MachineClass *mc = MACHINE_CLASS(oc);
+
+    mc->desc = "or1k virtual machine";
+    mc->init = openrisc_virt_init;
+    mc->max_cpus = VIRT_CPUS_MAX;
+    mc->is_default = false;
+    mc->default_cpu_type = OPENRISC_CPU_TYPE_NAME("or1200");
+}
+
+static const TypeInfo or1ksim_machine_typeinfo = {
+    .name       = TYPE_VIRT_MACHINE,
+    .parent     = TYPE_MACHINE,
+    .class_init = openrisc_virt_machine_init,
+    .instance_size = sizeof(OR1KVirtState),
+};
+
+static void or1ksim_machine_init_register_types(void)
+{
+    type_register_static(&or1ksim_machine_typeinfo);
+}
+
+type_init(or1ksim_machine_init_register_types)
-- 
2.31.1

[RFC PATCH 3/3] hw/openrisc: Add the OpenRISC virtual machine

[Stafford Horne]

This patch add the OpenRISC virtual machine 'virt' for OpenRISC.  This
platform allows for a convenient CI platform for toolchain, software
ports and the OpenRISC linux kernel port.

Much of this has been sourced from the m68k and riscv virt platforms.

The platform provides:
 - OpenRISC SMP with up to 8 cpus
 - A virtio bus with up to 8 devices
 - Standard ns16550a serial
 - Goldfish RTC
 - SiFive TEST device for poweroff and reboot
 - Generated RTC to automatically configure the guest kernel

Signed-off-by: Stafford Horne <shorne@gmail.com>
---
 configs/devices/or1k-softmmu/default.mak |   1 +
 hw/openrisc/Kconfig                      |   9 +
 hw/openrisc/meson.build                  |   1 +
 hw/openrisc/virt.c                       | 429 +++++++++++++++++++++++
 4 files changed, 440 insertions(+)
 create mode 100644 hw/openrisc/virt.c

diff --git a/configs/devices/or1k-softmmu/default.mak b/configs/devices/or1k-softmmu/default.mak
index 5b3ac89491..f3bf816067 100644
--- a/configs/devices/or1k-softmmu/default.mak
+++ b/configs/devices/or1k-softmmu/default.mak
@@ -5,3 +5,4 @@ CONFIG_SEMIHOSTING=y
 # Boards:
 #
 CONFIG_OR1K_SIM=y
+CONFIG_OR1K_VIRT=y
diff --git a/hw/openrisc/Kconfig b/hw/openrisc/Kconfig
index 8f284f3ba0..202134668e 100644
--- a/hw/openrisc/Kconfig
+++ b/hw/openrisc/Kconfig
@@ -4,3 +4,12 @@ config OR1K_SIM
     select OPENCORES_ETH
     select OMPIC
     select SPLIT_IRQ
+
+config OR1K_VIRT
+    bool
+    imply VIRTIO_VGA
+    imply TEST_DEVICES
+    select GOLDFISH_RTC
+    select SERIAL
+    select SIFIVE_TEST
+    select VIRTIO_MMIO
diff --git a/hw/openrisc/meson.build b/hw/openrisc/meson.build
index ab563820c5..2dbc6365bb 100644
--- a/hw/openrisc/meson.build
+++ b/hw/openrisc/meson.build
@@ -2,5 +2,6 @@ openrisc_ss = ss.source_set()
 openrisc_ss.add(files('cputimer.c'))
 openrisc_ss.add(files('boot.c'))
 openrisc_ss.add(when: 'CONFIG_OR1K_SIM', if_true: [files('openrisc_sim.c'), fdt])
+openrisc_ss.add(when: 'CONFIG_OR1K_VIRT', if_true: [files('virt.c'), fdt])
 
 hw_arch += {'openrisc': openrisc_ss}
diff --git a/hw/openrisc/virt.c b/hw/openrisc/virt.c
new file mode 100644
index 0000000000..147196fda3
--- /dev/null
+++ b/hw/openrisc/virt.c
@@ -0,0 +1,429 @@
+/*
+ * OpenRISC QEMU virtual machine.
+ *
+ * Copyright (c) 2022 Stafford Horne <shorne@gmail.com>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/error-report.h"
+#include "qapi/error.h"
+#include "cpu.h"
+#include "hw/irq.h"
+#include "hw/boards.h"
+#include "hw/char/serial.h"
+#include "hw/openrisc/boot.h"
+#include "hw/misc/sifive_test.h"
+#include "hw/qdev-properties.h"
+#include "exec/address-spaces.h"
+#include "sysemu/device_tree.h"
+#include "sysemu/sysemu.h"
+#include "hw/sysbus.h"
+#include "sysemu/qtest.h"
+#include "sysemu/reset.h"
+#include "hw/core/split-irq.h"
+
+#include <libfdt.h>
+
+#define VIRT_CPUS_MAX 4
+#define VIRT_CLK_MHZ 20000000
+
+#define TYPE_VIRT_MACHINE MACHINE_TYPE_NAME("virt")
+#define VIRT_MACHINE(obj) \
+    OBJECT_CHECK(OR1KVirtState, (obj), TYPE_VIRT_MACHINE)
+
+typedef struct OR1KVirtState {
+    /*< private >*/
+    MachineState parent_obj;
+
+    /*< public >*/
+    void *fdt;
+    int fdt_size;
+
+} OR1KVirtState;
+
+enum {
+    VIRT_DRAM,
+    VIRT_TEST,
+    VIRT_RTC,
+    VIRT_VIRTIO,
+    VIRT_UART,
+    VIRT_OMPIC,
+};
+
+enum {
+    VIRT_OMPIC_IRQ = 1,
+    VIRT_UART_IRQ = 2,
+    VIRT_RTC_IRQ = 3,
+    VIRT_VIRTIO_IRQ = 4, /* to 12 */
+    VIRTIO_COUNT = 8,
+};
+
+static const struct MemmapEntry {
+    hwaddr base;
+    hwaddr size;
+} virt_memmap[] = {
+    [VIRT_DRAM] =      { 0x00000000,          0 },
+    [VIRT_UART] =      { 0x90000000,      0x100 },
+    [VIRT_TEST] =      { 0x96000000,        0x8 },
+    [VIRT_RTC] =       { 0x96005000,     0x1000 },
+    [VIRT_VIRTIO] =    { 0x97000000,     0x1000 },
+    [VIRT_OMPIC] =     { 0x98000000, VIRT_CPUS_MAX * 8 },
+};
+
+static struct openrisc_boot_info {
+    uint32_t bootstrap_pc;
+    uint32_t fdt_addr;
+} boot_info;
+
+static void main_cpu_reset(void *opaque)
+{
+    OpenRISCCPU *cpu = opaque;
+    CPUState *cs = CPU(cpu);
+
+    cpu_reset(CPU(cpu));
+
+    cpu_set_pc(cs, boot_info.bootstrap_pc);
+    cpu_set_gpr(&cpu->env, 3, boot_info.fdt_addr);
+}
+
+static qemu_irq get_cpu_irq(OpenRISCCPU *cpus[], int cpunum, int irq_pin)
+{
+    return qdev_get_gpio_in_named(DEVICE(cpus[cpunum]), "IRQ", irq_pin);
+}
+
+static qemu_irq get_per_cpu_irq(OpenRISCCPU *cpus[], int num_cpus, int irq_pin)
+{
+    int i;
+
+    if (num_cpus > 1) {
+        DeviceState *splitter = qdev_new(TYPE_SPLIT_IRQ);
+        qdev_prop_set_uint32(splitter, "num-lines", num_cpus);
+        qdev_realize_and_unref(splitter, NULL, &error_fatal);
+        for (i = 0; i < num_cpus; i++) {
+            qdev_connect_gpio_out(splitter, i, get_cpu_irq(cpus, i, irq_pin));
+        }
+        return qdev_get_gpio_in(splitter, 0);
+    } else {
+        return get_cpu_irq(cpus, 0, irq_pin);
+    }
+}
+
+static void openrisc_create_fdt(OR1KVirtState *state,
+                                const struct MemmapEntry *memmap,
+                                int num_cpus, uint64_t mem_size,
+                                const char *cmdline)
+{
+    void *fdt;
+    int cpu;
+    char *nodename;
+    int pic_ph;
+
+    fdt = state->fdt = create_device_tree(&state->fdt_size);
+    if (!fdt) {
+        error_report("create_device_tree() failed");
+        exit(1);
+    }
+
+    qemu_fdt_setprop_string(fdt, "/", "compatible", "opencores,or1ksim");
+    qemu_fdt_setprop_cell(fdt, "/", "#address-cells", 0x1);
+    qemu_fdt_setprop_cell(fdt, "/", "#size-cells", 0x1);
+
+    qemu_fdt_add_subnode(fdt, "/soc");
+    qemu_fdt_setprop(fdt, "/soc", "ranges", NULL, 0);
+    qemu_fdt_setprop_string(fdt, "/soc", "compatible", "simple-bus");
+    qemu_fdt_setprop_cell(fdt, "/soc", "#address-cells", 0x1);
+    qemu_fdt_setprop_cell(fdt, "/soc", "#size-cells", 0x1);
+
+    nodename = g_strdup_printf("/memory@%" HWADDR_PRIx,
+                               memmap[VIRT_DRAM].base);
+    qemu_fdt_add_subnode(fdt, nodename);
+    qemu_fdt_setprop_cells(fdt, nodename, "reg",
+                           memmap[VIRT_DRAM].base, mem_size);
+    qemu_fdt_setprop_string(fdt, nodename, "device_type", "memory");
+    g_free(nodename);
+
+    qemu_fdt_add_subnode(fdt, "/cpus");
+    qemu_fdt_setprop_cell(fdt, "/cpus", "#size-cells", 0x0);
+    qemu_fdt_setprop_cell(fdt, "/cpus", "#address-cells", 0x1);
+
+    for (cpu = 0; cpu < num_cpus; cpu++) {
+        nodename = g_strdup_printf("/cpus/cpu@%d", cpu);
+        qemu_fdt_add_subnode(fdt, nodename);
+        qemu_fdt_setprop_string(fdt, nodename, "compatible",
+                                "opencores,or1200-rtlsvn481");
+        qemu_fdt_setprop_cell(fdt, nodename, "reg", cpu);
+        qemu_fdt_setprop_cell(fdt, nodename, "clock-frequency",
+                              VIRT_CLK_MHZ);
+        g_free(nodename);
+    }
+
+    nodename = (char *)"/pic";
+    qemu_fdt_add_subnode(fdt, nodename);
+    pic_ph = qemu_fdt_alloc_phandle(fdt);
+    qemu_fdt_setprop_string(fdt, nodename, "compatible",
+                            "opencores,or1k-pic-level");
+    qemu_fdt_setprop_cell(fdt, nodename, "#interrupt-cells", 1);
+    qemu_fdt_setprop(fdt, nodename, "interrupt-controller", NULL, 0);
+    qemu_fdt_setprop_cell(fdt, nodename, "phandle", pic_ph);
+
+    qemu_fdt_setprop_cell(fdt, "/", "interrupt-parent", pic_ph);
+
+    qemu_fdt_add_subnode(fdt, "/chosen");
+    if (cmdline) {
+        qemu_fdt_setprop_string(fdt, "/chosen", "bootargs", cmdline);
+    }
+
+    /* Create aliases node for use by devices. */
+    qemu_fdt_add_subnode(fdt, "/aliases");
+}
+
+static void openrisc_virt_ompic_init(OR1KVirtState *state, hwaddr base,
+                                    hwaddr size, int num_cpus,
+                                    OpenRISCCPU *cpus[], int irq_pin)
+{
+    void *fdt = state->fdt;
+    DeviceState *dev;
+    SysBusDevice *s;
+    char *nodename;
+    int i;
+
+    dev = qdev_new("or1k-ompic");
+    qdev_prop_set_uint32(dev, "num-cpus", num_cpus);
+
+    s = SYS_BUS_DEVICE(dev);
+    sysbus_realize_and_unref(s, &error_fatal);
+    for (i = 0; i < num_cpus; i++) {
+        sysbus_connect_irq(s, i, get_cpu_irq(cpus, i, irq_pin));
+    }
+    sysbus_mmio_map(s, 0, base);
+
+    /* Add device tree node for ompic. */
+    nodename = g_strdup_printf("/ompic@%" HWADDR_PRIx, base);
+    qemu_fdt_add_subnode(fdt, nodename);
+    qemu_fdt_setprop_string(fdt, nodename, "compatible", "openrisc,ompic");
+    qemu_fdt_setprop_cells(fdt, nodename, "reg", base, size);
+    qemu_fdt_setprop(fdt, nodename, "interrupt-controller", NULL, 0);
+    qemu_fdt_setprop_cell(fdt, nodename, "#interrupt-cells", 0);
+    qemu_fdt_setprop_cell(fdt, nodename, "interrupts", irq_pin);
+    g_free(nodename);
+}
+
+static void openrisc_virt_serial_init(OR1KVirtState *state, hwaddr base,
+                                     hwaddr size, int num_cpus,
+                                     OpenRISCCPU *cpus[], int irq_pin)
+{
+    void *fdt = state->fdt;
+    char *nodename;
+    qemu_irq serial_irq = get_per_cpu_irq(cpus, num_cpus, irq_pin);
+
+    serial_mm_init(get_system_memory(), base, 0, serial_irq, 115200,
+                   serial_hd(0), DEVICE_NATIVE_ENDIAN);
+
+    /* Add device tree node for serial. */
+    nodename = g_strdup_printf("/serial@%" HWADDR_PRIx, base);
+    qemu_fdt_add_subnode(fdt, nodename);
+    qemu_fdt_setprop_string(fdt, nodename, "compatible", "ns16550a");
+    qemu_fdt_setprop_cells(fdt, nodename, "reg", base, size);
+    qemu_fdt_setprop_cell(fdt, nodename, "interrupts", irq_pin);
+    qemu_fdt_setprop_cell(fdt, nodename, "clock-frequency", VIRT_CLK_MHZ);
+    qemu_fdt_setprop(fdt, nodename, "big-endian", NULL, 0);
+
+    /* The /chosen node is created during fdt creation. */
+    qemu_fdt_setprop_string(fdt, "/chosen", "stdout-path", nodename);
+    qemu_fdt_setprop_string(fdt, "/aliases", "uart0", nodename);
+    g_free(nodename);
+}
+
+static void openrisc_virt_test_init(OR1KVirtState *state, hwaddr base,
+                                   hwaddr size)
+{
+    void *fdt = state->fdt;
+    int test_ph;
+    char *nodename;
+
+    /* SiFive Test MMIO device */
+    sifive_test_create(base);
+
+    /* SiFive Test MMIO Reset device FDT */
+    nodename = g_strdup_printf("/soc/test@%" HWADDR_PRIx, base);
+    qemu_fdt_add_subnode(fdt, nodename);
+    qemu_fdt_setprop_string(fdt, nodename, "compatible", "syscon");
+    test_ph = qemu_fdt_alloc_phandle(fdt);
+    qemu_fdt_setprop_cells(fdt, nodename, "reg", base, size);
+    qemu_fdt_setprop_cell(fdt, nodename, "phandle", test_ph);
+    qemu_fdt_setprop(fdt, nodename, "big-endian", NULL, 0);
+    g_free(nodename);
+
+    nodename = g_strdup_printf("/soc/reboot");
+    qemu_fdt_add_subnode(fdt, nodename);
+    qemu_fdt_setprop_string(fdt, nodename, "compatible", "syscon-reboot");
+    qemu_fdt_setprop_cell(fdt, nodename, "regmap", test_ph);
+    qemu_fdt_setprop_cell(fdt, nodename, "offset", 0x0);
+    qemu_fdt_setprop_cell(fdt, nodename, "value", FINISHER_RESET);
+    g_free(nodename);
+
+    nodename = g_strdup_printf("/soc/poweroff");
+    qemu_fdt_add_subnode(fdt, nodename);
+    qemu_fdt_setprop_string(fdt, nodename, "compatible", "syscon-poweroff");
+    qemu_fdt_setprop_cell(fdt, nodename, "regmap", test_ph);
+    qemu_fdt_setprop_cell(fdt, nodename, "offset", 0x0);
+    qemu_fdt_setprop_cell(fdt, nodename, "value", FINISHER_PASS);
+    g_free(nodename);
+
+}
+static void openrisc_virt_rtc_init(OR1KVirtState *state, hwaddr base,
+                                   hwaddr size, int num_cpus,
+                                   OpenRISCCPU *cpus[], int irq_pin)
+{
+    void *fdt = state->fdt;
+    char *nodename;
+    qemu_irq rtc_irq = get_per_cpu_irq(cpus, num_cpus, irq_pin);
+
+    /* Goldfish RTC */
+    sysbus_create_simple("goldfish_rtc", base, rtc_irq);
+
+    /* Goldfish RTC FDT */
+    nodename = g_strdup_printf("/soc/rtc@%" HWADDR_PRIx, base);
+    qemu_fdt_add_subnode(fdt, nodename);
+    qemu_fdt_setprop_string(fdt, nodename, "compatible",
+                            "google,goldfish-rtc");
+    qemu_fdt_setprop_cells(fdt, nodename, "reg", base, size);
+    qemu_fdt_setprop_cell(fdt, nodename, "interrupts", irq_pin);
+    g_free(nodename);
+
+}
+static void openrisc_virt_virtio_init(OR1KVirtState *state, hwaddr base,
+                                      hwaddr size, int num_cpus,
+                                      OpenRISCCPU *cpus[], int irq_pin,
+                                      int virtio_idx)
+{
+    void *fdt = state->fdt;
+    char *nodename;
+    DeviceState *dev;
+    SysBusDevice *sysbus;
+    qemu_irq virtio_irq = get_per_cpu_irq(cpus, num_cpus, irq_pin + virtio_idx);
+
+    /* VirtIO MMIO devices */
+    dev = qdev_new("virtio-mmio");
+    qdev_prop_set_bit(dev, "force-legacy", false);
+    sysbus = SYS_BUS_DEVICE(dev);
+    sysbus_realize_and_unref(sysbus, &error_fatal);
+    sysbus_connect_irq(sysbus, 0, virtio_irq);
+    sysbus_mmio_map(sysbus, 0, base + virtio_idx * size);
+
+    /* VirtIO MMIO devices FDT */
+    nodename = g_strdup_printf("/soc/virtio_mmio@%" HWADDR_PRIx,
+                               base + virtio_idx * size);
+    qemu_fdt_add_subnode(fdt, nodename);
+    qemu_fdt_setprop_string(fdt, nodename, "compatible", "virtio,mmio");
+    qemu_fdt_setprop_cells(fdt, nodename, "reg",
+                           base + virtio_idx * size,
+                           size);
+    qemu_fdt_setprop_cell(fdt, nodename, "interrupts", irq_pin + virtio_idx);
+    g_free(nodename);
+}
+
+static void openrisc_virt_init(MachineState *machine)
+{
+    ram_addr_t ram_size = machine->ram_size;
+    const char *kernel_filename = machine->kernel_filename;
+    OpenRISCCPU *cpus[VIRT_CPUS_MAX] = {};
+    OR1KVirtState *state = VIRT_MACHINE(machine);
+    MemoryRegion *ram;
+    hwaddr load_addr;
+    int n;
+    unsigned int smp_cpus = machine->smp.cpus;
+
+    assert(smp_cpus >= 1 && smp_cpus <= VIRT_CPUS_MAX);
+    for (n = 0; n < smp_cpus; n++) {
+        cpus[n] = OPENRISC_CPU(cpu_create(machine->cpu_type));
+        if (cpus[n] == NULL) {
+            fprintf(stderr, "Unable to find CPU definition!\n");
+            exit(1);
+        }
+
+        cpu_openrisc_clock_init(cpus[n]);
+
+        qemu_register_reset(main_cpu_reset, cpus[n]);
+    }
+
+    ram = g_malloc(sizeof(*ram));
+    memory_region_init_ram(ram, NULL, "openrisc.ram", ram_size, &error_fatal);
+    memory_region_add_subregion(get_system_memory(), 0, ram);
+
+    openrisc_create_fdt(state, virt_memmap, smp_cpus, machine->ram_size,
+                        machine->kernel_cmdline);
+
+    if (smp_cpus > 1) {
+        openrisc_virt_ompic_init(state, virt_memmap[VIRT_OMPIC].base,
+                                 virt_memmap[VIRT_OMPIC].size,
+                                 smp_cpus, cpus, VIRT_OMPIC_IRQ);
+    }
+
+    openrisc_virt_serial_init(state, virt_memmap[VIRT_UART].base,
+                              virt_memmap[VIRT_UART].size,
+                              smp_cpus, cpus, VIRT_UART_IRQ);
+
+    openrisc_virt_test_init(state, virt_memmap[VIRT_TEST].base,
+                            virt_memmap[VIRT_TEST].size);
+
+    openrisc_virt_rtc_init(state, virt_memmap[VIRT_RTC].base,
+                           virt_memmap[VIRT_RTC].size, smp_cpus, cpus,
+                           VIRT_RTC_IRQ);
+
+    for (n = 0; n < VIRTIO_COUNT; n++) {
+        openrisc_virt_virtio_init(state, virt_memmap[VIRT_VIRTIO].base,
+                                  virt_memmap[VIRT_VIRTIO].size,
+                                  smp_cpus, cpus, VIRT_VIRTIO_IRQ, n);
+    }
+
+    load_addr = openrisc_load_kernel(ram_size, kernel_filename,
+                                     &boot_info.bootstrap_pc);
+    if (load_addr > 0) {
+        if (machine->initrd_filename) {
+            load_addr = openrisc_load_initrd(state->fdt,
+                                             machine->initrd_filename,
+                                             load_addr, machine->ram_size);
+        }
+        boot_info.fdt_addr = openrisc_load_fdt(state->fdt, load_addr,
+                                               machine->ram_size);
+    }
+}
+
+static void openrisc_virt_machine_init(ObjectClass *oc, void *data)
+{
+    MachineClass *mc = MACHINE_CLASS(oc);
+
+    mc->desc = "or1k virtual machine";
+    mc->init = openrisc_virt_init;
+    mc->max_cpus = VIRT_CPUS_MAX;
+    mc->is_default = false;
+    mc->default_cpu_type = OPENRISC_CPU_TYPE_NAME("or1200");
+}
+
+static const TypeInfo or1ksim_machine_typeinfo = {
+    .name       = TYPE_VIRT_MACHINE,
+    .parent     = TYPE_MACHINE,
+    .class_init = openrisc_virt_machine_init,
+    .instance_size = sizeof(OR1KVirtState),
+};
+
+static void or1ksim_machine_init_register_types(void)
+{
+    type_register_static(&or1ksim_machine_typeinfo);
+}
+
+type_init(or1ksim_machine_init_register_types)
-- 
2.31.1

Re: [RFC PATCH 3/3] hw/openrisc: Add the OpenRISC virtual machine

[Joel Stanley]

Hi Stafford,

On Fri, 27 May 2022 at 17:27, Stafford Horne <shorne@gmail.com> wrote:
>
> This patch add the OpenRISC virtual machine 'virt' for OpenRISC.  This
> platform allows for a convenient CI platform for toolchain, software
> ports and the OpenRISC linux kernel port.
>
> Much of this has been sourced from the m68k and riscv virt platforms.

It's a good idea! I did some playing around with your patch today.

I'd suggest adding something to docs/system/target-openrsic.rst,
including an example command lines.

>
> The platform provides:
>  - OpenRISC SMP with up to 8 cpus

You have this:

#define VIRT_CPUS_MAX 4

I tried booting with -smp 4 and it locked up when starting userspace
(or I stopped getting serial output?):

[    0.060000] smp: Brought up 1 node, 4 CPUs
...
[    0.960000] Run /init as init process

Running with -smp 2 and 3 worked. It does make booting much much slower.

>  - A virtio bus with up to 8 devices

How do we go about adding a virtio-net-device to this bus?

I tried this:

$ ./qemu-system-or1k -M virt  -nographic -kernel
~/dev/kernels/shenki/or1ksim/vmlinux -initrd
~/dev/buildroot/openrisc/images/rootfs.cpio -device
virtio-net-device,netdev=net0 -netdev user,id=net0

I thought it wasn't working, but I just needed to enable the drivers,
and from there it worked:

CONFIG_VIRTIO_NET=y
CONFIG_VIRTIO_MMIO=y

I also tested the virtio rng device which appeared to work.

# CONFIG_HW_RANDOM is not set
CONFIG_HW_RANDOM=y
CONFIG_HW_RANDOM_VIRTIO=y

>  - Standard ns16550a serial
>  - Goldfish RTC

-device virtio-rng-device,rng=rng0 -object rng-builtin,id=rng0

I enabled the options:

CONFIG_RTC_CLASS=y
# CONFIG_RTC_SYSTOHC is not set
# CONFIG_RTC_NVMEM is not set
CONFIG_RTC_DRV_GOLDFISH=y

But it didn't work. It seems the goldfish rtc model doesn't handle a
big endian guest running on my little endian host.

Doing this fixes it:

-    .endianness = DEVICE_NATIVE_ENDIAN,
+    .endianness = DEVICE_HOST_ENDIAN,

[    0.190000] goldfish_rtc 96005000.rtc: registered as rtc0
[    0.190000] goldfish_rtc 96005000.rtc: setting system clock to
2022-06-02T11:16:04 UTC (1654168564)

But literally no other model in the tree does this, so I suspect it's
not the right fix.

>  - SiFive TEST device for poweroff and reboot

CONFIG_POWER_RESET=y
CONFIG_POWER_RESET_SYSCON=y
CONFIG_POWER_RESET_SYSCON_POWEROFF=y
CONFIG_SYSCON_REBOOT_MODE=y

Adding the syscon/mfd cruft to the kernel adds about 43KB just for
rebooting. I guess that's okay as we're only dealing with a virtual
platform.

>  - Generated RTC to automatically configure the guest kernel

Did you mean device tree?

>
> Signed-off-by: Stafford Horne <shorne@gmail.com>
> ---
>  configs/devices/or1k-softmmu/default.mak |   1 +
>  hw/openrisc/Kconfig                      |   9 +
>  hw/openrisc/meson.build                  |   1 +
>  hw/openrisc/virt.c                       | 429 +++++++++++++++++++++++
>  4 files changed, 440 insertions(+)
>  create mode 100644 hw/openrisc/virt.c
>
> diff --git a/configs/devices/or1k-softmmu/default.mak b/configs/devices/or1k-softmmu/default.mak
> index 5b3ac89491..f3bf816067 100644
> --- a/configs/devices/or1k-softmmu/default.mak
> +++ b/configs/devices/or1k-softmmu/default.mak
> @@ -5,3 +5,4 @@ CONFIG_SEMIHOSTING=y
>  # Boards:
>  #
>  CONFIG_OR1K_SIM=y
> +CONFIG_OR1K_VIRT=y
> diff --git a/hw/openrisc/Kconfig b/hw/openrisc/Kconfig
> index 8f284f3ba0..202134668e 100644
> --- a/hw/openrisc/Kconfig
> +++ b/hw/openrisc/Kconfig
> @@ -4,3 +4,12 @@ config OR1K_SIM
>      select OPENCORES_ETH
>      select OMPIC
>      select SPLIT_IRQ
> +
> +config OR1K_VIRT
> +    bool
> +    imply VIRTIO_VGA
> +    imply TEST_DEVICES
> +    select GOLDFISH_RTC
> +    select SERIAL
> +    select SIFIVE_TEST
> +    select VIRTIO_MMIO

You could include the liteeth device too if we merged that.

> diff --git a/hw/openrisc/virt.c b/hw/openrisc/virt.c
> new file mode 100644
> index 0000000000..147196fda3
> --- /dev/null
> +++ b/hw/openrisc/virt.c
> @@ -0,0 +1,429 @@
> +/*
> + * OpenRISC QEMU virtual machine.
> + *
> + * Copyright (c) 2022 Stafford Horne <shorne@gmail.com>
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, see <http://www.gnu.org/licenses/>.

I think you can use the SPDX tag here instead of writing out the text.

> +static void openrisc_virt_init(MachineState *machine)
> +{
> +    ram_addr_t ram_size = machine->ram_size;
> +    const char *kernel_filename = machine->kernel_filename;
> +    OpenRISCCPU *cpus[VIRT_CPUS_MAX] = {};
> +    OR1KVirtState *state = VIRT_MACHINE(machine);
> +    MemoryRegion *ram;
> +    hwaddr load_addr;
> +    int n;
> +    unsigned int smp_cpus = machine->smp.cpus;
> +

> +    openrisc_virt_rtc_init(state, virt_memmap[VIRT_RTC].base,
> +                           virt_memmap[VIRT_RTC].size, smp_cpus, cpus,
> +                           VIRT_RTC_IRQ);
> +
> +    for (n = 0; n < VIRTIO_COUNT; n++) {

This would make more sense to me if you constructed the IRQ and base
here, and then passed the actual base and irq number to your
_virtio_init:

        size_t size = virt_memmap[VIRT_VIRTIO].size;
        openrisc_virt_virtio_init(state, virt_memmap[VIRT_VIRTIO].base
+ size * n,
                                  size, smp_cpus, cpus, VIRT_VIRTIO_IRQ + n);


> +        openrisc_virt_virtio_init(state, virt_memmap[VIRT_VIRTIO].base,
> +                                  virt_memmap[VIRT_VIRTIO].size,
> +                                  smp_cpus, cpus, VIRT_VIRTIO_IRQ, n);
> +    }
> +

Re: [RFC PATCH 3/3] hw/openrisc: Add the OpenRISC virtual machine

[Joel Stanley]

Hi Stafford,

On Fri, 27 May 2022 at 17:27, Stafford Horne <shorne@gmail.com> wrote:
>
> This patch add the OpenRISC virtual machine 'virt' for OpenRISC.  This
> platform allows for a convenient CI platform for toolchain, software
> ports and the OpenRISC linux kernel port.
>
> Much of this has been sourced from the m68k and riscv virt platforms.

It's a good idea! I did some playing around with your patch today.

I'd suggest adding something to docs/system/target-openrsic.rst,
including an example command lines.

>
> The platform provides:
>  - OpenRISC SMP with up to 8 cpus

You have this:

#define VIRT_CPUS_MAX 4

I tried booting with -smp 4 and it locked up when starting userspace
(or I stopped getting serial output?):

[    0.060000] smp: Brought up 1 node, 4 CPUs
...
[    0.960000] Run /init as init process

Running with -smp 2 and 3 worked. It does make booting much much slower.

>  - A virtio bus with up to 8 devices

How do we go about adding a virtio-net-device to this bus?

I tried this:

$ ./qemu-system-or1k -M virt  -nographic -kernel
~/dev/kernels/shenki/or1ksim/vmlinux -initrd
~/dev/buildroot/openrisc/images/rootfs.cpio -device
virtio-net-device,netdev=net0 -netdev user,id=net0

I thought it wasn't working, but I just needed to enable the drivers,
and from there it worked:

CONFIG_VIRTIO_NET=y
CONFIG_VIRTIO_MMIO=y

I also tested the virtio rng device which appeared to work.

# CONFIG_HW_RANDOM is not set
CONFIG_HW_RANDOM=y
CONFIG_HW_RANDOM_VIRTIO=y

>  - Standard ns16550a serial
>  - Goldfish RTC

-device virtio-rng-device,rng=rng0 -object rng-builtin,id=rng0

I enabled the options:

CONFIG_RTC_CLASS=y
# CONFIG_RTC_SYSTOHC is not set
# CONFIG_RTC_NVMEM is not set
CONFIG_RTC_DRV_GOLDFISH=y

But it didn't work. It seems the goldfish rtc model doesn't handle a
big endian guest running on my little endian host.

Doing this fixes it:

-    .endianness = DEVICE_NATIVE_ENDIAN,
+    .endianness = DEVICE_HOST_ENDIAN,

[    0.190000] goldfish_rtc 96005000.rtc: registered as rtc0
[    0.190000] goldfish_rtc 96005000.rtc: setting system clock to
2022-06-02T11:16:04 UTC (1654168564)

But literally no other model in the tree does this, so I suspect it's
not the right fix.

>  - SiFive TEST device for poweroff and reboot

CONFIG_POWER_RESET=y
CONFIG_POWER_RESET_SYSCON=y
CONFIG_POWER_RESET_SYSCON_POWEROFF=y
CONFIG_SYSCON_REBOOT_MODE=y

Adding the syscon/mfd cruft to the kernel adds about 43KB just for
rebooting. I guess that's okay as we're only dealing with a virtual
platform.

>  - Generated RTC to automatically configure the guest kernel

Did you mean device tree?

>
> Signed-off-by: Stafford Horne <shorne@gmail.com>
> ---
>  configs/devices/or1k-softmmu/default.mak |   1 +
>  hw/openrisc/Kconfig                      |   9 +
>  hw/openrisc/meson.build                  |   1 +
>  hw/openrisc/virt.c                       | 429 +++++++++++++++++++++++
>  4 files changed, 440 insertions(+)
>  create mode 100644 hw/openrisc/virt.c
>
> diff --git a/configs/devices/or1k-softmmu/default.mak b/configs/devices/or1k-softmmu/default.mak
> index 5b3ac89491..f3bf816067 100644
> --- a/configs/devices/or1k-softmmu/default.mak
> +++ b/configs/devices/or1k-softmmu/default.mak
> @@ -5,3 +5,4 @@ CONFIG_SEMIHOSTING=y
>  # Boards:
>  #
>  CONFIG_OR1K_SIM=y
> +CONFIG_OR1K_VIRT=y
> diff --git a/hw/openrisc/Kconfig b/hw/openrisc/Kconfig
> index 8f284f3ba0..202134668e 100644
> --- a/hw/openrisc/Kconfig
> +++ b/hw/openrisc/Kconfig
> @@ -4,3 +4,12 @@ config OR1K_SIM
>      select OPENCORES_ETH
>      select OMPIC
>      select SPLIT_IRQ
> +
> +config OR1K_VIRT
> +    bool
> +    imply VIRTIO_VGA
> +    imply TEST_DEVICES
> +    select GOLDFISH_RTC
> +    select SERIAL
> +    select SIFIVE_TEST
> +    select VIRTIO_MMIO

You could include the liteeth device too if we merged that.

> diff --git a/hw/openrisc/virt.c b/hw/openrisc/virt.c
> new file mode 100644
> index 0000000000..147196fda3
> --- /dev/null
> +++ b/hw/openrisc/virt.c
> @@ -0,0 +1,429 @@
> +/*
> + * OpenRISC QEMU virtual machine.
> + *
> + * Copyright (c) 2022 Stafford Horne <shorne@gmail.com>
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, see <http://www.gnu.org/licenses/>.

I think you can use the SPDX tag here instead of writing out the text.

> +static void openrisc_virt_init(MachineState *machine)
> +{
> +    ram_addr_t ram_size = machine->ram_size;
> +    const char *kernel_filename = machine->kernel_filename;
> +    OpenRISCCPU *cpus[VIRT_CPUS_MAX] = {};
> +    OR1KVirtState *state = VIRT_MACHINE(machine);
> +    MemoryRegion *ram;
> +    hwaddr load_addr;
> +    int n;
> +    unsigned int smp_cpus = machine->smp.cpus;
> +

> +    openrisc_virt_rtc_init(state, virt_memmap[VIRT_RTC].base,
> +                           virt_memmap[VIRT_RTC].size, smp_cpus, cpus,
> +                           VIRT_RTC_IRQ);
> +
> +    for (n = 0; n < VIRTIO_COUNT; n++) {

This would make more sense to me if you constructed the IRQ and base
here, and then passed the actual base and irq number to your
_virtio_init:

        size_t size = virt_memmap[VIRT_VIRTIO].size;
        openrisc_virt_virtio_init(state, virt_memmap[VIRT_VIRTIO].base
+ size * n,
                                  size, smp_cpus, cpus, VIRT_VIRTIO_IRQ + n);


> +        openrisc_virt_virtio_init(state, virt_memmap[VIRT_VIRTIO].base,
> +                                  virt_memmap[VIRT_VIRTIO].size,
> +                                  smp_cpus, cpus, VIRT_VIRTIO_IRQ, n);
> +    }
> +

Re: [RFC PATCH 3/3] hw/openrisc: Add the OpenRISC virtual machine

[Richard Henderson]

On 6/2/22 04:42, Joel Stanley wrote:
> Hi Stafford,
> 
> On Fri, 27 May 2022 at 17:27, Stafford Horne <shorne@gmail.com> wrote:
>>
>> This patch add the OpenRISC virtual machine 'virt' for OpenRISC.  This
>> platform allows for a convenient CI platform for toolchain, software
>> ports and the OpenRISC linux kernel port.
>>
>> Much of this has been sourced from the m68k and riscv virt platforms.
> 
> It's a good idea! I did some playing around with your patch today.
> 
> I'd suggest adding something to docs/system/target-openrsic.rst,
> including an example command lines.
> 
>>
>> The platform provides:
>>   - OpenRISC SMP with up to 8 cpus
> 
> You have this:
> 
> #define VIRT_CPUS_MAX 4
> 
> I tried booting with -smp 4 and it locked up when starting userspace
> (or I stopped getting serial output?):
> 
> [    0.060000] smp: Brought up 1 node, 4 CPUs
> ...
> [    0.960000] Run /init as init process
> 
> Running with -smp 2 and 3 worked. It does make booting much much slower.

target/openrisc/cpu.h is missing

#define TCG_GUEST_DEFAULT_MO      (0)


to tell the JIT about the weakly ordered guest memory model, and to enable MTTCG by default.

> I enabled the options:
> 
> CONFIG_RTC_CLASS=y
> # CONFIG_RTC_SYSTOHC is not set
> # CONFIG_RTC_NVMEM is not set
> CONFIG_RTC_DRV_GOLDFISH=y
> 
> But it didn't work. It seems the goldfish rtc model doesn't handle a
> big endian guest running on my little endian host.
> 
> Doing this fixes it:
> 
> -    .endianness = DEVICE_NATIVE_ENDIAN,
> +    .endianness = DEVICE_HOST_ENDIAN,
> 
> [    0.190000] goldfish_rtc 96005000.rtc: registered as rtc0
> [    0.190000] goldfish_rtc 96005000.rtc: setting system clock to
> 2022-06-02T11:16:04 UTC (1654168564)
> 
> But literally no other model in the tree does this, so I suspect it's
> not the right fix.

Correct.  The model might require

     .endianness = DEVICE_LITTLE_ENDIAN,

if that is the actual specification, or it may simply require fixes to handle a big-endian 
guest.

All that said, if we're going to make up a new virt platform, it should use PCI not 
virtio.  See the recent discussion about RISC-V virtual machines, where they made exactly 
this mistake several years ago.


r~

Re: [RFC PATCH 3/3] hw/openrisc: Add the OpenRISC virtual machine

[Richard Henderson]

On 6/2/22 04:42, Joel Stanley wrote:
> Hi Stafford,
> 
> On Fri, 27 May 2022 at 17:27, Stafford Horne <shorne@gmail.com> wrote:
>>
>> This patch add the OpenRISC virtual machine 'virt' for OpenRISC.  This
>> platform allows for a convenient CI platform for toolchain, software
>> ports and the OpenRISC linux kernel port.
>>
>> Much of this has been sourced from the m68k and riscv virt platforms.
> 
> It's a good idea! I did some playing around with your patch today.
> 
> I'd suggest adding something to docs/system/target-openrsic.rst,
> including an example command lines.
> 
>>
>> The platform provides:
>>   - OpenRISC SMP with up to 8 cpus
> 
> You have this:
> 
> #define VIRT_CPUS_MAX 4
> 
> I tried booting with -smp 4 and it locked up when starting userspace
> (or I stopped getting serial output?):
> 
> [    0.060000] smp: Brought up 1 node, 4 CPUs
> ...
> [    0.960000] Run /init as init process
> 
> Running with -smp 2 and 3 worked. It does make booting much much slower.

target/openrisc/cpu.h is missing

#define TCG_GUEST_DEFAULT_MO      (0)


to tell the JIT about the weakly ordered guest memory model, and to enable MTTCG by default.

> I enabled the options:
> 
> CONFIG_RTC_CLASS=y
> # CONFIG_RTC_SYSTOHC is not set
> # CONFIG_RTC_NVMEM is not set
> CONFIG_RTC_DRV_GOLDFISH=y
> 
> But it didn't work. It seems the goldfish rtc model doesn't handle a
> big endian guest running on my little endian host.
> 
> Doing this fixes it:
> 
> -    .endianness = DEVICE_NATIVE_ENDIAN,
> +    .endianness = DEVICE_HOST_ENDIAN,
> 
> [    0.190000] goldfish_rtc 96005000.rtc: registered as rtc0
> [    0.190000] goldfish_rtc 96005000.rtc: setting system clock to
> 2022-06-02T11:16:04 UTC (1654168564)
> 
> But literally no other model in the tree does this, so I suspect it's
> not the right fix.

Correct.  The model might require

     .endianness = DEVICE_LITTLE_ENDIAN,

if that is the actual specification, or it may simply require fixes to handle a big-endian 
guest.

All that said, if we're going to make up a new virt platform, it should use PCI not 
virtio.  See the recent discussion about RISC-V virtual machines, where they made exactly 
this mistake several years ago.


r~

Re: [RFC PATCH 3/3] hw/openrisc: Add the OpenRISC virtual machine

[Geert Uytterhoeven]

Hi Joel,

On Thu, Jun 2, 2022 at 1:42 PM Joel Stanley <joel@jms.id.au> wrote:
> On Fri, 27 May 2022 at 17:27, Stafford Horne <shorne@gmail.com> wrote:
> > This patch add the OpenRISC virtual machine 'virt' for OpenRISC.  This
> > platform allows for a convenient CI platform for toolchain, software
> > ports and the OpenRISC linux kernel port.
> >
> > Much of this has been sourced from the m68k and riscv virt platforms.

> I enabled the options:
>
> CONFIG_RTC_CLASS=y
> # CONFIG_RTC_SYSTOHC is not set
> # CONFIG_RTC_NVMEM is not set
> CONFIG_RTC_DRV_GOLDFISH=y
>
> But it didn't work. It seems the goldfish rtc model doesn't handle a
> big endian guest running on my little endian host.
>
> Doing this fixes it:
>
> -    .endianness = DEVICE_NATIVE_ENDIAN,
> +    .endianness = DEVICE_HOST_ENDIAN,
>
> [    0.190000] goldfish_rtc 96005000.rtc: registered as rtc0
> [    0.190000] goldfish_rtc 96005000.rtc: setting system clock to
> 2022-06-02T11:16:04 UTC (1654168564)
>
> But literally no other model in the tree does this, so I suspect it's
> not the right fix.

Goldfish devices are supposed to be little endian.
Unfortunately m68k got this wrong, cfr.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=2e2ac4a3327479f7e2744cdd88a5c823f2057bad
Please don't duplicate this bad behavior for new architectures.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [RFC PATCH 3/3] hw/openrisc: Add the OpenRISC virtual machine

[Geert Uytterhoeven]

Hi Joel,

On Thu, Jun 2, 2022 at 1:42 PM Joel Stanley <joel@jms.id.au> wrote:
> On Fri, 27 May 2022 at 17:27, Stafford Horne <shorne@gmail.com> wrote:
> > This patch add the OpenRISC virtual machine 'virt' for OpenRISC.  This
> > platform allows for a convenient CI platform for toolchain, software
> > ports and the OpenRISC linux kernel port.
> >
> > Much of this has been sourced from the m68k and riscv virt platforms.

> I enabled the options:
>
> CONFIG_RTC_CLASS=y
> # CONFIG_RTC_SYSTOHC is not set
> # CONFIG_RTC_NVMEM is not set
> CONFIG_RTC_DRV_GOLDFISH=y
>
> But it didn't work. It seems the goldfish rtc model doesn't handle a
> big endian guest running on my little endian host.
>
> Doing this fixes it:
>
> -    .endianness = DEVICE_NATIVE_ENDIAN,
> +    .endianness = DEVICE_HOST_ENDIAN,
>
> [    0.190000] goldfish_rtc 96005000.rtc: registered as rtc0
> [    0.190000] goldfish_rtc 96005000.rtc: setting system clock to
> 2022-06-02T11:16:04 UTC (1654168564)
>
> But literally no other model in the tree does this, so I suspect it's
> not the right fix.

Goldfish devices are supposed to be little endian.
Unfortunately m68k got this wrong, cfr.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=2e2ac4a3327479f7e2744cdd88a5c823f2057bad
Please don't duplicate this bad behavior for new architectures.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [RFC PATCH 3/3] hw/openrisc: Add the OpenRISC virtual machine

[Stafford Horne]

On Thu, Jun 02, 2022 at 09:08:52PM +0200, Geert Uytterhoeven wrote:
> Hi Joel,
> 
> On Thu, Jun 2, 2022 at 1:42 PM Joel Stanley <joel@jms.id.au> wrote:
> > On Fri, 27 May 2022 at 17:27, Stafford Horne <shorne@gmail.com> wrote:
> > > This patch add the OpenRISC virtual machine 'virt' for OpenRISC.  This
> > > platform allows for a convenient CI platform for toolchain, software
> > > ports and the OpenRISC linux kernel port.
> > >
> > > Much of this has been sourced from the m68k and riscv virt platforms.
> 
> > I enabled the options:
> >
> > CONFIG_RTC_CLASS=y
> > # CONFIG_RTC_SYSTOHC is not set
> > # CONFIG_RTC_NVMEM is not set
> > CONFIG_RTC_DRV_GOLDFISH=y
> >
> > But it didn't work. It seems the goldfish rtc model doesn't handle a
> > big endian guest running on my little endian host.
> >
> > Doing this fixes it:
> >
> > -    .endianness = DEVICE_NATIVE_ENDIAN,
> > +    .endianness = DEVICE_HOST_ENDIAN,
> >
> > [    0.190000] goldfish_rtc 96005000.rtc: registered as rtc0
> > [    0.190000] goldfish_rtc 96005000.rtc: setting system clock to
> > 2022-06-02T11:16:04 UTC (1654168564)
> >
> > But literally no other model in the tree does this, so I suspect it's
> > not the right fix.
> 
> Goldfish devices are supposed to be little endian.
> Unfortunately m68k got this wrong, cfr.
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=2e2ac4a3327479f7e2744cdd88a5c823f2057bad
> Please don't duplicate this bad behavior for new architectures

Thanks for the pointer, I just wired in the goldfish RTC because I wanted to
play with it.  I was not attached to it. I can either remove it our find another
RTC.

-Stafford

Re: [RFC PATCH 3/3] hw/openrisc: Add the OpenRISC virtual machine

[Stafford Horne]

On Thu, Jun 02, 2022 at 09:08:52PM +0200, Geert Uytterhoeven wrote:
> Hi Joel,
> 
> On Thu, Jun 2, 2022 at 1:42 PM Joel Stanley <joel@jms.id.au> wrote:
> > On Fri, 27 May 2022 at 17:27, Stafford Horne <shorne@gmail.com> wrote:
> > > This patch add the OpenRISC virtual machine 'virt' for OpenRISC.  This
> > > platform allows for a convenient CI platform for toolchain, software
> > > ports and the OpenRISC linux kernel port.
> > >
> > > Much of this has been sourced from the m68k and riscv virt platforms.
> 
> > I enabled the options:
> >
> > CONFIG_RTC_CLASS=y
> > # CONFIG_RTC_SYSTOHC is not set
> > # CONFIG_RTC_NVMEM is not set
> > CONFIG_RTC_DRV_GOLDFISH=y
> >
> > But it didn't work. It seems the goldfish rtc model doesn't handle a
> > big endian guest running on my little endian host.
> >
> > Doing this fixes it:
> >
> > -    .endianness = DEVICE_NATIVE_ENDIAN,
> > +    .endianness = DEVICE_HOST_ENDIAN,
> >
> > [    0.190000] goldfish_rtc 96005000.rtc: registered as rtc0
> > [    0.190000] goldfish_rtc 96005000.rtc: setting system clock to
> > 2022-06-02T11:16:04 UTC (1654168564)
> >
> > But literally no other model in the tree does this, so I suspect it's
> > not the right fix.
> 
> Goldfish devices are supposed to be little endian.
> Unfortunately m68k got this wrong, cfr.
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=2e2ac4a3327479f7e2744cdd88a5c823f2057bad
> Please don't duplicate this bad behavior for new architectures

Thanks for the pointer, I just wired in the goldfish RTC because I wanted to
play with it.  I was not attached to it. I can either remove it our find another
RTC.

-Stafford

Re: [RFC PATCH 3/3] hw/openrisc: Add the OpenRISC virtual machine

[Geert Uytterhoeven]

Hi Stafford,

On Thu, Jun 2, 2022 at 9:59 PM Stafford Horne <shorne@gmail.com> wrote:
> On Thu, Jun 02, 2022 at 09:08:52PM +0200, Geert Uytterhoeven wrote:
> > On Thu, Jun 2, 2022 at 1:42 PM Joel Stanley <joel@jms.id.au> wrote:
> > > On Fri, 27 May 2022 at 17:27, Stafford Horne <shorne@gmail.com> wrote:
> > > > This patch add the OpenRISC virtual machine 'virt' for OpenRISC.  This
> > > > platform allows for a convenient CI platform for toolchain, software
> > > > ports and the OpenRISC linux kernel port.
> > > >
> > > > Much of this has been sourced from the m68k and riscv virt platforms.
> >
> > > I enabled the options:
> > >
> > > CONFIG_RTC_CLASS=y
> > > # CONFIG_RTC_SYSTOHC is not set
> > > # CONFIG_RTC_NVMEM is not set
> > > CONFIG_RTC_DRV_GOLDFISH=y
> > >
> > > But it didn't work. It seems the goldfish rtc model doesn't handle a
> > > big endian guest running on my little endian host.
> > >
> > > Doing this fixes it:
> > >
> > > -    .endianness = DEVICE_NATIVE_ENDIAN,
> > > +    .endianness = DEVICE_HOST_ENDIAN,
> > >
> > > [    0.190000] goldfish_rtc 96005000.rtc: registered as rtc0
> > > [    0.190000] goldfish_rtc 96005000.rtc: setting system clock to
> > > 2022-06-02T11:16:04 UTC (1654168564)
> > >
> > > But literally no other model in the tree does this, so I suspect it's
> > > not the right fix.
> >
> > Goldfish devices are supposed to be little endian.
> > Unfortunately m68k got this wrong, cfr.
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=2e2ac4a3327479f7e2744cdd88a5c823f2057bad
> > Please don't duplicate this bad behavior for new architectures
>
> Thanks for the pointer, I just wired in the goldfish RTC because I wanted to
> play with it.  I was not attached to it. I can either remove it our find another
> RTC.

Sorry for being too unclear: the mistake was not to use the Goldfish
RTC, but to make its register accesses big-endian.
Using Goldfish devices as little-endian devices should be fine.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [RFC PATCH 3/3] hw/openrisc: Add the OpenRISC virtual machine

[Geert Uytterhoeven]

Hi Stafford,

On Thu, Jun 2, 2022 at 9:59 PM Stafford Horne <shorne@gmail.com> wrote:
> On Thu, Jun 02, 2022 at 09:08:52PM +0200, Geert Uytterhoeven wrote:
> > On Thu, Jun 2, 2022 at 1:42 PM Joel Stanley <joel@jms.id.au> wrote:
> > > On Fri, 27 May 2022 at 17:27, Stafford Horne <shorne@gmail.com> wrote:
> > > > This patch add the OpenRISC virtual machine 'virt' for OpenRISC.  This
> > > > platform allows for a convenient CI platform for toolchain, software
> > > > ports and the OpenRISC linux kernel port.
> > > >
> > > > Much of this has been sourced from the m68k and riscv virt platforms.
> >
> > > I enabled the options:
> > >
> > > CONFIG_RTC_CLASS=y
> > > # CONFIG_RTC_SYSTOHC is not set
> > > # CONFIG_RTC_NVMEM is not set
> > > CONFIG_RTC_DRV_GOLDFISH=y
> > >
> > > But it didn't work. It seems the goldfish rtc model doesn't handle a
> > > big endian guest running on my little endian host.
> > >
> > > Doing this fixes it:
> > >
> > > -    .endianness = DEVICE_NATIVE_ENDIAN,
> > > +    .endianness = DEVICE_HOST_ENDIAN,
> > >
> > > [    0.190000] goldfish_rtc 96005000.rtc: registered as rtc0
> > > [    0.190000] goldfish_rtc 96005000.rtc: setting system clock to
> > > 2022-06-02T11:16:04 UTC (1654168564)
> > >
> > > But literally no other model in the tree does this, so I suspect it's
> > > not the right fix.
> >
> > Goldfish devices are supposed to be little endian.
> > Unfortunately m68k got this wrong, cfr.
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=2e2ac4a3327479f7e2744cdd88a5c823f2057bad
> > Please don't duplicate this bad behavior for new architectures
>
> Thanks for the pointer, I just wired in the goldfish RTC because I wanted to
> play with it.  I was not attached to it. I can either remove it our find another
> RTC.

Sorry for being too unclear: the mistake was not to use the Goldfish
RTC, but to make its register accesses big-endian.
Using Goldfish devices as little-endian devices should be fine.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [RFC PATCH 3/3] hw/openrisc: Add the OpenRISC virtual machine

[Stafford Horne]

On Fri, Jun 03, 2022 at 09:05:09AM +0200, Geert Uytterhoeven wrote:
> Hi Stafford,
> 
> On Thu, Jun 2, 2022 at 9:59 PM Stafford Horne <shorne@gmail.com> wrote:
> > On Thu, Jun 02, 2022 at 09:08:52PM +0200, Geert Uytterhoeven wrote:
> > > On Thu, Jun 2, 2022 at 1:42 PM Joel Stanley <joel@jms.id.au> wrote:
> > > > On Fri, 27 May 2022 at 17:27, Stafford Horne <shorne@gmail.com> wrote:
> > > > > This patch add the OpenRISC virtual machine 'virt' for OpenRISC.  This
> > > > > platform allows for a convenient CI platform for toolchain, software
> > > > > ports and the OpenRISC linux kernel port.
> > > > >
> > > > > Much of this has been sourced from the m68k and riscv virt platforms.
> > >
> > > > I enabled the options:
> > > >
> > > > CONFIG_RTC_CLASS=y
> > > > # CONFIG_RTC_SYSTOHC is not set
> > > > # CONFIG_RTC_NVMEM is not set
> > > > CONFIG_RTC_DRV_GOLDFISH=y
> > > >
> > > > But it didn't work. It seems the goldfish rtc model doesn't handle a
> > > > big endian guest running on my little endian host.
> > > >
> > > > Doing this fixes it:
> > > >
> > > > -    .endianness = DEVICE_NATIVE_ENDIAN,
> > > > +    .endianness = DEVICE_HOST_ENDIAN,
> > > >
> > > > [    0.190000] goldfish_rtc 96005000.rtc: registered as rtc0
> > > > [    0.190000] goldfish_rtc 96005000.rtc: setting system clock to
> > > > 2022-06-02T11:16:04 UTC (1654168564)
> > > >
> > > > But literally no other model in the tree does this, so I suspect it's
> > > > not the right fix.
> > >
> > > Goldfish devices are supposed to be little endian.
> > > Unfortunately m68k got this wrong, cfr.
> > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=2e2ac4a3327479f7e2744cdd88a5c823f2057bad
> > > Please don't duplicate this bad behavior for new architectures
> >
> > Thanks for the pointer, I just wired in the goldfish RTC because I wanted to
> > play with it.  I was not attached to it. I can either remove it our find another
> > RTC.
> 
> Sorry for being too unclear: the mistake was not to use the Goldfish
> RTC, but to make its register accesses big-endian.
> Using Goldfish devices as little-endian devices should be fine.

OK, then I would think this patch would be needed on Goldfish.  I tested this
out and it seems to work:

Patch:

diff --git a/hw/rtc/goldfish_rtc.c b/hw/rtc/goldfish_rtc.c
index 35e493be31..f1dc5af297 100644
--- a/hw/rtc/goldfish_rtc.c
+++ b/hw/rtc/goldfish_rtc.c
@@ -219,7 +219,7 @@ static int goldfish_rtc_post_load(void *opaque, int
version_id)
 static const MemoryRegionOps goldfish_rtc_ops = {
     .read = goldfish_rtc_read,
     .write = goldfish_rtc_write,
-    .endianness = DEVICE_NATIVE_ENDIAN,
+    .endianness = DEVICE_LITTLE_ENDIAN,
     .valid = {
         .min_access_size = 4,
         .max_access_size = 4

Boot Log:

    io scheduler mq-deadline registered
    io scheduler kyber registered
    Serial: 8250/16550 driver, 4 ports, IRQ sharing disabled
    90000000.serial: ttyS0 at MMIO 0x90000000 (irq = 2, base_baud = 1250000) is a 16550A
    printk: console [ttyS0] enabled
    loop: module loaded
    virtio_blk virtio1: [vda] 32768 512-byte logical blocks (16.8 MB/16.0 MiB)
    Freeing initrd memory: 1696K
   *goldfish_rtc 96005000.rtc: registered as rtc0
   *goldfish_rtc 96005000.rtc: setting system clock to 2022-06-05T01:49:57 UTC (1654393797)
    NET: Registered PF_PACKET protocol family
    random: fast init done

-Stafford

Re: [RFC PATCH 3/3] hw/openrisc: Add the OpenRISC virtual machine

[Stafford Horne]

On Fri, Jun 03, 2022 at 09:05:09AM +0200, Geert Uytterhoeven wrote:
> Hi Stafford,
> 
> On Thu, Jun 2, 2022 at 9:59 PM Stafford Horne <shorne@gmail.com> wrote:
> > On Thu, Jun 02, 2022 at 09:08:52PM +0200, Geert Uytterhoeven wrote:
> > > On Thu, Jun 2, 2022 at 1:42 PM Joel Stanley <joel@jms.id.au> wrote:
> > > > On Fri, 27 May 2022 at 17:27, Stafford Horne <shorne@gmail.com> wrote:
> > > > > This patch add the OpenRISC virtual machine 'virt' for OpenRISC.  This
> > > > > platform allows for a convenient CI platform for toolchain, software
> > > > > ports and the OpenRISC linux kernel port.
> > > > >
> > > > > Much of this has been sourced from the m68k and riscv virt platforms.
> > >
> > > > I enabled the options:
> > > >
> > > > CONFIG_RTC_CLASS=y
> > > > # CONFIG_RTC_SYSTOHC is not set
> > > > # CONFIG_RTC_NVMEM is not set
> > > > CONFIG_RTC_DRV_GOLDFISH=y
> > > >
> > > > But it didn't work. It seems the goldfish rtc model doesn't handle a
> > > > big endian guest running on my little endian host.
> > > >
> > > > Doing this fixes it:
> > > >
> > > > -    .endianness = DEVICE_NATIVE_ENDIAN,
> > > > +    .endianness = DEVICE_HOST_ENDIAN,
> > > >
> > > > [    0.190000] goldfish_rtc 96005000.rtc: registered as rtc0
> > > > [    0.190000] goldfish_rtc 96005000.rtc: setting system clock to
> > > > 2022-06-02T11:16:04 UTC (1654168564)
> > > >
> > > > But literally no other model in the tree does this, so I suspect it's
> > > > not the right fix.
> > >
> > > Goldfish devices are supposed to be little endian.
> > > Unfortunately m68k got this wrong, cfr.
> > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=2e2ac4a3327479f7e2744cdd88a5c823f2057bad
> > > Please don't duplicate this bad behavior for new architectures
> >
> > Thanks for the pointer, I just wired in the goldfish RTC because I wanted to
> > play with it.  I was not attached to it. I can either remove it our find another
> > RTC.
> 
> Sorry for being too unclear: the mistake was not to use the Goldfish
> RTC, but to make its register accesses big-endian.
> Using Goldfish devices as little-endian devices should be fine.

OK, then I would think this patch would be needed on Goldfish.  I tested this
out and it seems to work:

Patch:

diff --git a/hw/rtc/goldfish_rtc.c b/hw/rtc/goldfish_rtc.c
index 35e493be31..f1dc5af297 100644
--- a/hw/rtc/goldfish_rtc.c
+++ b/hw/rtc/goldfish_rtc.c
@@ -219,7 +219,7 @@ static int goldfish_rtc_post_load(void *opaque, int
version_id)
 static const MemoryRegionOps goldfish_rtc_ops = {
     .read = goldfish_rtc_read,
     .write = goldfish_rtc_write,
-    .endianness = DEVICE_NATIVE_ENDIAN,
+    .endianness = DEVICE_LITTLE_ENDIAN,
     .valid = {
         .min_access_size = 4,
         .max_access_size = 4

Boot Log:

    io scheduler mq-deadline registered
    io scheduler kyber registered
    Serial: 8250/16550 driver, 4 ports, IRQ sharing disabled
    90000000.serial: ttyS0 at MMIO 0x90000000 (irq = 2, base_baud = 1250000) is a 16550A
    printk: console [ttyS0] enabled
    loop: module loaded
    virtio_blk virtio1: [vda] 32768 512-byte logical blocks (16.8 MB/16.0 MiB)
    Freeing initrd memory: 1696K
   *goldfish_rtc 96005000.rtc: registered as rtc0
   *goldfish_rtc 96005000.rtc: setting system clock to 2022-06-05T01:49:57 UTC (1654393797)
    NET: Registered PF_PACKET protocol family
    random: fast init done

-Stafford

Re: [RFC PATCH 3/3] hw/openrisc: Add the OpenRISC virtual machine

[Stafford Horne]

On Sun, Jun 05, 2022 at 10:58:14AM +0900, Stafford Horne wrote:
> On Fri, Jun 03, 2022 at 09:05:09AM +0200, Geert Uytterhoeven wrote:
> > Hi Stafford,
> > 
> > On Thu, Jun 2, 2022 at 9:59 PM Stafford Horne <shorne@gmail.com> wrote:
> > > On Thu, Jun 02, 2022 at 09:08:52PM +0200, Geert Uytterhoeven wrote:
> > > > On Thu, Jun 2, 2022 at 1:42 PM Joel Stanley <joel@jms.id.au> wrote:
> > > > > On Fri, 27 May 2022 at 17:27, Stafford Horne <shorne@gmail.com> wrote:
> > > > > > This patch add the OpenRISC virtual machine 'virt' for OpenRISC.  This
> > > > > > platform allows for a convenient CI platform for toolchain, software
> > > > > > ports and the OpenRISC linux kernel port.
> > > > > >
> > > > > > Much of this has been sourced from the m68k and riscv virt platforms.
> > > >
> > > > > I enabled the options:
> > > > >
> > > > > CONFIG_RTC_CLASS=y
> > > > > # CONFIG_RTC_SYSTOHC is not set
> > > > > # CONFIG_RTC_NVMEM is not set
> > > > > CONFIG_RTC_DRV_GOLDFISH=y
> > > > >
> > > > > But it didn't work. It seems the goldfish rtc model doesn't handle a
> > > > > big endian guest running on my little endian host.
> > > > >
> > > > > Doing this fixes it:
> > > > >
> > > > > -    .endianness = DEVICE_NATIVE_ENDIAN,
> > > > > +    .endianness = DEVICE_HOST_ENDIAN,
> > > > >
> > > > > [    0.190000] goldfish_rtc 96005000.rtc: registered as rtc0
> > > > > [    0.190000] goldfish_rtc 96005000.rtc: setting system clock to
> > > > > 2022-06-02T11:16:04 UTC (1654168564)
> > > > >
> > > > > But literally no other model in the tree does this, so I suspect it's
> > > > > not the right fix.
> > > >
> > > > Goldfish devices are supposed to be little endian.
> > > > Unfortunately m68k got this wrong, cfr.
> > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=2e2ac4a3327479f7e2744cdd88a5c823f2057bad
> > > > Please don't duplicate this bad behavior for new architectures
> > >
> > > Thanks for the pointer, I just wired in the goldfish RTC because I wanted to
> > > play with it.  I was not attached to it. I can either remove it our find another
> > > RTC.
> > 
> > Sorry for being too unclear: the mistake was not to use the Goldfish
> > RTC, but to make its register accesses big-endian.
> > Using Goldfish devices as little-endian devices should be fine.
> 
> OK, then I would think this patch would be needed on Goldfish.  I tested this
> out and it seems to work:

Sorry, it seems maybe I mis-understood this again.  In Arnd's mail [1] he, at
the end, mentions.

    It might be a good idea to revisit the qemu implementation and make
    sure that the extra byteswap is only inserted on m68k and not on
    other targets, but hopefully there are no new targets based on goldfish
    anymore and we don't need to care.

So, it seems that in addition to my patch we would need something in m68k to
switch it back to 'native' (big) endian?

Looking at the m68k kernel/qemu interface I see:

Pre 5.19:
   (data) <-- kernel(readl / little) <-- m68k qemu (native / big) - RTC/PIC
   (data) <-- kernel(__raw_readl / big) <-- m68k qemu (native / big) - TTY

5.19:
   (data) <-- kernel(gf_ioread32 / big) <-- m68k qemu (native / big) - all

The new fixes to add gf_ioread32/gf_iowrite32 fix this for goldfish and m68k.
This wouldn't have been an issue for little-endian platforms where readl/writel
were originally used.

Why can't m68k switch to little-endian in qemu and the kernel?  The m68k virt
platform is not that old, 1 year? Are there a lot of users that this would be a big
problem?

[1] https://lore.kernel.org/lkml/CAK8P3a1oN8NrUjkh2X8jHQbyz42Xo6GSa=5n0gD6vQcXRjmq1Q@mail.gmail.com/

-Stafford

> Patch:
> 
> diff --git a/hw/rtc/goldfish_rtc.c b/hw/rtc/goldfish_rtc.c
> index 35e493be31..f1dc5af297 100644
> --- a/hw/rtc/goldfish_rtc.c
> +++ b/hw/rtc/goldfish_rtc.c
> @@ -219,7 +219,7 @@ static int goldfish_rtc_post_load(void *opaque, int
> version_id)
>  static const MemoryRegionOps goldfish_rtc_ops = {
>      .read = goldfish_rtc_read,
>      .write = goldfish_rtc_write,
> -    .endianness = DEVICE_NATIVE_ENDIAN,
> +    .endianness = DEVICE_LITTLE_ENDIAN,
>      .valid = {
>          .min_access_size = 4,
>          .max_access_size = 4
> 
> Boot Log:
> 
>     io scheduler mq-deadline registered
>     io scheduler kyber registered
>     Serial: 8250/16550 driver, 4 ports, IRQ sharing disabled
>     90000000.serial: ttyS0 at MMIO 0x90000000 (irq = 2, base_baud = 1250000) is a 16550A
>     printk: console [ttyS0] enabled
>     loop: module loaded
>     virtio_blk virtio1: [vda] 32768 512-byte logical blocks (16.8 MB/16.0 MiB)
>     Freeing initrd memory: 1696K
>    *goldfish_rtc 96005000.rtc: registered as rtc0
>    *goldfish_rtc 96005000.rtc: setting system clock to 2022-06-05T01:49:57 UTC (1654393797)
>     NET: Registered PF_PACKET protocol family
>     random: fast init done
> 
> -Stafford

Re: [RFC PATCH 3/3] hw/openrisc: Add the OpenRISC virtual machine

[Stafford Horne]

On Sun, Jun 05, 2022 at 10:58:14AM +0900, Stafford Horne wrote:
> On Fri, Jun 03, 2022 at 09:05:09AM +0200, Geert Uytterhoeven wrote:
> > Hi Stafford,
> > 
> > On Thu, Jun 2, 2022 at 9:59 PM Stafford Horne <shorne@gmail.com> wrote:
> > > On Thu, Jun 02, 2022 at 09:08:52PM +0200, Geert Uytterhoeven wrote:
> > > > On Thu, Jun 2, 2022 at 1:42 PM Joel Stanley <joel@jms.id.au> wrote:
> > > > > On Fri, 27 May 2022 at 17:27, Stafford Horne <shorne@gmail.com> wrote:
> > > > > > This patch add the OpenRISC virtual machine 'virt' for OpenRISC.  This
> > > > > > platform allows for a convenient CI platform for toolchain, software
> > > > > > ports and the OpenRISC linux kernel port.
> > > > > >
> > > > > > Much of this has been sourced from the m68k and riscv virt platforms.
> > > >
> > > > > I enabled the options:
> > > > >
> > > > > CONFIG_RTC_CLASS=y
> > > > > # CONFIG_RTC_SYSTOHC is not set
> > > > > # CONFIG_RTC_NVMEM is not set
> > > > > CONFIG_RTC_DRV_GOLDFISH=y
> > > > >
> > > > > But it didn't work. It seems the goldfish rtc model doesn't handle a
> > > > > big endian guest running on my little endian host.
> > > > >
> > > > > Doing this fixes it:
> > > > >
> > > > > -    .endianness = DEVICE_NATIVE_ENDIAN,
> > > > > +    .endianness = DEVICE_HOST_ENDIAN,
> > > > >
> > > > > [    0.190000] goldfish_rtc 96005000.rtc: registered as rtc0
> > > > > [    0.190000] goldfish_rtc 96005000.rtc: setting system clock to
> > > > > 2022-06-02T11:16:04 UTC (1654168564)
> > > > >
> > > > > But literally no other model in the tree does this, so I suspect it's
> > > > > not the right fix.
> > > >
> > > > Goldfish devices are supposed to be little endian.
> > > > Unfortunately m68k got this wrong, cfr.
> > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=2e2ac4a3327479f7e2744cdd88a5c823f2057bad
> > > > Please don't duplicate this bad behavior for new architectures
> > >
> > > Thanks for the pointer, I just wired in the goldfish RTC because I wanted to
> > > play with it.  I was not attached to it. I can either remove it our find another
> > > RTC.
> > 
> > Sorry for being too unclear: the mistake was not to use the Goldfish
> > RTC, but to make its register accesses big-endian.
> > Using Goldfish devices as little-endian devices should be fine.
> 
> OK, then I would think this patch would be needed on Goldfish.  I tested this
> out and it seems to work:

Sorry, it seems maybe I mis-understood this again.  In Arnd's mail [1] he, at
the end, mentions.

    It might be a good idea to revisit the qemu implementation and make
    sure that the extra byteswap is only inserted on m68k and not on
    other targets, but hopefully there are no new targets based on goldfish
    anymore and we don't need to care.

So, it seems that in addition to my patch we would need something in m68k to
switch it back to 'native' (big) endian?

Looking at the m68k kernel/qemu interface I see:

Pre 5.19:
   (data) <-- kernel(readl / little) <-- m68k qemu (native / big) - RTC/PIC
   (data) <-- kernel(__raw_readl / big) <-- m68k qemu (native / big) - TTY

5.19:
   (data) <-- kernel(gf_ioread32 / big) <-- m68k qemu (native / big) - all

The new fixes to add gf_ioread32/gf_iowrite32 fix this for goldfish and m68k.
This wouldn't have been an issue for little-endian platforms where readl/writel
were originally used.

Why can't m68k switch to little-endian in qemu and the kernel?  The m68k virt
platform is not that old, 1 year? Are there a lot of users that this would be a big
problem?

[1] https://lore.kernel.org/lkml/CAK8P3a1oN8NrUjkh2X8jHQbyz42Xo6GSa=5n0gD6vQcXRjmq1Q@mail.gmail.com/

-Stafford

> Patch:
> 
> diff --git a/hw/rtc/goldfish_rtc.c b/hw/rtc/goldfish_rtc.c
> index 35e493be31..f1dc5af297 100644
> --- a/hw/rtc/goldfish_rtc.c
> +++ b/hw/rtc/goldfish_rtc.c
> @@ -219,7 +219,7 @@ static int goldfish_rtc_post_load(void *opaque, int
> version_id)
>  static const MemoryRegionOps goldfish_rtc_ops = {
>      .read = goldfish_rtc_read,
>      .write = goldfish_rtc_write,
> -    .endianness = DEVICE_NATIVE_ENDIAN,
> +    .endianness = DEVICE_LITTLE_ENDIAN,
>      .valid = {
>          .min_access_size = 4,
>          .max_access_size = 4
> 
> Boot Log:
> 
>     io scheduler mq-deadline registered
>     io scheduler kyber registered
>     Serial: 8250/16550 driver, 4 ports, IRQ sharing disabled
>     90000000.serial: ttyS0 at MMIO 0x90000000 (irq = 2, base_baud = 1250000) is a 16550A
>     printk: console [ttyS0] enabled
>     loop: module loaded
>     virtio_blk virtio1: [vda] 32768 512-byte logical blocks (16.8 MB/16.0 MiB)
>     Freeing initrd memory: 1696K
>    *goldfish_rtc 96005000.rtc: registered as rtc0
>    *goldfish_rtc 96005000.rtc: setting system clock to 2022-06-05T01:49:57 UTC (1654393797)
>     NET: Registered PF_PACKET protocol family
>     random: fast init done
> 
> -Stafford

Re: [RFC PATCH 3/3] hw/openrisc: Add the OpenRISC virtual machine

[Jason A. Donenfeld]

Hi folks,

On Sun, Jun 05, 2022 at 04:32:13PM +0900, Stafford Horne wrote:
> Why can't m68k switch to little-endian in qemu and the kernel?  The m68k virt
> platform is not that old, 1 year? Are there a lot of users that this would be a big
> problem?

I also share this perspective. AFAICT, m68k virt platform *just*
shipped. Fix this stuff instead of creating more compatibility bloat for
a platform with no new silicon. The risks of making life difficult for
15 minutes for all seven and a half users of that code that only now has
become operational is vastly dwarfed by the good sense to just fix the
mistake. Treat the endian thing as a *bug* rather than a sacred ABI.
Bugs only become sacred if you let them sit for years and large numbers
of people grow to rely on spacebar heating. Otherwise they're just bugs.
This can be fixed.

Jason

Re: [RFC PATCH 3/3] hw/openrisc: Add the OpenRISC virtual machine

[Jason A. Donenfeld]

Hi folks,

On Sun, Jun 05, 2022 at 04:32:13PM +0900, Stafford Horne wrote:
> Why can't m68k switch to little-endian in qemu and the kernel?  The m68k virt
> platform is not that old, 1 year? Are there a lot of users that this would be a big
> problem?

I also share this perspective. AFAICT, m68k virt platform *just*
shipped. Fix this stuff instead of creating more compatibility bloat for
a platform with no new silicon. The risks of making life difficult for
15 minutes for all seven and a half users of that code that only now has
become operational is vastly dwarfed by the good sense to just fix the
mistake. Treat the endian thing as a *bug* rather than a sacred ABI.
Bugs only become sacred if you let them sit for years and large numbers
of people grow to rely on spacebar heating. Otherwise they're just bugs.
This can be fixed.

Jason

Re: [RFC PATCH 3/3] hw/openrisc: Add the OpenRISC virtual machine

[Jason A. Donenfeld]

+ Arnd

On Sun, Jun 5, 2022 at 10:19 AM Jason A. Donenfeld <Jason@zx2c4.com> wrote:
>
> Hi folks,
>
> On Sun, Jun 05, 2022 at 04:32:13PM +0900, Stafford Horne wrote:
> > Why can't m68k switch to little-endian in qemu and the kernel?  The m68k virt
> > platform is not that old, 1 year? Are there a lot of users that this would be a big
> > problem?
>
> I also share this perspective. AFAICT, m68k virt platform *just*
> shipped. Fix this stuff instead of creating more compatibility bloat for
> a platform with no new silicon. The risks of making life difficult for
> 15 minutes for all seven and a half users of that code that only now has
> become operational is vastly dwarfed by the good sense to just fix the
> mistake. Treat the endian thing as a *bug* rather than a sacred ABI.
> Bugs only become sacred if you let them sit for years and large numbers
> of people grow to rely on spacebar heating. Otherwise they're just bugs.
> This can be fixed.
>
> Jason

Re: [RFC PATCH 3/3] hw/openrisc: Add the OpenRISC virtual machine

[Jason A. Donenfeld]

+ Arnd

On Sun, Jun 5, 2022 at 10:19 AM Jason A. Donenfeld <Jason@zx2c4.com> wrote:
>
> Hi folks,
>
> On Sun, Jun 05, 2022 at 04:32:13PM +0900, Stafford Horne wrote:
> > Why can't m68k switch to little-endian in qemu and the kernel?  The m68k virt
> > platform is not that old, 1 year? Are there a lot of users that this would be a big
> > problem?
>
> I also share this perspective. AFAICT, m68k virt platform *just*
> shipped. Fix this stuff instead of creating more compatibility bloat for
> a platform with no new silicon. The risks of making life difficult for
> 15 minutes for all seven and a half users of that code that only now has
> become operational is vastly dwarfed by the good sense to just fix the
> mistake. Treat the endian thing as a *bug* rather than a sacred ABI.
> Bugs only become sacred if you let them sit for years and large numbers
> of people grow to rely on spacebar heating. Otherwise they're just bugs.
> This can be fixed.
>
> Jason

Re: [RFC PATCH 3/3] hw/openrisc: Add the OpenRISC virtual machine

[Geert Uytterhoeven]

CC arnd

On Sun, Jun 5, 2022 at 9:32 AM Stafford Horne <shorne@gmail.com> wrote:
> On Sun, Jun 05, 2022 at 10:58:14AM +0900, Stafford Horne wrote:
> > On Fri, Jun 03, 2022 at 09:05:09AM +0200, Geert Uytterhoeven wrote:
> > > On Thu, Jun 2, 2022 at 9:59 PM Stafford Horne <shorne@gmail.com> wrote:
> > > > On Thu, Jun 02, 2022 at 09:08:52PM +0200, Geert Uytterhoeven wrote:
> > > > > On Thu, Jun 2, 2022 at 1:42 PM Joel Stanley <joel@jms.id.au> wrote:
> > > > > > On Fri, 27 May 2022 at 17:27, Stafford Horne <shorne@gmail.com> wrote:
> > > > > > > This patch add the OpenRISC virtual machine 'virt' for OpenRISC.  This
> > > > > > > platform allows for a convenient CI platform for toolchain, software
> > > > > > > ports and the OpenRISC linux kernel port.
> > > > > > >
> > > > > > > Much of this has been sourced from the m68k and riscv virt platforms.
> > > > >
> > > > > > I enabled the options:
> > > > > >
> > > > > > CONFIG_RTC_CLASS=y
> > > > > > # CONFIG_RTC_SYSTOHC is not set
> > > > > > # CONFIG_RTC_NVMEM is not set
> > > > > > CONFIG_RTC_DRV_GOLDFISH=y
> > > > > >
> > > > > > But it didn't work. It seems the goldfish rtc model doesn't handle a
> > > > > > big endian guest running on my little endian host.
> > > > > >
> > > > > > Doing this fixes it:
> > > > > >
> > > > > > -    .endianness = DEVICE_NATIVE_ENDIAN,
> > > > > > +    .endianness = DEVICE_HOST_ENDIAN,
> > > > > >
> > > > > > [    0.190000] goldfish_rtc 96005000.rtc: registered as rtc0
> > > > > > [    0.190000] goldfish_rtc 96005000.rtc: setting system clock to
> > > > > > 2022-06-02T11:16:04 UTC (1654168564)
> > > > > >
> > > > > > But literally no other model in the tree does this, so I suspect it's
> > > > > > not the right fix.
> > > > >
> > > > > Goldfish devices are supposed to be little endian.
> > > > > Unfortunately m68k got this wrong, cfr.
> > > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=2e2ac4a3327479f7e2744cdd88a5c823f2057bad
> > > > > Please don't duplicate this bad behavior for new architectures
> > > >
> > > > Thanks for the pointer, I just wired in the goldfish RTC because I wanted to
> > > > play with it.  I was not attached to it. I can either remove it our find another
> > > > RTC.
> > >
> > > Sorry for being too unclear: the mistake was not to use the Goldfish
> > > RTC, but to make its register accesses big-endian.
> > > Using Goldfish devices as little-endian devices should be fine.
> >
> > OK, then I would think this patch would be needed on Goldfish.  I tested this
> > out and it seems to work:
>
> Sorry, it seems maybe I mis-understood this again.  In Arnd's mail [1] he, at
> the end, mentions.
>
>     It might be a good idea to revisit the qemu implementation and make
>     sure that the extra byteswap is only inserted on m68k and not on
>     other targets, but hopefully there are no new targets based on goldfish
>     anymore and we don't need to care.
>
> So, it seems that in addition to my patch we would need something in m68k to
> switch it back to 'native' (big) endian?
>
> Looking at the m68k kernel/qemu interface I see:
>
> Pre 5.19:
>    (data) <-- kernel(readl / little) <-- m68k qemu (native / big) - RTC/PIC
>    (data) <-- kernel(__raw_readl / big) <-- m68k qemu (native / big) - TTY
>
> 5.19:
>    (data) <-- kernel(gf_ioread32 / big) <-- m68k qemu (native / big) - all
>
> The new fixes to add gf_ioread32/gf_iowrite32 fix this for goldfish and m68k.
> This wouldn't have been an issue for little-endian platforms where readl/writel
> were originally used.
>
> Why can't m68k switch to little-endian in qemu and the kernel?  The m68k virt
> platform is not that old, 1 year? Are there a lot of users that this would be a big
> problem?
>
> [1] https://lore.kernel.org/lkml/CAK8P3a1oN8NrUjkh2X8jHQbyz42Xo6GSa=5n0gD6vQcXRjmq1Q@mail.gmail.com/
>
> -Stafford
>
> > Patch:
> >
> > diff --git a/hw/rtc/goldfish_rtc.c b/hw/rtc/goldfish_rtc.c
> > index 35e493be31..f1dc5af297 100644
> > --- a/hw/rtc/goldfish_rtc.c
> > +++ b/hw/rtc/goldfish_rtc.c
> > @@ -219,7 +219,7 @@ static int goldfish_rtc_post_load(void *opaque, int
> > version_id)
> >  static const MemoryRegionOps goldfish_rtc_ops = {
> >      .read = goldfish_rtc_read,
> >      .write = goldfish_rtc_write,
> > -    .endianness = DEVICE_NATIVE_ENDIAN,
> > +    .endianness = DEVICE_LITTLE_ENDIAN,
> >      .valid = {
> >          .min_access_size = 4,
> >          .max_access_size = 4
> >
> > Boot Log:
> >
> >     io scheduler mq-deadline registered
> >     io scheduler kyber registered
> >     Serial: 8250/16550 driver, 4 ports, IRQ sharing disabled
> >     90000000.serial: ttyS0 at MMIO 0x90000000 (irq = 2, base_baud = 1250000) is a 16550A
> >     printk: console [ttyS0] enabled
> >     loop: module loaded
> >     virtio_blk virtio1: [vda] 32768 512-byte logical blocks (16.8 MB/16.0 MiB)
> >     Freeing initrd memory: 1696K
> >    *goldfish_rtc 96005000.rtc: registered as rtc0
> >    *goldfish_rtc 96005000.rtc: setting system clock to 2022-06-05T01:49:57 UTC (1654393797)
> >     NET: Registered PF_PACKET protocol family
> >     random: fast init done
> >
> > -Stafford

Re: [RFC PATCH 3/3] hw/openrisc: Add the OpenRISC virtual machine

[Geert Uytterhoeven]

CC arnd

On Sun, Jun 5, 2022 at 9:32 AM Stafford Horne <shorne@gmail.com> wrote:
> On Sun, Jun 05, 2022 at 10:58:14AM +0900, Stafford Horne wrote:
> > On Fri, Jun 03, 2022 at 09:05:09AM +0200, Geert Uytterhoeven wrote:
> > > On Thu, Jun 2, 2022 at 9:59 PM Stafford Horne <shorne@gmail.com> wrote:
> > > > On Thu, Jun 02, 2022 at 09:08:52PM +0200, Geert Uytterhoeven wrote:
> > > > > On Thu, Jun 2, 2022 at 1:42 PM Joel Stanley <joel@jms.id.au> wrote:
> > > > > > On Fri, 27 May 2022 at 17:27, Stafford Horne <shorne@gmail.com> wrote:
> > > > > > > This patch add the OpenRISC virtual machine 'virt' for OpenRISC.  This
> > > > > > > platform allows for a convenient CI platform for toolchain, software
> > > > > > > ports and the OpenRISC linux kernel port.
> > > > > > >
> > > > > > > Much of this has been sourced from the m68k and riscv virt platforms.
> > > > >
> > > > > > I enabled the options:
> > > > > >
> > > > > > CONFIG_RTC_CLASS=y
> > > > > > # CONFIG_RTC_SYSTOHC is not set
> > > > > > # CONFIG_RTC_NVMEM is not set
> > > > > > CONFIG_RTC_DRV_GOLDFISH=y
> > > > > >
> > > > > > But it didn't work. It seems the goldfish rtc model doesn't handle a
> > > > > > big endian guest running on my little endian host.
> > > > > >
> > > > > > Doing this fixes it:
> > > > > >
> > > > > > -    .endianness = DEVICE_NATIVE_ENDIAN,
> > > > > > +    .endianness = DEVICE_HOST_ENDIAN,
> > > > > >
> > > > > > [    0.190000] goldfish_rtc 96005000.rtc: registered as rtc0
> > > > > > [    0.190000] goldfish_rtc 96005000.rtc: setting system clock to
> > > > > > 2022-06-02T11:16:04 UTC (1654168564)
> > > > > >
> > > > > > But literally no other model in the tree does this, so I suspect it's
> > > > > > not the right fix.
> > > > >
> > > > > Goldfish devices are supposed to be little endian.
> > > > > Unfortunately m68k got this wrong, cfr.
> > > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=2e2ac4a3327479f7e2744cdd88a5c823f2057bad
> > > > > Please don't duplicate this bad behavior for new architectures
> > > >
> > > > Thanks for the pointer, I just wired in the goldfish RTC because I wanted to
> > > > play with it.  I was not attached to it. I can either remove it our find another
> > > > RTC.
> > >
> > > Sorry for being too unclear: the mistake was not to use the Goldfish
> > > RTC, but to make its register accesses big-endian.
> > > Using Goldfish devices as little-endian devices should be fine.
> >
> > OK, then I would think this patch would be needed on Goldfish.  I tested this
> > out and it seems to work:
>
> Sorry, it seems maybe I mis-understood this again.  In Arnd's mail [1] he, at
> the end, mentions.
>
>     It might be a good idea to revisit the qemu implementation and make
>     sure that the extra byteswap is only inserted on m68k and not on
>     other targets, but hopefully there are no new targets based on goldfish
>     anymore and we don't need to care.
>
> So, it seems that in addition to my patch we would need something in m68k to
> switch it back to 'native' (big) endian?
>
> Looking at the m68k kernel/qemu interface I see:
>
> Pre 5.19:
>    (data) <-- kernel(readl / little) <-- m68k qemu (native / big) - RTC/PIC
>    (data) <-- kernel(__raw_readl / big) <-- m68k qemu (native / big) - TTY
>
> 5.19:
>    (data) <-- kernel(gf_ioread32 / big) <-- m68k qemu (native / big) - all
>
> The new fixes to add gf_ioread32/gf_iowrite32 fix this for goldfish and m68k.
> This wouldn't have been an issue for little-endian platforms where readl/writel
> were originally used.
>
> Why can't m68k switch to little-endian in qemu and the kernel?  The m68k virt
> platform is not that old, 1 year? Are there a lot of users that this would be a big
> problem?
>
> [1] https://lore.kernel.org/lkml/CAK8P3a1oN8NrUjkh2X8jHQbyz42Xo6GSa=5n0gD6vQcXRjmq1Q@mail.gmail.com/
>
> -Stafford
>
> > Patch:
> >
> > diff --git a/hw/rtc/goldfish_rtc.c b/hw/rtc/goldfish_rtc.c
> > index 35e493be31..f1dc5af297 100644
> > --- a/hw/rtc/goldfish_rtc.c
> > +++ b/hw/rtc/goldfish_rtc.c
> > @@ -219,7 +219,7 @@ static int goldfish_rtc_post_load(void *opaque, int
> > version_id)
> >  static const MemoryRegionOps goldfish_rtc_ops = {
> >      .read = goldfish_rtc_read,
> >      .write = goldfish_rtc_write,
> > -    .endianness = DEVICE_NATIVE_ENDIAN,
> > +    .endianness = DEVICE_LITTLE_ENDIAN,
> >      .valid = {
> >          .min_access_size = 4,
> >          .max_access_size = 4
> >
> > Boot Log:
> >
> >     io scheduler mq-deadline registered
> >     io scheduler kyber registered
> >     Serial: 8250/16550 driver, 4 ports, IRQ sharing disabled
> >     90000000.serial: ttyS0 at MMIO 0x90000000 (irq = 2, base_baud = 1250000) is a 16550A
> >     printk: console [ttyS0] enabled
> >     loop: module loaded
> >     virtio_blk virtio1: [vda] 32768 512-byte logical blocks (16.8 MB/16.0 MiB)
> >     Freeing initrd memory: 1696K
> >    *goldfish_rtc 96005000.rtc: registered as rtc0
> >    *goldfish_rtc 96005000.rtc: setting system clock to 2022-06-05T01:49:57 UTC (1654393797)
> >     NET: Registered PF_PACKET protocol family
> >     random: fast init done
> >
> > -Stafford

Re: [RFC PATCH 3/3] hw/openrisc: Add the OpenRISC virtual machine

[Arnd Bergmann]

On Tue, Jun 7, 2022 at 10:11 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> On Sun, Jun 5, 2022 at 9:32 AM Stafford Horne <shorne@gmail.com> wrote:
> > On Sun, Jun 05, 2022 at 10:58:14AM +0900, Stafford Horne wrote:
> >     It might be a good idea to revisit the qemu implementation and make
> >     sure that the extra byteswap is only inserted on m68k and not on
> >     other targets, but hopefully there are no new targets based on goldfish
> >     anymore and we don't need to care.
> >
> > So, it seems that in addition to my patch we would need something in m68k to
> > switch it back to 'native' (big) endian?
> >
> > Looking at the m68k kernel/qemu interface I see:
> >
> > Pre 5.19:
> >    (data) <-- kernel(readl / little) <-- m68k qemu (native / big) - RTC/PIC
> >    (data) <-- kernel(__raw_readl / big) <-- m68k qemu (native / big) - TTY
> >
> > 5.19:
> >    (data) <-- kernel(gf_ioread32 / big) <-- m68k qemu (native / big) - all
> >
> > The new fixes to add gf_ioread32/gf_iowrite32 fix this for goldfish and m68k.
> > This wouldn't have been an issue for little-endian platforms where readl/writel
> > were originally used.
> >
> > Why can't m68k switch to little-endian in qemu and the kernel?  The m68k virt
> > platform is not that old, 1 year? Are there a lot of users that this would be a big
> > problem?
> >
> > [1] https://lore.kernel.org/lkml/CAK8P3a1oN8NrUjkh2X8jHQbyz42Xo6GSa=5n0gD6vQcXRjmq1Q@mail.gmail.com/

Goldfish is a very old platform, as far as I know only the kernel port is new.
I don't know when qemu started shipping goldfish, but changing it now would
surely break compatibility with whatever OS the port was originally made for.

      Arnd

Re: [RFC PATCH 3/3] hw/openrisc: Add the OpenRISC virtual machine

[Arnd Bergmann]

On Tue, Jun 7, 2022 at 10:11 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> On Sun, Jun 5, 2022 at 9:32 AM Stafford Horne <shorne@gmail.com> wrote:
> > On Sun, Jun 05, 2022 at 10:58:14AM +0900, Stafford Horne wrote:
> >     It might be a good idea to revisit the qemu implementation and make
> >     sure that the extra byteswap is only inserted on m68k and not on
> >     other targets, but hopefully there are no new targets based on goldfish
> >     anymore and we don't need to care.
> >
> > So, it seems that in addition to my patch we would need something in m68k to
> > switch it back to 'native' (big) endian?
> >
> > Looking at the m68k kernel/qemu interface I see:
> >
> > Pre 5.19:
> >    (data) <-- kernel(readl / little) <-- m68k qemu (native / big) - RTC/PIC
> >    (data) <-- kernel(__raw_readl / big) <-- m68k qemu (native / big) - TTY
> >
> > 5.19:
> >    (data) <-- kernel(gf_ioread32 / big) <-- m68k qemu (native / big) - all
> >
> > The new fixes to add gf_ioread32/gf_iowrite32 fix this for goldfish and m68k.
> > This wouldn't have been an issue for little-endian platforms where readl/writel
> > were originally used.
> >
> > Why can't m68k switch to little-endian in qemu and the kernel?  The m68k virt
> > platform is not that old, 1 year? Are there a lot of users that this would be a big
> > problem?
> >
> > [1] https://lore.kernel.org/lkml/CAK8P3a1oN8NrUjkh2X8jHQbyz42Xo6GSa=5n0gD6vQcXRjmq1Q@mail.gmail.com/

Goldfish is a very old platform, as far as I know only the kernel port is new.
I don't know when qemu started shipping goldfish, but changing it now would
surely break compatibility with whatever OS the port was originally made for.

      Arnd

Re: [RFC PATCH 3/3] hw/openrisc: Add the OpenRISC virtual machine

[Stafford Horne]

On Tue, Jun 07, 2022 at 10:42:08AM +0200, Arnd Bergmann wrote:
> On Tue, Jun 7, 2022 at 10:11 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > On Sun, Jun 5, 2022 at 9:32 AM Stafford Horne <shorne@gmail.com> wrote:
> > > On Sun, Jun 05, 2022 at 10:58:14AM +0900, Stafford Horne wrote:
> > >     It might be a good idea to revisit the qemu implementation and make
> > >     sure that the extra byteswap is only inserted on m68k and not on
> > >     other targets, but hopefully there are no new targets based on goldfish
> > >     anymore and we don't need to care.
> > >
> > > So, it seems that in addition to my patch we would need something in m68k to
> > > switch it back to 'native' (big) endian?
> > >
> > > Looking at the m68k kernel/qemu interface I see:
> > >
> > > Pre 5.19:
> > >    (data) <-- kernel(readl / little) <-- m68k qemu (native / big) - RTC/PIC
> > >    (data) <-- kernel(__raw_readl / big) <-- m68k qemu (native / big) - TTY
> > >
> > > 5.19:
> > >    (data) <-- kernel(gf_ioread32 / big) <-- m68k qemu (native / big) - all
> > >
> > > The new fixes to add gf_ioread32/gf_iowrite32 fix this for goldfish and m68k.
> > > This wouldn't have been an issue for little-endian platforms where readl/writel
> > > were originally used.
> > >
> > > Why can't m68k switch to little-endian in qemu and the kernel?  The m68k virt
> > > platform is not that old, 1 year? Are there a lot of users that this would be a big
> > > problem?
> > >
> > > [1] https://lore.kernel.org/lkml/CAK8P3a1oN8NrUjkh2X8jHQbyz42Xo6GSa=5n0gD6vQcXRjmq1Q@mail.gmail.com/
> 
> Goldfish is a very old platform, as far as I know only the kernel port is new.
> I don't know when qemu started shipping goldfish, but changing it now would
> surely break compatibility with whatever OS the port was originally made for.

Hi Arnd,

As far as I can tell goldfish in qemu is not very old. There are 3 devices, 2 were
added for the m68k virt machine, and 1 for riscv virt.

    $ git lo -- hw/rtc/goldfish_rtc.c
    2022-01-28 2f93d8b04a Peter Maydell    rtc: Move RTC function prototypes to their own header 
    2021-03-04 6b9409ba5f Laurent Vivier   goldfish_rtc: re-arm the alarm after migration 
    2020-10-13 16b66c5626 Laurent Vivier   goldfish_rtc: change MemoryRegionOps endianness to DEVICE_NATIVE_ENDIAN 
    2020-07-22 8380b3a453 Jessica Clarke   goldfish_rtc: Fix non-atomic read behaviour of TIME_LOW/TIME_HIGH 
    2020-02-10 9a5b40b842 Anup Patel       hw: rtc: Add Goldfish RTC device 

    $ git lo -- hw/char/goldfish_tty.c
    2021-11-09 65b4c8c759 Philippe Mathieu-Daudé hw/m68k: Fix typo in SPDX tag 
    2021-03-15 8c6df16ff6 Laurent Vivier   hw/char: add goldfish-tty 

    $  git lo -- hw/intc/goldfish_pic.c
    2021-11-09 65b4c8c759 Philippe Mathieu-Daudé hw/m68k: Fix typo in SPDX tag 
    2021-03-15 8785559390 Laurent Vivier   hw/intc: add goldfish-pic 

The mips/loongson3_virt machine now also uses the goldfish_rtc.

The problem with the goldfish device models is that they were defined as
DEVICE_NATIVE_ENDIAN.

    $ grep endianness hw/*/goldfish*
    hw/char/goldfish_tty.c:    .endianness = DEVICE_NATIVE_ENDIAN,
    hw/intc/goldfish_pic.c:    .endianness = DEVICE_NATIVE_ENDIAN,
    hw/rtc/goldfish_rtc.c:    .endianness = DEVICE_NATIVE_ENDIAN,

RISC-V is little-endian so when it was added there was no problem with running
linux goldfish drivers.

MIPS Longson3, added last year, seems to be running as little-endian well, I
understand MIPS can support both big and little endian. However according to
this all Loongson cores are little-endian.

    https://en.wikipedia.org/wiki/Loongson

As I understand when endianness of the devices in qemu are defined as
DEVICE_NATIVE_ENDIAN the device endian takes the endian of the target CPU.
This means that MIPS Loongson3 and RISC-V are affectively running as
little-endian which is what would be expected.

So it appears to me that in qemu that m68k is the only architecture that is
providing goldfish devices on a big-endian architecture.  Also, as far as I
know Linux is the only OS that was updated to cater for that.  If there are
other firmware/bootloaders that expect that maybe they could be updated too?

-Stafford

Re: [RFC PATCH 3/3] hw/openrisc: Add the OpenRISC virtual machine

[Stafford Horne]

On Tue, Jun 07, 2022 at 10:42:08AM +0200, Arnd Bergmann wrote:
> On Tue, Jun 7, 2022 at 10:11 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > On Sun, Jun 5, 2022 at 9:32 AM Stafford Horne <shorne@gmail.com> wrote:
> > > On Sun, Jun 05, 2022 at 10:58:14AM +0900, Stafford Horne wrote:
> > >     It might be a good idea to revisit the qemu implementation and make
> > >     sure that the extra byteswap is only inserted on m68k and not on
> > >     other targets, but hopefully there are no new targets based on goldfish
> > >     anymore and we don't need to care.
> > >
> > > So, it seems that in addition to my patch we would need something in m68k to
> > > switch it back to 'native' (big) endian?
> > >
> > > Looking at the m68k kernel/qemu interface I see:
> > >
> > > Pre 5.19:
> > >    (data) <-- kernel(readl / little) <-- m68k qemu (native / big) - RTC/PIC
> > >    (data) <-- kernel(__raw_readl / big) <-- m68k qemu (native / big) - TTY
> > >
> > > 5.19:
> > >    (data) <-- kernel(gf_ioread32 / big) <-- m68k qemu (native / big) - all
> > >
> > > The new fixes to add gf_ioread32/gf_iowrite32 fix this for goldfish and m68k.
> > > This wouldn't have been an issue for little-endian platforms where readl/writel
> > > were originally used.
> > >
> > > Why can't m68k switch to little-endian in qemu and the kernel?  The m68k virt
> > > platform is not that old, 1 year? Are there a lot of users that this would be a big
> > > problem?
> > >
> > > [1] https://lore.kernel.org/lkml/CAK8P3a1oN8NrUjkh2X8jHQbyz42Xo6GSa=5n0gD6vQcXRjmq1Q@mail.gmail.com/
> 
> Goldfish is a very old platform, as far as I know only the kernel port is new.
> I don't know when qemu started shipping goldfish, but changing it now would
> surely break compatibility with whatever OS the port was originally made for.

Hi Arnd,

As far as I can tell goldfish in qemu is not very old. There are 3 devices, 2 were
added for the m68k virt machine, and 1 for riscv virt.

    $ git lo -- hw/rtc/goldfish_rtc.c
    2022-01-28 2f93d8b04a Peter Maydell    rtc: Move RTC function prototypes to their own header 
    2021-03-04 6b9409ba5f Laurent Vivier   goldfish_rtc: re-arm the alarm after migration 
    2020-10-13 16b66c5626 Laurent Vivier   goldfish_rtc: change MemoryRegionOps endianness to DEVICE_NATIVE_ENDIAN 
    2020-07-22 8380b3a453 Jessica Clarke   goldfish_rtc: Fix non-atomic read behaviour of TIME_LOW/TIME_HIGH 
    2020-02-10 9a5b40b842 Anup Patel       hw: rtc: Add Goldfish RTC device 

    $ git lo -- hw/char/goldfish_tty.c
    2021-11-09 65b4c8c759 Philippe Mathieu-Daudé hw/m68k: Fix typo in SPDX tag 
    2021-03-15 8c6df16ff6 Laurent Vivier   hw/char: add goldfish-tty 

    $  git lo -- hw/intc/goldfish_pic.c
    2021-11-09 65b4c8c759 Philippe Mathieu-Daudé hw/m68k: Fix typo in SPDX tag 
    2021-03-15 8785559390 Laurent Vivier   hw/intc: add goldfish-pic 

The mips/loongson3_virt machine now also uses the goldfish_rtc.

The problem with the goldfish device models is that they were defined as
DEVICE_NATIVE_ENDIAN.

    $ grep endianness hw/*/goldfish*
    hw/char/goldfish_tty.c:    .endianness = DEVICE_NATIVE_ENDIAN,
    hw/intc/goldfish_pic.c:    .endianness = DEVICE_NATIVE_ENDIAN,
    hw/rtc/goldfish_rtc.c:    .endianness = DEVICE_NATIVE_ENDIAN,

RISC-V is little-endian so when it was added there was no problem with running
linux goldfish drivers.

MIPS Longson3, added last year, seems to be running as little-endian well, I
understand MIPS can support both big and little endian. However according to
this all Loongson cores are little-endian.

    https://en.wikipedia.org/wiki/Loongson

As I understand when endianness of the devices in qemu are defined as
DEVICE_NATIVE_ENDIAN the device endian takes the endian of the target CPU.
This means that MIPS Loongson3 and RISC-V are affectively running as
little-endian which is what would be expected.

So it appears to me that in qemu that m68k is the only architecture that is
providing goldfish devices on a big-endian architecture.  Also, as far as I
know Linux is the only OS that was updated to cater for that.  If there are
other firmware/bootloaders that expect that maybe they could be updated too?

-Stafford

Re: [RFC PATCH 3/3] hw/openrisc: Add the OpenRISC virtual machine

[Arnd Bergmann]

On Tue, Jun 7, 2022 at 11:47 AM Stafford Horne <shorne@gmail.com> wrote:
> On Tue, Jun 07, 2022 at 10:42:08AM +0200, Arnd Bergmann wrote:

> > Goldfish is a very old platform, as far as I know only the kernel port is new.
> > I don't know when qemu started shipping goldfish, but changing it now would
> > surely break compatibility with whatever OS the port was originally made for.
>
> Hi Arnd,
>
> As far as I can tell goldfish in qemu is not very old. There are 3 devices, 2 were
> added for the m68k virt machine, and 1 for riscv virt.
>
>     $ git lo -- hw/char/goldfish_tty.c
>     2021-11-09 65b4c8c759 Philippe Mathieu-Daudé hw/m68k: Fix typo in SPDX tag
>     2021-03-15 8c6df16ff6 Laurent Vivier   hw/char: add goldfish-tty
>
>     $  git lo -- hw/intc/goldfish_pic.c
>     2021-11-09 65b4c8c759 Philippe Mathieu-Daudé hw/m68k: Fix typo in SPDX tag
>     2021-03-15 8785559390 Laurent Vivier   hw/intc: add goldfish-pic

That is much younger than Laurent made it appear, from his earlier explanations
I expected this to have shipped a long time ago and been used in other
OSs to the
point where it cannot be fixed.

> The mips/loongson3_virt machine now also uses the goldfish_rtc.
>
> The problem with the goldfish device models is that they were defined as
> DEVICE_NATIVE_ENDIAN.
>
>     $ grep endianness hw/*/goldfish*
>     hw/char/goldfish_tty.c:    .endianness = DEVICE_NATIVE_ENDIAN,
>     hw/intc/goldfish_pic.c:    .endianness = DEVICE_NATIVE_ENDIAN,
>     hw/rtc/goldfish_rtc.c:    .endianness = DEVICE_NATIVE_ENDIAN,
>
> RISC-V is little-endian so when it was added there was no problem with running
> linux goldfish drivers.
>
> MIPS Longson3, added last year, seems to be running as little-endian well, I
> understand MIPS can support both big and little endian. However according to
> this all Loongson cores are little-endian.
>
>     https://en.wikipedia.org/wiki/Loongson
>
> As I understand when endianness of the devices in qemu are defined as
> DEVICE_NATIVE_ENDIAN the device endian takes the endian of the target CPU.
>
> This means that MIPS Loongson3 and RISC-V are affectively running as
> little-endian which is what would be expected.

Not really, the definition of DEVICE_NATIVE_ENDIAN in qemu is much less
well-defined than that as I understand, it is just whatever the person adding
support for that CPU thought would be the right one. A lot of CPUs can
run either big-endian or little-endian code, and e.g. on ARM, qemu
DEVICE_NATIVE_ENDIAN is just always little-endian, regardless of
what the CPU runs, while I think on MIPS it would be whatever the CPU
is actually executing.

      Arnd

Re: [RFC PATCH 3/3] hw/openrisc: Add the OpenRISC virtual machine

[Arnd Bergmann]

On Tue, Jun 7, 2022 at 11:47 AM Stafford Horne <shorne@gmail.com> wrote:
> On Tue, Jun 07, 2022 at 10:42:08AM +0200, Arnd Bergmann wrote:

> > Goldfish is a very old platform, as far as I know only the kernel port is new.
> > I don't know when qemu started shipping goldfish, but changing it now would
> > surely break compatibility with whatever OS the port was originally made for.
>
> Hi Arnd,
>
> As far as I can tell goldfish in qemu is not very old. There are 3 devices, 2 were
> added for the m68k virt machine, and 1 for riscv virt.
>
>     $ git lo -- hw/char/goldfish_tty.c
>     2021-11-09 65b4c8c759 Philippe Mathieu-Daudé hw/m68k: Fix typo in SPDX tag
>     2021-03-15 8c6df16ff6 Laurent Vivier   hw/char: add goldfish-tty
>
>     $  git lo -- hw/intc/goldfish_pic.c
>     2021-11-09 65b4c8c759 Philippe Mathieu-Daudé hw/m68k: Fix typo in SPDX tag
>     2021-03-15 8785559390 Laurent Vivier   hw/intc: add goldfish-pic

That is much younger than Laurent made it appear, from his earlier explanations
I expected this to have shipped a long time ago and been used in other
OSs to the
point where it cannot be fixed.

> The mips/loongson3_virt machine now also uses the goldfish_rtc.
>
> The problem with the goldfish device models is that they were defined as
> DEVICE_NATIVE_ENDIAN.
>
>     $ grep endianness hw/*/goldfish*
>     hw/char/goldfish_tty.c:    .endianness = DEVICE_NATIVE_ENDIAN,
>     hw/intc/goldfish_pic.c:    .endianness = DEVICE_NATIVE_ENDIAN,
>     hw/rtc/goldfish_rtc.c:    .endianness = DEVICE_NATIVE_ENDIAN,
>
> RISC-V is little-endian so when it was added there was no problem with running
> linux goldfish drivers.
>
> MIPS Longson3, added last year, seems to be running as little-endian well, I
> understand MIPS can support both big and little endian. However according to
> this all Loongson cores are little-endian.
>
>     https://en.wikipedia.org/wiki/Loongson
>
> As I understand when endianness of the devices in qemu are defined as
> DEVICE_NATIVE_ENDIAN the device endian takes the endian of the target CPU.
>
> This means that MIPS Loongson3 and RISC-V are affectively running as
> little-endian which is what would be expected.

Not really, the definition of DEVICE_NATIVE_ENDIAN in qemu is much less
well-defined than that as I understand, it is just whatever the person adding
support for that CPU thought would be the right one. A lot of CPUs can
run either big-endian or little-endian code, and e.g. on ARM, qemu
DEVICE_NATIVE_ENDIAN is just always little-endian, regardless of
what the CPU runs, while I think on MIPS it would be whatever the CPU
is actually executing.

      Arnd

Re: [RFC PATCH 3/3] hw/openrisc: Add the OpenRISC virtual machine

[Peter Maydell]

On Tue, 7 Jun 2022 at 11:12, Stafford Horne <shorne@gmail.com> wrote:
>
> On Tue, Jun 07, 2022 at 10:42:08AM +0200, Arnd Bergmann wrote:
> > Goldfish is a very old platform, as far as I know only the kernel port is new.
> > I don't know when qemu started shipping goldfish, but changing it now would
> > surely break compatibility with whatever OS the port was originally made for.
>
> Hi Arnd,
>
> As far as I can tell goldfish in qemu is not very old. There are 3 devices, 2 were
> added for the m68k virt machine, and 1 for riscv virt.

Yep, these are new for (upstream) QEMU, and AIUI the only OS we care
about for these is Linux, really. My understanding is that these devices
were added because they were conveniently available in Linux :-)
Where they do have a much older history is in the Android emulator,
but upstream QEMU doesn't care about that.

> The problem with the goldfish device models is that they were defined as
> DEVICE_NATIVE_ENDIAN.
>
>     $ grep endianness hw/*/goldfish*
>     hw/char/goldfish_tty.c:    .endianness = DEVICE_NATIVE_ENDIAN,
>     hw/intc/goldfish_pic.c:    .endianness = DEVICE_NATIVE_ENDIAN,
>     hw/rtc/goldfish_rtc.c:    .endianness = DEVICE_NATIVE_ENDIAN,
>
> RISC-V is little-endian so when it was added there was no problem with running
> linux goldfish drivers.
>
> MIPS Longson3, added last year, seems to be running as little-endian well, I
> understand MIPS can support both big and little endian. However according to
> this all Loongson cores are little-endian.
>
>     https://en.wikipedia.org/wiki/Loongson
>
> As I understand when endianness of the devices in qemu are defined as
> DEVICE_NATIVE_ENDIAN the device endian takes the endian of the target CPU.
> This means that MIPS Loongson3 and RISC-V are affectively running as
> little-endian which is what would be expected.

DEVICE_NATIVE_ENDIAN means "whatever the 'native' endianness of the target
CPU architecture is". This is a compile-time thing, and doesn't change
if the CPU changes its endianness at runtime. So for instance for Arm
boards DEVICE_NATIVE_ENDIAN and DEVICE_LITTLE_ENDIAN are the same thing,
even if the guest OS is running with SCTLR_EL1.EE set (and even if a
particular board in qemu-system-arm sets up the CPU so it leaves reset
with .EE set to 1) The analogy on real hardware is that the way these
"switch endianness" CPUs work is that they just flip the bytes in the
data on their way out of the CPU, so changing the endianness in the CPU
doesn't cause devices to change the way they behave. QEMU's "target
endianness" is kind of like a property of the interconnect/system design
in some ways.

From QEMU's point of view, the thing we really don't want is devices
that magically change behaviour when the CPU switches endianness
at runtime, because those are weirdly unlike real hardware. (Virtio
is the main offender in this regard, but we're stuck with that.)
Devices that happen to be wired up differently on different target
architectures are fine for us. I don't have any definite examples
to hand, but my understanding is that this happens with real hardware
too, where a device (maybe 8250-compatible UART or Lance ethernet
are examples?) with 32-bit registers might be typically wired up in
a system for a big-endian CPU such that the guest code can write
a 32-bit word to it and get the "obvious" ordering matching the
device datasheet. This sort of thing is what DEVICE_NATIVE_ENDIAN
was intended for. (There are also various places where we use it
where perhaps we should not where a device is exclusively used
on a CPU of a particular endianness, and so you could equally write
DEVICE_LITTLE_ENDIAN or whatever without any behaviour change.)

So I don't have a strong view on whether these devices should
be DEVICE_NATIVE_ENDIAN or DEVICE_LITTLE_ENDIAN (except that
my impression is that a DEVICE_LITTLE_ENDIAN device on a
big-endian system is a bit weird, because it means the guest
has to byteswap everything. You see that with PCI devices because
the PCI spec mandates LE, but not often elsewhere).

If there's an official-ish spec for how goldfish devices are
supposed to behave (does anybody have a pointer to one?) and it says
"always little-endian" then that would probably suggest that fixing
m68k would be nice if we can.

thanks
-- PMM

Re: [RFC PATCH 3/3] hw/openrisc: Add the OpenRISC virtual machine

[Peter Maydell]

On Tue, 7 Jun 2022 at 11:12, Stafford Horne <shorne@gmail.com> wrote:
>
> On Tue, Jun 07, 2022 at 10:42:08AM +0200, Arnd Bergmann wrote:
> > Goldfish is a very old platform, as far as I know only the kernel port is new.
> > I don't know when qemu started shipping goldfish, but changing it now would
> > surely break compatibility with whatever OS the port was originally made for.
>
> Hi Arnd,
>
> As far as I can tell goldfish in qemu is not very old. There are 3 devices, 2 were
> added for the m68k virt machine, and 1 for riscv virt.

Yep, these are new for (upstream) QEMU, and AIUI the only OS we care
about for these is Linux, really. My understanding is that these devices
were added because they were conveniently available in Linux :-)
Where they do have a much older history is in the Android emulator,
but upstream QEMU doesn't care about that.

> The problem with the goldfish device models is that they were defined as
> DEVICE_NATIVE_ENDIAN.
>
>     $ grep endianness hw/*/goldfish*
>     hw/char/goldfish_tty.c:    .endianness = DEVICE_NATIVE_ENDIAN,
>     hw/intc/goldfish_pic.c:    .endianness = DEVICE_NATIVE_ENDIAN,
>     hw/rtc/goldfish_rtc.c:    .endianness = DEVICE_NATIVE_ENDIAN,
>
> RISC-V is little-endian so when it was added there was no problem with running
> linux goldfish drivers.
>
> MIPS Longson3, added last year, seems to be running as little-endian well, I
> understand MIPS can support both big and little endian. However according to
> this all Loongson cores are little-endian.
>
>     https://en.wikipedia.org/wiki/Loongson
>
> As I understand when endianness of the devices in qemu are defined as
> DEVICE_NATIVE_ENDIAN the device endian takes the endian of the target CPU.
> This means that MIPS Loongson3 and RISC-V are affectively running as
> little-endian which is what would be expected.

DEVICE_NATIVE_ENDIAN means "whatever the 'native' endianness of the target
CPU architecture is". This is a compile-time thing, and doesn't change
if the CPU changes its endianness at runtime. So for instance for Arm
boards DEVICE_NATIVE_ENDIAN and DEVICE_LITTLE_ENDIAN are the same thing,
even if the guest OS is running with SCTLR_EL1.EE set (and even if a
particular board in qemu-system-arm sets up the CPU so it leaves reset
with .EE set to 1) The analogy on real hardware is that the way these
"switch endianness" CPUs work is that they just flip the bytes in the
data on their way out of the CPU, so changing the endianness in the CPU
doesn't cause devices to change the way they behave. QEMU's "target
endianness" is kind of like a property of the interconnect/system design
in some ways.

From QEMU's point of view, the thing we really don't want is devices
that magically change behaviour when the CPU switches endianness
at runtime, because those are weirdly unlike real hardware. (Virtio
is the main offender in this regard, but we're stuck with that.)
Devices that happen to be wired up differently on different target
architectures are fine for us. I don't have any definite examples
to hand, but my understanding is that this happens with real hardware
too, where a device (maybe 8250-compatible UART or Lance ethernet
are examples?) with 32-bit registers might be typically wired up in
a system for a big-endian CPU such that the guest code can write
a 32-bit word to it and get the "obvious" ordering matching the
device datasheet. This sort of thing is what DEVICE_NATIVE_ENDIAN
was intended for. (There are also various places where we use it
where perhaps we should not where a device is exclusively used
on a CPU of a particular endianness, and so you could equally write
DEVICE_LITTLE_ENDIAN or whatever without any behaviour change.)

So I don't have a strong view on whether these devices should
be DEVICE_NATIVE_ENDIAN or DEVICE_LITTLE_ENDIAN (except that
my impression is that a DEVICE_LITTLE_ENDIAN device on a
big-endian system is a bit weird, because it means the guest
has to byteswap everything. You see that with PCI devices because
the PCI spec mandates LE, but not often elsewhere).

If there's an official-ish spec for how goldfish devices are
supposed to behave (does anybody have a pointer to one?) and it says
"always little-endian" then that would probably suggest that fixing
m68k would be nice if we can.

thanks
-- PMM

Re: [RFC PATCH 3/3] hw/openrisc: Add the OpenRISC virtual machine

[Stafford Horne]

On Tue, Jun 07, 2022 at 11:43:08AM +0100, Peter Maydell wrote:
> So I don't have a strong view on whether these devices should
> be DEVICE_NATIVE_ENDIAN or DEVICE_LITTLE_ENDIAN (except that
> my impression is that a DEVICE_LITTLE_ENDIAN device on a
> big-endian system is a bit weird, because it means the guest
> has to byteswap everything. You see that with PCI devices because
> the PCI spec mandates LE, but not often elsewhere).
> 
> If there's an official-ish spec for how goldfish devices are
> supposed to behave (does anybody have a pointer to one?) and it says
> "always little-endian" then that would probably suggest that fixing
> m68k would be nice if we can.

I think there are some conflicting thoughts on this.

In Geert's he mentioned:

  Using Goldfish devices as little-endian devices should be fine.

In Arnd's mail he mentions:

  https://lore.kernel.org/lkml/CAK8P3a1oN8NrUjkh2X8jHQbyz42Xo6GSa=5n0gD6vQcXRjmq1Q@mail.gmail.com/#t

  ... the device was clearly defined as having little-endian
  registers,

Based on that I was thinking that switching to DEVICE_LITTLE_ENDIAN would make
sense.

However, in a followup mail from Laurent we see:

  https://lore.kernel.org/lkml/cb884368-0226-e913-80d2-62d2b7b2e761@vivier.eu/

  The reference document[1] doesn't define the endianness of goldfish.

  [1] https://android.googlesource.com/platform/external/qemu/+/master/docs/GOLDFISH-VIRTUAL-HARDWARE.TXT


The documentation does not clearly specify it.  So maybe maybe or1k should just
be updated on the linux side and add gf_ioread32/gf_iowrite32 big-endian
accessors.

-Stafford

Re: [RFC PATCH 3/3] hw/openrisc: Add the OpenRISC virtual machine

[Stafford Horne]

On Tue, Jun 07, 2022 at 11:43:08AM +0100, Peter Maydell wrote:
> So I don't have a strong view on whether these devices should
> be DEVICE_NATIVE_ENDIAN or DEVICE_LITTLE_ENDIAN (except that
> my impression is that a DEVICE_LITTLE_ENDIAN device on a
> big-endian system is a bit weird, because it means the guest
> has to byteswap everything. You see that with PCI devices because
> the PCI spec mandates LE, but not often elsewhere).
> 
> If there's an official-ish spec for how goldfish devices are
> supposed to behave (does anybody have a pointer to one?) and it says
> "always little-endian" then that would probably suggest that fixing
> m68k would be nice if we can.

I think there are some conflicting thoughts on this.

In Geert's he mentioned:

  Using Goldfish devices as little-endian devices should be fine.

In Arnd's mail he mentions:

  https://lore.kernel.org/lkml/CAK8P3a1oN8NrUjkh2X8jHQbyz42Xo6GSa=5n0gD6vQcXRjmq1Q@mail.gmail.com/#t

  ... the device was clearly defined as having little-endian
  registers,

Based on that I was thinking that switching to DEVICE_LITTLE_ENDIAN would make
sense.

However, in a followup mail from Laurent we see:

  https://lore.kernel.org/lkml/cb884368-0226-e913-80d2-62d2b7b2e761@vivier.eu/

  The reference document[1] doesn't define the endianness of goldfish.

  [1] https://android.googlesource.com/platform/external/qemu/+/master/docs/GOLDFISH-VIRTUAL-HARDWARE.TXT


The documentation does not clearly specify it.  So maybe maybe or1k should just
be updated on the linux side and add gf_ioread32/gf_iowrite32 big-endian
accessors.

-Stafford

Re: [RFC PATCH 3/3] hw/openrisc: Add the OpenRISC virtual machine

[Arnd Bergmann]

On Tue, Jun 7, 2022 at 2:12 PM Stafford Horne <shorne@gmail.com> wrote:
> On Tue, Jun 07, 2022 at 11:43:08AM +0100, Peter Maydell wrote:
>
> However, in a followup mail from Laurent we see:
>
>   https://lore.kernel.org/lkml/cb884368-0226-e913-80d2-62d2b7b2e761@vivier.eu/
>
>   The reference document[1] doesn't define the endianness of goldfish.
>
>   [1] https://android.googlesource.com/platform/external/qemu/+/master/docs/GOLDFISH-VIRTUAL-HARDWARE.TXT
>
>
> The documentation does not clearly specify it.  So maybe maybe or1k should just
> be updated on the linux side and add gf_ioread32/gf_iowrite32 big-endian
> accessors.

I don't think it makes any sense to use big-endian for a new
architecture, just use
the default little-endian implementation on the linux side, and change
the qemu code
to have the backward-compatibility hack for m68k while using big-endian for
the rest.

       Arnd

Re: [RFC PATCH 3/3] hw/openrisc: Add the OpenRISC virtual machine

[Arnd Bergmann]

On Tue, Jun 7, 2022 at 2:12 PM Stafford Horne <shorne@gmail.com> wrote:
> On Tue, Jun 07, 2022 at 11:43:08AM +0100, Peter Maydell wrote:
>
> However, in a followup mail from Laurent we see:
>
>   https://lore.kernel.org/lkml/cb884368-0226-e913-80d2-62d2b7b2e761@vivier.eu/
>
>   The reference document[1] doesn't define the endianness of goldfish.
>
>   [1] https://android.googlesource.com/platform/external/qemu/+/master/docs/GOLDFISH-VIRTUAL-HARDWARE.TXT
>
>
> The documentation does not clearly specify it.  So maybe maybe or1k should just
> be updated on the linux side and add gf_ioread32/gf_iowrite32 big-endian
> accessors.

I don't think it makes any sense to use big-endian for a new
architecture, just use
the default little-endian implementation on the linux side, and change
the qemu code
to have the backward-compatibility hack for m68k while using big-endian for
the rest.

       Arnd

Re: [RFC PATCH 3/3] hw/openrisc: Add the OpenRISC virtual machine

[Stafford Horne]

On Thu, Jun 02, 2022 at 11:42:30AM +0000, Joel Stanley wrote:
> Hi Stafford,
> 
> On Fri, 27 May 2022 at 17:27, Stafford Horne <shorne@gmail.com> wrote:
> >
> > This patch add the OpenRISC virtual machine 'virt' for OpenRISC.  This
> > platform allows for a convenient CI platform for toolchain, software
> > ports and the OpenRISC linux kernel port.
> >
> > Much of this has been sourced from the m68k and riscv virt platforms.
> 
> It's a good idea! I did some playing around with your patch today.
> 
> I'd suggest adding something to docs/system/target-openrsic.rst,
> including an example command lines.

Yeah, good idea, this is the command I am using:

qemu-system-or1k -cpu or1200 -M virt \
  -kernel /home/shorne/work/linux/vmlinux \
  -initrd /home/shorne/work/linux/initramfs.cpio.gz \
  -device virtio-net-device,netdev=user -netdev user,id=user,net=10.9.0.1/24,host=10.9.0.100 \
  -serial mon:stdio -nographic \
  -device virtio-blk-device,drive=d0 -drive file=/home/shorne/work/linux/virt.qcow2,id=d0,if=none,format=qcow2 \
  -gdb tcp::10001 -smp cpus=2 -m 64

I should have mentioned it but the config I am using is here:

  https://github.com/stffrdhrn/linux/commits/or1k-virt

> >
> > The platform provides:
> >  - OpenRISC SMP with up to 8 cpus
> 
> You have this:
> 
> #define VIRT_CPUS_MAX 4
> i
> I tried booting with -smp 4 and it locked up when starting userspace
> (or I stopped getting serial output?):
> 
> [    0.060000] smp: Brought up 1 node, 4 CPUs
> ...
> [    0.960000] Run /init as init process
> 
> Running with -smp 2 and 3 worked. It does make booting much much slower.

Right, it should be 4, I just write 8 from memory.  You are also, right I have
issues with running 4 CPU's.  I will try richard's suggestion.  I have some old
patches to configure MTTCG also, but it had some limitations.  I will dig those
up and get this fixed for this series.

> >  - Generated RTC to automatically configure the guest kernel
> 
> Did you mean device tree?

Yeah, thats what I meant.

> >
> > Signed-off-by: Stafford Horne <shorne@gmail.com>
> > ---
> >  configs/devices/or1k-softmmu/default.mak |   1 +
> >  hw/openrisc/Kconfig                      |   9 +
> >  hw/openrisc/meson.build                  |   1 +
> >  hw/openrisc/virt.c                       | 429 +++++++++++++++++++++++
> >  4 files changed, 440 insertions(+)
> >  create mode 100644 hw/openrisc/virt.c
> >
> > diff --git a/configs/devices/or1k-softmmu/default.mak b/configs/devices/or1k-softmmu/default.mak
> > index 5b3ac89491..f3bf816067 100644
> > --- a/configs/devices/or1k-softmmu/default.mak
> > +++ b/configs/devices/or1k-softmmu/default.mak
> > @@ -5,3 +5,4 @@ CONFIG_SEMIHOSTING=y
> >  # Boards:
> >  #
> >  CONFIG_OR1K_SIM=y
> > +CONFIG_OR1K_VIRT=y
> > diff --git a/hw/openrisc/Kconfig b/hw/openrisc/Kconfig
> > index 8f284f3ba0..202134668e 100644
> > --- a/hw/openrisc/Kconfig
> > +++ b/hw/openrisc/Kconfig
> > @@ -4,3 +4,12 @@ config OR1K_SIM
> >      select OPENCORES_ETH
> >      select OMPIC
> >      select SPLIT_IRQ
> > +
> > +config OR1K_VIRT
> > +    bool
> > +    imply VIRTIO_VGA
> > +    imply TEST_DEVICES
> > +    select GOLDFISH_RTC
> > +    select SERIAL
> > +    select SIFIVE_TEST
> > +    select VIRTIO_MMIO
> 
> You could include the liteeth device too if we merged that.

I think we could add that with a litex machine.  For that we would need at least
the litex UART and SoC for reset.

> > diff --git a/hw/openrisc/virt.c b/hw/openrisc/virt.c
> > new file mode 100644
> > index 0000000000..147196fda3
> > --- /dev/null
> > +++ b/hw/openrisc/virt.c
> > @@ -0,0 +1,429 @@
> > +/*
> > + * OpenRISC QEMU virtual machine.
> > + *
> > + * Copyright (c) 2022 Stafford Horne <shorne@gmail.com>
> > + *
> > + * This library is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU Lesser General Public
> > + * License as published by the Free Software Foundation; either
> > + * version 2.1 of the License, or (at your option) any later version.
> > + *
> > + * This library is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > + * Lesser General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU Lesser General Public
> > + * License along with this library; if not, see <http://www.gnu.org/licenses/>.
> 
> I think you can use the SPDX tag here instead of writing out the text.

Right.

> > +static void openrisc_virt_init(MachineState *machine)
> > +{
> > +    ram_addr_t ram_size = machine->ram_size;
> > +    const char *kernel_filename = machine->kernel_filename;
> > +    OpenRISCCPU *cpus[VIRT_CPUS_MAX] = {};
> > +    OR1KVirtState *state = VIRT_MACHINE(machine);
> > +    MemoryRegion *ram;
> > +    hwaddr load_addr;
> > +    int n;
> > +    unsigned int smp_cpus = machine->smp.cpus;
> > +
> 
> > +    openrisc_virt_rtc_init(state, virt_memmap[VIRT_RTC].base,
> > +                           virt_memmap[VIRT_RTC].size, smp_cpus, cpus,
> > +                           VIRT_RTC_IRQ);
> > +
> > +    for (n = 0; n < VIRTIO_COUNT; n++) {
> 
> This would make more sense to me if you constructed the IRQ and base
> here, and then passed the actual base and irq number to your
> _virtio_init:
> 
>         size_t size = virt_memmap[VIRT_VIRTIO].size;
>         openrisc_virt_virtio_init(state, virt_memmap[VIRT_VIRTIO].base
> + size * n,
>                                   size, smp_cpus, cpus, VIRT_VIRTIO_IRQ + n);
> 

OK, yes that is better.

> > +        openrisc_virt_virtio_init(state, virt_memmap[VIRT_VIRTIO].base,
> > +                                  virt_memmap[VIRT_VIRTIO].size,
> > +                                  smp_cpus, cpus, VIRT_VIRTIO_IRQ, n);
> > +    }
> > +

Thanks a lot for the review.

-Stafford

Re: [RFC PATCH 3/3] hw/openrisc: Add the OpenRISC virtual machine

[Stafford Horne]

On Thu, Jun 02, 2022 at 11:42:30AM +0000, Joel Stanley wrote:
> Hi Stafford,
> 
> On Fri, 27 May 2022 at 17:27, Stafford Horne <shorne@gmail.com> wrote:
> >
> > This patch add the OpenRISC virtual machine 'virt' for OpenRISC.  This
> > platform allows for a convenient CI platform for toolchain, software
> > ports and the OpenRISC linux kernel port.
> >
> > Much of this has been sourced from the m68k and riscv virt platforms.
> 
> It's a good idea! I did some playing around with your patch today.
> 
> I'd suggest adding something to docs/system/target-openrsic.rst,
> including an example command lines.

Yeah, good idea, this is the command I am using:

qemu-system-or1k -cpu or1200 -M virt \
  -kernel /home/shorne/work/linux/vmlinux \
  -initrd /home/shorne/work/linux/initramfs.cpio.gz \
  -device virtio-net-device,netdev=user -netdev user,id=user,net=10.9.0.1/24,host=10.9.0.100 \
  -serial mon:stdio -nographic \
  -device virtio-blk-device,drive=d0 -drive file=/home/shorne/work/linux/virt.qcow2,id=d0,if=none,format=qcow2 \
  -gdb tcp::10001 -smp cpus=2 -m 64

I should have mentioned it but the config I am using is here:

  https://github.com/stffrdhrn/linux/commits/or1k-virt

> >
> > The platform provides:
> >  - OpenRISC SMP with up to 8 cpus
> 
> You have this:
> 
> #define VIRT_CPUS_MAX 4
> i
> I tried booting with -smp 4 and it locked up when starting userspace
> (or I stopped getting serial output?):
> 
> [    0.060000] smp: Brought up 1 node, 4 CPUs
> ...
> [    0.960000] Run /init as init process
> 
> Running with -smp 2 and 3 worked. It does make booting much much slower.

Right, it should be 4, I just write 8 from memory.  You are also, right I have
issues with running 4 CPU's.  I will try richard's suggestion.  I have some old
patches to configure MTTCG also, but it had some limitations.  I will dig those
up and get this fixed for this series.

> >  - Generated RTC to automatically configure the guest kernel
> 
> Did you mean device tree?

Yeah, thats what I meant.

> >
> > Signed-off-by: Stafford Horne <shorne@gmail.com>
> > ---
> >  configs/devices/or1k-softmmu/default.mak |   1 +
> >  hw/openrisc/Kconfig                      |   9 +
> >  hw/openrisc/meson.build                  |   1 +
> >  hw/openrisc/virt.c                       | 429 +++++++++++++++++++++++
> >  4 files changed, 440 insertions(+)
> >  create mode 100644 hw/openrisc/virt.c
> >
> > diff --git a/configs/devices/or1k-softmmu/default.mak b/configs/devices/or1k-softmmu/default.mak
> > index 5b3ac89491..f3bf816067 100644
> > --- a/configs/devices/or1k-softmmu/default.mak
> > +++ b/configs/devices/or1k-softmmu/default.mak
> > @@ -5,3 +5,4 @@ CONFIG_SEMIHOSTING=y
> >  # Boards:
> >  #
> >  CONFIG_OR1K_SIM=y
> > +CONFIG_OR1K_VIRT=y
> > diff --git a/hw/openrisc/Kconfig b/hw/openrisc/Kconfig
> > index 8f284f3ba0..202134668e 100644
> > --- a/hw/openrisc/Kconfig
> > +++ b/hw/openrisc/Kconfig
> > @@ -4,3 +4,12 @@ config OR1K_SIM
> >      select OPENCORES_ETH
> >      select OMPIC
> >      select SPLIT_IRQ
> > +
> > +config OR1K_VIRT
> > +    bool
> > +    imply VIRTIO_VGA
> > +    imply TEST_DEVICES
> > +    select GOLDFISH_RTC
> > +    select SERIAL
> > +    select SIFIVE_TEST
> > +    select VIRTIO_MMIO
> 
> You could include the liteeth device too if we merged that.

I think we could add that with a litex machine.  For that we would need at least
the litex UART and SoC for reset.

> > diff --git a/hw/openrisc/virt.c b/hw/openrisc/virt.c
> > new file mode 100644
> > index 0000000000..147196fda3
> > --- /dev/null
> > +++ b/hw/openrisc/virt.c
> > @@ -0,0 +1,429 @@
> > +/*
> > + * OpenRISC QEMU virtual machine.
> > + *
> > + * Copyright (c) 2022 Stafford Horne <shorne@gmail.com>
> > + *
> > + * This library is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU Lesser General Public
> > + * License as published by the Free Software Foundation; either
> > + * version 2.1 of the License, or (at your option) any later version.
> > + *
> > + * This library is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > + * Lesser General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU Lesser General Public
> > + * License along with this library; if not, see <http://www.gnu.org/licenses/>.
> 
> I think you can use the SPDX tag here instead of writing out the text.

Right.

> > +static void openrisc_virt_init(MachineState *machine)
> > +{
> > +    ram_addr_t ram_size = machine->ram_size;
> > +    const char *kernel_filename = machine->kernel_filename;
> > +    OpenRISCCPU *cpus[VIRT_CPUS_MAX] = {};
> > +    OR1KVirtState *state = VIRT_MACHINE(machine);
> > +    MemoryRegion *ram;
> > +    hwaddr load_addr;
> > +    int n;
> > +    unsigned int smp_cpus = machine->smp.cpus;
> > +
> 
> > +    openrisc_virt_rtc_init(state, virt_memmap[VIRT_RTC].base,
> > +                           virt_memmap[VIRT_RTC].size, smp_cpus, cpus,
> > +                           VIRT_RTC_IRQ);
> > +
> > +    for (n = 0; n < VIRTIO_COUNT; n++) {
> 
> This would make more sense to me if you constructed the IRQ and base
> here, and then passed the actual base and irq number to your
> _virtio_init:
> 
>         size_t size = virt_memmap[VIRT_VIRTIO].size;
>         openrisc_virt_virtio_init(state, virt_memmap[VIRT_VIRTIO].base
> + size * n,
>                                   size, smp_cpus, cpus, VIRT_VIRTIO_IRQ + n);
> 

OK, yes that is better.

> > +        openrisc_virt_virtio_init(state, virt_memmap[VIRT_VIRTIO].base,
> > +                                  virt_memmap[VIRT_VIRTIO].size,
> > +                                  smp_cpus, cpus, VIRT_VIRTIO_IRQ, n);
> > +    }
> > +

Thanks a lot for the review.

-Stafford