[Linux-ia64] [PATCH] Performance Patch for TLB misses

[Linux-ia64] [PATCH] Performance Patch for TLB misses

[Christian Hildner]

[-- Attachment #1: Type: text/plain, Size: 977 bytes --]

Hi,

I created a patch that will (hopefully) increase the performance for TLB
miss handling.

- enlarge the distance between rsm psr.dt and srlz (the movl do not need
data access)

- let the processor check for the case of page not present (because for
useable systems the common case is that the page is present, so optimize
for that and drop the conditional branch)

- let the processor also check for privilege level because it's very
very rare

The second modification will result (rarely) in the insertion of an
unused TLB entry. But this entry will be overwritten with the useable
entry when the page is swapped in. The time needed for the insertion of
the TLB entry and the additional interruption will be neglectible
compared to the time needed for swapping in.

Since I'am waiting for Itanium 2 (Itanium 1 does not have any relevance
nowadays) I have done no performance testing. All Itanium 2 owners are
invited for testing.

Please tell me your comments.

Christian

[-- Attachment #2: ivt.S_patch_2.4.18 --]
[-- Type: text/plain, Size: 3361 bytes --]

--- ivt.S_orig	Tue Mar 19 11:22:28 2002
+++ ivt.S	Thu Nov  7 13:53:02 2002
@@ -114,8 +114,8 @@
 	 *	- the faulting virtual address has no L1, L2, or L3 mapping
 	 */
 	mov r16=cr.ifa				// get address that caused the TLB miss
-	;;
 	rsm psr.dt				// use physical addressing for data
+	;;
 	mov r31=pr				// save the predicate registers
 	mov r19=IA64_KR(PT_BASE)		// get page table base address
 	shl r21=r16,3				// shift bit 60 into sign bit
@@ -125,7 +125,6 @@
 	shr.u r18=r16,PGDIR_SHIFT		// get bits 33-63 of the faulting address
 	;;
 (p7)	dep r17=r17,r19,(PAGE_SHIFT-3),3	// put region number bits in place
-	srlz.d					// ensure "rsm psr.dt" has taken effect
 (p6)	movl r19=__pa(SWAPPER_PGD_ADDR)		// region 5 is rooted at swapper_pg_dir
 (p6)	shr.u r21=r21,PGDIR_SHIFT+PAGE_SHIFT
 (p7)	shr.u r21=r21,PGDIR_SHIFT+PAGE_SHIFT-3
@@ -135,6 +134,7 @@
 	cmp.eq p7,p6=0,r21			// unused address bits all zeroes?
 	shr.u r18=r16,PMD_SHIFT			// shift L2 index into position
 	;;
+	srlz.d					// ensure "rsm psr.dt" has taken effect
 	ld8 r17=[r17]				// fetch the L1 entry (may be 0)
 	;;
 (p7)	cmp.eq p6,p7=r17,r0			// was L1 entry NULL?
@@ -212,9 +212,6 @@
 1:	ld8 r18=[r17]				// read L3 PTE
 	;;
 	mov b0=r29
-	tbit.z p6,p0=r18,_PAGE_P_BIT		// page present bit cleared?
-(p6)	br.cond.spnt page_fault
-	;;
 	itc.i r18
 	;;
 #ifdef CONFIG_SMP
@@ -250,9 +247,6 @@
 1:	ld8 r18=[r17]				// read L3 PTE
 	;;
 	mov b0=r29
-	tbit.z p6,p0=r18,_PAGE_P_BIT		// page present bit cleared?
-(p6)	br.cond.spnt page_fault
-	;;
 	itc.d r18
 	;;
 #ifdef CONFIG_SMP
@@ -274,12 +268,11 @@
 	DBG_FAULT(3)
 	mov r16=cr.ifa		// get address that caused the TLB miss
 	movl r17=PAGE_KERNEL
-	mov r21=cr.ipsr
 	movl r19=(((1 << IA64_MAX_PHYS_BITS) - 1) & ~0xfff)
 	mov r31=pr
 	;;
 #ifdef CONFIG_DISABLE_VHPT
-	shr.u r22=r16,61			// get the region number into r21
+	shr.u r22=r16,61			// get the region number
 	;;
 	cmp.gt p8,p0=6,r22			// user mode
 	;;
@@ -289,16 +282,13 @@
 (p8)	mov r29=b0				// save b0
 (p8)	br.cond.dptk itlb_fault
 #endif
-	extr.u r23=r21,IA64_PSR_CPL0_BIT,2	// extract psr.cpl
 	and r19=r19,r16		// clear ed, reserved bits, and PTE control bits
 	shr.u r18=r16,57	// move address bit 61 to bit 4
 	;;
 	andcm r18=0x10,r18	// bit 4=~address-bit(61)
-	cmp.ne p8,p0=r0,r23	// psr.cpl != 0?
 	or r19=r17,r19		// insert PTE control bits into r19
 	;;
 	or r19=r19,r18		// set bit 4 (uncached) if the access was to region 6
-(p8)	br.cond.spnt page_fault
 	;;
 	itc.i r19		// insert the TLB entry
 	mov pr=r31,-1
@@ -318,7 +308,7 @@
 	mov r31=pr
 	;;
 #ifdef CONFIG_DISABLE_VHPT
-	shr.u r22=r16,61			// get the region number into r21
+	shr.u r22=r16,61			// get the region number
 	;;
 	cmp.gt p8,p0=6,r22			// access to region 0-5
 	;;
@@ -328,15 +318,11 @@
 (p8)	mov r29=b0				// save b0
 (p8)	br.cond.dptk dtlb_fault
 #endif
-	extr.u r23=r21,IA64_PSR_CPL0_BIT,2	// extract psr.cpl
 	tbit.nz p6,p7=r20,IA64_ISR_SP_BIT	// is speculation bit on?
 	and r19=r19,r16		// clear ed, reserved bits, and PTE control bits
 	shr.u r18=r16,57	// move address bit 61 to bit 4
 	;;
 	andcm r18=0x10,r18	// bit 4=~address-bit(61)
-	cmp.ne p8,p0=r0,r23
-(p8)	br.cond.spnt page_fault
-
 	dep r21=-1,r21,IA64_PSR_ED_BIT,1
 	or r19=r19,r17		// insert PTE control bits into r19
 	;;
@@ -1412,3 +1398,5 @@
 // 0x7f00 Entry 67 (size 16 bundles) Reserved
 	DBG_FAULT(67)
 	FAULT(67)
+
+

Re: [Linux-ia64] [PATCH] Performance Patch for TLB misses

[David Mosberger]

>>>>> On Thu, 07 Nov 2002 15:00:27 +0100, Christian Hildner <christian.hildner@hob.de> said:

  Christian> - let the processor also check for privilege level
  Christian> because it's very very rare

I didn't look at the patch too closely but this change looks wrong.
It would let user level insert arbitrary identity-mapped TLB
translations, which is not safe because it can trigger memory
attribute aliasing.

	--david

Re: [Linux-ia64] [PATCH] Performance Patch for TLB misses

[Mario Smarduch]

Christian Hildner wrote:

> Hi,
>
> I created a patch that will (hopefully) increase the performance for TLB
> miss handling.
>
> - enlarge the distance between rsm psr.dt and srlz (the movl do not need
> data access)
>
> - let the processor check for the case of page not present (because for
> useable systems the common case is that the page is present, so optimize
> for that and drop the conditional branch)
>
> - let the processor also check for privilege level because it's very
> very rare

Wouldn't removal of this possibly cause cached and uncahced mappings
to same page?

- mario.

>
>
> The second modification will result (rarely) in the insertion of an
> unused TLB entry. But this entry will be overwritten with the useable
> entry when the page is swapped in. The time needed for the insertion of
> the TLB entry and the additional interruption will be neglectible
> compared to the time needed for swapping in.
>
> Since I'am waiting for Itanium 2 (Itanium 1 does not have any relevance
> nowadays) I have done no performance testing. All Itanium 2 owners are
> invited for testing.
>
> Please tell me your comments.
>
> Christian
>
>   ------------------------------------------------------------------------
> --- ivt.S_orig  Tue Mar 19 11:22:28 2002
>  ivt.S  Thu Nov  7 13:53:02 2002
> @@ -114,8 +114-,8 @@
>          *      - the faulting virtual address has no L1, L2, or L3 mapping
>          */
>         mov r16=cr.ifa                          // get address that caused the TLB miss
> -       ;;
>         rsm psr.dt                              // use physical addressing for data
>         ;;
>         mov r31=pr                              // save the predicate registers
>         mov r19=IA64_KR(PT_BASE)                // get page table base address
>         shl r21=r16,3                           // shift bit 60 into sign bit
> @@ -125,7 +124-,6 @@
>         shr.u r18=r16,PGDIR_SHIFT               // get bits 33-63 of the faulting address
>         ;;
>  (p7)   dep r17=r17,r19,(PAGE_SHIFT-3),3        // put region number bits in place
> -       srlz.d                                  // ensure "rsm psr.dt" has taken effect
>  (p6)   movl r19=__pa(SWAPPER_PGD_ADDR)         // region 5 is rooted at swapper_pg_dir
>  (p6)   shr.u r21=r21,PGDIR_SHIFT+PAE-_SHIFT
>  (p7)   shr.u r21=r21,PGDIR_SHIFT+PAE-_SHIFT-3
> @@ -135,6 +134-,7 @@
>         cmp.eq p7,p6=0,r21                      // unused address bits all zeroes?
>         shr.u r18=r16,PMD_SHIFT                 // shift L2 index into position
>         ;;
>         srlz.d                                  // ensure "rsm psr.dt" has taken effect
>         ld8 r17=[r17]                           // fetch the L1 entry (may be 0)
>         ;;
>  (p7)   cmp.eq p6,p7=r17,r0                     // was L1 entry NULL?
> @@ -212,9 +210-,6 @@
>  1:     ld8 r18=[r17]                           // read L3 PTE
>         ;;
>         mov b0=r29
> -       tbit.z p6,p0=r18,_PAGE_P_BIT            // page present bit cleared?
> -(p6)   br.cond.spnt page_fault
> -       ;;
>         itc.i r18
>         ;;
>  #ifdef CONFIG_SMP
> @@ -250,9 +244-,6 @@
>  1:     ld8 r18=[r17]                           // read L3 PTE
>         ;;
>         mov b0=r29
> -       tbit.z p6,p0=r18,_PAGE_P_BIT            // page present bit cleared?
> -(p6)   br.cond.spnt page_fault
> -       ;;
>         itc.d r18
>         ;;
>  #ifdef CONFIG_SMP
> @@ -274,12 +268-,11 @@
>         DBG_FAULT(3)
>         mov r16=cr.ifa          // get address that caused the TLB miss
>         movl r17=PAGE_KERNEL
> -       mov r21=cr.ipsr
>         movl r19=(((1 << IA64_MAX_PHYS_BITS) - 1) & +AH4-0xfff)
>         mov r31=pr
>         ;;
>  #ifdef CONFIG_DISABLE_VHPT
> -       shr.u r22=r16,61                        // get the region number into r21
>         shr.u r22=r16,61                        // get the region number
>         ;;
>         cmp.gt p8,p0=6,r22                      // user mode
>         ;;
> @@ -289,16 +280-,13 @@
>  (p8)   mov r29°                              // save b0
>  (p8)   br.cond.dptk itlb_fault
>  #endif
> -       extr.u r23=r21,IA64_PSR_CPL0_BIT,2      // extract psr.cpl
>         and r19=r19,r16         // clear ed, reserved bits, and PTE control bits
>         shr.u r18=r16,57        // move address bit 61 to bit 4
>         ;;
>         andcm r18=0x10,r18      // bit 4=+AH4-address-bit(61)
> -       cmp.ne p8,p0=r0,r23     // psr.cpl != 0?
>         or r19=r17,r19          // insert PTE control bits into r19
>         ;;
>         or r19=r19,r18          // set bit 4 (uncached) if the access was to region 6
> -(p8)   br.cond.spnt page_fault
>         ;;
>         itc.i r19               // insert the TLB entry
>         mov pr=r31,-1
> @@ -318,7 +308-,7 @@
>         mov r31=pr
>         ;;
>  #ifdef CONFIG_DISABLE_VHPT
> -       shr.u r22=r16,61                        // get the region number into r21
>         shr.u r22=r16,61                        // get the region number
>         ;;
>         cmp.gt p8,p0=6,r22                      // access to region 0-5
>         ;;
> @@ -328,15 +318-,11 @@
>  (p8)   mov r29°                              // save b0
>  (p8)   br.cond.dptk dtlb_fault
>  #endif
> -       extr.u r23=r21,IA64_PSR_CPL0_BIT,2      // extract psr.cpl
>         tbit.nz p6,p7=r20,IA64_ISR_SP_BIT       // is speculation bit on?
>         and r19=r19,r16         // clear ed, reserved bits, and PTE control bits
>         shr.u r18=r16,57        // move address bit 61 to bit 4
>         ;;
>         andcm r18=0x10,r18      // bit 4=+AH4-address-bit(61)
> -       cmp.ne p8,p0=r0,r23
> -(p8)   br.cond.spnt page_fault
> -
>         dep r21=-1,r21,IA64_PSR_ED_BIT,1
>         or r19=r19,r17          // insert PTE control bits into r19
>         ;;
> @@ -1412,3 +138-,5 @@
>  // 0x7f00 Entry 67 (size 16 bundles) Reserved
>         DBG_FAULT(67)
>         FAULT(67)

