* [PATCH 0/4] Add smp support for Allwinner A20 and phy arch count timer
@ 2013-09-12 6:51 Fan Rong
2013-09-12 6:51 ` [PATCH 1/4] Add smp support for Allwinner A20(sunxi 7i) Fan Rong
` (4 more replies)
0 siblings, 5 replies; 31+ messages in thread
From: Fan Rong @ 2013-09-12 6:51 UTC (permalink / raw)
To: linux-arm-kernel
The patchs add smp support for Allwinner A20. It add cpuregister node in dts forsmp configure. The patchs also add a options for phy count timer to replace vir count timer as ARM arch timer clocksource. About ARM arch timer: 1. Current kernel use vir count timer, vir count timer can be accessed in any cpu mode for kernel, but it need bootloader set vir count offset rigister zero at first. 2. Phy count timer can be accessed in most cpu mode for kernel except NS-PL1 mode when register CNTHCTL.PL1PCTEN is set to zero. To ensure to use phy count timer, bootloader should set register CNTHCTL.PL1PCTEN is 1 at first. At all, to ensure kernel can use arch timer, bootload should set some generic timer register(cntvoff or cnthctl) at first. the kernel should select which count timer by reading current kernel running mode.
Fan Rong (4):
Add smp support for Allwinner A20(sunxi 7i).
Add cpuconfig nodes in dts for smp configure.
Add physical count arch timer support for clocksource in ARMv7.
Add arch count timer node in dts for Allwinner A20(sunxi 7i).
arch/arm/boot/dts/sun7i-a20.dtsi | 18 +-
arch/arm/include/asm/arch_timer.h | 11 ++
arch/arm/mach-sunxi/Makefile | 2 +
arch/arm/mach-sunxi/headsmp.S | 12 ++
arch/arm/mach-sunxi/platform.h | 346 ++++++++++++++++++++++++++++++++++
arch/arm/mach-sunxi/platsmp.c | 166 ++++++++++++++++
arch/arm/mach-sunxi/sunxi.c | 4 +
drivers/clocksource/Kconfig | 8 +
drivers/clocksource/arm_arch_timer.c | 10 +-
9 files changed, 574 insertions(+), 3 deletions(-)
create mode 100644 arch/arm/mach-sunxi/headsmp.S
create mode 100644 arch/arm/mach-sunxi/platform.h
create mode 100644 arch/arm/mach-sunxi/platsmp.c
--
1.7.9.5
^ permalink raw reply [flat|nested] 31+ messages in thread* [PATCH 1/4] Add smp support for Allwinner A20(sunxi 7i). 2013-09-12 6:51 [PATCH 0/4] Add smp support for Allwinner A20 and phy arch count timer Fan Rong @ 2013-09-12 6:51 ` Fan Rong 2013-09-12 14:26 ` Mark Rutland 2013-09-12 14:40 ` Russell King - ARM Linux 2013-09-12 6:51 ` [PATCH 2/4] Add cpuconfig nodes in dts for smp configure Fan Rong ` (3 subsequent siblings) 4 siblings, 2 replies; 31+ messages in thread From: Fan Rong @ 2013-09-12 6:51 UTC (permalink / raw) To: linux-arm-kernel Signed-off-by: Fan Rong <cinifr@gmail.com> --- arch/arm/mach-sunxi/Makefile | 2 + arch/arm/mach-sunxi/headsmp.S | 12 ++ arch/arm/mach-sunxi/platform.h | 346 ++++++++++++++++++++++++++++++++++++++++ arch/arm/mach-sunxi/platsmp.c | 166 +++++++++++++++++++ arch/arm/mach-sunxi/sunxi.c | 4 + 5 files changed, 530 insertions(+) create mode 100644 arch/arm/mach-sunxi/headsmp.S create mode 100644 arch/arm/mach-sunxi/platform.h create mode 100644 arch/arm/mach-sunxi/platsmp.c diff --git a/arch/arm/mach-sunxi/Makefile b/arch/arm/mach-sunxi/Makefile index 93bebfc..d7f1ef4 100644 --- a/arch/arm/mach-sunxi/Makefile +++ b/arch/arm/mach-sunxi/Makefile @@ -1 +1,3 @@ obj-$(CONFIG_ARCH_SUNXI) += sunxi.o +obj-$(CONFIG_ARCH_SUNXI) += platsmp.o +obj-$(CONFIG_ARCH_SUNXI) += headsmp.o diff --git a/arch/arm/mach-sunxi/headsmp.S b/arch/arm/mach-sunxi/headsmp.S new file mode 100644 index 0000000..b400602 --- /dev/null +++ b/arch/arm/mach-sunxi/headsmp.S @@ -0,0 +1,12 @@ +#include <linux/linkage.h> +#include <linux/init.h> + + .section ".text.head", "ax" + __CPUINIT + +ENTRY(sun7i_secondary_startup) + msr cpsr_fsxc, #0xd3 + mov r0, #0 + ldr r1, =0xffffffff + b secondary_startup +ENDPROC(sun7i_secondary_startup) diff --git a/arch/arm/mach-sunxi/platform.h b/arch/arm/mach-sunxi/platform.h new file mode 100644 index 0000000..3f80d22 --- /dev/null +++ b/arch/arm/mach-sunxi/platform.h @@ -0,0 +1,346 @@ +/* + * arch/arm/plat-sunxi/include/plat/platform.h + * + * (C) Copyright 2007-2012 + * Allwinner Technology Co., Ltd. <www.allwinnertech.com> + * Benn Huang <benn@allwinnertech.com> + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License as + * published by the Free Software Foundation; either version 2 of + * the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, + * MA 02111-1307 USA + */ +extern struct smp_operations sunxi7i_smp_ops; +void sun7i_secondary_startup(void); + +#ifndef __SW_PLATFORM_H +#define __SW_PLATFORM_H + + + +/* Physical Address */ +#define SW_PA_BROM_START 0xffff0000 +#define SW_PA_BROM_END 0xffff7fff /* 32KB */ + +#define SW_PA_SRAM_BASE 0x00000000 + +/* sun7i sram addresses */ +#define SW_PA_SRAM_A1_BASE 0x00000000 +#define SW_PA_SRAM_A2_BASE 0x00004000 +#define SW_PA_SRAM_A3_BASE 0x00008000 +#define SW_PA_SRAM_A4_BASE 0x0000b400 +#define SW_PA_SRAM_D_BASE 0x00010000 +#define SW_PA_SRAM_B_BASE 0x00020000 + +#define SW_PA_SDRAM_START 0x40000000 +#define SW_PA_IO_BASE 0x01c00000 +#define SW_PA_SRAM_IO_BASE 0x01c00000 /* 4KB */ +#define SW_PA_DRAM_IO_BASE 0x01c01000 +#define SW_PA_DMAC_IO_BASE 0x01c02000 +#define SW_PA_NANDFLASHC_IO_BASE 0x01c03000 +#define SW_PA_TSI_IO_BASE 0x01c04000 +#define SW_PA_SPI0_IO_BASE 0x01c05000 +#define SW_PA_SPI1_IO_BASE 0x01c06000 +#define SW_PA_MSCC_IO_BASE 0x01c07000 +#define SW_PA_TVD_IO_BASE 0x01c08000 +#define SW_PA_CSI0_IO_BASE 0x01c09000 +#define SW_PA_TVE_IO_BASE 0x01c0a000 +#define SW_PA_EMAC_IO_BASE 0x01c0b000 +#define SW_PA_TCON0_IO_BASE 0x01c0c000 +#define SW_PA_TCON1_IO_BASE 0x01c0d000 +#define SW_PA_VE_IO_BASE 0x01c0e000 +#define SW_PA_SDC0_IO_BASE 0x01c0f000 +#define SW_PA_SDC1_IO_BASE 0x01c10000 +#define SW_PA_SDC2_IO_BASE 0x01c11000 +#define SW_PA_SDC3_IO_BASE 0x01c12000 +#define SW_PA_USB0_IO_BASE 0x01c13000 +#define SW_PA_USB1_IO_BASE 0x01c14000 +#define SW_PA_SSE_IO_BASE 0x01c15000 +#define SW_PA_HDMI_IO_BASE 0x01c16000 +#define SW_PA_SPI2_IO_BASE 0x01c17000 +#define SW_PA_SATA_IO_BASE 0x01c18000 +#define SW_PA_PATA_IO_BASE 0x01c19000 +#define SW_PA_ACE_IO_BASE 0x01c1a000 +#define SW_PA_TVE1_IO_BASE 0x01c1b000 +#define SW_PA_USB2_IO_BASE 0x01c1c000 +#define SW_PA_CSI1_IO_BASE 0x01c1d000 +#define SW_PA_TZASC_IO_BASE 0x01c1e000 +#define SW_PA_SPI3_IO_BASE 0x01c1f000 +#define SW_PA_CCM_IO_BASE 0x01c20000 +#define SW_PA_INT_IO_BASE 0x01c20400 +#define SW_PA_PORTC_IO_BASE 0x01c20800 +#define SW_PA_TIMERC_IO_BASE 0x01c20c00 +#define SW_PA_SPDIF_IO_BASE 0x01c21000 +#define SW_PA_AC97_IO_BASE 0x01c21400 +#define SW_PA_IR0_IO_BASE 0x01c21800 +#define SW_PA_IR1_IO_BASE 0x01c21c00 +#define SW_PA_IIS1_IO_BASE 0x01c22000 +#define SW_PA_IIS_IO_BASE 0x01c22400 +#define SW_PA_LRADC_IO_BASE 0x01c22800 +#define SW_PA_ADDA_IO_BASE 0x01c22c00 +#define SW_PA_KEYPAD_IO_BASE 0x01c23000 +#define SW_PA_TZPC_IO_BASE 0x01c23400 +#define SW_PA_SID_IO_BASE 0x01c23800 +#define SW_PA_SJTAG_IO_BASE 0x01c23c00 +#define SW_PA_IIS2_IO_BASE 0x01c24400 +#define SW_PA_TP_IO_BASE 0x01c25000 +#define SW_PA_PMU_IO_BASE 0x01c25400 +#define SW_PA_CPUCFG_IO_BASE 0x01c25c00 +#define SW_PA_UART0_IO_BASE 0x01c28000 +#define SW_PA_UART1_IO_BASE 0x01c28400 +#define SW_PA_UART2_IO_BASE 0x01c28800 +#define SW_PA_UART3_IO_BASE 0x01c28c00 +#define SW_PA_UART4_IO_BASE 0x01c29000 +#define SW_PA_UART5_IO_BASE 0x01c29400 +#define SW_PA_UART6_IO_BASE 0x01c29800 +#define SW_PA_UART7_IO_BASE 0x01c29c00 +#define SW_PA_PS20_IO_BASE 0x01c2a000 +#define SW_PA_PS21_IO_BASE 0x01c2a400 +#define SW_PA_TWI0_IO_BASE 0x01c2ac00 +#define SW_PA_TWI1_IO_BASE 0x01c2b000 +#define SW_PA_TWI2_IO_BASE 0x01c2b400 +#define SW_PA_TWI3_IO_BASE 0x01c2b800 +#define SW_PA_CAN0_IO_BASE 0x01c2bc00 +#define SW_PA_CAN1_IO_BASE 0x01c2c000 /* sun4i */ +#define SW_PA_TWI4_IO_BASE 0x01c2c000 /* sun7i */ +#define SW_PA_SCR_IO_BASE 0x01c2c400 +#define SW_PA_GPS_IO_BASE 0x01c30000 +#define SW_PA_MALI_IO_BASE 0x01c40000 +#define SW_PA_GMAC_IO_BASE 0x01c50000 +#define SW_PA_HSTIMER_IO_BASE 0x01c60000 +#define SW_PA_GIC_IO_BASE 0x01c80000 +#define SW_PA_GIC_DIST_IO_BASE 0x01c81000 +#define SW_PA_GIC_CPU_IO_BASE 0x01c82000 +#define SW_PA_SCU_IO_BASE 0x01c80000 + +#define SW_PA_TIMER_G_IO_BASE 0x01c80200 +/* CPU global timer, not used */ + +#define SW_PA_TIMER_P_IO_BASE 0x01c80600 +/* CPU private timer, not used */ + +#define SW_PA_HDMI1_IO_BASE 0x01ce0000 +#define SW_PA_SRAM_C_IO_BASE 0x01d00000 +#define SW_PA_DEFE0_IO_BASE 0x01e00000 +#define SW_PA_DEFE1_IO_BASE 0x01e20000 +#define SW_PA_DEBE0_IO_BASE 0x01e60000 +#define SW_PA_DEBE1_IO_BASE 0x01e40000 +#define SW_PA_MP_IO_BASE 0x01e80000 +#define SW_PA_AVG_IO_BASE 0x01ea0000 +#define SW_PA_CPUBIST_IO_BASE 0x3f501000 +#define SW_PA_BROM_BASE 0xffff0000 + +/* Virtual Address */ +#define SW_VA_SRAM_BASE 0xf0000000 /*16KB*/ + +/* sun7i sram addresses */ +#define SW_VA_SRAM_A1_BASE 0xf0000000 +#define SW_VA_SRAM_A2_BASE 0xf0004000 +#define SW_VA_SRAM_A3_BASE 0xf0008000 +#define SW_VA_SRAM_A4_BASE 0xf000b400 +#define SW_VA_SRAM_D_BASE 0xf0010000 +#define SW_VA_SRAM_B_BASE 0xf0020000 + +#define SW_VA_BROM_BASE 0xf0100000 /*64KB*/ +#define SW_VA_BROM_START 0xf0100000 +#define SW_VA_BROM_END 0xf0107fff + +#define SW_VA_IO_BASE 0xf1c00000 +#define SW_VA_SRAM_IO_BASE 0xf1c00000 /* 4KB */ +#define SW_VA_DRAM_IO_BASE 0xf1c01000 +#define SW_VA_DMAC_IO_BASE 0xf1c02000 +#define SW_VA_NANDFLASHC_IO_BASE 0xf1c03000 +#define SW_VA_TSI_IO_BASE 0xf1c04000 +#define SW_VA_SPI0_IO_BASE 0xf1c05000 +#define SW_VA_SPI1_IO_BASE 0xf1c06000 +#define SW_VA_MSCC_IO_BASE 0xf1c07000 +#define SW_VA_TVD_IO_BASE 0xf1c08000 +#define SW_VA_CSI0_IO_BASE 0xf1c09000 +#define SW_VA_TVE_IO_BASE 0xf1c0a000 +#define SW_VA_EMAC_IO_BASE 0xf1c0b000 +#define SW_VA_TCON0_IO_BASE 0xf1c0c000 +#define SW_VA_TCON1_IO_BASE 0xf1c0d000 +#define SW_VA_VE_IO_BASE 0xf1c0e000 +#define SW_VA_SDC0_IO_BASE 0xf1c0f000 +#define SW_VA_SDC1_IO_BASE 0xf1c10000 +#define SW_VA_SDC2_IO_BASE 0xf1c11000 +#define SW_VA_SDC3_IO_BASE 0xf1c12000 +#define SW_VA_USB0_IO_BASE 0xf1c13000 +#define SW_VA_USB1_IO_BASE 0xf1c14000 +#define SW_VA_SSE_IO_BASE 0xf1c15000 +#define SW_VA_HDMI_IO_BASE 0xf1c16000 +#define SW_VA_SPI2_IO_BASE 0xf1c17000 +#define SW_VA_SATA_IO_BASE 0xf1c18000 +#define SW_VA_PATA_IO_BASE 0xf1c19000 +#define SW_VA_ACE_IO_BASE 0xf1c1a000 +#define SW_VA_TVE1_IO_BASE 0xf1c1b000 +#define SW_VA_USB2_IO_BASE 0xf1c1c000 +#define SW_VA_CSI1_IO_BASE 0xf1c1d000 +#define SW_VA_TZASC_IO_BASE 0xf1c1e000 +#define SW_VA_SPI3_IO_BASE 0xf1c1f000 +#define SW_VA_CCM_IO_BASE 0xf1c20000 +#define SW_VA_INT_IO_BASE 0xf1c20400 +#define SW_VA_PORTC_IO_BASE 0xf1c20800 +#define SW_VA_TIMERC_IO_BASE 0xf1c20c00 +#define SW_VA_SPDIF_IO_BASE 0xf1c21000 +#define SW_VA_AC97_IO_BASE 0xf1c21400 +#define SW_VA_IR0_IO_BASE 0xf1c21800 +#define SW_VA_IR1_IO_BASE 0xf1c21c00 +#define SW_VA_IIS1_IO_BASE 0xf1c22000 +#define SW_VA_IIS_IO_BASE 0xf1c22400 +#define SW_VA_LRADC_IO_BASE 0xf1c22800 +#define SW_VA_ADDA_IO_BASE 0xf1c22c00 +#define SW_VA_KEYPAD_IO_BASE 0xf1c23000 +#define SW_VA_TZPC_IO_BASE 0xf1c23400 +#define SW_VA_SID_IO_BASE 0xf1c23800 +#define SW_VA_SJTAG_IO_BASE 0xf1c23c00 +#define SW_VA_IIS2_IO_BASE 0xf1c24400 +#define SW_VA_TP_IO_BASE 0xf1c25000 +#define SW_VA_PMU_IO_BASE 0xf1c25400 +#define SW_VA_CPUCFG_IO_BASE 0xf1c25c00 +#define SW_VA_UART0_IO_BASE 0xf1c28000 +#define SW_VA_UART1_IO_BASE 0xf1c28400 +#define SW_VA_UART2_IO_BASE 0xf1c28800 +#define SW_VA_UART3_IO_BASE 0xf1c28c00 +#define SW_VA_UART4_IO_BASE 0xf1c29000 +#define SW_VA_UART5_IO_BASE 0xf1c29400 +#define SW_VA_UART6_IO_BASE 0xf1c29800 +#define SW_VA_UART7_IO_BASE 0xf1c29c00 +#define SW_VA_PS20_IO_BASE 0xf1c2a000 +#define SW_VA_PS21_IO_BASE 0xf1c2a400 +#define SW_VA_TWI0_IO_BASE 0xf1c2ac00 +#define SW_VA_TWI1_IO_BASE 0xf1c2b000 +#define SW_VA_TWI2_IO_BASE 0xf1c2b400 +#define SW_VA_TWI3_IO_BASE 0xf1c2b800 +#define SW_VA_CAN0_IO_BASE 0xf1c2bc00 +#define SW_VA_CAN1_IO_BASE 0xf1c2c000 /* sun4i */ +#define SW_VA_TWI4_IO_BASE 0xf1c2c000 /* sun7i */ +#define SW_VA_SCR_IO_BASE 0xf1c2c400 +#define SW_VA_GPS_IO_BASE 0xf1c30000 +#define SW_VA_MALI_IO_BASE 0xf1c40000 +#define SW_VA_GMAC_IO_BASE 0xf1c50000 +#define SW_VA_HSTIMER_IO_BASE 0xf1c60000 +#define SW_VA_GIC_IO_BASE 0xf1c80000 +#define SW_VA_GIC_DIST_IO_BASE 0xf1c81000 +#define SW_VA_GIC_CPU_IO_BASE 0xf1c82000 +#define SW_VA_SCU_IO_BASE 0xf1c80000 + +#define SW_VA_TIMER_G_IO_BASE 0xf1c80200 +/* CPU global timer, not used */ + +#define SW_VA_TIMER_P_IO_BASE 0xf1c80600 +/* CPU private timer, not used */ + +#define SW_VA_HDMI1_IO_BASE 0xf1ce0000 +#define SW_VA_SRAM_C_IO_BASE 0xf1d00000 +#define SW_VA_DEFE0_IO_BASE 0xf1e00000 +#define SW_VA_DEFE1_IO_BASE 0xf1e20000 +#define SW_VA_DEBE0_IO_BASE 0xf1e60000 +#define SW_VA_DEBE1_IO_BASE 0xf1e40000 +#define SW_VA_MP_IO_BASE 0xf1e80000 +#define SW_VA_AVG_IO_BASE 0xf1ea0000 + +/* memory size */ +#define SW_IO_SIZE 0x00400000 /* 4MB(Max) */ +#define SW_SRAM_A1_SIZE 0x00004000 /* 16k */ +#define SW_SRAM_A2_SIZE 0x00004000 /* 16k */ +#define SW_SRAM_A3_SIZE 0x00003400 /* 13k */ +#define SW_SRAM_A4_SIZE 0x00000c00 /* 3k */ +#define SW_SRAM_D_SIZE 0x00001000 /* 4k */ +#define SW_SRAM_B_SIZE 0x00010000 /* 64k */ +#define SW_BROM_SIZE 0x00008000 /* 32k */ + +/** + * Interrupt controller registers + * + */ +#define SW_INT_VECTOR_REG (SW_VA_INT_IO_BASE + 0x00) +#define SW_INT_BASE_ADR_REG (SW_VA_INT_IO_BASE + 0x04) +#define SW_INT_PROTECTION_REG (SW_VA_INT_IO_BASE + 0x08) +#define SW_INT_NMI_CTRL_REG (SW_VA_INT_IO_BASE + 0x0c) +#define SW_INT_IRQ_PENDING_REG0 (SW_VA_INT_IO_BASE + 0x10) +#define SW_INT_IRQ_PENDING_REG1 (SW_VA_INT_IO_BASE + 0x14) +#define SW_INT_IRQ_PENDING_REG2 (SW_VA_INT_IO_BASE + 0x18) + +#define SW_INT_FIQ_PENDING_REG0 (SW_VA_INT_IO_BASE + 0x20) +#define SW_INT_FIQ_PENDING_REG1 (SW_VA_INT_IO_BASE + 0x24) +#define SW_INT_FIQ_PENDING_REG2 (SW_VA_INT_IO_BASE + 0x28) + +#define SW_INT_SELECT_REG0 (SW_VA_INT_IO_BASE + 0x30) +#define SW_INT_SELECT_REG1 (SW_VA_INT_IO_BASE + 0x34) +#define SW_INT_SELECT_REG2 (SW_VA_INT_IO_BASE + 0x38) + +#define SW_INT_ENABLE_REG0 (SW_VA_INT_IO_BASE + 0x40) +#define SW_INT_ENABLE_REG1 (SW_VA_INT_IO_BASE + 0x44) +#define SW_INT_ENABLE_REG2 (SW_VA_INT_IO_BASE + 0x48) + +#define SW_INT_MASK_REG0 (SW_VA_INT_IO_BASE + 0x50) +#define SW_INT_MASK_REG1 (SW_VA_INT_IO_BASE + 0x54) +#define SW_INT_MASK_REG2 (SW_VA_INT_IO_BASE + 0x58) + +#define SW_INT_RESP_REG0 (SW_VA_INT_IO_BASE + 0x60) +#define SW_INT_RESP_REG1 (SW_VA_INT_IO_BASE + 0x64) +#define SW_INT_RESP_REG2 (SW_VA_INT_IO_BASE + 0x68) + +#define SW_INT_FASTFORCE_REG0 (SW_VA_INT_IO_BASE + 0x70) +#define SW_INT_FASTFORCE_REG1 (SW_VA_INT_IO_BASE + 0x74) +#define SW_INT_FASTFORCE_REG2 (SW_VA_INT_IO_BASE + 0x78) + +#define SW_INT_SRCPRIO_REG0 (SW_VA_INT_IO_BASE + 0x80) +#define SW_INT_SRCPRIO_REG1 (SW_VA_INT_IO_BASE + 0x84) +#define SW_INT_SRCPRIO_REG2 (SW_VA_INT_IO_BASE + 0x88) +#define SW_INT_SRCPRIO_REG3 (SW_VA_INT_IO_BASE + 0x8c) +#define SW_INT_SRCPRIO_REG4 (SW_VA_INT_IO_BASE + 0x90) +#ifdef CONFIG_ARCH_SUN5I +#define SW_INT_SRCPRIO_REG5 (SW_VA_INT_IO_BASE + 0x94) +#endif + + +#define PA_VIC_BASE 0x01c20400 +#define VA_VIC_BASE IO_ADDRESS(PA_VIC_BASE) +#define PIO_BASE SW_PA_PORTC_IO_BASE + +/** +*@name DRAM controller register address +*@{ +*/ +#define SW_DRAM_SDR_CTL_REG (SW_VA_DRAM_IO_BASE + 0x0C) +#define SW_DRAM_SDR_DCR (SW_VA_DRAM_IO_BASE + 0x04) + +/* + * other reg defination, not found in spec + */ +#define AW_GIC_DIST_BASE 0x01c81000 +#define AW_GIC_CPU_BASE 0x01c82000 + +#define AW_TIMER_G_BASE 0x01c80200 +/* CPU global timer, not used */ + +#define AW_TIMER_P_BASE 0x01c80600 +/* CPU private timer, not used */ + +/* + * CPUCFG + */ +#define AW_CPUCFG_P_REG0 0x01a4 +#define CPUX_RESET_CTL(x) (0x40 + (x)*0x40) +#define CPUX_CONTROL(x) (0x44 + (x)*0x40) +#define CPUX_STATUS(x) (0x48 + (x)*0x40) +#define AW_CPUCFG_GENCTL 0x0184 +#define AW_CPUCFG_DBGCTL0 0x01e0 +#define AW_CPUCFG_DBGCTL1 0x01e4 + +#define AW_CPU1_PWR_CLAMP 0x01b0 +#define AW_CPU1_PWROFF_REG 0x01b4 + +#endif diff --git a/arch/arm/mach-sunxi/platsmp.c b/arch/arm/mach-sunxi/platsmp.c new file mode 100644 index 0000000..3bb6147 --- /dev/null +++ b/arch/arm/mach-sunxi/platsmp.c @@ -0,0 +1,166 @@ +/* + * linux/arch/arm/mach-sun7i/platsmp.c + * + * Copyright (C) 2012-2016 Allwinner Ltd. + * All Rights Reserved + * + * merge it to 3.11 by cini <cinifr@gmail.com> + * + * 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. + */ +#include <linux/init.h> +#include <linux/errno.h> +#include <linux/smp.h> +#include <linux/io.h> +#include <linux/delay.h> +#include <linux/device.h> +#include <linux/jiffies.h> +#include <linux/smp.h> + +#include <linux/of_address.h> +#include <linux/of_irq.h> +#include <linux/of_platform.h> + + +#include <linux/irqchip/arm-gic.h> +#include <asm/mach-types.h> +#include <asm/smp_scu.h> +#include <asm/cacheflush.h> +#include <asm/smp_plat.h> + +#include "platform.h" +#include <asm/arch_timer.h> + + + +static DEFINE_SPINLOCK(boot_lock); + + +static void __iomem *cc_base; /*CPU Configure Base*/ + +static struct of_device_id sunxi_cc_ids[] = { + { .compatible = "allwinner,sun7i-cc"}, + { /*sentinel*/ } +}; + + +static void enable_aw_cpu(int cpu) +{ + long paddr; + u32 pwr_reg; + + struct device_node *np; + np = of_find_matching_node(NULL, sunxi_cc_ids); + cc_base = of_iomap(np, 0); + pr_debug("cc_base is %x\n", (unsigned int)cc_base); + if (!cc_base) { + pr_debug("error map cc configure\n"); + return; + } + + paddr = virt_to_phys(sun7i_secondary_startup); + writel(paddr, cc_base + AW_CPUCFG_P_REG0); + /* step1: Assert nCOREPORESET LOW and hold L1RSTDISABLE LOW. + Ensure DBGPWRDUP is held LOW to prevent any external + debug access to the processor. + */ + /* assert cpu core reset */ + writel(0, cc_base + CPUX_RESET_CTL(cpu)); + /* L1RSTDISABLE hold low */ + pwr_reg = readl(cc_base + AW_CPUCFG_GENCTL); + pwr_reg &= ~(1<<cpu); + writel(pwr_reg, cc_base + AW_CPUCFG_GENCTL); + /* DBGPWRDUP hold low */ + pwr_reg = readl(cc_base + AW_CPUCFG_DBGCTL1); + pwr_reg &= ~(1<<cpu); + writel(pwr_reg, cc_base + AW_CPUCFG_DBGCTL1); + + /* step2: release power clamp */ + writel(0xff, cc_base + AW_CPU1_PWR_CLAMP); + writel(0x7f, cc_base + AW_CPU1_PWR_CLAMP); + writel(0x3f, cc_base + AW_CPU1_PWR_CLAMP); + writel(0x1f, cc_base + AW_CPU1_PWR_CLAMP); + writel(0x0f, cc_base + AW_CPU1_PWR_CLAMP); + writel(0x07, cc_base + AW_CPU1_PWR_CLAMP); + writel(0x03, cc_base + AW_CPU1_PWR_CLAMP); + writel(0x01, cc_base + AW_CPU1_PWR_CLAMP); + writel(0x00, cc_base + AW_CPU1_PWR_CLAMP); + mdelay(10); + + /* step3: clear power-off gating */ + pwr_reg = readl(cc_base + AW_CPU1_PWROFF_REG); + pwr_reg &= ~(1); + writel(pwr_reg, cc_base + AW_CPU1_PWROFF_REG); + mdelay(1); + + /* step4: de-assert core reset */ + writel(3, cc_base + CPUX_RESET_CTL(cpu)); + + /* step5: assert DBGPWRDUP signal */ + pwr_reg = readl(cc_base + AW_CPUCFG_DBGCTL1); + pwr_reg |= (1<<cpu); + writel(pwr_reg, cc_base + AW_CPUCFG_DBGCTL1); + +} + + + +static void sunxi7i_smp_init_cpus(void) +{ + unsigned int i, ncores; + + + /* HDG: we do not use scu_get_core_count() here as that does not + work on the A20 ? */ + + /* Read current CP15 Cache Size ID Register */ + asm volatile ("mrc p15, 1, %0, c9, c0, 2" : "=r" (ncores)); + ncores = ((ncores >> 24) & 0x3) + 1; + + pr_debug("[%s] ncores=%d\n", __func__, ncores); + + for (i = 0; i < ncores; i++) + set_cpu_possible(i, true); + +} + +/* + * for arch/arm/kernel/smp.c:smp_prepare_cpus(unsigned int max_cpus) + */ +static void sunxi7i_smp_prepare_cpus(unsigned int max_cpus) +{ + /* + * HDG: we do not call scu_enable() here as the sun6i source dump has + * a modified arch/arm/kernel/smp_scu.c, where scu_enable() is a nop. + */ +} + + + + + + +/* + * for linux/arch/arm/kernel/smp.c:__cpu_up(..) + */ +static int sunxi7i_boot_secondary(unsigned int cpu, struct task_struct *idle) +{ + pr_debug("[%s] enter cpu %d\n", __func__, cpu); + spin_lock(&boot_lock); + enable_aw_cpu(cpu); + spin_unlock(&boot_lock); + return 0; +} + + + + +struct smp_operations sunxi7i_smp_ops __initdata = { + .smp_init_cpus = sunxi7i_smp_init_cpus, + .smp_prepare_cpus = sunxi7i_smp_prepare_cpus, + .smp_boot_secondary = sunxi7i_boot_secondary, +}; + + diff --git a/arch/arm/mach-sunxi/sunxi.c b/arch/arm/mach-sunxi/sunxi.c index e79fb34..f3594e6 100644 --- a/arch/arm/mach-sunxi/sunxi.c +++ b/arch/arm/mach-sunxi/sunxi.c @@ -26,6 +26,9 @@ #include <asm/mach/map.h> #include <asm/system_misc.h> +#include "platform.h" + + #define SUN4I_WATCHDOG_CTRL_REG 0x00 #define SUN4I_WATCHDOG_CTRL_RESTART BIT(0) #define SUN4I_WATCHDOG_MODE_REG 0x04 @@ -139,6 +142,7 @@ static const char * const sunxi_board_dt_compat[] = { }; DT_MACHINE_START(SUNXI_DT, "Allwinner A1X (Device Tree)") + .smp = smp_ops(sunxi7i_smp_ops), .init_machine = sunxi_dt_init, .init_time = sunxi_timer_init, .dt_compat = sunxi_board_dt_compat, -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH 1/4] Add smp support for Allwinner A20(sunxi 7i). 2013-09-12 6:51 ` [PATCH 1/4] Add smp support for Allwinner A20(sunxi 7i) Fan Rong @ 2013-09-12 14:26 ` Mark Rutland 2013-09-12 15:53 ` cinifr 2013-09-12 14:40 ` Russell King - ARM Linux 1 sibling, 1 reply; 31+ messages in thread From: Mark Rutland @ 2013-09-12 14:26 UTC (permalink / raw) To: linux-arm-kernel On Thu, Sep 12, 2013 at 07:51:24AM +0100, Fan Rong wrote: > Signed-off-by: Fan Rong <cinifr@gmail.com> > --- > arch/arm/mach-sunxi/Makefile | 2 + > arch/arm/mach-sunxi/headsmp.S | 12 ++ > arch/arm/mach-sunxi/platform.h | 346 ++++++++++++++++++++++++++++++++++++++++ > arch/arm/mach-sunxi/platsmp.c | 166 +++++++++++++++++++ > arch/arm/mach-sunxi/sunxi.c | 4 + > 5 files changed, 530 insertions(+) > create mode 100644 arch/arm/mach-sunxi/headsmp.S > create mode 100644 arch/arm/mach-sunxi/platform.h > create mode 100644 arch/arm/mach-sunxi/platsmp.c > [...] > +static struct of_device_id sunxi_cc_ids[] = { > + { .compatible = "allwinner,sun7i-cc"}, > + { /*sentinel*/ } > +}; NAK - There's no binding document for this in mainline or this series. > + > + > +static void enable_aw_cpu(int cpu) > +{ > + long paddr; > + u32 pwr_reg; > + > + struct device_node *np; > + np = of_find_matching_node(NULL, sunxi_cc_ids); > + cc_base = of_iomap(np, 0); This seems to get called repeatedly in sunxi7i_boot_secondary, so you're repeatedly trying to find the cc node and mapping it, but never unmapping it. That's both a waste of time and a waste of address space. It would be nicer to map this once at the start. That seems simpler than stashing the physical address and mapping/unmapping it repeatedly. smp_boot_secondary. > + pr_debug("cc_base is %x\n", (unsigned int)cc_base); > + if (!cc_base) { You can use %p to print pointers, without casting to an integer type. > + pr_debug("error map cc configure\n"); > + return; As this may be called repeatedly, from a function that can return errors, it would be nice to propagate an error code if there's a failure. > + } > + > + paddr = virt_to_phys(sun7i_secondary_startup); > + writel(paddr, cc_base + AW_CPUCFG_P_REG0); > + /* step1: Assert nCOREPORESET LOW and hold L1RSTDISABLE LOW. > + Ensure DBGPWRDUP is held LOW to prevent any external > + debug access to the processor. > + */ > + /* assert cpu core reset */ > + writel(0, cc_base + CPUX_RESET_CTL(cpu)); > + /* L1RSTDISABLE hold low */ > + pwr_reg = readl(cc_base + AW_CPUCFG_GENCTL); > + pwr_reg &= ~(1<<cpu); > + writel(pwr_reg, cc_base + AW_CPUCFG_GENCTL); > + /* DBGPWRDUP hold low */ > + pwr_reg = readl(cc_base + AW_CPUCFG_DBGCTL1); > + pwr_reg &= ~(1<<cpu); > + writel(pwr_reg, cc_base + AW_CPUCFG_DBGCTL1); > + > + /* step2: release power clamp */ > + writel(0xff, cc_base + AW_CPU1_PWR_CLAMP); > + writel(0x7f, cc_base + AW_CPU1_PWR_CLAMP); > + writel(0x3f, cc_base + AW_CPU1_PWR_CLAMP); > + writel(0x1f, cc_base + AW_CPU1_PWR_CLAMP); > + writel(0x0f, cc_base + AW_CPU1_PWR_CLAMP); > + writel(0x07, cc_base + AW_CPU1_PWR_CLAMP); > + writel(0x03, cc_base + AW_CPU1_PWR_CLAMP); > + writel(0x01, cc_base + AW_CPU1_PWR_CLAMP); > + writel(0x00, cc_base + AW_CPU1_PWR_CLAMP); > + mdelay(10); > + > + /* step3: clear power-off gating */ > + pwr_reg = readl(cc_base + AW_CPU1_PWROFF_REG); > + pwr_reg &= ~(1); > + writel(pwr_reg, cc_base + AW_CPU1_PWROFF_REG); > + mdelay(1); > + > + /* step4: de-assert core reset */ > + writel(3, cc_base + CPUX_RESET_CTL(cpu)); > + > + /* step5: assert DBGPWRDUP signal */ > + pwr_reg = readl(cc_base + AW_CPUCFG_DBGCTL1); > + pwr_reg |= (1<<cpu); > + writel(pwr_reg, cc_base + AW_CPUCFG_DBGCTL1); > + > +} > + > + > + > +static void sunxi7i_smp_init_cpus(void) > +{ > + unsigned int i, ncores; > + > + > + /* HDG: we do not use scu_get_core_count() here as that does not > + work on the A20 ? */ > + > + /* Read current CP15 Cache Size ID Register */ Invalid comment. Judging by the encoding, this is the L2CTLR, not the CCSIDR. > + asm volatile ("mrc p15, 1, %0, c9, c0, 2" : "=r" (ncores)); > + ncores = ((ncores >> 24) & 0x3) + 1; > + > + pr_debug("[%s] ncores=%d\n", __func__, ncores); > + > + for (i = 0; i < ncores; i++) > + set_cpu_possible(i, true); > + > +} Even ignoring the above, as long as your dt is correct arm_dt_init_cpu_maps (called from stup_arch) will set the cpus as possible (and handles arbitrary MPIDR values as may be the case in multi-cluster). You don't need this function -- please remove it. > + > +/* > + * for arch/arm/kernel/smp.c:smp_prepare_cpus(unsigned int max_cpus) > + */ > +static void sunxi7i_smp_prepare_cpus(unsigned int max_cpus) > +{ > + /* > + * HDG: we do not call scu_enable() here as the sun6i source dump has > + * a modified arch/arm/kernel/smp_scu.c, where scu_enable() is a nop. > + */ > +} A look in smp.c shows smp_prepare_cpus is perfectly happy to not have a smp_prepare_cpus callback. You don't need this function -- please remove it. > + > + > + > + > + > + Why so much space here (and elsewhere)? > +/* > + * for linux/arch/arm/kernel/smp.c:__cpu_up(..) > + */ > +static int sunxi7i_boot_secondary(unsigned int cpu, struct task_struct *idle) > +{ > + pr_debug("[%s] enter cpu %d\n", __func__, cpu); > + spin_lock(&boot_lock); > + enable_aw_cpu(cpu); This can fail. You should propagate the error when it does. > + spin_unlock(&boot_lock); > + return 0; > +} > + > + > + > + > +struct smp_operations sunxi7i_smp_ops __initdata = { > + .smp_init_cpus = sunxi7i_smp_init_cpus, > + .smp_prepare_cpus = sunxi7i_smp_prepare_cpus, > + .smp_boot_secondary = sunxi7i_boot_secondary, > +}; I believe only smp_boot_secondary is necessary here. Thanks, Mark. > + > + > diff --git a/arch/arm/mach-sunxi/sunxi.c b/arch/arm/mach-sunxi/sunxi.c > index e79fb34..f3594e6 100644 > --- a/arch/arm/mach-sunxi/sunxi.c > +++ b/arch/arm/mach-sunxi/sunxi.c > @@ -26,6 +26,9 @@ > #include <asm/mach/map.h> > #include <asm/system_misc.h> > > +#include "platform.h" > + > + > #define SUN4I_WATCHDOG_CTRL_REG 0x00 > #define SUN4I_WATCHDOG_CTRL_RESTART BIT(0) > #define SUN4I_WATCHDOG_MODE_REG 0x04 > @@ -139,6 +142,7 @@ static const char * const sunxi_board_dt_compat[] = { > }; > > DT_MACHINE_START(SUNXI_DT, "Allwinner A1X (Device Tree)") > + .smp = smp_ops(sunxi7i_smp_ops), > .init_machine = sunxi_dt_init, > .init_time = sunxi_timer_init, > .dt_compat = sunxi_board_dt_compat, > -- > 1.7.9.5 > > ^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 1/4] Add smp support for Allwinner A20(sunxi 7i). 2013-09-12 14:26 ` Mark Rutland @ 2013-09-12 15:53 ` cinifr 0 siblings, 0 replies; 31+ messages in thread From: cinifr @ 2013-09-12 15:53 UTC (permalink / raw) To: linux-arm-kernel Thanks for you advice, you are right, I will reupdate the patch. :) On 12 September 2013 22:26, Mark Rutland <mark.rutland@arm.com> wrote: > On Thu, Sep 12, 2013 at 07:51:24AM +0100, Fan Rong wrote: >> Signed-off-by: Fan Rong <cinifr@gmail.com> >> --- >> arch/arm/mach-sunxi/Makefile | 2 + >> arch/arm/mach-sunxi/headsmp.S | 12 ++ >> arch/arm/mach-sunxi/platform.h | 346 ++++++++++++++++++++++++++++++++++++++++ >> arch/arm/mach-sunxi/platsmp.c | 166 +++++++++++++++++++ >> arch/arm/mach-sunxi/sunxi.c | 4 + >> 5 files changed, 530 insertions(+) >> create mode 100644 arch/arm/mach-sunxi/headsmp.S >> create mode 100644 arch/arm/mach-sunxi/platform.h >> create mode 100644 arch/arm/mach-sunxi/platsmp.c >> > > [...] > >> +static struct of_device_id sunxi_cc_ids[] = { >> + { .compatible = "allwinner,sun7i-cc"}, >> + { /*sentinel*/ } >> +}; > > NAK - There's no binding document for this in mainline or this series. > >> + >> + >> +static void enable_aw_cpu(int cpu) >> +{ >> + long paddr; >> + u32 pwr_reg; >> + >> + struct device_node *np; >> + np = of_find_matching_node(NULL, sunxi_cc_ids); >> + cc_base = of_iomap(np, 0); > > This seems to get called repeatedly in sunxi7i_boot_secondary, so you're > repeatedly trying to find the cc node and mapping it, but never > unmapping it. That's both a waste of time and a waste of address space. > > It would be nicer to map this once at the start. That seems simpler than > stashing the physical address and mapping/unmapping it repeatedly. > smp_boot_secondary. > >> + pr_debug("cc_base is %x\n", (unsigned int)cc_base); >> + if (!cc_base) { > > You can use %p to print pointers, without casting to an integer type. > >> + pr_debug("error map cc configure\n"); >> + return; > > As this may be called repeatedly, from a function that can return > errors, it would be nice to propagate an error code if there's a > failure. > >> + } >> + >> + paddr = virt_to_phys(sun7i_secondary_startup); >> + writel(paddr, cc_base + AW_CPUCFG_P_REG0); >> + /* step1: Assert nCOREPORESET LOW and hold L1RSTDISABLE LOW. >> + Ensure DBGPWRDUP is held LOW to prevent any external >> + debug access to the processor. >> + */ >> + /* assert cpu core reset */ >> + writel(0, cc_base + CPUX_RESET_CTL(cpu)); >> + /* L1RSTDISABLE hold low */ >> + pwr_reg = readl(cc_base + AW_CPUCFG_GENCTL); >> + pwr_reg &= ~(1<<cpu); >> + writel(pwr_reg, cc_base + AW_CPUCFG_GENCTL); >> + /* DBGPWRDUP hold low */ >> + pwr_reg = readl(cc_base + AW_CPUCFG_DBGCTL1); >> + pwr_reg &= ~(1<<cpu); >> + writel(pwr_reg, cc_base + AW_CPUCFG_DBGCTL1); >> + >> + /* step2: release power clamp */ >> + writel(0xff, cc_base + AW_CPU1_PWR_CLAMP); >> + writel(0x7f, cc_base + AW_CPU1_PWR_CLAMP); >> + writel(0x3f, cc_base + AW_CPU1_PWR_CLAMP); >> + writel(0x1f, cc_base + AW_CPU1_PWR_CLAMP); >> + writel(0x0f, cc_base + AW_CPU1_PWR_CLAMP); >> + writel(0x07, cc_base + AW_CPU1_PWR_CLAMP); >> + writel(0x03, cc_base + AW_CPU1_PWR_CLAMP); >> + writel(0x01, cc_base + AW_CPU1_PWR_CLAMP); >> + writel(0x00, cc_base + AW_CPU1_PWR_CLAMP); >> + mdelay(10); >> + >> + /* step3: clear power-off gating */ >> + pwr_reg = readl(cc_base + AW_CPU1_PWROFF_REG); >> + pwr_reg &= ~(1); >> + writel(pwr_reg, cc_base + AW_CPU1_PWROFF_REG); >> + mdelay(1); >> + >> + /* step4: de-assert core reset */ >> + writel(3, cc_base + CPUX_RESET_CTL(cpu)); >> + >> + /* step5: assert DBGPWRDUP signal */ >> + pwr_reg = readl(cc_base + AW_CPUCFG_DBGCTL1); >> + pwr_reg |= (1<<cpu); >> + writel(pwr_reg, cc_base + AW_CPUCFG_DBGCTL1); >> + >> +} >> + >> + >> + >> +static void sunxi7i_smp_init_cpus(void) >> +{ >> + unsigned int i, ncores; >> + >> + >> + /* HDG: we do not use scu_get_core_count() here as that does not >> + work on the A20 ? */ >> + >> + /* Read current CP15 Cache Size ID Register */ > > Invalid comment. Judging by the encoding, this is the L2CTLR, not the > CCSIDR. > >> + asm volatile ("mrc p15, 1, %0, c9, c0, 2" : "=r" (ncores)); >> + ncores = ((ncores >> 24) & 0x3) + 1; >> + >> + pr_debug("[%s] ncores=%d\n", __func__, ncores); >> + >> + for (i = 0; i < ncores; i++) >> + set_cpu_possible(i, true); >> + >> +} > > Even ignoring the above, as long as your dt is correct > arm_dt_init_cpu_maps (called from stup_arch) will set the cpus as > possible (and handles arbitrary MPIDR values as may be the case in > multi-cluster). > > You don't need this function -- please remove it. > >> + >> +/* >> + * for arch/arm/kernel/smp.c:smp_prepare_cpus(unsigned int max_cpus) >> + */ >> +static void sunxi7i_smp_prepare_cpus(unsigned int max_cpus) >> +{ >> + /* >> + * HDG: we do not call scu_enable() here as the sun6i source dump has >> + * a modified arch/arm/kernel/smp_scu.c, where scu_enable() is a nop. >> + */ >> +} > > A look in smp.c shows smp_prepare_cpus is perfectly happy to not have a > smp_prepare_cpus callback. You don't need this function -- please remove > it. > >> + >> + >> + >> + >> + >> + > > Why so much space here (and elsewhere)? > >> +/* >> + * for linux/arch/arm/kernel/smp.c:__cpu_up(..) >> + */ >> +static int sunxi7i_boot_secondary(unsigned int cpu, struct task_struct *idle) >> +{ >> + pr_debug("[%s] enter cpu %d\n", __func__, cpu); >> + spin_lock(&boot_lock); >> + enable_aw_cpu(cpu); > > This can fail. You should propagate the error when it does. > >> + spin_unlock(&boot_lock); >> + return 0; >> +} >> + >> + >> + >> + >> +struct smp_operations sunxi7i_smp_ops __initdata = { >> + .smp_init_cpus = sunxi7i_smp_init_cpus, >> + .smp_prepare_cpus = sunxi7i_smp_prepare_cpus, >> + .smp_boot_secondary = sunxi7i_boot_secondary, >> +}; > > I believe only smp_boot_secondary is necessary here. > > Thanks, > Mark. > >> + >> + >> diff --git a/arch/arm/mach-sunxi/sunxi.c b/arch/arm/mach-sunxi/sunxi.c >> index e79fb34..f3594e6 100644 >> --- a/arch/arm/mach-sunxi/sunxi.c >> +++ b/arch/arm/mach-sunxi/sunxi.c >> @@ -26,6 +26,9 @@ >> #include <asm/mach/map.h> >> #include <asm/system_misc.h> >> >> +#include "platform.h" >> + >> + >> #define SUN4I_WATCHDOG_CTRL_REG 0x00 >> #define SUN4I_WATCHDOG_CTRL_RESTART BIT(0) >> #define SUN4I_WATCHDOG_MODE_REG 0x04 >> @@ -139,6 +142,7 @@ static const char * const sunxi_board_dt_compat[] = { >> }; >> >> DT_MACHINE_START(SUNXI_DT, "Allwinner A1X (Device Tree)") >> + .smp = smp_ops(sunxi7i_smp_ops), >> .init_machine = sunxi_dt_init, >> .init_time = sunxi_timer_init, >> .dt_compat = sunxi_board_dt_compat, >> -- >> 1.7.9.5 >> >> ^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 1/4] Add smp support for Allwinner A20(sunxi 7i). 2013-09-12 6:51 ` [PATCH 1/4] Add smp support for Allwinner A20(sunxi 7i) Fan Rong 2013-09-12 14:26 ` Mark Rutland @ 2013-09-12 14:40 ` Russell King - ARM Linux 1 sibling, 0 replies; 31+ messages in thread From: Russell King - ARM Linux @ 2013-09-12 14:40 UTC (permalink / raw) To: linux-arm-kernel Some other comments: On Thu, Sep 12, 2013 at 02:51:24PM +0800, Fan Rong wrote: > + /* L1RSTDISABLE hold low */ > + pwr_reg = readl(cc_base + AW_CPUCFG_GENCTL); > + pwr_reg &= ~(1<<cpu); If you pass your patch through checkpatch.pl, it will warn about some of this. You should have one space each side of <<. > + /* step3: clear power-off gating */ > + pwr_reg = readl(cc_base + AW_CPU1_PWROFF_REG); > + pwr_reg &= ~(1); You don't need the parens here. > + pwr_reg = readl(cc_base + AW_CPUCFG_DBGCTL1); > + pwr_reg |= (1<<cpu); Nor here. > +static int sunxi7i_boot_secondary(unsigned int cpu, struct task_struct *idle) > +{ > + pr_debug("[%s] enter cpu %d\n", __func__, cpu); > + spin_lock(&boot_lock); > + enable_aw_cpu(cpu); > + spin_unlock(&boot_lock); What exactly does this spinlock protect? The core code already provides the guarantee that only one CPU will be brought online or taken offline at a time. ^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 2/4] Add cpuconfig nodes in dts for smp configure. 2013-09-12 6:51 [PATCH 0/4] Add smp support for Allwinner A20 and phy arch count timer Fan Rong 2013-09-12 6:51 ` [PATCH 1/4] Add smp support for Allwinner A20(sunxi 7i) Fan Rong @ 2013-09-12 6:51 ` Fan Rong 2013-09-14 11:50 ` Maxime Ripard 2013-09-12 6:51 ` [PATCH 3/4] Add physical count arch timer support for clocksource in ARMv7 Fan Rong ` (2 subsequent siblings) 4 siblings, 1 reply; 31+ messages in thread From: Fan Rong @ 2013-09-12 6:51 UTC (permalink / raw) To: linux-arm-kernel Signed-off-by: Fan Rong <cinifr@gmail.com> --- arch/arm/boot/dts/sun7i-a20.dtsi | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi index 999ff45..bfedcb2 100644 --- a/arch/arm/boot/dts/sun7i-a20.dtsi +++ b/arch/arm/boot/dts/sun7i-a20.dtsi @@ -20,13 +20,13 @@ #address-cells = <1>; #size-cells = <0>; - cpu at 0 { + cpu0: cpu at 0 { compatible = "arm,cortex-a7"; device_type = "cpu"; reg = <0>; }; - cpu at 1 { + cpu1: cpu at 1 { compatible = "arm,cortex-a7"; device_type = "cpu"; reg = <1>; @@ -167,6 +167,11 @@ #size-cells = <1>; ranges; + cc: cpuconfig at 01c25c00 { + compatible = "allwinner,sun7i-cc"; + reg = <0x01c25c00 0x400>; + }; + pio: pinctrl at 01c20800 { compatible = "allwinner,sun7i-a20-pinctrl"; reg = <0x01c20800 0x400>; -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH 2/4] Add cpuconfig nodes in dts for smp configure. 2013-09-12 6:51 ` [PATCH 2/4] Add cpuconfig nodes in dts for smp configure Fan Rong @ 2013-09-14 11:50 ` Maxime Ripard 0 siblings, 0 replies; 31+ messages in thread From: Maxime Ripard @ 2013-09-14 11:50 UTC (permalink / raw) To: linux-arm-kernel Hi Fan, On Thu, Sep 12, 2013 at 02:51:25PM +0800, Fan Rong wrote: > Signed-off-by: Fan Rong <cinifr@gmail.com> > --- > arch/arm/boot/dts/sun7i-a20.dtsi | 9 +++++++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi > index 999ff45..bfedcb2 100644 > --- a/arch/arm/boot/dts/sun7i-a20.dtsi > +++ b/arch/arm/boot/dts/sun7i-a20.dtsi > @@ -20,13 +20,13 @@ > #address-cells = <1>; > #size-cells = <0>; > > - cpu at 0 { > + cpu0: cpu at 0 { > compatible = "arm,cortex-a7"; > device_type = "cpu"; > reg = <0>; > }; > > - cpu at 1 { > + cpu1: cpu at 1 { Both these changes don't seem really needed, are they? > compatible = "arm,cortex-a7"; > device_type = "cpu"; > reg = <1>; > @@ -167,6 +167,11 @@ > #size-cells = <1>; > ranges; > > + cc: cpuconfig at 01c25c00 { > + compatible = "allwinner,sun7i-cc"; Please use the sun7i-a20 prefix, and I'd prefer cpu-config instead of "cc". Thanks for working on this! Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 836 bytes Desc: Digital signature URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130914/4cf66fd8/attachment.sig> ^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 3/4] Add physical count arch timer support for clocksource in ARMv7. 2013-09-12 6:51 [PATCH 0/4] Add smp support for Allwinner A20 and phy arch count timer Fan Rong 2013-09-12 6:51 ` [PATCH 1/4] Add smp support for Allwinner A20(sunxi 7i) Fan Rong 2013-09-12 6:51 ` [PATCH 2/4] Add cpuconfig nodes in dts for smp configure Fan Rong @ 2013-09-12 6:51 ` Fan Rong 2013-09-12 11:24 ` Mark Rutland 2013-09-12 6:51 ` [PATCH 4/4] Add arch count timer node in dts for Allwinner A20(sunxi 7i) Fan Rong 2013-09-12 14:39 ` [PATCH 0/4] Add smp support for Allwinner A20 and phy arch count timer Mark Rutland 4 siblings, 1 reply; 31+ messages in thread From: Fan Rong @ 2013-09-12 6:51 UTC (permalink / raw) To: linux-arm-kernel Signed-off-by: Fan Rong <cinifr@gmail.com> --- arch/arm/include/asm/arch_timer.h | 11 +++++++++++ drivers/clocksource/Kconfig | 8 ++++++++ drivers/clocksource/arm_arch_timer.c | 10 +++++++++- 3 files changed, 28 insertions(+), 1 deletion(-) diff --git a/arch/arm/include/asm/arch_timer.h b/arch/arm/include/asm/arch_timer.h index 5665134..24c904a 100644 --- a/arch/arm/include/asm/arch_timer.h +++ b/arch/arm/include/asm/arch_timer.h @@ -87,6 +87,17 @@ static inline u64 arch_counter_get_cntvct(void) return cval; } +static inline u64 arch_counter_get_cntpct(void) +{ + u64 cval; + + isb(); + asm volatile("mrrc p15, 0, %Q0, %R0, c14" : "=r" (cval)); + return cval; +} + + + static inline void arch_counter_set_user_access(void) { u32 cntkctl; diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig index 41c6946..a4981d2 100644 --- a/drivers/clocksource/Kconfig +++ b/drivers/clocksource/Kconfig @@ -109,3 +109,11 @@ config VF_PIT_TIMER bool help Support for Period Interrupt Timer on Freescale Vybrid Family SoCs. +menu "Clock Source" + +config ARM_ARCH_TIMER_USE_PHYCNT + bool "Use Physical Count Timer" + depends on ARM_ARCH_TIMER + help + If bootloader dont set Virtual Offset register,Physical Count Timer is needed to replace Virtual Count Timer. +endmenu diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c index fbd9ccd..884e4d1 100644 --- a/drivers/clocksource/arm_arch_timer.c +++ b/drivers/clocksource/arm_arch_timer.c @@ -372,7 +372,11 @@ static u64 arch_counter_get_cntvct_mem(void) * to exist on arm64. arm doesn't use this before DT is probed so even * if we don't have the cp15 accessors we won't have a problem. */ -u64 (*arch_timer_read_counter)(void) = arch_counter_get_cntvct; +#ifdef CONFIG_ARM_ARCH_TIMER_USE_PHYCNT + u64 (*arch_timer_read_counter)(void) = arch_counter_get_cntpct; +#else + u64 (*arch_timer_read_counter)(void) = arch_counter_get_cntvct; +#endif static cycle_t arch_counter_read(struct clocksource *cs) { @@ -410,7 +414,11 @@ static void __init arch_counter_register(unsigned type) /* Register the CP15 based counter if we have one */ if (type & ARCH_CP15_TIMER) +#ifdef CONFIG_ARM_ARCH_TIMER_USE_PHYCNT + arch_timer_read_counter = arch_counter_get_cntpct; +#else arch_timer_read_counter = arch_counter_get_cntvct; +#endif else arch_timer_read_counter = arch_counter_get_cntvct_mem; -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH 3/4] Add physical count arch timer support for clocksource in ARMv7. 2013-09-12 6:51 ` [PATCH 3/4] Add physical count arch timer support for clocksource in ARMv7 Fan Rong @ 2013-09-12 11:24 ` Mark Rutland 2013-09-12 14:33 ` Russell King - ARM Linux ` (2 more replies) 0 siblings, 3 replies; 31+ messages in thread From: Mark Rutland @ 2013-09-12 11:24 UTC (permalink / raw) To: linux-arm-kernel On Thu, Sep 12, 2013 at 07:51:26AM +0100, Fan Rong wrote: > Signed-off-by: Fan Rong <cinifr@gmail.com> > --- > arch/arm/include/asm/arch_timer.h | 11 +++++++++++ > drivers/clocksource/Kconfig | 8 ++++++++ > drivers/clocksource/arm_arch_timer.c | 10 +++++++++- > 3 files changed, 28 insertions(+), 1 deletion(-) > > diff --git a/arch/arm/include/asm/arch_timer.h b/arch/arm/include/asm/arch_timer.h > index 5665134..24c904a 100644 > --- a/arch/arm/include/asm/arch_timer.h > +++ b/arch/arm/include/asm/arch_timer.h > @@ -87,6 +87,17 @@ static inline u64 arch_counter_get_cntvct(void) > return cval; > } > > +static inline u64 arch_counter_get_cntpct(void) > +{ > + u64 cval; > + > + isb(); > + asm volatile("mrrc p15, 0, %Q0, %R0, c14" : "=r" (cval)); > + return cval; > +} > + > + > + > static inline void arch_counter_set_user_access(void) > { > u32 cntkctl; > diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig > index 41c6946..a4981d2 100644 > --- a/drivers/clocksource/Kconfig > +++ b/drivers/clocksource/Kconfig > @@ -109,3 +109,11 @@ config VF_PIT_TIMER > bool > help > Support for Period Interrupt Timer on Freescale Vybrid Family SoCs. > +menu "Clock Source" > + > +config ARM_ARCH_TIMER_USE_PHYCNT > + bool "Use Physical Count Timer" > + depends on ARM_ARCH_TIMER > + help > + If bootloader dont set Virtual Offset register,Physical Count Timer is needed to replace Virtual Count Timer. > +endmenu This cannot be a compile-time option as above in a multiplatform build. Other paltforms (e.g. KVM guests) *must* use the virtual counters to get any semblance of a consistent view of time. Why can the bootloader not be fixed to set CNTVOFF (or to boot the kernel in Hyp mode)? Thanks, Mark. > diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c > index fbd9ccd..884e4d1 100644 > --- a/drivers/clocksource/arm_arch_timer.c > +++ b/drivers/clocksource/arm_arch_timer.c > @@ -372,7 +372,11 @@ static u64 arch_counter_get_cntvct_mem(void) > * to exist on arm64. arm doesn't use this before DT is probed so even > * if we don't have the cp15 accessors we won't have a problem. > */ > -u64 (*arch_timer_read_counter)(void) = arch_counter_get_cntvct; > +#ifdef CONFIG_ARM_ARCH_TIMER_USE_PHYCNT > + u64 (*arch_timer_read_counter)(void) = arch_counter_get_cntpct; > +#else > + u64 (*arch_timer_read_counter)(void) = arch_counter_get_cntvct; > +#endif > > static cycle_t arch_counter_read(struct clocksource *cs) > { > @@ -410,7 +414,11 @@ static void __init arch_counter_register(unsigned type) > > /* Register the CP15 based counter if we have one */ > if (type & ARCH_CP15_TIMER) > +#ifdef CONFIG_ARM_ARCH_TIMER_USE_PHYCNT > + arch_timer_read_counter = arch_counter_get_cntpct; > +#else > arch_timer_read_counter = arch_counter_get_cntvct; > +#endif > else > arch_timer_read_counter = arch_counter_get_cntvct_mem; > > -- > 1.7.9.5 > > ^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 3/4] Add physical count arch timer support for clocksource in ARMv7. 2013-09-12 11:24 ` Mark Rutland @ 2013-09-12 14:33 ` Russell King - ARM Linux 2013-09-12 16:09 ` cinifr 2013-09-12 15:39 ` [linux-sunxi] " Ian Campbell 2013-09-12 16:07 ` cinifr 2 siblings, 1 reply; 31+ messages in thread From: Russell King - ARM Linux @ 2013-09-12 14:33 UTC (permalink / raw) To: linux-arm-kernel On Thu, Sep 12, 2013 at 12:24:52PM +0100, Mark Rutland wrote: > On Thu, Sep 12, 2013 at 07:51:26AM +0100, Fan Rong wrote: > > diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig > > index 41c6946..a4981d2 100644 > > --- a/drivers/clocksource/Kconfig > > +++ b/drivers/clocksource/Kconfig > > @@ -109,3 +109,11 @@ config VF_PIT_TIMER > > bool > > help > > Support for Period Interrupt Timer on Freescale Vybrid Family SoCs. > > +menu "Clock Source" > > + > > +config ARM_ARCH_TIMER_USE_PHYCNT > > + bool "Use Physical Count Timer" > > + depends on ARM_ARCH_TIMER > > + help > > + If bootloader dont set Virtual Offset register,Physical Count Timer is needed to replace Virtual Count Timer. > > +endmenu > > This cannot be a compile-time option as above in a multiplatform build. > Other paltforms (e.g. KVM guests) *must* use the virtual counters to get > any semblance of a consistent view of time. Also, for future reference: 1. Use a single blank lines to separate thing like 'menu' and 'endmenu'. 2. Wrap your the help sensibly, don't put it all on one line. 3. It's conventional to use tabs up to below "help" and then two spaces to indent the help text. 4. "," always has one space after it. Thanks. ^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 3/4] Add physical count arch timer support for clocksource in ARMv7. 2013-09-12 14:33 ` Russell King - ARM Linux @ 2013-09-12 16:09 ` cinifr 0 siblings, 0 replies; 31+ messages in thread From: cinifr @ 2013-09-12 16:09 UTC (permalink / raw) To: linux-arm-kernel Thanks, but I am tring a solution in running time. On 12 September 2013 22:33, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: > On Thu, Sep 12, 2013 at 12:24:52PM +0100, Mark Rutland wrote: >> On Thu, Sep 12, 2013 at 07:51:26AM +0100, Fan Rong wrote: >> > diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig >> > index 41c6946..a4981d2 100644 >> > --- a/drivers/clocksource/Kconfig >> > +++ b/drivers/clocksource/Kconfig >> > @@ -109,3 +109,11 @@ config VF_PIT_TIMER >> > bool >> > help >> > Support for Period Interrupt Timer on Freescale Vybrid Family SoCs. >> > +menu "Clock Source" >> > + >> > +config ARM_ARCH_TIMER_USE_PHYCNT >> > + bool "Use Physical Count Timer" >> > + depends on ARM_ARCH_TIMER >> > + help >> > + If bootloader dont set Virtual Offset register,Physical Count Timer is needed to replace Virtual Count Timer. >> > +endmenu >> >> This cannot be a compile-time option as above in a multiplatform build. >> Other paltforms (e.g. KVM guests) *must* use the virtual counters to get >> any semblance of a consistent view of time. > > Also, for future reference: > > 1. Use a single blank lines to separate thing like 'menu' and 'endmenu'. > 2. Wrap your the help sensibly, don't put it all on one line. > 3. It's conventional to use tabs up to below "help" and then two spaces > to indent the help text. > 4. "," always has one space after it. > > Thanks. ^ permalink raw reply [flat|nested] 31+ messages in thread
* [linux-sunxi] Re: [PATCH 3/4] Add physical count arch timer support for clocksource in ARMv7. 2013-09-12 11:24 ` Mark Rutland 2013-09-12 14:33 ` Russell King - ARM Linux @ 2013-09-12 15:39 ` Ian Campbell 2013-09-12 16:07 ` cinifr 2 siblings, 0 replies; 31+ messages in thread From: Ian Campbell @ 2013-09-12 15:39 UTC (permalink / raw) To: linux-arm-kernel On Thu, 2013-09-12 at 12:24 +0100, Mark Rutland wrote: > > Why can the bootloader not be fixed to set CNTVOFF (or to boot the > kernel in Hyp mode)? It can: http://lists.xen.org/archives/html/xen-devel/2013-09/msg00880.html I was waiting for Andre's hyp mode u-boot patches to land before posting the sunxi bits on top. I'm not sure what the status of that is. Ian. ^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 3/4] Add physical count arch timer support for clocksource in ARMv7. 2013-09-12 11:24 ` Mark Rutland 2013-09-12 14:33 ` Russell King - ARM Linux 2013-09-12 15:39 ` [linux-sunxi] " Ian Campbell @ 2013-09-12 16:07 ` cinifr 2013-09-12 16:39 ` Marc Zyngier 2 siblings, 1 reply; 31+ messages in thread From: cinifr @ 2013-09-12 16:07 UTC (permalink / raw) To: linux-arm-kernel >This cannot be a compile-time option as above in a multiplatform build. >Other paltforms (e.g. KVM guests) *must* use the virtual counters to get >any semblance of a consistent view of time. Yes I accept compile-time option is not perfect in my pre email, But,Why Ohter paltforms *must* use the virtual counters? I think KVM should not limit how to use arch timer in its guest OS. Of cause, if KVM guest use vct can be more efficiency then that use pct. but KVM should and must support guest OS to access pct. >Why can the bootloader not be fixed to set CNTVOFF (or to boot the >kernel in Hyp mode)? I want kernel should try its best to boot and run with the bootloader not be fixed. BTW, I also hope all bootloader fix the problem. ^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 3/4] Add physical count arch timer support for clocksource in ARMv7. 2013-09-12 16:07 ` cinifr @ 2013-09-12 16:39 ` Marc Zyngier 2013-09-13 8:49 ` cinifr 0 siblings, 1 reply; 31+ messages in thread From: Marc Zyngier @ 2013-09-12 16:39 UTC (permalink / raw) To: linux-arm-kernel On 12/09/13 17:07, cinifr wrote: >> This cannot be a compile-time option as above in a multiplatform build. >> Other paltforms (e.g. KVM guests) *must* use the virtual counters to get >> any semblance of a consistent view of time. > Yes I accept compile-time option is not perfect in my pre email, > But,Why Ohter paltforms *must* use the virtual counters? I think KVM > should not limit how to use arch timer in its guest OS. Of cause, if > KVM guest use vct can be more efficiency then that use pct. but KVM > should and must support guest OS to access pct. The virtual counter is there for a good reason: it allows a virtual machine to: - see its time starting at zero - be migrated to another host without seeing time shifting one way or another. So using the physical counter in a VM is a recipe for disaster if you're doing any kind of time tracking. The counter being used for sched_clock(), we cannot afford to see it being shifted one way or another. If you have issues with the use of the virtual counter, I suggest you fix your firmware to have a consistent CNTVOFF across CPUs. And/or even better, boot your kernel in HYP mode, as it will take care of setting CNTVOFF to zero. Cheers, M. -- Jazz is not dead. It just smells funny... ^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 3/4] Add physical count arch timer support for clocksource in ARMv7. 2013-09-12 16:39 ` Marc Zyngier @ 2013-09-13 8:49 ` cinifr 2013-09-13 9:30 ` Marc Zyngier 0 siblings, 1 reply; 31+ messages in thread From: cinifr @ 2013-09-13 8:49 UTC (permalink / raw) To: linux-arm-kernel On 13 September 2013 00:39, Marc Zyngier <marc.zyngier@arm.com> wrote: > On 12/09/13 17:07, cinifr wrote: >>> This cannot be a compile-time option as above in a multiplatform build. >>> Other paltforms (e.g. KVM guests) *must* use the virtual counters to get >>> any semblance of a consistent view of time. >> Yes I accept compile-time option is not perfect in my pre email, >> But,Why Ohter paltforms *must* use the virtual counters? I think KVM >> should not limit how to use arch timer in its guest OS. Of cause, if >> KVM guest use vct can be more efficiency then that use pct. but KVM >> should and must support guest OS to access pct. > > The virtual counter is there for a good reason: it allows a virtual > machine to: > - see its time starting at zero > - be migrated to another host without seeing time shifting one way or > another. > > So using the physical counter in a VM is a recipe for disaster if you're > doing any kind of time tracking. The counter being used for > sched_clock(), we cannot afford to see it being shifted one way or another. I accept that virtual count is better in VM than physical counter because hypversion can modify VM timer by set CNTVOFF. But I think hypversior should support that VM should can access physical counter, When VM use physical count. hypversior could trap accessing physical count from guest OS, and return a value that guest OS want liking hypervisor set CNTVOFF for virtual counter. On this way, VM could too see its timer at zero and VM could too be migrated to another host without seeing time shifting. > If you have issues with the use of the virtual counter, I suggest you > fix your firmware to have a consistent CNTVOFF across CPUs. And/or even > better, boot your kernel in HYP mode, as it will take care of setting > CNTVOFF to zero. > I am wondering what is the principle between kernel and bootload? What should be done in bootloader and what should be done in kernel? As you said, If kernel boot from hyp, Kernel can set CNTVOFF to zero directly, does we add the code to set CNTVOFF in kernel? But, if kernel boot from PL1 NS=0, Does kernel need to switch hyp mode to set CNTVOFF and return PL1 NS=0 mode? Or,kernel dont care it because kernel believe bootloader have set CNTVOFF before? Thanks, Fan. ^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 3/4] Add physical count arch timer support for clocksource in ARMv7. 2013-09-13 8:49 ` cinifr @ 2013-09-13 9:30 ` Marc Zyngier 2013-09-13 13:09 ` cinifr 2013-09-14 12:05 ` maxime.ripard at free-electrons.com 0 siblings, 2 replies; 31+ messages in thread From: Marc Zyngier @ 2013-09-13 9:30 UTC (permalink / raw) To: linux-arm-kernel On 13/09/13 09:49, cinifr wrote: > On 13 September 2013 00:39, Marc Zyngier <marc.zyngier@arm.com> wrote: >> On 12/09/13 17:07, cinifr wrote: >>>> This cannot be a compile-time option as above in a multiplatform build. >>>> Other paltforms (e.g. KVM guests) *must* use the virtual counters to get >>>> any semblance of a consistent view of time. >>> Yes I accept compile-time option is not perfect in my pre email, >>> But,Why Ohter paltforms *must* use the virtual counters? I think KVM >>> should not limit how to use arch timer in its guest OS. Of cause, if >>> KVM guest use vct can be more efficiency then that use pct. but KVM >>> should and must support guest OS to access pct. >> >> The virtual counter is there for a good reason: it allows a virtual >> machine to: >> - see its time starting at zero >> - be migrated to another host without seeing time shifting one way or >> another. >> >> So using the physical counter in a VM is a recipe for disaster if you're >> doing any kind of time tracking. The counter being used for >> sched_clock(), we cannot afford to see it being shifted one way or another. > I accept that virtual count is better in VM than physical counter > because hypversion can modify VM timer by set CNTVOFF. But I think > hypversior should support that VM should can access physical counter, > When VM use physical count. hypversior could trap accessing physical > count from guest OS, and return a value that guest OS want liking > hypervisor set CNTVOFF for virtual counter. On this way, VM could too > see its timer at zero and VM could too be migrated to another host > without seeing time shifting. I urge you to read the ARM ARM, and specifically the section dedicated to trapping access to CP15 operations. If you do, you'll quickly notice that you *cannot* trap accesses to the timer subsystem. All you can do is disable access to the physical timer/counter, resulting in an UNDEF in the *guest*. Additionally, please realise the overhead of trapping is enormous, and that we do try very hard to minimise it. Why do you think we went out of our way to ensure that host and guest would use different timers, always? >> If you have issues with the use of the virtual counter, I suggest you >> fix your firmware to have a consistent CNTVOFF across CPUs. And/or even >> better, boot your kernel in HYP mode, as it will take care of setting >> CNTVOFF to zero. >> > I am wondering what is the principle between kernel and bootload? > What should be done in bootloader and what should be done in kernel? > As you said, If kernel boot from hyp, Kernel can set CNTVOFF to zero > directly, does we add the code to set CNTVOFF in kernel? But, if > kernel boot from PL1 NS=0, Does kernel need to switch hyp mode to > set CNTVOFF and return PL1 NS=0 mode? Or,kernel dont care it because > kernel believe bootloader have set CNTVOFF before? In an ideal world, the bootloader should set CNTVOFF to zero. The fact that the kernel does it too when booted in HYP mode is to preserve itself from from broken bootloaders. CNTVOFF can only be setup from either HYP or Secure Monitor mode with SCR.NS == 1, so if you run your kernel in secure mode, it is always best to do it in the bootloader. M. -- Jazz is not dead. It just smells funny... ^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 3/4] Add physical count arch timer support for clocksource in ARMv7. 2013-09-13 9:30 ` Marc Zyngier @ 2013-09-13 13:09 ` cinifr 2013-09-13 13:40 ` Marc Zyngier 2013-09-14 12:05 ` maxime.ripard at free-electrons.com 1 sibling, 1 reply; 31+ messages in thread From: cinifr @ 2013-09-13 13:09 UTC (permalink / raw) To: linux-arm-kernel > I urge you to read the ARM ARM, and specifically the section dedicated > to trapping access to CP15 operations. If you do, you'll quickly notice > that you *cannot* trap accesses to the timer subsystem. > I read it again. The ARMv7 manual said "Is accessible from Non-secure PL1 modes only when CNTHCTL.PL1PCTEN is set to 1. When CNTHCTL.PL1PCTEN is set to 0, any attempt to access CNTPCT from a Non-secure PL1 mode ***generates a Hyp Trap exception***, see Hyp Trap exception on page B1-1206" in B8.1.2. but I dont find a special hyp trap control for accessing CNTPCT in manual. As you said HSTR cannot trap accessing of CP15 c14. What happer when OS access CNTPCT from PL1 NS=1 mode with CNTHCTL.PL1PCTEN=0 ??? AmI wrong for understanding the manual? Thanks. BTW: And I fond xen have support to trap accessing CNTPCT by this patch on http://lists.xen.org/archives/html/xen-devel/2012-08/msg00452.html. > All you can do is disable access to the physical timer/counter, > resulting in an UNDEF in the *guest*. > > Additionally, please realise the overhead of trapping is enormous, and > that we do try very hard to minimise it. Why do you think we went out of > our way to ensure that host and guest would use different timers, always? > > CNTVOFF can only be setup from either HYP or Secure Monitor mode with > SCR.NS == 1, so if you run your kernel in secure mode, it is always best > to do it in the bootloader. I agree it, Bootloader do it better. Fan ^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 3/4] Add physical count arch timer support for clocksource in ARMv7. 2013-09-13 13:09 ` cinifr @ 2013-09-13 13:40 ` Marc Zyngier 2013-09-13 14:55 ` cinifr 2013-09-18 9:03 ` cinifr 0 siblings, 2 replies; 31+ messages in thread From: Marc Zyngier @ 2013-09-13 13:40 UTC (permalink / raw) To: linux-arm-kernel On 13/09/13 14:09, cinifr wrote: >> I urge you to read the ARM ARM, and specifically the section dedicated >> to trapping access to CP15 operations. If you do, you'll quickly notice >> that you *cannot* trap accesses to the timer subsystem. >> > I read it again. The ARMv7 manual said "Is accessible from Non-secure > PL1 modes only when CNTHCTL.PL1PCTEN is set to 1. When > CNTHCTL.PL1PCTEN is set to 0, any attempt to access CNTPCT from a > Non-secure PL1 mode ***generates a Hyp Trap exception***, see Hyp Trap > exception on page B1-1206" in B8.1.2. but I dont find a special hyp > trap control for accessing CNTPCT in manual. As you said HSTR cannot > trap accessing of CP15 c14. What happer when OS access CNTPCT from PL1 > NS=1 mode with CNTHCTL.PL1PCTEN=0 ??? AmI wrong for understanding > the manual? That's interesting, as I never noticed this particular line in the ARM ARM. I'll investigate this, thanks for bringing it up. This doesn't change the fact that using the physical timer/counter in a VM is (or can be) horribly expensive, and should be avoided at all cost. Thanks, M. -- Jazz is not dead. It just smells funny... ^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 3/4] Add physical count arch timer support for clocksource in ARMv7. 2013-09-13 13:40 ` Marc Zyngier @ 2013-09-13 14:55 ` cinifr 2013-09-18 9:03 ` cinifr 1 sibling, 0 replies; 31+ messages in thread From: cinifr @ 2013-09-13 14:55 UTC (permalink / raw) To: linux-arm-kernel > > This doesn't change the fact that using the physical timer/counter in a > VM is (or can be) horribly expensive, and should be avoided at all cost. > I will remove suport phy timer counter patch and try to fix it in bootloader. I agree that is elegant only vct is be used.in kernel now. ^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 3/4] Add physical count arch timer support for clocksource in ARMv7. 2013-09-13 13:40 ` Marc Zyngier 2013-09-13 14:55 ` cinifr @ 2013-09-18 9:03 ` cinifr 1 sibling, 0 replies; 31+ messages in thread From: cinifr @ 2013-09-18 9:03 UTC (permalink / raw) To: linux-arm-kernel HI all, I have modified uboot code to switch monitor mode and to set cntvoff for all smp cpus for allwinner a20 cpu. It works, kernel can run ok without using cntpct. I will commit the path for uboot after reviewing the code. Rong On 13 September 2013 21:40, Marc Zyngier <marc.zyngier@arm.com> wrote: > On 13/09/13 14:09, cinifr wrote: >>> I urge you to read the ARM ARM, and specifically the section dedicated >>> to trapping access to CP15 operations. If you do, you'll quickly notice >>> that you *cannot* trap accesses to the timer subsystem. >>> >> I read it again. The ARMv7 manual said "Is accessible from Non-secure >> PL1 modes only when CNTHCTL.PL1PCTEN is set to 1. When >> CNTHCTL.PL1PCTEN is set to 0, any attempt to access CNTPCT from a >> Non-secure PL1 mode ***generates a Hyp Trap exception***, see Hyp Trap >> exception on page B1-1206" in B8.1.2. but I dont find a special hyp >> trap control for accessing CNTPCT in manual. As you said HSTR cannot >> trap accessing of CP15 c14. What happer when OS access CNTPCT from PL1 >> NS=1 mode with CNTHCTL.PL1PCTEN=0 ??? AmI wrong for understanding >> the manual? > > That's interesting, as I never noticed this particular line in the ARM > ARM. I'll investigate this, thanks for bringing it up. > > This doesn't change the fact that using the physical timer/counter in a > VM is (or can be) horribly expensive, and should be avoided at all cost. > > Thanks, > > M. > -- > Jazz is not dead. It just smells funny... > ^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 3/4] Add physical count arch timer support for clocksource in ARMv7. 2013-09-13 9:30 ` Marc Zyngier 2013-09-13 13:09 ` cinifr @ 2013-09-14 12:05 ` maxime.ripard at free-electrons.com 2013-09-16 8:16 ` Marc Zyngier 1 sibling, 1 reply; 31+ messages in thread From: maxime.ripard at free-electrons.com @ 2013-09-14 12:05 UTC (permalink / raw) To: linux-arm-kernel Hi Marc, Fan, On Fri, Sep 13, 2013 at 10:30:49AM +0100, Marc Zyngier wrote: > On 13/09/13 09:49, cinifr wrote: > > On 13 September 2013 00:39, Marc Zyngier <marc.zyngier@arm.com> wrote: > > I am wondering what is the principle between kernel and bootload? > > What should be done in bootloader and what should be done in kernel? > > As you said, If kernel boot from hyp, Kernel can set CNTVOFF to zero > > directly, does we add the code to set CNTVOFF in kernel? But, if > > kernel boot from PL1 NS=0, Does kernel need to switch hyp mode to > > set CNTVOFF and return PL1 NS=0 mode? Or,kernel dont care it because > > kernel believe bootloader have set CNTVOFF before? > > In an ideal world, the bootloader should set CNTVOFF to zero. The fact > that the kernel does it too when booted in HYP mode is to preserve > itself from from broken bootloaders. > > CNTVOFF can only be setup from either HYP or Secure Monitor mode with > SCR.NS == 1, so if you run your kernel in secure mode, it is always best > to do it in the bootloader. What would happen exactly if a kernel expects CNTVOFF to be set to 0, and that your bootloader don't set it? >From what you're saying, it's will be set by the kernel if it's booted in hypervisor mode, but what if it's not? The ARM documentation says that the CNTVOFF register will hold an undefined value, how would that affect the kernel? Thanks, Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 836 bytes Desc: Digital signature URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130914/6c9e137f/attachment.sig> ^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 3/4] Add physical count arch timer support for clocksource in ARMv7. 2013-09-14 12:05 ` maxime.ripard at free-electrons.com @ 2013-09-16 8:16 ` Marc Zyngier 0 siblings, 0 replies; 31+ messages in thread From: Marc Zyngier @ 2013-09-16 8:16 UTC (permalink / raw) To: linux-arm-kernel Hi Maxime, On 14/09/13 13:05, maxime.ripard at free-electrons.com wrote: > Hi Marc, Fan, > > On Fri, Sep 13, 2013 at 10:30:49AM +0100, Marc Zyngier wrote: >> On 13/09/13 09:49, cinifr wrote: >>> On 13 September 2013 00:39, Marc Zyngier <marc.zyngier@arm.com> >>> wrote: I am wondering what is the principle between kernel >>> and bootload? What should be done in bootloader and what should >>> be done in kernel? As you said, If kernel boot from hyp, >>> Kernel can set CNTVOFF to zero directly, does we add the code >>> to set CNTVOFF in kernel? But, if kernel boot from PL1 NS=0, >>> Does kernel need to switch hyp mode to set CNTVOFF and return >>> PL1 NS=0 mode? Or,kernel dont care it because kernel believe >>> bootloader have set CNTVOFF before? >> >> In an ideal world, the bootloader should set CNTVOFF to zero. The >> fact that the kernel does it too when booted in HYP mode is to >> preserve itself from from broken bootloaders. >> >> CNTVOFF can only be setup from either HYP or Secure Monitor mode >> with SCR.NS == 1, so if you run your kernel in secure mode, it is >> always best to do it in the bootloader. > > What would happen exactly if a kernel expects CNTVOFF to be set to > 0, and that your bootloader don't set it? It doesn't really matter if it is set to 0. What actually matters is that all the CPUs have the same value. Otherwise, you will have time reported differently depending on the CPU you're looking from. > From what you're saying, it's will be set by the kernel if it's > booted in hypervisor mode, but what if it's not? > > The ARM documentation says that the CNTVOFF register will hold an > undefined value, how would that affect the kernel? See above. Without a consistent view of time across CPUs, you're in deep trouble. M. -- Jazz is not dead. It just smells funny... ^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 4/4] Add arch count timer node in dts for Allwinner A20(sunxi 7i). 2013-09-12 6:51 [PATCH 0/4] Add smp support for Allwinner A20 and phy arch count timer Fan Rong ` (2 preceding siblings ...) 2013-09-12 6:51 ` [PATCH 3/4] Add physical count arch timer support for clocksource in ARMv7 Fan Rong @ 2013-09-12 6:51 ` Fan Rong 2013-09-12 14:57 ` Mark Rutland 2013-09-12 14:39 ` [PATCH 0/4] Add smp support for Allwinner A20 and phy arch count timer Mark Rutland 4 siblings, 1 reply; 31+ messages in thread From: Fan Rong @ 2013-09-12 6:51 UTC (permalink / raw) To: linux-arm-kernel Signed-off-by: Fan Rong <cinifr@gmail.com> --- arch/arm/boot/dts/sun7i-a20.dtsi | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi index bfedcb2..ce138f7 100644 --- a/arch/arm/boot/dts/sun7i-a20.dtsi +++ b/arch/arm/boot/dts/sun7i-a20.dtsi @@ -312,5 +312,14 @@ #interrupt-cells = <3>; interrupts = <1 9 0xf04>; }; + + timer { + compatible ="arm,armv7-timer"; + interrupts = <1 13 0x308>, + <1 14 0x308>, + <1 11 0x308>, + <1 10 0x308>; + clock-frequency = <24000000>; + }; }; }; -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH 4/4] Add arch count timer node in dts for Allwinner A20(sunxi 7i). 2013-09-12 6:51 ` [PATCH 4/4] Add arch count timer node in dts for Allwinner A20(sunxi 7i) Fan Rong @ 2013-09-12 14:57 ` Mark Rutland 2013-09-12 15:44 ` [linux-sunxi] " Ian Campbell 0 siblings, 1 reply; 31+ messages in thread From: Mark Rutland @ 2013-09-12 14:57 UTC (permalink / raw) To: linux-arm-kernel On Thu, Sep 12, 2013 at 07:51:27AM +0100, Fan Rong wrote: > Signed-off-by: Fan Rong <cinifr@gmail.com> > --- > arch/arm/boot/dts/sun7i-a20.dtsi | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi > index bfedcb2..ce138f7 100644 > --- a/arch/arm/boot/dts/sun7i-a20.dtsi > +++ b/arch/arm/boot/dts/sun7i-a20.dtsi > @@ -312,5 +312,14 @@ > #interrupt-cells = <3>; > interrupts = <1 9 0xf04>; > }; > + > + timer { > + compatible ="arm,armv7-timer"; Space after the '=' please. > + interrupts = <1 13 0x308>, > + <1 14 0x308>, > + <1 11 0x308>, > + <1 10 0x308>; > + clock-frequency = <24000000>; If at all possible, your bootloader should set CNTFREQ, and clock-frequency should only be used as a last resort when it's impossible to get it to set CNTFREQ. It's not possible to set CNTFREQ from the non-secure side, so guests (which might not be Linux, and might not handle DT in the same way) may see a completely wrong frequency value. Thanks, Mark. > + }; > }; > }; > -- > 1.7.9.5 > > ^ permalink raw reply [flat|nested] 31+ messages in thread
* [linux-sunxi] Re: [PATCH 4/4] Add arch count timer node in dts for Allwinner A20(sunxi 7i). 2013-09-12 14:57 ` Mark Rutland @ 2013-09-12 15:44 ` Ian Campbell 2013-09-12 16:23 ` cinifr 0 siblings, 1 reply; 31+ messages in thread From: Ian Campbell @ 2013-09-12 15:44 UTC (permalink / raw) To: linux-arm-kernel On Thu, 2013-09-12 at 15:57 +0100, Mark Rutland wrote: > > + interrupts = <1 13 0x308>, > > + <1 14 0x308>, > > + <1 11 0x308>, > > + <1 10 0x308>; > > + clock-frequency = <24000000>; > > If at all possible, your bootloader should set CNTFREQ, and > clock-frequency should only be used as a last resort when it's > impossible to get it to set CNTFREQ. It's not possible to set CNTFREQ > from the non-secure side, so guests (which might not be Linux, and might > not handle DT in the same way) may see a completely wrong frequency > value. https://groups.google.com/forum/#!topic/linux-sunxi/56Wj1IT-pKU Ian. ^ permalink raw reply [flat|nested] 31+ messages in thread
* [linux-sunxi] Re: [PATCH 4/4] Add arch count timer node in dts for Allwinner A20(sunxi 7i). 2013-09-12 15:44 ` [linux-sunxi] " Ian Campbell @ 2013-09-12 16:23 ` cinifr 2013-09-12 19:53 ` Henrik Nordström 0 siblings, 1 reply; 31+ messages in thread From: cinifr @ 2013-09-12 16:23 UTC (permalink / raw) To: linux-arm-kernel On 12 September 2013 23:44, Ian Campbell <ijc@hellion.org.uk> wrote: > On Thu, 2013-09-12 at 15:57 +0100, Mark Rutland wrote: > >> > + interrupts = <1 13 0x308>, >> > + <1 14 0x308>, >> > + <1 11 0x308>, >> > + <1 10 0x308>; >> > + clock-frequency = <24000000>; >> >> If at all possible, your bootloader should set CNTFREQ, and >> clock-frequency should only be used as a last resort when it's >> impossible to get it to set CNTFREQ. It's not possible to set CNTFREQ >> from the non-secure side, so guests (which might not be Linux, and might >> not handle DT in the same way) may see a completely wrong frequency >> value. Yes, My bootloader dont set it, and I have to set it at dtsi fot boot, I found <clock-frequency> is be set in dtsi file for ohters cups. ^ permalink raw reply [flat|nested] 31+ messages in thread
* [linux-sunxi] Re: [PATCH 4/4] Add arch count timer node in dts for Allwinner A20(sunxi 7i). 2013-09-12 16:23 ` cinifr @ 2013-09-12 19:53 ` Henrik Nordström 0 siblings, 0 replies; 31+ messages in thread From: Henrik Nordström @ 2013-09-12 19:53 UTC (permalink / raw) To: linux-arm-kernel fre 2013-09-13 klockan 00:23 +0800 skrev cinifr: > Yes, My bootloader dont set it, and I have to set it at dtsi fot > boot, I found <clock-frequency> is be set in dtsi file for ohters > cups. The patch from Ian is now in sunxi u-boot. Regards Henrik ^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 0/4] Add smp support for Allwinner A20 and phy arch count timer 2013-09-12 6:51 [PATCH 0/4] Add smp support for Allwinner A20 and phy arch count timer Fan Rong ` (3 preceding siblings ...) 2013-09-12 6:51 ` [PATCH 4/4] Add arch count timer node in dts for Allwinner A20(sunxi 7i) Fan Rong @ 2013-09-12 14:39 ` Mark Rutland 2013-09-12 15:46 ` cinifr 4 siblings, 1 reply; 31+ messages in thread From: Mark Rutland @ 2013-09-12 14:39 UTC (permalink / raw) To: linux-arm-kernel On Thu, Sep 12, 2013 at 07:51:23AM +0100, Fan Rong wrote: > The patchs add smp support for Allwinner A20. It add cpuregister node > in dts forsmp configure. The patchs also add a options for phy count > timer to replace vir count timer as ARM arch timer clocksource. About > ARM arch timer: 1. Current kernel use vir count timer, vir count timer > can be accessed in any cpu mode for kernel, but it need bootloader set > vir count offset rigister zero at first. 2. Phy count timer can be > accessed in most cpu mode for kernel except NS-PL1 mode when register > CNTHCTL.PL1PCTEN is set to zero. To ensure to use phy count timer, > bootloader should set register CNTHCTL.PL1PCTEN is 1 at first. At all, > to ensure kernel can use arch timer, bootload should set some generic > timer register(cntvoff or cnthctl) at first. the kernel should select > which count timer by reading current kernel running mode. Sorry, but I find the above text difficult to understand. It jumps between several issues and isn't well formatted. You seem to be suggesting a kernel change (using CNTPCT), but also bootloader changes (setting CNTHCTL.PL1PCTEN) to make this possible at all. If the bootloader needs to be modified, why can it not be modified to set CNTVOFF (or to boot the kernel in Hyp where it can set it itself)? I'm not sure what you mean by selecting which timer to use be reading the current running mode. We currently decide to use CNTVCT if booted in PL1, or CNTPCT if booted in Hyp. I assume this isn't the mode you're referring to? Thanks, Mark. > > Fan Rong (4): > Add smp support for Allwinner A20(sunxi 7i). > Add cpuconfig nodes in dts for smp configure. > Add physical count arch timer support for clocksource in ARMv7. > Add arch count timer node in dts for Allwinner A20(sunxi 7i). > > arch/arm/boot/dts/sun7i-a20.dtsi | 18 +- > arch/arm/include/asm/arch_timer.h | 11 ++ > arch/arm/mach-sunxi/Makefile | 2 + > arch/arm/mach-sunxi/headsmp.S | 12 ++ > arch/arm/mach-sunxi/platform.h | 346 ++++++++++++++++++++++++++++++++++ > arch/arm/mach-sunxi/platsmp.c | 166 ++++++++++++++++ > arch/arm/mach-sunxi/sunxi.c | 4 + > drivers/clocksource/Kconfig | 8 + > drivers/clocksource/arm_arch_timer.c | 10 +- > 9 files changed, 574 insertions(+), 3 deletions(-) > create mode 100644 arch/arm/mach-sunxi/headsmp.S > create mode 100644 arch/arm/mach-sunxi/platform.h > create mode 100644 arch/arm/mach-sunxi/platsmp.c > > -- > 1.7.9.5 > > ^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 0/4] Add smp support for Allwinner A20 and phy arch count timer 2013-09-12 14:39 ` [PATCH 0/4] Add smp support for Allwinner A20 and phy arch count timer Mark Rutland @ 2013-09-12 15:46 ` cinifr 2013-09-13 11:20 ` Mark Rutland 0 siblings, 1 reply; 31+ messages in thread From: cinifr @ 2013-09-12 15:46 UTC (permalink / raw) To: linux-arm-kernel >You seem to be suggesting a kernel change (using CNTPCT), but also >bootloader changes (setting CNTHCTL.PL1PCTEN) to make this possible at >all. If the bootloader needs to be modified, why can it not be modified >to set CNTVOFF (or to boot the kernel in Hyp where it can set it >itself)? I think kernel should can support both CNTVCT and CNTPCT. Yes, if bootloader have set CNTVOFF to zero, then CNTVCT is OK, kvm guest using CNTVCT can run more efficient then that using CNTPCT. but if bootloader dont set it, how about kernel booting? I think kernel should try it's best to boot and run ok even bootload dont set any generic timer register including CNTVOFF and CNTHCTL. So i gave a compile options using CNTPCT. That is only options, If CNTVCT can not working, you have others choice. Of cause, It is best that kerne can select which timer count is used in running time, >I'm not sure what you mean by selecting which timer to use be reading >the current running mode. We currently decide to use CNTVCT if booted in >PL1, or CNTPCT if booted in Hyp. I assume this isn't the mode you're >referring to? Yep, kernel can run PL1 NS=1, PL1 NS=0, PL2. If kernel can know current running Mode.then kernel can chose which timer is OK in running time. 1: If kernel is runing at PL2 and PL1 NS=0, then CNTPCT is OK in any case even CNTVOFF is not zero and CNTHCTL.PL1PCTEN is zero. 2: if kernel is running at PL1 NS=1,then kernel maybe should select CNTVCT. But it has risk to using CNTVCT when CNTVOFF is not zero. How to deal with the case CNTHCTL.PL1PCTEN is zero and CNTVOFF is not zero? current kernel cant using any arch timer incluing CNTVCT and CNTPCT. with this case, I think kernel should use CNTVCT by other ways: Kernel runing CPUn read CNTVCT and save it to local variable for example InitVctVALUEn in initialization, then kernel running CPUn read timer later return a value as ReadTIMERn=CNTVCTn-InitVctVALUEn, This way can run in any generic timer registe set and in any kernel runing mode. I try to write this patch for new way. But the new way should need more time than old in read timer funcation because it need more calculate. . Thanks for your question. On 12 September 2013 22:39, Mark Rutland <mark.rutland@arm.com> wrote: > On Thu, Sep 12, 2013 at 07:51:23AM +0100, Fan Rong wrote: >> The patchs add smp support for Allwinner A20. It add cpuregister node >> in dts forsmp configure. The patchs also add a options for phy count >> timer to replace vir count timer as ARM arch timer clocksource. About >> ARM arch timer: 1. Current kernel use vir count timer, vir count timer >> can be accessed in any cpu mode for kernel, but it need bootloader set >> vir count offset rigister zero at first. 2. Phy count timer can be >> accessed in most cpu mode for kernel except NS-PL1 mode when register >> CNTHCTL.PL1PCTEN is set to zero. To ensure to use phy count timer, >> bootloader should set register CNTHCTL.PL1PCTEN is 1 at first. At all, >> to ensure kernel can use arch timer, bootload should set some generic >> timer register(cntvoff or cnthctl) at first. the kernel should select >> which count timer by reading current kernel running mode. > > Sorry, but I find the above text difficult to understand. It jumps > between several issues and isn't well formatted. > > You seem to be suggesting a kernel change (using CNTPCT), but also > bootloader changes (setting CNTHCTL.PL1PCTEN) to make this possible at > all. If the bootloader needs to be modified, why can it not be modified > to set CNTVOFF (or to boot the kernel in Hyp where it can set it > itself)? > > I'm not sure what you mean by selecting which timer to use be reading > the current running mode. We currently decide to use CNTVCT if booted in > PL1, or CNTPCT if booted in Hyp. I assume this isn't the mode you're > referring to? > > Thanks, > Mark. > >> >> Fan Rong (4): >> Add smp support for Allwinner A20(sunxi 7i). >> Add cpuconfig nodes in dts for smp configure. >> Add physical count arch timer support for clocksource in ARMv7. >> Add arch count timer node in dts for Allwinner A20(sunxi 7i). >> >> arch/arm/boot/dts/sun7i-a20.dtsi | 18 +- >> arch/arm/include/asm/arch_timer.h | 11 ++ >> arch/arm/mach-sunxi/Makefile | 2 + >> arch/arm/mach-sunxi/headsmp.S | 12 ++ >> arch/arm/mach-sunxi/platform.h | 346 ++++++++++++++++++++++++++++++++++ >> arch/arm/mach-sunxi/platsmp.c | 166 ++++++++++++++++ >> arch/arm/mach-sunxi/sunxi.c | 4 + >> drivers/clocksource/Kconfig | 8 + >> drivers/clocksource/arm_arch_timer.c | 10 +- >> 9 files changed, 574 insertions(+), 3 deletions(-) >> create mode 100644 arch/arm/mach-sunxi/headsmp.S >> create mode 100644 arch/arm/mach-sunxi/platform.h >> create mode 100644 arch/arm/mach-sunxi/platsmp.c >> >> -- >> 1.7.9.5 >> >> ^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 0/4] Add smp support for Allwinner A20 and phy arch count timer 2013-09-12 15:46 ` cinifr @ 2013-09-13 11:20 ` Mark Rutland 2013-09-13 13:23 ` cinifr 0 siblings, 1 reply; 31+ messages in thread From: Mark Rutland @ 2013-09-13 11:20 UTC (permalink / raw) To: linux-arm-kernel On Thu, Sep 12, 2013 at 04:46:42PM +0100, cinifr wrote: > >You seem to be suggesting a kernel change (using CNTPCT), but also > >bootloader changes (setting CNTHCTL.PL1PCTEN) to make this possible at > >all. If the bootloader needs to be modified, why can it not be modified > >to set CNTVOFF (or to boot the kernel in Hyp where it can set it > >itself)? > > I think kernel should can support both CNTVCT and CNTPCT. Yes, if > bootloader have set CNTVOFF to zero, then CNTVCT is OK, kvm guest > using CNTVCT can run more efficient then that using CNTPCT. but if > bootloader dont set it, how about kernel booting? I think kernel > should try it's best to boot and run ok even bootload dont set any > generic timer register including CNTVOFF and CNTHCTL. So i gave a > compile options using CNTPCT. That is only options, If CNTVCT can not > working, you have others choice. > Of cause, It is best that kerne can select which timer count is used > in running time, CNTVOFF doesn't need to be zero -- when a guest runs under a hypervisor, CNTVOFF may change across a suspend/resume of a VM (to give the guest the illusion that time wasn't ticking when it wasn't running). All that's required is that all the CPUs have the same CNTVOFF value, and this has been valid for all platforms so far. Does CNTVOFF vary between your CPUs, or are they a consistent value (event if it's not zero)? For ARMv8 systems, CNTVOFF and CNTHCTL reset to an UNDEFINED value, so we cannot rely on the physical timers and counters being available -- the firmware and/or hypervisor must set at least one of them for an OS to be able to use the system. The virtual timers and counters are *always* available to PL1/EL1, so our best bet is to use them. I'd prefer not to have to have a run-time solution to a problem that can be avoided entirely with a simple modification to the bootloader now. > > >I'm not sure what you mean by selecting which timer to use be reading > >the current running mode. We currently decide to use CNTVCT if booted in > >PL1, or CNTPCT if booted in Hyp. I assume this isn't the mode you're > >referring to? > Yep, kernel can run PL1 NS=1, PL1 NS=0, PL2. If kernel can know > current running Mode.then kernel can chose which timer is OK in > running time. 1: If kernel is runing at PL2 and PL1 NS=0, then CNTPCT > is OK in any case even CNTVOFF is not zero and CNTHCTL.PL1PCTEN is > zero. 2: if kernel is running at PL1 NS=1,then kernel maybe should > select CNTVCT. But it has risk to using CNTVCT when CNTVOFF is not > zero. > How to deal with the case CNTHCTL.PL1PCTEN is zero and CNTVOFF is not > zero? current kernel cant using any arch timer incluing CNTVCT and > CNTPCT. with this case, I think kernel should use CNTVCT by other > ways: Kernel runing CPUn read CNTVCT and save it to local variable > for example InitVctVALUEn in initialization, then kernel running CPUn > read timer later return a value as ReadTIMERn=CNTVCTn-InitVctVALUEn, > This way can run in any generic timer registe set and in any kernel > runing mode. I try to write this patch for new way. But the new way > should need more time than old in read timer funcation because it > need more calculate. I don't think that would work. You have no way of ensuring that all CPUs read CNTVCT at the same time, so they may record offsets that give them different views of time. Consider the case that CNTVOFF was zero on all CPUs. CPU0 and CPU1 might read CNTVCT at different instants, and CPU1 could record its offset as 100 while CPU1 could record its offset as 2000. That would leave CPU1 thinking it's further ahead in time than CPU0, which could break all sorts of things. AFAICS there's no way of telling this apart from each CPU booting with a different CNTVOFF. As SMP support for this platform is not yet in mainline, and the bootloader can be fixed to set CNTVOFF (as KVM and Xen do for guests), we should get the bootloader to set CNTVOFF to a consistent value across all CPUs. Thanks, Mark. > > . > Thanks for your question. > > On 12 September 2013 22:39, Mark Rutland <mark.rutland@arm.com> wrote: > > On Thu, Sep 12, 2013 at 07:51:23AM +0100, Fan Rong wrote: > >> The patchs add smp support for Allwinner A20. It add cpuregister node > >> in dts forsmp configure. The patchs also add a options for phy count > >> timer to replace vir count timer as ARM arch timer clocksource. About > >> ARM arch timer: 1. Current kernel use vir count timer, vir count timer > >> can be accessed in any cpu mode for kernel, but it need bootloader set > >> vir count offset rigister zero at first. 2. Phy count timer can be > >> accessed in most cpu mode for kernel except NS-PL1 mode when register > >> CNTHCTL.PL1PCTEN is set to zero. To ensure to use phy count timer, > >> bootloader should set register CNTHCTL.PL1PCTEN is 1 at first. At all, > >> to ensure kernel can use arch timer, bootload should set some generic > >> timer register(cntvoff or cnthctl) at first. the kernel should select > >> which count timer by reading current kernel running mode. > > > > Sorry, but I find the above text difficult to understand. It jumps > > between several issues and isn't well formatted. > > > > You seem to be suggesting a kernel change (using CNTPCT), but also > > bootloader changes (setting CNTHCTL.PL1PCTEN) to make this possible at > > all. If the bootloader needs to be modified, why can it not be modified > > to set CNTVOFF (or to boot the kernel in Hyp where it can set it > > itself)? > > > > I'm not sure what you mean by selecting which timer to use be reading > > the current running mode. We currently decide to use CNTVCT if booted in > > PL1, or CNTPCT if booted in Hyp. I assume this isn't the mode you're > > referring to? > > > > Thanks, > > Mark. > > > >> > >> Fan Rong (4): > >> Add smp support for Allwinner A20(sunxi 7i). > >> Add cpuconfig nodes in dts for smp configure. > >> Add physical count arch timer support for clocksource in ARMv7. > >> Add arch count timer node in dts for Allwinner A20(sunxi 7i). > >> > >> arch/arm/boot/dts/sun7i-a20.dtsi | 18 +- > >> arch/arm/include/asm/arch_timer.h | 11 ++ > >> arch/arm/mach-sunxi/Makefile | 2 + > >> arch/arm/mach-sunxi/headsmp.S | 12 ++ > >> arch/arm/mach-sunxi/platform.h | 346 ++++++++++++++++++++++++++++++++++ > >> arch/arm/mach-sunxi/platsmp.c | 166 ++++++++++++++++ > >> arch/arm/mach-sunxi/sunxi.c | 4 + > >> drivers/clocksource/Kconfig | 8 + > >> drivers/clocksource/arm_arch_timer.c | 10 +- > >> 9 files changed, 574 insertions(+), 3 deletions(-) > >> create mode 100644 arch/arm/mach-sunxi/headsmp.S > >> create mode 100644 arch/arm/mach-sunxi/platform.h > >> create mode 100644 arch/arm/mach-sunxi/platsmp.c > >> > >> -- > >> 1.7.9.5 > >> > >> > ^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 0/4] Add smp support for Allwinner A20 and phy arch count timer 2013-09-13 11:20 ` Mark Rutland @ 2013-09-13 13:23 ` cinifr 0 siblings, 0 replies; 31+ messages in thread From: cinifr @ 2013-09-13 13:23 UTC (permalink / raw) To: linux-arm-kernel On 13 September 2013 19:20, Mark Rutland <mark.rutland@arm.com> wrote: > On Thu, Sep 12, 2013 at 04:46:42PM +0100, cinifr wrote: >> >You seem to be suggesting a kernel change (using CNTPCT), but also >> >bootloader changes (setting CNTHCTL.PL1PCTEN) to make this possible at >> >all. If the bootloader needs to be modified, why can it not be modified >> >to set CNTVOFF (or to boot the kernel in Hyp where it can set it >> >itself)? >> >> I think kernel should can support both CNTVCT and CNTPCT. Yes, if >> bootloader have set CNTVOFF to zero, then CNTVCT is OK, kvm guest >> using CNTVCT can run more efficient then that using CNTPCT. but if >> bootloader dont set it, how about kernel booting? I think kernel >> should try it's best to boot and run ok even bootload dont set any >> generic timer register including CNTVOFF and CNTHCTL. So i gave a >> compile options using CNTPCT. That is only options, If CNTVCT can not >> working, you have others choice. >> Of cause, It is best that kerne can select which timer count is used >> in running time, > > CNTVOFF doesn't need to be zero -- when a guest runs under a hypervisor, > CNTVOFF may change across a suspend/resume of a VM (to give the guest > the illusion that time wasn't ticking when it wasn't running). All > that's required is that all the CPUs have the same CNTVOFF value, and > this has been valid for all platforms so far. > > Does CNTVOFF vary between your CPUs, or are they a consistent value > (event if it's not zero)? Yes, It is vary in A20 CPUS, > For ARMv8 systems, CNTVOFF and CNTHCTL reset to an UNDEFINED value, so > we cannot rely on the physical timers and counters being available -- > the firmware and/or hypervisor must set at least one of them for an OS > to be able to use the system. The virtual timers and counters are > *always* available to PL1/EL1, so our best bet is to use them. > > I'd prefer not to have to have a run-time solution to a problem that can > be avoided entirely with a simple modification to the bootloader now. I agree it now, >> >> >I'm not sure what you mean by selecting which timer to use be reading >> >the current running mode. We currently decide to use CNTVCT if booted in >> >PL1, or CNTPCT if booted in Hyp. I assume this isn't the mode you're >> >referring to? >> Yep, kernel can run PL1 NS=1, PL1 NS=0, PL2. If kernel can know >> current running Mode.then kernel can chose which timer is OK in >> running time. 1: If kernel is runing at PL2 and PL1 NS=0, then CNTPCT >> is OK in any case even CNTVOFF is not zero and CNTHCTL.PL1PCTEN is >> zero. 2: if kernel is running at PL1 NS=1,then kernel maybe should >> select CNTVCT. But it has risk to using CNTVCT when CNTVOFF is not >> zero. >> How to deal with the case CNTHCTL.PL1PCTEN is zero and CNTVOFF is not >> zero? current kernel cant using any arch timer incluing CNTVCT and >> CNTPCT. with this case, I think kernel should use CNTVCT by other >> ways: Kernel runing CPUn read CNTVCT and save it to local variable >> for example InitVctVALUEn in initialization, then kernel running CPUn >> read timer later return a value as ReadTIMERn=CNTVCTn-InitVctVALUEn, >> This way can run in any generic timer registe set and in any kernel >> runing mode. I try to write this patch for new way. But the new way >> should need more time than old in read timer funcation because it >> need more calculate. > > I don't think that would work. You have no way of ensuring that all CPUs > read CNTVCT at the same time, so they may record offsets that give them > different views of time. Consider the case that CNTVOFF was zero on all > CPUs. CPU0 and CPU1 might read CNTVCT at different instants, and CPU1 > could record its offset as 100 while CPU1 could record its offset as > 2000. That would leave CPU1 thinking it's further ahead in time than > CPU0, which could break all sorts of things. AFAICS there's no way of > telling this apart from each CPU booting with a different CNTVOFF. > As SMP support for this platform is not yet in mainline, and the > bootloader can be fixed to set CNTVOFF (as KVM and Xen do for guests), > we should get the bootloader to set CNTVOFF to a consistent value across > all CPUs. Yes, you are right, I will remove phy count timer support patch.and try to modify bootloader for fix the problem. Bootloader do it better, more simple and more efficiency. Thanks Fan. ^ permalink raw reply [flat|nested] 31+ messages in thread
end of thread, other threads:[~2013-09-18 9:03 UTC | newest] Thread overview: 31+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-09-12 6:51 [PATCH 0/4] Add smp support for Allwinner A20 and phy arch count timer Fan Rong 2013-09-12 6:51 ` [PATCH 1/4] Add smp support for Allwinner A20(sunxi 7i) Fan Rong 2013-09-12 14:26 ` Mark Rutland 2013-09-12 15:53 ` cinifr 2013-09-12 14:40 ` Russell King - ARM Linux 2013-09-12 6:51 ` [PATCH 2/4] Add cpuconfig nodes in dts for smp configure Fan Rong 2013-09-14 11:50 ` Maxime Ripard 2013-09-12 6:51 ` [PATCH 3/4] Add physical count arch timer support for clocksource in ARMv7 Fan Rong 2013-09-12 11:24 ` Mark Rutland 2013-09-12 14:33 ` Russell King - ARM Linux 2013-09-12 16:09 ` cinifr 2013-09-12 15:39 ` [linux-sunxi] " Ian Campbell 2013-09-12 16:07 ` cinifr 2013-09-12 16:39 ` Marc Zyngier 2013-09-13 8:49 ` cinifr 2013-09-13 9:30 ` Marc Zyngier 2013-09-13 13:09 ` cinifr 2013-09-13 13:40 ` Marc Zyngier 2013-09-13 14:55 ` cinifr 2013-09-18 9:03 ` cinifr 2013-09-14 12:05 ` maxime.ripard at free-electrons.com 2013-09-16 8:16 ` Marc Zyngier 2013-09-12 6:51 ` [PATCH 4/4] Add arch count timer node in dts for Allwinner A20(sunxi 7i) Fan Rong 2013-09-12 14:57 ` Mark Rutland 2013-09-12 15:44 ` [linux-sunxi] " Ian Campbell 2013-09-12 16:23 ` cinifr 2013-09-12 19:53 ` Henrik Nordström 2013-09-12 14:39 ` [PATCH 0/4] Add smp support for Allwinner A20 and phy arch count timer Mark Rutland 2013-09-12 15:46 ` cinifr 2013-09-13 11:20 ` Mark Rutland 2013-09-13 13:23 ` cinifr
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).