On 08/13/2018 07:57 PM, Nicholas Piggin wrote:
> On Mon, 13 Aug 2018 09:47:04 +0530
> Mahesh Jagannath Salgaonkar <mah...@linux.vnet.ibm.com> wrote:
>
>> On 08/11/2018 10:03 AM, Nicholas Piggin wrote:
>>> On Tue, 07 Aug 2018 19:47:39 +0530
>>> Mahesh J Salgaonkar <mah...@linux.vnet.ibm.com> wrote:
>>>
>>>> From: Mahesh Salgaonkar <mah...@linux.vnet.ibm.com>
>>>>
>>>> If we get a machine check exceptions due to SLB errors then dump the
>>>> current SLB contents which will be very much helpful in debugging the
>>>> root cause of SLB errors. Introduce an exclusive buffer per cpu to hold
>>>> faulty SLB entries. In real mode mce handler saves the old SLB contents
>>>> into this buffer accessible through paca and print it out later in virtual
>>>> mode.
>>>>
>>>> With this patch the console will log SLB contents like below on SLB MCE
>>>> errors:
>>>>
>>>> [ 507.297236] SLB contents of cpu 0x1
>>>> [ 507.297237] Last SLB entry inserted at slot 16
>>>> [ 507.297238] 00 c000000008000000 400ea1b217000500
>>>> [ 507.297239] 1T ESID= c00000 VSID= ea1b217 LLP:100
>>>> [ 507.297240] 01 d000000008000000 400d43642f000510
>>>> [ 507.297242] 1T ESID= d00000 VSID= d43642f LLP:110
>>>> [ 507.297243] 11 f000000008000000 400a86c85f000500
>>>> [ 507.297244] 1T ESID= f00000 VSID= a86c85f LLP:100
>>>> [ 507.297245] 12 00007f0008000000 4008119624000d90
>>>> [ 507.297246] 1T ESID= 7f VSID= 8119624 LLP:110
>>>> [ 507.297247] 13 0000000018000000 00092885f5150d90
>>>> [ 507.297247] 256M ESID= 1 VSID= 92885f5150 LLP:110
>>>> [ 507.297248] 14 0000010008000000 4009e7cb50000d90
>>>> [ 507.297249] 1T ESID= 1 VSID= 9e7cb50 LLP:110
>>>> [ 507.297250] 15 d000000008000000 400d43642f000510
>>>> [ 507.297251] 1T ESID= d00000 VSID= d43642f LLP:110
>>>> [ 507.297252] 16 d000000008000000 400d43642f000510
>>>> [ 507.297253] 1T ESID= d00000 VSID= d43642f LLP:110
>>>> [ 507.297253] ----------------------------------
>>>> [ 507.297254] SLB cache ptr value = 3
>>>> [ 507.297254] Valid SLB cache entries:
>>>> [ 507.297255] 00 EA[0-35]= 7f000
>>>> [ 507.297256] 01 EA[0-35]= 1
>>>> [ 507.297257] 02 EA[0-35]= 1000
>>>> [ 507.297257] Rest of SLB cache entries:
>>>> [ 507.297258] 03 EA[0-35]= 7f000
>>>> [ 507.297258] 04 EA[0-35]= 1
>>>> [ 507.297259] 05 EA[0-35]= 1000
>>>> [ 507.297260] 06 EA[0-35]= 12
>>>> [ 507.297260] 07 EA[0-35]= 7f000
>>>>
>>>> Suggested-by: Aneesh Kumar K.V <aneesh.ku...@linux.vnet.ibm.com>
>>>> Suggested-by: Michael Ellerman <m...@ellerman.id.au>
>>>> Signed-off-by: Mahesh Salgaonkar <mah...@linux.vnet.ibm.com>
>>>> ---
>>>>
>>>> Changes in V7:
>>>> - Print slb cache ptr value and slb cache data
>>>> ---
>>>> arch/powerpc/include/asm/book3s/64/mmu-hash.h | 7 ++
>>>> arch/powerpc/include/asm/paca.h | 4 +
>>>> arch/powerpc/mm/slb.c | 73
>>>> +++++++++++++++++++++++++
>>>> arch/powerpc/platforms/pseries/ras.c | 10 +++
>>>> arch/powerpc/platforms/pseries/setup.c | 10 +++
>>>> 5 files changed, 103 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/arch/powerpc/include/asm/book3s/64/mmu-hash.h
>>>> b/arch/powerpc/include/asm/book3s/64/mmu-hash.h
>>>> index cc00a7088cf3..5a3fe282076d 100644
>>>> --- a/arch/powerpc/include/asm/book3s/64/mmu-hash.h
>>>> +++ b/arch/powerpc/include/asm/book3s/64/mmu-hash.h
>>>> @@ -485,9 +485,16 @@ static inline void hpte_init_pseries(void) { }
>>>>
>>>> extern void hpte_init_native(void);
>>>>
>>>> +struct slb_entry {
>>>> + u64 esid;
>>>> + u64 vsid;
>>>> +};
>>>> +
>>>> extern void slb_initialize(void);
>>>> extern void slb_flush_and_rebolt(void);
>>>> extern void slb_flush_and_rebolt_realmode(void);
>>>> +extern void slb_save_contents(struct slb_entry *slb_ptr);
>>>> +extern void slb_dump_contents(struct slb_entry *slb_ptr);
>>>>
>>>> extern void slb_vmalloc_update(void);
>>>> extern void slb_set_size(u16 size);
>>>> diff --git a/arch/powerpc/include/asm/paca.h
>>>> b/arch/powerpc/include/asm/paca.h
>>>> index 7f22929ce915..233d25ff6f64 100644
>>>> --- a/arch/powerpc/include/asm/paca.h
>>>> +++ b/arch/powerpc/include/asm/paca.h
>>>> @@ -254,6 +254,10 @@ struct paca_struct {
>>>> #endif
>>>> #ifdef CONFIG_PPC_PSERIES
>>>> u8 *mce_data_buf; /* buffer to hold per cpu rtas errlog */
>>>> +
>>>> + /* Capture SLB related old contents in MCE handler. */
>>>> + struct slb_entry *mce_faulty_slbs;
>>>> + u16 slb_save_cache_ptr;
>>>> #endif /* CONFIG_PPC_PSERIES */
>>>> } ____cacheline_aligned;
>>>>
>>>> diff --git a/arch/powerpc/mm/slb.c b/arch/powerpc/mm/slb.c
>>>> index e89f675f1b5e..16a53689ffd4 100644
>>>> --- a/arch/powerpc/mm/slb.c
>>>> +++ b/arch/powerpc/mm/slb.c
>>>> @@ -151,6 +151,79 @@ void slb_flush_and_rebolt_realmode(void)
>>>> get_paca()->slb_cache_ptr = 0;
>>>> }
>>>>
>>>> +void slb_save_contents(struct slb_entry *slb_ptr)
>>>> +{
>>>> + int i;
>>>> + unsigned long e, v;
>>>> +
>>>> + /* Save slb_cache_ptr value. */
>>>> + get_paca()->slb_save_cache_ptr = get_paca()->slb_cache_ptr;
>>>
>>> What's the point of saving this?
>>
>> This is to know how many valid cache entries were present at the time of
>> SLB mutlihit. We use this index value while dumping the slb cahce entries.
>
> Oh I see you're dumping that thing as well. I don't know if that's
> worth doing, it just gives you the first 8 SLB entries installed but
> you already have those (or they're overwritten and irrelevat).
Aneesh, Can you comment on this ?
>
>>
>>>
>>>> +
>>>> + if (!slb_ptr)
>>>> + return;
>>>
>>> Can this ever happen?
>>
>> May be Never. We allocate the memory at very early stage. But just added
>> as sanity check.
>
> Okay if you think it's needed.
>
>>
>>>
>>>> +
>>>> + for (i = 0; i < mmu_slb_size; i++) {
>>>> + asm volatile("slbmfee %0,%1" : "=r" (e) : "r" (i));
>>>> + asm volatile("slbmfev %0,%1" : "=r" (v) : "r" (i));
>>>
>>> Does the UM say these instructions can cause machine checks if the SLB
>>> is corrupted? It talks about mfslb instruction causing MCE, but there
>>> seems to be no such instruction so I wonder if that's a typo for slbmf?
>>>
>>> Seems like a parity error in the SLB should cause a MCE, at least,
>>> because it can't guarantee valid data for the instruction in that case
>>> (multi-hit may be different because you aren't searching by EA).
>>>
>>> You could limit slb saving to a single level of recursion to avoid
>>> the problem.
>>
>> Yeah, we could do this OR restrict slb saving only for SLB multi-hit.
>> Parity errors are anyway hardware errors. If parity error is transient
>> then saving of SLBs may not trigger another MCE. In that case old SLB
>> content would look ok even if we dump them on console. What do you say ?
>
> I'm not sure. A parity error I think can cause a multi hit. Can you be
> sure of a software caused multi hit? Would be a good idea if you can I
> think. It may be a good idea to avoid recursion as well, just in case.
yeah, you are right. Parity errors can also cause multi-hit. Will limit
slb saving to single level of recursion.
Thanks for your review.
-Mahesh.
>
> Thanks,
> Nick
>
