Add support for A7-A11 SoCs by if-ing out some features only present on
A12 & newer (UNCORE2 registers) or M1 & newer (EL2 registers - the
older SoCs don't implement EL2).
Also, annotate IPI regs support (A11 and newer*) so that the driver can
tell whether the SoC supports these (they are written to even if fast
IPI is disabled, when the registers are there of course).
*A11 is supposed to use this feature, but it is currently not working.
That said, it is not yet necessary, especially with only one core up,
and it works a-ok using the same featureset as earlier SoCs.
Signed-off-by: Konrad Dybcio <[email protected]>
---
drivers/irqchip/irq-apple-aic.c | 54 +++++++++++++++++++++++----------
1 file changed, 38 insertions(+), 16 deletions(-)
diff --git a/drivers/irqchip/irq-apple-aic.c b/drivers/irqchip/irq-apple-aic.c
index 12dd48727a15..36f4b52addc2 100644
--- a/drivers/irqchip/irq-apple-aic.c
+++ b/drivers/irqchip/irq-apple-aic.c
@@ -245,7 +245,10 @@ struct aic_info {
u32 die_stride;
/* Features */
+ bool el2_regs;
bool fast_ipi;
+ bool ipi_regs;
+ bool uncore2_regs;
};
static const struct aic_info aic1_info = {
@@ -261,7 +264,10 @@ static const struct aic_info aic1_fipi_info = {
.event = AIC_EVENT,
.target_cpu = AIC_TARGET_CPU,
+ .el2_regs = true,
.fast_ipi = true,
+ .ipi_regs = true,
+ .uncore2_regs = true,
};
static const struct aic_info aic2_info = {
@@ -269,7 +275,10 @@ static const struct aic_info aic2_info = {
.irq_cfg = AIC2_IRQ_CFG,
+ .el2_regs = true,
.fast_ipi = true,
+ .ipi_regs = true,
+ .uncore2_regs = true,
};
static const struct of_device_id aic_info_match[] = {
@@ -452,6 +461,9 @@ static unsigned long aic_fiq_get_idx(struct irq_data *d)
static void aic_fiq_set_mask(struct irq_data *d)
{
+ if (!aic_irqc->info.el2_regs)
+ return;
+
/* Only the guest timers have real mask bits, unfortunately. */
switch (aic_fiq_get_idx(d)) {
case AIC_TMR_EL02_PHYS:
@@ -469,6 +481,9 @@ static void aic_fiq_set_mask(struct irq_data *d)
static void aic_fiq_clear_mask(struct irq_data *d)
{
+ if (!aic_irqc->info.el2_regs)
+ return;
+
switch (aic_fiq_get_idx(d)) {
case AIC_TMR_EL02_PHYS:
sysreg_clear_set_s(SYS_IMP_APL_VM_TMR_FIQ_ENA_EL2, 0, VM_TMR_FIQ_ENABLE_P);
@@ -524,12 +539,14 @@ 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 (aic_irqc->info.ipi_regs) {
+ 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);
+ }
}
}
@@ -566,12 +583,14 @@ 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)) {
- /* 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,
- FIELD_PREP(UPMCR0_IMODE, UPMCR0_IMODE_OFF));
+ if (aic_irqc->info.uncore2_regs) {
+ 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)) {
+ /* 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,
+ FIELD_PREP(UPMCR0_IMODE, UPMCR0_IMODE_OFF));
+ }
}
}
@@ -676,7 +695,8 @@ static int aic_irq_domain_translate(struct irq_domain *id,
break;
case AIC_TMR_HV_PHYS:
case AIC_TMR_HV_VIRT:
- return -ENOENT;
+ if (aic_irqc->info.el2_regs)
+ return -ENOENT;
default:
break;
}
@@ -944,7 +964,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 (aic_irqc->info.ipi_regs)
+ 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);
@@ -965,8 +986,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 (aic_irqc->info.uncore2_regs)
+ 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.37.0
Tested-by: Nick Chan <[email protected]> # iPad Pro 9.7 Inch (Wi-Fi), iPhone X (Global)
On 12.07.2022 20:23, Alyssa Rosenzweig wrote:
> I don't see where you actually set the new features to false, or indeed
> check the compatible at all? Am I missing a patch?
No, it's okay. The 'features' are enabled for aic1_fipi (M1) and
aic2 (newer) and not enabled (as in the struct is partially
initialized so they are implicitly 0-initialized) on aic1, so that
'the most basic' compatible works for all chipsets.
Konrad
>
>> Add support for A7-A11 SoCs by if-ing out some features only present on
>> A12 & newer (UNCORE2 registers) or M1 & newer (EL2 registers - the
>> older SoCs don't implement EL2).
>>
>> Also, annotate IPI regs support (A11 and newer*) so that the driver can
>> tell whether the SoC supports these (they are written to even if fast
>> IPI is disabled, when the registers are there of course).
>>
>> *A11 is supposed to use this feature, but it is currently not working.
>> That said, it is not yet necessary, especially with only one core up,
>> and it works a-ok using the same featureset as earlier SoCs.
>>
>> Signed-off-by: Konrad Dybcio <[email protected]>
>> ---
>> drivers/irqchip/irq-apple-aic.c | 54 +++++++++++++++++++++++----------
>> 1 file changed, 38 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/irqchip/irq-apple-aic.c b/drivers/irqchip/irq-apple-aic.c
>> index 12dd48727a15..36f4b52addc2 100644
>> --- a/drivers/irqchip/irq-apple-aic.c
>> +++ b/drivers/irqchip/irq-apple-aic.c
>> @@ -245,7 +245,10 @@ struct aic_info {
>> u32 die_stride;
>>
>> /* Features */
>> + bool el2_regs;
>> bool fast_ipi;
>> + bool ipi_regs;
>> + bool uncore2_regs;
>> };
>>
>> static const struct aic_info aic1_info = {
>> @@ -261,7 +264,10 @@ static const struct aic_info aic1_fipi_info = {
>> .event = AIC_EVENT,
>> .target_cpu = AIC_TARGET_CPU,
>>
>> + .el2_regs = true,
>> .fast_ipi = true,
>> + .ipi_regs = true,
>> + .uncore2_regs = true,
>> };
>>
>> static const struct aic_info aic2_info = {
>> @@ -269,7 +275,10 @@ static const struct aic_info aic2_info = {
>>
>> .irq_cfg = AIC2_IRQ_CFG,
>>
>> + .el2_regs = true,
>> .fast_ipi = true,
>> + .ipi_regs = true,
>> + .uncore2_regs = true,
>> };
>>
>> static const struct of_device_id aic_info_match[] = {
>> @@ -452,6 +461,9 @@ static unsigned long aic_fiq_get_idx(struct irq_data *d)
>>
>> static void aic_fiq_set_mask(struct irq_data *d)
>> {
>> + if (!aic_irqc->info.el2_regs)
>> + return;
>> +
>> /* Only the guest timers have real mask bits, unfortunately. */
>> switch (aic_fiq_get_idx(d)) {
>> case AIC_TMR_EL02_PHYS:
>> @@ -469,6 +481,9 @@ static void aic_fiq_set_mask(struct irq_data *d)
>>
>> static void aic_fiq_clear_mask(struct irq_data *d)
>> {
>> + if (!aic_irqc->info.el2_regs)
>> + return;
>> +
>> switch (aic_fiq_get_idx(d)) {
>> case AIC_TMR_EL02_PHYS:
>> sysreg_clear_set_s(SYS_IMP_APL_VM_TMR_FIQ_ENA_EL2, 0, VM_TMR_FIQ_ENABLE_P);
>> @@ -524,12 +539,14 @@ 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 (aic_irqc->info.ipi_regs) {
>> + 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);
>> + }
>> }
>> }
>>
>> @@ -566,12 +583,14 @@ 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)) {
>> - /* 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,
>> - FIELD_PREP(UPMCR0_IMODE, UPMCR0_IMODE_OFF));
>> + if (aic_irqc->info.uncore2_regs) {
>> + 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)) {
>> + /* 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,
>> + FIELD_PREP(UPMCR0_IMODE, UPMCR0_IMODE_OFF));
>> + }
>> }
>> }
>>
>> @@ -676,7 +695,8 @@ static int aic_irq_domain_translate(struct irq_domain *id,
>> break;
>> case AIC_TMR_HV_PHYS:
>> case AIC_TMR_HV_VIRT:
>> - return -ENOENT;
>> + if (aic_irqc->info.el2_regs)
>> + return -ENOENT;
>> default:
>> break;
>> }
>> @@ -944,7 +964,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 (aic_irqc->info.ipi_regs)
>> + 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);
>> @@ -965,8 +986,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 (aic_irqc->info.uncore2_regs)
>> + 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.37.0
>>
marcan probably has to review this in detail but two comments from me:
On Tue, Jul 12, 2022, at 18:09, Konrad Dybcio wrote:
> Add support for A7-A11 SoCs by if-ing out some features only present on
> A12 & newer (UNCORE2 registers) or M1 & newer (EL2 registers - the
> older SoCs don't implement EL2).
>
> Also, annotate IPI regs support (A11 and newer*) so that the driver can
> tell whether the SoC supports these (they are written to even if fast
> IPI is disabled, when the registers are there of course).
>
> *A11 is supposed to use this feature, but it is currently not working.
> That said, it is not yet necessary, especially with only one core up,
> and it works a-ok using the same featureset as earlier SoCs.
>
> Signed-off-by: Konrad Dybcio <[email protected]>
> ---
> drivers/irqchip/irq-apple-aic.c | 54 +++++++++++++++++++++++----------
> 1 file changed, 38 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/irqchip/irq-apple-aic.c b/drivers/irqchip/irq-apple-aic.c
> index 12dd48727a15..36f4b52addc2 100644
> --- a/drivers/irqchip/irq-apple-aic.c
> +++ b/drivers/irqchip/irq-apple-aic.c
> @@ -245,7 +245,10 @@ struct aic_info {
> u32 die_stride;
>
> /* Features */
> + bool el2_regs;
> bool fast_ipi;
> + bool ipi_regs;
> + bool uncore2_regs;
I don't quite understand the difference between fast_ipi and ipi_regs.
Don't we always have fast_ipi suppport when those regs are available?
> };
>
> static const struct aic_info aic1_info = {
> @@ -261,7 +264,10 @@ static const struct aic_info aic1_fipi_info = {
> .event = AIC_EVENT,
> .target_cpu = AIC_TARGET_CPU,
>
> + .el2_regs = true,
> .fast_ipi = true,
> + .ipi_regs = true,
> + .uncore2_regs = true,
> };
>
> static const struct aic_info aic2_info = {
> @@ -269,7 +275,10 @@ static const struct aic_info aic2_info = {
>
> .irq_cfg = AIC2_IRQ_CFG,
>
> + .el2_regs = true,
> .fast_ipi = true,
> + .ipi_regs = true,
> + .uncore2_regs = true,
> };
>
> static const struct of_device_id aic_info_match[] = {
> @@ -452,6 +461,9 @@ static unsigned long aic_fiq_get_idx(struct irq_data *d)
>
> static void aic_fiq_set_mask(struct irq_data *d)
> {
> + if (!aic_irqc->info.el2_regs)
> + return;
> +
> /* Only the guest timers have real mask bits, unfortunately. */
> switch (aic_fiq_get_idx(d)) {
> case AIC_TMR_EL02_PHYS:
> @@ -469,6 +481,9 @@ static void aic_fiq_set_mask(struct irq_data *d)
>
> static void aic_fiq_clear_mask(struct irq_data *d)
> {
> + if (!aic_irqc->info.el2_regs)
> + return;
> +
> switch (aic_fiq_get_idx(d)) {
> case AIC_TMR_EL02_PHYS:
> sysreg_clear_set_s(SYS_IMP_APL_VM_TMR_FIQ_ENA_EL2, 0,
> VM_TMR_FIQ_ENABLE_P);
> @@ -524,12 +539,14 @@ 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 (aic_irqc->info.ipi_regs) {
> + 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);
> + }
> }
> }
This is a pretty hot path and the use_fast_ipi check uses the jump label support
(static_branch_likely, static_branch_enable) to avoid dereferencing memory here.
We'll probably want the same for the other features.
For this branch here the else can probably just be removed: I think that's
a leftover from when this driver just didn't support fastipi at all even
when the registers were available.
Sven
I don't see where you actually set the new features to false, or indeed
check the compatible at all? Am I missing a patch?
> Add support for A7-A11 SoCs by if-ing out some features only present on
> A12 & newer (UNCORE2 registers) or M1 & newer (EL2 registers - the
> older SoCs don't implement EL2).
>
> Also, annotate IPI regs support (A11 and newer*) so that the driver can
> tell whether the SoC supports these (they are written to even if fast
> IPI is disabled, when the registers are there of course).
>
> *A11 is supposed to use this feature, but it is currently not working.
> That said, it is not yet necessary, especially with only one core up,
> and it works a-ok using the same featureset as earlier SoCs.
>
> Signed-off-by: Konrad Dybcio <[email protected]>
> ---
> drivers/irqchip/irq-apple-aic.c | 54 +++++++++++++++++++++++----------
> 1 file changed, 38 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/irqchip/irq-apple-aic.c b/drivers/irqchip/irq-apple-aic.c
> index 12dd48727a15..36f4b52addc2 100644
> --- a/drivers/irqchip/irq-apple-aic.c
> +++ b/drivers/irqchip/irq-apple-aic.c
> @@ -245,7 +245,10 @@ struct aic_info {
> u32 die_stride;
>
> /* Features */
> + bool el2_regs;
> bool fast_ipi;
> + bool ipi_regs;
> + bool uncore2_regs;
> };
>
> static const struct aic_info aic1_info = {
> @@ -261,7 +264,10 @@ static const struct aic_info aic1_fipi_info = {
> .event = AIC_EVENT,
> .target_cpu = AIC_TARGET_CPU,
>
> + .el2_regs = true,
> .fast_ipi = true,
> + .ipi_regs = true,
> + .uncore2_regs = true,
> };
>
> static const struct aic_info aic2_info = {
> @@ -269,7 +275,10 @@ static const struct aic_info aic2_info = {
>
> .irq_cfg = AIC2_IRQ_CFG,
>
> + .el2_regs = true,
> .fast_ipi = true,
> + .ipi_regs = true,
> + .uncore2_regs = true,
> };
>
> static const struct of_device_id aic_info_match[] = {
> @@ -452,6 +461,9 @@ static unsigned long aic_fiq_get_idx(struct irq_data *d)
>
> static void aic_fiq_set_mask(struct irq_data *d)
> {
> + if (!aic_irqc->info.el2_regs)
> + return;
> +
> /* Only the guest timers have real mask bits, unfortunately. */
> switch (aic_fiq_get_idx(d)) {
> case AIC_TMR_EL02_PHYS:
> @@ -469,6 +481,9 @@ static void aic_fiq_set_mask(struct irq_data *d)
>
> static void aic_fiq_clear_mask(struct irq_data *d)
> {
> + if (!aic_irqc->info.el2_regs)
> + return;
> +
> switch (aic_fiq_get_idx(d)) {
> case AIC_TMR_EL02_PHYS:
> sysreg_clear_set_s(SYS_IMP_APL_VM_TMR_FIQ_ENA_EL2, 0, VM_TMR_FIQ_ENABLE_P);
> @@ -524,12 +539,14 @@ 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 (aic_irqc->info.ipi_regs) {
> + 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);
> + }
> }
> }
>
> @@ -566,12 +583,14 @@ 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)) {
> - /* 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,
> - FIELD_PREP(UPMCR0_IMODE, UPMCR0_IMODE_OFF));
> + if (aic_irqc->info.uncore2_regs) {
> + 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)) {
> + /* 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,
> + FIELD_PREP(UPMCR0_IMODE, UPMCR0_IMODE_OFF));
> + }
> }
> }
>
> @@ -676,7 +695,8 @@ static int aic_irq_domain_translate(struct irq_domain *id,
> break;
> case AIC_TMR_HV_PHYS:
> case AIC_TMR_HV_VIRT:
> - return -ENOENT;
> + if (aic_irqc->info.el2_regs)
> + return -ENOENT;
> default:
> break;
> }
> @@ -944,7 +964,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 (aic_irqc->info.ipi_regs)
> + 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);
> @@ -965,8 +986,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 (aic_irqc->info.uncore2_regs)
> + 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.37.0
>
Hi Konrad,
Please add a cover letter when sending more than a single patch.
On Tue, 12 Jul 2022 17:09:19 +0100,
Konrad Dybcio <[email protected]> wrote:
>
> Add support for A7-A11 SoCs by if-ing out some features only present on
> A12 & newer (UNCORE2 registers) or M1 & newer (EL2 registers - the
> older SoCs don't implement EL2).
>
> Also, annotate IPI regs support (A11 and newer*) so that the driver can
> tell whether the SoC supports these (they are written to even if fast
> IPI is disabled, when the registers are there of course).
>
> *A11 is supposed to use this feature, but it is currently not working.
> That said, it is not yet necessary, especially with only one core up,
> and it works a-ok using the same featureset as earlier SoCs.
>
> Signed-off-by: Konrad Dybcio <[email protected]>
> ---
> drivers/irqchip/irq-apple-aic.c | 54 +++++++++++++++++++++++----------
> 1 file changed, 38 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/irqchip/irq-apple-aic.c b/drivers/irqchip/irq-apple-aic.c
> index 12dd48727a15..36f4b52addc2 100644
> --- a/drivers/irqchip/irq-apple-aic.c
> +++ b/drivers/irqchip/irq-apple-aic.c
> @@ -245,7 +245,10 @@ struct aic_info {
> u32 die_stride;
>
> /* Features */
> + bool el2_regs;
> bool fast_ipi;
> + bool ipi_regs;
> + bool uncore2_regs;
> };
>
> static const struct aic_info aic1_info = {
> @@ -261,7 +264,10 @@ static const struct aic_info aic1_fipi_info = {
> .event = AIC_EVENT,
> .target_cpu = AIC_TARGET_CPU,
>
> + .el2_regs = true,
> .fast_ipi = true,
> + .ipi_regs = true,
> + .uncore2_regs = true,
> };
>
> static const struct aic_info aic2_info = {
> @@ -269,7 +275,10 @@ static const struct aic_info aic2_info = {
>
> .irq_cfg = AIC2_IRQ_CFG,
>
> + .el2_regs = true,
> .fast_ipi = true,
> + .ipi_regs = true,
> + .uncore2_regs = true,
So to sum it up, all recent cores have all the cool features, and the
older ones have none of them. Surely we can do better than adding 3
fields that have the same value. Turn 'fast_ipi' into something that
means 'full_fat', and key everything on that.
And if this is meant to evolve into a more differentiated set of
features, the usual idiom is to have a set of flags as part of an
unsigned long instead of a set of booleans.
> };
>
> static const struct of_device_id aic_info_match[] = {
> @@ -452,6 +461,9 @@ static unsigned long aic_fiq_get_idx(struct irq_data *d)
>
> static void aic_fiq_set_mask(struct irq_data *d)
> {
> + if (!aic_irqc->info.el2_regs)
> + return;
Why? AIC_TMR_EL02_PHYS is defined as the interrupt that fires in the
context of a guest. There is no guest here (no EL2 either), so what
you should have as interrupt number is AIC_TMR_EL0_{PHYS,VIRT}, and
this change becomes irrelevant (nothing to mask). Which is also what
happens when running an M1 under the m1n1 hypervisor.
> +
> /* Only the guest timers have real mask bits, unfortunately. */
> switch (aic_fiq_get_idx(d)) {
> case AIC_TMR_EL02_PHYS:
> @@ -469,6 +481,9 @@ static void aic_fiq_set_mask(struct irq_data *d)
>
> static void aic_fiq_clear_mask(struct irq_data *d)
> {
> + if (!aic_irqc->info.el2_regs)
> + return;
> +
> switch (aic_fiq_get_idx(d)) {
> case AIC_TMR_EL02_PHYS:
> sysreg_clear_set_s(SYS_IMP_APL_VM_TMR_FIQ_ENA_EL2, 0, VM_TMR_FIQ_ENABLE_P);
> @@ -524,12 +539,14 @@ 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 (aic_irqc->info.ipi_regs) {
This is probably the hottest path in the whole kernel. Do we want an
extra read here? Absolutely not. At the very least, this should be a
static key.
> + 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);
> + }
> }
> }
>
> @@ -566,12 +583,14 @@ 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)) {
> - /* 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,
> - FIELD_PREP(UPMCR0_IMODE, UPMCR0_IMODE_OFF));
> + if (aic_irqc->info.uncore2_regs) {
Same thing.
> + 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)) {
> + /* 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,
> + FIELD_PREP(UPMCR0_IMODE, UPMCR0_IMODE_OFF));
> + }
> }
> }
>
> @@ -676,7 +695,8 @@ static int aic_irq_domain_translate(struct irq_domain *id,
> break;
> case AIC_TMR_HV_PHYS:
> case AIC_TMR_HV_VIRT:
> - return -ENOENT;
> + if (aic_irqc->info.el2_regs)
> + return -ENOENT;
See my comment above about the use of these interrupt numbers.
> default:
> break;
> }
> @@ -944,7 +964,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 (aic_irqc->info.ipi_regs)
> + 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);
> @@ -965,8 +986,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 (aic_irqc->info.uncore2_regs)
> + sysreg_clear_set_s(SYS_IMP_APL_UPMCR0_EL1, UPMCR0_IMODE,
> + FIELD_PREP(UPMCR0_IMODE, UPMCR0_IMODE_OFF));
>
> /* Commit all of the above */
> isb();
I must be missing something though. Where is the code that actually
enables support for the SoCs mentioned in $SUBJECT?
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
On 12.07.2022 21:12, Marc Zyngier wrote:
> Hi Konrad,
>
> Please add a cover letter when sending more than a single patch.
>
> On Tue, 12 Jul 2022 17:09:19 +0100,
> Konrad Dybcio <[email protected]> wrote:
>>
>> Add support for A7-A11 SoCs by if-ing out some features only present on
>> A12 & newer (UNCORE2 registers) or M1 & newer (EL2 registers - the
>> older SoCs don't implement EL2).
>>
>> Also, annotate IPI regs support (A11 and newer*) so that the driver can
>> tell whether the SoC supports these (they are written to even if fast
>> IPI is disabled, when the registers are there of course).
>>
>> *A11 is supposed to use this feature, but it is currently not working.
>> That said, it is not yet necessary, especially with only one core up,
>> and it works a-ok using the same featureset as earlier SoCs.
>>
>> Signed-off-by: Konrad Dybcio <[email protected]>
>> ---
>> drivers/irqchip/irq-apple-aic.c | 54 +++++++++++++++++++++++----------
>> 1 file changed, 38 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/irqchip/irq-apple-aic.c b/drivers/irqchip/irq-apple-aic.c
>> index 12dd48727a15..36f4b52addc2 100644
>> --- a/drivers/irqchip/irq-apple-aic.c
>> +++ b/drivers/irqchip/irq-apple-aic.c
>> @@ -245,7 +245,10 @@ struct aic_info {
>> u32 die_stride;
>>
>> /* Features */
>> + bool el2_regs;
>> bool fast_ipi;
>> + bool ipi_regs;
>> + bool uncore2_regs;
>> };
>>
>> static const struct aic_info aic1_info = {
>> @@ -261,7 +264,10 @@ static const struct aic_info aic1_fipi_info = {
>> .event = AIC_EVENT,
>> .target_cpu = AIC_TARGET_CPU,
>>
>> + .el2_regs = true,
>> .fast_ipi = true,
>> + .ipi_regs = true,
>> + .uncore2_regs = true,
>> };
>>
>> static const struct aic_info aic2_info = {
>> @@ -269,7 +275,10 @@ static const struct aic_info aic2_info = {
>>
>> .irq_cfg = AIC2_IRQ_CFG,
>>
>> + .el2_regs = true,
>> .fast_ipi = true,
>> + .ipi_regs = true,
>> + .uncore2_regs = true,
>
> So to sum it up, all recent cores have all the cool features, and the
> older ones have none of them. Surely we can do better than adding 3
> fields that have the same value. Turn 'fast_ipi' into something that
> means 'full_fat', and key everything on that.
>
> And if this is meant to evolve into a more differentiated set of
> features, the usual idiom is to have a set of flags as part of an
> unsigned long instead of a set of booleans.
The latter would be true if a bootrom exploit or any equivalent means
of booting Linux would be found for A12 (M1 is family with A14 for context).
We can think of 4 feature levels, I think:
A7-A10: 'nothing fancy'
A11: fast_ipi (broken currently, need to investigate)
A12: A11 + UNCORE2 regs
M1+: A12 + EL2
We *could* squash the A12-A14 case into M1, but then if a means of booting
Linux appears, this would have to be untangled again..
>
>> };
>>
>> static const struct of_device_id aic_info_match[] = {
>> @@ -452,6 +461,9 @@ static unsigned long aic_fiq_get_idx(struct irq_data *d)
>>
>> static void aic_fiq_set_mask(struct irq_data *d)
>> {
>> + if (!aic_irqc->info.el2_regs)
>> + return;
>
> Why? AIC_TMR_EL02_PHYS is defined as the interrupt that fires in the
> context of a guest. There is no guest here (no EL2 either), so what
> you should have as interrupt number is AIC_TMR_EL0_{PHYS,VIRT}, and
> this change becomes irrelevant (nothing to mask). Which is also what
> happens when running an M1 under the m1n1 hypervisor.
This func accesses impl-defined regs that are not present on earlier SoCs.
>
>> +
>> /* Only the guest timers have real mask bits, unfortunately. */
>> switch (aic_fiq_get_idx(d)) {
>> case AIC_TMR_EL02_PHYS:
>> @@ -469,6 +481,9 @@ static void aic_fiq_set_mask(struct irq_data *d)
>>
>> static void aic_fiq_clear_mask(struct irq_data *d)
>> {
>> + if (!aic_irqc->info.el2_regs)
>> + return;
>> +
>> switch (aic_fiq_get_idx(d)) {
>> case AIC_TMR_EL02_PHYS:
>> sysreg_clear_set_s(SYS_IMP_APL_VM_TMR_FIQ_ENA_EL2, 0, VM_TMR_FIQ_ENABLE_P);
>> @@ -524,12 +539,14 @@ 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 (aic_irqc->info.ipi_regs) {
>
> This is probably the hottest path in the whole kernel. Do we want an
> extra read here? Absolutely not. At the very least, this should be a
> static key.
Yeah, makes sense..
>
>> + 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);
>> + }
>> }
>> }
>>
>> @@ -566,12 +583,14 @@ 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)) {
>> - /* 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,
>> - FIELD_PREP(UPMCR0_IMODE, UPMCR0_IMODE_OFF));
>> + if (aic_irqc->info.uncore2_regs) {
>
> Same thing.
>
>> + 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)) {
>> + /* 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,
>> + FIELD_PREP(UPMCR0_IMODE, UPMCR0_IMODE_OFF));
>> + }
>> }
>> }
>>
>> @@ -676,7 +695,8 @@ static int aic_irq_domain_translate(struct irq_domain *id,
>> break;
>> case AIC_TMR_HV_PHYS:
>> case AIC_TMR_HV_VIRT:
>> - return -ENOENT;
>> + if (aic_irqc->info.el2_regs)
>> + return -ENOENT;
>
> See my comment above about the use of these interrupt numbers.
`if (!is_kernel_in_hyp_mode()) {` always evaluates to true, since there's
no EL2. Hence, accessing AIC_TMR_HV_{VIRT,PHYS} makes this return ENOENT,
which means timer can't probe and that's no bueno.
>
>> default:
>> break;
>> }
>> @@ -944,7 +964,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 (aic_irqc->info.ipi_regs)
>> + 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);
>> @@ -965,8 +986,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 (aic_irqc->info.uncore2_regs)
>> + sysreg_clear_set_s(SYS_IMP_APL_UPMCR0_EL1, UPMCR0_IMODE,
>> + FIELD_PREP(UPMCR0_IMODE, UPMCR0_IMODE_OFF));
>>
>> /* Commit all of the above */
>> isb();
>
> I must be missing something though. Where is the code that actually
> enables support for the SoCs mentioned in $SUBJECT?
In this peculiar case, enabling support means stripping away the so-called
'features', otherwise the interrupt controller won't budge.
Konrad
>
> Thanks,
>
> M.
>
On 12.07.2022 20:52, Sven Peter wrote:
> marcan probably has to review this in detail but two comments from me:
>
> On Tue, Jul 12, 2022, at 18:09, Konrad Dybcio wrote:
>> Add support for A7-A11 SoCs by if-ing out some features only present on
>> A12 & newer (UNCORE2 registers) or M1 & newer (EL2 registers - the
>> older SoCs don't implement EL2).
>>
>> Also, annotate IPI regs support (A11 and newer*) so that the driver can
>> tell whether the SoC supports these (they are written to even if fast
>> IPI is disabled, when the registers are there of course).
>>
>> *A11 is supposed to use this feature, but it is currently not working.
>> That said, it is not yet necessary, especially with only one core up,
>> and it works a-ok using the same featureset as earlier SoCs.
>>
>> Signed-off-by: Konrad Dybcio <[email protected]>
>> ---
>> drivers/irqchip/irq-apple-aic.c | 54 +++++++++++++++++++++++----------
>> 1 file changed, 38 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/irqchip/irq-apple-aic.c b/drivers/irqchip/irq-apple-aic.c
>> index 12dd48727a15..36f4b52addc2 100644
>> --- a/drivers/irqchip/irq-apple-aic.c
>> +++ b/drivers/irqchip/irq-apple-aic.c
>> @@ -245,7 +245,10 @@ struct aic_info {
>> u32 die_stride;
>>
>> /* Features */
>> + bool el2_regs;
>> bool fast_ipi;
>> + bool ipi_regs;
>> + bool uncore2_regs;
>
> I don't quite understand the difference between fast_ipi and ipi_regs.
> Don't we always have fast_ipi suppport when those regs are available?
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/irqchip/irq-apple-aic.c?h=next-20220712#n532
Both cases invoke accessing IPI regs, and there was a ipi/no-ipi variant
before, so I didn't want to mess with that.
>
>> };
>>
>> static const struct aic_info aic1_info = {
>> @@ -261,7 +264,10 @@ static const struct aic_info aic1_fipi_info = {
>> .event = AIC_EVENT,
>> .target_cpu = AIC_TARGET_CPU,
>>
>> + .el2_regs = true,
>> .fast_ipi = true,
>> + .ipi_regs = true,
>> + .uncore2_regs = true,
>> };
>>
>> static const struct aic_info aic2_info = {
>> @@ -269,7 +275,10 @@ static const struct aic_info aic2_info = {
>>
>> .irq_cfg = AIC2_IRQ_CFG,
>>
>> + .el2_regs = true,
>> .fast_ipi = true,
>> + .ipi_regs = true,
>> + .uncore2_regs = true,
>> };
>>
>> static const struct of_device_id aic_info_match[] = {
>> @@ -452,6 +461,9 @@ static unsigned long aic_fiq_get_idx(struct irq_data *d)
>>
>> static void aic_fiq_set_mask(struct irq_data *d)
>> {
>> + if (!aic_irqc->info.el2_regs)
>> + return;
>> +
>> /* Only the guest timers have real mask bits, unfortunately. */
>> switch (aic_fiq_get_idx(d)) {
>> case AIC_TMR_EL02_PHYS:
>> @@ -469,6 +481,9 @@ static void aic_fiq_set_mask(struct irq_data *d)
>>
>> static void aic_fiq_clear_mask(struct irq_data *d)
>> {
>> + if (!aic_irqc->info.el2_regs)
>> + return;
>> +
>> switch (aic_fiq_get_idx(d)) {
>> case AIC_TMR_EL02_PHYS:
>> sysreg_clear_set_s(SYS_IMP_APL_VM_TMR_FIQ_ENA_EL2, 0,
>> VM_TMR_FIQ_ENABLE_P);
>> @@ -524,12 +539,14 @@ 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 (aic_irqc->info.ipi_regs) {
>> + 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);
>> + }
>> }
>> }
>
> This is a pretty hot path and the use_fast_ipi check uses the jump label support
> (static_branch_likely, static_branch_enable) to avoid dereferencing memory here.
> We'll probably want the same for the other features.
>
> For this branch here the else can probably just be removed: I think that's
> a leftover from when this driver just didn't support fastipi at all even
> when the registers were available.
If there's no use for non-fast-ipi paths, perhaps they can just be removed?
That could simplify the fast_ipi/ipi_regs situation.
Konrad
>
>
>
> Sven
>
On Tue, Jul 12, 2022, at 21:23, Konrad Dybcio wrote:
> On 12.07.2022 21:12, Marc Zyngier wrote:
>> Hi Konrad,
>>
>> Please add a cover letter when sending more than a single patch.
>>
>> On Tue, 12 Jul 2022 17:09:19 +0100,
>> Konrad Dybcio <[email protected]> wrote:
>>>
>>> Add support for A7-A11 SoCs by if-ing out some features only present on
>>> A12 & newer (UNCORE2 registers) or M1 & newer (EL2 registers - the
>>> older SoCs don't implement EL2).
>>>
>>> Also, annotate IPI regs support (A11 and newer*) so that the driver can
>>> tell whether the SoC supports these (they are written to even if fast
>>> IPI is disabled, when the registers are there of course).
>>>
>>> *A11 is supposed to use this feature, but it is currently not working.
>>> That said, it is not yet necessary, especially with only one core up,
>>> and it works a-ok using the same featureset as earlier SoCs.
>>>
>>> Signed-off-by: Konrad Dybcio <[email protected]>
>>> ---
>>> drivers/irqchip/irq-apple-aic.c | 54 +++++++++++++++++++++++----------
>>> 1 file changed, 38 insertions(+), 16 deletions(-)
>>>
>>> diff --git a/drivers/irqchip/irq-apple-aic.c b/drivers/irqchip/irq-apple-aic.c
>>> index 12dd48727a15..36f4b52addc2 100644
>>> --- a/drivers/irqchip/irq-apple-aic.c
>>> +++ b/drivers/irqchip/irq-apple-aic.c
>>> @@ -245,7 +245,10 @@ struct aic_info {
>>> u32 die_stride;
>>>
>>> /* Features */
>>> + bool el2_regs;
>>> bool fast_ipi;
>>> + bool ipi_regs;
>>> + bool uncore2_regs;
>>> };
>>>
>>> static const struct aic_info aic1_info = {
>>> @@ -261,7 +264,10 @@ static const struct aic_info aic1_fipi_info = {
>>> .event = AIC_EVENT,
>>> .target_cpu = AIC_TARGET_CPU,
>>>
>>> + .el2_regs = true,
>>> .fast_ipi = true,
>>> + .ipi_regs = true,
>>> + .uncore2_regs = true,
>>> };
>>>
>>> static const struct aic_info aic2_info = {
>>> @@ -269,7 +275,10 @@ static const struct aic_info aic2_info = {
>>>
>>> .irq_cfg = AIC2_IRQ_CFG,
>>>
>>> + .el2_regs = true,
>>> .fast_ipi = true,
>>> + .ipi_regs = true,
>>> + .uncore2_regs = true,
>>
>> So to sum it up, all recent cores have all the cool features, and the
>> older ones have none of them. Surely we can do better than adding 3
>> fields that have the same value. Turn 'fast_ipi' into something that
>> means 'full_fat', and key everything on that.
>>
>> And if this is meant to evolve into a more differentiated set of
>> features, the usual idiom is to have a set of flags as part of an
>> unsigned long instead of a set of booleans.
> The latter would be true if a bootrom exploit or any equivalent means
> of booting Linux would be found for A12 (M1 is family with A14 for context).
>
> We can think of 4 feature levels, I think:
>
> A7-A10: 'nothing fancy'
> A11: fast_ipi (broken currently, need to investigate)
> A12: A11 + UNCORE2 regs
> M1+: A12 + EL2
>
> We *could* squash the A12-A14 case into M1, but then if a means of booting
> Linux appears, this would have to be untangled again..
>
>>
>>> };
>>>
>>> static const struct of_device_id aic_info_match[] = {
>>> @@ -452,6 +461,9 @@ static unsigned long aic_fiq_get_idx(struct irq_data *d)
>>>
>>> static void aic_fiq_set_mask(struct irq_data *d)
>>> {
>>> + if (!aic_irqc->info.el2_regs)
>>> + return;
>>
>> Why? AIC_TMR_EL02_PHYS is defined as the interrupt that fires in the
>> context of a guest. There is no guest here (no EL2 either), so what
>> you should have as interrupt number is AIC_TMR_EL0_{PHYS,VIRT}, and
>> this change becomes irrelevant (nothing to mask). Which is also what
>> happens when running an M1 under the m1n1 hypervisor.
> This func accesses impl-defined regs that are not present on earlier SoCs.
>
>>
>>> +
>>> /* Only the guest timers have real mask bits, unfortunately. */
>>> switch (aic_fiq_get_idx(d)) {
>>> case AIC_TMR_EL02_PHYS:
>>> @@ -469,6 +481,9 @@ static void aic_fiq_set_mask(struct irq_data *d)
>>>
>>> static void aic_fiq_clear_mask(struct irq_data *d)
>>> {
>>> + if (!aic_irqc->info.el2_regs)
>>> + return;
>>> +
>>> switch (aic_fiq_get_idx(d)) {
>>> case AIC_TMR_EL02_PHYS:
>>> sysreg_clear_set_s(SYS_IMP_APL_VM_TMR_FIQ_ENA_EL2, 0, VM_TMR_FIQ_ENABLE_P);
>>> @@ -524,12 +539,14 @@ 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 (aic_irqc->info.ipi_regs) {
>>
>> This is probably the hottest path in the whole kernel. Do we want an
>> extra read here? Absolutely not. At the very least, this should be a
>> static key.
> Yeah, makes sense..
>
>
>>
>>> + 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);
>>> + }
>>> }
>>> }
>>>
>>> @@ -566,12 +583,14 @@ 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)) {
>>> - /* 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,
>>> - FIELD_PREP(UPMCR0_IMODE, UPMCR0_IMODE_OFF));
>>> + if (aic_irqc->info.uncore2_regs) {
>>
>> Same thing.
>>
>>> + 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)) {
>>> + /* 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,
>>> + FIELD_PREP(UPMCR0_IMODE, UPMCR0_IMODE_OFF));
>>> + }
>>> }
>>> }
>>>
>>> @@ -676,7 +695,8 @@ static int aic_irq_domain_translate(struct irq_domain *id,
>>> break;
>>> case AIC_TMR_HV_PHYS:
>>> case AIC_TMR_HV_VIRT:
>>> - return -ENOENT;
>>> + if (aic_irqc->info.el2_regs)
>>> + return -ENOENT;
>>
>> See my comment above about the use of these interrupt numbers.
> `if (!is_kernel_in_hyp_mode()) {` always evaluates to true, since there's
> no EL2. Hence, accessing AIC_TMR_HV_{VIRT,PHYS} makes this return ENOENT,
> which means timer can't probe and that's no bueno.
Sounds like an issue with your device tree. There should be no reference to
AIC_TMR_HV_{VIRT,PHYS} in there.
Sven
On Tue, 12 Jul 2022 20:23:31 +0100,
Konrad Dybcio <[email protected]> wrote:
>
>
>
> On 12.07.2022 21:12, Marc Zyngier wrote:
> > Hi Konrad,
> >
> > Please add a cover letter when sending more than a single patch.
> >
> > On Tue, 12 Jul 2022 17:09:19 +0100,
> > Konrad Dybcio <[email protected]> wrote:
> >>
> >> Add support for A7-A11 SoCs by if-ing out some features only present on
> >> A12 & newer (UNCORE2 registers) or M1 & newer (EL2 registers - the
> >> older SoCs don't implement EL2).
> >>
> >> Also, annotate IPI regs support (A11 and newer*) so that the driver can
> >> tell whether the SoC supports these (they are written to even if fast
> >> IPI is disabled, when the registers are there of course).
> >>
> >> *A11 is supposed to use this feature, but it is currently not working.
> >> That said, it is not yet necessary, especially with only one core up,
> >> and it works a-ok using the same featureset as earlier SoCs.
> >>
> >> Signed-off-by: Konrad Dybcio <[email protected]>
> >> ---
> >> drivers/irqchip/irq-apple-aic.c | 54 +++++++++++++++++++++++----------
> >> 1 file changed, 38 insertions(+), 16 deletions(-)
> >>
> >> diff --git a/drivers/irqchip/irq-apple-aic.c b/drivers/irqchip/irq-apple-aic.c
> >> index 12dd48727a15..36f4b52addc2 100644
> >> --- a/drivers/irqchip/irq-apple-aic.c
> >> +++ b/drivers/irqchip/irq-apple-aic.c
> >> @@ -245,7 +245,10 @@ struct aic_info {
> >> u32 die_stride;
> >>
> >> /* Features */
> >> + bool el2_regs;
> >> bool fast_ipi;
> >> + bool ipi_regs;
> >> + bool uncore2_regs;
> >> };
> >>
> >> static const struct aic_info aic1_info = {
> >> @@ -261,7 +264,10 @@ static const struct aic_info aic1_fipi_info = {
> >> .event = AIC_EVENT,
> >> .target_cpu = AIC_TARGET_CPU,
> >>
> >> + .el2_regs = true,
> >> .fast_ipi = true,
> >> + .ipi_regs = true,
> >> + .uncore2_regs = true,
> >> };
> >>
> >> static const struct aic_info aic2_info = {
> >> @@ -269,7 +275,10 @@ static const struct aic_info aic2_info = {
> >>
> >> .irq_cfg = AIC2_IRQ_CFG,
> >>
> >> + .el2_regs = true,
> >> .fast_ipi = true,
> >> + .ipi_regs = true,
> >> + .uncore2_regs = true,
> >
> > So to sum it up, all recent cores have all the cool features, and the
> > older ones have none of them. Surely we can do better than adding 3
> > fields that have the same value. Turn 'fast_ipi' into something that
> > means 'full_fat', and key everything on that.
> >
> > And if this is meant to evolve into a more differentiated set of
> > features, the usual idiom is to have a set of flags as part of an
> > unsigned long instead of a set of booleans.
> The latter would be true if a bootrom exploit or any equivalent means
> of booting Linux would be found for A12 (M1 is family with A14 for context).
>
> We can think of 4 feature levels, I think:
>
> A7-A10: 'nothing fancy'
> A11: fast_ipi (broken currently, need to investigate)
> A12: A11 + UNCORE2 regs
> M1+: A12 + EL2
>
> We *could* squash the A12-A14 case into M1, but then if a means of booting
> Linux appears, this would have to be untangled again..
We don't add code for systems that could only hypothetically run
Linux. If and when this becomes possible, we'll add support for
them. In the meantime, I suggest you focus on supporting what actually
works.
>
> >
> >> };
> >>
> >> static const struct of_device_id aic_info_match[] = {
> >> @@ -452,6 +461,9 @@ static unsigned long aic_fiq_get_idx(struct irq_data *d)
> >>
> >> static void aic_fiq_set_mask(struct irq_data *d)
> >> {
> >> + if (!aic_irqc->info.el2_regs)
> >> + return;
> >
> > Why? AIC_TMR_EL02_PHYS is defined as the interrupt that fires in the
> > context of a guest. There is no guest here (no EL2 either), so what
> > you should have as interrupt number is AIC_TMR_EL0_{PHYS,VIRT}, and
> > this change becomes irrelevant (nothing to mask). Which is also what
> > happens when running an M1 under the m1n1 hypervisor.
> This func accesses impl-defined regs that are not present on earlier SoCs.
You're missing my point. Why are you encoding your timer interrupts
with a hwirq that requires you to skip existing code? If you used the
interrupt number that represent a bare-metal interrupt, you'd be just
fine. Specially considering that the interrupt numbers in the DT are
nothing but made-up numbers, as there is no interrupt controller to
speak of.
> >> @@ -676,7 +695,8 @@ static int aic_irq_domain_translate(struct irq_domain *id,
> >> break;
> >> case AIC_TMR_HV_PHYS:
> >> case AIC_TMR_HV_VIRT:
> >> - return -ENOENT;
> >> + if (aic_irqc->info.el2_regs)
> >> + return -ENOENT;
> >
> > See my comment above about the use of these interrupt numbers.
> `if (!is_kernel_in_hyp_mode()) {` always evaluates to true, since there's
> no EL2. Hence, accessing AIC_TMR_HV_{VIRT,PHYS} makes this return ENOENT,
> which means timer can't probe and that's no bueno.
Again, you have the wrong end of the stick, and this is about changing
your DT rather than the driver.
>
>
> >
> >> default:
> >> break;
> >> }
> >> @@ -944,7 +964,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 (aic_irqc->info.ipi_regs)
> >> + 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);
> >> @@ -965,8 +986,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 (aic_irqc->info.uncore2_regs)
> >> + sysreg_clear_set_s(SYS_IMP_APL_UPMCR0_EL1, UPMCR0_IMODE,
> >> + FIELD_PREP(UPMCR0_IMODE, UPMCR0_IMODE_OFF));
> >>
> >> /* Commit all of the above */
> >> isb();
> >
> > I must be missing something though. Where is the code that actually
> > enables support for the SoCs mentioned in $SUBJECT?
> In this peculiar case, enabling support means stripping away the so-called
> 'features', otherwise the interrupt controller won't budge.
What I would like to see is a different compatibility string that
makes the support for a given IP explicit instead of making everything
implicit.
M.
--
Without deviation from the norm, progress is not possible.
Hi Konrad,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on tip/irq/core]
[also build test WARNING on robh/for-next linus/master v5.19-rc6 next-20220714]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Konrad-Dybcio/dt-bindings-apple-aic-Document-A7-A11-compatibles/20220713-001059
base: https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git ac165aab469895de059a4a191a2e04ddb5421d0e
config: arm64-randconfig-r014-20220715
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project 2da550140aa98cf6a3e96417c87f1e89e3a26047)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install arm64 cross compiling tool for clang build
# apt-get install binutils-aarch64-linux-gnu
# https://github.com/intel-lab-lkp/linux/commit/afa007835ace3d280cbc9aed6b1a1c8a1acd3275
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Konrad-Dybcio/dt-bindings-apple-aic-Document-A7-A11-compatibles/20220713-001059
git checkout afa007835ace3d280cbc9aed6b1a1c8a1acd3275
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm64 SHELL=/bin/bash drivers/irqchip/
If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <[email protected]>
All warnings (new ones prefixed by >>):
>> drivers/irqchip/irq-apple-aic.c:700:4: warning: unannotated fall-through between switch labels [-Wimplicit-fallthrough]
default:
^
drivers/irqchip/irq-apple-aic.c:700:4: note: insert 'break;' to avoid fall-through
default:
^
break;
1 warning generated.
vim +700 drivers/irqchip/irq-apple-aic.c
76cde26394114f6 Hector Martin 2021-01-21 648
76cde26394114f6 Hector Martin 2021-01-21 649 static int aic_irq_domain_translate(struct irq_domain *id,
76cde26394114f6 Hector Martin 2021-01-21 650 struct irq_fwspec *fwspec,
76cde26394114f6 Hector Martin 2021-01-21 651 unsigned long *hwirq,
76cde26394114f6 Hector Martin 2021-01-21 652 unsigned int *type)
76cde26394114f6 Hector Martin 2021-01-21 653 {
76cde26394114f6 Hector Martin 2021-01-21 654 struct aic_irq_chip *ic = id->host_data;
a801f0ee563b818 Hector Martin 2022-03-10 655 u32 *args;
a801f0ee563b818 Hector Martin 2022-03-10 656 u32 die = 0;
76cde26394114f6 Hector Martin 2021-01-21 657
a801f0ee563b818 Hector Martin 2022-03-10 658 if (fwspec->param_count < 3 || fwspec->param_count > 4 ||
a801f0ee563b818 Hector Martin 2022-03-10 659 !is_of_node(fwspec->fwnode))
76cde26394114f6 Hector Martin 2021-01-21 660 return -EINVAL;
76cde26394114f6 Hector Martin 2021-01-21 661
a801f0ee563b818 Hector Martin 2022-03-10 662 args = &fwspec->param[1];
a801f0ee563b818 Hector Martin 2022-03-10 663
a801f0ee563b818 Hector Martin 2022-03-10 664 if (fwspec->param_count == 4) {
a801f0ee563b818 Hector Martin 2022-03-10 665 die = args[0];
a801f0ee563b818 Hector Martin 2022-03-10 666 args++;
a801f0ee563b818 Hector Martin 2022-03-10 667 }
a801f0ee563b818 Hector Martin 2022-03-10 668
76cde26394114f6 Hector Martin 2021-01-21 669 switch (fwspec->param[0]) {
76cde26394114f6 Hector Martin 2021-01-21 670 case AIC_IRQ:
a801f0ee563b818 Hector Martin 2022-03-10 671 if (die >= ic->nr_die)
76cde26394114f6 Hector Martin 2021-01-21 672 return -EINVAL;
a801f0ee563b818 Hector Martin 2022-03-10 673 if (args[0] >= ic->nr_irq)
76cde26394114f6 Hector Martin 2021-01-21 674 return -EINVAL;
a801f0ee563b818 Hector Martin 2022-03-10 675 *hwirq = AIC_IRQ_HWIRQ(die, args[0]);
76cde26394114f6 Hector Martin 2021-01-21 676 break;
76cde26394114f6 Hector Martin 2021-01-21 677 case AIC_FIQ:
a801f0ee563b818 Hector Martin 2022-03-10 678 if (die != 0)
76cde26394114f6 Hector Martin 2021-01-21 679 return -EINVAL;
a801f0ee563b818 Hector Martin 2022-03-10 680 if (args[0] >= AIC_NR_FIQ)
76cde26394114f6 Hector Martin 2021-01-21 681 return -EINVAL;
a801f0ee563b818 Hector Martin 2022-03-10 682 *hwirq = AIC_FIQ_HWIRQ(args[0]);
76cde26394114f6 Hector Martin 2021-01-21 683
76cde26394114f6 Hector Martin 2021-01-21 684 /*
76cde26394114f6 Hector Martin 2021-01-21 685 * In EL1 the non-redirected registers are the guest's,
76cde26394114f6 Hector Martin 2021-01-21 686 * not EL2's, so remap the hwirqs to match.
76cde26394114f6 Hector Martin 2021-01-21 687 */
76cde26394114f6 Hector Martin 2021-01-21 688 if (!is_kernel_in_hyp_mode()) {
a801f0ee563b818 Hector Martin 2022-03-10 689 switch (args[0]) {
76cde26394114f6 Hector Martin 2021-01-21 690 case AIC_TMR_GUEST_PHYS:
7c841f5f6fa3f99 Hector Martin 2022-03-10 691 *hwirq = AIC_FIQ_HWIRQ(AIC_TMR_EL0_PHYS);
76cde26394114f6 Hector Martin 2021-01-21 692 break;
76cde26394114f6 Hector Martin 2021-01-21 693 case AIC_TMR_GUEST_VIRT:
7c841f5f6fa3f99 Hector Martin 2022-03-10 694 *hwirq = AIC_FIQ_HWIRQ(AIC_TMR_EL0_VIRT);
76cde26394114f6 Hector Martin 2021-01-21 695 break;
76cde26394114f6 Hector Martin 2021-01-21 696 case AIC_TMR_HV_PHYS:
76cde26394114f6 Hector Martin 2021-01-21 697 case AIC_TMR_HV_VIRT:
afa007835ace3d2 Konrad Dybcio 2022-07-12 698 if (aic_irqc->info.el2_regs)
76cde26394114f6 Hector Martin 2021-01-21 699 return -ENOENT;
76cde26394114f6 Hector Martin 2021-01-21 @700 default:
76cde26394114f6 Hector Martin 2021-01-21 701 break;
76cde26394114f6 Hector Martin 2021-01-21 702 }
76cde26394114f6 Hector Martin 2021-01-21 703 }
76cde26394114f6 Hector Martin 2021-01-21 704 break;
76cde26394114f6 Hector Martin 2021-01-21 705 default:
76cde26394114f6 Hector Martin 2021-01-21 706 return -EINVAL;
76cde26394114f6 Hector Martin 2021-01-21 707 }
76cde26394114f6 Hector Martin 2021-01-21 708
a801f0ee563b818 Hector Martin 2022-03-10 709 *type = args[1] & IRQ_TYPE_SENSE_MASK;
76cde26394114f6 Hector Martin 2021-01-21 710
76cde26394114f6 Hector Martin 2021-01-21 711 return 0;
76cde26394114f6 Hector Martin 2021-01-21 712 }
76cde26394114f6 Hector Martin 2021-01-21 713
--
0-DAY CI Kernel Test Service
https://01.org/lkp