Message ID | 20220527172731.1742837-4-shorne@gmail.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | OpenRISC Semihosting and Virt | expand |
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); > + } > +
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~
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
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
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
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
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
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
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
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
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
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
+ 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
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
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
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
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 --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)
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