diff mbox series

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

Message ID 20220527172731.1742837-4-shorne@gmail.com (mailing list archive)
State New, archived
Headers show
Series OpenRISC Semihosting and Virt | expand

Commit Message

Stafford Horne May 27, 2022, 5:27 p.m. UTC
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

Comments

Joel Stanley June 2, 2022, 11:42 a.m. UTC | #1
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);
> +    }
> +
Richard Henderson June 2, 2022, 3:49 p.m. UTC | #2
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~
Geert Uytterhoeven June 2, 2022, 7:08 p.m. UTC | #3
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
Stafford Horne June 2, 2022, 7:59 p.m. UTC | #4
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
Geert Uytterhoeven June 3, 2022, 7:05 a.m. UTC | #5
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
Stafford Horne June 5, 2022, 1:58 a.m. UTC | #6
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
Stafford Horne June 5, 2022, 2:36 a.m. UTC | #7
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
Stafford Horne June 5, 2022, 7:32 a.m. UTC | #8
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
Jason A. Donenfeld June 5, 2022, 8:19 a.m. UTC | #9
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
Geert Uytterhoeven June 7, 2022, 8:11 a.m. UTC | #10
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
Arnd Bergmann June 7, 2022, 8:42 a.m. UTC | #11
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
Stafford Horne June 7, 2022, 9:47 a.m. UTC | #12
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
Jason A. Donenfeld June 7, 2022, 9:48 a.m. UTC | #13
+ 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
Arnd Bergmann June 7, 2022, 10:04 a.m. UTC | #14
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
Peter Maydell June 7, 2022, 10:43 a.m. UTC | #15
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
Stafford Horne June 7, 2022, 12:12 p.m. UTC | #16
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
Arnd Bergmann June 7, 2022, 2:08 p.m. UTC | #17
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
diff mbox series

Patch

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)