Re: [PATCH] powerpc/pasemi, cbe: Do not process decremeter or external wakeup from powersave

[Nicholas Piggin]

On Fri, 17 Mar 2017 15:43:22 +0100
Christian Zigotzky  wrote:

> Nicholas Piggin  writes:
> 
>  > Hi,
>  >
>  > I would like to start using a dedicated stack for system reset interrupt
>  > and treat it as a Linux nmi, which makes it tricky to call complex
>  > interrupt handlers directly from the system reset trap handler.
>  >
>  > So I would like to remove the decrementer and external handler calls from
>  > Cell and Pasemi platforms' system reset handler. I think we can just
>  > remove them if they can be handled when they re-fire as normal   
> interrupts?
>  > At the moment I don't have environments set up to test if this works.  
> 
> 
> It works fine with my AmigaOne X1000. (Nemo board with a P.A. Semi  
> PA6T-1682M CPU)

Thanks for testing, that's very useful. I will add your tested-by line
for the pasemi part of the patch.

Thanks,
Nick

[PATCH] powerpc/pasemi, cbe: Do not process decremeter or external wakeup from powersave

[Christian Zigotzky]

Nicholas Piggin  writes:

> Hi,
>
> I would like to start using a dedicated stack for system reset interrupt
> and treat it as a Linux nmi, which makes it tricky to call complex
> interrupt handlers directly from the system reset trap handler.
>
> So I would like to remove the decrementer and external handler calls from
> Cell and Pasemi platforms' system reset handler. I think we can just
> remove them if they can be handled when they re-fire as normal 
interrupts?

> At the moment I don't have environments set up to test if this works.


It works fine with my AmigaOne X1000. (Nemo board with a P.A. Semi  
PA6T-1682M CPU)


-- Christian

Re: [PATCH] powerpc/pasemi, cbe: Do not process decremeter or external wakeup from powersave

[Michael Ellerman]

