linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH 0/7] Linux on Apple Silicon
@ 2021-01-20 13:27 Mohamed Mediouni
  2021-01-20 13:27 ` [RFC PATCH 1/7] arm64: kernel: FIQ support Mohamed Mediouni
                   ` (6 more replies)
  0 siblings, 7 replies; 55+ messages in thread
From: Mohamed Mediouni @ 2021-01-20 13:27 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Catalin Marinas, Will Deacon, Mark Rutland, Marc Zyngier,
	Hector Martin, linux-kernel, Mohamed Mediouni

This patch series contains the changes for a minimal
Linux on Apple Silicon boot, including SMP.

(sorry for the resubmission, I didn't attach the drivers
to the ones beforehand, and didn't submit it properly)

The changes:

- Support for FIQ interrupts in-kernel

This is required for the timer and IPIs on Apple SoCs.

- WFI hook

Apple processors do not keep register state across WFI.
As such, put a mechanism in cpu_ops to put a custom
sleep function instead.

- use nGnRnE instead of nGnRE on Apple processors

Device-nGnRE writes go to nowhere on Apple processors, as
such use MAIR to change those to Device-nGnRE writes.

- Apple AIC driver

Driver for the Apple AIC interrupt controller.

- Apple CPU start driver

On Apple Macs, RVBAR is locked by the bootloader.
And the hardware doesn't have EL3 to provide PSCI
as an option either. This also implements the workaround
for WFI on the hardware.

What is not present:

- Device tree, will be present in a future version of this
patchset

- More devices.

Thank you,

Mohamed Mediouni (1):
  arm64: mm: use nGnRnE instead of nGnRE on Apple processors

Stan Skowronek (6):
  arm64: kernel: FIQ support
  arm64: kernel: Add a WFI hook.
  irqchip/apple-aic: Add support for Apple AIC
  arm64/Kconfig: Add Apple Silicon SoC platform
  arm64: kernel: Apple CPU start driver
  irqchip/apple-aic: add SMP support to the Apple AIC driver.

 .../devicetree/bindings/arm/cpus.yaml         |   1 +
 .../interrupt-controller/apple,aic.yaml       |  49 +++
 MAINTAINERS                                   |   6 +
 arch/arm64/Kconfig.platforms                  |   7 +
 arch/arm64/include/asm/arch_gicv3.h           |   2 +-
 arch/arm64/include/asm/assembler.h            |   8 +-
 arch/arm64/include/asm/cpu_ops.h              |   2 +
 arch/arm64/include/asm/daifflags.h            |   4 +-
 arch/arm64/include/asm/irq.h                  |   4 +
 arch/arm64/include/asm/irqflags.h             |   6 +-
 arch/arm64/kernel/Makefile                    |   1 +
 arch/arm64/kernel/apple_cpustart.c            | 153 ++++++++
 arch/arm64/kernel/cpu_ops.c                   |   6 +
 arch/arm64/kernel/entry.S                     |  74 +++-
 arch/arm64/kernel/irq.c                       |  14 +
 arch/arm64/kernel/process.c                   |  13 +-
 arch/arm64/mm/proc.S                          |  26 ++
 drivers/irqchip/Kconfig                       |   6 +
 drivers/irqchip/Makefile                      |   1 +
 drivers/irqchip/irq-apple-aic.c               | 364 ++++++++++++++++++
 20 files changed, 728 insertions(+), 19 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/apple,aic.yaml
 create mode 100644 arch/arm64/kernel/apple_cpustart.c
 create mode 100644 drivers/irqchip/irq-apple-aic.c

--
2.29.2


^ permalink raw reply	[flat|nested] 55+ messages in thread

* [RFC PATCH 1/7] arm64: kernel: FIQ support
  2021-01-20 13:27 [RFC PATCH 0/7] Linux on Apple Silicon Mohamed Mediouni
@ 2021-01-20 13:27 ` Mohamed Mediouni
  2021-01-20 13:27 ` [RFC PATCH 2/7] arm64: kernel: Add a WFI hook Mohamed Mediouni
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 55+ messages in thread
From: Mohamed Mediouni @ 2021-01-20 13:27 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Catalin Marinas, Will Deacon, Mark Rutland, Marc Zyngier,
	Hector Martin, linux-kernel, Stan Skowronek, Mohamed Mediouni

From: Stan Skowronek <stan@corellium.com>

On Apple processors, the timer is wired through FIQ.
As such, add FIQ support to the kernel.

Signed-off-by: Stan Skowronek <stan@corellium.com>
Signed-off-by: Mohamed Mediouni <mohamed.mediouni@caramail.com>
---
 arch/arm64/include/asm/arch_gicv3.h |  2 +-
 arch/arm64/include/asm/assembler.h  |  8 ++--
 arch/arm64/include/asm/daifflags.h  |  4 +-
 arch/arm64/include/asm/irq.h        |  4 ++
 arch/arm64/include/asm/irqflags.h   |  6 +--
 arch/arm64/kernel/entry.S           | 74 ++++++++++++++++++++++++++---
 arch/arm64/kernel/irq.c             | 14 ++++++
 arch/arm64/kernel/process.c         |  2 +-
 8 files changed, 97 insertions(+), 17 deletions(-)

diff --git a/arch/arm64/include/asm/arch_gicv3.h b/arch/arm64/include/asm/arch_gicv3.h
index 880b9054d75c..934b9be582d2 100644
--- a/arch/arm64/include/asm/arch_gicv3.h
+++ b/arch/arm64/include/asm/arch_gicv3.h
@@ -173,7 +173,7 @@ static inline void gic_pmr_mask_irqs(void)

 static inline void gic_arch_enable_irqs(void)
 {
-	asm volatile ("msr daifclr, #2" : : : "memory");
+	asm volatile ("msr daifclr, #3" : : : "memory");
 }

 #endif /* __ASSEMBLY__ */
diff --git a/arch/arm64/include/asm/assembler.h b/arch/arm64/include/asm/assembler.h
index bf125c591116..6fe55713dfe0 100644
--- a/arch/arm64/include/asm/assembler.h
+++ b/arch/arm64/include/asm/assembler.h
@@ -40,9 +40,9 @@
 	msr	daif, \flags
 	.endm

-	/* IRQ is the lowest priority flag, unconditionally unmask the rest. */
-	.macro enable_da_f
-	msr	daifclr, #(8 | 4 | 1)
+	/* IRQ/FIQ is the lowest priority flag, unconditionally unmask the rest. */
+	.macro enable_da
+	msr	daifclr, #(8 | 4)
 	.endm

 /*
@@ -50,7 +50,7 @@
  */
 	.macro	save_and_disable_irq, flags
 	mrs	\flags, daif
-	msr	daifset, #2
+	msr	daifset, #3
 	.endm

 	.macro	restore_irq, flags
diff --git a/arch/arm64/include/asm/daifflags.h b/arch/arm64/include/asm/daifflags.h
index 1c26d7baa67f..44de96c7fb1a 100644
--- a/arch/arm64/include/asm/daifflags.h
+++ b/arch/arm64/include/asm/daifflags.h
@@ -13,8 +13,8 @@
 #include <asm/ptrace.h>

 #define DAIF_PROCCTX		0
-#define DAIF_PROCCTX_NOIRQ	PSR_I_BIT
-#define DAIF_ERRCTX		(PSR_I_BIT | PSR_A_BIT)
+#define DAIF_PROCCTX_NOIRQ	(PSR_I_BIT | PSR_F_BIT)
+#define DAIF_ERRCTX		(PSR_I_BIT | PSR_F_BIT | PSR_A_BIT)
 #define DAIF_MASK		(PSR_D_BIT | PSR_A_BIT | PSR_I_BIT | PSR_F_BIT)


diff --git a/arch/arm64/include/asm/irq.h b/arch/arm64/include/asm/irq.h
index b2b0c6405eb0..2d1537d3a245 100644
--- a/arch/arm64/include/asm/irq.h
+++ b/arch/arm64/include/asm/irq.h
@@ -13,5 +13,9 @@ static inline int nr_legacy_irqs(void)
 	return 0;
 }

+int set_handle_fiq(void (*handle_fiq)(struct pt_regs *));
+
+extern void (*handle_arch_fiq)(struct pt_regs *) __ro_after_init;
+
 #endif /* !__ASSEMBLER__ */
 #endif
diff --git a/arch/arm64/include/asm/irqflags.h b/arch/arm64/include/asm/irqflags.h
index ff328e5bbb75..26d7f378113e 100644
--- a/arch/arm64/include/asm/irqflags.h
+++ b/arch/arm64/include/asm/irqflags.h
@@ -35,7 +35,7 @@ static inline void arch_local_irq_enable(void)
 	}

 	asm volatile(ALTERNATIVE(
-		"msr	daifclr, #2		// arch_local_irq_enable",
+		"msr	daifclr, #3		// arch_local_irq_enable",
 		__msr_s(SYS_ICC_PMR_EL1, "%0"),
 		ARM64_HAS_IRQ_PRIO_MASKING)
 		:
@@ -54,7 +54,7 @@ static inline void arch_local_irq_disable(void)
 	}

 	asm volatile(ALTERNATIVE(
-		"msr	daifset, #2		// arch_local_irq_disable",
+		"msr	daifset, #3		// arch_local_irq_disable",
 		__msr_s(SYS_ICC_PMR_EL1, "%0"),
 		ARM64_HAS_IRQ_PRIO_MASKING)
 		:
@@ -85,7 +85,7 @@ static inline int arch_irqs_disabled_flags(unsigned long flags)
 	int res;

 	asm volatile(ALTERNATIVE(
-		"and	%w0, %w1, #" __stringify(PSR_I_BIT),
+		"and	%w0, %w1, #" __stringify(PSR_I_BIT | PSR_F_BIT),
 		"eor	%w0, %w1, #" __stringify(GIC_PRIO_IRQON),
 		ARM64_HAS_IRQ_PRIO_MASKING)
 		: "=&r" (res)
diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
index c9bae73f2621..abcca0db0736 100644
--- a/arch/arm64/kernel/entry.S
+++ b/arch/arm64/kernel/entry.S
@@ -499,6 +499,14 @@ tsk	.req	x28		// current thread_info
 	irq_stack_exit
 	.endm

+	.macro	fiq_handler
+	ldr_l	x1, handle_arch_fiq
+	mov	x0, sp
+	irq_stack_entry
+	blr	x1
+	irq_stack_exit
+	.endm
+
 #ifdef CONFIG_ARM64_PSEUDO_NMI
 	/*
 	 * Set res to 0 if irqs were unmasked in interrupted context.
@@ -547,18 +555,18 @@ SYM_CODE_START(vectors)

 	kernel_ventry	1, sync				// Synchronous EL1h
 	kernel_ventry	1, irq				// IRQ EL1h
-	kernel_ventry	1, fiq_invalid			// FIQ EL1h
+	kernel_ventry	1, fiq				// FIQ EL1h
 	kernel_ventry	1, error			// Error EL1h

 	kernel_ventry	0, sync				// Synchronous 64-bit EL0
 	kernel_ventry	0, irq				// IRQ 64-bit EL0
-	kernel_ventry	0, fiq_invalid			// FIQ 64-bit EL0
+	kernel_ventry	0, fiq				// FIQ 64-bit EL0
 	kernel_ventry	0, error			// Error 64-bit EL0

 #ifdef CONFIG_COMPAT
 	kernel_ventry	0, sync_compat, 32		// Synchronous 32-bit EL0
 	kernel_ventry	0, irq_compat, 32		// IRQ 32-bit EL0
-	kernel_ventry	0, fiq_invalid_compat, 32	// FIQ 32-bit EL0
+	kernel_ventry	0, fiq_compat, 32		// FIQ 32-bit EL0
 	kernel_ventry	0, error_compat, 32		// Error 32-bit EL0
 #else
 	kernel_ventry	0, sync_invalid, 32		// Synchronous 32-bit EL0
@@ -661,7 +669,7 @@ SYM_CODE_END(el1_sync)
 SYM_CODE_START_LOCAL_NOALIGN(el1_irq)
 	kernel_entry 1
 	gic_prio_irq_setup pmr=x20, tmp=x1
-	enable_da_f
+	enable_da

 	mov	x0, sp
 	bl	enter_el1_irq_or_nmi
@@ -689,6 +697,38 @@ alternative_else_nop_endif
 	kernel_exit 1
 SYM_CODE_END(el1_irq)

+	.align	6
+SYM_CODE_START_LOCAL_NOALIGN(el1_fiq)
+	kernel_entry 1
+	gic_prio_irq_setup pmr=x20, tmp=x1
+	enable_da
+
+	mov	x0, sp
+	bl	enter_el1_irq_or_nmi
+
+	fiq_handler
+
+#ifdef CONFIG_PREEMPTION
+	ldr	x24, [tsk, #TSK_TI_PREEMPT]	// get preempt count
+alternative_if ARM64_HAS_IRQ_PRIO_MASKING
+	/*
+	 * DA_F were cleared at start of handling. If anything is set in DAIF,
+	 * we come back from an NMI, so skip preemption
+	 */
+	mrs	x0, daif
+	orr	x24, x24, x0
+alternative_else_nop_endif
+	cbnz	x24, 1f				// preempt count != 0 || NMI return path
+	bl	arm64_preempt_schedule_irq	// irq en/disable is done inside
+1:
+#endif
+
+	mov	x0, sp
+	bl	exit_el1_irq_or_nmi
+
+	kernel_exit 1
+SYM_CODE_END(el1_fiq)
+
 /*
  * EL0 mode handlers.
  */
@@ -715,6 +755,12 @@ SYM_CODE_START_LOCAL_NOALIGN(el0_irq_compat)
 	b	el0_irq_naked
 SYM_CODE_END(el0_irq_compat)

+	.align	6
+SYM_CODE_START_LOCAL_NOALIGN(el0_fiq_compat)
+	kernel_entry 0, 32
+	b	el0_fiq_naked
+SYM_CODE_END(el0_fiq_compat)
+
 SYM_CODE_START_LOCAL_NOALIGN(el0_error_compat)
 	kernel_entry 0, 32
 	b	el0_error_naked
@@ -727,7 +773,7 @@ SYM_CODE_START_LOCAL_NOALIGN(el0_irq)
 el0_irq_naked:
 	gic_prio_irq_setup pmr=x20, tmp=x0
 	user_exit_irqoff
-	enable_da_f
+	enable_da

 	tbz	x22, #55, 1f
 	bl	do_el0_irq_bp_hardening
@@ -737,6 +783,22 @@ el0_irq_naked:
 	b	ret_to_user
 SYM_CODE_END(el0_irq)

+	.align	6
+SYM_CODE_START_LOCAL_NOALIGN(el0_fiq)
+	kernel_entry 0
+el0_fiq_naked:
+	gic_prio_irq_setup pmr=x20, tmp=x0
+	user_exit_irqoff
+	enable_da
+
+	tbz	x22, #55, 1f
+	bl	do_el0_irq_bp_hardening
+1:
+	fiq_handler
+
+	b	ret_to_user
+SYM_CODE_END(el0_fiq)
+
 SYM_CODE_START_LOCAL(el1_error)
 	kernel_entry 1
 	mrs	x1, esr_el1
@@ -757,7 +819,7 @@ el0_error_naked:
 	mov	x0, sp
 	mov	x1, x25
 	bl	do_serror
-	enable_da_f
+	enable_da
 	b	ret_to_user
 SYM_CODE_END(el0_error)

diff --git a/arch/arm64/kernel/irq.c b/arch/arm64/kernel/irq.c
index dfb1feab867d..4d7a9fb41d93 100644
--- a/arch/arm64/kernel/irq.c
+++ b/arch/arm64/kernel/irq.c
@@ -88,3 +88,17 @@ void __init init_IRQ(void)
 		local_daif_restore(DAIF_PROCCTX_NOIRQ);
 	}
 }
+
+/*
+ * Analogous to the generic handle_arch_irq / set_handle_irq
+ */
+void (*handle_arch_fiq)(struct pt_regs *) __ro_after_init;
+
+int __init set_handle_fiq(void (*handle_fiq)(struct pt_regs *))
+{
+	if (handle_arch_fiq)
+		return -EBUSY;
+
+	handle_arch_fiq = handle_fiq;
+	return 0;
+}
diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
index 6616486a58fe..34ec400288d0 100644
--- a/arch/arm64/kernel/process.c
+++ b/arch/arm64/kernel/process.c
@@ -84,7 +84,7 @@ static void noinstr __cpu_do_idle_irqprio(void)
 	unsigned long daif_bits;

 	daif_bits = read_sysreg(daif);
-	write_sysreg(daif_bits | PSR_I_BIT, daif);
+	write_sysreg(daif_bits | PSR_I_BIT | PSR_F_BIT, daif);

 	/*
 	 * Unmask PMR before going idle to make sure interrupts can
--
2.29.2


^ permalink raw reply related	[flat|nested] 55+ messages in thread

* [RFC PATCH 2/7] arm64: kernel: Add a WFI hook.
  2021-01-20 13:27 [RFC PATCH 0/7] Linux on Apple Silicon Mohamed Mediouni
  2021-01-20 13:27 ` [RFC PATCH 1/7] arm64: kernel: FIQ support Mohamed Mediouni
@ 2021-01-20 13:27 ` Mohamed Mediouni
  2021-01-20 16:46   ` Alexander Graf
  2021-01-21 10:52   ` Arnd Bergmann
  2021-01-20 13:27 ` [RFC PATCH 3/7] arm64: mm: use nGnRnE instead of nGnRE on Apple processors Mohamed Mediouni
                   ` (4 subsequent siblings)
  6 siblings, 2 replies; 55+ messages in thread
From: Mohamed Mediouni @ 2021-01-20 13:27 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Catalin Marinas, Will Deacon, Mark Rutland, Marc Zyngier,
	Hector Martin, linux-kernel, Stan Skowronek, Mohamed Mediouni

From: Stan Skowronek <stan@corellium.com>

WFI drops register state on Apple Silicon for SMP systems.

This hook will be used for a hardware workaround in the
Apple CPU start driver.

Signed-off-by: Stan Skowronek <stan@corellium.com>
Signed-off-by: Mohamed Mediouni <mohamed.mediouni@caramail.com>
---
 arch/arm64/include/asm/cpu_ops.h |  2 ++
 arch/arm64/kernel/cpu_ops.c      |  6 ++++++
 arch/arm64/kernel/process.c      | 11 +++++++++--
 3 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/include/asm/cpu_ops.h b/arch/arm64/include/asm/cpu_ops.h
index e95c4df83911..4be0fc5bcaf9 100644
--- a/arch/arm64/include/asm/cpu_ops.h
+++ b/arch/arm64/include/asm/cpu_ops.h
@@ -23,6 +23,7 @@
  * @cpu_boot:	Boots a cpu into the kernel.
  * @cpu_postboot: Optionally, perform any post-boot cleanup or necessary
  *		synchronisation. Called from the cpu being booted.
+ * @cpu_wfi:    Optionally, replace calls to WFI in default idle with this.
  * @cpu_can_disable: Determines whether a CPU can be disabled based on
  *		mechanism-specific information.
  * @cpu_disable: Prepares a cpu to die. May fail for some mechanism-specific
@@ -43,6 +44,7 @@ struct cpu_operations {
 	int		(*cpu_prepare)(unsigned int);
 	int		(*cpu_boot)(unsigned int);
 	void		(*cpu_postboot)(void);
+	void		(*cpu_wfi)(void);
 #ifdef CONFIG_HOTPLUG_CPU
 	bool		(*cpu_can_disable)(unsigned int cpu);
 	int		(*cpu_disable)(unsigned int cpu);
diff --git a/arch/arm64/kernel/cpu_ops.c b/arch/arm64/kernel/cpu_ops.c
index e133011f64b5..6979fc4490b2 100644
--- a/arch/arm64/kernel/cpu_ops.c
+++ b/arch/arm64/kernel/cpu_ops.c
@@ -19,12 +19,18 @@ extern const struct cpu_operations smp_spin_table_ops;
 extern const struct cpu_operations acpi_parking_protocol_ops;
 #endif
 extern const struct cpu_operations cpu_psci_ops;
+#ifdef CONFIG_ARCH_APPLE
+extern const struct cpu_operations cpu_apple_start_ops;
+#endif

 static const struct cpu_operations *cpu_ops[NR_CPUS] __ro_after_init;

 static const struct cpu_operations *const dt_supported_cpu_ops[] __initconst = {
 	&smp_spin_table_ops,
 	&cpu_psci_ops,
+#ifdef CONFIG_ARCH_APPLE
+	&cpu_apple_start_ops,
+#endif
 	NULL,
 };

diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
index 34ec400288d0..611c639e20be 100644
--- a/arch/arm64/kernel/process.c
+++ b/arch/arm64/kernel/process.c
@@ -57,6 +57,7 @@
 #include <asm/processor.h>
 #include <asm/pointer_auth.h>
 #include <asm/stacktrace.h>
+#include <asm/cpu_ops.h>

 #if defined(CONFIG_STACKPROTECTOR) && !defined(CONFIG_STACKPROTECTOR_PER_TASK)
 #include <linux/stackprotector.h>
@@ -74,8 +75,14 @@ void (*arm_pm_restart)(enum reboot_mode reboot_mode, const char *cmd);

 static void noinstr __cpu_do_idle(void)
 {
-	dsb(sy);
-	wfi();
+	const struct cpu_operations *ops = get_cpu_ops(task_cpu(current));
+
+	if (ops->cpu_wfi) {
+		ops->cpu_wfi();
+	} else {
+		dsb(sy);
+		wfi();
+	}
 }

 static void noinstr __cpu_do_idle_irqprio(void)
--
2.29.2


^ permalink raw reply related	[flat|nested] 55+ messages in thread

* [RFC PATCH 3/7] arm64: mm: use nGnRnE instead of nGnRE on Apple processors
  2021-01-20 13:27 [RFC PATCH 0/7] Linux on Apple Silicon Mohamed Mediouni
  2021-01-20 13:27 ` [RFC PATCH 1/7] arm64: kernel: FIQ support Mohamed Mediouni
  2021-01-20 13:27 ` [RFC PATCH 2/7] arm64: kernel: Add a WFI hook Mohamed Mediouni
@ 2021-01-20 13:27 ` Mohamed Mediouni
  2021-01-20 16:47   ` Alexander Graf
  2021-01-21 11:27   ` Will Deacon
  2021-01-20 13:27 ` [RFC PATCH 4/7] irqchip/apple-aic: Add support for Apple AIC Mohamed Mediouni
                   ` (3 subsequent siblings)
  6 siblings, 2 replies; 55+ messages in thread
From: Mohamed Mediouni @ 2021-01-20 13:27 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Catalin Marinas, Will Deacon, Mark Rutland, Marc Zyngier,
	Hector Martin, linux-kernel, Mohamed Mediouni, Stan Skowronek

Use nGnRnE instead of nGnRE on Apple SoCs to workaround a serious hardware quirk.

On Apple processors, writes using the nGnRE device memory type get dropped in flight,
getting to nowhere.

Signed-off-by: Stan Skowronek <stan@corellium.com>
Signed-off-by: Mohamed Mediouni <mohamed.mediouni@caramail.com>
---
 arch/arm64/mm/proc.S | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/arch/arm64/mm/proc.S b/arch/arm64/mm/proc.S
index 1f7ee8c8b7b8..06436916f137 100644
--- a/arch/arm64/mm/proc.S
+++ b/arch/arm64/mm/proc.S
@@ -51,6 +51,25 @@
 #define TCR_KASAN_HW_FLAGS 0
 #endif

+#ifdef CONFIG_ARCH_APPLE
+
+/*
+ * Apple cores appear to black-hole writes done with nGnRE.
+ * We settled on a work-around that uses MAIR vs changing every single user of
+ * nGnRE across the arm64 code.
+ */
+
+#define MAIR_EL1_SET_APPLE						\
+	(MAIR_ATTRIDX(MAIR_ATTR_DEVICE_nGnRnE, MT_DEVICE_nGnRnE) |	\
+	 MAIR_ATTRIDX(MAIR_ATTR_DEVICE_nGnRnE, MT_DEVICE_nGnRE) |	\
+	 MAIR_ATTRIDX(MAIR_ATTR_DEVICE_GRE, MT_DEVICE_GRE) |		\
+	 MAIR_ATTRIDX(MAIR_ATTR_NORMAL_NC, MT_NORMAL_NC) |		\
+	 MAIR_ATTRIDX(MAIR_ATTR_NORMAL, MT_NORMAL) |			\
+	 MAIR_ATTRIDX(MAIR_ATTR_NORMAL_WT, MT_NORMAL_WT) |		\
+	 MAIR_ATTRIDX(MAIR_ATTR_NORMAL, MT_NORMAL_TAGGED))
+
+#endif
+
 /*
  * Default MAIR_EL1. MT_NORMAL_TAGGED is initially mapped as Normal memory and
  * changed during __cpu_setup to Normal Tagged if the system supports MTE.
@@ -432,6 +451,13 @@ SYM_FUNC_START(__cpu_setup)
 	 * Memory region attributes
 	 */
 	mov_q	x5, MAIR_EL1_SET
+#ifdef CONFIG_ARCH_APPLE
+	mrs	x0, MIDR_EL1
+	lsr	w0, w0, #24
+	mov_q	x1, MAIR_EL1_SET_APPLE
+	cmp	x0, #0x61			// 0x61 = Implementer: Apple
+	csel	x5, x1, x5, eq
+#endif
 #ifdef CONFIG_ARM64_MTE
 	mte_tcr	.req	x20

--
2.29.2


^ permalink raw reply related	[flat|nested] 55+ messages in thread

* [RFC PATCH 4/7] irqchip/apple-aic: Add support for Apple AIC
  2021-01-20 13:27 [RFC PATCH 0/7] Linux on Apple Silicon Mohamed Mediouni
                   ` (2 preceding siblings ...)
  2021-01-20 13:27 ` [RFC PATCH 3/7] arm64: mm: use nGnRnE instead of nGnRE on Apple processors Mohamed Mediouni
@ 2021-01-20 13:27 ` Mohamed Mediouni
  2021-01-20 17:11   ` Alexander Graf
                     ` (3 more replies)
  2021-01-20 13:27 ` [RFC PATCH 5/7] arm64/Kconfig: Add Apple Silicon SoC platform Mohamed Mediouni
                   ` (2 subsequent siblings)
  6 siblings, 4 replies; 55+ messages in thread
From: Mohamed Mediouni @ 2021-01-20 13:27 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Catalin Marinas, Will Deacon, Mark Rutland, Marc Zyngier,
	Hector Martin, linux-kernel, Stan Skowronek, Mohamed Mediouni

From: Stan Skowronek <stan@corellium.com>

Apple SoCs use the Apple AIC interrupt controller.
The Arm architectural timers is wired over FIQ on that hardware.

Signed-off-by: Stan Skowronek <stan@corellium.com>
Signed-off-by: Mohamed Mediouni <mohamed.mediouni@caramail.com>
---
 .../interrupt-controller/apple,aic.yaml       |  49 ++++
 MAINTAINERS                                   |   6 +
 drivers/irqchip/Kconfig                       |   6 +
 drivers/irqchip/Makefile                      |   1 +
 drivers/irqchip/irq-apple-aic.c               | 211 ++++++++++++++++++
 5 files changed, 273 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/apple,aic.yaml
 create mode 100644 drivers/irqchip/irq-apple-aic.c

diff --git a/Documentation/devicetree/bindings/interrupt-controller/apple,aic.yaml b/Documentation/devicetree/bindings/interrupt-controller/apple,aic.yaml
new file mode 100644
index 000000000000..e615eaaca869
--- /dev/null
+++ b/Documentation/devicetree/bindings/interrupt-controller/apple,aic.yaml
@@ -0,0 +1,49 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/interrupt-controller/apple,aic.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Apple Advanced Interrupt Controller (AIC)
+
+description:
+  Interrupt controller present on Apple processors. AIC
+  is used by Apple on their AArch64 SoCs since the Apple A7.
+
+maintainers:
+  - Stan Skowronek <stan@corellium.com>
+
+properties:
+  compatible:
+    items:
+      - const: apple,aic
+
+  reg:
+    maxItems: 1
+
+  '#interrupt-cells':
+    const: 3
+
+  interrupt-controller: true
+
+  fast-ipi:
+    description:
+      Fast IPI support.
+
+required:
+  - compatible
+  - '#interrupt-cells'
+  - interrupt-controller
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+        aic: interrupt-controller@23b100000 {
+             compatible = "apple,aic";
+             #interrupt-cells = <3>;
+             interrupt-controller;
+             reg = <0x2 0x3b100000 0x0 0x8000>;
+             fast-ipi;
+         };
diff --git a/MAINTAINERS b/MAINTAINERS
index 00836f6452f0..e609ede99dd4 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1218,6 +1218,12 @@ T:	git git://git.kernel.org/pub/scm/linux/kernel/git/jj/linux-apparmor
 F:	Documentation/admin-guide/LSM/apparmor.rst
 F:	security/apparmor/

+APPLE ADVANCED INTERRUPT CONTROLLER DRIVER
+M:	Stan Skowronek <stan@corellium.com>
+L:	linux-arm-kernel@lists.infradead.org
+S:	Maintained
+F:	drivers/irqchip/irq-apple-aic.c
+
 APPLE BCM5974 MULTITOUCH DRIVER
 M:	Henrik Rydberg <rydberg@bitmath.org>
 L:	linux-input@vger.kernel.org
diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index 94920a51c628..3aa9e711324b 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -56,6 +56,12 @@ config ARM_GIC_V3_ITS_FSL_MC
 	depends on FSL_MC_BUS
 	default ARM_GIC_V3_ITS

+config APPLE_AIC
+	bool
+	select IRQ_DOMAIN_HIERARCHY
+	select GENERIC_IRQ_MULTI_HANDLER
+	select GENERIC_IRQ_EFFECTIVE_AFF_MASK
+
 config ARM_NVIC
 	bool
 	select IRQ_DOMAIN_HIERARCHY
diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index 0ac93bfaec61..2f5a9a0cf40f 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -34,6 +34,7 @@ obj-$(CONFIG_ARM_GIC_V3)		+= irq-gic-v3.o irq-gic-v3-mbi.o irq-gic-common.o
 obj-$(CONFIG_ARM_GIC_V3_ITS)		+= irq-gic-v3-its.o irq-gic-v3-its-platform-msi.o irq-gic-v4.o
 obj-$(CONFIG_ARM_GIC_V3_ITS_PCI)	+= irq-gic-v3-its-pci-msi.o
 obj-$(CONFIG_ARM_GIC_V3_ITS_FSL_MC)	+= irq-gic-v3-its-fsl-mc-msi.o
+obj-$(CONFIG_APPLE_AIC)			+= irq-apple-aic.o
 obj-$(CONFIG_PARTITION_PERCPU)		+= irq-partition-percpu.o
 obj-$(CONFIG_HISILICON_IRQ_MBIGEN)	+= irq-mbigen.o
 obj-$(CONFIG_ARM_NVIC)			+= irq-nvic.o
diff --git a/drivers/irqchip/irq-apple-aic.c b/drivers/irqchip/irq-apple-aic.c
new file mode 100644
index 000000000000..c601bc4b501a
--- /dev/null
+++ b/drivers/irqchip/irq-apple-aic.c
@@ -0,0 +1,211 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Apple chip interrupt controller
+ *
+ * Copyright (C) 2020 Corellium LLC
+ * Copyright (C) 1992, 1998 Linus Torvalds, Ingo Molnar
+ *
+ */
+
+#include <linux/interrupt.h>
+#include <linux/err.h>
+#include <linux/irqdomain.h>
+#include <linux/irq.h>
+#include <linux/irqchip.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+
+#include <asm/exception.h>
+#include <asm/irq.h>
+
+#define REG_ID_REVISION 0x0000
+#define REG_ID_CONFIG 0x0004
+#define REG_GLOBAL_CFG 0x0010
+#define REG_TIME_LO 0x0020
+#define REG_TIME_HI 0x0028
+#define REG_ID_CPUID 0x2000
+#define REG_IRQ_ACK 0x2004
+#define REG_IRQ_ACK_TYPE_MASK (15 << 16)
+#define REG_IRQ_ACK_TYPE_NONE (0 << 16)
+#define REG_IRQ_ACK_TYPE_IRQ (1 << 16)
+#define REG_IRQ_ACK_TYPE_IPI (4 << 16)
+#define REG_IRQ_ACK_IPI_OTHER 0x40001
+#define REG_IRQ_ACK_IPI_SELF 0x40002
+#define REG_IRQ_ACK_NUM_MASK (4095)
+#define REG_IPI_SET 0x2008
+#define REG_IPI_FLAG_SELF (1 << 31)
+#define REG_IPI_FLAG_OTHER (1 << 0)
+#define REG_IPI_CLEAR 0x200C
+#define REG_IPI_DISABLE 0x2024
+#define REG_IPI_ENABLE 0x2028
+#define REG_IPI_DEFER_SET 0x202C
+#define REG_IPI_DEFER_CLEAR 0x2030
+#define REG_TSTAMP_CTRL 0x2040
+#define REG_TSTAMP_LO 0x2048
+#define REG_TSTAMP_HI 0x204C
+#define REG_IRQ_AFFINITY(i) (0x3000 + ((i) << 2))
+#define REG_IRQ_DISABLE(i) (0x4100 + (((i) >> 5) << 2))
+#define REG_IRQ_xABLE_MASK(i) (1 << ((i)&31))
+#define REG_IRQ_ENABLE(i) (0x4180 + (((i) >> 5) << 2))
+#define REG_CPU_REGION 0x5000
+#define REG_CPU_LOCAL 0x2000
+#define REG_CPU_SHIFT 7
+#define REG_PERCPU(r, c)                                                       \
+	((r) + REG_CPU_REGION - REG_CPU_LOCAL + ((c) << REG_CPU_SHIFT))
+
+static struct aic_chip_data {
+	void __iomem *base;
+	struct irq_domain *domain;
+	unsigned int num_irqs;
+} aic;
+
+static void apple_aic_irq_mask(struct irq_data *d)
+{
+	writel(REG_IRQ_xABLE_MASK(d->hwirq),
+	       aic.base + REG_IRQ_DISABLE(d->hwirq));
+}
+
+static void apple_aic_irq_unmask(struct irq_data *d)
+{
+	writel(REG_IRQ_xABLE_MASK(d->hwirq),
+	       aic.base + REG_IRQ_ENABLE(d->hwirq));
+}
+
+static struct irq_chip apple_aic_irq_chip = {
+	.name = "apple_aic",
+	.irq_mask = apple_aic_irq_mask,
+	.irq_mask_ack = apple_aic_irq_mask,
+	.irq_unmask = apple_aic_irq_unmask,
+};
+
+static void apple_aic_fiq_mask(struct irq_data *d)
+{
+}
+
+static void apple_aic_fiq_unmask(struct irq_data *d)
+{
+}
+
+static struct irq_chip apple_aic_irq_chip_fiq = {
+	.name = "apple_aic_fiq",
+	.irq_mask = apple_aic_fiq_mask,
+	.irq_unmask = apple_aic_fiq_unmask,
+};
+
+static int apple_aic_irq_domain_xlate(struct irq_domain *d,
+				      struct device_node *ctrlr,
+				      const u32 *intspec, unsigned int intsize,
+				      unsigned long *out_hwirq,
+				      unsigned int *out_type)
+{
+	if (intspec[0]) { /* FIQ */
+		if (intspec[1] >= 2)
+			return -EINVAL;
+		if (out_hwirq)
+			*out_hwirq = aic.num_irqs + intspec[1];
+	} else {
+		if (intspec[1] >= aic.num_irqs)
+			return -EINVAL;
+		if (out_hwirq)
+			*out_hwirq = intspec[1];
+	}
+
+	if (out_type)
+		*out_type = intspec[2] & IRQ_TYPE_SENSE_MASK;
+	return 0;
+}
+
+static int apple_aic_irq_domain_map(struct irq_domain *d, unsigned int virq,
+				    irq_hw_number_t hw)
+{
+	if (hw >= aic.num_irqs) {
+		irq_set_percpu_devid(virq);
+		irq_domain_set_info(d, virq, hw, &apple_aic_irq_chip_fiq,
+				    d->host_data, handle_percpu_devid_irq, NULL,
+				    NULL);
+		irq_set_status_flags(virq, IRQ_NOAUTOEN);
+	} else {
+		irq_domain_set_info(d, virq, hw, &apple_aic_irq_chip,
+				    d->host_data, handle_level_irq, NULL, NULL);
+		irq_set_probe(virq);
+		irqd_set_single_target(
+			irq_desc_get_irq_data(irq_to_desc(virq)));
+	}
+	return 0;
+}
+
+static const struct irq_domain_ops apple_aic_irq_domain_ops = {
+	.xlate = apple_aic_irq_domain_xlate,
+	.map = apple_aic_irq_domain_map,
+};
+
+static void __exception_irq_entry apple_aic_handle_irq(struct pt_regs *regs)
+{
+	uint32_t ack;
+	unsigned done = 0;
+
+	while (1) {
+		ack = readl(aic.base + REG_IRQ_ACK);
+		switch (ack & REG_IRQ_ACK_TYPE_MASK) {
+		case REG_IRQ_ACK_TYPE_NONE:
+			done = 1;
+			break;
+		case REG_IRQ_ACK_TYPE_IRQ:
+			handle_domain_irq(aic.domain,
+					  ack & REG_IRQ_ACK_NUM_MASK, regs);
+			break;
+		}
+		if (done)
+			break;
+	}
+}
+
+static void __exception_irq_entry apple_aic_handle_fiq(struct pt_regs *regs)
+{
+	handle_domain_irq(aic.domain, aic.num_irqs, regs);
+}
+
+void apple_aic_cpu_prepare(unsigned int cpu)
+{
+	unsigned i;
+
+	for (i = 0; i < aic.num_irqs; i++)
+		writel(readl(aic.base + REG_IRQ_AFFINITY(i)) | (1u << cpu),
+		       aic.base + REG_IRQ_AFFINITY(i));
+}
+
+static int __init apple_aic_init(struct device_node *node,
+				 struct device_node *interrupt_parent)
+{
+	unsigned i;
+
+	if (!node)
+		return -ENODEV;
+
+	aic.base = of_iomap(node, 0);
+	if (WARN(!aic.base, "unable to map aic registers\n"))
+		return -EINVAL;
+
+	aic.num_irqs = readl(aic.base + REG_ID_CONFIG) & 0xFFFF;
+	pr_info("Apple AIC: %d IRQs + 1 FIQ + 1 dummy\n", aic.num_irqs);
+
+	for (i = 0; i < aic.num_irqs; i++)
+		writel(1, aic.base + REG_IRQ_AFFINITY(i));
+	for (i = 0; i < aic.num_irqs; i += 32)
+		writel(-1u, aic.base + REG_IRQ_DISABLE(i));
+	writel((readl(aic.base + REG_GLOBAL_CFG) & ~0xF00000) | 0x700000,
+	       aic.base + REG_GLOBAL_CFG);
+
+	set_handle_irq(apple_aic_handle_irq);
+	set_handle_fiq(apple_aic_handle_fiq);
+
+	apple_aic_cpu_prepare(0);
+
+	aic.domain = irq_domain_add_linear(node, aic.num_irqs + 2,
+					   &apple_aic_irq_domain_ops,
+					   &apple_aic_irq_chip);
+	irq_set_default_host(aic.domain);
+	return 0;
+}
+
+IRQCHIP_DECLARE(apple_aic_irq_chip, "apple,aic", apple_aic_init);
--
2.29.2


^ permalink raw reply related	[flat|nested] 55+ messages in thread

* [RFC PATCH 5/7] arm64/Kconfig: Add Apple Silicon SoC platform
  2021-01-20 13:27 [RFC PATCH 0/7] Linux on Apple Silicon Mohamed Mediouni
                   ` (3 preceding siblings ...)
  2021-01-20 13:27 ` [RFC PATCH 4/7] irqchip/apple-aic: Add support for Apple AIC Mohamed Mediouni
@ 2021-01-20 13:27 ` Mohamed Mediouni
  2021-01-20 13:27 ` [RFC PATCH 6/7] arm64: kernel: Apple CPU start driver Mohamed Mediouni
  2021-01-20 13:27 ` [RFC PATCH 7/7] irqchip/apple-aic: add SMP support to the Apple AIC driver Mohamed Mediouni
  6 siblings, 0 replies; 55+ messages in thread
From: Mohamed Mediouni @ 2021-01-20 13:27 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Catalin Marinas, Will Deacon, Mark Rutland, Marc Zyngier,
	Hector Martin, linux-kernel, Stan Skowronek, Mohamed Mediouni

From: Stan Skowronek <stan@corellium.com>

Signed-off-by: Stan Skowronek <stan@corellium.com>
Signed-off-by: Mohamed Mediouni <mohamed.mediouni@caramail.com>
---
 arch/arm64/Kconfig.platforms | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms
index 6eecdef538bd..cc52519d4f67 100644
--- a/arch/arm64/Kconfig.platforms
+++ b/arch/arm64/Kconfig.platforms
@@ -328,4 +328,11 @@ config ARCH_ZYNQMP
 	help
 	  This enables support for Xilinx ZynqMP Family

+config ARCH_APPLE
+	bool "Apple Silicon SoC Family"
+	select APPLE_AIC
+	help
+	  This enables support for Apple processors present
+	  on Mac computers.
+
 endmenu
--
2.29.2


^ permalink raw reply related	[flat|nested] 55+ messages in thread

* [RFC PATCH 6/7] arm64: kernel: Apple CPU start driver
  2021-01-20 13:27 [RFC PATCH 0/7] Linux on Apple Silicon Mohamed Mediouni
                   ` (4 preceding siblings ...)
  2021-01-20 13:27 ` [RFC PATCH 5/7] arm64/Kconfig: Add Apple Silicon SoC platform Mohamed Mediouni
@ 2021-01-20 13:27 ` Mohamed Mediouni
  2021-01-21 11:14   ` Arnd Bergmann
  2021-01-20 13:27 ` [RFC PATCH 7/7] irqchip/apple-aic: add SMP support to the Apple AIC driver Mohamed Mediouni
  6 siblings, 1 reply; 55+ messages in thread
From: Mohamed Mediouni @ 2021-01-20 13:27 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Catalin Marinas, Will Deacon, Mark Rutland, Marc Zyngier,
	Hector Martin, linux-kernel, Stan Skowronek, Mohamed Mediouni

From: Stan Skowronek <stan@corellium.com>

This driver is needed to spawn CPUs for SMP
on Apple Silicon platforms.

Signed-off-by: Stan Skowronek <stan@corellium.com>
Signed-off-by: Mohamed Mediouni <mohamed.mediouni@caramail.com>
---
 .../devicetree/bindings/arm/cpus.yaml         |   1 +
 arch/arm64/kernel/Makefile                    |   1 +
 arch/arm64/kernel/apple_cpustart.c            | 153 ++++++++++++++++++
 3 files changed, 155 insertions(+)
 create mode 100644 arch/arm64/kernel/apple_cpustart.c

diff --git a/Documentation/devicetree/bindings/arm/cpus.yaml b/Documentation/devicetree/bindings/arm/cpus.yaml
index 14cd727d3c4b..a6ff8cb3db1e 100644
--- a/Documentation/devicetree/bindings/arm/cpus.yaml
+++ b/Documentation/devicetree/bindings/arm/cpus.yaml
@@ -176,6 +176,7 @@ properties:
     oneOf:
       # On ARM v8 64-bit this property is required
       - enum:
+          - apple
           - psci
           - spin-table
       # On ARM 32-bit systems this property is optional
diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
index 86364ab6f13f..497f43ca7f0f 100644
--- a/arch/arm64/kernel/Makefile
+++ b/arch/arm64/kernel/Makefile
@@ -56,6 +56,7 @@ obj-$(CONFIG_ARM64_RELOC_TEST)		+= arm64-reloc-test.o
 arm64-reloc-test-y := reloc_test_core.o reloc_test_syms.o
 obj-$(CONFIG_CRASH_DUMP)		+= crash_dump.o
 obj-$(CONFIG_CRASH_CORE)		+= crash_core.o
+obj-$(CONFIG_ARCH_APPLE)                += apple_cpustart.o
 obj-$(CONFIG_ARM_SDE_INTERFACE)		+= sdei.o
 obj-$(CONFIG_ARM64_PTR_AUTH)		+= pointer_auth.o
 obj-$(CONFIG_ARM64_MTE)			+= mte.o
diff --git a/arch/arm64/kernel/apple_cpustart.c b/arch/arm64/kernel/apple_cpustart.c
new file mode 100644
index 000000000000..41d049eaaec7
--- /dev/null
+++ b/arch/arm64/kernel/apple_cpustart.c
@@ -0,0 +1,153 @@
+/* SPDX-License-Identifier: (GPL-2.0 or BSD-3-Clause) */
+/*
+ * Copyright (C) 2020 Corellium LLC
+ */
+
+#include <linux/init.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/smp.h>
+#include <linux/delay.h>
+#include <linux/mm.h>
+
+#include <asm/cpu_ops.h>
+#include <asm/errno.h>
+#include <asm/smp_plat.h>
+#include <asm/io.h>
+
+#define MAGIC_UNLOCK 0xc5acce55
+
+struct cpu_apple_start_info {
+	void __iomem *pmgr_start;
+	u64 pmgr_start_size;
+	void __iomem *cputrc_rvbar;
+	void __iomem *dbg_unlock;
+};
+
+extern void apple_aic_cpu_prepare(unsigned int cpu);
+
+static int cpu_apple_start0_unlocked = 0;
+static DEFINE_PER_CPU(struct cpu_apple_start_info, cpu_apple_start_info);
+
+static int __init cpu_apple_start_init(unsigned int cpu)
+{
+	return 0;
+}
+
+static int cpu_apple_start_prepare(unsigned int cpu)
+{
+	struct device_node *node;
+	struct cpu_apple_start_info *info;
+
+	info = per_cpu_ptr(&cpu_apple_start_info, cpu);
+
+	if (info->pmgr_start && info->cputrc_rvbar && info->dbg_unlock)
+		return 0;
+
+	node = of_find_compatible_node(NULL, NULL, "apple,startcpu");
+	if (!node) {
+		pr_err("%s: missing startcpu node in device tree.\n", __func__);
+		return -EINVAL;
+	}
+
+	if (!info->pmgr_start) {
+		info->pmgr_start = of_iomap(node, cpu * 3);
+		if (!info->pmgr_start) {
+			pr_err("%s: failed to map start register for CPU %d.\n",
+			       __func__, cpu);
+			return -EINVAL;
+		}
+		if (!of_get_address(node, cpu * 3, &info->pmgr_start_size,
+				    NULL))
+			info->pmgr_start_size = 8;
+	}
+
+	if (!info->cputrc_rvbar) {
+		info->cputrc_rvbar = of_iomap(node, cpu * 3 + 1);
+		if (!info->cputrc_rvbar) {
+			pr_err("%s: failed to map reset address register for CPU %d.\n",
+			       __func__, cpu);
+			return -EINVAL;
+		}
+	}
+
+	if (!info->dbg_unlock) {
+		info->dbg_unlock = of_iomap(node, cpu * 3 + 2);
+		if (!info->dbg_unlock) {
+			pr_err("%s: failed to map unlock register for CPU %d.\n",
+			       __func__, cpu);
+			return -EINVAL;
+		}
+	}
+
+	if (cpu)
+		apple_aic_cpu_prepare(cpu);
+
+	return 0;
+}
+
+static int cpu_apple_start_boot(unsigned int cpu)
+{
+	struct cpu_apple_start_info *info;
+	unsigned long addr;
+
+	if (!cpu_apple_start0_unlocked) {
+		if (!cpu_apple_start_prepare(0)) {
+			info = per_cpu_ptr(&cpu_apple_start_info, 0);
+			writel(MAGIC_UNLOCK, info->dbg_unlock);
+			cpu_apple_start0_unlocked = 1;
+		} else
+			pr_err("%s: failed to unlock boot CPU\n", __func__);
+	}
+
+	info = per_cpu_ptr(&cpu_apple_start_info, cpu);
+
+	if (!info->pmgr_start || !info->cputrc_rvbar || !info->dbg_unlock)
+		return -EINVAL;
+
+	writeq(__pa_symbol(secondary_entry) | 1, info->cputrc_rvbar);
+	readq(info->cputrc_rvbar);
+	writeq(__pa_symbol(secondary_entry) | 1, info->cputrc_rvbar);
+	addr = readq(info->cputrc_rvbar) & 0xFFFFFFFFFul;
+	dsb(sy);
+
+	if (addr != (__pa_symbol(secondary_entry) | 1))
+		pr_err("%s: CPU%d reset address: 0x%lx, failed to set to 0x%lx.\n",
+		       __func__, cpu, addr,
+		       (unsigned long)(__pa_symbol(secondary_entry) | 1));
+
+	writel(MAGIC_UNLOCK, info->dbg_unlock);
+
+	writel(1 << cpu, info->pmgr_start);
+	if (info->pmgr_start_size >= 12) {
+		if (cpu < 4) {
+			writel(1 << cpu, info->pmgr_start + 4);
+			writel(0, info->pmgr_start + 8);
+		} else {
+			writel(0, info->pmgr_start + 4);
+			writel(1 << (cpu - 4), info->pmgr_start + 8);
+		}
+	} else
+		writel(1 << cpu, info->pmgr_start + 4);
+
+	dsb(sy);
+	sev();
+
+	return 0;
+}
+
+static void cpu_apple_wfi(void)
+{
+	/* can't do a proper WFI, because the CPU tends to lose state; will need
+       a proper wrapper sequence */
+	dsb(sy);
+	wfe();
+}
+
+const struct cpu_operations cpu_apple_start_ops = {
+	.name = "apple",
+	.cpu_init = cpu_apple_start_init,
+	.cpu_prepare = cpu_apple_start_prepare,
+	.cpu_boot = cpu_apple_start_boot,
+	.cpu_wfi = cpu_apple_wfi,
+};
--
2.29.2


^ permalink raw reply related	[flat|nested] 55+ messages in thread

* [RFC PATCH 7/7] irqchip/apple-aic: add SMP support to the Apple AIC driver.
  2021-01-20 13:27 [RFC PATCH 0/7] Linux on Apple Silicon Mohamed Mediouni
                   ` (5 preceding siblings ...)
  2021-01-20 13:27 ` [RFC PATCH 6/7] arm64: kernel: Apple CPU start driver Mohamed Mediouni
@ 2021-01-20 13:27 ` Mohamed Mediouni
  2021-01-21 12:44   ` Arnd Bergmann
  2021-02-02 19:15   ` Linus Walleij
  6 siblings, 2 replies; 55+ messages in thread
