From: Finn Thain <fthain@linux-m68k.org>
To: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Cc: "Peter Maydell" <peter.maydell@linaro.org>,
qemu-devel@nongnu.org, "Greg Kurz" <groug@kaod.org>,
"Philippe Mathieu-Daudé" <f4bug@amsat.org>,
qemu-ppc@nongnu.org, "David Gibson" <david@gibson.dropbear.id.au>
Subject: Re: [RFC 00/10] hw/mos6522: VIA timer emulation fixes and improvements
Date: Sat, 11 Sep 2021 10:08:26 +1000 (AEST) [thread overview]
Message-ID: <d06666-ce8a-88d5-e47e-d2439241a74@linux-m68k.org> (raw)
In-Reply-To: <39ba85bb-0bba-20b1-ef39-9362bb23bedc@ilande.co.uk>
On Fri, 10 Sep 2021, Mark Cave-Ayland wrote:
> On 01/09/2021 09:06, Mark Cave-Ayland wrote:
>
> > I'll have a go at some basic timer measurements using your patch to
> > see what sort of numbers I get for the latency here. Obviously QEMU
> > doesn't guarantee response times but over 20ms does seem high.
>
> I was able to spend some time today looking at this and came up with
> https://github.com/mcayland/qemu/tree/via-hacks with the aim of starting
> with your patches to reduce the calls to the timer update functions (to
> reduce jitter) and fixing up the places where mos6522_update_irq() isn't
> called.
>
What kind of guest was that? What impact does jitter have on that guest?
Was the jitter measurement increased, decreased or unchanged by this patch
series?
> That seemed okay, but the part I'm struggling with at the moment is
> removing the re-assertion of the timer interrupt if the timer has
> expired when any of the registers are read i.e.
>
> diff --git a/hw/misc/mos6522.c b/hw/misc/mos6522.c
> index 3c33cbebde..9884d7e178 100644
> --- a/hw/misc/mos6522.c
> +++ b/hw/misc/mos6522.c
> @@ -229,16 +229,7 @@ uint64_t mos6522_read(void *opaque, hwaddr addr, unsigned
> size)
> {
> MOS6522State *s = opaque;
> uint32_t val;
> - int64_t now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
>
> - if (now >= s->timers[0].next_irq_time) {
> - mos6522_timer1_update(s, &s->timers[0], now);
> - s->ifr |= T1_INT;
> - }
> - if (now >= s->timers[1].next_irq_time) {
> - mos6522_timer2_update(s, &s->timers[1], now);
> - s->ifr |= T2_INT;
> - }
> switch (addr) {
> case VIA_REG_B:
> val = s->b;
>
> If I apply this then I see a hang in about roughly 1 in 10 boots. Poking
> it with a debugger shows that the VIA1 timer interrupts are constantly
> being fired as IER and IFR have the timer 1 bit set, but it is being
> filtered out by SR being set to 0x2700 on the CPU.
>
> The existing code above isn't correct since T1_INT should only be
> asserted by the expiry of the timer 1 via mos6522_timer1(), so I'm
> wondering if this is tickling a CPU bug somewhere? In what circumstances
> could interrupts be disabled via SR but not enabled again?
>
The code you're patching here was part of Laurent's commit cd8843ff25
("mos6522: fix T1 and T2 timers"). You've mentioned that elsewhere in this
thread. My response is the same as before: this patch series removes that
code so it's moot.
Please test the patch series I sent, unmodified. If you find a problem
with my code, please do report it here. I believe that you will see no
hangs at all.
next prev parent reply other threads:[~2021-09-11 0:10 UTC|newest]
Thread overview: 47+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-08-24 10:09 [RFC 00/10] hw/mos6522: VIA timer emulation fixes and improvements Finn Thain
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 [this message]
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=d06666-ce8a-88d5-e47e-d2439241a74@linux-m68k.org \
--to=fthain@linux-m68k.org \
--cc=david@gibson.dropbear.id.au \
--cc=f4bug@amsat.org \
--cc=groug@kaod.org \
--cc=mark.cave-ayland@ilande.co.uk \
--cc=peter.maydell@linaro.org \
--cc=qemu-devel@nongnu.org \
--cc=qemu-ppc@nongnu.org \
/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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).