Re: [PATCH 7 of 7] KEXEC: correctly revert x2apic state when kexecing

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 15/06/11 11:44, Jan Beulich wrote:
>>>> On 15.06.11 at 12:36, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>> On 15/06/11 11:14, Jan Beulich wrote:
>>>>>> On 15.06.11 at 11:40, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>>>> On 15/06/11 10:26, Jan Beulich wrote:
>>>>>>>> On 13.06.11 at 19:02, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>>>>>> @@ -345,6 +346,33 @@ void disable_local_APIC(void)
>>>>>>          wrmsrl(MSR_IA32_APICBASE, msr_content &
>>>>>>                 ~(MSR_IA32_APICBASE_ENABLE|MSR_IA32_APICBASE_EXTD));
>>>>>>      }
>>>>>> +
>>>>>> +    if ( kexecing )
>>>>>> +    {
>>>>>> +        uint64_t msr_content;
>>>>>> +        rdmsrl(MSR_IA32_APICBASE, msr_content);
>>>>>> +        msr_content &= ~(MSR_IA32_APICBASE_ENABLE|MSR_IA32_APICBASE_EXTD);
>>>>>> +        wrmsrl(MSR_IA32_APICBASE, msr_content);
>>>>>> +
>>>>>> +        switch ( apic_boot_mode )
>>>>> Did you verify this gets executed only for the single remaining CPU?
>>>> It most definitely runs on all CPUs.  Because of the difference between
>>>> x2apic and xapic interrupts, it is stark-raving mad to try and run a
>>>> system with different lapics in different modes as the default operating
>>>> state.
>>> But you're not returning each CPU to its boot state - you're instead
>>> forcing them all into the state the boot CPU was in (and hence
>>> possibly out of sync with other firmware provided information).
>> My point was that the apic state of the boot processor really cant be
>> different to the boot state of any other processors, due to the triple
>> faulting fun you get when lapics receive interrupts in the wrong mode.
> As long as the APIC is (software) disabled, it won't receive any
> interrupts (apart from the special SMP boot messages).

True, until the kdump kernel soft-enabled the local apic, tries to
configure it and falls over because it is using MMIO when Xen left the
lapic in x2apic mode.  (I guess one argument is "use a newer kernel" but
that is not helpful for those of us needing to support older kernels)

>>>>>> +    if ( current_local_apic_mode() == APIC_MODE_X2APIC )
>>>>>> +        x2apic_enabled = 1;
>>>>>> +    else
>>>>>> +        x2apic_enabled = 0;
>>>>> Do you really need to force x2apic_enabled *both* ways to avoid
>>>>> described protection fault? And really I still don't follow why the
>>>>> variable at the end of the life of the system all of the sudden needs
>>>>> tweaking, when the system lived happily with its "normal" value.
>>>> You do need to force it both ways.  disable_IO_APIC() which is the
>>>> following call runs the risk of causing a protection fault, when setting
>>>> virtual wire mode back up.  However, in the alternate case where the
>>>> local APIC is in x2apic mode, and x2apic_enabled is false, all the APIC
>>>> code will attempt to use MMIO and get confused when twiddling it does
>>>> nothing.  (This is one of the problems the linux kdump kernel had until
>>>> very recently)
>>> How would the APIC end up in x2apic mode when x2apic_enabled
>>> is not set (or vice versa)?
>>>
>>> Jan
>>>
>> You get into that state when taring down the local APICs on the kexec
>> path, which is why we need to go and tweak the x2apic_enabled variable. 
>> Without this patch, there is nowhere in the Xen code which ever sets
>> x2apic_enabled to 0 (It defaults to 0 in the .data section).
> Past that point there shouldn't be any accesses anymore. If there
> are, I'd rather say that is what needs fixing.
>
> Jan
>
I tried swapping the order of disable_local_APIC and disable_IO_APICs on
the crash path, but that resulted in the kdump kernel not getting any
interrupts whatsoever, and hanging indefinitely when checking the hlt
instruction.  I am still not certain why that happened, but given that
the kdump kernel is fine when booting natively, I didn't explore the
problem very much.

I dislike this hack as much as you do, but I have not found any other
way of doing which is anything like as simple.

-- 
Andrew Cooper - Dom0 Kernel Engineer, Citrix XenServer
T: +44 (0)1223 225 900, http://www.citrix.com