From: Nicholas Piggin <npiggin@gmail•com>
To: Christophe Leroy <christophe.leroy@c-s•fr>,
linuxppc-dev@lists•ozlabs.org
Cc: Mahesh Salgaonkar <mahesh@linux•vnet.ibm.com>,
Ganesh Goudar <ganeshgr@linux•ibm.com>,
Mahesh Salgaonkar <mahesh@linux•ibm.com>
Subject: Re: [PATCH v2 05/14] powerpc/pseries/ras: avoid calling rtas_token in NMI paths
Date: Tue, 07 Apr 2020 14:35:37 +1000 [thread overview]
Message-ID: <1586234078.wddist9jdn.astroid@bobo.none> (raw)
In-Reply-To: <26bde8af-b499-78f1-2f9a-91972ab343c6@c-s.fr>
Christophe Leroy's on April 4, 2020 12:30 am:
>
>
> Le 03/04/2020 à 15:26, Nicholas Piggin a écrit :
>> In the interest of reducing code and possible failures in the
>> machine check and system reset paths, grab the "ibm,nmi-interlock"
>> token at init time.
>>
>> Reviewed-by: Mahesh Salgaonkar <mahesh@linux•ibm.com>
>> Signed-off-by: Nicholas Piggin <npiggin@gmail•com>
>> ---
>> arch/powerpc/include/asm/firmware.h | 1 +
>> arch/powerpc/platforms/pseries/ras.c | 2 +-
>> arch/powerpc/platforms/pseries/setup.c | 13 ++++++++++---
>> 3 files changed, 12 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/firmware.h b/arch/powerpc/include/asm/firmware.h
>> index ca33f4ef6cb4..6003c2e533a0 100644
>> --- a/arch/powerpc/include/asm/firmware.h
>> +++ b/arch/powerpc/include/asm/firmware.h
>> @@ -128,6 +128,7 @@ extern void machine_check_fwnmi(void);
>>
>> /* This is true if we are using the firmware NMI handler (typically LPAR) */
>> extern int fwnmi_active;
>> +extern int ibm_nmi_interlock_token;
>>
>> extern unsigned int __start___fw_ftr_fixup, __stop___fw_ftr_fixup;
>>
>> diff --git a/arch/powerpc/platforms/pseries/ras.c b/arch/powerpc/platforms/pseries/ras.c
>> index 1d7f973c647b..c74d5e740922 100644
>> --- a/arch/powerpc/platforms/pseries/ras.c
>> +++ b/arch/powerpc/platforms/pseries/ras.c
>> @@ -458,7 +458,7 @@ static struct rtas_error_log *fwnmi_get_errinfo(struct pt_regs *regs)
>> */
>> static void fwnmi_release_errinfo(void)
>> {
>> - int ret = rtas_call(rtas_token("ibm,nmi-interlock"), 0, 1, NULL);
>> + int ret = rtas_call(ibm_nmi_interlock_token, 0, 1, NULL);
>> if (ret != 0)
>> printk(KERN_ERR "FWNMI: nmi-interlock failed: %d\n", ret);
>> }
>> diff --git a/arch/powerpc/platforms/pseries/setup.c b/arch/powerpc/platforms/pseries/setup.c
>> index 0c8421dd01ab..b582198be284 100644
>> --- a/arch/powerpc/platforms/pseries/setup.c
>> +++ b/arch/powerpc/platforms/pseries/setup.c
>> @@ -83,6 +83,7 @@ unsigned long CMO_PageSize = (ASM_CONST(1) << IOMMU_PAGE_SHIFT_4K);
>> EXPORT_SYMBOL(CMO_PageSize);
>>
>> int fwnmi_active; /* TRUE if an FWNMI handler is present */
>> +int ibm_nmi_interlock_token;
>>
>> static void pSeries_show_cpuinfo(struct seq_file *m)
>> {
>> @@ -113,9 +114,14 @@ static void __init fwnmi_init(void)
>> struct slb_entry *slb_ptr;
>> size_t size;
>> #endif
>> + int ibm_nmi_register_token;
>>
>> - int ibm_nmi_register = rtas_token("ibm,nmi-register");
>> - if (ibm_nmi_register == RTAS_UNKNOWN_SERVICE)
>> + ibm_nmi_register_token = rtas_token("ibm,nmi-register");
>> + if (ibm_nmi_register_token == RTAS_UNKNOWN_SERVICE)
>> + return;
>> +
>> + ibm_nmi_interlock_token = rtas_token("ibm,nmi-interlock");
>> + if (WARN_ON(ibm_nmi_interlock_token == RTAS_UNKNOWN_SERVICE))
>> return;
>>
>> /* If the kernel's not linked at zero we point the firmware at low
>> @@ -123,7 +129,8 @@ static void __init fwnmi_init(void)
>> system_reset_addr = __pa(system_reset_fwnmi) - PHYSICAL_START;
>> machine_check_addr = __pa(machine_check_fwnmi) - PHYSICAL_START;
>>
>> - if (0 == rtas_call(ibm_nmi_register, 2, 1, NULL, system_reset_addr,
>> + if (0 == rtas_call(ibm_nmi_register_token, 2, 1, NULL,
>> + system_reset_addr,
>> machine_check_addr))
>
> Alignment is wrong.
> And you could put system_reset_addr and
> machine_check_addr on the same line to limit the number of lines of the if.
I don't really like using spaces to align but I'll put it on the same
line.
Thanks,
Nick
next prev parent reply other threads:[~2020-04-07 4:38 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-04-03 13:26 [PATCH v2 00/14] powerpc/64: machine check and system reset fixes Nicholas Piggin
2020-04-03 13:26 ` [PATCH v2 01/14] powerpc/64s/exception: Fix machine check no-loss idle wakeup Nicholas Piggin
2020-04-03 13:26 ` [PATCH v2 02/14] powerpc/64s/exceptions: Fix in_mce accounting in unrecoverable path Nicholas Piggin
2020-04-03 13:26 ` [PATCH v2 03/14] powerpc/64s/exceptions: Change irq reconcile for NMIs from reusing _DAR to RESULT Nicholas Piggin
2020-04-03 13:26 ` [PATCH v2 04/14] powerpc/64s/exceptions: machine check reconcile irq state Nicholas Piggin
2020-04-03 13:26 ` [PATCH v2 05/14] powerpc/pseries/ras: avoid calling rtas_token in NMI paths Nicholas Piggin
2020-04-03 14:30 ` Christophe Leroy
2020-04-07 4:35 ` Nicholas Piggin [this message]
2020-04-03 13:26 ` [PATCH v2 06/14] powerpc/pseries/ras: FWNMI_VALID off by one Nicholas Piggin
2020-04-03 14:33 ` Christophe Leroy
2020-04-03 13:26 ` [PATCH v2 07/14] powerpc/pseries/ras: fwnmi avoid modifying r3 in error case Nicholas Piggin
2020-04-03 13:26 ` [PATCH v2 08/14] powerpc/pseries/ras: fwnmi sreset should not interlock Nicholas Piggin
2020-04-03 14:35 ` Christophe Leroy
2020-04-07 4:26 ` Nicholas Piggin
2020-04-03 13:26 ` [PATCH v2 09/14] powerpc/pseries: limit machine check stack to 4GB Nicholas Piggin
2020-04-03 14:19 ` Christophe Leroy
2020-04-07 4:31 ` Nicholas Piggin
2020-04-03 13:26 ` [PATCH v2 10/14] powerpc/pseries: machine check use rtas_call_unlocked with args on stack Nicholas Piggin
2020-04-03 13:26 ` [PATCH v2 11/14] powerpc/64s: machine check interrupt update NMI accounting Nicholas Piggin
2020-04-03 13:26 ` [PATCH v2 12/14] powerpc/64s: machine check do not trace real-mode handler Nicholas Piggin
2020-04-03 13:26 ` [PATCH v2 13/14] powerpc/64s: system reset do not trace Nicholas Piggin
2020-04-03 14:45 ` Christophe Leroy
2020-04-07 4:30 ` Nicholas Piggin
2020-04-04 21:40 ` kbuild test robot
2020-04-03 13:26 ` [PATCH v2 14/14] powerpc: make unrecoverable NMIs die instead of panic Nicholas Piggin
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=1586234078.wddist9jdn.astroid@bobo.none \
--to=npiggin@gmail$(echo .)com \
--cc=christophe.leroy@c-s$(echo .)fr \
--cc=ganeshgr@linux$(echo .)ibm.com \
--cc=linuxppc-dev@lists$(echo .)ozlabs.org \
--cc=mahesh@linux$(echo .)ibm.com \
--cc=mahesh@linux$(echo .)vnet.ibm.com \
/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