Re: [Linux-ia64] [PATCH] Performance Patch for TLB misses

[Christian Hildner]

David Mosberger schrieb:

> >>>>> On Thu, 07 Nov 2002 15:00:27 +0100, Christian Hildner <christian.hildner@hob.de> said:
>
>   Christian> - let the processor also check for privilege level
>   Christian> because it's very very rare
>
> I didn't look at the patch too closely but this change looks wrong.
> It would let user level insert arbitrary identity-mapped TLB
> translations, which is not safe because it can trigger memory
> attribute aliasing.

Yes, user level would be allowed to insert translations. But the page attributes are hardcoded
(PAGE_KERNEL) and this will not allow the user to touch this pages.

Christian

Re: [Linux-ia64] [PATCH] Performance Patch for TLB misses

[Christian Hildner]

Mario Smarduch schrieb:

> Christian Hildner wrote:
>
> > Hi,
> >
> > I created a patch that will (hopefully) increase the performance for TLB
> > miss handling.
> >
> > - enlarge the distance between rsm psr.dt and srlz (the movl do not need
> > data access)
> >
> > - let the processor check for the case of page not present (because for
> > useable systems the common case is that the page is present, so optimize
> > for that and drop the conditional branch)
> >
> > - let the processor also check for privilege level because it's very
> > very rare
>
> Wouldn't removal of this possibly cause cached and uncahced mappings
> to same page?

