[PATCH 0/3] KVM: arm64: Fix handling of ID_PFR1_EL1.GIC

[PATCH 0/3] KVM: arm64: Fix handling of ID_PFR1_EL1.GIC

[Marc Zyngier]

Peter reported[1] that restoring a GICv2 VM fails badly, and correctly
points out that ID_PFR1_EL1.GIC isn't writable, while its 64bit
equivalent is. I broke that in 6.12.

The other thing is that fixing the ID regs at runtime isn't great.
specially when we could adjust them at the point where the GIC gets
created.

This small series aims at fixing these issues. I've only tagged the
first one as a stable candidate. With these fixes, I can happily
save/restore a GICv2 VM (both 32 and 64bit) on my trusty Synquacer.

[1] https://lore.kernel.org/r/CAFEAcA8TpQduexT=8rdRYC=yxm_073COjzgWJAvc26_T+-F5vA@mail.gmail.com

Marc Zyngier (3):
  KVM: arm64: Make ID_PFR1_EL1.GIC writable
  KVM: arm64: Set ID_{AA64PFR0,PFR1}_EL1.GIC when GICv3 is configured
  KVM: arm64: Limit clearing of ID_{AA64PFR0,PFR1}_EL1.GIC to userspace
    irqchip

 arch/arm64/kvm/sys_regs.c       | 12 ++++++++----
 arch/arm64/kvm/vgic/vgic-init.c | 10 ++++++++--
 2 files changed, 16 insertions(+), 6 deletions(-)

-- 
2.47.3

[PATCH 1/3] KVM: arm64: Make ID_PFR1_EL1.GIC writable

[Marc Zyngier]

Similarly to ID_AA64PFR0_EL1.GIC, relax ID_PFR1_EL1.GIC to be writable.

Fixes: 5cb57a1aff755 ("KVM: arm64: Zero ID_AA64PFR0_EL1.GIC when no GICv3 is presented to the guest")
Reported-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Marc Zyngier <maz@kernel.org>
Cc: stable@vger.kernel.org
---
 arch/arm64/kvm/sys_regs.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index b29f72478a50d..73dcefe51a3e7 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -2528,6 +2528,12 @@ static bool bad_redir_trap(struct kvm_vcpu *vcpu,
 	.val = mask,				\
 }
 
+#define AA32_ID_WRITABLE(name, mask) {		\
+	ID_DESC(name),				\
+	.visibility = aa32_id_visibility,	\
+	.val = mask,				\
+}
+
 /* sys_reg_desc initialiser for cpufeature ID registers that need filtering */
 #define ID_FILTERED(sysreg, name, mask) {	\
 	ID_DESC(sysreg),				\
