On Wed, Jun 7, 2017 at 11:48 AM, Chen-Yu Tsai wens@csie.org wrote:

On Wed, Jun 7, 2017 at 8:47 AM, Icenowy Zheng icenowy@aosc.io wrote:

...

A83T SoC has two part of CPUCFG configurations -- one part is at 0x01700000, which contains most of the controls, and is like the one in A80; the another part is at 0x01f01c00 (like other post-sun6i SoCs), but contains now only a few registers.

Call it SUNXI_R_CPUCFG_BASE, like what the BSP Linux source code did.

Signed-off-by: Icenowy Zheng icenowy@aosc.io

arch/arm/include/asm/arch-sunxi/cpu_sun4i.h | 2 ++ 1 file changed, 2 insertions(+)

diff --git a/arch/arm/include/asm/arch-sunxi/cpu_sun4i.h b/arch/arm/include/asm/arch-sunxi/cpu_sun4i.h index a96680d8e8..5c74714084 100644 --- a/arch/arm/include/asm/arch-sunxi/cpu_sun4i.h +++ b/arch/arm/include/asm/arch-sunxi/cpu_sun4i.h @@ -179,6 +179,8 @@ defined(CONFIG_MACH_SUN50I) !defined CONFIG_MACH_SUN8I_A83T && \ !defined CONFIG_MACH_SUN8I_R40 #define SUNXI_CPUCFG_BASE 0x01f01c00 +#elif defined CONFIG_MACH_SUN8I_A83T +#define SUNXI_R_CPUCFG_BASE 0x01f01c00 #endif

Since this is unrelated to CPUCFG (which we might use later on), Just put it in a separate #ifdef section, instead of with the other CPUCFG ones.

Also this looks like you don't define CPUCFG, which you later use in the implementation? Can you elaborate on that?

ChenYu

On Wed, Jun 7, 2017 at 8:47 AM, Icenowy Zheng icenowy@aosc.io wrote:

From: Chen-Yu Tsai wens@csie.org

The A80/A83T SoCs has a different CPUCFG register layout, likely due to having 2 clusters. The A83T SoC has also a small extra CPUCFG part located at single cluster SoCs' CPUCFG address (in CPUs domain).

Add a cpucfg header file for it, rename the original cpucfg.h to cpucfg_sun4i.h and add a new cpucfg.h to automatically switch between the two cpucfg header file.

Signed-off-by: Chen-Yu Tsai wens@csie.org Signed-off-by: Icenowy Zheng icenowy@aosc.io

---

arch/arm/include/asm/arch-sunxi/cpucfg.h | 64 +++--------------------- arch/arm/include/asm/arch-sunxi/cpucfg_sun4i.h | 68 ++++++++++++++++++++++++++ arch/arm/include/asm/arch-sunxi/cpucfg_sun9i.h | 60 +++++++++++++++++++++++ 3 files changed, 134 insertions(+), 58 deletions(-) create mode 100644 arch/arm/include/asm/arch-sunxi/cpucfg_sun4i.h create mode 100644 arch/arm/include/asm/arch-sunxi/cpucfg_sun9i.h

diff --git a/arch/arm/include/asm/arch-sunxi/cpucfg.h b/arch/arm/include/asm/arch-sunxi/cpucfg.h index 297cdd28c0..cf60ff81b6 100644 --- a/arch/arm/include/asm/arch-sunxi/cpucfg.h +++ b/arch/arm/include/asm/arch-sunxi/cpucfg.h @@ -1,7 +1,5 @@ /*

• • Sunxi A31 CPUCFG register definition.
• • (C) Copyright 2014 Hans de Goede <hdegoede@redhat.com

• • (C) Copyright 2017 Icenowy Zheng icenowy@aosc.io
  • SPDX-License-Identifier: GPL-2.0+
  */

@@ -9,60 +7,10 @@ #ifndef _SUNXI_CPUCFG_H #define _SUNXI_CPUCFG_H

-#include <linux/compiler.h> -#include <linux/types.h>

-#ifndef __ASSEMBLY__

-struct __packed sunxi_cpucfg_cpu {

•   u32 rst;                /* base + 0x0 */
  

•   u32 ctrl;               /* base + 0x4 */
  

•   u32 status;             /* base + 0x8 */
  

•   u8 res[0x34];           /* base + 0xc */
  

-};

-struct __packed sunxi_cpucfg_reg {

•   u8 res0[0x40];          /* 0x000 */
  

•   struct sunxi_cpucfg_cpu cpu[4];         /* 0x040 */
  

•   u8 res1[0x44];          /* 0x140 */
  

•   u32 gen_ctrl;           /* 0x184 */
  

•   u32 l2_status;          /* 0x188 */
  

•   u8 res2[0x4];           /* 0x18c */
  

•   u32 event_in;           /* 0x190 */
  

•   u8 res3[0xc];           /* 0x194 */
  

•   u32 super_standy_flag;  /* 0x1a0 */
  

•   u32 priv0;              /* 0x1a4 */
  

•   u32 priv1;              /* 0x1a8 */
  

•   u8 res4[0x4];           /* 0x1ac */
  

•   u32 cpu1_pwr_clamp;     /* 0x1b0 sun7i only */
  

•   u32 cpu1_pwroff;        /* 0x1b4 sun7i only */
  

•   u8 res5[0x2c];          /* 0x1b8 */
  

•   u32 dbg_ctrl1;          /* 0x1e4 */
  

•   u8 res6[0x18];          /* 0x1e8 */
  

•   u32 idle_cnt0_low;      /* 0x200 */
  

•   u32 idle_cnt0_high;     /* 0x204 */
  

•   u32 idle_cnt0_ctrl;     /* 0x208 */
  

•   u8 res8[0x4];           /* 0x20c */
  

•   u32 idle_cnt1_low;      /* 0x210 */
  

•   u32 idle_cnt1_high;     /* 0x214 */
  

•   u32 idle_cnt1_ctrl;     /* 0x218 */
  

•   u8 res9[0x4];           /* 0x21c */
  

•   u32 idle_cnt2_low;      /* 0x220 */
  

•   u32 idle_cnt2_high;     /* 0x224 */
  

•   u32 idle_cnt2_ctrl;     /* 0x228 */
  

•   u8 res10[0x4];          /* 0x22c */
  

•   u32 idle_cnt3_low;      /* 0x230 */
  

•   u32 idle_cnt3_high;     /* 0x234 */
  

•   u32 idle_cnt3_ctrl;     /* 0x238 */
  

•   u8 res11[0x4];          /* 0x23c */
  

•   u32 idle_cnt4_low;      /* 0x240 */
  

•   u32 idle_cnt4_high;     /* 0x244 */
  

•   u32 idle_cnt4_ctrl;     /* 0x248 */
  

•   u8 res12[0x34];         /* 0x24c */
  

•   u32 cnt64_ctrl;         /* 0x280 */
  

•   u32 cnt64_low;          /* 0x284 */
  

•   u32 cnt64_high;         /* 0x288 */
  

-}; +#if defined(CONFIG_MACH_SUN8I_A83T) || defined(CONFIG_MACH_SUN9I) +#include <asm/arch/cpucfg_sun9i.h> +#else +#include <asm/arch/cpucfg_sun4i.h> +#endif

-#endif /* __ASSEMBLY__ */ #endif /* _SUNXI_CPUCFG_H */ diff --git a/arch/arm/include/asm/arch-sunxi/cpucfg_sun4i.h b/arch/arm/include/asm/arch-sunxi/cpucfg_sun4i.h new file mode 100644 index 0000000000..af1a1d56c9 --- /dev/null +++ b/arch/arm/include/asm/arch-sunxi/cpucfg_sun4i.h @@ -0,0 +1,68 @@ +/*

• • Sunxi A31 CPUCFG register definition.
• • (C) Copyright 2014 Hans de Goede <hdegoede@redhat.com
• • SPDX-License-Identifier: GPL-2.0+
• */

+#ifndef _SUNXI_CPUCFG_SUN4I_H +#define _SUNXI_CPUCFG_SUN4I_H

+#include <linux/compiler.h> +#include <linux/types.h>

+#ifndef __ASSEMBLY__

+struct __packed sunxi_cpucfg_cpu {

•   u32 rst;                /* base + 0x0 */
  

•   u32 ctrl;               /* base + 0x4 */
  

•   u32 status;             /* base + 0x8 */
  

•   u8 res[0x34];           /* base + 0xc */
  

+};

