From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH 7 of 7] KEXEC: correctly revert x2apic state when kexecing Date: Wed, 15 Jun 2011 12:59:46 +0100 Message-ID: <4DF89EB2.3000609@citrix.com> References: <4DF896CC0200007800047532@nat28.tlf.novell.com> <4DF87DF1.4000000@citrix.com> <4DF8A218020000780004756C@nat28.tlf.novell.com> <4DF88B1B.6090503@citrix.com> <4DF8A942020000780004758A@nat28.tlf.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="ISO-8859-1" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <4DF8A942020000780004758A@nat28.tlf.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Jan Beulich Cc: "xen-devel@lists.xensource.com" List-Id: xen-devel@lists.xenproject.org On 15/06/11 11:44, Jan Beulich wrote: >>>> On 15.06.11 at 12:36, Andrew Cooper wrote: >> On 15/06/11 11:14, Jan Beulich wrote: >>>>>> On 15.06.11 at 11:40, Andrew Cooper wrote: >>>> On 15/06/11 10:26, Jan Beulich wrote: >>>>>>>> On 13.06.11 at 19:02, Andrew Cooper 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