On Mon, 13 Aug 2018 09:57:33 +0530
Mahesh Jagannath Salgaonkar <mahesh@xxxxxxxxxxxxxxxxxx> wrote:
> On 08/10/2018 12:12 PM, Nicholas Piggin wrote:
> > The machine check code that flushes and restores bolted segments in
> > real mode belongs in mm/slb.c. This will also be used by pseries
> > machine check and idle code in future changes.
> >
> > Signed-off-by: Nicholas Piggin <npiggin@xxxxxxxxx>
> >
> > Since v1:
> > - Restore the test for slb_shadow (mpe)
> > ---
> > arch/powerpc/include/asm/book3s/64/mmu-hash.h | 3 ++
> > arch/powerpc/kernel/mce_power.c | 26 +++++--------
> > arch/powerpc/mm/slb.c | 39 +++++++++++++++++++
> > 3 files changed, 51 insertions(+), 17 deletions(-)
> >
> > diff --git a/arch/powerpc/include/asm/book3s/64/mmu-hash.h b/arch/powerpc/include/asm/book3s/64/mmu-hash.h
> > index 2f74bdc805e0..d4e398185b3a 100644
> > --- a/arch/powerpc/include/asm/book3s/64/mmu-hash.h
> > +++ b/arch/powerpc/include/asm/book3s/64/mmu-hash.h
> > @@ -497,6 +497,9 @@ extern void hpte_init_native(void);
> >
> > extern void slb_initialize(void);
> > extern void slb_flush_and_rebolt(void);
> > +extern void slb_flush_all_realmode(void);
> > +extern void __slb_restore_bolted_realmode(void);
> > +extern void slb_restore_bolted_realmode(void);
> >
> > extern void slb_vmalloc_update(void);
> > extern void slb_set_size(u16 size);
> > diff --git a/arch/powerpc/kernel/mce_power.c b/arch/powerpc/kernel/mce_power.c
> > index d6756af6ec78..3497c8329c1d 100644
> > --- a/arch/powerpc/kernel/mce_power.c
> > +++ b/arch/powerpc/kernel/mce_power.c
> > @@ -62,11 +62,8 @@ static unsigned long addr_to_pfn(struct pt_regs *regs, unsigned long addr)
> > #ifdef CONFIG_PPC_BOOK3S_64
> > static void flush_and_reload_slb(void)
> > {
> > - struct slb_shadow *slb;
> > - unsigned long i, n;
> > -
> > /* Invalidate all SLBs */
> > - asm volatile("slbmte %0,%0; slbia" : : "r" (0));
> > + slb_flush_all_realmode();
> >
> > #ifdef CONFIG_KVM_BOOK3S_HANDLER
> > /*
> > @@ -76,22 +73,17 @@ static void flush_and_reload_slb(void)
> > if (get_paca()->kvm_hstate.in_guest)
> > return;
> > #endif
> > -
> > - /* For host kernel, reload the SLBs from shadow SLB buffer. */
> > - slb = get_slb_shadow();
> > - if (!slb)
> > + if (early_radix_enabled())
> > return;
>
> Would we ever get MCE for SLB errors when radix is enabled ?
Well I'm not 100% sure. I don't think the MMU should in radix mode,
but KVM will put guests into HPT mode and put entries into the SLB.
I'm not completely sure we would never get a MCE come through here.
>
> >
> > - n = min_t(u32, be32_to_cpu(slb->persistent), SLB_MIN_SIZE);
> > -
> > - /* Load up the SLB entries from shadow SLB */
> > - for (i = 0; i < n; i++) {
> > - unsigned long rb = be64_to_cpu(slb->save_area[i].esid);
> > - unsigned long rs = be64_to_cpu(slb->save_area[i].vsid);
> > + /*
> > + * This probably shouldn't happen, but it may be possible it's
> > + * called in early boot before SLB shadows are allocated.
> > + */
> > + if (!get_slb_shadow())
> > + return;
>
> Any reason you added above check here instead on mm/slb.c ? Should we
> move above check inside slb_restore_bolted_realmode() ? I guess mm/slb.c
> is right place for this check. This will also help pseries machine check
> to avoid calling this extra check explicitly.
I thought it was a corner case because the slb.c code should not
be called before it's initialised. I'd prefer these exceptional
machine check cases be tested in the MCE code. Anything else calling
flush_and_reload_slb so early would be a bad bug.
Thanks,
Nick