From: Mohamed Mediouni @ 2021-01-20 13:27 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Catalin Marinas, Will Deacon, Mark Rutland, Marc Zyngier,
	Hector Martin, linux-kernel, Stan Skowronek, Mohamed Mediouni

From: Stan Skowronek <stan@corellium.com>

This includes IPI support and a workaround for non-working WFI on
Apple processors.

Signed-off-by: Stan Skowronek <stan@corellium.com>
Signed-off-by: Mohamed Mediouni <mohamed.mediouni@caramail.com>
---
 drivers/irqchip/irq-apple-aic.c | 177 +++++++++++++++++++++++++++++---
 1 file changed, 165 insertions(+), 12 deletions(-)

diff --git a/drivers/irqchip/irq-apple-aic.c b/drivers/irqchip/irq-apple-aic.c
index c601bc4b501a..ce4e39d56fcf 100644
--- a/drivers/irqchip/irq-apple-aic.c
+++ b/drivers/irqchip/irq-apple-aic.c
@@ -17,6 +17,7 @@

 #include <asm/exception.h>
 #include <asm/irq.h>
+#include <asm/smp.h>

 #define REG_ID_REVISION 0x0000
 #define REG_ID_CONFIG 0x0004
@@ -53,12 +54,17 @@
 #define REG_PERCPU(r, c)                                                       \
 	((r) + REG_CPU_REGION - REG_CPU_LOCAL + ((c) << REG_CPU_SHIFT))

+#define NUM_IPI 8
+
 static struct aic_chip_data {
 	void __iomem *base;
 	struct irq_domain *domain;
 	unsigned int num_irqs;
+	bool fast_ipi;
 } aic;

