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

[Stafford Horne <shorne@gmail.com>]

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