All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v7 01/11] arm: traps: use only least 32 bits of fid in PSCI handler
From: Volodymyr Babchuk @ 2017-10-04 21:00 UTC (permalink / raw)
  To: xen-devel
  Cc: Edgar E . Iglesias, Julien Grall, Stefano Stabellini,
	Volodymyr Babchuk
In-Reply-To: <1507150827-7858-1-git-send-email-volodymyr_babchuk@epam.com>

According to SMCCC (ARM DEN 0028B, page 12), function id is
stored in least 32 bits of r0/x0 register:

    The least significant 32-bits are used, and the most significant
    32-bits are zero. Implementations must ignore the least significant
    bits.

Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
---
 xen/arch/arm/traps.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 701fdc8..0cff83e 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -1463,14 +1463,14 @@ static void do_debug_trap(struct cpu_user_regs *regs, unsigned int code)
 #endif
 
 /* helper function for checking arm mode 32/64 bit */
-static inline int psci_mode_check(struct domain *d, register_t fid)
+static inline int psci_mode_check(struct domain *d, uint32_t fid)
 {
         return !( is_64bit_domain(d)^( (fid & PSCI_0_2_64BIT) >> 30 ) );
 }
 
 static void do_trap_psci(struct cpu_user_regs *regs)
 {
-    register_t fid = PSCI_ARG(regs,0);
+    uint32_t fid = PSCI_ARG32(regs,0);
 
     /* preloading in case psci_mode_check fails */
     PSCI_RESULT_REG(regs) = PSCI_INVALID_PARAMETERS;
-- 
2.7.4


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

^ permalink raw reply related

* [PATCH v7 02/11] arm: traps: use generic register accessors in the PSCI code
From: Volodymyr Babchuk @ 2017-10-04 21:00 UTC (permalink / raw)
  To: xen-devel
  Cc: Edgar E . Iglesias, Julien Grall, Stefano Stabellini,
	Volodymyr Babchuk
In-Reply-To: <1507150827-7858-1-git-send-email-volodymyr_babchuk@epam.com>

There are standard functions set_user_reg() and get_user_reg(). We can
use them in PSCI_SET_RESULT()/PSCI_ARG() macros instead of relying on
CONFIG_ARM_64 definition.

Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
Reviewed-by: Julien Grall <julien.grall@arm.com>
---
 xen/arch/arm/traps.c | 38 +++++++++++++++++---------------------
 1 file changed, 17 insertions(+), 21 deletions(-)

diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 0cff83e..ace5a43 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -1452,13 +1452,12 @@ static void do_debug_trap(struct cpu_user_regs *regs, unsigned int code)
 }
 #endif
 
+#define PSCI_SET_RESULT(reg, val) set_user_reg(reg, 0, val)
+#define PSCI_ARG(reg,n) get_user_reg(reg, n)
+
 #ifdef CONFIG_ARM_64
-#define PSCI_RESULT_REG(reg) (reg)->x0
-#define PSCI_ARG(reg,n) (reg)->x##n
-#define PSCI_ARG32(reg,n) (uint32_t)( (reg)->x##n & 0x00000000FFFFFFFF )
+#define PSCI_ARG32(reg,n) (uint32_t)get_user_reg(reg,n)
 #else
-#define PSCI_RESULT_REG(reg) (reg)->r0
-#define PSCI_ARG(reg,n) (reg)->r##n
 #define PSCI_ARG32(reg,n) PSCI_ARG(reg,n)
 #endif
 
@@ -1473,14 +1472,14 @@ static void do_trap_psci(struct cpu_user_regs *regs)
     uint32_t fid = PSCI_ARG32(regs,0);
 
     /* preloading in case psci_mode_check fails */
-    PSCI_RESULT_REG(regs) = PSCI_INVALID_PARAMETERS;
+    PSCI_SET_RESULT(regs, PSCI_INVALID_PARAMETERS);
     switch( fid )
     {
     case PSCI_cpu_off:
         {
             uint32_t pstate = PSCI_ARG32(regs,1);
             perfc_incr(vpsci_cpu_off);
-            PSCI_RESULT_REG(regs) = do_psci_cpu_off(pstate);
+            PSCI_SET_RESULT(regs, do_psci_cpu_off(pstate));
         }
         break;
     case PSCI_cpu_on:
@@ -1488,36 +1487,36 @@ static void do_trap_psci(struct cpu_user_regs *regs)
             uint32_t vcpuid = PSCI_ARG32(regs,1);
             register_t epoint = PSCI_ARG(regs,2);
             perfc_incr(vpsci_cpu_on);
-            PSCI_RESULT_REG(regs) = do_psci_cpu_on(vcpuid, epoint);
+            PSCI_SET_RESULT(regs, do_psci_cpu_on(vcpuid, epoint));
         }
         break;
     case PSCI_0_2_FN_PSCI_VERSION:
         perfc_incr(vpsci_version);
-        PSCI_RESULT_REG(regs) = do_psci_0_2_version();
+        PSCI_SET_RESULT(regs, do_psci_0_2_version());
         break;
     case PSCI_0_2_FN_CPU_OFF:
         perfc_incr(vpsci_cpu_off);
-        PSCI_RESULT_REG(regs) = do_psci_0_2_cpu_off();
+        PSCI_SET_RESULT(regs, do_psci_0_2_cpu_off());
         break;
     case PSCI_0_2_FN_MIGRATE_INFO_TYPE:
         perfc_incr(vpsci_migrate_info_type);
-        PSCI_RESULT_REG(regs) = do_psci_0_2_migrate_info_type();
+        PSCI_SET_RESULT(regs, do_psci_0_2_migrate_info_type());
         break;
     case PSCI_0_2_FN_MIGRATE_INFO_UP_CPU:
     case PSCI_0_2_FN64_MIGRATE_INFO_UP_CPU:
         perfc_incr(vpsci_migrate_info_up_cpu);
         if ( psci_mode_check(current->domain, fid) )
-            PSCI_RESULT_REG(regs) = do_psci_0_2_migrate_info_up_cpu();
+            PSCI_SET_RESULT(regs, do_psci_0_2_migrate_info_up_cpu());
         break;
     case PSCI_0_2_FN_SYSTEM_OFF:
         perfc_incr(vpsci_system_off);
         do_psci_0_2_system_off();
-        PSCI_RESULT_REG(regs) = PSCI_INTERNAL_FAILURE;
+        PSCI_SET_RESULT(regs, PSCI_INTERNAL_FAILURE);
         break;
     case PSCI_0_2_FN_SYSTEM_RESET:
         perfc_incr(vpsci_system_reset);
         do_psci_0_2_system_reset();
-        PSCI_RESULT_REG(regs) = PSCI_INTERNAL_FAILURE;
+        PSCI_SET_RESULT(regs, PSCI_INTERNAL_FAILURE);
         break;
     case PSCI_0_2_FN_CPU_ON:
     case PSCI_0_2_FN64_CPU_ON:
@@ -1527,8 +1526,7 @@ static void do_trap_psci(struct cpu_user_regs *regs)
             register_t vcpuid = PSCI_ARG(regs,1);
             register_t epoint = PSCI_ARG(regs,2);
             register_t cid = PSCI_ARG(regs,3);
-            PSCI_RESULT_REG(regs) =
-                do_psci_0_2_cpu_on(vcpuid, epoint, cid);
+            PSCI_SET_RESULT(regs, do_psci_0_2_cpu_on(vcpuid, epoint, cid));
         }
         break;
     case PSCI_0_2_FN_CPU_SUSPEND:
@@ -1539,8 +1537,7 @@ static void do_trap_psci(struct cpu_user_regs *regs)
             uint32_t pstate = PSCI_ARG32(regs,1);
             register_t epoint = PSCI_ARG(regs,2);
             register_t cid = PSCI_ARG(regs,3);
-            PSCI_RESULT_REG(regs) =
-                do_psci_0_2_cpu_suspend(pstate, epoint, cid);
+            PSCI_SET_RESULT(regs, do_psci_0_2_cpu_suspend(pstate, epoint, cid));
         }
         break;
     case PSCI_0_2_FN_AFFINITY_INFO:
@@ -1550,8 +1547,7 @@ static void do_trap_psci(struct cpu_user_regs *regs)
         {
             register_t taff = PSCI_ARG(regs,1);
             uint32_t laff = PSCI_ARG32(regs,2);
-            PSCI_RESULT_REG(regs) =
-                do_psci_0_2_affinity_info(taff, laff);
+            PSCI_SET_RESULT(regs, do_psci_0_2_affinity_info(taff, laff));
         }
         break;
     case PSCI_0_2_FN_MIGRATE:
@@ -1560,7 +1556,7 @@ static void do_trap_psci(struct cpu_user_regs *regs)
         if ( psci_mode_check(current->domain, fid) )
         {
             uint32_t tcpu = PSCI_ARG32(regs,1);
-            PSCI_RESULT_REG(regs) = do_psci_0_2_migrate(tcpu);
+            PSCI_SET_RESULT(regs, do_psci_0_2_migrate(tcpu));
         }
         break;
     default:
-- 
2.7.4


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

^ permalink raw reply related

* [PATCH v7 03/11] arm: traps: check if SMC was conditional before handling it
From: Volodymyr Babchuk @ 2017-10-04 21:00 UTC (permalink / raw)
  To: xen-devel
  Cc: Edgar E . Iglesias, Julien Grall, Stefano Stabellini,
	Volodymyr Babchuk
In-Reply-To: <1507150827-7858-1-git-send-email-volodymyr_babchuk@epam.com>

Trapped SMC instruction can fail condition check on ARMv8 architecture
(ARM DDI 0487B.a page D7-2271). So we need to check if condition was meet.

Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
Reviewed-by: Julien Grall <julien.grall@arm.com>
---
 xen/arch/arm/traps.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index ace5a43..5b91e6c 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -2199,6 +2199,12 @@ 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();
 
-- 
2.7.4


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

^ permalink raw reply related

* [PATCH v7 04/11] public: xen.h: add definitions for UUID handling
From: Volodymyr Babchuk @ 2017-10-04 21:00 UTC (permalink / raw)
  To: xen-devel
  Cc: Edgar E . Iglesias, Stefano Stabellini, Wei Liu,
	Konrad Rzeszutek Wilk, George Dunlap, Andrew Cooper, Ian Jackson,
	Tim Deegan, Julien Grall, Jan Beulich, Volodymyr Babchuk
In-Reply-To: <1507150827-7858-1-git-send-email-volodymyr_babchuk@epam.com>

Added type xen_uuid_t. This type represents UUID as an array of 16
bytes in big endian format.

Added macro XEN_DEFINE_UUID that constructs UUID in the usual way:

 XEN_DEFINE_UUID(0x00112233, 0x4455, 0x6677, 0x8899,
		0xaa, 0xbb, 0xcc, 0xdd, 0xee, 0xff)

will construct UUID 00112233-4455-6677-8899-aabbccddeeff presented as
 {0x00, 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88,
  0x99, 0xaa, 0xbb, 0xcc, 0xdd, 0xee, 0xff}

NB: This is compatible with Linux kernel and with libuuid, but it is not
compatible with Microsoft, as they use mixed-endian encoding (some
components are little-endian, some are big-endian).

Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
---

* Fixed example for XEN_DEFINE_UUID() usage. Was
  XEN_DEFINE_UUID(0x00112233, 0x4455, 0x6677, 0x8899, 0xaabbccddeeff)

* Added comment to xen.h

* Used
  #if defined (__STDC_VERSION__) && __STDC_VERSION__ >= 199901L
  instead of
  #if defined(__GNUC__) && !defined(__STRICT_ANSI__)

* Used generic macro XEN_DEFINE_UUID_

---
xen/include/public/xen.h | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/xen/include/public/xen.h b/xen/include/public/xen.h
index 2ac6b1e..1a6255f 100644
--- a/xen/include/public/xen.h
+++ b/xen/include/public/xen.h
@@ -930,6 +930,39 @@ __DEFINE_XEN_GUEST_HANDLE(uint16, uint16_t);
 __DEFINE_XEN_GUEST_HANDLE(uint32, uint32_t);
 __DEFINE_XEN_GUEST_HANDLE(uint64, uint64_t);
 
+typedef struct
+{
+    uint8_t a[16];
+} xen_uuid_t;
+
+/*
+ * XEN_DEFINE_UUID(0x00112233, 0x4455, 0x6677, 0x8899,
+ *                 0xaa, 0xbb, 0xcc, 0xdd, 0xee, 0xff)
+ * will construct UUID 00112233-4455-6677-8899-aabbccddeeff presented as
+ * {0x00, 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88,
+ * 0x99, 0xaa, 0xbb, 0xcc, 0xdd, 0xee, 0xff};
+ *
+ * NB: This is compatible with Linux kernel and with libuuid, but it is not
+ * compatible with Microsoft, as they use mixed-endian encoding (some
+ * components are little-endian, some are big-endian).
+ */
+#define XEN_DEFINE_UUID_(a, b, c, d, e1, e2, e3, e4, e5, e6)            \
+    {{((a) >> 24) & 0xFF, ((a) >> 16) & 0xFF,                           \
+      ((a) >>  8) & 0xFF, ((a) >>  0) & 0xFF,                           \
+      ((b) >>  8) & 0xFF, ((b) >>  0) & 0xFF,                           \
+      ((c) >>  8) & 0xFF, ((c) >>  0) & 0xFF,                           \
+      ((d) >>  8) & 0xFF, ((d) >>  0) & 0xFF,                           \
+                e1, e2, e3, e4, e5, e6}}
+
+/* Compound literals are supported in C99 and later. */
+#if defined (__STDC_VERSION__) && __STDC_VERSION__ >= 199901L
+#define XEN_DEFINE_UUID(a, b, c, d, e1, e2, e3, e4, e5, e6)             \
+    (xen_uuid_t)XEN_DEFINE_UUID_(a, b, c, d, e1, e2, e3, e4, e5, e6)
+#else
+#define XEN_DEFINE_UUID(a, b, c, d, e1, e2, e3, e4, e5, e6)             \
+    XEN_DEFINE_UUID_(a, b, c, d, e1, e2, e3, e4, e5, e6)
+#endif /* defined (__STDC_VERSION__) && __STDC_VERSION__ >= 199901L */
+
 #endif /* !__ASSEMBLY__ */
 
 /* Default definitions for macros used by domctl/sysctl. */
-- 
2.7.4


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

^ permalink raw reply related

* [PATCH v7 05/11] arm: processor.h: add definition for immediate value mask
From: Volodymyr Babchuk @ 2017-10-04 21:00 UTC (permalink / raw)
  To: xen-devel
  Cc: Edgar E . Iglesias, Julien Grall, Stefano Stabellini,
	Volodymyr Babchuk
In-Reply-To: <1507150827-7858-1-git-send-email-volodymyr_babchuk@epam.com>

