Subject: macppc softint handling, also Does PPP work on MacPPC? Yes!
To: None <port-macppc@netbsd.org>
From: M L Riechers <mlr@rse.com>
List: port-macppc
Date: 01/02/2001 14:16:18
Ben Harris <bjh21@netbsd.org>,
From: Bill Sommerfeld <sommerfeld@orchard.arlington.ma.us>,
Subject: macppc softint handling, also Does PPP work on MacPPC? Yes!
Mon Jan  1 08:21:54 2001, Ben Harris sez

>>The console on /dev/tty00 seems to work, but if you mess with it, by
>>trying to change the speed or hooking up a modem, it stops working.
>
>Hmm.  I get that kind of behaviour too.  I'm using a serial console on an
>Apus 2000, and it stops working if I turn the terminal off, and recovers on
>reboot.  I haven't had a chance to look closely at it though.

I've noticed that things like keyboard ctl-C and ctl-Z don't work very
well: they don't return the shell prompt without banging on the keyboard.

On Mon Jan  1 11:12:03 2001 der Mouse sez:

>...
>I *suspect* the delay is "ipending &= pcpl" four lines from the end of
>do_pending_int (arch/macppc/macppc/extintr.c).  The reason is:
>
>...
>
>It looks to me as though there is similar risk for other soft
>interrupts.  I suspect do_pending_int() needs a bit of a rethink.
>
>                                        der Mouse

First Class analysis! Really Good Work. Thanks much. Methinks you've
hit on the reason for my "slow keyboard."

On Mon, 1 Jan 2001 14:38:59 -0600, Donald Lee responds:

>...
>
>On the nose!  Cool.  Many thanks to der Mouse.  I owe him some cheese. ;->
>
>I made the following change - it's not "right", but it solves my problem,
>and seems not to have adverse effect.
>
>(sys/arch/macppc/macppc/extintr.c
>/*      $NetBSD: extintr.c,v 1.12.4.1 2000/11/01 16:26:07 tv Exp $      */)
>
>charm$ diff -c extintr.c ~/extintr.c
>*** extintr.c   Wed Nov  1 10:26:07 2000
>--- /home/user/donlee/extintr.c Mon Jan  1 14:32:50 2001
>***************
>*** 620,625 ****
>--- 620,626 ----
>        int hwpend;
>        int emsr, dmsr;
>        static int processing;
>+       int tryit;
>
>        if (processing)
>                return;
>***************
>*** 667,673 ****
>                softserial();
>                intrcnt[CNT_SOFTSERIAL]++;
>        }
>!       ipending &= pcpl;
>        cpl = pcpl;     /* Don't use splx... we are here already! */
>        asm volatile("mtmsr %0" :: "r"(emsr));
>        processing = 0;
>--- 668,675 ----
>                softserial();
>                intrcnt[CNT_SOFTSERIAL]++;
>        }
>!       tryit = (1 << SIR_CLOCK) | (1 << SIR_NET) | (1 << SIR_SERIAL);
>!       ipending &= (pcpl | tryit);
>        cpl = pcpl;     /* Don't use splx... we are here already! */
>        asm volatile("mtmsr %0" :: "r"(emsr));
>        processing = 0;
>
>ping times are now around 16 ms, which is about what I expect.
>
>Life is good.
>
>-dgl-

This preserves the soft clocks, nets, and serials soft interrupts, but
still clobbers any other recently made up pending interrupts. as

Mon, 1 Jan 2001 17:36:20 -0500 (EST) der Mouse rejoins:

>> ping times are now around 16 ms, which is about what I expect.
>
>Most excellent.  Glad I could help.
>
>> P.S. I suspect that this may be similar to my problem with the "lost
>> interrupts" in the Cyclades serial driver, but I don't understand the
>> code enough to play with it....
>
>On reflection, I think you may well be right.  Any interrupt (including
>soft "interrupts" like setsoftnet()) that occurs while do_pending_int()
>is running will get lost, as I read it, unless that interrupt was
>blocked when do_pending_int() got called - which usually won't be the
>case.  Or, of course, unless the interrupting hardware persists in
>banging on the CPU's interrupt line until it gets acknowledged - does
>your Cyclades hardware do that?
>
>                                        der Mouse
>
>                               mouse@rodents.montreal.qc.ca
>                     7D C8 61 52 5D E7 2D 39  4E F1 31 3E E8 B3 27 4B

I agree with your suspicions.  But what is all this deferred interrupt
falderal, anyway?

Then on Mon, 01 Jan 2001 12:07:35 -0500 Bill Sommerfeld interposes:

