Re: [edk2] [PATCH v7 03/24] ArmPkg: add ArmHvcLib

Subject: Re: [edk2] [PATCH v7 03/24] ArmPkg: add ArmHvcLib

From: Laszlo Ersek <lersek@redhat.com>

To: Ard Biesheuvel <ard.biesheuvel@linaro.org>, edk2-devel@lists.sourceforge.net, olivier.martin@arm.com

Date: 2014-09-07 03:11:12

two notes:

(1) a great many ARM libraries don't use EFIAPI, so I'll assume that
EFIAPI is a no-op on ARM. This library is certainly not an exception;
under ArmPkg/Include/Library, there are 7 library headers without the
word EFIAPI in them. Okay.

On 09/05/14 13:56, Ard Biesheuvel wrote:
> This is a utility library closely modeled after ArmSmcLib, that allows
> hypervisor call (HVC) instructions to be issued from C code.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel 
> ---
>  ArmPkg/Include/Library/ArmHvcLib.h        | 42 ++++++++++++++++++++++++++++++
>  ArmPkg/Library/ArmHvcLib/AArch64/ArmHvc.S | 38 +++++++++++++++++++++++++++
>  ArmPkg/Library/ArmHvcLib/Arm/ArmHvc.S     | 43 +++++++++++++++++++++++++++++++
>  ArmPkg/Library/ArmHvcLib/Arm/ArmHvc.asm   | 43 +++++++++++++++++++++++++++++++
>  ArmPkg/Library/ArmHvcLib/ArmHvcLib.inf    | 32 +++++++++++++++++++++++
>  5 files changed, 198 insertions(+)
>  create mode 100644 ArmPkg/Include/Library/ArmHvcLib.h
>  create mode 100644 ArmPkg/Library/ArmHvcLib/AArch64/ArmHvc.S
>  create mode 100644 ArmPkg/Library/ArmHvcLib/Arm/ArmHvc.S
>  create mode 100644 ArmPkg/Library/ArmHvcLib/Arm/ArmHvc.asm
>  create mode 100644 ArmPkg/Library/ArmHvcLib/ArmHvcLib.inf
> 
> diff --git a/ArmPkg/Include/Library/ArmHvcLib.h b/ArmPkg/Include/Library/ArmHvcLib.h
> new file mode 100644
> index 000000000000..b5c3d81cdf3d
> --- /dev/null
> +++ b/ArmPkg/Include/Library/ArmHvcLib.h
> @@ -0,0 +1,42 @@
> +/** @file
> +*
> +*  Copyright (c) 2012-2014, ARM Limited. All rights reserved.
> +*
> +*  This program and the accompanying materials
> +*  are licensed and made available under the terms and conditions of the BSD License
> +*  which accompanies this distribution.  The full text of the license may be found at
> +*  http://opensource.org/licenses/bsd-license.php
> +*
> +*  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +*  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +*
> +**/
> +
> +#ifndef __ARM_HVC_LIB__
> +#define __ARM_HVC_LIB__
> +
> +/**
> + * The size of the HVC arguments are different between AArch64 and AArch32.
> + * The native size is used for the arguments.
> + */
> +typedef struct {
> +  UINTN  Arg0;
> +  UINTN  Arg1;
> +  UINTN  Arg2;
> +  UINTN  Arg3;
> +} ARM_HVC_ARGS;
> +
> +/**
> +  Trigger an HVC call
> +
> +  HVC calls can take up to 4 arguments and return up to 4 return values.
> +  Therefore, the 4 first fields in the ARM_HVC_ARGS structure are used
> +  for both input and output values.
> +
> +**/
> +VOID
> +ArmCallHvc (
> +  IN OUT ARM_HVC_ARGS *Args
> +  );
> +
> +#endif
> diff --git a/ArmPkg/Library/ArmHvcLib/AArch64/ArmHvc.S b/ArmPkg/Library/ArmHvcLib/AArch64/ArmHvc.S
> new file mode 100644
> index 000000000000..156e5554cf5b
> --- /dev/null
> +++ b/ArmPkg/Library/ArmHvcLib/AArch64/ArmHvc.S
> @@ -0,0 +1,38 @@
> +//
> +//  Copyright (c) 2012-2014, ARM Limited. All rights reserved.
> +//  Copyright (c) 2014, Linaro Limited. All rights reserved.
> +//
> +//  This program and the accompanying materials
> +//  are licensed and made available under the terms and conditions of the BSD License
> +//  which accompanies this distribution.  The full text of the license may be found at
> +//  http://opensource.org/licenses/bsd-license.php
> +//
> +//  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +//  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +//
> +//
> +
> +.text
> +.align 3
> +
> +GCC_ASM_EXPORT(ArmCallHvc)
> +
> +ASM_PFX(ArmCallHvc):
> +  // Push x0 on the stack
> +  str   x0, [sp, #-8]!
> +
> +  // Load the HVC arguments values into the appropriate registers
> +  ldp   x2, x3, [x0, #16]
> +  ldp   x0, x1, [x0, #0]
> +
> +  hvc   #0
> +
> +  // Pop the ARM_HVC_ARGS structure address from the stack into x9
> +  ldr   x9, [sp], #8
> +
> +  // Store the HVC returned values into the appropriate registers
> +  // A HVC call can return up to 4 values
> +  stp   x2, x3, [x9, #16]
> +  stp   x0, x1, [x9, #0]
> +
> +  ret
> diff --git a/ArmPkg/Library/ArmHvcLib/Arm/ArmHvc.S b/ArmPkg/Library/ArmHvcLib/Arm/ArmHvc.S
> new file mode 100644
> index 000000000000..e48f2ebc2032
> --- /dev/null
> +++ b/ArmPkg/Library/ArmHvcLib/Arm/ArmHvc.S
> @@ -0,0 +1,43 @@
> +//
> +//  Copyright (c) 2012-2014, ARM Limited. All rights reserved.
> +//  Copyright (c) 2014, Linaro Limited. All rights reserved.
> +//
> +//  This program and the accompanying materials
> +//  are licensed and made available under the terms and conditions of the BSD License
> +//  which accompanies this distribution.  The full text of the license may be found at
> +//  http://opensource.org/licenses/bsd-license.php
> +//
> +//  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +//  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +//
> +//
> +
> +.text
> +.align 3
> +.arch_extension virt
> +
> +GCC_ASM_EXPORT(ArmCallHvc)
> +
> +ASM_PFX(ArmCallHvc):
> +    // r0 will be popped just after the HVC call
> +    push    {r0}
> +
> +    // Load the HVC arguments values into the appropriate registers
> +    ldr     r3, [r0, #12]
> +    ldr     r2, [r0, #8]
> +    ldr     r1, [r0, #4]
> +    ldr     r0, [r0, #0]
> +
> +    hvc     #0
> +
> +    // Pop the ARM_HVC_ARGS structure address from the stack into ip
> +    pop     {ip}
> +
> +    // Load the HVC returned values into the appropriate registers
> +    // A HVC call can return up to 4 values
> +    str     r3, [ip, #12]
> +    str     r2, [ip, #8]
> +    str     r1, [ip, #4]
> +    str     r0, [ip, #0]
> +
> +    bx      lr
> diff --git a/ArmPkg/Library/ArmHvcLib/Arm/ArmHvc.asm b/ArmPkg/Library/ArmHvcLib/Arm/ArmHvc.asm
> new file mode 100644
> index 000000000000..1275f590c8aa
> --- /dev/null
> +++ b/ArmPkg/Library/ArmHvcLib/Arm/ArmHvc.asm
> @@ -0,0 +1,43 @@
> +//
> +//  Copyright (c) 2012-2014, ARM Limited. All rights reserved.
> +//  Copyright (c) 2014, Linaro Limited. All rights reserved.
> +//
> +//  This program and the accompanying materials
> +//  are licensed and made available under the terms and conditions of the BSD License
> +//  which accompanies this distribution.  The full text of the license may be found at
> +//  http://opensource.org/licenses/bsd-license.php
> +//
> +//  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +//  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +//
> +//
> +
> +    EXPORT ArmCallHvc
> +
> +    AREA   ArmHvc, CODE, READONLY
> +
> +ArmCallHvc
> +    // r0 will be popped just after the HVC call
> +    pop     {r0}

(2) Shouldn't this be a push then? (Consistently with the GCC source?)

(/me high-fives himself for catching this :))

I guess you didn't test a 32-bit build with RVCT; it would have blown up
pretty quickly I guess.

Please do not repost though, fix it in a followup series (as far as I'm
concerned).

... FILE_GUID below is unique; good. And, this lib will be put to use in
ArmPsciResetSystemLib (the next patch).

Acked-by: Laszlo Ersek 

Thanks,
Laszlo

> +
> +    // Load the HVC arguments values into the appropriate registers
> +    ldr     r3, [r0, #12]
> +    ldr     r2, [r0, #8]
> +    ldr     r1, [r0, #4]
> +    ldr     r0, [r0, #0]
> +
> +    hvc     #0
> +
> +    // Pop the ARM_HVC_ARGS structure address from the stack into ip
> +    pop     {ip}
> +
> +    // Load the HVC returned values into the appropriate registers
> +    // A HVC call can return up to 4 values
> +    str     r3, [ip, #12]
> +    str     r2, [ip, #8]
> +    str     r1, [ip, #4]
> +    str     r0, [ip, #0]
> +
> +    bx      lr
> +
> +    END
> diff --git a/ArmPkg/Library/ArmHvcLib/ArmHvcLib.inf b/ArmPkg/Library/ArmHvcLib/ArmHvcLib.inf
> new file mode 100644
> index 000000000000..92efac5741c8
> --- /dev/null
> +++ b/ArmPkg/Library/ArmHvcLib/ArmHvcLib.inf
> @@ -0,0 +1,32 @@
> +#/** @file
> +#
> +#  Copyright (c) 2012-2013, ARM Ltd. All rights reserved.
> +# Copyright (c) 2014, Linaro Ltd. All rights reserved.
> +# This program and the accompanying materials > +# are licensed and made available under the terms and conditions of the BSD License > +# which accompanies this distribution. The full text of the license may be found at > +# http://opensource.org/licenses/bsd-license.php > +# > +# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, > +# WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. > +# > +#**/ > + > +[Defines] > + INF_VERSION = 0x00010005 > + BASE_NAME = ArmHvcLib > + FILE_GUID = E594959A-D150-44D3-963B-BA90329D3D9A > + MODULE_TYPE = BASE > + VERSION_STRING = 1.0 > + LIBRARY_CLASS = ArmHvcLib > + > +[Sources.ARM] > + Arm/ArmHvc.asm | RVCT > + Arm/ArmHvc.S | GCC > + > +[Sources.AARCH64] > + AArch64/ArmHvc.S > + > +[Packages] > + MdePkg/MdePkg.dec > + ArmPkg/ArmPkg.dec > ------------------------------------------------------------------------------ Slashdot TV. Video for Nerds. Stuff that matters. http://tv.slashdot.org/ _______________________________________________ edk2-devel mailing list edk2-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/edk2-devel