Nicholas Piggin  writes:
> On Fri, 17 Mar 2017 17:12:11 +1100
> Michael Ellerman  wrote:
>> Michael Ellerman  writes:
>> > Nicholas Piggin  writes:
>> >> I would like to start using a dedicated stack for system reset interrupt
>> >> and treat it as a Linux nmi, which makes it tricky to call complex
>> >> interrupt handlers directly from the system reset trap handler.
>> >>
>> >> So I would like to remove the decrementer and external handler calls from
>> >> Cell and Pasemi platforms' system reset handler. I think we can just
>> >> remove them if they can be handled when they re-fire as normal interrupts?
>> >> At the moment I don't have environments set up to test if this works.  
>> >
>> > My QS22 has booted OK with it applied, so it seems OK.
>> >
>> > I'll test it a bit more tomorrow.  
>> 
>> OK, seems fine, and I have a trace that shows it's definitely going
>> through that path:
>
> Thanks for testing it.
>
>>  1)   |.default_idle_call() {
>>  1)   |  .arch_cpu_idle() {
>>  1)   |.cbe_power_save() {
>>  1)   0.128 us|  .prep_irq_for_idle();
>>  1)   |  .system_reset_exception() {
>>  1)   0.512 us|.cbe_system_reset_exception();
>>  1)   6.016 us|  }
>>  1)   |  .do_IRQ() {
>
> It seems to do the right thing. I think decrementer should be working
> proprely by setting it to 1 to get another exception?

Yeah it is. It wasn't showing up in traces but I fixed that and here it
is below.

I'll try and test pasemi on Monday, but this looks fine for Cell.

cheers


 1)   |.default_idle_call() {
 1)   |  .arch_cpu_idle() {
 1)   |.cbe_power_save() {
 1)   0.192 us|  .prep_irq_for_idle();
 1)   |  .system_reset_exception() {
 1)   0.320 us|.cbe_system_reset_exception();
 1)   6.016 us|  }
 1)   |  .timer_interrupt() {
 1)   |.irq_enter() {
 1)   0.768 us|  .rcu_irq_enter();
 1)   |  .tick_irq_enter() {
 1)   0.256 us|.tick_check_oneshot_broadcast_this_cpu();
 1)   |.ktime_get() {
 1)   0.192 us|  .timebase_read();
 1)   5.504 us|}
 1)   |.update_ts_time_stats() {
 1)   0.640 us|  .nr_iowait_cpu();
 1)   6.656 us|}
 1) + 28.160 us   |  }
 1)   |  ._local_bh_enable() {
 1)   0.192 us|.__local_bh_enable();
 1)   5.120 us|  }
 1)   |  .vtime_account_irq_enter() {
 1)   |.vtime_account_idle() {
 1)   0.320 us|  .vtime_delta.isra.5();
 1)   5.504 us|}
 1) + 10.560 us   |  }
 1) + 66.048 us   |}
 1)   |.__timer_interrupt() {
 1)   |  .hrtimer_interrupt() {
 1)   0.448 us|._raw_spin_lock();
 1)   |.ktime_get_update_offsets_now() {
 1)   0.256 us|  .timebase_read();
 1)   5.760 us|}
 1)   |.__hrtimer_run_queues() {
 1)   0.960 us|  .__remove_hrtimer();
 1)   |  .tick_sched_timer() {
 1)   |.ktime_get() {
 1)   0.320 us|  .timebase_read();
 1)   5.888 us|}
 1)   |.tick_sched_do_timer() {
 1)   |  .tick_do_update_jiffies64.part.14() {
 1)   0.384 us|._raw_spin_lock();
 1)   |.do_timer() {
 1)   0.640 us|  .calc_global_load();
 1)   5.632 us|}
 1)   |.update_wall_time() {
 1)   0.576 us|  ._raw_spin_lock_irqsave();
 1)   0.192 us|  .timebase_read();
 1)   0.192 us|  .ntp_tick_length();
 1)   0.256 us|  .ntp_tick_length();
 1)   0.192 us|  .ntp_tick_length();
 1)   |  .timekeeping_update() {
 1)   0.192 us|.ntp_get_next_leap();
 1)   1.600 us|.update_vsyscall_old();
 1)   |.raw_notifier_call_chain() {
 1)   0.768 us|  .notifier_call_chain();
 1)   5.760 us|}
 1)   0.768 us|.update_fast_timekeeper();
 1)   0.384 us 

Re: [PATCH] powerpc/pasemi,cbe: Do not process decremeter or external wakeup from powersave

[Nicholas Piggin]

On Fri, 17 Mar 2017 17:12:11 +1100
Michael Ellerman  wrote:

> Michael Ellerman  writes:
> 
> > Nicholas Piggin  writes:
> >  
> >> Hi,
> >>
> >> I would like to start using a dedicated stack for system reset interrupt
> >> and treat it as a Linux nmi, which makes it tricky to call complex
> >> interrupt handlers directly from the system reset trap handler.
> >>
> >> So I would like to remove the decrementer and external handler calls from
> >> Cell and Pasemi platforms' system reset handler. I think we can just
> >> remove them if they can be handled when they re-fire as normal interrupts?
> >> At the moment I don't have environments set up to test if this works.  
> >
> > My QS22 has booted OK with it applied, so it seems OK.
> >
> > I'll test it a bit more tomorrow.  
> 
> OK, seems fine, and I have a trace that shows it's definitely going
> through that path:

Thanks for testing it.

