* [PATCH v2 0/4] Add support for A7-A11 AIC
@ 2024-08-31 5:48 Nick Chan
2024-08-31 5:48 ` [PATCH v2 1/4] dt-bindings: apple,aic: Document A7-A11 compatibles Nick Chan
` (3 more replies)
0 siblings, 4 replies; 10+ messages in thread
From: Nick Chan @ 2024-08-31 5:48 UTC (permalink / raw)
To: Hector Martin, Sven Peter, Alyssa Rosenzweig, Thomas Gleixner,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, asahi,
linux-arm-kernel, linux-kernel, devicetree, konrad.dybcio
Cc: ~postmarketos/upstreaming, Nick Chan
Hi,
This patch series introduce support for A7-A11 SoCs into the AIC driver by
if'ing out some features unavailable on them. For background information
about these SoCs, see v1.
Changes since v1:
- Corrected dt-binding issues pointed out by Rob, all of A7-M1 gets its own
SoC compat, as well as the "apple,aic" generic fallback.
- Order the AIC dt-bindings of the SoCs by release dates instead of
alphabetical order: 's5l8960x' (A7) -> 't7000' (A8) -> 's8000' (A9) ->
't8010' (A10) -> 't8015' (A11) -> 't8103' (M1)
- The patch to disallow local fast IPI on A11 now comes before the patch
that enables to AIC driver to work on slow IPI systems. This ensures all
fast IPI capable systems can use one of the IPI capable compatibles before
the IPI-always-ack code path in aic_handle_fiq() is removed by the slow IPI
patch.
- Keep local fast IPI enabled on AIC2, mistake in V1
- Describe the feature levels in the description of AIC.
- Added new patch to remove unneccessary enablement of use_fast_ipi.
- Do not try to enable use_local_fast_ipi, because it is enabled by default.
In theory M1 can also get the apple,t8015-aic (A11) compatible. However, I
do not see a point in doing that.
Previous version:
v1: https://lore.kernel.org/asahi/20240829092610.89731-1-towinchenmi@gmail.com
Nick Chan
Konrad Dybcio (1):
irqchip/apple-aic: Only access system registers on SoCs which provide
them
Nick Chan (3):
dt-bindings: apple,aic: Document A7-A11 compatibles
irqchip/apple-aic: Skip unnecessary setting of use_fast_ipi
irqchip/apple-aic: Add a new "Global fast IPIs only" feature level
.../interrupt-controller/apple,aic.yaml | 14 ++++-
drivers/irqchip/irq-apple-aic.c | 53 +++++++++++++------
2 files changed, 49 insertions(+), 18 deletions(-)
base-commit: 985bf40edf4343dcb04c33f58b40b4a85c1776d4
--
2.46.0
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 1/4] dt-bindings: apple,aic: Document A7-A11 compatibles
2024-08-31 5:48 [PATCH v2 0/4] Add support for A7-A11 AIC Nick Chan
@ 2024-08-31 5:48 ` Nick Chan
2024-08-31 6:34 ` Krzysztof Kozlowski
2024-08-31 6:43 ` Sven Peter
2024-08-31 5:48 ` [PATCH v2 2/4] irqchip/apple-aic: Skip unnecessary setting of use_fast_ipi Nick Chan
` (2 subsequent siblings)
3 siblings, 2 replies; 10+ messages in thread
From: Nick Chan @ 2024-08-31 5:48 UTC (permalink / raw)
To: Hector Martin, Sven Peter, Alyssa Rosenzweig, Thomas Gleixner,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, asahi,
linux-arm-kernel, linux-kernel, devicetree, konrad.dybcio
Cc: ~postmarketos/upstreaming, Nick Chan
Document and describe the compatibles for Apple A7-A11 SoCs.
There are three feature levels:
- apple,aic: No fast IPI, for A7-A10
- apple,t8015-aic: fast IPI, global only, for A11
- apple,t8103-aic: fast IPI with local and global support, for M1
Each feature level is an extension of the previous, for example, M1 will
also work with the A7 feature level.
All of A7-M1 gets its own SoC-specific compatible, and the "apple,aic"
compatible as a fallback.
Signed-off-by: Nick Chan <towinchenmi@gmail.com>
---
.../bindings/interrupt-controller/apple,aic.yaml | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/interrupt-controller/apple,aic.yaml b/Documentation/devicetree/bindings/interrupt-controller/apple,aic.yaml
index 698588e9aa86..4be9b596a790 100644
--- a/Documentation/devicetree/bindings/interrupt-controller/apple,aic.yaml
+++ b/Documentation/devicetree/bindings/interrupt-controller/apple,aic.yaml
@@ -31,13 +31,25 @@ description: |
This device also represents the FIQ interrupt sources on platforms using AIC,
which do not go through a discrete interrupt controller.
+ IPIs may be performed via MMIO registers on all variants of AIC. Starting
+ from A11, system registers may also be used for "fast" IPIs. Starting from
+ M1, even faster IPIs within the same cluster may be achieved by writing to
+ a "local" fast IPI register as opposed to using the "global" fast IPI
+ register.
+
allOf:
- $ref: /schemas/interrupt-controller.yaml#
properties:
compatible:
items:
- - const: apple,t8103-aic
+ - enum:
+ - apple,s5l8960x-aic
+ - apple,t7000-aic
+ - apple,s8000-aic
+ - apple,t8010-aic
+ - apple,t8015-aic
+ - apple,t8103-aic
- const: apple,aic
interrupt-controller: true
--
2.46.0
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 2/4] irqchip/apple-aic: Skip unnecessary setting of use_fast_ipi
2024-08-31 5:48 [PATCH v2 0/4] Add support for A7-A11 AIC Nick Chan
2024-08-31 5:48 ` [PATCH v2 1/4] dt-bindings: apple,aic: Document A7-A11 compatibles Nick Chan
@ 2024-08-31 5:48 ` Nick Chan
2024-08-31 6:48 ` Sven Peter
2024-08-31 5:48 ` [PATCH v2 3/4] irqchip/apple-aic: Add a new "Global fast IPIs only" feature level Nick Chan
2024-08-31 5:48 ` [PATCH v2 4/4] irqchip/apple-aic: Only access system registers on SoCs which provide them Nick Chan
3 siblings, 1 reply; 10+ messages in thread
From: Nick Chan @ 2024-08-31 5:48 UTC (permalink / raw)
To: Hector Martin, Sven Peter, Alyssa Rosenzweig, Thomas Gleixner,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, asahi,
linux-arm-kernel, linux-kernel, devicetree, konrad.dybcio
Cc: ~postmarketos/upstreaming, Nick Chan
use_fast_ipi is true by default and there is no need to "enable" it.
Signed-off-by: Nick Chan <towinchenmi@gmail.com>
---
drivers/irqchip/irq-apple-aic.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/irqchip/irq-apple-aic.c b/drivers/irqchip/irq-apple-aic.c
index 5c534d9fd2b0..8d81d5fb3c50 100644
--- a/drivers/irqchip/irq-apple-aic.c
+++ b/drivers/irqchip/irq-apple-aic.c
@@ -987,9 +987,7 @@ static int __init aic_of_ic_init(struct device_node *node, struct device_node *p
off += sizeof(u32) * (irqc->max_irq >> 5); /* MASK_CLR */
off += sizeof(u32) * (irqc->max_irq >> 5); /* HW_STATE */
- if (irqc->info.fast_ipi)
- static_branch_enable(&use_fast_ipi);
- else
+ if (!irqc->info.fast_ipi)
static_branch_disable(&use_fast_ipi);
irqc->info.die_stride = off - start_off;
--
2.46.0
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 3/4] irqchip/apple-aic: Add a new "Global fast IPIs only" feature level
2024-08-31 5:48 [PATCH v2 0/4] Add support for A7-A11 AIC Nick Chan
2024-08-31 5:48 ` [PATCH v2 1/4] dt-bindings: apple,aic: Document A7-A11 compatibles Nick Chan
2024-08-31 5:48 ` [PATCH v2 2/4] irqchip/apple-aic: Skip unnecessary setting of use_fast_ipi Nick Chan
@ 2024-08-31 5:48 ` Nick Chan
2024-08-31 7:05 ` Sven Peter
2024-08-31 5:48 ` [PATCH v2 4/4] irqchip/apple-aic: Only access system registers on SoCs which provide them Nick Chan
3 siblings, 1 reply; 10+ messages in thread
From: Nick Chan @ 2024-08-31 5:48 UTC (permalink / raw)
To: Hector Martin, Sven Peter, Alyssa Rosenzweig, Thomas Gleixner,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, asahi,
linux-arm-kernel, linux-kernel, devicetree, konrad.dybcio
Cc: ~postmarketos/upstreaming, Nick Chan
Starting with the A11 (T8015) SoC, Apple began using arm64 sysregs for
fast IPIs. However, on A11, there is no such things as "Local" fast IPIs,
as the SYS_IMP_APL_IPI_RR_LOCAL_EL1 register does not seem to exist.
Add a new feature level, used by the compatible "apple,t8015-aic",
controlled by a static branch key named use_local_fast_ipi. When
use_fast_ipi is true and use_local_fast_ipi is false, fast IPIs are used
but all IPIs goes through the register SYS_IMP_APL_IPI_RR_GLOBAL_EL1, as
"global" IPIs.
Signed-off-by: Nick Chan <towinchenmi@gmail.com>
---
drivers/irqchip/irq-apple-aic.c | 24 +++++++++++++++++++++++-
1 file changed, 23 insertions(+), 1 deletion(-)
diff --git a/drivers/irqchip/irq-apple-aic.c b/drivers/irqchip/irq-apple-aic.c
index 8d81d5fb3c50..01a3c79054f5 100644
--- a/drivers/irqchip/irq-apple-aic.c
+++ b/drivers/irqchip/irq-apple-aic.c
@@ -235,6 +235,8 @@ enum fiq_hwirq {
};
static DEFINE_STATIC_KEY_TRUE(use_fast_ipi);
+/* True if SYS_IMP_APL_IPI_RR_LOCAL_EL1 exists for local fast IPIs (M1+) */
+static DEFINE_STATIC_KEY_TRUE(use_local_fast_ipi);
struct aic_info {
int version;
@@ -252,6 +254,7 @@ struct aic_info {
/* Features */
bool fast_ipi;
+ bool local_fast_ipi;
};
static const struct aic_info aic1_info __initconst = {
@@ -270,17 +273,32 @@ static const struct aic_info aic1_fipi_info __initconst = {
.fast_ipi = true,
};
+static const struct aic_info aic1_local_fipi_info __initconst = {
+ .version = 1,
+
+ .event = AIC_EVENT,
+ .target_cpu = AIC_TARGET_CPU,
+
+ .fast_ipi = true,
+ .local_fast_ipi = true,
+};
+
static const struct aic_info aic2_info __initconst = {
.version = 2,
.irq_cfg = AIC2_IRQ_CFG,
.fast_ipi = true,
+ .local_fast_ipi = true,
};
static const struct of_device_id aic_info_match[] = {
{
.compatible = "apple,t8103-aic",
+ .data = &aic1_local_fipi_info,
+ },
+ {
+ .compatible = "apple,t8015-aic",
.data = &aic1_fipi_info,
},
{
@@ -750,7 +768,8 @@ static void aic_ipi_send_fast(int cpu)
u64 cluster = MPIDR_CLUSTER(mpidr);
u64 idx = MPIDR_CPU(mpidr);
- if (MPIDR_CLUSTER(my_mpidr) == cluster)
+ if (static_branch_likely(&use_local_fast_ipi) &&
+ MPIDR_CLUSTER(my_mpidr) == cluster)
write_sysreg_s(FIELD_PREP(IPI_RR_CPU, idx),
SYS_IMP_APL_IPI_RR_LOCAL_EL1);
else
@@ -990,6 +1009,9 @@ static int __init aic_of_ic_init(struct device_node *node, struct device_node *p
if (!irqc->info.fast_ipi)
static_branch_disable(&use_fast_ipi);
+ if (!irqc->info.local_fast_ipi)
+ static_branch_disable(&use_local_fast_ipi);
+
irqc->info.die_stride = off - start_off;
irqc->hw_domain = irq_domain_create_tree(of_node_to_fwnode(node),
--
2.46.0
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 4/4] irqchip/apple-aic: Only access system registers on SoCs which provide them
2024-08-31 5:48 [PATCH v2 0/4] Add support for A7-A11 AIC Nick Chan
` (2 preceding siblings ...)
2024-08-31 5:48 ` [PATCH v2 3/4] irqchip/apple-aic: Add a new "Global fast IPIs only" feature level Nick Chan
@ 2024-08-31 5:48 ` Nick Chan
2024-08-31 10:01 ` Sven Peter
3 siblings, 1 reply; 10+ messages in thread
From: Nick Chan @ 2024-08-31 5:48 UTC (permalink / raw)
To: Hector Martin, Sven Peter, Alyssa Rosenzweig, Thomas Gleixner,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, asahi,
linux-arm-kernel, linux-kernel, devicetree, konrad.dybcio
Cc: ~postmarketos/upstreaming, Nick Chan
From: Konrad Dybcio <konrad.dybcio@somainline.org>
Starting from the A11 (T8015) SoC, Apple introuced system registers for
fast IPI and UNCORE PMC control. These sysregs do not exist on earlier
A7-A10 SoCs and trying to access them results in an instant crash.
Restrict sysreg access within the AIC driver to configurations where
use_fast_ipi is true to allow AIC to function properly on A7-A10 SoCs.
As part of the restriction, the IPI-always-ack path in aic_handle_fiq()
has been removed. If the code path were to be reached, the system is
fast IPI capable and one of the fast IPI compatibles should be used
instead.
Signed-off-by: Konrad Dybcio <konrad.dybcio@somainline.org>
Co-developed-by: Nick Chan <towinchenmi@gmail.com>
Signed-off-by: Nick Chan <towinchenmi@gmail.com>
---
drivers/irqchip/irq-apple-aic.c | 25 ++++++++++++-------------
1 file changed, 12 insertions(+), 13 deletions(-)
diff --git a/drivers/irqchip/irq-apple-aic.c b/drivers/irqchip/irq-apple-aic.c
index 01a3c79054f5..a6d812afb10d 100644
--- a/drivers/irqchip/irq-apple-aic.c
+++ b/drivers/irqchip/irq-apple-aic.c
@@ -234,6 +234,7 @@ enum fiq_hwirq {
AIC_NR_FIQ
};
+/* True if UNCORE/UNCORE2 and Sn_... IPI registers are present and used (A11+) */
static DEFINE_STATIC_KEY_TRUE(use_fast_ipi);
/* True if SYS_IMP_APL_IPI_RR_LOCAL_EL1 exists for local fast IPIs (M1+) */
static DEFINE_STATIC_KEY_TRUE(use_local_fast_ipi);
@@ -550,14 +551,9 @@ static void __exception_irq_entry aic_handle_fiq(struct pt_regs *regs)
* we check for everything here, even things we don't support yet.
*/
- if (read_sysreg_s(SYS_IMP_APL_IPI_SR_EL1) & IPI_SR_PENDING) {
- if (static_branch_likely(&use_fast_ipi)) {
- aic_handle_ipi(regs);
- } else {
- pr_err_ratelimited("Fast IPI fired. Acking.\n");
- write_sysreg_s(IPI_SR_PENDING, SYS_IMP_APL_IPI_SR_EL1);
- }
- }
+ if (static_branch_likely(&use_fast_ipi) &&
+ (read_sysreg_s(SYS_IMP_APL_IPI_SR_EL1) & IPI_SR_PENDING))
+ aic_handle_ipi(regs);
if (TIMER_FIRING(read_sysreg(cntp_ctl_el0)))
generic_handle_domain_irq(aic_irqc->hw_domain,
@@ -592,8 +588,9 @@ static void __exception_irq_entry aic_handle_fiq(struct pt_regs *regs)
AIC_FIQ_HWIRQ(irq));
}
- if (FIELD_GET(UPMCR0_IMODE, read_sysreg_s(SYS_IMP_APL_UPMCR0_EL1)) == UPMCR0_IMODE_FIQ &&
- (read_sysreg_s(SYS_IMP_APL_UPMSR_EL1) & UPMSR_IACT)) {
+ if (static_branch_likely(&use_fast_ipi) &&
+ (FIELD_GET(UPMCR0_IMODE, read_sysreg_s(SYS_IMP_APL_UPMCR0_EL1)) == UPMCR0_IMODE_FIQ) &&
+ (read_sysreg_s(SYS_IMP_APL_UPMSR_EL1) & UPMSR_IACT)) {
/* Same story with uncore PMCs */
pr_err_ratelimited("Uncore PMC FIQ fired. Masking.\n");
sysreg_clear_set_s(SYS_IMP_APL_UPMCR0_EL1, UPMCR0_IMODE,
@@ -830,7 +827,8 @@ static int aic_init_cpu(unsigned int cpu)
/* Mask all hard-wired per-CPU IRQ/FIQ sources */
/* Pending Fast IPI FIQs */
- write_sysreg_s(IPI_SR_PENDING, SYS_IMP_APL_IPI_SR_EL1);
+ if (static_branch_likely(&use_fast_ipi))
+ write_sysreg_s(IPI_SR_PENDING, SYS_IMP_APL_IPI_SR_EL1);
/* Timer FIQs */
sysreg_clear_set(cntp_ctl_el0, 0, ARCH_TIMER_CTRL_IT_MASK);
@@ -851,8 +849,9 @@ static int aic_init_cpu(unsigned int cpu)
FIELD_PREP(PMCR0_IMODE, PMCR0_IMODE_OFF));
/* Uncore PMC FIQ */
- sysreg_clear_set_s(SYS_IMP_APL_UPMCR0_EL1, UPMCR0_IMODE,
- FIELD_PREP(UPMCR0_IMODE, UPMCR0_IMODE_OFF));
+ if (static_branch_likely(&use_fast_ipi))
+ sysreg_clear_set_s(SYS_IMP_APL_UPMCR0_EL1, UPMCR0_IMODE,
+ FIELD_PREP(UPMCR0_IMODE, UPMCR0_IMODE_OFF));
/* Commit all of the above */
isb();
--
2.46.0
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/4] dt-bindings: apple,aic: Document A7-A11 compatibles
2024-08-31 5:48 ` [PATCH v2 1/4] dt-bindings: apple,aic: Document A7-A11 compatibles Nick Chan
@ 2024-08-31 6:34 ` Krzysztof Kozlowski
2024-08-31 6:43 ` Sven Peter
1 sibling, 0 replies; 10+ messages in thread
From: Krzysztof Kozlowski @ 2024-08-31 6:34 UTC (permalink / raw)
To: Nick Chan
Cc: Hector Martin, Sven Peter, Alyssa Rosenzweig, Thomas Gleixner,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, asahi,
linux-arm-kernel, linux-kernel, devicetree, konrad.dybcio,
~postmarketos/upstreaming
On Sat, Aug 31, 2024 at 01:48:16PM +0800, Nick Chan wrote:
> Document and describe the compatibles for Apple A7-A11 SoCs.
>
> There are three feature levels:
> - apple,aic: No fast IPI, for A7-A10
> - apple,t8015-aic: fast IPI, global only, for A11
> - apple,t8103-aic: fast IPI with local and global support, for M1
>
> Each feature level is an extension of the previous, for example, M1 will
> also work with the A7 feature level.
>
> All of A7-M1 gets its own SoC-specific compatible, and the "apple,aic"
> compatible as a fallback.
>
> Signed-off-by: Nick Chan <towinchenmi@gmail.com>
> ---
> .../bindings/interrupt-controller/apple,aic.yaml | 14 +++++++++++++-
> 1 file changed, 13 insertions(+), 1 deletion(-)
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
<form letter>
This is an automated instruction, just in case, because many review tags
are being ignored. If you know the process, you can skip it (please do
not feel offended by me posting it here - no bad intentions intended).
If you do not know the process, here is a short explanation:
Please add Acked-by/Reviewed-by/Tested-by tags when posting new
versions, under or above your Signed-off-by tag. Tag is "received", when
provided in a message replied to you on the mailing list. Tools like b4
can help here. However, there's no need to repost patches *only* to add
the tags. The upstream maintainer will do that for tags received on the
version they apply.
https://elixir.bootlin.com/linux/v6.5-rc3/source/Documentation/process/submitting-patches.rst#L577
</form letter>
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/4] dt-bindings: apple,aic: Document A7-A11 compatibles
2024-08-31 5:48 ` [PATCH v2 1/4] dt-bindings: apple,aic: Document A7-A11 compatibles Nick Chan
2024-08-31 6:34 ` Krzysztof Kozlowski
@ 2024-08-31 6:43 ` Sven Peter
1 sibling, 0 replies; 10+ messages in thread
From: Sven Peter @ 2024-08-31 6:43 UTC (permalink / raw)
To: Nick Chan
Cc: Hector Martin, Alyssa Rosenzweig, Thomas Gleixner, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, asahi, linux-arm-kernel,
linux-kernel, devicetree, konrad.dybcio,
~postmarketos/upstreaming
> On 31. Aug 2024, at 07:57, Nick Chan <towinchenmi@gmail.com> wrote:
>
> Document and describe the compatibles for Apple A7-A11 SoCs.
>
> There are three feature levels:
> - apple,aic: No fast IPI, for A7-A10
> - apple,t8015-aic: fast IPI, global only, for A11
> - apple,t8103-aic: fast IPI with local and global support, for M1
>
> Each feature level is an extension of the previous, for example, M1 will
> also work with the A7 feature level.
>
> All of A7-M1 gets its own SoC-specific compatible, and the "apple,aic"
> compatible as a fallback.
>
> Signed-off-by: Nick Chan <towinchenmi@gmail.com>
> ---
Reviewed-by: Sven Peter <sven@svenpeter.dev>
> .../bindings/interrupt-controller/apple,aic.yaml | 14 +++++++++++++-
> 1 file changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/apple,aic.yaml b/Documentation/devicetree/bindings/interrupt-controller/apple,aic.yaml
> index 698588e9aa86..4be9b596a790 100644
> --- a/Documentation/devicetree/bindings/interrupt-controller/apple,aic.yaml
> +++ b/Documentation/devicetree/bindings/interrupt-controller/apple,aic.yaml
> @@ -31,13 +31,25 @@ description: |
> This device also represents the FIQ interrupt sources on platforms using AIC,
> which do not go through a discrete interrupt controller.
>
> + IPIs may be performed via MMIO registers on all variants of AIC. Starting
> + from A11, system registers may also be used for "fast" IPIs. Starting from
> + M1, even faster IPIs within the same cluster may be achieved by writing to
> + a "local" fast IPI register as opposed to using the "global" fast IPI
> + register.
> +
> allOf:
> - $ref: /schemas/interrupt-controller.yaml#
>
> properties:
> compatible:
> items:
> - - const: apple,t8103-aic
> + - enum:
> + - apple,s5l8960x-aic
> + - apple,t7000-aic
> + - apple,s8000-aic
> + - apple,t8010-aic
> + - apple,t8015-aic
> + - apple,t8103-aic
> - const: apple,aic
>
> interrupt-controller: true
> --
> 2.46.0
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/4] irqchip/apple-aic: Skip unnecessary setting of use_fast_ipi
2024-08-31 5:48 ` [PATCH v2 2/4] irqchip/apple-aic: Skip unnecessary setting of use_fast_ipi Nick Chan
@ 2024-08-31 6:48 ` Sven Peter
0 siblings, 0 replies; 10+ messages in thread
From: Sven Peter @ 2024-08-31 6:48 UTC (permalink / raw)
To: Nick Chan
Cc: Hector Martin, Alyssa Rosenzweig, Thomas Gleixner, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, asahi, linux-arm-kernel,
linux-kernel, devicetree, konrad.dybcio,
~postmarketos/upstreaming
> On 31. Aug 2024, at 07:57, Nick Chan <towinchenmi@gmail.com> wrote:
>
> use_fast_ipi is true by default and there is no need to "enable" it.
>
> Signed-off-by: Nick Chan <towinchenmi@gmail.com>
> ---
Acked-by: Sven Peter <sven@svenpeter.dev>
> drivers/irqchip/irq-apple-aic.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/irqchip/irq-apple-aic.c b/drivers/irqchip/irq-apple-aic.c
> index 5c534d9fd2b0..8d81d5fb3c50 100644
> --- a/drivers/irqchip/irq-apple-aic.c
> +++ b/drivers/irqchip/irq-apple-aic.c
> @@ -987,9 +987,7 @@ static int __init aic_of_ic_init(struct device_node *node, struct device_node *p
> off += sizeof(u32) * (irqc->max_irq >> 5); /* MASK_CLR */
> off += sizeof(u32) * (irqc->max_irq >> 5); /* HW_STATE */
>
> - if (irqc->info.fast_ipi)
> - static_branch_enable(&use_fast_ipi);
> - else
> + if (!irqc->info.fast_ipi)
> static_branch_disable(&use_fast_ipi);
>
> irqc->info.die_stride = off - start_off;
> --
> 2.46.0
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 3/4] irqchip/apple-aic: Add a new "Global fast IPIs only" feature level
2024-08-31 5:48 ` [PATCH v2 3/4] irqchip/apple-aic: Add a new "Global fast IPIs only" feature level Nick Chan
@ 2024-08-31 7:05 ` Sven Peter
0 siblings, 0 replies; 10+ messages in thread
From: Sven Peter @ 2024-08-31 7:05 UTC (permalink / raw)
To: Nick Chan
Cc: Hector Martin, Alyssa Rosenzweig, Thomas Gleixner, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, asahi, linux-arm-kernel,
linux-kernel, devicetree, konrad.dybcio,
~postmarketos/upstreaming
>
> On 31. Aug 2024, at 07:57, Nick Chan <towinchenmi@gmail.com> wrote:
>
> Starting with the A11 (T8015) SoC, Apple began using arm64 sysregs for
> fast IPIs. However, on A11, there is no such things as "Local" fast IPIs,
> as the SYS_IMP_APL_IPI_RR_LOCAL_EL1 register does not seem to exist.
>
> Add a new feature level, used by the compatible "apple,t8015-aic",
> controlled by a static branch key named use_local_fast_ipi. When
> use_fast_ipi is true and use_local_fast_ipi is false, fast IPIs are used
> but all IPIs goes through the register SYS_IMP_APL_IPI_RR_GLOBAL_EL1, as
> "global" IPIs.
>
> Signed-off-by: Nick Chan <towinchenmi@gmail.com>
> ---
Reviewed-by: Sven Peter <sven@svenpeter.dev>
> drivers/irqchip/irq-apple-aic.c | 24 +++++++++++++++++++++++-
> 1 file changed, 23 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/irqchip/irq-apple-aic.c b/drivers/irqchip/irq-apple-aic.c
> index 8d81d5fb3c50..01a3c79054f5 100644
> --- a/drivers/irqchip/irq-apple-aic.c
> +++ b/drivers/irqchip/irq-apple-aic.c
> @@ -235,6 +235,8 @@ enum fiq_hwirq {
> };
>
> static DEFINE_STATIC_KEY_TRUE(use_fast_ipi);
> +/* True if SYS_IMP_APL_IPI_RR_LOCAL_EL1 exists for local fast IPIs (M1+) */
> +static DEFINE_STATIC_KEY_TRUE(use_local_fast_ipi);
>
> struct aic_info {
> int version;
> @@ -252,6 +254,7 @@ struct aic_info {
>
> /* Features */
> bool fast_ipi;
> + bool local_fast_ipi;
> };
>
> static const struct aic_info aic1_info __initconst = {
> @@ -270,17 +273,32 @@ static const struct aic_info aic1_fipi_info __initconst = {
> .fast_ipi = true,
> };
>
> +static const struct aic_info aic1_local_fipi_info __initconst = {
> + .version = 1,
> +
> + .event = AIC_EVENT,
> + .target_cpu = AIC_TARGET_CPU,
> +
> + .fast_ipi = true,
> + .local_fast_ipi = true,
> +};
> +
> static const struct aic_info aic2_info __initconst = {
> .version = 2,
>
> .irq_cfg = AIC2_IRQ_CFG,
>
> .fast_ipi = true,
> + .local_fast_ipi = true,
> };
>
> static const struct of_device_id aic_info_match[] = {
> {
> .compatible = "apple,t8103-aic",
> + .data = &aic1_local_fipi_info,
> + },
> + {
> + .compatible = "apple,t8015-aic",
> .data = &aic1_fipi_info,
> },
> {
> @@ -750,7 +768,8 @@ static void aic_ipi_send_fast(int cpu)
> u64 cluster = MPIDR_CLUSTER(mpidr);
> u64 idx = MPIDR_CPU(mpidr);
>
> - if (MPIDR_CLUSTER(my_mpidr) == cluster)
> + if (static_branch_likely(&use_local_fast_ipi) &&
> + MPIDR_CLUSTER(my_mpidr) == cluster)
> write_sysreg_s(FIELD_PREP(IPI_RR_CPU, idx),
> SYS_IMP_APL_IPI_RR_LOCAL_EL1);
> else
> @@ -990,6 +1009,9 @@ static int __init aic_of_ic_init(struct device_node *node, struct device_node *p
> if (!irqc->info.fast_ipi)
> static_branch_disable(&use_fast_ipi);
>
> + if (!irqc->info.local_fast_ipi)
> + static_branch_disable(&use_local_fast_ipi);
> +
> irqc->info.die_stride = off - start_off;
>
> irqc->hw_domain = irq_domain_create_tree(of_node_to_fwnode(node),
> --
> 2.46.0
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 4/4] irqchip/apple-aic: Only access system registers on SoCs which provide them
2024-08-31 5:48 ` [PATCH v2 4/4] irqchip/apple-aic: Only access system registers on SoCs which provide them Nick Chan
@ 2024-08-31 10:01 ` Sven Peter
0 siblings, 0 replies; 10+ messages in thread
From: Sven Peter @ 2024-08-31 10:01 UTC (permalink / raw)
To: Nick Chan, Hector Martin, Alyssa Rosenzweig, Thomas Gleixner,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, asahi,
linux-arm-kernel, linux-kernel, devicetree, Konrad Dybcio
Cc: ~postmarketos/upstreaming
On Sat, Aug 31, 2024, at 07:48, Nick Chan wrote:
> From: Konrad Dybcio <konrad.dybcio@somainline.org>
>
> Starting from the A11 (T8015) SoC, Apple introuced system registers for
> fast IPI and UNCORE PMC control. These sysregs do not exist on earlier
> A7-A10 SoCs and trying to access them results in an instant crash.
>
> Restrict sysreg access within the AIC driver to configurations where
> use_fast_ipi is true to allow AIC to function properly on A7-A10 SoCs.
> As part of the restriction, the IPI-always-ack path in aic_handle_fiq()
> has been removed. If the code path were to be reached, the system is
> fast IPI capable and one of the fast IPI compatibles should be used
> instead.
The last part is a bit weird. This is really just a bug in the driver.
We either have a system with fastipi and then we can handle it.
Or we have a system without fastipi and we shouldn't even try to read
SYS_IMP_APL_IPI_SR_EL1.
Your first sentence "Restrict sysreg access within the AIC driver to
configurations where use_fast_ipi is true to allow AIC to function properly
on A7-A10 SoCs." already explains what's going on here. You're fixing
a bug where we tried to access SYS_IMP_APL_IPI_SR_EL1 unconditionally.
It just didn't matter so far because there's only upstream support
for M1+ where SYS_IMP_APL_IPI_SR_EL1 always exist.
With a fixed commit message:
Reviewed-by: Sven Peter <sven@svenpeter.dev>
>
> Signed-off-by: Konrad Dybcio <konrad.dybcio@somainline.org>
> Co-developed-by: Nick Chan <towinchenmi@gmail.com>
> Signed-off-by: Nick Chan <towinchenmi@gmail.com>
> ---
> drivers/irqchip/irq-apple-aic.c | 25 ++++++++++++-------------
> 1 file changed, 12 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/irqchip/irq-apple-aic.c b/drivers/irqchip/irq-apple-aic.c
> index 01a3c79054f5..a6d812afb10d 100644
> --- a/drivers/irqchip/irq-apple-aic.c
> +++ b/drivers/irqchip/irq-apple-aic.c
> @@ -234,6 +234,7 @@ enum fiq_hwirq {
> AIC_NR_FIQ
> };
>
> +/* True if UNCORE/UNCORE2 and Sn_... IPI registers are present and
> used (A11+) */
> static DEFINE_STATIC_KEY_TRUE(use_fast_ipi);
> /* True if SYS_IMP_APL_IPI_RR_LOCAL_EL1 exists for local fast IPIs
> (M1+) */
> static DEFINE_STATIC_KEY_TRUE(use_local_fast_ipi);
> @@ -550,14 +551,9 @@ static void __exception_irq_entry
> aic_handle_fiq(struct pt_regs *regs)
> * we check for everything here, even things we don't support yet.
> */
>
> - if (read_sysreg_s(SYS_IMP_APL_IPI_SR_EL1) & IPI_SR_PENDING) {
> - if (static_branch_likely(&use_fast_ipi)) {
> - aic_handle_ipi(regs);
> - } else {
> - pr_err_ratelimited("Fast IPI fired. Acking.\n");
> - write_sysreg_s(IPI_SR_PENDING, SYS_IMP_APL_IPI_SR_EL1);
> - }
> - }
> + if (static_branch_likely(&use_fast_ipi) &&
> + (read_sysreg_s(SYS_IMP_APL_IPI_SR_EL1) & IPI_SR_PENDING))
> + aic_handle_ipi(regs);
>
> if (TIMER_FIRING(read_sysreg(cntp_ctl_el0)))
> generic_handle_domain_irq(aic_irqc->hw_domain,
> @@ -592,8 +588,9 @@ static void __exception_irq_entry
> aic_handle_fiq(struct pt_regs *regs)
> AIC_FIQ_HWIRQ(irq));
> }
>
> - if (FIELD_GET(UPMCR0_IMODE, read_sysreg_s(SYS_IMP_APL_UPMCR0_EL1)) ==
> UPMCR0_IMODE_FIQ &&
> - (read_sysreg_s(SYS_IMP_APL_UPMSR_EL1) & UPMSR_IACT)) {
> + if (static_branch_likely(&use_fast_ipi) &&
> + (FIELD_GET(UPMCR0_IMODE, read_sysreg_s(SYS_IMP_APL_UPMCR0_EL1))
> == UPMCR0_IMODE_FIQ) &&
> + (read_sysreg_s(SYS_IMP_APL_UPMSR_EL1) & UPMSR_IACT)) {
> /* Same story with uncore PMCs */
> pr_err_ratelimited("Uncore PMC FIQ fired. Masking.\n");
> sysreg_clear_set_s(SYS_IMP_APL_UPMCR0_EL1, UPMCR0_IMODE,
> @@ -830,7 +827,8 @@ static int aic_init_cpu(unsigned int cpu)
> /* Mask all hard-wired per-CPU IRQ/FIQ sources */
>
> /* Pending Fast IPI FIQs */
> - write_sysreg_s(IPI_SR_PENDING, SYS_IMP_APL_IPI_SR_EL1);
> + if (static_branch_likely(&use_fast_ipi))
> + write_sysreg_s(IPI_SR_PENDING, SYS_IMP_APL_IPI_SR_EL1);
>
> /* Timer FIQs */
> sysreg_clear_set(cntp_ctl_el0, 0, ARCH_TIMER_CTRL_IT_MASK);
> @@ -851,8 +849,9 @@ static int aic_init_cpu(unsigned int cpu)
> FIELD_PREP(PMCR0_IMODE, PMCR0_IMODE_OFF));
>
> /* Uncore PMC FIQ */
> - sysreg_clear_set_s(SYS_IMP_APL_UPMCR0_EL1, UPMCR0_IMODE,
> - FIELD_PREP(UPMCR0_IMODE, UPMCR0_IMODE_OFF));
> + if (static_branch_likely(&use_fast_ipi))
> + sysreg_clear_set_s(SYS_IMP_APL_UPMCR0_EL1, UPMCR0_IMODE,
> + FIELD_PREP(UPMCR0_IMODE, UPMCR0_IMODE_OFF));
>
> /* Commit all of the above */
> isb();
> --
> 2.46.0
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2024-08-31 10:02 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-08-31 5:48 [PATCH v2 0/4] Add support for A7-A11 AIC Nick Chan
2024-08-31 5:48 ` [PATCH v2 1/4] dt-bindings: apple,aic: Document A7-A11 compatibles Nick Chan
2024-08-31 6:34 ` Krzysztof Kozlowski
2024-08-31 6:43 ` Sven Peter
2024-08-31 5:48 ` [PATCH v2 2/4] irqchip/apple-aic: Skip unnecessary setting of use_fast_ipi Nick Chan
2024-08-31 6:48 ` Sven Peter
2024-08-31 5:48 ` [PATCH v2 3/4] irqchip/apple-aic: Add a new "Global fast IPIs only" feature level Nick Chan
2024-08-31 7:05 ` Sven Peter
2024-08-31 5:48 ` [PATCH v2 4/4] irqchip/apple-aic: Only access system registers on SoCs which provide them Nick Chan
2024-08-31 10:01 ` Sven Peter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®