On Sun, Aug 20, 2023 at 10:39:46PM -0500, Scott Cheloha wrote:
> pOn amd64 we lie about the interrupts established during
> i8254_initclocks(). We claim they are MP-safe in order to mollify a
> KASSERT in intr_establish() and continue booting.
>
> See amd64/isa/clock.c:
> 279 void
> 280 i8254_initclocks(void)
> 281 {
> 282 i8254_inittimecounter(); /* hook the interrupt-based
> i8254 tc */
> 283
> 284 stathz = 128;
> 285 profhz = 1024; /* XXX does not divide into 1 billion
> */
> 286 clockintr_init(0);
> 287
> 288 clockintr_cpu_init(NULL);
> 289
> 290 /*
> 291 * While the clock interrupt handler isn't really MPSAFE, the
> 292 * i8254 can't really be used as a clock on a true MP system.
> 293 */
> 294 isa_intr_establish(NULL, 0, IST_PULSE, IPL_CLOCK | IPL_MPSAFE,
> 295 clockintr, 0, "clock");
> 296 isa_intr_establish(NULL, 8, IST_PULSE, IPL_STATCLOCK |
> IPL_MPSAFE,
> 297 rtcintr, 0, "rtc");
>
> and amd64/amd64/intr.c:
>
> 332 void *
> 333 intr_establish(int legacy_irq, struct pic *pic, int pin, int type,
> int level,
> 334 struct cpu_info *ci, int (*handler)(void *), void *arg, const
> char *what)
> 335 {
> 336 struct intrhand **p, *q, *ih;
> 337 int slot, error, idt_vec;
> 338 struct intrsource *source;
> 339 struct intrstub *stubp;
> 340 int flags;
> 341
> 342 #ifdef DIAGNOSTIC
> 343 if (legacy_irq != -1 && (legacy_irq < 0 || legacy_irq > 15))
> 344 panic("intr_establish: bad legacy IRQ value");
> 345
> 346 if (legacy_irq == -1 && pic == &i8259_pic)
> 347 panic("intr_establish: non-legacy IRQ on i8259");
> 348 #endif
> 349
> 350 flags = level & IPL_MPSAFE;
> 351 level &= ~IPL_MPSAFE;
> 352
> 353 KASSERT(level <= IPL_TTY || level >= IPL_CLOCK || flags &
> IPL_MPSAFE);
>
> Can we do the same on i386? I'm trying to test the i8254 path on
> modern hardware and I'm tripping the equivalent KASSERT in
> apic_intr_establish().
>
> See i386/i386/ioapic.c:
>
> 661 void *
> 662 apic_intr_establish(int irq, int type, int level, int (*ih_fun)(void
> *),
> 663 void *ih_arg, const char *ih_what)
> 664 {
> 665 unsigned int ioapic = APIC_IRQ_APIC(irq);
> 666 unsigned int intr = APIC_IRQ_PIN(irq);
> 667 struct ioapic_softc *sc = ioapic_find(ioapic);
> 668 struct ioapic_pin *pin;
> 669 struct intrhand **p, *q, *ih;
> 670 extern int cold;
> 671 int minlevel, maxlevel;
> 672 extern void intr_calculatemasks(void); /* XXX */
> 673 int flags;
> 674
> 675 flags = level & IPL_MPSAFE;
> 676 level &= ~IPL_MPSAFE;
> 677
> 678 KASSERT(level <= IPL_TTY || flags & IPL_MPSAFE);
>
> The patch below lets me test the i8254 clockintr path on modern
> hardware in 32-bit mode without needing to rototill the GENERIC
> config to delete all the things that implicitly depend upon the
> ioapic.
>
> I don't think lying in this case is harmful. We can only get to
> i8254_initclocks() if we have no local APIC, or if
> lapic_calibrate_timer() fails.
>
> ok?
>
> Index: clock.c
> ===================================================================
> RCS file: /cvs/src/sys/arch/i386/isa/clock.c,v
> retrieving revision 1.65
> diff -u -p -r1.65 clock.c
> --- clock.c 25 Jul 2023 18:16:20 -0000 1.65
> +++ clock.c 21 Aug 2023 03:26:39 -0000
> @@ -431,9 +431,9 @@ i8254_initclocks(void)
> clockintr_cpu_init(NULL);
>
> /* When using i8254 for clock, we also use the rtc for profclock */
> - (void)isa_intr_establish(NULL, 0, IST_PULSE, IPL_CLOCK,
> + (void)isa_intr_establish(NULL, 0, IST_PULSE, IPL_CLOCK | IPL_MPSAFE,
> clockintr, 0, "clock");
> - (void)isa_intr_establish(NULL, 8, IST_PULSE, IPL_STATCLOCK,
> + (void)isa_intr_establish(NULL, 8, IST_PULSE, IPL_STATCLOCK | IPL_MPSAFE,
> rtcintr, 0, "rtc");
>
> rtcstart(); /* start the mc146818 clock */
I think this is fine. I tried to come up with a scenario where you'd be doing
smp i386 without a local apic and even the ancient 82489 (for 80486 systems)
acted as a lapic. And since we don't run on real 80386 anymore, I think we can
ignore someone trying to do smp there.
ok mlarkin if it makes your work easier.
