On Mon, 2020-02-17 at 07:40 +0100, Christophe Leroy wrote: > > Le 16/02/2020 à 23:40, Michael Neuling a écrit : > > On Fri, 2020-02-14 at 08:33 +0000, Christophe Leroy wrote: > > > With CONFIG_VMAP_STACK, data MMU has to be enabled > > > to read data on the stack. > > > > Can you describe what goes wrong without this? Some oops message? rtas blows > > up? > > Get corrupt data? > > Larry reported a machine check. Or in fact, he reported a Oops in > kprobe_handler(), that Oops being a bug in kprobe_handle() triggered by > this machine check. > > By converting a VM address to a phys-like address as if is was linear > mem, you get in the dark. Either there is some physical memory at that > address and you corrupt it. Or there is none and you get a machine check.
Excellent. Please put that in the commit message. > > Also can you say what you're actually doing (ie turning on MSR[DR]) > > Euh ... I'm saying that data MMU has to be enabled, so I'm enabling it. Yeah, it's a minor point. > > > > > Signed-off-by: Christophe Leroy <christophe.le...@c-s.fr> > > > --- > > > arch/powerpc/kernel/entry_32.S | 9 +++++++-- > > > 1 file changed, 7 insertions(+), 2 deletions(-) > > > > > > diff --git a/arch/powerpc/kernel/entry_32.S > > > b/arch/powerpc/kernel/entry_32.S > > > index 0713daa651d9..bc056d906b51 100644 > > > --- a/arch/powerpc/kernel/entry_32.S > > > +++ b/arch/powerpc/kernel/entry_32.S > > > @@ -1354,12 +1354,17 @@ _GLOBAL(enter_rtas) > > > mtspr SPRN_SRR0,r8 > > > mtspr SPRN_SRR1,r9 > > > RFI > > > -1: tophys(r9,r1) > > > +1: tophys_novmstack r9, r1 > > > +#ifdef CONFIG_VMAP_STACKisntruction > > > + li r0, MSR_KERNEL & ~MSR_IR /* can take DTLB miss */ > > > > You're potentially turning on more than MSR DR here. This should be clear in > > the > > commit message. > > Am I ? > > At the time of the RFI just above, SRR1 contains the value of r9 which > has been set 2 lines before to MSR_KERNEL & ~(MSR_IR|MSR_DR). > > What should be clear in the commit message ? You're right. I was just looking at the patch and not the code. It's clearer in the code. Mikey > > > > + mtmsr r0 > > > + isync > > > +#endif > > > lwz r8,INT_FRAME_SIZE+4(r9) /* get return address */ > > > lwz r9,8(r9) /* original msr value */ > > > addi r1,r1,INT_FRAME_SIZE > > > li r0,0 > > > - tophys(r7, r2) > > > + tophys_novmstack r7, r2 > > > stw r0, THREAD + RTAS_SP(r7) > > > mtspr SPRN_SRR0,r8 > > > mtspr SPRN_SRR1,r9 > > Christophe >