Re: hw/clock: What clock rate for virt machines?

[Palmer Dabbelt]

On Fri, 04 Sep 2020 10:52:06 PDT (-0700), alistair23@gmail.com wrote:
> On Wed, Sep 2, 2020 at 12:48 PM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> > On 9/2/20 8:18 PM, Peter Maydell wrote:
> > > On Wed, 2 Sep 2020 at 18:03, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> > >>
> > >> On 9/2/20 6:49 PM, Peter Maydell wrote:
> > >>> On Wed, 2 Sep 2020 at 17:35, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> > >>>> Peter said "'clock' is basically meaningless for virt machines",
> > >>>>
> > >>>> I understand and agree. But how to make that explicit/obvious in
> > >>>> the code, when a device expects a clock frequency/period?
> > >>>
> > >>> When a particular *device* needs a clock, then presumably
> > >>> it has a defined purpose for it, and we can pick a
> > >>> frequency for it then.
> > >>>
> > >>>> See for example hw/riscv/virt.c, it uses the following (confusing
> > >>>> to me) in virt_machine_init():
> > >>>>
> > >>>>    serial_mm_init(system_memory, memmap[VIRT_UART0].base,
> > >>>>        0, qdev_get_gpio_in(DEVICE(mmio_plic), UART0_IRQ), 399193,
> > >>>>        serial_hd(0), DEVICE_LITTLE_ENDIAN);
> > >>>
> > >>> In this case, the board has a model of a 16550A UART on it,
> > >>> which uses its input clock to determine what the actual baud
> > >>> rate is for particular guest settings of the divisor registers.
> > >>> So we need to look at:
> > >>>  * what does guest software expect the frequency to be?
> > >>
> > >> QEMU is supposed to model machine with no knowledge of the guest,
> > >> but the virt case is a particular one indeed... as it has to know
> > >> it is virtualized.
> > >>
> > >>>  * what is a "good" frequency which gives the guest the best
> > >>>    possible choices of baud rate?
> > >>
> > >> I'll think about it...
> > >
> > > My guess is that guest code assumes "same frequency an
> > > x86 PC uses", but a risc-v person might know better...
> > >
> > > (For QEMU I think it only makes a visible difference when
> > > you pass a host serial port through to the guest as
> > > otherwise we ignore whatever baud rate the guest sets.)
>
> I think that's the case. I suspect we just ignore whatever the guest
> sets. I have never really looked into it though.

IIRC that's essentially what we do for all the clock stuff in QEMU.  Sometimes
we fake things a bit better by doing something like always instantly setting
the "clock has locked" bit pattern or snaping multipliers to something close to
viable, but I don't think it gets a whole lot better than that.

> > It makes a difference with low baudrates (TBH I only tested
> > what the firmwares I have use: 9600-8N1). No clue why (from
> > design PoV) but some odd fw use the time spent to display
> > chars to have a nicer 'user experience' (using polling).
> > With incorrect timing everything is displayed at once partly
> > scrambled.
> >
> > The following devices are modeled with timers limiting the
> > transmit rate:
> >
> > $ git grep qemu_clock_get_ns hw/char/
> > hw/char/cadence_uart.c:269:    uint64_t new_rx_time =
> > qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
> > hw/char/exynos4210_uart.c:393:
> > qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + timeout);
> > hw/char/ibex_uart.c:155:    uint64_t current_time =
> > qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
>
> For the Ibex UART we want  to apply back pressure on the guest (as
> that's what the main OT software expects) which is why we use this. I
> think most other guest software doesn't care as much.

I'd bet a bunch of embedded software blows up when the UART suddenly becomes
very fast, whether it just exposes some bug or because the programmer knows
that's how the system behaves and the software is just taking advantage of it.
While I guess we can fix what we run in to I fell like this is going to be a
losing battle in QEMU -- I guess it's an extreme case, but imagine trying to
emulate the behavior of our DDR controller (right now we just treat all the
control registers as opaque memory, they're far from that).

> Alistair
>
> > hw/char/renesas_sci.c:74:    if (sci->rx_next >
> > qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL)) {
> > hw/char/renesas_sci.c:84:    sci->rx_next =
> > qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + sci->trtime;
> > hw/char/serial.c:290:    s->last_xmit_ts =
> > qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
> > hw/char/serial.c:899:    s->last_xmit_ts =
> > qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
> > hw/char/sh_serial.c:352:
> > qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + 15 * s->etu);
> >
> > >
> > > thanks
> > > -- PMM
> > >