+static DEFINE_PER_CPU(atomic_t, aic_ipi_mask);
+
 static void apple_aic_irq_mask(struct irq_data *d)
 {
 	writel(REG_IRQ_xABLE_MASK(d->hwirq),
@@ -78,18 +84,71 @@ static struct irq_chip apple_aic_irq_chip = {
 	.irq_unmask = apple_aic_irq_unmask,
 };

-static void apple_aic_fiq_mask(struct irq_data *d)
+static void apple_aic_fiq_ipi_mask(struct irq_data *d)
 {
 }

-static void apple_aic_fiq_unmask(struct irq_data *d)
+static void apple_aic_fiq_ipi_unmask(struct irq_data *d)
 {
 }

 static struct irq_chip apple_aic_irq_chip_fiq = {
 	.name = "apple_aic_fiq",
-	.irq_mask = apple_aic_fiq_mask,
-	.irq_unmask = apple_aic_fiq_unmask,
+	.irq_mask = apple_aic_fiq_ipi_mask,
+	.irq_unmask = apple_aic_fiq_ipi_unmask,
+};
+
+#define SR_APPLE_IPI_LOCAL s3_5_c15_c0_0
+#define SR_APPLE_IPI_REMOTE s3_5_c15_c0_1
+#define SR_APPLE_IPI_STAT s3_5_c15_c1_1
+
+#ifdef CONFIG_SMP
+static void apple_aic_ipi_send_mask(struct irq_data *d,
+				    const struct cpumask *mask)
+{
+	int cpu, lcpu;
+	int irqnr = d->hwirq - (aic.num_irqs + 2);
+
+	if (WARN_ON(irqnr < 0 || irqnr >= NUM_IPI))
+		return;
+
+	/*
+     * Ensure that stores to Normal memory are visible to the
+     * other CPUs before issuing the IPI.
+     */
+	wmb();
+
+	for_each_cpu (cpu, mask) {
+		smp_mb__before_atomic();
+		atomic_or(1u << irqnr, per_cpu_ptr(&aic_ipi_mask, cpu));
+		smp_mb__after_atomic();
+		lcpu = get_cpu();
+		if (aic.fast_ipi) {
+			if ((lcpu >> 2) == (cpu >> 2))
+				write_sysreg(cpu & 3, SR_APPLE_IPI_LOCAL);
+			else
+				write_sysreg((cpu & 3) | ((cpu >> 2) << 16),
+					     SR_APPLE_IPI_REMOTE);
+		} else
+			writel(lcpu == cpu ? REG_IPI_FLAG_SELF :
+						   (REG_IPI_FLAG_OTHER << cpu),
+			       aic.base + REG_IPI_SET);
+		put_cpu();
+	}
+
+	/* Force the above writes to be executed */
+	if (aic.fast_ipi)
+		isb();
+}
+#else
+#define apple_aic_ipi_send_mask NULL
+#endif
+
+static struct irq_chip apple_aic_irq_chip_ipi = {
+	.name = "apple_aic_ipi",
+	.irq_mask = apple_aic_fiq_ipi_mask,
+	.irq_unmask = apple_aic_fiq_ipi_unmask,
+	.ipi_send_mask = apple_aic_ipi_send_mask,
 };

 static int apple_aic_irq_domain_xlate(struct irq_domain *d,
@@ -98,16 +157,27 @@ static int apple_aic_irq_domain_xlate(struct irq_domain *d,
 				      unsigned long *out_hwirq,
 				      unsigned int *out_type)
 {
-	if (intspec[0]) { /* FIQ */
+	switch (intspec[0]) {
+	case 0: /* IRQ */
+		if (intspec[1] >= aic.num_irqs)
+			return -EINVAL;
+		if (out_hwirq)
+			*out_hwirq = intspec[1];
+		break;
+	case 1: /* FIQ */
 		if (intspec[1] >= 2)
 			return -EINVAL;
 		if (out_hwirq)
 			*out_hwirq = aic.num_irqs + intspec[1];
-	} else {
-		if (intspec[1] >= aic.num_irqs)
+		break;
+	case 2: /* IPI */
+		if (intspec[1] >= NUM_IPI)
 			return -EINVAL;
 		if (out_hwirq)
-			*out_hwirq = intspec[1];
+			*out_hwirq = aic.num_irqs + 2 + intspec[1];
+		break;
+	default:
+		return -EINVAL;
 	}

 	if (out_type)
@@ -118,7 +188,13 @@ static int apple_aic_irq_domain_xlate(struct irq_domain *d,
 static int apple_aic_irq_domain_map(struct irq_domain *d, unsigned int virq,
 				    irq_hw_number_t hw)
 {
-	if (hw >= aic.num_irqs) {
+	if (hw >= aic.num_irqs + 2) {
+		irq_set_percpu_devid(virq);
+		irq_domain_set_info(d, virq, hw, &apple_aic_irq_chip_ipi,
+				    d->host_data, handle_percpu_devid_irq, NULL,
+				    NULL);
+		irq_set_status_flags(virq, IRQ_NOAUTOEN);
+	} else if (hw >= aic.num_irqs) {
 		irq_set_percpu_devid(virq);
 		irq_domain_set_info(d, virq, hw, &apple_aic_irq_chip_fiq,
 				    d->host_data, handle_percpu_devid_irq, NULL,
@@ -141,8 +217,10 @@ static const struct irq_domain_ops apple_aic_irq_domain_ops = {

 static void __exception_irq_entry apple_aic_handle_irq(struct pt_regs *regs)
 {
+	atomic_t *maskptr;
 	uint32_t ack;
-	unsigned done = 0;
+	unsigned done = 0, irqnr;
+	unsigned long mask;

 	while (1) {
 		ack = readl(aic.base + REG_IRQ_ACK);
@@ -154,6 +232,36 @@ static void __exception_irq_entry apple_aic_handle_irq(struct pt_regs *regs)
 			handle_domain_irq(aic.domain,
 					  ack & REG_IRQ_ACK_NUM_MASK, regs);
 			break;
+		case REG_IRQ_ACK_TYPE_IPI:
+#ifdef CONFIG_SMP
+			if (ack == REG_IRQ_ACK_IPI_SELF)
+				writel(REG_IPI_FLAG_SELF,
+				       aic.base + REG_IPI_CLEAR);
+			else
+				writel(REG_IPI_FLAG_OTHER,
+				       aic.base + REG_IPI_CLEAR);
+			maskptr = get_cpu_ptr(&aic_ipi_mask);
+			smp_mb__before_atomic();
+			mask = atomic_xchg(maskptr, 0);
+			smp_mb__after_atomic();
+			put_cpu_ptr(&aic_ipi_mask);
+			for_each_set_bit (irqnr, &mask, NUM_IPI) {
+				handle_domain_irq(aic.domain,
+						  aic.num_irqs + 2 + irqnr,
+						  regs);
+			}
+			if (ack == REG_IRQ_ACK_IPI_SELF)
+				writel(REG_IPI_FLAG_SELF,
+				       aic.base +
+					       REG_PERCPU(REG_IPI_ENABLE,
+							  __smp_processor_id()));
+			else
+				writel(REG_IPI_FLAG_OTHER,
+				       aic.base +
+					       REG_PERCPU(REG_IPI_ENABLE,
+							  __smp_processor_id()));
+#endif
+			break;
 		}
 		if (done)
 			break;
@@ -162,6 +270,27 @@ static void __exception_irq_entry apple_aic_handle_irq(struct pt_regs *regs)

 static void __exception_irq_entry apple_aic_handle_fiq(struct pt_regs *regs)
 {
+#ifdef CONFIG_SMP
+	atomic_t *maskptr;
+	unsigned long mask;
+	unsigned irqnr;
+
+	if (aic.fast_ipi) {
+		if (read_sysreg(SR_APPLE_IPI_STAT)) {
+			write_sysreg(1, SR_APPLE_IPI_STAT);
+
+			maskptr = get_cpu_ptr(&aic_ipi_mask);
+			smp_mb__before_atomic();
+			mask = atomic_xchg(maskptr, 0);
+			smp_mb__after_atomic();
+			put_cpu_ptr(&aic_ipi_mask);
+			for_each_set_bit (irqnr, &mask, NUM_IPI)
+				handle_domain_irq(aic.domain,
+						  aic.num_irqs + 2 + irqnr,
+						  regs);
+		}
+	}
+#endif
 	handle_domain_irq(aic.domain, aic.num_irqs, regs);
 }

@@ -169,6 +298,13 @@ void apple_aic_cpu_prepare(unsigned int cpu)
 {
 	unsigned i;

+	if (aic.fast_ipi)
+		writel(REG_IPI_FLAG_SELF | REG_IPI_FLAG_OTHER,
+		       aic.base + REG_PERCPU(REG_IPI_DISABLE, cpu));
+	else
+		writel(REG_IPI_FLAG_SELF | REG_IPI_FLAG_OTHER,
+		       aic.base + REG_PERCPU(REG_IPI_ENABLE, cpu));
+
 	for (i = 0; i < aic.num_irqs; i++)
 		writel(readl(aic.base + REG_IRQ_AFFINITY(i)) | (1u << cpu),
 		       aic.base + REG_IRQ_AFFINITY(i));
@@ -178,6 +314,9 @@ static int __init apple_aic_init(struct device_node *node,
 				 struct device_node *interrupt_parent)
 {
 	unsigned i;
+#ifdef CONFIG_SMP
+	int base_ipi, ret;
+#endif

 	if (!node)
 		return -ENODEV;
@@ -186,8 +325,11 @@ static int __init apple_aic_init(struct device_node *node,
 	if (WARN(!aic.base, "unable to map aic registers\n"))
 		return -EINVAL;

+	aic.fast_ipi = of_property_read_bool(node, "fast-ipi");
+
 	aic.num_irqs = readl(aic.base + REG_ID_CONFIG) & 0xFFFF;
-	pr_info("Apple AIC: %d IRQs + 1 FIQ + 1 dummy\n", aic.num_irqs);
+	pr_info("Apple AIC: %d IRQs + 1 FIQ + 1 dummy + %d IPIs%s\n",
+		aic.num_irqs, NUM_IPI, aic.fast_ipi ? " (fast)" : "");

 	for (i = 0; i < aic.num_irqs; i++)
 		writel(1, aic.base + REG_IRQ_AFFINITY(i));
@@ -201,10 +343,21 @@ static int __init apple_aic_init(struct device_node *node,

 	apple_aic_cpu_prepare(0);

-	aic.domain = irq_domain_add_linear(node, aic.num_irqs + 2,
+	aic.domain = irq_domain_add_linear(node, aic.num_irqs + 2 + NUM_IPI,
 					   &apple_aic_irq_domain_ops,
 					   &apple_aic_irq_chip);
 	irq_set_default_host(aic.domain);
+
+#ifdef CONFIG_SMP
+	base_ipi = aic.num_irqs + 2;
+	ret = irq_create_strict_mappings(aic.domain, base_ipi, aic.num_irqs + 2,
+					 NUM_IPI);
+	if (ret < 0)
+		pr_err("%s: irq_create_strict_mappings failed with %d\n",
+		       __func__, ret);
+	set_smp_ipi_range(base_ipi, NUM_IPI);
+#endif
+
 	return 0;
 }

--
2.29.2


^ permalink raw reply related	[flat|nested] 55+ messages in thread

* Re: [RFC PATCH 2/7] arm64: kernel: Add a WFI hook.
  2021-01-20 13:27 ` [RFC PATCH 2/7] arm64: kernel: Add a WFI hook Mohamed Mediouni
@ 2021-01-20 16:46   ` Alexander Graf
       [not found]     ` <94C20F55-D3B8-4349-B26F-9EA8AAEBF639@caramail.com>
  2021-01-21 10:52   ` Arnd Bergmann
  1 sibling, 1 reply; 55+ messages in thread
From: Alexander Graf @ 2021-01-20 16:46 UTC (permalink / raw)
  To: Mohamed Mediouni, linux-arm-kernel
  Cc: Catalin Marinas, Will Deacon, Mark Rutland, Marc Zyngier,
	Hector Martin, linux-kernel, Stan Skowronek

On 20.01.21 14:27, Mohamed Mediouni wrote:
> From: Stan Skowronek <stan@corellium.com>
> 
> WFI drops register state on Apple Silicon for SMP systems.

It probably drops the register because it loses power on WFI, right?

Have you tried to set the ARM64_REG_CYC_OVRD_ok2pwrdn_force_up bit in 
ARM64_REG_CYC_OVRD yet? XNU has a call for that[1]. Maybe that's enough 
to convince the core to preserve its register state for now.

For real power savings, we will probably want much more sophisticated 
deep sleep capabilities later that would reach beyond just register 
saving on WFI (different wakeup mechanisms, IRQ balancing, etc).


Alex

[1] 
https://github.com/opensource-apple/xnu/blob/master/osfmk/arm64/machine_routines_asm.s#L797

> 
> This hook will be used for a hardware workaround in the
> Apple CPU start driver.
> 
> Signed-off-by: Stan Skowronek <stan@corellium.com>
> Signed-off-by: Mohamed Mediouni <mohamed.mediouni@caramail.com>
> ---
>   arch/arm64/include/asm/cpu_ops.h |  2 ++
>   arch/arm64/kernel/cpu_ops.c      |  6 ++++++
>   arch/arm64/kernel/process.c      | 11 +++++++++--
>   3 files changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/cpu_ops.h b/arch/arm64/include/asm/cpu_ops.h
> index e95c4df83911..4be0fc5bcaf9 100644
> --- a/arch/arm64/include/asm/cpu_ops.h
> +++ b/arch/arm64/include/asm/cpu_ops.h
> @@ -23,6 +23,7 @@
>    * @cpu_boot:	Boots a cpu into the kernel.
>    * @cpu_postboot: Optionally, perform any post-boot cleanup or necessary
>    *		synchronisation. Called from the cpu being booted.
> + * @cpu_wfi:    Optionally, replace calls to WFI in default idle with this.
>    * @cpu_can_disable: Determines whether a CPU can be disabled based on
>    *		mechanism-specific information.
>    * @cpu_disable: Prepares a cpu to die. May fail for some mechanism-specific
> @@ -43,6 +44,7 @@ struct cpu_operations {
>   	int		(*cpu_prepare)(unsigned int);
>   	int		(*cpu_boot)(unsigned int);
>   	void		(*cpu_postboot)(void);
> +	void		(*cpu_wfi)(void);
>   #ifdef CONFIG_HOTPLUG_CPU
>   	bool		(*cpu_can_disable)(unsigned int cpu);
>   	int		(*cpu_disable)(unsigned int cpu);
> diff --git a/arch/arm64/kernel/cpu_ops.c b/arch/arm64/kernel/cpu_ops.c
> index e133011f64b5..6979fc4490b2 100644
> --- a/arch/arm64/kernel/cpu_ops.c
> +++ b/arch/arm64/kernel/cpu_ops.c
> @@ -19,12 +19,18 @@ extern const struct cpu_operations smp_spin_table_ops;
>   extern const struct cpu_operations acpi_parking_protocol_ops;
>   #endif
>   extern const struct cpu_operations cpu_psci_ops;
> +#ifdef CONFIG_ARCH_APPLE
> +extern const struct cpu_operations cpu_apple_start_ops;
> +#endif
> 
>   static const struct cpu_operations *cpu_ops[NR_CPUS] __ro_after_init;
> 
>   static const struct cpu_operations *const dt_supported_cpu_ops[] __initconst = {
>   	&smp_spin_table_ops,
>   	&cpu_psci_ops,
> +#ifdef CONFIG_ARCH_APPLE
> +	&cpu_apple_start_ops,
> +#endif
>   	NULL,
>   };
> 
> diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
> index 34ec400288d0..611c639e20be 100644
> --- a/arch/arm64/kernel/process.c
> +++ b/arch/arm64/kernel/process.c
> @@ -57,6 +57,7 @@
>   #include <asm/processor.h>
>   #include <asm/pointer_auth.h>
>   #include <asm/stacktrace.h>
> +#include <asm/cpu_ops.h>
> 
>   #if defined(CONFIG_STACKPROTECTOR) && !defined(CONFIG_STACKPROTECTOR_PER_TASK)
>   #include <linux/stackprotector.h>
> @@ -74,8 +75,14 @@ void (*arm_pm_restart)(enum reboot_mode reboot_mode, const char *cmd);
> 
>   static void noinstr __cpu_do_idle(void)
>   {
> -	dsb(sy);
> -	wfi();
> +	const struct cpu_operations *ops = get_cpu_ops(task_cpu(current));
> +
> +	if (ops->cpu_wfi) {
> +		ops->cpu_wfi();
> +	} else {
> +		dsb(sy);
> +		wfi();
> +	}
>   }
> 
>   static void noinstr __cpu_do_idle_irqprio(void)
> --
> 2.29.2
> 




Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879




^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: [RFC PATCH 3/7] arm64: mm: use nGnRnE instead of nGnRE on Apple processors
  2021-01-20 13:27 ` [RFC PATCH 3/7] arm64: mm: use nGnRnE instead of nGnRE on Apple processors Mohamed Mediouni
@ 2021-01-20 16:47   ` Alexander Graf
  2021-01-20 18:06     ` Mohamed Mediouni
  2021-01-21 11:27   ` Will Deacon
  1 sibling, 1 reply; 55+ messages in thread
From: Alexander Graf @ 2021-01-20 16:47 UTC (permalink / raw)
  To: Mohamed Mediouni, linux-arm-kernel
  Cc: Catalin Marinas, Will Deacon, Mark Rutland, Marc Zyngier,
	Hector Martin, linux-kernel, Stan Skowronek

On 20.01.21 14:27, Mohamed Mediouni wrote:
> Use nGnRnE instead of nGnRE on Apple SoCs to workaround a serious hardware quirk.
> 
> On Apple processors, writes using the nGnRE device memory type get dropped in flight,
> getting to nowhere.
> 
> Signed-off-by: Stan Skowronek <stan@corellium.com>
> Signed-off-by: Mohamed Mediouni <mohamed.mediouni@caramail.com>
> ---
>   arch/arm64/mm/proc.S | 26 ++++++++++++++++++++++++++
>   1 file changed, 26 insertions(+)
> 
> diff --git a/arch/arm64/mm/proc.S b/arch/arm64/mm/proc.S
> index 1f7ee8c8b7b8..06436916f137 100644
> --- a/arch/arm64/mm/proc.S
> +++ b/arch/arm64/mm/proc.S
> @@ -51,6 +51,25 @@
>   #define TCR_KASAN_HW_FLAGS 0
>   #endif
> 
> +#ifdef CONFIG_ARCH_APPLE

Is there any particular reason for this #ifdef?


Alex

> +
> +/*
> + * Apple cores appear to black-hole writes done with nGnRE.
> + * We settled on a work-around that uses MAIR vs changing every single user of
> + * nGnRE across the arm64 code.
> + */
> +
> +#define MAIR_EL1_SET_APPLE						\
> +	(MAIR_ATTRIDX(MAIR_ATTR_DEVICE_nGnRnE, MT_DEVICE_nGnRnE) |	\
> +	 MAIR_ATTRIDX(MAIR_ATTR_DEVICE_nGnRnE, MT_DEVICE_nGnRE) |	\
> +	 MAIR_ATTRIDX(MAIR_ATTR_DEVICE_GRE, MT_DEVICE_GRE) |		\
> +	 MAIR_ATTRIDX(MAIR_ATTR_NORMAL_NC, MT_NORMAL_NC) |		\
> +	 MAIR_ATTRIDX(MAIR_ATTR_NORMAL, MT_NORMAL) |			\
> +	 MAIR_ATTRIDX(MAIR_ATTR_NORMAL_WT, MT_NORMAL_WT) |		\
> +	 MAIR_ATTRIDX(MAIR_ATTR_NORMAL, MT_NORMAL_TAGGED))
> +
> +#endif
> +
>   /*
>    * Default MAIR_EL1. MT_NORMAL_TAGGED is initially mapped as Normal memory and
>    * changed during __cpu_setup to Normal Tagged if the system supports MTE.
> @@ -432,6 +451,13 @@ SYM_FUNC_START(__cpu_setup)
>   	 * Memory region attributes
>   	 */
>   	mov_q	x5, MAIR_EL1_SET
> +#ifdef CONFIG_ARCH_APPLE
> +	mrs	x0, MIDR_EL1
> +	lsr	w0, w0, #24
> +	mov_q	x1, MAIR_EL1_SET_APPLE
> +	cmp	x0, #0x61			// 0x61 = Implementer: Apple
> +	csel	x5, x1, x5, eq
> +#endif
>   #ifdef CONFIG_ARM64_MTE
>   	mte_tcr	.req	x20
> 
> --
> 2.29.2
> 




Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879




^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: [RFC PATCH 4/7] irqchip/apple-aic: Add support for Apple AIC
  2021-01-20 13:27 ` [RFC PATCH 4/7] irqchip/apple-aic: Add support for Apple AIC Mohamed Mediouni
@ 2021-01-20 17:11   ` Alexander Graf
  2021-01-20 18:04     ` Mohamed Mediouni
  2021-01-20 21:18   ` Stan Skowronek
                     ` (2 subsequent siblings)
  3 siblings, 1 reply; 55+ messages in thread
From: Alexander Graf @ 2021-01-20 17:11 UTC (permalink / raw)
  To: Mohamed Mediouni, linux-arm-kernel
  Cc: Mark Rutland, Catalin Marinas, Hector Martin, linux-kernel,
	Marc Zyngier, Will Deacon, Stan Skowronek

On 20.01.21 14:27, Mohamed Mediouni wrote:
> From: Stan Skowronek <stan@corellium.com>
> 
> Apple SoCs use the Apple AIC interrupt controller.
> The Arm architectural timers is wired over FIQ on that hardware.
> 
> Signed-off-by: Stan Skowronek <stan@corellium.com>
> Signed-off-by: Mohamed Mediouni <mohamed.mediouni@caramail.com>
> ---
>   .../interrupt-controller/apple,aic.yaml       |  49 ++++
>   MAINTAINERS                                   |   6 +
>   drivers/irqchip/Kconfig                       |   6 +
>   drivers/irqchip/Makefile                      |   1 +
>   drivers/irqchip/irq-apple-aic.c               | 211 ++++++++++++++++++
>   5 files changed, 273 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/interrupt-controller/apple,aic.yaml
>   create mode 100644 drivers/irqchip/irq-apple-aic.c
> 
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/apple,aic.yaml b/Documentation/devicetree/bindings/interrupt-controller/apple,aic.yaml
> new file mode 100644
> index 000000000000..e615eaaca869
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/interrupt-controller/apple,aic.yaml
> @@ -0,0 +1,49 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/interrupt-controller/apple,aic.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Apple Advanced Interrupt Controller (AIC)
> +
> +description:
> +  Interrupt controller present on Apple processors. AIC
> +  is used by Apple on their AArch64 SoCs since the Apple A7.
> +
> +maintainers:
> +  - Stan Skowronek <stan@corellium.com>
> +
> +properties:
> +  compatible:
> +    items:
> +      - const: apple,aic
> +
> +  reg:
> +    maxItems: 1
> +
> +  '#interrupt-cells':
> +    const: 3
> +
> +  interrupt-controller: true
> +
> +  fast-ipi:
> +    description:
> +      Fast IPI support.
> +
> +required:
> +  - compatible
> +  - '#interrupt-cells'
> +  - interrupt-controller
> +  - reg
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +        aic: interrupt-controller@23b100000 {
> +             compatible = "apple,aic";
> +             #interrupt-cells = <3>;
> +             interrupt-controller;
> +             reg = <0x2 0x3b100000 0x0 0x8000>;
> +             fast-ipi;
> +         };
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 00836f6452f0..e609ede99dd4 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1218,6 +1218,12 @@ T:	git git://git.kernel.org/pub/scm/linux/kernel/git/jj/linux-apparmor
>   F:	Documentation/admin-guide/LSM/apparmor.rst
>   F:	security/apparmor/
> 
> +APPLE ADVANCED INTERRUPT CONTROLLER DRIVER
> +M:	Stan Skowronek <stan@corellium.com>

Signing someone else up for maintainership is ... unusual :). Do you 
have buy in from Stan that he'll be responsive and handle patch reviews?

> +L:	linux-arm-kernel@lists.infradead.org
> +S:	Maintained
> +F:	drivers/irqchip/irq-apple-aic.c
> +


Alex



Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879




^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: [RFC PATCH 4/7] irqchip/apple-aic: Add support for Apple AIC
  2021-01-20 17:11   ` Alexander Graf
@ 2021-01-20 18:04     ` Mohamed Mediouni
  2021-01-20 20:16       ` Andrew Lunn
  0 siblings, 1 reply; 55+ messages in thread
From: Mohamed Mediouni @ 2021-01-20 18:04 UTC (permalink / raw)
  To: Alexander Graf
  Cc: linux-arm-kernel, Mark Rutland, Catalin Marinas, Hector Martin,
	linux-kernel, Marc Zyngier, Will Deacon, Stan Skowronek



> On 20 Jan 2021, at 18:11, Alexander Graf <graf@amazon.com> wrote:
> 
> On 20.01.21 14:27, Mohamed Mediouni wrote:
>> From: Stan Skowronek <stan@corellium.com>
>> Apple SoCs use the Apple AIC interrupt controller.
>> The Arm architectural timers is wired over FIQ on that hardware.
>> Signed-off-by: Stan Skowronek <stan@corellium.com>
>> Signed-off-by: Mohamed Mediouni <mohamed.mediouni@caramail.com>
>> ---
>>  .../interrupt-controller/apple,aic.yaml       |  49 ++++
>>  MAINTAINERS                                   |   6 +
>>  drivers/irqchip/Kconfig                       |   6 +
>>  drivers/irqchip/Makefile                      |   1 +
>>  drivers/irqchip/irq-apple-aic.c               | 211 ++++++++++++++++++
>>  5 files changed, 273 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/interrupt-controller/apple,aic.yaml
>>  create mode 100644 drivers/irqchip/irq-apple-aic.c
>> diff --git a/Documentation/devicetree/bindings/interrupt-controller/apple,aic.yaml b/Documentation/devicetree/bindings/interrupt-controller/apple,aic.yaml
>> new file mode 100644
>> index 000000000000..e615eaaca869
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/interrupt-controller/apple,aic.yaml
>> @@ -0,0 +1,49 @@
>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/interrupt-controller/apple,aic.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Apple Advanced Interrupt Controller (AIC)
>> +
>> +description:
>> +  Interrupt controller present on Apple processors. AIC
>> +  is used by Apple on their AArch64 SoCs since the Apple A7.
>> +
>> +maintainers:
>> +  - Stan Skowronek <stan@corellium.com>
>> +
>> +properties:
>> +  compatible:
>> +    items:
>> +      - const: apple,aic
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  '#interrupt-cells':
>> +    const: 3
>> +
>> +  interrupt-controller: true
>> +
>> +  fast-ipi:
>> +    description:
>> +      Fast IPI support.
>> +
>> +required:
>> +  - compatible
>> +  - '#interrupt-cells'
>> +  - interrupt-controller
>> +  - reg
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +        aic: interrupt-controller@23b100000 {
>> +             compatible = "apple,aic";
>> +             #interrupt-cells = <3>;
>> +             interrupt-controller;
>> +             reg = <0x2 0x3b100000 0x0 0x8000>;
>> +             fast-ipi;
>> +         };
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 00836f6452f0..e609ede99dd4 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -1218,6 +1218,12 @@ T:	git git://git.kernel.org/pub/scm/linux/kernel/git/jj/linux-apparmor
>>  F:	Documentation/admin-guide/LSM/apparmor.rst
>>  F:	security/apparmor/
>> +APPLE ADVANCED INTERRUPT CONTROLLER DRIVER
>> +M:	Stan Skowronek <stan@corellium.com>
> 
> Signing someone else up for maintainership is ... unusual :). Do you have buy in from Stan that he'll be responsive and handle patch reviews?

Yeah, I asked Corellium about it explicitly. :)

>> +L:	linux-arm-kernel@lists.infradead.org
>> +S:	Maintained
>> +F:	drivers/irqchip/irq-apple-aic.c
>> +
> 
> 
> Alex
> 
> 
> 
> Amazon Development Center Germany GmbH
> Krausenstr. 38
> 10117 Berlin
> Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
> Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
> Sitz: Berlin
> Ust-ID: DE 289 237 879


^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: [RFC PATCH 3/7] arm64: mm: use nGnRnE instead of nGnRE on Apple processors
  2021-01-20 16:47   ` Alexander Graf
@ 2021-01-20 18:06     ` Mohamed Mediouni
  2021-01-20 18:10       ` Alexander Graf
  0 siblings, 1 reply; 55+ messages in thread
From: Mohamed Mediouni @ 2021-01-20 18:06 UTC (permalink / raw)
  To: Alexander Graf
  Cc: linux-arm-kernel, Catalin Marinas, Will Deacon, Mark Rutland,
	Marc Zyngier, Hector Martin, linux-kernel, Stan Skowronek



> On 20 Jan 2021, at 17:47, Alexander Graf <graf@amazon.com> wrote:
> 
> On 20.01.21 14:27, Mohamed Mediouni wrote:
>> Use nGnRnE instead of nGnRE on Apple SoCs to workaround a serious hardware quirk.
>> On Apple processors, writes using the nGnRE device memory type get dropped in flight,
>> getting to nowhere.
>> Signed-off-by: Stan Skowronek <stan@corellium.com>
>> Signed-off-by: Mohamed Mediouni <mohamed.mediouni@caramail.com>
>> ---
>>  arch/arm64/mm/proc.S | 26 ++++++++++++++++++++++++++
>>  1 file changed, 26 insertions(+)
>> diff --git a/arch/arm64/mm/proc.S b/arch/arm64/mm/proc.S
>> index 1f7ee8c8b7b8..06436916f137 100644
>> --- a/arch/arm64/mm/proc.S
>> +++ b/arch/arm64/mm/proc.S
>> @@ -51,6 +51,25 @@
>>  #define TCR_KASAN_HW_FLAGS 0
>>  #endif
>> +#ifdef CONFIG_ARCH_APPLE
> 
> Is there any particular reason for this #ifdef?
> 
> 
> Alex
> 
Not a particular reason, as we explicitly check for the implementer ID. However,
without CONFIG_ARCH_APPLE, other parts of the support for Apple CPUs
will not be available anyway.
>> +
>> +/*
>> + * Apple cores appear to black-hole writes done with nGnRE.
>> + * We settled on a work-around that uses MAIR vs changing every single user of
>> + * nGnRE across the arm64 code.
>> + */
>> +
>> +#define MAIR_EL1_SET_APPLE						\
>> +	(MAIR_ATTRIDX(MAIR_ATTR_DEVICE_nGnRnE, MT_DEVICE_nGnRnE) |	\
>> +	 MAIR_ATTRIDX(MAIR_ATTR_DEVICE_nGnRnE, MT_DEVICE_nGnRE) |	\
>> +	 MAIR_ATTRIDX(MAIR_ATTR_DEVICE_GRE, MT_DEVICE_GRE) |		\
>> +	 MAIR_ATTRIDX(MAIR_ATTR_NORMAL_NC, MT_NORMAL_NC) |		\
>> +	 MAIR_ATTRIDX(MAIR_ATTR_NORMAL, MT_NORMAL) |			\
>> +	 MAIR_ATTRIDX(MAIR_ATTR_NORMAL_WT, MT_NORMAL_WT) |		\
>> +	 MAIR_ATTRIDX(MAIR_ATTR_NORMAL, MT_NORMAL_TAGGED))
>> +
>> +#endif
>> +
>>  /*
>>   * Default MAIR_EL1. MT_NORMAL_TAGGED is initially mapped as Normal memory and
>>   * changed during __cpu_setup to Normal Tagged if the system supports MTE.
>> @@ -432,6 +451,13 @@ SYM_FUNC_START(__cpu_setup)
>>  	 * Memory region attributes
>>  	 */
>>  	mov_q	x5, MAIR_EL1_SET
>> +#ifdef CONFIG_ARCH_APPLE
>> +	mrs	x0, MIDR_EL1
>> +	lsr	w0, w0, #24
>> +	mov_q	x1, MAIR_EL1_SET_APPLE
>> +	cmp	x0, #0x61			// 0x61 = Implementer: Apple
>> +	csel	x5, x1, x5, eq
>> +#endif
>>  #ifdef CONFIG_ARM64_MTE
>>  	mte_tcr	.req	x20
>> --
>> 2.29.2
> 
> 
> 
> 
> Amazon Development Center Germany GmbH
> Krausenstr. 38
> 10117 Berlin
> Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
> Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
> Sitz: Berlin
> Ust-ID: DE 289 237 879


^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: [RFC PATCH 3/7] arm64: mm: use nGnRnE instead of nGnRE on Apple processors
  2021-01-20 18:06     ` Mohamed Mediouni
@ 2021-01-20 18:10       ` Alexander Graf
  0 siblings, 0 replies; 55+ messages in thread
From: Alexander Graf @ 2021-01-20 18:10 UTC (permalink / raw)
  To: Mohamed Mediouni
  Cc: linux-arm-kernel, Catalin Marinas, Will Deacon, Mark Rutland,
	Marc Zyngier, Hector Martin, linux-kernel, Stan Skowronek



On 20.01.21 19:06, Mohamed Mediouni wrote:
> 
>> On 20 Jan 2021, at 17:47, Alexander Graf <graf@amazon.com> wrote:
>>
>> On 20.01.21 14:27, Mohamed Mediouni wrote:
>>> Use nGnRnE instead of nGnRE on Apple SoCs to workaround a serious hardware quirk.
>>> On Apple processors, writes using the nGnRE device memory type get dropped in flight,
>>> getting to nowhere.
>>> Signed-off-by: Stan Skowronek <stan@corellium.com>
>>> Signed-off-by: Mohamed Mediouni <mohamed.mediouni@caramail.com>
>>> ---
>>>   arch/arm64/mm/proc.S | 26 ++++++++++++++++++++++++++
>>>   1 file changed, 26 insertions(+)
>>> diff --git a/arch/arm64/mm/proc.S b/arch/arm64/mm/proc.S
>>> index 1f7ee8c8b7b8..06436916f137 100644
>>> --- a/arch/arm64/mm/proc.S
>>> +++ b/arch/arm64/mm/proc.S
>>> @@ -51,6 +51,25 @@
>>>   #define TCR_KASAN_HW_FLAGS 0
>>>   #endif
>>> +#ifdef CONFIG_ARCH_APPLE
>>
>> Is there any particular reason for this #ifdef?
>>
>>
>> Alex
>>
> Not a particular reason, as we explicitly check for the implementer ID. However,
> without CONFIG_ARCH_APPLE, other parts of the support for Apple CPUs
> will not be available anyway.

The ifdef below for code looks ok to me, I'm explicitly wondering why 
you're guarding the #define :)

Alex

>>> +
>>> +/*
>>> + * Apple cores appear to black-hole writes done with nGnRE.
>>> + * We settled on a work-around that uses MAIR vs changing every single user of
>>> + * nGnRE across the arm64 code.
>>> + */
>>> +
>>> +#define MAIR_EL1_SET_APPLE                                          \
>>> +    (MAIR_ATTRIDX(MAIR_ATTR_DEVICE_nGnRnE, MT_DEVICE_nGnRnE) |      \
>>> +     MAIR_ATTRIDX(MAIR_ATTR_DEVICE_nGnRnE, MT_DEVICE_nGnRE) |       \
>>> +     MAIR_ATTRIDX(MAIR_ATTR_DEVICE_GRE, MT_DEVICE_GRE) |            \
>>> +     MAIR_ATTRIDX(MAIR_ATTR_NORMAL_NC, MT_NORMAL_NC) |              \
>>> +     MAIR_ATTRIDX(MAIR_ATTR_NORMAL, MT_NORMAL) |                    \
>>> +     MAIR_ATTRIDX(MAIR_ATTR_NORMAL_WT, MT_NORMAL_WT) |              \
>>> +     MAIR_ATTRIDX(MAIR_ATTR_NORMAL, MT_NORMAL_TAGGED))
>>> +
>>> +#endif
>>> +
>>>   /*
>>>    * Default MAIR_EL1. MT_NORMAL_TAGGED is initially mapped as Normal memory and
>>>    * changed during __cpu_setup to Normal Tagged if the system supports MTE.
>>> @@ -432,6 +451,13 @@ SYM_FUNC_START(__cpu_setup)
>>>        * Memory region attributes
>>>        */
>>>       mov_q   x5, MAIR_EL1_SET
>>> +#ifdef CONFIG_ARCH_APPLE
>>> +    mrs     x0, MIDR_EL1
>>> +    lsr     w0, w0, #24
>>> +    mov_q   x1, MAIR_EL1_SET_APPLE
>>> +    cmp     x0, #0x61                       // 0x61 = Implementer: Apple
>>> +    csel    x5, x1, x5, eq
>>> +#endif
>>>   #ifdef CONFIG_ARM64_MTE
>>>       mte_tcr .req    x20
>>> --
>>> 2.29.2




Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879




^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: [RFC PATCH 4/7] irqchip/apple-aic: Add support for Apple AIC
  2021-01-20 18:04     ` Mohamed Mediouni
@ 2021-01-20 20:16       ` Andrew Lunn
  0 siblings, 0 replies; 55+ messages in thread
From: Andrew Lunn @ 2021-01-20 20:16 UTC (permalink / raw)
  To: Mohamed Mediouni
  Cc: Alexander Graf, Mark Rutland, Catalin Marinas, Hector Martin,
	linux-kernel, Marc Zyngier, Will Deacon, linux-arm-kernel,
	Stan Skowronek

> >> @@ -1218,6 +1218,12 @@ T:	git git://git.kernel.org/pub/scm/linux/kernel/git/jj/linux-apparmor
> >>  F:	Documentation/admin-guide/LSM/apparmor.rst
> >>  F:	security/apparmor/
> >> +APPLE ADVANCED INTERRUPT CONTROLLER DRIVER
> >> +M:	Stan Skowronek <stan@corellium.com>
> > 
> > Signing someone else up for maintainership is ... unusual :). Do you have buy in from Stan that he'll be responsive and handle patch reviews?
> 
> Yeah, I asked Corellium about it explicitly. :)

Having an Ack-by: from Stan Skowronek would remove all doubt.

       Andrew

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: [RFC PATCH 4/7] irqchip/apple-aic: Add support for Apple AIC
  2021-01-20 13:27 ` [RFC PATCH 4/7] irqchip/apple-aic: Add support for Apple AIC Mohamed Mediouni
  2021-01-20 17:11   ` Alexander Graf
@ 2021-01-20 21:18   ` Stan Skowronek
  2021-01-21  9:48   ` Linus Walleij
  2021-01-21 16:53   ` Hector Martin 'marcan'
  3 siblings, 0 replies; 55+ messages in thread
From: Stan Skowronek @ 2021-01-20 21:18 UTC (permalink / raw)
  To: Mohamed Mediouni, linux-arm-kernel
  Cc: Catalin Marinas, Will Deacon, Mark Rutland, Marc Zyngier,
	Hector Martin, linux-kernel

Acked-by: Stan Skowronek <stan@corellium.com>

On 1/20/21 8:27 AM, Mohamed Mediouni wrote:
> From: Stan Skowronek <stan@corellium.com>
>
> Apple SoCs use the Apple AIC interrupt controller.
> The Arm architectural timers is wired over FIQ on that hardware.
>
> Signed-off-by: Stan Skowronek <stan@corellium.com>
> Signed-off-by: Mohamed Mediouni <mohamed.mediouni@caramail.com>
> ---
>   .../interrupt-controller/apple,aic.yaml       |  49 ++++
>   MAINTAINERS                                   |   6 +
>   drivers/irqchip/Kconfig                       |   6 +
>   drivers/irqchip/Makefile                      |   1 +
>   drivers/irqchip/irq-apple-aic.c               | 211 ++++++++++++++++++
>   5 files changed, 273 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/interrupt-controller/apple,aic.yaml
>   create mode 100644 drivers/irqchip/irq-apple-aic.c

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: [RFC PATCH 4/7] irqchip/apple-aic: Add support for Apple AIC
  2021-01-20 13:27 ` [RFC PATCH 4/7] irqchip/apple-aic: Add support for Apple AIC Mohamed Mediouni
  2021-01-20 17:11   ` Alexander Graf
  2021-01-20 21:18   ` Stan Skowronek
@ 2021-01-21  9:48   ` Linus Walleij
  2021-01-21 10:37     ` Arnd Bergmann
  2021-01-21 16:44     ` Rob Herring
  2021-01-21 16:53   ` Hector Martin 'marcan'
  3 siblings, 2 replies; 55+ messages in thread
From: Linus Walleij @ 2021-01-21  9:48 UTC (permalink / raw)
  To: Mohamed Mediouni
  Cc: Linux ARM, Mark Rutland, Catalin Marinas, Hector Martin,
	linux-kernel, Marc Zyngier, Will Deacon, Stan Skowronek

Hi Mohamed,

thanks for your patch!

On Wed, Jan 20, 2021 at 2:31 PM Mohamed Mediouni
<mohamed.mediouni@caramail.com> wrote:

> +properties:
> +  compatible:
> +    items:
> +      - const: apple,aic

However weird it may seem, Apple is not in the file
Documentation/devicetree/bindings/vendor-prefixes.yaml

(I think it's weird because I remember clearly that they have been
using device tree for PPC since ages.)

Could you add this 2-liner to that file and send it directly to
DT binding maintainers as a single patch as a preparation?

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: [RFC PATCH 4/7] irqchip/apple-aic: Add support for Apple AIC
  2021-01-21  9:48   ` Linus Walleij
@ 2021-01-21 10:37     ` Arnd Bergmann
  2021-01-21 15:29       ` Hector Martin 'marcan'
  2021-01-21 17:45       ` Rob Herring
  2021-01-21 16:44     ` Rob Herring
  1 sibling, 2 replies; 55+ messages in thread
From: Arnd Bergmann @ 2021-01-21 10:37 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Mohamed Mediouni, Mark Rutland, Catalin Marinas, Hector Martin,
	linux-kernel, Marc Zyngier, Will Deacon, Linux ARM,
	Stan Skowronek

On Thu, Jan 21, 2021 at 10:48 AM Linus Walleij <linus.walleij@linaro.org> wrote:
>
> Hi Mohamed,
>
> thanks for your patch!
>
> On Wed, Jan 20, 2021 at 2:31 PM Mohamed Mediouni
> <mohamed.mediouni@caramail.com> wrote:
>
> > +properties:
> > +  compatible:
> > +    items:
> > +      - const: apple,aic
>
> However weird it may seem, Apple is not in the file
> Documentation/devicetree/bindings/vendor-prefixes.yaml
>
> (I think it's weird because I remember clearly that they have been
> using device tree for PPC since ages.)
>
> Could you add this 2-liner to that file and send it directly to
> DT binding maintainers as a single patch as a preparation?

Choosing the vendor prefix here is going to be a little tricky
and non-obvious.

Background:

Traditionally, it should have been the stock ticker symbol of the
company (clearly only publicly traded companies would be able
to produce a Unix capable computer, right?), but there were
already inconsistent: IBM used "ibm" (in small letters), Sun
used "SUNW" (in capitals) but in 2007 changed the stock ticker
symbol to "JAVA", obviously without changing the firmware bindings.

Apple traditionally used "AAPL" (also in caps) in the device tree,
and there is one remnant of that in the M1 device tree, in the form
of the "AAPL,phandle" property in each node, which corresponds
to our "linux,phandle". (phandles were introduced as properties in
both of the flattened DT formats, .dtb and apple's own format).
There are also "AAPL,unit-string properties and some device_type
strings (AAPL,display-crossbar, AAPL,atc-dpxbar, ...) in the M1 DT,
and the CPU nodes (and only those) use "apple" in small letters
as in "apple,icestorm","ARM,v8". The other nodes tend to not have
a vendor prefix, but a lot use a subsystem name as the prefix, such
as compatible="gpio,t8101" or compatible="tempsensor,t8020".

Since Apple are already using both the "AAPL" and the "apple"
prefix themselves, I have a bad feeling about reusing either of
them for defining the devicetree.org bindings that we add to
linux/Documentation/devicetree/bindings. The question is: if
not "apple", what else should we use here?

      Arnd

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: [RFC PATCH 2/7] arm64: kernel: Add a WFI hook.
  2021-01-20 13:27 ` [RFC PATCH 2/7] arm64: kernel: Add a WFI hook Mohamed Mediouni
  2021-01-20 16:46   ` Alexander Graf
@ 2021-01-21 10:52   ` Arnd Bergmann
  2021-01-21 11:01     ` Mohamed Mediouni
  1 sibling, 1 reply; 55+ messages in thread
From: Arnd Bergmann @ 2021-01-21 10:52 UTC (permalink / raw)
  To: Mohamed Mediouni
  Cc: Linux ARM, Mark Rutland, Catalin Marinas, Hector Martin,
	linux-kernel, Marc Zyngier, Will Deacon, Stan Skowronek

On Wed, Jan 20, 2021 at 2:27 PM Mohamed Mediouni
<mohamed.mediouni@caramail.com> wrote:
> --- a/arch/arm64/kernel/cpu_ops.c
> +++ b/arch/arm64/kernel/cpu_ops.c

>  #if defined(CONFIG_STACKPROTECTOR) && !defined(CONFIG_STACKPROTECTOR_PER_TASK)
>  #include <linux/stackprotector.h>
> @@ -74,8 +75,14 @@ void (*arm_pm_restart)(enum reboot_mode reboot_mode, const char *cmd);
>
>  static void noinstr __cpu_do_idle(void)
>  {
> -       dsb(sy);
> -       wfi();
> +       const struct cpu_operations *ops = get_cpu_ops(task_cpu(current));
> +
> +       if (ops->cpu_wfi) {
> +               ops->cpu_wfi();
> +       } else {
> +               dsb(sy);
> +               wfi();
> +       }
>  }

I think the correct place to put this would be a platform specific driver
in drivers/cpuidle/ instead of an added low-level callback in the
default idle function and a custom cpu_operations structure.

       Arnd

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: [RFC PATCH 2/7] arm64: kernel: Add a WFI hook.
  2021-01-21 10:52   ` Arnd Bergmann
@ 2021-01-21 11:01     ` Mohamed Mediouni
  2021-01-21 11:36       ` Arnd Bergmann
  0 siblings, 1 reply; 55+ messages in thread
From: Mohamed Mediouni @ 2021-01-21 11:01 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Linux ARM, Mark Rutland, Catalin Marinas, Hector Martin,
	linux-kernel, Marc Zyngier, Will Deacon, Stan Skowronek



> On 21 Jan 2021, at 11:52, Arnd Bergmann <arnd@kernel.org> wrote:
> 
> On Wed, Jan 20, 2021 at 2:27 PM Mohamed Mediouni
> <mohamed.mediouni@caramail.com> wrote:
>> --- a/arch/arm64/kernel/cpu_ops.c
>> +++ b/arch/arm64/kernel/cpu_ops.c
> 
>> #if defined(CONFIG_STACKPROTECTOR) && !defined(CONFIG_STACKPROTECTOR_PER_TASK)
>> #include <linux/stackprotector.h>
>> @@ -74,8 +75,14 @@ void (*arm_pm_restart)(enum reboot_mode reboot_mode, const char *cmd);
>> 
>> static void noinstr __cpu_do_idle(void)
>> {
>> -       dsb(sy);
>> -       wfi();
>> +       const struct cpu_operations *ops = get_cpu_ops(task_cpu(current));
>> +
>> +       if (ops->cpu_wfi) {
>> +               ops->cpu_wfi();
>> +       } else {
>> +               dsb(sy);
>> +               wfi();
>> +       }
>> }
> 
> I think the correct place to put this would be a platform specific driver
> in drivers/cpuidle/ instead of an added low-level callback in the
> default idle function and a custom cpu_operations structure.
Can we make sure that wfi never gets called even on early
boot when using a cpuidle driver?

Thank you,
> 
>       Arnd


^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: [RFC PATCH 6/7] arm64: kernel: Apple CPU start driver
  2021-01-20 13:27 ` [RFC PATCH 6/7] arm64: kernel: Apple CPU start driver Mohamed Mediouni
@ 2021-01-21 11:14   ` Arnd Bergmann
  0 siblings, 0 replies; 55+ messages in thread
From: Arnd Bergmann @ 2021-01-21 11:14 UTC (permalink / raw)
  To: Mohamed Mediouni
  Cc: Linux ARM, Mark Rutland, Catalin Marinas, Hector Martin,
	linux-kernel, Marc Zyngier, Will Deacon, Stan Skowronek

On Wed, Jan 20, 2021 at 2:27 PM Mohamed Mediouni
<mohamed.mediouni@caramail.com> wrote:
> diff --git a/Documentation/devicetree/bindings/arm/cpus.yaml b/Documentation/devicetree/bindings/arm/cpus.yaml
> index 14cd727d3c4b..a6ff8cb3db1e 100644
> --- a/Documentation/devicetree/bindings/arm/cpus.yaml
> +++ b/Documentation/devicetree/bindings/arm/cpus.yaml
> @@ -176,6 +176,7 @@ properties:
>      oneOf:
>        # On ARM v8 64-bit this property is required
>        - enum:
> +          - apple
>            - psci
>            - spin-table
>        # On ARM 32-bit systems this property is optional

This uses a very generic identifier for doing something that may
be very specific to a particular SoC generation. It's going to be hard
to decide what the right abstraction will be for long-term maintenance,
so I'd recommend starting with a boot loader that implements
spin-table for secondary startup, and getting back to this after more
of the basic stuff works.

> +static int cpu_apple_start_prepare(unsigned int cpu)
> +{
> +       struct device_node *node;
> +       struct cpu_apple_start_info *info;
> +
> +       info = per_cpu_ptr(&cpu_apple_start_info, cpu);
> +
> +       if (info->pmgr_start && info->cputrc_rvbar && info->dbg_unlock)
> +               return 0;
> +
> +       node = of_find_compatible_node(NULL, NULL, "apple,startcpu");
> +       if (!node) {
> +               pr_err("%s: missing startcpu node in device tree.\n", __func__);
> +               return -EINVAL;
> +       }

Where is the binding documentation for this? The way you do a separate
of_iomap() for each CPU suggests that this is not a great binding to
start with. Are these perhaps just individual registers within a larger IP
block in the end?

      Arnd

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: [RFC PATCH 3/7] arm64: mm: use nGnRnE instead of nGnRE on Apple processors
  2021-01-20 13:27 ` [RFC PATCH 3/7] arm64: mm: use nGnRnE instead of nGnRE on Apple processors Mohamed Mediouni
  2021-01-20 16:47   ` Alexander Graf
@ 2021-01-21 11:27   ` Will Deacon
  2021-01-21 11:38     ` Arnd Bergmann
  2021-01-21 11:44     ` Marc Zyngier
  1 sibling, 2 replies; 55+ messages in thread
From: Will Deacon @ 2021-01-21 11:27 UTC (permalink / raw)
  To: Mohamed Mediouni
  Cc: linux-arm-kernel, Catalin Marinas, Mark Rutland, Marc Zyngier,
	Hector Martin, linux-kernel, Stan Skowronek

On Wed, Jan 20, 2021 at 02:27:13PM +0100, Mohamed Mediouni wrote:
> Use nGnRnE instead of nGnRE on Apple SoCs to workaround a serious hardware quirk.
> 
> On Apple processors, writes using the nGnRE device memory type get dropped in flight,
> getting to nowhere.
> 
> Signed-off-by: Stan Skowronek <stan@corellium.com>
> Signed-off-by: Mohamed Mediouni <mohamed.mediouni@caramail.com>
> ---
>  arch/arm64/mm/proc.S | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
> 
> diff --git a/arch/arm64/mm/proc.S b/arch/arm64/mm/proc.S
> index 1f7ee8c8b7b8..06436916f137 100644
> --- a/arch/arm64/mm/proc.S
> +++ b/arch/arm64/mm/proc.S
> @@ -51,6 +51,25 @@
>  #define TCR_KASAN_HW_FLAGS 0
>  #endif
> 
> +#ifdef CONFIG_ARCH_APPLE
> +
> +/*
> + * Apple cores appear to black-hole writes done with nGnRE.
> + * We settled on a work-around that uses MAIR vs changing every single user of
> + * nGnRE across the arm64 code.
> + */
> +
> +#define MAIR_EL1_SET_APPLE						\
> +	(MAIR_ATTRIDX(MAIR_ATTR_DEVICE_nGnRnE, MT_DEVICE_nGnRnE) |	\
> +	 MAIR_ATTRIDX(MAIR_ATTR_DEVICE_nGnRnE, MT_DEVICE_nGnRE) |	\
> +	 MAIR_ATTRIDX(MAIR_ATTR_DEVICE_GRE, MT_DEVICE_GRE) |		\
> +	 MAIR_ATTRIDX(MAIR_ATTR_NORMAL_NC, MT_NORMAL_NC) |		\
> +	 MAIR_ATTRIDX(MAIR_ATTR_NORMAL, MT_NORMAL) |			\
> +	 MAIR_ATTRIDX(MAIR_ATTR_NORMAL_WT, MT_NORMAL_WT) |		\
> +	 MAIR_ATTRIDX(MAIR_ATTR_NORMAL, MT_NORMAL_TAGGED))
> +
> +#endif
> +
>  /*
>   * Default MAIR_EL1. MT_NORMAL_TAGGED is initially mapped as Normal memory and
>   * changed during __cpu_setup to Normal Tagged if the system supports MTE.
> @@ -432,6 +451,13 @@ SYM_FUNC_START(__cpu_setup)
>  	 * Memory region attributes
>  	 */
>  	mov_q	x5, MAIR_EL1_SET
> +#ifdef CONFIG_ARCH_APPLE
> +	mrs	x0, MIDR_EL1
> +	lsr	w0, w0, #24
> +	mov_q	x1, MAIR_EL1_SET_APPLE
> +	cmp	x0, #0x61			// 0x61 = Implementer: Apple
> +	csel	x5, x1, x5, eq

Why does this need to be done so early? It would be a lot cleaner if we
could detect this in a similar fashion to other errata and update the MAIR
appropriately. If that's not possible because of early IO mappings (which
ones?), then we could instead initialise to nGnRnE unconditionally, but
relax it to nGnRE if we detect that we _don't_ have the erratum.

Will

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: [RFC PATCH 2/7] arm64: kernel: Add a WFI hook.
  2021-01-21 11:01     ` Mohamed Mediouni
@ 2021-01-21 11:36       ` Arnd Bergmann
  0 siblings, 0 replies; 55+ messages in thread
From: Arnd Bergmann @ 2021-01-21 11:36 UTC (permalink / raw)
  To: Mohamed Mediouni
  Cc: Linux ARM, Mark Rutland, Catalin Marinas, Hector Martin,
	linux-kernel, Marc Zyngier, Will Deacon, Stan Skowronek

On Thu, Jan 21, 2021 at 12:01 PM Mohamed Mediouni
<mohamed.mediouni@caramail.com> wrote:
> > On 21 Jan 2021, at 11:52, Arnd Bergmann <arnd@kernel.org> wrote:
> >
> > On Wed, Jan 20, 2021 at 2:27 PM Mohamed Mediouni
> > <mohamed.mediouni@caramail.com> wrote:
> >> --- a/arch/arm64/kernel/cpu_ops.c
> >> +++ b/arch/arm64/kernel/cpu_ops.c
> >
> >> #if defined(CONFIG_STACKPROTECTOR) && !defined(CONFIG_STACKPROTECTOR_PER_TASK)
> >> #include <linux/stackprotector.h>
> >> @@ -74,8 +75,14 @@ void (*arm_pm_restart)(enum reboot_mode reboot_mode, const char *cmd);
> >>
> >> static void noinstr __cpu_do_idle(void)
> >> {
> >> -       dsb(sy);
> >> -       wfi();
> >> +       const struct cpu_operations *ops = get_cpu_ops(task_cpu(current));
> >> +
> >> +       if (ops->cpu_wfi) {
> >> +               ops->cpu_wfi();
> >> +       } else {
> >> +               dsb(sy);
> >> +               wfi();
> >> +       }
> >> }
> >
> > I think the correct place to put this would be a platform specific driver
> > in drivers/cpuidle/ instead of an added low-level callback in the
> > default idle function and a custom cpu_operations structure.
>
> Can we make sure that wfi never gets called even on early
> boot when using a cpuidle driver?

Good question, I don't know what all the possible call sites are
for this, but if there is nothing else works (such as what Alex suggested),
it may be possible to just patch out the wfi instruction here and
do a busy loop until the cpuidle driver has come up.

The main issue here is the existence of the custom cpu_operations
in the first place: I don't think we want or need the custom start_secondary
at the moment (as commented in the other patch), but then there is
no obvious place to put the custom wfi.

Note that there are a few other uses of the wfi instruction besides the
one in __cpu_do_idle(), so whatever you do here may also apply to the
others.

arch/arm64/include/asm/smp.h:           wfi();
arch/arm64/kernel/head.S:       wfi
arch/arm64/kernel/head.S:       wfi
arch/arm64/kernel/head.S:       wfi
arch/arm64/kernel/process.c:    wfi();
arch/arm64/kvm/hyp/nvhe/hyp-init.S:     wfi

       Arnd

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: [RFC PATCH 3/7] arm64: mm: use nGnRnE instead of nGnRE on Apple processors
  2021-01-21 11:27   ` Will Deacon
@ 2021-01-21 11:38     ` Arnd Bergmann
  2021-01-21 11:44     ` Marc Zyngier
  1 sibling, 0 replies; 55+ messages in thread
From: Arnd Bergmann @ 2021-01-21 11:38 UTC (permalink / raw)
  To: Will Deacon
  Cc: Mohamed Mediouni, Linux ARM, Catalin Marinas, Mark Rutland,
	Marc Zyngier, Hector Martin, linux-kernel, Stan Skowronek

On Thu, Jan 21, 2021 at 12:32 PM Will Deacon <will@kernel.org> wrote:
> On Wed, Jan 20, 2021 at 02:27:13PM +0100, Mohamed Mediouni wrote:
> > Use nGnRnE instead of nGnRE on Apple SoCs to workaround a serious hardware quirk.
> >  /*
> >   * Default MAIR_EL1. MT_NORMAL_TAGGED is initially mapped as Normal memory and
> >   * changed during __cpu_setup to Normal Tagged if the system supports MTE.
> > @@ -432,6 +451,13 @@ SYM_FUNC_START(__cpu_setup)
> >        * Memory region attributes
> >        */
> >       mov_q   x5, MAIR_EL1_SET
> > +#ifdef CONFIG_ARCH_APPLE
> > +     mrs     x0, MIDR_EL1
> > +     lsr     w0, w0, #24
> > +     mov_q   x1, MAIR_EL1_SET_APPLE
> > +     cmp     x0, #0x61                       // 0x61 = Implementer: Apple
> > +     csel    x5, x1, x5, eq
>
> Why does this need to be done so early? It would be a lot cleaner if we
> could detect this in a similar fashion to other errata and update the MAIR
> appropriately. If that's not possible because of early IO mappings (which
> ones?), then we could instead initialise to nGnRnE unconditionally, but
> relax it to nGnRE if we detect that we _don't_ have the erratum.

There is (at least) the custom SMP startup code that uses device
mappings. If that's the only thing that needs the modified MAIR
to be used early, I'd consider that one more reason against doing the
custom cpu_operations for now.

       Arnd

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: [RFC PATCH 3/7] arm64: mm: use nGnRnE instead of nGnRE on Apple processors
  2021-01-21 11:27   ` Will Deacon
  2021-01-21 11:38     ` Arnd Bergmann
@ 2021-01-21 11:44     ` Marc Zyngier
  2021-01-21 12:47       ` Will Deacon
  1 sibling, 1 reply; 55+ messages in thread
From: Marc Zyngier @ 2021-01-21 11:44 UTC (permalink / raw)
  To: Will Deacon
  Cc: Mohamed Mediouni, linux-arm-kernel, Catalin Marinas,
	Mark Rutland, Hector Martin, linux-kernel, Stan Skowronek

On 2021-01-21 11:27, Will Deacon wrote:
> On Wed, Jan 20, 2021 at 02:27:13PM +0100, Mohamed Mediouni wrote:
>> Use nGnRnE instead of nGnRE on Apple SoCs to workaround a serious 
>> hardware quirk.
>> 
>> On Apple processors, writes using the nGnRE device memory type get 
>> dropped in flight,
>> getting to nowhere.
>> 
>> Signed-off-by: Stan Skowronek <stan@corellium.com>
>> Signed-off-by: Mohamed Mediouni <mohamed.mediouni@caramail.com>
>> ---
>>  arch/arm64/mm/proc.S | 26 ++++++++++++++++++++++++++
>>  1 file changed, 26 insertions(+)
>> 
>> diff --git a/arch/arm64/mm/proc.S b/arch/arm64/mm/proc.S
>> index 1f7ee8c8b7b8..06436916f137 100644
>> --- a/arch/arm64/mm/proc.S
>> +++ b/arch/arm64/mm/proc.S
>> @@ -51,6 +51,25 @@
>>  #define TCR_KASAN_HW_FLAGS 0
>>  #endif
>> 
>> +#ifdef CONFIG_ARCH_APPLE
>> +
>> +/*
>> + * Apple cores appear to black-hole writes done with nGnRE.
>> + * We settled on a work-around that uses MAIR vs changing every 
>> single user of
>> + * nGnRE across the arm64 code.
>> + */
>> +
>> +#define MAIR_EL1_SET_APPLE						\
>> +	(MAIR_ATTRIDX(MAIR_ATTR_DEVICE_nGnRnE, MT_DEVICE_nGnRnE) |	\
>> +	 MAIR_ATTRIDX(MAIR_ATTR_DEVICE_nGnRnE, MT_DEVICE_nGnRE) |	\
>> +	 MAIR_ATTRIDX(MAIR_ATTR_DEVICE_GRE, MT_DEVICE_GRE) |		\
>> +	 MAIR_ATTRIDX(MAIR_ATTR_NORMAL_NC, MT_NORMAL_NC) |		\
>> +	 MAIR_ATTRIDX(MAIR_ATTR_NORMAL, MT_NORMAL) |			\
>> +	 MAIR_ATTRIDX(MAIR_ATTR_NORMAL_WT, MT_NORMAL_WT) |		\
>> +	 MAIR_ATTRIDX(MAIR_ATTR_NORMAL, MT_NORMAL_TAGGED))
>> +
>> +#endif
>> +
>>  /*
>>   * Default MAIR_EL1. MT_NORMAL_TAGGED is initially mapped as Normal 
>> memory and
>>   * changed during __cpu_setup to Normal Tagged if the system supports 
>> MTE.
>> @@ -432,6 +451,13 @@ SYM_FUNC_START(__cpu_setup)
>>  	 * Memory region attributes
>>  	 */
>>  	mov_q	x5, MAIR_EL1_SET
>> +#ifdef CONFIG_ARCH_APPLE
>> +	mrs	x0, MIDR_EL1
>> +	lsr	w0, w0, #24
>> +	mov_q	x1, MAIR_EL1_SET_APPLE
>> +	cmp	x0, #0x61			// 0x61 = Implementer: Apple
>> +	csel	x5, x1, x5, eq
> 
> Why does this need to be done so early? It would be a lot cleaner if we
> could detect this in a similar fashion to other errata and update the 
> MAIR
> appropriately. If that's not possible because of early IO mappings 
> (which
> ones?), then we could instead initialise to nGnRnE unconditionally, but
> relax it to nGnRE if we detect that we _don't_ have the erratum.

Would that imply another round-trip into the idmap, much like we do
when we switch to non-global mappings? Or do you expect that we can 
change
the MAIR with live mappings?

         M.
-- 
Jazz is not dead. It just smells funny...

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: [RFC PATCH 2/7] arm64: kernel: Add a WFI hook.
       [not found]     ` <94C20F55-D3B8-4349-B26F-9EA8AAEBF639@caramail.com>
@ 2021-01-21 12:33       ` Hector Martin 'marcan'
  0 siblings, 0 replies; 55+ messages in thread
From: Hector Martin 'marcan' @ 2021-01-21 12:33 UTC (permalink / raw)
  To: Mohamed Mediouni, Alexander Graf
  Cc: linux-arm-kernel, Catalin Marinas, Will Deacon, Mark Rutland,
	Marc Zyngier, linux-kernel, Stan Skowronek

I already mentioned this privately, but for the benefit of the ML:

On 21/01/2021 09.48, Mohamed Mediouni wrote:
> If we explicitly use the hardware override registers for this, then
> we'll be unable to use the deep sleep support provided by wfi on
> Apple SoCs later on without touching Apple-specific MSRs.
> 
> Our current policy is to avoid having those modified inside the kernel
> at all costs, considering it to be a job for the bootloader instead.

I don't think there is any particular reason to do this; the bootloader 
should be responsible for setting up all the chicken bits that make the 
CPU work properly, including doing so for all SMP cores before entering 
the kernel, but that's not the same thing as power management bits.

It seems entirely reasonable to me to configure WFI as clockgate only 
(so it keeps state), not touch this part of kernel code at all, and then 
in the cpuidle driver (which can come later) just do:

- switch to powerdown mode
- save state
- wfi
- restore state
- switch to clockgate mode

-- 
Hector Martin "marcan" (marcan@marcan.st)
Public Key: https://mrcn.st/pub

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: [RFC PATCH 7/7] irqchip/apple-aic: add SMP support to the Apple AIC driver.
  2021-01-20 13:27 ` [RFC PATCH 7/7] irqchip/apple-aic: add SMP support to the Apple AIC driver Mohamed Mediouni
@ 2021-01-21 12:44   ` Arnd Bergmann
  2021-01-21 12:50     ` Mohamed Mediouni
  2021-02-02 19:15   ` Linus Walleij
  1 sibling, 1 reply; 55+ messages in thread
From: Arnd Bergmann @ 2021-01-21 12:44 UTC (permalink / raw)
  To: Mohamed Mediouni
  Cc: Linux ARM, Mark Rutland, Catalin Marinas, Hector Martin,
	linux-kernel, Marc Zyngier, Will Deacon, Stan Skowronek

On Wed, Jan 20, 2021 at 2:27 PM Mohamed Mediouni
<mohamed.mediouni@caramail.com> wrote:

> +#ifdef CONFIG_SMP
> +static void apple_aic_ipi_send_mask(struct irq_data *d,
> +                                   const struct cpumask *mask)

Not sure we care about the #ifdef here, given that arch/arm64 does not
allow building a kernel without CONFIG_SMP.

> +       /*
> +     * Ensure that stores to Normal memory are visible to the
> +     * other CPUs before issuing the IPI.
> +     */
> +       wmb();
> +
> +       for_each_cpu (cpu, mask) {
> +               smp_mb__before_atomic();
> +               atomic_or(1u << irqnr, per_cpu_ptr(&aic_ipi_mask, cpu));
> +               smp_mb__after_atomic();
> +               lcpu = get_cpu();
> +               if (aic.fast_ipi) {
> +                       if ((lcpu >> 2) == (cpu >> 2))
> +                               write_sysreg(cpu & 3, SR_APPLE_IPI_LOCAL);
> +                       else
> +                               write_sysreg((cpu & 3) | ((cpu >> 2) << 16),
> +                                            SR_APPLE_IPI_REMOTE);
> +               } else
> +                       writel(lcpu == cpu ? REG_IPI_FLAG_SELF :
> +                                                  (REG_IPI_FLAG_OTHER << cpu),
> +                              aic.base + REG_IPI_SET);
> +               put_cpu();
> +       }
> +
> +       /* Force the above writes to be executed */
> +       if (aic.fast_ipi)
> +               isb();
> +}

Since this just loops over all CPUs, I'd probably just turn it into
an ipi_send_single() callback and have the caller do the
loop for simplicity.

I also have the feeling that splitting one hardware IPI into multiple
logical interrupts, which are then all registered by the same irq
handler adds a little more complexity than necessary.

Changing this would of course require modifications to
arch/arm64/kernel/smp.c, which is hardwired to use
CONFIG_GENERIC_IRQ_IPI in smp_cross_call(), and allowing
a different code path there may be worse than emulating an
irqchip.

> @@ -186,8 +325,11 @@ static int __init apple_aic_init(struct device_node *node,
>         if (WARN(!aic.base, "unable to map aic registers\n"))
>                 return -EINVAL;
>
> +       aic.fast_ipi = of_property_read_bool(node, "fast-ipi");

Where is this property documented, and what decides which one to use?

        Arnd

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: [RFC PATCH 3/7] arm64: mm: use nGnRnE instead of nGnRE on Apple processors
  2021-01-21 11:44     ` Marc Zyngier
@ 2021-01-21 12:47       ` Will Deacon
  2021-01-21 15:12         ` Mohamed Mediouni
  0 siblings, 1 reply; 55+ messages in thread
From: Will Deacon @ 2021-01-21 12:47 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: Mohamed Mediouni, linux-arm-kernel, Catalin Marinas,
	Mark Rutland, Hector Martin, linux-kernel, Stan Skowronek

On Thu, Jan 21, 2021 at 11:44:23AM +0000, Marc Zyngier wrote:
> On 2021-01-21 11:27, Will Deacon wrote:
> > On Wed, Jan 20, 2021 at 02:27:13PM +0100, Mohamed Mediouni wrote:
> > > Use nGnRnE instead of nGnRE on Apple SoCs to workaround a serious
> > > hardware quirk.
> > > 
> > > On Apple processors, writes using the nGnRE device memory type get
> > > dropped in flight,
> > > getting to nowhere.
> > > 
> > > Signed-off-by: Stan Skowronek <stan@corellium.com>
> > > Signed-off-by: Mohamed Mediouni <mohamed.mediouni@caramail.com>
> > > ---
> > >  arch/arm64/mm/proc.S | 26 ++++++++++++++++++++++++++
> > >  1 file changed, 26 insertions(+)
> > > 
> > > diff --git a/arch/arm64/mm/proc.S b/arch/arm64/mm/proc.S
> > > index 1f7ee8c8b7b8..06436916f137 100644
> > > --- a/arch/arm64/mm/proc.S
> > > +++ b/arch/arm64/mm/proc.S
> > > @@ -51,6 +51,25 @@
> > >  #define TCR_KASAN_HW_FLAGS 0
> > >  #endif
> > > 
> > > +#ifdef CONFIG_ARCH_APPLE
> > > +
> > > +/*
> > > + * Apple cores appear to black-hole writes done with nGnRE.
> > > + * We settled on a work-around that uses MAIR vs changing every
> > > single user of
> > > + * nGnRE across the arm64 code.
> > > + */
> > > +
> > > +#define MAIR_EL1_SET_APPLE						\
> > > +	(MAIR_ATTRIDX(MAIR_ATTR_DEVICE_nGnRnE, MT_DEVICE_nGnRnE) |	\
> > > +	 MAIR_ATTRIDX(MAIR_ATTR_DEVICE_nGnRnE, MT_DEVICE_nGnRE) |	\
> > > +	 MAIR_ATTRIDX(MAIR_ATTR_DEVICE_GRE, MT_DEVICE_GRE) |		\
> > > +	 MAIR_ATTRIDX(MAIR_ATTR_NORMAL_NC, MT_NORMAL_NC) |		\
> > > +	 MAIR_ATTRIDX(MAIR_ATTR_NORMAL, MT_NORMAL) |			\
> > > +	 MAIR_ATTRIDX(MAIR_ATTR_NORMAL_WT, MT_NORMAL_WT) |		\
> > > +	 MAIR_ATTRIDX(MAIR_ATTR_NORMAL, MT_NORMAL_TAGGED))
> > > +
> > > +#endif
> > > +
> > >  /*
> > >   * Default MAIR_EL1. MT_NORMAL_TAGGED is initially mapped as Normal
> > > memory and
> > >   * changed during __cpu_setup to Normal Tagged if the system
> > > supports MTE.
> > > @@ -432,6 +451,13 @@ SYM_FUNC_START(__cpu_setup)
> > >  	 * Memory region attributes
> > >  	 */
> > >  	mov_q	x5, MAIR_EL1_SET
> > > +#ifdef CONFIG_ARCH_APPLE
> > > +	mrs	x0, MIDR_EL1
> > > +	lsr	w0, w0, #24
> > > +	mov_q	x1, MAIR_EL1_SET_APPLE
> > > +	cmp	x0, #0x61			// 0x61 = Implementer: Apple
> > > +	csel	x5, x1, x5, eq
> > 
> > Why does this need to be done so early? It would be a lot cleaner if we
> > could detect this in a similar fashion to other errata and update the
> > MAIR
> > appropriately. If that's not possible because of early IO mappings
> > (which
> > ones?), then we could instead initialise to nGnRnE unconditionally, but
> > relax it to nGnRE if we detect that we _don't_ have the erratum.
> 
> Would that imply another round-trip into the idmap, much like we do
> when we switch to non-global mappings? Or do you expect that we can change
> the MAIR with live mappings?

I think we should be able to change it live and then invalidate the TLB. At
least, my reading of the BBM requirements suggests that it isn't required
for changing between different types of device memory. I can seek
clarification from Arm if necessary.

Will

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: [RFC PATCH 7/7] irqchip/apple-aic: add SMP support to the Apple AIC driver.
  2021-01-21 12:44   ` Arnd Bergmann
@ 2021-01-21 12:50     ` Mohamed Mediouni
  2021-01-21 13:00       ` Arnd Bergmann
                         ` (2 more replies)
  0 siblings, 3 replies; 55+ messages in thread
From: Mohamed Mediouni @ 2021-01-21 12:50 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Linux ARM, Mark Rutland, Catalin Marinas, Hector Martin,
	linux-kernel, Marc Zyngier, Will Deacon, Stan Skowronek



> On 21 Jan 2021, at 13:44, Arnd Bergmann <arnd@kernel.org> wrote:
> 
> On Wed, Jan 20, 2021 at 2:27 PM Mohamed Mediouni
> <mohamed.mediouni@caramail.com> wrote:
> 
>> +#ifdef CONFIG_SMP
>> +static void apple_aic_ipi_send_mask(struct irq_data *d,
>> +                                   const struct cpumask *mask)
> 
> Not sure we care about the #ifdef here, given that arch/arm64 does not
> allow building a kernel without CONFIG_SMP.
> 
>> +       /*
>> +     * Ensure that stores to Normal memory are visible to the
>> +     * other CPUs before issuing the IPI.
>> +     */
>> +       wmb();
>> +
>> +       for_each_cpu (cpu, mask) {
>> +               smp_mb__before_atomic();
>> +               atomic_or(1u << irqnr, per_cpu_ptr(&aic_ipi_mask, cpu));
>> +               smp_mb__after_atomic();
>> +               lcpu = get_cpu();
>> +               if (aic.fast_ipi) {
>> +                       if ((lcpu >> 2) == (cpu >> 2))
>> +                               write_sysreg(cpu & 3, SR_APPLE_IPI_LOCAL);
>> +                       else
>> +                               write_sysreg((cpu & 3) | ((cpu >> 2) << 16),
>> +                                            SR_APPLE_IPI_REMOTE);
>> +               } else
>> +                       writel(lcpu == cpu ? REG_IPI_FLAG_SELF :
>> +                                                  (REG_IPI_FLAG_OTHER << cpu),
>> +                              aic.base + REG_IPI_SET);
>> +               put_cpu();
>> +       }
>> +
>> +       /* Force the above writes to be executed */
>> +       if (aic.fast_ipi)
>> +               isb();
>> +}
> 
> Since this just loops over all CPUs, I'd probably just turn it into
> an ipi_send_single() callback and have the caller do the
> loop for simplicity.
> 
> I also have the feeling that splitting one hardware IPI into multiple
> logical interrupts, which are then all registered by the same irq
> handler adds a little more complexity than necessary.
> 
> Changing this would of course require modifications to
> arch/arm64/kernel/smp.c, which is hardwired to use
> CONFIG_GENERIC_IRQ_IPI in smp_cross_call(), and allowing
> a different code path there may be worse than emulating an
> irqchip.
> 
>> @@ -186,8 +325,11 @@ static int __init apple_aic_init(struct device_node *node,
>>       if (WARN(!aic.base, "unable to map aic registers\n"))
>>               return -EINVAL;
>> 
>> +       aic.fast_ipi = of_property_read_bool(node, "fast-ipi");
> 
> Where is this property documented, and what decides which one to use?
It’s getting documented in the next patch set.

This property is there to enable support for older iPhone processors
later on, some of which do not have fast IPI support.

On Apple M1, fast-ipi is always on.

Thank you,
>       Arnd


^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: [RFC PATCH 7/7] irqchip/apple-aic: add SMP support to the Apple AIC driver.
  2021-01-21 12:50     ` Mohamed Mediouni
@ 2021-01-21 13:00       ` Arnd Bergmann
  2021-01-21 13:01         ` Hector Martin 'marcan'
  2021-01-21 13:22       ` Marc Zyngier
  2021-01-21 16:40       ` Rob Herring
  2 siblings, 1 reply; 55+ messages in thread
From: Arnd Bergmann @ 2021-01-21 13:00 UTC (permalink / raw)
  To: Mohamed Mediouni
  Cc: Linux ARM, Mark Rutland, Catalin Marinas, Hector Martin,
	linux-kernel, Marc Zyngier, Will Deacon, Stan Skowronek

On Thu, Jan 21, 2021 at 1:50 PM Mohamed Mediouni
<mohamed.mediouni@caramail.com> wrote:
> > On 21 Jan 2021, at 13:44, Arnd Bergmann <arnd@kernel.org> wrote:
> >> @@ -186,8 +325,11 @@ static int __init apple_aic_init(struct device_node *node,
> >>       if (WARN(!aic.base, "unable to map aic registers\n"))
> >>               return -EINVAL;
> >>
> >> +       aic.fast_ipi = of_property_read_bool(node, "fast-ipi");
> >
> > Where is this property documented, and what decides which one to use?
> It’s getting documented in the next patch set.
>
> This property is there to enable support for older iPhone processors
> later on, some of which do not have fast IPI support.
>
> On Apple M1, fast-ipi is always on.

Ok, makes sense. Does fast-ipi mean you cannot use the other mode at
all, or is it just faster as implied by the name? If so, how much faster?

       Arnd

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: [RFC PATCH 7/7] irqchip/apple-aic: add SMP support to the Apple AIC driver.
  2021-01-21 13:00       ` Arnd Bergmann
@ 2021-01-21 13:01         ` Hector Martin 'marcan'
  0 siblings, 0 replies; 55+ messages in thread
From: Hector Martin 'marcan' @ 2021-01-21 13:01 UTC (permalink / raw)
  To: Arnd Bergmann, Mohamed Mediouni
  Cc: Linux ARM, Mark Rutland, Catalin Marinas, linux-kernel,
	Marc Zyngier, Will Deacon, Stan Skowronek

On 21/01/2021 22.00, Arnd Bergmann wrote:
> Ok, makes sense. Does fast-ipi mean you cannot use the other mode at
> all, or is it just faster as implied by the name? If so, how much faster?

The other mode still works, as the AIC IPI registers are still there on 
M1 and work properly. I don't have performance numbers though.

-- 
Hector Martin "marcan" (marcan@marcan.st)
Public Key: https://mrcn.st/pub

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: [RFC PATCH 7/7] irqchip/apple-aic: add SMP support to the Apple AIC driver.
  2021-01-21 12:50     ` Mohamed Mediouni
  2021-01-21 13:00       ` Arnd Bergmann
@ 2021-01-21 13:22       ` Marc Zyngier
  2021-01-21 13:32         ` Mark Rutland
  2021-01-21 13:34         ` Mohamed Mediouni
  2021-01-21 16:40       ` Rob Herring
  2 siblings, 2 replies; 55+ messages in thread
From: Marc Zyngier @ 2021-01-21 13:22 UTC (permalink / raw)
  To: Mohamed Mediouni
  Cc: Arnd Bergmann, Linux ARM, Mark Rutland, Catalin Marinas,
	Hector Martin, linux-kernel, Will Deacon, Stan Skowronek

On 2021-01-21 12:50, Mohamed Mediouni wrote:
>> On 21 Jan 2021, at 13:44, Arnd Bergmann <arnd@kernel.org> wrote:
>> 
>> On Wed, Jan 20, 2021 at 2:27 PM Mohamed Mediouni

[...]

>>> +       aic.fast_ipi = of_property_read_bool(node, "fast-ipi");
>> 
>> Where is this property documented, and what decides which one to use?
> It’s getting documented in the next patch set.
> 
> This property is there to enable support for older iPhone processors
> later on, some of which do not have fast IPI support.
> 
> On Apple M1, fast-ipi is always on.

Then please focus on a single implementation. Additional features can
always be merged later once something is up and running.

Also, there sysregs can be detected by matching the MIDR, so I don't
think we need a DT property for that.

Thanks,

         M.
-- 
Jazz is not dead. It just smells funny...

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: [RFC PATCH 7/7] irqchip/apple-aic: add SMP support to the Apple AIC driver.
  2021-01-21 13:22       ` Marc Zyngier
@ 2021-01-21 13:32         ` Mark Rutland
  2021-01-21 14:05           ` Marc Zyngier
  2021-01-21 13:34         ` Mohamed Mediouni
  1 sibling, 1 reply; 55+ messages in thread
From: Mark Rutland @ 2021-01-21 13:32 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: Mohamed Mediouni, Arnd Bergmann, Linux ARM, Catalin Marinas,
	Hector Martin, linux-kernel, Will Deacon, Stan Skowronek

On Thu, Jan 21, 2021 at 01:22:37PM +0000, Marc Zyngier wrote:
> On 2021-01-21 12:50, Mohamed Mediouni wrote:
> > > On 21 Jan 2021, at 13:44, Arnd Bergmann <arnd@kernel.org> wrote:
> > > 
> > > On Wed, Jan 20, 2021 at 2:27 PM Mohamed Mediouni
> 
> [...]
> 
> > > > +       aic.fast_ipi = of_property_read_bool(node, "fast-ipi");
> > > 
> > > Where is this property documented, and what decides which one to use?
> > It’s getting documented in the next patch set.
> > 
> > This property is there to enable support for older iPhone processors
> > later on, some of which do not have fast IPI support.
> > 
> > On Apple M1, fast-ipi is always on.
> 
> Then please focus on a single implementation. Additional features can
> always be merged later once something is up and running.
> 
> Also, there sysregs can be detected by matching the MIDR, so I don't
> think we need a DT property for that.

Generally we do not detect IMP-DEF sysregs based on MIDR because they
won't necessarily be exposed to a VM, so I suspect that we do need DT
properties to describe that IMP-DEF sysregs are accessible, and should
not rely on the MIDR alone. Maybe that's implicit in another property,
but worth bearing in mind.

Thanks,
Mark.

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: [RFC PATCH 7/7] irqchip/apple-aic: add SMP support to the Apple AIC driver.
  2021-01-21 13:22       ` Marc Zyngier
  2021-01-21 13:32         ` Mark Rutland
@ 2021-01-21 13:34         ` Mohamed Mediouni
  2021-01-21 14:10           ` Marc Zyngier
  1 sibling, 1 reply; 55+ messages in thread
From: Mohamed Mediouni @ 2021-01-21 13:34 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: Arnd Bergmann, Linux ARM, Mark Rutland, Catalin Marinas,
	Hector Martin, linux-kernel, Will Deacon, Stan Skowronek



> On 21 Jan 2021, at 14:22, Marc Zyngier <maz@kernel.org> wrote:
> 
> On 2021-01-21 12:50, Mohamed Mediouni wrote:
>>> On 21 Jan 2021, at 13:44, Arnd Bergmann <arnd@kernel.org> wrote:
>>> On Wed, Jan 20, 2021 at 2:27 PM Mohamed Mediouni
> 
> [...]
> 
>>>> +       aic.fast_ipi = of_property_read_bool(node, "fast-ipi");
>>> Where is this property documented, and what decides which one to use?
>> It’s getting documented in the next patch set.
>> This property is there to enable support for older iPhone processors
>> later on, some of which do not have fast IPI support.
>> On Apple M1, fast-ipi is always on.
> 
> Then please focus on a single implementation. Additional features can
> always be merged later once something is up and running.
> 
> Also, there sysregs can be detected by matching the MIDR, so I don't
> think we need a DT property for that.
> 
> Thanks,
> 
Because UART access adapters for the new M1 Macs aren’t plentiful
at all, I actually use this for development, with iPhones which have 
much more easy to buy Lightning-to-UART adapters.

(That’s why the old implementation is there too)

Might be worth splitting the new one to a new commit though...

Thank you,
>        M.
> -- 
> Jazz is not dead. It just smells funny...


^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: [RFC PATCH 7/7] irqchip/apple-aic: add SMP support to the Apple AIC driver.
  2021-01-21 13:32         ` Mark Rutland
@ 2021-01-21 14:05           ` Marc Zyngier
  0 siblings, 0 replies; 55+ messages in thread
From: Marc Zyngier @ 2021-01-21 14:05 UTC (permalink / raw)
  To: Mark Rutland
  Cc: Mohamed Mediouni, Arnd Bergmann, Linux ARM, Catalin Marinas,
	Hector Martin, linux-kernel, Will Deacon, Stan Skowronek

On 2021-01-21 13:32, Mark Rutland wrote:
> On Thu, Jan 21, 2021 at 01:22:37PM +0000, Marc Zyngier wrote:
>> On 2021-01-21 12:50, Mohamed Mediouni wrote:
>> > > On 21 Jan 2021, at 13:44, Arnd Bergmann <arnd@kernel.org> wrote:
>> > >
>> > > On Wed, Jan 20, 2021 at 2:27 PM Mohamed Mediouni
>> 
>> [...]
>> 
>> > > > +       aic.fast_ipi = of_property_read_bool(node, "fast-ipi");
>> > >
>> > > Where is this property documented, and what decides which one to use?
>> > It’s getting documented in the next patch set.
>> >
>> > This property is there to enable support for older iPhone processors
>> > later on, some of which do not have fast IPI support.
>> >
>> > On Apple M1, fast-ipi is always on.
>> 
>> Then please focus on a single implementation. Additional features can
>> always be merged later once something is up and running.
>> 
>> Also, there sysregs can be detected by matching the MIDR, so I don't
>> think we need a DT property for that.
> 
> Generally we do not detect IMP-DEF sysregs based on MIDR because they
> won't necessarily be exposed to a VM, so I suspect that we do need DT
> properties to describe that IMP-DEF sysregs are accessible, and should
> not rely on the MIDR alone. Maybe that's implicit in another property,
> but worth bearing in mind.

Hmm. That's a good point. I think this could be keyed off
the compatible property, which should accurately reflect
the version of the interrupt controller.

Thanks,

         M.
-- 
Jazz is not dead. It just smells funny...

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: [RFC PATCH 7/7] irqchip/apple-aic: add SMP support to the Apple AIC driver.
  2021-01-21 13:34         ` Mohamed Mediouni
@ 2021-01-21 14:10           ` Marc Zyngier
  2021-01-21 15:09             ` Arnd Bergmann
  0 siblings, 1 reply; 55+ messages in thread
From: Marc Zyngier @ 2021-01-21 14:10 UTC (permalink / raw)
  To: Mohamed Mediouni
  Cc: Arnd Bergmann, Linux ARM, Mark Rutland, Catalin Marinas,
	Hector Martin, linux-kernel, Will Deacon, Stan Skowronek

On 2021-01-21 13:34, Mohamed Mediouni wrote:
>> On 21 Jan 2021, at 14:22, Marc Zyngier <maz@kernel.org> wrote:
>> 
>> On 2021-01-21 12:50, Mohamed Mediouni wrote:
>>>> On 21 Jan 2021, at 13:44, Arnd Bergmann <arnd@kernel.org> wrote:
>>>> On Wed, Jan 20, 2021 at 2:27 PM Mohamed Mediouni
>> 
>> [...]
>> 
>>>>> +       aic.fast_ipi = of_property_read_bool(node, "fast-ipi");
>>>> Where is this property documented, and what decides which one to 
>>>> use?
>>> It’s getting documented in the next patch set.
>>> This property is there to enable support for older iPhone processors
>>> later on, some of which do not have fast IPI support.
>>> On Apple M1, fast-ipi is always on.
>> 
>> Then please focus on a single implementation. Additional features can
>> always be merged later once something is up and running.
>> 
>> Also, there sysregs can be detected by matching the MIDR, so I don't
>> think we need a DT property for that.
>> 
>> Thanks,
>> 
> Because UART access adapters for the new M1 Macs aren’t plentiful
> at all, I actually use this for development, with iPhones which have
> much more easy to buy Lightning-to-UART adapters.
> 
> (That’s why the old implementation is there too)
> 
> Might be worth splitting the new one to a new commit though...

This series is supposed to cover M1 only, and adding extra support
as part of it is only likely to make the code harder to review.

I'd rather you focus on a single IPI interface (fast or slow,
I don't really care). Extra features can come in later.

Thanks,

         M.
-- 
Jazz is not dead. It just smells funny...

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: [RFC PATCH 7/7] irqchip/apple-aic: add SMP support to the Apple AIC driver.
  2021-01-21 14:10           ` Marc Zyngier
@ 2021-01-21 15:09             ` Arnd Bergmann
  2021-01-21 15:18               ` Mohamed Mediouni
  0 siblings, 1 reply; 55+ messages in thread
From: Arnd Bergmann @ 2021-01-21 15:09 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: Mohamed Mediouni, Mark Rutland, Catalin Marinas, Hector Martin,
	linux-kernel, Will Deacon, Linux ARM, Stan Skowronek

On Thu, Jan 21, 2021 at 3:10 PM Marc Zyngier <maz@kernel.org> wrote:
> On 2021-01-21 13:34, Mohamed Mediouni wrote:
> >> On 21 Jan 2021, at 14:22, Marc Zyngier <maz@kernel.org> wrote:
> > Because UART access adapters for the new M1 Macs aren’t plentiful
> > at all, I actually use this for development, with iPhones which have
> > much more easy to buy Lightning-to-UART adapters.
> >
> > (That’s why the old implementation is there too)
> >
> > Might be worth splitting the new one to a new commit though...
>
> This series is supposed to cover M1 only, and adding extra support
> as part of it is only likely to make the code harder to review.
>
> I'd rather you focus on a single IPI interface (fast or slow,
> I don't really care). Extra features can come in later.

Agreed. The slow interface is probably easier to start with,
because it avoids hooking into the FIQ, so the FIQ can be
completely decoupled from AIC and just used for the timer.

Maybe there is even a way to use more than one hardware IPI in
the AIC?

       Arnd

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: [RFC PATCH 3/7] arm64: mm: use nGnRnE instead of nGnRE on Apple processors
  2021-01-21 12:47       ` Will Deacon
@ 2021-01-21 15:12         ` Mohamed Mediouni
  2021-01-21 16:25           ` Marc Zyngier
  0 siblings, 1 reply; 55+ messages in thread
From: Mohamed Mediouni @ 2021-01-21 15:12 UTC (permalink / raw)
  To: Will Deacon
  Cc: Marc Zyngier, Linux ARM, Catalin Marinas, Mark Rutland,
	Hector Martin, linux-kernel, Stan Skowronek



> On 21 Jan 2021, at 13:47, Will Deacon <will@kernel.org> wrote:
> 
> On Thu, Jan 21, 2021 at 11:44:23AM +0000, Marc Zyngier wrote:
>> On 2021-01-21 11:27, Will Deacon wrote:
>>> On Wed, Jan 20, 2021 at 02:27:13PM +0100, Mohamed Mediouni wrote:
>>>> Use nGnRnE instead of nGnRE on Apple SoCs to workaround a serious
>>>> hardware quirk.
>>>> 
>>>> On Apple processors, writes using the nGnRE device memory type get
>>>> dropped in flight,
>>>> getting to nowhere.
>>>> 
>>>> Signed-off-by: Stan Skowronek <stan@corellium.com>
>>>> Signed-off-by: Mohamed Mediouni <mohamed.mediouni@caramail.com>
>>>> ---
>>>> arch/arm64/mm/proc.S | 26 ++++++++++++++++++++++++++
>>>> 1 file changed, 26 insertions(+)
>>>> 
>>>> diff --git a/arch/arm64/mm/proc.S b/arch/arm64/mm/proc.S
>>>> index 1f7ee8c8b7b8..06436916f137 100644
>>>> --- a/arch/arm64/mm/proc.S
>>>> +++ b/arch/arm64/mm/proc.S
>>>> @@ -51,6 +51,25 @@
>>>> #define TCR_KASAN_HW_FLAGS 0
>>>> #endif
>>>> 
>>>> +#ifdef CONFIG_ARCH_APPLE
>>>> +
>>>> +/*
>>>> + * Apple cores appear to black-hole writes done with nGnRE.
>>>> + * We settled on a work-around that uses MAIR vs changing every
>>>> single user of
>>>> + * nGnRE across the arm64 code.
>>>> + */
>>>> +
>>>> +#define MAIR_EL1_SET_APPLE						\
>>>> +	(MAIR_ATTRIDX(MAIR_ATTR_DEVICE_nGnRnE, MT_DEVICE_nGnRnE) |	\
>>>> +	 MAIR_ATTRIDX(MAIR_ATTR_DEVICE_nGnRnE, MT_DEVICE_nGnRE) |	\
>>>> +	 MAIR_ATTRIDX(MAIR_ATTR_DEVICE_GRE, MT_DEVICE_GRE) |		\
>>>> +	 MAIR_ATTRIDX(MAIR_ATTR_NORMAL_NC, MT_NORMAL_NC) |		\
>>>> +	 MAIR_ATTRIDX(MAIR_ATTR_NORMAL, MT_NORMAL) |			\
>>>> +	 MAIR_ATTRIDX(MAIR_ATTR_NORMAL_WT, MT_NORMAL_WT) |		\
>>>> +	 MAIR_ATTRIDX(MAIR_ATTR_NORMAL, MT_NORMAL_TAGGED))
>>>> +
>>>> +#endif
>>>> +
>>>> /*
>>>>  * Default MAIR_EL1. MT_NORMAL_TAGGED is initially mapped as Normal
>>>> memory and
>>>>  * changed during __cpu_setup to Normal Tagged if the system
>>>> supports MTE.
>>>> @@ -432,6 +451,13 @@ SYM_FUNC_START(__cpu_setup)
>>>> 	 * Memory region attributes
>>>> 	 */
>>>> 	mov_q	x5, MAIR_EL1_SET
>>>> +#ifdef CONFIG_ARCH_APPLE
>>>> +	mrs	x0, MIDR_EL1
>>>> +	lsr	w0, w0, #24
>>>> +	mov_q	x1, MAIR_EL1_SET_APPLE
>>>> +	cmp	x0, #0x61			// 0x61 = Implementer: Apple
>>>> +	csel	x5, x1, x5, eq
>>> 
>>> Why does this need to be done so early? It would be a lot cleaner if we
>>> could detect this in a similar fashion to other errata and update the
>>> MAIR
>>> appropriately. If that's not possible because of early IO mappings
>>> (which
>>> ones?), then we could instead initialise to nGnRnE unconditionally, but
>>> relax it to nGnRE if we detect that we _don't_ have the erratum.
>> 
>> Would that imply another round-trip into the idmap, much like we do
>> when we switch to non-global mappings? Or do you expect that we can change
>> the MAIR with live mappings?
> 
> I think we should be able to change it live and then invalidate the TLB. At
> least, my reading of the BBM requirements suggests that it isn't required
> for changing between different types of device memory. I can seek
> clarification from Arm if necessary.
> 
Please ignore that patch.

It turns out that the PCIe controller on Apple M1 expects posted writes and so the memory range for it ought to be set nGnRE. 
So, we need to use nGnRnE for on-chip MMIO and nGnRE for PCIe BARs.

The MAIR approach isn’t adequate for such a thing, so we’ll have to look elsewhere.

Thank you,
> Will


^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: [RFC PATCH 7/7] irqchip/apple-aic: add SMP support to the Apple AIC driver.
  2021-01-21 15:09             ` Arnd Bergmann
@ 2021-01-21 15:18               ` Mohamed Mediouni
  0 siblings, 0 replies; 55+ messages in thread
From: Mohamed Mediouni @ 2021-01-21 15:18 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Marc Zyngier, Mark Rutland, Catalin Marinas, Hector Martin,
	linux-kernel, Will Deacon, Linux ARM, Stan Skowronek



> On 21 Jan 2021, at 16:09, Arnd Bergmann <arnd@kernel.org> wrote:
> 
> On Thu, Jan 21, 2021 at 3:10 PM Marc Zyngier <maz@kernel.org> wrote:
>> On 2021-01-21 13:34, Mohamed Mediouni wrote:
>>>> On 21 Jan 2021, at 14:22, Marc Zyngier <maz@kernel.org> wrote:
>>> Because UART access adapters for the new M1 Macs aren’t plentiful
>>> at all, I actually use this for development, with iPhones which have
>>> much more easy to buy Lightning-to-UART adapters.
>>> 
>>> (That’s why the old implementation is there too)
>>> 
>>> Might be worth splitting the new one to a new commit though...
>> 
>> This series is supposed to cover M1 only, and adding extra support
>> as part of it is only likely to make the code harder to review.
>> 
>> I'd rather you focus on a single IPI interface (fast or slow,
>> I don't really care). Extra features can come in later.
> 
> Agreed. The slow interface is probably easier to start with,
> because it avoids hooking into the FIQ, so the FIQ can be
> completely decoupled from AIC and just used for the timer.
> 
> Maybe there is even a way to use more than one hardware IPI in
> the AIC?
> 
>       Arnd
Hello,

Decided to only have only the slow interface in the second patch series.

Thank you,

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: [RFC PATCH 4/7] irqchip/apple-aic: Add support for Apple AIC
  2021-01-21 10:37     ` Arnd Bergmann
@ 2021-01-21 15:29       ` Hector Martin 'marcan'
  2021-01-21 17:09         ` Rob Herring
  2021-01-21 17:45       ` Rob Herring
  1 sibling, 1 reply; 55+ messages in thread
From: Hector Martin 'marcan' @ 2021-01-21 15:29 UTC (permalink / raw)
  To: Arnd Bergmann, Linus Walleij
  Cc: Mohamed Mediouni, Mark Rutland, Catalin Marinas, linux-kernel,
	Marc Zyngier, Will Deacon, Linux ARM, Stan Skowronek

On 21/01/2021 19.37, Arnd Bergmann wrote:
> On Thu, Jan 21, 2021 at 10:48 AM Linus Walleij <linus.walleij@linaro.org> wrote:
>>
>> However weird it may seem, Apple is not in the file
>> Documentation/devicetree/bindings/vendor-prefixes.yaml
> 
> Since Apple are already using both the "AAPL" and the "apple"
> prefix themselves, I have a bad feeling about reusing either of
> them for defining the devicetree.org bindings that we add to
> linux/Documentation/devicetree/bindings. The question is: if
> not "apple", what else should we use here?

This ties into the larger question of how we should handle devicetrees 
in general on these platforms.

The two extremes are:

1) Have the bootloader outright convert ADT to FDT and make Linux 
support the entirety of Apple's devicetree structure, or

2) Maintain our own devicetrees and ignore Apple's entirely

My feeling is that 1) is a non-starter, because Linux ARM device trees 
and Apple ARM device trees have seen independent evolution from the 
PowerPC era, and many details are completely different. Plus conversion 
is non-trivial, because the endianness is different and the format is 
too ambiguous to do programmatically without complex logic.

On the other hand, cranking out devicetrees by hand for every device 
variant that Apple puts out is a waste of time.

Obviously at the bare minimum the bootloader will need to move some 
dynamic information from the ADT to the FDT, but that can be a very 
specific set of properties (memory layout, MAC addresses, etc).

My current thinking is that we should write offline, automated tooling 
to parse, diff, and automatically convert portions of Apple devicetrees 
into Linux ones. Then we can more easily maintain our own, but still 
ultimately have humans decide what goes into the Linux device trees.

It's worth noting that AIUI Apple does not consider their devicetree 
layout to be stable, and it may change any time. On M1 devices, the 
devicetree is provided as part of the iBoot2 firmware bundle, which 
means it changes from one macOS version to the next (this is paired with 
the Darwin kernel itself, and they are upgraded as a unit). It includes 
placeholder values that iBoot2 then replaces with data from NOR before 
handing control over to the kernel. My goal for our long-term project 
[1] is to keep up with iBoot2 updates so that we do not have to instruct 
users to dig up old macOS versions.

Quick TL;DR on how these things boot:
- Boot ROM boots
- iBoot1 (system firmware) in NOR flash which looks for a bootable OS in 
internal storage (only!) in the form of an APFS container+volume and 
then boots
- iBoot2 (OS loader) which loads a bunch of firmware blobs and the 
devicetree off of storage, customizes it with system data from NOR, and 
then loads a wrapped mach-o file containing
- A Darwin kernel, or in our case a Linux bootloader which then boots
- A standard arm64 Linux blob

The boot ROM is ROM. iBoot1 only ever rolls forward (downgrades 
impossible). iBoot2 downgrades are possible but Apple already proved 
they can break this willingly or not, at least in betas (macOS 11.2 
Beta2 iBoot1 cannot boot Beta1 iBoot2). The secureboot chain goes all 
the way up to the mach-o kernel load, that is the first point where we 
can change boot policy to load anything we want (with user consent).

[1] https://asahilinux.org/

-- 
Hector Martin "marcan" (marcan@marcan.st)
Public Key: https://mrcn.st/pub

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: [RFC PATCH 3/7] arm64: mm: use nGnRnE instead of nGnRE on Apple processors
  2021-01-21 15:12         ` Mohamed Mediouni
@ 2021-01-21 16:25           ` Marc Zyngier
  2021-01-21 17:55             ` Will Deacon
  0 siblings, 1 reply; 55+ messages in thread
From: Marc Zyngier @ 2021-01-21 16:25 UTC (permalink / raw)
  To: Mohamed Mediouni
  Cc: Will Deacon, Linux ARM, Catalin Marinas, Mark Rutland,
	Hector Martin, linux-kernel, Stan Skowronek

On 2021-01-21 15:12, Mohamed Mediouni wrote:
> Please ignore that patch.
> 
> It turns out that the PCIe controller on Apple M1 expects posted
> writes and so the memory range for it ought to be set nGnRE.
> So, we need to use nGnRnE for on-chip MMIO and nGnRE for PCIe BARs.
> 
> The MAIR approach isn’t adequate for such a thing, so we’ll have to
> look elsewhere.

Well, there isn't many alternative to having a memory type defined
in MAIR if you want to access your PCIe devices with specific
semantics.

It probably means defining a memory type for PCI only, but:
- we only have a single free MT entry, and I'm not sure we can
   afford to waste this on a specific platform (can we re-purpose
   GRE instead?),
- we'd need to teach the PCI code to use this...

Thanks,

         M.
-- 
Jazz is not dead. It just smells funny...

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: [RFC PATCH 7/7] irqchip/apple-aic: add SMP support to the Apple AIC driver.
  2021-01-21 12:50     ` Mohamed Mediouni
  2021-01-21 13:00       ` Arnd Bergmann
  2021-01-21 13:22       ` Marc Zyngier
@ 2021-01-21 16:40       ` Rob Herring
  2021-01-21 16:43         ` Mohamed Mediouni
  2 siblings, 1 reply; 55+ messages in thread
From: Rob Herring @ 2021-01-21 16:40 UTC (permalink / raw)
  To: Mohamed Mediouni
  Cc: Arnd Bergmann, Mark Rutland, Catalin Marinas, Hector Martin,
	linux-kernel, Marc Zyngier, Will Deacon, Linux ARM,
	Stan Skowronek

On Thu, Jan 21, 2021 at 6:52 AM Mohamed Mediouni
<mohamed.mediouni@caramail.com> wrote:
>
>
>
> > On 21 Jan 2021, at 13:44, Arnd Bergmann <arnd@kernel.org> wrote:
> >
> > On Wed, Jan 20, 2021 at 2:27 PM Mohamed Mediouni
> > <mohamed.mediouni@caramail.com> wrote:
> >
> >> +#ifdef CONFIG_SMP
> >> +static void apple_aic_ipi_send_mask(struct irq_data *d,
> >> +                                   const struct cpumask *mask)
> >
> > Not sure we care about the #ifdef here, given that arch/arm64 does not
> > allow building a kernel without CONFIG_SMP.
> >
> >> +       /*
> >> +     * Ensure that stores to Normal memory are visible to the
> >> +     * other CPUs before issuing the IPI.
> >> +     */
> >> +       wmb();
> >> +
> >> +       for_each_cpu (cpu, mask) {
> >> +               smp_mb__before_atomic();
> >> +               atomic_or(1u << irqnr, per_cpu_ptr(&aic_ipi_mask, cpu));
> >> +               smp_mb__after_atomic();
> >> +               lcpu = get_cpu();
> >> +               if (aic.fast_ipi) {
> >> +                       if ((lcpu >> 2) == (cpu >> 2))
> >> +                               write_sysreg(cpu & 3, SR_APPLE_IPI_LOCAL);
> >> +                       else
> >> +                               write_sysreg((cpu & 3) | ((cpu >> 2) << 16),
> >> +                                            SR_APPLE_IPI_REMOTE);
> >> +               } else
> >> +                       writel(lcpu == cpu ? REG_IPI_FLAG_SELF :
> >> +                                                  (REG_IPI_FLAG_OTHER << cpu),
> >> +                              aic.base + REG_IPI_SET);
> >> +               put_cpu();
> >> +       }
> >> +
> >> +       /* Force the above writes to be executed */
> >> +       if (aic.fast_ipi)
> >> +               isb();
> >> +}
> >
> > Since this just loops over all CPUs, I'd probably just turn it into
> > an ipi_send_single() callback and have the caller do the
> > loop for simplicity.
> >
> > I also have the feeling that splitting one hardware IPI into multiple
> > logical interrupts, which are then all registered by the same irq
> > handler adds a little more complexity than necessary.
> >
> > Changing this would of course require modifications to
> > arch/arm64/kernel/smp.c, which is hardwired to use
> > CONFIG_GENERIC_IRQ_IPI in smp_cross_call(), and allowing
> > a different code path there may be worse than emulating an
> > irqchip.
> >
> >> @@ -186,8 +325,11 @@ static int __init apple_aic_init(struct device_node *node,
> >>       if (WARN(!aic.base, "unable to map aic registers\n"))
> >>               return -EINVAL;
> >>
> >> +       aic.fast_ipi = of_property_read_bool(node, "fast-ipi");
> >
> > Where is this property documented, and what decides which one to use?
> It’s getting documented in the next patch set.
>
> This property is there to enable support for older iPhone processors
> later on, some of which do not have fast IPI support.
>
> On Apple M1, fast-ipi is always on.

This should be implied by the compatible string which needs to be more
specific and include the SoC name.

Rob

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: [RFC PATCH 7/7] irqchip/apple-aic: add SMP support to the Apple AIC driver.
  2021-01-21 16:40       ` Rob Herring
@ 2021-01-21 16:43         ` Mohamed Mediouni
  2021-01-21 17:37           ` Rob Herring
  0 siblings, 1 reply; 55+ messages in thread
From: Mohamed Mediouni @ 2021-01-21 16:43 UTC (permalink / raw)
  To: Rob Herring
  Cc: Arnd Bergmann, Mark Rutland, Catalin Marinas, Hector Martin,
	linux-kernel, Marc Zyngier, Will Deacon, Linux ARM,
	Stan Skowronek



> On 21 Jan 2021, at 17:40, Rob Herring <robh@kernel.org> wrote:
> 
> On Thu, Jan 21, 2021 at 6:52 AM Mohamed Mediouni
> <mohamed.mediouni@caramail.com> wrote:
>> 
>> 
>> 
>>> On 21 Jan 2021, at 13:44, Arnd Bergmann <arnd@kernel.org> wrote:
>>> 
>>> On Wed, Jan 20, 2021 at 2:27 PM Mohamed Mediouni
>>> <mohamed.mediouni@caramail.com> wrote:
>>> 
>>>> +#ifdef CONFIG_SMP
>>>> +static void apple_aic_ipi_send_mask(struct irq_data *d,
>>>> +                                   const struct cpumask *mask)
>>> 
>>> Not sure we care about the #ifdef here, given that arch/arm64 does not
>>> allow building a kernel without CONFIG_SMP.
>>> 
>>>> +       /*
>>>> +     * Ensure that stores to Normal memory are visible to the
>>>> +     * other CPUs before issuing the IPI.
>>>> +     */
>>>> +       wmb();
>>>> +
>>>> +       for_each_cpu (cpu, mask) {
>>>> +               smp_mb__before_atomic();
>>>> +               atomic_or(1u << irqnr, per_cpu_ptr(&aic_ipi_mask, cpu));
>>>> +               smp_mb__after_atomic();
>>>> +               lcpu = get_cpu();
>>>> +               if (aic.fast_ipi) {
>>>> +                       if ((lcpu >> 2) == (cpu >> 2))
>>>> +                               write_sysreg(cpu & 3, SR_APPLE_IPI_LOCAL);
>>>> +                       else
>>>> +                               write_sysreg((cpu & 3) | ((cpu >> 2) << 16),
>>>> +                                            SR_APPLE_IPI_REMOTE);
>>>> +               } else
>>>> +                       writel(lcpu == cpu ? REG_IPI_FLAG_SELF :
>>>> +                                                  (REG_IPI_FLAG_OTHER << cpu),
>>>> +                              aic.base + REG_IPI_SET);
>>>> +               put_cpu();
>>>> +       }
>>>> +
>>>> +       /* Force the above writes to be executed */
>>>> +       if (aic.fast_ipi)
>>>> +               isb();
>>>> +}
>>> 
>>> Since this just loops over all CPUs, I'd probably just turn it into
>>> an ipi_send_single() callback and have the caller do the
>>> loop for simplicity.
>>> 
>>> I also have the feeling that splitting one hardware IPI into multiple
>>> logical interrupts, which are then all registered by the same irq
>>> handler adds a little more complexity than necessary.
>>> 
>>> Changing this would of course require modifications to
>>> arch/arm64/kernel/smp.c, which is hardwired to use
>>> CONFIG_GENERIC_IRQ_IPI in smp_cross_call(), and allowing
>>> a different code path there may be worse than emulating an
>>> irqchip.
>>> 
>>>> @@ -186,8 +325,11 @@ static int __init apple_aic_init(struct device_node *node,
>>>>      if (WARN(!aic.base, "unable to map aic registers\n"))
>>>>              return -EINVAL;
>>>> 
>>>> +       aic.fast_ipi = of_property_read_bool(node, "fast-ipi");
>>> 
>>> Where is this property documented, and what decides which one to use?
>> It’s getting documented in the next patch set.
>> 
>> This property is there to enable support for older iPhone processors
>> later on, some of which do not have fast IPI support.
>> 
>> On Apple M1, fast-ipi is always on.
> 
> This should be implied by the compatible string which needs to be more
> specific and include the SoC name.
> 
> Rob

Then we’ll eventually have two aic compatible strings, aic which is compatible
with Apple A7 onwards and aicv2 which is a superset with fast IPI (introduced 
on the Apple A11, 3 years ago, with no further programmer-visible changes since 
then).

Does that look right?

Thank you, 


^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: [RFC PATCH 4/7] irqchip/apple-aic: Add support for Apple AIC
  2021-01-21  9:48   ` Linus Walleij
  2021-01-21 10:37     ` Arnd Bergmann
@ 2021-01-21 16:44     ` Rob Herring
  1 sibling, 0 replies; 55+ messages in thread
From: Rob Herring @ 2021-01-21 16:44 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Mohamed Mediouni, Mark Rutland, Catalin Marinas, Hector Martin,
	linux-kernel, Marc Zyngier, Will Deacon, Linux ARM,
	Stan Skowronek

On Thu, Jan 21, 2021 at 3:49 AM Linus Walleij <linus.walleij@linaro.org> wrote:
>
> Hi Mohamed,
>
> thanks for your patch!
>
> On Wed, Jan 20, 2021 at 2:31 PM Mohamed Mediouni
> <mohamed.mediouni@caramail.com> wrote:
>
> > +properties:
> > +  compatible:
> > +    items:
> > +      - const: apple,aic

As mentioned in patch 7, this needs to be SoC specific.

Also, bindings should be separate patch. checkpatch.pl will tell you this.

> However weird it may seem, Apple is not in the file
> Documentation/devicetree/bindings/vendor-prefixes.yaml
>
> (I think it's weird because I remember clearly that they have been
> using device tree for PPC since ages.)

That's because the vendor prefix is 'AAPL' which is the stock ticker
and it predates documenting anything.

> Could you add this 2-liner to that file and send it directly to
> DT binding maintainers as a single patch as a preparation?

So this is still needed. Happy to take that given already in use.

Rob

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: [RFC PATCH 4/7] irqchip/apple-aic: Add support for Apple AIC
  2021-01-20 13:27 ` [RFC PATCH 4/7] irqchip/apple-aic: Add support for Apple AIC Mohamed Mediouni
                     ` (2 preceding siblings ...)
  2021-01-21  9:48   ` Linus Walleij
@ 2021-01-21 16:53   ` Hector Martin 'marcan'
  3 siblings, 0 replies; 55+ messages in thread
From: Hector Martin 'marcan' @ 2021-01-21 16:53 UTC (permalink / raw)
  To: Mohamed Mediouni, linux-arm-kernel
  Cc: Catalin Marinas, Will Deacon, Mark Rutland, Marc Zyngier,
	linux-kernel, Stan Skowronek

On 20/01/2021 22.27, Mohamed Mediouni wrote:
> +		irq_domain_set_info(d, virq, hw, &apple_aic_irq_chip,
> +				    d->host_data, handle_level_irq, NULL, NULL);

The AIC automatically masks IRQs on reason fetch, which means the 
handle_level_irq flow is redundant. Using the fasteoi flow, as in [1], 
should be more efficient.

[1] https://github.com/AsahiLinux/linux/commit/d4cb18c93

-- 
Hector Martin "marcan" (marcan@marcan.st)
Public Key: https://mrcn.st/pub

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: [RFC PATCH 4/7] irqchip/apple-aic: Add support for Apple AIC
  2021-01-21 15:29       ` Hector Martin 'marcan'
@ 2021-01-21 17:09         ` Rob Herring
  0 siblings, 0 replies; 55+ messages in thread
From: Rob Herring @ 2021-01-21 17:09 UTC (permalink / raw)
  To: Hector Martin 'marcan'
  Cc: Arnd Bergmann, Linus Walleij, Mark Rutland, Marc Zyngier,
	linux-kernel, Catalin Marinas, Mohamed Mediouni, Will Deacon,
	Linux ARM, Stan Skowronek

On Thu, Jan 21, 2021 at 9:31 AM Hector Martin 'marcan' <marcan@marcan.st> wrote:
>
> On 21/01/2021 19.37, Arnd Bergmann wrote:
> > On Thu, Jan 21, 2021 at 10:48 AM Linus Walleij <linus.walleij@linaro.org> wrote:
> >>
> >> However weird it may seem, Apple is not in the file
> >> Documentation/devicetree/bindings/vendor-prefixes.yaml
> >
> > Since Apple are already using both the "AAPL" and the "apple"
> > prefix themselves, I have a bad feeling about reusing either of
> > them for defining the devicetree.org bindings that we add to
> > linux/Documentation/devicetree/bindings. The question is: if
> > not "apple", what else should we use here?
>
> This ties into the larger question of how we should handle devicetrees
> in general on these platforms.
>
> The two extremes are:
>
> 1) Have the bootloader outright convert ADT to FDT and make Linux
> support the entirety of Apple's devicetree structure, or
>
> 2) Maintain our own devicetrees and ignore Apple's entirely
>
> My feeling is that 1) is a non-starter, because Linux ARM device trees
> and Apple ARM device trees have seen independent evolution from the
> PowerPC era, and many details are completely different. Plus conversion
> is non-trivial, because the endianness is different and the format is
> too ambiguous to do programmatically without complex logic.

You are right it's a non-starter. Apple's DT even from PowerPC days
were weird and the hardware was much simpler then. Given we're still
maintaining that code I don't care to add what they've evolved on
their own over the last 15 years and support it for the next 20+ years
(given folks notice when we break 1998 era Macs).

> On the other hand, cranking out devicetrees by hand for every device
> variant that Apple puts out is a waste of time.
>
> Obviously at the bare minimum the bootloader will need to move some
> dynamic information from the ADT to the FDT, but that can be a very
> specific set of properties (memory layout, MAC addresses, etc).
>
> My current thinking is that we should write offline, automated tooling
> to parse, diff, and automatically convert portions of Apple devicetrees
> into Linux ones. Then we can more easily maintain our own, but still
> ultimately have humans decide what goes into the Linux device trees.

Seems reasonable.

> It's worth noting that AIUI Apple does not consider their devicetree
> layout to be stable, and it may change any time.

Yeah, also not something we want to support.

> On M1 devices, the
> devicetree is provided as part of the iBoot2 firmware bundle, which
> means it changes from one macOS version to the next (this is paired with
> the Darwin kernel itself, and they are upgraded as a unit). It includes
> placeholder values that iBoot2 then replaces with data from NOR before
> handing control over to the kernel. My goal for our long-term project
> [1] is to keep up with iBoot2 updates so that we do not have to instruct
> users to dig up old macOS versions.
>
> Quick TL;DR on how these things boot:
> - Boot ROM boots
> - iBoot1 (system firmware) in NOR flash which looks for a bootable OS in
> internal storage (only!) in the form of an APFS container+volume and
> then boots
> - iBoot2 (OS loader) which loads a bunch of firmware blobs and the
> devicetree off of storage, customizes it with system data from NOR, and
> then loads a wrapped mach-o file containing
> - A Darwin kernel, or in our case a Linux bootloader which then boots
> - A standard arm64 Linux blob
>
> The boot ROM is ROM. iBoot1 only ever rolls forward (downgrades
> impossible). iBoot2 downgrades are possible but Apple already proved
> they can break this willingly or not, at least in betas (macOS 11.2
> Beta2 iBoot1 cannot boot Beta1 iBoot2). The secureboot chain goes all
> the way up to the mach-o kernel load, that is the first point where we
> can change boot policy to load anything we want (with user consent).
>
> [1] https://asahilinux.org/
>
> --
> Hector Martin "marcan" (marcan@marcan.st)
> Public Key: https://mrcn.st/pub
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: [RFC PATCH 7/7] irqchip/apple-aic: add SMP support to the Apple AIC driver.
  2021-01-21 16:43         ` Mohamed Mediouni
@ 2021-01-21 17:37           ` Rob Herring
  2021-01-21 18:08             ` Mohamed Mediouni
  0 siblings, 1 reply; 55+ messages in thread
From: Rob Herring @ 2021-01-21 17:37 UTC (permalink / raw)
  To: Mohamed Mediouni
  Cc: Arnd Bergmann, Mark Rutland, Catalin Marinas, Hector Martin,
	linux-kernel, Marc Zyngier, Will Deacon, Linux ARM,
	Stan Skowronek

On Thu, Jan 21, 2021 at 10:43 AM Mohamed Mediouni
<mohamed.mediouni@caramail.com> wrote:
> > On 21 Jan 2021, at 17:40, Rob Herring <robh@kernel.org> wrote:
> > On Thu, Jan 21, 2021 at 6:52 AM Mohamed Mediouni
> > <mohamed.mediouni@caramail.com> wrote:
> >>> On 21 Jan 2021, at 13:44, Arnd Bergmann <arnd@kernel.org> wrote:
> >>> On Wed, Jan 20, 2021 at 2:27 PM Mohamed Mediouni
> >>> <mohamed.mediouni@caramail.com> wrote:

[...]

> >>>> @@ -186,8 +325,11 @@ static int __init apple_aic_init(struct device_node *node,
> >>>>      if (WARN(!aic.base, "unable to map aic registers\n"))
> >>>>              return -EINVAL;
> >>>>
> >>>> +       aic.fast_ipi = of_property_read_bool(node, "fast-ipi");
> >>>
> >>> Where is this property documented, and what decides which one to use?
> >> It’s getting documented in the next patch set.
> >>
> >> This property is there to enable support for older iPhone processors
> >> later on, some of which do not have fast IPI support.
> >>
> >> On Apple M1, fast-ipi is always on.
> >
> > This should be implied by the compatible string which needs to be more
> > specific and include the SoC name.
> >
> > Rob
>
> Then we’ll eventually have two aic compatible strings, aic which is compatible
> with Apple A7 onwards and aicv2 which is a superset with fast IPI (introduced
> on the Apple A11, 3 years ago, with no further programmer-visible changes since
> then).
>
> Does that look right?

If we did this from the start, it would evolve like this:

A7: "AAPL,a7-aic"
A8: "AAPL,a8-aic", "AAPL,a7-aic"  # Read this as A8 AIC is backwards
compatible with A7 AIC
A9: "AAPL,a9-aic", "AAPL,a7-aic"

A11: "AAPL,a11-aic", "AAPL,a7-aic"

If the A11 version could work on an OS that only supported the
original model (sounds like this is the case) Or if it's not backwards
compatible:

A11: "AAPL,a11-aic"

If the A11 is different and not backwards compatible.

Then M1 could be:

M1: "AAPL,m1-aic", "AAPL,a11-aic"

Or to even support an OS with only v1 support:

M1: "AAPL,m1-aic", "AAPL,a11-aic", "AAPL,a7-aic"

You don't really need the fallback here because there isn't any
existing OS support and the baseline is the M1.

If you want to have generic fallback compatible strings with versions,
that's fine too. I'm not really a fan of version numbers that are just
made up by the binding author though. Most SoC vendors don't have
rigorous versioning of their IP and those that do seem to have a new
version on every SoC.

The important part is *always* having an SoC specific compatible so
you can deal with any quirk or feature without having to change the
DTB. Everyone says blocks are 'the same' until they aren't.

Rob

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: [RFC PATCH 4/7] irqchip/apple-aic: Add support for Apple AIC
  2021-01-21 10:37     ` Arnd Bergmann
  2021-01-21 15:29       ` Hector Martin 'marcan'
@ 2021-01-21 17:45       ` Rob Herring
  1 sibling, 0 replies; 55+ messages in thread
From: Rob Herring @ 2021-01-21 17:45 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Linus Walleij, Mark Rutland, Marc Zyngier, Hector Martin,
	linux-kernel, Catalin Marinas, Mohamed Mediouni, Will Deacon,
	Linux ARM, Stan Skowronek

On Thu, Jan 21, 2021 at 4:38 AM Arnd Bergmann <arnd@kernel.org> wrote:
>
> On Thu, Jan 21, 2021 at 10:48 AM Linus Walleij <linus.walleij@linaro.org> wrote:
> >
> > Hi Mohamed,
> >
> > thanks for your patch!
> >
> > On Wed, Jan 20, 2021 at 2:31 PM Mohamed Mediouni
> > <mohamed.mediouni@caramail.com> wrote:
> >
> > > +properties:
> > > +  compatible:
> > > +    items:
> > > +      - const: apple,aic
> >
> > However weird it may seem, Apple is not in the file
> > Documentation/devicetree/bindings/vendor-prefixes.yaml
> >
> > (I think it's weird because I remember clearly that they have been
> > using device tree for PPC since ages.)
> >
> > Could you add this 2-liner to that file and send it directly to
> > DT binding maintainers as a single patch as a preparation?
>
> Choosing the vendor prefix here is going to be a little tricky
> and non-obvious.
>
> Background:
>
> Traditionally, it should have been the stock ticker symbol of the
> company (clearly only publicly traded companies would be able
> to produce a Unix capable computer, right?), but there were
> already inconsistent: IBM used "ibm" (in small letters), Sun
> used "SUNW" (in capitals) but in 2007 changed the stock ticker
> symbol to "JAVA", obviously without changing the firmware bindings.
>
> Apple traditionally used "AAPL" (also in caps) in the device tree,
> and there is one remnant of that in the M1 device tree, in the form
> of the "AAPL,phandle" property in each node, which corresponds
> to our "linux,phandle". (phandles were introduced as properties in
> both of the flattened DT formats, .dtb and apple's own format).
> There are also "AAPL,unit-string properties and some device_type
> strings (AAPL,display-crossbar, AAPL,atc-dpxbar, ...) in the M1 DT,
> and the CPU nodes (and only those) use "apple" in small letters
> as in "apple,icestorm","ARM,v8". The other nodes tend to not have
> a vendor prefix, but a lot use a subsystem name as the prefix, such
> as compatible="gpio,t8101" or compatible="tempsensor,t8020".
>
> Since Apple are already using both the "AAPL" and the "apple"
> prefix themselves, I have a bad feeling about reusing either of
> them for defining the devicetree.org bindings that we add to
> linux/Documentation/devicetree/bindings. The question is: if
> not "apple", what else should we use here?

IMO, 'AAPL' as that is what is already in use in the kernel. I don't
see why it matters at all what Apple is using. It might if we used any
of the ADT as-is, but I don't see that happening from what I've seen.

Rob

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: [RFC PATCH 3/7] arm64: mm: use nGnRnE instead of nGnRE on Apple processors
  2021-01-21 16:25           ` Marc Zyngier
@ 2021-01-21 17:55             ` Will Deacon
  2021-01-21 18:15               ` Marc Zyngier
  0 siblings, 1 reply; 55+ messages in thread
From: Will Deacon @ 2021-01-21 17:55 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: Mohamed Mediouni, Linux ARM, Catalin Marinas, Mark Rutland,
	Hector Martin, linux-kernel, Stan Skowronek

On Thu, Jan 21, 2021 at 04:25:54PM +0000, Marc Zyngier wrote:
> On 2021-01-21 15:12, Mohamed Mediouni wrote:
> > Please ignore that patch.
> > 
> > It turns out that the PCIe controller on Apple M1 expects posted
> > writes and so the memory range for it ought to be set nGnRE.
> > So, we need to use nGnRnE for on-chip MMIO and nGnRE for PCIe BARs.
> > 
> > The MAIR approach isn’t adequate for such a thing, so we’ll have to
> > look elsewhere.
> 
> Well, there isn't many alternative to having a memory type defined
> in MAIR if you want to access your PCIe devices with specific
> semantics.
> 
> It probably means defining a memory type for PCI only, but:
> - we only have a single free MT entry, and I'm not sure we can
>   afford to waste this on a specific platform (can we re-purpose
>   GRE instead?),

We already have an nGnRnE MAIR for config space accesses.

Will

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: [RFC PATCH 7/7] irqchip/apple-aic: add SMP support to the Apple AIC driver.
  2021-01-21 17:37           ` Rob Herring
@ 2021-01-21 18:08             ` Mohamed Mediouni
  2021-01-21 18:57               ` Rob Herring
  0 siblings, 1 reply; 55+ messages in thread
From: Mohamed Mediouni @ 2021-01-21 18:08 UTC (permalink / raw)
  To: Rob Herring
  Cc: Arnd Bergmann, Mark Rutland, Catalin Marinas, Hector Martin,
	linux-kernel, Marc Zyngier, Will Deacon, Linux ARM,
	Stan Skowronek



> On 21 Jan 2021, at 18:37, Rob Herring <robh@kernel.org> wrote:
> 
> On Thu, Jan 21, 2021 at 10:43 AM Mohamed Mediouni
> <mohamed.mediouni@caramail.com> wrote:
>>> On 21 Jan 2021, at 17:40, Rob Herring <robh@kernel.org> wrote:
>>> On Thu, Jan 21, 2021 at 6:52 AM Mohamed Mediouni
>>> <mohamed.mediouni@caramail.com> wrote:
>>>>> On 21 Jan 2021, at 13:44, Arnd Bergmann <arnd@kernel.org> wrote:
>>>>> On Wed, Jan 20, 2021 at 2:27 PM Mohamed Mediouni
>>>>> <mohamed.mediouni@caramail.com> wrote:
> 
> [...]
> 
>>>>>> @@ -186,8 +325,11 @@ static int __init apple_aic_init(struct device_node *node,
>>>>>>     if (WARN(!aic.base, "unable to map aic registers\n"))
>>>>>>             return -EINVAL;
>>>>>> 
>>>>>> +       aic.fast_ipi = of_property_read_bool(node, "fast-ipi");
>>>>> 
>>>>> Where is this property documented, and what decides which one to use?
>>>> It’s getting documented in the next patch set.
>>>> 
>>>> This property is there to enable support for older iPhone processors
>>>> later on, some of which do not have fast IPI support.
>>>> 
>>>> On Apple M1, fast-ipi is always on.
>>> 
>>> This should be implied by the compatible string which needs to be more
>>> specific and include the SoC name.
>>> 
>>> Rob
>> 
>> Then we’ll eventually have two aic compatible strings, aic which is compatible
>> with Apple A7 onwards and aicv2 which is a superset with fast IPI (introduced
>> on the Apple A11, 3 years ago, with no further programmer-visible changes since
>> then).
>> 
>> Does that look right?
> 
> If we did this from the start, it would evolve like this:
> 
> A7: "AAPL,a7-aic"
> A8: "AAPL,a8-aic", "AAPL,a7-aic"  # Read this as A8 AIC is backwards
> compatible with A7 AIC
> A9: "AAPL,a9-aic", "AAPL,a7-aic"
> 
> A11: "AAPL,a11-aic", "AAPL,a7-aic"
> 
> If the A11 version could work on an OS that only supported the
> original model (sounds like this is the case) Or if it's not backwards
> compatible:
> 

The A11 AIC indeed can be used by older drivers that aren’t aware
of the fast IPI path introduced on A11 just fine.

> A11: "AAPL,a11-aic"
> 
> If the A11 is different and not backwards compatible.
> 
> Then M1 could be:
> 
> M1: "AAPL,m1-aic", "AAPL,a11-aic"
> 
> Or to even support an OS with only v1 support:
> 
> M1: "AAPL,m1-aic", "AAPL,a11-aic", "AAPL,a7-aic"
> 
> You don't really need the fallback here because there isn't any
> existing OS support and the baseline is the M1.
> 
> If you want to have generic fallback compatible strings with versions,
> that's fine too. I'm not really a fan of version numbers that are just
> made up by the binding author though. Most SoC vendors don't have
> rigorous versioning of their IP and those that do seem to have a new
> version on every SoC.
> 
> The important part is *always* having an SoC specific compatible so
> you can deal with any quirk or feature without having to change the
> DTB. Everyone says blocks are 'the same' until they aren’t.
> 
Is it fine if such a SoC-specific compatible is present but with having
the driver only know about AAPL,a11-aic for example?
(To just have it when it’d be needed if ever in the future, but not uselessly
add entries to the driver that will not be currently used)

On a tangent:

The internal naming scheme used by Apple is off-by-one:

Apple A14 for example is Apple H13P (H-series 13th gen processor, Phone)
Apple M1 is Apple H13G (H-series 13th gen, G series)
(And Apple A12X is Apple H11G for example, with A12 being H11P)

Should we bother with those or use the marketing names? Especially because
the beefier SoCs might not be of the H series anyway… as the internal scheme
reveals that M1 could as well have been an A14X.

And there’s also the other internal naming scheme:
Apple A12 being t8020, Apple A12X being t8027
Apple A14 being t8101
Apple M1 being t8103

T there means the foundry at which the chip was manufactured, in the cases above TSMC.

Of course Apple itself uses both… with the marketing name being nowhere in their device
trees.

Thank you,

> Rob


^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: [RFC PATCH 3/7] arm64: mm: use nGnRnE instead of nGnRE on Apple processors
  2021-01-21 17:55             ` Will Deacon
@ 2021-01-21 18:15               ` Marc Zyngier
  2021-01-21 18:22                 ` Mohamed Mediouni
  2021-01-21 18:22                 ` Will Deacon
  0 siblings, 2 replies; 55+ messages in thread
From: Marc Zyngier @ 2021-01-21 18:15 UTC (permalink / raw)
  To: Will Deacon
  Cc: Mohamed Mediouni, Linux ARM, Catalin Marinas, Mark Rutland,
	Hector Martin, linux-kernel, Stan Skowronek

On 2021-01-21 17:55, Will Deacon wrote:
> On Thu, Jan 21, 2021 at 04:25:54PM +0000, Marc Zyngier wrote:
>> On 2021-01-21 15:12, Mohamed Mediouni wrote:
>> > Please ignore that patch.
>> >
>> > It turns out that the PCIe controller on Apple M1 expects posted
>> > writes and so the memory range for it ought to be set nGnRE.
>> > So, we need to use nGnRnE for on-chip MMIO and nGnRE for PCIe BARs.
>> >
>> > The MAIR approach isn’t adequate for such a thing, so we’ll have to
>> > look elsewhere.
>> 
>> Well, there isn't many alternative to having a memory type defined
>> in MAIR if you want to access your PCIe devices with specific
>> semantics.
>> 
>> It probably means defining a memory type for PCI only, but:
>> - we only have a single free MT entry, and I'm not sure we can
>>   afford to waste this on a specific platform (can we re-purpose
>>   GRE instead?),
> 
> We already have an nGnRnE MAIR for config space accesses.

I'm confused. If M1 needs nGnRE for PCI, and overrides nGnRE to nE
for its in-SoC accesses, where does nGnRE goes?

Or do you propose that it is the page tables that get a different
MT index?

         M.
-- 
Jazz is not dead. It just smells funny...

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: [RFC PATCH 3/7] arm64: mm: use nGnRnE instead of nGnRE on Apple processors
  2021-01-21 18:15               ` Marc Zyngier
@ 2021-01-21 18:22                 ` Mohamed Mediouni
  2021-01-21 18:22                 ` Will Deacon
  1 sibling, 0 replies; 55+ messages in thread
From: Mohamed Mediouni @ 2021-01-21 18:22 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: Will Deacon, Linux ARM, Catalin Marinas, Mark Rutland,
	Hector Martin, linux-kernel, Stan Skowronek


> On 21 Jan 2021, at 19:15, Marc Zyngier <maz@kernel.org> wrote:
> 
> On 2021-01-21 17:55, Will Deacon wrote:
>> On Thu, Jan 21, 2021 at 04:25:54PM +0000, Marc Zyngier wrote:
>>> On 2021-01-21 15:12, Mohamed Mediouni wrote:
>>>> Please ignore that patch.
>>>> 
>>>> It turns out that the PCIe controller on Apple M1 expects posted
>>>> writes and so the memory range for it ought to be set nGnRE.
>>>> So, we need to use nGnRnE for on-chip MMIO and nGnRE for PCIe BARs.
>>>> 
>>>> The MAIR approach isn’t adequate for such a thing, so we’ll have to
>>>> look elsewhere.
>>> Well, there isn't many alternative to having a memory type defined
>>> in MAIR if you want to access your PCIe devices with specific
>>> semantics.
>>> It probably means defining a memory type for PCI only, but:
>>> - we only have a single free MT entry, and I'm not sure we can
>>> afford to waste this on a specific platform (can we re-purpose
>>> GRE instead?),
>> We already have an nGnRnE MAIR for config space accesses.
> 
> I'm confused. If M1 needs nGnRE for PCI, and overrides nGnRE to nE
> for its in-SoC accesses, where does nGnRE goes?
> 
> Or do you propose that it is the page tables that get a different
> MT index?
> 

That MAIR patch that I added overrides nGnRE accesses to nGnRnE.

Linux tries to access to those SoC devices using nGnRE as the device
memory type without that workaround.

Maybe have a device tree property to override the used device memory type
for a given device on the SoC? Or that’s too big for what’s at the end just one 
particular set of SoCs?

But what the hardware wants is accesses to in-SoC devices being nGnRnE
and access to the PCIe BARs being nGnRE.

So both have to be supported…

>       M.
> -- 
> Jazz is not dead. It just smells funny...


^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: [RFC PATCH 3/7] arm64: mm: use nGnRnE instead of nGnRE on Apple processors
  2021-01-21 18:15               ` Marc Zyngier
  2021-01-21 18:22                 ` Mohamed Mediouni
@ 2021-01-21 18:22                 ` Will Deacon
  1 sibling, 0 replies; 55+ messages in thread
From: Will Deacon @ 2021-01-21 18:22 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: Mohamed Mediouni, Linux ARM, Catalin Marinas, Mark Rutland,
	Hector Martin, linux-kernel, Stan Skowronek

On Thu, Jan 21, 2021 at 06:15:06PM +0000, Marc Zyngier wrote:
> On 2021-01-21 17:55, Will Deacon wrote:
> > On Thu, Jan 21, 2021 at 04:25:54PM +0000, Marc Zyngier wrote:
> > > On 2021-01-21 15:12, Mohamed Mediouni wrote:
> > > > Please ignore that patch.
> > > >
> > > > It turns out that the PCIe controller on Apple M1 expects posted
> > > > writes and so the memory range for it ought to be set nGnRE.
> > > > So, we need to use nGnRnE for on-chip MMIO and nGnRE for PCIe BARs.
> > > >
> > > > The MAIR approach isn’t adequate for such a thing, so we’ll have to
> > > > look elsewhere.
> > > 
> > > Well, there isn't many alternative to having a memory type defined
> > > in MAIR if you want to access your PCIe devices with specific
> > > semantics.
> > > 
> > > It probably means defining a memory type for PCI only, but:
> > > - we only have a single free MT entry, and I'm not sure we can
> > >   afford to waste this on a specific platform (can we re-purpose
> > >   GRE instead?),
> > 
> > We already have an nGnRnE MAIR for config space accesses.
> 
> I'm confused. If M1 needs nGnRE for PCI, and overrides nGnRE to nE
> for its in-SoC accesses, where does nGnRE goes?
> 
> Or do you propose that it is the page tables that get a different
> MT index?

Right, I'm just saying that we already have an nGnRnE MAIR configuration
so there's no need to worry about running out of entries if we need both
nGnRE and nGnRnE to co-exist. The nasty part is how to plumb this into
the mappings only for on-chip MMIO; I guess either a new API or we get
ioremap() to pick the memory type based on the address :/

Will

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: [RFC PATCH 7/7] irqchip/apple-aic: add SMP support to the Apple AIC driver.
  2021-01-21 18:08             ` Mohamed Mediouni
@ 2021-01-21 18:57               ` Rob Herring
  0 siblings, 0 replies; 55+ messages in thread
From: Rob Herring @ 2021-01-21 18:57 UTC (permalink / raw)
  To: Mohamed Mediouni
  Cc: Arnd Bergmann, Mark Rutland, Catalin Marinas, Hector Martin,
	linux-kernel, Marc Zyngier, Will Deacon, Linux ARM,
	Stan Skowronek

On Thu, Jan 21, 2021 at 12:09 PM Mohamed Mediouni
<mohamed.mediouni@caramail.com> wrote:
>
>
>
> > On 21 Jan 2021, at 18:37, Rob Herring <robh@kernel.org> wrote:
> >
> > On Thu, Jan 21, 2021 at 10:43 AM Mohamed Mediouni
> > <mohamed.mediouni@caramail.com> wrote:
> >>> On 21 Jan 2021, at 17:40, Rob Herring <robh@kernel.org> wrote:
> >>> On Thu, Jan 21, 2021 at 6:52 AM Mohamed Mediouni
> >>> <mohamed.mediouni@caramail.com> wrote:
> >>>>> On 21 Jan 2021, at 13:44, Arnd Bergmann <arnd@kernel.org> wrote:
> >>>>> On Wed, Jan 20, 2021 at 2:27 PM Mohamed Mediouni
> >>>>> <mohamed.mediouni@caramail.com> wrote:
> >
> > [...]
> >
> >>>>>> @@ -186,8 +325,11 @@ static int __init apple_aic_init(struct device_node *node,
> >>>>>>     if (WARN(!aic.base, "unable to map aic registers\n"))
> >>>>>>             return -EINVAL;
> >>>>>>
> >>>>>> +       aic.fast_ipi = of_property_read_bool(node, "fast-ipi");
> >>>>>
> >>>>> Where is this property documented, and what decides which one to use?
> >>>> It’s getting documented in the next patch set.
> >>>>
> >>>> This property is there to enable support for older iPhone processors
> >>>> later on, some of which do not have fast IPI support.
> >>>>
> >>>> On Apple M1, fast-ipi is always on.
> >>>
> >>> This should be implied by the compatible string which needs to be more
> >>> specific and include the SoC name.
> >>>
> >>> Rob
> >>
> >> Then we’ll eventually have two aic compatible strings, aic which is compatible
> >> with Apple A7 onwards and aicv2 which is a superset with fast IPI (introduced
> >> on the Apple A11, 3 years ago, with no further programmer-visible changes since
> >> then).
> >>
> >> Does that look right?
> >
> > If we did this from the start, it would evolve like this:
> >
> > A7: "AAPL,a7-aic"
> > A8: "AAPL,a8-aic", "AAPL,a7-aic"  # Read this as A8 AIC is backwards
> > compatible with A7 AIC
> > A9: "AAPL,a9-aic", "AAPL,a7-aic"
> >
> > A11: "AAPL,a11-aic", "AAPL,a7-aic"
> >
> > If the A11 version could work on an OS that only supported the
> > original model (sounds like this is the case) Or if it's not backwards
> > compatible:
> >
>
> The A11 AIC indeed can be used by older drivers that aren’t aware
> of the fast IPI path introduced on A11 just fine.
>
> > A11: "AAPL,a11-aic"
> >
> > If the A11 is different and not backwards compatible.
> >
> > Then M1 could be:
> >
> > M1: "AAPL,m1-aic", "AAPL,a11-aic"
> >
> > Or to even support an OS with only v1 support:
> >
> > M1: "AAPL,m1-aic", "AAPL,a11-aic", "AAPL,a7-aic"
> >
> > You don't really need the fallback here because there isn't any
> > existing OS support and the baseline is the M1.
> >
> > If you want to have generic fallback compatible strings with versions,
> > that's fine too. I'm not really a fan of version numbers that are just
> > made up by the binding author though. Most SoC vendors don't have
> > rigorous versioning of their IP and those that do seem to have a new
> > version on every SoC.
> >
> > The important part is *always* having an SoC specific compatible so
> > you can deal with any quirk or feature without having to change the
> > DTB. Everyone says blocks are 'the same' until they aren’t.
> >
> Is it fine if such a SoC-specific compatible is present but with having
> the driver only know about AAPL,a11-aic for example?
> (To just have it when it’d be needed if ever in the future, but not uselessly
> add entries to the driver that will not be currently used)

Yes, that's expected. You add the more specific compatible when you
add the feature or quirk work-around.

>
> On a tangent:
>
> The internal naming scheme used by Apple is off-by-one:
>
> Apple A14 for example is Apple H13P (H-series 13th gen processor, Phone)
> Apple M1 is Apple H13G (H-series 13th gen, G series)
> (And Apple A12X is Apple H11G for example, with A12 being H11P)
>
> Should we bother with those or use the marketing names? Especially because
> the beefier SoCs might not be of the H series anyway… as the internal scheme
> reveals that M1 could as well have been an A14X.
>
> And there’s also the other internal naming scheme:
> Apple A12 being t8020, Apple A12X being t8027
> Apple A14 being t8101
> Apple M1 being t8103
>
> T there means the foundry at which the chip was manufactured, in the cases above TSMC.
>
> Of course Apple itself uses both… with the marketing name being nowhere in their device
> trees.

I'd probably lean toward the marketing names, but don't really care as
long as you're consistent both for a given SoC and across generations.

Rob

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: [RFC PATCH 7/7] irqchip/apple-aic: add SMP support to the Apple AIC driver.
  2021-01-20 13:27 ` [RFC PATCH 7/7] irqchip/apple-aic: add SMP support to the Apple AIC driver Mohamed Mediouni
  2021-01-21 12:44   ` Arnd Bergmann
@ 2021-02-02 19:15   ` Linus Walleij
  1 sibling, 0 replies; 55+ messages in thread
From: Linus Walleij @ 2021-02-02 19:15 UTC (permalink / raw)
  To: Mohamed Mediouni
  Cc: Linux ARM, Mark Rutland, Catalin Marinas, Hector Martin,
	linux-kernel, Marc Zyngier, Will Deacon, Stan Skowronek

Hi Mohamed,

just a small drive-by comment:

On Wed, Jan 20, 2021 at 2:32 PM Mohamed Mediouni
<mohamed.mediouni@caramail.com> wrote:

> +       for_each_cpu (cpu, mask) {
> +               smp_mb__before_atomic();
> +               atomic_or(1u << irqnr, per_cpu_ptr(&aic_ipi_mask, cpu));

Use:

#include <linux/bits.h>

atomic_or(BIT(irqnr), per_cpu_ptr(&aic_ipi_mask, cpu));

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 55+ messages in thread

end of thread, other threads:[~2021-02-02 19:18 UTC | newest]

Thread overview: 55+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-20 13:27 [RFC PATCH 0/7] Linux on Apple Silicon Mohamed Mediouni
2021-01-20 13:27 ` [RFC PATCH 1/7] arm64: kernel: FIQ support Mohamed Mediouni
2021-01-20 13:27 ` [RFC PATCH 2/7] arm64: kernel: Add a WFI hook Mohamed Mediouni
2021-01-20 16:46   ` Alexander Graf
     [not found]     ` <94C20F55-D3B8-4349-B26F-9EA8AAEBF639@caramail.com>
2021-01-21 12:33       ` Hector Martin 'marcan'
2021-01-21 10:52   ` Arnd Bergmann
2021-01-21 11:01     ` Mohamed Mediouni
2021-01-21 11:36       ` Arnd Bergmann
2021-01-20 13:27 ` [RFC PATCH 3/7] arm64: mm: use nGnRnE instead of nGnRE on Apple processors Mohamed Mediouni
2021-01-20 16:47   ` Alexander Graf
2021-01-20 18:06     ` Mohamed Mediouni
2021-01-20 18:10       ` Alexander Graf
2021-01-21 11:27   ` Will Deacon
2021-01-21 11:38     ` Arnd Bergmann
2021-01-21 11:44     ` Marc Zyngier
2021-01-21 12:47       ` Will Deacon
2021-01-21 15:12         ` Mohamed Mediouni
2021-01-21 16:25           ` Marc Zyngier
2021-01-21 17:55             ` Will Deacon
2021-01-21 18:15               ` Marc Zyngier
2021-01-21 18:22                 ` Mohamed Mediouni
2021-01-21 18:22                 ` Will Deacon
2021-01-20 13:27 ` [RFC PATCH 4/7] irqchip/apple-aic: Add support for Apple AIC Mohamed Mediouni
2021-01-20 17:11   ` Alexander Graf
2021-01-20 18:04     ` Mohamed Mediouni
2021-01-20 20:16       ` Andrew Lunn
2021-01-20 21:18   ` Stan Skowronek
2021-01-21  9:48   ` Linus Walleij
2021-01-21 10:37     ` Arnd Bergmann
2021-01-21 15:29       ` Hector Martin 'marcan'
2021-01-21 17:09         ` Rob Herring
2021-01-21 17:45       ` Rob Herring
2021-01-21 16:44     ` Rob Herring
2021-01-21 16:53   ` Hector Martin 'marcan'
2021-01-20 13:27 ` [RFC PATCH 5/7] arm64/Kconfig: Add Apple Silicon SoC platform Mohamed Mediouni
2021-01-20 13:27 ` [RFC PATCH 6/7] arm64: kernel: Apple CPU start driver Mohamed Mediouni
2021-01-21 11:14   ` Arnd Bergmann
2021-01-20 13:27 ` [RFC PATCH 7/7] irqchip/apple-aic: add SMP support to the Apple AIC driver Mohamed Mediouni
2021-01-21 12:44   ` Arnd Bergmann
2021-01-21 12:50     ` Mohamed Mediouni
2021-01-21 13:00       ` Arnd Bergmann
2021-01-21 13:01         ` Hector Martin 'marcan'
2021-01-21 13:22       ` Marc Zyngier
2021-01-21 13:32         ` Mark Rutland
2021-01-21 14:05           ` Marc Zyngier
2021-01-21 13:34         ` Mohamed Mediouni
2021-01-21 14:10           ` Marc Zyngier
2021-01-21 15:09             ` Arnd Bergmann
2021-01-21 15:18               ` Mohamed Mediouni
2021-01-21 16:40       ` Rob Herring
2021-01-21 16:43         ` Mohamed Mediouni
2021-01-21 17:37           ` Rob Herring
2021-01-21 18:08             ` Mohamed Mediouni
2021-01-21 18:57               ` Rob Herring
2021-02-02 19:15   ` Linus Walleij

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).