Re: [Xen-devel] [PATCH 1/2] arm: smccc: handle SMCs/HVCs according to SMCCC

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Wed, 14 Jun 2017, Volodymyr Babchuk wrote:
> SMCCC (SMC Call Convention) describes how to handle both HVCs and SMCs.
> SMCCC states that both HVC and SMC are valid conduits to call to a different
> firmware functions. Thus, for example PSCI calls can be made both by
> SMC or HVC. Also SMCCC defines function number coding for such calls.
> Besides functional calls there are query calls, which allows underling
> OS determine version, UID and number of functions provided by service
> provider.
> 
> This patch adds new file `smccc.c`, which handles both generic SMCs
> and HVC according to SMC. At this moment it implements only one
> service: Standard Hypervisor Service.
> 
> Standard Hypervisor Service only supports query calls, so caller can
> ask about hypervisor UID and determine that it is XEN running.
> 
> This change allows more generic handling for SMCs and HVCs and it can
> be easily extended to support new services and functions.
> 
> Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@xxxxxxxx>
> Reviewed-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@xxxxxxxx>
> Reviewed-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx>
> ---
>  xen/arch/arm/Makefile       |  1 +
>  xen/arch/arm/smccc.c        | 96 
> +++++++++++++++++++++++++++++++++++++++++++++
>  xen/arch/arm/traps.c        | 10 ++++-
>  xen/include/asm-arm/smccc.h | 89 +++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 194 insertions(+), 2 deletions(-)
>  create mode 100644 xen/arch/arm/smccc.c
>  create mode 100644 xen/include/asm-arm/smccc.h
> 
> diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
> index 49e1fb2..b8728cf 100644
> --- a/xen/arch/arm/Makefile
> +++ b/xen/arch/arm/Makefile
> @@ -39,6 +39,7 @@ obj-y += psci.o
>  obj-y += setup.o
>  obj-y += shutdown.o
>  obj-y += smc.o
> +obj-y += smccc.o
>  obj-y += smp.o
>  obj-y += smpboot.o
>  obj-y += sysctl.o
> diff --git a/xen/arch/arm/smccc.c b/xen/arch/arm/smccc.c
> new file mode 100644
> index 0000000..5d10964
> --- /dev/null
> +++ b/xen/arch/arm/smccc.c
> @@ -0,0 +1,96 @@
> +/*
> + * xen/arch/arm/smccc.c
> + *
> + * Generic handler for SMC and HVC calls according to
> + * ARM SMC callling convention
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +
> +#include <xen/config.h>
> +#include <xen/lib.h>
> +#include <xen/perfc.h>
> +/* Need to include xen/sched.h before asm/domain.h or it breaks build*/
> +#include <xen/sched.h>
> +#include <xen/stdbool.h>
> +#include <xen/types.h>
> +#include <asm/domain.h>
> +#include <asm/psci.h>
> +#include <asm/smccc.h>
> +#include <asm/regs.h>
> +
> +#define XEN_SMCCC_UID ARM_SMCCC_UID(0xa71812dc, 0xc698, 0x4369, \
> +                                    0x9a, 0xcf, 0x79, 0xd1, \
> +                                    0x8d, 0xde, 0xe6, 0x67)
> +
> +/*
> + * We can't use XEN version here:
> + * Major revision should change every time SMC/HVC function is removed.
> + * Minor revision should change every time SMC/HVC function is added.
> + * So, it is SMCCC protocol revision code, not XEN version
> + */
> +#define XEN_SMCCC_MAJOR_REVISION 0
> +#define XEN_SMCCC_MINOR_REVISION 1
> +#define XEN_SMCCC_FUNCTION_COUNT 3

Both ARM_SMCCC_UID, and XEN_SMCCC_MAJOR/MINOR_REVISION become part of the
Xen public ABI. Please explain in the commit message why you chose them
as indentifier, and add them to a separate new header file under
xen/include/public/arch-arm/ (because they are public).



> +/* SMCCC interface for hypervisor. Tell about self */
> +static bool handle_hypervisor(struct cpu_user_regs *regs, const union hsr 
> hsr)
> +{
> +    switch ( ARM_SMCCC_FUNC_NUM(get_user_reg(regs, 0)) )
> +    {
> +    case ARM_SMCCC_FUNC_CALL_COUNT:
> +        set_user_reg(regs, 0, XEN_SMCCC_FUNCTION_COUNT);
> +        return true;
> +    case ARM_SMCCC_FUNC_CALL_UID:
> +        set_user_reg(regs, 0, XEN_SMCCC_UID.a[0]);
> +        set_user_reg(regs, 1, XEN_SMCCC_UID.a[1]);
> +        set_user_reg(regs, 2, XEN_SMCCC_UID.a[2]);
> +        set_user_reg(regs, 3, XEN_SMCCC_UID.a[3]);
> +        return true;
> +    case ARM_SMCCC_FUNC_CALL_REVISION:
> +        set_user_reg(regs, 0, XEN_SMCCC_MAJOR_REVISION);
> +        set_user_reg(regs, 1, XEN_SMCCC_MINOR_REVISION);
> +        return true;
> +    }
> +    return false;
> +}

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel