Hi all,
After merging the kvm tree, today's linux-next build (arm64 defconfig)
failed like this:
In file included from <command-line>:
In function 'check_res_bits',
inlined from 'kvm_sys_reg_table_init' at arch/arm64/kvm/sys_regs.c:4109:2:
include/linux/compiler_types.h:449:45: error: call to '__compiletime_assert_591' declared with attribute error: BUILD_BUG_ON failed: ID_AA64DFR1_EL1_RES0 != (GENMASK_ULL(63, 0))
449 | _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
| ^
include/linux/compiler_types.h:430:25: note: in definition of macro '__compiletime_assert'
430 | prefix ## suffix(); \
| ^~~~~~
include/linux/compiler_types.h:449:9: note: in expansion of macro '_compiletime_assert'
449 | _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
| ^~~~~~~~~~~~~~~~~~~
include/linux/build_bug.h:39:37: note: in expansion of macro 'compiletime_assert'
39 | #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
| ^~~~~~~~~~~~~~~~~~
include/linux/build_bug.h:50:9: note: in expansion of macro 'BUILD_BUG_ON_MSG'
50 | BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " #condition)
| ^~~~~~~~~~~~~~~~
arch/arm64/kvm/check-res-bits.h:58:9: note: in expansion of macro 'BUILD_BUG_ON'
58 | BUILD_BUG_ON(ID_AA64DFR1_EL1_RES0 != (GENMASK_ULL(63, 0)));
| ^~~~~~~~~~~~
I bisected this to the merge of the kvm-arm tree into linux-next but I
could not figure out why it fails :-(
--
Cheers,
Stephen Rothwell
On Thu, Feb 22, 2024 at 10:03:49PM +1100, Stephen Rothwell wrote:
> Hi all,
>
> After merging the kvm tree, today's linux-next build (arm64 defconfig)
> failed like this:
>
> In file included from <command-line>:
> In function 'check_res_bits',
> inlined from 'kvm_sys_reg_table_init' at arch/arm64/kvm/sys_regs.c:4109:2:
> include/linux/compiler_types.h:449:45: error: call to '__compiletime_assert_591' declared with attribute error: BUILD_BUG_ON failed: ID_AA64DFR1_EL1_RES0 != (GENMASK_ULL(63, 0))
> 449 | _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
> | ^
> include/linux/compiler_types.h:430:25: note: in definition of macro '__compiletime_assert'
> 430 | prefix ## suffix(); \
> | ^~~~~~
> include/linux/compiler_types.h:449:9: note: in expansion of macro '_compiletime_assert'
> 449 | _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
> | ^~~~~~~~~~~~~~~~~~~
> include/linux/build_bug.h:39:37: note: in expansion of macro 'compiletime_assert'
> 39 | #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
> | ^~~~~~~~~~~~~~~~~~
> include/linux/build_bug.h:50:9: note: in expansion of macro 'BUILD_BUG_ON_MSG'
> 50 | BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " #condition)
> | ^~~~~~~~~~~~~~~~
> arch/arm64/kvm/check-res-bits.h:58:9: note: in expansion of macro 'BUILD_BUG_ON'
> 58 | BUILD_BUG_ON(ID_AA64DFR1_EL1_RES0 != (GENMASK_ULL(63, 0)));
> | ^~~~~~~~~~~~
>
> I bisected this to the merge of the kvm-arm tree into linux-next but I
> could not figure out why it fails :-(
>
> --
> Cheers,
> Stephen Rothwell
This fails because https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git/commit/?id=fdd867fe9b32
added new fields to that register (ID_AA64DFR1_EL1)
and commit b80b701d5a6 ("KVM: arm64: Snapshot all non-zero RES0/RES1 sysreg fields for later checking")
took a snapshot of the fields, so the RES0 (reserved 0) bits don't match anymore.
Not sure how to resolve it in the git branches though.
Thanks,
Joey
Hi Joey,
On Thu, 22 Feb 2024 11:11:29 +0000 Joey Gouly <[email protected]> wrote:
>
> On Thu, Feb 22, 2024 at 10:03:49PM +1100, Stephen Rothwell wrote:
> >
> > After merging the kvm tree, today's linux-next build (arm64 defconfig)
> > failed like this:
> >
> > In file included from <command-line>:
> > In function 'check_res_bits',
> > inlined from 'kvm_sys_reg_table_init' at arch/arm64/kvm/sys_regs.c:4109:2:
> > include/linux/compiler_types.h:449:45: error: call to '__compiletime_assert_591' declared with attribute error: BUILD_BUG_ON failed: ID_AA64DFR1_EL1_RES0 != (GENMASK_ULL(63, 0))
> > 449 | _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
> > | ^
> > include/linux/compiler_types.h:430:25: note: in definition of macro '__compiletime_assert'
> > 430 | prefix ## suffix(); \
> > | ^~~~~~
> > include/linux/compiler_types.h:449:9: note: in expansion of macro '_compiletime_assert'
> > 449 | _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
> > | ^~~~~~~~~~~~~~~~~~~
> > include/linux/build_bug.h:39:37: note: in expansion of macro 'compiletime_assert'
> > 39 | #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
> > | ^~~~~~~~~~~~~~~~~~
> > include/linux/build_bug.h:50:9: note: in expansion of macro 'BUILD_BUG_ON_MSG'
> > 50 | BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " #condition)
> > | ^~~~~~~~~~~~~~~~
> > arch/arm64/kvm/check-res-bits.h:58:9: note: in expansion of macro 'BUILD_BUG_ON'
> > 58 | BUILD_BUG_ON(ID_AA64DFR1_EL1_RES0 != (GENMASK_ULL(63, 0)));
> > | ^~~~~~~~~~~~
> >
> > I bisected this to the merge of the kvm-arm tree into linux-next but I
> > could not figure out why it fails :-(
> >
> > --
> > Cheers,
> > Stephen Rothwell
>
> This fails because https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git/commit/?id=fdd867fe9b32
> added new fields to that register (ID_AA64DFR1_EL1)
>
> and commit b80b701d5a6 ("KVM: arm64: Snapshot all non-zero RES0/RES1 sysreg fields for later checking")
> took a snapshot of the fields, so the RES0 (reserved 0) bits don't match anymore.
>
> Not sure how to resolve it in the git branches though.
Thanks. I will apply this patch to the merge of the kvm-arm tree from
tomorrow (and at the end of today's tree).
From: Stephen Rothwell <[email protected]>
Date: Thu, 22 Feb 2024 22:31:22 +1100
Subject: [PATCH] fix up for "arm64/sysreg: Add register fields for ID_AA64DFR1_EL1"
interacting with "KVM: arm64: Snapshot all non-zero RES0/RES1 sysreg fields for later checking"
Signed-off-by: Stephen Rothwell <[email protected]>
---
arch/arm64/kvm/check-res-bits.h | 1 -
1 file changed, 1 deletion(-)
diff --git a/arch/arm64/kvm/check-res-bits.h b/arch/arm64/kvm/check-res-bits.h
index 967b5d171d53..39f537875d17 100644
--- a/arch/arm64/kvm/check-res-bits.h
+++ b/arch/arm64/kvm/check-res-bits.h
@@ -55,7 +55,6 @@ static inline void check_res_bits(void)
BUILD_BUG_ON(ID_AA64SMFR0_EL1_RES0 != (GENMASK_ULL(62, 61) | GENMASK_ULL(51, 49) | GENMASK_ULL(31, 31) | GENMASK_ULL(27, 0)));
BUILD_BUG_ON(ID_AA64FPFR0_EL1_RES0 != (GENMASK_ULL(63, 32) | GENMASK_ULL(27, 2)));
BUILD_BUG_ON(ID_AA64DFR0_EL1_RES0 != (GENMASK_ULL(27, 24) | GENMASK_ULL(19, 16)));
- BUILD_BUG_ON(ID_AA64DFR1_EL1_RES0 != (GENMASK_ULL(63, 0)));
BUILD_BUG_ON(ID_AA64AFR0_EL1_RES0 != (GENMASK_ULL(63, 32)));
BUILD_BUG_ON(ID_AA64AFR1_EL1_RES0 != (GENMASK_ULL(63, 0)));
BUILD_BUG_ON(ID_AA64ISAR0_EL1_RES0 != (GENMASK_ULL(3, 0)));
--
2.43.0
--
Cheers,
Stephen Rothwell
On 2/22/24 12:40, Stephen Rothwell wrote:
>> This fails because https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git/commit/?id=fdd867fe9b32
>> added new fields to that register (ID_AA64DFR1_EL1)
>>
>> and commit b80b701d5a6 ("KVM: arm64: Snapshot all non-zero RES0/RES1 sysreg fields for later checking")
>> took a snapshot of the fields, so the RES0 (reserved 0) bits don't match anymore.
>>
>> Not sure how to resolve it in the git branches though.
>
> Thanks. I will apply this patch to the merge of the kvm-arm tree from
> tomorrow (and at the end of today's tree).
Marc, Iliver, can you get a topic branch from Catalin and friends for
this sysreg patch, and apply the fixup directly to the kvm-arm branch in
the merge commit?
Not _necessary_, as I can always ask Linus to do the fixup, but
generally he prefers to have this sorted out by the maintainers if it is
detected by linux-next.
Paolo
> From: Stephen Rothwell <[email protected]>
> Date: Thu, 22 Feb 2024 22:31:22 +1100
> Subject: [PATCH] fix up for "arm64/sysreg: Add register fields for ID_AA64DFR1_EL1"
>
> interacting with "KVM: arm64: Snapshot all non-zero RES0/RES1 sysreg fields for later checking"
>
> Signed-off-by: Stephen Rothwell <[email protected]>
> ---
> arch/arm64/kvm/check-res-bits.h | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/arch/arm64/kvm/check-res-bits.h b/arch/arm64/kvm/check-res-bits.h
> index 967b5d171d53..39f537875d17 100644
> --- a/arch/arm64/kvm/check-res-bits.h
> +++ b/arch/arm64/kvm/check-res-bits.h
> @@ -55,7 +55,6 @@ static inline void check_res_bits(void)
> BUILD_BUG_ON(ID_AA64SMFR0_EL1_RES0 != (GENMASK_ULL(62, 61) | GENMASK_ULL(51, 49) | GENMASK_ULL(31, 31) | GENMASK_ULL(27, 0)));
> BUILD_BUG_ON(ID_AA64FPFR0_EL1_RES0 != (GENMASK_ULL(63, 32) | GENMASK_ULL(27, 2)));
> BUILD_BUG_ON(ID_AA64DFR0_EL1_RES0 != (GENMASK_ULL(27, 24) | GENMASK_ULL(19, 16)));
> - BUILD_BUG_ON(ID_AA64DFR1_EL1_RES0 != (GENMASK_ULL(63, 0)));
> BUILD_BUG_ON(ID_AA64AFR0_EL1_RES0 != (GENMASK_ULL(63, 32)));
> BUILD_BUG_ON(ID_AA64AFR1_EL1_RES0 != (GENMASK_ULL(63, 0)));
> BUILD_BUG_ON(ID_AA64ISAR0_EL1_RES0 != (GENMASK_ULL(3, 0)));
On 2/22/24 12:40, Stephen Rothwell wrote:
>> This fails because https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git/commit/?id=fdd867fe9b32
>> added new fields to that register (ID_AA64DFR1_EL1)
>>
>> and commit b80b701d5a6 ("KVM: arm64: Snapshot all non-zero RES0/RES1 sysreg fields for later checking")
>> took a snapshot of the fields, so the RES0 (reserved 0) bits don't match anymore.
>>
>> Not sure how to resolve it in the git branches though.
>
> Thanks. I will apply this patch to the merge of the kvm-arm tree from
> tomorrow (and at the end of today's tree).
Marc, Oliver, can you get a topic branch from Catalin and friends for
this sysreg patch, and apply the fixup directly to the kvm-arm branch in
the merge commit?
Not _necessary_, as I can always ask Linus to do the fixup, but
generally he prefers to have this sorted out by the maintainers if it is
detected by linux-next.
Paolo
> From: Stephen Rothwell <[email protected]>
> Date: Thu, 22 Feb 2024 22:31:22 +1100
> Subject: [PATCH] fix up for "arm64/sysreg: Add register fields for ID_AA64DFR1_EL1"
>
> interacting with "KVM: arm64: Snapshot all non-zero RES0/RES1 sysreg fields for later checking"
>
> Signed-off-by: Stephen Rothwell <[email protected]>
> ---
> arch/arm64/kvm/check-res-bits.h | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/arch/arm64/kvm/check-res-bits.h b/arch/arm64/kvm/check-res-bits.h
> index 967b5d171d53..39f537875d17 100644
> --- a/arch/arm64/kvm/check-res-bits.h
> +++ b/arch/arm64/kvm/check-res-bits.h
> @@ -55,7 +55,6 @@ static inline void check_res_bits(void)
> BUILD_BUG_ON(ID_AA64SMFR0_EL1_RES0 != (GENMASK_ULL(62, 61) | GENMASK_ULL(51, 49) | GENMASK_ULL(31, 31) | GENMASK_ULL(27, 0)));
> BUILD_BUG_ON(ID_AA64FPFR0_EL1_RES0 != (GENMASK_ULL(63, 32) | GENMASK_ULL(27, 2)));
> BUILD_BUG_ON(ID_AA64DFR0_EL1_RES0 != (GENMASK_ULL(27, 24) | GENMASK_ULL(19, 16)));
> - BUILD_BUG_ON(ID_AA64DFR1_EL1_RES0 != (GENMASK_ULL(63, 0)));
> BUILD_BUG_ON(ID_AA64AFR0_EL1_RES0 != (GENMASK_ULL(63, 32)));
> BUILD_BUG_ON(ID_AA64AFR1_EL1_RES0 != (GENMASK_ULL(63, 0)));
> BUILD_BUG_ON(ID_AA64ISAR0_EL1_RES0 != (GENMASK_ULL(3, 0)));
On Thu, 22 Feb 2024 13:11:59 +0000,
Paolo Bonzini <[email protected]> wrote:
>
> On 2/22/24 12:40, Stephen Rothwell wrote:
> >> This fails because https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git/commit/?id=fdd867fe9b32
> >> added new fields to that register (ID_AA64DFR1_EL1)
> >>
> >> and commit b80b701d5a6 ("KVM: arm64: Snapshot all non-zero RES0/RES1 sysreg fields for later checking")
> >> took a snapshot of the fields, so the RES0 (reserved 0) bits don't match anymore.
> >>
> >> Not sure how to resolve it in the git branches though.
> >
> > Thanks. I will apply this patch to the merge of the kvm-arm tree from
> > tomorrow (and at the end of today's tree).
>
> Marc, Oliver, can you get a topic branch from Catalin and friends for
> this sysreg patch, and apply the fixup directly to the kvm-arm branch
> in the merge commit?
>
> Not _necessary_, as I can always ask Linus to do the fixup, but
> generally he prefers to have this sorted out by the maintainers if it
> is detected by linux-next.
I think that's not the correct thing to do at this time. I should have
timed the introduction of these checks a bit later, after the merge
window.
But more to the point, the proposed patch is also not the best thing
to merge, because it hides that there is a discrepancy between what
the architecture describes, and what KVM knows. I really want to know
about it, or it will be yet another bug that we wont detect easily.
Specially for ID_AA64DFR*_EL1 which are a bloody mine-field.
So I'd rather we make the check optional, and we'll play catch up for
a bit longer. Something like the patch below.
Oliver, do you mind queuing this ASAP (also pushed out to my dev
branch)?
Thanks,
M.
From 85d861a6ca055c7681c826c580e7c90d74c26ac5 Mon Sep 17 00:00:00 2001
From: Marc Zyngier <[email protected]>
Date: Thu, 22 Feb 2024 14:12:09 +0000
Subject: [PATCH] KVM: arm64: Make build-time check of RES0/RES1 bits optional
In order to ease the transition towards a state of absolute
paranoia where all RES0/RES1 bits gets checked against what
KVM know of them, make the checks optional and garded by a
config symbol (CONFIG_KVM_ARM64_RES_BITS_PARANOIA) default to n.
Signed-off-by: Marc Zyngier <[email protected]>
---
arch/arm64/kvm/Kconfig | 11 +++++++++++
arch/arm64/kvm/check-res-bits.h | 4 ++++
2 files changed, 15 insertions(+)
diff --git a/arch/arm64/kvm/Kconfig b/arch/arm64/kvm/Kconfig
index 5c2a672c06a8..fa9389270cfe 100644
--- a/arch/arm64/kvm/Kconfig
+++ b/arch/arm64/kvm/Kconfig
@@ -67,4 +67,15 @@ config PROTECTED_NVHE_STACKTRACE
If unsure, or not using protected nVHE (pKVM), say N.
+config KVM_ARM64_RES_BITS_PARANOIA
+ bool "Build-time check of RES0/RES1 bits"
+ depends on KVM
+ default n
+ help
+ Say Y here to validate that KVM's knowledge of most system
+ registers' RES0/RES1 bits matches when the rest of the kernel
+ defines. Expect the build to fail badly if you enable this.
+
+ Just say N.
+
endif # VIRTUALIZATION
diff --git a/arch/arm64/kvm/check-res-bits.h b/arch/arm64/kvm/check-res-bits.h
index 967b5d171d53..2d98e60efc3c 100644
--- a/arch/arm64/kvm/check-res-bits.h
+++ b/arch/arm64/kvm/check-res-bits.h
@@ -21,6 +21,8 @@
*/
static inline void check_res_bits(void)
{
+#ifdef CONFIG_KVM_ARM64_RES_BITS_PARANOIA
+
BUILD_BUG_ON(OSDTRRX_EL1_RES0 != (GENMASK_ULL(63, 32)));
BUILD_BUG_ON(MDCCINT_EL1_RES0 != (GENMASK_ULL(63, 31) | GENMASK_ULL(28, 0)));
BUILD_BUG_ON(MDSCR_EL1_RES0 != (GENMASK_ULL(63, 36) | GENMASK_ULL(28, 28) | GENMASK_ULL(25, 24) | GENMASK_ULL(20, 20) | GENMASK_ULL(18, 16) | GENMASK_ULL(11, 7) | GENMASK_ULL(5, 1)));
@@ -118,4 +120,6 @@ static inline void check_res_bits(void)
BUILD_BUG_ON(TRBMAR_EL1_RES0 != (GENMASK_ULL(63, 12)));
BUILD_BUG_ON(TRBTRG_EL1_RES0 != (GENMASK_ULL(63, 32)));
BUILD_BUG_ON(TRBIDR_EL1_RES0 != (GENMASK_ULL(63, 12) | GENMASK_ULL(7, 6)));
+
+#endif
}
--
2.39.2
--
Without deviation from the norm, progress is not possible.
On Thu, Feb 22, 2024 at 02:31:38PM +0000, Marc Zyngier wrote:
> On Thu, 22 Feb 2024 13:11:59 +0000,
> Paolo Bonzini <[email protected]> wrote:
> >
> > On 2/22/24 12:40, Stephen Rothwell wrote:
> > >> This fails because https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git/commit/?id=fdd867fe9b32
> > >> added new fields to that register (ID_AA64DFR1_EL1)
> > >>
> > >> and commit b80b701d5a6 ("KVM: arm64: Snapshot all non-zero RES0/RES1 sysreg fields for later checking")
> > >> took a snapshot of the fields, so the RES0 (reserved 0) bits don't match anymore.
> > >>
> > >> Not sure how to resolve it in the git branches though.
> > >
> > > Thanks. I will apply this patch to the merge of the kvm-arm tree from
> > > tomorrow (and at the end of today's tree).
> >
> > Marc, Oliver, can you get a topic branch from Catalin and friends for
> > this sysreg patch, and apply the fixup directly to the kvm-arm branch
> > in the merge commit?
> >
> > Not _necessary_, as I can always ask Linus to do the fixup, but
> > generally he prefers to have this sorted out by the maintainers if it
> > is detected by linux-next.
>
> I think that's not the correct thing to do at this time. I should have
> timed the introduction of these checks a bit later, after the merge
> window.
>
> But more to the point, the proposed patch is also not the best thing
> to merge, because it hides that there is a discrepancy between what
> the architecture describes, and what KVM knows. I really want to know
> about it, or it will be yet another bug that we wont detect easily.
> Specially for ID_AA64DFR*_EL1 which are a bloody mine-field.
>
> So I'd rather we make the check optional, and we'll play catch up for
> a bit longer. Something like the patch below.
>
> Oliver, do you mind queuing this ASAP (also pushed out to my dev
> branch)?
>
> Thanks,
>
> M.
>
> From 85d861a6ca055c7681c826c580e7c90d74c26ac5 Mon Sep 17 00:00:00 2001
> From: Marc Zyngier <[email protected]>
> Date: Thu, 22 Feb 2024 14:12:09 +0000
> Subject: [PATCH] KVM: arm64: Make build-time check of RES0/RES1 bits optional
>
> In order to ease the transition towards a state of absolute
> paranoia where all RES0/RES1 bits gets checked against what
> KVM know of them, make the checks optional and garded by a
> config symbol (CONFIG_KVM_ARM64_RES_BITS_PARANOIA) default to n.
>
> Signed-off-by: Marc Zyngier <[email protected]>
Applied as commit 99101dda29e3 ("KVM: arm64: Make build-time check of
RES0/RES1 bits optional") on the kvmarm/next branch.
--
Thanks,
Oliver