> Someone asked me to look at the macppc softint code.
> 
> der Mouse's evaluation is correct; the
>         ipending &= pcpl;
> 
> at the end of do_pending_int() is bogus.

Totally agree.  My rule is that I make a local copy of the flags, then
"clear the slate," then process the flags.  That way any new flags set
will be processed at some point.  The possible error is to visit an
interrupt routine unnecessarily (i.e. we clear the flags, something
interrupts and sets a flag again, we process the interrupt routine
because the flag was set, clearing both the original and the new
causes of the interrupt, and then the interrupt routing is re-visited
later with nothing to do, because the interrupt pending flag was set,
but not reset.) But this is trivial, and should be trivial, because
_all_ interrupt routines should protect themselves from null calls.

> there  since any set ipending bits
> are cleared before the handler runs; if code called from the
> softserial() handler sets SIR_NET, you can lose softnet interrupts.
> If your only network interface is ppp, nothing else is likely to
> generate them.
> 
> some suggestions: 
> 
> 1) clear bits out of ipending before the handlers are invoked:
> 
> ...
> 
> 2) there should be a similar loop around the softint handlers; you
> could reorder the invocation of the handlers (softserial first, then
> softnet) as an "optimization" but for correctness purposes you're
> better off assuming that any softint routine could post any other
> softint.
> 
> 3) since both software and hardware interrupts use "ipending", don't
> bother with hwpend and HWIRQ_MASK; put the softint handler into the
> intrhand[] tables, and clear bits out of ipending just before invoking
> the handlers...
> 
>                                         - Bill

Yes, good points, but, except for the soft interrupts, (clock, net,
and serial), I say again, what is all this deferred interrupt
falderal, anyway?

Anyway, here's a quick hack.  It's a bit paranoid, but I've tried it,
and at the least it seems to "do no harm" (tm).  It seems to quicken
the xterm response over our local ethernet.  I'll have to wait until I
get home to see for sure if it solves _my_ problem (most noticeable
when I'm logged into nnwest over a modem, and telnet'ed to our ppc mac
7500).

--- extintr.c.ori       Thu Nov  2 06:29:55 2000
+++ extintr.c   Mon Jan  1 21:34:57 2001
@@ -614,26 +614,30 @@
 void
 do_pending_int()
 {
        struct intrhand *ih;
        int irq;
        int pcpl;
-       int hwpend;
+       int donowpends, hwpend;
        int emsr, dmsr;
        static int processing;
 
        if (processing)
                return;
 
        processing = 1;
        asm volatile("mfmsr %0" : "=r"(emsr));
        dmsr = emsr & ~PSL_EE;
        asm volatile("mtmsr %0" :: "r"(dmsr));
 
        pcpl = splhigh();               /* Turn off all */
+       donowpends =
        hwpend = ipending & ~pcpl;      /* Do now unmasked pendings */
+                                       /* Wipe the slate clean for future */
+                                       /* pendings: */
+       ipending &= pcpl;               /* Turn off all pendings we'll do. */
        if (!have_openpic) {
                imen &= ~hwpend;
                enable_irq(~imen);
        }
        hwpend &= HWIRQ_MASK;
        while (hwpend) {
@@ -649,28 +653,28 @@
                if (have_openpic)
                        openpic_enable_irq(hwirq[irq], intrtype[irq]);
        }
 
        /*out32rb(INT_ENABLE_REG, ~imen);*/
 
-       if ((ipending & ~pcpl) & (1 << SIR_CLOCK)) {
+                                       /* What the hell, be paranoid: */
+       if ((donowpends | (ipending & ~pcpl)) & (1 << SIR_CLOCK)) {
                ipending &= ~(1 << SIR_CLOCK);
                softclock();
                intrcnt[CNT_SOFTCLOCK]++;
        }
-       if ((ipending & ~pcpl) & (1 << SIR_NET)) {
+       if ((donowpends | (ipending & ~pcpl)) & (1 << SIR_NET)) {
                ipending &= ~(1 << SIR_NET);
                softnet();
                intrcnt[CNT_SOFTNET]++;
        }
-       if ((ipending & ~pcpl) & (1 << SIR_SERIAL)) {
+       if ((donowpends | (ipending & ~pcpl)) & (1 << SIR_SERIAL)) {
                ipending &= ~(1 << SIR_SERIAL);
                softserial();
                intrcnt[CNT_SOFTSERIAL]++;
        }
-       ipending &= pcpl;
        cpl = pcpl;     /* Don't use splx... we are here already! */
        asm volatile("mtmsr %0" :: "r"(emsr));
        processing = 0;
 }

-Mike