Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1441091 > unrolled thread

Re: [PATCH v2] kexec: Fix kdump failure with notsc

Started byXunlei Pang <xpang@redhat.com>
First post2016-07-12 09:00 +0200
Last post2016-07-12 11:30 +0200
Articles 5 — 3 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  Re: [PATCH v2] kexec: Fix kdump failure with notsc Xunlei Pang <xpang@redhat.com> - 2016-07-12 09:00 +0200
    Re: [PATCH v2] kexec: Fix kdump failure with notsc Baoquan He <bhe@redhat.com> - 2016-07-12 10:30 +0200
      Re: [PATCH v2] kexec: Fix kdump failure with notsc "Wei, Jiangang" <weijg.fnst@cn.fujitsu.com> - 2016-07-12 11:20 +0200
    Re: [PATCH v2] kexec: Fix kdump failure with notsc "Wei, Jiangang" <weijg.fnst@cn.fujitsu.com> - 2016-07-12 11:10 +0200
      Re: [PATCH v2] kexec: Fix kdump failure with notsc Baoquan He <bhe@redhat.com> - 2016-07-12 11:30 +0200

#1441091 — Re: [PATCH v2] kexec: Fix kdump failure with notsc

FromXunlei Pang <xpang@redhat.com>
Date2016-07-12 09:00 +0200
SubjectRe: [PATCH v2] kexec: Fix kdump failure with notsc
Message-ID<rTZPH-1oG-9@gated-at.bofh.it>
On 2016/07/07 at 18:17, Wei Jiangang wrote:
> If we specify the 'notsc' boot 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():
>
>     /* wait for "start of" clock tick */
>     ticks = jiffies;
>     while (ticks == jiffies)
>         ; /* nothing */
>
> serial log of the hang is as follows:
>
>     tsc: Fast TSC calibration using PIT
>     tsc: Detected 2099.947 MHz processor
>     Calibrating delay loop...
>
> The reason is that the dump-capture kernel hangs in while loops and
> waits for jiffies to be updated, but no timer interrupts is passed
> to BSP by APIC.
>
> In fact, the local APIC was disabled in reboot and crash path by
> lapic_shutdown(). We need to put APIC in legacy mode in kexec jump path
> (put the system into PIT during the crash kernel),
> so that the dump-capture kernel can get timer interrupts.
>
> BTW,
> I found the buggy commit 522e66464467 ("x86/apic: Disable I/O APIC
> before shutdown of the local APIC") via bisection.
>
> Originally, I want to revert it.
> But Ingo Molnar comments that "By reverting the change can paper over
> the bug, but re-introduce the bug that can result in certain CPUs hanging
> if IO-APIC sends an APIC message if the lapic is disabled prematurely"
> And I think it's pertinent.
>
> Signed-off-by: Wei Jiangang <weijg.fnst@cn.fujitsu.com>
> ---
>  arch/x86/include/asm/apic.h        | 5 +++++
>  arch/x86/kernel/apic/apic.c        | 9 +++++++++
>  arch/x86/kernel/machine_kexec_32.c | 5 ++---
>  arch/x86/kernel/machine_kexec_64.c | 6 +++---
>  4 files changed, 19 insertions(+), 6 deletions(-)
>
> diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h
> index bc27611fa58f..5d7e635e580a 100644
> --- a/arch/x86/include/asm/apic.h
> +++ b/arch/x86/include/asm/apic.h
> @@ -128,6 +128,7 @@ extern void clear_local_APIC(void);
>  extern void disconnect_bsp_APIC(int virt_wire_setup);
>  extern void disable_local_APIC(void);
>  extern void lapic_shutdown(void);
> +extern int lapic_disabled(void);
>  extern void sync_Arb_IDs(void);
>  extern void init_bsp_APIC(void);
>  extern void setup_local_APIC(void);
> @@ -165,6 +166,10 @@ extern int setup_APIC_eilvt(u8 lvt_off, u8 vector, u8 msg_type, u8 mask);
>  
>  #else /* !CONFIG_X86_LOCAL_APIC */
>  static inline void lapic_shutdown(void) { }
> +static inline int lapic_disabled(void)
> +{
> +	return 0;
> +}
>  #define local_apic_timer_c2_ok		1
>  static inline void init_apic_mappings(void) { }
>  static inline void disable_local_APIC(void) { }
> diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
> index 60078a67d7e3..d1df250994bb 100644
> --- a/arch/x86/kernel/apic/apic.c
> +++ b/arch/x86/kernel/apic/apic.c
> @@ -133,6 +133,9 @@ static inline void imcr_apic_to_pic(void)
>  }
>  #endif
>  
> +/* Local APIC is disabled by the kernel for crash or reboot path */
> +static int disabled_local_apic;
> +
>  /*
>   * Knob to control our willingness to enable the local APIC.
>   *
> @@ -1097,10 +1100,16 @@ void lapic_shutdown(void)
>  #endif
>  		disable_local_APIC();
>  
> +	disabled_local_apic = 1;
>  
>  	local_irq_restore(flags);
>  }
>  
> +int lapic_disabled(void)
> +{
> +	return disabled_local_apic;
> +}
> +
>  /**
>   * sync_Arb_IDs - synchronize APIC bus arbitration IDs
>   */
> diff --git a/arch/x86/kernel/machine_kexec_32.c b/arch/x86/kernel/machine_kexec_32.c
> index 469b23d6acc2..c934a7868e6b 100644
> --- a/arch/x86/kernel/machine_kexec_32.c
> +++ b/arch/x86/kernel/machine_kexec_32.c
> @@ -202,14 +202,13 @@ void machine_kexec(struct kimage *image)
>  	local_irq_disable();
>  	hw_breakpoint_disable();
>  
> -	if (image->preserve_context) {
> +	if (image->preserve_context || lapic_disabled()) {
>  #ifdef CONFIG_X86_IO_APIC
>  		/*
>  		 * We need to put APICs in legacy mode so that we can
>  		 * get timer interrupts in second kernel. kexec/kdump
>  		 * paths already have calls to disable_IO_APIC() in
> -		 * one form or other. kexec jump path also need
> -		 * one.
> +		 * one form or other. kexec jump path also need one.
>  		 */
>  		disable_IO_APIC();

Hi Wei,

As the comment says, kexec/kdump paths already have disable_IO_APIC(), why again here?

Regards,
Xunlei

>  #endif
> diff --git a/arch/x86/kernel/machine_kexec_64.c b/arch/x86/kernel/machine_kexec_64.c
> index 5a294e48b185..d3598cdd6437 100644
> --- a/arch/x86/kernel/machine_kexec_64.c
> +++ b/arch/x86/kernel/machine_kexec_64.c
> @@ -23,6 +23,7 @@
>  #include <asm/pgtable.h>
>  #include <asm/tlbflush.h>
>  #include <asm/mmu_context.h>
> +#include <asm/apic.h>
>  #include <asm/io_apic.h>
>  #include <asm/debugreg.h>
>  #include <asm/kexec-bzimage64.h>
> @@ -269,14 +270,13 @@ void machine_kexec(struct kimage *image)
>  	local_irq_disable();
>  	hw_breakpoint_disable();
>  
> -	if (image->preserve_context) {
> +	if (image->preserve_context || lapic_disabled()) {
>  #ifdef CONFIG_X86_IO_APIC
>  		/*
>  		 * We need to put APICs in legacy mode so that we can
>  		 * get timer interrupts in second kernel. kexec/kdump
>  		 * paths already have calls to disable_IO_APIC() in
> -		 * one form or other. kexec jump path also need
> -		 * one.
> +		 * one form or other. kexec jump path also need one.
>  		 */
>  		disable_IO_APIC();
>  #endif

[toc] | [next] | [standalone]


#1441131

FromBaoquan He <bhe@redhat.com>
Date2016-07-12 10:30 +0200
Message-ID<rU1eO-2pJ-11@gated-at.bofh.it>
In reply to#1441091
On 07/12/16 at 02:52pm, Xunlei Pang wrote:
> On 2016/07/07 at 18:17, Wei Jiangang wrote:
> > Signed-off-by: Wei Jiangang <weijg.fnst@cn.fujitsu.com>
> > ---
> > +/* Local APIC is disabled by the kernel for crash or reboot path */
> > +static int disabled_local_apic;
> > +
> >  /*
> >   * Knob to control our willingness to enable the local APIC.
> >   *
> > @@ -1097,10 +1100,16 @@ void lapic_shutdown(void)
> >  #endif
> >  		disable_local_APIC();
> >  
> > +	disabled_local_apic = 1;
> >  
> >  	local_irq_restore(flags);
> >  }
> >  
> > +int lapic_disabled(void)
> > +{
> > +	return disabled_local_apic;
> > +}
> > +
> >  /**
> >   * sync_Arb_IDs - synchronize APIC bus arbitration IDs
> >   */
> > diff --git a/arch/x86/kernel/machine_kexec_32.c b/arch/x86/kernel/machine_kexec_32.c
> > index 469b23d6acc2..c934a7868e6b 100644
> > --- a/arch/x86/kernel/machine_kexec_32.c
> > +++ b/arch/x86/kernel/machine_kexec_32.c
> > @@ -202,14 +202,13 @@ void machine_kexec(struct kimage *image)
> >  	local_irq_disable();
> >  	hw_breakpoint_disable();
> >  
> > -	if (image->preserve_context) {
> > +	if (image->preserve_context || lapic_disabled()) {
> >  #ifdef CONFIG_X86_IO_APIC
> >  		/*
> >  		 * We need to put APICs in legacy mode so that we can
> >  		 * get timer interrupts in second kernel. kexec/kdump
> >  		 * paths already have calls to disable_IO_APIC() in
> > -		 * one form or other. kexec jump path also need
> > -		 * one.
> > +		 * one form or other. kexec jump path also need one.
> >  		 */
> >  		disable_IO_APIC();
> 
> Hi Wei,
> 
> As the comment says, kexec/kdump paths already have disable_IO_APIC(), why again here?

I also have this question. I guess Jiangang didn't post his modification
correctly. He should remove calling of disable_IO_APIC in
native_machine_crash_shutdown(). Assume his test was done on correct
code change. 

> 
> Regards,
> Xunlei
> 
> >  #endif
> > diff --git a/arch/x86/kernel/machine_kexec_64.c b/arch/x86/kernel/machine_kexec_64.c
> > index 5a294e48b185..d3598cdd6437 100644
> > --- a/arch/x86/kernel/machine_kexec_64.c
> > +++ b/arch/x86/kernel/machine_kexec_64.c
> > @@ -23,6 +23,7 @@
> >  #include <asm/pgtable.h>
> >  #include <asm/tlbflush.h>
> >  #include <asm/mmu_context.h>
> > +#include <asm/apic.h>
> >  #include <asm/io_apic.h>
> >  #include <asm/debugreg.h>
> >  #include <asm/kexec-bzimage64.h>
> > @@ -269,14 +270,13 @@ void machine_kexec(struct kimage *image)
> >  	local_irq_disable();
> >  	hw_breakpoint_disable();
> >  
> > -	if (image->preserve_context) {
> > +	if (image->preserve_context || lapic_disabled()) {
> >  #ifdef CONFIG_X86_IO_APIC
> >  		/*
> >  		 * We need to put APICs in legacy mode so that we can
> >  		 * get timer interrupts in second kernel. kexec/kdump
> >  		 * paths already have calls to disable_IO_APIC() in
> > -		 * one form or other. kexec jump path also need
> > -		 * one.
> > +		 * one form or other. kexec jump path also need one.
> >  		 */
> >  		disable_IO_APIC();
> >  #endif
> 
> 
> _______________________________________________
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec

[toc] | [prev] | [next] | [standalone]


#1441165

From"Wei, Jiangang" <weijg.fnst@cn.fujitsu.com>
Date2016-07-12 11:20 +0200
Message-ID<rU21c-2W2-9@gated-at.bofh.it>
In reply to#1441131
On Tue, 2016-07-12 at 16:21 +0800, Baoquan He wrote:
> On 07/12/16 at 02:52pm, Xunlei Pang wrote:
> > On 2016/07/07 at 18:17, Wei Jiangang wrote:
> > > Signed-off-by: Wei Jiangang <weijg.fnst@cn.fujitsu.com>
> > > ---
> > > +/* Local APIC is disabled by the kernel for crash or reboot path */
> > > +static int disabled_local_apic;
> > > +
> > >  /*
> > >   * Knob to control our willingness to enable the local APIC.
> > >   *
> > > @@ -1097,10 +1100,16 @@ void lapic_shutdown(void)
> > >  #endif
> > >  		disable_local_APIC();
> > >  
> > > +	disabled_local_apic = 1;
> > >  
> > >  	local_irq_restore(flags);
> > >  }
> > >  
> > > +int lapic_disabled(void)
> > > +{
> > > +	return disabled_local_apic;
> > > +}
> > > +
> > >  /**
> > >   * sync_Arb_IDs - synchronize APIC bus arbitration IDs
> > >   */
> > > diff --git a/arch/x86/kernel/machine_kexec_32.c b/arch/x86/kernel/machine_kexec_32.c
> > > index 469b23d6acc2..c934a7868e6b 100644
> > > --- a/arch/x86/kernel/machine_kexec_32.c
> > > +++ b/arch/x86/kernel/machine_kexec_32.c
> > > @@ -202,14 +202,13 @@ void machine_kexec(struct kimage *image)
> > >  	local_irq_disable();
> > >  	hw_breakpoint_disable();
> > >  
> > > -	if (image->preserve_context) {
> > > +	if (image->preserve_context || lapic_disabled()) {
> > >  #ifdef CONFIG_X86_IO_APIC
> > >  		/*
> > >  		 * We need to put APICs in legacy mode so that we can
> > >  		 * get timer interrupts in second kernel. kexec/kdump
> > >  		 * paths already have calls to disable_IO_APIC() in
> > > -		 * one form or other. kexec jump path also need
> > > -		 * one.
> > > +		 * one form or other. kexec jump path also need one.
> > >  		 */
> > >  		disable_IO_APIC();
> > 
> > Hi Wei,
> > 
> > As the comment says, kexec/kdump paths already have disable_IO_APIC(), why again here?
> 
> I also have this question. I guess Jiangang didn't post his modification
> correctly. He should remove calling of disable_IO_APIC in
> native_machine_crash_shutdown(). Assume his test was done on correct
> code change. 
Hi he,
Thanks for your suggestion firstly.

In fact, Eric had suggested me to do that last week
(https://lkml.org/lkml/2016/7/8/14).
And i had a try to remove the calling of disable_IO_APIC in
native_machine_crash_shutdown().
But it doesn't work for kdump with notsc.

Thanks,
wei
> 
> > 
> > Regards,
> > Xunlei
> > 
> > >  #endif
> > > diff --git a/arch/x86/kernel/machine_kexec_64.c b/arch/x86/kernel/machine_kexec_64.c
> > > index 5a294e48b185..d3598cdd6437 100644
> > > --- a/arch/x86/kernel/machine_kexec_64.c
> > > +++ b/arch/x86/kernel/machine_kexec_64.c
> > > @@ -23,6 +23,7 @@
> > >  #include <asm/pgtable.h>
> > >  #include <asm/tlbflush.h>
> > >  #include <asm/mmu_context.h>
> > > +#include <asm/apic.h>
> > >  #include <asm/io_apic.h>
> > >  #include <asm/debugreg.h>
> > >  #include <asm/kexec-bzimage64.h>
> > > @@ -269,14 +270,13 @@ void machine_kexec(struct kimage *image)
> > >  	local_irq_disable();
> > >  	hw_breakpoint_disable();
> > >  
> > > -	if (image->preserve_context) {
> > > +	if (image->preserve_context || lapic_disabled()) {
> > >  #ifdef CONFIG_X86_IO_APIC
> > >  		/*
> > >  		 * We need to put APICs in legacy mode so that we can
> > >  		 * get timer interrupts in second kernel. kexec/kdump
> > >  		 * paths already have calls to disable_IO_APIC() in
> > > -		 * one form or other. kexec jump path also need
> > > -		 * one.
> > > +		 * one form or other. kexec jump path also need one.
> > >  		 */
> > >  		disable_IO_APIC();
> > >  #endif
> > 
> > 
> > _______________________________________________
> > kexec mailing list
> > kexec@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/kexec
> 
> 



[toc] | [prev] | [next] | [standalone]


#1441161

From"Wei, Jiangang" <weijg.fnst@cn.fujitsu.com>
Date2016-07-12 11:10 +0200
Message-ID<rU1Rw-2SM-17@gated-at.bofh.it>
In reply to#1441091
On Tue, 2016-07-12 at 14:52 +0800, Xunlei Pang wrote:
> On 2016/07/07 at 18:17, Wei Jiangang wrote:
> > If we specify the 'notsc' boot 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():
> >
> >     /* wait for "start of" clock tick */
> >     ticks = jiffies;
> >     while (ticks == jiffies)
> >         ; /* nothing */
> >
> > serial log of the hang is as follows:
> >
> >     tsc: Fast TSC calibration using PIT
> >     tsc: Detected 2099.947 MHz processor
> >     Calibrating delay loop...
> >
> > The reason is that the dump-capture kernel hangs in while loops and
> > waits for jiffies to be updated, but no timer interrupts is passed
> > to BSP by APIC.
> >
> > In fact, the local APIC was disabled in reboot and crash path by
> > lapic_shutdown(). We need to put APIC in legacy mode in kexec jump path
> > (put the system into PIT during the crash kernel),
> > so that the dump-capture kernel can get timer interrupts.
> >
> > BTW,
> > I found the buggy commit 522e66464467 ("x86/apic: Disable I/O APIC
> > before shutdown of the local APIC") via bisection.
> >
> > Originally, I want to revert it.
> > But Ingo Molnar comments that "By reverting the change can paper over
> > the bug, but re-introduce the bug that can result in certain CPUs hanging
> > if IO-APIC sends an APIC message if the lapic is disabled prematurely"
> > And I think it's pertinent.
> >
> > Signed-off-by: Wei Jiangang <weijg.fnst@cn.fujitsu.com>
> > ---
> >  arch/x86/include/asm/apic.h        | 5 +++++
> >  arch/x86/kernel/apic/apic.c        | 9 +++++++++
> >  arch/x86/kernel/machine_kexec_32.c | 5 ++---
> >  arch/x86/kernel/machine_kexec_64.c | 6 +++---
> >  4 files changed, 19 insertions(+), 6 deletions(-)
> >
> > diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h
> > index bc27611fa58f..5d7e635e580a 100644
> > --- a/arch/x86/include/asm/apic.h
> > +++ b/arch/x86/include/asm/apic.h
> > @@ -128,6 +128,7 @@ extern void clear_local_APIC(void);
> >  extern void disconnect_bsp_APIC(int virt_wire_setup);
> >  extern void disable_local_APIC(void);
> >  extern void lapic_shutdown(void);
> > +extern int lapic_disabled(void);
> >  extern void sync_Arb_IDs(void);
> >  extern void init_bsp_APIC(void);
> >  extern void setup_local_APIC(void);
> > @@ -165,6 +166,10 @@ extern int setup_APIC_eilvt(u8 lvt_off, u8 vector, u8 msg_type, u8 mask);
> >  
> >  #else /* !CONFIG_X86_LOCAL_APIC */
> >  static inline void lapic_shutdown(void) { }
> > +static inline int lapic_disabled(void)
> > +{
> > +	return 0;
> > +}
> >  #define local_apic_timer_c2_ok		1
> >  static inline void init_apic_mappings(void) { }
> >  static inline void disable_local_APIC(void) { }
> > diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
> > index 60078a67d7e3..d1df250994bb 100644
> > --- a/arch/x86/kernel/apic/apic.c
> > +++ b/arch/x86/kernel/apic/apic.c
> > @@ -133,6 +133,9 @@ static inline void imcr_apic_to_pic(void)
> >  }
> >  #endif
> >  
> > +/* Local APIC is disabled by the kernel for crash or reboot path */
> > +static int disabled_local_apic;
> > +
> >  /*
> >   * Knob to control our willingness to enable the local APIC.
> >   *
> > @@ -1097,10 +1100,16 @@ void lapic_shutdown(void)
> >  #endif
> >  		disable_local_APIC();
> >  
> > +	disabled_local_apic = 1;
> >  
> >  	local_irq_restore(flags);
> >  }
> >  
> > +int lapic_disabled(void)
> > +{
> > +	return disabled_local_apic;
> > +}
> > +
> >  /**
> >   * sync_Arb_IDs - synchronize APIC bus arbitration IDs
> >   */
> > diff --git a/arch/x86/kernel/machine_kexec_32.c b/arch/x86/kernel/machine_kexec_32.c
> > index 469b23d6acc2..c934a7868e6b 100644
> > --- a/arch/x86/kernel/machine_kexec_32.c
> > +++ b/arch/x86/kernel/machine_kexec_32.c
> > @@ -202,14 +202,13 @@ void machine_kexec(struct kimage *image)
> >  	local_irq_disable();
> >  	hw_breakpoint_disable();
> >  
> > -	if (image->preserve_context) {
> > +	if (image->preserve_context || lapic_disabled()) {
> >  #ifdef CONFIG_X86_IO_APIC
> >  		/*
> >  		 * We need to put APICs in legacy mode so that we can
> >  		 * get timer interrupts in second kernel. kexec/kdump
> >  		 * paths already have calls to disable_IO_APIC() in
> > -		 * one form or other. kexec jump path also need
> > -		 * one.
> > +		 * one form or other. kexec jump path also need one.
> >  		 */
> >  		disable_IO_APIC();
> 
> Hi Wei,
> 
> As the comment says, kexec/kdump paths already have disable_IO_APIC(), why again here?
Frankly, I am not very clear about it as well...

Originally the codes and comment added by Huang Ying
<ying.huang@intel.com>
There may be some special reason...

The comment also said that it's used to put APICs in legacy mode so that
we can get timer interrupts in second kernel. and we also need it in
kexec jump path.

In fact, the jiffies calibration needs timer interrupts in second
kernel. and the lapic and timer are not ready when second kernel waits
them to update the jiffies value.

so I just want to reuse them and enable legacy mode (PIT timer) for
notsc case.
kernel.

Thanks,
wei
> 
> Regards,
> Xunlei
> 
> >  #endif
> > diff --git a/arch/x86/kernel/machine_kexec_64.c b/arch/x86/kernel/machine_kexec_64.c
> > index 5a294e48b185..d3598cdd6437 100644
> > --- a/arch/x86/kernel/machine_kexec_64.c
> > +++ b/arch/x86/kernel/machine_kexec_64.c
> > @@ -23,6 +23,7 @@
> >  #include <asm/pgtable.h>
> >  #include <asm/tlbflush.h>
> >  #include <asm/mmu_context.h>
> > +#include <asm/apic.h>
> >  #include <asm/io_apic.h>
> >  #include <asm/debugreg.h>
> >  #include <asm/kexec-bzimage64.h>
> > @@ -269,14 +270,13 @@ void machine_kexec(struct kimage *image)
> >  	local_irq_disable();
> >  	hw_breakpoint_disable();
> >  
> > -	if (image->preserve_context) {
> > +	if (image->preserve_context || lapic_disabled()) {
> >  #ifdef CONFIG_X86_IO_APIC
> >  		/*
> >  		 * We need to put APICs in legacy mode so that we can
> >  		 * get timer interrupts in second kernel. kexec/kdump
> >  		 * paths already have calls to disable_IO_APIC() in
> > -		 * one form or other. kexec jump path also need
> > -		 * one.
> > +		 * one form or other. kexec jump path also need one.
> >  		 */
> >  		disable_IO_APIC();
> >  #endif
> 
> 
> 



[toc] | [prev] | [next] | [standalone]


#1441168

FromBaoquan He <bhe@redhat.com>
Date2016-07-12 11:30 +0200
Message-ID<rU2aR-2Zl-1@gated-at.bofh.it>
In reply to#1441161
On 07/12/16 at 09:09am, Wei, Jiangang wrote:
> On Tue, 2016-07-12 at 14:52 +0800, Xunlei Pang wrote:
> > On 2016/07/07 at 18:17, Wei Jiangang wrote:

> > > diff --git a/arch/x86/kernel/machine_kexec_32.c b/arch/x86/kernel/machine_kexec_32.c
> > > index 469b23d6acc2..c934a7868e6b 100644
> > > --- a/arch/x86/kernel/machine_kexec_32.c
> > > +++ b/arch/x86/kernel/machine_kexec_32.c
> > > @@ -202,14 +202,13 @@ void machine_kexec(struct kimage *image)
> > >  	local_irq_disable();
> > >  	hw_breakpoint_disable();
> > >  
> > > -	if (image->preserve_context) {
> > > +	if (image->preserve_context || lapic_disabled()) {
> > >  #ifdef CONFIG_X86_IO_APIC
> > >  		/*
> > >  		 * We need to put APICs in legacy mode so that we can
> > >  		 * get timer interrupts in second kernel. kexec/kdump
> > >  		 * paths already have calls to disable_IO_APIC() in
> > > -		 * one form or other. kexec jump path also need
> > > -		 * one.
> > > +		 * one form or other. kexec jump path also need one.
> > >  		 */
> > >  		disable_IO_APIC();
> > 
> > Hi Wei,
> > 
> > As the comment says, kexec/kdump paths already have disable_IO_APIC(), why again here?
> Frankly, I am not very clear about it as well...
> 
> Originally the codes and comment added by Huang Ying
> <ying.huang@intel.com>
> There may be some special reason...

Check code again, it might be native_disable_io_apic() is called in
disable_IO_APIC(). See the code comments at the beginning of
native_disable_io_apic(), ioapic_i8259 is used to record the pin of
ioapic which connect to 8259. Here ioapic is configured to virtual wire
mode to pass through interrupt to cpu. That might be why Huang Ying
said it can put it into legacy irq mode with calling with
disable_IO_APIC.

void native_disable_io_apic(void)
{
        /*
         * If the i8259 is routed through an IOAPIC
         * Put that IOAPIC in virtual wire mode
         * so legacy interrupts can be delivered.
         */
...
}

Now question is why disable_IO_APIC need be called twice to make ioapic
to be virtual wire mode.

Add Huang Ying to this thread, don't know if he can help explain.

> 
> The comment also said that it's used to put APICs in legacy mode so that
> we can get timer interrupts in second kernel. and we also need it in
> kexec jump path.
> 
> In fact, the jiffies calibration needs timer interrupts in second
> kernel. and the lapic and timer are not ready when second kernel waits
> them to update the jiffies value.
> 
> so I just want to reuse them and enable legacy mode (PIT timer) for
> notsc case.
> kernel.

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web