Re: [PATCH 2/2] irqchip/gic-v3: Support pseudo-NMIs when SCR_EL3.FIQ == 0
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
>
> 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 d
Re: [PATCH 2/2] irqchip/gic-v3: Support pseudo-NMIs when SCR_EL3.FIQ == 0
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 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 >
[PATCH 2/2] irqchip/gic-v3: Support pseudo-NMIs when SCR_EL3.FIQ == 0
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
---
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
