All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] irqchip/gic-v3: Support pseudo-NMIs when SCR_EL3.FIQ == 0
@ 2020-06-25 15:00 ` Alexandru Elisei
  0 siblings, 0 replies; 16+ messages in thread
From: Alexandru Elisei @ 2020-06-25 15:00 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel, kvmarm
  Cc: maz, tglx, jason, yuzenghui, julien.thierry.kdev, will,
	catalin.marinas, mark.rutland

Trusted Firmware-A's default interrupt routing model is to clear
SCR_EL3.FIQ, which is the only case that GICv3 doesn't support. This series
tries to fix that by detecting it at runtime and using a different priority
value for ICC_PMR_EL1 when masking regular interrupts. As a result, we will
be able to support pseudo-NMIs on all combinations of GIC security states
and firmware configurations.

The series is based on v5.8-rc1, the same as the PMU NMI patches [1] which
I used for testing.

The first patch was there because when I started working on the PMU NMI
patches I found it confusing that there was no clear message stating that
NMIs were successfully enabled. The second patch is the main patch of the
series, where pseudo-NMIs are enabled even if SCR_EL3.FIQ == 0.

The are still some things I'm not 100% sure about regarding the last patch:

- From my very limited experience of trying pseudo-NMIs on 3 machines
  (rockpro64, espressobin-v5 and v7), all of them had SCR_EL3.FIQ zero.
  I tend to believe that since this is the default on TFA, this will also
  be the common case on hardware. However, when Linux is a KVM guest, the
  original set of priorities is used because GIC security is disabled.
  I erred on the side of caution and chose the original set of priorities
  as the common case.

- Most of the changes to arch_local_irq_enable() might seem pointless, but
  I chose to make them so the function looks similar to
  arch_local_irq_disable(). The generated code is identical if the static
  branch is not taken. I tried changing only arch_local_irq_disable(), but
  the enable function ended up looking strangely asymmetrical. If someone
  suggests a better way of doing things, I'd be happy to implement it.

As an aside, the set of priorities that I've added would work in all cases
if there was no need to use GIC_PRIO_PSR_I_SET, but that is a much more
intrusive change and I'm not comfortable attempting it. I'm pretty sure I
will end up breaking things really badly.

I've tested the series using PMU NMIs [1] on the model and on
espressobin-v7.

1. On the model:
- Host with SCR_EL3.FIQ == 1 (so using the original priorities), ran
  perf record -a -- iperf3 -c 127.0.0.1 -t 30.
- On a KVM guest (security disabled, so using the original priorities),
  ran the same command as above.

2. On an espressobin-v7:
- Host with SCR_EL3.FIQ == 0 (using the priority added by the series),
  ran perf record -a -- iperf3 -c 127.0.0.1 -t 60.
- On a KVM guest (security disabled, so using the original priorities),
  ran the same command.
- Stress test for an hour with CONFIG_ARM64_DEBUG_PRIORITY_MASKING set for
  the host and guest. On the host, I ran in parallel
  perf record -a -- iperf3 -c 127.0.0.1 -t 3600  and
  perf record -ae L1-dcache-loads -a -- sleep 3600. On the guest, I
  ran the same iperf3 command as on the host.

[1] https://www.spinics.net/lists/kernel/msg3554236.html

Alexandru Elisei (2):
  irqchip/gicv3: Spell out when pseudo-NMIs are enabled
  irqchip/gic-v3: Support pseudo-NMIs when SCR_EL3.FIQ == 0

 arch/arm64/include/asm/arch_gicv3.h |  8 ++++-
 arch/arm64/include/asm/daifflags.h  |  4 +--
 arch/arm64/include/asm/irqflags.h   | 14 +++++---
 arch/arm64/include/asm/ptrace.h     | 12 +++++++
 arch/arm64/kernel/entry.S           |  2 +-
 arch/arm64/kvm/hyp/switch.c         |  2 +-
 drivers/irqchip/irq-gic-v3.c        | 56 +++++++++++++++++++++--------
 7 files changed, 75 insertions(+), 23 deletions(-)

-- 
2.27.0


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

* [PATCH 0/2] irqchip/gic-v3: Support pseudo-NMIs when SCR_EL3.FIQ == 0
@ 2020-06-25 15:00 ` Alexandru Elisei
  0 siblings, 0 replies; 16+ messages in thread
From: Alexandru Elisei @ 2020-06-25 15:00 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel, kvmarm
  Cc: jason, maz, catalin.marinas, tglx, will

Trusted Firmware-A's default interrupt routing model is to clear
SCR_EL3.FIQ, which is the only case that GICv3 doesn't support. This series
tries to fix that by detecting it at runtime and using a different priority
value for ICC_PMR_EL1 when masking regular interrupts. As a result, we will
be able to support pseudo-NMIs on all combinations of GIC security states
and firmware configurations.

The series is based on v5.8-rc1, the same as the PMU NMI patches [1] which
I used for testing.

The first patch was there because when I started working on the PMU NMI
patches I found it confusing that there was no clear message stating that
NMIs were successfully enabled. The second patch is the main patch of the
series, where pseudo-NMIs are enabled even if SCR_EL3.FIQ == 0.

The are still some things I'm not 100% sure about regarding the last patch:

- From my very limited experience of trying pseudo-NMIs on 3 machines
  (rockpro64, espressobin-v5 and v7), all of them had SCR_EL3.FIQ zero.
  I tend to believe that since this is the default on TFA, this will also
  be the common case on hardware. However, when Linux is a KVM guest, the
  original set of priorities is used because GIC security is disabled.
  I erred on the side of caution and chose the original set of priorities
  as the common case.

- Most of the changes to arch_local_irq_enable() might seem pointless, but
  I chose to make them so the function looks similar to
  arch_local_irq_disable(). The generated code is identical if the static
  branch is not taken. I tried changing only arch_local_irq_disable(), but
  the enable function ended up looking strangely asymmetrical. If someone
  suggests a better way of doing things, I'd be happy to implement it.

As an aside, the set of priorities that I've added would work in all cases
if there was no need to use GIC_PRIO_PSR_I_SET, but that is a much more
intrusive change and I'm not comfortable attempting it. I'm pretty sure I
will end up breaking things really badly.

I've tested the series using PMU NMIs [1] on the model and on
espressobin-v7.

1. On the model:
- Host with SCR_EL3.FIQ == 1 (so using the original priorities), ran
  perf record -a -- iperf3 -c 127.0.0.1 -t 30.
- On a KVM guest (security disabled, so using the original priorities),
  ran the same command as above.

2. On an espressobin-v7:
- Host with SCR_EL3.FIQ == 0 (using the priority added by the series),
  ran perf record -a -- iperf3 -c 127.0.0.1 -t 60.
- On a KVM guest (security disabled, so using the original priorities),
  ran the same command.
- Stress test for an hour with CONFIG_ARM64_DEBUG_PRIORITY_MASKING set for
  the host and guest. On the host, I ran in parallel
  perf record -a -- iperf3 -c 127.0.0.1 -t 3600  and
  perf record -ae L1-dcache-loads -a -- sleep 3600. On the guest, I
  ran the same iperf3 command as on the host.

[1] https://www.spinics.net/lists/kernel/msg3554236.html

Alexandru Elisei (2):
  irqchip/gicv3: Spell out when pseudo-NMIs are enabled
  irqchip/gic-v3: Support pseudo-NMIs when SCR_EL3.FIQ == 0

 arch/arm64/include/asm/arch_gicv3.h |  8 ++++-
 arch/arm64/include/asm/daifflags.h  |  4 +--
 arch/arm64/include/asm/irqflags.h   | 14 +++++---
 arch/arm64/include/asm/ptrace.h     | 12 +++++++
 arch/arm64/kernel/entry.S           |  2 +-
 arch/arm64/kvm/hyp/switch.c         |  2 +-
 drivers/irqchip/irq-gic-v3.c        | 56 +++++++++++++++++++++--------
 7 files changed, 75 insertions(+), 23 deletions(-)

-- 
2.27.0

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

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

* [PATCH 0/2] irqchip/gic-v3: Support pseudo-NMIs when SCR_EL3.FIQ == 0
@ 2020-06-25 15:00 ` Alexandru Elisei
  0 siblings, 0 replies; 16+ messages in thread
From: Alexandru Elisei @ 2020-06-25 15:00 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel, kvmarm
  Cc: mark.rutland, jason, maz, catalin.marinas, yuzenghui, tglx, will,
	julien.thierry.kdev

Trusted Firmware-A's default interrupt routing model is to clear
SCR_EL3.FIQ, which is the only case that GICv3 doesn't support. This series
tries to fix that by detecting it at runtime and using a different priority
value for ICC_PMR_EL1 when masking regular interrupts. As a result, we will
be able to support pseudo-NMIs on all combinations of GIC security states
and firmware configurations.

The series is based on v5.8-rc1, the same as the PMU NMI patches [1] which
I used for testing.

The first patch was there because when I started working on the PMU NMI
patches I found it confusing that there was no clear message stating that
NMIs were successfully enabled. The second patch is the main patch of the
series, where pseudo-NMIs are enabled even if SCR_EL3.FIQ == 0.

The are still some things I'm not 100% sure about regarding the last patch:

- From my very limited experience of trying pseudo-NMIs on 3 machines
  (rockpro64, espressobin-v5 and v7), all of them had SCR_EL3.FIQ zero.
  I tend to believe that since this is the default on TFA, this will also
  be the common case on hardware. However, when Linux is a KVM guest, the
  original set of priorities is used because GIC security is disabled.
  I erred on the side of caution and chose the original set of priorities
  as the common case.

- Most of the changes to arch_local_irq_enable() might seem pointless, but
  I chose to make them so the function looks similar to
  arch_local_irq_disable(). The generated code is identical if the static
  branch is not taken. I tried changing only arch_local_irq_disable(), but
  the enable function ended up looking strangely asymmetrical. If someone
  suggests a better way of doing things, I'd be happy to implement it.

As an aside, the set of priorities that I've added would work in all cases
if there was no need to use GIC_PRIO_PSR_I_SET, but that is a much more
intrusive change and I'm not comfortable attempting it. I'm pretty sure I
will end up breaking things really badly.

I've tested the series using PMU NMIs [1] on the model and on
espressobin-v7.

1. On the model:
- Host with SCR_EL3.FIQ == 1 (so using the original priorities), ran
  perf record -a -- iperf3 -c 127.0.0.1 -t 30.
- On a KVM guest (security disabled, so using the original priorities),
  ran the same command as above.

2. On an espressobin-v7:
- Host with SCR_EL3.FIQ == 0 (using the priority added by the series),
  ran perf record -a -- iperf3 -c 127.0.0.1 -t 60.
- On a KVM guest (security disabled, so using the original priorities),
  ran the same command.
- Stress test for an hour with CONFIG_ARM64_DEBUG_PRIORITY_MASKING set for
  the host and guest. On the host, I ran in parallel
  perf record -a -- iperf3 -c 127.0.0.1 -t 3600  and
  perf record -ae L1-dcache-loads -a -- sleep 3600. On the guest, I
  ran the same iperf3 command as on the host.

[1] https://www.spinics.net/lists/kernel/msg3554236.html

Alexandru Elisei (2):
  irqchip/gicv3: Spell out when pseudo-NMIs are enabled
  irqchip/gic-v3: Support pseudo-NMIs when SCR_EL3.FIQ == 0

 arch/arm64/include/asm/arch_gicv3.h |  8 ++++-
 arch/arm64/include/asm/daifflags.h  |  4 +--
 arch/arm64/include/asm/irqflags.h   | 14 +++++---
 arch/arm64/include/asm/ptrace.h     | 12 +++++++
 arch/arm64/kernel/entry.S           |  2 +-
 arch/arm64/kvm/hyp/switch.c         |  2 +-
 drivers/irqchip/irq-gic-v3.c        | 56 +++++++++++++++++++++--------
 7 files changed, 75 insertions(+), 23 deletions(-)

-- 
2.27.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* [PATCH 1/2] irqchip/gicv3: Spell out when pseudo-NMIs are enabled
  2020-06-25 15:00 ` Alexandru Elisei
  (?)
@ 2020-06-25 15:00   ` Alexandru Elisei
  -1 siblings, 0 replies; 16+ messages in thread
From: Alexandru Elisei @ 2020-06-25 15:00 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel, kvmarm
  Cc: maz, tglx, jason, yuzenghui, julien.thierry.kdev, will,
	catalin.marinas, mark.rutland

When NMIs cannot be enabled, the driver prints a message stating that
unambiguously. When they are enabled, the only feedback we get is a message
regarding the use of synchronization for ICC_PMR_EL1 writes, which is not
as useful for a user who is not intimately familiar with how NMIs are
implemented.

Let's make it obvious that pseudo-NMIs are enabled. Keep the message about
using a barrier for ICC_PMR_EL1 writes, because it has a non-negligible
impact on performance.

Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
---
 drivers/irqchip/irq-gic-v3.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
index cc46bc2d634b..83103277d2a9 100644
--- a/drivers/irqchip/irq-gic-v3.c
+++ b/drivers/irqchip/irq-gic-v3.c
@@ -1564,8 +1564,8 @@ static void gic_enable_nmi_support(void)
 	if (gic_read_ctlr() & ICC_CTLR_EL1_PMHE_MASK)
 		static_branch_enable(&gic_pmr_sync);
 
-	pr_info("%s ICC_PMR_EL1 synchronisation\n",
-		static_branch_unlikely(&gic_pmr_sync) ? "Forcing" : "Relaxing");
+	pr_info("Pseudo-NMIs enabled using %s ICC_PMR_EL1 synchronisation\n",
+		static_branch_unlikely(&gic_pmr_sync) ? "forced" : "relaxed");
 
 	static_branch_enable(&supports_pseudo_nmis);
 
-- 
2.27.0


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

* [PATCH 1/2] irqchip/gicv3: Spell out when pseudo-NMIs are enabled
@ 2020-06-25 15:00   ` Alexandru Elisei
  0 siblings, 0 replies; 16+ messages in thread
From: Alexandru Elisei @ 2020-06-25 15:00 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel, kvmarm
  Cc: jason, maz, catalin.marinas, tglx, will