>  1)   |.default_idle_call() {
>  1)   |  .arch_cpu_idle() {
>  1)   |.cbe_power_save() {
>  1)   0.128 us|  .prep_irq_for_idle();
>  1)   |  .system_reset_exception() {
>  1)   0.512 us|.cbe_system_reset_exception();
>  1)   6.016 us|  }
>  1)   |  .do_IRQ() {

It seems to do the right thing. I think decrementer should be working
proprely by setting it to 1 to get another exception?

In that case I will add a changelog and submit the patch.

Thanks,
Nick

Re: [PATCH] powerpc/pasemi, cbe: Do not process decremeter or external wakeup from powersave

[Michael Ellerman]

Michael Ellerman  writes:

> Nicholas Piggin  writes:
>
>> Hi,
>>
>> I would like to start using a dedicated stack for system reset interrupt
>> and treat it as a Linux nmi, which makes it tricky to call complex
>> interrupt handlers directly from the system reset trap handler.
>>
>> So I would like to remove the decrementer and external handler calls from
>> Cell and Pasemi platforms' system reset handler. I think we can just
>> remove them if they can be handled when they re-fire as normal interrupts?
>> At the moment I don't have environments set up to test if this works.
>
> My QS22 has booted OK with it applied, so it seems OK.
>
> I'll test it a bit more tomorrow.

OK, seems fine, and I have a trace that shows it's definitely going
through that path:


 1)   |.default_idle_call() {
 1)   |  .arch_cpu_idle() {
 1)   |.cbe_power_save() {
 1)   0.128 us|  .prep_irq_for_idle();
 1)   |  .system_reset_exception() {
 1)   0.512 us|.cbe_system_reset_exception();
 1)   6.016 us|  }
 1)   |  .do_IRQ() {
 1)   |.__do_irq() {
 1)   |  .irq_enter() {
 1)   0.704 us|.rcu_irq_enter();
 1)   |.tick_irq_enter() {
 1)   0.448 us|  .tick_check_oneshot_broadcast_this_cpu();
 1)   0.768 us|  .ktime_get();
 1)   |  .update_ts_time_stats() {
 1)   0.256 us|.nr_iowait_cpu();
 1)   5.632 us|  }
 1)   0.192 us|  .touch_softlockup_watchdog_sched();
 1) + 27.904 us   |}
 1)   |._local_bh_enable() {
 1)   0.320 us|  .__local_bh_enable();
 1)   5.760 us|}
 1)   1.024 us|.vtime_account_irq_enter();
 1) + 55.744 us   |  }
 1)   1.024 us|  .iic_get_irq();
 1)   |  .generic_handle_irq() {
 1)   |.handle_percpu_irq() {
 1)   |  .handle_irq_event_percpu() {
 1)   |.__handle_irq_event_percpu() {
 1)   |  .reschedule_action() {
 1)   |.scheduler_ipi() {
 1)   |  .irq_enter() {
 1)   0.640 us|.rcu_irq_enter();
 1)   0.384 us|.vtime_account_irq_enter();
 1) + 11.776 us   |  }
 1)   |  .sched_ttwu_pending() {
 1)   0.448 us|._raw_spin_lock_irqsave();
 1)   |.ttwu_do_activate() {
 1)   |  .activate_task() {
 1)   0.256 us|.update_rq_clock();
 1)   |.enqueue_task_fair() {
 1)   0.320 us|  .update_curr();
 1)   0.256 us|  
.__compute_runnable_contrib();
 1)   0.384 us|  .attach_entity_load_avg();
 1)   0.256 us|  .__enqueue_entity();
 1)   0.192 us|  .hrtick_update();
 1) + 30.592 us   |}
 1) + 41.216 us   |  }
 1)   |  .wq_worker_waking_up() {
 1)   0.576 us|.kthread_data();
 1)   6.464 us|  }
 1)   |  .ttwu_do_wakeup() {
 1)   |.check_preempt_curr() {
 1)   0.576 us|  .resched_curr();
 1)   6.272 us|}
 1) + 12.224 us   |  }
 1) + 75.904 us   |}
 1)   0.320 us|._raw_spin_unlock_irqrestore();
 1) + 92.736 us   |  }
 1)   |  .irq_exit() {
 1)   0.192 us|.idle_cpu();
 1)   0.256 us|.rcu_irq_exit();
 1) + 11.520 us   |  }
 1) ! 132.032 us  |}
 1) ! 137.280 us  |  }
 1) ! 142.784 us  |}
 1)   |.add_interrupt_randomness() {
 1)   0.704 us|  ._raw_spin_trylock();
 1)   |  .__mix_pool_bytes() {
 1)   1.152 us|._mix_pool_bytes();
 1)   6.464 us|  }
 1)   |  