This patch defines HSR_XXC_IMM_MASK. It can be used to extract
immediate value for trapped HVC32, HVC64, SMC64, SVC32, SVC64
instructions, as described in the ARM ARM
(ARM DDI 0487B.a pages D7-2270, D7-2272).

Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
Acked-by: Julien Grall <julien.grall@arm.com>
---
 xen/include/asm-arm/processor.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/xen/include/asm-arm/processor.h b/xen/include/asm-arm/processor.h
index cd92176..65eb107 100644
--- a/xen/include/asm-arm/processor.h
+++ b/xen/include/asm-arm/processor.h
@@ -673,6 +673,9 @@ union hsr {
                               HSR_SYSREG_CRN_MASK|HSR_SYSREG_CRM_MASK|\
                               HSR_SYSREG_OP2_MASK)
 
+/* HSR.EC == HSR_{HVC32, HVC64, SMC64, SVC32, SVC64} */
+#define HSR_XXC_IMM_MASK     (0xffff)
+
 /* Physical Address Register */
 #define PAR_F           (_AC(1,U)<<0)
 
-- 
2.7.4


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

^ permalink raw reply related

* [PATCH v7 06/11] arm: add SMCCC protocol definitions
From: Volodymyr Babchuk @ 2017-10-04 21:00 UTC (permalink / raw)
  To: xen-devel
  Cc: Edgar E . Iglesias, Stefano Stabellini, Wei Liu,
	Konrad Rzeszutek Wilk, George Dunlap, Andrew Cooper, Ian Jackson,
	Tim Deegan, Julien Grall, Volodymyr Babchuk
In-Reply-To: <1507150827-7858-1-git-send-email-volodymyr_babchuk@epam.com>

Add generic definitions used in ARM SMC call convention.
Those definitions was originally added to Linux kernel as
include/linux/arm-smccc.h by commit 98dd64f34f47
("ARM: 8478/2: arm/arm64: add arm-smccc")

I extended them and formatted according to XEN coding style. Some
of the macros were converted to inlined functions to ease parsing.

They can be used by both SMCCC clients (like PSCI) and by SMCCC
servers (like vPSCI or upcoming generic SMCCC handler).

Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
Acked-by: Julien Grall <julien.grall@arm.com>
---
 xen/include/asm-arm/smccc.h | 105 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 105 insertions(+)
 create mode 100644 xen/include/asm-arm/smccc.h

diff --git a/xen/include/asm-arm/smccc.h b/xen/include/asm-arm/smccc.h
new file mode 100644
index 0000000..f543dea
--- /dev/null
+++ b/xen/include/asm-arm/smccc.h
@@ -0,0 +1,105 @@
+/*
+ * Copyright (c) 2015, Linaro Limited
+ * 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_SMCCC_H__
+#define __ASM_ARM_SMCCC_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_CONV_32               0U
+#define ARM_SMCCC_CONV_64               1U
+#define ARM_SMCCC_CONV_SHIFT            30
+
+#define ARM_SMCCC_OWNER_MASK            0x3FU
+#define ARM_SMCCC_OWNER_SHIFT           24
+
+#define ARM_SMCCC_FUNC_MASK             0xFFFFU
+
+/* Check if this is fast call. */
+static inline bool smccc_is_fast_call(register_t funcid)
+{
+    return funcid & (ARM_SMCCC_FAST_CALL << ARM_SMCCC_TYPE_SHIFT);
+}
+
+/* Chek if this is 64-bit call. */
+static inline bool smccc_is_conv_64(register_t funcid)
+{
+    return funcid & (ARM_SMCCC_CONV_64 << ARM_SMCCC_CONV_SHIFT);
+}
+
+/* Get function number from function identifier. */
+static inline uint32_t smccc_get_fn(register_t funcid)
+{
+    return funcid & ARM_SMCCC_FUNC_MASK;
+}
+
+/* Get service owner number from function identifier. */
+static inline uint32_t smccc_get_owner(register_t funcid)
+{
+    return (funcid >> 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_CONV_SHIFT) |                       \
+         (((owner) & ARM_SMCCC_OWNER_MASK) << ARM_SMCCC_OWNER_SHIFT) |          \
+         (func_num))
+
+/* 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)
+
+/* SMCCC function identifier range which is reserved for existing APIs */
+#define ARM_SMCCC_RESERVED_RANGE_START  0x0
+#define ARM_SMCCC_RESERVED_RANGE_END    0x0100FFFF
+
+#endif  /* __ASM_ARM_SMCCC_H__ */
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:b
+ */
-- 
2.7.4


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

^ permalink raw reply related

* [PATCH v7 07/11] arm: smccc: handle SMCs according to SMCCC
From: Volodymyr Babchuk @ 2017-10-04 21:00 UTC (permalink / raw)
  To: xen-devel
  Cc: Edgar E . Iglesias, Stefano Stabellini, Wei Liu,
	Konrad Rzeszutek Wilk, George Dunlap, Andrew Cooper, Ian Jackson,
	Tim Deegan, Julien Grall, Volodymyr Babchuk
In-Reply-To: <1507150827-7858-1-git-send-email-volodymyr_babchuk@epam.com>

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 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, UUID and number of functions provided by service
provider.

This patch adds new file `vsmc.c`, which handles both generic SMCs
and HVC according to SMCCC. At this moment it implements only one
service: Standard Hypervisor Service.

At this time 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_babchuk@epam.com>
Reviewed-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
Reviewed-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
Acked-by: Julien Grall <julien.grall@arm.com>
---
 xen/arch/arm/Makefile               |   1 +
 xen/arch/arm/traps.c                |  17 ----
 xen/arch/arm/vsmc.c                 | 191 ++++++++++++++++++++++++++++++++++++
 xen/include/asm-arm/traps.h         |   3 +
 xen/include/public/arch-arm/smccc.h |  58 +++++++++++
 5 files changed, 253 insertions(+), 17 deletions(-)
 create mode 100644 xen/arch/arm/vsmc.c
 create mode 100644 xen/include/public/arch-arm/smccc.h

diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
index 424580b..30a2a65 100644
--- a/xen/arch/arm/Makefile
+++ b/xen/arch/arm/Makefile
@@ -53,6 +53,7 @@ obj-$(CONFIG_HAS_ITS) += vgic-v3-its.o
 obj-y += vm_event.o
 obj-y += vtimer.o
 obj-$(CONFIG_SBSA_VUART_CONSOLE) += vpl011.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 5b91e6c..6ea0090 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -2195,23 +2195,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..38df821
--- /dev/null
+++ b/xen/arch/arm/vsmc.c
@@ -0,0 +1,191 @@
+/*
+ * 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/smccc.h>
+#include <asm/monitor.h>
+#include <asm/regs.h>
+#include <asm/smccc.h>
+#include <asm/traps.h>
+
+/* Number of functions currently supported by Hypervisor Service. */
+#define XEN_SMCCC_FUNCTION_COUNT 3
+
+static bool fill_uid(struct cpu_user_regs *regs, xen_uuid_t uuid)
+{
+    int n;
+
+    /*
+     * UID is returned in registers r0..r3, four bytes per register,
+     * first byte is stored in low-order bits of a register.
+     * (ARM DEN 0028B page 14)
+     */
+    for (n = 0; n < 4; n++)
+    {
+        const uint8_t *bytes = uuid.a + n * 4;
+        uint32_t r;
+
+        r = bytes[0];
+        r |= bytes[1] << 8;
+        r |= bytes[2] << 16;
+        r |= bytes[3] << 24;
+
+        set_user_reg(regs, n, r);
+    }
+
+    return true;
+}
+
+static bool fill_revision(struct cpu_user_regs *regs, uint32_t major,
+                         uint32_t minor)
+{
+    /*
+     * Revision is returned in registers r0 and r1.
+     * r0 stores major part of the version
+     * r1 stores minor part of the version
+     * (ARM DEN 0028B page 15)
+     */
+    set_user_reg(regs, 0, major);
+    set_user_reg(regs, 1, minor);
+
+    return true;
+}
+
+static bool fill_function_call_count(struct cpu_user_regs *regs, uint32_t cnt)
+{
+    /*
+     * Function call count is retuned as any other return value in register r0
+     * (ARM DEN 0028B page 17)
+     */
+    set_user_reg(regs, 0, cnt);
+
+    return true;
+}
+
+/* SMCCC interface for hypervisor. Tell about itself. */
+static bool handle_hypervisor(struct cpu_user_regs *regs)
+{
+    switch ( smccc_get_fn(get_user_reg(regs, 0)) )
+    {
+    case ARM_SMCCC_FUNC_CALL_COUNT:
+        return fill_function_call_count(regs, XEN_SMCCC_FUNCTION_COUNT);
+    case ARM_SMCCC_FUNC_CALL_UID:
+        return fill_uid(regs, XEN_SMCCC_UID);
+    case ARM_SMCCC_FUNC_CALL_REVISION:
+        return fill_revision(regs, XEN_SMCCC_MAJOR_REVISION,
+                             XEN_SMCCC_MINOR_REVISION);
+    default:
+        return false;
+    }
+}
+
+/*
+ * vsmccc_handle_call() - handle SMC/HVC call according to ARM SMCCC.
+ * returns true if that was valid SMCCC call (even if function number
+ * was unknown).
+ */
+static bool vsmccc_handle_call(struct cpu_user_regs *regs)
+{
+    bool handled = false;
+    const union hsr hsr = { .bits = regs->hsr };
+    register_t funcid = get_user_reg(regs, 0);
+
+    /*
+     * 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 disassemble instruction at current pc, which
+     * 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 & HSR_XXC_IMM_MASK) != 0)
+            return false;
+        break;
+    case HSR_EC_SMC32:
+        break;
+    default:
+        return false;
+    }
+
+    /* 64 bit calls are allowed only from 64 bit domains. */
+    if ( smccc_is_conv_64(funcid) && is_32bit_domain(current->domain) )
+    {
+        set_user_reg(regs, 0, ARM_SMCCC_ERR_UNKNOWN_FUNCTION);
+        return true;
+    }
+
+    switch ( smccc_get_owner(funcid) )
+    {
+    case ARM_SMCCC_OWNER_HYPERVISOR:
+        handled = handle_hypervisor(regs);
+        break;
+    }
+
+    if ( !handled )
+    {
+        gprintk(XENLOG_INFO, "Unhandled SMC/HVC: %08"PRIregister"\n", funcid);
+
+        /* Inform caller that function is not supported. */
+        set_user_reg(regs, 0, ARM_SMCCC_ERR_UNKNOWN_FUNCTION);
+    }
+
+    return true;
+}
+
+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.
+     * vsmccc_handle_call() will return false if this call is not
+     * SMCCC compatible (e.g. immediate value != 0). As it is not
+     * compatible, we can't be sure that guest will understand
+     * ARM_SMCCC_ERR_UNKNOWN_FUNCTION.
+     */
+    if ( vsmccc_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/traps.h b/xen/include/asm-arm/traps.h
index 7508af8..325c15f 100644
--- a/xen/include/asm-arm/traps.h
+++ b/xen/include/asm-arm/traps.h
@@ -35,6 +35,9 @@ void do_cp14_64(struct cpu_user_regs *regs, const union hsr hsr);
 void do_cp14_dbg(struct cpu_user_regs *regs, const union hsr hsr);
 void do_cp(struct cpu_user_regs *regs, const union hsr hsr);
 
+/* SMCCC handling */
+void do_trap_smc(struct cpu_user_regs *regs, const union hsr hsr);
+
 #endif /* __ASM_ARM_TRAPS__ */
 /*
  * Local variables:
diff --git a/xen/include/public/arch-arm/smccc.h b/xen/include/public/arch-arm/smccc.h
new file mode 100644
index 0000000..2bee5b3
--- /dev/null
+++ b/xen/include/public/arch-arm/smccc.h
@@ -0,0 +1,58 @@
+/*
+ * smccc.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_SMCCC_H__
+#define __XEN_PUBLIC_ARCH_ARM_SMCCC_H__
+
+#include "public/xen.h"
+
+/*
+ * 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.
+ */
+#define XEN_SMCCC_MAJOR_REVISION 0
+#define XEN_SMCCC_MINOR_REVISION 1
+
+/* Hypervisor Service UID. Randomly generated with uuidgen. */
+#define XEN_SMCCC_UID XEN_DEFINE_UUID(0xa71812dc, 0xc698, 0x4369, 0x9acf, \
+                                      0x79, 0xd1, 0x8d, 0xde, 0xe6, 0x67)
+
+#endif /* __XEN_PUBLIC_ARCH_ARM_SMCCC_H__ */
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:b
+ */
-- 
2.7.4


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

^ permalink raw reply related

* [PATCH v7 08/11] arm: traps: handle PSCI calls inside `vsmc.c`
From: Volodymyr Babchuk @ 2017-10-04 21:00 UTC (permalink / raw)
  To: xen-devel
  Cc: Edgar E . Iglesias, Julien Grall, Stefano Stabellini,
	Volodymyr Babchuk
In-Reply-To: <1507150827-7858-1-git-send-email-volodymyr_babchuk@epam.com>

PSCI is part of HVC/SMC interface, so it should be handled in
appropriate place: `vsmc.c`. This patch moves PSCI handler
calls from `traps.c` to `vsmc.c`. Also it corrects coding
style of the PSCI handler functions.

Older PSCI 0.1 uses SMC function identifiers in range that is
reserved for existing APIs (ARM DEN 0028B, page 16), while newer
PSCI 0.2 and later is defined as "standard secure service" with its
own ranges (ARM DEN 0028B, page 18).

Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
Reviewed-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
Reviewed-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
---

 * Used least 32 bits for fid
 * Removed extra return true

---
xen/arch/arm/traps.c                | 117 +----------------------
 xen/arch/arm/vsmc.c                 | 183 +++++++++++++++++++++++++++++++++++-
 xen/include/asm-arm/traps.h         |   1 +
 xen/include/public/arch-arm/smccc.h |   8 ++
 4 files changed, 190 insertions(+), 119 deletions(-)

diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 6ea0090..f6f6de3 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -1452,119 +1452,6 @@ static void do_debug_trap(struct cpu_user_regs *regs, unsigned int code)
 }
 #endif
 
-#define PSCI_SET_RESULT(reg, val) set_user_reg(reg, 0, val)
-#define PSCI_ARG(reg,n) get_user_reg(reg, n)
-
-#ifdef CONFIG_ARM_64
-#define PSCI_ARG32(reg,n) (uint32_t)get_user_reg(reg,n)
-#else
-#define PSCI_ARG32(reg,n) PSCI_ARG(reg,n)
-#endif
-
-/* helper function for checking arm mode 32/64 bit */
-static inline int psci_mode_check(struct domain *d, uint32_t fid)
-{
-        return !( is_64bit_domain(d)^( (fid & PSCI_0_2_64BIT) >> 30 ) );
-}
-
-static void do_trap_psci(struct cpu_user_regs *regs)
-{
-    uint32_t fid = PSCI_ARG32(regs,0);
-
-    /* preloading in case psci_mode_check fails */
-    PSCI_SET_RESULT(regs, PSCI_INVALID_PARAMETERS);
-    switch( fid )
-    {
-    case PSCI_cpu_off:
-        {
-            uint32_t pstate = PSCI_ARG32(regs,1);
-            perfc_incr(vpsci_cpu_off);
-            PSCI_SET_RESULT(regs, do_psci_cpu_off(pstate));
-        }
-        break;
-    case PSCI_cpu_on:
-        {
-            uint32_t vcpuid = PSCI_ARG32(regs,1);
-            register_t epoint = PSCI_ARG(regs,2);
-            perfc_incr(vpsci_cpu_on);
-            PSCI_SET_RESULT(regs, do_psci_cpu_on(vcpuid, epoint));
-        }
-        break;
-    case PSCI_0_2_FN_PSCI_VERSION:
-        perfc_incr(vpsci_version);
-        PSCI_SET_RESULT(regs, do_psci_0_2_version());
-        break;
-    case PSCI_0_2_FN_CPU_OFF:
-        perfc_incr(vpsci_cpu_off);
-        PSCI_SET_RESULT(regs, do_psci_0_2_cpu_off());
-        break;
-    case PSCI_0_2_FN_MIGRATE_INFO_TYPE:
-        perfc_incr(vpsci_migrate_info_type);
-        PSCI_SET_RESULT(regs, do_psci_0_2_migrate_info_type());
-        break;
-    case PSCI_0_2_FN_MIGRATE_INFO_UP_CPU:
-    case PSCI_0_2_FN64_MIGRATE_INFO_UP_CPU:
-        perfc_incr(vpsci_migrate_info_up_cpu);
-        if ( psci_mode_check(current->domain, fid) )
-            PSCI_SET_RESULT(regs, do_psci_0_2_migrate_info_up_cpu());
-        break;
-    case PSCI_0_2_FN_SYSTEM_OFF:
-        perfc_incr(vpsci_system_off);
-        do_psci_0_2_system_off();
-        PSCI_SET_RESULT(regs, PSCI_INTERNAL_FAILURE);
-        break;
-    case PSCI_0_2_FN_SYSTEM_RESET:
-        perfc_incr(vpsci_system_reset);
-        do_psci_0_2_system_reset();
-        PSCI_SET_RESULT(regs, PSCI_INTERNAL_FAILURE);
-        break;
-    case PSCI_0_2_FN_CPU_ON:
-    case PSCI_0_2_FN64_CPU_ON:
-        perfc_incr(vpsci_cpu_on);
-        if ( psci_mode_check(current->domain, fid) )
-        {
-            register_t vcpuid = PSCI_ARG(regs,1);
-            register_t epoint = PSCI_ARG(regs,2);
-            register_t cid = PSCI_ARG(regs,3);
-            PSCI_SET_RESULT(regs, do_psci_0_2_cpu_on(vcpuid, epoint, cid));
-        }
-        break;
-    case PSCI_0_2_FN_CPU_SUSPEND:
-    case PSCI_0_2_FN64_CPU_SUSPEND:
-        perfc_incr(vpsci_cpu_suspend);
-        if ( psci_mode_check(current->domain, fid) )
-        {
-            uint32_t pstate = PSCI_ARG32(regs,1);
-            register_t epoint = PSCI_ARG(regs,2);
-            register_t cid = PSCI_ARG(regs,3);
-            PSCI_SET_RESULT(regs, do_psci_0_2_cpu_suspend(pstate, epoint, cid));
-        }
-        break;
-    case PSCI_0_2_FN_AFFINITY_INFO:
-    case PSCI_0_2_FN64_AFFINITY_INFO:
-        perfc_incr(vpsci_cpu_affinity_info);
-        if ( psci_mode_check(current->domain, fid) )
-        {
-            register_t taff = PSCI_ARG(regs,1);
-            uint32_t laff = PSCI_ARG32(regs,2);
-            PSCI_SET_RESULT(regs, do_psci_0_2_affinity_info(taff, laff));
-        }
-        break;
-    case PSCI_0_2_FN_MIGRATE:
-    case PSCI_0_2_FN64_MIGRATE:
-        perfc_incr(vpsci_cpu_migrate);
-        if ( psci_mode_check(current->domain, fid) )
-        {
-            uint32_t tcpu = PSCI_ARG32(regs,1);
-            PSCI_SET_RESULT(regs, do_psci_0_2_migrate(tcpu));
-        }
-        break;
-    default:
-        domain_crash_synchronous();
-        return;
-    }
-}
-
 #ifdef CONFIG_ARM_64
 #define HYPERCALL_RESULT_REG(r) (r)->x0
 #define HYPERCALL_ARG1(r) (r)->x0
@@ -2291,7 +2178,7 @@ void do_trap_guest_sync(struct cpu_user_regs *regs)
             return do_debug_trap(regs, hsr.iss & 0x00ff);
 #endif
         if ( hsr.iss == 0 )
-            return do_trap_psci(regs);
+            return do_trap_hvc_smccc(regs);
         do_trap_hypercall(regs, (register_t *)&regs->r12, hsr.iss);
         break;
 #ifdef CONFIG_ARM_64
@@ -2303,7 +2190,7 @@ void do_trap_guest_sync(struct cpu_user_regs *regs)
             return do_debug_trap(regs, hsr.iss & 0x00ff);
 #endif
         if ( hsr.iss == 0 )
-            return do_trap_psci(regs);
+            return do_trap_hvc_smccc(regs);
         do_trap_hypercall(regs, &regs->x16, hsr.iss);
         break;
     case HSR_EC_SMC64:
diff --git a/xen/arch/arm/vsmc.c b/xen/arch/arm/vsmc.c
index 38df821..9dc41b4 100644
--- a/xen/arch/arm/vsmc.c
+++ b/xen/arch/arm/vsmc.c
@@ -19,6 +19,7 @@
 #include <xen/types.h>
 #include <public/arch-arm/smccc.h>
 #include <asm/monitor.h>
+#include <asm/psci.h>
 #include <asm/regs.h>
 #include <asm/smccc.h>
 #include <asm/traps.h>
@@ -26,6 +27,9 @@
 /* Number of functions currently supported by Hypervisor Service. */
 #define XEN_SMCCC_FUNCTION_COUNT 3
 
+/* Number of functions currently supported by Standard Service Service Calls. */
+#define SSSC_SMCCC_FUNCTION_COUNT 13
+
 static bool fill_uid(struct cpu_user_regs *regs, xen_uuid_t uuid)
 {
     int n;
@@ -94,6 +98,148 @@ static bool handle_hypervisor(struct cpu_user_regs *regs)
     }
 }
 
+#define PSCI_SET_RESULT(reg, val) set_user_reg(reg, 0, val)
+#define PSCI_ARG(reg, n) get_user_reg(reg, n)
+
+#ifdef CONFIG_ARM_64
+#define PSCI_ARG32(reg, n) (uint32_t)(get_user_reg(reg, n))
+#else
+#define PSCI_ARG32(reg, n) PSCI_ARG(reg, n)
+#endif
+
+/* Existing (pre SMCCC) APIs. This includes PSCI 0.1 interface */
+static bool handle_existing_apis(struct cpu_user_regs *regs)
+{
+    /* Only least 32 bits are significant (ARM DEN 0028B, page 12) */
+    switch ( (uint32_t)get_user_reg(regs, 0) )
+    {
+    case PSCI_cpu_off:
+    {
+        uint32_t pstate = PSCI_ARG32(regs, 1);
+
+        perfc_incr(vpsci_cpu_off);
+        PSCI_SET_RESULT(regs, do_psci_cpu_off(pstate));
+        return true;
+    }
+    case PSCI_cpu_on:
+    {
+        uint32_t vcpuid = PSCI_ARG32(regs, 1);
+        register_t epoint = PSCI_ARG(regs, 2);
+
+        perfc_incr(vpsci_cpu_on);
+        PSCI_SET_RESULT(regs, do_psci_cpu_on(vcpuid, epoint));
+        return true;
+    }
+    default:
+        return false;
+    }
+}
+
+/* helper function for checking arm mode 32/64 bit */
+static inline int psci_mode_check(struct domain *d, uint32_t fid)
+{
+    return !( is_64bit_domain(d)^( (fid & PSCI_0_2_64BIT) >> 30 ) );
+}
+
+/* PSCI 0.2 interface and other Standard Secure Calls */
+static bool handle_sssc(struct cpu_user_regs *regs)
+{
+    uint32_t fid = PSCI_ARG32(regs, 0);
+
+    switch ( fid )
+    {
+    case PSCI_0_2_FN_PSCI_VERSION:
+        perfc_incr(vpsci_version);
+        PSCI_SET_RESULT(regs, do_psci_0_2_version());
+        return true;
+
+    case PSCI_0_2_FN_CPU_OFF:
+        perfc_incr(vpsci_cpu_off);
+        PSCI_SET_RESULT(regs, do_psci_0_2_cpu_off());
+        return true;
+
+    case PSCI_0_2_FN_MIGRATE_INFO_TYPE:
+        perfc_incr(vpsci_migrate_info_type);
+        PSCI_SET_RESULT(regs, do_psci_0_2_migrate_info_type());
+        return true;
+
+    case PSCI_0_2_FN_MIGRATE_INFO_UP_CPU:
+        perfc_incr(vpsci_migrate_info_up_cpu);
+        if ( psci_mode_check(current->domain, fid) )
+            PSCI_SET_RESULT(regs, do_psci_0_2_migrate_info_up_cpu());
+        return true;
+
+    case PSCI_0_2_FN_SYSTEM_OFF:
+        perfc_incr(vpsci_system_off);
+        do_psci_0_2_system_off();
+        PSCI_SET_RESULT(regs, PSCI_INTERNAL_FAILURE);
+        return true;
+
+    case PSCI_0_2_FN_SYSTEM_RESET:
+        perfc_incr(vpsci_system_reset);
+        do_psci_0_2_system_reset();
+        PSCI_SET_RESULT(regs, PSCI_INTERNAL_FAILURE);
+        return true;
+
+    case PSCI_0_2_FN_CPU_ON:
+        perfc_incr(vpsci_cpu_on);
+        if ( psci_mode_check(current->domain, fid) )
+        {
+            register_t vcpuid = PSCI_ARG(regs, 1);
+            register_t epoint = PSCI_ARG(regs, 2);
+            register_t cid = PSCI_ARG(regs, 3);
+
+            PSCI_SET_RESULT(regs, do_psci_0_2_cpu_on(vcpuid, epoint, cid));
+        }
+        return true;
+
+    case PSCI_0_2_FN_CPU_SUSPEND:
+        perfc_incr(vpsci_cpu_suspend);
+        if ( psci_mode_check(current->domain, fid) )
+        {
+            uint32_t pstate = PSCI_ARG32(regs, 1);
+            register_t epoint = PSCI_ARG(regs, 2);
+            register_t cid = PSCI_ARG(regs, 3);
+
+            PSCI_SET_RESULT(regs, do_psci_0_2_cpu_suspend(pstate, epoint, cid));
+        }
+        return true;
+
+    case PSCI_0_2_FN_AFFINITY_INFO:
+        perfc_incr(vpsci_cpu_affinity_info);
+        if ( psci_mode_check(current->domain, fid) )
+        {
+            register_t taff = PSCI_ARG(regs, 1);
+            uint32_t laff = PSCI_ARG32(regs, 2);
+            PSCI_SET_RESULT(regs, do_psci_0_2_affinity_info(taff, laff));
+        }
+        return true;
+
+    case PSCI_0_2_FN_MIGRATE:
+        perfc_incr(vpsci_cpu_migrate);
+        if ( psci_mode_check(current->domain, fid) )
+        {
+            uint32_t tcpu = PSCI_ARG32(regs, 1);
+
+            PSCI_SET_RESULT(regs, do_psci_0_2_migrate(tcpu));
+        }
+        return true;
+
+    case ARM_SMCCC_FUNC_CALL_COUNT:
+        return fill_function_call_count(regs, SSSC_SMCCC_FUNCTION_COUNT);
+
+    case ARM_SMCCC_FUNC_CALL_UID:
+        return fill_uid(regs, SSSC_SMCCC_UID);
+
+    case ARM_SMCCC_FUNC_CALL_REVISION:
+        return fill_revision(regs, SSSC_SMCCC_MAJOR_REVISION,
+                             SSSC_SMCCC_MINOR_REVISION);
+
+    default:
+        return false;
+    }
+}
+
 /*
  * vsmccc_handle_call() - handle SMC/HVC call according to ARM SMCCC.
  * returns true if that was valid SMCCC call (even if function number
@@ -133,11 +279,26 @@ static bool vsmccc_handle_call(struct cpu_user_regs *regs)
         return true;
     }
 
-    switch ( smccc_get_owner(funcid) )
+    /*
+     * Special case: identifier range for existing APIs.
+     * This range is described in SMCCC (ARM DEN 0028B, page 16),
+     * but it does not conforms to standard function identifier
+     * encoding.
+     */
+    if ( funcid >= ARM_SMCCC_RESERVED_RANGE_START &&
+         funcid <= ARM_SMCCC_RESERVED_RANGE_END )
+        handled = handle_existing_apis(regs);
+    else
     {
-    case ARM_SMCCC_OWNER_HYPERVISOR:
-        handled = handle_hypervisor(regs);
-        break;
+        switch ( smccc_get_owner(funcid) )
+        {
+        case ARM_SMCCC_OWNER_HYPERVISOR:
+            handled = handle_hypervisor(regs);
+            break;
+        case ARM_SMCCC_OWNER_STANDARD:
+            handled = handle_sssc(regs);
+            break;
+        }
     }
 
     if ( !handled )
@@ -181,6 +342,20 @@ void do_trap_smc(struct cpu_user_regs *regs, const union hsr hsr)
         inject_undef_exception(regs, hsr);
 }
 
+void do_trap_hvc_smccc(struct cpu_user_regs *regs)
+{
+    const union hsr hsr = { .bits = regs->hsr };
+
+    /*
+     * vsmccc_handle_call() will return false if this call is not
+     * SMCCC compatible (e.g. immediate value != 0). As it is not
+     * compatible, we can't be sure that guest will understand
+     * ARM_SMCCC_ERR_UNKNOWN_FUNCTION.
+     */
+    if ( !vsmccc_handle_call(regs) )
+        inject_undef_exception(regs, hsr);
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/include/asm-arm/traps.h b/xen/include/asm-arm/traps.h
index 325c15f..a0e5e92 100644
--- a/xen/include/asm-arm/traps.h
+++ b/xen/include/asm-arm/traps.h
@@ -37,6 +37,7 @@ void do_cp(struct cpu_user_regs *regs, const union hsr hsr);
 
 /* SMCCC handling */
 void do_trap_smc(struct cpu_user_regs *regs, const union hsr hsr);
+void do_trap_hvc_smccc(struct cpu_user_regs *regs);
 
 #endif /* __ASM_ARM_TRAPS__ */
 /*
diff --git a/xen/include/public/arch-arm/smccc.h b/xen/include/public/arch-arm/smccc.h
index 2bee5b3..17dc6d8 100644
--- a/xen/include/public/arch-arm/smccc.h
+++ b/xen/include/public/arch-arm/smccc.h
@@ -46,6 +46,14 @@
 #define XEN_SMCCC_UID XEN_DEFINE_UUID(0xa71812dc, 0xc698, 0x4369, 0x9acf, \
                                       0x79, 0xd1, 0x8d, 0xde, 0xe6, 0x67)
 
+/* Standard Service Service Call version. */
+#define SSSC_SMCCC_MAJOR_REVISION 0
+#define SSSC_SMCCC_MINOR_REVISION 1
+
+/* Standard Service Call UID. Randomly generated with uuidgen. */
+#define SSSC_SMCCC_UID XEN_DEFINE_UUID(0xf863386f, 0x4b39, 0x4cbd, 0x9220,\
+                                       0xce, 0x16, 0x41, 0xe5, 0x9f, 0x6f)
+
 #endif /* __XEN_PUBLIC_ARCH_ARM_SMCCC_H__ */
 
 /*
-- 
2.7.4


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

^ permalink raw reply related

* [PATCH v7 09/11] arm: PSCI: use definitions provided by asm/smccc.h
From: Volodymyr Babchuk @ 2017-10-04 21:00 UTC (permalink / raw)
  To: xen-devel
  Cc: Edgar E . Iglesias, Julien Grall, Stefano Stabellini,
	Volodymyr Babchuk
In-Reply-To: <1507150827-7858-1-git-send-email-volodymyr_babchuk@epam.com>

smccc.h provides definitions to construct SMC call function number according
to SMCCC. We don't need multiple definitions for one thing, and definitions
in smccc.h are more generic than ones used in psci.h.

So psci.h will only provide function codes, while whole SMC function
identifier will be constructed using generic macros from smccc.h.

Function psci_mode_check() in vsmc.c will be removed in a next patch,
so there are no need to review it. I had to rework it, because
PSCI_0_2_64BIT definition is dropped now.

Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
---
 * Fixed indentation for
 #define PSCI_0_2_FN_NATIVE(name)    PSCI_0_2_FN64(name)
 and
 #define PSCI_0_2_FN_NATIVE(name)    PSCI_0_2_FN32(name)

---
 xen/arch/arm/platforms/seattle.c |  4 ++--
 xen/arch/arm/psci.c              | 10 +++++-----
 xen/arch/arm/vsmc.c              |  4 ++--
 xen/include/asm-arm/psci.h       | 43 +++++++++++++++++++---------------------
 4 files changed, 29 insertions(+), 32 deletions(-)

diff --git a/xen/arch/arm/platforms/seattle.c b/xen/arch/arm/platforms/seattle.c
index 86dce91..22c0622 100644
--- a/xen/arch/arm/platforms/seattle.c
+++ b/xen/arch/arm/platforms/seattle.c
@@ -33,12 +33,12 @@ static const char * const seattle_dt_compat[] __initconst =
  */
 static void seattle_system_reset(void)
 {
-    call_smc(PSCI_0_2_FN_SYSTEM_RESET, 0, 0, 0);
+    call_smc(PSCI_0_2_FN32(SYSTEM_RESET), 0, 0, 0);
 }
 
 static void seattle_system_off(void)
 {
-    call_smc(PSCI_0_2_FN_SYSTEM_OFF, 0, 0, 0);
+    call_smc(PSCI_0_2_FN32(SYSTEM_OFF), 0, 0, 0);
 }
 
 PLATFORM_START(seattle, "SEATTLE")
diff --git a/xen/arch/arm/psci.c b/xen/arch/arm/psci.c
index 34ee97e..1508a3b 100644
--- a/xen/arch/arm/psci.c
+++ b/xen/arch/arm/psci.c
@@ -31,9 +31,9 @@
  * (native-width) function ID.
  */
 #ifdef CONFIG_ARM_64
-#define PSCI_0_2_FN_NATIVE(name)	PSCI_0_2_FN64_##name
+#define PSCI_0_2_FN_NATIVE(name)    PSCI_0_2_FN64(name)
 #else
-#define PSCI_0_2_FN_NATIVE(name)	PSCI_0_2_FN_##name
+#define PSCI_0_2_FN_NATIVE(name)    PSCI_0_2_FN32(name)
 #endif
 
 uint32_t psci_ver;
@@ -48,13 +48,13 @@ int call_psci_cpu_on(int cpu)
 void call_psci_system_off(void)
 {
     if ( psci_ver > PSCI_VERSION(0, 1) )
-        call_smc(PSCI_0_2_FN_SYSTEM_OFF, 0, 0, 0);
+        call_smc(PSCI_0_2_FN32(SYSTEM_OFF), 0, 0, 0);
 }
 
 void call_psci_system_reset(void)
 {
     if ( psci_ver > PSCI_VERSION(0, 1) )
-        call_smc(PSCI_0_2_FN_SYSTEM_RESET, 0, 0, 0);
+        call_smc(PSCI_0_2_FN32(SYSTEM_RESET), 0, 0, 0);
 }
 
 int __init psci_is_smc_method(const struct dt_device_node *psci)
@@ -144,7 +144,7 @@ int __init psci_init_0_2(void)
         }
     }
 
-    psci_ver = call_smc(PSCI_0_2_FN_PSCI_VERSION, 0, 0, 0);
+    psci_ver = call_smc(PSCI_0_2_FN32(PSCI_VERSION), 0, 0, 0);
 
     /* For the moment, we only support PSCI 0.2 and PSCI 1.x */
     if ( psci_ver != PSCI_VERSION(0, 2) && PSCI_VERSION_MAJOR(psci_ver) != 1 )
diff --git a/xen/arch/arm/vsmc.c b/xen/arch/arm/vsmc.c
index 9dc41b4..77fc915 100644
--- a/xen/arch/arm/vsmc.c
+++ b/xen/arch/arm/vsmc.c
@@ -138,7 +138,7 @@ static bool handle_existing_apis(struct cpu_user_regs *regs)
 /* helper function for checking arm mode 32/64 bit */
 static inline int psci_mode_check(struct domain *d, uint32_t fid)
 {
-    return !( is_64bit_domain(d)^( (fid & PSCI_0_2_64BIT) >> 30 ) );
+    return is_64bit_domain(d) || !smccc_is_conv_64(fid);
 }
 
 /* PSCI 0.2 interface and other Standard Secure Calls */
@@ -146,7 +146,7 @@ static bool handle_sssc(struct cpu_user_regs *regs)
 {
     uint32_t fid = PSCI_ARG32(regs, 0);
 
-    switch ( fid )
+    switch ( smccc_get_fn(fid) )
     {
     case PSCI_0_2_FN_PSCI_VERSION:
         perfc_incr(vpsci_version);
diff --git a/xen/include/asm-arm/psci.h b/xen/include/asm-arm/psci.h
index be2458a..635ea5d 100644
--- a/xen/include/asm-arm/psci.h
+++ b/xen/include/asm-arm/psci.h
@@ -1,6 +1,8 @@
 #ifndef __ASM_PSCI_H__
 #define __ASM_PSCI_H__
 
+#include <asm/smccc.h>
+
 /* PSCI return values (inclusive of all PSCI versions) */
 #define PSCI_SUCCESS                 0
 #define PSCI_NOT_SUPPORTED          -1
@@ -42,29 +44,24 @@ void do_psci_0_2_system_off(void);
 void do_psci_0_2_system_reset(void);
 
 /* PSCI v0.2 interface */
-#define PSCI_0_2_FN_BASE        0x84000000
-#define PSCI_0_2_FN(n)          (PSCI_0_2_FN_BASE + (n))
-#define PSCI_0_2_64BIT          0x40000000
-#define PSCI_0_2_FN64_BASE      \
-                        (PSCI_0_2_FN_BASE + PSCI_0_2_64BIT)
-#define PSCI_0_2_FN64(n)        (PSCI_0_2_FN64_BASE + (n))
-
-#define PSCI_0_2_FN_PSCI_VERSION        PSCI_0_2_FN(0)
-#define PSCI_0_2_FN_CPU_SUSPEND         PSCI_0_2_FN(1)
-#define PSCI_0_2_FN_CPU_OFF             PSCI_0_2_FN(2)
-#define PSCI_0_2_FN_CPU_ON              PSCI_0_2_FN(3)
-#define PSCI_0_2_FN_AFFINITY_INFO       PSCI_0_2_FN(4)
-#define PSCI_0_2_FN_MIGRATE             PSCI_0_2_FN(5)
-#define PSCI_0_2_FN_MIGRATE_INFO_TYPE   PSCI_0_2_FN(6)
-#define PSCI_0_2_FN_MIGRATE_INFO_UP_CPU PSCI_0_2_FN(7)
-#define PSCI_0_2_FN_SYSTEM_OFF          PSCI_0_2_FN(8)
-#define PSCI_0_2_FN_SYSTEM_RESET        PSCI_0_2_FN(9)
-
-#define PSCI_0_2_FN64_CPU_SUSPEND       PSCI_0_2_FN64(1)
-#define PSCI_0_2_FN64_CPU_ON            PSCI_0_2_FN64(3)
-#define PSCI_0_2_FN64_AFFINITY_INFO     PSCI_0_2_FN64(4)
-#define PSCI_0_2_FN64_MIGRATE           PSCI_0_2_FN64(5)
-#define PSCI_0_2_FN64_MIGRATE_INFO_UP_CPU   PSCI_0_2_FN64(7)
+#define PSCI_0_2_FN32(name) ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL,             \
+                                               ARM_SMCCC_CONV_32,               \
+                                               ARM_SMCCC_OWNER_STANDARD,        \
+                                               PSCI_0_2_FN_##name)
+#define PSCI_0_2_FN64(name) ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL,             \
+                                               ARM_SMCCC_CONV_64,               \
+                                               ARM_SMCCC_OWNER_STANDARD,        \
+                                               PSCI_0_2_FN_##name)
+#define PSCI_0_2_FN_PSCI_VERSION        0
+#define PSCI_0_2_FN_CPU_SUSPEND         1
+#define PSCI_0_2_FN_CPU_OFF             2
+#define PSCI_0_2_FN_CPU_ON              3
+#define PSCI_0_2_FN_AFFINITY_INFO       4
+#define PSCI_0_2_FN_MIGRATE             5
+#define PSCI_0_2_FN_MIGRATE_INFO_TYPE   6
+#define PSCI_0_2_FN_MIGRATE_INFO_UP_CPU 7
+#define PSCI_0_2_FN_SYSTEM_OFF          8
+#define PSCI_0_2_FN_SYSTEM_RESET        9
 
 /* PSCI v0.2 affinity level state returned by AFFINITY_INFO */
 #define PSCI_0_2_AFFINITY_LEVEL_ON      0
-- 
2.7.4


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

^ permalink raw reply related

* [PATCH v7 10/11] arm: vsmc: remove 64 bit mode check in PSCI handler
From: Volodymyr Babchuk @ 2017-10-04 21:00 UTC (permalink / raw)
  To: xen-devel
  Cc: Edgar E . Iglesias, Julien Grall, Stefano Stabellini,
	Volodymyr Babchuk
In-Reply-To: <1507150827-7858-1-git-send-email-volodymyr_babchuk@epam.com>

PSCI handling code had helper routine that checked calling convention.
It does not needed anymore, because:

 - Generic handler checks that 64 bit calls can be made only by
   64 bit guests.

 - SMCCC requires that 64-bit handler should support both 32 and 64 bit
   calls even if they originate from 64 bit caller.

This patch removes that extra check.

Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
---
 xen/arch/arm/vsmc.c | 62 ++++++++++++++++++++++-------------------------------
 1 file changed, 26 insertions(+), 36 deletions(-)

diff --git a/xen/arch/arm/vsmc.c b/xen/arch/arm/vsmc.c
index 77fc915..ff84442 100644
--- a/xen/arch/arm/vsmc.c
+++ b/xen/arch/arm/vsmc.c
@@ -135,12 +135,6 @@ static bool handle_existing_apis(struct cpu_user_regs *regs)
     }
 }
 
