Christophe Leroy's on April 4, 2020 12:35 am:
>
>
> Le 03/04/2020 à 15:26, Nicholas Piggin a écrit :
>> PAPR does not specify that fwnmi sreset should be interlocked, and
>> PowerVM (and therefore now QEMU) do not require it.
>>
>> These "ibm,nmi-interlock" calls are ignored by firmware, but there
>> is a possibility that the sreset could have interrupted a machine
>> check and release the machine check's interlock too early, corrupting
>> it if another machine check came in.
>>
>> This is an extremely rare case, but it should be fixed for clarity
>> and reducing the code executed in the sreset path. Firmware also
>> does not provide error information for the sreset case to look at, so
>> remove that comment.
>>
>> Signed-off-by: Nicholas Piggin <npig...@gmail.com>
>> ---
>> arch/powerpc/platforms/pseries/ras.c | 48 ++++++++++++++++++++--------
>> 1 file changed, 34 insertions(+), 14 deletions(-)
>>
>> diff --git a/arch/powerpc/platforms/pseries/ras.c
>> b/arch/powerpc/platforms/pseries/ras.c
>> index a40598e6e525..833ae34b7fec 100644
>> --- a/arch/powerpc/platforms/pseries/ras.c
>> +++ b/arch/powerpc/platforms/pseries/ras.c
>> @@ -406,6 +406,20 @@ static inline struct rtas_error_log
>> *fwnmi_get_errlog(void)
>> return (struct rtas_error_log *)local_paca->mce_data_buf;
>> }
>>
>> +static unsigned long *fwnmi_get_savep(struct pt_regs *regs)
>> +{
>> + unsigned long savep_ra;
>> +
>> + /* Mask top two bits */
>> + savep_ra = regs->gpr[3] & ~(0x3UL << 62);
>> + if (!VALID_FWNMI_BUFFER(savep_ra)) {
>> + printk(KERN_ERR "FWNMI: corrupt r3 0x%016lx\n", regs->gpr[3]);
>
> Can't you use pr_err() instead ?
I think so.
>> + return NULL;
>> + }
>> +
>> + return __va(savep_ra);
>> +}
>> +
>> /*
>> * Get the error information for errors coming through the
>> * FWNMI vectors. The pt_regs' r3 will be updated to reflect
>> @@ -423,20 +437,15 @@ static inline struct rtas_error_log
>> *fwnmi_get_errlog(void)
>> */
>> static struct rtas_error_log *fwnmi_get_errinfo(struct pt_regs *regs)
>> {
>> - unsigned long savep_ra;
>> unsigned long *savep;
>> struct rtas_error_log *h;
>>
>> - /* Mask top two bits */
>> - savep_ra = regs->gpr[3] & ~(0x3UL << 62);
>> -
>> - if (!VALID_FWNMI_BUFFER(savep_ra)) {
>> - printk(KERN_ERR "FWNMI: corrupt r3 0x%016lx\n", regs->gpr[3]);
>> + savep = fwnmi_get_savep(regs);
>> + if (!savep)
>> return NULL;
>> - }
>>
>> - savep = __va(savep_ra);
>> - regs->gpr[3] = be64_to_cpu(savep[0]); /* restore original r3 */
>> + /* restore original r3 */
>> + regs->gpr[3] = be64_to_cpu(savep[0]);
>
> Is it needed to change the location of the comment ?
No, I originally had other changes I think. Will fix.
Thanks,
Nick
