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

[Geert Uytterhoeven <geert@linux-m68k.org>]

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