Re: [PATCH] powerpc/book3e-64: use a separate TLB handler when linear map is bolted

[Scott Wood <scottwood@freescale.com>]

On Fri, 17 Jun 2011 12:00:50 +1000
Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:

> Does this completely replace your previous series of 7 patches ? (IE.
> Should I ditch them in patchwork ?) Or does it apply on top of them ?

It replaces them.

> >  #define SET_IVOR(vector_number, vector_offset)	\
> > diff --git a/arch/powerpc/include/asm/mmu_context.h b/arch/powerpc/include/asm/mmu_context.h
> > index a73668a..9d9e444 100644
> > --- a/arch/powerpc/include/asm/mmu_context.h
> > +++ b/arch/powerpc/include/asm/mmu_context.h
> > @@ -54,6 +54,7 @@ static inline void switch_mm(struct mm_struct *prev, struct mm_struct *next,
> >  	/* 64-bit Book3E keeps track of current PGD in the PACA */
> >  #ifdef CONFIG_PPC_BOOK3E_64
> >  	get_paca()->pgd = next->pgd;
> > +	get_paca()->extlb[0][EX_TLB_PGD / 8] = (unsigned long)next->pgd;
> >  #endif
> >  	/* Nothing else to do if we aren't actually switching */
> >  	if (prev == next)
> > @@ -110,6 +111,7 @@ static inline void enter_lazy_tlb(struct mm_struct *mm,
> >  	/* 64-bit Book3E keeps track of current PGD in the PACA */
> >  #ifdef CONFIG_PPC_BOOK3E_64
> >  	get_paca()->pgd = NULL;
> > +	get_paca()->extlb[0][EX_TLB_PGD / 8] = 0;
> >  #endif
> >  }
> 
> Why do you keep a copy of the pgd there since it's in the PACA already
> and you have r13 setup in your handlers ?

This cache line is already being used by the TLB miss handler.  The cache
line containing paca->pgd isn't otherwise used there.

> > +.macro tlb_prolog_bolted addr
> > +	mtspr	SPRN_SPRG_TLB_SCRATCH,r13
> > +	mfspr	r13,SPRN_SPRG_PACA
> > +	std	r10,PACA_EXTLB+EX_TLB_R10(r13)
> > +	mfcr	r10
> > +	std	r11,PACA_EXTLB+EX_TLB_R11(r13)
> > +	mfspr	r11,SPRN_SPRG_TLB_SCRATCH
> 
> Do you need that ? Can't you leave r13 in scratch the whole way and
> just pop it out in the error case when branching to DSI/ISI ? The only
> thing is that TLB_SCRATCH needs to be saved/restored by
> crit/debug/mcheck but thats worth saving cycles in the TLB miss handler
> no ?

Sounds right... I'll try it.

> > +	mfspr	r11,SPRN_ESR
> > +
> > +	srdi	r15,r16,60		/* get region */
> > +	rldicl.	r10,r16,64-PGTABLE_EADDR_SIZE,PGTABLE_EADDR_SIZE+4
> > +	bne-	dtlb_miss_fault_bolted
> 
> Ok so I'm not familiar with your pipeline here, but wouldn't it be
> better to move the srdi to after the bne above and make it srdi., thus
> avoiding the compare below ?
> 
> > +	rlwinm	r10,r11,32-19,27,27
> > +	rlwimi	r10,r11,32-16,19,19
> > +	cmpwi	r15,0
> > +	ori	r10,r10,_PAGE_PRESENT
> > +	oris	r11,r10,_PAGE_ACCESSED@h

The cmpwi will run in parallel with the rlwimi above on e5500, but
changing it shouldn't hurt, and it's one less instruction to sit in the
icache...

-Scott