The GIC's internal view of the priority mask register and the assigned
interrupt priorities are based on whether GIC security is enabled and
whether firmware routes Group 0 interrupts to EL3. At the moment, we
support priority masking when ICC_PMR_EL1 and interrupt priorities are
either both modified by the GIC, or both left unchanged.
Trusted Firmware-A's default interrupt routing model allows Group 0
interrupts to be delivered to the non-secure world (SCR_EL3.FIQ == 0).
Unfortunately, this is precisely the case that the GIC driver doesn't
support: ICC_PMR_EL1 remains unchanged, but the GIC's view of interrupt
priorities is different from the software programmed values.
Support pseudo-NMIs when SCR_EL3.FIQ == 0 by using a different value to
mask regular interrupts. All the other values remain the same.
Signed-off-by: Alexandru Elisei <[email protected]>
---
arch/arm64/include/asm/arch_gicv3.h | 8 ++++-
arch/arm64/include/asm/daifflags.h | 4 +--
arch/arm64/include/asm/irqflags.h | 18 ++++++----
arch/arm64/include/asm/ptrace.h | 12 +++++++
arch/arm64/kernel/entry.S | 2 +-
arch/arm64/kernel/image-vars.h | 2 ++
arch/arm64/kvm/hyp/nvhe/switch.c | 2 +-
drivers/irqchip/irq-gic-v3.c | 52 ++++++++++++++++++++++-------
8 files changed, 77 insertions(+), 23 deletions(-)
diff --git a/arch/arm64/include/asm/arch_gicv3.h b/arch/arm64/include/asm/arch_gicv3.h
index 6647ae4f0231..908152e8659b 100644
--- a/arch/arm64/include/asm/arch_gicv3.h
+++ b/arch/arm64/include/asm/arch_gicv3.h
@@ -162,7 +162,13 @@ static inline void gic_pmr_mask_irqs(void)
* are applied to IRQ priorities
*/
BUILD_BUG_ON((0x80 | (GICD_INT_DEF_PRI >> 1)) >= GIC_PRIO_IRQON);
- gic_write_pmr(GIC_PRIO_IRQOFF);
+ /*
+ * Same situation as above, but now we make sure that we can mask
+ * regular interrupts.
+ */
+ BUILD_BUG_ON((0x80 | (GICD_INT_DEF_PRI >> 1)) < (GIC_PRIO_IRQOFF_NS |
+ GIC_PRIO_PSR_I_SET));
+ gic_write_pmr(gic_prio_irqoff());
}
static inline void gic_arch_enable_irqs(void)
diff --git a/arch/arm64/include/asm/daifflags.h b/arch/arm64/include/asm/daifflags.h
index ec213b4a1650..3efa240a6c48 100644
--- a/arch/arm64/include/asm/daifflags.h
+++ b/arch/arm64/include/asm/daifflags.h
@@ -22,7 +22,7 @@
static inline void local_daif_mask(void)
{
WARN_ON(system_has_prio_mask_debugging() &&
- (read_sysreg_s(SYS_ICC_PMR_EL1) == (GIC_PRIO_IRQOFF |
+ (read_sysreg_s(SYS_ICC_PMR_EL1) == (gic_prio_irqoff() |
GIC_PRIO_PSR_I_SET)));
asm volatile(
@@ -87,7 +87,7 @@ static inline void local_daif_restore(unsigned long flags)
* asynchronous errors, we can take NMIs
*/
flags &= ~PSR_I_BIT;
- pmr = GIC_PRIO_IRQOFF;
+ pmr = gic_prio_irqoff();
} else {
pmr = GIC_PRIO_IRQON | GIC_PRIO_PSR_I_SET;
}
diff --git a/arch/arm64/include/asm/irqflags.h b/arch/arm64/include/asm/irqflags.h
index aa4b6521ef14..af353c78d5f8 100644
--- a/arch/arm64/include/asm/irqflags.h
+++ b/arch/arm64/include/asm/irqflags.h
@@ -28,10 +28,13 @@
*/
static inline void arch_local_irq_enable(void)
{
+ u64 pmr_irqon = GIC_PRIO_IRQON;
+
if (system_has_prio_mask_debugging()) {
- u32 pmr = read_sysreg_s(SYS_ICC_PMR_EL1);
+ u64 pmr = read_sysreg_s(SYS_ICC_PMR_EL1);
+ u64 pmr_irqoff = gic_prio_irqoff();
- WARN_ON_ONCE(pmr != GIC_PRIO_IRQON && pmr != GIC_PRIO_IRQOFF);
+ WARN_ON_ONCE(pmr != pmr_irqon && pmr != pmr_irqoff);
}
asm volatile(ALTERNATIVE(
@@ -39,7 +42,7 @@ static inline void arch_local_irq_enable(void)
__msr_s(SYS_ICC_PMR_EL1, "%0"),
ARM64_HAS_IRQ_PRIO_MASKING)
:
- : "r" ((unsigned long) GIC_PRIO_IRQON)
+ : "r" (pmr_irqon)
: "memory");
pmr_sync();
@@ -47,10 +50,13 @@ static inline void arch_local_irq_enable(void)
static inline void arch_local_irq_disable(void)
{
+ u64 pmr_irqoff = gic_prio_irqoff();
+
if (system_has_prio_mask_debugging()) {
- u32 pmr = read_sysreg_s(SYS_ICC_PMR_EL1);
+ u64 pmr = read_sysreg_s(SYS_ICC_PMR_EL1);
+ u64 pmr_irqon = GIC_PRIO_IRQON;
- WARN_ON_ONCE(pmr != GIC_PRIO_IRQON && pmr != GIC_PRIO_IRQOFF);
+ WARN_ON_ONCE(pmr != pmr_irqon && pmr != pmr_irqoff);
}
asm volatile(ALTERNATIVE(
@@ -58,7 +64,7 @@ static inline void arch_local_irq_disable(void)
__msr_s(SYS_ICC_PMR_EL1, "%0"),
ARM64_HAS_IRQ_PRIO_MASKING)
:
- : "r" ((unsigned long) GIC_PRIO_IRQOFF)
+ : "r" (pmr_irqoff)
: "memory");
}
diff --git a/arch/arm64/include/asm/ptrace.h b/arch/arm64/include/asm/ptrace.h
index 966ed30ed5f7..a19cd6ff4d1b 100644
--- a/arch/arm64/include/asm/ptrace.h
+++ b/arch/arm64/include/asm/ptrace.h
@@ -32,6 +32,7 @@
*/
#define GIC_PRIO_IRQON 0xe0
#define GIC_PRIO_IRQOFF (GIC_PRIO_IRQON & ~0x80)
+#define GIC_PRIO_IRQOFF_NS 0xa0
#define GIC_PRIO_PSR_I_SET (1 << 4)
/* Additional SPSR bits not exposed in the UABI */
@@ -129,6 +130,17 @@
#define compat_sp_fiq regs[29]
#define compat_lr_fiq regs[30]
+#define gic_prio_irqoff() \
+ ({ \
+ extern struct static_key_false gic_nonsecure_priorities;\
+ u8 __prio = GIC_PRIO_IRQOFF; \
+ \
+ if (static_branch_unlikely(&gic_nonsecure_priorities)) \
+ __prio = GIC_PRIO_IRQOFF_NS; \
+ \
+ __prio; \
+ })
+
static inline unsigned long compat_psr_to_pstate(const unsigned long psr)
{
unsigned long pstate;
diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
index 2646178c8329..e4fa944dbf1d 100644
--- a/arch/arm64/kernel/entry.S
+++ b/arch/arm64/kernel/entry.S
@@ -653,7 +653,7 @@ alternative_else_nop_endif
#ifdef CONFIG_ARM64_PSEUDO_NMI
/*
* When using IRQ priority masking, we can get spurious interrupts while
- * PMR is set to GIC_PRIO_IRQOFF. An NMI might also have occurred in a
+ * PMR is set to mask interrupts. An NMI might also have occurred in a
* section with interrupts disabled. Skip tracing in those cases.
*/
test_irqs_unmasked res=x0, pmr=x20
diff --git a/arch/arm64/kernel/image-vars.h b/arch/arm64/kernel/image-vars.h
index 9e897c500237..c4476a99dee8 100644
--- a/arch/arm64/kernel/image-vars.h
+++ b/arch/arm64/kernel/image-vars.h
@@ -101,6 +101,8 @@ KVM_NVHE_ALIAS(vgic_v3_cpuif_trap);
/* Static key checked in pmr_sync(). */
#ifdef CONFIG_ARM64_PSEUDO_NMI
KVM_NVHE_ALIAS(gic_pmr_sync);
+/* Static key checked in gic_prio_irqoff(). */
+KVM_NVHE_ALIAS(gic_nonsecure_priorities);
#endif
#endif /* CONFIG_KVM */
diff --git a/arch/arm64/kvm/hyp/nvhe/switch.c b/arch/arm64/kvm/hyp/nvhe/switch.c
index 341be2f2f312..729a3a59ad6a 100644
--- a/arch/arm64/kvm/hyp/nvhe/switch.c
+++ b/arch/arm64/kvm/hyp/nvhe/switch.c
@@ -237,7 +237,7 @@ int __kvm_vcpu_run(struct kvm_vcpu *vcpu)
/* Returning to host will clear PSR.I, remask PMR if needed */
if (system_uses_irq_prio_masking())
- gic_write_pmr(GIC_PRIO_IRQOFF);
+ gic_write_pmr(gic_prio_irqoff());
return exit_code;
}
diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
index ce8944ae1b84..19e52c025c59 100644
--- a/drivers/irqchip/irq-gic-v3.c
+++ b/drivers/irqchip/irq-gic-v3.c
@@ -75,16 +75,14 @@ static DEFINE_STATIC_KEY_TRUE(supports_deactivate_key);
*
* If SCR_EL3.FIQ == 1, the values writen to/read from PMR and RPR at non-secure
* EL1 are subject to a similar operation thus matching the priorities presented
- * from the (re)distributor when security is enabled.
+ * from the (re)distributor when security is enabled. When SCR_EL3.FIQ == 0,
+ * these values are unchanched by the GIC.
*
* see GICv3/GICv4 Architecture Specification (IHI0069D):
* - section 4.8.1 Non-secure accesses to register fields for Secure interrupt
* priorities.
* - Figure 4-7 Secure read of the priority field for a Non-secure Group 1
* interrupt.
- *
- * For now, we only support pseudo-NMIs if we have non-secure view of
- * priorities.
*/
static DEFINE_STATIC_KEY_FALSE(supports_pseudo_nmis);
@@ -97,6 +95,9 @@ static DEFINE_STATIC_KEY_FALSE(supports_pseudo_nmis);
DEFINE_STATIC_KEY_FALSE(gic_pmr_sync);
EXPORT_SYMBOL(gic_pmr_sync);
+DEFINE_STATIC_KEY_FALSE(gic_nonsecure_priorities);
+EXPORT_SYMBOL(gic_nonsecure_priorities);
+
/* ppi_nmi_refs[n] == number of cpus having ppi[n + 16] set as NMI */
static refcount_t *ppi_nmi_refs;
@@ -932,14 +933,16 @@ static void gic_cpu_sys_reg_init(void)
/* Set priority mask register */
if (!gic_prio_masking_enabled()) {
write_gicreg(DEFAULT_PMR_VALUE, ICC_PMR_EL1);
- } else {
+ } else if (gic_supports_nmi()) {
/*
* Mismatch configuration with boot CPU, the system is likely
* to die as interrupt masking will not work properly on all
* CPUs
*/
- WARN_ON(gic_supports_nmi() && group0 &&
- !gic_dist_security_disabled());
+ if (static_branch_unlikely(&gic_nonsecure_priorities))
+ WARN_ON(!group0 || gic_dist_security_disabled());
+ else
+ WARN_ON(group0 && !gic_dist_security_disabled());
}
/*
@@ -1544,11 +1547,6 @@ static void gic_enable_nmi_support(void)
if (!gic_prio_masking_enabled())
return;
- if (gic_has_group0() && !gic_dist_security_disabled()) {
- pr_warn("SCR_EL3.FIQ is cleared, cannot enable use of pseudo-NMIs\n");
- return;
- }
-
ppi_nmi_refs = kcalloc(gic_data.ppi_nr, sizeof(*ppi_nmi_refs), GFP_KERNEL);
if (!ppi_nmi_refs)
return;
@@ -1567,6 +1565,36 @@ static void gic_enable_nmi_support(void)
pr_info("Pseudo-NMIs enabled using %s ICC_PMR_EL1 synchronisation\n",
static_branch_unlikely(&gic_pmr_sync) ? "forced" : "relaxed");
+ /*
+ * How priority values are used by the GIC depends on two things:
+ * the security state of the GIC (controlled by the GICD_CTRL.DS bit)
+ * and if Group 0 interrupts can be delivered to Linux in the non-secure
+ * world as FIQs (controlled by the SCR_EL3.FIQ bit). These affect the
+ * the ICC_PMR_EL1 register and the priority that software assigns to
+ * interrupts:
+ *
+ * GICD_CTRL.DS | SCR_EL3.FIQ | ICC_PMR_EL1 | Group 1 priority
+ * -----------------------------------------------------------
+ * 1 | - | unchanged | unchanged
+ * -----------------------------------------------------------
+ * 0 | 1 | non-secure | non-secure
+ * -----------------------------------------------------------
+ * 0 | 0 | unchanged | non-secure
+ *
+ * where non-secure means that the value is right-shifted by one and the
+ * MSB bit set, to make it fit in the non-secure priority range.
+ *
+ * In the first two cases, where ICC_PMR_EL1 and the interrupt priority
+ * are both either modified, or unchanged, we can use the same set of
+ * priorities.
+ *
+ * In the last case, where only the interrupt priorities are modified to
+ * be in the non-secure range, we use a different PMR value to mask IRQs
+ * and the rest of the values that we use remain unchanged.
+ */
+ if (gic_has_group0() && !gic_dist_security_disabled())
+ static_branch_enable(&gic_nonsecure_priorities);
+
static_branch_enable(&supports_pseudo_nmis);
if (static_branch_likely(&supports_deactivate_key))
--
2.28.0
Hi Alex,
Thanks for taking the time for putting this together, as it has been a
long standing issue that needed fixing.
On Wed, 19 Aug 2020 14:36:30 +0100,
Alexandru Elisei <[email protected]> wrote:
>
> The GIC's internal view of the priority mask register and the assigned
> interrupt priorities are based on whether GIC security is enabled and
> whether firmware routes Group 0 interrupts to EL3. At the moment, we
> support priority masking when ICC_PMR_EL1 and interrupt priorities are
> either both modified by the GIC, or both left unchanged.
>
> Trusted Firmware-A's default interrupt routing model allows Group 0
> interrupts to be delivered to the non-secure world (SCR_EL3.FIQ == 0).
> Unfortunately, this is precisely the case that the GIC driver doesn't
> support: ICC_PMR_EL1 remains unchanged, but the GIC's view of interrupt
> priorities is different from the software programmed values.
>
> Support pseudo-NMIs when SCR_EL3.FIQ == 0 by using a different value to
> mask regular interrupts. All the other values remain the same.
>
> Signed-off-by: Alexandru Elisei <[email protected]>
> ---
> arch/arm64/include/asm/arch_gicv3.h | 8 ++++-
> arch/arm64/include/asm/daifflags.h | 4 +--
> arch/arm64/include/asm/irqflags.h | 18 ++++++----
> arch/arm64/include/asm/ptrace.h | 12 +++++++
> arch/arm64/kernel/entry.S | 2 +-
> arch/arm64/kernel/image-vars.h | 2 ++
> arch/arm64/kvm/hyp/nvhe/switch.c | 2 +-
> drivers/irqchip/irq-gic-v3.c | 52 ++++++++++++++++++++++-------
> 8 files changed, 77 insertions(+), 23 deletions(-)
>
> diff --git a/arch/arm64/include/asm/arch_gicv3.h b/arch/arm64/include/asm/arch_gicv3.h
> index 6647ae4f0231..908152e8659b 100644
> --- a/arch/arm64/include/asm/arch_gicv3.h
> +++ b/arch/arm64/include/asm/arch_gicv3.h
> @@ -162,7 +162,13 @@ static inline void gic_pmr_mask_irqs(void)
> * are applied to IRQ priorities
> */
> BUILD_BUG_ON((0x80 | (GICD_INT_DEF_PRI >> 1)) >= GIC_PRIO_IRQON);
> - gic_write_pmr(GIC_PRIO_IRQOFF);
> + /*
> + * Same situation as above, but now we make sure that we can mask
> + * regular interrupts.
> + */
> + BUILD_BUG_ON((0x80 | (GICD_INT_DEF_PRI >> 1)) < (GIC_PRIO_IRQOFF_NS |
> + GIC_PRIO_PSR_I_SET));
> + gic_write_pmr(gic_prio_irqoff());
> }
>
> static inline void gic_arch_enable_irqs(void)
> diff --git a/arch/arm64/include/asm/daifflags.h b/arch/arm64/include/asm/daifflags.h
> index ec213b4a1650..3efa240a6c48 100644
> --- a/arch/arm64/include/asm/daifflags.h
> +++ b/arch/arm64/include/asm/daifflags.h
> @@ -22,7 +22,7 @@
> static inline void local_daif_mask(void)
> {
> WARN_ON(system_has_prio_mask_debugging() &&
> - (read_sysreg_s(SYS_ICC_PMR_EL1) == (GIC_PRIO_IRQOFF |
> + (read_sysreg_s(SYS_ICC_PMR_EL1) == (gic_prio_irqoff() |
> GIC_PRIO_PSR_I_SET)));
>
> asm volatile(
> @@ -87,7 +87,7 @@ static inline void local_daif_restore(unsigned long flags)
> * asynchronous errors, we can take NMIs
> */
> flags &= ~PSR_I_BIT;
> - pmr = GIC_PRIO_IRQOFF;
> + pmr = gic_prio_irqoff();
> } else {
> pmr = GIC_PRIO_IRQON | GIC_PRIO_PSR_I_SET;
> }
> diff --git a/arch/arm64/include/asm/irqflags.h b/arch/arm64/include/asm/irqflags.h
> index aa4b6521ef14..af353c78d5f8 100644
> --- a/arch/arm64/include/asm/irqflags.h
> +++ b/arch/arm64/include/asm/irqflags.h
> @@ -28,10 +28,13 @@
> */
> static inline void arch_local_irq_enable(void)
> {
> + u64 pmr_irqon = GIC_PRIO_IRQON;
> +
> if (system_has_prio_mask_debugging()) {
> - u32 pmr = read_sysreg_s(SYS_ICC_PMR_EL1);
> + u64 pmr = read_sysreg_s(SYS_ICC_PMR_EL1);
> + u64 pmr_irqoff = gic_prio_irqoff();
>
> - WARN_ON_ONCE(pmr != GIC_PRIO_IRQON && pmr != GIC_PRIO_IRQOFF);
> + WARN_ON_ONCE(pmr != pmr_irqon && pmr != pmr_irqoff);
> }
>
> asm volatile(ALTERNATIVE(
> @@ -39,7 +42,7 @@ static inline void arch_local_irq_enable(void)
> __msr_s(SYS_ICC_PMR_EL1, "%0"),
> ARM64_HAS_IRQ_PRIO_MASKING)
> :
> - : "r" ((unsigned long) GIC_PRIO_IRQON)
> + : "r" (pmr_irqon)
> : "memory");
>
> pmr_sync();
> @@ -47,10 +50,13 @@ static inline void arch_local_irq_enable(void)
>
> static inline void arch_local_irq_disable(void)
> {
> + u64 pmr_irqoff = gic_prio_irqoff();
> +
> if (system_has_prio_mask_debugging()) {
> - u32 pmr = read_sysreg_s(SYS_ICC_PMR_EL1);
> + u64 pmr = read_sysreg_s(SYS_ICC_PMR_EL1);
> + u64 pmr_irqon = GIC_PRIO_IRQON;
>
> - WARN_ON_ONCE(pmr != GIC_PRIO_IRQON && pmr != GIC_PRIO_IRQOFF);
> + WARN_ON_ONCE(pmr != pmr_irqon && pmr != pmr_irqoff);
> }
>
> asm volatile(ALTERNATIVE(
> @@ -58,7 +64,7 @@ static inline void arch_local_irq_disable(void)
> __msr_s(SYS_ICC_PMR_EL1, "%0"),
> ARM64_HAS_IRQ_PRIO_MASKING)
> :
> - : "r" ((unsigned long) GIC_PRIO_IRQOFF)
> + : "r" (pmr_irqoff)
> : "memory");
> }
I believe all the changes in this file can be avoided, see below.
>
> diff --git a/arch/arm64/include/asm/ptrace.h b/arch/arm64/include/asm/ptrace.h
> index 966ed30ed5f7..a19cd6ff4d1b 100644
> --- a/arch/arm64/include/asm/ptrace.h
> +++ b/arch/arm64/include/asm/ptrace.h
> @@ -32,6 +32,7 @@
> */
> #define GIC_PRIO_IRQON 0xe0
> #define GIC_PRIO_IRQOFF (GIC_PRIO_IRQON & ~0x80)
> +#define GIC_PRIO_IRQOFF_NS 0xa0
> #define GIC_PRIO_PSR_I_SET (1 << 4)
>
> /* Additional SPSR bits not exposed in the UABI */
> @@ -129,6 +130,17 @@
> #define compat_sp_fiq regs[29]
> #define compat_lr_fiq regs[30]
>
> +#define gic_prio_irqoff() \
> + ({ \
> + extern struct static_key_false gic_nonsecure_priorities;\
> + u8 __prio = GIC_PRIO_IRQOFF; \
> + \
> + if (static_branch_unlikely(&gic_nonsecure_priorities)) \
> + __prio = GIC_PRIO_IRQOFF_NS; \
> + \
> + __prio; \
> + })
> +
This single change is causing quite a lot of churn, most of which
could be avoided if you actually reused the macro name:
diff --git a/arch/arm64/include/asm/ptrace.h b/arch/arm64/include/asm/ptrace.h
index 966ed30ed5f7..f85a00817fa5 100644
--- a/arch/arm64/include/asm/ptrace.h
+++ b/arch/arm64/include/asm/ptrace.h
@@ -31,9 +31,21 @@
* interrupt disabling temporarily does not rely on IRQ priorities.
*/
#define GIC_PRIO_IRQON 0xe0
-#define GIC_PRIO_IRQOFF (GIC_PRIO_IRQON & ~0x80)
+#define __GIC_PRIO_IRQOFF (GIC_PRIO_IRQON & ~0x80)
+#define __GIC_PRIO_IRQOFF_NS 0xa0
#define GIC_PRIO_PSR_I_SET (1 << 4)
+#define GIC_PRIO_IRQOFF \
+ ({ \
+ extern struct static_key_false gic_nonsecure_priorities;\
+ u8 __prio = __GIC_PRIO_IRQOFF; \
+ \
+ if (static_branch_unlikely(&gic_nonsecure_priorities)) \
+ __prio = __GIC_PRIO_IRQOFF_NS; \
+ \
+ __prio; \
+ })
+
/* Additional SPSR bits not exposed in the UABI */
#define PSR_MODE_THREAD_BIT (1 << 0)
#define PSR_IL_BIT (1 << 20)
"With this simple trick", a lot of the churn in this patch vanishes:
arch/arm64/include/asm/arch_gicv3.h | 8 +++++-
arch/arm64/include/asm/ptrace.h | 14 +++++++++-
arch/arm64/kernel/entry.S | 2 +-
arch/arm64/kernel/image-vars.h | 2 ++
drivers/irqchip/irq-gic-v3.c | 52 ++++++++++++++++++++++++++++---------
5 files changed, 63 insertions(+), 15 deletions(-)
> static inline unsigned long compat_psr_to_pstate(const unsigned long psr)
> {
> unsigned long pstate;
> diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
> index 2646178c8329..e4fa944dbf1d 100644
> --- a/arch/arm64/kernel/entry.S
> +++ b/arch/arm64/kernel/entry.S
> @@ -653,7 +653,7 @@ alternative_else_nop_endif
> #ifdef CONFIG_ARM64_PSEUDO_NMI
> /*
> * When using IRQ priority masking, we can get spurious interrupts while
> - * PMR is set to GIC_PRIO_IRQOFF. An NMI might also have occurred in a
> + * PMR is set to mask interrupts. An NMI might also have occurred in a
> * section with interrupts disabled. Skip tracing in those cases.
> */
> test_irqs_unmasked res=x0, pmr=x20
> diff --git a/arch/arm64/kernel/image-vars.h b/arch/arm64/kernel/image-vars.h
> index 9e897c500237..c4476a99dee8 100644
> --- a/arch/arm64/kernel/image-vars.h
> +++ b/arch/arm64/kernel/image-vars.h
> @@ -101,6 +101,8 @@ KVM_NVHE_ALIAS(vgic_v3_cpuif_trap);
> /* Static key checked in pmr_sync(). */
> #ifdef CONFIG_ARM64_PSEUDO_NMI
> KVM_NVHE_ALIAS(gic_pmr_sync);
> +/* Static key checked in gic_prio_irqoff(). */
> +KVM_NVHE_ALIAS(gic_nonsecure_priorities);
> #endif
>
> #endif /* CONFIG_KVM */
> diff --git a/arch/arm64/kvm/hyp/nvhe/switch.c b/arch/arm64/kvm/hyp/nvhe/switch.c
> index 341be2f2f312..729a3a59ad6a 100644
> --- a/arch/arm64/kvm/hyp/nvhe/switch.c
> +++ b/arch/arm64/kvm/hyp/nvhe/switch.c
> @@ -237,7 +237,7 @@ int __kvm_vcpu_run(struct kvm_vcpu *vcpu)
>
> /* Returning to host will clear PSR.I, remask PMR if needed */
> if (system_uses_irq_prio_masking())
> - gic_write_pmr(GIC_PRIO_IRQOFF);
> + gic_write_pmr(gic_prio_irqoff());
>
> return exit_code;
> }
> diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
> index ce8944ae1b84..19e52c025c59 100644
> --- a/drivers/irqchip/irq-gic-v3.c
> +++ b/drivers/irqchip/irq-gic-v3.c
> @@ -75,16 +75,14 @@ static DEFINE_STATIC_KEY_TRUE(supports_deactivate_key);
> *
> * If SCR_EL3.FIQ == 1, the values writen to/read from PMR and RPR at non-secure
> * EL1 are subject to a similar operation thus matching the priorities presented
> - * from the (re)distributor when security is enabled.
> + * from the (re)distributor when security is enabled. When SCR_EL3.FIQ == 0,
> + * these values are unchanched by the GIC.
> *
> * see GICv3/GICv4 Architecture Specification (IHI0069D):
> * - section 4.8.1 Non-secure accesses to register fields for Secure interrupt
> * priorities.
> * - Figure 4-7 Secure read of the priority field for a Non-secure Group 1
> * interrupt.
> - *
> - * For now, we only support pseudo-NMIs if we have non-secure view of
> - * priorities.
> */
> static DEFINE_STATIC_KEY_FALSE(supports_pseudo_nmis);
>
> @@ -97,6 +95,9 @@ static DEFINE_STATIC_KEY_FALSE(supports_pseudo_nmis);
> DEFINE_STATIC_KEY_FALSE(gic_pmr_sync);
> EXPORT_SYMBOL(gic_pmr_sync);
>
> +DEFINE_STATIC_KEY_FALSE(gic_nonsecure_priorities);
> +EXPORT_SYMBOL(gic_nonsecure_priorities);
> +
> /* ppi_nmi_refs[n] == number of cpus having ppi[n + 16] set as NMI */
> static refcount_t *ppi_nmi_refs;
>
> @@ -932,14 +933,16 @@ static void gic_cpu_sys_reg_init(void)
> /* Set priority mask register */
> if (!gic_prio_masking_enabled()) {
> write_gicreg(DEFAULT_PMR_VALUE, ICC_PMR_EL1);
> - } else {
> + } else if (gic_supports_nmi()) {
> /*
> * Mismatch configuration with boot CPU, the system is likely
> * to die as interrupt masking will not work properly on all
> * CPUs
> */
> - WARN_ON(gic_supports_nmi() && group0 &&
> - !gic_dist_security_disabled());
> + if (static_branch_unlikely(&gic_nonsecure_priorities))
> + WARN_ON(!group0 || gic_dist_security_disabled());
> + else
> + WARN_ON(group0 && !gic_dist_security_disabled());
It'd be worth adding a comment saying that this never runs on the boot
CPU (I just spent 10 minutes wondering why this worked).
> }
>
> /*
> @@ -1544,11 +1547,6 @@ static void gic_enable_nmi_support(void)
> if (!gic_prio_masking_enabled())
> return;
>
> - if (gic_has_group0() && !gic_dist_security_disabled()) {
> - pr_warn("SCR_EL3.FIQ is cleared, cannot enable use of pseudo-NMIs\n");
> - return;
> - }
> -
> ppi_nmi_refs = kcalloc(gic_data.ppi_nr, sizeof(*ppi_nmi_refs), GFP_KERNEL);
> if (!ppi_nmi_refs)
> return;
> @@ -1567,6 +1565,36 @@ static void gic_enable_nmi_support(void)
> pr_info("Pseudo-NMIs enabled using %s ICC_PMR_EL1 synchronisation\n",
> static_branch_unlikely(&gic_pmr_sync) ? "forced" : "relaxed");
>
> + /*
> + * How priority values are used by the GIC depends on two things:
> + * the security state of the GIC (controlled by the GICD_CTRL.DS bit)
> + * and if Group 0 interrupts can be delivered to Linux in the non-secure
> + * world as FIQs (controlled by the SCR_EL3.FIQ bit). These affect the
> + * the ICC_PMR_EL1 register and the priority that software assigns to
> + * interrupts:
> + *
> + * GICD_CTRL.DS | SCR_EL3.FIQ | ICC_PMR_EL1 | Group 1 priority
> + * -----------------------------------------------------------
> + * 1 | - | unchanged | unchanged
> + * -----------------------------------------------------------
> + * 0 | 1 | non-secure | non-secure
> + * -----------------------------------------------------------
> + * 0 | 0 | unchanged | non-secure
> + *
> + * where non-secure means that the value is right-shifted by one and the
> + * MSB bit set, to make it fit in the non-secure priority range.
> + *
> + * In the first two cases, where ICC_PMR_EL1 and the interrupt priority
> + * are both either modified, or unchanged, we can use the same set of
> + * priorities.
> + *
> + * In the last case, where only the interrupt priorities are modified to
> + * be in the non-secure range, we use a different PMR value to mask IRQs
> + * and the rest of the values that we use remain unchanged.
> + */
> + if (gic_has_group0() && !gic_dist_security_disabled())
> + static_branch_enable(&gic_nonsecure_priorities);
> +
> static_branch_enable(&supports_pseudo_nmis);
>
> if (static_branch_likely(&supports_deactivate_key))
> --
> 2.28.0
>
>
Otherwise, this looks pretty good, and I'd like to take this into 5.10
if you can respin it quickly.
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
Hi Marc,
On 9/12/20 1:00 PM, Marc Zyngier wrote:
> Hi Alex,
>
> Thanks for taking the time for putting this together, as it has been a
> long standing issue that needed fixing.
>
> On Wed, 19 Aug 2020 14:36:30 +0100,
> Alexandru Elisei <[email protected]> wrote:
>> The GIC's internal view of the priority mask register and the assigned
>> interrupt priorities are based on whether GIC security is enabled and
>> whether firmware routes Group 0 interrupts to EL3. At the moment, we
>> support priority masking when ICC_PMR_EL1 and interrupt priorities are
>> either both modified by the GIC, or both left unchanged.
>>
>> Trusted Firmware-A's default interrupt routing model allows Group 0
>> interrupts to be delivered to the non-secure world (SCR_EL3.FIQ == 0).
>> Unfortunately, this is precisely the case that the GIC driver doesn't
>> support: ICC_PMR_EL1 remains unchanged, but the GIC's view of interrupt
>> priorities is different from the software programmed values.
>>
>> Support pseudo-NMIs when SCR_EL3.FIQ == 0 by using a different value to
>> mask regular interrupts. All the other values remain the same.
>>
>> Signed-off-by: Alexandru Elisei <[email protected]>
>> ---
>> arch/arm64/include/asm/arch_gicv3.h | 8 ++++-
>> arch/arm64/include/asm/daifflags.h | 4 +--
>> arch/arm64/include/asm/irqflags.h | 18 ++++++----
>> arch/arm64/include/asm/ptrace.h | 12 +++++++
>> arch/arm64/kernel/entry.S | 2 +-
>> arch/arm64/kernel/image-vars.h | 2 ++
>> arch/arm64/kvm/hyp/nvhe/switch.c | 2 +-
>> drivers/irqchip/irq-gic-v3.c | 52 ++++++++++++++++++++++-------
>> 8 files changed, 77 insertions(+), 23 deletions(-)
>>
>> diff --git a/arch/arm64/include/asm/arch_gicv3.h b/arch/arm64/include/asm/arch_gicv3.h
>> index 6647ae4f0231..908152e8659b 100644
>> --- a/arch/arm64/include/asm/arch_gicv3.h
>> +++ b/arch/arm64/include/asm/arch_gicv3.h
>> @@ -162,7 +162,13 @@ static inline void gic_pmr_mask_irqs(void)
>> * are applied to IRQ priorities
>> */
>> BUILD_BUG_ON((0x80 | (GICD_INT_DEF_PRI >> 1)) >= GIC_PRIO_IRQON);
>> - gic_write_pmr(GIC_PRIO_IRQOFF);
>> + /*
>> + * Same situation as above, but now we make sure that we can mask
>> + * regular interrupts.
>> + */
>> + BUILD_BUG_ON((0x80 | (GICD_INT_DEF_PRI >> 1)) < (GIC_PRIO_IRQOFF_NS |
>> + GIC_PRIO_PSR_I_SET));
>> + gic_write_pmr(gic_prio_irqoff());
>> }
>>
>> static inline void gic_arch_enable_irqs(void)
>> diff --git a/arch/arm64/include/asm/daifflags.h b/arch/arm64/include/asm/daifflags.h
>> index ec213b4a1650..3efa240a6c48 100644
>> --- a/arch/arm64/include/asm/daifflags.h
>> +++ b/arch/arm64/include/asm/daifflags.h
>> @@ -22,7 +22,7 @@
>> static inline void local_daif_mask(void)
>> {
>> WARN_ON(system_has_prio_mask_debugging() &&
>> - (read_sysreg_s(SYS_ICC_PMR_EL1) == (GIC_PRIO_IRQOFF |
>> + (read_sysreg_s(SYS_ICC_PMR_EL1) == (gic_prio_irqoff() |
>> GIC_PRIO_PSR_I_SET)));
>>
>> asm volatile(
>> @@ -87,7 +87,7 @@ static inline void local_daif_restore(unsigned long flags)
>> * asynchronous errors, we can take NMIs
>> */
>> flags &= ~PSR_I_BIT;
>> - pmr = GIC_PRIO_IRQOFF;
>> + pmr = gic_prio_irqoff();
>> } else {
>> pmr = GIC_PRIO_IRQON | GIC_PRIO_PSR_I_SET;
>> }
>> diff --git a/arch/arm64/include/asm/irqflags.h b/arch/arm64/include/asm/irqflags.h
>> index aa4b6521ef14..af353c78d5f8 100644
>> --- a/arch/arm64/include/asm/irqflags.h
>> +++ b/arch/arm64/include/asm/irqflags.h
>> @@ -28,10 +28,13 @@
>> */
>> static inline void arch_local_irq_enable(void)
>> {
>> + u64 pmr_irqon = GIC_PRIO_IRQON;
>> +
>> if (system_has_prio_mask_debugging()) {
>> - u32 pmr = read_sysreg_s(SYS_ICC_PMR_EL1);
>> + u64 pmr = read_sysreg_s(SYS_ICC_PMR_EL1);
>> + u64 pmr_irqoff = gic_prio_irqoff();
>>
>> - WARN_ON_ONCE(pmr != GIC_PRIO_IRQON && pmr != GIC_PRIO_IRQOFF);
>> + WARN_ON_ONCE(pmr != pmr_irqon && pmr != pmr_irqoff);
>> }
>>
>> asm volatile(ALTERNATIVE(
>> @@ -39,7 +42,7 @@ static inline void arch_local_irq_enable(void)
>> __msr_s(SYS_ICC_PMR_EL1, "%0"),
>> ARM64_HAS_IRQ_PRIO_MASKING)
>> :
>> - : "r" ((unsigned long) GIC_PRIO_IRQON)
>> + : "r" (pmr_irqon)
>> : "memory");
>>
>> pmr_sync();
>> @@ -47,10 +50,13 @@ static inline void arch_local_irq_enable(void)
>>
>> static inline void arch_local_irq_disable(void)
>> {
>> + u64 pmr_irqoff = gic_prio_irqoff();
>> +
>> if (system_has_prio_mask_debugging()) {
>> - u32 pmr = read_sysreg_s(SYS_ICC_PMR_EL1);
>> + u64 pmr = read_sysreg_s(SYS_ICC_PMR_EL1);
>> + u64 pmr_irqon = GIC_PRIO_IRQON;
>>
>> - WARN_ON_ONCE(pmr != GIC_PRIO_IRQON && pmr != GIC_PRIO_IRQOFF);
>> + WARN_ON_ONCE(pmr != pmr_irqon && pmr != pmr_irqoff);
>> }
>>
>> asm volatile(ALTERNATIVE(
>> @@ -58,7 +64,7 @@ static inline void arch_local_irq_disable(void)
>> __msr_s(SYS_ICC_PMR_EL1, "%0"),
>> ARM64_HAS_IRQ_PRIO_MASKING)
>> :
>> - : "r" ((unsigned long) GIC_PRIO_IRQOFF)
>> + : "r" (pmr_irqoff)
>> : "memory");
>> }
> I believe all the changes in this file can be avoided, see below.
>
>>
>> diff --git a/arch/arm64/include/asm/ptrace.h b/arch/arm64/include/asm/ptrace.h
>> index 966ed30ed5f7..a19cd6ff4d1b 100644
>> --- a/arch/arm64/include/asm/ptrace.h
>> +++ b/arch/arm64/include/asm/ptrace.h
>> @@ -32,6 +32,7 @@
>> */
>> #define GIC_PRIO_IRQON 0xe0
>> #define GIC_PRIO_IRQOFF (GIC_PRIO_IRQON & ~0x80)
>> +#define GIC_PRIO_IRQOFF_NS 0xa0
>> #define GIC_PRIO_PSR_I_SET (1 << 4)
>>
>> /* Additional SPSR bits not exposed in the UABI */
>> @@ -129,6 +130,17 @@
>> #define compat_sp_fiq regs[29]
>> #define compat_lr_fiq regs[30]
>>
>> +#define gic_prio_irqoff() \
>> + ({ \
>> + extern struct static_key_false gic_nonsecure_priorities;\
>> + u8 __prio = GIC_PRIO_IRQOFF; \
>> + \
>> + if (static_branch_unlikely(&gic_nonsecure_priorities)) \
>> + __prio = GIC_PRIO_IRQOFF_NS; \
>> + \
>> + __prio; \
>> + })
>> +
> This single change is causing quite a lot of churn, most of which
> could be avoided if you actually reused the macro name:
>
> diff --git a/arch/arm64/include/asm/ptrace.h b/arch/arm64/include/asm/ptrace.h
> index 966ed30ed5f7..f85a00817fa5 100644
> --- a/arch/arm64/include/asm/ptrace.h
> +++ b/arch/arm64/include/asm/ptrace.h
> @@ -31,9 +31,21 @@
> * interrupt disabling temporarily does not rely on IRQ priorities.
> */
> #define GIC_PRIO_IRQON 0xe0
> -#define GIC_PRIO_IRQOFF (GIC_PRIO_IRQON & ~0x80)
> +#define __GIC_PRIO_IRQOFF (GIC_PRIO_IRQON & ~0x80)
> +#define __GIC_PRIO_IRQOFF_NS 0xa0
> #define GIC_PRIO_PSR_I_SET (1 << 4)
>
> +#define GIC_PRIO_IRQOFF \
> + ({ \
> + extern struct static_key_false gic_nonsecure_priorities;\
> + u8 __prio = __GIC_PRIO_IRQOFF; \
> + \
> + if (static_branch_unlikely(&gic_nonsecure_priorities)) \
> + __prio = __GIC_PRIO_IRQOFF_NS; \
> + \
> + __prio; \
> + })
> +
> /* Additional SPSR bits not exposed in the UABI */
> #define PSR_MODE_THREAD_BIT (1 << 0)
> #define PSR_IL_BIT (1 << 20)
>
> "With this simple trick", a lot of the churn in this patch vanishes:
That's a very good idea, and the diffstat looks much better. I'll change it for v3.
>
> arch/arm64/include/asm/arch_gicv3.h | 8 +++++-
> arch/arm64/include/asm/ptrace.h | 14 +++++++++-
> arch/arm64/kernel/entry.S | 2 +-
> arch/arm64/kernel/image-vars.h | 2 ++
> drivers/irqchip/irq-gic-v3.c | 52 ++++++++++++++++++++++++++++---------
> 5 files changed, 63 insertions(+), 15 deletions(-)
>
>> static inline unsigned long compat_psr_to_pstate(const unsigned long psr)
>> {
>> unsigned long pstate;
>> diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
>> index 2646178c8329..e4fa944dbf1d 100644
>> --- a/arch/arm64/kernel/entry.S
>> +++ b/arch/arm64/kernel/entry.S
>> @@ -653,7 +653,7 @@ alternative_else_nop_endif
>> #ifdef CONFIG_ARM64_PSEUDO_NMI
>> /*
>> * When using IRQ priority masking, we can get spurious interrupts while
>> - * PMR is set to GIC_PRIO_IRQOFF. An NMI might also have occurred in a
>> + * PMR is set to mask interrupts. An NMI might also have occurred in a
>> * section with interrupts disabled. Skip tracing in those cases.
>> */
>> test_irqs_unmasked res=x0, pmr=x20
>> diff --git a/arch/arm64/kernel/image-vars.h b/arch/arm64/kernel/image-vars.h
>> index 9e897c500237..c4476a99dee8 100644
>> --- a/arch/arm64/kernel/image-vars.h
>> +++ b/arch/arm64/kernel/image-vars.h
>> @@ -101,6 +101,8 @@ KVM_NVHE_ALIAS(vgic_v3_cpuif_trap);
>> /* Static key checked in pmr_sync(). */
>> #ifdef CONFIG_ARM64_PSEUDO_NMI
>> KVM_NVHE_ALIAS(gic_pmr_sync);
>> +/* Static key checked in gic_prio_irqoff(). */
>> +KVM_NVHE_ALIAS(gic_nonsecure_priorities);
>> #endif
>>
>> #endif /* CONFIG_KVM */
>> diff --git a/arch/arm64/kvm/hyp/nvhe/switch.c b/arch/arm64/kvm/hyp/nvhe/switch.c
>> index 341be2f2f312..729a3a59ad6a 100644
>> --- a/arch/arm64/kvm/hyp/nvhe/switch.c
>> +++ b/arch/arm64/kvm/hyp/nvhe/switch.c
>> @@ -237,7 +237,7 @@ int __kvm_vcpu_run(struct kvm_vcpu *vcpu)
>>
>> /* Returning to host will clear PSR.I, remask PMR if needed */
>> if (system_uses_irq_prio_masking())
>> - gic_write_pmr(GIC_PRIO_IRQOFF);
>> + gic_write_pmr(gic_prio_irqoff());
>>
>> return exit_code;
>> }
>> diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
>> index ce8944ae1b84..19e52c025c59 100644
>> --- a/drivers/irqchip/irq-gic-v3.c
>> +++ b/drivers/irqchip/irq-gic-v3.c
>> @@ -75,16 +75,14 @@ static DEFINE_STATIC_KEY_TRUE(supports_deactivate_key);
>> *
>> * If SCR_EL3.FIQ == 1, the values writen to/read from PMR and RPR at non-secure
>> * EL1 are subject to a similar operation thus matching the priorities presented
>> - * from the (re)distributor when security is enabled.
>> + * from the (re)distributor when security is enabled. When SCR_EL3.FIQ == 0,
>> + * these values are unchanched by the GIC.
>> *
>> * see GICv3/GICv4 Architecture Specification (IHI0069D):
>> * - section 4.8.1 Non-secure accesses to register fields for Secure interrupt
>> * priorities.
>> * - Figure 4-7 Secure read of the priority field for a Non-secure Group 1
>> * interrupt.
>> - *
>> - * For now, we only support pseudo-NMIs if we have non-secure view of
>> - * priorities.
>> */
>> static DEFINE_STATIC_KEY_FALSE(supports_pseudo_nmis);
>>
>> @@ -97,6 +95,9 @@ static DEFINE_STATIC_KEY_FALSE(supports_pseudo_nmis);
>> DEFINE_STATIC_KEY_FALSE(gic_pmr_sync);
>> EXPORT_SYMBOL(gic_pmr_sync);
>>
>> +DEFINE_STATIC_KEY_FALSE(gic_nonsecure_priorities);
>> +EXPORT_SYMBOL(gic_nonsecure_priorities);
>> +
>> /* ppi_nmi_refs[n] == number of cpus having ppi[n + 16] set as NMI */
>> static refcount_t *ppi_nmi_refs;
>>
>> @@ -932,14 +933,16 @@ static void gic_cpu_sys_reg_init(void)
>> /* Set priority mask register */
>> if (!gic_prio_masking_enabled()) {
>> write_gicreg(DEFAULT_PMR_VALUE, ICC_PMR_EL1);
>> - } else {
>> + } else if (gic_supports_nmi()) {
>> /*
>> * Mismatch configuration with boot CPU, the system is likely
>> * to die as interrupt masking will not work properly on all
>> * CPUs
>> */
>> - WARN_ON(gic_supports_nmi() && group0 &&
>> - !gic_dist_security_disabled());
>> + if (static_branch_unlikely(&gic_nonsecure_priorities))
>> + WARN_ON(!group0 || gic_dist_security_disabled());
>> + else
>> + WARN_ON(group0 && !gic_dist_security_disabled());
> It'd be worth adding a comment saying that this never runs on the boot
> CPU (I just spent 10 minutes wondering why this worked).
Sure, I'll update the comment to state that this branch is only taken before the
boot CPU has enabled pseudo-NMIs, because the call to gic_enable_nmi_support()
comes after gic_cpu_init() in gic_init_bases().
>
>> }
>>
>> /*
>> @@ -1544,11 +1547,6 @@ static void gic_enable_nmi_support(void)
>> if (!gic_prio_masking_enabled())
>> return;
>>
>> - if (gic_has_group0() && !gic_dist_security_disabled()) {
>> - pr_warn("SCR_EL3.FIQ is cleared, cannot enable use of pseudo-NMIs\n");
>> - return;
>> - }
>> -
>> ppi_nmi_refs = kcalloc(gic_data.ppi_nr, sizeof(*ppi_nmi_refs), GFP_KERNEL);
>> if (!ppi_nmi_refs)
>> return;
>> @@ -1567,6 +1565,36 @@ static void gic_enable_nmi_support(void)
>> pr_info("Pseudo-NMIs enabled using %s ICC_PMR_EL1 synchronisation\n",
>> static_branch_unlikely(&gic_pmr_sync) ? "forced" : "relaxed");
>>
>> + /*
>> + * How priority values are used by the GIC depends on two things:
>> + * the security state of the GIC (controlled by the GICD_CTRL.DS bit)
>> + * and if Group 0 interrupts can be delivered to Linux in the non-secure
>> + * world as FIQs (controlled by the SCR_EL3.FIQ bit). These affect the
>> + * the ICC_PMR_EL1 register and the priority that software assigns to
>> + * interrupts:
>> + *
>> + * GICD_CTRL.DS | SCR_EL3.FIQ | ICC_PMR_EL1 | Group 1 priority
>> + * -----------------------------------------------------------
>> + * 1 | - | unchanged | unchanged
>> + * -----------------------------------------------------------
>> + * 0 | 1 | non-secure | non-secure
>> + * -----------------------------------------------------------
>> + * 0 | 0 | unchanged | non-secure
>> + *
>> + * where non-secure means that the value is right-shifted by one and the
>> + * MSB bit set, to make it fit in the non-secure priority range.
>> + *
>> + * In the first two cases, where ICC_PMR_EL1 and the interrupt priority
>> + * are both either modified, or unchanged, we can use the same set of
>> + * priorities.
>> + *
>> + * In the last case, where only the interrupt priorities are modified to
>> + * be in the non-secure range, we use a different PMR value to mask IRQs
>> + * and the rest of the values that we use remain unchanged.
>> + */
>> + if (gic_has_group0() && !gic_dist_security_disabled())
>> + static_branch_enable(&gic_nonsecure_priorities);
>> +
>> static_branch_enable(&supports_pseudo_nmis);
>>
>> if (static_branch_likely(&supports_deactivate_key))
>> --
>> 2.28.0
>>
>>
> Otherwise, this looks pretty good, and I'd like to take this into 5.10
> if you can respin it quickly.
Sure, I'll respin it as soon as possible and send a v3.
Have you tested the series using the PMU NMI branch from the cover letter? If so,
would you mind giving a Tested-by tag for that series [1]?
[1] https://lkml.org/lkml/2020/8/19/671
Thanks,
Alex
On 2020-09-12 14:34, Alexandru Elisei wrote:
> Hi Marc,
[...]
> Have you tested the series using the PMU NMI branch from the cover
> letter? If so, would you mind giving a Tested-by tag for that series
> [1]?
I haven't had a chance, but that's next on my list once I've gone
through some other bits of review that have been lingering in my
inbox for too long...
M.
--
Jazz is not dead. It just smells funny...