Yes, but for the kernel it is allowed also without the patch and the the user could insert a
duplicate mapping for the same physical page but this useless for him because of the access
rights and also the kernel would not use these pages (because the kernel has no knowledge of
this pages).

Christian

Re: [Linux-ia64] [PATCH] Performance Patch for TLB misses

[John Marvin]

> > Wouldn't removal of this possibly cause cached and uncahced mappings
> > to same page?
>
> Yes, but for the kernel it is allowed also without the patch and the the user c
> ould insert a
> duplicate mapping for the same physical page but this useless for him because o
> f the access
> rights and also the kernel would not use these pages (because the kernel has no
>  knowledge of
> this pages).
>
Christian, you must not have understood what Dave was referring to when
he talked about attribute aliasing. Having cached and uncached mappings
to the same page might be OK on a Pentium, but it is definitely not OK
on the ia64 architecture. It can lead to MCA's. Here's a quote from
the architecture manual that might make it a little clearer (from the
end of section 4.4.1):

    It is recommended that processor models report a Machine Check abort
    if any of the following attribute aliases are detected:

    * Coalescing buffer hit on a non-coalescing page.
    * Cache hit on an uncacheable page, other than as the target of a
      local or remote flush cache instruction.

Just because the user can't use the bad mapping that he may have inserted
doesn't mean that it is not a problem.  Once that translation has been
inserted, it is a potential problem.  Even if the kernel doesn't directly
use the translation, the processor may use it to prefetch.