-/* helper function for checking arm mode 32/64 bit */
-static inline int psci_mode_check(struct domain *d, uint32_t fid)
-{
-    return is_64bit_domain(d) || !smccc_is_conv_64(fid);
-}
-
 /* PSCI 0.2 interface and other Standard Secure Calls */
 static bool handle_sssc(struct cpu_user_regs *regs)
 {
@@ -165,8 +159,7 @@ static bool handle_sssc(struct cpu_user_regs *regs)
 
     case PSCI_0_2_FN_MIGRATE_INFO_UP_CPU:
         perfc_incr(vpsci_migrate_info_up_cpu);
-        if ( psci_mode_check(current->domain, fid) )
-            PSCI_SET_RESULT(regs, do_psci_0_2_migrate_info_up_cpu());
+        PSCI_SET_RESULT(regs, do_psci_0_2_migrate_info_up_cpu());
         return true;
 
     case PSCI_0_2_FN_SYSTEM_OFF:
@@ -182,48 +175,45 @@ static bool handle_sssc(struct cpu_user_regs *regs)
         return true;
 
     case PSCI_0_2_FN_CPU_ON:
-        perfc_incr(vpsci_cpu_on);
-        if ( psci_mode_check(current->domain, fid) )
-        {
-            register_t vcpuid = PSCI_ARG(regs, 1);
-            register_t epoint = PSCI_ARG(regs, 2);
-            register_t cid = PSCI_ARG(regs, 3);
+    {
+        register_t vcpuid = PSCI_ARG(regs, 1);
+        register_t epoint = PSCI_ARG(regs, 2);
+        register_t cid = PSCI_ARG(regs, 3);
 
-            PSCI_SET_RESULT(regs, do_psci_0_2_cpu_on(vcpuid, epoint, cid));
-        }
+        perfc_incr(vpsci_cpu_on);
+        PSCI_SET_RESULT(regs, do_psci_0_2_cpu_on(vcpuid, epoint, cid));
         return true;
+    }
 
     case PSCI_0_2_FN_CPU_SUSPEND:
-        perfc_incr(vpsci_cpu_suspend);
-        if ( psci_mode_check(current->domain, fid) )
-        {
-            uint32_t pstate = PSCI_ARG32(regs, 1);
-            register_t epoint = PSCI_ARG(regs, 2);
-            register_t cid = PSCI_ARG(regs, 3);
+    {
+        uint32_t pstate = PSCI_ARG32(regs, 1);
+        register_t epoint = PSCI_ARG(regs, 2);
+        register_t cid = PSCI_ARG(regs, 3);
 
-            PSCI_SET_RESULT(regs, do_psci_0_2_cpu_suspend(pstate, epoint, cid));
-        }
+        perfc_incr(vpsci_cpu_suspend);
+        PSCI_SET_RESULT(regs, do_psci_0_2_cpu_suspend(pstate, epoint, cid));
         return true;
+    }
 
     case PSCI_0_2_FN_AFFINITY_INFO:
+    {
+        register_t taff = PSCI_ARG(regs, 1);
+        uint32_t laff = PSCI_ARG32(regs, 2);
+
         perfc_incr(vpsci_cpu_affinity_info);
-        if ( psci_mode_check(current->domain, fid) )
-        {
-            register_t taff = PSCI_ARG(regs, 1);
-            uint32_t laff = PSCI_ARG32(regs, 2);
-            PSCI_SET_RESULT(regs, do_psci_0_2_affinity_info(taff, laff));
-        }
+        PSCI_SET_RESULT(regs, do_psci_0_2_affinity_info(taff, laff));
         return true;
+    }
 
     case PSCI_0_2_FN_MIGRATE:
-        perfc_incr(vpsci_cpu_migrate);
-        if ( psci_mode_check(current->domain, fid) )
-        {
-            uint32_t tcpu = PSCI_ARG32(regs, 1);
+    {
+        uint32_t tcpu = PSCI_ARG32(regs, 1);
 
-            PSCI_SET_RESULT(regs, do_psci_0_2_migrate(tcpu));
-        }
+        perfc_incr(vpsci_cpu_migrate);
+        PSCI_SET_RESULT(regs, do_psci_0_2_migrate(tcpu));
         return true;
+    }
 
     case ARM_SMCCC_FUNC_CALL_COUNT:
         return fill_function_call_count(regs, SSSC_SMCCC_FUNCTION_COUNT);
-- 
2.7.4


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

^ permalink raw reply related

* [PATCH v7 11/11] public: add and enable XENFEAT_ARM_SMCCC_supported feature
From: Volodymyr Babchuk @ 2017-10-04 21:00 UTC (permalink / raw)
  To: xen-devel
  Cc: Edgar E . Iglesias, Stefano Stabellini, Wei Liu,
	Konrad Rzeszutek Wilk, George Dunlap, Andrew Cooper, Ian Jackson,
	Tim Deegan, Julien Grall, Jan Beulich, Volodymyr Babchuk
In-Reply-To: <1507150827-7858-1-git-send-email-volodymyr_babchuk@epam.com>

This feature indicates that hypervisor is compatible with ARM
SMC calling convention. Previously hypervisor would inject an
undefined instruction exception if an invalid SMC function were
called or would crash a domain if an invalid HVC function
were invoked.
XENFEAT_ARM_SMCCC_supported feature means that it safe to invoke
SMC/HVC calls that are compatible with SMC calling convention.

Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
---
 xen/common/kernel.c           | 3 +++
 xen/include/public/features.h | 3 +++
 2 files changed, 6 insertions(+)

diff --git a/xen/common/kernel.c b/xen/common/kernel.c
index 94fdf5c..8d137c5 100644
--- a/xen/common/kernel.c
+++ b/xen/common/kernel.c
@@ -394,6 +394,9 @@ DO(xen_version)(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
                     (1U << XENFEAT_auto_translated_physmap);
             if ( is_hardware_domain(d) )
                 fi.submap |= 1U << XENFEAT_dom0;
+#ifdef CONFIG_ARM
+            fi.submap |= (1U << XENFEAT_ARM_SMCCC_supported);
+#endif
 #ifdef CONFIG_X86
             switch ( d->guest_type )
             {
diff --git a/xen/include/public/features.h b/xen/include/public/features.h
index 2110b04..1a989b8 100644
--- a/xen/include/public/features.h
+++ b/xen/include/public/features.h
@@ -102,6 +102,9 @@
 /* Guest can use XENMEMF_vnode to specify virtual node for memory op. */
 #define XENFEAT_memory_op_vnode_supported 13
 
+/* arm: Hypervisor supports ARM SMC calling convention. */
+#define XENFEAT_ARM_SMCCC_supported       14
+
 #define XENFEAT_NR_SUBMAPS 1
 
 #endif /* __XEN_PUBLIC_FEATURES_H__ */
-- 
2.7.4


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

^ permalink raw reply related

* Re: [PATCH 1/2] Revert "vmalloc: back off when the current task is killed"
From: Johannes Weiner @ 2017-10-04 21:00 UTC (permalink / raw)
  To: Tetsuo Handa
  Cc: Andrew Morton, Alan Cox, Christoph Hellwig, Michal Hocko,
	linux-mm, linux-kernel, kernel-team
In-Reply-To: <ab688e7c-75c1-e942-ef44-44615d9fb394@I-love.SAKURA.ne.jp>

On Thu, Oct 05, 2017 at 05:49:43AM +0900, Tetsuo Handa wrote:
> On 2017/10/05 3:59, Johannes Weiner wrote:
> > But the justification to make that vmalloc() call fail like this isn't
> > convincing, either. The patch mentions an OOM victim exhausting the
> > memory reserves and thus deadlocking the machine. But the OOM killer
> > is only one, improbable source of fatal signals. It doesn't make sense
> > to fail allocations preemptively with plenty of memory in most cases.
> 
> By the time the current thread reaches do_exit(), fatal_signal_pending(current)
> should become false. As far as I can guess, the source of fatal signal will be
> tty_signal_session_leader(tty, exit_session) which is called just before
> tty_ldisc_hangup(tty, cons_filp != NULL) rather than the OOM killer. I don't
> know whether it is possible to make fatal_signal_pending(current) true inside
> do_exit() though...

It's definitely not the OOM killer, the memory situation looks fine
when this happens. I didn't look closer where the signal comes from.

That said, we trigger this issue fairly easily. We tested the revert
over night on a couple thousand machines, and it fixed the issue
(whereas the control group still saw the crashes).

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply

* Re: [PATCH 1/2] Revert "vmalloc: back off when the current task is killed"
From: Johannes Weiner @ 2017-10-04 21:00 UTC (permalink / raw)
  To: Tetsuo Handa
  Cc: Andrew Morton, Alan Cox, Christoph Hellwig, Michal Hocko,
	linux-mm, linux-kernel, kernel-team
In-Reply-To: <ab688e7c-75c1-e942-ef44-44615d9fb394@I-love.SAKURA.ne.jp>

On Thu, Oct 05, 2017 at 05:49:43AM +0900, Tetsuo Handa wrote:
> On 2017/10/05 3:59, Johannes Weiner wrote:
> > But the justification to make that vmalloc() call fail like this isn't
> > convincing, either. The patch mentions an OOM victim exhausting the
> > memory reserves and thus deadlocking the machine. But the OOM killer
> > is only one, improbable source of fatal signals. It doesn't make sense
> > to fail allocations preemptively with plenty of memory in most cases.
> 
> By the time the current thread reaches do_exit(), fatal_signal_pending(current)
> should become false. As far as I can guess, the source of fatal signal will be
> tty_signal_session_leader(tty, exit_session) which is called just before
> tty_ldisc_hangup(tty, cons_filp != NULL) rather than the OOM killer. I don't
> know whether it is possible to make fatal_signal_pending(current) true inside
> do_exit() though...

It's definitely not the OOM killer, the memory situation looks fine
when this happens. I didn't look closer where the signal comes from.

That said, we trigger this issue fairly easily. We tested the revert
over night on a couple thousand machines, and it fixed the issue
(whereas the control group still saw the crashes).

^ permalink raw reply

* [Buildroot] [PATCH v3 1/4] package/libbcg729: new package
From: Bernd Kuhls @ 2017-10-04 21:02 UTC (permalink / raw)
  To: buildroot

Quoting http://www.linphone.org/technical-corner/bcg729/downloads
regarding patent information:

"ITU G729 Annex A/B were offically released October/November 1996
(https://www.itu.int/rec/T-REC-G.729), hence all patents covering these
specifications shall have expired in November 2016.

Patent pool administrator confirmed most licensed patents under the
G.729 Consortium have expired (http://www.sipro.com/G729.html)."

Signed-off-by: Bernd Kuhls <bernd.kuhls@t-online.de>
---
v3: bumped to version 1.0.4, switched to cmake
v2: no changes

Tested v3 using this defconfig

BR2_PACKAGE_LIBBCG729=y

with positive results:

                armv5-ctng-linux-gnueabi [ 1/47]: OK
              armv7-ctng-linux-gnueabihf [ 2/47]: OK
                        br-aarch64-glibc [ 3/47]: OK
                           br-arcle-hs38 [ 4/47]: OK
                            br-arm-basic [ 5/47]: OK
                  br-arm-cortex-a9-glibc [ 6/47]: OK
                   br-arm-cortex-a9-musl [ 7/47]: OK
                   br-arm-cortex-m4-full [ 8/47]: OK
                             br-arm-full [ 9/47]: OK
                    br-arm-full-nothread [10/47]: OK
                      br-arm-full-static [11/47]: OK
                            br-bfin-full [12/47]: OK
                   br-i386-pentium4-full [13/47]: OK
                br-i386-pentium-mmx-musl [14/47]: OK
                       br-m68k-5208-full [15/47]: OK
                      br-m68k-68040-full [16/47]: OK
                    br-microblazeel-full [17/47]: OK
                 br-mips32r6-el-hf-glibc [18/47]: OK
                      br-mips64-n64-full [19/47]: OK
                 br-mips64r6-el-hf-glibc [20/47]: OK
                      br-mipsel-o32-full [21/47]: OK
                          br-nios2-glibc [22/47]: OK
                      br-openrisc-uclibc [23/47]: OK
               br-powerpc-603e-basic-cpp [24/47]: OK
             br-powerpc64le-power8-glibc [25/47]: OK
               br-powerpc64-power7-glibc [26/47]: OK
                  br-powerpc-e500mc-full [27/47]: OK
                             br-sh4-full [28/47]: OK
                        br-sparc64-glibc [29/47]: OK
                         br-sparc-uclibc [30/47]: OK
                    br-x86-64-core2-full [31/47]: OK
                          br-x86-64-musl [32/47]: OK
                          br-xtensa-full [33/47]: OK
                     i686-ctng-linux-gnu [34/47]: OK
                          linaro-aarch64 [35/47]: OK
                              linaro-arm [36/47]: OK
             mips64el-ctng_n32-linux-gnu [37/47]: OK
             mips64el-ctng_n64-linux-gnu [38/47]: OK
        powerpc-ctng_e500v2-linux-gnuspe [39/47]: OK
                     sourcery-arm-armv4t [40/47]: OK
                            sourcery-arm [41/47]: OK
                     sourcery-arm-thumb2 [42/47]: OK
                         sourcery-mips64 [43/47]: OK
                           sourcery-mips [44/47]: OK
                          sourcery-nios2 [45/47]: OK
                         sourcery-x86-64 [46/47]: OK
           x86_64-ctng_locales-linux-gnu [47/47]: OK
47 builds, 0 skipped, 0 build failed, 0 legal-info failed

 DEVELOPERS                       |  1 +
 package/Config.in                |  1 +
 package/libbcg729/Config.in      |  7 +++++++
 package/libbcg729/libbcg729.hash |  2 ++
 package/libbcg729/libbcg729.mk   | 21 +++++++++++++++++++++
 5 files changed, 32 insertions(+)
 create mode 100644 package/libbcg729/Config.in
 create mode 100644 package/libbcg729/libbcg729.hash
 create mode 100644 package/libbcg729/libbcg729.mk

diff --git a/DEVELOPERS b/DEVELOPERS
index 43ad9e5f8a..46daf6295c 100644
--- a/DEVELOPERS
+++ b/DEVELOPERS
@@ -218,6 +218,7 @@ F:	package/leafnode2/
 F:	package/libaacs/
 F:	package/libasplib/
 F:	package/libass/
+F:	package/libbcg729/
 F:	package/libbdplus/
 F:	package/libbluray/
 F:	package/libbroadvoice/
diff --git a/package/Config.in b/package/Config.in
index 36b42d24f1..7b99a6a510 100644
--- a/package/Config.in
+++ b/package/Config.in
@@ -950,6 +950,7 @@ menu "Audio/Sound"
 	source "package/fdk-aac/Config.in"
 	source "package/libao/Config.in"
 	source "package/libasplib/Config.in"
+	source "package/libbcg729/Config.in"
 	source "package/libbroadvoice/Config.in"
 	source "package/libcdaudio/Config.in"
 	source "package/libcddb/Config.in"
diff --git a/package/libbcg729/Config.in b/package/libbcg729/Config.in
new file mode 100644
index 0000000000..82847a0b72
--- /dev/null
+++ b/package/libbcg729/Config.in
@@ -0,0 +1,7 @@
+config BR2_PACKAGE_LIBBCG729
+	bool "libbcg729"
+	help
+	  Bcg729 is an opensource implementation of both encoder and
+	  decoder of the ITU G729 Annex A/B speech codec.
+
+	  http://www.linphone.org/technical-corner/bcg729/overview
diff --git a/package/libbcg729/libbcg729.hash b/package/libbcg729/libbcg729.hash
new file mode 100644
index 0000000000..7fa0b4a044
--- /dev/null
+++ b/package/libbcg729/libbcg729.hash
@@ -0,0 +1,2 @@
+# Locally computed
+sha256 94b3542a06cbd96306efc19f959f9febae62806a22599063f82a8c33e989d48b  libbcg729-1.0.4.tar.gz
diff --git a/package/libbcg729/libbcg729.mk b/package/libbcg729/libbcg729.mk
new file mode 100644
index 0000000000..2f1c12dfa5
--- /dev/null
+++ b/package/libbcg729/libbcg729.mk
@@ -0,0 +1,21 @@
+################################################################################
+#
+# libbcg729
+#
+################################################################################
+
+LIBBCG729_VERSION = 1.0.4
+LIBBCG729_SITE = $(call github,BelledonneCommunications,bcg729,$(LIBBCG729_VERSION))
+LIBBCG729_LICENSE = GPL-2.0+
+LIBBCG729_LICENSE_FILES = COPYING
+LIBBCG729_INSTALL_STAGING = YES
+
+ifeq ($(BR2_STATIC_LIBS),y)
+LIBBCG729_CONF_OPTS += -DENABLE_SHARED=OFF -DENABLE_STATIC=ON
+else ifeq ($(BR2_SHARED_STATIC_LIBS),y)
+LIBBCG729_CONF_OPTS += -DENABLE_SHARED=ON -DENABLE_STATIC=ON
+else ifeq ($(BR2_SHARED_LIBS),y)
+LIBBCG729_CONF_OPTS += -DENABLE_SHARED=ON -DENABLE_STATIC=OFF
+endif
+
+$(eval $(cmake-package))
-- 
2.11.0

^ permalink raw reply related

* [refpolicy] [PATCH v6] fc_sort: memory leakages
From: Guido Trentalancia @ 2017-10-04 21:02 UTC (permalink / raw)
  To: refpolicy
In-Reply-To: <1506811454.32049.1.camel@trentalancia.com>

Avoid memory leakages in the fc_sort executable (now passes
all valgrind AND Clang static analyzer tests fine).

Some NULL pointer checks with or without associated error
reporting.

Some white space and comment formatting fixes.

Optimization: avoid unnecessary operations (unnecessary
memory allocation/deallocation and list copying).

Reverts 7821eb6f37d785ab6ac3bbdc39282c799ad22393 as such
trick is no longer needed, given that all memory leakages
have now been fixed.

This is the sixth version of this patch. Please do not use
the first version as it introduces a serious bug.

For reference, the original issue reported by the Cland
static analyzer is as follows:

support/fc_sort.c:494:6: warning: Potential leak of memory 
pointed to by 'head'
            malloc(sizeof(file_context_bucket_t));

Signed-off-by: Guido Trentalancia <guido@trentalancia.com>
Acked-by: William Roberts <william.c.roberts@intel.com>
---
 support/fc_sort.c |  108 ++++++++++++++++++++++++++++++++++--------------------
 1 file changed, 69 insertions(+), 39 deletions(-)

--- a/support/fc_sort.c	2017-09-30 02:44:18.137342226 +0200
+++ b/support/fc_sort.c	2017-09-30 21:16:04.899069510 +0200
@@ -46,6 +46,9 @@ typedef struct file_context_node {
 
 void file_context_node_destroy(file_context_node_t *x)
 {
+	if (!x)
+		return;
+
 	free(x->path);
 	free(x->file_type);
 	free(x->context);
@@ -135,8 +138,6 @@ file_context_node_t *fc_merge(file_conte
 	file_context_node_t *temp;
 	file_context_node_t *jumpto;
 
-
-
 	/* If a is a empty list, and b is not,
 	 *  set a as b and proceed to the end. */
 	if (!a && b)
@@ -164,7 +165,6 @@ file_context_node_t *fc_merge(file_conte
 			       fc_compare(a_current->next,
 					  b_current) != -1) {
 
-
 				temp = a_current->next;
 				a_current->next = b_current;
 				b_current = b_current->next;
@@ -177,7 +177,6 @@ file_context_node_t *fc_merge(file_conte
 			a_current = jumpto;
 		}
 
-
 		/* if there is anything left in b to be inserted,
 		   put it on the end */
 		if (b_current) {
@@ -209,11 +208,12 @@ file_context_node_t *fc_merge(file_conte
  */
 void fc_merge_sort(file_context_bucket_t *master)
 {
-
-
 	file_context_bucket_t *current;
 	file_context_bucket_t *temp;
 
+	if (!master)
+		return;
+
 	/* Loop until master is the only bucket left
 	 * so that this will stop when master contains
 	 * the sorted list. */
@@ -222,28 +222,20 @@ void fc_merge_sort(file_context_bucket_t
 
 		/* This loop merges buckets two-by-two. */
 		while (current) {
-
 			if (current->next) {
-
 				current->data =
 				    fc_merge(current->data,
 					     current->next->data);
 
-
-
 				temp = current->next;
 				current->next = current->next->next;
 
 				free(temp);
-
 			}
 
-
 			current = current->next;
 		}
 	}
-
-
 }
 
 
@@ -307,6 +299,25 @@ void fc_fill_data(file_context_node_t *f
 	}
 }
 
+
+
+/* fc_free_file_context_node_list
+ * Free the memory allocated to the linked list and its elements.
+ */
+void fc_free_file_context_node_list(struct file_context_node *node)
+{
+	struct file_context_node *next;
+
+	while (node) {
+		next = node->next;
+		file_context_node_destroy(node);
+		free(node);
+		node = next;
+	}
+}
+
+
+
 /* main
  * This program takes in two arguments, the input filename and the
  *  output filename. The input file should be syntactically correct.
@@ -328,7 +339,6 @@ int main(int argc, char *argv[])
 
 	FILE *in_file, *out_file;
 
-
 	/* Check for the correct number of command line arguments. */
 	if (argc < 2 || argc > 3) {
 		fprintf(stderr, "Usage: %s <infile> [<outfile>]\n",argv[0]);
@@ -348,24 +358,33 @@ int main(int argc, char *argv[])
 
 	/* Initialize the head of the linked list. */
 	head = current = (file_context_node_t*)malloc(sizeof(file_context_node_t));
+	if (!head) {
+		fprintf(stderr, "Error: failure allocating memory.\n");
+		return 1;
+	}
 	head->next = NULL;
+	head->path = NULL;
+	head->file_type = NULL;
+	head->context = NULL;
 
 	/* Parse the file into a file_context linked list. */
 	line_buf = NULL;
 
 	while ( getline(&line_buf, &buf_len, in_file) != -1 ){
 		line_len = strlen(line_buf);
+
 		if( line_len == 0 || line_len == 1)
 			continue;
+
 		/* Get rid of whitespace from the front of the line. */
 		for (i = 0; i < line_len; i++) {
 			if (!isspace(line_buf[i]))
 				break;
 		}
 
-
 		if (i >= line_len)
 			continue;
+
 		/* Check if the line isn't empty and isn't a comment */
 		if (line_buf[i] == '#')
 			continue;
@@ -373,7 +392,9 @@ int main(int argc, char *argv[])
 		/* We have a valid line - allocate a new node. */
 		temp = (file_context_node_t *)malloc(sizeof(file_context_node_t));
 		if (!temp) {
+			free(line_buf);
 			fprintf(stderr, "Error: failure allocating memory.\n");
+			fc_free_file_context_node_list(head);
 			return 1;
 		}
 		temp->next = NULL;
@@ -382,19 +403,15 @@ int main(int argc, char *argv[])
 		/* Parse out the regular expression from the line. */
 		start = i;
 
-
 		while (i < line_len && (!isspace(line_buf[i])))
 			i++;
 		finish = i;
 
-
 		regex_len = finish - start;
 
 		if (regex_len == 0) {
 			file_context_node_destroy(temp);
 			free(temp);
-
-
 			continue;
 		}
 
@@ -402,13 +419,14 @@ int main(int argc, char *argv[])
 		if (!temp->path) {
 			file_context_node_destroy(temp);
 			free(temp);
+			free(line_buf);
 			fprintf(stderr, "Error: failure allocating memory.\n");
+			fc_free_file_context_node_list(head);
 			return 1;
 		}
 
 		/* Get rid of whitespace after the regular expression. */
 		for (; i < line_len; i++) {
-
 			if (!isspace(line_buf[i]))
 				break;
 		}
@@ -420,18 +438,21 @@ int main(int argc, char *argv[])
 		}
 
 		/* Parse out the type from the line (if it
-			*  is there). */
+		 * is there). */
 		if (line_buf[i] == '-') {
 			temp->file_type = (char *)malloc(sizeof(char) * 3);
 			if (!(temp->file_type)) {
+				file_context_node_destroy(temp);
+				free(temp);
+				free(line_buf);
 				fprintf(stderr, "Error: failure allocating memory.\n");
+				fc_free_file_context_node_list(head);
 				return 1;
 			}
 
 			if( i + 2 >= line_len ) {
 				file_context_node_destroy(temp);
 				free(temp);
-
 				continue;
 			}
 
@@ -448,7 +469,6 @@ int main(int argc, char *argv[])
 			}
 
 			if (i == line_len) {
-
 				file_context_node_destroy(temp);
 				free(temp);
 				continue;
@@ -467,24 +487,23 @@ int main(int argc, char *argv[])
 		if (!temp->context) {
 			file_context_node_destroy(temp);
 			free(temp);
+			free(line_buf);
 			fprintf(stderr, "Error: failure allocating memory.\n");
+			fc_free_file_context_node_list(head);
 			return 1;
 		}
 
 		/* Set all the data about the regular
-			*  expression. */
+		 * expression. */
 		fc_fill_data(temp);
 
 		/* Link this line of code at the end of
-			*  the linked list. */
+		 * the linked list. */
 		current->next = temp;
 		current = current->next;
 		lines++;
-
-
-		free(line_buf);
-		line_buf = NULL;
 	}
+	free(line_buf);
 	fclose(in_file);
 
 	/* Create the bucket linked list from the earlier linked list. */
@@ -492,6 +511,12 @@ int main(int argc, char *argv[])
 	bcurrent = master =
 	    (file_context_bucket_t *)
 	    malloc(sizeof(file_context_bucket_t));
+	if (!bcurrent) {
+		printf
+		    ("Error: failure allocating memory.\n");
+		fc_free_file_context_node_list(head);
+		return -1;
+	}
 	bcurrent->next = NULL;
 	bcurrent->data = NULL;
 
@@ -512,25 +537,33 @@ int main(int argc, char *argv[])
 			if (!(bcurrent->next)) {
 				printf
 				    ("Error: failure allocating memory.\n");
-				exit(-1);
+				free(head);
+				fc_free_file_context_node_list(current);
+				fc_merge_sort(master);
+				fc_free_file_context_node_list(master->data);
+				free(master);
+				return -1;
 			}
 
 			/* Make sure the new bucket thinks it's the end of the
-			 *  list. */
+			 * list. */
 			bcurrent->next->next = NULL;
 
 			bcurrent = bcurrent->next;
 		}
-
 	}
 
 	/* Sort the bucket list. */
 	fc_merge_sort(master);
 
+	free(head);
+
 	/* Open the output file. */
 	if (output_name) {
 		if (!(out_file = fopen(output_name, "w"))) {
 			printf("Error: failure opening output file for write.\n");
+			fc_free_file_context_node_list(master->data);
+			free(master);
 			return -1;
 		}
 	} else {
@@ -539,6 +572,7 @@ int main(int argc, char *argv[])
 
 	/* Output the sorted file_context linked list to the output file. */
 	current = master->data;
+
 	while (current) {
 		/* Output the path. */
 		fprintf(out_file, "%s\t\t", current->path);
@@ -551,14 +585,10 @@ int main(int argc, char *argv[])
 		/* Output the context. */
 		fprintf(out_file, "%s\n", current->context);
 
-		/* Remove the node. */
-		temp = current;
 		current = current->next;
-
-		file_context_node_destroy(temp);
-		free(temp);
-
 	}
+
+	fc_free_file_context_node_list(master->data);
 	free(master);
 
 	if (output_name) {

^ permalink raw reply

* [Buildroot] [PATCH v3 2/4] package/freeswitch: install to staging
From: Bernd Kuhls @ 2017-10-04 21:02 UTC (permalink / raw)
  To: buildroot
In-Reply-To: <20171004210224.12763-1-bernd.kuhls@t-online.de>

Needed for upcoming package freeswitch-mod-bcg729.

Signed-off-by: Bernd Kuhls <bernd.kuhls@t-online.de>
---
v3: no changes
v2: no changes

 package/freeswitch/freeswitch.mk | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/package/freeswitch/freeswitch.mk b/package/freeswitch/freeswitch.mk
index e5c8854e36..0adb35d751 100644
--- a/package/freeswitch/freeswitch.mk
+++ b/package/freeswitch/freeswitch.mk
@@ -7,6 +7,8 @@
 FREESWITCH_VERSION = 1.6.19
 FREESWITCH_SOURCE = freeswitch-$(FREESWITCH_VERSION).tar.xz
 FREESWITCH_SITE = http://files.freeswitch.org/freeswitch-releases
+# needed for external modules
+FREESWITCH_INSTALL_STAGING = YES
 FREESWITCH_LICENSE = MPL-1.1, \
 	GPL-3.0+ with font exception (fonts), \
 	Apache-2.0 (apr, apr-util), \
-- 
2.11.0

^ permalink raw reply related

* [Buildroot] [PATCH v3 3/4] package/freeswitch-mod-bcg729: new package
From: Bernd Kuhls @ 2017-10-04 21:02 UTC (permalink / raw)
  To: buildroot
In-Reply-To: <20171004210224.12763-1-bernd.kuhls@t-online.de>

This package is based on the bcg729 library from Belledonne
Communications which is wrapped into a freeswitch module to provide a
native G729.A codec.

Signed-off-by: Bernd Kuhls <bernd.kuhls@t-online.de>
---
v3: bumped to latest version
v2: renamed package from freeswitch_mod_bcg729 to freeswitch-mod-bcg729
    (Thomas)

 DEVELOPERS                                         |  1 +
 package/Config.in                                  |  1 +
 package/freeswitch-mod-bcg729/Config.in            |  9 +++++++
 .../freeswitch-mod-bcg729.hash                     |  2 ++
 .../freeswitch-mod-bcg729/freeswitch-mod-bcg729.mk | 30 ++++++++++++++++++++++
 5 files changed, 43 insertions(+)
 create mode 100644 package/freeswitch-mod-bcg729/Config.in
 create mode 100644 package/freeswitch-mod-bcg729/freeswitch-mod-bcg729.hash
 create mode 100644 package/freeswitch-mod-bcg729/freeswitch-mod-bcg729.mk

diff --git a/DEVELOPERS b/DEVELOPERS
index 46daf6295c..fe0dd01331 100644
--- a/DEVELOPERS
+++ b/DEVELOPERS
@@ -206,6 +206,7 @@ F:	package/eudev/
 F:	package/exim/
 F:	package/fetchmail/
 F:	package/freeswitch/
+F:	package/freeswitch-mod-bcg729/
 F:	package/ffmpeg/
 F:	package/ghostscript/
 F:	package/giflib/
diff --git a/package/Config.in b/package/Config.in
index 7b99a6a510..6146de2781 100644
--- a/package/Config.in
+++ b/package/Config.in
@@ -1638,6 +1638,7 @@ menu "Networking applications"
 	source "package/fmc/Config.in"
 	source "package/fping/Config.in"
 	source "package/freeswitch/Config.in"
+	source "package/freeswitch-mod-bcg729/Config.in"
 	source "package/gesftpserver/Config.in"
 	source "package/gupnp-tools/Config.in"
 	source "package/gutenprint/Config.in"
diff --git a/package/freeswitch-mod-bcg729/Config.in b/package/freeswitch-mod-bcg729/Config.in
new file mode 100644
index 0000000000..848d4ee67e
--- /dev/null
+++ b/package/freeswitch-mod-bcg729/Config.in
@@ -0,0 +1,9 @@
+config BR2_PACKAGE_FREESWITCH_MOD_BCG729
+	bool "freeswitch-mod-bcg729"
+	depends on BR2_PACKAGE_FREESWITCH
+	select BR2_PACKAGE_LIBBCG729
+	help
+	  FreeSWITCH G.729A module using the opensource bcg729
+	  implementation by Belledonne Communications.
+
+	  https://github.com/xadhoom/mod_bcg729
diff --git a/package/freeswitch-mod-bcg729/freeswitch-mod-bcg729.hash b/package/freeswitch-mod-bcg729/freeswitch-mod-bcg729.hash
new file mode 100644
index 0000000000..35a228179c
--- /dev/null
+++ b/package/freeswitch-mod-bcg729/freeswitch-mod-bcg729.hash
@@ -0,0 +1,2 @@
+# Locally computed
+sha256 135f545ad0eef49d1228f8d66a4e66f1ff5f1f8fdf115e88c1df3df8df49fdcb  freeswitch-mod-bcg729-c504eea91f225014380ae17c00b35e7173e316ad.tar.gz
diff --git a/package/freeswitch-mod-bcg729/freeswitch-mod-bcg729.mk b/package/freeswitch-mod-bcg729/freeswitch-mod-bcg729.mk
new file mode 100644
index 0000000000..c83fc50446
--- /dev/null
+++ b/package/freeswitch-mod-bcg729/freeswitch-mod-bcg729.mk
@@ -0,0 +1,30 @@
+################################################################################
+#
+# freeswitch-mod-bcg729
+#
+################################################################################
+
+FREESWITCH_MOD_BCG729_VERSION = c504eea91f225014380ae17c00b35e7173e316ad
+FREESWITCH_MOD_BCG729_SITE = $(call github,xadhoom,mod_bcg729,$(FREESWITCH_MOD_BCG729_VERSION))
+FREESWITCH_MOD_BCG729_LICENSE = MPL-1.1
+FREESWITCH_MOD_BCG729_LICENSE_FILES = LICENSE
+FREESWITCH_MOD_BCG729_DEPENDENCIES = freeswitch libbcg729
+
+# instead of patching the not cross-compile friendly Makefile from
+# upstream we issue the necessary build commands ourselves
+define FREESWITCH_MOD_BCG729_BUILD_CMDS
+	$(TARGET_CC) $(TARGET_CFLAGS) $(TARGET_LDFLAGS) \
+		-I$(STAGING_DIR)/usr/include/freeswitch \
+		-fPIC -fomit-frame-pointer -fno-exceptions \
+		-c $(@D)/mod_bcg729.c -o $(@D)/mod_bcg729.o
+	$(TARGET_CC) $(TARGET_CFLAGS) $(TARGET_LDFLAGS) \
+		-fPIC -fomit-frame-pointer -fno-exceptions \
+		-shared -Xlinker -x -lm -lbcg729 -Wl,-Bdynamic \
+		-o $(@D)/mod_bcg729.so $(@D)/mod_bcg729.o
+endef
+
+define FREESWITCH_MOD_BCG729_INSTALL_TARGET_CMDS
+	$(INSTALL) -m 0755 $(@D)/mod_bcg729.so $(TARGET_DIR)/usr/lib/freeswitch/mod/mod_bcg729.so
+endef
+
+$(eval $(generic-package))
-- 
2.11.0

^ permalink raw reply related

* [Buildroot] [PATCH v3 4/4] package/freeswitch: Use mod_g729 only when freeswitch-mod-bcg729 is disabled
From: Bernd Kuhls @ 2017-10-04 21:02 UTC (permalink / raw)
  To: buildroot
In-Reply-To: <20171004210224.12763-1-bernd.kuhls@t-online.de>

mod_g729 contained in freeswitch source provides pass-through:
https://freeswitch.org/confluence/display/FREESWITCH/mod_g729

Signed-off-by: Bernd Kuhls <bernd.kuhls@t-online.de>
---
v3: no changes
v2: no changes

 package/freeswitch/freeswitch.mk | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/package/freeswitch/freeswitch.mk b/package/freeswitch/freeswitch.mk
index 0adb35d751..6c1dc7c012 100644
--- a/package/freeswitch/freeswitch.mk
+++ b/package/freeswitch/freeswitch.mk
@@ -116,7 +116,6 @@ FREESWITCH_ENABLED_MODULES += \
 	applications/mod_valet_parking \
 	applications/mod_voicemail \
 	codecs/mod_g723_1 \
-	codecs/mod_g729 \
 	dialplans/mod_dialplan_asterisk \
 	dialplans/mod_dialplan_xml \
 	endpoints/mod_loopback \
@@ -175,6 +174,10 @@ FREESWITCH_DEPENDENCIES += alsa-lib
 FREESWITCH_ENABLED_MODULES += endpoints/mod_alsa
 endif
 
+ifeq ($(BR2_PACKAGE_FREESWITCH_MOD_BCG729),)
+FREESWITCH_ENABLED_MODULES += codecs/mod_g729
+endif
+
 ifeq ($(BR2_PACKAGE_FREETYPE),y)
 FREESWITCH_DEPENDENCIES += freetype
 endif
-- 
2.11.0

^ permalink raw reply related

* Re: [Qemu-devel] [PATCH v2 2/3] qom: introduce type_register_static_array()
From: Eduardo Habkost @ 2017-10-04 21:03 UTC (permalink / raw)
  To: Igor Mammedov; +Cc: qemu-devel, peter.maydell
In-Reply-To: <1507111682-66171-3-git-send-email-imammedo@redhat.com>

On Wed, Oct 04, 2017 at 12:08:01PM +0200, Igor Mammedov wrote:
> it will help to remove code duplication of registration
> static types in places that have open coded loop to
> perform batch type registering.
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>

Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>

I will wait for 1 day or 2 before queueing it.

-- 
Eduardo

^ permalink raw reply

* Re: system hung up when offlining CPUs
From: Thomas Gleixner @ 2017-10-04 21:04 UTC (permalink / raw)
  To: YASUAKI ISHIMATSU
  Cc: Kashyap Desai, Hannes Reinecke, Marc Zyngier, Christoph Hellwig,
	axboe, mpe, keith.busch, peterz, LKML, linux-scsi, Sumit Saxena,
	Shivasharan Srikanteshwara
In-Reply-To: <alpine.DEB.2.20.1710032328280.2278@nanos>

On Tue, 3 Oct 2017, Thomas Gleixner wrote:
> Can you please apply the debug patch below.

I found an issue with managed interrupts when the affinity mask of an
managed interrupt spawns multiple CPUs. Explanation in the changelog
below. I'm not sure that this cures the problems you have, but at least I
could prove that it's not doing what it should do. The failure I'm seing is
fixed, but I can't test that megasas driver due to -ENOHARDWARE.

Can you please apply the patch below on top of Linus tree and retest?

Please send me the outputs I asked you to provide last time in any case
(success or fail).

@block/scsi folks: Can you please run that through your tests as well?

Thanks,

	tglx

8<-----------------------
Subject: genirq/cpuhotplug: Enforce affinity setting on startup of managed irqs
From: Thomas Gleixner <tglx@linutronix.de>
Date: Wed, 04 Oct 2017 21:07:38 +0200

Managed interrupts can end up in a stale state on CPU hotplug. If the
interrupt is not targeting a single CPU, i.e. the affinity mask spawns
multiple CPUs then the following can happen:

After boot:

dstate:   0x01601200
            IRQD_ACTIVATED
            IRQD_IRQ_STARTED
            IRQD_SINGLE_TARGET
            IRQD_AFFINITY_SET
            IRQD_AFFINITY_MANAGED
node:     0
affinity: 24-31
effectiv: 24
pending:  0

After offlining CPU 31 - 24

dstate:   0x01a31000
            IRQD_IRQ_DISABLED
            IRQD_IRQ_MASKED
            IRQD_SINGLE_TARGET
            IRQD_AFFINITY_SET
            IRQD_AFFINITY_MANAGED
            IRQD_MANAGED_SHUTDOWN
node:     0
affinity: 24-31
effectiv: 24
pending:  0

Now CPU 25 gets onlined again, so it should get the effective interrupt
affinity for this interruopt, but due to the x86 interrupt affinity setter
restrictions this ends up after restarting the interrupt with:

dstate:   0x01601300
            IRQD_ACTIVATED
            IRQD_IRQ_STARTED
            IRQD_SINGLE_TARGET
            IRQD_AFFINITY_SET
            IRQD_SETAFFINITY_PENDING
            IRQD_AFFINITY_MANAGED
node:     0
affinity: 24-31
effectiv: 24
pending:  24-31

So the interrupt is still affine to CPU 24, which was the last CPU to go
offline of that affinity set and the move to an online CPU within 24-31,
in this case 25, is pending. This mechanism is x86/ia64 specific as those
architectures cannot move interrupts from thread context and do this when
an interrupt is actually handled. So the move is set to pending.

Whats worse is that offlining CPU 25 again results in:

dstate:   0x01601300
            IRQD_ACTIVATED
            IRQD_IRQ_STARTED
            IRQD_SINGLE_TARGET
            IRQD_AFFINITY_SET
            IRQD_SETAFFINITY_PENDING
            IRQD_AFFINITY_MANAGED
node:     0
affinity: 24-31
effectiv: 24
pending:  24-31

This means the interrupt has not been shut down, because the outgoing CPU
is not in the effective affinity mask, but of course nothing notices that
the effective affinity mask is pointing at an offline CPU.

In the case of restarting a managed interrupt the move restriction does not
apply, so the affinity setting can be made unconditional. This needs to be
done _before_ the interrupt is started up as otherwise the condition for
moving it from thread context would not longer be fulfilled.

With that change applied onlining CPU 25 after offlining 31-24 results in:

dstate:   0x01600200
            IRQD_ACTIVATED
            IRQD_IRQ_STARTED
            IRQD_SINGLE_TARGET
            IRQD_AFFINITY_MANAGED
node:     0
affinity: 24-31
effectiv: 25
pending:  

And after offlining CPU 25:

dstate:   0x01a30000
            IRQD_IRQ_DISABLED
            IRQD_IRQ_MASKED
            IRQD_SINGLE_TARGET
            IRQD_AFFINITY_MANAGED
            IRQD_MANAGED_SHUTDOWN
node:     0
affinity: 24-31
effectiv: 25
pending:  

which is the correct and expected result.

To complete that, add some debug code to catch this kind of situation in
the cpu offline code and warn about interrupt chips which allow affinity
setting and do not update the effective affinity mask if that feature is
enabled.

Reported-by: YASUAKI ISHIMATSU <yasu.isimatu@gmail.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

---
 kernel/irq/chip.c       |    2 +-
 kernel/irq/cpuhotplug.c |   28 +++++++++++++++++++++++++++-
 kernel/irq/manage.c     |   17 +++++++++++++++++
 3 files changed, 45 insertions(+), 2 deletions(-)

--- a/kernel/irq/chip.c
+++ b/kernel/irq/chip.c
@@ -265,8 +265,8 @@ int irq_startup(struct irq_desc *desc, b
 			irq_setup_affinity(desc);
 			break;
 		case IRQ_STARTUP_MANAGED:
+			irq_do_set_affinity(d, aff, false);
 			ret = __irq_startup(desc);
-			irq_set_affinity_locked(d, aff, false);
 			break;
 		case IRQ_STARTUP_ABORT:
 			return 0;
--- a/kernel/irq/cpuhotplug.c
+++ b/kernel/irq/cpuhotplug.c
@@ -18,8 +18,34 @@
 static inline bool irq_needs_fixup(struct irq_data *d)
 {
 	const struct cpumask *m = irq_data_get_effective_affinity_mask(d);
+	unsigned int cpu = smp_processor_id();
 
-	return cpumask_test_cpu(smp_processor_id(), m);
+#ifdef CONFIG_GENERIC_IRQ_EFFECTIVE_AFF_MASK
+	/*
+	 * The cpumask_empty() check is a workaround for interrupt chips,
+	 * which do not implement effective affinity, but the architecture has
+	 * enabled the config switch. Use the general affinity mask instead.
+	 */
+	if (cpumask_empty(m))
+		m = irq_data_get_affinity_mask(d);
+
+	/*
+	 * Sanity check. If the mask is not empty when excluding the outgoing
+	 * CPU then it must contain at least one online CPU. The outgoing CPU
+	 * has been removed from the online mask already.
+	 */
+	if (cpumask_any_but(m, cpu) < nr_cpu_ids &&
+	    cpumask_any_and(m, cpu_online_mask) >= nr_cpu_ids) {
+		/*
+		 * If this happens then there was a missed IRQ fixup at some
+		 * point. Warn about it and enforce fixup.
+		 */
+		pr_warn("Eff. affinity %*pbl of IRQ %u contains only offline CPUs after offlining CPU %u\n",
+			cpumask_pr_args(m), d->irq, cpu);
+		return true;
+	}
+#endif
+	return cpumask_test_cpu(cpu, m);
 }
 
 static bool migrate_one_irq(struct irq_desc *desc)
--- a/kernel/irq/manage.c
+++ b/kernel/irq/manage.c
@@ -168,6 +168,19 @@ void irq_set_thread_affinity(struct irq_
 			set_bit(IRQTF_AFFINITY, &action->thread_flags);
 }
 
+static void irq_validate_effective_affinity(struct irq_data *data)
+{
+#ifdef CONFIG_GENERIC_IRQ_EFFECTIVE_AFF_MASK
+	const struct cpumask *m = irq_data_get_effective_affinity_mask(data);
+	struct irq_chip *chip = irq_data_get_irq_chip(data);
+
+	if (!cpumask_empty(m))
+		return;
+	pr_warn_once("irq_chip %s did not update eff. affinity mask of irq %u\n",
+		     chip->name, data->irq);
+#endif
+}
+
 int irq_do_set_affinity(struct irq_data *data, const struct cpumask *mask,
 			bool force)
 {
@@ -175,12 +188,16 @@ int irq_do_set_affinity(struct irq_data
 	struct irq_chip *chip = irq_data_get_irq_chip(data);
 	int ret;
 
+	if (!chip || !chip->irq_set_affinity)
+		return -EINVAL;
+
 	ret = chip->irq_set_affinity(data, mask, force);
 	switch (ret) {
 	case IRQ_SET_MASK_OK:
 	case IRQ_SET_MASK_OK_DONE:
 		cpumask_copy(desc->irq_common_data.affinity, mask);
 	case IRQ_SET_MASK_OK_NOCOPY:
+		irq_validate_effective_affinity(data);
 		irq_set_thread_affinity(desc);
 		ret = 0;
 	}

^ permalink raw reply

* Re: [Qemu-devel] [PATCH v2 3/3] qom: add helper macro DEFINE_TYPES()
From: Eduardo Habkost @ 2017-10-04 21:04 UTC (permalink / raw)
  To: Igor Mammedov; +Cc: qemu-devel, peter.maydell
In-Reply-To: <1507111682-66171-4-git-send-email-imammedo@redhat.com>

On Wed, Oct 04, 2017 at 12:08:02PM +0200, Igor Mammedov wrote:
> DEFINE_TYPES() will help to simplify following routine patterns:
> 
>  static void foo_register_types(void)
>  {
>     type_register_static(&foo1_type_info);
>     type_register_static(&foo2_type_info);
>     ...
>  }
> 
>  type_init(foo_register_types)
> 
> or
> 
>  static void foo_register_types(void)
>  {
>     int i;
> 
>     for (i = 0; i < ARRAY_SIZE(type_infos); i++) {
>         type_register_static(&type_infos[i]);
>     }
>  }
> 
>  type_init(foo_register_types)
> 
> with a single line
> 
>  DEFINE_TYPES(type_infos)
> 
> where types have static definition which could be consolidated in
> a single array of TypeInfo structures.
> It saves us ~6-10LOC per use case and would help to replace
> imperative foo_register_types() there with declarative style of
> type registration.
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>

Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>

I will wait for 1 day or 2 before queueing it.

-- 
Eduardo

^ permalink raw reply

* Re: [Qemu-devel] [Qemu-block] [PATCH RFC] block: add block-insert-node QMP command
From: Manos Pitsidianakis @ 2017-10-04 21:04 UTC (permalink / raw)
  To: Max Reitz; +Cc: qemu-devel, Kevin Wolf, Stefan Hajnoczi, qemu-block
In-Reply-To: <704ce78f-f344-4e49-6e4d-a53b7c868ac2@redhat.com>

[-- Attachment #1: Type: text/plain, Size: 6517 bytes --]

On Wed, Oct 04, 2017 at 08:09:24PM +0200, Max Reitz wrote:
>On 2017-10-04 19:05, Manos Pitsidianakis wrote:
>> On Wed, Oct 04, 2017 at 02:49:27PM +0200, Max Reitz wrote:
>>> On 2017-08-15 09:45, Manos Pitsidianakis wrote:
>>>> block-insert-node and its pair command block-remove-node provide runtime
>>>> insertion and removal of filter nodes.
>>>>
>>>> block-insert-node takes a (parent, child) and (node, child) pair of
>>>> edges and unrefs the (parent, child) BdrvChild relationship and creates
>>>> a new (parent, node) child with the same BdrvChildRole.
>>>>
>>>> This is a different approach than x-blockdev-change which uses the
>>>> driver
>>>> methods bdrv_add_child() and bdrv_del_child(),
>>>
>>> Why? :-)
>>>
>>> Can't we reuse x-blockdev-change? As far as I'm concerned, this was one
>>> of its roles, and at least I don't want to have both x-blockdev-change
>>> and these new commands.
>>>
>>> I think it would be a good idea just to change bdrv_add_child() and
>>> bdrv_del_child(): If the driver has a bdrv_{add,del}_child() callback,
>>> invoke that.  If it doesn't, then just attach the child.
>>>
>>> Of course, it may turn out that x-blockdev-change is lacking something
>>> (e.g. a way to specify as what kind of child a new node is to be
>>> attached).  Then we should either extend it so that it covers what it's
>>> lacking, or replace it completely by these new commands.  In the latter
>>> case, however, they would need to cover the existing use case which is
>>> reconfiguring the quorum driver.  (And that would mean it would have to
>>> invoke bdrv_add_child()/bdrv_del_child() when the driver has them.)
>>>
>>> Max
>>>
>>
>> I think the two use cases are this:
>>
>> a) Adding/removing a replication child to an existing quorum node
>> b) Insert a filter between two existing nodes.
>
>For me both are the same: Adding/removing nodes into/from the graph.
>
>The difference is how those children are added (or removed, but it's the
>same in reverse): In case of quorum, you can simply attach a new child
>because it supports a variable number of children.  Another case where
>this would work are all block drivers which support backing files (you
>can freely attach/detach those).

Doesn't blockdev-snapshot-sync cover this? (I may be missing something).
Now that we're on this topic, quorum might be a good candidate for 
*_reopen when and if it lands on QMP: Reconfiguring the children could 
be done by reopening the BDS with new options.
>
>In this series, it's not about adding or removing new children, but
>instead "injecting" them into an edge: An existing child is replaced,
>but it now serves as some child of the new one.
>
>(I guess writing too much trying to get my point across, sorry...)
>
>The gist is that for me it's not so much about "quorum" or "filter
>nodes".  It's about adding a new child to a node vs. replacing an
>existing one.
>
>> These are not directly compatible semantically, but as you said
>> x-blockdev-change can perform b) if bdrv_add_child()/bdrv_del_child()
>> are not implemented. Wouldn't that be unnecessary overloading?
>
>Yes, I think it would be. :-)
>
>So say we have these two trees in our graph:
>
>[ Parent BDS -> Child BDS ]
>[ Filter BDS -> Child BDS ]
>
>So here's what you can do with x-blockdev-change (in theory; in practice
>it can only do that for quorum):
>- Remove a child, so
>    [ Parent BDS -> Child BDS ]
>  is split into two trees
>    [ Parent BDS ] and
>    [ Child BDS ]
>- Add a child, so we can merge
>    [ Parent BDS ] and
>    [ Filter BDS -> Child BDS ]
>  into
>    [ Parent BDS -> Filter BDS -> Child BDS ]
>

Yes, of course this would have to be done in one transaction.

>However, this is only possible with quorum because usually block drivers
>don't support detaching children.
>
>And here's what you can do with your commands (from what I can see):
>- Replace a child (you call it insertion, but it really is just
>  replacement of an existing child with the constraint that both nodes
>  involved must have the same child):
>    [ Parent BDS -> Child BDS ]
>    [ Filter BDS -> Child BDS ]
>  to
>    [ Parent BDS -> Filter BDS -> Child BDS ]
>- Replace a child (you call it removal, but it really is just
>  replacement of a child with its child):
>    [ Parent BDS -> Filter BDS -> Child BDS ]
>  to
>    [ Parent BDS -> Child BDS ]
>
>This works on all BDSs because you don't change the number of children.
>
>
>The interesting thing of course is that the "change" command can
>actually add and remove children; where as the "insert" and "remove"
>commands can only replace children.  So that's already a bit funny (one
>command does two things; two commands do one thing).

That is true, but the replacing is more in terms of inserting and 
removing a node in a BDS chain.
>
>And then of course you can simply modify x-blockdev-change so it can do
>the same thing block-insert-node and block-remove-node can do: It just
>needs another mode which can be used to replace a child (and its
>description already states that it is supposed to be usable for that at
>some point in the future).
>
>
>So after I've written all of this, I feel like the new insert-node and
>remove-node commands are exactly what x-blockdev-change should do when
>asked to replace a node.  The only difference is that x-blockdev-change
>would allow you to replace any node with anything, without the
>constraints that block-insert-node and block-remove-node exact.
>
>(And node replacement with x-blockdev-change would work by specifying
>all three parameters.)
>
>Not sure if that makes sense, I hope it does. :-)
>

Hm, I can't think of a way to fit that into x-blockdev-change *and* keep 
the bdrv_add_child/bdrv_del_child functionality into consideration 
(since we'd have to keep both). This is what the current doco is:

  If @node is specified, it will be inserted under @parent. @child
  may not be specified in this case. If both @parent and @child are
  specified but @node is not, @child will be detached from @parent.

The simplest thing would be to add a flag as to what operation you want 
to perform (add/del child versus filter insertion/removal from edges) 
but this is what I was thinking about overloading it, it feels 
convoluted yet the insert and remove commands felt intuitive enough to 
me after experimenting with it a little. What do you think?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* [Bug 103080] DC driver from drm-next-4.15-dc branch (upstream PR) not working
From: bugzilla-daemon @ 2017-10-04 21:05 UTC (permalink / raw)
  To: dri-devel
In-Reply-To: <bug-103080-502@http.bugs.freedesktop.org/>


[-- Attachment #1.1: Type: text/plain, Size: 295 bytes --]

https://bugs.freedesktop.org/show_bug.cgi?id=103080

--- Comment #3 from Harry Wentland <harry.wentland@amd.com> ---
We've got a fix incoming in a day or two for that kernel oops with the next set
of DC patches.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[-- Attachment #1.2: Type: text/html, Size: 1113 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply

* Re: [lockdep] b09be676e0 BUG: unable to handle kernel NULL pointer dereference at 000001f2
From: Josh Poimboeuf @ 2017-10-04 21:06 UTC (permalink / raw)
  To: lkp
In-Reply-To: <201710040644.GJE89219.FOHMFSFOVtOQLJ@I-love.SAKURA.ne.jp>

[-- Attachment #1: Type: text/plain, Size: 2976 bytes --]

On Wed, Oct 04, 2017 at 06:44:50AM +0900, Tetsuo Handa wrote:
> Josh Poimboeuf wrote:
> > On Tue, Oct 03, 2017 at 11:28:15AM -0500, Josh Poimboeuf wrote:
> > > There are two bugs:
> > > 
> > > 1) Somebody -- presumably lockdep -- is corrupting the stack.  Need the
> > >    lockdep people to look at that.
> > > 
> > > 2) The 32-bit FP unwinder isn't handling the corrupt stack very well,
> > >    It's blindly dereferencing untrusted data:
> > > 
> > > 	/* Is the next frame pointer an encoded pointer to pt_regs? */
> > > 	regs = decode_frame_pointer(next_bp);
> > > 	if (regs) {
> > > 		frame = (unsigned long *)regs;
> > > 		len = regs_size(regs);
> > > 		state->got_irq = true;
> > > 
> > >   On 32-bit, regs_size() dereferences the regs pointer before we know it
> > >   points to a valid stack.  I'll fix that, along with the other unwinder
> > >   improvements I discussed with Linus.
> > 
> > Tetsuo and/or Fengguang,
> > 
> > Would you mind testing with this patch?  It should at least prevent the
> > unwinder panic and should hopefully print a useful unwinder dump
> > instead.
> > 
> Here are two outputs.

Thanks, both outputs were helpful.

This is another unwinder-related issue, unrelated to lockdep this time.

I compiled the same kernel with a similar version of GCC.  It turns out
that GCC *does* create unaligned stacks with frame pointers enabled:

  c124a388 <acpi_rs_move_data>:
  c124a388:       55                      push   %ebp
  c124a389:       89 e5                   mov    %esp,%ebp
  c124a38b:       57                      push   %edi
  c124a38c:       56                      push   %esi
  c124a38d:       89 d6                   mov    %edx,%esi
  c124a38f:       53                      push   %ebx
  c124a390:       31 db                   xor    %ebx,%ebx
  c124a392:       83 ec 03                sub    $0x3,%esp
  ...
  c124a3e3:       83 c4 03                add    $0x3,%esp
  c124a3e6:       5b                      pop    %ebx
  c124a3e7:       5e                      pop    %esi
  c124a3e8:       5f                      pop    %edi
  c124a3e9:       5d                      pop    %ebp
  c124a3ea:       c3                      ret

This was a leaf function.  For no apparent reason, GCC 4.8 decided to
subtract 3 from the stack pointer in the prologue.

Then in the middle of the function, it got an interrupt.  On 64-bit,
interrupts always align the stack, but on 32-bit they don't.  So the
pt_regs were stored at an unaligned address (0xf60bbb25).

The frame pointer encoding logic assumes an aligned stack pointer, so it
didn't work as expected.

  .macro ENCODE_FRAME_POINTER
  #ifdef CONFIG_FRAME_POINTER
  	mov %esp, %ebp
  	orl $0x1, %ebp
  #endif
  .endm

That effectively cleared the LSB of the encoded pt_regs address,
confusing the unwinder.

So on 32-bit, maybe the encoding should clear the MSB instead of setting
the LSB.

-- 
Josh

^ permalink raw reply

* Re: [lockdep] b09be676e0 BUG: unable to handle kernel NULL pointer dereference at 000001f2
From: Josh Poimboeuf @ 2017-10-04 21:06 UTC (permalink / raw)
  To: Tetsuo Handa
  Cc: fengguang.wu, byungchul.park, mingo, peterz, linux-kernel, lkp,
	torvalds, bp, x86, hpa, tglx
In-Reply-To: <201710040644.GJE89219.FOHMFSFOVtOQLJ@I-love.SAKURA.ne.jp>

On Wed, Oct 04, 2017 at 06:44:50AM +0900, Tetsuo Handa wrote:
> Josh Poimboeuf wrote:
> > On Tue, Oct 03, 2017 at 11:28:15AM -0500, Josh Poimboeuf wrote:
> > > There are two bugs:
> > > 
> > > 1) Somebody -- presumably lockdep -- is corrupting the stack.  Need the
> > >    lockdep people to look at that.
> > > 
> > > 2) The 32-bit FP unwinder isn't handling the corrupt stack very well,
> > >    It's blindly dereferencing untrusted data:
> > > 
> > > 	/* Is the next frame pointer an encoded pointer to pt_regs? */
> > > 	regs = decode_frame_pointer(next_bp);
> > > 	if (regs) {
> > > 		frame = (unsigned long *)regs;
> > > 		len = regs_size(regs);
> > > 		state->got_irq = true;
> > > 
> > >   On 32-bit, regs_size() dereferences the regs pointer before we know it
> > >   points to a valid stack.  I'll fix that, along with the other unwinder
> > >   improvements I discussed with Linus.
> > 
> > Tetsuo and/or Fengguang,
> > 
> > Would you mind testing with this patch?  It should at least prevent the
> > unwinder panic and should hopefully print a useful unwinder dump
> > instead.
> > 
> Here are two outputs.

Thanks, both outputs were helpful.

This is another unwinder-related issue, unrelated to lockdep this time.

I compiled the same kernel with a similar version of GCC.  It turns out
that GCC *does* create unaligned stacks with frame pointers enabled:

  c124a388 <acpi_rs_move_data>:
  c124a388:       55                      push   %ebp
  c124a389:       89 e5                   mov    %esp,%ebp
  c124a38b:       57                      push   %edi
  c124a38c:       56                      push   %esi
  c124a38d:       89 d6                   mov    %edx,%esi
  c124a38f:       53                      push   %ebx
  c124a390:       31 db                   xor    %ebx,%ebx
  c124a392:       83 ec 03                sub    $0x3,%esp
  ...
  c124a3e3:       83 c4 03                add    $0x3,%esp
  c124a3e6:       5b                      pop    %ebx
  c124a3e7:       5e                      pop    %esi
  c124a3e8:       5f                      pop    %edi
  c124a3e9:       5d                      pop    %ebp
  c124a3ea:       c3                      ret

This was a leaf function.  For no apparent reason, GCC 4.8 decided to
subtract 3 from the stack pointer in the prologue.

Then in the middle of the function, it got an interrupt.  On 64-bit,
interrupts always align the stack, but on 32-bit they don't.  So the
pt_regs were stored at an unaligned address (0xf60bbb25).

The frame pointer encoding logic assumes an aligned stack pointer, so it
didn't work as expected.

  .macro ENCODE_FRAME_POINTER
  #ifdef CONFIG_FRAME_POINTER
  	mov %esp, %ebp
  	orl $0x1, %ebp
  #endif
  .endm

That effectively cleared the LSB of the encoded pt_regs address,
confusing the unwinder.

So on 32-bit, maybe the encoding should clear the MSB instead of setting
the LSB.

-- 
Josh

^ permalink raw reply


This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.