qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Finn Thain <fthain@linux-m68k.org>
To: David Gibson <david@gibson.dropbear.id.au>,
	Greg Kurz <groug@kaod.org>,
	Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Cc: qemu-ppc@nongnu.org, Laurent Vivier <laurent@vivier.eu>,
	qemu-devel@nongnu.org
Subject: [RFC 00/10] hw/mos6522: VIA timer emulation fixes and improvements
Date: Tue, 24 Aug 2021 20:09:36 +1000	[thread overview]
Message-ID: <cover.1629799776.git.fthain@linux-m68k.org> (raw)

This is a patch series that I started last year. The aim was to try to 
get a monotonic clocksource for Linux/m68k guests. That aim hasn't been 
achieved yet (for q800 machines) but I'm submitting the patch series as 
an RFC because,

 - It does improve 6522 emulation fidelity.

 - It allows Linux/m68k to make use of the additional timer that the 
   hardware indeed offers but which QEMU omits. This has several 
   benefits for Linux guests [1].

 - I see that Mark has been working on timer emulation issues in his 
   github repo [2] and it seems likely that MacOS, NetBSD or A/UX guests 
   will also require better 6522 emulation.

To make collaboration easier these patches can also be fetched from 
github [3].

On a real Quadra, accesses to the SY6522 chips are slow because they are 
synchronous with the 783360 Hz "phase 2" clock. In QEMU, they are slow 
only because of the division operation in the timer count calculation.

This patch series improves the fidelity of the emulated chip, but the 
price is more division ops. I haven't tried to measure this yet.

The emulated 6522 still deviates from the behaviour of the real thing, 
however. For example, two consecutive accesses to a real 6522 timer 
counter can never yield the same value. This is not true of the 6522 in 
QEMU 6 wherein two consecutive accesses to a timer count register have 
been observed to yield the same value.

Linux is not particularly robust in the face of a 6522 that deviates 
from the usual behaviour. The problem presently affecting a Linux guest 
is that its 'via' clocksource is prone to monotonicity failure. That is, 
the clocksource counter can jump backwards. This can be observed by 
patching Linux like so:

diff --git a/arch/m68k/mac/via.c b/arch/m68k/mac/via.c
--- a/arch/m68k/mac/via.c
+++ b/arch/m68k/mac/via.c
@@ -606,6 +606,8 @@ void __init via_init_clock(void)
 	clocksource_register_hz(&mac_clk, VIA_CLOCK_FREQ);
 }
 
