| Message ID | 1481913337-9331-5-git-send-email-mike.kravetz@oracle.com |
|---|---|
| State | RFC |
| Delegated to: | David Miller |
| Headers | show |
Hi Mike > diff --git a/arch/sparc/kernel/fpu_traps.S b/arch/sparc/kernel/fpu_traps.S > index 336d275..f85a034 100644 > --- a/arch/sparc/kernel/fpu_traps.S > +++ b/arch/sparc/kernel/fpu_traps.S > @@ -73,6 +73,16 @@ do_fpdis: > ldxa [%g3] ASI_MMU, %g5 > .previous > > +661: nop > + nop > + .section .sun4v_2insn_patch, "ax" > + .word 661b > + mov SECONDARY_CONTEXT_R1, %g3 > + ldxa [%g3] ASI_MMU, %g4 > + .previous > + /* Unnecessary on sun4u and pre-Niagara 2 sun4v */ > + mov SECONDARY_CONTEXT, %g3 > + > sethi %hi(sparc64_kern_sec_context), %g2 You missed the second instruction to patch with here. This bug repeats itself further down. Just noted while briefly reading the code - did not really follow the code. Sam -- To unsubscribe from this list: send the line "unsubscribe sparclinux" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Mike Kravetz <mike.kravetz@oracle.com> Date: Fri, 16 Dec 2016 10:35:27 -0800 > In current code, only context ID register 0 is set and used by the MMU. > On sun4v platforms that support MMU shared context, there is an additional > context ID register: specifically context register 1. When searching > the TLB, the MMU will find a match if the virtual address matches and > the ID contained in context register 0 -OR- context register 1 matches. > > Load the shared context ID into context ID register 1. Care must be > taken to load register 1 after register 0, as loading register 0 > overwrites both register 0 and 1. Modify code loading register 0 to > also load register one if applicable. > > Signed-off-by: Mike Kravetz <mike.kravetz@oracle.com> You can't make these register accesses if the feature isn't being used. Considering the percentage of applications which will actually use this thing, incuring the overhead of even loading the shared context register is simply unacceptable. -- To unsubscribe from this list: send the line "unsubscribe sparclinux" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 12/17/2016 07:14 PM, David Miller wrote: > From: Mike Kravetz <mike.kravetz@oracle.com> > Date: Fri, 16 Dec 2016 10:35:27 -0800 > >> In current code, only context ID register 0 is set and used by the MMU. >> On sun4v platforms that support MMU shared context, there is an additional >> context ID register: specifically context register 1. When searching >> the TLB, the MMU will find a match if the virtual address matches and >> the ID contained in context register 0 -OR- context register 1 matches. >> >> Load the shared context ID into context ID register 1. Care must be >> taken to load register 1 after register 0, as loading register 0 >> overwrites both register 0 and 1. Modify code loading register 0 to >> also load register one if applicable. >> >> Signed-off-by: Mike Kravetz <mike.kravetz@oracle.com> > > You can't make these register accesses if the feature isn't being > used. > > Considering the percentage of applications which will actually use > this thing, incuring the overhead of even loading the shared context > register is simply unacceptable. Ok, let me try to find a way to eliminate these loads unless the application is using shared context. Part of the issue is a 'backwards compatibility' feature of the processor which loads/overwrites register 1 every time register 0 is loaded. Somewhere in the evolution of the processor, a feature was added so that register 0 could be loaded without overwriting register 1. That could be used to eliminate the extra load in some/many cases. But, that would likely lead to more runtime kernel patching based on processor level. And, I don't really want to add more of that if possible. Or, perhaps we only enable the shared context ID feature on processors which have the ability to work around the backwards compatibility feature.
On 12/16/2016 11:45 PM, Sam Ravnborg wrote: > Hi Mike > >> diff --git a/arch/sparc/kernel/fpu_traps.S b/arch/sparc/kernel/fpu_traps.S >> index 336d275..f85a034 100644 >> --- a/arch/sparc/kernel/fpu_traps.S >> +++ b/arch/sparc/kernel/fpu_traps.S >> @@ -73,6 +73,16 @@ do_fpdis: >> ldxa [%g3] ASI_MMU, %g5 >> .previous >> >> +661: nop >> + nop >> + .section .sun4v_2insn_patch, "ax" >> + .word 661b >> + mov SECONDARY_CONTEXT_R1, %g3 >> + ldxa [%g3] ASI_MMU, %g4 >> + .previous >> + /* Unnecessary on sun4u and pre-Niagara 2 sun4v */ >> + mov SECONDARY_CONTEXT, %g3 >> + >> sethi %hi(sparc64_kern_sec_context), %g2 > > You missed the second instruction to patch with here. > This bug repeats itself further down. > > Just noted while briefly reading the code - did not really follow the code. Hi Sam, This is my first sparc assembly code, so I could certainly have this wrong. The code I was trying to write has the two nop instructions, that get patched with the mov and ldxa on sun4v. Certainly, this is not elegant. And, the formatting may lead to some confusion. Did you perhaps think the mov instruction after the comment was for patching? I am just trying to understand your comment.
From: Mike Kravetz <mike.kravetz@oracle.com> Date: Sun, 18 Dec 2016 16:06:01 -0800 > Ok, let me try to find a way to eliminate these loads unless the application > is using shared context. > > Part of the issue is a 'backwards compatibility' feature of the processor > which loads/overwrites register 1 every time register 0 is loaded. Somewhere > in the evolution of the processor, a feature was added so that register 0 > could be loaded without overwriting register 1. That could be used to > eliminate the extra load in some/many cases. But, that would likely lead > to more runtime kernel patching based on processor level. And, I don't > really want to add more of that if possible. Or, perhaps we only enable > the shared context ID feature on processors which have the ability to work > around the backwards compatibility feature. Until the first process uses shared mappings, you should not touch the context 1 register in any way for any reason at all. And even once a process _does_ use shared mappings, you only need to access the context 1 register in 2 cases: 1) TLB processing for the processes using shared mappings. 2) Context switch MMU state handling, where either the previous or next process is using shared mappings. -- To unsubscribe from this list: send the line "unsubscribe sparclinux" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 12/20/2016 10:33 AM, David Miller wrote: > From: Mike Kravetz <mike.kravetz@oracle.com> > Date: Sun, 18 Dec 2016 16:06:01 -0800 > >> Ok, let me try to find a way to eliminate these loads unless the application >> is using shared context. >> >> Part of the issue is a 'backwards compatibility' feature of the processor >> which loads/overwrites register 1 every time register 0 is loaded. Somewhere >> in the evolution of the processor, a feature was added so that register 0 >> could be loaded without overwriting register 1. That could be used to >> eliminate the extra load in some/many cases. But, that would likely lead >> to more runtime kernel patching based on processor level. And, I don't >> really want to add more of that if possible. Or, perhaps we only enable >> the shared context ID feature on processors which have the ability to work >> around the backwards compatibility feature. > > Until the first process uses shared mappings, you should not touch the > context 1 register in any way for any reason at all. > > And even once a process _does_ use shared mappings, you only need to > access the context 1 register in 2 cases: > > 1) TLB processing for the processes using shared mappings. > > 2) Context switch MMU state handling, where either the previous or > next process is using shared mappings. I agree. But, we still need to address the issue of existing code that is overwriting context register 1 today. Due to that backwards compatibility feature, code like: mov SECONDARY_CONTEXT, %g3 stxa %g2, [%g3] ASI_DMMU will store not only to register 0, but register 1 as well. In this RFC, I used an ugly brute force method of always restoring register 1 after storing register 0 to make sure any unique value in register 1 was preserved. I agree this is not acceptable and needs to be fixed. We could check if register 1 is in use and only do the save/restore in that case. But, that is still an additional check. The Sparc M7 processor has new ASIs to handle this better: ASI ASI Name R/W VA Per Strand Description 0x21 ASI_MMU RW 0x28 Y I/DMMUPrimary Context register 0 (no Primary Context register 1 update) 0x21 ASI_MMU RW 0x30 Y DMMUSecondary Context register 0 (no Secondary Context register 1 update) More details at, http://www.oracle.com/technetwork/server-storage/sun-sparc-enterprise/documentation/sparc-architecture-supplement-3093429.pdf Of course, this could only be used on processors where the new ASIs are available. Still need to think about the best way to handle this.
On Sun, Dec 18, 2016 at 04:22:31PM -0800, Mike Kravetz wrote: > On 12/16/2016 11:45 PM, Sam Ravnborg wrote: > > Hi Mike > > > >> diff --git a/arch/sparc/kernel/fpu_traps.S b/arch/sparc/kernel/fpu_traps.S > >> index 336d275..f85a034 100644 > >> --- a/arch/sparc/kernel/fpu_traps.S > >> +++ b/arch/sparc/kernel/fpu_traps.S > >> @@ -73,6 +73,16 @@ do_fpdis: > >> ldxa [%g3] ASI_MMU, %g5 > >> .previous > >> > >> +661: nop > >> + nop > >> + .section .sun4v_2insn_patch, "ax" > >> + .word 661b > >> + mov SECONDARY_CONTEXT_R1, %g3 > >> + ldxa [%g3] ASI_MMU, %g4 > >> + .previous > >> + /* Unnecessary on sun4u and pre-Niagara 2 sun4v */ > >> + mov SECONDARY_CONTEXT, %g3 > >> + > >> sethi %hi(sparc64_kern_sec_context), %g2 > > > > You missed the second instruction to patch with here. > > This bug repeats itself further down. > > > > Just noted while briefly reading the code - did not really follow the code. > > Hi Sam, > > This is my first sparc assembly code, so I could certainly have this > wrong. Nope. I was to quick in my reading and in the reply. when I looked at this with fresh eyes it looked perfectly OK. That is to say - the patching part. I did not follow the code logic. Sam -- To unsubscribe from this list: send the line "unsubscribe sparclinux" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Mike. > Or, perhaps we only enable > the shared context ID feature on processors which have the ability to work > around the backwards compatibility feature. Start out like this, and then see if it is really needed with the older processors. This should keep the code logic simpler - which is always good for this complicated stuff. Sam -- To unsubscribe from this list: send the line "unsubscribe sparclinux" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/sparc/include/asm/mmu_context_64.h b/arch/sparc/include/asm/mmu_context_64.h index acaea6d..84268df 100644 --- a/arch/sparc/include/asm/mmu_context_64.h +++ b/arch/sparc/include/asm/mmu_context_64.h @@ -61,8 +61,11 @@ void smp_tsb_sync(struct mm_struct *mm); #define smp_tsb_sync(__mm) do { } while (0) #endif -/* Set MMU context in the actual hardware. */ -#define load_secondary_context(__mm) \ +/* + * Set MMU context in the actual hardware. Secondary context register + * zero is loaded with task specific context. + */ +#define load_secondary_context_0(__mm) \ __asm__ __volatile__( \ "\n661: stxa %0, [%1] %2\n" \ " .section .sun4v_1insn_patch, \"ax\"\n" \ @@ -74,6 +77,36 @@ void smp_tsb_sync(struct mm_struct *mm); : "r" (CTX_HWBITS((__mm)->context)), \ "r" (SECONDARY_CONTEXT), "i" (ASI_DMMU), "i" (ASI_MMU)) +/* + * Secondary context register one is loaded with shared context if + * it exists for the task. + */ +#define load_secondary_context_1(__mm) \ + __asm__ __volatile__( \ + "\n661: stxa %0, [%1] %2\n" \ + " .section .sun4v_1insn_patch, \"ax\"\n" \ + " .word 661b\n" \ + " stxa %0, [%1] %3\n" \ + " .previous\n" \ + " flush %%g6\n" \ + : /* No outputs */ \ + : "r" (SHARED_CTX_HWBITS((__mm)->context)), \ + "r" (SECONDARY_CONTEXT_R1), "i" (ASI_DMMU), "i" (ASI_MMU)) + +#if defined(CONFIG_SHARED_MMU_CTX) +#define load_secondary_context(__mm) \ + do { \ + load_secondary_context_0(__mm); \ + if ((__mm)->context.shared_ctx) \ + load_secondary_context_1(__mm); \ + } while (0) +#else +#define load_secondary_context(__mm) \ + do { \ + load_secondary_context_0(__mm); \ + } while (0) +#endif + void __flush_tlb_mm(unsigned long, unsigned long); /* Switch the current MM context. */ diff --git a/arch/sparc/include/asm/spitfire.h b/arch/sparc/include/asm/spitfire.h index 1d8321c..1fa4594 100644 --- a/arch/sparc/include/asm/spitfire.h +++ b/arch/sparc/include/asm/spitfire.h @@ -33,6 +33,8 @@ #define DMMU_SFAR 0x0000000000000020 #define VIRT_WATCHPOINT 0x0000000000000038 #define PHYS_WATCHPOINT 0x0000000000000040 +#define PRIMARY_CONTEXT_R1 0x0000000000000108 +#define SECONDARY_CONTEXT_R1 0x0000000000000110 #define SPITFIRE_HIGHEST_LOCKED_TLBENT (64 - 1) #define CHEETAH_HIGHEST_LOCKED_TLBENT (16 - 1) diff --git a/arch/sparc/kernel/fpu_traps.S b/arch/sparc/kernel/fpu_traps.S index 336d275..f85a034 100644 --- a/arch/sparc/kernel/fpu_traps.S +++ b/arch/sparc/kernel/fpu_traps.S @@ -73,6 +73,16 @@ do_fpdis: ldxa [%g3] ASI_MMU, %g5 .previous +661: nop + nop + .section .sun4v_2insn_patch, "ax" + .word 661b + mov SECONDARY_CONTEXT_R1, %g3 + ldxa [%g3] ASI_MMU, %g4 + .previous + /* Unnecessary on sun4u and pre-Niagara 2 sun4v */ + mov SECONDARY_CONTEXT, %g3 + sethi %hi(sparc64_kern_sec_context), %g2 ldx [%g2 + %lo(sparc64_kern_sec_context)], %g2 @@ -114,6 +124,16 @@ do_fpdis: ldxa [%g3] ASI_MMU, %g5 .previous +661: nop + nop + .section .sun4v_2insn_patch, "ax" + .word 661b + mov SECONDARY_CONTEXT_R1, %g3 + ldxa [%g3] ASI_MMU, %g4 + .previous + /* Unnecessary on sun4u and pre-Niagara 2 sun4v */ + mov SECONDARY_CONTEXT, %g3 + add %g6, TI_FPREGS, %g1 sethi %hi(sparc64_kern_sec_context), %g2 ldx [%g2 + %lo(sparc64_kern_sec_context)], %g2 @@ -155,6 +175,16 @@ do_fpdis: ldxa [%g3] ASI_MMU, %g5 .previous +661: nop + nop + .section .sun4v_2insn_patch, "ax" + .word 661b + mov SECONDARY_CONTEXT_R1, %g3 + ldxa [%g3] ASI_MMU, %g4 + .previous + /* Unnecessary on sun4u and pre-Niagara 2 sun4v */ + mov SECONDARY_CONTEXT, %g3 + sethi %hi(sparc64_kern_sec_context), %g2 ldx [%g2 + %lo(sparc64_kern_sec_context)], %g2 @@ -181,11 +211,24 @@ fpdis_exit: stxa %g5, [%g3] ASI_MMU .previous +661: nop + nop + .section .sun4v_2insn_patch, "ax" + .word 661b + mov SECONDARY_CONTEXT_R1, %g3 + stxa %g4, [%g3] ASI_MMU + .previous + membar #Sync fpdis_exit2: wr %g7, 0, %gsr ldx [%g6 + TI_XFSR], %fsr rdpr %tstate, %g3 +661: nop + .section .sun4v_1insn_patch, "ax" + .word 661b + sethi %hi(TSTATE_PEF), %g4 + .previous or %g3, %g4, %g3 ! anal... wrpr %g3, %tstate wr %g0, FPRS_FEF, %fprs ! clean DU/DL bits @@ -347,6 +390,16 @@ do_fptrap_after_fsr: ldxa [%g3] ASI_MMU, %g5 .previous +661: nop + nop + .section .sun4v_2insn_patch, "ax" + .word 661b + mov SECONDARY_CONTEXT_R1, %g3 + ldxa [%g3] ASI_MMU, %g4 + .previous + /* Unnecessary on sun4u and pre-Niagara 2 sun4v */ + mov SECONDARY_CONTEXT, %g3 + sethi %hi(sparc64_kern_sec_context), %g2 ldx [%g2 + %lo(sparc64_kern_sec_context)], %g2 @@ -377,7 +430,17 @@ do_fptrap_after_fsr: stxa %g5, [%g1] ASI_MMU .previous +661: nop + nop + .section .sun4v_2insn_patch, "ax" + .word 661b + mov SECONDARY_CONTEXT_R1, %g1 + stxa %g4, [%g1] ASI_MMU + .previous + membar #Sync + /* Unnecessary on sun4u and pre-Niagara 2 sun4v */ + mov SECONDARY_CONTEXT, %g1 ba,pt %xcc, etrap wr %g0, 0, %fprs .size do_fptrap,.-do_fptrap diff --git a/arch/sparc/kernel/rtrap_64.S b/arch/sparc/kernel/rtrap_64.S index 216948c..d409d84 100644 --- a/arch/sparc/kernel/rtrap_64.S +++ b/arch/sparc/kernel/rtrap_64.S @@ -202,6 +202,7 @@ rt_continue: ldx [%sp + PTREGS_OFF + PT_V9_G1], %g1 brnz,pn %l3, kern_rtt mov PRIMARY_CONTEXT, %l7 + /* Get value from SECONDARY_CONTEXT register */ 661: ldxa [%l7 + %l7] ASI_DMMU, %l0 .section .sun4v_1insn_patch, "ax" .word 661b @@ -212,12 +213,31 @@ rt_continue: ldx [%sp + PTREGS_OFF + PT_V9_G1], %g1 ldx [%l1 + %lo(sparc64_kern_pri_nuc_bits)], %l1 or %l0, %l1, %l0 + /* and, put into PRIMARY_CONTEXT register */ 661: stxa %l0, [%l7] ASI_DMMU .section .sun4v_1insn_patch, "ax" .word 661b stxa %l0, [%l7] ASI_MMU .previous + /* Get value from SECONDARY_CONTEXT_R1 register */ +661: nop + nop + .section .sun4v_2insn_patch, "ax" + .word 661b + mov SECONDARY_CONTEXT_R1, %l7 + ldxa [%l7] ASI_MMU, %l0 + .previous + + /* and, put into PRIMARY_CONTEXT_R1 register */ +661: nop + nop + .section .sun4v_2insn_patch, "ax" + .word 661b + mov PRIMARY_CONTEXT_R1, %l7 + stxa %l0, [%l7] ASI_MMU + .previous + sethi %hi(KERNBASE), %l7 flush %l7 rdpr %wstate, %l1 diff --git a/arch/sparc/kernel/trampoline_64.S b/arch/sparc/kernel/trampoline_64.S index 88ede1d..7c4ab3b 100644 --- a/arch/sparc/kernel/trampoline_64.S +++ b/arch/sparc/kernel/trampoline_64.S @@ -260,6 +260,16 @@ after_lock_tlb: stxa %g0, [%g7] ASI_MMU .previous + /* Save SECONDARY_CONTEXT_R1, membar should be part of patch */ + membar #Sync +661: nop + nop + .section .sun4v_2insn_patch, "ax" + .word 661b + mov SECONDARY_CONTEXT_R1, %g7 + ldxa [%g7] ASI_MMU, %g1 + .previous + membar #Sync mov SECONDARY_CONTEXT, %g7 @@ -269,6 +279,16 @@ after_lock_tlb: stxa %g0, [%g7] ASI_MMU .previous + /* Restore SECONDARY_CONTEXT_R1, membar should be part of patch */ + membar #Sync +661: nop + nop + .section .sun4v_2insn_patch, "ax" + .word 661b + mov SECONDARY_CONTEXT_R1, %g7 + stxa %g1, [%g7] ASI_MMU + .previous + membar #Sync /* Everything we do here, until we properly take over the
In current code, only context ID register 0 is set and used by the MMU. On sun4v platforms that support MMU shared context, there is an additional context ID register: specifically context register 1. When searching the TLB, the MMU will find a match if the virtual address matches and the ID contained in context register 0 -OR- context register 1 matches. Load the shared context ID into context ID register 1. Care must be taken to load register 1 after register 0, as loading register 0 overwrites both register 0 and 1. Modify code loading register 0 to also load register one if applicable. Signed-off-by: Mike Kravetz <mike.kravetz@oracle.com> --- arch/sparc/include/asm/mmu_context_64.h | 37 +++++++++++++++++-- arch/sparc/include/asm/spitfire.h | 2 ++ arch/sparc/kernel/fpu_traps.S | 63 +++++++++++++++++++++++++++++++++ arch/sparc/kernel/rtrap_64.S | 20 +++++++++++ arch/sparc/kernel/trampoline_64.S | 20 +++++++++++ 5 files changed, 140 insertions(+), 2 deletions(-)