Re: [PATCH] powerpc/pasemi, cbe: Do not process decremeter or external wakeup from powersave

[Michael Ellerman]

Nicholas Piggin  writes:

> Hi,
>
> I would like to start using a dedicated stack for system reset interrupt
> and treat it as a Linux nmi, which makes it tricky to call complex
> interrupt handlers directly from the system reset trap handler.
>
> So I would like to remove the decrementer and external handler calls from
> Cell and Pasemi platforms' system reset handler. I think we can just
> remove them if they can be handled when they re-fire as normal interrupts?
> At the moment I don't have environments set up to test if this works.

My QS22 has booted OK with it applied, so it seems OK.

I'll test it a bit more tomorrow.

cheers

[PATCH] powerpc/pasemi, cbe: Do not process decremeter or external wakeup from powersave

[Nicholas Piggin]

Hi,

I would like to start using a dedicated stack for system reset interrupt
and treat it as a Linux nmi, which makes it tricky to call complex
interrupt handlers directly from the system reset trap handler.

So I would like to remove the decrementer and external handler calls from
Cell and Pasemi platforms' system reset handler. I think we can just
remove them if they can be handled when they re-fire as normal interrupts?
At the moment I don't have environments set up to test if this works.

This will make the powersave wakeup a bit slower. We could add alternate
code to IDLETEST here to branch out to powersave wakeup handler like
powernv does. I haven't got a patch for that yet but I could help write
one if it is important and it can be tested.

Thanks,
Nick
---
 arch/powerpc/platforms/cell/pervasive.c | 11 +++
 arch/powerpc/platforms/pasemi/idle.c| 11 +++
 2 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/arch/powerpc/platforms/cell/pervasive.c 
b/arch/powerpc/platforms/cell/pervasive.c
index e7d075077cb0..a88944db9fc3 100644
--- a/arch/powerpc/platforms/cell/pervasive.c
+++ b/arch/powerpc/platforms/cell/pervasive.c
@@ -88,11 +88,14 @@ static void cbe_power_save(void)
 static int cbe_system_reset_exception(struct pt_regs *regs)
 {
switch (regs->msr & SRR1_WAKEMASK) {
-   case SRR1_WAKEEE:
-   do_IRQ(regs);
-   break;
case SRR1_WAKEDEC:
-   timer_interrupt(regs);
+   set_dec(1);
+   case SRR1_WAKEEE:
+   /*
+* Handle these when interrupts get re-enabled and we take
+* them as regular exceptions. We are in an NMI context
+* and can't handle these here.
+*/
break;
case SRR1_WAKEMT:
return cbe_sysreset_hack();
diff --git a/arch/powerpc/platforms/pasemi/idle.c 
b/arch/powerpc/platforms/pasemi/idle.c
index 75b296bc51af..44e0d9226f0a 100644
--- a/arch/powerpc/platforms/pasemi/idle.c
+++ b/arch/powerpc/platforms/pasemi/idle.c
@@ -53,11 +53,14 @@ static int pasemi_system_reset_exception(struct pt_regs 
*regs)
regs->nip = regs->link;
 
switch (regs->msr & SRR1_WAKEMASK) {
-   case SRR1_WAKEEE:
-   do_IRQ(regs);
-   break;
case SRR1_WAKEDEC:
-   timer_interrupt(regs);
+   set_dec(1);
+   case SRR1_WAKEEE:
+   /*
+* Handle these when interrupts get re-enabled and we take
+* them as regular exceptions. We are in an NMI context
+* and can't handle these here.
+*/
break;
default:
/* do system reset */
-- 
2.11.0