@@ -3040,7 +3046,7 @@ static const struct sys_reg_desc sys_reg_descs[] = {
 	/* AArch64 mappings of the AArch32 ID registers */
 	/* CRm=1 */
 	AA32_ID_SANITISED(ID_PFR0_EL1),
-	AA32_ID_SANITISED(ID_PFR1_EL1),
+	AA32_ID_WRITABLE(ID_PFR1_EL1, ID_PFR1_EL1_GIC),
 	{ SYS_DESC(SYS_ID_DFR0_EL1),
 	  .access = access_id_reg,
 	  .get_user = get_id_reg,
-- 
2.47.3

Re: [PATCH 1/3] KVM: arm64: Make ID_PFR1_EL1.GIC writable

[Oliver Upton]

Hey,

On Mon, Oct 13, 2025 at 09:32:05AM +0100, Marc Zyngier wrote:
> Similarly to ID_AA64PFR0_EL1.GIC, relax ID_PFR1_EL1.GIC to be writable.

This looks fine to me, although I do wonder if we should just allow
userspace to write whatever value it wants to the 32-bit ID registers
and be done with it.

Nowhere do we use a 32-bit ID register value as a condition for trap
configuration / emulation, so even if the VMM lies to the guest it
shouldn't trip up KVM.

Thanks,
Oliver

Re: [PATCH 1/3] KVM: arm64: Make ID_PFR1_EL1.GIC writable

[Marc Zyngier]

On Wed, 22 Oct 2025 08:00:47 +0100,
Oliver Upton <oliver.upton@linux.dev> wrote:
> 
> Hey,
> 
> On Mon, Oct 13, 2025 at 09:32:05AM +0100, Marc Zyngier wrote:
> > Similarly to ID_AA64PFR0_EL1.GIC, relax ID_PFR1_EL1.GIC to be writable.
> 
> This looks fine to me, although I do wonder if we should just allow
> userspace to write whatever value it wants to the 32-bit ID registers
> and be done with it.

That's a good point. Nobody really cares about 32bit anyway, and I'd
be happy to just let the VMM write whatever it wants. Might be a bit
harder to backport, but whoever is interested in AArch32 will be able
to do it.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

[PATCH 2/3] KVM: arm64: Set ID_{AA64PFR0,PFR1}_EL1.GIC when GICv3 is configured

[Marc Zyngier]

Drive the idreg fields indicating the presence of GICv3 directly from
the vgic code. This avoids having to do any sort of runtime clearing
of the idreg.

Fixes: 5cb57a1aff755 ("KVM: arm64: Zero ID_AA64PFR0_EL1.GIC when no GICv3 is presented to the guest")
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 arch/arm64/kvm/vgic/vgic-init.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/kvm/vgic/vgic-init.c b/arch/arm64/kvm/vgic/vgic-init.c
index 4c3c0d82e4760..2c518b0a4d81b 100644
--- a/arch/arm64/kvm/vgic/vgic-init.c
+++ b/arch/arm64/kvm/vgic/vgic-init.c
@@ -161,10 +161,16 @@ int kvm_vgic_create(struct kvm *kvm, u32 type)
 
 	kvm->arch.vgic.vgic_dist_base = VGIC_ADDR_UNDEF;
 
-	if (type == KVM_DEV_TYPE_ARM_VGIC_V2)
+	*__vm_id_reg(&kvm->arch, SYS_ID_AA64PFR0_EL1) &= ~ID_AA64PFR0_EL1_GIC;
+	*__vm_id_reg(&kvm->arch, SYS_ID_PFR1_EL1) &= ~ID_PFR1_EL1_GIC;
+
+	if (type == KVM_DEV_TYPE_ARM_VGIC_V2) {
 		kvm->arch.vgic.vgic_cpu_base = VGIC_ADDR_UNDEF;
-	else
+	} else {
 		INIT_LIST_HEAD(&kvm->arch.vgic.rd_regions);
+		*__vm_id_reg(&kvm->arch, SYS_ID_AA64PFR0_EL1) |= SYS_FIELD_VALUE(ID_AA64PFR0_EL1, GIC, IMP);
+		*__vm_id_reg(&kvm->arch, SYS_ID_PFR1_EL1) |= SYS_FIELD_VALUE(ID_PFR1_EL1, GIC, GICv3);
+	}
 
 	if (type == KVM_DEV_TYPE_ARM_VGIC_V3)
 		kvm->arch.vgic.nassgicap = system_supports_direct_sgis();
-- 
2.47.3

Re: [PATCH 2/3] KVM: arm64: Set ID_{AA64PFR0,PFR1}_EL1.GIC when GICv3 is configured

[Oliver Upton]

On Mon, Oct 13, 2025 at 09:32:06AM +0100, Marc Zyngier wrote:
> Drive the idreg fields indicating the presence of GICv3 directly from
> the vgic code. This avoids having to do any sort of runtime clearing
> of the idreg.
> 
> Fixes: 5cb57a1aff755 ("KVM: arm64: Zero ID_AA64PFR0_EL1.GIC when no GICv3 is presented to the guest")
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
>  arch/arm64/kvm/vgic/vgic-init.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/kvm/vgic/vgic-init.c b/arch/arm64/kvm/vgic/vgic-init.c
> index 4c3c0d82e4760..2c518b0a4d81b 100644
> --- a/arch/arm64/kvm/vgic/vgic-init.c
> +++ b/arch/arm64/kvm/vgic/vgic-init.c
> @@ -161,10 +161,16 @@ int kvm_vgic_create(struct kvm *kvm, u32 type)
>  
>  	kvm->arch.vgic.vgic_dist_base = VGIC_ADDR_UNDEF;
>  
> -	if (type == KVM_DEV_TYPE_ARM_VGIC_V2)
> +	*__vm_id_reg(&kvm->arch, SYS_ID_AA64PFR0_EL1) &= ~ID_AA64PFR0_EL1_GIC;
> +	*__vm_id_reg(&kvm->arch, SYS_ID_PFR1_EL1) &= ~ID_PFR1_EL1_GIC;

I'd prefer this to be done as a kvm_read_vm_id_reg() / kvm_set_vm_id_reg(),
if only to ensure this flow hits the config_lock lockdep assertion like
other writers of the ID registers.

Thanks,
Oliver

[PATCH 3/3] KVM: arm64: Limit clearing of ID_{AA64PFR0,PFR1}_EL1.GIC to userspace irqchip

[Marc Zyngier]

Now that the idreg's GIC field is in sync with the irqchip, limit
the runtime clearing of these fields to the pathological case where
we do not have an in-kernel GIC.

Fixes: 5cb57a1aff755 ("KVM: arm64: Zero ID_AA64PFR0_EL1.GIC when no GICv3 is presented to the guest")
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 arch/arm64/kvm/sys_regs.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index 73dcefe51a3e7..25cfd0f9541f5 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -5494,9 +5494,7 @@ int kvm_finalize_sys_regs(struct kvm_vcpu *vcpu)
 
 	guard(mutex)(&kvm->arch.config_lock);
 
-	if (!(static_branch_unlikely(&kvm_vgic_global_state.gicv3_cpuif) &&
-	      irqchip_in_kernel(kvm) &&
-	      kvm->arch.vgic.vgic_model == KVM_DEV_TYPE_ARM_VGIC_V3)) {
+	if (!irqchip_in_kernel(kvm)) {
 		kvm->arch.id_regs[IDREG_IDX(SYS_ID_AA64PFR0_EL1)] &= ~ID_AA64PFR0_EL1_GIC_MASK;
 		kvm->arch.id_regs[IDREG_IDX(SYS_ID_PFR1_EL1)] &= ~ID_PFR1_EL1_GIC_MASK;
 	}
-- 
2.47.3

Re: [PATCH 3/3] KVM: arm64: Limit clearing of ID_{AA64PFR0,PFR1}_EL1.GIC to userspace irqchip

[Oliver Upton]

On Mon, Oct 13, 2025 at 09:32:07AM +0100, Marc Zyngier wrote:
> Now that the idreg's GIC field is in sync with the irqchip, limit
> the runtime clearing of these fields to the pathological case where
> we do not have an in-kernel GIC.
> 
> Fixes: 5cb57a1aff755 ("KVM: arm64: Zero ID_AA64PFR0_EL1.GIC when no GICv3 is presented to the guest")
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
>  arch/arm64/kvm/sys_regs.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index 73dcefe51a3e7..25cfd0f9541f5 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -5494,9 +5494,7 @@ int kvm_finalize_sys_regs(struct kvm_vcpu *vcpu)
>  
>  	guard(mutex)(&kvm->arch.config_lock);
>  
> -	if (!(static_branch_unlikely(&kvm_vgic_global_state.gicv3_cpuif) &&
> -	      irqchip_in_kernel(kvm) &&
> -	      kvm->arch.vgic.vgic_model == KVM_DEV_TYPE_ARM_VGIC_V3)) {
> +	if (!irqchip_in_kernel(kvm)) {
>  		kvm->arch.id_regs[IDREG_IDX(SYS_ID_AA64PFR0_EL1)] &= ~ID_AA64PFR0_EL1_GIC_MASK;
>  		kvm->arch.id_regs[IDREG_IDX(SYS_ID_PFR1_EL1)] &= ~ID_PFR1_EL1_GIC_MASK;

Pre-existing, but converting this to the accessors would be a good idea.

Thanks,
Oliver