| Message ID | 20220330222834.139769-1-jarkko@kernel.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] selftests/sgx: Use rip relative addressing for encl_stack | expand |
Hi Jarkko, On 3/30/2022 3:28 PM, Jarkko Sakkinen wrote: > Simplify the test_encl_bootstrap.S flow by using RIP-relative addressing. It is not clear to me how this is simpler. At this point there is no functional change (except for what appears to be an unintended bug - more below). At this time the change seems more code utilizing subtle compiler features to accomplish the same. Could you please share more about your plans following this change? I need to understand this better since it is also an area changed by the SGX2 testing code. > The compiler automatically puts relative addresses for RIP index addresses. I was not aware of this. A comment would be helpful to understand the implementation. > > In order to get a clean and tweakless solution, define separate entry point > for each TCS. > > Cc: Reinette Chatre <reinette.chatre@intel.com> > Cc: Dave Hansen <dave.hansen@linux.intel.com> > Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org> > --- > v2: > * Based on Reinette's example, make proper structuring with separate > entry points for each TCS. > --- > .../selftests/sgx/test_encl_bootstrap.S | 30 +++++++++++-------- > 1 file changed, 18 insertions(+), 12 deletions(-) > > diff --git a/tools/testing/selftests/sgx/test_encl_bootstrap.S b/tools/testing/selftests/sgx/test_encl_bootstrap.S > index 82fb0dfcbd23..cc2353f38bcc 100644 > --- a/tools/testing/selftests/sgx/test_encl_bootstrap.S > +++ b/tools/testing/selftests/sgx/test_encl_bootstrap.S > @@ -10,12 +10,13 @@ > .section ".tcs", "aw" > .balign 4096 > > +encl_tcs1: > .fill 1, 8, 0 # STATE (set by CPU) > .fill 1, 8, 0 # FLAGS > .quad encl_ssa_tcs1 # OSSA > .fill 1, 4, 0 # CSSA (set by CPU) > .fill 1, 4, 1 # NSSA > - .quad encl_entry # OENTRY > + .quad encl_entry1 # OENTRY > .fill 1, 8, 0 # AEP (set by EENTER and ERESUME) > .fill 1, 8, 0 # OFSBASE > .fill 1, 8, 0 # OGSBASE > @@ -23,13 +24,13 @@ > .fill 1, 4, 0xFFFFFFFF # GSLIMIT > .fill 4024, 1, 0 # Reserved > > - # TCS2 > +encl_tcs2: > .fill 1, 8, 0 # STATE (set by CPU) > .fill 1, 8, 0 # FLAGS > .quad encl_ssa_tcs2 # OSSA > .fill 1, 4, 0 # CSSA (set by CPU) > .fill 1, 4, 1 # NSSA > - .quad encl_entry # OENTRY > + .quad encl_entry2 # OENTRY > .fill 1, 8, 0 # AEP (set by EENTER and ERESUME) > .fill 1, 8, 0 # OFSBASE > .fill 1, 8, 0 # OGSBASE > @@ -39,15 +40,19 @@ > > .text > > -encl_entry: > - # RBX contains the base address for TCS, which is the first address > - # inside the enclave for TCS #1 and one page into the enclave for > - # TCS #2. By adding the value of encl_stack to it, we get > - # the absolute address for the stack. > - lea (encl_stack)(%rbx), %rax > +encl_entry1: > + lea (encl_stack1)(%rip), %rax > xchg %rsp, %rax > push %rax > + jmp encl_continue > > +encl_entry2: > + lea (encl_stack2)(%rip), %rax > + xchg %rsp, %rax > + push %rax > + jmp encl_continue > + The code duplication (xchg and push) is not needed. > +encl_continue: > push %rcx # push the address after EENTER > push %rbx # push the enclave base address > > @@ -84,13 +89,14 @@ encl_entry: > > encl_ssa_tcs1: > .space 4096 > + > encl_ssa_tcs2: > .space 4096 > > +encl_stack1: Stack grows the other way so by placing the entry here the stack of TCS #1 will clobber the SSA of TCS #2. > .balign 4096 > - # Stack of TCS #1 > .space 4096 > -encl_stack: > + > +encl_stack2: Here the stack of TCS #2 will actually use the stack of TCS #1. > .balign 4096 > - # Stack of TCS #2 > .space 4096 Last page will be unused. Reinette
On Wed, 2022-03-30 at 17:10 -0700, Reinette Chatre wrote: > Hi Jarkko, > > On 3/30/2022 3:28 PM, Jarkko Sakkinen wrote: > > Simplify the test_encl_bootstrap.S flow by using RIP-relative addressing. > > It is not clear to me how this is simpler. At this point there is no > functional change (except for what appears to be an unintended bug - more below). > At this time the change seems more code utilizing subtle compiler features > to accomplish the same. > > Could you please share more about your plans following this change? I need > to understand this better since it is also an area changed by the SGX2 testing > code. > > > The compiler automatically puts relative addresses for RIP index addresses. > > I was not aware of this. A comment would be helpful to understand the implementation. > > > > > In order to get a clean and tweakless solution, define separate entry point > > for each TCS. > > > > Cc: Reinette Chatre <reinette.chatre@intel.com> > > Cc: Dave Hansen <dave.hansen@linux.intel.com> > > Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org> > > --- > > v2: > > * Based on Reinette's example, make proper structuring with separate > > entry points for each TCS. > > --- > > .../selftests/sgx/test_encl_bootstrap.S | 30 +++++++++++-------- > > 1 file changed, 18 insertions(+), 12 deletions(-) > > > > diff --git a/tools/testing/selftests/sgx/test_encl_bootstrap.S b/tools/testing/selftests/sgx/test_encl_bootstrap.S > > index 82fb0dfcbd23..cc2353f38bcc 100644 > > --- a/tools/testing/selftests/sgx/test_encl_bootstrap.S > > +++ b/tools/testing/selftests/sgx/test_encl_bootstrap.S > > @@ -10,12 +10,13 @@ > > .section ".tcs", "aw" > > .balign 4096 > > > > +encl_tcs1: > > .fill 1, 8, 0 # STATE (set by CPU) > > .fill 1, 8, 0 # FLAGS > > .quad encl_ssa_tcs1 # OSSA > > .fill 1, 4, 0 # CSSA (set by CPU) > > .fill 1, 4, 1 # NSSA > > - .quad encl_entry # OENTRY > > + .quad encl_entry1 # OENTRY > > .fill 1, 8, 0 # AEP (set by EENTER and ERESUME) > > .fill 1, 8, 0 # OFSBASE > > .fill 1, 8, 0 # OGSBASE > > @@ -23,13 +24,13 @@ > > .fill 1, 4, 0xFFFFFFFF # GSLIMIT > > .fill 4024, 1, 0 # Reserved > > > > - # TCS2 > > +encl_tcs2: > > .fill 1, 8, 0 # STATE (set by CPU) > > .fill 1, 8, 0 # FLAGS > > .quad encl_ssa_tcs2 # OSSA > > .fill 1, 4, 0 # CSSA (set by CPU) > > .fill 1, 4, 1 # NSSA > > - .quad encl_entry # OENTRY > > + .quad encl_entry2 # OENTRY > > .fill 1, 8, 0 # AEP (set by EENTER and ERESUME) > > .fill 1, 8, 0 # OFSBASE > > .fill 1, 8, 0 # OGSBASE > > @@ -39,15 +40,19 @@ > > > > .text > > > > -encl_entry: > > - # RBX contains the base address for TCS, which is the first address > > - # inside the enclave for TCS #1 and one page into the enclave for > > - # TCS #2. By adding the value of encl_stack to it, we get > > - # the absolute address for the stack. > > - lea (encl_stack)(%rbx), %rax > > +encl_entry1: > > + lea (encl_stack1)(%rip), %rax > > xchg %rsp, %rax > > push %rax > > + jmp encl_continue > > > > +encl_entry2: > > + lea (encl_stack2)(%rip), %rax > > + xchg %rsp, %rax > > + push %rax > > + jmp encl_continue > > + > > The code duplication (xchg and push) is not needed. > > > +encl_continue: > > push %rcx # push the address after EENTER > > push %rbx # push the enclave base address > > > > @@ -84,13 +89,14 @@ encl_entry: > > > > encl_ssa_tcs1: > > .space 4096 > > + > > encl_ssa_tcs2: > > .space 4096 > > > > +encl_stack1: > > Stack grows the other way so by placing the entry here the stack of > TCS #1 will clobber the SSA of TCS #2. > > > .balign 4096 > > - # Stack of TCS #1 > > .space 4096 > > -encl_stack: > > + > > +encl_stack2: > > Here the stack of TCS #2 will actually use the stack of TCS #1. > > > .balign 4096 > > - # Stack of TCS #2 > > .space 4096 > > Last page will be unused. > > Reinette Thanks for the remarks. I'll fix the issues. BR, Jarkko
diff --git a/tools/testing/selftests/sgx/test_encl_bootstrap.S b/tools/testing/selftests/sgx/test_encl_bootstrap.S index 82fb0dfcbd23..cc2353f38bcc 100644 --- a/tools/testing/selftests/sgx/test_encl_bootstrap.S +++ b/tools/testing/selftests/sgx/test_encl_bootstrap.S @@ -10,12 +10,13 @@ .section ".tcs", "aw" .balign 4096 +encl_tcs1: .fill 1, 8, 0 # STATE (set by CPU) .fill 1, 8, 0 # FLAGS .quad encl_ssa_tcs1 # OSSA .fill 1, 4, 0 # CSSA (set by CPU) .fill 1, 4, 1 # NSSA - .quad encl_entry # OENTRY + .quad encl_entry1 # OENTRY .fill 1, 8, 0 # AEP (set by EENTER and ERESUME) .fill 1, 8, 0 # OFSBASE .fill 1, 8, 0 # OGSBASE @@ -23,13 +24,13 @@ .fill 1, 4, 0xFFFFFFFF # GSLIMIT .fill 4024, 1, 0 # Reserved - # TCS2 +encl_tcs2: .fill 1, 8, 0 # STATE (set by CPU) .fill 1, 8, 0 # FLAGS .quad encl_ssa_tcs2 # OSSA .fill 1, 4, 0 # CSSA (set by CPU) .fill 1, 4, 1 # NSSA - .quad encl_entry # OENTRY + .quad encl_entry2 # OENTRY .fill 1, 8, 0 # AEP (set by EENTER and ERESUME) .fill 1, 8, 0 # OFSBASE .fill 1, 8, 0 # OGSBASE @@ -39,15 +40,19 @@ .text -encl_entry: - # RBX contains the base address for TCS, which is the first address - # inside the enclave for TCS #1 and one page into the enclave for - # TCS #2. By adding the value of encl_stack to it, we get - # the absolute address for the stack. - lea (encl_stack)(%rbx), %rax +encl_entry1: + lea (encl_stack1)(%rip), %rax xchg %rsp, %rax push %rax + jmp encl_continue +encl_entry2: + lea (encl_stack2)(%rip), %rax + xchg %rsp, %rax + push %rax + jmp encl_continue + +encl_continue: push %rcx # push the address after EENTER push %rbx # push the enclave base address @@ -84,13 +89,14 @@ encl_entry: encl_ssa_tcs1: .space 4096 + encl_ssa_tcs2: .space 4096 +encl_stack1: .balign 4096 - # Stack of TCS #1 .space 4096 -encl_stack: + +encl_stack2: .balign 4096 - # Stack of TCS #2 .space 4096
Simplify the test_encl_bootstrap.S flow by using RIP-relative addressing. The compiler automatically puts relative addresses for RIP index addresses. In order to get a clean and tweakless solution, define separate entry point for each TCS. Cc: Reinette Chatre <reinette.chatre@intel.com> Cc: Dave Hansen <dave.hansen@linux.intel.com> Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org> --- v2: * Based on Reinette's example, make proper structuring with separate entry points for each TCS. --- .../selftests/sgx/test_encl_bootstrap.S | 30 +++++++++++-------- 1 file changed, 18 insertions(+), 12 deletions(-)