John Marvin
jsm@fc.hp.com

Re: [Linux-ia64] [PATCH] Performance Patch for TLB misses

[David Mosberger]

>>>>> On Mon, 11 Nov 2002 09:49:49 +0100, Christian Hildner <christian.hildner@hob.de> said:

  Christian> Yes, but for the kernel it is allowed also without the
  Christian> patch and the the user could insert a duplicate mapping
  Christian> for the same physical page but this useless for him
  Christian> because of the access rights and also the kernel would
  Christian> not use these pages (because the kernel has no knowledge
  Christian> of this pages).

Useless to the user, but dangerous to the health of the machine!
Please note that attribute aliasing can lead to a machine-check and
therefore is an absolute no-no (the machine check could be triggered
either by a hardware-prefetch or due to speculative memory accesses
inside the kernel).

	--david

Re: [Linux-ia64] [PATCH] Performance Patch for TLB misses

[Christian Hildner]

David Mosberger schrieb:

> Useless to the user, but dangerous to the health of the machine!
> Please note that attribute aliasing can lead to a machine-check and
> therefore is an absolute no-no (the machine check could be triggered
> either by a hardware-prefetch or due to speculative memory accesses
> inside the kernel).
>
>         --david

David, John, Mario,

I still haven't read volume 2 (System Architecture) to the end, so I learned another piece of
the total complexity of IA64 now. Maybe you want to use the other two ideas for 2.5 kernels. But
only if you like them.

