* [PATCH RESEND 0/3] Add AIC support for A7-A11 SoCs
@ 2024-08-29 11:02 Nick Chan
2024-08-29 11:02 ` [PATCH RESEND 1/3] dt-bindings: apple,aic: Document A7-A11 compatibles Nick Chan
` (3 more replies)
0 siblings, 4 replies; 12+ messages in thread
From: Nick Chan @ 2024-08-29 11:02 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
Cc: ~postmarketos/upstreaming, Nick Chan
Resend to correct dt-bindings issues pointed out by Rob.
Hi,
This series is a second attempt at adding support for A7-A11 SoCs to
Linux, it is based on a previous attempt, which you can find at [1].
However, there have been quite a bit of changes.
First, the boot process has changed, now, the boot process includes
a "1337" version of checkra1n [2], a custom PongoOS binary [3], and
a modified version of m1n1 [4]. The kernel is appended to m1n1 and loaded
by it.
This attempt also supports SMP, which has uncovered some differences
in the A7-A11 AIC. Namely, although A11 supported fast IPI, it only
supported "global" fast IPIs via SYS_IMP_APL_IPI_RR_GLOBAL_EL1,
and SYS_IMP_APL_IPI_RR_LOCAL_EL1 does not exist on A11. As a result,
there are now three feature levels:
A7 - A10: No fast IPI
A11: "Global" fast IPI
M1: Global and Local fast IPI
Each feature level is strictly an extension of the previous, for example,
M1 will also work with the A7-A10 compatible. As a result, the
modifications only includes if'ing out of features, in order to make
the existing driver work on older SoCs.
The A10(X) contains P-core and E-core pairs where only one core in each
pair may be active at one time, controlled by CPU frequency. A RFC patch
will be posted to disable 32-bit executable support on A10(X), as it only
supported 16KB page size anyways. However, such a patch is not required
to run AArch64 Linux on A10. At worst, any attempt to run 32-bit
executables will result in the process crashing.
Initial device trees will be posted in a later patch series, likely when
the AIC modifications are accepted.
Asahi Linux downstream kernel note:
These patches will not work with the Asahi Linux downstream kernel,
as these earlier SoCs do not support state retention across deep WFI,
which results in the CPUs going back to RVBAR on cpuidle.
[1]: https://lore.kernel.org/asahi/20221007200022.22844-1-konrad.dybcio@somainline.org/
[2]: https://checkra.in/1337
[3]: https://github.com/asdfugil/pongoOS/tree/mini
[4]: https://github.com/asdfugil/m1n1-idevice
Nick Chan (3):
dt-bindings: apple,aic: Document A7-A11 compatibles
irqchip/apple-aic: Only access IPI sysregs when use_fast_ipi is true
irqchip/apple-aic: Add a new "Global fast IPIs only" feature level
.../interrupt-controller/apple,aic.yaml | 8 ++-
drivers/irqchip/irq-apple-aic.c | 49 ++++++++++++++-----
2 files changed, 43 insertions(+), 14 deletions(-)
--
2.46.0
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH RESEND 1/3] dt-bindings: apple,aic: Document A7-A11 compatibles
2024-08-29 11:02 [PATCH RESEND 0/3] Add AIC support for A7-A11 SoCs Nick Chan
@ 2024-08-29 11:02 ` Nick Chan
2024-08-29 11:46 ` Krzysztof Kozlowski
2024-08-29 11:02 ` [PATCH RESEND 2/3] irqchip/apple-aic: Only access IPI sysregs when use_fast_ipi is true Nick Chan
` (2 subsequent siblings)
3 siblings, 1 reply; 12+ messages in thread
From: Nick Chan @ 2024-08-29 11:02 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
Cc: ~postmarketos/upstreaming, Nick Chan
Document the compatibles for Apple A7-A11 SoCs.
There are three feature levels:
- A7-A10: No fast IPI
- A11: fast IPI, global only
- M1: fast IPI with local and global support
Signed-off-by: Nick Chan <towinchenmi@gmail.com>
---
.../bindings/interrupt-controller/apple,aic.yaml | 8 +++++++-
1 file changed, 7 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..2e38b585a722 100644
--- a/Documentation/devicetree/bindings/interrupt-controller/apple,aic.yaml
+++ b/Documentation/devicetree/bindings/interrupt-controller/apple,aic.yaml
@@ -37,7 +37,13 @@ allOf:
properties:
compatible:
items:
- - const: apple,t8103-aic
+ - enum:
+ - apple,s5l8960x-aic
+ - apple,s8000-aic
+ - apple,t7000-aic
+ - apple,t8010-aic
+ - apple,t8015-aic
+ - apple,t8103-aic
- const: apple,aic
interrupt-controller: true
--
2.46.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH RESEND 2/3] irqchip/apple-aic: Only access IPI sysregs when use_fast_ipi is true
2024-08-29 11:02 [PATCH RESEND 0/3] Add AIC support for A7-A11 SoCs Nick Chan
2024-08-29 11:02 ` [PATCH RESEND 1/3] dt-bindings: apple,aic: Document A7-A11 compatibles Nick Chan
@ 2024-08-29 11:02 ` Nick Chan
2024-08-29 13:08 ` Thomas Gleixner
2024-08-29 11:02 ` [PATCH RESEND 3/3] irqchip/apple-aic: Add a new "Global fast IPIs only" feature level Nick Chan
2024-08-29 11:47 ` [PATCH RESEND 0/3] Add AIC support for A7-A11 SoCs Krzysztof Kozlowski
3 siblings, 1 reply; 12+ messages in thread
From: Nick Chan @ 2024-08-29 11:02 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
Cc: ~postmarketos/upstreaming, Nick Chan, Konrad Dybcio
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.
While at it, remove the IPI-always-ack path on aic_handle_fiq. If we are
able to reach there, we are on an IPI-capable system and should be using
one of the IPI-capable compatibles, anyway.
Signed-off-by: Konrad Dybcio <konrad.dybcio@somainline.org>
Signed-off-by: Nick Chan <towinchenmi@gmail.com>
---
drivers/irqchip/irq-apple-aic.c | 24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/drivers/irqchip/irq-apple-aic.c b/drivers/irqchip/irq-apple-aic.c
index 5c534d9fd2b0..626aaeafa96c 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);
struct aic_info {
@@ -532,13 +533,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)))
@@ -574,8 +571,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,
@@ -811,7 +809,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);
@@ -832,8 +831,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 related [flat|nested] 12+ messages in thread
* [PATCH RESEND 3/3] irqchip/apple-aic: Add a new "Global fast IPIs only" feature level
2024-08-29 11:02 [PATCH RESEND 0/3] Add AIC support for A7-A11 SoCs Nick Chan
2024-08-29 11:02 ` [PATCH RESEND 1/3] dt-bindings: apple,aic: Document A7-A11 compatibles Nick Chan
2024-08-29 11:02 ` [PATCH RESEND 2/3] irqchip/apple-aic: Only access IPI sysregs when use_fast_ipi is true Nick Chan
@ 2024-08-29 11:02 ` Nick Chan
2024-08-29 13:12 ` Thomas Gleixner
2024-08-29 14:06 ` Nick Chan
2024-08-29 11:47 ` [PATCH RESEND 0/3] Add AIC support for A7-A11 SoCs Krzysztof Kozlowski
3 siblings, 2 replies; 12+ messages in thread
From: Nick Chan @ 2024-08-29 11:02 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
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 | 25 ++++++++++++++++++++++++-
1 file changed, 24 insertions(+), 1 deletion(-)
diff --git a/drivers/irqchip/irq-apple-aic.c b/drivers/irqchip/irq-apple-aic.c
index 626aaeafa96c..1640074af2e1 100644
--- a/drivers/irqchip/irq-apple-aic.c
+++ b/drivers/irqchip/irq-apple-aic.c
@@ -236,6 +236,8 @@ enum fiq_hwirq {
/* 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 (M1+) */
+static DEFINE_STATIC_KEY_TRUE(use_local_fast_ipi);
struct aic_info {
int version;
@@ -253,6 +255,7 @@ struct aic_info {
/* Features */
bool fast_ipi;
+ bool local_fast_ipi;
};
static const struct aic_info aic1_info __initconst = {
@@ -271,6 +274,16 @@ 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,
@@ -282,6 +295,10 @@ static const struct aic_info aic2_info __initconst = {
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,
},
{
@@ -748,7 +765,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
@@ -992,6 +1010,11 @@ static int __init aic_of_ic_init(struct device_node *node, struct device_node *p
else
static_branch_disable(&use_fast_ipi);
+ if (irqc->info.local_fast_ipi)
+ static_branch_enable(&use_local_fast_ipi);
+ else
+ 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 related [flat|nested] 12+ messages in thread
* Re: [PATCH RESEND 1/3] dt-bindings: apple,aic: Document A7-A11 compatibles
2024-08-29 11:02 ` [PATCH RESEND 1/3] dt-bindings: apple,aic: Document A7-A11 compatibles Nick Chan
@ 2024-08-29 11:46 ` Krzysztof Kozlowski
2024-08-29 12:00 ` Nick Chan
0 siblings, 1 reply; 12+ messages in thread
From: Krzysztof Kozlowski @ 2024-08-29 11:46 UTC (permalink / raw)
To: Nick Chan, Hector Martin, Sven Peter, Alyssa Rosenzweig,
Thomas Gleixner, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
asahi, linux-arm-kernel, linux-kernel, devicetree
Cc: ~postmarketos/upstreaming
On 29/08/2024 13:02, Nick Chan wrote:
> Document the compatibles for Apple A7-A11 SoCs.
>
> There are three feature levels:
> - A7-A10: No fast IPI
> - A11: fast IPI, global only
> - M1: fast IPI with local and global support
>
> Signed-off-by: Nick Chan <towinchenmi@gmail.com>
> ---
Please do not resend different patch. Or rather explain - is this the
same? Looks different, so RESEND is not appropriate.
Follow submitting patches in this regard, you need v2. Just use b4 for
this process.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH RESEND 0/3] Add AIC support for A7-A11 SoCs
2024-08-29 11:02 [PATCH RESEND 0/3] Add AIC support for A7-A11 SoCs Nick Chan
` (2 preceding siblings ...)
2024-08-29 11:02 ` [PATCH RESEND 3/3] irqchip/apple-aic: Add a new "Global fast IPIs only" feature level Nick Chan
@ 2024-08-29 11:47 ` Krzysztof Kozlowski
3 siblings, 0 replies; 12+ messages in thread
From: Krzysztof Kozlowski @ 2024-08-29 11:47 UTC (permalink / raw)
To: Nick Chan, Hector Martin, Sven Peter, Alyssa Rosenzweig,
Thomas Gleixner, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
asahi, linux-arm-kernel, linux-kernel, devicetree
Cc: ~postmarketos/upstreaming
On 29/08/2024 13:02, Nick Chan wrote:
> Resend to correct dt-bindings issues pointed out by Rob.
RESEND means the same patch, so nothing gets corrected. How the tools
should know which patch to pick up?
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH RESEND 1/3] dt-bindings: apple,aic: Document A7-A11 compatibles
2024-08-29 11:46 ` Krzysztof Kozlowski
@ 2024-08-29 12:00 ` Nick Chan
2024-08-29 12:37 ` Krzysztof Kozlowski
0 siblings, 1 reply; 12+ messages in thread
From: Nick Chan @ 2024-08-29 12:00 UTC (permalink / raw)
To: Krzysztof Kozlowski, Hector Martin, Sven Peter, Alyssa Rosenzweig,
Thomas Gleixner, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
asahi, linux-arm-kernel, linux-kernel, devicetree
Cc: ~postmarketos/upstreaming
On 29/8/2024 19:46, Krzysztof Kozlowski wrote:
> Please do not resend different patch. Or rather explain - is this the
> same? Looks different, so RESEND is not appropriate.
>
> Follow submitting patches in this regard, you need v2. Just use b4 for
> this process.
Right. Since versions of patch series should not be sent too frequently,
I suppose a v2 should be sent next week. (Sorry - not super familiar
with the process)
>
> Best regards,
Nick Chan
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH RESEND 1/3] dt-bindings: apple,aic: Document A7-A11 compatibles
2024-08-29 12:00 ` Nick Chan
@ 2024-08-29 12:37 ` Krzysztof Kozlowski
0 siblings, 0 replies; 12+ messages in thread
From: Krzysztof Kozlowski @ 2024-08-29 12:37 UTC (permalink / raw)
To: Nick Chan, Hector Martin, Sven Peter, Alyssa Rosenzweig,
Thomas Gleixner, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
asahi, linux-arm-kernel, linux-kernel, devicetree
Cc: ~postmarketos/upstreaming
On 29/08/2024 14:00, Nick Chan wrote:
>
>
> On 29/8/2024 19:46, Krzysztof Kozlowski wrote:
>> Please do not resend different patch. Or rather explain - is this the
>> same? Looks different, so RESEND is not appropriate.
>>
>> Follow submitting patches in this regard, you need v2. Just use b4 for
>> this process.
> Right. Since versions of patch series should not be sent too frequently,
> I suppose a v2 should be sent next week. (Sorry - not super familiar
> with the process)
Once per day, so other reviewers will have time to respond. I also
responded to your earlier posting, before I noticed the resend, so
please improve the commit msg as I suggested.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH RESEND 2/3] irqchip/apple-aic: Only access IPI sysregs when use_fast_ipi is true
2024-08-29 11:02 ` [PATCH RESEND 2/3] irqchip/apple-aic: Only access IPI sysregs when use_fast_ipi is true Nick Chan
@ 2024-08-29 13:08 ` Thomas Gleixner
2024-08-29 13:48 ` Nick Chan
0 siblings, 1 reply; 12+ messages in thread
From: Thomas Gleixner @ 2024-08-29 13:08 UTC (permalink / raw)
To: Nick Chan, Hector Martin, Sven Peter, Alyssa Rosenzweig,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, asahi,
linux-arm-kernel, linux-kernel, devicetree
Cc: ~postmarketos/upstreaming, Nick Chan, Konrad Dybcio
On Thu, Aug 29 2024 at 19:02, Nick Chan wrote:
> 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.
use_fast_ipi is an implementation detail and does not mean anything
here. It's sufficient to say:
Only access system registers on SoCs which provide them.
Hmm?
> While at it, remove the IPI-always-ack path on aic_handle_fiq.
It's not while at it. It's part of handling this correctly.
> If we are able to reach there, we are on an IPI-capable system and
> should be using one of the IPI-capable compatibles, anyway.
'we' can't reach that code ever.
https://www.kernel.org/doc/html/latest/process/maintainer-tip.html#changelog
> Signed-off-by: Konrad Dybcio <konrad.dybcio@somainline.org>
> Signed-off-by: Nick Chan <towinchenmi@gmail.com>
This Signed-off-by chain is invalid. If Konrad authored the patch then
you need to have a 'From: Konrad ...' line at the top of the change log.
If you worked together on this then this needs a Co-developed-by
tag. See Documentation/process/...
>
> - 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)))
> @@ -574,8 +571,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,
> @@ -811,7 +809,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);
> @@ -832,8 +831,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));
Please see the bracket rules in the tip maintainers doc.
Thanks,
tglx
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH RESEND 3/3] irqchip/apple-aic: Add a new "Global fast IPIs only" feature level
2024-08-29 11:02 ` [PATCH RESEND 3/3] irqchip/apple-aic: Add a new "Global fast IPIs only" feature level Nick Chan
@ 2024-08-29 13:12 ` Thomas Gleixner
2024-08-29 14:06 ` Nick Chan
1 sibling, 0 replies; 12+ messages in thread
From: Thomas Gleixner @ 2024-08-29 13:12 UTC (permalink / raw)
To: Nick Chan, Hector Martin, Sven Peter, Alyssa Rosenzweig,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, asahi,
linux-arm-kernel, linux-kernel, devicetree
Cc: ~postmarketos/upstreaming, Nick Chan
On Thu, Aug 29 2024 at 19:02, Nick Chan wrote:
>
> + if (irqc->info.local_fast_ipi)
> + static_branch_enable(&use_local_fast_ipi);
> + else
> + static_branch_disable(&use_local_fast_ipi);
As you define the key to be true by default there is no point to enable
the branch some more. Copy & pasta ...
Thanks,
tglx
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH RESEND 2/3] irqchip/apple-aic: Only access IPI sysregs when use_fast_ipi is true
2024-08-29 13:08 ` Thomas Gleixner
@ 2024-08-29 13:48 ` Nick Chan
0 siblings, 0 replies; 12+ messages in thread
From: Nick Chan @ 2024-08-29 13:48 UTC (permalink / raw)
To: Thomas Gleixner, Hector Martin, Sven Peter, Alyssa Rosenzweig,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, asahi,
linux-arm-kernel, linux-kernel, devicetree
Cc: ~postmarketos/upstreaming, Konrad Dybcio
On 29/8/2024 21:08, Thomas Gleixner wrote:
> On Thu, Aug 29 2024 at 19:02, Nick Chan wrote:
>> 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.
>
> use_fast_ipi is an implementation detail and does not mean anything
> here. It's sufficient to say:
>
> Only access system registers on SoCs which provide them.
>
> Hmm?
Seems like a good idea. Will be in v2. Though if the commit message
is that generic, do you think it's correct to squash the third patch
into this commit? It is also preventing sysreg access on SoC that
do not provide them.
>
>> While at it, remove the IPI-always-ack path on aic_handle_fiq.
>
> It's not while at it. It's part of handling this correctly.
In v2 it will be mentioned as as part of the sysreg access restriction.
>
>> If we are able to reach there, we are on an IPI-capable system and
>> should be using one of the IPI-capable compatibles, anyway.
>
> 'we' can't reach that code ever.
>
> https://www.kernel.org/doc/html/latest/process/maintainer-tip.html#changelog
Acked. In v2 the commit message will not impersonate the code anymore.
>
>
>> Signed-off-by: Konrad Dybcio <konrad.dybcio@somainline.org>
>> Signed-off-by: Nick Chan <towinchenmi@gmail.com>
>
> This Signed-off-by chain is invalid. If Konrad authored the patch then
> you need to have a 'From: Konrad ...' line at the top of the change log.
>
> If you worked together on this then this needs a Co-developed-by
> tag. See Documentation/process/...
This patch was entirely made by Konrad, but now that changes are requested,
it will be a Co-developed-by in v2.
>
>>
>> - 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)))
>> @@ -574,8 +571,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,
>> @@ -811,7 +809,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);
>> @@ -832,8 +831,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));
>
> Please see the bracket rules in the tip maintainers doc.
Acked. Brackets will be added to function references in commit message
of v2.
>
> Thanks,
>
> tglx
Nick Chan
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH RESEND 3/3] irqchip/apple-aic: Add a new "Global fast IPIs only" feature level
2024-08-29 11:02 ` [PATCH RESEND 3/3] irqchip/apple-aic: Add a new "Global fast IPIs only" feature level Nick Chan
2024-08-29 13:12 ` Thomas Gleixner
@ 2024-08-29 14:06 ` Nick Chan
1 sibling, 0 replies; 12+ messages in thread
From: Nick Chan @ 2024-08-29 14:06 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
Cc: ~postmarketos/upstreaming
On 29/8/2024 19:02, Nick Chan 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>
> ---
> drivers/irqchip/irq-apple-aic.c | 25 ++++++++++++++++++++++++-
> 1 file changed, 24 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/irqchip/irq-apple-aic.c b/drivers/irqchip/irq-apple-aic.c
> index 626aaeafa96c..1640074af2e1 100644
> --- a/drivers/irqchip/irq-apple-aic.c
> +++ b/drivers/irqchip/irq-apple-aic.c
> @@ -236,6 +236,8 @@ enum fiq_hwirq {
>
> /* 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 (M1+) */
> +static DEFINE_STATIC_KEY_TRUE(use_local_fast_ipi);
>
> struct aic_info {
> int version;
> @@ -253,6 +255,7 @@ struct aic_info {
>
> /* Features */
> bool fast_ipi;
> + bool local_fast_ipi;
> };
>
> static const struct aic_info aic1_info __initconst = {
> @@ -271,6 +274,16 @@ 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 = {
This patch is incorrectly disabling local fast IPI on aic2, it will be
corrected in v2.
> .version = 2,
>
> @@ -282,6 +295,10 @@ static const struct aic_info aic2_info __initconst = {
> 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,
> },
> {
> @@ -748,7 +765,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
> @@ -992,6 +1010,11 @@ static int __init aic_of_ic_init(struct device_node *node, struct device_node *p
> else
> static_branch_disable(&use_fast_ipi);
>
> + if (irqc->info.local_fast_ipi)
> + static_branch_enable(&use_local_fast_ipi);
> + else
> + 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),
Nick Chan
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2024-08-29 14:07 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-08-29 11:02 [PATCH RESEND 0/3] Add AIC support for A7-A11 SoCs Nick Chan
2024-08-29 11:02 ` [PATCH RESEND 1/3] dt-bindings: apple,aic: Document A7-A11 compatibles Nick Chan
2024-08-29 11:46 ` Krzysztof Kozlowski
2024-08-29 12:00 ` Nick Chan
2024-08-29 12:37 ` Krzysztof Kozlowski
2024-08-29 11:02 ` [PATCH RESEND 2/3] irqchip/apple-aic: Only access IPI sysregs when use_fast_ipi is true Nick Chan
2024-08-29 13:08 ` Thomas Gleixner
2024-08-29 13:48 ` Nick Chan
2024-08-29 11:02 ` [PATCH RESEND 3/3] irqchip/apple-aic: Add a new "Global fast IPIs only" feature level Nick Chan
2024-08-29 13:12 ` Thomas Gleixner
2024-08-29 14:06 ` Nick Chan
2024-08-29 11:47 ` [PATCH RESEND 0/3] Add AIC support for A7-A11 SoCs Krzysztof Kozlowski
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).