Re: [PATCH v8 3/7] powerpc/mce: Fix MCE handling for huge pages
Mahesh Jagannath Salgaonkar writes: > On 8/7/19 8:26 PM, Santosh Sivaraj wrote: >> From: Balbir Singh >> >> The current code would fail on huge pages addresses, since the shift would >> be incorrect. Use the correct page shift value returned by >> __find_linux_pte() to get the correct physical address. The code is more >> generic and can handle both regular and compound pages. >> >> Fixes: ba41e1e1ccb9 ("powerpc/mce: Hookup derror (load/store) UE errors") >> Signed-off-by: Balbir Singh >> [ar...@linux.ibm.com: Fixup pseries_do_memory_failure()] >> Signed-off-by: Reza Arbab >> Co-developed-by: Santosh Sivaraj >> Signed-off-by: Santosh Sivaraj >> --- >> arch/powerpc/include/asm/mce.h | 2 +- >> arch/powerpc/kernel/mce_power.c | 50 ++-- >> arch/powerpc/platforms/pseries/ras.c | 9 ++--- >> 3 files changed, 29 insertions(+), 32 deletions(-) >> >> diff --git a/arch/powerpc/include/asm/mce.h b/arch/powerpc/include/asm/mce.h >> index a4c6a74ad2fb..f3a6036b6bc0 100644 >> --- a/arch/powerpc/include/asm/mce.h >> +++ b/arch/powerpc/include/asm/mce.h >> @@ -209,7 +209,7 @@ extern void release_mce_event(void); >> extern void machine_check_queue_event(void); >> extern void machine_check_print_event_info(struct machine_check_event *evt, >> bool user_mode, bool in_guest); >> -unsigned long addr_to_pfn(struct pt_regs *regs, unsigned long addr); >> +unsigned long addr_to_phys(struct pt_regs *regs, unsigned long addr); >> #ifdef CONFIG_PPC_BOOK3S_64 >> void flush_and_reload_slb(void); >> #endif /* CONFIG_PPC_BOOK3S_64 */ >> diff --git a/arch/powerpc/kernel/mce_power.c >> b/arch/powerpc/kernel/mce_power.c >> index a814d2dfb5b0..bed38a8e2e50 100644 >> --- a/arch/powerpc/kernel/mce_power.c >> +++ b/arch/powerpc/kernel/mce_power.c >> @@ -20,13 +20,14 @@ >> #include >> >> /* >> - * Convert an address related to an mm to a PFN. NOTE: we are in real >> - * mode, we could potentially race with page table updates. >> + * Convert an address related to an mm to a physical address. >> + * NOTE: we are in real mode, we could potentially race with page table >> updates. >> */ >> -unsigned long addr_to_pfn(struct pt_regs *regs, unsigned long addr) >> +unsigned long addr_to_phys(struct pt_regs *regs, unsigned long addr) >> { >> -pte_t *ptep; >> -unsigned long flags; >> +pte_t *ptep, pte; >> +unsigned int shift; >> +unsigned long flags, phys_addr; >> struct mm_struct *mm; >> >> if (user_mode(regs)) >> @@ -35,14 +36,21 @@ unsigned long addr_to_pfn(struct pt_regs *regs, unsigned >> long addr) >> mm = _mm; >> >> local_irq_save(flags); >> -if (mm == current->mm) >> -ptep = find_current_mm_pte(mm->pgd, addr, NULL, NULL); >> -else >> -ptep = find_init_mm_pte(addr, NULL); >> +ptep = __find_linux_pte(mm->pgd, addr, NULL, ); >> local_irq_restore(flags); >> + >> if (!ptep || pte_special(*ptep)) >> return ULONG_MAX; >> -return pte_pfn(*ptep); >> + >> +pte = *ptep; >> +if (shift > PAGE_SHIFT) { >> +unsigned long rpnmask = (1ul << shift) - PAGE_SIZE; >> + >> +pte = __pte(pte_val(pte) | (addr & rpnmask)); >> +} >> +phys_addr = pte_pfn(pte) << PAGE_SHIFT; >> + >> +return phys_addr; >> } >> >> /* flush SLBs and reload */ >> @@ -354,18 +362,16 @@ static int mce_find_instr_ea_and_pfn(struct pt_regs >> *regs, uint64_t *addr, > > Now that we have addr_to_phys() can we change this function name as well > to mce_find_instr_ea_and_phys() ? Makes sense, will avoid confusions. > > Tested-by: Mahesh Salgaonkar > > This should go to stable tree. Can you move this patch to 2nd position ? Thanks for testing. Sure. Will reorder and mark stable as well. Thanks, Santosh > > Thanks, > -Mahesh. >
Re: [PATCH v8 3/7] powerpc/mce: Fix MCE handling for huge pages
On 8/7/19 8:26 PM, Santosh Sivaraj wrote: > From: Balbir Singh > > The current code would fail on huge pages addresses, since the shift would > be incorrect. Use the correct page shift value returned by > __find_linux_pte() to get the correct physical address. The code is more > generic and can handle both regular and compound pages. > > Fixes: ba41e1e1ccb9 ("powerpc/mce: Hookup derror (load/store) UE errors") > Signed-off-by: Balbir Singh > [ar...@linux.ibm.com: Fixup pseries_do_memory_failure()] > Signed-off-by: Reza Arbab > Co-developed-by: Santosh Sivaraj > Signed-off-by: Santosh Sivaraj > --- > arch/powerpc/include/asm/mce.h | 2 +- > arch/powerpc/kernel/mce_power.c | 50 ++-- > arch/powerpc/platforms/pseries/ras.c | 9 ++--- > 3 files changed, 29 insertions(+), 32 deletions(-) > > diff --git a/arch/powerpc/include/asm/mce.h b/arch/powerpc/include/asm/mce.h > index a4c6a74ad2fb..f3a6036b6bc0 100644 > --- a/arch/powerpc/include/asm/mce.h > +++ b/arch/powerpc/include/asm/mce.h > @@ -209,7 +209,7 @@ extern void release_mce_event(void); > extern void machine_check_queue_event(void); > extern void machine_check_print_event_info(struct machine_check_event *evt, > bool user_mode, bool in_guest); > -unsigned long addr_to_pfn(struct pt_regs *regs, unsigned long addr); > +unsigned long addr_to_phys(struct pt_regs *regs, unsigned long addr); > #ifdef CONFIG_PPC_BOOK3S_64 > void flush_and_reload_slb(void); > #endif /* CONFIG_PPC_BOOK3S_64 */ > diff --git a/arch/powerpc/kernel/mce_power.c b/arch/powerpc/kernel/mce_power.c > index a814d2dfb5b0..bed38a8e2e50 100644 > --- a/arch/powerpc/kernel/mce_power.c > +++ b/arch/powerpc/kernel/mce_power.c > @@ -20,13 +20,14 @@ > #include > > /* > - * Convert an address related to an mm to a PFN. NOTE: we are in real > - * mode, we could potentially race with page table updates. > + * Convert an address related to an mm to a physical address. > + * NOTE: we are in real mode, we could potentially race with page table > updates. > */ > -unsigned long addr_to_pfn(struct pt_regs *regs, unsigned long addr) > +unsigned long addr_to_phys(struct pt_regs *regs, unsigned long addr) > { > - pte_t *ptep; > - unsigned long flags; > + pte_t *ptep, pte; > + unsigned int shift; > + unsigned long flags, phys_addr; > struct mm_struct *mm; > > if (user_mode(regs)) > @@ -35,14 +36,21 @@ unsigned long addr_to_pfn(struct pt_regs *regs, unsigned > long addr) > mm = _mm; > > local_irq_save(flags); > - if (mm == current->mm) > - ptep = find_current_mm_pte(mm->pgd, addr, NULL, NULL); > - else > - ptep = find_init_mm_pte(addr, NULL); > + ptep = __find_linux_pte(mm->pgd, addr, NULL, ); > local_irq_restore(flags); > + > if (!ptep || pte_special(*ptep)) > return ULONG_MAX; > - return pte_pfn(*ptep); > + > + pte = *ptep; > + if (shift > PAGE_SHIFT) { > + unsigned long rpnmask = (1ul << shift) - PAGE_SIZE; > + > + pte = __pte(pte_val(pte) | (addr & rpnmask)); > + } > + phys_addr = pte_pfn(pte) << PAGE_SHIFT; > + > + return phys_addr; > } > > /* flush SLBs and reload */ > @@ -354,18 +362,16 @@ static int mce_find_instr_ea_and_pfn(struct pt_regs > *regs, uint64_t *addr, Now that we have addr_to_phys() can we change this function name as well to mce_find_instr_ea_and_phys() ? Tested-by: Mahesh Salgaonkar This should go to stable tree. Can you move this patch to 2nd position ? Thanks, -Mahesh.
[PATCH v8 3/7] powerpc/mce: Fix MCE handling for huge pages
From: Balbir Singh The current code would fail on huge pages addresses, since the shift would be incorrect. Use the correct page shift value returned by __find_linux_pte() to get the correct physical address. The code is more generic and can handle both regular and compound pages. Fixes: ba41e1e1ccb9 ("powerpc/mce: Hookup derror (load/store) UE errors") Signed-off-by: Balbir Singh [ar...@linux.ibm.com: Fixup pseries_do_memory_failure()] Signed-off-by: Reza Arbab Co-developed-by: Santosh Sivaraj Signed-off-by: Santosh Sivaraj --- arch/powerpc/include/asm/mce.h | 2 +- arch/powerpc/kernel/mce_power.c | 50 ++-- arch/powerpc/platforms/pseries/ras.c | 9 ++--- 3 files changed, 29 insertions(+), 32 deletions(-) diff --git a/arch/powerpc/include/asm/mce.h b/arch/powerpc/include/asm/mce.h index a4c6a74ad2fb..f3a6036b6bc0 100644 --- a/arch/powerpc/include/asm/mce.h +++ b/arch/powerpc/include/asm/mce.h @@ -209,7 +209,7 @@ extern void release_mce_event(void); extern void machine_check_queue_event(void); extern void machine_check_print_event_info(struct machine_check_event *evt, bool user_mode, bool in_guest); -unsigned long addr_to_pfn(struct pt_regs *regs, unsigned long addr); +unsigned long addr_to_phys(struct pt_regs *regs, unsigned long addr); #ifdef CONFIG_PPC_BOOK3S_64 void flush_and_reload_slb(void); #endif /* CONFIG_PPC_BOOK3S_64 */ diff --git a/arch/powerpc/kernel/mce_power.c b/arch/powerpc/kernel/mce_power.c index a814d2dfb5b0..bed38a8e2e50 100644 --- a/arch/powerpc/kernel/mce_power.c +++ b/arch/powerpc/kernel/mce_power.c @@ -20,13 +20,14 @@ #include /* - * Convert an address related to an mm to a PFN. NOTE: we are in real - * mode, we could potentially race with page table updates. + * Convert an address related to an mm to a physical address. + * NOTE: we are in real mode, we could potentially race with page table updates. */ -unsigned long addr_to_pfn(struct pt_regs *regs, unsigned long addr) +unsigned long addr_to_phys(struct pt_regs *regs, unsigned long addr) { - pte_t *ptep; - unsigned long flags; + pte_t *ptep, pte; + unsigned int shift; + unsigned long flags, phys_addr; struct mm_struct *mm; if (user_mode(regs)) @@ -35,14 +36,21 @@ unsigned long addr_to_pfn(struct pt_regs *regs, unsigned long addr) mm = _mm; local_irq_save(flags); - if (mm == current->mm) - ptep = find_current_mm_pte(mm->pgd, addr, NULL, NULL); - else - ptep = find_init_mm_pte(addr, NULL); + ptep = __find_linux_pte(mm->pgd, addr, NULL, ); local_irq_restore(flags); + if (!ptep || pte_special(*ptep)) return ULONG_MAX; - return pte_pfn(*ptep); + + pte = *ptep; + if (shift > PAGE_SHIFT) { + unsigned long rpnmask = (1ul << shift) - PAGE_SIZE; + + pte = __pte(pte_val(pte) | (addr & rpnmask)); + } + phys_addr = pte_pfn(pte) << PAGE_SHIFT; + + return phys_addr; } /* flush SLBs and reload */ @@ -354,18 +362,16 @@ static int mce_find_instr_ea_and_pfn(struct pt_regs *regs, uint64_t *addr, * faults */ int instr; - unsigned long pfn, instr_addr; + unsigned long instr_addr; struct instruction_op op; struct pt_regs tmp = *regs; - pfn = addr_to_pfn(regs, regs->nip); - if (pfn != ULONG_MAX) { - instr_addr = (pfn << PAGE_SHIFT) + (regs->nip & ~PAGE_MASK); + instr_addr = addr_to_phys(regs, regs->nip) + (regs->nip & ~PAGE_MASK); + if (instr_addr != ULONG_MAX) { instr = *(unsigned int *)(instr_addr); if (!analyse_instr(, , instr)) { - pfn = addr_to_pfn(regs, op.ea); *addr = op.ea; - *phys_addr = (pfn << PAGE_SHIFT); + *phys_addr = addr_to_phys(regs, op.ea); return 0; } /* @@ -440,15 +446,9 @@ static int mce_handle_ierror(struct pt_regs *regs, *addr = regs->nip; if (mce_err->sync_error && table[i].error_type == MCE_ERROR_TYPE_UE) { - unsigned long pfn; - - if (get_paca()->in_mce < MAX_MCE_DEPTH) { - pfn = addr_to_pfn(regs, regs->nip); - if (pfn != ULONG_MAX) { - *phys_addr = - (pfn << PAGE_SHIFT); - } - } + if (get_paca()->in_mce < MAX_MCE_DEPTH) + *phys_addr = addr_to_phys(regs, +