Subject: Re: [PATCH 1/5] powerpc/mm: Add MMU features for TLB reservation & Paired MAS registers

From: Benjamin Herrenschmidt <[email-masked]>
Date: Wed, 19 Aug 2009 17:25:56 +1000

On Wed, 2009-08-19 at 00:08 -0500, Kumar Gala wrote:
> Support for TLB reservation (or TLB Write Conditional) and Paired MAS
> registers are optional for a processor implementation so we handle
> them via MMU feature sections.
> 
> We currently only used paired MAS registers to access the full RPN + perm
> bits that are kept in MAS7||MAS3.  We assume that if an implementation has
> hardware page table at this time it also implements in TLB reservations.

You also need to be careful with this code:

virt_page_table_tlb_miss_done:

	/* We have overriden MAS2:EPN but currently our primary TLB miss
	 * handler will always restore it so that should not be an issue,
	 * if we ever optimize the primary handler to not write MAS2 on
	 * some cases, we'll have to restore MAS2:EPN here based on the
	 * original fault's DEAR. If we do that we have to modify the
	 * ITLB miss handler to also store SRR0 in the exception frame
	 * as DEAR.
	 *
	 * However, one nasty thing we did is we cleared the reservation
	 * (well, potentially we did). We do a trick here thus if we
	 * are not a level 0 exception (we interrupted the TLB miss) we
	 * offset the return address by -4 in order to replay the tlbsrx
	 * instruction there
	 */
	subf	r10,r13,r12
	cmpldi	cr0,r10,PACA_EXTLB+EX_TLB_SIZE
	bne-	1f
	ld	r11,PACA_EXTLB+EX_TLB_SIZE+EX_TLB_SRR0(r13)
	addi	r10,r11,-4
	std	r10,PACA_EXTLB+EX_TLB_SIZE+EX_TLB_SRR0(r13)

You may want to make the 3 last lines conditional on having tlbsrx.

Right now, in the no-tlbsrx. case, what happens is that it will go back
to the previous instruction, an or, which hopefully should be harmless
-but- this code is nasty enough you really don't want to take that
sort of chances.

Feel free to add a fat comment next to the ld in the tlbsrx case itself
explaining why those two instructions must be kept together and any
change here must be reflected in the second level handler.

Cheers,
Ben.