Re: powerpc, macppc: retrigger deferred DEC interrupts from splx(9)
On Thu, Jun 23, 2022 at 09:58:48PM -0500, Scott Cheloha wrote:
>
> [...]
>
> Thoughts? Tweaks?
>
> [...]
miod: Any issues?
kettenis: Anything to add? ok?
drahn: Anything to add? ok?
--
It would be nice (but not strictly necessary) to test this on a
machine doing "real work".
Who does the macppc package builds?
Index: macppc/macppc/clock.c
===
RCS file: /cvs/src/sys/arch/macppc/macppc/clock.c,v
retrieving revision 1.48
diff -u -p -r1.48 clock.c
--- macppc/macppc/clock.c 23 Feb 2021 04:44:30 - 1.48
+++ macppc/macppc/clock.c 24 Jun 2022 02:49:58 -
@@ -128,6 +128,20 @@ decr_intr(struct clockframe *frame)
return;
/*
+* We can't actually mask DEC interrupts, i.e. mask MSR(EE),
+* at or above IPL_CLOCK without masking other essential
+* interrupts. To simulate masking, we retrigger the DEC
+* by hand from splx(9) the next time our IPL drops below
+* IPL_CLOCK.
+*/
+ if (ci->ci_cpl >= IPL_CLOCK) {
+ ci->ci_dec_deferred = 1;
+ ppc_mtdec(UINT32_MAX >> 1); /* clear DEC exception */
+ return;
+ }
+ ci->ci_dec_deferred = 0;
+
+ /*
* Based on the actual time delay since the last decrementer reload,
* we arrange for earlier interrupt next time.
*/
@@ -160,39 +174,35 @@ decr_intr(struct clockframe *frame)
*/
ppc_mtdec(nextevent - tb);
- if (ci->ci_cpl >= IPL_CLOCK) {
- ci->ci_statspending += nstats;
- } else {
- nstats += ci->ci_statspending;
- ci->ci_statspending = 0;
-
- s = splclock();
-
- /*
-* Reenable interrupts
-*/
- ppc_intr_enable(1);
-
- /*
-* Do standard timer interrupt stuff.
-*/
- while (ci->ci_lasttb < ci->ci_prevtb) {
- /* sync lasttb with hardclock */
- ci->ci_lasttb += ticks_per_intr;
- clk_count.ec_count++;
- hardclock(frame);
- }
-
- while (nstats-- > 0)
- statclock(frame);
-
- splx(s);
- (void) ppc_intr_disable();
-
- /* if a tick has occurred while dealing with these,
-* dont service it now, delay until the next tick.
-*/
+ nstats += ci->ci_statspending;
+ ci->ci_statspending = 0;
+
+ s = splclock();
+
+ /*
+* Reenable interrupts
+*/
+ ppc_intr_enable(1);
+
+ /*
+* Do standard timer interrupt stuff.
+*/
+ while (ci->ci_lasttb < ci->ci_prevtb) {
+ /* sync lasttb with hardclock */
+ ci->ci_lasttb += ticks_per_intr;
+ clk_count.ec_count++;
+ hardclock(frame);
}
+
+ while (nstats-- > 0)
+ statclock(frame);
+
+ splx(s);
+ (void) ppc_intr_disable();
+
+ /* if a tick has occurred while dealing with these,
+* dont service it now, delay until the next tick.
+*/
}
void cpu_startclock(void);
Index: macppc/dev/openpic.c
===
RCS file: /cvs/src/sys/arch/macppc/dev/openpic.c,v
retrieving revision 1.89
diff -u -p -r1.89 openpic.c
--- macppc/dev/openpic.c21 Feb 2022 10:38:50 - 1.89
+++ macppc/dev/openpic.c24 Jun 2022 02:49:59 -
@@ -382,6 +382,10 @@ openpic_splx(int newcpl)
intr = ppc_intr_disable();
openpic_setipl(newcpl);
+ if (ci->ci_dec_deferred && newcpl < IPL_CLOCK) {
+ ppc_mtdec(0);
+ ppc_mtdec(UINT32_MAX); /* raise DEC exception */
+ }
if (newcpl < IPL_SOFTTTY && (ci->ci_ipending & ppc_smask[newcpl])) {
s = splsofttty();
dosoftint(newcpl);
Index: macppc/dev/macintr.c
===
RCS file: /cvs/src/sys/arch/macppc/dev/macintr.c,v
retrieving revision 1.56
diff -u -p -r1.56 macintr.c
--- macppc/dev/macintr.c13 Mar 2022 12:33:01 - 1.56
+++ macppc/dev/macintr.c24 Jun 2022 02:49:59 -
@@ -170,6 +170,10 @@ macintr_splx(int newcpl)
intr = ppc_intr_disable();
macintr_setipl(newcpl);
+ if (ci->ci_dec_deferred && newcpl < IPL_CLOCK) {
+ ppc_mtdec(0);
+ ppc_mtdec(UINT32_MAX); /* raise DEC exception */
+ }
if ((newcpl < IPL_SOFTTTY && ci->ci_ipending & ppc_smask[newcpl])) {
s = splsofttty();
dosoftint(newcpl);
Index: powerpc/powerpc/intr.c
===
RCS file: /cvs/src/sys/arch/powerpc/powerpc/intr.c,v
retrieving revision 1.9
Re: powerpc, macppc: retrigger deferred DEC interrupts from splx(9)
On Wed, 29 Jun 2022 22:47:19 -0500
Scott Cheloha wrote:
> To be perfectly clear, you are concerned about this scenario:
>
> > > + if (ci->ci_dec_deferred && newcpl < IPL_CLOCK) {
> > > + ppc_mtdec(0);
>
> /* DEC interrupt fires *here*. */
> /* We jump to decrint() and then call decr_intr(). */
>
> > > + ppc_mtdec(UINT32_MAX); /* raise DEC exception */
> > > + }
>
> I think it's possible for the DEC exception to occur in that spot.
> However, external/DEC *interrupts* are explicitly disabled, so I don't
> think that we will jump to decrint() until the next time we do
>
> ppc_intr_enable(1);
I missed the ppc_intr_disable(), which disables PSL_EE, in
macintr_splx and openpic_splx. You are correct, it can't call
decr_intr until ppc_intr_enable(1).
ppc_dflt_splx also looks good, because we don't enable PSL_EE until
we switch to macintr_splx or openpic_splx.
> > Would this be better?
> >
> > ppc_mtdec(1 >> UINT32_MAX);
> > ppc_mtdec(UINT32_MAX);
>
> I assume you meant to type
>
> ppc_mtdec(UINT32_MAX >> 1);
Yes, I meant UINT32_MAX >> 1, but you have persuaded me that the
existing ppc_mtdec(0) is correct, and no change is necessary. I
will continue running your diff with ppc_mtdec(0).
Re: powerpc, macppc: retrigger deferred DEC interrupts from splx(9)
> We only run on New World Macs, and the only ones without openpic(4) > might be the oldest models of iMac G3 from 1998; these would attach > macintr0 and not openpic0 in dmesg. I don't know anyone who might > have such an iMac. The iMac model PowerMac2,1 from 1999 (with the > (slot-loading cd drive) does have openpic(4). This diff appears to work on PowerMac1,1 using macintr0 (dmesg below). vmstat -i reports 99 clock and stat interrupts per second, ntpd does not complain about clock drift so far. [ using 1319132 bytes of bsd ELF symbol table ] console out [ATY,Rage128y] console in [keyboard]USB and ADB found, using USB : memaddr 8400, size 400 : consaddr 8400 : ioaddr 80b2, size 2: width 640 linebytes 640 height 480 depth 8 Copyright (c) 1982, 1986, 1989, 1991, 1993 The Regents of the University of California. All rights reserved. Copyright (c) 1995-2022 OpenBSD. All rights reserved. https://www.OpenBSD.org OpenBSD 7.1-current (GENERIC) #1: Thu Jun 30 13:45:19 GMT 2022 m...@allanche.gentiane.org:/usr/src/sys/arch/macppc/compile/GENERIC real mem = 268435456 (256MB) avail mem = 244858880 (233MB) random: good seed from bootblocks mpath0 at root scsibus0 at mpath0: 256 targets mainbus0 at root: model PowerMac1,1 cpu0 at mainbus0: 750 (Revision 0x202): 400 MHz: 1MB backside cache mem0 at mainbus0 mpcpcibr0 at mainbus0 pci: grackle pci0 at mpcpcibr0 bus 0 ppb0 at pci0 dev 13 function 0 "DEC 21154" rev 0x02 pci1 at ppb0 bus 1 macobio0 at pci1 dev 5 function 0 "Apple Paddington" rev 0x00 macintr0 at macobio0 offset 0x10 "scsi" at macobio0 offset 0x1 not configured "escc-legacy" at macobio0 offset 0x12000 not configured zs0 at macobio0 offset 0x13000: irq 15,16 zstty0 at zs0 channel 0 zstty1 at zs0 channel 1 awacs0 at macobio0 offset 0x14000: irq 17,8,9 headphones audio0 at awacs0 "power-mgt" at macobio0 offset 0x0 not configured "fdc" at macobio0 offset 0x15000 not configured adb0 at macobio0 offset 0x16000: irq 18, via-cuda, 0 targets wdc0 at macobio0 offset 0x2 irq 13: DMA atapiscsi0 at wdc0 channel 0 drive 0 scsibus1 at atapiscsi0: 2 targets cd0 at scsibus1 targ 0 lun 0: removable cd0(wdc0:0:0): using BIOS timings, DMA mode 2 bm0 at macobio0 offset 0x11000 irq 42,33: address 00:50:e4:fa:e4:72 lxtphy0 at bm0 phy 0: LXT970 10/100 PHY, rev. 1 "nvram" at macobio0 offset 0x6 not configured "TI TSB12LV21 FireWire" rev 0x02 at pci1 dev 0 function 0 not configured pciide0 at pci1 dev 1 function 0 "CMD Technology PCI0646" rev 0x07: DMA, channel 0 configured to native-PCI, channel 1 configured to native-PCI pciide0: using irq 26 for native-PCI interrupt wd0 at pciide0 channel 0 drive 0: wd0: 16-sector PIO, LBA, 12427MB, 25450992 sectors wd0(pciide0:0:0): using PIO mode 4, Ultra-DMA mode 2 pciide0: channel 1 ignored (disabled) fxp0 at pci1 dev 4 function 0 "Intel 8255x" rev 0x02, i82557: irq 25, address 00:a0:c9:ab:37:29 inphy0 at fxp0 phy 1: i82555 10/100 PHY, rev. 0 ohci0 at pci1 dev 6 function 0 "Opti 82C861" rev 0x10: irq 28, version 1.0, legacy support usb0 at ohci0: USB revision 1.0 uhub0 at usb0 configuration 1 interface 0 "Opti OHCI root hub" rev 1.00/1.00 addr 1 vgafb0 at pci0 dev 16 function 0 "ATI Rage 128" rev 0x00, mmio wsdisplay0 at vgafb0 mux 1: console (std, vt100 emulation) wsdisplay0: screen 1-5 added (std, vt100 emulation) uhub1 at uhub0 port 1 configuration 1 interface 0 "Mitsumi Electric Hub in Apple Extended USB Keyboard" rev 1.10/4.10 addr 2 uhidev0 at uhub1 port 3 configuration 1 interface 0 "Mitsumi Electric Apple Extended USB Keyboard" rev 1.10/4.10 addr 3 uhidev0: iclass 3/1 ukbd0 at uhidev0: 8 variable keys, 6 key codes, country code 13 wskbd0 at ukbd0: console keyboard, using wsdisplay0 uhidev1 at uhub1 port 3 configuration 1 interface 1 "Mitsumi Electric Apple Extended USB Keyboard" rev 1.10/4.10 addr 3 uhidev1: iclass 3/0, 3 report ids uhid0 at uhidev1 reportid 2: input=1, output=0, feature=0 ucc0 at uhidev1 reportid 3: 4 usages, 4 keys, enum wskbd1 at ucc0 mux 1 wskbd1: connecting to wsdisplay0 vscsi0 at root scsibus2 at vscsi0: 256 targets softraid0 at root scsibus3 at softraid0: 256 targets bootpath: /pci/@d/pci-ata@1/ata-4@0/disk@0:/bsd root on wd0a (410f22971b6a6734.a) swap on wd0b dump on wd0b
Re: powerpc, macppc: retrigger deferred DEC interrupts from splx(9)
On Wed, Jun 29, 2022 at 10:34:53PM -0400, George Koehler wrote:
> Hi. I have a question about splx, below.
>
> On Thu, 23 Jun 2022 21:58:48 -0500
> Scott Cheloha wrote:
>
> > My machine uses openpic(4). I would appreciate tests on a
> > non-openpic(4) Mac, though all tests are appreciated.
>
> We only run on New World Macs, and the only ones without openpic(4)
> might be the oldest models of iMac G3 from 1998; these would attach
> macintr0 and not openpic0 in dmesg. I don't know anyone who might
> have such an iMac. The iMac model PowerMac2,1 from 1999 (with the
> (slot-loading cd drive) does have openpic(4).
If it's imperative we test it on a non-openpic(4) machine I might be
able to scrounge one on craigslist.
... they can't be completely extinct, right?
> > Index: macppc/dev/openpic.c
> > ===
> > RCS file: /cvs/src/sys/arch/macppc/dev/openpic.c,v
> > retrieving revision 1.89
> > diff -u -p -r1.89 openpic.c
> > --- macppc/dev/openpic.c21 Feb 2022 10:38:50 - 1.89
> > +++ macppc/dev/openpic.c24 Jun 2022 02:49:59 -
> > @@ -382,6 +382,10 @@ openpic_splx(int newcpl)
> >
> > intr = ppc_intr_disable();
> > openpic_setipl(newcpl);
> > + if (ci->ci_dec_deferred && newcpl < IPL_CLOCK) {
> > + ppc_mtdec(0);
> > + ppc_mtdec(UINT32_MAX); /* raise DEC exception */
> > + }
> > if (newcpl < IPL_SOFTTTY && (ci->ci_ipending & ppc_smask[newcpl])) {
> > s = splsofttty();
> > dosoftint(newcpl);
>
> The 2nd mtdec tries to raise dec_intr by changing bit 1 << 31 of the
> decrementer register from 0 to 1.
Yes, exactly. My read of PowerPC 2.01 is that the DEC exception
is raised when the DEC's MSB goes from 0 to 1.
> I suspect if the decrementer can
> also decrement itself from 0 to UINT32_MAX, and raise dec_intr early,
> before we reach the 2nd mtdec. This would be bad, because this
> ppc_mtdec(UINT32_MAX) would override the ppc_mtdec(nextevent - tb) in
> dec_intr and lose the next scheduled clock interrupt.
To be perfectly clear, you are concerned about this scenario:
> > + if (ci->ci_dec_deferred && newcpl < IPL_CLOCK) {
> > + ppc_mtdec(0);
/* DEC interrupt fires *here*. */
/* We jump to decrint() and then call decr_intr(). */
> > + ppc_mtdec(UINT32_MAX); /* raise DEC exception */
> > + }
I think it's possible for the DEC exception to occur in that spot.
However, external/DEC *interrupts* are explicitly disabled, so I don't
think that we will jump to decrint() until the next time we do
ppc_intr_enable(1);
That first happens in dosoftint(). If we don't call dosoftint(), it
may happen at the end of splx(), provided that interrupts weren't
already disabled when we called splx().
> Testing might miss this problem. For example, a randomly reordered
> kernel might place the 2 mtdec instructions in different pages, which
> has a small chance of a page fault on a Mac G5.
>
> Would this be better?
>
> ppc_mtdec(1 >> UINT32_MAX);
> ppc_mtdec(UINT32_MAX);
I assume you meant to type
ppc_mtdec(UINT32_MAX >> 1);
I will tweak the code and try this out. My reading of PowerPC 2.01
suggests that this will do the job just fine.
But again, I'm unsure whether we need this. External and DEC
interrupts should be masked when we run this code unless I'm
misunderstanding what ppc_intr_disable() actually does.
Re: powerpc, macppc: retrigger deferred DEC interrupts from splx(9)
Hi. I have a question about splx, below.
On Thu, 23 Jun 2022 21:58:48 -0500
Scott Cheloha wrote:
> My machine uses openpic(4). I would appreciate tests on a
> non-openpic(4) Mac, though all tests are appreciated.
We only run on New World Macs, and the only ones without openpic(4)
might be the oldest models of iMac G3 from 1998; these would attach
macintr0 and not openpic0 in dmesg. I don't know anyone who might
have such an iMac. The iMac model PowerMac2,1 from 1999 (with the
(slot-loading cd drive) does have openpic(4).
> Index: macppc/dev/openpic.c
> ===
> RCS file: /cvs/src/sys/arch/macppc/dev/openpic.c,v
> retrieving revision 1.89
> diff -u -p -r1.89 openpic.c
> --- macppc/dev/openpic.c 21 Feb 2022 10:38:50 - 1.89
> +++ macppc/dev/openpic.c 24 Jun 2022 02:49:59 -
> @@ -382,6 +382,10 @@ openpic_splx(int newcpl)
>
> intr = ppc_intr_disable();
> openpic_setipl(newcpl);
> + if (ci->ci_dec_deferred && newcpl < IPL_CLOCK) {
> + ppc_mtdec(0);
> + ppc_mtdec(UINT32_MAX); /* raise DEC exception */
> + }
> if (newcpl < IPL_SOFTTTY && (ci->ci_ipending & ppc_smask[newcpl])) {
> s = splsofttty();
> dosoftint(newcpl);
The 2nd mtdec tries to raise dec_intr by changing bit 1 << 31 of the
decrementer register from 0 to 1. I suspect if the decrementer can
also decrement itself from 0 to UINT32_MAX, and raise dec_intr early,
before we reach the 2nd mtdec. This would be bad, because this
ppc_mtdec(UINT32_MAX) would override the ppc_mtdec(nextevent - tb) in
dec_intr and lose the next scheduled clock interrupt.
Testing might miss this problem. For example, a randomly reordered
kernel might place the 2 mtdec instructions in different pages, which
has a small chance of a page fault on a Mac G5.
Would this be better?
ppc_mtdec(1 >> UINT32_MAX);
ppc_mtdec(UINT32_MAX);
powerpc, macppc: retrigger deferred DEC interrupts from splx(9)
Hi,
One of the problems obstructing my dynamic clock interrupt patch is
that clock interrupts on powerpc don't (can't?) behave the same as
clock interrupts on amd64, arm64, and sparc64.
In particular, for historical reasons, on powerpc you cannot mask
decrementer (DEC) interrupts without *also* masking other interrupts
that we need to (generally) leave unmasked.
The upshot is that the DEC is unmasked at IPL_CLOCK and IPL_HIGH on
powerpc. It's always running, it can arrive at any time. We work
around the obvious problem this poses by postponing clock interrupt
work to a later tick if a DEC interrupt arrives when the CPU is at
IPL_CLOCK or IPL_HIGH.
This solution is insufficient for a machine-independent clock
interrupt subsystem like the one in my patch.
The only way forward I can see is to instead postpone clock interrupt
work until the next splx(9) call wherein the CPU's IPL is dropping
below IPL_CLOCK.
This patch does that. We need to raise the DEC exception immediately
after we change the IPL, so there is a little bit of code duplicated
across the various splx(9) implementations for powerpc and macppc.
The changes in macppc/clock.c are hopefully straightforward.
This boots on my PowerMac G5 ("PowerMac7,3") and has survived two
`make build` runs and several more kernel builds.
Caveat: this machine is not quite stable. I can't run parallel
userland builds, i.e. `make -j2 build` without eventually hanging the
machine. Also, sometimes it hangs at boot while /etc/netstart is
running. These problems existed before this patch and remain after
the patch is applied.
However, this patch does not seem to have made the machine *more*
unstable, which is a good sign, I think.
My machine uses openpic(4). I would appreciate tests on a
non-openpic(4) Mac, though all tests are appreciated.
Thoughts? Tweaks?
If we merge this change I plan make an equivalent change on powerpc64.
dmesg below, patch attached at the end.
-Scott
[ using 1321372 bytes of bsd ELF symbol table ]
console out [ATY,Whelk_A] console in [keyboard], using USB
using parent ATY,WhelkParent:: memaddr a000, size 1000 : consaddr
a0008000 : ioaddr 9002, size 2: width 1152 linebytes 1280 height 870
depth 8
Copyright (c) 1982, 1986, 1989, 1991, 1993
The Regents of the University of California. All rights reserved.
Copyright (c) 1995-2022 OpenBSD. All rights reserved. https://www.OpenBSD.org
OpenBSD 7.1-current (GENERIC.MP) #4: Thu Jun 23 10:55:15 CDT 2022
ssc@peanut.local:/usr/src/sys/arch/macppc/compile/GENERIC.MP
real mem = 2147483648 (2048MB)
avail mem = 2051514368 (1956MB)
random: good seed from bootblocks
mpath0 at root
scsibus0 at mpath0: 256 targets
mainbus0 at root: model PowerMac7,3
cpu0 at mainbus0: 970FX (Revision 0x300): 1800 MHz
cpu1 at mainbus0: 970FX (Revision 0x300): 1800 MHz
mem0 at mainbus0
spdmem0 at mem0: 1GB DDR SDRAM non-parity PC3200CL3.0
spdmem1 at mem0: 1GB DDR SDRAM non-parity PC3200CL3.0
spdmem2 at mem0: 1GB DDR SDRAM non-parity PC3200CL3.0
spdmem3 at mem0: 1GB DDR SDRAM non-parity PC3200CL3.0
memc0 at mainbus0: u3 rev 0xb3
kiic0 at memc0 offset 0xf8001000
iic0 at kiic0
lmtemp0 at iic0 addr 0x4a: ds1775
maxtmp0 at iic0 addr 0x4c: max6690
maxtmp1 at iic0 addr 0x4e: max6690
"cy28508" at iic0 addr 0x69 not configured
"cy2213" at iic0 addr 0x65 not configured
fcu0 at iic0 addr 0xaf
"pca9556" at iic0 addr 0x18 not configured
adc0 at iic0 addr 0x2c: ad7417
"24256" at iic0 addr 0x50 not configured
"pca9556" at iic0 addr 0x19 not configured
adc1 at iic0 addr 0x2d: ad7417
"24256" at iic0 addr 0x51 not configured
"dart" at memc0 offset 0xf8033000 not configured
"mpic" at memc0 offset 0xf804 not configured
mpcpcibr0 at mainbus0 pci: u3-agp
pci0 at mpcpcibr0 bus 0
pchb0 at pci0 dev 11 function 0 "Apple U3 AGP" rev 0x00
appleagp0 at pchb0
agp0 at appleagp0: aperture at 0x0, size 0x1000
radeondrm0 at pci0 dev 16 function 0 "ATI Radeon 9600" rev 0x00
drm0 at radeondrm0
radeondrm0: irq 48
ht0 at mainbus0: u3-ht, 6 devices
pci1 at ht0 bus 0
hpb0 at pci1 dev 1 function 0 "Apple U3" rev 0x00: 85 sources
pci2 at hpb0 bus 1
macobio0 at pci2 dev 7 function 0 "Apple K2 Macio" rev 0x60
openpic0 at macobio0 offset 0x4: version 0x4614 feature 770302 LE
macgpio0 at macobio0 offset 0x50
"pmu-interrupt" at macgpio0 offset 0x9 not configured
"programmer-switch" at macgpio0 offset 0x11 not configured
"modem-reset" at macgpio0 offset 0x1d not configured
"modem-power" at macgpio0 offset 0x1e not configured
"fcu-interrupt" at macgpio0 offset 0x15 not configured
"fcu-hw-reset" at macgpio0 offset 0x3a not configured
"slewing-done" at macgpio0 offset 0x23 not configured
"codec-input-data-mux" at macgpio0 offset 0xb not configured
"line-input-detect" at macgpio0 offset 0xc not configured
"codec-error-irq" at macgpio0 offset 0xd not configured
"dig-hw-reset" at macgpio0 offset 0x14 not configured
"line-output-detect" at macgpio0 offset 0x16 not configured
"headphone-detect" at macgpio0 offset 0x17 not
