RE: [PATCH v5 1/9] xen/arm: smccc: add support for SMCCCv1.2 extended input/output registers

[Jiamei Xie <Jiamei.Xie@xxxxxxx>]

Hi
I build and run it on armv8a, and start dom0 with two cpus. Cpu off and on 
tests passed. It seems it don't break current cpu basic functions.

Best wishes
Jiamei Xie


> -----Original Message-----
> From: Xen-devel <xen-devel-bounces@xxxxxxxxxxxxxxxxxxxx> On Behalf Of
> Jens Wiklander
> Sent: Thursday, August 18, 2022 6:56 PM
> To: xen-devel@xxxxxxxxxxxxxxxxxxxx
> Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>; Julien Grall 
> <julien@xxxxxxx>;
> Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>; Bertrand Marquis
> <Bertrand.Marquis@xxxxxxx>; Anthony PERARD
> <anthony.perard@xxxxxxxxxx>; Juergen Gross <jgross@xxxxxxxx>; Wei Liu
> <wl@xxxxxxx>; Jens Wiklander <jens.wiklander@xxxxxxxxxx>; Luca Fancellu
> <Luca.Fancellu@xxxxxxx>
> Subject: [PATCH v5 1/9] xen/arm: smccc: add support for SMCCCv1.2
> extended input/output registers
> 
> SMCCC v1.2 [1] AArch64 allows x0-x17 to be used as both parameter
> registers and result registers for the SMC and HVC instructions.
> 
> Arm Firmware Framework for Armv8-A specification makes use of x0-x7 as
> parameter and result registers.
> 
> Let us add new interface to support this extended set of input/output
> registers.
> 
> This is based on 3fdc0cb59d97 ("arm64: smccc: Add support for SMCCCv1.2
> extended input/output registers") by Sudeep Holla from the Linux kernel
> 
> The SMCCC version reported to the VM is bumped to 1.2 in order to support
> handling FF-A messages.
> 
> [1] https://developer.arm.com/documentation/den0028/c/?lang=en
> 
> Reviewed-by: Luca Fancellu <luca.fancellu@xxxxxxx>
> Signed-off-by: Jens Wiklander <jens.wiklander@xxxxxxxxxx>
> ---
>  xen/arch/arm/arm64/asm-offsets.c |  9 +++++++
>  xen/arch/arm/arm64/smc.S         | 43
> ++++++++++++++++++++++++++++++++
>  xen/arch/arm/include/asm/smccc.h | 40
> +++++++++++++++++++++++++++++
>  xen/arch/arm/vsmc.c              |  2 +-
>  4 files changed, 93 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/arch/arm/arm64/asm-offsets.c b/xen/arch/arm/arm64/asm-
> offsets.c
> index 280ddb55bfd4..1721e1ed26e1 100644
> --- a/xen/arch/arm/arm64/asm-offsets.c
> +++ b/xen/arch/arm/arm64/asm-offsets.c
> @@ -56,6 +56,15 @@ void __dummy__(void)
>     BLANK();
>     OFFSET(SMCCC_RES_a0, struct arm_smccc_res, a0);
>     OFFSET(SMCCC_RES_a2, struct arm_smccc_res, a2);
> +   OFFSET(ARM_SMCCC_1_2_REGS_X0_OFFS, struct arm_smccc_1_2_regs,
> a0);
> +   OFFSET(ARM_SMCCC_1_2_REGS_X2_OFFS, struct arm_smccc_1_2_regs,
> a2);
> +   OFFSET(ARM_SMCCC_1_2_REGS_X4_OFFS, struct arm_smccc_1_2_regs,
> a4);
> +   OFFSET(ARM_SMCCC_1_2_REGS_X6_OFFS, struct arm_smccc_1_2_regs,
> a6);
> +   OFFSET(ARM_SMCCC_1_2_REGS_X8_OFFS, struct arm_smccc_1_2_regs,
> a8);
> +   OFFSET(ARM_SMCCC_1_2_REGS_X10_OFFS, struct arm_smccc_1_2_regs,
> a10);
> +   OFFSET(ARM_SMCCC_1_2_REGS_X12_OFFS, struct arm_smccc_1_2_regs,
> a12);
> +   OFFSET(ARM_SMCCC_1_2_REGS_X14_OFFS, struct arm_smccc_1_2_regs,
> a14);
> +   OFFSET(ARM_SMCCC_1_2_REGS_X16_OFFS, struct arm_smccc_1_2_regs,
> a16);
>  }
> 
>  /*
> diff --git a/xen/arch/arm/arm64/smc.S b/xen/arch/arm/arm64/smc.S
> index 91bae62dd4d2..c546192e7f2d 100644
> --- a/xen/arch/arm/arm64/smc.S
> +++ b/xen/arch/arm/arm64/smc.S
> @@ -27,3 +27,46 @@ ENTRY(__arm_smccc_1_0_smc)
>          stp     x2, x3, [x4, #SMCCC_RES_a2]
>  1:
>          ret
> +
> +
> +/*
> + * void arm_smccc_1_2_smc(const struct arm_smccc_1_2_regs *args,
> + *                        struct arm_smccc_1_2_regs *res)
> + */
> +ENTRY(arm_smccc_1_2_smc)
> +    /* Save `res` and free a GPR that won't be clobbered */
> +    stp     x1, x19, [sp, #-16]!
> +
> +    /* Ensure `args` won't be clobbered while loading regs in next step */
> +    mov      x19, x0
> +
> +    /* Load the registers x0 - x17 from the struct arm_smccc_1_2_regs */
> +    ldp      x0, x1, [x19, #ARM_SMCCC_1_2_REGS_X0_OFFS]
> +    ldp      x2, x3, [x19, #ARM_SMCCC_1_2_REGS_X2_OFFS]
> +    ldp      x4, x5, [x19, #ARM_SMCCC_1_2_REGS_X4_OFFS]
> +    ldp      x6, x7, [x19, #ARM_SMCCC_1_2_REGS_X6_OFFS]
> +    ldp      x8, x9, [x19, #ARM_SMCCC_1_2_REGS_X8_OFFS]
> +    ldp      x10, x11, [x19, #ARM_SMCCC_1_2_REGS_X10_OFFS]
> +    ldp      x12, x13, [x19, #ARM_SMCCC_1_2_REGS_X12_OFFS]
> +    ldp      x14, x15, [x19, #ARM_SMCCC_1_2_REGS_X14_OFFS]
> +    ldp      x16, x17, [x19, #ARM_SMCCC_1_2_REGS_X16_OFFS]
> +
> +    smc #0
> +
> +    /* Load the `res` from the stack */
> +    ldr      x19, [sp]
> +
> +    /* Store the registers x0 - x17 into the result structure */
> +    stp      x0, x1, [x19, #ARM_SMCCC_1_2_REGS_X0_OFFS]
> +    stp      x2, x3, [x19, #ARM_SMCCC_1_2_REGS_X2_OFFS]
> +    stp      x4, x5, [x19, #ARM_SMCCC_1_2_REGS_X4_OFFS]
> +    stp      x6, x7, [x19, #ARM_SMCCC_1_2_REGS_X6_OFFS]
> +    stp      x8, x9, [x19, #ARM_SMCCC_1_2_REGS_X8_OFFS]
> +    stp      x10, x11, [x19, #ARM_SMCCC_1_2_REGS_X10_OFFS]
> +    stp      x12, x13, [x19, #ARM_SMCCC_1_2_REGS_X12_OFFS]
> +    stp      x14, x15, [x19, #ARM_SMCCC_1_2_REGS_X14_OFFS]
> +    stp      x16, x17, [x19, #ARM_SMCCC_1_2_REGS_X16_OFFS]
> +
> +    /* Restore original x19 */
> +    ldp     xzr, x19, [sp], #16
> +    ret
> diff --git a/xen/arch/arm/include/asm/smccc.h
> b/xen/arch/arm/include/asm/smccc.h
> index b3dbeecc90ad..b5e3f67eb34e 100644
> --- a/xen/arch/arm/include/asm/smccc.h
> +++ b/xen/arch/arm/include/asm/smccc.h
> @@ -33,6 +33,7 @@
> 
>  #define ARM_SMCCC_VERSION_1_0   SMCCC_VERSION(1, 0)
>  #define ARM_SMCCC_VERSION_1_1   SMCCC_VERSION(1, 1)
> +#define ARM_SMCCC_VERSION_1_2   SMCCC_VERSION(1, 2)
> 
>  /*
>   * This file provides common defines for ARM SMC Calling Convention as
> @@ -265,6 +266,45 @@ void __arm_smccc_1_0_smc(register_t a0,
> register_t a1, register_t a2,
>          else                                                    \
>              arm_smccc_1_0_smc(__VA_ARGS__);                     \
>      } while ( 0 )
> +
> +/**
> + * struct arm_smccc_1_2_regs - Arguments for or Results from SMC call
> + * @a0-a17 argument values from registers 0 to 17
> + */
> +struct arm_smccc_1_2_regs {
> +    unsigned long a0;
> +    unsigned long a1;
> +    unsigned long a2;
> +    unsigned long a3;
> +    unsigned long a4;
> +    unsigned long a5;
> +    unsigned long a6;
> +    unsigned long a7;
> +    unsigned long a8;
> +    unsigned long a9;
> +    unsigned long a10;
> +    unsigned long a11;
> +    unsigned long a12;
> +    unsigned long a13;
> +    unsigned long a14;
> +    unsigned long a15;
> +    unsigned long a16;
> +    unsigned long a17;
> +};
> +
> +/**
> + * arm_smccc_1_2_smc() - make SMC calls
> + * @args: arguments passed via struct arm_smccc_1_2_regs
> + * @res: result values via struct arm_smccc_1_2_regs
> + *
> + * This function is used to make SMC calls following SMC Calling Convention
> + * v1.2 or above. The content of the supplied param are copied from the
> + * structure to registers prior to the SMC instruction. The return values
> + * are updated with the content from registers on return from the SMC
> + * instruction.
> + */
> +void arm_smccc_1_2_smc(const struct arm_smccc_1_2_regs *args,
> +                       struct arm_smccc_1_2_regs *res);
>  #endif /* CONFIG_ARM_64 */
> 
>  #endif /* __ASSEMBLY__ */
> diff --git a/xen/arch/arm/vsmc.c b/xen/arch/arm/vsmc.c
> index 676740ef1520..6f90c08a6304 100644
> --- a/xen/arch/arm/vsmc.c
> +++ b/xen/arch/arm/vsmc.c
> @@ -93,7 +93,7 @@ static bool handle_arch(struct cpu_user_regs *regs)
>      switch ( fid )
>      {
>      case ARM_SMCCC_VERSION_FID:
> -        set_user_reg(regs, 0, ARM_SMCCC_VERSION_1_1);
> +        set_user_reg(regs, 0, ARM_SMCCC_VERSION_1_2);
>          return true;
> 
>      case ARM_SMCCC_ARCH_FEATURES_FID:
> --
> 2.31.1
>