Message ID | 20210926025144.55674-1-nixiaoming@huawei.com (mailing list archive) |
---|---|
State | Superseded, archived |
Headers | show |
Series | powerpc:85xx: fix timebase sync issue when CONFIG_HOTPLUG_CPU=n | expand |
Related | show |
Context | Check | Description |
---|---|---|
snowpatch_ozlabs/github-powerpc_ppctests | success | Successfully ran 8 jobs. |
snowpatch_ozlabs/github-powerpc_selftests | success | Successfully ran 8 jobs. |
snowpatch_ozlabs/github-powerpc_clang | success | Successfully ran 7 jobs. |
snowpatch_ozlabs/github-powerpc_kernel_qemu | success | Successfully ran 24 jobs. |
snowpatch_ozlabs/github-powerpc_sparse | success | Successfully ran 4 jobs. |
On 2021/9/26 10:51, Xiaoming Ni wrote: > When CONFIG_SMP=y, timebase synchronization is required when the second > kernel is started. > arch/powerpc/kernel/smp.c: > int __cpu_up(unsigned int cpu, struct task_struct *tidle) > { > ... > if (smp_ops->give_timebase) > smp_ops->give_timebase(); > ... > } > > void start_secondary(void *unused) > { > ... > if (smp_ops->take_timebase) > smp_ops->take_timebase(); > ... > } > > When CONFIG_HOTPLUG_CPU=n and CONFIG_KEXEC_CORE=n, > smp_85xx_ops.give_timebase is NULL, > smp_85xx_ops.take_timebase is NULL, > As a result, the timebase is not synchronized. > > Timebase synchronization does not depend on CONFIG_HOTPLUG_CPU. > > Fixes: 56f1ba280719 ("powerpc/mpc85xx: refactor the PM operations") > Cc: stable@vger.kernel.org #v4.6 > Signed-off-by: Xiaoming Ni <nixiaoming@huawei.com> > --- > arch/powerpc/platforms/85xx/Makefile | 2 +- > arch/powerpc/platforms/85xx/mpc85xx_pm_ops.c | 4 ++++ > arch/powerpc/platforms/85xx/smp.c | 9 ++++----- > 3 files changed, 9 insertions(+), 6 deletions(-) > > diff --git a/arch/powerpc/platforms/85xx/Makefile b/arch/powerpc/platforms/85xx/Makefile > index 60e4e97a929d..71ce1f6b6966 100644 > --- a/arch/powerpc/platforms/85xx/Makefile > +++ b/arch/powerpc/platforms/85xx/Makefile > @@ -3,7 +3,7 @@ > # Makefile for the PowerPC 85xx linux kernel. > # > obj-$(CONFIG_SMP) += smp.o > -obj-$(CONFIG_FSL_PMC) += mpc85xx_pm_ops.o > +obj-$(CONFIG_SMP) += mpc85xx_pm_ops.o > > obj-y += common.o > > diff --git a/arch/powerpc/platforms/85xx/mpc85xx_pm_ops.c b/arch/powerpc/platforms/85xx/mpc85xx_pm_ops.c > index 7c0133f558d0..a5656b3e9701 100644 > --- a/arch/powerpc/platforms/85xx/mpc85xx_pm_ops.c > +++ b/arch/powerpc/platforms/85xx/mpc85xx_pm_ops.c > @@ -17,6 +17,7 @@ > > static struct ccsr_guts __iomem *guts; > > +#ifdef CONFIG_FSL_PMC > static void mpc85xx_irq_mask(int cpu) > { > > @@ -49,6 +50,7 @@ static void mpc85xx_cpu_up_prepare(int cpu) > { > > } > +#endif > > static void mpc85xx_freeze_time_base(bool freeze) > { > @@ -76,10 +78,12 @@ static const struct of_device_id mpc85xx_smp_guts_ids[] = { > > static const struct fsl_pm_ops mpc85xx_pm_ops = { > .freeze_time_base = mpc85xx_freeze_time_base, > +#ifdef CONFIG_FSL_PMC > .irq_mask = mpc85xx_irq_mask, > .irq_unmask = mpc85xx_irq_unmask, > .cpu_die = mpc85xx_cpu_die, > .cpu_up_prepare = mpc85xx_cpu_up_prepare, > +#endif > }; > > int __init mpc85xx_setup_pmc(void) > diff --git a/arch/powerpc/platforms/85xx/smp.c b/arch/powerpc/platforms/85xx/smp.c > index c6df294054fe..349298cd9671 100644 > --- a/arch/powerpc/platforms/85xx/smp.c > +++ b/arch/powerpc/platforms/85xx/smp.c > @@ -40,7 +40,6 @@ struct epapr_spin_table { > u32 pir; > }; > > -#ifdef CONFIG_HOTPLUG_CPU > static u64 timebase; > static int tb_req; > static int tb_valid; > @@ -112,6 +111,7 @@ static void mpc85xx_take_timebase(void) > local_irq_restore(flags); > } > > +#ifdef CONFIG_HOTPLUG_CPU > static void smp_85xx_cpu_offline_self(void) > { > unsigned int cpu = smp_processor_id(); > @@ -499,17 +499,16 @@ void __init mpc85xx_smp_init(void) > #ifdef CONFIG_FSL_CORENET_RCPM > fsl_rcpm_init(); > #endif > - > -#ifdef CONFIG_FSL_PMC > - mpc85xx_setup_pmc(); > #endif > + mpc85xx_setup_pmc(); > if (qoriq_pm_ops) { > smp_85xx_ops.give_timebase = mpc85xx_give_timebase; > smp_85xx_ops.take_timebase = mpc85xx_take_timebase; > +#ifdef CONFIG_HOTPLUG_CPU > smp_85xx_ops.cpu_offline_self = smp_85xx_cpu_offline_self; > smp_85xx_ops.cpu_die = qoriq_cpu_kill; > - } > #endif > + } > smp_ops = &smp_85xx_ops; > > #ifdef CONFIG_KEXEC_CORE > I found inconsistent time values on different CPUs on my mpc8572 and used this patch to fix it. But today I found out in ppc64 testing that this patch causes the system to trigger oops in the function mpc85xx_freeze_time_base(): the variable "guts" is a null pointer. I'm sorry to bother you. I'll fix it and resend v2 later, Thanks Xiaoming Ni
diff --git a/arch/powerpc/platforms/85xx/Makefile b/arch/powerpc/platforms/85xx/Makefile index 60e4e97a929d..71ce1f6b6966 100644 --- a/arch/powerpc/platforms/85xx/Makefile +++ b/arch/powerpc/platforms/85xx/Makefile @@ -3,7 +3,7 @@ # Makefile for the PowerPC 85xx linux kernel. # obj-$(CONFIG_SMP) += smp.o -obj-$(CONFIG_FSL_PMC) += mpc85xx_pm_ops.o +obj-$(CONFIG_SMP) += mpc85xx_pm_ops.o obj-y += common.o diff --git a/arch/powerpc/platforms/85xx/mpc85xx_pm_ops.c b/arch/powerpc/platforms/85xx/mpc85xx_pm_ops.c index 7c0133f558d0..a5656b3e9701 100644 --- a/arch/powerpc/platforms/85xx/mpc85xx_pm_ops.c +++ b/arch/powerpc/platforms/85xx/mpc85xx_pm_ops.c @@ -17,6 +17,7 @@ static struct ccsr_guts __iomem *guts; +#ifdef CONFIG_FSL_PMC static void mpc85xx_irq_mask(int cpu) { @@ -49,6 +50,7 @@ static void mpc85xx_cpu_up_prepare(int cpu) { } +#endif static void mpc85xx_freeze_time_base(bool freeze) { @@ -76,10 +78,12 @@ static const struct of_device_id mpc85xx_smp_guts_ids[] = { static const struct fsl_pm_ops mpc85xx_pm_ops = { .freeze_time_base = mpc85xx_freeze_time_base, +#ifdef CONFIG_FSL_PMC .irq_mask = mpc85xx_irq_mask, .irq_unmask = mpc85xx_irq_unmask, .cpu_die = mpc85xx_cpu_die, .cpu_up_prepare = mpc85xx_cpu_up_prepare, +#endif }; int __init mpc85xx_setup_pmc(void) diff --git a/arch/powerpc/platforms/85xx/smp.c b/arch/powerpc/platforms/85xx/smp.c index c6df294054fe..349298cd9671 100644 --- a/arch/powerpc/platforms/85xx/smp.c +++ b/arch/powerpc/platforms/85xx/smp.c @@ -40,7 +40,6 @@ struct epapr_spin_table { u32 pir; }; -#ifdef CONFIG_HOTPLUG_CPU static u64 timebase; static int tb_req; static int tb_valid; @@ -112,6 +111,7 @@ static void mpc85xx_take_timebase(void) local_irq_restore(flags); } +#ifdef CONFIG_HOTPLUG_CPU static void smp_85xx_cpu_offline_self(void) { unsigned int cpu = smp_processor_id(); @@ -499,17 +499,16 @@ void __init mpc85xx_smp_init(void) #ifdef CONFIG_FSL_CORENET_RCPM fsl_rcpm_init(); #endif - -#ifdef CONFIG_FSL_PMC - mpc85xx_setup_pmc(); #endif + mpc85xx_setup_pmc(); if (qoriq_pm_ops) { smp_85xx_ops.give_timebase = mpc85xx_give_timebase; smp_85xx_ops.take_timebase = mpc85xx_take_timebase; +#ifdef CONFIG_HOTPLUG_CPU smp_85xx_ops.cpu_offline_self = smp_85xx_cpu_offline_self; smp_85xx_ops.cpu_die = qoriq_cpu_kill; - } #endif + } smp_ops = &smp_85xx_ops; #ifdef CONFIG_KEXEC_CORE
When CONFIG_SMP=y, timebase synchronization is required when the second kernel is started. arch/powerpc/kernel/smp.c: int __cpu_up(unsigned int cpu, struct task_struct *tidle) { ... if (smp_ops->give_timebase) smp_ops->give_timebase(); ... } void start_secondary(void *unused) { ... if (smp_ops->take_timebase) smp_ops->take_timebase(); ... } When CONFIG_HOTPLUG_CPU=n and CONFIG_KEXEC_CORE=n, smp_85xx_ops.give_timebase is NULL, smp_85xx_ops.take_timebase is NULL, As a result, the timebase is not synchronized. Timebase synchronization does not depend on CONFIG_HOTPLUG_CPU. Fixes: 56f1ba280719 ("powerpc/mpc85xx: refactor the PM operations") Cc: stable@vger.kernel.org #v4.6 Signed-off-by: Xiaoming Ni <nixiaoming@huawei.com> --- arch/powerpc/platforms/85xx/Makefile | 2 +- arch/powerpc/platforms/85xx/mpc85xx_pm_ops.c | 4 ++++ arch/powerpc/platforms/85xx/smp.c | 9 ++++----- 3 files changed, 9 insertions(+), 6 deletions(-)