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.
