Hi Volodymyr,
CC "THE REST" maintainers to get an opinion on the public headers.
On 08/08/17 21:08, 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 `vsmc.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.
But, before SMC is forwarded to standard SMCCC handler, it can be routed
to a domain monitor, if one is installed.
Signed-off-by: Volodymyr Babchuk <volodymyr_babc...@epam.com>
Reviewed-by: Oleksandr Andrushchenko <oleksandr_andrushche...@epam.com>
Reviewed-by: Oleksandr Tyshchenko <oleksandr_tyshche...@epam.com>
---
- Updated description to indicate that this patch affects only SMC call path.
- added "xen_" prefix to definitions in include/public/arch-arm/smc.h
- moved do_trap_smc() into vsmc.c from traps.c
- replaced all tabs with spaces
I would have really appreciated a summary of the discussion we had on
the previous version regarding the bindings. This is a real blocker for
this series and should not be ignored.
---
xen/arch/arm/Makefile | 1 +
xen/arch/arm/traps.c | 19 +-----
xen/arch/arm/vsmc.c | 139 ++++++++++++++++++++++++++++++++++++++
xen/include/asm-arm/vsmc.h | 94 ++++++++++++++++++++++++++
xen/include/public/arch-arm/smc.h | 68 +++++++++++++++++++
5 files changed, 303 insertions(+), 18 deletions(-)
create mode 100644 xen/arch/arm/vsmc.c
create mode 100644 xen/include/asm-arm/vsmc.h
create mode 100644 xen/include/public/arch-arm/smc.h
diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
index 49e1fb2..4efd01c 100644
--- a/xen/arch/arm/Makefile
+++ b/xen/arch/arm/Makefile
@@ -50,6 +50,7 @@ obj-$(CONFIG_HAS_GICV3) += vgic-v3.o
obj-$(CONFIG_HAS_ITS) += vgic-v3-its.o
obj-y += vm_event.o
obj-y += vtimer.o
+obj-y += vsmc.o
obj-y += vpsci.o
obj-y += vuart.o
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index e14e7c0..b119dc0 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -43,7 +43,7 @@
#include <asm/mmio.h>
#include <asm/cpufeature.h>
#include <asm/flushtlb.h>
-#include <asm/monitor.h>
+#include <asm/vsmc.h>
#include "decode.h"
#include "vtimer.h"
@@ -2769,23 +2769,6 @@ static void do_trap_data_abort_guest(struct
cpu_user_regs *regs,
inject_dabt_exception(regs, info.gva, hsr.len);
}
-static void do_trap_smc(struct cpu_user_regs *regs, const union hsr hsr)
-{
- int rc = 0;
-
- if ( !check_conditional_instr(regs, hsr) )
- {
- advance_pc(regs, hsr);
- return;
- }
-
- if ( current->domain->arch.monitor.privileged_call_enabled )
- rc = monitor_smc();
-
- if ( rc != 1 )
- inject_undef_exception(regs, hsr);
-}
-
static void enter_hypervisor_head(struct cpu_user_regs *regs)
{
if ( guest_mode(regs) )
diff --git a/xen/arch/arm/vsmc.c b/xen/arch/arm/vsmc.c
new file mode 100644
index 0000000..5ef6167
--- /dev/null
+++ b/xen/arch/arm/vsmc.c
@@ -0,0 +1,139 @@
+/*
+ * xen/arch/arm/vsmc.c
+ *
+ * Generic handler for SMC and HVC calls according to
+ * ARM SMC calling convention
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * 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/lib.h>
+#include <xen/types.h>
+#include <public/arch-arm/smc.h>
+#include <asm/monitor.h>
+#include <asm/vsmc.h>
+#include <asm/regs.h>
<asm/regs.h> should be before <asm/vsmc.h> to keep the headers ordered
alphabetically.
+
+/* Number of functions currently supported by Hypervisor Service. */
+#define XEN_SMCCC_FUNCTION_COUNT 3
+
+/* SMCCC interface for hypervisor. Tell about itself. */
+static bool handle_hypervisor(struct cpu_user_regs *regs)
+{
+ 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;
+}
+
+/*
+ * vsmc_handle_call() - handle SMC/HVC call according to ARM SMCCC.
+ * returns true if that was valid SMCCC call (even if function number
+ * was unkown).
+ */
+static bool vsmc_handle_call(struct cpu_user_regs *regs)
Something is wrong here. The comment says "Handle SMC/HVC" but the name
of the function is "vsmc".
+{
+ bool handled = false;
+ const union hsr hsr = { .bits = regs->hsr };
+
+ /*
+ * Check immediate value for HVC32, HVC64 and SMC64.
+ * It is not so easy to check immediate value for SMC32,
+ * because it is not stored in HSR.ISS field. To get immediate
+ * value we need to dissasemble instruction at current pc, which
s/dissasemble/disassemble/
+ * is expensive. So we will assume that it is 0x0.
+ */
+ switch ( hsr.ec )
+ {
+ case HSR_EC_HVC32:
+ case HSR_EC_HVC64:
+ case HSR_EC_SMC64:
+ if ( (hsr.iss & 0xFF) != 0)
The immediate is 16 bits. So it should be 0xFFFF. It would also be nice
to have a comment explaining it and probably a define.
+ return false;
+ break;
+ case HSR_EC_SMC32:
+ break;
+ default:
+ return false;
+ }
+
+ /* 64 bit calls are allowed only from 64 bit domains */
Missing full stop.
+ if ( ARM_SMCCC_IS_64(get_user_reg(regs, 0)) &&
+ is_32bit_domain(current->domain) )
+ {
+ set_user_reg(regs, 0, ARM_SMCCC_ERR_UNKNOWN_FUNCTION);
+ return true;
+ }
+
+ switch ( ARM_SMCCC_OWNER_NUM(get_user_reg(regs, 0)) )
+ {
+ case ARM_SMCCC_OWNER_HYPERVISOR:
+ handled = handle_hypervisor(regs);
+ break;
+ }
+
+ if ( !handled )
+ {
+ gprintk(XENLOG_INFO, "Unhandled SMC/HVC: %08"PRIregister"\n",
+ get_user_reg(regs, 0));
+ /* Inform caller that function is not supported */
+ set_user_reg(regs, 0, ARM_SMCCC_ERR_UNKNOWN_FUNCTION);
+ }
+
+ return true;
+}
+
+/* This function will be called from traps.c */
+void do_trap_smc(struct cpu_user_regs *regs, const union hsr hsr)
+{
+ int rc = 0;
+
+ if ( !check_conditional_instr(regs, hsr) )
+ {
+ advance_pc(regs, hsr);
+ return;
+ }
+
+ /* If monitor is enabled, let it handle the call */
+ if ( current->domain->arch.monitor.privileged_call_enabled )
+ rc = monitor_smc();
+
+ if ( rc == 1 )
+ return;
+
+ /* Use standard routines to handle the call */
+ if ( vsmc_handle_call(regs) )
+ advance_pc(regs, hsr);
+ else
+ inject_undef_exception(regs, hsr);
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/include/asm-arm/vsmc.h b/xen/include/asm-arm/vsmc.h
new file mode 100644
index 0000000..7baabef
--- /dev/null
+++ b/xen/include/asm-arm/vsmc.h
@@ -0,0 +1,94 @@
+/*
+ * Copyright (c) 2017, EPAM Systems
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * 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.
+ *
+ */
+#ifndef __ASM_ARM_VSMC_H__
+#define __ASM_ARM_VSMC_H__
+
+#include <xen/types.h>
+
+/*
+ * This file provides common defines for ARM SMC Calling Convention as
+ * specified in
+ * http://infocenter.arm.com/help/topic/com.arm.doc.den0028a/index.html
+ */
+
+#define ARM_SMCCC_STD_CALL 0U
+#define ARM_SMCCC_FAST_CALL 1U
+#define ARM_SMCCC_TYPE_SHIFT 31
+
+#define ARM_SMCCC_SMC_32 0U
+#define ARM_SMCCC_SMC_64 1U
+#define ARM_SMCCC_CALL_CONV_SHIFT 30
+
+#define ARM_SMCCC_OWNER_MASK 0x3F
+#define ARM_SMCCC_OWNER_SHIFT 24
+
+#define ARM_SMCCC_FUNC_MASK 0xFFFF
+
+/* Check if this is fast call */
+#define ARM_SMCCC_IS_FAST_CALL(smc_val) \
+ ((smc_val) & (ARM_SMCCC_FAST_CALL << ARM_SMCCC_TYPE_SHIFT))
+
+/* Check if this is 64 bit call */
+#define ARM_SMCCC_IS_64(smc_val) \
+ ((smc_val) & (ARM_SMCCC_SMC_64 << ARM_SMCCC_CALL_CONV_SHIFT))
+
+/* Get function number from function identifier */
+#define ARM_SMCCC_FUNC_NUM(smc_val) ((smc_val) & ARM_SMCCC_FUNC_MASK)
+
+/* Get service owner number from function identifier */
+#define ARM_SMCCC_OWNER_NUM(smc_val) \
+ (((smc_val) >> ARM_SMCCC_OWNER_SHIFT) & ARM_SMCCC_OWNER_MASK)
+
+/*
+ * Construct function identifier from call type (fast or standard),
+ * calling convention (32 or 64 bit), service owner and function number
+ */
+#define ARM_SMCCC_CALL_VAL(type, calling_convention, owner, func_num) \
+ (((type) << ARM_SMCCC_TYPE_SHIFT) | \
+ ((calling_convention) << ARM_SMCCC_CALL_CONV_SHIFT) | \
+ (((owner) & ARM_SMCCC_OWNER_MASK) << ARM_SMCCC_OWNER_SHIFT) | \
+ ((func_num) & ARM_SMCCC_FUNC_MASK))
+
+/* List of known service owners */
+#define ARM_SMCCC_OWNER_ARCH 0
+#define ARM_SMCCC_OWNER_CPU 1
+#define ARM_SMCCC_OWNER_SIP 2
+#define ARM_SMCCC_OWNER_OEM 3
+#define ARM_SMCCC_OWNER_STANDARD 4
+#define ARM_SMCCC_OWNER_HYPERVISOR 5
+#define ARM_SMCCC_OWNER_TRUSTED_APP 48
+#define ARM_SMCCC_OWNER_TRUSTED_APP_END 49
+#define ARM_SMCCC_OWNER_TRUSTED_OS 50
+#define ARM_SMCCC_OWNER_TRUSTED_OS_END 63
+
+/* List of generic function numbers */
+#define ARM_SMCCC_FUNC_CALL_COUNT 0xFF00
+#define ARM_SMCCC_FUNC_CALL_UID 0xFF01
+#define ARM_SMCCC_FUNC_CALL_REVISION 0xFF03
+
+/* Only one error code defined in SMCCC */
+#define ARM_SMCCC_ERR_UNKNOWN_FUNCTION (-1)
+
+void do_trap_smc(struct cpu_user_regs *regs, const union hsr hsr);
+
+#endif /* __ASM_ARM_VSMC_H__ */
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:b
+ */
diff --git a/xen/include/public/arch-arm/smc.h
b/xen/include/public/arch-arm/smc.h
new file mode 100644
index 0000000..42f3165
--- /dev/null
+++ b/xen/include/public/arch-arm/smc.h
@@ -0,0 +1,68 @@
+/*
+ * smc.h
+ *
+ * SMC/HVC interface in accordance with SMC Calling Convention.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to
+ * deal in the Software without restriction, including without limitation the
+ * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or
+ * sell copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ *
+ * Copyright 2017 (C) EPAM Systems
+ */
+
+#ifndef __XEN_PUBLIC_ARCH_ARM_SMC_H__
+#define __XEN_PUBLIC_ARCH_ARM_SMC_H__
+
+typedef struct {
+ uint32_t a[4];
+} xen_arm_smccc_uid;
+
+#define XEN_ARM_SMCCC_UID(a, b, c, d0, d1, d2, d3, d4, d5, d6, d7) \
+ ((xen_arm_smccc_uid) {{(a), ((b) << 16 | (c) ), \
+ ((d0) << 24 | (d1) << 16 | (d2) << 8 | (d3) << 0), \
+ ((d4) << 24 | (d5) << 16 | (d6) << 8 | (d7) << 0)}})
+
+/*
+ * Hypervisor Service version.
+ *
+ * We can't use XEN version here, because of SMCCC requirements:
+ * 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.
+ *
+ * Those values are subjected to change, when interface will be extended.
+ * They should not be stored in public/asm-arm/smc.h because they should
+ * be queried by guest using SMC/HVC interface.
+ */
+#define XEN_SMCCC_MAJOR_REVISION 0
+#define XEN_SMCCC_MINOR_REVISION 1
+
+/* Hypervisor Service UID. Randomly generated with 3rd party tool. */
+#define XEN_SMCCC_UID XEN_ARM_SMCCC_UID(0xa71812dc, 0xc698, 0x4369, \
+ 0x9a, 0xcf, 0x79, 0xd1, \
+ 0x8d, 0xde, 0xe6, 0x67)
+
+#endif /* __XEN_PUBLIC_ARCH_ARM_SMC_H__ */
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:b
+ */
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