+struct __packed sunxi_cpucfg_reg {

•   u8 res0[0x40];          /* 0x000 */
  

•   struct sunxi_cpucfg_cpu cpu[4];         /* 0x040 */
  

•   u8 res1[0x44];          /* 0x140 */
  

•   u32 gen_ctrl;           /* 0x184 */
  

•   u32 l2_status;          /* 0x188 */
  

•   u8 res2[0x4];           /* 0x18c */
  

•   u32 event_in;           /* 0x190 */
  

•   u8 res3[0xc];           /* 0x194 */
  

•   u32 super_standy_flag;  /* 0x1a0 */
  

•   u32 priv0;              /* 0x1a4 */
  

•   u32 priv1;              /* 0x1a8 */
  

•   u8 res4[0x4];           /* 0x1ac */
  

•   u32 cpu1_pwr_clamp;     /* 0x1b0 sun7i only */
  

•   u32 cpu1_pwroff;        /* 0x1b4 sun7i only */
  

•   u8 res5[0x2c];          /* 0x1b8 */
  

•   u32 dbg_ctrl1;          /* 0x1e4 */
  

•   u8 res6[0x18];          /* 0x1e8 */
  

•   u32 idle_cnt0_low;      /* 0x200 */
  

•   u32 idle_cnt0_high;     /* 0x204 */
  

•   u32 idle_cnt0_ctrl;     /* 0x208 */
  

•   u8 res8[0x4];           /* 0x20c */
  

•   u32 idle_cnt1_low;      /* 0x210 */
  

•   u32 idle_cnt1_high;     /* 0x214 */
  

•   u32 idle_cnt1_ctrl;     /* 0x218 */
  

•   u8 res9[0x4];           /* 0x21c */
  

•   u32 idle_cnt2_low;      /* 0x220 */
  

•   u32 idle_cnt2_high;     /* 0x224 */
  

•   u32 idle_cnt2_ctrl;     /* 0x228 */
  

•   u8 res10[0x4];          /* 0x22c */
  

•   u32 idle_cnt3_low;      /* 0x230 */
  

•   u32 idle_cnt3_high;     /* 0x234 */
  

•   u32 idle_cnt3_ctrl;     /* 0x238 */
  

•   u8 res11[0x4];          /* 0x23c */
  

•   u32 idle_cnt4_low;      /* 0x240 */
  

•   u32 idle_cnt4_high;     /* 0x244 */
  

•   u32 idle_cnt4_ctrl;     /* 0x248 */
  

•   u8 res12[0x34];         /* 0x24c */
  

•   u32 cnt64_ctrl;         /* 0x280 */
  

•   u32 cnt64_low;          /* 0x284 */
  

•   u32 cnt64_high;         /* 0x288 */
  

+};

+#endif /* __ASSEMBLY__ */ +#endif /* _SUNXI_CPUCFG_SUN4I_H */ diff --git a/arch/arm/include/asm/arch-sunxi/cpucfg_sun9i.h b/arch/arm/include/asm/arch-sunxi/cpucfg_sun9i.h new file mode 100644 index 0000000000..2ce315736b --- /dev/null +++ b/arch/arm/include/asm/arch-sunxi/cpucfg_sun9i.h @@ -0,0 +1,60 @@ +/*

• • Sunxi A80 CPUCFG register definition.
• • (C) Copyright 2016 Chen-Yu Tsai wens@csie.org
• • SPDX-License-Identifier: GPL-2.0+
• */

+#ifndef _SUNXI_CPUCFG_SUN9I_H +#define _SUNXI_CPUCFG_SUN9I_H

+#include <linux/compiler.h> +#include <linux/types.h>

+#define CPUCFG_CX_CTRL0_L1_RST_DISABLE(core) BIT(core)

+#define CPUCFG_CX_STATUS_STANDBYWFI(core) BIT(16 + core)

+#define CPUCFG_CX_RST_CORE(core) BIT(core) +#define CPUCFG_CX_RST_NEON(core) BIT(4 + core) /* A15 only */ +#define CPUCFG_CX_RST_L2 BIT(8) +#define CPUCFG_CX_RST_HRESET BIT(12) +#define CPUCFG_CX_RST_DBG(core) BIT(16 + core) +#define CPUCFG_CX_RST_ETM(core) BIT(20 + core) +#define CPUCFG_CX_RST_SOC_DBG BIT(24)

+#ifndef __ASSEMBLY__

+struct __packed sunxi_cpucfg_cluster {

•   u32 ctrl0;              /* base + 0x0 */
  

•   u32 ctrl1;              /* base + 0x4 */
  

•   u32 adb400_pwrdnreqn;   /* base + 0x8 */
  

•   u8 res[0x4];            /* base + 0xc */
  

+};

+struct __packed sunxi_cpucfg_reg {

•   struct sunxi_cpucfg_cluster cluster[2]; /* 0x00 */
  

•   u8 res0[0x8];           /* 0x20 */
  

•   u32 gen_ctrl0;          /* 0x28 */
  

•   u32 gen_ctrl1;          /* 0x2c */
  

•   u32 cluster_status[2];  /* 0x30 */
  

•   u8 res1[0x4];           /* 0x38 */
  

•   u32 irq_fiq_status;     /* 0x3c */
  

•   u32 irq_fiq_mask;       /* 0x40 */
  

•   u8 res2[0x3c];          /* 0x44 */
  

•   u32 cluster_reset[2];   /* 0x80 */
  

•   u32 gic_jtag_reset;     /* 0x88 */
  

+};

+#ifdef CONFIG_MACH_SUN8I_A83T +struct __packed sunxi_r_cpucfg_reg {

•   u8 res1[0x30];          /* 0x00 */
  

•   u32 cpu_rst[2];         /* 0x30 */
  

•   u8 res2[0x16c];         /* 0x38 */
  

•   u32 priv0;              /* 0x1a4 */
  

+}; +#endif /* CONFIG_MACH_SUN8I_A83T */

+#endif /* __ASSEMBLY__ */

+#endif /* _SUNXI_CPUCFG_SUN9I_H */

2.12.2

This patch looks a bit weird. Do you have --find-renames and --find-copies set when you run `git format-patch`? It should help with renames.

ChenYu

On Wed, Jun 07, 2017 at 08:47:18AM +0800, Icenowy Zheng wrote:

