* [PATCH v3 1/3] KVM: arm64: Finalize guest-wide sysregs prior to per-vCPU sysregs
2026-09-01 18:18 [PATCH v3 0/3] KVM: arm64: ID register finalisation fixes Mark Brown
@ 2026-09-01 18:18 ` Mark Brown
2026-09-28 13:26 ` Lorenzo Stoakes (ARM)
2026-09-01 18:18 ` [PATCH v3 2/3] KVM: arm64: Block ID register changes after we rely on the values Mark Brown
` (2 subsequent siblings)
3 siblings, 1 reply; 11+ messages in thread
From: Mark Brown @ 2026-09-01 18:18 UTC (permalink / raw)
To: Marc Zyngier, Oliver Upton, Joey Gouly, Steffen Eiden,
Suzuki K Poulose, Zenghui Yu, Catalin Marinas, Will Deacon,
Fuad Tabba
Cc: Peter Maydell, linux-arm-kernel, kvmarm, linux-kernel,
Mark Brown, Fuad Tabba
In commit d82d09d5ba4b ("KVM: arm64: Don't skip per-vcpu NV
initialisation") the NV register sanitisation was moved earlier in
kvm_finalize_sys_regs() so that it runs for each vCPU rather than only
once per guest. This means that for the first vCPU it runs prior to vGIC
finalization, but the vGIC finalization updates the ID registers which
the NV initialization uses so we may end up with a mismatch. For
example, HFGRTR_EL2.ICC_IGRPENn_EL1 depends on GICv3 being enabled in
ID_AA64PFR0_EL1.GIC so may be mistakenly marked or not marked as RES0.
Split the initialization which runs once per guest into a separate
function and run that before the per-vCPU initialisation for NV,
renaming the per-vCPU function to make it clear that it does per-vCPU
setup.
Fixes: d82d09d5ba4b ("KVM: arm64: Don't skip per-vcpu NV initialisation")
Reviewed-by: Fuad Tabba <fuad.tabba@linux.dev>
Tested-by: Fuad Tabba <fuad.tabba@linux.dev>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
arch/arm64/kvm/arm.c | 2 +-
arch/arm64/kvm/sys_regs.c | 40 ++++++++++++++++++++++++++--------------
arch/arm64/kvm/sys_regs.h | 2 +-
3 files changed, 28 insertions(+), 16 deletions(-)
diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index 8b080804bc90..4f044280dec0 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -949,7 +949,7 @@ int kvm_arch_vcpu_run_pid_change(struct kvm_vcpu *vcpu)
return ret;
}
- ret = kvm_finalize_sys_regs(vcpu);
+ ret = kvm_vcpu_finalize_sys_regs(vcpu);
if (ret)
return ret;
diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index 44aae52c473d..880f84248427 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -5861,25 +5861,14 @@ void kvm_calculate_traps(struct kvm_vcpu *vcpu)
}
/*
- * Perform last adjustments to the ID registers that are implied by the
+ * Do system register finalization that is shared by the whole guest. This
+ * includes last adjustments to the ID registers that are implied by the
* configuration outside of the ID regs themselves, as well as any
* initialisation that directly depend on these ID registers (such as
* RES0/RES1 behaviours). This is not the place to configure traps though.
- *
- * Because this can be called once per CPU, changes must be idempotent.
*/
-int kvm_finalize_sys_regs(struct kvm_vcpu *vcpu)
+static int kvm_vm_finalize_sys_regs(struct kvm *kvm)
{
- struct kvm *kvm = vcpu->kvm;
-
- guard(mutex)(&kvm->arch.config_lock);
-
- if (vcpu_has_nv(vcpu)) {
- int ret = kvm_init_nv_sysregs(vcpu);
- if (ret)
- return ret;
- }
-
if (kvm_vm_has_ran_once(kvm))
return 0;
@@ -5931,6 +5920,29 @@ int kvm_finalize_sys_regs(struct kvm_vcpu *vcpu)
return 0;
}
+/*
+ * Because this can be called once per CPU, changes must be idempotent.
+ */
+int kvm_vcpu_finalize_sys_regs(struct kvm_vcpu *vcpu)
+{
+ struct kvm *kvm = vcpu->kvm;
+ int ret;
+
+ guard(mutex)(&kvm->arch.config_lock);
+
+ ret = kvm_vm_finalize_sys_regs(kvm);
+ if (ret)
+ return ret;
+
+ if (vcpu_has_nv(vcpu)) {
+ ret = kvm_init_nv_sysregs(vcpu);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+
int __init kvm_sys_reg_table_init(void)
{
const struct sys_reg_desc *gicv3_regs;
diff --git a/arch/arm64/kvm/sys_regs.h b/arch/arm64/kvm/sys_regs.h
index bd56a45abbf9..a3cccad2766f 100644
--- a/arch/arm64/kvm/sys_regs.h
+++ b/arch/arm64/kvm/sys_regs.h
@@ -254,7 +254,7 @@ int kvm_sys_reg_set_user(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg,
bool triage_sysreg_trap(struct kvm_vcpu *vcpu, int *sr_index);
-int kvm_finalize_sys_regs(struct kvm_vcpu *vcpu);
+int kvm_vcpu_finalize_sys_regs(struct kvm_vcpu *vcpu);
#define AA32(_x) .aarch32_map = AA32_##_x
#define Op0(_x) .Op0 = _x
--
2.47.3
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v3 1/3] KVM: arm64: Finalize guest-wide sysregs prior to per-vCPU sysregs
2026-09-01 18:18 ` [PATCH v3 1/3] KVM: arm64: Finalize guest-wide sysregs prior to per-vCPU sysregs Mark Brown
@ 2026-09-28 13:26 ` Lorenzo Stoakes (ARM)
0 siblings, 0 replies; 11+ messages in thread
From: Lorenzo Stoakes (ARM) @ 2026-09-28 13:26 UTC (permalink / raw)
To: Mark Brown
Cc: Marc Zyngier, Oliver Upton, Joey Gouly, Steffen Eiden,
Suzuki K Poulose, Zenghui Yu, Catalin Marinas, Will Deacon,
Fuad Tabba, Peter Maydell, linux-arm-kernel, kvmarm,
linux-kernel
On Tue, Sep 01, 2026 at 07:18:48PM +0100, Mark Brown wrote:
> In commit d82d09d5ba4b ("KVM: arm64: Don't skip per-vcpu NV
> initialisation") the NV register sanitisation was moved earlier in
> kvm_finalize_sys_regs() so that it runs for each vCPU rather than only
> once per guest. This means that for the first vCPU it runs prior to vGIC
> finalization, but the vGIC finalization updates the ID registers which
> the NV initialization uses so we may end up with a mismatch. For
> example, HFGRTR_EL2.ICC_IGRPENn_EL1 depends on GICv3 being enabled in
> ID_AA64PFR0_EL1.GIC so may be mistakenly marked or not marked as RES0.
Hand me some rope here, but I'm thinking this GIC case is because of:
kvm_init_nv_sysregs()
-> get_reg_fixed_bits(kvm, HFGRTR_EL2)
-> compute_reg_resx_bits()
-> compute_resx_bits()
[ does the dependency checks ]
?
Generally speaking it seems like a good idea that the broader system
registers are set up prior to nested in any case.
>
> Split the initialization which runs once per guest into a separate
> function and run that before the per-vCPU initialisation for NV,
> renaming the per-vCPU function to make it clear that it does per-vCPU
> setup.
>
> Fixes: d82d09d5ba4b ("KVM: arm64: Don't skip per-vcpu NV initialisation")
> Reviewed-by: Fuad Tabba <fuad.tabba@linux.dev>
> Tested-by: Fuad Tabba <fuad.tabba@linux.dev>
> Signed-off-by: Mark Brown <broonie@kernel.org>
One nit below but LGTM, so:
Reviewed-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
> ---
> arch/arm64/kvm/arm.c | 2 +-
> arch/arm64/kvm/sys_regs.c | 40 ++++++++++++++++++++++++++--------------
> arch/arm64/kvm/sys_regs.h | 2 +-
> 3 files changed, 28 insertions(+), 16 deletions(-)
>
> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> index 8b080804bc90..4f044280dec0 100644
> --- a/arch/arm64/kvm/arm.c
> +++ b/arch/arm64/kvm/arm.c
> @@ -949,7 +949,7 @@ int kvm_arch_vcpu_run_pid_change(struct kvm_vcpu *vcpu)
> return ret;
> }
>
> - ret = kvm_finalize_sys_regs(vcpu);
> + ret = kvm_vcpu_finalize_sys_regs(vcpu);
> if (ret)
> return ret;
>
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index 44aae52c473d..880f84248427 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -5861,25 +5861,14 @@ void kvm_calculate_traps(struct kvm_vcpu *vcpu)
> }
>
> /*
> - * Perform last adjustments to the ID registers that are implied by the
> + * Do system register finalization that is shared by the whole guest. This
> + * includes last adjustments to the ID registers that are implied by the
> * configuration outside of the ID regs themselves, as well as any
> * initialisation that directly depend on these ID registers (such as
> * RES0/RES1 behaviours). This is not the place to configure traps though.
> - *
> - * Because this can be called once per CPU, changes must be idempotent.
> */
> -int kvm_finalize_sys_regs(struct kvm_vcpu *vcpu)
> +static int kvm_vm_finalize_sys_regs(struct kvm *kvm)
> {
> - struct kvm *kvm = vcpu->kvm;
> -
> - guard(mutex)(&kvm->arch.config_lock);
NIT: Maybe worth a comment or an assert that the config_lock is held here?
> -
> - if (vcpu_has_nv(vcpu)) {
> - int ret = kvm_init_nv_sysregs(vcpu);
> - if (ret)
> - return ret;
> - }
> -
> if (kvm_vm_has_ran_once(kvm))
> return 0;
>
> @@ -5931,6 +5920,29 @@ int kvm_finalize_sys_regs(struct kvm_vcpu *vcpu)
> return 0;
> }
>
> +/*
> + * Because this can be called once per CPU, changes must be idempotent.
> + */
> +int kvm_vcpu_finalize_sys_regs(struct kvm_vcpu *vcpu)
> +{
> + struct kvm *kvm = vcpu->kvm;
> + int ret;
> +
> + guard(mutex)(&kvm->arch.config_lock);
> +
> + ret = kvm_vm_finalize_sys_regs(kvm);
> + if (ret)
> + return ret;
> +
> + if (vcpu_has_nv(vcpu)) {
> + ret = kvm_init_nv_sysregs(vcpu);
> + if (ret)
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> int __init kvm_sys_reg_table_init(void)
> {
> const struct sys_reg_desc *gicv3_regs;
> diff --git a/arch/arm64/kvm/sys_regs.h b/arch/arm64/kvm/sys_regs.h
> index bd56a45abbf9..a3cccad2766f 100644
> --- a/arch/arm64/kvm/sys_regs.h
> +++ b/arch/arm64/kvm/sys_regs.h
> @@ -254,7 +254,7 @@ int kvm_sys_reg_set_user(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg,
>
> bool triage_sysreg_trap(struct kvm_vcpu *vcpu, int *sr_index);
>
> -int kvm_finalize_sys_regs(struct kvm_vcpu *vcpu);
> +int kvm_vcpu_finalize_sys_regs(struct kvm_vcpu *vcpu);
>
> #define AA32(_x) .aarch32_map = AA32_##_x
> #define Op0(_x) .Op0 = _x
>
> --
> 2.47.3
>
>
--
Cheers, Lorenzo
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v3 2/3] KVM: arm64: Block ID register changes after we rely on the values
2026-09-01 18:18 [PATCH v3 0/3] KVM: arm64: ID register finalisation fixes Mark Brown
2026-09-01 18:18 ` [PATCH v3 1/3] KVM: arm64: Finalize guest-wide sysregs prior to per-vCPU sysregs Mark Brown
@ 2026-09-01 18:18 ` Mark Brown
2026-09-28 14:36 ` Lorenzo Stoakes (ARM)
2026-09-01 18:18 ` [PATCH v3 3/3] KVM: arm64: selftests: Check ID regs are immutable after a failed run Mark Brown
2026-09-28 14:52 ` [PATCH v3 0/3] KVM: arm64: ID register finalisation fixes Lorenzo Stoakes (ARM)
3 siblings, 1 reply; 11+ messages in thread
From: Mark Brown @ 2026-09-01 18:18 UTC (permalink / raw)
To: Marc Zyngier, Oliver Upton, Joey Gouly, Steffen Eiden,
Suzuki K Poulose, Zenghui Yu, Catalin Marinas, Will Deacon,
Fuad Tabba
Cc: Peter Maydell, linux-arm-kernel, kvmarm, linux-kernel,
Mark Brown, Fuad Tabba
In commit c5bac1ef7df6b ("KVM: arm64: Move existing feature disabling
over to FGU infrastructure") a check was added to suppress duplicate
recalculation of FGUs based on a flag KVM_ARCH_FLAG_FGU_INITIALIZED. This
flag is set when we complete kvm_calculate_traps(), which is called from
kvm_arch_vcpu_run_pid_change(). There are several points where that
function could fail after we have calculated FGUs (eg, due to an invalid
timer configuration). If this happens then userspace will still be able
to write to the ID registers, writes to which are gated on
KVM_ARCH_FLAG_HAS_RAN_ONCE being set. This in turn means that the FGU
configuration for a running guest may not match the ID register
configuration.
This will result in issues based on the hypervisor assuming a consistent
configuration, for example it allows the creation of guests which have
untrapped access to system registers which are not context switched for
the guest.
A similar issue exists in kvm_init_nv_sysregs() where once sysreg_masks
is allocated the RES0/RES1 masks for registers are fixed based on the ID
register values at the time the function ran, and also for copying the
implementation ID registers to the hypervisor for pKVM.
There is a further issue with vGIC setup, creating a vGIC includes
updating the ID registers to reflect the GIC configuration. We refuse
to create a vGIC after the first vCPU has run but if a vCPU fails its
first run we may already have finalized the ID register values.
Avoid these issues by adding a new flag that we set when we finalize the
system registers, blocking ID register changes after that has been set
even if something fails later on. Do this in kvm_vm_finalize_sys_regs(),
this is where we finalize the GIC fields in the ID registers and happens
before we do the FGU and RES0/1 setup. A VMM which tries to create an
irqchip after failing to run a vCPU will now get -EBUSY rather than a
likely misconfigured guest. Userspace is not expected to try to run a
guest that fails to start, never mind try to repair the guest
configuration after doing so, so this is not expected to have any impact
on practical users.
There is a preexisting flag KVM_ARCH_FLAG_ID_REGS_INITIALIZED, this was
added as part of the series that originally enabled writable ID
registers[1]. That is set when the vCPU feature flags are finalized in
KVM_ARM_VCPU_INIT when we initiailise the ID registers, we need to be
able to write to the ID registers after that point since the features
can influence ID registers (eg, ID_AA64ZFR0_EL1). Given this and the
fact that the flag was introduced as part of making the ID registers
writable it appears to be a deliberate and desired ABI design decision
to not use this flag to block writes to the ID registers. Introducing
the new flag preserves the existing behaviour.
[1] https://lore.kernel.org/r/20230609190054.1542113-7-oliver.upton@linux.dev
Fixes: c5bac1ef7df6b ("KVM: arm64: Move existing feature disabling over to FGU infrastructure")
Fixes: 888f088070229 ("KVM: arm64: nv: Add sanitising to VNCR-backed sysregs")
Fixes: 03e1b89d051f ("KVM: arm64: Copy MIDR_EL1 into hyp VM when it is writable")
Fixes: 8a9866ff8600 ("KVM: arm64: Set ID_{AA64PFR0,PFR1}_EL1.GIC when GICv3 is configured")
Reviewed-by: Fuad Tabba <fuad.tabba@linux.dev>
Tested-by: Fuad Tabba <fuad.tabba@linux.dev>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
arch/arm64/include/asm/kvm_host.h | 8 ++++++++
arch/arm64/kvm/sys_regs.c | 17 ++++++++++-------
arch/arm64/kvm/vgic/vgic-init.c | 6 ++----
3 files changed, 20 insertions(+), 11 deletions(-)
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 27fe0cd5b2d7..777c46b34bb5 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -367,6 +367,8 @@ struct kvm_arch {
#define KVM_ARCH_FLAG_WRITABLE_IMP_ID_REGS 10
/* Unhandled SEAs are taken to userspace */
#define KVM_ARCH_FLAG_EXIT_SEA 11
+ /* No further ID register changes possible */
+#define KVM_ARCH_FLAG_ID_REGS_FINAL 12
unsigned long flags;
/* VM-wide vCPU feature set */
@@ -1149,6 +1151,12 @@ struct kvm_vcpu_arch {
#define vcpu_has_ptrauth(vcpu) false
#endif
+#define kvm_id_regs_final(kvm) \
+ test_bit(KVM_ARCH_FLAG_ID_REGS_FINAL, &(kvm)->arch.flags)
+
+#define vcpu_id_regs_final(vcpu) \
+ kvm_id_regs_final((vcpu)->kvm)
+
#define vcpu_on_unsupported_cpu(vcpu) \
vcpu_get_flag(vcpu, ON_UNSUPPORTED_CPU)
diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index 880f84248427..df0c3831094d 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -2511,9 +2511,10 @@ static int set_id_reg(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
/*
* Once the VM has started the ID registers are immutable. Reject any
- * write that does not match the final register value.
+ * write that does not match the final register value once we have
+ * got far enough into first running the VM to use the values.
*/
- if (kvm_vm_has_ran_once(vcpu->kvm)) {
+ if (vcpu_id_regs_final(vcpu)) {
if (val != read_id_reg(vcpu, rd))
ret = -EBUSY;
else
@@ -2547,7 +2548,7 @@ void kvm_set_vm_id_reg(struct kvm *kvm, u32 reg, u64 val)
lockdep_assert_held(&kvm->arch.config_lock);
- if (KVM_BUG_ON(kvm_vm_has_ran_once(kvm) || !p, kvm))
+ if (KVM_BUG_ON(kvm_id_regs_final(kvm) || !p, kvm))
return;
*p = val;
@@ -3243,10 +3244,10 @@ static int set_imp_id_reg(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r,
return -EINVAL;
/*
- * Once the VM has started the ID registers are immutable. Reject the
- * write if userspace tries to change it.
+ * Once we have been far enough into starting the VM the ID registers
+ * are immutable. Reject the write if userspace tries to change it.
*/
- if (kvm_vm_has_ran_once(kvm))
+ if (kvm_id_regs_final(kvm))
return -EBUSY;
/*
@@ -5869,7 +5870,7 @@ void kvm_calculate_traps(struct kvm_vcpu *vcpu)
*/
static int kvm_vm_finalize_sys_regs(struct kvm *kvm)
{
- if (kvm_vm_has_ran_once(kvm))
+ if (kvm_id_regs_final(kvm))
return 0;
/*
@@ -5917,6 +5918,8 @@ static int kvm_vm_finalize_sys_regs(struct kvm *kvm)
kvm_vgic_finalize_idregs(kvm);
}
+ set_bit(KVM_ARCH_FLAG_ID_REGS_FINAL, &kvm->arch.flags);
+
return 0;
}
diff --git a/arch/arm64/kvm/vgic/vgic-init.c b/arch/arm64/kvm/vgic/vgic-init.c
index 4012df6002ea..247c211bd68b 100644
--- a/arch/arm64/kvm/vgic/vgic-init.c
+++ b/arch/arm64/kvm/vgic/vgic-init.c
@@ -123,10 +123,8 @@ int kvm_vgic_create(struct kvm *kvm, u32 type)
goto out_unlock;
}
- kvm_for_each_vcpu(i, vcpu, kvm) {
- if (vcpu_has_run_once(vcpu))
- goto out_unlock;
- }
+ if (kvm_id_regs_final(kvm))
+ goto out_unlock;
ret = 0;
if (type == KVM_DEV_TYPE_ARM_VGIC_V2)
--
2.47.3
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v3 2/3] KVM: arm64: Block ID register changes after we rely on the values
2026-09-01 18:18 ` [PATCH v3 2/3] KVM: arm64: Block ID register changes after we rely on the values Mark Brown
@ 2026-09-28 14:36 ` Lorenzo Stoakes (ARM)
2026-09-28 16:22 ` Mark Brown
0 siblings, 1 reply; 11+ messages in thread
From: Lorenzo Stoakes (ARM) @ 2026-09-28 14:36 UTC (permalink / raw)
To: Mark Brown
Cc: Marc Zyngier, Oliver Upton, Joey Gouly, Steffen Eiden,
Suzuki K Poulose, Zenghui Yu, Catalin Marinas, Will Deacon,
Fuad Tabba, Peter Maydell, linux-arm-kernel, kvmarm,
linux-kernel
On Tue, Sep 01, 2026 at 07:18:49PM +0100, Mark Brown wrote:
> In commit c5bac1ef7df6b ("KVM: arm64: Move existing feature disabling
> over to FGU infrastructure") a check was added to suppress duplicate
> recalculation of FGUs based on a flag KVM_ARCH_FLAG_FGU_INITIALIZED. This
Just for my understanding:
FGU = Fine-Grained UNDEF -> if feature not present really act as if it's not
present, don't just not advertise it.
And that's about ensuring RES0, RES1 handled correctly?
Also (just noting for myself here) - it's a bit in the arch.flags bitmask:
kvm_calculate_traps() ->
if (test_bit(KVM_ARCH_FLAG_FGU_INITIALIZED, &kvm->arch.flags))
goto out;
...
set_bit(KVM_ARCH_FLAG_FGU_INITIALIZED, &kvm->arch.flags);
> flag is set when we complete kvm_calculate_traps(), which is called from
> kvm_arch_vcpu_run_pid_change(). There are several points where that
> function could fail after we have calculated FGUs (eg, due to an invalid
> timer configuration). If this happens then userspace will still be able
> to write to the ID registers, writes to which are gated on
> KVM_ARCH_FLAG_HAS_RAN_ONCE being set. This in turn means that the FGU
OK so the issue is that the writes to registers are permitted while
KVM_ARCH_FLAG_HAS_RAN_ONCE is cleared, but there's a dependency:
reg config -> what features enabled -> what FGU sets RES0/1 for
So you can finalise FGU state on incorrect information.
And RES0/RES1 matters meaningfully for NV because VNCR backs registers
with... memory :) Whereas the actual sysregs will just do what they do.
> configuration for a running guest may not match the ID register
> configuration.
Ack yeah.
>
> This will result in issues based on the hypervisor assuming a consistent
> configuration, for example it allows the creation of guests which have
> untrapped access to system registers which are not context switched for
> the guest.
>
> A similar issue exists in kvm_init_nv_sysregs() where once sysreg_masks
> is allocated the RES0/RES1 masks for registers are fixed based on the ID
> register values at the time the function ran, and also for copying the
> implementation ID registers to the hypervisor for pKVM.
>
> There is a further issue with vGIC setup, creating a vGIC includes
> updating the ID registers to reflect the GIC configuration. We refuse
> to create a vGIC after the first vCPU has run but if a vCPU fails its
> first run we may already have finalized the ID register values.
It seems like all are similar to 1/3 in that there are dependencies that
are not correctly expressed atm.
>
> Avoid these issues by adding a new flag that we set when we finalize the
> system registers, blocking ID register changes after that has been set
> even if something fails later on. Do this in kvm_vm_finalize_sys_regs(),
> this is where we finalize the GIC fields in the ID registers and happens
> before we do the FGU and RES0/1 setup. A VMM which tries to create an
OK so resolve things by tracking when the regs _should_ actually be set up
by, rather than just gating on KVM_ARCH_FLAG_HAS_RAN_ONCE once.
> irqchip after failing to run a vCPU will now get -EBUSY rather than a
> likely misconfigured guest. Userspace is not expected to try to run a
> guest that fails to start, never mind try to repair the guest
> configuration after doing so, so this is not expected to have any impact
> on practical users.
Sounds reasonable.
>
> There is a preexisting flag KVM_ARCH_FLAG_ID_REGS_INITIALIZED, this was
> added as part of the series that originally enabled writable ID
> registers[1]. That is set when the vCPU feature flags are finalized in
> KVM_ARM_VCPU_INIT when we initiailise the ID registers, we need to be
> able to write to the ID registers after that point since the features
> can influence ID registers (eg, ID_AA64ZFR0_EL1). Given this and the
> fact that the flag was introduced as part of making the ID registers
> writable it appears to be a deliberate and desired ABI design decision
> to not use this flag to block writes to the ID registers. Introducing
> the new flag preserves the existing behaviour.
Also sounds reasonable.
>
> [1] https://lore.kernel.org/r/20230609190054.1542113-7-oliver.upton@linux.dev
>
> Fixes: c5bac1ef7df6b ("KVM: arm64: Move existing feature disabling over to FGU infrastructure")
> Fixes: 888f088070229 ("KVM: arm64: nv: Add sanitising to VNCR-backed sysregs")
> Fixes: 03e1b89d051f ("KVM: arm64: Copy MIDR_EL1 into hyp VM when it is writable")
> Fixes: 8a9866ff8600 ("KVM: arm64: Set ID_{AA64PFR0,PFR1}_EL1.GIC when GICv3 is configured")
> Reviewed-by: Fuad Tabba <fuad.tabba@linux.dev>
> Tested-by: Fuad Tabba <fuad.tabba@linux.dev>
> Signed-off-by: Mark Brown <broonie@kernel.org>
All LGTM so:
Reviewed-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
> ---
> arch/arm64/include/asm/kvm_host.h | 8 ++++++++
> arch/arm64/kvm/sys_regs.c | 17 ++++++++++-------
> arch/arm64/kvm/vgic/vgic-init.c | 6 ++----
> 3 files changed, 20 insertions(+), 11 deletions(-)
>
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index 27fe0cd5b2d7..777c46b34bb5 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -367,6 +367,8 @@ struct kvm_arch {
> #define KVM_ARCH_FLAG_WRITABLE_IMP_ID_REGS 10
> /* Unhandled SEAs are taken to userspace */
> #define KVM_ARCH_FLAG_EXIT_SEA 11
> + /* No further ID register changes possible */
> +#define KVM_ARCH_FLAG_ID_REGS_FINAL 12
> unsigned long flags;
>
> /* VM-wide vCPU feature set */
> @@ -1149,6 +1151,12 @@ struct kvm_vcpu_arch {
> #define vcpu_has_ptrauth(vcpu) false
> #endif
>
> +#define kvm_id_regs_final(kvm) \
> + test_bit(KVM_ARCH_FLAG_ID_REGS_FINAL, &(kvm)->arch.flags)
> +
> +#define vcpu_id_regs_final(vcpu) \
> + kvm_id_regs_final((vcpu)->kvm)
> +
> #define vcpu_on_unsupported_cpu(vcpu) \
> vcpu_get_flag(vcpu, ON_UNSUPPORTED_CPU)
>
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index 880f84248427..df0c3831094d 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -2511,9 +2511,10 @@ static int set_id_reg(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
>
> /*
> * Once the VM has started the ID registers are immutable. Reject any
> - * write that does not match the final register value.
> + * write that does not match the final register value once we have
> + * got far enough into first running the VM to use the values.
> */
> - if (kvm_vm_has_ran_once(vcpu->kvm)) {
> + if (vcpu_id_regs_final(vcpu)) {
> if (val != read_id_reg(vcpu, rd))
> ret = -EBUSY;
> else
> @@ -2547,7 +2548,7 @@ void kvm_set_vm_id_reg(struct kvm *kvm, u32 reg, u64 val)
>
> lockdep_assert_held(&kvm->arch.config_lock);
>
> - if (KVM_BUG_ON(kvm_vm_has_ran_once(kvm) || !p, kvm))
> + if (KVM_BUG_ON(kvm_id_regs_final(kvm) || !p, kvm))
> return;
>
> *p = val;
> @@ -3243,10 +3244,10 @@ static int set_imp_id_reg(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r,
> return -EINVAL;
>
> /*
> - * Once the VM has started the ID registers are immutable. Reject the
> - * write if userspace tries to change it.
> + * Once we have been far enough into starting the VM the ID registers
> + * are immutable. Reject the write if userspace tries to change it.
> */
> - if (kvm_vm_has_ran_once(kvm))
> + if (kvm_id_regs_final(kvm))
> return -EBUSY;
>
> /*
> @@ -5869,7 +5870,7 @@ void kvm_calculate_traps(struct kvm_vcpu *vcpu)
> */
> static int kvm_vm_finalize_sys_regs(struct kvm *kvm)
> {
> - if (kvm_vm_has_ran_once(kvm))
> + if (kvm_id_regs_final(kvm))
> return 0;
>
> /*
> @@ -5917,6 +5918,8 @@ static int kvm_vm_finalize_sys_regs(struct kvm *kvm)
> kvm_vgic_finalize_idregs(kvm);
> }
>
> + set_bit(KVM_ARCH_FLAG_ID_REGS_FINAL, &kvm->arch.flags);
> +
> return 0;
> }
>
> diff --git a/arch/arm64/kvm/vgic/vgic-init.c b/arch/arm64/kvm/vgic/vgic-init.c
> index 4012df6002ea..247c211bd68b 100644
> --- a/arch/arm64/kvm/vgic/vgic-init.c
> +++ b/arch/arm64/kvm/vgic/vgic-init.c
> @@ -123,10 +123,8 @@ int kvm_vgic_create(struct kvm *kvm, u32 type)
> goto out_unlock;
> }
>
> - kvm_for_each_vcpu(i, vcpu, kvm) {
> - if (vcpu_has_run_once(vcpu))
> - goto out_unlock;
> - }
> + if (kvm_id_regs_final(kvm))
> + goto out_unlock;
This is a nice improvement! :)
> ret = 0;
>
> if (type == KVM_DEV_TYPE_ARM_VGIC_V2)
>
> --
> 2.47.3
>
>
--
Cheers, Lorenzo
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v3 2/3] KVM: arm64: Block ID register changes after we rely on the values
2026-09-28 14:36 ` Lorenzo Stoakes (ARM)
@ 2026-09-28 16:22 ` Mark Brown
0 siblings, 0 replies; 11+ messages in thread
From: Mark Brown @ 2026-09-28 16:22 UTC (permalink / raw)
To: Lorenzo Stoakes (ARM)
Cc: Marc Zyngier, Oliver Upton, Joey Gouly, Steffen Eiden,
Suzuki K Poulose, Zenghui Yu, Catalin Marinas, Will Deacon,
Fuad Tabba, Peter Maydell, linux-arm-kernel, kvmarm,
linux-kernel
[-- Attachment #1: Type: text/plain, Size: 614 bytes --]
On Mon, Sep 28, 2026 at 03:36:36PM +0100, Lorenzo Stoakes (ARM) wrote:
> On Tue, Sep 01, 2026 at 07:18:49PM +0100, Mark Brown wrote:
> > In commit c5bac1ef7df6b ("KVM: arm64: Move existing feature disabling
> > over to FGU infrastructure") a check was added to suppress duplicate
> > recalculation of FGUs based on a flag KVM_ARCH_FLAG_FGU_INITIALIZED. This
> Just for my understanding:
> FGU = Fine-Grained UNDEF -> if feature not present really act as if it's not
> present, don't just not advertise it.
> And that's about ensuring RES0, RES1 handled correctly?
That and registers that are just fully UNDEF.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v3 3/3] KVM: arm64: selftests: Check ID regs are immutable after a failed run
2026-09-01 18:18 [PATCH v3 0/3] KVM: arm64: ID register finalisation fixes Mark Brown
2026-09-01 18:18 ` [PATCH v3 1/3] KVM: arm64: Finalize guest-wide sysregs prior to per-vCPU sysregs Mark Brown
2026-09-01 18:18 ` [PATCH v3 2/3] KVM: arm64: Block ID register changes after we rely on the values Mark Brown
@ 2026-09-01 18:18 ` Mark Brown
2026-09-28 14:46 ` Lorenzo Stoakes (ARM)
2026-09-28 14:52 ` [PATCH v3 0/3] KVM: arm64: ID register finalisation fixes Lorenzo Stoakes (ARM)
3 siblings, 1 reply; 11+ messages in thread
From: Mark Brown @ 2026-09-01 18:18 UTC (permalink / raw)
To: Marc Zyngier, Oliver Upton, Joey Gouly, Steffen Eiden,
Suzuki K Poulose, Zenghui Yu, Catalin Marinas, Will Deacon,
Fuad Tabba
Cc: Peter Maydell, linux-arm-kernel, kvmarm, linux-kernel,
Mark Brown, Fuad Tabba
From: Fuad Tabba <fuad.tabba@linux.dev>
Add a set_id_regs case covering ID register immutability when a vCPU's
first KVM_RUN fails after finalization but before
KVM_ARCH_FLAG_HAS_RAN_ONCE is set. The test provokes such a failure with
a PMUv3-enabled vCPU whose PMU is left uninitialized, then checks that
KVM_SET_ONE_REG on the feature and implementation ID registers, and
KVM_CREATE_DEVICE for a vGIC, are all rejected with -EBUSY.
Assisted-by: Antigravity:gemini-3.1-pro
Signed-off-by: Fuad Tabba <fuad.tabba@linux.dev>
Signed-off-by: Mark Brown <broonie@kernel.org>
Link: https://patch.msgid.link/20260805064740.3013538-1-fuad.tabba@linux.dev
---
tools/testing/selftests/kvm/arm64/set_id_regs.c | 106 +++++++++++++++++++++++-
1 file changed, 105 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/kvm/arm64/set_id_regs.c b/tools/testing/selftests/kvm/arm64/set_id_regs.c
index 7429a1055df5..10849d21c0dd 100644
--- a/tools/testing/selftests/kvm/arm64/set_id_regs.c
+++ b/tools/testing/selftests/kvm/arm64/set_id_regs.c
@@ -13,6 +13,7 @@
#include "kvm_util.h"
#include "processor.h"
#include "test_util.h"
+#include "vgic.h"
#include <linux/bitfield.h>
enum ftr_type {
@@ -803,6 +804,107 @@ static void test_reset_preserves_id_regs(struct kvm_vcpu *vcpu)
ksft_test_result_pass("%s\n", __func__);
}
+/*
+ * ID registers must stay immutable even when a vCPU's first KVM_RUN fails
+ * after finalization but before KVM_ARCH_FLAG_HAS_RAN_ONCE is set.
+ */
+static void test_idreg_frozen_after_failed_run(void)
+{
+ static const u32 imp_id_regs[] = {
+ SYS_MIDR_EL1,
+ SYS_REVIDR_EL1,
+ SYS_AIDR_EL1,
+ };
+ struct kvm_vcpu_init init;
+ struct kvm_vcpu *vcpu;
+ struct kvm_vm *vm;
+ int r;
+
+ if (!kvm_has_cap(KVM_CAP_ARM_PMU_V3)) {
+ ksft_print_msg("PMUv3 unsupported, cannot fail the first run\n");
+ ksft_test_result_skip("%s\n", __func__);
+ return;
+ }
+
+ /* Skip the default vGIC so the KVM_CREATE_DEVICE gate is reachable. */
+ test_disable_default_vgic();
+
+ vm = vm_create(1);
+ vm_enable_cap(vm, KVM_CAP_ARM_WRITABLE_IMP_ID_REGS, 0);
+ kvm_get_default_vcpu_target(vm, &init);
+ init.features[0] |= (1 << KVM_ARM_VCPU_PMU_V3);
+ vcpu = aarch64_vcpu_add(vm, 0, &init, guest_code);
+ kvm_arch_vm_finalize_vcpus(vm);
+
+ /*
+ * A PMUv3 vCPU left without PMU init is rejected by
+ * kvm_arm_pmu_v3_enable(), which runs after sysreg finalization.
+ */
+ r = _vcpu_run(vcpu);
+ TEST_ASSERT(r < 0 && errno == EINVAL,
+ "first KVM_RUN should fail post-finalization: r=%d errno=%d",
+ r, errno);
+
+ /*
+ * Feature ID registers: use values that would have been accepted before
+ * finalization, so that a rejection means the registers are final
+ * rather than the value being invalid.
+ */
+ for (int i = 0; i < ARRAY_SIZE(test_regs); i++) {
+ const struct reg_ftr_bits *ftr_bits = test_regs[i].ftr_bits;
+ u64 reg = KVM_ARM64_SYS_REG(test_regs[i].reg);
+ u64 val = vcpu_get_reg(vcpu, reg);
+
+ for (int j = 0; ftr_bits[j].type != FTR_END; j++) {
+ u64 ftr = (val & ftr_bits[j].mask) >> ftr_bits[j].shift;
+ u64 safe = get_safe_value(&ftr_bits[j], ftr);
+ u64 new_val;
+
+ if (safe == ftr)
+ continue;
+
+ new_val = (val & ~ftr_bits[j].mask) |
+ (safe << ftr_bits[j].shift);
+
+ r = __vcpu_set_reg(vcpu, reg, new_val);
+ TEST_ASSERT(r < 0 && errno == EBUSY,
+ "%s write after failed first run: r=%d errno=%d",
+ ftr_bits[j].name, r, errno);
+ TEST_ASSERT_EQ(vcpu_get_reg(vcpu, reg), val);
+ }
+
+ /* A write matching the finalized value is still accepted. */
+ vcpu_set_reg(vcpu, reg, val);
+ }
+
+ /*
+ * The VM-wide implementation ID registers are gated separately. Bit 0
+ * is within the writable mask of all three, so flipping it is a change
+ * KVM would otherwise accept.
+ */
+ for (int i = 0; i < ARRAY_SIZE(imp_id_regs); i++) {
+ u64 reg = KVM_ARM64_SYS_REG(imp_id_regs[i]);
+ u64 val = vcpu_get_reg(vcpu, reg);
+
+ r = __vcpu_set_reg(vcpu, reg, val ^ 1);
+ TEST_ASSERT(r < 0 && errno == EBUSY,
+ "implementation ID reg write after failed first run: r=%d errno=%d",
+ r, errno);
+ TEST_ASSERT_EQ(vcpu_get_reg(vcpu, reg), val);
+ }
+
+ /* Creating an in-kernel irqchip would change the ID registers too. */
+ if (kvm_supports_vgic_v3()) {
+ r = __kvm_create_device(vm, KVM_DEV_TYPE_ARM_VGIC_V3);
+ TEST_ASSERT(r < 0 && errno == EBUSY,
+ "vGIC creation after failed first run: r=%d errno=%d",
+ r, errno);
+ }
+
+ kvm_vm_free(vm);
+ ksft_test_result_pass("%s\n", __func__);
+}
+
int main(void)
{
struct kvm_vcpu *vcpu;
@@ -828,7 +930,7 @@ int main(void)
ksft_print_header();
- test_cnt = 3 + MPAM_IDREG_TEST + MTE_IDREG_TEST;
+ test_cnt = 4 + MPAM_IDREG_TEST + MTE_IDREG_TEST;
for (i = 0; i < ARRAY_SIZE(test_regs); i++)
for (j = 0; test_regs[i].ftr_bits[j].type != FTR_END; j++)
test_cnt++;
@@ -847,5 +949,7 @@ int main(void)
kvm_vm_free(vm);
+ test_idreg_frozen_after_failed_run();
+
ksft_finished();
}
--
2.47.3
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v3 3/3] KVM: arm64: selftests: Check ID regs are immutable after a failed run
2026-09-01 18:18 ` [PATCH v3 3/3] KVM: arm64: selftests: Check ID regs are immutable after a failed run Mark Brown
@ 2026-09-28 14:46 ` Lorenzo Stoakes (ARM)
2026-09-28 16:31 ` Mark Brown
2026-09-28 16:56 ` Fuad Tabba
0 siblings, 2 replies; 11+ messages in thread
From: Lorenzo Stoakes (ARM) @ 2026-09-28 14:46 UTC (permalink / raw)
To: Mark Brown
Cc: Marc Zyngier, Oliver Upton, Joey Gouly, Steffen Eiden,
Suzuki K Poulose, Zenghui Yu, Catalin Marinas, Will Deacon,
Fuad Tabba, Peter Maydell, linux-arm-kernel, kvmarm,
linux-kernel
On Tue, Sep 01, 2026 at 07:18:50PM +0100, Mark Brown wrote:
> From: Fuad Tabba <fuad.tabba@linux.dev>
>
> Add a set_id_regs case covering ID register immutability when a vCPU's
> first KVM_RUN fails after finalization but before
> KVM_ARCH_FLAG_HAS_RAN_ONCE is set. The test provokes such a failure with
> a PMUv3-enabled vCPU whose PMU is left uninitialized, then checks that
> KVM_SET_ONE_REG on the feature and implementation ID registers, and
> KVM_CREATE_DEVICE for a vGIC, are all rejected with -EBUSY.
>
> Assisted-by: Antigravity:gemini-3.1-pro
NIT: we changed the rules for the Assisted-by tag, it only needs to be
'Assisted-by: LLM' now. Not a huge big deal but FYI :>)
https://docs.kernel.org/process/coding-assistants.html
> Signed-off-by: Fuad Tabba <fuad.tabba@linux.dev>
> Signed-off-by: Mark Brown <broonie@kernel.org>
> Link: https://patch.msgid.link/20260805064740.3013538-1-fuad.tabba@linux.dev
LGTM so:
Acked-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
> ---
> tools/testing/selftests/kvm/arm64/set_id_regs.c | 106 +++++++++++++++++++++++-
> 1 file changed, 105 insertions(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/kvm/arm64/set_id_regs.c b/tools/testing/selftests/kvm/arm64/set_id_regs.c
> index 7429a1055df5..10849d21c0dd 100644
> --- a/tools/testing/selftests/kvm/arm64/set_id_regs.c
> +++ b/tools/testing/selftests/kvm/arm64/set_id_regs.c
> @@ -13,6 +13,7 @@
> #include "kvm_util.h"
> #include "processor.h"
> #include "test_util.h"
> +#include "vgic.h"
> #include <linux/bitfield.h>
>
> enum ftr_type {
> @@ -803,6 +804,107 @@ static void test_reset_preserves_id_regs(struct kvm_vcpu *vcpu)
> ksft_test_result_pass("%s\n", __func__);
> }
>
> +/*
> + * ID registers must stay immutable even when a vCPU's first KVM_RUN fails
> + * after finalization but before KVM_ARCH_FLAG_HAS_RAN_ONCE is set.
> + */
> +static void test_idreg_frozen_after_failed_run(void)
> +{
> + static const u32 imp_id_regs[] = {
> + SYS_MIDR_EL1,
> + SYS_REVIDR_EL1,
> + SYS_AIDR_EL1,
> + };
> + struct kvm_vcpu_init init;
> + struct kvm_vcpu *vcpu;
> + struct kvm_vm *vm;
> + int r;
> +
> + if (!kvm_has_cap(KVM_CAP_ARM_PMU_V3)) {
> + ksft_print_msg("PMUv3 unsupported, cannot fail the first run\n");
> + ksft_test_result_skip("%s\n", __func__);
> + return;
> + }
> +
> + /* Skip the default vGIC so the KVM_CREATE_DEVICE gate is reachable. */
> + test_disable_default_vgic();
> +
> + vm = vm_create(1);
> + vm_enable_cap(vm, KVM_CAP_ARM_WRITABLE_IMP_ID_REGS, 0);
> + kvm_get_default_vcpu_target(vm, &init);
> + init.features[0] |= (1 << KVM_ARM_VCPU_PMU_V3);
> + vcpu = aarch64_vcpu_add(vm, 0, &init, guest_code);
> + kvm_arch_vm_finalize_vcpus(vm);
> +
> + /*
> + * A PMUv3 vCPU left without PMU init is rejected by
> + * kvm_arm_pmu_v3_enable(), which runs after sysreg finalization.
> + */
> + r = _vcpu_run(vcpu);
> + TEST_ASSERT(r < 0 && errno == EINVAL,
> + "first KVM_RUN should fail post-finalization: r=%d errno=%d",
> + r, errno);
Ah nice determinate way of triggering the issue.
> +
> + /*
> + * Feature ID registers: use values that would have been accepted before
> + * finalization, so that a rejection means the registers are final
> + * rather than the value being invalid.
> + */
> + for (int i = 0; i < ARRAY_SIZE(test_regs); i++) {
> + const struct reg_ftr_bits *ftr_bits = test_regs[i].ftr_bits;
> + u64 reg = KVM_ARM64_SYS_REG(test_regs[i].reg);
> + u64 val = vcpu_get_reg(vcpu, reg);
> +
> + for (int j = 0; ftr_bits[j].type != FTR_END; j++) {
> + u64 ftr = (val & ftr_bits[j].mask) >> ftr_bits[j].shift;
> + u64 safe = get_safe_value(&ftr_bits[j], ftr);
> + u64 new_val;
> +
> + if (safe == ftr)
> + continue;
> +
> + new_val = (val & ~ftr_bits[j].mask) |
> + (safe << ftr_bits[j].shift);
> +
> + r = __vcpu_set_reg(vcpu, reg, new_val);
> + TEST_ASSERT(r < 0 && errno == EBUSY,
> + "%s write after failed first run: r=%d errno=%d",
> + ftr_bits[j].name, r, errno);
> + TEST_ASSERT_EQ(vcpu_get_reg(vcpu, reg), val);
> + }
> +
> + /* A write matching the finalized value is still accepted. */
> + vcpu_set_reg(vcpu, reg, val);
> + }
> +
> + /*
> + * The VM-wide implementation ID registers are gated separately. Bit 0
> + * is within the writable mask of all three, so flipping it is a change
> + * KVM would otherwise accept.
> + */
Nice.
> + for (int i = 0; i < ARRAY_SIZE(imp_id_regs); i++) {
> + u64 reg = KVM_ARM64_SYS_REG(imp_id_regs[i]);
> + u64 val = vcpu_get_reg(vcpu, reg);
> +
> + r = __vcpu_set_reg(vcpu, reg, val ^ 1);
Ah literally a flip.
> + TEST_ASSERT(r < 0 && errno == EBUSY,
> + "implementation ID reg write after failed first run: r=%d errno=%d",
> + r, errno);
> + TEST_ASSERT_EQ(vcpu_get_reg(vcpu, reg), val);
> + }
> +
> + /* Creating an in-kernel irqchip would change the ID registers too. */
> + if (kvm_supports_vgic_v3()) {
> + r = __kvm_create_device(vm, KVM_DEV_TYPE_ARM_VGIC_V3);
> + TEST_ASSERT(r < 0 && errno == EBUSY,
> + "vGIC creation after failed first run: r=%d errno=%d",
> + r, errno);
I guess in general it's making sure - where there should now be finalisation on
reg setting that this is actually enforced?
> + }
> +
> + kvm_vm_free(vm);
> + ksft_test_result_pass("%s\n", __func__);
> +}
> +
> int main(void)
> {
> struct kvm_vcpu *vcpu;
> @@ -828,7 +930,7 @@ int main(void)
>
> ksft_print_header();
>
> - test_cnt = 3 + MPAM_IDREG_TEST + MTE_IDREG_TEST;
> + test_cnt = 4 + MPAM_IDREG_TEST + MTE_IDREG_TEST;
> for (i = 0; i < ARRAY_SIZE(test_regs); i++)
> for (j = 0; test_regs[i].ftr_bits[j].type != FTR_END; j++)
> test_cnt++;
> @@ -847,5 +949,7 @@ int main(void)
>
> kvm_vm_free(vm);
>
> + test_idreg_frozen_after_failed_run();
> +
> ksft_finished();
> }
>
> --
> 2.47.3
>
>
--
Cheers, Lorenzo
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v3 3/3] KVM: arm64: selftests: Check ID regs are immutable after a failed run
2026-09-28 14:46 ` Lorenzo Stoakes (ARM)
@ 2026-09-28 16:31 ` Mark Brown
2026-09-28 16:56 ` Fuad Tabba
1 sibling, 0 replies; 11+ messages in thread
From: Mark Brown @ 2026-09-28 16:31 UTC (permalink / raw)
To: Lorenzo Stoakes (ARM)
Cc: Marc Zyngier, Oliver Upton, Joey Gouly, Steffen Eiden,
Suzuki K Poulose, Zenghui Yu, Catalin Marinas, Will Deacon,
Fuad Tabba, Peter Maydell, linux-arm-kernel, kvmarm,
linux-kernel
[-- Attachment #1: Type: text/plain, Size: 482 bytes --]
On Mon, Sep 28, 2026 at 03:46:51PM +0100, Lorenzo Stoakes (ARM) wrote:
> On Tue, Sep 01, 2026 at 07:18:50PM +0100, Mark Brown wrote:
> > From: Fuad Tabba <fuad.tabba@linux.dev>
> > Assisted-by: Antigravity:gemini-3.1-pro
> NIT: we changed the rules for the Assisted-by tag, it only needs to be
> 'Assisted-by: LLM' now. Not a huge big deal but FYI :>)
> https://docs.kernel.org/process/coding-assistants.html
That's Fuad's tag not mine, I've just picked up his change verbatim.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 3/3] KVM: arm64: selftests: Check ID regs are immutable after a failed run
2026-09-28 14:46 ` Lorenzo Stoakes (ARM)
2026-09-28 16:31 ` Mark Brown
@ 2026-09-28 16:56 ` Fuad Tabba
1 sibling, 0 replies; 11+ messages in thread
From: Fuad Tabba @ 2026-09-28 16:56 UTC (permalink / raw)
To: Lorenzo Stoakes (ARM)
Cc: Mark Brown, Marc Zyngier, Oliver Upton, Joey Gouly,
Steffen Eiden, Suzuki K Poulose, Zenghui Yu, Catalin Marinas,
Will Deacon, Peter Maydell, linux-arm-kernel, kvmarm,
linux-kernel
Hi Lorenzo,
On Mon, 28 Sep 2026 15:46:51 +0100, "Lorenzo Stoakes (ARM)"
<ljs@kernel.org> wrote:
> On Tue, Sep 01, 2026 at 07:18:50PM +0100, Mark Brown wrote:
[...]
> > Assisted-by: Antigravity:gemini-3.1-pro
>
> NIT: we changed the rules for the Assisted-by tag, it only needs to be
> 'Assisted-by: LLM' now. Not a huge big deal but FYI :>)
>
> https://docs.kernel.org/process/coding-assistants.html
That tag dates from way back when the format was still
AGENT_NAME:MODEL_VERSION :)
[...]
> > diff --git a/tools/testing/selftests/kvm/arm64/set_id_regs.c b/tools/testing/selftests/kvm/arm64/set_id_regs.c
[...]
> > +static void test_idreg_frozen_after_failed_run(void)
[...]
> > + /* Creating an in-kernel irqchip would change the ID registers too. */
> > + if (kvm_supports_vgic_v3()) {
> > + r = __kvm_create_device(vm, KVM_DEV_TYPE_ARM_VGIC_V3);
> > + TEST_ASSERT(r < 0 && errno == EBUSY,
> > + "vGIC creation after failed first run: r=%d errno=%d",
> > + r, errno);
>
> I guess in general it's making sure - where there should now be finalisation on
> reg setting that this is actually enforced?
Yes. The test exercises each of the userspace-facing checks that 2/3
moves to KVM_ARCH_FLAG_ID_REGS_FINAL: set_id_reg(), set_imp_id_reg()
and kvm_vgic_create().
Thanks,
/fuad
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 0/3] KVM: arm64: ID register finalisation fixes
2026-09-01 18:18 [PATCH v3 0/3] KVM: arm64: ID register finalisation fixes Mark Brown
` (2 preceding siblings ...)
2026-09-01 18:18 ` [PATCH v3 3/3] KVM: arm64: selftests: Check ID regs are immutable after a failed run Mark Brown
@ 2026-09-28 14:52 ` Lorenzo Stoakes (ARM)
3 siblings, 0 replies; 11+ messages in thread
From: Lorenzo Stoakes (ARM) @ 2026-09-28 14:52 UTC (permalink / raw)
To: Mark Brown
Cc: Marc Zyngier, Oliver Upton, Joey Gouly, Steffen Eiden,
Suzuki K Poulose, Zenghui Yu, Catalin Marinas, Will Deacon,
Fuad Tabba, Peter Maydell, linux-arm-kernel, kvmarm,
linux-kernel
On Tue, Sep 01, 2026 at 07:18:47PM +0100, Mark Brown wrote:
> While looking at some feature enablement I noticed that there are some
> scenarios where we can end up with an inconsistently configured vCPU due
> to finalizing configuration based on ID registers before we have blocked
> writes to the ID registers. This series aims to clean up these issues.
>
> Signed-off-by: Mark Brown <broonie@kernel.org>
Ran the 3/3 test locally and confirmed the test fails without 2/3, passes
with it.
Also got our AI overloads to test 1/3 locally by flipping
ID_AA64PFR0_EL1.GIC after vGIC creation:
Before:
GIC field after vGIC create: 1
GIC field after userspace write: 0
GIC field after first run: 1
HFGRTR_EL2 written 0000008000000000 read back 0000000000000000
RESULT: FAIL - ICC_IGRPENn_EL1 forced RES0 (NV masks computed with GIC=0)
After:
GIC field after vGIC create: 1
GIC field after userspace write: 0
GIC field after first run: 1
HFGRTR_EL2 written 0000008000000000 read back 0000008000000000
RESULT: PASS - ICC_IGRPENn_EL1 writable (NV masks match GIC=IMP)
So for the whole series:
Tested-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
> ---
> Changes in v3:
> - Rebase onto v7.3-rc1.
> - Explain why KVM_ARCH_FLAG_ID_REGS_INITIALIZED is not reused.
> - Pull in Fuad's test cases.
> - Link to v2: https://patch.msgid.link/20260803-kvm-arm64-idreg-final-v2-0-d7d7e4efc640@kernel.org
>
> Changes in v2:
> - Also replace the kvm_vm_has_ran_once() usage in kvm_set_vm_id_rg() and
> kvm_vm_finalize_sys_regs() with kvm_id_regs_final().
> - Link to v1: https://patch.msgid.link/20260731-kvm-arm64-idreg-final-v1-0-3de2a5616dc6@kernel.org
>
> ---
> Fuad Tabba (1):
> KVM: arm64: selftests: Check ID regs are immutable after a failed run
>
> Mark Brown (2):
> KVM: arm64: Finalize guest-wide sysregs prior to per-vCPU sysregs
> KVM: arm64: Block ID register changes after we rely on the values
>
> arch/arm64/include/asm/kvm_host.h | 8 ++
> arch/arm64/kvm/arm.c | 2 +-
> arch/arm64/kvm/sys_regs.c | 57 ++++++++-----
> arch/arm64/kvm/sys_regs.h | 2 +-
> arch/arm64/kvm/vgic/vgic-init.c | 6 +-
> tools/testing/selftests/kvm/arm64/set_id_regs.c | 106 +++++++++++++++++++++++-
> 6 files changed, 153 insertions(+), 28 deletions(-)
> ---
> base-commit: cee9395acd8043be0644b25c34bfa86623f2b935
> change-id: 20260729-kvm-arm64-idreg-final-77cbc46b558a
>
> Best regards,
> --
> Mark Brown <broonie@kernel.org>
>
>
--
Cheers, Lorenzo
^ permalink raw reply [flat|nested] 11+ messages in thread