| Message ID | 20230515221517.646549-1-nikos.nikoleris@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [kvm-unit-tests] arm64: Make vector_table and vector_stub weak symbols | expand |
On Mon, May 15, 2023 at 11:15:17PM +0100, Nikos Nikoleris wrote: > This changes allows a test to define and override the declared symbols, > taking control of the whole vector_table or a vector_stub. Hi Nikos, Can you add some motivation for this change to the commit message or submit it along with some test that needs it? Thanks, drew > > Signed-off-by: Nikos Nikoleris <nikos.nikoleris@arm.com> > --- > arm/cstart64.S | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/arm/cstart64.S b/arm/cstart64.S > index e4ab7d06..eda0daa5 100644 > --- a/arm/cstart64.S > +++ b/arm/cstart64.S > @@ -275,8 +275,11 @@ exceptions_init: > /* > * Vector stubs > * Adapted from arch/arm64/kernel/entry.S > + * Declare as weak to allow external tests to redefine and override a > + * vector_stub. > */ > .macro vector_stub, name, vec > +.weak \name > \name: > stp x0, x1, [sp, #-S_FRAME_SIZE]! > stp x2, x3, [sp, #16] > @@ -369,7 +372,13 @@ vector_stub el0_error_32, 15 > b \label > .endm > > + > +/* > + * Declare as weak to allow external tests to redefine and override the > + * default vector table. > + */ > .align 11 > +.weak vector_table > vector_table: > ventry el1t_sync // Synchronous EL1t > ventry el1t_irq // IRQ EL1t > -- > 2.25.1 >
On 18/05/2023 17:06, Andrew Jones wrote: > On Mon, May 15, 2023 at 11:15:17PM +0100, Nikos Nikoleris wrote: >> This changes allows a test to define and override the declared symbols, >> taking control of the whole vector_table or a vector_stub. > > Hi Nikos, > > Can you add some motivation for this change to the commit message or > submit it along with some test that needs it? > Hi Drew, Thanks for reviewing this. What do you think about adding the following to the commit message? > With the ability to override specific exception handlers, litmus7 > [1] a tool used to generate c sources for a given memory model > litmus test, can override the el1h_sync symbol to implement tests > with explicit exception handlers. For example: > > AArch64 LDRv0+I2V-dsb.ishst > { > [PTE(x)]=(oa:PA(x),valid:0); > x=1; > > 0:X1=x; > 0:X3=PTE(x); > 0:X2=(oa:PA(x),valid:1); > }> P0 | P0.F ; > L0: | ADD X8,X8,#1 ; > LDR W0,[X1] | STR X2,[X3] ; > | DSB ISHST ; > | ERET ; > exists(0:X0=0 \/ 0:X8!=1) > > In this test, a thread running in core P0 executes a load to a memory > location x. The PTE of the virtual address x is initially invalid. > The execution of the load causes a synchronous EL1 exception which is > handled by the code in P0.F. P0.F increments a counter which is > maintained in X8, updates the PTE of x and makes it valid, executes a > DSB ISHST and calls ERET which is expected to return and retry the > execution of the load in P0:L0. > > The postcondition checks if there is any execution where the load > wasn't executed (X0 its destination register is not update), or that > the P0.F > handler was invoked more than once (the counter X8 is not > 1). > > For this tests, litmus7 needs to control the el1h_sync. Calling > install_exception_handler() would be suboptimal because the > vector_stub would wrap around the code of P0.F and disturb the test. > > [1]: https://diy.inria.fr/doc/litmus.html If you think this is sufficient, I will update the patch. If not I could look into adding a test in KUT but generally the code of these tests are generated by a tool and the coding style (if one can call it a style) is very different. Thanks, Nikos > Thanks, > drew > >> >> Signed-off-by: Nikos Nikoleris <nikos.nikoleris@arm.com> >> --- >> arm/cstart64.S | 9 +++++++++ >> 1 file changed, 9 insertions(+) >> >> diff --git a/arm/cstart64.S b/arm/cstart64.S >> index e4ab7d06..eda0daa5 100644 >> --- a/arm/cstart64.S >> +++ b/arm/cstart64.S >> @@ -275,8 +275,11 @@ exceptions_init: >> /* >> * Vector stubs >> * Adapted from arch/arm64/kernel/entry.S >> + * Declare as weak to allow external tests to redefine and override a >> + * vector_stub. >> */ >> .macro vector_stub, name, vec >> +.weak \name >> \name: >> stp x0, x1, [sp, #-S_FRAME_SIZE]! >> stp x2, x3, [sp, #16] >> @@ -369,7 +372,13 @@ vector_stub el0_error_32, 15 >> b \label >> .endm >> >> + >> +/* >> + * Declare as weak to allow external tests to redefine and override the >> + * default vector table. >> + */ >> .align 11 >> +.weak vector_table >> vector_table: >> ventry el1t_sync // Synchronous EL1t >> ventry el1t_irq // IRQ EL1t >> -- >> 2.25.1 >>
On Mon, May 22, 2023 at 02:54:14PM +0100, Nikos Nikoleris wrote: > On 18/05/2023 17:06, Andrew Jones wrote: > > On Mon, May 15, 2023 at 11:15:17PM +0100, Nikos Nikoleris wrote: > > > This changes allows a test to define and override the declared symbols, > > > taking control of the whole vector_table or a vector_stub. > > > > Hi Nikos, > > > > Can you add some motivation for this change to the commit message or > > submit it along with some test that needs it? > > > > Hi Drew, > > Thanks for reviewing this. > > What do you think about adding the following to the commit message? > > > With the ability to override specific exception handlers, litmus7 > > [1] a tool used to generate c sources for a given memory model > > litmus test, can override the el1h_sync symbol to implement tests > > with explicit exception handlers. For example: > > > > AArch64 LDRv0+I2V-dsb.ishst > > { > [PTE(x)]=(oa:PA(x),valid:0); > > x=1; > > > > 0:X1=x; > > 0:X3=PTE(x); > 0:X2=(oa:PA(x),valid:1); > > }> P0 | P0.F ; > > L0: | ADD X8,X8,#1 ; > > LDR W0,[X1] | STR X2,[X3] ; > > | DSB ISHST ; > > | ERET ; > exists(0:X0=0 \/ 0:X8!=1) > > > > In this test, a thread running in core P0 executes a load to a memory > > location x. The PTE of the virtual address x is initially invalid. > > The execution of the load causes a synchronous EL1 exception which is > > handled by the code in P0.F. P0.F increments a counter which is > > maintained in X8, updates the PTE of x and makes it valid, executes a > > DSB ISHST and calls ERET which is expected to return and retry the > > execution of the load in P0:L0. > > > > The postcondition checks if there is any execution where the load wasn't > > executed (X0 its destination register is not update), or that the P0.F > > > handler was invoked more than once (the counter X8 is not 1). > > > > For this tests, litmus7 needs to control the el1h_sync. Calling > > install_exception_handler() would be suboptimal because the vector_stub > > would wrap around the code of P0.F and disturb the test. > > > > [1]: https://diy.inria.fr/doc/litmus.html > If you think this is sufficient, I will update the patch. The above works for me. (Unrelated: Sorry I haven't had a chance to give your latest efi branch a test drive. I think you can probably go ahead and post the next version of the series, though. That'll help bring it to the forefront for me to prioritize.) Thanks, drew
On 23/05/2023 09:56, Andrew Jones wrote: > On Mon, May 22, 2023 at 02:54:14PM +0100, Nikos Nikoleris wrote: >> On 18/05/2023 17:06, Andrew Jones wrote: >>> On Mon, May 15, 2023 at 11:15:17PM +0100, Nikos Nikoleris wrote: >>>> This changes allows a test to define and override the declared symbols, >>>> taking control of the whole vector_table or a vector_stub. >>> >>> Hi Nikos, >>> >>> Can you add some motivation for this change to the commit message or >>> submit it along with some test that needs it? >>> >> >> Hi Drew, >> >> Thanks for reviewing this. >> >> What do you think about adding the following to the commit message? >> >>> With the ability to override specific exception handlers, litmus7 >>> [1] a tool used to generate c sources for a given memory model >>> litmus test, can override the el1h_sync symbol to implement tests >>> with explicit exception handlers. For example: >>> >>> AArch64 LDRv0+I2V-dsb.ishst >>> { > [PTE(x)]=(oa:PA(x),valid:0); >>> x=1; >>> >>> 0:X1=x; >>> 0:X3=PTE(x); > 0:X2=(oa:PA(x),valid:1); >>> }> P0 | P0.F ; >>> L0: | ADD X8,X8,#1 ; >>> LDR W0,[X1] | STR X2,[X3] ; >>> | DSB ISHST ; >>> | ERET ; > exists(0:X0=0 \/ 0:X8!=1) >>> >>> In this test, a thread running in core P0 executes a load to a memory >>> location x. The PTE of the virtual address x is initially invalid. >>> The execution of the load causes a synchronous EL1 exception which is >>> handled by the code in P0.F. P0.F increments a counter which is >>> maintained in X8, updates the PTE of x and makes it valid, executes a >>> DSB ISHST and calls ERET which is expected to return and retry the >>> execution of the load in P0:L0. >>> >>> The postcondition checks if there is any execution where the load wasn't >>> executed (X0 its destination register is not update), or that the P0.F > >>> handler was invoked more than once (the counter X8 is not 1). >>> >>> For this tests, litmus7 needs to control the el1h_sync. Calling >>> install_exception_handler() would be suboptimal because the vector_stub >>> would wrap around the code of P0.F and disturb the test. >>> >>> [1]: https://diy.inria.fr/doc/litmus.html >> If you think this is sufficient, I will update the patch. > > The above works for me. > > (Unrelated: Sorry I haven't had a chance to give your latest efi branch > a test drive. I think you can probably go ahead and post the next version > of the series, though. That'll help bring it to the forefront for me to > prioritize.) > Thanks Drew, I'll send a new revision of this patch and the EFI series. Nikos
diff --git a/arm/cstart64.S b/arm/cstart64.S index e4ab7d06..eda0daa5 100644 --- a/arm/cstart64.S +++ b/arm/cstart64.S @@ -275,8 +275,11 @@ exceptions_init: /* * Vector stubs * Adapted from arch/arm64/kernel/entry.S + * Declare as weak to allow external tests to redefine and override a + * vector_stub. */ .macro vector_stub, name, vec +.weak \name \name: stp x0, x1, [sp, #-S_FRAME_SIZE]! stp x2, x3, [sp, #16] @@ -369,7 +372,13 @@ vector_stub el0_error_32, 15 b \label .endm + +/* + * Declare as weak to allow external tests to redefine and override the + * default vector table. + */ .align 11 +.weak vector_table vector_table: ventry el1t_sync // Synchronous EL1t ventry el1t_irq // IRQ EL1t
This changes allows a test to define and override the declared symbols, taking control of the whole vector_table or a vector_stub. Signed-off-by: Nikos Nikoleris <nikos.nikoleris@arm.com> --- arm/cstart64.S | 9 +++++++++ 1 file changed, 9 insertions(+)