On 2016/07/25 at 11:04, Wei, Jiangang wrote:
> Hi He,
>
> Thanks for your response firstly.
>
> On Fri, 2016-07-22 at 18:40 +0800, Baoquan He wrote:
>> Hi Jiangang,
>>
>> This is very nice, should be the stuff Eric and Ingo would like to see.
>> But I have several questions:
>>
>> 1) Are you not going to clean up the old legacy irq mode setting code in
>> 1st kernel?
> Yes, I would like to pay more attention on fixing kdump's failure with
> notsc.
> No plan to clean up the irq mode setting codes in the crash kernel
> reboot path.
> If you are interested in it, please go on.
>
>> 2)I call them legacy irq mode because not only apic virtual wire mode
>> exists, but the PIC mode in x86 32bit system. You need consider it too.
>> Then init_bsp_APIC need be renamaed to an appropriate one. And assume
>> this has been tested on x86 32bit system.
> Thanks for your reminders.
> As the comment of init_bsp_APIC(), it's just used to setup the virtual
> wire mode.
> so I won't change its name.
>
> But i agree with that PIC mode should be considered.
> Next version will be like below,
>
> diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
> index 04358e0cf1e2..d40bab947a2a 100644
> --- a/arch/x86/kernel/apic/apic.c
> +++ b/arch/x86/kernel/apic/apic.c
> @@ -1186,7 +1186,7 @@ void __init init_bsp_APIC(void)
> * the worst case is that both of them are inactive,
> * If so, We need to enable the virtual wire mode via
> through-local-APIC
> */
> - if (smp_found_config || !boot_cpu_has(X86_FEATURE_APIC)
> + if ( pic_mode || (smp_found_config && check_virtual_wire_mode())
I think this is needless, as init_bsp_APIC() is made under the following condition:
#if defined(CONFIG_X86_64) || defined(CONFIG_X86_LOCAL_APIC)
> ||
> !boot_cpu_has(X86_FEATURE_APIC))
> return;
>> 3)
>>
>> *)About IO-APIC setting as virtual wire mode, I am always confused. In
>> MP Spec 3.6.2.2, it says "the interrupt signal passes through both the
>> I/O APIC and the BSP?s local APIC". That means IO-APIC virtual wire mode
>> need both IO-APIC and LAPIC to be set, and with my understanding only
>> pin of IO-APIC is set as ExtInt, LAPIC should be pass-through.
>>
>> *)But in Intel Arch manual 3A 10.1, there's only below sentences to mention
>> it:
>>
>> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> The I/O APIC also has a ?virtual wire mode? that allows it to communicate
>> with a standard 8259A-style external interrupt controller. Note that the
>> local APIC can be disabled. This allows an associated processor core to
>> receive interrupts directly from an 8259A interrupt controller.
>> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>
>> Eric's code in native_disable_io_apic() has the same point as above
>> words.
> IMO?
> the through-IO-APIC mode has no relationship with the setting of local
> APIC.
> that's why i only check the pin of IO-APIC in virtual_wire_via_ioapic().
For through-IO-APIC mode, I think the bsp local apic should be on, but its LINT0 pin must not
be set to be "ExtINT"(I guess with this mode set will make it to be through-LAPIC mode), and
we can clearly see the fact from disconnect_bsp_APIC()'s implementation.
Regards,
Xunlei
> Thanks
> wei
>> *)However please read code comments in irq_remapping_disable_io_apic(),
>> Joerg's description give me a different impression that we can choose
>> to only use LAPIC virtual wire mode. Joerg is IOMMU maintainers, he is
>> very familiar with io-apic since IOMMU need take over io-apic entry
>> filling, there must be reason he wrote that. Add Joerg to CC list.
>>
>> Seems it's difficult to find a system with IO-APIC virtual wire mode,
>> maybe we can just keep it as is. Not sure if Intel engineers can help
>> explain and confirm this.
>>
>> That's all I can think of.
>>
>> Thanks
>> Baoquan
>>
>> On 07/22/16 at 04:10pm, Wei Jiangang wrote:
>>> If we specify the 'notsc' parameter for the dump-capture kernel,
>>> and then trigger a crash(panic) by using "ALT-SysRq-c" or
>>> "echo c > /proc/sysrq-trigger", the dump-capture kernel will
>>> hang in calibrate_delay_converge() and wait for jiffies changes.
>>> serial log as follows:
>>>
>>> tsc: Fast TSC calibration using PIT
>>> tsc: Detected 2099.947 MHz processor
>>> Calibrating delay loop...
>>>
>>> The reason for jiffies not changes is there's no timer interrupt
>>> passed to dump-capture kernel.
>>>
>>> In fact, once kernel panic occurs, the local APIC is disabled
>>> by lapic_shutdown() in reboot path.
>>> generly speaking, local APIC state can be initialized by BIOS
>>> after Power-Up or Reset, which doesn't apply to kdump case.
>>> so the kernel has to be responsible for initialize the interrupt
>>> mode properly according the latest status of APIC in bootup path.
>>>
>>> An MP operating system is booted under either PIC mode or
>>> virtual wire mode. Later, the operating system switches to
>>> symmetric I/O mode as it enters multiprocessor mode.
>>> Two kinds of virtual wire mode are defined in Intel MP spec:
>>> virtual wire mode via local APIC or via I/O APIC.
>>>
>>> Now we determine the mode of APIC only through a SMP BIOS(MP table).
>>> That's not enough.
>>> It's better to do further check if APIC works with effective mode,
>>> and do some porper setting.
>>>
>>> Signed-off-by: Cao jin <caoj.fnst at cn.fujitsu.com>
>>> Signed-off-by: Wei Jiangang <weijg.fnst at cn.fujitsu.com>
>>> ---
>>> arch/x86/include/asm/io_apic.h | 5 ++++
>>> arch/x86/kernel/apic/apic.c | 55 +++++++++++++++++++++++++++++++++++++++++-
>>> arch/x86/kernel/apic/io_apic.c | 28 +++++++++++++++++++++
>>> 3 files changed, 87 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/x86/include/asm/io_apic.h b/arch/x86/include/asm/io_apic.h
>>> index 6cbf2cfb3f8a..6550bd43fa39 100644
>>> --- a/arch/x86/include/asm/io_apic.h
>>> +++ b/arch/x86/include/asm/io_apic.h
>>> @@ -190,6 +190,7 @@ static inline unsigned int io_apic_read(unsigned int apic, unsigned int reg)
>>> }
>>>
>>> extern void setup_IO_APIC(void);
>>> +extern bool virtual_wire_via_ioapic(void);
>>> extern void enable_IO_APIC(void);
>>> extern void disable_IO_APIC(void);
>>> extern void setup_ioapic_dest(void);
>>> @@ -231,6 +232,10 @@ static inline void io_apic_init_mappings(void) { }
>>> #define native_disable_io_apic NULL
>>>
>>> static inline void setup_IO_APIC(void) { }
>>> +static inline bool virtual_wire_via_ioapic(void)
>>> +{
>>> + return true;
>>> +}
>>> static inline void enable_IO_APIC(void) { }
>>> static inline void setup_ioapic_dest(void) { }
>>>
>>> diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
>>> index 8e25b9b2d351..04358e0cf1e2 100644
>>> --- a/arch/x86/kernel/apic/apic.c
>>> +++ b/arch/x86/kernel/apic/apic.c
>>> @@ -1124,6 +1124,53 @@ void __init sync_Arb_IDs(void)
>>> }
>>>
>>> /*
>>> + * Return false means the virtual wire mode through-local-apic is inactive
>>> + */
>>> +static bool virtual_wire_via_lapic(void)
>>> +{
>>> + unsigned int value;
>>> +
>>> + /* Check the APIC global enable/disable flag firstly */
>>> + if (boot_cpu_data.x86 >= 6) {
>>> + u32 h, l;
>>> +
>>> + rdmsr(MSR_IA32_APICBASE, l, h);
>>> + /*
>>> + * If local APIC is disabled by BIOS
>>> + * do nothing, but return true
>>> + */
>>> + if (!(l & MSR_IA32_APICBASE_ENABLE))
>>> + return true;
>>> + }
>>> +
>>> + /* Check the software enable/disable flag */
>>> + value = apic_read(APIC_SPIV);
>>> + if (!(value & APIC_SPIV_APIC_ENABLED))
>>> + return false;
>>> +
>>> + /*
>>> + * Virtual wire mode via local APIC requests
>>> + * APIC to enable the LINT0 for edge-trggered ExtINT delivery mode
>>> + * and LINT1 for level-triggered NMI delivery mode
>>> + */
>>> + value = apic_read(APIC_LVT0);
>>> + if (GET_APIC_DELIVERY_MODE(value) != APIC_MODE_EXTINT)
>>> + return false;
>>> +
>>> + value = apic_read(APIC_LVT1);
>>> + if (GET_APIC_DELIVERY_MODE(value) != APIC_MODE_NMI)
>>> + return false;
>>> +
>>> + return true;
>>> +}
>>> +
>>> +static bool check_virtual_wire_mode(void)
>>> +{
>>> + /* Neither of virtual wire mode is active, return false */
>>> + return virtual_wire_via_lapic() || virtual_wire_via_ioapic();
>>> +}
>>> +
>>> +/*
>>> * An initial setup of the virtual wire mode.
>>> */
>>> void __init init_bsp_APIC(void)
>>> @@ -1133,8 +1180,14 @@ void __init init_bsp_APIC(void)
>>> /*
>>> * Don't do the setup now if we have a SMP BIOS as the
>>> * through-I/O-APIC virtual wire mode might be active.
>>> + *
>>> + * It's better to do further check if either through-I/O-APIC
>>> + * or through-local-APIC is active.
>>> + * the worst case is that both of them are inactive,
>>> + * If so, We need to enable the virtual wire mode via through-local-APIC
>>> */
>>> - if (smp_found_config || !boot_cpu_has(X86_FEATURE_APIC))
>>> + if ((smp_found_config && check_virtual_wire_mode()) ||
>>> + !boot_cpu_has(X86_FEATURE_APIC))
>>> return;
>>>
>>> /*
>>> diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
>>> index 446702ed99dc..5a32c26938ac 100644
>>> --- a/arch/x86/kernel/apic/io_apic.c
>>> +++ b/arch/x86/kernel/apic/io_apic.c
>>> @@ -1379,6 +1379,34 @@ void __init print_IO_APICs(void)
>>> /* Where if anywhere is the i8259 connect in external int mode */
>>> static struct { int pin, apic; } ioapic_i8259 = { -1, -1 };
>>>
>>> +/*
>>> + * Return false means the virtual wire mode via I/O APIC is inactive
>>> + */
>>> +bool virtual_wire_via_ioapic(void)
>>> +{
>>> + int apic, pin;
>>> +
>>> + for_each_ioapic_pin(apic, pin) {
>>> + /* See if any of the pins is in ExtINT mode */
>>> + struct IO_APIC_route_entry entry = ioapic_read_entry(apic, pin);
>>> +
>>> + /*
>>> + * If the interrupt line is enabled and in ExtInt mode
>>> + * I have found the pin where the i8259 is connected.
>>> + */
>>> + if ((entry.mask == 0) && (entry.delivery_mode == dest_ExtINT))
>>> + return true;
>>> + }
>>> +
>>> + /*
>>> + * Virtual wire mode via I/O APIC requests
>>> + * I/O APIC be connected to i8259 in chapter 3.6.2.2 of the MP v1.4 spec
>>> + * If no pin in ExtInt mode,
>>> + * the through-I/O-APIC virtual wire mode can be regarded inactive.
>>> + */
>>> + return false;
>>> +}
>>> +
>>> void __init enable_IO_APIC(void)
>>> {
>>> int i8259_apic, i8259_pin;
>>> --
>>> 1.9.3
>>>
>>>
>>>
>>
>
>