When NMIs cannot be enabled, the driver prints a message stating that
unambiguously. When they are enabled, the only feedback we get is a message
regarding the use of synchronization for ICC_PMR_EL1 writes, which is not
as useful for a user who is not intimately familiar with how NMIs are
implemented.

Let's make it obvious that pseudo-NMIs are enabled. Keep the message about
using a barrier for ICC_PMR_EL1 writes, because it has a non-negligible
impact on performance.

Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
---
 drivers/irqchip/irq-gic-v3.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
index cc46bc2d634b..83103277d2a9 100644
--- a/drivers/irqchip/irq-gic-v3.c
+++ b/drivers/irqchip/irq-gic-v3.c
@@ -1564,8 +1564,8 @@ static void gic_enable_nmi_support(void)
 	if (gic_read_ctlr() & ICC_CTLR_EL1_PMHE_MASK)
 		static_branch_enable(&gic_pmr_sync);
 
-	pr_info("%s ICC_PMR_EL1 synchronisation\n",
-		static_branch_unlikely(&gic_pmr_sync) ? "Forcing" : "Relaxing");
+	pr_info("Pseudo-NMIs enabled using %s ICC_PMR_EL1 synchronisation\n",
+		static_branch_unlikely(&gic_pmr_sync) ? "forced" : "relaxed");
 
 	static_branch_enable(&supports_pseudo_nmis);
 
-- 
2.27.0

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

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

* [PATCH 1/2] irqchip/gicv3: Spell out when pseudo-NMIs are enabled
@ 2020-06-25 15:00   ` Alexandru Elisei
  0 siblings, 0 replies; 16+ messages in thread
From: Alexandru Elisei @ 2020-06-25 15:00 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel, kvmarm
  Cc: mark.rutland, jason, maz, catalin.marinas, yuzenghui, tglx, will,
	julien.thierry.kdev

When NMIs cannot be enabled, the driver prints a message stating that
unambiguously. When they are enabled, the only feedback we get is a message
regarding the use of synchronization for ICC_PMR_EL1 writes, which is not
as useful for a user who is not intimately familiar with how NMIs are
implemented.

Let's make it obvious that pseudo-NMIs are enabled. Keep the message about
using a barrier for ICC_PMR_EL1 writes, because it has a non-negligible
impact on performance.

Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
---
 drivers/irqchip/irq-gic-v3.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
index cc46bc2d634b..83103277d2a9 100644
--- a/drivers/irqchip/irq-gic-v3.c
+++ b/drivers/irqchip/irq-gic-v3.c
@@ -1564,8 +1564,8 @@ static void gic_enable_nmi_support(void)
 	if (gic_read_ctlr() & ICC_CTLR_EL1_PMHE_MASK)
 		static_branch_enable(&gic_pmr_sync);
 
-	pr_info("%s ICC_PMR_EL1 synchronisation\n",
-		static_branch_unlikely(&gic_pmr_sync) ? "Forcing" : "Relaxing");
+	pr_info("Pseudo-NMIs enabled using %s ICC_PMR_EL1 synchronisation\n",
+		static_branch_unlikely(&gic_pmr_sync) ? "forced" : "relaxed");
 
 	static_branch_enable(&supports_pseudo_nmis);
 
-- 
2.27.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* [PATCH 2/2] irqchip/gic-v3: Support pseudo-NMIs when SCR_EL3.FIQ == 0
  2020-06-25 15:00 ` Alexandru Elisei
  (?)
@ 2020-06-25 15:00   ` Alexandru Elisei
  -1 siblings, 0 replies; 16+ messages in thread
From: Alexandru Elisei @ 2020-06-25 15:00 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel, kvmarm
  Cc: maz, tglx, jason, yuzenghui, julien.thierry.kdev, will,
	catalin.marinas, mark.rutland

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 <alexandru.elisei@arm.com>
---
 arch/arm64/include/asm/arch_gicv3.h |  8 ++++-
 arch/arm64/include/asm/daifflags.h  |  4 +--
 arch/arm64/include/asm/irqflags.h   | 14 +++++---
 arch/arm64/include/asm/ptrace.h     | 12 +++++++
 arch/arm64/kernel/entry.S           |  2 +-
 arch/arm64/kvm/hyp/switch.c         |  2 +-
 drivers/irqchip/irq-gic-v3.c        | 52 ++++++++++++++++++++++-------
 7 files changed, 73 insertions(+), 21 deletions(-)

diff --git a/arch/arm64/include/asm/arch_gicv3.h b/arch/arm64/include/asm/arch_gicv3.h
index a358e97572c1..c2a67a81e39d 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..dc68e11c63a1 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)
 {
+	u32 pmr_irqon = GIC_PRIO_IRQON;
+
 	if (system_has_prio_mask_debugging()) {
 		u32 pmr = read_sysreg_s(SYS_ICC_PMR_EL1);
+		u32 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)
 {
+	u32 pmr_irqoff = gic_prio_irqoff();
+
 	if (system_has_prio_mask_debugging()) {
 		u32 pmr = read_sysreg_s(SYS_ICC_PMR_EL1);
+		u32 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 953b6a1ce549..ad58f05544a1 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 5304d193c79d..73654234f454 100644
--- a/arch/arm64/kernel/entry.S
+++ b/arch/arm64/kernel/entry.S
@@ -643,7 +643,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/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
index db1c4487d95d..40d1041ab46d 100644
--- a/arch/arm64/kvm/hyp/switch.c
+++ b/arch/arm64/kvm/hyp/switch.c
@@ -870,7 +870,7 @@ int __hyp_text __kvm_vcpu_run_nvhe(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 83103277d2a9..012ff8819313 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.27.0


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

* [PATCH 2/2] irqchip/gic-v3: Support pseudo-NMIs when SCR_EL3.FIQ == 0
@ 2020-06-25 15:00   ` Alexandru Elisei
  0 siblings, 0 replies; 16+ messages in thread
From: Alexandru Elisei @ 2020-06-25 15:00 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel, kvmarm
  Cc: jason, maz, catalin.marinas, tglx, will

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 <alexandru.elisei@arm.com>
---
 arch/arm64/include/asm/arch_gicv3.h |  8 ++++-
 arch/arm64/include/asm/daifflags.h  |  4 +--
 arch/arm64/include/asm/irqflags.h   | 14 +++++---
 arch/arm64/include/asm/ptrace.h     | 12 +++++++
 arch/arm64/kernel/entry.S           |  2 +-
 arch/arm64/kvm/hyp/switch.c         |  2 +-
 drivers/irqchip/irq-gic-v3.c        | 52 ++++++++++++++++++++++-------
 7 files changed, 73 insertions(+), 21 deletions(-)