Christian

Re: [Linux-ia64] [PATCH] Performance Patch for TLB misses

[David Mosberger]

>>>>> On Tue, 12 Nov 2002 09:43:02 +0100, Christian Hildner <christian.hildner@hob.de> said:

  Christian> David Mosberger schrieb:

  >> Useless to the user, but dangerous to the health of the machine!
  >> Please note that attribute aliasing can lead to a machine-check
  >> and therefore is an absolute no-no (the machine check could be
  >> triggered either by a hardware-prefetch or due to speculative
  >> memory accesses inside the kernel).

  Christian> I still haven't read volume 2 (System Architecture) to
  Christian> the end, so I learned another piece of the total
  Christian> complexity of IA64 now.

Note that attribute-aliasing is a problem any architecture with
multiple page-size somehow needs to deal with.  Remember the Athlon
AGP bug?  It caused problems for a long time and it was due to
attribute-aliasing (between 4KB user pages and 4MB kernel mappings).
The difference with ia64 is that the architects chose to make
attribute-aliasing illegal upfront (so that you may get an MCA when
inserting an invalid translation in the TLB), rather than waiting for
silent data corruption (as it happened with the Athlong AGP bug).
Given the choice between silent data corruption and an explicit MCA, I
think the latter is much preferable.

	--david

Re: [Linux-ia64] [PATCH] Performance Patch for TLB misses

[David Mosberger]

>>>>> On Tue, 12 Nov 2002 09:00:26 -0800, David Mosberger <davidm@linux.hpl.hp.com> said:

  David> Note that attribute-aliasing is a problem any architecture
  David> with multiple page-size somehow needs to deal with.

Oops, typed a bit too fast here: I meant to say: "any architecture
with multiple page-sizes and per-translation memory attributes" (as is
the case for ia64 and x86, for example).

	--david