+static u32 prev_ticks;
+
 static u64 mac_read_clk(struct clocksource *cs)
 {
 	unsigned long flags;
@@ -631,6 +633,8 @@ static u64 mac_read_clk(struct clocksource *cs)
 	count = count_high << 8;
 	ticks = VIA_TIMER_CYCLES - count;
 	ticks += clk_offset + clk_total;
+if (ticks < prev_ticks) pr_warn("%s: %u < %u\n", __func__, ticks, prev_ticks);
+prev_ticks = ticks;
 	local_irq_restore(flags);
 
 	return ticks;

This problem can be partly blamed on a 6522 design limitation, which is 
that the timer counter has no overflow register. Hence, if a timer 
counter wraps around and the kernel is late to handle the subsequent 
interrupt, the kernel can't account for any missed ticks.

On a real Quadra, the kernel mitigates this limitation by minimizing 
interrupt latency. But on QEMU, interrupt latency is unbounded. This 
can't be mitigated by the guest kernel at all and leads to clock drift. 
This can be observed by patching QEMU like so:

diff --git a/hw/misc/mos6522.c b/hw/misc/mos6522.c
--- a/hw/misc/mos6522.c
+++ b/hw/misc/mos6522.c
@@ -379,6 +379,12 @@ void mos6522_write(void *opaque, hwaddr addr, uint64_t val, unsigned size)
         s->pcr = val;
         break;
     case VIA_REG_IFR:
+        if (val & T1_INT) {
+            static int64_t last_t1_int_cleared;
+            int64_t now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
+            if (now - last_t1_int_cleared > 20000000) printf("\t%s: t1 int clear is late\n", __func__);
+            last_t1_int_cleared = now;
+        }
         /* reset bits */
         s->ifr &= ~val;
         mos6522_update_irq(s);

This logic asserts that, given that Linux/m68k sets CONFIG_HZ to 100, 
the emulator will theoretically see each timer 1 interrupt cleared 
within 20 ms of the previous one. But that deadline is often missed on 
my QEMU host [4].

On real Mac hardware you could observe the same scenario if a high 
priority interrupt were to sufficiently delay the timer interrupt 
handler. (This is the reason why the VIA1 interrupt priority gets 
increased from level 1 to level 5 when running on Quadras.)

Anyway, for now, the clocksource monotonicity problem in Linux/mac68k 
guests is still unresolved. Nonetheless, I think this patch series does 
improve the situation.

[1] I've also been working on some improvements to Linux/m68k based on 
Arnd Bergman's clockevent RFC patch, 
https://lore.kernel.org/linux-m68k/20201008154651.1901126-14-arnd@arndb.de/ 
The idea is to add a oneshot clockevent device by making use of the 
second VIA1 timer. This approach should help mitigate the clock drift 
problem as well as assist with GENERIC_CLOCKEVENTS adoption.

[2] https://github.com/mcayland/qemu/commits/q800.upstream

[3] https://github.com/fthain/qemu/commits/via-timer/

[4] This theoretical 20 ms deadline is not missed prior to every 
backwards jump in the clocksource counter. AFAICT, that's because the 
true deadline is somewhat shorter than 20 ms.


Finn Thain (10):
  hw/mos6522: Remove get_load_time() methods and functions
  hw/mos6522: Remove get_counter_value() methods and functions
  hw/mos6522: Remove redundant mos6522_timer1_update() calls
  hw/mos6522: Rename timer callback functions
  hw/mos6522: Don't clear T1 interrupt flag on latch write
  hw/mos6522: Implement oneshot mode
  hw/mos6522: Fix initial timer counter reload
  hw/mos6522: Call mos6522_update_irq() when appropriate
  hw/mos6522: Avoid using discrepant QEMU clock values
  hw/mos6522: Synchronize timer interrupt and timer counter

 hw/misc/mos6522.c         | 232 +++++++++++++++++---------------------
 hw/misc/trace-events      |   2 +-
 include/hw/misc/mos6522.h |   9 ++
 3 files changed, 113 insertions(+), 130 deletions(-)

-- 
2.26.3



             reply	other threads:[~2021-08-24 10:15 UTC|newest]

Thread overview: 47+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-24 10:09 Finn Thain [this message]
2021-08-24 10:09 ` [RFC 09/10] hw/mos6522: Avoid using discrepant QEMU clock values Finn Thain
2021-08-24 10:28   ` Philippe Mathieu-Daudé
2021-08-29  1:23     ` Finn Thain
2021-08-25  8:44   ` Mark Cave-Ayland
2021-08-29  1:55     ` Finn Thain
2021-08-24 10:09 ` [RFC 06/10] hw/mos6522: Implement oneshot mode Finn Thain
2021-08-25  8:18   ` Mark Cave-Ayland
2021-08-29  1:20     ` Finn Thain
2021-08-24 10:09 ` [RFC 01/10] hw/mos6522: Remove get_load_time() methods and functions Finn Thain
2021-08-24 10:29   ` Philippe Mathieu-Daudé
2021-08-25  6:55   ` Mark Cave-Ayland
2021-08-28  1:00     ` Finn Thain
2021-08-24 10:09 ` [RFC 08/10] hw/mos6522: Call mos6522_update_irq() when appropriate Finn Thain
2021-08-24 10:22   ` Philippe Mathieu-Daudé
2021-08-25  8:26   ` Mark Cave-Ayland
2021-08-24 10:09 ` [RFC 07/10] hw/mos6522: Fix initial timer counter reload Finn Thain
2021-08-25  8:23   ` Mark Cave-Ayland
2021-08-28  0:46     ` Finn Thain
2021-08-24 10:09 ` [RFC 10/10] hw/mos6522: Synchronize timer interrupt and timer counter Finn Thain
2021-08-25  8:52   ` Mark Cave-Ayland
2021-08-26  6:43     ` Finn Thain
2021-08-24 10:09 ` [RFC 04/10] hw/mos6522: Rename timer callback functions Finn Thain
2021-08-24 10:28   ` Philippe Mathieu-Daudé
2021-08-25  7:11   ` Mark Cave-Ayland
2021-08-26  7:42     ` Philippe Mathieu-Daudé
2021-08-24 10:09 ` [RFC 02/10] hw/mos6522: Remove get_counter_value() methods and functions Finn Thain
2021-08-24 10:29   ` Philippe Mathieu-Daudé
2021-08-24 10:09 ` [RFC 05/10] hw/mos6522: Don't clear T1 interrupt flag on latch write Finn Thain
2021-08-25  7:20   ` Mark Cave-Ayland
2021-08-26  5:21     ` Finn Thain
2021-09-01 14:32       ` Laurent Vivier
2021-09-01 22:26         ` Finn Thain
2021-08-24 10:09 ` [RFC 03/10] hw/mos6522: Remove redundant mos6522_timer1_update() calls Finn Thain
2021-08-25  7:09   ` Mark Cave-Ayland
2021-08-24 10:34 ` [RFC 00/10] hw/mos6522: VIA timer emulation fixes and improvements Philippe Mathieu-Daudé
2021-08-28  1:22   ` Finn Thain
2021-08-31 21:14     ` Mark Cave-Ayland
2021-08-31 22:44       ` Finn Thain
2021-09-01  7:57         ` Mark Cave-Ayland
2021-09-01  8:06           ` Mark Cave-Ayland
2021-09-10 17:29             ` Mark Cave-Ayland
2021-09-11  0:08               ` Finn Thain
2021-09-01  2:20       ` Finn Thain
2021-08-25  3:11 ` David Gibson
2021-08-25  9:10 ` Mark Cave-Ayland
2021-08-28  4:11   ` Finn Thain

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=cover.1629799776.git.fthain@linux-m68k.org \
    --to=fthain@linux-m68k.org \
    --cc=david@gibson.dropbear.id.au \
    --cc=groug@kaod.org \
    --cc=laurent@vivier.eu \
    --cc=mark.cave-ayland@ilande.co.uk \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-ppc@nongnu.org \
    --subject='Re: [RFC 00/10] hw/mos6522: VIA timer emulation fixes and improvements' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).