diff --git a/arch/arm64/include/asm/arch_gicv3.h b/arch/arm64/include/asm/arch_gicv3.h
index a358e97572c1..c2a67a81e39d 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..dc68e11c63a1 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)
 {
+	u32 pmr_irqon = GIC_PRIO_IRQON;
+
 	if (system_has_prio_mask_debugging()) {
 		u32 pmr = read_sysreg_s(SYS_ICC_PMR_EL1);
+		u32 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)
 {
+	u32 pmr_irqoff = gic_prio_irqoff();
+
 	if (system_has_prio_mask_debugging()) {
 		u32 pmr = read_sysreg_s(SYS_ICC_PMR_EL1);
+		u32 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 953b6a1ce549..ad58f05544a1 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 5304d193c79d..73654234f454 100644
--- a/arch/arm64/kernel/entry.S
+++ b/arch/arm64/kernel/entry.S
@@ -643,7 +643,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/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
index db1c4487d95d..40d1041ab46d 100644
--- a/arch/arm64/kvm/hyp/switch.c
+++ b/arch/arm64/kvm/hyp/switch.c
@@ -870,7 +870,7 @@ int __hyp_text __kvm_vcpu_run_nvhe(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 83103277d2a9..012ff8819313 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.27.0

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

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

* [PATCH 2/2] irqchip/gic-v3: Support pseudo-NMIs when SCR_EL3.FIQ == 0
@ 2020-06-25 15:00   ` Alexandru Elisei
  0 siblings, 0 replies; 16+ messages in thread
From: Alexandru Elisei @ 2020-06-25 15:00 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel, kvmarm
  Cc: mark.rutland, jason, maz, catalin.marinas, yuzenghui, tglx, will,
	julien.thierry.kdev

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 <alexandru.elisei@arm.com>
---
 arch/arm64/include/asm/arch_gicv3.h |  8 ++++-
 arch/arm64/include/asm/daifflags.h  |  4 +--
 arch/arm64/include/asm/irqflags.h   | 14 +++++---
 arch/arm64/include/asm/ptrace.h     | 12 +++++++
 arch/arm64/kernel/entry.S           |  2 +-
 arch/arm64/kvm/hyp/switch.c         |  2 +-
 drivers/irqchip/irq-gic-v3.c        | 52 ++++++++++++++++++++++-------
 7 files changed, 73 insertions(+), 21 deletions(-)

diff --git a/arch/arm64/include/asm/arch_gicv3.h b/arch/arm64/include/asm/arch_gicv3.h
index a358e97572c1..c2a67a81e39d 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..dc68e11c63a1 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)
 {
+	u32 pmr_irqon = GIC_PRIO_IRQON;
+
 	if (system_has_prio_mask_debugging()) {
 		u32 pmr = read_sysreg_s(SYS_ICC_PMR_EL1);
+		u32 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)
 {
+	u32 pmr_irqoff = gic_prio_irqoff();
+
 	if (system_has_prio_mask_debugging()) {
 		u32 pmr = read_sysreg_s(SYS_ICC_PMR_EL1);
+		u32 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 953b6a1ce549..ad58f05544a1 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 5304d193c79d..73654234f454 100644
--- a/arch/arm64/kernel/entry.S
+++ b/arch/arm64/kernel/entry.S
@@ -643,7 +643,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/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
index db1c4487d95d..40d1041ab46d 100644
--- a/arch/arm64/kvm/hyp/switch.c
+++ b/arch/arm64/kvm/hyp/switch.c
@@ -870,7 +870,7 @@ int __hyp_text __kvm_vcpu_run_nvhe(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 83103277d2a9..012ff8819313 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.27.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH 2/2] irqchip/gic-v3: Support pseudo-NMIs when SCR_EL3.FIQ == 0
  2020-06-25 15:00   ` Alexandru Elisei
  (?)
@ 2020-06-26  1:51     ` kernel test robot
  -1 siblings, 0 replies; 16+ messages in thread
From: kernel test robot @ 2020-06-26  1:51 UTC (permalink / raw)
  To: Alexandru Elisei, linux-arm-kernel, linux-kernel, kvmarm
  Cc: kbuild-all, clang-built-linux, maz, tglx, jason, yuzenghui,
	julien.thierry.kdev, will, catalin.marinas

[-- Attachment #1: Type: text/plain, Size: 13188 bytes --]

Hi Alexandru,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on arm64/for-next/core]
[also build test WARNING on tip/irq/core v5.8-rc2 next-20200625]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use  as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Alexandru-Elisei/irqchip-gic-v3-Support-pseudo-NMIs-when-SCR_EL3-FIQ-0/20200625-230144
base:   https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git for-next/core
config: arm64-randconfig-r025-20200624 (attached as .config)
compiler: clang version 11.0.0 (https://github.com/llvm/llvm-project 8911a35180c6777188fefe0954a2451a2b91deaf)
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
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=arm64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   In file included from arch/arm64/kernel/asm-offsets.c:10:
   In file included from include/linux/arm_sdei.h:8:
   In file included from include/acpi/ghes.h:5:
   In file included from include/acpi/apei.h:9:
   In file included from include/linux/acpi.h:13:
   In file included from include/linux/irqdomain.h:35:
   In file included from include/linux/of.h:17:
   In file included from include/linux/kobject.h:20:
   In file included from include/linux/sysfs.h:16:
   In file included from include/linux/kernfs.h:13:
   In file included from include/linux/idr.h:15:
   In file included from include/linux/radix-tree.h:15:
   In file included from include/linux/rcupdate.h:26:
   In file included from include/linux/irqflags.h:16:
>> arch/arm64/include/asm/irqflags.h:45:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
                   : "r" (pmr_irqon)
                          ^
   arch/arm64/include/asm/irqflags.h:42:29: note: use constraint modifier "w"
                   __msr_s(SYS_ICC_PMR_EL1, "%0"),
                                             ^
   arch/arm64/include/asm/irqflags.h:67:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
                   : "r" (pmr_irqoff)
                          ^
   arch/arm64/include/asm/irqflags.h:64:29: note: use constraint modifier "w"
                   __msr_s(SYS_ICC_PMR_EL1, "%0"),
                                             ^
   2 warnings generated.
--
   In file included from drivers/power/supply/ltc2941-battery-gauge.c:12:
   In file included from include/linux/module.h:13:
   In file included from include/linux/stat.h:6:
   In file included from arch/arm64/include/asm/stat.h:12:
   In file included from include/linux/time.h:6:
   In file included from include/linux/seqlock.h:36:
   In file included from include/linux/spinlock.h:54:
   In file included from include/linux/irqflags.h:16:
>> arch/arm64/include/asm/irqflags.h:45:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
                   : "r" (pmr_irqon)
                          ^
   arch/arm64/include/asm/irqflags.h:42:29: note: use constraint modifier "w"
                   __msr_s(SYS_ICC_PMR_EL1, "%0"),
                                             ^
   arch/arm64/include/asm/irqflags.h:67:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
                   : "r" (pmr_irqoff)
                          ^
   arch/arm64/include/asm/irqflags.h:64:29: note: use constraint modifier "w"
                   __msr_s(SYS_ICC_PMR_EL1, "%0"),
                                             ^
   drivers/power/supply/ltc2941-battery-gauge.c:476:13: warning: cast to smaller integer type 'enum ltc294x_id' from 'const void *' [-Wvoid-pointer-to-enum-cast]
           info->id = (enum ltc294x_id)of_device_get_match_data(&client->dev);
                      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   3 warnings generated.
--
   In file included from drivers/power/supply/goldfish_battery.c:11:
   In file included from include/linux/module.h:13:
   In file included from include/linux/stat.h:6:
   In file included from arch/arm64/include/asm/stat.h:12:
   In file included from include/linux/time.h:6:
   In file included from include/linux/seqlock.h:36:
   In file included from include/linux/spinlock.h:54:
   In file included from include/linux/irqflags.h:16:
>> arch/arm64/include/asm/irqflags.h:45:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
                   : "r" (pmr_irqon)
                          ^
   arch/arm64/include/asm/irqflags.h:42:29: note: use constraint modifier "w"
                   __msr_s(SYS_ICC_PMR_EL1, "%0"),
                                             ^
   arch/arm64/include/asm/irqflags.h:67:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
                   : "r" (pmr_irqoff)
                          ^
   arch/arm64/include/asm/irqflags.h:64:29: note: use constraint modifier "w"
                   __msr_s(SYS_ICC_PMR_EL1, "%0"),
                                             ^
   drivers/power/supply/goldfish_battery.c:269:36: warning: unused variable 'goldfish_battery_acpi_match' [-Wunused-const-variable]
   static const struct acpi_device_id goldfish_battery_acpi_match[] = {
                                      ^
   3 warnings generated.
--
   In file included from drivers/power/supply/bq25890_charger.c:8:
   In file included from include/linux/module.h:13:
   In file included from include/linux/stat.h:6:
   In file included from arch/arm64/include/asm/stat.h:12:
   In file included from include/linux/time.h:6:
   In file included from include/linux/seqlock.h:36:
   In file included from include/linux/spinlock.h:54:
   In file included from include/linux/irqflags.h:16:
>> arch/arm64/include/asm/irqflags.h:45:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
                   : "r" (pmr_irqon)
                          ^
   arch/arm64/include/asm/irqflags.h:42:29: note: use constraint modifier "w"
                   __msr_s(SYS_ICC_PMR_EL1, "%0"),
                                             ^
   arch/arm64/include/asm/irqflags.h:67:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
                   : "r" (pmr_irqoff)
                          ^
   arch/arm64/include/asm/irqflags.h:64:29: note: use constraint modifier "w"
                   __msr_s(SYS_ICC_PMR_EL1, "%0"),
                                             ^
   drivers/power/supply/bq25890_charger.c:1060:36: warning: unused variable 'bq25890_acpi_match' [-Wunused-const-variable]
   static const struct acpi_device_id bq25890_acpi_match[] = {
                                      ^
   3 warnings generated.
--
   In file included from drivers/power/reset/vexpress-poweroff.c:8:
   In file included from include/linux/notifier.h:15:
   In file included from include/linux/rwsem.h:16:
   In file included from include/linux/spinlock.h:54:
   In file included from include/linux/irqflags.h:16:
>> arch/arm64/include/asm/irqflags.h:45:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
                   : "r" (pmr_irqon)
                          ^
   arch/arm64/include/asm/irqflags.h:42:29: note: use constraint modifier "w"
                   __msr_s(SYS_ICC_PMR_EL1, "%0"),
                                             ^
   arch/arm64/include/asm/irqflags.h:67:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
                   : "r" (pmr_irqoff)
                          ^
   arch/arm64/include/asm/irqflags.h:64:29: note: use constraint modifier "w"
                   __msr_s(SYS_ICC_PMR_EL1, "%0"),
                                             ^
   drivers/power/reset/vexpress-poweroff.c:124:10: warning: cast to smaller integer type 'enum vexpress_reset_func' from 'const void *' [-Wvoid-pointer-to-enum-cast]
           switch ((enum vexpress_reset_func)match->data) {
                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   3 warnings generated.
--
   In file included from arch/arm64/kernel/asm-offsets.c:10:
   In file included from include/linux/arm_sdei.h:8:
   In file included from include/acpi/ghes.h:5:
   In file included from include/acpi/apei.h:9:
   In file included from include/linux/acpi.h:13:
   In file included from include/linux/irqdomain.h:35:
   In file included from include/linux/of.h:17:
   In file included from include/linux/kobject.h:20:
   In file included from include/linux/sysfs.h:16:
   In file included from include/linux/kernfs.h:13:
   In file included from include/linux/idr.h:15:
   In file included from include/linux/radix-tree.h:15:
   In file included from include/linux/rcupdate.h:26:
   In file included from include/linux/irqflags.h:16:
>> arch/arm64/include/asm/irqflags.h:45:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
                   : "r" (pmr_irqon)
                          ^
   arch/arm64/include/asm/irqflags.h:42:29: note: use constraint modifier "w"
                   __msr_s(SYS_ICC_PMR_EL1, "%0"),
                                             ^
   arch/arm64/include/asm/irqflags.h:67:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
                   : "r" (pmr_irqoff)
                          ^
   arch/arm64/include/asm/irqflags.h:64:29: note: use constraint modifier "w"
                   __msr_s(SYS_ICC_PMR_EL1, "%0"),
                                             ^
   2 warnings generated.
   arch/arm64/kernel/vdso/vgettimeofday.c:9:5: warning: no previous prototype for function '__kernel_clock_gettime' [-Wmissing-prototypes]
   int __kernel_clock_gettime(clockid_t clock,
       ^
   arch/arm64/kernel/vdso/vgettimeofday.c:9:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   int __kernel_clock_gettime(clockid_t clock,
   ^
   static 
   arch/arm64/kernel/vdso/vgettimeofday.c:15:5: warning: no previous prototype for function '__kernel_gettimeofday' [-Wmissing-prototypes]
   int __kernel_gettimeofday(struct __kernel_old_timeval *tv,
       ^
   arch/arm64/kernel/vdso/vgettimeofday.c:15:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   int __kernel_gettimeofday(struct __kernel_old_timeval *tv,
   ^
   static 
   arch/arm64/kernel/vdso/vgettimeofday.c:21:5: warning: no previous prototype for function '__kernel_clock_getres' [-Wmissing-prototypes]
   int __kernel_clock_getres(clockid_t clock_id,
       ^
   arch/arm64/kernel/vdso/vgettimeofday.c:21:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   int __kernel_clock_getres(clockid_t clock_id,
   ^
   static 
   3 warnings generated.

vim +45 arch/arm64/include/asm/irqflags.h

    12	
    13	/*
    14	 * Aarch64 has flags for masking: Debug, Asynchronous (serror), Interrupts and
    15	 * FIQ exceptions, in the 'daif' register. We mask and unmask them in 'dai'
    16	 * order:
    17	 * Masking debug exceptions causes all other exceptions to be masked too/
    18	 * Masking SError masks irq, but not debug exceptions. Masking irqs has no
    19	 * side effects for other flags. Keeping to this order makes it easier for
    20	 * entry.S to know which exceptions should be unmasked.
    21	 *
    22	 * FIQ is never expected, but we mask it when we disable debug exceptions, and
    23	 * unmask it at all other times.
    24	 */
    25	
    26	/*
    27	 * CPU interrupt mask handling.
    28	 */
    29	static inline void arch_local_irq_enable(void)
    30	{
    31		u32 pmr_irqon = GIC_PRIO_IRQON;
    32	
    33		if (system_has_prio_mask_debugging()) {
    34			u32 pmr = read_sysreg_s(SYS_ICC_PMR_EL1);
    35			u32 pmr_irqoff = gic_prio_irqoff();
    36	
    37			WARN_ON_ONCE(pmr != pmr_irqon && pmr != pmr_irqoff);
    38		}
    39	
    40		asm volatile(ALTERNATIVE(
    41			"msr	daifclr, #2		// arch_local_irq_enable",
    42			__msr_s(SYS_ICC_PMR_EL1, "%0"),
    43			ARM64_HAS_IRQ_PRIO_MASKING)
    44			:
  > 45			: "r" (pmr_irqon)
    46			: "memory");
    47	
    48		pmr_sync();
    49	}
    50	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 31564 bytes --]

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

* Re: [PATCH 2/2] irqchip/gic-v3: Support pseudo-NMIs when SCR_EL3.FIQ == 0
@ 2020-06-26  1:51     ` kernel test robot
  0 siblings, 0 replies; 16+ messages in thread
From: kernel test robot @ 2020-06-26  1:51 UTC (permalink / raw)
  To: Alexandru Elisei, linux-arm-kernel, linux-kernel, kvmarm
  Cc: kbuild-all, jason, maz, clang-built-linux, catalin.marinas, tglx, will

[-- Attachment #1: Type: text/plain, Size: 13188 bytes --]

Hi Alexandru,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on arm64/for-next/core]
[also build test WARNING on tip/irq/core v5.8-rc2 next-20200625]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use  as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Alexandru-Elisei/irqchip-gic-v3-Support-pseudo-NMIs-when-SCR_EL3-FIQ-0/20200625-230144
base:   https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git for-next/core
config: arm64-randconfig-r025-20200624 (attached as .config)
compiler: clang version 11.0.0 (https://github.com/llvm/llvm-project 8911a35180c6777188fefe0954a2451a2b91deaf)
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
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=arm64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   In file included from arch/arm64/kernel/asm-offsets.c:10:
   In file included from include/linux/arm_sdei.h:8:
   In file included from include/acpi/ghes.h:5:
   In file included from include/acpi/apei.h:9:
   In file included from include/linux/acpi.h:13:
   In file included from include/linux/irqdomain.h:35:
   In file included from include/linux/of.h:17:
   In file included from include/linux/kobject.h:20:
   In file included from include/linux/sysfs.h:16:
   In file included from include/linux/kernfs.h:13:
   In file included from include/linux/idr.h:15:
   In file included from include/linux/radix-tree.h:15:
   In file included from include/linux/rcupdate.h:26:
   In file included from include/linux/irqflags.h:16:
>> arch/arm64/include/asm/irqflags.h:45:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
                   : "r" (pmr_irqon)
                          ^
   arch/arm64/include/asm/irqflags.h:42:29: note: use constraint modifier "w"
                   __msr_s(SYS_ICC_PMR_EL1, "%0"),
                                             ^
   arch/arm64/include/asm/irqflags.h:67:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
                   : "r" (pmr_irqoff)
                          ^
   arch/arm64/include/asm/irqflags.h:64:29: note: use constraint modifier "w"
                   __msr_s(SYS_ICC_PMR_EL1, "%0"),
                                             ^
   2 warnings generated.
--
   In file included from drivers/power/supply/ltc2941-battery-gauge.c:12:
   In file included from include/linux/module.h:13:
   In file included from include/linux/stat.h:6:
   In file included from arch/arm64/include/asm/stat.h:12:
   In file included from include/linux/time.h:6:
   In file included from include/linux/seqlock.h:36:
   In file included from include/linux/spinlock.h:54:
   In file included from include/linux/irqflags.h:16:
>> arch/arm64/include/asm/irqflags.h:45:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
                   : "r" (pmr_irqon)
                          ^
   arch/arm64/include/asm/irqflags.h:42:29: note: use constraint modifier "w"
                   __msr_s(SYS_ICC_PMR_EL1, "%0"),
                                             ^
   arch/arm64/include/asm/irqflags.h:67:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
                   : "r" (pmr_irqoff)
                          ^
   arch/arm64/include/asm/irqflags.h:64:29: note: use constraint modifier "w"
                   __msr_s(SYS_ICC_PMR_EL1, "%0"),
                                             ^
   drivers/power/supply/ltc2941-battery-gauge.c:476:13: warning: cast to smaller integer type 'enum ltc294x_id' from 'const void *' [-Wvoid-pointer-to-enum-cast]
           info->id = (enum ltc294x_id)of_device_get_match_data(&client->dev);
                      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   3 warnings generated.
--
   In file included from drivers/power/supply/goldfish_battery.c:11:
   In file included from include/linux/module.h:13:
   In file included from include/linux/stat.h:6:
   In file included from arch/arm64/include/asm/stat.h:12:
   In file included from include/linux/time.h:6:
   In file included from include/linux/seqlock.h:36:
   In file included from include/linux/spinlock.h:54:
   In file included from include/linux/irqflags.h:16:
>> arch/arm64/include/asm/irqflags.h:45:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
                   : "r" (pmr_irqon)
                          ^
   arch/arm64/include/asm/irqflags.h:42:29: note: use constraint modifier "w"
                   __msr_s(SYS_ICC_PMR_EL1, "%0"),
                                             ^
   arch/arm64/include/asm/irqflags.h:67:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
                   : "r" (pmr_irqoff)
                          ^
   arch/arm64/include/asm/irqflags.h:64:29: note: use constraint modifier "w"
                   __msr_s(SYS_ICC_PMR_EL1, "%0"),
                                             ^
   drivers/power/supply/goldfish_battery.c:269:36: warning: unused variable 'goldfish_battery_acpi_match' [-Wunused-const-variable]
   static const struct acpi_device_id goldfish_battery_acpi_match[] = {
                                      ^
   3 warnings generated.
--
   In file included from drivers/power/supply/bq25890_charger.c:8:
   In file included from include/linux/module.h:13:
   In file included from include/linux/stat.h:6:
   In file included from arch/arm64/include/asm/stat.h:12:
   In file included from include/linux/time.h:6:
   In file included from include/linux/seqlock.h:36:
   In file included from include/linux/spinlock.h:54:
   In file included from include/linux/irqflags.h:16:
>> arch/arm64/include/asm/irqflags.h:45:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
                   : "r" (pmr_irqon)
                          ^
   arch/arm64/include/asm/irqflags.h:42:29: note: use constraint modifier "w"
                   __msr_s(SYS_ICC_PMR_EL1, "%0"),
                                             ^
   arch/arm64/include/asm/irqflags.h:67:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
                   : "r" (pmr_irqoff)
                          ^
   arch/arm64/include/asm/irqflags.h:64:29: note: use constraint modifier "w"
                   __msr_s(SYS_ICC_PMR_EL1, "%0"),
                                             ^
   drivers/power/supply/bq25890_charger.c:1060:36: warning: unused variable 'bq25890_acpi_match' [-Wunused-const-variable]
   static const struct acpi_device_id bq25890_acpi_match[] = {
                                      ^
   3 warnings generated.
--
   In file included from drivers/power/reset/vexpress-poweroff.c:8:
   In file included from include/linux/notifier.h:15:
   In file included from include/linux/rwsem.h:16:
   In file included from include/linux/spinlock.h:54:
   In file included from include/linux/irqflags.h:16:
>> arch/arm64/include/asm/irqflags.h:45:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
                   : "r" (pmr_irqon)
                          ^
   arch/arm64/include/asm/irqflags.h:42:29: note: use constraint modifier "w"
                   __msr_s(SYS_ICC_PMR_EL1, "%0"),
                                             ^
   arch/arm64/include/asm/irqflags.h:67:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
                   : "r" (pmr_irqoff)
                          ^
   arch/arm64/include/asm/irqflags.h:64:29: note: use constraint modifier "w"
                   __msr_s(SYS_ICC_PMR_EL1, "%0"),
                                             ^
   drivers/power/reset/vexpress-poweroff.c:124:10: warning: cast to smaller integer type 'enum vexpress_reset_func' from 'const void *' [-Wvoid-pointer-to-enum-cast]
           switch ((enum vexpress_reset_func)match->data) {
                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   3 warnings generated.
--
   In file included from arch/arm64/kernel/asm-offsets.c:10:
   In file included from include/linux/arm_sdei.h:8:
   In file included from include/acpi/ghes.h:5:
   In file included from include/acpi/apei.h:9:
   In file included from include/linux/acpi.h:13:
   In file included from include/linux/irqdomain.h:35:
   In file included from include/linux/of.h:17:
   In file included from include/linux/kobject.h:20:
   In file included from include/linux/sysfs.h:16:
   In file included from include/linux/kernfs.h:13:
   In file included from include/linux/idr.h:15:
   In file included from include/linux/radix-tree.h:15:
   In file included from include/linux/rcupdate.h:26:
   In file included from include/linux/irqflags.h:16:
>> arch/arm64/include/asm/irqflags.h:45:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
                   : "r" (pmr_irqon)
                          ^
   arch/arm64/include/asm/irqflags.h:42:29: note: use constraint modifier "w"
                   __msr_s(SYS_ICC_PMR_EL1, "%0"),
                                             ^
   arch/arm64/include/asm/irqflags.h:67:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
                   : "r" (pmr_irqoff)
                          ^
   arch/arm64/include/asm/irqflags.h:64:29: note: use constraint modifier "w"
                   __msr_s(SYS_ICC_PMR_EL1, "%0"),
                                             ^
   2 warnings generated.
   arch/arm64/kernel/vdso/vgettimeofday.c:9:5: warning: no previous prototype for function '__kernel_clock_gettime' [-Wmissing-prototypes]
   int __kernel_clock_gettime(clockid_t clock,
       ^
   arch/arm64/kernel/vdso/vgettimeofday.c:9:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   int __kernel_clock_gettime(clockid_t clock,
   ^
   static 
   arch/arm64/kernel/vdso/vgettimeofday.c:15:5: warning: no previous prototype for function '__kernel_gettimeofday' [-Wmissing-prototypes]
   int __kernel_gettimeofday(struct __kernel_old_timeval *tv,
       ^
   arch/arm64/kernel/vdso/vgettimeofday.c:15:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   int __kernel_gettimeofday(struct __kernel_old_timeval *tv,
   ^
   static 
   arch/arm64/kernel/vdso/vgettimeofday.c:21:5: warning: no previous prototype for function '__kernel_clock_getres' [-Wmissing-prototypes]
   int __kernel_clock_getres(clockid_t clock_id,
       ^
   arch/arm64/kernel/vdso/vgettimeofday.c:21:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   int __kernel_clock_getres(clockid_t clock_id,
   ^
   static 
   3 warnings generated.

vim +45 arch/arm64/include/asm/irqflags.h

    12	
    13	/*
    14	 * Aarch64 has flags for masking: Debug, Asynchronous (serror), Interrupts and
    15	 * FIQ exceptions, in the 'daif' register. We mask and unmask them in 'dai'
    16	 * order:
    17	 * Masking debug exceptions causes all other exceptions to be masked too/
    18	 * Masking SError masks irq, but not debug exceptions. Masking irqs has no
    19	 * side effects for other flags. Keeping to this order makes it easier for
    20	 * entry.S to know which exceptions should be unmasked.
    21	 *
    22	 * FIQ is never expected, but we mask it when we disable debug exceptions, and
    23	 * unmask it at all other times.
    24	 */
    25	
    26	/*
    27	 * CPU interrupt mask handling.
    28	 */
    29	static inline void arch_local_irq_enable(void)
    30	{
    31		u32 pmr_irqon = GIC_PRIO_IRQON;
    32	
    33		if (system_has_prio_mask_debugging()) {
    34			u32 pmr = read_sysreg_s(SYS_ICC_PMR_EL1);
    35			u32 pmr_irqoff = gic_prio_irqoff();
    36	
    37			WARN_ON_ONCE(pmr != pmr_irqon && pmr != pmr_irqoff);
    38		}
    39	
    40		asm volatile(ALTERNATIVE(
    41			"msr	daifclr, #2		// arch_local_irq_enable",
    42			__msr_s(SYS_ICC_PMR_EL1, "%0"),
    43			ARM64_HAS_IRQ_PRIO_MASKING)
    44			:
  > 45			: "r" (pmr_irqon)
    46			: "memory");
    47	
    48		pmr_sync();
    49	}
    50	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 31564 bytes --]

[-- Attachment #3: Type: text/plain, Size: 151 bytes --]

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

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

* Re: [PATCH 2/2] irqchip/gic-v3: Support pseudo-NMIs when SCR_EL3.FIQ == 0
@ 2020-06-26  1:51     ` kernel test robot
  0 siblings, 0 replies; 16+ messages in thread
From: kernel test robot @ 2020-06-26  1:51 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 13437 bytes --]

Hi Alexandru,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on arm64/for-next/core]
[also build test WARNING on tip/irq/core v5.8-rc2 next-20200625]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use  as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Alexandru-Elisei/irqchip-gic-v3-Support-pseudo-NMIs-when-SCR_EL3-FIQ-0/20200625-230144
base:   https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git for-next/core
config: arm64-randconfig-r025-20200624 (attached as .config)
compiler: clang version 11.0.0 (https://github.com/llvm/llvm-project 8911a35180c6777188fefe0954a2451a2b91deaf)
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
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=arm64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   In file included from arch/arm64/kernel/asm-offsets.c:10:
   In file included from include/linux/arm_sdei.h:8:
   In file included from include/acpi/ghes.h:5:
   In file included from include/acpi/apei.h:9:
   In file included from include/linux/acpi.h:13:
   In file included from include/linux/irqdomain.h:35:
   In file included from include/linux/of.h:17:
   In file included from include/linux/kobject.h:20:
   In file included from include/linux/sysfs.h:16:
   In file included from include/linux/kernfs.h:13:
   In file included from include/linux/idr.h:15:
   In file included from include/linux/radix-tree.h:15:
   In file included from include/linux/rcupdate.h:26:
   In file included from include/linux/irqflags.h:16:
>> arch/arm64/include/asm/irqflags.h:45:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
                   : "r" (pmr_irqon)
                          ^
   arch/arm64/include/asm/irqflags.h:42:29: note: use constraint modifier "w"
                   __msr_s(SYS_ICC_PMR_EL1, "%0"),
                                             ^
   arch/arm64/include/asm/irqflags.h:67:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
                   : "r" (pmr_irqoff)
                          ^
   arch/arm64/include/asm/irqflags.h:64:29: note: use constraint modifier "w"
                   __msr_s(SYS_ICC_PMR_EL1, "%0"),
                                             ^
   2 warnings generated.
--
   In file included from drivers/power/supply/ltc2941-battery-gauge.c:12:
   In file included from include/linux/module.h:13:
   In file included from include/linux/stat.h:6:
   In file included from arch/arm64/include/asm/stat.h:12:
   In file included from include/linux/time.h:6:
   In file included from include/linux/seqlock.h:36:
   In file included from include/linux/spinlock.h:54:
   In file included from include/linux/irqflags.h:16:
>> arch/arm64/include/asm/irqflags.h:45:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
                   : "r" (pmr_irqon)
                          ^
   arch/arm64/include/asm/irqflags.h:42:29: note: use constraint modifier "w"
                   __msr_s(SYS_ICC_PMR_EL1, "%0"),
                                             ^
   arch/arm64/include/asm/irqflags.h:67:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
                   : "r" (pmr_irqoff)
                          ^
   arch/arm64/include/asm/irqflags.h:64:29: note: use constraint modifier "w"
                   __msr_s(SYS_ICC_PMR_EL1, "%0"),
                                             ^
   drivers/power/supply/ltc2941-battery-gauge.c:476:13: warning: cast to smaller integer type 'enum ltc294x_id' from 'const void *' [-Wvoid-pointer-to-enum-cast]
           info->id = (enum ltc294x_id)of_device_get_match_data(&client->dev);
                      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   3 warnings generated.
--
   In file included from drivers/power/supply/goldfish_battery.c:11:
   In file included from include/linux/module.h:13:
   In file included from include/linux/stat.h:6:
   In file included from arch/arm64/include/asm/stat.h:12:
   In file included from include/linux/time.h:6:
   In file included from include/linux/seqlock.h:36:
   In file included from include/linux/spinlock.h:54:
   In file included from include/linux/irqflags.h:16:
>> arch/arm64/include/asm/irqflags.h:45:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
                   : "r" (pmr_irqon)
                          ^
   arch/arm64/include/asm/irqflags.h:42:29: note: use constraint modifier "w"
                   __msr_s(SYS_ICC_PMR_EL1, "%0"),
                                             ^
   arch/arm64/include/asm/irqflags.h:67:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
                   : "r" (pmr_irqoff)
                          ^
   arch/arm64/include/asm/irqflags.h:64:29: note: use constraint modifier "w"
                   __msr_s(SYS_ICC_PMR_EL1, "%0"),
                                             ^
   drivers/power/supply/goldfish_battery.c:269:36: warning: unused variable 'goldfish_battery_acpi_match' [-Wunused-const-variable]
   static const struct acpi_device_id goldfish_battery_acpi_match[] = {
                                      ^
   3 warnings generated.
--
   In file included from drivers/power/supply/bq25890_charger.c:8:
   In file included from include/linux/module.h:13:
   In file included from include/linux/stat.h:6:
   In file included from arch/arm64/include/asm/stat.h:12:
   In file included from include/linux/time.h:6:
   In file included from include/linux/seqlock.h:36:
   In file included from include/linux/spinlock.h:54:
   In file included from include/linux/irqflags.h:16:
>> arch/arm64/include/asm/irqflags.h:45:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
                   : "r" (pmr_irqon)
                          ^
   arch/arm64/include/asm/irqflags.h:42:29: note: use constraint modifier "w"
                   __msr_s(SYS_ICC_PMR_EL1, "%0"),
                                             ^
   arch/arm64/include/asm/irqflags.h:67:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
                   : "r" (pmr_irqoff)
                          ^
   arch/arm64/include/asm/irqflags.h:64:29: note: use constraint modifier "w"
                   __msr_s(SYS_ICC_PMR_EL1, "%0"),
                                             ^
   drivers/power/supply/bq25890_charger.c:1060:36: warning: unused variable 'bq25890_acpi_match' [-Wunused-const-variable]
   static const struct acpi_device_id bq25890_acpi_match[] = {
                                      ^
   3 warnings generated.
--
   In file included from drivers/power/reset/vexpress-poweroff.c:8:
   In file included from include/linux/notifier.h:15:
   In file included from include/linux/rwsem.h:16:
   In file included from include/linux/spinlock.h:54:
   In file included from include/linux/irqflags.h:16:
>> arch/arm64/include/asm/irqflags.h:45:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
                   : "r" (pmr_irqon)
                          ^
   arch/arm64/include/asm/irqflags.h:42:29: note: use constraint modifier "w"
                   __msr_s(SYS_ICC_PMR_EL1, "%0"),
                                             ^
   arch/arm64/include/asm/irqflags.h:67:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
                   : "r" (pmr_irqoff)
                          ^
   arch/arm64/include/asm/irqflags.h:64:29: note: use constraint modifier "w"
                   __msr_s(SYS_ICC_PMR_EL1, "%0"),
                                             ^
   drivers/power/reset/vexpress-poweroff.c:124:10: warning: cast to smaller integer type 'enum vexpress_reset_func' from 'const void *' [-Wvoid-pointer-to-enum-cast]
           switch ((enum vexpress_reset_func)match->data) {
                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   3 warnings generated.
--
   In file included from arch/arm64/kernel/asm-offsets.c:10:
   In file included from include/linux/arm_sdei.h:8:
   In file included from include/acpi/ghes.h:5:
   In file included from include/acpi/apei.h:9:
   In file included from include/linux/acpi.h:13:
   In file included from include/linux/irqdomain.h:35:
   In file included from include/linux/of.h:17:
   In file included from include/linux/kobject.h:20:
   In file included from include/linux/sysfs.h:16:
   In file included from include/linux/kernfs.h:13:
   In file included from include/linux/idr.h:15:
   In file included from include/linux/radix-tree.h:15:
   In file included from include/linux/rcupdate.h:26:
   In file included from include/linux/irqflags.h:16:
>> arch/arm64/include/asm/irqflags.h:45:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
                   : "r" (pmr_irqon)
                          ^
   arch/arm64/include/asm/irqflags.h:42:29: note: use constraint modifier "w"
                   __msr_s(SYS_ICC_PMR_EL1, "%0"),
                                             ^
   arch/arm64/include/asm/irqflags.h:67:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
                   : "r" (pmr_irqoff)
                          ^
   arch/arm64/include/asm/irqflags.h:64:29: note: use constraint modifier "w"
                   __msr_s(SYS_ICC_PMR_EL1, "%0"),
                                             ^
   2 warnings generated.
   arch/arm64/kernel/vdso/vgettimeofday.c:9:5: warning: no previous prototype for function '__kernel_clock_gettime' [-Wmissing-prototypes]
   int __kernel_clock_gettime(clockid_t clock,
       ^
   arch/arm64/kernel/vdso/vgettimeofday.c:9:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   int __kernel_clock_gettime(clockid_t clock,
   ^
   static 
   arch/arm64/kernel/vdso/vgettimeofday.c:15:5: warning: no previous prototype for function '__kernel_gettimeofday' [-Wmissing-prototypes]
   int __kernel_gettimeofday(struct __kernel_old_timeval *tv,
       ^
   arch/arm64/kernel/vdso/vgettimeofday.c:15:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   int __kernel_gettimeofday(struct __kernel_old_timeval *tv,
   ^
   static 
   arch/arm64/kernel/vdso/vgettimeofday.c:21:5: warning: no previous prototype for function '__kernel_clock_getres' [-Wmissing-prototypes]
   int __kernel_clock_getres(clockid_t clock_id,
       ^
   arch/arm64/kernel/vdso/vgettimeofday.c:21:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   int __kernel_clock_getres(clockid_t clock_id,
   ^
   static 
   3 warnings generated.

vim +45 arch/arm64/include/asm/irqflags.h

    12	
    13	/*
    14	 * Aarch64 has flags for masking: Debug, Asynchronous (serror), Interrupts and
    15	 * FIQ exceptions, in the 'daif' register. We mask and unmask them in 'dai'
    16	 * order:
    17	 * Masking debug exceptions causes all other exceptions to be masked too/
    18	 * Masking SError masks irq, but not debug exceptions. Masking irqs has no
    19	 * side effects for other flags. Keeping to this order makes it easier for
    20	 * entry.S to know which exceptions should be unmasked.
    21	 *
    22	 * FIQ is never expected, but we mask it when we disable debug exceptions, and
    23	 * unmask it at all other times.
    24	 */
    25	
    26	/*
    27	 * CPU interrupt mask handling.
    28	 */
    29	static inline void arch_local_irq_enable(void)
    30	{
    31		u32 pmr_irqon = GIC_PRIO_IRQON;
    32	
    33		if (system_has_prio_mask_debugging()) {
    34			u32 pmr = read_sysreg_s(SYS_ICC_PMR_EL1);
    35			u32 pmr_irqoff = gic_prio_irqoff();
    36	
    37			WARN_ON_ONCE(pmr != pmr_irqon && pmr != pmr_irqoff);
    38		}
    39	
    40		asm volatile(ALTERNATIVE(
    41			"msr	daifclr, #2		// arch_local_irq_enable",
    42			__msr_s(SYS_ICC_PMR_EL1, "%0"),
    43			ARM64_HAS_IRQ_PRIO_MASKING)
    44			:
  > 45			: "r" (pmr_irqon)
    46			: "memory");
    47	
    48		pmr_sync();
    49	}
    50	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 31564 bytes --]

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

* Re: [PATCH 2/2] irqchip/gic-v3: Support pseudo-NMIs when SCR_EL3.FIQ == 0
  2020-06-26  1:51     ` kernel test robot
  (?)
  (?)
@ 2020-06-26  9:41       ` Alexandru Elisei
  -1 siblings, 0 replies; 16+ messages in thread
From: Alexandru Elisei @ 2020-06-26  9:41 UTC (permalink / raw)
  To: kernel test robot, linux-arm-kernel, linux-kernel, kvmarm
  Cc: kbuild-all, clang-built-linux, maz, tglx, jason, yuzenghui,
	julien.thierry.kdev, will, catalin.marinas

Hi,

On 6/26/20 2:51 AM, kernel test robot wrote:
> Hi Alexandru,
>
> Thank you for the patch! Perhaps something to improve:
>
> [auto build test WARNING on arm64/for-next/core]
> [also build test WARNING on tip/irq/core v5.8-rc2 next-20200625]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use  as documented in
> https://git-scm.com/docs/git-format-patch]
>
> url:    https://github.com/0day-ci/linux/commits/Alexandru-Elisei/irqchip-gic-v3-Support-pseudo-NMIs-when-SCR_EL3-FIQ-0/20200625-230144
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git for-next/core
> config: arm64-randconfig-r025-20200624 (attached as .config)
> compiler: clang version 11.0.0 (https://github.com/llvm/llvm-project 8911a35180c6777188fefe0954a2451a2b91deaf)
> 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
>         # save the attached .config to linux build tree
>         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=arm64 

My mistake, I'll start compiling the kernel with clang too.

The register width for ICC_PMR_EL1 in the kernel is rather inconsistent: in
arch_gicv3.h, the accessors use 32 bits for the PMR value which gets casted to 64
bit by the {read,write}_sysreg_s macros anyway, in struct pt_regs the register is
64-bit, in __cpu_do_idle_irqprio it's declared as an unsigned long,
arch_local_irqs_{disable,enable} declares it as u32 and casts it to an unsigned
long when used by the inline assembly, the gicv3 irqchip driver uses it as a 32
bit variable.

I think the confusion stems from the fact that originally it was a 32 bit
register, but was changed to 64 bits in Arm IHI 0069E (January 2019).

I could cast it to an unsigned long in the inline assembly, but IMO that looks a
bit awkward. Before sending the patches I was considering changing it everywhere
to 64 bits, but Mark Rutland had a different idea. Mark, would you mind explaining
why keeping it 32 bit wide makes more sense?

Thanks,
Alex
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
>
> All warnings (new ones prefixed by >>):
>
>    In file included from arch/arm64/kernel/asm-offsets.c:10:
>    In file included from include/linux/arm_sdei.h:8:
>    In file included from include/acpi/ghes.h:5:
>    In file included from include/acpi/apei.h:9:
>    In file included from include/linux/acpi.h:13:
>    In file included from include/linux/irqdomain.h:35:
>    In file included from include/linux/of.h:17:
>    In file included from include/linux/kobject.h:20:
>    In file included from include/linux/sysfs.h:16:
>    In file included from include/linux/kernfs.h:13:
>    In file included from include/linux/idr.h:15:
>    In file included from include/linux/radix-tree.h:15:
>    In file included from include/linux/rcupdate.h:26:
>    In file included from include/linux/irqflags.h:16:
>>> arch/arm64/include/asm/irqflags.h:45:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
>                    : "r" (pmr_irqon)
>                           ^
>    arch/arm64/include/asm/irqflags.h:42:29: note: use constraint modifier "w"
>                    __msr_s(SYS_ICC_PMR_EL1, "%0"),
>                                              ^
>    arch/arm64/include/asm/irqflags.h:67:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
>                    : "r" (pmr_irqoff)
>                           ^
>    arch/arm64/include/asm/irqflags.h:64:29: note: use constraint modifier "w"
>                    __msr_s(SYS_ICC_PMR_EL1, "%0"),
>                                              ^
>    2 warnings generated.
> --
>    In file included from drivers/power/supply/ltc2941-battery-gauge.c:12:
>    In file included from include/linux/module.h:13:
>    In file included from include/linux/stat.h:6:
>    In file included from arch/arm64/include/asm/stat.h:12:
>    In file included from include/linux/time.h:6:
>    In file included from include/linux/seqlock.h:36:
>    In file included from include/linux/spinlock.h:54:
>    In file included from include/linux/irqflags.h:16:
>>> arch/arm64/include/asm/irqflags.h:45:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
>                    : "r" (pmr_irqon)
>                           ^
>    arch/arm64/include/asm/irqflags.h:42:29: note: use constraint modifier "w"
>                    __msr_s(SYS_ICC_PMR_EL1, "%0"),
>                                              ^
>    arch/arm64/include/asm/irqflags.h:67:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
>                    : "r" (pmr_irqoff)
>                           ^
>    arch/arm64/include/asm/irqflags.h:64:29: note: use constraint modifier "w"
>                    __msr_s(SYS_ICC_PMR_EL1, "%0"),
>                                              ^
>    drivers/power/supply/ltc2941-battery-gauge.c:476:13: warning: cast to smaller integer type 'enum ltc294x_id' from 'const void *' [-Wvoid-pointer-to-enum-cast]
>            info->id = (enum ltc294x_id)of_device_get_match_data(&client->dev);
>                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>    3 warnings generated.
> --
>    In file included from drivers/power/supply/goldfish_battery.c:11:
>    In file included from include/linux/module.h:13:
>    In file included from include/linux/stat.h:6:
>    In file included from arch/arm64/include/asm/stat.h:12:
>    In file included from include/linux/time.h:6:
>    In file included from include/linux/seqlock.h:36:
>    In file included from include/linux/spinlock.h:54:
>    In file included from include/linux/irqflags.h:16:
>>> arch/arm64/include/asm/irqflags.h:45:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
>                    : "r" (pmr_irqon)
>                           ^
>    arch/arm64/include/asm/irqflags.h:42:29: note: use constraint modifier "w"
>                    __msr_s(SYS_ICC_PMR_EL1, "%0"),
>                                              ^
>    arch/arm64/include/asm/irqflags.h:67:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
>                    : "r" (pmr_irqoff)
>                           ^
>    arch/arm64/include/asm/irqflags.h:64:29: note: use constraint modifier "w"
>                    __msr_s(SYS_ICC_PMR_EL1, "%0"),
>                                              ^
>    drivers/power/supply/goldfish_battery.c:269:36: warning: unused variable 'goldfish_battery_acpi_match' [-Wunused-const-variable]
>    static const struct acpi_device_id goldfish_battery_acpi_match[] = {
>                                       ^
>    3 warnings generated.
> --
>    In file included from drivers/power/supply/bq25890_charger.c:8:
>    In file included from include/linux/module.h:13:
>    In file included from include/linux/stat.h:6:
>    In file included from arch/arm64/include/asm/stat.h:12:
>    In file included from include/linux/time.h:6:
>    In file included from include/linux/seqlock.h:36:
>    In file included from include/linux/spinlock.h:54:
>    In file included from include/linux/irqflags.h:16:
>>> arch/arm64/include/asm/irqflags.h:45:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
>                    : "r" (pmr_irqon)
>                           ^
>    arch/arm64/include/asm/irqflags.h:42:29: note: use constraint modifier "w"
>                    __msr_s(SYS_ICC_PMR_EL1, "%0"),
>                                              ^
>    arch/arm64/include/asm/irqflags.h:67:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
>                    : "r" (pmr_irqoff)
>                           ^
>    arch/arm64/include/asm/irqflags.h:64:29: note: use constraint modifier "w"
>                    __msr_s(SYS_ICC_PMR_EL1, "%0"),
>                                              ^
>    drivers/power/supply/bq25890_charger.c:1060:36: warning: unused variable 'bq25890_acpi_match' [-Wunused-const-variable]
>    static const struct acpi_device_id bq25890_acpi_match[] = {
>                                       ^
>    3 warnings generated.
> --
>    In file included from drivers/power/reset/vexpress-poweroff.c:8:
>    In file included from include/linux/notifier.h:15:
>    In file included from include/linux/rwsem.h:16:
>    In file included from include/linux/spinlock.h:54:
>    In file included from include/linux/irqflags.h:16:
>>> arch/arm64/include/asm/irqflags.h:45:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
>                    : "r" (pmr_irqon)
>                           ^
>    arch/arm64/include/asm/irqflags.h:42:29: note: use constraint modifier "w"
>                    __msr_s(SYS_ICC_PMR_EL1, "%0"),
>                                              ^
>    arch/arm64/include/asm/irqflags.h:67:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
>                    : "r" (pmr_irqoff)
>                           ^
>    arch/arm64/include/asm/irqflags.h:64:29: note: use constraint modifier "w"
>                    __msr_s(SYS_ICC_PMR_EL1, "%0"),
>                                              ^
>    drivers/power/reset/vexpress-poweroff.c:124:10: warning: cast to smaller integer type 'enum vexpress_reset_func' from 'const void *' [-Wvoid-pointer-to-enum-cast]
>            switch ((enum vexpress_reset_func)match->data) {
>                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>    3 warnings generated.
> --
>    In file included from arch/arm64/kernel/asm-offsets.c:10:
>    In file included from include/linux/arm_sdei.h:8:
>    In file included from include/acpi/ghes.h:5:
>    In file included from include/acpi/apei.h:9:
>    In file included from include/linux/acpi.h:13:
>    In file included from include/linux/irqdomain.h:35:
>    In file included from include/linux/of.h:17:
>    In file included from include/linux/kobject.h:20:
>    In file included from include/linux/sysfs.h:16:
>    In file included from include/linux/kernfs.h:13:
>    In file included from include/linux/idr.h:15:
>    In file included from include/linux/radix-tree.h:15:
>    In file included from include/linux/rcupdate.h:26:
>    In file included from include/linux/irqflags.h:16:
>>> arch/arm64/include/asm/irqflags.h:45:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
>                    : "r" (pmr_irqon)
>                           ^
>    arch/arm64/include/asm/irqflags.h:42:29: note: use constraint modifier "w"
>                    __msr_s(SYS_ICC_PMR_EL1, "%0"),
>                                              ^
>    arch/arm64/include/asm/irqflags.h:67:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
>                    : "r" (pmr_irqoff)
>                           ^
>    arch/arm64/include/asm/irqflags.h:64:29: note: use constraint modifier "w"
>                    __msr_s(SYS_ICC_PMR_EL1, "%0"),
>                                              ^
>    2 warnings generated.
>    arch/arm64/kernel/vdso/vgettimeofday.c:9:5: warning: no previous prototype for function '__kernel_clock_gettime' [-Wmissing-prototypes]
>    int __kernel_clock_gettime(clockid_t clock,
>        ^
>    arch/arm64/kernel/vdso/vgettimeofday.c:9:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
>    int __kernel_clock_gettime(clockid_t clock,
>    ^
>    static 
>    arch/arm64/kernel/vdso/vgettimeofday.c:15:5: warning: no previous prototype for function '__kernel_gettimeofday' [-Wmissing-prototypes]
>    int __kernel_gettimeofday(struct __kernel_old_timeval *tv,
>        ^
>    arch/arm64/kernel/vdso/vgettimeofday.c:15:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
>    int __kernel_gettimeofday(struct __kernel_old_timeval *tv,
>    ^
>    static 
>    arch/arm64/kernel/vdso/vgettimeofday.c:21:5: warning: no previous prototype for function '__kernel_clock_getres' [-Wmissing-prototypes]
>    int __kernel_clock_getres(clockid_t clock_id,
>        ^
>    arch/arm64/kernel/vdso/vgettimeofday.c:21:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
>    int __kernel_clock_getres(clockid_t clock_id,
>    ^
>    static 
>    3 warnings generated.
>
> vim +45 arch/arm64/include/asm/irqflags.h
>
>     12	
>     13	/*
>     14	 * Aarch64 has flags for masking: Debug, Asynchronous (serror), Interrupts and
>     15	 * FIQ exceptions, in the 'daif' register. We mask and unmask them in 'dai'
>     16	 * order:
>     17	 * Masking debug exceptions causes all other exceptions to be masked too/
>     18	 * Masking SError masks irq, but not debug exceptions. Masking irqs has no
>     19	 * side effects for other flags. Keeping to this order makes it easier for
>     20	 * entry.S to know which exceptions should be unmasked.
>     21	 *
>     22	 * FIQ is never expected, but we mask it when we disable debug exceptions, and
>     23	 * unmask it at all other times.
>     24	 */
>     25	
>     26	/*
>     27	 * CPU interrupt mask handling.
>     28	 */
>     29	static inline void arch_local_irq_enable(void)
>     30	{
>     31		u32 pmr_irqon = GIC_PRIO_IRQON;
>     32	
>     33		if (system_has_prio_mask_debugging()) {
>     34			u32 pmr = read_sysreg_s(SYS_ICC_PMR_EL1);
>     35			u32 pmr_irqoff = gic_prio_irqoff();
>     36	
>     37			WARN_ON_ONCE(pmr != pmr_irqon && pmr != pmr_irqoff);
>     38		}
>     39	
>     40		asm volatile(ALTERNATIVE(
>     41			"msr	daifclr, #2		// arch_local_irq_enable",
>     42			__msr_s(SYS_ICC_PMR_EL1, "%0"),
>     43			ARM64_HAS_IRQ_PRIO_MASKING)
>     44			:
>   > 45			: "r" (pmr_irqon)
>     46			: "memory");
>     47	
>     48		pmr_sync();
>     49	}
>     50	
>
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

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

* Re: [PATCH 2/2] irqchip/gic-v3: Support pseudo-NMIs when SCR_EL3.FIQ == 0
@ 2020-06-26  9:41       ` Alexandru Elisei
  0 siblings, 0 replies; 16+ messages in thread
From: Alexandru Elisei @ 2020-06-26  9:41 UTC (permalink / raw)
  To: kernel test robot, linux-arm-kernel, linux-kernel, kvmarm
  Cc: kbuild-all, jason, maz, clang-built-linux, catalin.marinas, tglx, will

Hi,

On 6/26/20 2:51 AM, kernel test robot wrote:
> Hi Alexandru,
>
> Thank you for the patch! Perhaps something to improve:
>
> [auto build test WARNING on arm64/for-next/core]
> [also build test WARNING on tip/irq/core v5.8-rc2 next-20200625]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use  as documented in
> https://git-scm.com/docs/git-format-patch]
>
> url:    https://github.com/0day-ci/linux/commits/Alexandru-Elisei/irqchip-gic-v3-Support-pseudo-NMIs-when-SCR_EL3-FIQ-0/20200625-230144
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git for-next/core
> config: arm64-randconfig-r025-20200624 (attached as .config)
> compiler: clang version 11.0.0 (https://github.com/llvm/llvm-project 8911a35180c6777188fefe0954a2451a2b91deaf)
> 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
>         # save the attached .config to linux build tree
>         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=arm64 

My mistake, I'll start compiling the kernel with clang too.

The register width for ICC_PMR_EL1 in the kernel is rather inconsistent: in
arch_gicv3.h, the accessors use 32 bits for the PMR value which gets casted to 64
bit by the {read,write}_sysreg_s macros anyway, in struct pt_regs the register is
64-bit, in __cpu_do_idle_irqprio it's declared as an unsigned long,
arch_local_irqs_{disable,enable} declares it as u32 and casts it to an unsigned
long when used by the inline assembly, the gicv3 irqchip driver uses it as a 32
bit variable.

I think the confusion stems from the fact that originally it was a 32 bit
register, but was changed to 64 bits in Arm IHI 0069E (January 2019).

I could cast it to an unsigned long in the inline assembly, but IMO that looks a
bit awkward. Before sending the patches I was considering changing it everywhere
to 64 bits, but Mark Rutland had a different idea. Mark, would you mind explaining
why keeping it 32 bit wide makes more sense?

Thanks,
Alex
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
>
> All warnings (new ones prefixed by >>):
>
>    In file included from arch/arm64/kernel/asm-offsets.c:10:
>    In file included from include/linux/arm_sdei.h:8:
>    In file included from include/acpi/ghes.h:5:
>    In file included from include/acpi/apei.h:9:
>    In file included from include/linux/acpi.h:13:
>    In file included from include/linux/irqdomain.h:35:
>    In file included from include/linux/of.h:17:
>    In file included from include/linux/kobject.h:20:
>    In file included from include/linux/sysfs.h:16:
>    In file included from include/linux/kernfs.h:13:
>    In file included from include/linux/idr.h:15:
>    In file included from include/linux/radix-tree.h:15:
>    In file included from include/linux/rcupdate.h:26:
>    In file included from include/linux/irqflags.h:16:
>>> arch/arm64/include/asm/irqflags.h:45:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
>                    : "r" (pmr_irqon)
>                           ^
>    arch/arm64/include/asm/irqflags.h:42:29: note: use constraint modifier "w"
>                    __msr_s(SYS_ICC_PMR_EL1, "%0"),
>                                              ^
>    arch/arm64/include/asm/irqflags.h:67:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
>                    : "r" (pmr_irqoff)
>                           ^
>    arch/arm64/include/asm/irqflags.h:64:29: note: use constraint modifier "w"
>                    __msr_s(SYS_ICC_PMR_EL1, "%0"),
>                                              ^
>    2 warnings generated.
> --
>    In file included from drivers/power/supply/ltc2941-battery-gauge.c:12:
>    In file included from include/linux/module.h:13:
>    In file included from include/linux/stat.h:6:
>    In file included from arch/arm64/include/asm/stat.h:12:
>    In file included from include/linux/time.h:6:
>    In file included from include/linux/seqlock.h:36:
>    In file included from include/linux/spinlock.h:54:
>    In file included from include/linux/irqflags.h:16:
>>> arch/arm64/include/asm/irqflags.h:45:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
>                    : "r" (pmr_irqon)
>                           ^
>    arch/arm64/include/asm/irqflags.h:42:29: note: use constraint modifier "w"
>                    __msr_s(SYS_ICC_PMR_EL1, "%0"),
>                                              ^
>    arch/arm64/include/asm/irqflags.h:67:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
>                    : "r" (pmr_irqoff)
>                           ^
>    arch/arm64/include/asm/irqflags.h:64:29: note: use constraint modifier "w"
>                    __msr_s(SYS_ICC_PMR_EL1, "%0"),
>                                              ^
>    drivers/power/supply/ltc2941-battery-gauge.c:476:13: warning: cast to smaller integer type 'enum ltc294x_id' from 'const void *' [-Wvoid-pointer-to-enum-cast]
>            info->id = (enum ltc294x_id)of_device_get_match_data(&client->dev);
>                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>    3 warnings generated.
> --
>    In file included from drivers/power/supply/goldfish_battery.c:11:
>    In file included from include/linux/module.h:13:
>    In file included from include/linux/stat.h:6:
>    In file included from arch/arm64/include/asm/stat.h:12:
>    In file included from include/linux/time.h:6:
>    In file included from include/linux/seqlock.h:36:
>    In file included from include/linux/spinlock.h:54:
>    In file included from include/linux/irqflags.h:16:
>>> arch/arm64/include/asm/irqflags.h:45:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
>                    : "r" (pmr_irqon)
>                           ^
>    arch/arm64/include/asm/irqflags.h:42:29: note: use constraint modifier "w"
>                    __msr_s(SYS_ICC_PMR_EL1, "%0"),
>                                              ^
>    arch/arm64/include/asm/irqflags.h:67:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
>                    : "r" (pmr_irqoff)
>                           ^
>    arch/arm64/include/asm/irqflags.h:64:29: note: use constraint modifier "w"
>                    __msr_s(SYS_ICC_PMR_EL1, "%0"),
>                                              ^
>    drivers/power/supply/goldfish_battery.c:269:36: warning: unused variable 'goldfish_battery_acpi_match' [-Wunused-const-variable]
>    static const struct acpi_device_id goldfish_battery_acpi_match[] = {
>                                       ^
>    3 warnings generated.
> --
>    In file included from drivers/power/supply/bq25890_charger.c:8:
>    In file included from include/linux/module.h:13:
>    In file included from include/linux/stat.h:6:
>    In file included from arch/arm64/include/asm/stat.h:12:
>    In file included from include/linux/time.h:6:
>    In file included from include/linux/seqlock.h:36:
>    In file included from include/linux/spinlock.h:54:
>    In file included from include/linux/irqflags.h:16:
>>> arch/arm64/include/asm/irqflags.h:45:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
>                    : "r" (pmr_irqon)
>                           ^
>    arch/arm64/include/asm/irqflags.h:42:29: note: use constraint modifier "w"
>                    __msr_s(SYS_ICC_PMR_EL1, "%0"),
>                                              ^
>    arch/arm64/include/asm/irqflags.h:67:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
>                    : "r" (pmr_irqoff)
>                           ^
>    arch/arm64/include/asm/irqflags.h:64:29: note: use constraint modifier "w"
>                    __msr_s(SYS_ICC_PMR_EL1, "%0"),
>                                              ^
>    drivers/power/supply/bq25890_charger.c:1060:36: warning: unused variable 'bq25890_acpi_match' [-Wunused-const-variable]
>    static const struct acpi_device_id bq25890_acpi_match[] = {
>                                       ^
>    3 warnings generated.
> --
>    In file included from drivers/power/reset/vexpress-poweroff.c:8:
>    In file included from include/linux/notifier.h:15:
>    In file included from include/linux/rwsem.h:16:
>    In file included from include/linux/spinlock.h:54:
>    In file included from include/linux/irqflags.h:16:
>>> arch/arm64/include/asm/irqflags.h:45:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
>                    : "r" (pmr_irqon)
>                           ^
>    arch/arm64/include/asm/irqflags.h:42:29: note: use constraint modifier "w"
>                    __msr_s(SYS_ICC_PMR_EL1, "%0"),
>                                              ^
>    arch/arm64/include/asm/irqflags.h:67:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
>                    : "r" (pmr_irqoff)
>                           ^
>    arch/arm64/include/asm/irqflags.h:64:29: note: use constraint modifier "w"
>                    __msr_s(SYS_ICC_PMR_EL1, "%0"),
>                                              ^
>    drivers/power/reset/vexpress-poweroff.c:124:10: warning: cast to smaller integer type 'enum vexpress_reset_func' from 'const void *' [-Wvoid-pointer-to-enum-cast]
>            switch ((enum vexpress_reset_func)match->data) {
>                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>    3 warnings generated.
> --
>    In file included from arch/arm64/kernel/asm-offsets.c:10:
>    In file included from include/linux/arm_sdei.h:8:
>    In file included from include/acpi/ghes.h:5:
>    In file included from include/acpi/apei.h:9:
>    In file included from include/linux/acpi.h:13:
>    In file included from include/linux/irqdomain.h:35:
>    In file included from include/linux/of.h:17:
>    In file included from include/linux/kobject.h:20:
>    In file included from include/linux/sysfs.h:16:
>    In file included from include/linux/kernfs.h:13:
>    In file included from include/linux/idr.h:15:
>    In file included from include/linux/radix-tree.h:15:
>    In file included from include/linux/rcupdate.h:26:
>    In file included from include/linux/irqflags.h:16:
>>> arch/arm64/include/asm/irqflags.h:45:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
>                    : "r" (pmr_irqon)
>                           ^
>    arch/arm64/include/asm/irqflags.h:42:29: note: use constraint modifier "w"
>                    __msr_s(SYS_ICC_PMR_EL1, "%0"),
>                                              ^
>    arch/arm64/include/asm/irqflags.h:67:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
>                    : "r" (pmr_irqoff)
>                           ^
>    arch/arm64/include/asm/irqflags.h:64:29: note: use constraint modifier "w"
>                    __msr_s(SYS_ICC_PMR_EL1, "%0"),
>                                              ^
>    2 warnings generated.
>    arch/arm64/kernel/vdso/vgettimeofday.c:9:5: warning: no previous prototype for function '__kernel_clock_gettime' [-Wmissing-prototypes]
>    int __kernel_clock_gettime(clockid_t clock,
>        ^
>    arch/arm64/kernel/vdso/vgettimeofday.c:9:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
>    int __kernel_clock_gettime(clockid_t clock,
>    ^
>    static 
>    arch/arm64/kernel/vdso/vgettimeofday.c:15:5: warning: no previous prototype for function '__kernel_gettimeofday' [-Wmissing-prototypes]
>    int __kernel_gettimeofday(struct __kernel_old_timeval *tv,
>        ^
>    arch/arm64/kernel/vdso/vgettimeofday.c:15:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
>    int __kernel_gettimeofday(struct __kernel_old_timeval *tv,
>    ^
>    static 
>    arch/arm64/kernel/vdso/vgettimeofday.c:21:5: warning: no previous prototype for function '__kernel_clock_getres' [-Wmissing-prototypes]
>    int __kernel_clock_getres(clockid_t clock_id,
>        ^
>    arch/arm64/kernel/vdso/vgettimeofday.c:21:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
>    int __kernel_clock_getres(clockid_t clock_id,
>    ^
>    static 
>    3 warnings generated.
>
> vim +45 arch/arm64/include/asm/irqflags.h
>
>     12	
>     13	/*
>     14	 * Aarch64 has flags for masking: Debug, Asynchronous (serror), Interrupts and
>     15	 * FIQ exceptions, in the 'daif' register. We mask and unmask them in 'dai'
>     16	 * order:
>     17	 * Masking debug exceptions causes all other exceptions to be masked too/
>     18	 * Masking SError masks irq, but not debug exceptions. Masking irqs has no
>     19	 * side effects for other flags. Keeping to this order makes it easier for
>     20	 * entry.S to know which exceptions should be unmasked.
>     21	 *
>     22	 * FIQ is never expected, but we mask it when we disable debug exceptions, and
>     23	 * unmask it at all other times.
>     24	 */
>     25	
>     26	/*
>     27	 * CPU interrupt mask handling.
>     28	 */
>     29	static inline void arch_local_irq_enable(void)
>     30	{
>     31		u32 pmr_irqon = GIC_PRIO_IRQON;
>     32	
>     33		if (system_has_prio_mask_debugging()) {
>     34			u32 pmr = read_sysreg_s(SYS_ICC_PMR_EL1);
>     35			u32 pmr_irqoff = gic_prio_irqoff();
>     36	
>     37			WARN_ON_ONCE(pmr != pmr_irqon && pmr != pmr_irqoff);
>     38		}
>     39	
>     40		asm volatile(ALTERNATIVE(
>     41			"msr	daifclr, #2		// arch_local_irq_enable",
>     42			__msr_s(SYS_ICC_PMR_EL1, "%0"),
>     43			ARM64_HAS_IRQ_PRIO_MASKING)
>     44			:
>   > 45			: "r" (pmr_irqon)
>     46			: "memory");
>     47	
>     48		pmr_sync();
>     49	}
>     50	
>
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

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

* Re: [PATCH 2/2] irqchip/gic-v3: Support pseudo-NMIs when SCR_EL3.FIQ == 0
@ 2020-06-26  9:41       ` Alexandru Elisei
  0 siblings, 0 replies; 16+ messages in thread
From: Alexandru Elisei @ 2020-06-26  9:41 UTC (permalink / raw)
  To: kernel test robot, linux-arm-kernel, linux-kernel, kvmarm
  Cc: kbuild-all, jason, maz, clang-built-linux, catalin.marinas,
	yuzenghui, tglx, will, julien.thierry.kdev

Hi,

On 6/26/20 2:51 AM, kernel test robot wrote:
> Hi Alexandru,
>
> Thank you for the patch! Perhaps something to improve:
>
> [auto build test WARNING on arm64/for-next/core]
> [also build test WARNING on tip/irq/core v5.8-rc2 next-20200625]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use  as documented in
> https://git-scm.com/docs/git-format-patch]
>
> url:    https://github.com/0day-ci/linux/commits/Alexandru-Elisei/irqchip-gic-v3-Support-pseudo-NMIs-when-SCR_EL3-FIQ-0/20200625-230144
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git for-next/core
> config: arm64-randconfig-r025-20200624 (attached as .config)
> compiler: clang version 11.0.0 (https://github.com/llvm/llvm-project 8911a35180c6777188fefe0954a2451a2b91deaf)
> 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
>         # save the attached .config to linux build tree
>         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=arm64 

My mistake, I'll start compiling the kernel with clang too.

The register width for ICC_PMR_EL1 in the kernel is rather inconsistent: in
arch_gicv3.h, the accessors use 32 bits for the PMR value which gets casted to 64
bit by the {read,write}_sysreg_s macros anyway, in struct pt_regs the register is
64-bit, in __cpu_do_idle_irqprio it's declared as an unsigned long,
arch_local_irqs_{disable,enable} declares it as u32 and casts it to an unsigned
long when used by the inline assembly, the gicv3 irqchip driver uses it as a 32
bit variable.

I think the confusion stems from the fact that originally it was a 32 bit
register, but was changed to 64 bits in Arm IHI 0069E (January 2019).

I could cast it to an unsigned long in the inline assembly, but IMO that looks a
bit awkward. Before sending the patches I was considering changing it everywhere
to 64 bits, but Mark Rutland had a different idea. Mark, would you mind explaining
why keeping it 32 bit wide makes more sense?

Thanks,
Alex
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
>
> All warnings (new ones prefixed by >>):
>
>    In file included from arch/arm64/kernel/asm-offsets.c:10:
>    In file included from include/linux/arm_sdei.h:8:
>    In file included from include/acpi/ghes.h:5:
>    In file included from include/acpi/apei.h:9:
>    In file included from include/linux/acpi.h:13:
>    In file included from include/linux/irqdomain.h:35:
>    In file included from include/linux/of.h:17:
>    In file included from include/linux/kobject.h:20:
>    In file included from include/linux/sysfs.h:16:
>    In file included from include/linux/kernfs.h:13:
>    In file included from include/linux/idr.h:15:
>    In file included from include/linux/radix-tree.h:15:
>    In file included from include/linux/rcupdate.h:26:
>    In file included from include/linux/irqflags.h:16:
>>> arch/arm64/include/asm/irqflags.h:45:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
>                    : "r" (pmr_irqon)
>                           ^
>    arch/arm64/include/asm/irqflags.h:42:29: note: use constraint modifier "w"
>                    __msr_s(SYS_ICC_PMR_EL1, "%0"),
>                                              ^
>    arch/arm64/include/asm/irqflags.h:67:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
>                    : "r" (pmr_irqoff)
>                           ^
>    arch/arm64/include/asm/irqflags.h:64:29: note: use constraint modifier "w"
>                    __msr_s(SYS_ICC_PMR_EL1, "%0"),
>                                              ^
>    2 warnings generated.
> --
>    In file included from drivers/power/supply/ltc2941-battery-gauge.c:12:
>    In file included from include/linux/module.h:13:
>    In file included from include/linux/stat.h:6:
>    In file included from arch/arm64/include/asm/stat.h:12:
>    In file included from include/linux/time.h:6:
>    In file included from include/linux/seqlock.h:36:
>    In file included from include/linux/spinlock.h:54:
>    In file included from include/linux/irqflags.h:16:
>>> arch/arm64/include/asm/irqflags.h:45:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
>                    : "r" (pmr_irqon)
>                           ^
>    arch/arm64/include/asm/irqflags.h:42:29: note: use constraint modifier "w"
>                    __msr_s(SYS_ICC_PMR_EL1, "%0"),
>                                              ^
>    arch/arm64/include/asm/irqflags.h:67:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
>                    : "r" (pmr_irqoff)
>                           ^
>    arch/arm64/include/asm/irqflags.h:64:29: note: use constraint modifier "w"
>                    __msr_s(SYS_ICC_PMR_EL1, "%0"),
>                                              ^
>    drivers/power/supply/ltc2941-battery-gauge.c:476:13: warning: cast to smaller integer type 'enum ltc294x_id' from 'const void *' [-Wvoid-pointer-to-enum-cast]
>            info->id = (enum ltc294x_id)of_device_get_match_data(&client->dev);
>                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>    3 warnings generated.
> --
>    In file included from drivers/power/supply/goldfish_battery.c:11:
>    In file included from include/linux/module.h:13:
>    In file included from include/linux/stat.h:6:
>    In file included from arch/arm64/include/asm/stat.h:12:
>    In file included from include/linux/time.h:6:
>    In file included from include/linux/seqlock.h:36:
>    In file included from include/linux/spinlock.h:54:
>    In file included from include/linux/irqflags.h:16:
>>> arch/arm64/include/asm/irqflags.h:45:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
>                    : "r" (pmr_irqon)
>                           ^
>    arch/arm64/include/asm/irqflags.h:42:29: note: use constraint modifier "w"
>                    __msr_s(SYS_ICC_PMR_EL1, "%0"),
>                                              ^
>    arch/arm64/include/asm/irqflags.h:67:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
>                    : "r" (pmr_irqoff)
>                           ^
>    arch/arm64/include/asm/irqflags.h:64:29: note: use constraint modifier "w"
>                    __msr_s(SYS_ICC_PMR_EL1, "%0"),
>                                              ^
>    drivers/power/supply/goldfish_battery.c:269:36: warning: unused variable 'goldfish_battery_acpi_match' [-Wunused-const-variable]
>    static const struct acpi_device_id goldfish_battery_acpi_match[] = {
>                                       ^
>    3 warnings generated.
> --
>    In file included from drivers/power/supply/bq25890_charger.c:8:
>    In file included from include/linux/module.h:13:
>    In file included from include/linux/stat.h:6:
>    In file included from arch/arm64/include/asm/stat.h:12:
>    In file included from include/linux/time.h:6:
>    In file included from include/linux/seqlock.h:36:
>    In file included from include/linux/spinlock.h:54:
>    In file included from include/linux/irqflags.h:16:
>>> arch/arm64/include/asm/irqflags.h:45:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
>                    : "r" (pmr_irqon)
>                           ^
>    arch/arm64/include/asm/irqflags.h:42:29: note: use constraint modifier "w"
>                    __msr_s(SYS_ICC_PMR_EL1, "%0"),
>                                              ^
>    arch/arm64/include/asm/irqflags.h:67:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
>                    : "r" (pmr_irqoff)
>                           ^
>    arch/arm64/include/asm/irqflags.h:64:29: note: use constraint modifier "w"
>                    __msr_s(SYS_ICC_PMR_EL1, "%0"),
>                                              ^
>    drivers/power/supply/bq25890_charger.c:1060:36: warning: unused variable 'bq25890_acpi_match' [-Wunused-const-variable]
>    static const struct acpi_device_id bq25890_acpi_match[] = {
>                                       ^
>    3 warnings generated.
> --
>    In file included from drivers/power/reset/vexpress-poweroff.c:8:
>    In file included from include/linux/notifier.h:15:
>    In file included from include/linux/rwsem.h:16:
>    In file included from include/linux/spinlock.h:54:
>    In file included from include/linux/irqflags.h:16:
>>> arch/arm64/include/asm/irqflags.h:45:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
>                    : "r" (pmr_irqon)
>                           ^
>    arch/arm64/include/asm/irqflags.h:42:29: note: use constraint modifier "w"
>                    __msr_s(SYS_ICC_PMR_EL1, "%0"),
>                                              ^
>    arch/arm64/include/asm/irqflags.h:67:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
>                    : "r" (pmr_irqoff)
>                           ^
>    arch/arm64/include/asm/irqflags.h:64:29: note: use constraint modifier "w"
>                    __msr_s(SYS_ICC_PMR_EL1, "%0"),
>                                              ^
>    drivers/power/reset/vexpress-poweroff.c:124:10: warning: cast to smaller integer type 'enum vexpress_reset_func' from 'const void *' [-Wvoid-pointer-to-enum-cast]
>            switch ((enum vexpress_reset_func)match->data) {
>                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>    3 warnings generated.
> --
>    In file included from arch/arm64/kernel/asm-offsets.c:10:
>    In file included from include/linux/arm_sdei.h:8:
>    In file included from include/acpi/ghes.h:5:
>    In file included from include/acpi/apei.h:9:
>    In file included from include/linux/acpi.h:13:
>    In file included from include/linux/irqdomain.h:35:
>    In file included from include/linux/of.h:17:
>    In file included from include/linux/kobject.h:20:
>    In file included from include/linux/sysfs.h:16:
>    In file included from include/linux/kernfs.h:13:
>    In file included from include/linux/idr.h:15:
>    In file included from include/linux/radix-tree.h:15:
>    In file included from include/linux/rcupdate.h:26:
>    In file included from include/linux/irqflags.h:16:
>>> arch/arm64/include/asm/irqflags.h:45:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
>                    : "r" (pmr_irqon)
>                           ^
>    arch/arm64/include/asm/irqflags.h:42:29: note: use constraint modifier "w"
>                    __msr_s(SYS_ICC_PMR_EL1, "%0"),
>                                              ^
>    arch/arm64/include/asm/irqflags.h:67:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
>                    : "r" (pmr_irqoff)
>                           ^
>    arch/arm64/include/asm/irqflags.h:64:29: note: use constraint modifier "w"
>                    __msr_s(SYS_ICC_PMR_EL1, "%0"),
>                                              ^
>    2 warnings generated.
>    arch/arm64/kernel/vdso/vgettimeofday.c:9:5: warning: no previous prototype for function '__kernel_clock_gettime' [-Wmissing-prototypes]
>    int __kernel_clock_gettime(clockid_t clock,
>        ^
>    arch/arm64/kernel/vdso/vgettimeofday.c:9:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
>    int __kernel_clock_gettime(clockid_t clock,
>    ^
>    static 
>    arch/arm64/kernel/vdso/vgettimeofday.c:15:5: warning: no previous prototype for function '__kernel_gettimeofday' [-Wmissing-prototypes]
>    int __kernel_gettimeofday(struct __kernel_old_timeval *tv,
>        ^
>    arch/arm64/kernel/vdso/vgettimeofday.c:15:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
>    int __kernel_gettimeofday(struct __kernel_old_timeval *tv,
>    ^
>    static 
>    arch/arm64/kernel/vdso/vgettimeofday.c:21:5: warning: no previous prototype for function '__kernel_clock_getres' [-Wmissing-prototypes]
>    int __kernel_clock_getres(clockid_t clock_id,
>        ^
>    arch/arm64/kernel/vdso/vgettimeofday.c:21:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
>    int __kernel_clock_getres(clockid_t clock_id,
>    ^
>    static 
>    3 warnings generated.
>
> vim +45 arch/arm64/include/asm/irqflags.h
>
>     12	
>     13	/*
>     14	 * Aarch64 has flags for masking: Debug, Asynchronous (serror), Interrupts and
>     15	 * FIQ exceptions, in the 'daif' register. We mask and unmask them in 'dai'
>     16	 * order:
>     17	 * Masking debug exceptions causes all other exceptions to be masked too/
>     18	 * Masking SError masks irq, but not debug exceptions. Masking irqs has no
>     19	 * side effects for other flags. Keeping to this order makes it easier for
>     20	 * entry.S to know which exceptions should be unmasked.
>     21	 *
>     22	 * FIQ is never expected, but we mask it when we disable debug exceptions, and
>     23	 * unmask it at all other times.
>     24	 */
>     25	
>     26	/*
>     27	 * CPU interrupt mask handling.
>     28	 */
>     29	static inline void arch_local_irq_enable(void)
>     30	{
>     31		u32 pmr_irqon = GIC_PRIO_IRQON;
>     32	
>     33		if (system_has_prio_mask_debugging()) {
>     34			u32 pmr = read_sysreg_s(SYS_ICC_PMR_EL1);
>     35			u32 pmr_irqoff = gic_prio_irqoff();
>     36	
>     37			WARN_ON_ONCE(pmr != pmr_irqon && pmr != pmr_irqoff);
>     38		}
>     39	
>     40		asm volatile(ALTERNATIVE(
>     41			"msr	daifclr, #2		// arch_local_irq_enable",
>     42			__msr_s(SYS_ICC_PMR_EL1, "%0"),
>     43			ARM64_HAS_IRQ_PRIO_MASKING)
>     44			:
>   > 45			: "r" (pmr_irqon)
>     46			: "memory");
>     47	
>     48		pmr_sync();
>     49	}
>     50	
>
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH 2/2] irqchip/gic-v3: Support pseudo-NMIs when SCR_EL3.FIQ == 0
@ 2020-06-26  9:41       ` Alexandru Elisei
  0 siblings, 0 replies; 16+ messages in thread
From: Alexandru Elisei @ 2020-06-26  9:41 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 14979 bytes --]

Hi,

On 6/26/20 2:51 AM, kernel test robot wrote:
> Hi Alexandru,
>
> Thank you for the patch! Perhaps something to improve:
>
> [auto build test WARNING on arm64/for-next/core]
> [also build test WARNING on tip/irq/core v5.8-rc2 next-20200625]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use  as documented in
> https://git-scm.com/docs/git-format-patch]
>
> url:    https://github.com/0day-ci/linux/commits/Alexandru-Elisei/irqchip-gic-v3-Support-pseudo-NMIs-when-SCR_EL3-FIQ-0/20200625-230144
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git for-next/core
> config: arm64-randconfig-r025-20200624 (attached as .config)
> compiler: clang version 11.0.0 (https://github.com/llvm/llvm-project 8911a35180c6777188fefe0954a2451a2b91deaf)
> 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
>         # save the attached .config to linux build tree
>         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=arm64 

My mistake, I'll start compiling the kernel with clang too.

The register width for ICC_PMR_EL1 in the kernel is rather inconsistent: in
arch_gicv3.h, the accessors use 32 bits for the PMR value which gets casted to 64
bit by the {read,write}_sysreg_s macros anyway, in struct pt_regs the register is
64-bit, in __cpu_do_idle_irqprio it's declared as an unsigned long,
arch_local_irqs_{disable,enable} declares it as u32 and casts it to an unsigned
long when used by the inline assembly, the gicv3 irqchip driver uses it as a 32
bit variable.

I think the confusion stems from the fact that originally it was a 32 bit
register, but was changed to 64 bits in Arm IHI 0069E (January 2019).

I could cast it to an unsigned long in the inline assembly, but IMO that looks a
bit awkward. Before sending the patches I was considering changing it everywhere
to 64 bits, but Mark Rutland had a different idea. Mark, would you mind explaining
why keeping it 32 bit wide makes more sense?

Thanks,
Alex
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
>
> All warnings (new ones prefixed by >>):
>
>    In file included from arch/arm64/kernel/asm-offsets.c:10:
>    In file included from include/linux/arm_sdei.h:8:
>    In file included from include/acpi/ghes.h:5:
>    In file included from include/acpi/apei.h:9:
>    In file included from include/linux/acpi.h:13:
>    In file included from include/linux/irqdomain.h:35:
>    In file included from include/linux/of.h:17:
>    In file included from include/linux/kobject.h:20:
>    In file included from include/linux/sysfs.h:16:
>    In file included from include/linux/kernfs.h:13:
>    In file included from include/linux/idr.h:15:
>    In file included from include/linux/radix-tree.h:15:
>    In file included from include/linux/rcupdate.h:26:
>    In file included from include/linux/irqflags.h:16:
>>> arch/arm64/include/asm/irqflags.h:45:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
>                    : "r" (pmr_irqon)
>                           ^
>    arch/arm64/include/asm/irqflags.h:42:29: note: use constraint modifier "w"
>                    __msr_s(SYS_ICC_PMR_EL1, "%0"),
>                                              ^
>    arch/arm64/include/asm/irqflags.h:67:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
>                    : "r" (pmr_irqoff)
>                           ^
>    arch/arm64/include/asm/irqflags.h:64:29: note: use constraint modifier "w"
>                    __msr_s(SYS_ICC_PMR_EL1, "%0"),
>                                              ^
>    2 warnings generated.
> --
>    In file included from drivers/power/supply/ltc2941-battery-gauge.c:12:
>    In file included from include/linux/module.h:13:
>    In file included from include/linux/stat.h:6:
>    In file included from arch/arm64/include/asm/stat.h:12:
>    In file included from include/linux/time.h:6:
>    In file included from include/linux/seqlock.h:36:
>    In file included from include/linux/spinlock.h:54:
>    In file included from include/linux/irqflags.h:16:
>>> arch/arm64/include/asm/irqflags.h:45:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
>                    : "r" (pmr_irqon)
>                           ^
>    arch/arm64/include/asm/irqflags.h:42:29: note: use constraint modifier "w"
>                    __msr_s(SYS_ICC_PMR_EL1, "%0"),
>                                              ^
>    arch/arm64/include/asm/irqflags.h:67:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
>                    : "r" (pmr_irqoff)
>                           ^
>    arch/arm64/include/asm/irqflags.h:64:29: note: use constraint modifier "w"
>                    __msr_s(SYS_ICC_PMR_EL1, "%0"),
>                                              ^
>    drivers/power/supply/ltc2941-battery-gauge.c:476:13: warning: cast to smaller integer type 'enum ltc294x_id' from 'const void *' [-Wvoid-pointer-to-enum-cast]
>            info->id = (enum ltc294x_id)of_device_get_match_data(&client->dev);
>                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>    3 warnings generated.
> --
>    In file included from drivers/power/supply/goldfish_battery.c:11:
>    In file included from include/linux/module.h:13:
>    In file included from include/linux/stat.h:6:
>    In file included from arch/arm64/include/asm/stat.h:12:
>    In file included from include/linux/time.h:6:
>    In file included from include/linux/seqlock.h:36:
>    In file included from include/linux/spinlock.h:54:
>    In file included from include/linux/irqflags.h:16:
>>> arch/arm64/include/asm/irqflags.h:45:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
>                    : "r" (pmr_irqon)
>                           ^
>    arch/arm64/include/asm/irqflags.h:42:29: note: use constraint modifier "w"
>                    __msr_s(SYS_ICC_PMR_EL1, "%0"),
>                                              ^
>    arch/arm64/include/asm/irqflags.h:67:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
>                    : "r" (pmr_irqoff)
>                           ^
>    arch/arm64/include/asm/irqflags.h:64:29: note: use constraint modifier "w"
>                    __msr_s(SYS_ICC_PMR_EL1, "%0"),
>                                              ^
>    drivers/power/supply/goldfish_battery.c:269:36: warning: unused variable 'goldfish_battery_acpi_match' [-Wunused-const-variable]
>    static const struct acpi_device_id goldfish_battery_acpi_match[] = {
>                                       ^
>    3 warnings generated.
> --
>    In file included from drivers/power/supply/bq25890_charger.c:8:
>    In file included from include/linux/module.h:13:
>    In file included from include/linux/stat.h:6:
>    In file included from arch/arm64/include/asm/stat.h:12:
>    In file included from include/linux/time.h:6:
>    In file included from include/linux/seqlock.h:36:
>    In file included from include/linux/spinlock.h:54:
>    In file included from include/linux/irqflags.h:16:
>>> arch/arm64/include/asm/irqflags.h:45:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
>                    : "r" (pmr_irqon)
>                           ^
>    arch/arm64/include/asm/irqflags.h:42:29: note: use constraint modifier "w"
>                    __msr_s(SYS_ICC_PMR_EL1, "%0"),
>                                              ^
>    arch/arm64/include/asm/irqflags.h:67:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
>                    : "r" (pmr_irqoff)
>                           ^
>    arch/arm64/include/asm/irqflags.h:64:29: note: use constraint modifier "w"
>                    __msr_s(SYS_ICC_PMR_EL1, "%0"),
>                                              ^
>    drivers/power/supply/bq25890_charger.c:1060:36: warning: unused variable 'bq25890_acpi_match' [-Wunused-const-variable]
>    static const struct acpi_device_id bq25890_acpi_match[] = {
>                                       ^
>    3 warnings generated.
> --
>    In file included from drivers/power/reset/vexpress-poweroff.c:8:
>    In file included from include/linux/notifier.h:15:
>    In file included from include/linux/rwsem.h:16:
>    In file included from include/linux/spinlock.h:54:
>    In file included from include/linux/irqflags.h:16:
>>> arch/arm64/include/asm/irqflags.h:45:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
>                    : "r" (pmr_irqon)
>                           ^
>    arch/arm64/include/asm/irqflags.h:42:29: note: use constraint modifier "w"
>                    __msr_s(SYS_ICC_PMR_EL1, "%0"),
>                                              ^
>    arch/arm64/include/asm/irqflags.h:67:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
>                    : "r" (pmr_irqoff)
>                           ^
>    arch/arm64/include/asm/irqflags.h:64:29: note: use constraint modifier "w"
>                    __msr_s(SYS_ICC_PMR_EL1, "%0"),
>                                              ^
>    drivers/power/reset/vexpress-poweroff.c:124:10: warning: cast to smaller integer type 'enum vexpress_reset_func' from 'const void *' [-Wvoid-pointer-to-enum-cast]
>            switch ((enum vexpress_reset_func)match->data) {
>                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>    3 warnings generated.
> --
>    In file included from arch/arm64/kernel/asm-offsets.c:10:
>    In file included from include/linux/arm_sdei.h:8:
>    In file included from include/acpi/ghes.h:5:
>    In file included from include/acpi/apei.h:9:
>    In file included from include/linux/acpi.h:13:
>    In file included from include/linux/irqdomain.h:35:
>    In file included from include/linux/of.h:17:
>    In file included from include/linux/kobject.h:20:
>    In file included from include/linux/sysfs.h:16:
>    In file included from include/linux/kernfs.h:13:
>    In file included from include/linux/idr.h:15:
>    In file included from include/linux/radix-tree.h:15:
>    In file included from include/linux/rcupdate.h:26:
>    In file included from include/linux/irqflags.h:16:
>>> arch/arm64/include/asm/irqflags.h:45:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
>                    : "r" (pmr_irqon)
>                           ^
>    arch/arm64/include/asm/irqflags.h:42:29: note: use constraint modifier "w"
>                    __msr_s(SYS_ICC_PMR_EL1, "%0"),
>                                              ^
>    arch/arm64/include/asm/irqflags.h:67:10: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
>                    : "r" (pmr_irqoff)
>                           ^
>    arch/arm64/include/asm/irqflags.h:64:29: note: use constraint modifier "w"
>                    __msr_s(SYS_ICC_PMR_EL1, "%0"),
>                                              ^
>    2 warnings generated.
>    arch/arm64/kernel/vdso/vgettimeofday.c:9:5: warning: no previous prototype for function '__kernel_clock_gettime' [-Wmissing-prototypes]
>    int __kernel_clock_gettime(clockid_t clock,
>        ^
>    arch/arm64/kernel/vdso/vgettimeofday.c:9:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
>    int __kernel_clock_gettime(clockid_t clock,
>    ^
>    static 
>    arch/arm64/kernel/vdso/vgettimeofday.c:15:5: warning: no previous prototype for function '__kernel_gettimeofday' [-Wmissing-prototypes]
>    int __kernel_gettimeofday(struct __kernel_old_timeval *tv,
>        ^
>    arch/arm64/kernel/vdso/vgettimeofday.c:15:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
>    int __kernel_gettimeofday(struct __kernel_old_timeval *tv,
>    ^
>    static 
>    arch/arm64/kernel/vdso/vgettimeofday.c:21:5: warning: no previous prototype for function '__kernel_clock_getres' [-Wmissing-prototypes]
>    int __kernel_clock_getres(clockid_t clock_id,
>        ^
>    arch/arm64/kernel/vdso/vgettimeofday.c:21:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
>    int __kernel_clock_getres(clockid_t clock_id,
>    ^
>    static 
>    3 warnings generated.
>
> vim +45 arch/arm64/include/asm/irqflags.h
>
>     12	
>     13	/*
>     14	 * Aarch64 has flags for masking: Debug, Asynchronous (serror), Interrupts and
>     15	 * FIQ exceptions, in the 'daif' register. We mask and unmask them in 'dai'
>     16	 * order:
>     17	 * Masking debug exceptions causes all other exceptions to be masked too/
>     18	 * Masking SError masks irq, but not debug exceptions. Masking irqs has no
>     19	 * side effects for other flags. Keeping to this order makes it easier for
>     20	 * entry.S to know which exceptions should be unmasked.
>     21	 *
>     22	 * FIQ is never expected, but we mask it when we disable debug exceptions, and
>     23	 * unmask it at all other times.
>     24	 */
>     25	
>     26	/*
>     27	 * CPU interrupt mask handling.
>     28	 */
>     29	static inline void arch_local_irq_enable(void)
>     30	{
>     31		u32 pmr_irqon = GIC_PRIO_IRQON;
>     32	
>     33		if (system_has_prio_mask_debugging()) {
>     34			u32 pmr = read_sysreg_s(SYS_ICC_PMR_EL1);
>     35			u32 pmr_irqoff = gic_prio_irqoff();
>     36	
>     37			WARN_ON_ONCE(pmr != pmr_irqon && pmr != pmr_irqoff);
>     38		}
>     39	
>     40		asm volatile(ALTERNATIVE(
>     41			"msr	daifclr, #2		// arch_local_irq_enable",
>     42			__msr_s(SYS_ICC_PMR_EL1, "%0"),
>     43			ARM64_HAS_IRQ_PRIO_MASKING)
>     44			:
>   > 45			: "r" (pmr_irqon)
>     46			: "memory");
>     47	
>     48		pmr_sync();
>     49	}
>     50	
>
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

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

end of thread, other threads:[~2020-06-26 11:29 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-25 15:00 [PATCH 0/2] irqchip/gic-v3: Support pseudo-NMIs when SCR_EL3.FIQ == 0 Alexandru Elisei
2020-06-25 15:00 ` Alexandru Elisei
2020-06-25 15:00 ` Alexandru Elisei
2020-06-25 15:00 ` [PATCH 1/2] irqchip/gicv3: Spell out when pseudo-NMIs are enabled Alexandru Elisei
2020-06-25 15:00   ` Alexandru Elisei
2020-06-25 15:00   ` Alexandru Elisei
2020-06-25 15:00 ` [PATCH 2/2] irqchip/gic-v3: Support pseudo-NMIs when SCR_EL3.FIQ == 0 Alexandru Elisei
2020-06-25 15:00   ` Alexandru Elisei
2020-06-25 15:00   ` Alexandru Elisei
2020-06-26  1:51   ` kernel test robot
2020-06-26  1:51     ` kernel test robot
2020-06-26  1:51     ` kernel test robot
2020-06-26  9:41     ` Alexandru Elisei
2020-06-26  9:41       ` Alexandru Elisei
2020-06-26  9:41       ` Alexandru Elisei
2020-06-26  9:41       ` Alexandru Elisei

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.