Messages in this thread | | | Subject | Re: [RESEND PATCH 3/3] x86/apic: Clean up the names of legacy irq mode setting related functions | From | Dou Liyang <> | Date | Fri, 19 Jan 2018 16:06:57 +0800 |
| |
At 01/19/2018 03:21 PM, Baoquan He wrote: > On 01/19/18 at 02:42pm, Dou Liyang wrote: >> Hi Baoquan, >> >> At 01/05/2018 12:39 PM, Baoquan He wrote: >> [...] >>> /* >>> - * Not an __init, needed by kexec/kdump code. >>> - * For safety IO-APIC and Local APIC need be cleared before this. >>> + * In legacy irq mode, full DOS compatibility with the uniprocessor PC/AT is >>> + * provided by using the APICs in conjunction with standard 8259A-equivalent >>> + * programmable interrupt controllers (PICs). It's necessary to deliver legacy >>> + * interrupts even when APIC mode is not enabled. This is required by kexec/ >>> + * kdump before enter into the 2nd kernel. >>> */ >>> void switch_to_legacy_irq_mode(void) >>> { >>> if (!nr_legacy_irqs()) >>> return; >>> - x86_io_apic_ops.disable(); >>> + ioapic_set_virtual_wire_mode(); >>> + >>> + if (boot_cpu_has(X86_FEATURE_APIC) || apic_from_smp_config()) >>> + lapic_set_legacy_irq_mode(ioapic_i8259.pin != -1); >> >> Seems these two function, ioapic/lapic_set_legacy_irq_mode should be >> exclusive. > > Thanks for looking into this, dou! > > It might be not exclusive. You can see mp_spec 3.6.2.2 Virtual Wire Mode
Oops! Sorry to confuse you, here what I want to say is that the code make me think that we set through IO-APIC first, then set through Local-APIC then. But, we can be only in one mode of them.
Just worry about this code seems ignore the irq remapping situation.
We call switch_to_legacy_irq_mode() directly in machine_kexec(), why we also set x86_io_apic_ops:
.disable = switch_to_legacy_irq_mode
> subsection, there are two kinds of virtual wire mode, one is > 8259A-Equivalent pics is connected to lint0 of boot cpu LAPIC, the other > is 8259A-Equivalent pics go through IO-APIC, then is connected to lint0 > of LAPIC. Whatever it is, LAPIC need be set as through-lapic. >
Yes, Absolutely right!
> Above is what I got from mp_spec. But from function > native_disable_io_apic() and disconnect_bsp_APIC(), the code seems to be > telling that if io-apic is connected to 8259A-Equivalent pics, we need > mask lvt0 of LAPIC. This conflicts with mp_spec 3.6.2.2. >
I think so.
Thanks, dou. > Thanks > Baoquan >> >> But We do that because both the through-lapic and through-ioapic virtual >> wire mode need setup the APIC_SPIV_APIC_ENABLED which is only located in >> the lapic_set_legacy_irq_mode(). So we need call them both. >> >> IMO, this cleanup may not make it clear. we can separate these two mode >> totally or just keep it like before. >> >> Thanks, >> dou. >>> } >>> #ifdef CONFIG_X86_32 >>> diff --git a/arch/x86/kernel/x86_init.c b/arch/x86/kernel/x86_init.c >>> index 1151ccd72ce9..c30f0f273dbd 100644 >>> --- a/arch/x86/kernel/x86_init.c >>> +++ b/arch/x86/kernel/x86_init.c >>> @@ -148,5 +148,5 @@ void arch_restore_msi_irqs(struct pci_dev *dev) >>> struct x86_io_apic_ops x86_io_apic_ops __ro_after_init = { >>> .read = native_io_apic_read, >>> - .disable = native_disable_io_apic, >>> + .disable = switch_to_legacy_irq_mode, >>> }; >>> diff --git a/drivers/iommu/irq_remapping.c b/drivers/iommu/irq_remapping.c >>> index 49721b4e1975..751472ddf536 100644 >>> --- a/drivers/iommu/irq_remapping.c >>> +++ b/drivers/iommu/irq_remapping.c >>> @@ -37,7 +37,7 @@ static void irq_remapping_disable_io_apic(void) >>> * now. >>> */ >>> if (boot_cpu_has(X86_FEATURE_APIC) || apic_from_smp_config()) >>> - disconnect_bsp_APIC(0); >>> + lapic_set_legacy_irq_mode(0); >>> } >>> static void __init irq_remapping_modify_x86_ops(void) >>> >> >> > > >
| |