Re: [Minios-devel] [UNIKRAFT PATCHv3 2/5] arch/arm64: Define the size of callee-saved-registers for thread switch

[Santiago Pagani <Santiago.Pagani@xxxxxxxxx>]

Hi Justin,

Thanks fort he patch. Please see my comments inline.

On 31.07.19, 04:48, "Jia He" <justin.he@xxxxxxx> wrote:

    From: Wei Chen <wei.chen@xxxxxxx>
    
    In thread context switch, we will save the callee-saved registers
    (x19 ~ x28) and Frame Point Register and Link Register to prev's
    thread stack [1]
    
    [1] 
http://infocenter.arm.com/help/topic/com.arm.doc.ihi0055b/IHI0055B_aapcs64.pdf
    
    Signed-off-by: Wei Chen <wei.chen@xxxxxxx>
    Signed-off-by: Jianyong Wu <jianyong.wu@xxxxxxx>
    Signed-off-by: Jia He <justin.he@xxxxxxx>
    ---
     arch/arm/arm64/include/uk/asm/lcpu.h | 24 ++++++++++++++++++++++++
     1 file changed, 24 insertions(+)
    
    diff --git a/arch/arm/arm64/include/uk/asm/lcpu.h 
b/arch/arm/arm64/include/uk/asm/lcpu.h
    index 9eea746..9d839a6 100644
    --- a/arch/arm/arm64/include/uk/asm/lcpu.h
    +++ b/arch/arm/arm64/include/uk/asm/lcpu.h
    @@ -44,6 +44,15 @@
     #define __TRAP_STACK_SIZE  288
     #define __SP_OFFSET                272
     #define __SP_EL0_OFFSET            280
    +
    +/*
    + * In thread context switch, we will save the callee-saved registers
    + * (x19 ~ x28) and Frame Point Register and Link Register to prev's
    + * thread stack:
    + * 
http://infocenter.arm.com/help/topic/com.arm.doc.ihi0055b/IHI0055B_aapcs64.pdf
    + */
    +#define __CALLEE_SAVED_SIZE    96
    +
     #else
     /*
      * Change this structure must update TRAP_STACK_SIZE at the same time.
    @@ -72,6 +81,21 @@ struct __regs {
        unsigned long sp_el0;
     };
     
    +/*
    + * Change this structure must update __CALLEE_SAVED_SIZE at the
    + * same time.
    + */
    +struct __callee_saved_regs {
    +   /* Callee-saved registers, from x19 ~ x28 */
    +   unsigned long callee[10];
I would suggest to use uint64_t here. Not that unsigned long is not the correct 
type, just that it makes it very explicit that this is to save 64-bit registers.
    +
    +   /* Frame Point Register (x29) */
    +   unsigned long fp;
    +
    +   /* Link Register (x30) */
    +   unsigned long lr;
    +};
    +
     /*
      * Instruction Synchronization Barrier flushes the pipeline in the
      * processor, so that all instructions following the ISB are fetched
    -- 
    2.17.1
    
    
    

_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel