From: Nicholas Piggin <npiggin@gmail•com>
To: Christophe Leroy <christophe.leroy@csgroup•eu>,
linuxppc-dev@lists•ozlabs.org
Subject: Re: [PATCH 18/18] powerpc/64s: move power4 idle entirely to C
Date: Tue, 10 Nov 2020 18:43:48 +1000 [thread overview]
Message-ID: <1604997329.cf7j9ms575.astroid@bobo.none> (raw)
In-Reply-To: <7de6fd21-da79-fe8f-5db4-f99ee0dd7d23@csgroup.eu>
Excerpts from Christophe Leroy's message of November 7, 2020 7:43 pm:
>
>
> Le 05/11/2020 à 15:34, Nicholas Piggin a écrit :
>> Christophe asked about doing this, most of the code is still in
>> asm but maybe it's slightly nicer? I don't know if it's worthwhile.
>
> Heu... I don't think I was asking for that, but why not, see later comments.
>
> At first I was just asking to write the following in C:
>
> +
> + .globl power4_idle_nap_return
> +power4_idle_nap_return:
> + blr
>
>
> In extenso, instead of the above do somewhere something like:
>
> void power4_idle_nap_return(void)
> {
> }
Ah! Well either was a good question. I don't mind attempting it :)
>> ---
>> arch/powerpc/kernel/idle.c | 25 ++++++++++++++++++++-----
>> arch/powerpc/kernel/idle_book3s.S | 22 ----------------------
>> 2 files changed, 20 insertions(+), 27 deletions(-)
>>
>> diff --git a/arch/powerpc/kernel/idle.c b/arch/powerpc/kernel/idle.c
>> index ae0e2632393d..849e77a45915 100644
>> --- a/arch/powerpc/kernel/idle.c
>> +++ b/arch/powerpc/kernel/idle.c
>> @@ -72,6 +72,9 @@ int powersave_nap;
>> #ifdef CONFIG_PPC_970_NAP
>> void power4_idle(void)
>> {
>> + unsigned long msr_idle = MSR_KERNEL|MSR_EE|MSR_POW;
>> + unsigned long tmp1, tmp2;
>> +
>> if (!cpu_has_feature(CPU_FTR_CAN_NAP))
>> return;
>>
>> @@ -84,13 +87,25 @@ void power4_idle(void)
>> if (cpu_has_feature(CPU_FTR_ALTIVEC))
>> asm volatile("DSSALL ; sync" ::: "memory");
>>
>> - power4_idle_nap();
>> -
>> + asm volatile(
>> +" ld %0,PACA_THREAD_INFO(r13) \n"
>> +" ld %1,TI_LOCAL_FLAGS(%0) \n"
>> +" ori %1,%1,_TLF_NAPPING \n"
>> +" std %1,TI_LOCAL_FLAGS(%0) \n"
>
> Can't this just be:
>
> current_thread_info()->local_flags |= _TLF_NAPPING;
>
>> /*
>> - * power4_idle_nap returns with interrupts enabled (soft and hard).
>> - * to our caller with interrupts enabled (soft and hard). Our caller
>> - * can cope with either interrupts disabled or enabled upon return.
>> + * NAPPING bit is set, from this point onward nap_adjust_return()
>> + * will cause interrupts to return to power4_idle_nap_return.
>> */
>> +"1: sync \n"
>> +" isync \n"
>> +" mtmsrd %2 \n"
>> +" isync \n"
>> +" b 1b \n"
>
> And this:
>
> for (;;) {
> mb();
> isync();
> mtmsr(MSR_KERNEL|MSR_EE|MSR_POW);
> isync();
> }
I was hoping something nicer like this but I think not because as soon
as we set _TLF_NAPPING, we might take an interrupt which returns
somewhere else, and you aren't allowed to do that in C code (mainly
because the stack and register state would be unknown). Even going
immediately to blr or end of function might miss restoring NVGPRs etc.
There might be some tricks we could play with soft-masking interrupts,
using MSR[EE]=0, and then doing all this and returning to right after
the mtmsr POW with a flag set... But it's a bit of tricky churn for an
old CPU that works okay.
Thanks,
Nick
>
>
>> +" .globl power4_idle_nap_return \n"
>> +"power4_idle_nap_return: \n"
>> + : "=r"(tmp1), "=r"(tmp2)
>> + : "r"(msr_idle)
>> + );
>> }
>> #endif
>>
>
> Christophe
>
prev parent reply other threads:[~2020-11-10 8:45 UTC|newest]
Thread overview: 33+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-11-05 14:34 [PATCH 00/18] powerpc: interrupt wrappers Nicholas Piggin
2020-11-05 14:34 ` [PATCH 01/18] powerpc/64s: move the last of the page fault handling logic to C Nicholas Piggin
2020-11-05 21:54 ` kernel test robot
2020-11-05 14:34 ` [PATCH 02/18] powerpc: remove arguments from fault handler functions Nicholas Piggin
2020-11-06 7:59 ` Christophe Leroy
2020-11-10 8:29 ` Nicholas Piggin
2020-11-10 11:15 ` Christophe Leroy
2020-11-11 4:45 ` Nicholas Piggin
2020-11-05 14:34 ` [PATCH 03/18] powerpc: bad_page_fault, do_break get registers from regs Nicholas Piggin
2020-11-05 20:43 ` kernel test robot
2020-11-06 8:14 ` Christophe Leroy
2020-11-10 8:34 ` Nicholas Piggin
2020-11-10 11:19 ` Christophe Leroy
2020-11-11 4:46 ` Nicholas Piggin
2020-11-11 6:39 ` Nicholas Piggin
2020-11-05 14:34 ` [PATCH 04/18] powerpc: interrupt handler wrapper functions Nicholas Piggin
2020-11-05 14:34 ` [PATCH 05/18] powerpc: add interrupt wrapper entry / exit stub functions Nicholas Piggin
2020-11-05 14:34 ` [PATCH 06/18] powerpc: add interrupt_cond_local_irq_enable helper Nicholas Piggin
2020-11-05 14:34 ` [PATCH 07/18] powerpc/64: context tracking remove _TIF_NOHZ Nicholas Piggin
2020-11-05 14:34 ` [PATCH 08/18] powerpc/64: context tracking move to interrupt wrappers Nicholas Piggin
2020-11-05 14:34 ` [PATCH 09/18] powerpc/64: add context tracking to asynchronous interrupts Nicholas Piggin
2020-11-05 14:34 ` [PATCH 10/18] powerpc/64s: move context tracking exit to interrupt exit path Nicholas Piggin
2020-11-05 14:34 ` [PATCH 11/18] powerpc/64s: reconcile interrupts in C Nicholas Piggin
2020-11-05 14:34 ` [PATCH 12/18] powerpc/64: move account_stolen_time into its own function Nicholas Piggin
2020-11-05 14:34 ` [PATCH 13/18] powerpc/64: entry cpu time accounting in C Nicholas Piggin
2020-11-05 14:34 ` [PATCH 14/18] powerpc: move NMI entry/exit code into wrapper Nicholas Piggin
2020-11-05 14:34 ` [PATCH 15/18] powerpc/64s: move NMI soft-mask handling to C Nicholas Piggin
2020-11-05 14:34 ` [PATCH 16/18] powerpc/64s: runlatch interrupt handling in C Nicholas Piggin
2020-11-05 14:34 ` [PATCH 17/18] powerpc/64s: power4 nap fixup " Nicholas Piggin
2020-11-05 14:34 ` [PATCH 18/18] powerpc/64s: move power4 idle entirely to C Nicholas Piggin
2020-11-06 7:34 ` kernel test robot
2020-11-07 9:43 ` Christophe Leroy
2020-11-10 8:43 ` Nicholas Piggin [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1604997329.cf7j9ms575.astroid@bobo.none \
--to=npiggin@gmail$(echo .)com \
--cc=christophe.leroy@csgroup$(echo .)eu \
--cc=linuxppc-dev@lists$(echo .)ozlabs.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox