Comment 145 for bug 2034477

(In reply to Mario Limonciello (AMD) from comment #65)
> #62
>
> It's setting the 8259 vector base to 0x68 and then configuring it to run in
> 8086 mode. You can see some similar code examples here:
>
> https://wiki.osdev.org/8259_PIC#Code_Examples

Thanks this is useful, this does show though that your init sequence seems to be wrong, the proper init sequence is:

        outb_pic(0x11, PIC_MASTER_CMD); /* ICW1: select 8259A-1 init, missing */
        outb_pic(ISA_IRQ_VECTOR(0), PIC_MASTER_IMR); /* your 0x68 */
        outb_pic(0x04, PIC_MASTER_IMR); /* missing */
        outb_pic(0x01, PIC_MASTER_IMR);

But this is all done by init_8259A(), so for the force-init you can
just call init_8259A(false). Except that you need to unlock the i8259A_lock first and relock it after.

This also illustrates why I think the retry is a good idea, that will test if a re-init is sufficient or if the PIC is disabled at some deeper level.

What I believe happens with your patch now is that since you make probe() succeed then either:

1. init_8259A() fixes things up properly, so it would be enough with the quirk to just make probe() succeed; or

2. The PIC really is disabled at some lower level, but Linux re-routes the IRQs to the IOAPIC anyway so this does not really matter. This is basically what my hack does, add mappings for the Legacy IRQs to make them work and then rely on the IOAPIC to handle them.

> I think we need the author of this commit to confirm. When we agree on a
> patch we need them in the "To" line.
> e179f6914152 ("x86, irq, pic: Probe for legacy PIC and set legacy_pic
> appropriately")

Ack.

> > This is non AMD specific and should be harmless in the case where there
> > really is no PIC:
>
> So the problem with this approach is that it's attempting to program a
> vector base that might not work for all systems. It /happens/ to work on
> these Lenovo ones, but I don't know about others.

See above, the kernel resets the vector base to its own value in
init_8259A(). But since it re-inits anyways the only thing which
we really need to do is make probe_8259A() not set the NULL PIC,
although I would prefer to make probe_8259A() call init_8259A()
when we hit the bug and then have it retry the probe.