diff --git a/arch/arm/include/asm/arch-sunxi/cpucfg_sun4i.h b/arch/arm/include/asm/arch-sunxi/cpucfg_sun4i.h new file mode 100644 index 0000000000..af1a1d56c9 --- /dev/null +++ b/arch/arm/include/asm/arch-sunxi/cpucfg_sun4i.h @@ -0,0 +1,68 @@ +/*

• • Sunxi A31 CPUCFG register definition.

Calling that file sun4i doesn't make much sense, the A31 is part of sun6i, and the cpucfg block wasn't there with sun4i.

Maxime

On Wed, Jun 7, 2017 at 2:58 PM, Icenowy Zheng icenowy@aosc.io wrote:

于 2017年6月7日 GMT+08:00 下午2:44:21, Maxime Ripard maxime.ripard@free-electrons.com 写到:

...

On Wed, Jun 07, 2017 at 08:47:18AM +0800, Icenowy Zheng wrote:

...

diff --git a/arch/arm/include/asm/arch-sunxi/cpucfg_sun4i.h

b/arch/arm/include/asm/arch-sunxi/cpucfg_sun4i.h

...

new file mode 100644 index 0000000000..af1a1d56c9 --- /dev/null +++ b/arch/arm/include/asm/arch-sunxi/cpucfg_sun4i.h @@ -0,0 +1,68 @@ +/*

• • Sunxi A31 CPUCFG register definition.

Calling that file sun4i doesn't make much sense, the A31 is part of sun6i, and the cpucfg block wasn't there with sun4i.

So... call it sun6i?

No. The original cpucfg.h is for the A20 and the A31, and all the single cluster SoC that followed.

The A83T's CPUCFG has more in common with the A80. You can just keep cpucfg.h unchanged, and rename the new one cpucfg-mcpm.h. You don't even need the generic cpucfg.h stub header you added.

Regards ChenYu

On Wed, Jun 7, 2017 at 8:47 AM, Icenowy Zheng icenowy@aosc.io wrote:

A83T come with two clusters of CPU, for each cluster 1 the new registers are in the reserved spaces after the original cluster 0.

Make the registers to have an array with length 2 (2 clusters), and change the current code to reference only cluster 0 registers.

Signed-off-by: Icenowy Zheng icenowy@aosc.io

arch/arm/cpu/armv7/sunxi/psci.c | 2 +- arch/arm/include/asm/arch-sunxi/prcm.h | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/arm/cpu/armv7/sunxi/psci.c b/arch/arm/cpu/armv7/sunxi/psci.c index b3a34de1aa..8caef6a85f 100644 --- a/arch/arm/cpu/armv7/sunxi/psci.c +++ b/arch/arm/cpu/armv7/sunxi/psci.c @@ -144,7 +144,7 @@ static void __secure sunxi_cpu_set_power(int cpu, bool on) struct sunxi_prcm_reg *prcm = (struct sunxi_prcm_reg *)SUNXI_PRCM_BASE;

•   sunxi_power_switch(&prcm->cpu_pwr_clamp[cpu], &prcm->cpu_pwroff,
  

•   sunxi_power_switch(&prcm->cpu_pwr_clamp[0][cpu], &prcm->cpu_pwroff[0],
                       on, cpu);
  

} #endif /* CONFIG_MACH_SUN7I */ diff --git a/arch/arm/include/asm/arch-sunxi/prcm.h b/arch/arm/include/asm/arch-sunxi/prcm.h index ae3880b13b..c2a6e39ffc 100644 --- a/arch/arm/include/asm/arch-sunxi/prcm.h +++ b/arch/arm/include/asm/arch-sunxi/prcm.h @@ -220,16 +220,16 @@ struct __packed sunxi_prcm_reg { u8 res5[0x3c]; /* 0x0b4 */ u32 clk_outd; /* 0x0f0 */ u8 res6[0xc]; /* 0x0f4 */

•   u32 cpu_pwroff;         /* 0x100 */
  

•   u8 res7[0xc];           /* 0x104 */
  

•   u32 cpu_pwroff[2];      /* 0x100 */
  

•   u8 res7[0x8];           /* 0x108 */
    u32 vdd_sys_pwroff;     /* 0x110 */
    u8 res8[0x4];           /* 0x114 */
    u32 gpu_pwroff;         /* 0x118 */
    u8 res9[0x4];           /* 0x11c */
    u32 vdd_pwr_reset;      /* 0x120 */
    u8 res10[0x1c];         /* 0x124 */
  

•   u32 cpu_pwr_clamp[4];   /* 0x140 but first one is actually unused */
  

•   u8 res11[0x30];         /* 0x150 */
  

•   u32 cpu_pwr_clamp[2][4];/* 0x140 but first one is actually unused */
  

The comment needs to be fixed. It only applies to the single cluster SoCs.

Otherwise,

Reviewed-by: Chen-Yu Tsai wens@csie.org

•   u8 res11[0x20];         /* 0x160 */
    u32 dram_pwr;           /* 0x180 */
    u8 res12[0xc];          /* 0x184 */
    u32 dram_tst;           /* 0x190 */
  

-- 2.12.2

-- You received this message because you are subscribed to the Google Groups "linux-sunxi" group. To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe@googlegroups.com. For more options, visit https://groups.google.com/d/optout.

On Wed, Jun 7, 2017 at 8:47 AM, Icenowy Zheng icenowy@aosc.io wrote:

From: Chen-Yu Tsai wens@csie.org

Allwinner A80 and A83T SoCs have two clusters of CPU, each cluster contains 4 cores. A80 is Cortex-A15 + Cortex-A7 configuration, while A83T has two clusters of Cortex-A7.

This patch adds a basic version that allows bringing up the four cores in the first cluster. The structure is based on existing sunxi PSCI code.

Signed-off-by: Chen-Yu Tsai wens@csie.org [Icenowy: adapt for A83T] Signed-off-by: Icenowy Zheng icenowy@aosc.io

---

arch/arm/cpu/armv7/sunxi/Makefile | 4 + arch/arm/cpu/armv7/sunxi/psci-mcpm.c | 258 +++++++++++++++++++++++++++++++++++ 2 files changed, 262 insertions(+) create mode 100644 arch/arm/cpu/armv7/sunxi/psci-mcpm.c

I suggest using --find-copies and --find-copies-harder when you generate the patch. It should limit the patch to differences between psci.c and psci-mcpm.c.

diff --git a/arch/arm/cpu/armv7/sunxi/Makefile b/arch/arm/cpu/armv7/sunxi/Makefile index 8c026ff052..c789f686fd 100644 --- a/arch/arm/cpu/armv7/sunxi/Makefile +++ b/arch/arm/cpu/armv7/sunxi/Makefile @@ -14,8 +14,12 @@ obj-$(CONFIG_MACH_SUN8I_H3) += tzpc.o obj-$(CONFIG_MACH_SUN8I_A83T) += tzpc.o

ifndef CONFIG_SPL_BUILD +ifdef CONFIG_MACH_SUN8I_A83T +obj-$(CONFIG_ARMV7_PSCI) += psci-mcpm.o +else obj-$(CONFIG_ARMV7_PSCI) += psci.o endif +endif

ifdef CONFIG_SPL_BUILD obj-y += fel_utils.o diff --git a/arch/arm/cpu/armv7/sunxi/psci-mcpm.c b/arch/arm/cpu/armv7/sunxi/psci-mcpm.c new file mode 100644 index 0000000000..ba8d669c7e --- /dev/null +++ b/arch/arm/cpu/armv7/sunxi/psci-mcpm.c @@ -0,0 +1,258 @@ +/*

• • Copyright (C) 2016
• • Author: Chen-Yu Tsai wens@csie.org
• • Based on assembly code by Marc Zyngier marc.zyngier@arm.com,
• • which was based on code by Carl van Schaik carl@ok-labs.com.
• • SPDX-License-Identifier: GPL-2.0
• */

+#include <config.h> +#include <common.h>

+#include <asm/arch/cpu.h> +#include <asm/arch/cpucfg.h> +#include <asm/arch/prcm.h> +#include <asm/armv7.h> +#include <asm/io.h> +#include <asm/psci.h> +#include <asm/secure.h>

+#include <linux/bitops.h>

+/*

• • NOTE dense CPU IDs (0~3 for first cluster of 4 cores, 4~7 for the
• • second cluster) are used throughout the PSCI code. Any MPIDR style
• • values must be converted.
• */

+/*

• • Provide a dense CPU ID for 2-cluster systems. This must be coded in
• • assembly as it gets called from psci_stack_setup, when the stack isn't
• • available yet.
• • Only r0 and r3 is usable. r8 - r12 are available if this function is
• • only called from psci_stack_setup, which we cannot guarantee.
• */

+u32 __secure __naked psci_get_cpu_id(void) +{

•   asm volatile (
  

•           "mrc    p15, 0, r3, c0, c0, 5   @ Get MPIDR\n"
  

•           "lsr    r0, r3, #6\n"
  

•           "and    r3, r3, #3\n"
  

•           "and    r0, r0, #4\n"
  

•           "orr    r0, r0, r3\n"
  

•           "bx     lr\n"
  

•   );
  

•   /*
  

•    * The last five lines are the compiler generated assembly code for
  

•    *
  

•    *      return (reg & 0x3) | (((reg >> 8) & 0x1) << 2);
  

•    *
  

•    * We can't guarantee that all compilers correctly use only r0 and
  

•    * r3, so we use inline assembly here.
  

•    */
  

+}

+static void __secure cp15_write_cntp_tval(u32 tval) +{

•   asm volatile ("mcr p15, 0, %0, c14, c2, 0" : : "r" (tval));
  

+}

+static void __secure cp15_write_cntp_ctl(u32 val) +{

•   asm volatile ("mcr p15, 0, %0, c14, c2, 1" : : "r" (val));
  

+}

+static u32 __secure cp15_read_cntp_ctl(void) +{

•   u32 val;
  

•   asm volatile ("mrc p15, 0, %0, c14, c2, 1" : "=r" (val));
  

•   return val;
  

+}

+#define ONE_US (COUNTER_FREQUENCY / 1000000)

+/* Use a different name to avoid clashing with the non-secure function */ +static void __secure __udelay_sec(unsigned long us) +{

•   u32 reg = ONE_US * us;
  

•   cp15_write_cntp_tval(reg);
  

•   isb();
  

•   cp15_write_cntp_ctl(3);
  

•   do {
  

•           isb();
  

•           reg = cp15_read_cntp_ctl();
  

•   } while (!(reg & BIT(2)));
  

•   cp15_write_cntp_ctl(0);
  

•   isb();
  

+}

+static void __secure clamp_release(u32 *clamp) +{

•   writel(0xff, clamp);
  

•   __udelay_sec(10);
  

•   writel(0xfe, clamp);
  

•   __udelay_sec(10);
  

•   writel(0xf8, clamp);
  

•   __udelay_sec(10);
  

•   writel(0xf0, clamp);
  

•   __udelay_sec(10);
  

•   writel(0x00, clamp);
  

+}

+static void __secure clamp_set(u32 *clamp) +{

•   writel(0xff, clamp);
  

+}

+static void __secure sunxi_core_power_switch(u32 *clamp, u32 *pwroff,

•                                        bool on, int cpu)
  

+{

•   if (on) {
  

•           /* Release power clamp */
  

•           clamp_release(clamp);
  

•           __udelay_sec(20);
  

•           /* Clear power gating */
  

•           clrbits_le32(pwroff, BIT(cpu));
  

•   } else {
  

•           /* Set power gating */
  

•           setbits_le32(pwroff, BIT(cpu));
  

•           __udelay_sec(20);
  

•           /* Activate power clamp */
  

•           clamp_set(clamp);
  

•   }
  

+}

+static int __secure sunxi_cluster_is_a7(int cluster) +{ +#ifdef CONFIG_MACH_SUN8I_A83T

•   return 1;
  

+#else

•   return (clustter == 0);
  

+#endif +}

+static void __secure sunxi_cpu_set_power(int cluster, int cpu, bool on) +{

•   struct sunxi_prcm_reg *prcm =
  

•           (struct sunxi_prcm_reg *)SUNXI_PRCM_BASE;
  

•   sunxi_core_power_switch(&prcm->cpu_pwr_clamp[cluster][cpu],
  

•                           &prcm->cpu_pwroff[cluster], on, cpu);
  

+}

+static u32 __secure cp15_read_scr(void) +{

•   u32 scr;
  

•   asm volatile ("mrc p15, 0, %0, c1, c1, 0" : "=r" (scr));
  

•   return scr;
  

+}

+static void __secure cp15_write_scr(u32 scr) +{

•   asm volatile ("mcr p15, 0, %0, c1, c1, 0" : : "r" (scr));
  

•   isb();
  

+}

+int __secure psci_cpu_on(u32 __always_unused unused, u32 mpidr, u32 pc) +{

•   struct sunxi_cpucfg_reg *cpucfg =
  

•           (struct sunxi_cpucfg_reg *)SUNXI_CPUCFG_BASE;
  

+#ifdef CONFIG_MACH_SUN8I_A83T

•   struct sunxi_r_cpucfg_reg *r_cpucfg =
  

•           (struct sunxi_r_cpucfg_reg *)SUNXI_R_CPUCFG_BASE;
  

+#else

•   struct sunxi_prcm_reg *prcm =
  

•           (struct sunxi_prcm_reg *)SUNXI_PRCM_BASE;
  

+#endif

•   u32 cluster = (mpidr >> 8) & 0x1;
  

•   u32 cpu = mpidr & 0x3;
  

•   u32 cpuid = cpu | (cluster << 2);
  

•   /* TODO We don't support multi-cluster yet */
  

•   if (cluster > 0)
  

•           return ARM_PSCI_RET_INVAL;
  

•   /* store target PC */
  

•   psci_save_target_pc(cpuid, pc);
  

•   /* Set secondary core power on PC */
  

+#ifdef CONFIG_MACH_SUN8I_A83T

•   writel((u32)&psci_cpu_entry, &r_cpucfg->priv0);
  

+#else

•   writel((u32)&psci_cpu_entry, &prcm->cpu_soft_entry);
  

+#endif

Could you take a look at the non-MCPM version and move the #ifdefs into separate functions? Basically we want the main functions to be as clean and descriptive as possible. You could also structure it as two patches: the first adding the original A80 version, and the second modifying it to support the A83T as well.

I'll send a fix for the non-MCPM version fixing the entry address.

Regards ChenYu

•   /* Assert power-on reset on target CPU */
  

+#ifdef CONFIG_MACH_SUN8I_A83T

•   clrbits_le32(&r_cpucfg->cpu_rst[cluster], BIT(cpu));
  

+#else

•   clrbits_le32(&prcm->cpu_rst[cluster], BIT(cpu));
  

+#endif

•   /* Cortex-A7: hold L1 cache reset disable signal low */
  

•   if (sunxi_cluster_is_a7(cluster))
  

•           clrbits_le32(&cpucfg->cluster[cluster].ctrl0,
  

•                        CPUCFG_CX_CTRL0_L1_RST_DISABLE(cpu));
  

•   /* Lock CPU (Disable external debug access) */
  

•   clrbits_le32(&cpucfg->cluster_reset[cluster],
  

•                CPUCFG_CX_RST_DBG(cpu));
  

•   /* Cortex-A7: Assert ETM reset */
  

•   if (sunxi_cluster_is_a7(cluster))
  

•           clrbits_le32(&cpucfg->cluster_reset[cluster],
  

•                        CPUCFG_CX_RST_ETM(cpu));
  

•   /*
  

•    * Allwinner code also asserts resets for NEON on A15. According
  

•    * to ARM manuals, asserting power-on reset is sufficient.
  

•    */
  

•   /* Power up target CPU */
  

•   sunxi_cpu_set_power(cluster, cpu, true);
  

•   /* De-assert power-on reset on target CPU */
  

+#ifdef CONFIG_MACH_SUN8I_A83T

•   setbits_le32(&r_cpucfg->cpu_rst[cluster], BIT(cpu));
  

+#else

•   setbits_le32(&prcm->cpu_rst[cluster], BIT(cpu));
  

+#endif

•   /* De-assert core reset on target CPU */
  

•   setbits_le32(&cpucfg->cluster_reset[cluster],
  

•                CPUCFG_CX_RST_CORE(cpu));
  

•   /* Cortex-A7: De-assert ETM reset */
  

•   if (sunxi_cluster_is_a7(cluster))
  

•           setbits_le32(&cpucfg->cluster_reset[cluster],
  

•                        CPUCFG_CX_RST_ETM(cpu));
  

•   /* Unlock CPU (Disable external debug access) */
  

•   setbits_le32(&cpucfg->cluster_reset[cluster],
  

•                CPUCFG_CX_RST_DBG(cpu));
  

•   return ARM_PSCI_RET_SUCCESS;
  

+}

+void __secure psci_arch_init(void) +{

•   u32 reg;
  

•   reg = cp15_read_scr();
  

•   reg &= ~BIT(0); /* Secure mode */
  

•   cp15_write_scr(reg);
  

+}

2.12.2

-- You received this message because you are subscribed to the Google Groups "linux-sunxi" group. To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe@googlegroups.com. For more options, visit https://groups.google.com/d/optout.

于 2017年6月7日 GMT+08:00 下午2:48:55, Maxime Ripard maxime.ripard@free-electrons.com 写到:

Hi,

On Wed, Jun 07, 2017 at 08:47:20AM +0800, Icenowy Zheng wrote:

...

From: Chen-Yu Tsai wens@csie.org

Allwinner A80 and A83T SoCs have two clusters of CPU, each cluster contains 4 cores. A80 is Cortex-A15 + Cortex-A7 configuration, while A83T has two clusters of Cortex-A7.

This patch adds a basic version that allows bringing up the four

cores

...

in the first cluster. The structure is based on existing sunxi PSCI

code.

...

Signed-off-by: Chen-Yu Tsai wens@csie.org [Icenowy: adapt for A83T] Signed-off-by: Icenowy Zheng icenowy@aosc.io

Ideally, Marc should be in Cc in order to review this.

Who?

...

---

arch/arm/cpu/armv7/sunxi/Makefile | 4 + arch/arm/cpu/armv7/sunxi/psci-mcpm.c | 258

+++++++++++++++++++++++++++++++++++

...

2 files changed, 262 insertions(+) create mode 100644 arch/arm/cpu/armv7/sunxi/psci-mcpm.c

diff --git a/arch/arm/cpu/armv7/sunxi/Makefile

b/arch/arm/cpu/armv7/sunxi/Makefile

...

index 8c026ff052..c789f686fd 100644 --- a/arch/arm/cpu/armv7/sunxi/Makefile +++ b/arch/arm/cpu/armv7/sunxi/Makefile @@ -14,8 +14,12 @@ obj-$(CONFIG_MACH_SUN8I_H3) += tzpc.o obj-$(CONFIG_MACH_SUN8I_A83T) += tzpc.o

ifndef CONFIG_SPL_BUILD +ifdef CONFIG_MACH_SUN8I_A83T +obj-$(CONFIG_ARMV7_PSCI) += psci-mcpm.o +else obj-$(CONFIG_ARMV7_PSCI) += psci.o endif +endif

ifdef CONFIG_SPL_BUILD obj-y += fel_utils.o diff --git a/arch/arm/cpu/armv7/sunxi/psci-mcpm.c

b/arch/arm/cpu/armv7/sunxi/psci-mcpm.c

...

new file mode 100644 index 0000000000..ba8d669c7e --- /dev/null +++ b/arch/arm/cpu/armv7/sunxi/psci-mcpm.c @@ -0,0 +1,258 @@ +/*

• • Copyright (C) 2016
• • Author: Chen-Yu Tsai wens@csie.org
• • Based on assembly code by Marc Zyngier marc.zyngier@arm.com,
• • which was based on code by Carl van Schaik carl@ok-labs.com.
• • SPDX-License-Identifier: GPL-2.0
• */

+#include <config.h> +#include <common.h>

+#include <asm/arch/cpu.h> +#include <asm/arch/cpucfg.h> +#include <asm/arch/prcm.h> +#include <asm/armv7.h> +#include <asm/io.h> +#include <asm/psci.h> +#include <asm/secure.h>

+#include <linux/bitops.h>

+/*

• • NOTE dense CPU IDs (0~3 for first cluster of 4 cores, 4~7 for the
• • second cluster) are used throughout the PSCI code. Any MPIDR

style

...

• • values must be converted.
• */

+/*

• • Provide a dense CPU ID for 2-cluster systems. This must be coded

in

...

• • assembly as it gets called from psci_stack_setup, when the stack

isn't

...

• • available yet.
• • Only r0 and r3 is usable. r8 - r12 are available if this function

is

...

• • only called from psci_stack_setup, which we cannot guarantee.
• */

+u32 __secure __naked psci_get_cpu_id(void) +{

• asm volatile (

• "mrc	p15, 0, r3, c0, c0, 5	@ Get MPIDR\n"
  

• "lsr	r0, r3, #6\n"
  

• "and	r3, r3, #3\n"
  

• "and	r0, r0, #4\n"
  

• "orr	r0, r0, r3\n"
  

• "bx	lr\n"
  

• );
• /*

• * The last five lines are the compiler generated assembly code for
  

• *
  

• *	return (reg & 0x3) | (((reg >> 8) & 0x1) << 2);
  

• *
  

• * We can't guarantee that all compilers correctly use only r0 and
  

• * r3, so we use inline assembly here.
  

• */
  

+}

+static void __secure cp15_write_cntp_tval(u32 tval) +{

• asm volatile ("mcr p15, 0, %0, c14, c2, 0" : : "r" (tval));

+}

+static void __secure cp15_write_cntp_ctl(u32 val) +{

• asm volatile ("mcr p15, 0, %0, c14, c2, 1" : : "r" (val));

+}

+static u32 __secure cp15_read_cntp_ctl(void) +{

• u32 val;
• asm volatile ("mrc p15, 0, %0, c14, c2, 1" : "=r" (val));
• return val;

+}

+#define ONE_US (COUNTER_FREQUENCY / 1000000)

+/* Use a different name to avoid clashing with the non-secure

function */

...

+static void __secure __udelay_sec(unsigned long us) +{

• u32 reg = ONE_US * us;
• cp15_write_cntp_tval(reg);
• isb();
• cp15_write_cntp_ctl(3);
• do {

• isb();
  

• reg = cp15_read_cntp_ctl();
  

• } while (!(reg & BIT(2)));
• cp15_write_cntp_ctl(0);
• isb();

+}

+static void __secure clamp_release(u32 *clamp) +{

• writel(0xff, clamp);
• __udelay_sec(10);
• writel(0xfe, clamp);
• __udelay_sec(10);
• writel(0xf8, clamp);
• __udelay_sec(10);
• writel(0xf0, clamp);
• __udelay_sec(10);
• writel(0x00, clamp);

+}

+static void __secure clamp_set(u32 *clamp) +{

• writel(0xff, clamp);

+}

+static void __secure sunxi_core_power_switch(u32 *clamp, u32

*pwroff,

...

• 			     bool on, int cpu)
  

+{

• if (on) {

• /* Release power clamp */
  

• clamp_release(clamp);
  

• __udelay_sec(20);
  

• /* Clear power gating */
  

• clrbits_le32(pwroff, BIT(cpu));
  

• } else {

• /* Set power gating */
  

• setbits_le32(pwroff, BIT(cpu));
  

• __udelay_sec(20);
  

• /* Activate power clamp */
  

• clamp_set(clamp);
  

• }

+}

+static int __secure sunxi_cluster_is_a7(int cluster) +{ +#ifdef CONFIG_MACH_SUN8I_A83T

• return 1;

+#else

• return (clustter == 0);

This doesn't compile.

Maxime

于 2017年6月7日 GMT+08:00 上午11:36:27, Chen-Yu Tsai wens@csie.org 写到:

On Wed, Jun 7, 2017 at 8:47 AM, Icenowy Zheng icenowy@aosc.io wrote:

...

As we have now a basical implementation of PSCI for A83T, enable non-secure boot support and PSCI on A83T now.

Signed-off-by: Icenowy Zheng icenowy@aosc.io

arch/arm/mach-sunxi/Kconfig | 4 ++++ 1 file changed, 4 insertions(+)

diff --git a/arch/arm/mach-sunxi/Kconfig

b/arch/arm/mach-sunxi/Kconfig

...

index 7ced838d6a..31d29de428 100644 --- a/arch/arm/mach-sunxi/Kconfig +++ b/arch/arm/mach-sunxi/Kconfig @@ -98,8 +98,12 @@ config MACH_SUN8I_A33 config MACH_SUN8I_A83T bool "sun8i (Allwinner A83T)" select CPU_V7

•   select CPU_V7_HAS_NONSEC
  

•   select CPU_V7_HAS_VIRT
  

•   select ARCH_SUPPORT_PSCI
    select SUNXI_GEN_SUN6I
    select SUPPORT_SPL
  

•   select ARMV7_BOOT_SEC_DEFAULT if OLD_SUNXI_KERNEL_COMPAT
  

The kernel does not work yet. Please have it boot to secure by default regardless of the kernel. We can have it boot non-secure once the kernel has been working for a reasonable amount of time.

I don't want clueless users coming and asking why it suddenly stopped working. This should be an experimental feature.

Maybe you should send out the fix, and tag them to also apply to stable tree.

GIC is really broken, UP systems only work by chance. We shouldn't depend on this behavior.

ChenYu

On Wed, Jun 07, 2017 at 11:47:24AM +0800, Chen-Yu Tsai wrote:

On Wed, Jun 7, 2017 at 11:40 AM, Icenowy Zheng icenowy@aosc.io wrote:

...

于 2017年6月7日 GMT+08:00 上午11:36:27, Chen-Yu Tsai wens@csie.org 写到:

...

On Wed, Jun 7, 2017 at 8:47 AM, Icenowy Zheng icenowy@aosc.io wrote:

...

As we have now a basical implementation of PSCI for A83T, enable non-secure boot support and PSCI on A83T now.

Signed-off-by: Icenowy Zheng icenowy@aosc.io

arch/arm/mach-sunxi/Kconfig | 4 ++++ 1 file changed, 4 insertions(+)

diff --git a/arch/arm/mach-sunxi/Kconfig

b/arch/arm/mach-sunxi/Kconfig

...

index 7ced838d6a..31d29de428 100644 --- a/arch/arm/mach-sunxi/Kconfig +++ b/arch/arm/mach-sunxi/Kconfig @@ -98,8 +98,12 @@ config MACH_SUN8I_A33 config MACH_SUN8I_A83T bool "sun8i (Allwinner A83T)" select CPU_V7

•   select CPU_V7_HAS_NONSEC
  

•   select CPU_V7_HAS_VIRT
  

•   select ARCH_SUPPORT_PSCI
    select SUNXI_GEN_SUN6I
    select SUPPORT_SPL
  

•   select ARMV7_BOOT_SEC_DEFAULT if OLD_SUNXI_KERNEL_COMPAT
  

The kernel does not work yet. Please have it boot to secure by default regardless of the kernel. We can have it boot non-secure once the kernel has been working for a reasonable amount of time.

I don't want clueless users coming and asking why it suddenly stopped working. This should be an experimental feature.

Maybe you should send out the fix, and tag them to also apply to stable tree.

GIC is really broken, UP systems only work by chance. We shouldn't depend on this behavior.

As I previously explained, it is not the GIC that is broken. I believe the GIC is working exactly as it is supposed to with regards to its input signals.

Allwinner's security extensions implementation simply does not properly forward the AXI secure bit when the e-fuse's secure bit isn't burned.

Is that on all revisions, or just the revB ?

Maxime

On Wed, Jun 7, 2017 at 2:50 PM, Maxime Ripard maxime.ripard@free-electrons.com wrote:

On Wed, Jun 07, 2017 at 11:47:24AM +0800, Chen-Yu Tsai wrote:

...

On Wed, Jun 7, 2017 at 11:40 AM, Icenowy Zheng icenowy@aosc.io wrote:

...

于 2017年6月7日 GMT+08:00 上午11:36:27, Chen-Yu Tsai wens@csie.org 写到:

...

On Wed, Jun 7, 2017 at 8:47 AM, Icenowy Zheng icenowy@aosc.io wrote:

...

As we have now a basical implementation of PSCI for A83T, enable non-secure boot support and PSCI on A83T now.

Signed-off-by: Icenowy Zheng icenowy@aosc.io

arch/arm/mach-sunxi/Kconfig | 4 ++++ 1 file changed, 4 insertions(+)

diff --git a/arch/arm/mach-sunxi/Kconfig

b/arch/arm/mach-sunxi/Kconfig

...

index 7ced838d6a..31d29de428 100644 --- a/arch/arm/mach-sunxi/Kconfig +++ b/arch/arm/mach-sunxi/Kconfig @@ -98,8 +98,12 @@ config MACH_SUN8I_A33 config MACH_SUN8I_A83T bool "sun8i (Allwinner A83T)" select CPU_V7

•   select CPU_V7_HAS_NONSEC
  

•   select CPU_V7_HAS_VIRT
  

•   select ARCH_SUPPORT_PSCI
    select SUNXI_GEN_SUN6I
    select SUPPORT_SPL
  

•   select ARMV7_BOOT_SEC_DEFAULT if OLD_SUNXI_KERNEL_COMPAT
  

The kernel does not work yet. Please have it boot to secure by default regardless of the kernel. We can have it boot non-secure once the kernel has been working for a reasonable amount of time.

I don't want clueless users coming and asking why it suddenly stopped working. This should be an experimental feature.

Maybe you should send out the fix, and tag them to also apply to stable tree.

GIC is really broken, UP systems only work by chance. We shouldn't depend on this behavior.

As I previously explained, it is not the GIC that is broken. I believe the GIC is working exactly as it is supposed to with regards to its input signals.

Allwinner's security extensions implementation simply does not properly forward the AXI secure bit when the e-fuse's secure bit isn't burned.

Is that on all revisions, or just the revB ?

It's the A80, but I'm guessing the same applies to the A83T. It's more of a guess really, but I think it's a logical one. If the e-fuse isn't programmed, the TZPC doesn't work, and access to all secure peripherals still work, even from non-secure mode. The only one that does work is the secure SRAM.

The GIC still has the banked secure/non-secure registers, just that all cores access the secure bank, even when in non-secure mode. The workaround is to use the alias set of non-secure registers in Linux.

I'm not about to waste one of my boards programming the e-fuse to find out the hard way though. The CCU doesn't have a security setting. It might as well be secure-only. If one sets the e-fuse and the SoC's security extensions work as they're supposed to, then it will no longer work with mainline Linux. Or any software we have for that matter.

Regards ChenYu

于 2017年6月7日 GMT+08:00 下午8:11:12, Marc Zyngier marc.zyngier@arm.com 写到:

On 07/06/17 08:00, Chen-Yu Tsai wrote:

...

On Wed, Jun 7, 2017 at 2:50 PM, Maxime Ripard maxime.ripard@free-electrons.com wrote:

...

On Wed, Jun 07, 2017 at 11:47:24AM +0800, Chen-Yu Tsai wrote:

...

On Wed, Jun 7, 2017 at 11:40 AM, Icenowy Zheng icenowy@aosc.io

wrote:

...

...

...

...

于 2017年6月7日 GMT+08:00 上午11:36:27, Chen-Yu Tsai wens@csie.org 写到:

...

On Wed, Jun 7, 2017 at 8:47 AM, Icenowy Zheng icenowy@aosc.io

wrote:

...

...

...

...

...

> As we have now a basical implementation of PSCI for A83T, enable > non-secure boot support and PSCI on A83T now. > > Signed-off-by: Icenowy Zheng icenowy@aosc.io > --- > arch/arm/mach-sunxi/Kconfig | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/arch/arm/mach-sunxi/Kconfig b/arch/arm/mach-sunxi/Kconfig > index 7ced838d6a..31d29de428 100644 > --- a/arch/arm/mach-sunxi/Kconfig > +++ b/arch/arm/mach-sunxi/Kconfig > @@ -98,8 +98,12 @@ config MACH_SUN8I_A33 > config MACH_SUN8I_A83T > bool "sun8i (Allwinner A83T)" > select CPU_V7 > + select CPU_V7_HAS_NONSEC > + select CPU_V7_HAS_VIRT > + select ARCH_SUPPORT_PSCI > select SUNXI_GEN_SUN6I > select SUPPORT_SPL > + select ARMV7_BOOT_SEC_DEFAULT if OLD_SUNXI_KERNEL_COMPAT

The kernel does not work yet. Please have it boot to secure by

default

...

...

...

...

...

regardless of the kernel. We can have it boot non-secure once the kernel has been working for a reasonable amount of time.

I don't want clueless users coming and asking why it suddenly

stopped

...

...

...

...

...

working. This should be an experimental feature.

Maybe you should send out the fix, and tag them to also apply to stable tree.

GIC is really broken, UP systems only work by chance. We shouldn't depend on this behavior.

As I previously explained, it is not the GIC that is broken. I

believe

...

...

...

the GIC is working exactly as it is supposed to with regards to its input signals.

Allwinner's security extensions implementation simply does not

properly

...

...

...

forward the AXI secure bit when the e-fuse's secure bit isn't

burned.

Arghh. Puke. Now I remember this, and I wish I didn't...

...

...

Is that on all revisions, or just the revB ?

It's the A80, but I'm guessing the same applies to the A83T. It's

more

...

of a guess really, but I think it's a logical one. If the e-fuse

isn't

...

programmed, the TZPC doesn't work, and access to all secure

peripherals

...

still work, even from non-secure mode. The only one that does work is the secure SRAM.

The GIC still has the banked secure/non-secure registers, just that

all

...

cores access the secure bank, even when in non-secure mode. The

workaround

...

is to use the alias set of non-secure registers in Linux.

That's a pretty dire workaround. Also, I expect that secure writes to GICV/GICH will not do the right thing. At this point, what is the requirement for running non-secure?

Write Secure Boot eFUSE, which will break *all* existing softwares.

...

I'm not about to waste one of my boards programming the e-fuse to

find

...

out the hard way though. The CCU doesn't have a security setting. It might as well be secure-only. If one sets the e-fuse and the SoC's security extensions work as they're supposed to, then it will no

longer

...

work with mainline Linux. Or any software we have for that matter.

Fair enough.

Thanks,

M.

On Wed, Jun 07, 2017 at 01:51:48PM +0100, Marc Zyngier wrote:

On 07/06/17 13:12, Icenowy Zheng wrote:

...

于 2017年6月7日 GMT+08:00 下午8:11:12, Marc Zyngier marc.zyngier@arm.com 写到:

...

On 07/06/17 08:00, Chen-Yu Tsai wrote:

...

On Wed, Jun 7, 2017 at 2:50 PM, Maxime Ripard maxime.ripard@free-electrons.com wrote:

...

On Wed, Jun 07, 2017 at 11:47:24AM +0800, Chen-Yu Tsai wrote:

...

On Wed, Jun 7, 2017 at 11:40 AM, Icenowy Zheng icenowy@aosc.io

wrote:

...

...

...

> > > 于 2017年6月7日 GMT+08:00 上午11:36:27, Chen-Yu Tsai wens@csie.org 写到: >> On Wed, Jun 7, 2017 at 8:47 AM, Icenowy Zheng icenowy@aosc.io

wrote:

...

...

...

>>> As we have now a basical implementation of PSCI for A83T, enable >>> non-secure boot support and PSCI on A83T now. >>> >>> Signed-off-by: Icenowy Zheng icenowy@aosc.io >>> --- >>> arch/arm/mach-sunxi/Kconfig | 4 ++++ >>> 1 file changed, 4 insertions(+) >>> >>> diff --git a/arch/arm/mach-sunxi/Kconfig >> b/arch/arm/mach-sunxi/Kconfig >>> index 7ced838d6a..31d29de428 100644 >>> --- a/arch/arm/mach-sunxi/Kconfig >>> +++ b/arch/arm/mach-sunxi/Kconfig >>> @@ -98,8 +98,12 @@ config MACH_SUN8I_A33 >>> config MACH_SUN8I_A83T >>> bool "sun8i (Allwinner A83T)" >>> select CPU_V7 >>> + select CPU_V7_HAS_NONSEC >>> + select CPU_V7_HAS_VIRT >>> + select ARCH_SUPPORT_PSCI >>> select SUNXI_GEN_SUN6I >>> select SUPPORT_SPL >>> + select ARMV7_BOOT_SEC_DEFAULT if OLD_SUNXI_KERNEL_COMPAT >> >> The kernel does not work yet. Please have it boot to secure by

default

...

...

...

>> regardless of the kernel. We can have it boot non-secure once the >> kernel >> has been working for a reasonable amount of time. >> >> I don't want clueless users coming and asking why it suddenly

stopped

...

...

...

>> working. This should be an experimental feature. > > Maybe you should send out the fix, and tag them to also apply to > stable tree. > > GIC is really broken, UP systems only work by chance. We > shouldn't depend on this behavior.

As I previously explained, it is not the GIC that is broken. I

believe

...

...

...

the GIC is working exactly as it is supposed to with regards to its input signals.

Allwinner's security extensions implementation simply does not

properly

...

...

...

forward the AXI secure bit when the e-fuse's secure bit isn't

burned.

Arghh. Puke. Now I remember this, and I wish I didn't...

...

...

Is that on all revisions, or just the revB ?

It's the A80, but I'm guessing the same applies to the A83T. It's

more

...

of a guess really, but I think it's a logical one. If the e-fuse

isn't

...

programmed, the TZPC doesn't work, and access to all secure

peripherals

...

still work, even from non-secure mode. The only one that does work is the secure SRAM.

The GIC still has the banked secure/non-secure registers, just that

all

...

cores access the secure bank, even when in non-secure mode. The

workaround

...

is to use the alias set of non-secure registers in Linux.

That's a pretty dire workaround. Also, I expect that secure writes to GICV/GICH will not do the right thing. At this point, what is the requirement for running non-secure?

Write Secure Boot eFUSE, which will break *all* existing softwares.

Don't do it, then.

Any other *real* use case for running non-secure? As in "Stuff that would benefit to a user"? Because if the answer is "none" as I suspect it is, you might as well keep the system in secure mode.

The initial idea was to use PSCI for the core bringup, which afaik requires Linux to run non-secure, right?

If that is so fundamentally broken that this is the only benefit, I guess we might as well use some old-style SMP ops.

Maxime

On Fri, Jun 23, 2017 at 09:24:25PM +0800, icenowy@aosc.io wrote:

在 2017-06-07 20:51，Marc Zyngier 写道：

...

On 07/06/17 13:12, Icenowy Zheng wrote:

...

于 2017年6月7日 GMT+08:00 下午8:11:12, Marc Zyngier marc.zyngier@arm.com 写到:

...

On 07/06/17 08:00, Chen-Yu Tsai wrote:

...

On Wed, Jun 7, 2017 at 2:50 PM, Maxime Ripard maxime.ripard@free-electrons.com wrote:

...

On Wed, Jun 07, 2017 at 11:47:24AM +0800, Chen-Yu Tsai wrote: > On Wed, Jun 7, 2017 at 11:40 AM, Icenowy Zheng icenowy@aosc.io

wrote:

...

...

> > > > > > 于 2017年6月7日 GMT+08:00 上午11:36:27, Chen-Yu > > Tsai wens@csie.org 写到: > > > On Wed, Jun 7, 2017 at 8:47 AM, Icenowy Zheng icenowy@aosc.io

wrote:

...

...

> > > > As we have now a basical implementation > > > > of PSCI for A83T, enable > > > > non-secure boot support and PSCI on A83T now. > > > > > > > > Signed-off-by: Icenowy Zheng icenowy@aosc.io > > > > --- > > > > arch/arm/mach-sunxi/Kconfig | 4 ++++ > > > > 1 file changed, 4 insertions(+) > > > > > > > > diff --git a/arch/arm/mach-sunxi/Kconfig > > > b/arch/arm/mach-sunxi/Kconfig > > > > index 7ced838d6a..31d29de428 100644 > > > > --- a/arch/arm/mach-sunxi/Kconfig > > > > +++ b/arch/arm/mach-sunxi/Kconfig > > > > @@ -98,8 +98,12 @@ config MACH_SUN8I_A33 > > > > config MACH_SUN8I_A83T > > > > bool "sun8i (Allwinner A83T)" > > > > select CPU_V7 > > > > + select CPU_V7_HAS_NONSEC > > > > + select CPU_V7_HAS_VIRT > > > > + select ARCH_SUPPORT_PSCI > > > > select SUNXI_GEN_SUN6I > > > > select SUPPORT_SPL > > > > + select ARMV7_BOOT_SEC_DEFAULT if > > > > OLD_SUNXI_KERNEL_COMPAT > > > > > > The kernel does not work yet. Please have it boot to secure by

default

...

...

> > > regardless of the kernel. We can have it > > > boot non-secure once the > > > kernel > > > has been working for a reasonable amount of time. > > > > > > I don't want clueless users coming and asking why it suddenly

stopped

...

...

> > > working. This should be an experimental feature. > > > > Maybe you should send out the fix, and tag them to also apply to > > stable tree. > > > > GIC is really broken, UP systems only work by chance. We > > shouldn't depend on this behavior. > > As I previously explained, it is not the GIC that is broken. I

believe

...

...

> the GIC is working exactly as it is supposed to with > regards to its > input signals. > > Allwinner's security extensions implementation simply does not

properly

...

...

> forward the AXI secure bit when the e-fuse's secure bit isn't

burned.

Arghh. Puke. Now I remember this, and I wish I didn't...

...

...

Is that on all revisions, or just the revB ?

It's the A80, but I'm guessing the same applies to the A83T. It's

more

...

of a guess really, but I think it's a logical one. If the e-fuse

isn't

...

programmed, the TZPC doesn't work, and access to all secure

peripherals

...

still work, even from non-secure mode. The only one that does work is the secure SRAM.

The GIC still has the banked secure/non-secure registers, just that

all

...

cores access the secure bank, even when in non-secure mode. The

workaround

...

is to use the alias set of non-secure registers in Linux.

That's a pretty dire workaround. Also, I expect that secure writes to GICV/GICH will not do the right thing. At this point, what is the requirement for running non-secure?

Write Secure Boot eFUSE, which will break *all* existing softwares.

Don't do it, then.

Any other *real* use case for running non-secure? As in "Stuff that would benefit to a user"? Because if the answer is "none" as I suspect it is, you might as well keep the system in secure mode.

Maybe we should then use legacy SMP bringup method (code in kernel) rather than PSCI?

I guess it all depends on the answer to Marc's question. If virtualization doesn't work, then we don't have any incentive anymore to use PSCI and that would be a sensible option, yes.

Maxime

On Fri, Jun 23, 2017 at 9:39 PM, icenowy@aosc.io wrote:

在 2017-06-23 21:35，Maxime Ripard 写道：

...

On Fri, Jun 23, 2017 at 09:24:25PM +0800, icenowy@aosc.io wrote:

...

在 2017-06-07 20:51，Marc Zyngier 写道：

...

On 07/06/17 13:12, Icenowy Zheng wrote:

...

于 2017年6月7日 GMT+08:00 下午8:11:12, Marc Zyngier marc.zyngier@arm.com 写到:

...

On 07/06/17 08:00, Chen-Yu Tsai wrote: > On Wed, Jun 7, 2017 at 2:50 PM, Maxime Ripard > maxime.ripard@free-electrons.com wrote: > > On Wed, Jun 07, 2017 at 11:47:24AM +0800, Chen-Yu Tsai wrote: > > > On Wed, Jun 7, 2017 at 11:40 AM, Icenowy Zheng > > > icenowy@aosc.io wrote: > > > > > > > > > > > > 于 2017年6月7日 GMT+08:00 上午11:36:27, Chen-Yu > > > > Tsai wens@csie.org 写到: > > > > > On Wed, Jun 7, 2017 at 8:47 AM, Icenowy Zheng > > > > > icenowy@aosc.io wrote: > > > > > > As we have now a basical implementation > > > > > > of PSCI for A83T, enable > > > > > > non-secure boot support and PSCI on A83T now. > > > > > > > > > > > > Signed-off-by: Icenowy Zheng icenowy@aosc.io > > > > > > --- > > > > > > arch/arm/mach-sunxi/Kconfig | 4 ++++ > > > > > > 1 file changed, 4 insertions(+) > > > > > > > > > > > > diff --git a/arch/arm/mach-sunxi/Kconfig > > > > > b/arch/arm/mach-sunxi/Kconfig > > > > > > index 7ced838d6a..31d29de428 100644 > > > > > > --- a/arch/arm/mach-sunxi/Kconfig > > > > > > +++ b/arch/arm/mach-sunxi/Kconfig > > > > > > @@ -98,8 +98,12 @@ config MACH_SUN8I_A33 > > > > > > config MACH_SUN8I_A83T > > > > > > bool "sun8i (Allwinner A83T)" > > > > > > select CPU_V7 > > > > > > + select CPU_V7_HAS_NONSEC > > > > > > + select CPU_V7_HAS_VIRT > > > > > > + select ARCH_SUPPORT_PSCI > > > > > > select SUNXI_GEN_SUN6I > > > > > > select SUPPORT_SPL > > > > > > + select ARMV7_BOOT_SEC_DEFAULT if > > > > > > OLD_SUNXI_KERNEL_COMPAT > > > > > > > > > > The kernel does not work yet. Please have it boot to > > > > > secure by default > > > > > regardless of the kernel. We can have it > > > > > boot non-secure once the > > > > > kernel > > > > > has been working for a reasonable amount of time. > > > > > > > > > > I don't want clueless users coming and asking why it > > > > > suddenly stopped > > > > > working. This should be an experimental feature. > > > > > > > > Maybe you should send out the fix, and tag them to also > > > > apply to > > > > stable tree. > > > > > > > > GIC is really broken, UP systems only work by chance. We > > > > shouldn't depend on this behavior. > > > > > > As I previously explained, it is not the GIC that is broken. > > > I believe > > > the GIC is working exactly as it is supposed to with > > > regards to its > > > input signals. > > > > > > Allwinner's security extensions implementation simply does > > > not properly > > > forward the AXI secure bit when the e-fuse's secure bit isn't burned.

Arghh. Puke. Now I remember this, and I wish I didn't...

> > Is that on all revisions, or just the revB ? > > It's the A80, but I'm guessing the same applies to the A83T. It's more > of a guess really, but I think it's a logical one. If the e-fuse isn't > programmed, the TZPC doesn't work, and access to all secure peripherals > still work, even from non-secure mode. The only one that > does work is > the secure SRAM. > > The GIC still has the banked secure/non-secure registers, just > that all > cores access the secure bank, even when in non-secure mode. The workaround > is to use the alias set of non-secure registers in Linux.

That's a pretty dire workaround. Also, I expect that secure writes to GICV/GICH will not do the right thing. At this point, what is the requirement for running non-secure?

Write Secure Boot eFUSE, which will break *all* existing softwares.

Don't do it, then.

Any other *real* use case for running non-secure? As in "Stuff that would benefit to a user"? Because if the answer is "none" as I suspect it is, you might as well keep the system in secure mode.

Maybe we should then use legacy SMP bringup method (code in kernel) rather than PSCI?

I guess it all depends on the answer to Marc's question. If virtualization doesn't work, then we don't have any incentive anymore to use PSCI and that would be a sensible option, yes.

I remember non-secure is a dependency for virtualization (HYP mode).

So if we do not do the workaround on GIC, we won't have stable non-secure, then we won't have HYP mode, then we can drop PSCI.

I think you got it the other way around.

If virtualization doesn't work, despite the workaround, then there's no need for it, and we can just do legacy SMP.

ChenYu

On Sun, 2 Jul 2017 15:08:12 +0800 icenowy@aosc.io wrote:

在 2017-06-23 21:50，Chen-Yu Tsai 写道：

...

On Fri, Jun 23, 2017 at 9:39 PM, icenowy@aosc.io wrote:

...

在 2017-06-23 21:35，Maxime Ripard 写道：

...

On Fri, Jun 23, 2017 at 09:24:25PM +0800, icenowy@aosc.io wrote:

...

在 2017-06-07 20:51，Marc Zyngier 写道：

...

On 07/06/17 13:12, Icenowy Zheng wrote: > > > 于 2017年6月7日 GMT+08:00 下午8:11:12, Marc Zyngier > marc.zyngier@arm.com 写到: > > On 07/06/17 08:00, Chen-Yu Tsai wrote: > > > On Wed, Jun 7, 2017 at 2:50 PM, Maxime Ripard > > > maxime.ripard@free-electrons.com wrote: > > > > On Wed, Jun 07, 2017 at 11:47:24AM +0800, Chen-Yu Tsai wrote: > > > > > On Wed, Jun 7, 2017 at 11:40 AM, Icenowy Zheng > > > > > icenowy@aosc.io > > wrote: > > > > > > > > > > > > > > > > > > 于 2017年6月7日 GMT+08:00 上午11:36:27, Chen-Yu > > > > > > Tsai wens@csie.org 写到: > > > > > > > On Wed, Jun 7, 2017 at 8:47 AM, Icenowy Zheng > > > > > > > icenowy@aosc.io > > wrote: > > > > > > > > As we have now a basical implementation > > > > > > > > of PSCI for A83T, enable > > > > > > > > non-secure boot support and PSCI on A83T now. > > > > > > > > > > > > > > > > Signed-off-by: Icenowy Zheng icenowy@aosc.io > > > > > > > > --- > > > > > > > > arch/arm/mach-sunxi/Kconfig | 4 ++++ > > > > > > > > 1 file changed, 4 insertions(+) > > > > > > > > > > > > > > > > diff --git a/arch/arm/mach-sunxi/Kconfig > > > > > > > b/arch/arm/mach-sunxi/Kconfig > > > > > > > > index 7ced838d6a..31d29de428 100644 > > > > > > > > --- a/arch/arm/mach-sunxi/Kconfig > > > > > > > > +++ b/arch/arm/mach-sunxi/Kconfig > > > > > > > > @@ -98,8 +98,12 @@ config MACH_SUN8I_A33 > > > > > > > > config MACH_SUN8I_A83T > > > > > > > > bool "sun8i (Allwinner A83T)" > > > > > > > > select CPU_V7 > > > > > > > > + select CPU_V7_HAS_NONSEC > > > > > > > > + select CPU_V7_HAS_VIRT > > > > > > > > + select ARCH_SUPPORT_PSCI > > > > > > > > select SUNXI_GEN_SUN6I > > > > > > > > select SUPPORT_SPL > > > > > > > > + select ARMV7_BOOT_SEC_DEFAULT if > > > > > > > > OLD_SUNXI_KERNEL_COMPAT > > > > > > > > > > > > > > The kernel does not work yet. Please have it boot to > > > > > > > secure by > > default > > > > > > > regardless of the kernel. We can have it > > > > > > > boot non-secure once the > > > > > > > kernel > > > > > > > has been working for a reasonable amount of time. > > > > > > > > > > > > > > I don't want clueless users coming and asking why it > > > > > > > suddenly > > stopped > > > > > > > working. This should be an experimental feature. > > > > > > > > > > > > Maybe you should send out the fix, and tag them to also > > > > > > apply to > > > > > > stable tree. > > > > > > > > > > > > GIC is really broken, UP systems only work by chance. We > > > > > > shouldn't depend on this behavior. > > > > > > > > > > As I previously explained, it is not the GIC that is broken. > > > > > I > > believe > > > > > the GIC is working exactly as it is supposed to with > > > > > regards to its > > > > > input signals. > > > > > > > > > > Allwinner's security extensions implementation simply does > > > > > not > > properly > > > > > forward the AXI secure bit when the e-fuse's secure bit isn't > > burned. > > > > Arghh. Puke. Now I remember this, and I wish I didn't... > > > > > > Is that on all revisions, or just the revB ? > > > > > > It's the A80, but I'm guessing the same applies to the A83T. It's > > more > > > of a guess really, but I think it's a logical one. If the e-fuse > > isn't > > > programmed, the TZPC doesn't work, and access to all secure > > peripherals > > > still work, even from non-secure mode. The only one that > > > does work is > > > the secure SRAM. > > > > > > The GIC still has the banked secure/non-secure registers, just > > > that > > all > > > cores access the secure bank, even when in non-secure mode. The > > workaround > > > is to use the alias set of non-secure registers in Linux. > > > > That's a pretty dire workaround. Also, I expect that secure writes > > to > > GICV/GICH will not do the right thing. At this point, what is the > > requirement for running non-secure? > > Write Secure Boot eFUSE, which will break *all* existing softwares.

Don't do it, then.

Any other *real* use case for running non-secure? As in "Stuff that would benefit to a user"? Because if the answer is "none" as I suspect it is, you might as well keep the system in secure mode.

Maybe we should then use legacy SMP bringup method (code in kernel) rather than PSCI?

I guess it all depends on the answer to Marc's question. If virtualization doesn't work, then we don't have any incentive anymore to use PSCI and that would be a sensible option, yes.

I remember non-secure is a dependency for virtualization (HYP mode).

So if we do not do the workaround on GIC, we won't have stable non-secure, then we won't have HYP mode, then we can drop PSCI.

I think you got it the other way around.

If virtualization doesn't work, despite the workaround, then there's no need for it, and we can just do legacy SMP.

I tried `qemu-system-arm -enable-kvm` on A83T with this patchset and Chen-Yu's GIC workaround patchset, and *FAILED*.

The workaround patchset in fact slightly broke vGIC code by changing a macro name -- it's easy to fix.

However, it seems that with this fixed the KVM cannot still work -- I tried to start a virtual machine, but it silently fails (no kernel log are shown when the VM starting fails).

So, at least this workaround cannot let virtualization work.

Before discounting it altogether, it'd be interesting to find out what breaks exactly. How did you start the guest? What is the failure mode?

Thanks,

M.

On Sun, 2 Jul 2017 20:36:04 +0800 icenowy@aosc.io wrote:

在 2017-07-02 19:22，Marc Zyngier 写道：

...

On Sun, 2 Jul 2017 15:08:12 +0800 icenowy@aosc.io wrote:

...

在 2017-06-23 21:50，Chen-Yu Tsai 写道：

...

On Fri, Jun 23, 2017 at 9:39 PM, icenowy@aosc.io wrote:

...

在 2017-06-23 21:35，Maxime Ripard 写道：

...

On Fri, Jun 23, 2017 at 09:24:25PM +0800, icenowy@aosc.io wrote: > > 在 2017-06-07 20:51，Marc Zyngier 写道： > > On 07/06/17 13:12, Icenowy Zheng wrote: > > > > > > > > > 于 2017年6月7日 GMT+08:00 下午8:11:12, Marc Zyngier > > > marc.zyngier@arm.com 写到: > > > > On 07/06/17 08:00, Chen-Yu Tsai wrote: > > > > > On Wed, Jun 7, 2017 at 2:50 PM, Maxime Ripard > > > > > maxime.ripard@free-electrons.com wrote: > > > > > > On Wed, Jun 07, 2017 at 11:47:24AM +0800, Chen-Yu Tsai wrote: > > > > > > > On Wed, Jun 7, 2017 at 11:40 AM, Icenowy Zheng > > > > > > > icenowy@aosc.io > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > 于 2017年6月7日 GMT+08:00 上午11:36:27, Chen-Yu > > > > > > > > Tsai wens@csie.org 写到: > > > > > > > > > On Wed, Jun 7, 2017 at 8:47 AM, Icenowy Zheng > > > > > > > > > icenowy@aosc.io > > > > wrote: > > > > > > > > > > As we have now a basical implementation > > > > > > > > > > of PSCI for A83T, enable > > > > > > > > > > non-secure boot support and PSCI on A83T now. > > > > > > > > > > > > > > > > > > > > Signed-off-by: Icenowy Zheng icenowy@aosc.io > > > > > > > > > > --- > > > > > > > > > > arch/arm/mach-sunxi/Kconfig | 4 ++++ > > > > > > > > > > 1 file changed, 4 insertions(+) > > > > > > > > > > > > > > > > > > > > diff --git a/arch/arm/mach-sunxi/Kconfig > > > > > > > > > b/arch/arm/mach-sunxi/Kconfig > > > > > > > > > > index 7ced838d6a..31d29de428 100644 > > > > > > > > > > --- a/arch/arm/mach-sunxi/Kconfig > > > > > > > > > > +++ b/arch/arm/mach-sunxi/Kconfig > > > > > > > > > > @@ -98,8 +98,12 @@ config MACH_SUN8I_A33 > > > > > > > > > > config MACH_SUN8I_A83T > > > > > > > > > > bool "sun8i (Allwinner A83T)" > > > > > > > > > > select CPU_V7 > > > > > > > > > > + select CPU_V7_HAS_NONSEC > > > > > > > > > > + select CPU_V7_HAS_VIRT > > > > > > > > > > + select ARCH_SUPPORT_PSCI > > > > > > > > > > select SUNXI_GEN_SUN6I > > > > > > > > > > select SUPPORT_SPL > > > > > > > > > > + select ARMV7_BOOT_SEC_DEFAULT if > > > > > > > > > > OLD_SUNXI_KERNEL_COMPAT > > > > > > > > > > > > > > > > > > The kernel does not work yet. Please have it boot to > > > > > > > > > secure by > > > > default > > > > > > > > > regardless of the kernel. We can have it > > > > > > > > > boot non-secure once the > > > > > > > > > kernel > > > > > > > > > has been working for a reasonable amount of time. > > > > > > > > > > > > > > > > > > I don't want clueless users coming and asking why it > > > > > > > > > suddenly > > > > stopped > > > > > > > > > working. This should be an experimental feature. > > > > > > > > > > > > > > > > Maybe you should send out the fix, and tag them to also > > > > > > > > apply to > > > > > > > > stable tree. > > > > > > > > > > > > > > > > GIC is really broken, UP systems only work by chance. We > > > > > > > > shouldn't depend on this behavior. > > > > > > > > > > > > > > As I previously explained, it is not the GIC that is broken. > > > > > > > I > > > > believe > > > > > > > the GIC is working exactly as it is supposed to with > > > > > > > regards to its > > > > > > > input signals. > > > > > > > > > > > > > > Allwinner's security extensions implementation simply does > > > > > > > not > > > > properly > > > > > > > forward the AXI secure bit when the e-fuse's secure bit isn't > > > > burned. > > > > > > > > Arghh. Puke. Now I remember this, and I wish I didn't... > > > > > > > > > > Is that on all revisions, or just the revB ? > > > > > > > > > > It's the A80, but I'm guessing the same applies to the A83T. It's > > > > more > > > > > of a guess really, but I think it's a logical one. If the e-fuse > > > > isn't > > > > > programmed, the TZPC doesn't work, and access to all secure > > > > peripherals > > > > > still work, even from non-secure mode. The only one that > > > > > does work is > > > > > the secure SRAM. > > > > > > > > > > The GIC still has the banked secure/non-secure registers, just > > > > > that > > > > all > > > > > cores access the secure bank, even when in non-secure mode. The > > > > workaround > > > > > is to use the alias set of non-secure registers in Linux. > > > > > > > > That's a pretty dire workaround. Also, I expect that secure writes > > > > to > > > > GICV/GICH will not do the right thing. At this point, what is the > > > > requirement for running non-secure? > > > > > > Write Secure Boot eFUSE, which will break *all* existing softwares. > > > > Don't do it, then. > > > > Any other *real* use case for running non-secure? As in "Stuff that > > would benefit to a user"? Because if the answer is "none" as I suspect > > it is, you might as well keep the system in secure mode. > > Maybe we should then use legacy SMP bringup method (code in kernel) > rather than PSCI?

I guess it all depends on the answer to Marc's question. If virtualization doesn't work, then we don't have any incentive anymore to use PSCI and that would be a sensible option, yes.

I remember non-secure is a dependency for virtualization (HYP mode).

So if we do not do the workaround on GIC, we won't have stable non-secure, then we won't have HYP mode, then we can drop PSCI.

I think you got it the other way around.

If virtualization doesn't work, despite the workaround, then there's no need for it, and we can just do legacy SMP.

I tried `qemu-system-arm -enable-kvm` on A83T with this patchset and Chen-Yu's GIC workaround patchset, and *FAILED*.

The workaround patchset in fact slightly broke vGIC code by changing a macro name -- it's easy to fix.

However, it seems that with this fixed the KVM cannot still work -- I tried to start a virtual machine, but it silently fails (no kernel log are shown when the VM starting fails).

So, at least this workaround cannot let virtualization work.

Before discounting it altogether, it'd be interesting to find out what breaks exactly. How did you start the guest? What is the failure mode?

Oh, I'm sorry. I used wrong command for it... (forgot -cpu host parameter)

Now it works.

Virtual machine boot log is available at [1].

[1] https://pastebin.anthonos.org/view/66a9ca54

So it may be valuable to apply the workaround now?

It definitely requires extra testing (booting the kernel hardly shakes the GIC), but that's certainly encouraging. Can you run some significant workload in a guest (kernel compilation, hackbench) and report the results?

If the above shows that nothing bad happens, I'd like to see a full picture of the required changes on the host kernel side and see how we can support this in a non-invasive way.

Thanks,

M.