[PATCH v4 0/6] KVM: arm64: Allow userspace to write GICD_TYPER2.nASSGIcap

[PATCH v4 0/6] KVM: arm64: Allow userspace to write GICD_TYPER2.nASSGIcap

[Oliver Upton]

Raghu -- sorry for stealing your series again. We've got another feature
under discussion that needs to allow writes to VGIC ID registers, I'd
like to have this in ASAP as a template.

This is hopefully the final take on this series, now modeled around
the GICD_TYPER2 register itself, allowing userspace to change the value
of it prior to initializing the VGIC.

Marc, looks like we do not allow GICD_IIDR to be written prior to init
after all. I've relaxed that register to have the same pre-init behavior
while still allowing it to be written post-init for ABI's sake.

Plan is for this to go in 6.17.

Oliver Upton (4):
  KVM: arm64: Disambiguate support for vSGIs v. vLPIs
  KVM: arm64: vgic-v3: Consolidate MAINT_IRQ handling
  KVM: arm64: vgic-v3: Allow access to GICD_IIDR prior to initialization
  Documentation: KVM: arm64: Describe VGICv3 registers writable pre-init

Raghavendra Rao Ananta (2):
  KVM: arm64: vgic-v3: Allow userspace to write GICD_TYPER2.nASSGIcap
  KVM: arm64: selftests: Add test for nASSGIcap attribute

 .../virt/kvm/devices/arm-vgic-v3.rst          | 17 +++++
 arch/arm64/kvm/vgic/vgic-init.c               | 16 ++---
 arch/arm64/kvm/vgic/vgic-kvm-device.c         | 70 ++++++++++++-------
 arch/arm64/kvm/vgic/vgic-mmio-v3.c            | 33 +++++++--
 arch/arm64/kvm/vgic/vgic-v4.c                 |  4 +-
 arch/arm64/kvm/vgic/vgic.c                    |  4 +-
 arch/arm64/kvm/vgic/vgic.h                    |  8 +++
 include/kvm/arm_vgic.h                        |  3 +
 tools/testing/selftests/kvm/arm64/vgic_init.c | 42 ++++++++++-
 9 files changed, 148 insertions(+), 49 deletions(-)


base-commit: 86731a2a651e58953fc949573895f2fa6d456841
-- 
2.39.5

[PATCH v4 1/6] KVM: arm64: Disambiguate support for vSGIs v. vLPIs

[Oliver Upton]

vgic_supports_direct_msis() is a bit of a misnomer, as it returns true
if either vSGIs or vLPIs are supported. Pick it apart into a few
predicates and replace some open-coded checks for vSGIs.

Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
---
 arch/arm64/kvm/vgic/vgic-init.c    |  4 ++--
 arch/arm64/kvm/vgic/vgic-mmio-v3.c | 16 ++++++++++------
 arch/arm64/kvm/vgic/vgic-v4.c      |  4 ++--
 arch/arm64/kvm/vgic/vgic.c         |  4 ++--
 arch/arm64/kvm/vgic/vgic.h         |  7 +++++++
 5 files changed, 23 insertions(+), 12 deletions(-)

diff --git a/arch/arm64/kvm/vgic/vgic-init.c b/arch/arm64/kvm/vgic/vgic-init.c
index eb1205654ac8..5e0e4559004b 100644
--- a/arch/arm64/kvm/vgic/vgic-init.c
+++ b/arch/arm64/kvm/vgic/vgic-init.c
@@ -395,7 +395,7 @@ int vgic_init(struct kvm *kvm)
 	 * v4 support so that we get HW-accelerated vSGIs. Otherwise, only
 	 * enable it if we present a virtual ITS to the guest.
 	 */
-	if (vgic_supports_direct_msis(kvm)) {
+	if (vgic_supports_direct_irqs(kvm)) {
 		ret = vgic_v4_init(kvm);
 		if (ret)
 			goto out;
@@ -443,7 +443,7 @@ static void kvm_vgic_dist_destroy(struct kvm *kvm)
 		dist->vgic_cpu_base = VGIC_ADDR_UNDEF;
 	}
 
-	if (vgic_supports_direct_msis(kvm))
+	if (vgic_supports_direct_irqs(kvm))
 		vgic_v4_teardown(kvm);
 
 	xa_destroy(&dist->lpi_xa);
diff --git a/arch/arm64/kvm/vgic/vgic-mmio-v3.c b/arch/arm64/kvm/vgic/vgic-mmio-v3.c
index ae4c0593d114..1a9c5b4418b2 100644
--- a/arch/arm64/kvm/vgic/vgic-mmio-v3.c
+++ b/arch/arm64/kvm/vgic/vgic-mmio-v3.c
@@ -50,8 +50,12 @@ bool vgic_has_its(struct kvm *kvm)
 
 bool vgic_supports_direct_msis(struct kvm *kvm)
 {
-	return (kvm_vgic_global_state.has_gicv4_1 ||
-		(kvm_vgic_global_state.has_gicv4 && vgic_has_its(kvm)));
+	return kvm_vgic_global_state.has_gicv4 && vgic_has_its(kvm);
+}
+
+bool vgic_supports_direct_sgis(struct kvm *kvm)
+{
+	return kvm_vgic_global_state.has_gicv4_1 && gic_cpuif_has_vsgi();
 }
 
 /*
@@ -86,7 +90,7 @@ static unsigned long vgic_mmio_read_v3_misc(struct kvm_vcpu *vcpu,
 		}
 		break;
 	case GICD_TYPER2:
-		if (kvm_vgic_global_state.has_gicv4_1 && gic_cpuif_has_vsgi())
+		if (vgic_supports_direct_sgis(vcpu->kvm))
 			value = GICD_TYPER2_nASSGIcap;
 		break;
 	case GICD_IIDR:
@@ -119,7 +123,7 @@ static void vgic_mmio_write_v3_misc(struct kvm_vcpu *vcpu,
 		dist->enabled = val & GICD_CTLR_ENABLE_SS_G1;
 
 		/* Not a GICv4.1? No HW SGIs */
-		if (!kvm_vgic_global_state.has_gicv4_1 || !gic_cpuif_has_vsgi())
+		if (!vgic_supports_direct_sgis(vcpu->kvm))
 			val &= ~GICD_CTLR_nASSGIreq;
 
 		/* Dist stays enabled? nASSGIreq is RO */
@@ -133,7 +137,7 @@ static void vgic_mmio_write_v3_misc(struct kvm_vcpu *vcpu,
 		if (is_hwsgi != dist->nassgireq)
 			vgic_v4_configure_vsgis(vcpu->kvm);
 
-		if (kvm_vgic_global_state.has_gicv4_1 &&
+		if (vgic_supports_direct_sgis(vcpu->kvm) &&
 		    was_enabled != dist->enabled)
 			kvm_make_all_cpus_request(vcpu->kvm, KVM_REQ_RELOAD_GICv4);
 		else if (!was_enabled && dist->enabled)
@@ -178,7 +182,7 @@ static int vgic_mmio_uaccess_write_v3_misc(struct kvm_vcpu *vcpu,
 		}
 	case GICD_CTLR:
 		/* Not a GICv4.1? No HW SGIs */
-		if (!kvm_vgic_global_state.has_gicv4_1)
+		if (vgic_supports_direct_sgis(vcpu->kvm))
 			val &= ~GICD_CTLR_nASSGIreq;
 
 		dist->enabled = val & GICD_CTLR_ENABLE_SS_G1;
diff --git a/arch/arm64/kvm/vgic/vgic-v4.c b/arch/arm64/kvm/vgic/vgic-v4.c
index 193946108192..e7e284d47a77 100644
--- a/arch/arm64/kvm/vgic/vgic-v4.c
+++ b/arch/arm64/kvm/vgic/vgic-v4.c
@@ -356,7 +356,7 @@ int vgic_v4_put(struct kvm_vcpu *vcpu)
 {
 	struct its_vpe *vpe = &vcpu->arch.vgic_cpu.vgic_v3.its_vpe;
 
-	if (!vgic_supports_direct_msis(vcpu->kvm) || !vpe->resident)
+	if (!vgic_supports_direct_irqs(vcpu->kvm) || !vpe->resident)
 		return 0;
 
 	return its_make_vpe_non_resident(vpe, vgic_v4_want_doorbell(vcpu));
@@ -367,7 +367,7 @@ int vgic_v4_load(struct kvm_vcpu *vcpu)
 	struct its_vpe *vpe = &vcpu->arch.vgic_cpu.vgic_v3.its_vpe;
 	int err;
 
-	if (!vgic_supports_direct_msis(vcpu->kvm) || vpe->resident)
+	if (!vgic_supports_direct_irqs(vcpu->kvm) || vpe->resident)
 		return 0;
 
 	if (vcpu_get_flag(vcpu, IN_WFI))
diff --git a/arch/arm64/kvm/vgic/vgic.c b/arch/arm64/kvm/vgic/vgic.c
index 8f8096d48925..f5148b38120a 100644
--- a/arch/arm64/kvm/vgic/vgic.c
+++ b/arch/arm64/kvm/vgic/vgic.c
@@ -951,7 +951,7 @@ void kvm_vgic_flush_hwstate(struct kvm_vcpu *vcpu)
 	 * can be directly injected (GICv4).
 	 */
 	if (list_empty(&vcpu->arch.vgic_cpu.ap_list_head) &&
-	    !vgic_supports_direct_msis(vcpu->kvm))
+	    !vgic_supports_direct_irqs(vcpu->kvm))
 		return;
 
 	DEBUG_SPINLOCK_BUG_ON(!irqs_disabled());
@@ -965,7 +965,7 @@ void kvm_vgic_flush_hwstate(struct kvm_vcpu *vcpu)
 	if (can_access_vgic_from_kernel())
 		vgic_restore_state(vcpu);
 
-	if (vgic_supports_direct_msis(vcpu->kvm))
+	if (vgic_supports_direct_irqs(vcpu->kvm))
 		vgic_v4_commit(vcpu);
 }
 
diff --git a/arch/arm64/kvm/vgic/vgic.h b/arch/arm64/kvm/vgic/vgic.h
index 4349084cb9a6..ebf9ed6adeac 100644
--- a/arch/arm64/kvm/vgic/vgic.h
+++ b/arch/arm64/kvm/vgic/vgic.h
@@ -370,6 +370,13 @@ int vgic_its_inv_lpi(struct kvm *kvm, struct vgic_irq *irq);
 int vgic_its_invall(struct kvm_vcpu *vcpu);
 
 bool vgic_supports_direct_msis(struct kvm *kvm);
+bool vgic_supports_direct_sgis(struct kvm *kvm);
+
+static inline bool vgic_supports_direct_irqs(struct kvm *kvm)
+{
+	return vgic_supports_direct_msis(kvm) || vgic_supports_direct_sgis(kvm);
+}
+
 int vgic_v4_init(struct kvm *kvm);
 void vgic_v4_teardown(struct kvm *kvm);
 void vgic_v4_configure_vsgis(struct kvm *kvm);
-- 
2.39.5

Re: [PATCH v4 1/6] KVM: arm64: Disambiguate support for vSGIs v. vLPIs

[Ben Horgan]

Hi Oliver,

On 7/9/25 22:14, Oliver Upton wrote:
> vgic_supports_direct_msis() is a bit of a misnomer, as it returns true
> if either vSGIs or vLPIs are supported. Pick it apart into a few
> predicates and replace some open-coded checks for vSGIs.
> 
> Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
> ---
>   arch/arm64/kvm/vgic/vgic-init.c    |  4 ++--
>   arch/arm64/kvm/vgic/vgic-mmio-v3.c | 16 ++++++++++------
>   arch/arm64/kvm/vgic/vgic-v4.c      |  4 ++--
>   arch/arm64/kvm/vgic/vgic.c         |  4 ++--
>   arch/arm64/kvm/vgic/vgic.h         |  7 +++++++
>   5 files changed, 23 insertions(+), 12 deletions(-)
> 
[snip]
>   		/* Dist stays enabled? nASSGIreq is RO */
> @@ -133,7 +137,7 @@ static void vgic_mmio_write_v3_misc(struct kvm_vcpu *vcpu,
>   		if (is_hwsgi != dist->nassgireq)
>   			vgic_v4_configure_vsgis(vcpu->kvm);
>   
> -		if (kvm_vgic_global_state.has_gicv4_1 &&
> +		if (vgic_supports_direct_sgis(vcpu->kvm) &&
>   		    was_enabled != dist->enabled)
>   			kvm_make_all_cpus_request(vcpu->kvm, KVM_REQ_RELOAD_GICv4);
>   		else if (!was_enabled && dist->enabled)
> @@ -178,7 +182,7 @@ static int vgic_mmio_uaccess_write_v3_misc(struct kvm_vcpu *vcpu,
>   		}
>   	case GICD_CTLR:
>   		/* Not a GICv4.1? No HW SGIs */
> -		if (!kvm_vgic_global_state.has_gicv4_1)
> +		if (vgic_supports_direct_sgis(vcpu->kvm))
Missing negation.
>   			val &= ~GICD_CTLR_nASSGIreq;
>   
>   		dist->enabled = val & GICD_CTLR_ENABLE_SS_G1;
> diff --git a/arch/arm64/kvm/vgic/vgic-v4.c b/arch/arm64/kvm/vgic/vgic-v4.c
> index 193946108192..e7e284d47a77 100644
> --- a/arch/arm64/kvm/vgic/vgic-v4.c
> +++ b/arch/arm64/kvm/vgic/vgic-v4.c
I note that the two further calls to vgic_supports_direct_msis() in this 
file look correct.
> @@ -356,7 +356,7 @@ int vgic_v4_put(struct kvm_vcpu *vcpu)
>   {
>   	struct its_vpe *vpe = &vcpu->arch.vgic_cpu.vgic_v3.its_vpe;
>   
> -	if (!vgic_supports_direct_msis(vcpu->kvm) || !vpe->resident)
> +	if (!vgic_supports_direct_irqs(vcpu->kvm) || !vpe->resident)
>   		return 0;
>   
>   	return its_make_vpe_non_resident(vpe, vgic_v4_want_doorbell(vcpu));
> @@ -367,7 +367,7 @@ int vgic_v4_load(struct kvm_vcpu *vcpu)
>   	struct its_vpe *vpe = &vcpu->arch.vgic_cpu.vgic_v3.its_vpe;
>   	int err;
>   
> -	if (!vgic_supports_direct_msis(vcpu->kvm) || vpe->resident)
> +	if (!vgic_supports_direct_irqs(vcpu->kvm) || vpe->resident)
>   		return 0;
>   
>   	if (vcpu_get_flag(vcpu, IN_WFI))
> diff --git a/arch/arm64/kvm/vgic/vgic.c b/arch/arm64/kvm/vgic/vgic.c
> index 8f8096d48925..f5148b38120a 100644
> --- a/arch/arm64/kvm/vgic/vgic.c
> +++ b/arch/arm64/kvm/vgic/vgic.c
[snip]

Thanks,

Ben

Re: [PATCH v4 1/6] KVM: arm64: Disambiguate support for vSGIs v. vLPIs

[Oliver Upton]

On Thu, Jul 10, 2025 at 09:59:14AM +0100, Ben Horgan wrote:
> Hi Oliver,
> 
> On 7/9/25 22:14, Oliver Upton wrote:
> > vgic_supports_direct_msis() is a bit of a misnomer, as it returns true
> > if either vSGIs or vLPIs are supported. Pick it apart into a few
> > predicates and replace some open-coded checks for vSGIs.
> > 
> > Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
> > ---
> >   arch/arm64/kvm/vgic/vgic-init.c    |  4 ++--
> >   arch/arm64/kvm/vgic/vgic-mmio-v3.c | 16 ++++++++++------
> >   arch/arm64/kvm/vgic/vgic-v4.c      |  4 ++--
> >   arch/arm64/kvm/vgic/vgic.c         |  4 ++--
> >   arch/arm64/kvm/vgic/vgic.h         |  7 +++++++
> >   5 files changed, 23 insertions(+), 12 deletions(-)
> > 
> [snip]
> >   		/* Dist stays enabled? nASSGIreq is RO */
> > @@ -133,7 +137,7 @@ static void vgic_mmio_write_v3_misc(struct kvm_vcpu *vcpu,
> >   		if (is_hwsgi != dist->nassgireq)
> >   			vgic_v4_configure_vsgis(vcpu->kvm);
> > -		if (kvm_vgic_global_state.has_gicv4_1 &&
> > +		if (vgic_supports_direct_sgis(vcpu->kvm) &&
> >   		    was_enabled != dist->enabled)
> >   			kvm_make_all_cpus_request(vcpu->kvm, KVM_REQ_RELOAD_GICv4);
> >   		else if (!was_enabled && dist->enabled)
> > @@ -178,7 +182,7 @@ static int vgic_mmio_uaccess_write_v3_misc(struct kvm_vcpu *vcpu,
> >   		}
> >   	case GICD_CTLR:
> >   		/* Not a GICv4.1? No HW SGIs */
> > -		if (!kvm_vgic_global_state.has_gicv4_1)
> > +		if (vgic_supports_direct_sgis(vcpu->kvm))
> Missing negation.

Oops, very well spotted. Thanks Ben!

Thanks,
Oliver

Re: [PATCH v4 1/6] KVM: arm64: Disambiguate support for vSGIs v. vLPIs

[Eric Auger]

Ho Oliver,

On 7/9/25 11:14 PM, Oliver Upton wrote:
> vgic_supports_direct_msis() is a bit of a misnomer, as it returns true
> if either vSGIs or vLPIs are supported. Pick it apart into a few
> predicates and replace some open-coded checks for vSGIs.
> 
> Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
> ---
>  arch/arm64/kvm/vgic/vgic-init.c    |  4 ++--
>  arch/arm64/kvm/vgic/vgic-mmio-v3.c | 16 ++++++++++------
>  arch/arm64/kvm/vgic/vgic-v4.c      |  4 ++--
>  arch/arm64/kvm/vgic/vgic.c         |  4 ++--
>  arch/arm64/kvm/vgic/vgic.h         |  7 +++++++
>  5 files changed, 23 insertions(+), 12 deletions(-)
> 
> diff --git a/arch/arm64/kvm/vgic/vgic-init.c b/arch/arm64/kvm/vgic/vgic-init.c
> index eb1205654ac8..5e0e4559004b 100644
> --- a/arch/arm64/kvm/vgic/vgic-init.c
> +++ b/arch/arm64/kvm/vgic/vgic-init.c
> @@ -395,7 +395,7 @@ int vgic_init(struct kvm *kvm)
>  	 * v4 support so that we get HW-accelerated vSGIs. Otherwise, only
>  	 * enable it if we present a virtual ITS to the guest.
the above comment refers to vSGISs. While the below change looks OK, I
feel difficult to associate it to the code below.
>  	 */
> -	if (vgic_supports_direct_msis(kvm)) {
> +	if (vgic_supports_direct_irqs(kvm)) {
>  		ret = vgic_v4_init(kvm);
>  		if (ret)
>  			goto out;
> @@ -443,7 +443,7 @@ static void kvm_vgic_dist_destroy(struct kvm *kvm)
>  		dist->vgic_cpu_base = VGIC_ADDR_UNDEF;
>  	}
>  
> -	if (vgic_supports_direct_msis(kvm))
> +	if (vgic_supports_direct_irqs(kvm))
>  		vgic_v4_teardown(kvm);
>  
>  	xa_destroy(&dist->lpi_xa);
> diff --git a/arch/arm64/kvm/vgic/vgic-mmio-v3.c b/arch/arm64/kvm/vgic/vgic-mmio-v3.c
> index ae4c0593d114..1a9c5b4418b2 100644
> --- a/arch/arm64/kvm/vgic/vgic-mmio-v3.c
> +++ b/arch/arm64/kvm/vgic/vgic-mmio-v3.c
> @@ -50,8 +50,12 @@ bool vgic_has_its(struct kvm *kvm)
>  
>  bool vgic_supports_direct_msis(struct kvm *kvm)
>  {
> -	return (kvm_vgic_global_state.has_gicv4_1 ||
> -		(kvm_vgic_global_state.has_gicv4 && vgic_has_its(kvm)));
> +	return kvm_vgic_global_state.has_gicv4 && vgic_has_its(kvm);
> +}
> +
> +bool vgic_supports_direct_sgis(struct kvm *kvm)
> +{
> +	return kvm_vgic_global_state.has_gicv4_1 && gic_cpuif_has_vsgi();
In some checks we now have "&& gic_cpuif_has_vsgi()" added whereas we
previously only checked kvm_vgic_global_state.has_gicv4_1. This may be
described in the commit message too and explain whether this is a fix
for some configs.

Thanks

Eric
>  }
>  
>  /*
> @@ -86,7 +90,7 @@ static unsigned long vgic_mmio_read_v3_misc(struct kvm_vcpu *vcpu,
>  		}
>  		break;
>  	case GICD_TYPER2:
> -		if (kvm_vgic_global_state.has_gicv4_1 && gic_cpuif_has_vsgi())
> +		if (vgic_supports_direct_sgis(vcpu->kvm))
>  			value = GICD_TYPER2_nASSGIcap;
>  		break;
>  	case GICD_IIDR:
> @@ -119,7 +123,7 @@ static void vgic_mmio_write_v3_misc(struct kvm_vcpu *vcpu,
>  		dist->enabled = val & GICD_CTLR_ENABLE_SS_G1;
>  
>  		/* Not a GICv4.1? No HW SGIs */
> -		if (!kvm_vgic_global_state.has_gicv4_1 || !gic_cpuif_has_vsgi())
> +		if (!vgic_supports_direct_sgis(vcpu->kvm))
>  			val &= ~GICD_CTLR_nASSGIreq;
>  
>  		/* Dist stays enabled? nASSGIreq is RO */
> @@ -133,7 +137,7 @@ static void vgic_mmio_write_v3_misc(struct kvm_vcpu *vcpu,
>  		if (is_hwsgi != dist->nassgireq)
>  			vgic_v4_configure_vsgis(vcpu->kvm);
>  
> -		if (kvm_vgic_global_state.has_gicv4_1 &&
> +		if (vgic_supports_direct_sgis(vcpu->kvm) &&
>  		    was_enabled != dist->enabled)
>  			kvm_make_all_cpus_request(vcpu->kvm, KVM_REQ_RELOAD_GICv4);
>  		else if (!was_enabled && dist->enabled)
> @@ -178,7 +182,7 @@ static int vgic_mmio_uaccess_write_v3_misc(struct kvm_vcpu *vcpu,
>  		}
>  	case GICD_CTLR:
>  		/* Not a GICv4.1? No HW SGIs */
> -		if (!kvm_vgic_global_state.has_gicv4_1)
> +		if (vgic_supports_direct_sgis(vcpu->kvm))
>  			val &= ~GICD_CTLR_nASSGIreq;
>  
>  		dist->enabled = val & GICD_CTLR_ENABLE_SS_G1;
> diff --git a/arch/arm64/kvm/vgic/vgic-v4.c b/arch/arm64/kvm/vgic/vgic-v4.c
> index 193946108192..e7e284d47a77 100644
> --- a/arch/arm64/kvm/vgic/vgic-v4.c
> +++ b/arch/arm64/kvm/vgic/vgic-v4.c
> @@ -356,7 +356,7 @@ int vgic_v4_put(struct kvm_vcpu *vcpu)
>  {
>  	struct its_vpe *vpe = &vcpu->arch.vgic_cpu.vgic_v3.its_vpe;
>  
> -	if (!vgic_supports_direct_msis(vcpu->kvm) || !vpe->resident)
> +	if (!vgic_supports_direct_irqs(vcpu->kvm) || !vpe->resident)
>  		return 0;
>  
>  	return its_make_vpe_non_resident(vpe, vgic_v4_want_doorbell(vcpu));
> @@ -367,7 +367,7 @@ int vgic_v4_load(struct kvm_vcpu *vcpu)
>  	struct its_vpe *vpe = &vcpu->arch.vgic_cpu.vgic_v3.its_vpe;
>  	int err;
>  
> -	if (!vgic_supports_direct_msis(vcpu->kvm) || vpe->resident)
> +	if (!vgic_supports_direct_irqs(vcpu->kvm) || vpe->resident)
>  		return 0;
>  
>  	if (vcpu_get_flag(vcpu, IN_WFI))
> diff --git a/arch/arm64/kvm/vgic/vgic.c b/arch/arm64/kvm/vgic/vgic.c
> index 8f8096d48925..f5148b38120a 100644
> --- a/arch/arm64/kvm/vgic/vgic.c
> +++ b/arch/arm64/kvm/vgic/vgic.c
> @@ -951,7 +951,7 @@ void kvm_vgic_flush_hwstate(struct kvm_vcpu *vcpu)
>  	 * can be directly injected (GICv4).
>  	 */
>  	if (list_empty(&vcpu->arch.vgic_cpu.ap_list_head) &&
> -	    !vgic_supports_direct_msis(vcpu->kvm))
> +	    !vgic_supports_direct_irqs(vcpu->kvm))
>  		return;
>  
>  	DEBUG_SPINLOCK_BUG_ON(!irqs_disabled());
> @@ -965,7 +965,7 @@ void kvm_vgic_flush_hwstate(struct kvm_vcpu *vcpu)
>  	if (can_access_vgic_from_kernel())
>  		vgic_restore_state(vcpu);
>  
> -	if (vgic_supports_direct_msis(vcpu->kvm))
> +	if (vgic_supports_direct_irqs(vcpu->kvm))
>  		vgic_v4_commit(vcpu);
>  }
>  
> diff --git a/arch/arm64/kvm/vgic/vgic.h b/arch/arm64/kvm/vgic/vgic.h
> index 4349084cb9a6..ebf9ed6adeac 100644
> --- a/arch/arm64/kvm/vgic/vgic.h
> +++ b/arch/arm64/kvm/vgic/vgic.h
> @@ -370,6 +370,13 @@ int vgic_its_inv_lpi(struct kvm *kvm, struct vgic_irq *irq);
>  int vgic_its_invall(struct kvm_vcpu *vcpu);
>  
>  bool vgic_supports_direct_msis(struct kvm *kvm);
> +bool vgic_supports_direct_sgis(struct kvm *kvm);
> +
> +static inline bool vgic_supports_direct_irqs(struct kvm *kvm)
> +{
> +	return vgic_supports_direct_msis(kvm) || vgic_supports_direct_sgis(kvm);
> +}
> +
>  int vgic_v4_init(struct kvm *kvm);
>  void vgic_v4_teardown(struct kvm *kvm);
>  void vgic_v4_configure_vsgis(struct kvm *kvm);

Re: [PATCH v4 1/6] KVM: arm64: Disambiguate support for vSGIs v. vLPIs

[Oliver Upton]

On Mon, Jul 14, 2025 at 12:20:14PM +0200, Eric Auger wrote:
> Ho Oliver,

Yo -- Eric! Sorry for the latency, ${WORK}.

> On 7/9/25 11:14 PM, Oliver Upton wrote:
> > vgic_supports_direct_msis() is a bit of a misnomer, as it returns true
> > if either vSGIs or vLPIs are supported. Pick it apart into a few
> > predicates and replace some open-coded checks for vSGIs.
> > 
> > Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
> > ---
> >  arch/arm64/kvm/vgic/vgic-init.c    |  4 ++--
> >  arch/arm64/kvm/vgic/vgic-mmio-v3.c | 16 ++++++++++------
> >  arch/arm64/kvm/vgic/vgic-v4.c      |  4 ++--
> >  arch/arm64/kvm/vgic/vgic.c         |  4 ++--
> >  arch/arm64/kvm/vgic/vgic.h         |  7 +++++++
> >  5 files changed, 23 insertions(+), 12 deletions(-)
> > 
> > diff --git a/arch/arm64/kvm/vgic/vgic-init.c b/arch/arm64/kvm/vgic/vgic-init.c
> > index eb1205654ac8..5e0e4559004b 100644
> > --- a/arch/arm64/kvm/vgic/vgic-init.c
> > +++ b/arch/arm64/kvm/vgic/vgic-init.c
> > @@ -395,7 +395,7 @@ int vgic_init(struct kvm *kvm)
> >  	 * v4 support so that we get HW-accelerated vSGIs. Otherwise, only
> >  	 * enable it if we present a virtual ITS to the guest.
> the above comment refers to vSGISs. While the below change looks OK, I
> feel difficult to associate it to the code below.

I'm not sure the comment serves much purpose at this point. If anything,
it should go with the implementation of vgic_supports_direct_irqs()

> >  	 */
> > -	if (vgic_supports_direct_msis(kvm)) {
> > +	if (vgic_supports_direct_irqs(kvm)) {
> >  		ret = vgic_v4_init(kvm);
> >  		if (ret)
> >  			goto out;
> > @@ -443,7 +443,7 @@ static void kvm_vgic_dist_destroy(struct kvm *kvm)
> >  		dist->vgic_cpu_base = VGIC_ADDR_UNDEF;
> >  	}
> >  
> > -	if (vgic_supports_direct_msis(kvm))
> > +	if (vgic_supports_direct_irqs(kvm))
> >  		vgic_v4_teardown(kvm);
> >  
> >  	xa_destroy(&dist->lpi_xa);
> > diff --git a/arch/arm64/kvm/vgic/vgic-mmio-v3.c b/arch/arm64/kvm/vgic/vgic-mmio-v3.c
> > index ae4c0593d114..1a9c5b4418b2 100644
> > --- a/arch/arm64/kvm/vgic/vgic-mmio-v3.c
> > +++ b/arch/arm64/kvm/vgic/vgic-mmio-v3.c
> > @@ -50,8 +50,12 @@ bool vgic_has_its(struct kvm *kvm)
> >  
> >  bool vgic_supports_direct_msis(struct kvm *kvm)
> >  {
> > -	return (kvm_vgic_global_state.has_gicv4_1 ||
> > -		(kvm_vgic_global_state.has_gicv4 && vgic_has_its(kvm)));
> > +	return kvm_vgic_global_state.has_gicv4 && vgic_has_its(kvm);
> > +}
> > +
> > +bool vgic_supports_direct_sgis(struct kvm *kvm)
> > +{
> > +	return kvm_vgic_global_state.has_gicv4_1 && gic_cpuif_has_vsgi();
> In some checks we now have "&& gic_cpuif_has_vsgi()" added whereas we
> previously only checked kvm_vgic_global_state.has_gicv4_1. This may be
> described in the commit message too and explain whether this is a fix
> for some configs.

Happy to add a mention to the changelog since this is definitely a
subtle bugfix. Although I hope there aren't any actual implementations
like this (i.e. GICv4.1 ITS w/o ability to deliver vSGIs)

Thanks for having a look,
Oliver

[PATCH v4 2/6] KVM: arm64: vgic-v3: Consolidate MAINT_IRQ handling

[Oliver Upton]

Consolidate the duplicated handling of the VGICv3 maintenance IRQ
attribute as a regular GICv3 attribute, as it is neither a register nor
a common attribute.

Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
---
 arch/arm64/kvm/vgic/vgic-kvm-device.c | 51 +++++++++++++--------------
 1 file changed, 25 insertions(+), 26 deletions(-)

diff --git a/arch/arm64/kvm/vgic/vgic-kvm-device.c b/arch/arm64/kvm/vgic/vgic-kvm-device.c
index f9ae790163fb..e28cf68a49c3 100644
--- a/arch/arm64/kvm/vgic/vgic-kvm-device.c
+++ b/arch/arm64/kvm/vgic/vgic-kvm-device.c
@@ -303,12 +303,6 @@ static int vgic_get_common_attr(struct kvm_device *dev,
 			     VGIC_NR_PRIVATE_IRQS, uaddr);
 		break;
 	}
-	case KVM_DEV_ARM_VGIC_GRP_MAINT_IRQ: {
-		u32 __user *uaddr = (u32 __user *)(long)attr->addr;
-
-		r = put_user(dev->kvm->arch.vgic.mi_intid, uaddr);
-		break;
-	}
 	}
 
 	return r;
@@ -523,7 +517,7 @@ static int vgic_v3_attr_regs_access(struct kvm_device *dev,
 	struct vgic_reg_attr reg_attr;
 	gpa_t addr;
 	struct kvm_vcpu *vcpu;
-	bool uaccess, post_init = true;
+	bool uaccess;
 	u32 val;
 	int ret;
 
@@ -539,9 +533,6 @@ static int vgic_v3_attr_regs_access(struct kvm_device *dev,
 		/* Sysregs uaccess is performed by the sysreg handling code */
 		uaccess = false;
 		break;
-	case KVM_DEV_ARM_VGIC_GRP_MAINT_IRQ:
-		post_init = false;
-		fallthrough;
 	default:
 		uaccess = true;
 	}
@@ -561,7 +552,7 @@ static int vgic_v3_attr_regs_access(struct kvm_device *dev,
 
 	mutex_lock(&dev->kvm->arch.config_lock);
 
-	if (post_init != vgic_initialized(dev->kvm)) {
+	if (!vgic_initialized(dev->kvm)) {
 		ret = -EBUSY;
 		goto out;
 	}
@@ -591,19 +582,6 @@ static int vgic_v3_attr_regs_access(struct kvm_device *dev,
 		}
 		break;
 	}
-	case KVM_DEV_ARM_VGIC_GRP_MAINT_IRQ:
-		if (!is_write) {
-			val = dev->kvm->arch.vgic.mi_intid;
-			ret = 0;
-			break;
-		}
-
-		ret = -EINVAL;
-		if ((val < VGIC_NR_PRIVATE_IRQS) && (val >= VGIC_NR_SGIS)) {
-			dev->kvm->arch.vgic.mi_intid = val;
-			ret = 0;
-		}
-		break;
 	default:
 		ret = -EINVAL;
 		break;
@@ -630,8 +608,24 @@ static int vgic_v3_set_attr(struct kvm_device *dev,
 	case KVM_DEV_ARM_VGIC_GRP_REDIST_REGS:
 	case KVM_DEV_ARM_VGIC_GRP_CPU_SYSREGS:
 	case KVM_DEV_ARM_VGIC_GRP_LEVEL_INFO:
-	case KVM_DEV_ARM_VGIC_GRP_MAINT_IRQ:
 		return vgic_v3_attr_regs_access(dev, attr, true);
+	case KVM_DEV_ARM_VGIC_GRP_MAINT_IRQ: {
+		u32 __user *uaddr = (u32 __user *)attr->addr;
+		u32 val;
+
+		if (get_user(val, uaddr))
+			return -EFAULT;
+
+		guard(mutex)(&dev->kvm->arch.config_lock);
+		if (vgic_initialized(dev->kvm))
+			return -EBUSY;
+
+		if ((val < VGIC_NR_SGIS) || (val >= VGIC_NR_PRIVATE_IRQS))
+			return -EINVAL;
+
+		dev->kvm->arch.vgic.mi_intid = val;
+		return 0;
+	}
 	default:
 		return vgic_set_common_attr(dev, attr);
 	}
@@ -645,8 +639,13 @@ static int vgic_v3_get_attr(struct kvm_device *dev,
 	case KVM_DEV_ARM_VGIC_GRP_REDIST_REGS:
 	case KVM_DEV_ARM_VGIC_GRP_CPU_SYSREGS:
 	case KVM_DEV_ARM_VGIC_GRP_LEVEL_INFO:
-	case KVM_DEV_ARM_VGIC_GRP_MAINT_IRQ:
 		return vgic_v3_attr_regs_access(dev, attr, false);
+	case KVM_DEV_ARM_VGIC_GRP_MAINT_IRQ: {
+		u32 __user *uaddr = (u32 __user *)(long)attr->addr;
+
+		guard(mutex)(&dev->kvm->arch.config_lock);
+		return put_user(dev->kvm->arch.vgic.mi_intid, uaddr);
+	}
 	default:
 		return vgic_get_common_attr(dev, attr);
 	}
-- 
2.39.5

Re: [PATCH v4 2/6] KVM: arm64: vgic-v3: Consolidate MAINT_IRQ handling

[Eric Auger]

Hi Oliver,

On 7/9/25 11:14 PM, Oliver Upton wrote:
> Consolidate the duplicated handling of the VGICv3 maintenance IRQ
> attribute as a regular GICv3 attribute, as it is neither a register nor
> a common attribute.
> 
> Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
> ---
>  arch/arm64/kvm/vgic/vgic-kvm-device.c | 51 +++++++++++++--------------
>  1 file changed, 25 insertions(+), 26 deletions(-)
> 
> diff --git a/arch/arm64/kvm/vgic/vgic-kvm-device.c b/arch/arm64/kvm/vgic/vgic-kvm-device.c
> index f9ae790163fb..e28cf68a49c3 100644
> --- a/arch/arm64/kvm/vgic/vgic-kvm-device.c
> +++ b/arch/arm64/kvm/vgic/vgic-kvm-device.c
> @@ -303,12 +303,6 @@ static int vgic_get_common_attr(struct kvm_device *dev,
>  			     VGIC_NR_PRIVATE_IRQS, uaddr);
>  		break;
>  	}
> -	case KVM_DEV_ARM_VGIC_GRP_MAINT_IRQ: {
> -		u32 __user *uaddr = (u32 __user *)(long)attr->addr;
> -
> -		r = put_user(dev->kvm->arch.vgic.mi_intid, uaddr);
> -		break;
> -	}
>  	}
>  
>  	return r;
> @@ -523,7 +517,7 @@ static int vgic_v3_attr_regs_access(struct kvm_device *dev,
>  	struct vgic_reg_attr reg_attr;
>  	gpa_t addr;
>  	struct kvm_vcpu *vcpu;
> -	bool uaccess, post_init = true;
> +	bool uaccess;
>  	u32 val;
>  	int ret;
>  
> @@ -539,9 +533,6 @@ static int vgic_v3_attr_regs_access(struct kvm_device *dev,
>  		/* Sysregs uaccess is performed by the sysreg handling code */
>  		uaccess = false;
>  		break;
> -	case KVM_DEV_ARM_VGIC_GRP_MAINT_IRQ:
> -		post_init = false;
> -		fallthrough;
>  	default:
>  		uaccess = true;
>  	}
> @@ -561,7 +552,7 @@ static int vgic_v3_attr_regs_access(struct kvm_device *dev,
>  
>  	mutex_lock(&dev->kvm->arch.config_lock);
>  
> -	if (post_init != vgic_initialized(dev->kvm)) {
> +	if (!vgic_initialized(dev->kvm)) {
>  		ret = -EBUSY;
>  		goto out;
>  	}
> @@ -591,19 +582,6 @@ static int vgic_v3_attr_regs_access(struct kvm_device *dev,
>  		}
>  		break;
>  	}
> -	case KVM_DEV_ARM_VGIC_GRP_MAINT_IRQ:
> -		if (!is_write) {
> -			val = dev->kvm->arch.vgic.mi_intid;
> -			ret = 0;
> -			break;
> -		}
> -
> -		ret = -EINVAL;
> -		if ((val < VGIC_NR_PRIVATE_IRQS) && (val >= VGIC_NR_SGIS)) {

> -			dev->kvm->arch.vgic.mi_intid = val;
> -			ret = 0;
> -		}
> -		break;
>  	default:
>  		ret = -EINVAL;
>  		break;
> @@ -630,8 +608,24 @@ static int vgic_v3_set_attr(struct kvm_device *dev,
>  	case KVM_DEV_ARM_VGIC_GRP_REDIST_REGS:
>  	case KVM_DEV_ARM_VGIC_GRP_CPU_SYSREGS:
>  	case KVM_DEV_ARM_VGIC_GRP_LEVEL_INFO:
> -	case KVM_DEV_ARM_VGIC_GRP_MAINT_IRQ:
>  		return vgic_v3_attr_regs_access(dev, attr, true);
> +	case KVM_DEV_ARM_VGIC_GRP_MAINT_IRQ: {
> +		u32 __user *uaddr = (u32 __user *)attr->addr;
> +		u32 val;
> +
> +		if (get_user(val, uaddr))
> +			return -EFAULT;
> +
> +		guard(mutex)(&dev->kvm->arch.config_lock);
Previously we had a more elaborate lock sequence in
vgic_v3_attr_regs_access() including
kvm_trylock_all_vcpus(dev->kvm) and mutex_lock(&dev->kvm->arch.config_lock);
Maybe worth to document in the commit msg why it is safe to remove that?



> +		if (vgic_initialized(dev->kvm))
> +			return -EBUSY;
> +
> +		if ((val < VGIC_NR_SGIS) || (val >= VGIC_NR_PRIVATE_IRQS))
nit: !irq_is_ppi()?

Thanks

Eric
> +			return -EINVAL;
> +
> +		dev->kvm->arch.vgic.mi_intid = val;
> +		return 0;
> +	}
>  	default:
>  		return vgic_set_common_attr(dev, attr);
>  	}
> @@ -645,8 +639,13 @@ static int vgic_v3_get_attr(struct kvm_device *dev,
>  	case KVM_DEV_ARM_VGIC_GRP_REDIST_REGS:
>  	case KVM_DEV_ARM_VGIC_GRP_CPU_SYSREGS:
>  	case KVM_DEV_ARM_VGIC_GRP_LEVEL_INFO:
> -	case KVM_DEV_ARM_VGIC_GRP_MAINT_IRQ:
>  		return vgic_v3_attr_regs_access(dev, attr, false);
> +	case KVM_DEV_ARM_VGIC_GRP_MAINT_IRQ: {
> +		u32 __user *uaddr = (u32 __user *)(long)attr->addr;
> +
> +		guard(mutex)(&dev->kvm->arch.config_lock);
> +		return put_user(dev->kvm->arch.vgic.mi_intid, uaddr);
> +	}
>  	default:
>  		return vgic_get_common_attr(dev, attr);
>  	}

Re: [PATCH v4 2/6] KVM: arm64: vgic-v3: Consolidate MAINT_IRQ handling

[Eric Auger]

On 7/14/25 2:52 PM, Eric Auger wrote:
> Hi Oliver,
> 
> On 7/9/25 11:14 PM, Oliver Upton wrote:
>> Consolidate the duplicated handling of the VGICv3 maintenance IRQ
>> attribute as a regular GICv3 attribute, as it is neither a register nor
>> a common attribute.
>>
>> Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
>> ---
>>  arch/arm64/kvm/vgic/vgic-kvm-device.c | 51 +++++++++++++--------------
>>  1 file changed, 25 insertions(+), 26 deletions(-)
>>
>> diff --git a/arch/arm64/kvm/vgic/vgic-kvm-device.c b/arch/arm64/kvm/vgic/vgic-kvm-device.c
>> index f9ae790163fb..e28cf68a49c3 100644
>> --- a/arch/arm64/kvm/vgic/vgic-kvm-device.c
>> +++ b/arch/arm64/kvm/vgic/vgic-kvm-device.c
>> @@ -303,12 +303,6 @@ static int vgic_get_common_attr(struct kvm_device *dev,
>>  			     VGIC_NR_PRIVATE_IRQS, uaddr);
>>  		break;
>>  	}
>> -	case KVM_DEV_ARM_VGIC_GRP_MAINT_IRQ: {
>> -		u32 __user *uaddr = (u32 __user *)(long)attr->addr;
>> -
>> -		r = put_user(dev->kvm->arch.vgic.mi_intid, uaddr);
>> -		break;
>> -	}
>>  	}
>>  
>>  	return r;
>> @@ -523,7 +517,7 @@ static int vgic_v3_attr_regs_access(struct kvm_device *dev,
>>  	struct vgic_reg_attr reg_attr;
>>  	gpa_t addr;
>>  	struct kvm_vcpu *vcpu;
>> -	bool uaccess, post_init = true;
>> +	bool uaccess;
>>  	u32 val;
>>  	int ret;
>>  
>> @@ -539,9 +533,6 @@ static int vgic_v3_attr_regs_access(struct kvm_device *dev,
>>  		/* Sysregs uaccess is performed by the sysreg handling code */
>>  		uaccess = false;
>>  		break;
>> -	case KVM_DEV_ARM_VGIC_GRP_MAINT_IRQ:
>> -		post_init = false;
>> -		fallthrough;
>>  	default:
>>  		uaccess = true;
>>  	}
>> @@ -561,7 +552,7 @@ static int vgic_v3_attr_regs_access(struct kvm_device *dev,
>>  
>>  	mutex_lock(&dev->kvm->arch.config_lock);
>>  
>> -	if (post_init != vgic_initialized(dev->kvm)) {
>> +	if (!vgic_initialized(dev->kvm)) {
>>  		ret = -EBUSY;
>>  		goto out;
>>  	}
>> @@ -591,19 +582,6 @@ static int vgic_v3_attr_regs_access(struct kvm_device *dev,
>>  		}
>>  		break;
>>  	}
>> -	case KVM_DEV_ARM_VGIC_GRP_MAINT_IRQ:
>> -		if (!is_write) {
>> -			val = dev->kvm->arch.vgic.mi_intid;
>> -			ret = 0;
>> -			break;
>> -		}
>> -
>> -		ret = -EINVAL;
>> -		if ((val < VGIC_NR_PRIVATE_IRQS) && (val >= VGIC_NR_SGIS)) {
> 
>> -			dev->kvm->arch.vgic.mi_intid = val;
>> -			ret = 0;
>> -		}
>> -		break;
>>  	default:
>>  		ret = -EINVAL;
>>  		break;
>> @@ -630,8 +608,24 @@ static int vgic_v3_set_attr(struct kvm_device *dev,
>>  	case KVM_DEV_ARM_VGIC_GRP_REDIST_REGS:
>>  	case KVM_DEV_ARM_VGIC_GRP_CPU_SYSREGS:
>>  	case KVM_DEV_ARM_VGIC_GRP_LEVEL_INFO:
>> -	case KVM_DEV_ARM_VGIC_GRP_MAINT_IRQ:
>>  		return vgic_v3_attr_regs_access(dev, attr, true);
>> +	case KVM_DEV_ARM_VGIC_GRP_MAINT_IRQ: {
>> +		u32 __user *uaddr = (u32 __user *)attr->addr;
>> +		u32 val;
>> +
>> +		if (get_user(val, uaddr))
>> +			return -EFAULT;
>> +
>> +		guard(mutex)(&dev->kvm->arch.config_lock);
> Previously we had a more elaborate lock sequence in
> vgic_v3_attr_regs_access() including
> kvm_trylock_all_vcpus(dev->kvm) and mutex_lock(&dev->kvm->arch.config_lock);
forget the last one which is obviously covered by the guard(mutex)

Eric
> Maybe worth to document in the commit msg why it is safe to remove that?
> 
> 
> 
>> +		if (vgic_initialized(dev->kvm))
>> +			return -EBUSY;
>> +
>> +		if ((val < VGIC_NR_SGIS) || (val >= VGIC_NR_PRIVATE_IRQS))
> nit: !irq_is_ppi()?
> 
> Thanks
> 
> Eric
>> +			return -EINVAL;
>> +
>> +		dev->kvm->arch.vgic.mi_intid = val;
>> +		return 0;
>> +	}
>>  	default:
>>  		return vgic_set_common_attr(dev, attr);
>>  	}
>> @@ -645,8 +639,13 @@ static int vgic_v3_get_attr(struct kvm_device *dev,
>>  	case KVM_DEV_ARM_VGIC_GRP_REDIST_REGS:
>>  	case KVM_DEV_ARM_VGIC_GRP_CPU_SYSREGS:
>>  	case KVM_DEV_ARM_VGIC_GRP_LEVEL_INFO:
>> -	case KVM_DEV_ARM_VGIC_GRP_MAINT_IRQ:
>>  		return vgic_v3_attr_regs_access(dev, attr, false);
>> +	case KVM_DEV_ARM_VGIC_GRP_MAINT_IRQ: {
>> +		u32 __user *uaddr = (u32 __user *)(long)attr->addr;
>> +
>> +		guard(mutex)(&dev->kvm->arch.config_lock);
>> +		return put_user(dev->kvm->arch.vgic.mi_intid, uaddr);
>> +	}
>>  	default:
>>  		return vgic_get_common_attr(dev, attr);
>>  	}
> 

Re: [PATCH v4 2/6] KVM: arm64: vgic-v3: Consolidate MAINT_IRQ handling

[Oliver Upton]

On Mon, Jul 14, 2025 at 02:52:37PM +0200, Eric Auger wrote:
> On 7/9/25 11:14 PM, Oliver Upton wrote:
> > @@ -630,8 +608,24 @@ static int vgic_v3_set_attr(struct kvm_device *dev,
> >  	case KVM_DEV_ARM_VGIC_GRP_REDIST_REGS:
> >  	case KVM_DEV_ARM_VGIC_GRP_CPU_SYSREGS:
> >  	case KVM_DEV_ARM_VGIC_GRP_LEVEL_INFO:
> > -	case KVM_DEV_ARM_VGIC_GRP_MAINT_IRQ:
> >  		return vgic_v3_attr_regs_access(dev, attr, true);
> > +	case KVM_DEV_ARM_VGIC_GRP_MAINT_IRQ: {
> > +		u32 __user *uaddr = (u32 __user *)attr->addr;
> > +		u32 val;
> > +
> > +		if (get_user(val, uaddr))
> > +			return -EFAULT;
> > +
> > +		guard(mutex)(&dev->kvm->arch.config_lock);
> Previously we had a more elaborate lock sequence in
> vgic_v3_attr_regs_access() including
> kvm_trylock_all_vcpus(dev->kvm) and mutex_lock(&dev->kvm->arch.config_lock);
> Maybe worth to document in the commit msg why it is safe to remove that?
> 

Can do, just adding FTR here that kvm_vgic_vcpu_nv_init() only takes the
config_lock so the rest is unneeded.

> 
> > +		if (vgic_initialized(dev->kvm))
> > +			return -EBUSY;
> > +
> > +		if ((val < VGIC_NR_SGIS) || (val >= VGIC_NR_PRIVATE_IRQS))
> nit: !irq_is_ppi()?

Much nicer.

Thanks,
Oliver

[PATCH v4 3/6] KVM: arm64: vgic-v3: Allow access to GICD_IIDR prior to initialization

[Oliver Upton]

KVM allows userspace to write GICD_IIDR for backwards-compatibility with
older kernels, where new implementation revisions have new features.
Unfortunately this is allowed to happen at runtime, and ripping features
out from underneath a running guest is a terrible idea.

While we can't do anything about the ABI, prepare for more ID-like
registers by allowing access to GICD_IIDR prior to VGIC initialization.
Subsequent changes will allow the VMM to further provision the GIC
feature set, e.g. the presence of nASSGIcap.

Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
---
 arch/arm64/kvm/vgic/vgic-init.c       |  9 +--------
 arch/arm64/kvm/vgic/vgic-kvm-device.c | 20 +++++++++++++++++++-
 2 files changed, 20 insertions(+), 9 deletions(-)

diff --git a/arch/arm64/kvm/vgic/vgic-init.c b/arch/arm64/kvm/vgic/vgic-init.c
index 5e0e4559004b..487e902b040c 100644
--- a/arch/arm64/kvm/vgic/vgic-init.c
+++ b/arch/arm64/kvm/vgic/vgic-init.c
@@ -157,6 +157,7 @@ int kvm_vgic_create(struct kvm *kvm, u32 type)
 
 	kvm->arch.vgic.in_kernel = true;
 	kvm->arch.vgic.vgic_model = type;
+	kvm->arch.vgic.implementation_rev = KVM_VGIC_IMP_REV_LATEST;
 
 	kvm->arch.vgic.vgic_dist_base = VGIC_ADDR_UNDEF;
 
@@ -409,15 +410,7 @@ int vgic_init(struct kvm *kvm)
 		goto out;
 
 	vgic_debug_init(kvm);
-
-	/*
-	 * If userspace didn't set the GIC implementation revision,
-	 * default to the latest and greatest. You know want it.
-	 */
-	if (!dist->implementation_rev)
-		dist->implementation_rev = KVM_VGIC_IMP_REV_LATEST;
 	dist->initialized = true;
-
 out:
 	return ret;
 }
diff --git a/arch/arm64/kvm/vgic/vgic-kvm-device.c b/arch/arm64/kvm/vgic/vgic-kvm-device.c
index e28cf68a49c3..15d9772a53c8 100644
--- a/arch/arm64/kvm/vgic/vgic-kvm-device.c
+++ b/arch/arm64/kvm/vgic/vgic-kvm-device.c
@@ -5,6 +5,7 @@
  * Copyright (C) 2015 ARM Ltd.
  * Author: Marc Zyngier <marc.zyngier@arm.com>
  */
+#include <linux/irqchip/arm-gic-v3.h>
 #include <linux/kvm_host.h>
 #include <kvm/arm_vgic.h>
 #include <linux/uaccess.h>
@@ -503,6 +504,23 @@ int vgic_v3_parse_attr(struct kvm_device *dev, struct kvm_device_attr *attr,
 	return 0;
 }
 
+/*
+ * Allow access to certain ID-like registers prior to VGIC initialization,
+ * thereby allowing the VMM to provision the features / sizing of the VGIC.
+ */
+static bool reg_allowed_pre_init(struct kvm_device_attr *attr)
+{
+	if (attr->group != KVM_DEV_ARM_VGIC_GRP_DIST_REGS)
+		return false;
+
+	switch (attr->attr & KVM_DEV_ARM_VGIC_OFFSET_MASK) {
+	case GICD_IIDR:
+		return true;
+	default:
+		return false;
+	}
+}
+
 /*
  * vgic_v3_attr_regs_access - allows user space to access VGIC v3 state
  *
@@ -552,7 +570,7 @@ static int vgic_v3_attr_regs_access(struct kvm_device *dev,
 
 	mutex_lock(&dev->kvm->arch.config_lock);
 
-	if (!vgic_initialized(dev->kvm)) {
+	if (!(vgic_initialized(dev->kvm) || reg_allowed_pre_init(attr))) {
 		ret = -EBUSY;
 		goto out;
 	}
-- 
2.39.5

Re: [PATCH v4 3/6] KVM: arm64: vgic-v3: Allow access to GICD_IIDR prior to initialization

[Eric Auger]

Hi Oliver,

On 7/9/25 11:14 PM, Oliver Upton wrote:
> KVM allows userspace to write GICD_IIDR for backwards-compatibility with
> older kernels, where new implementation revisions have new features.
> Unfortunately this is allowed to happen at runtime, and ripping features
> out from underneath a running guest is a terrible idea.
> 
> While we can't do anything about the ABI, prepare for more ID-like
> registers by allowing access to GICD_IIDR prior to VGIC initialization.
> Subsequent changes will allow the VMM to further provision the GIC
> feature set, e.g. the presence of nASSGIcap.
> 
> Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
> ---
>  arch/arm64/kvm/vgic/vgic-init.c       |  9 +--------
>  arch/arm64/kvm/vgic/vgic-kvm-device.c | 20 +++++++++++++++++++-
>  2 files changed, 20 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/arm64/kvm/vgic/vgic-init.c b/arch/arm64/kvm/vgic/vgic-init.c
> index 5e0e4559004b..487e902b040c 100644
> --- a/arch/arm64/kvm/vgic/vgic-init.c
> +++ b/arch/arm64/kvm/vgic/vgic-init.c
> @@ -157,6 +157,7 @@ int kvm_vgic_create(struct kvm *kvm, u32 type)
>  
>  	kvm->arch.vgic.in_kernel = true;
>  	kvm->arch.vgic.vgic_model = type;
> +	kvm->arch.vgic.implementation_rev = KVM_VGIC_IMP_REV_LATEST;
>  
>  	kvm->arch.vgic.vgic_dist_base = VGIC_ADDR_UNDEF;
>  
> @@ -409,15 +410,7 @@ int vgic_init(struct kvm *kvm)
>  		goto out;
>  
>  	vgic_debug_init(kvm);
> -
> -	/*
> -	 * If userspace didn't set the GIC implementation revision,
> -	 * default to the latest and greatest. You know want it.
> -	 */
> -	if (!dist->implementation_rev)
> -		dist->implementation_rev = KVM_VGIC_IMP_REV_LATEST;
The change related to KVM_VGIC_IMP_REV_LATEST looks unrelated to the
commit title/msg, isn't it?
>  	dist->initialized = true;
> -
>  out:
>  	return ret;
>  }
> diff --git a/arch/arm64/kvm/vgic/vgic-kvm-device.c b/arch/arm64/kvm/vgic/vgic-kvm-device.c
> index e28cf68a49c3..15d9772a53c8 100644
> --- a/arch/arm64/kvm/vgic/vgic-kvm-device.c
> +++ b/arch/arm64/kvm/vgic/vgic-kvm-device.c
> @@ -5,6 +5,7 @@
>   * Copyright (C) 2015 ARM Ltd.
>   * Author: Marc Zyngier <marc.zyngier@arm.com>
>   */
> +#include <linux/irqchip/arm-gic-v3.h>
>  #include <linux/kvm_host.h>
>  #include <kvm/arm_vgic.h>
>  #include <linux/uaccess.h>
> @@ -503,6 +504,23 @@ int vgic_v3_parse_attr(struct kvm_device *dev, struct kvm_device_attr *attr,
>  	return 0;
>  }
>  
> +/*
> + * Allow access to certain ID-like registers prior to VGIC initialization,
> + * thereby allowing the VMM to provision the features / sizing of the VGIC.
> + */
> +static bool reg_allowed_pre_init(struct kvm_device_attr *attr)
> +{
> +	if (attr->group != KVM_DEV_ARM_VGIC_GRP_DIST_REGS)
> +		return false;
> +
> +	switch (attr->attr & KVM_DEV_ARM_VGIC_OFFSET_MASK) {
> +	case GICD_IIDR:
> +		return true;
> +	default:
> +		return false;
> +	}
> +}
> +
>  /*
>   * vgic_v3_attr_regs_access - allows user space to access VGIC v3 state
>   *
> @@ -552,7 +570,7 @@ static int vgic_v3_attr_regs_access(struct kvm_device *dev,
>  
>  	mutex_lock(&dev->kvm->arch.config_lock);
>  
> -	if (!vgic_initialized(dev->kvm)) {
> +	if (!(vgic_initialized(dev->kvm) || reg_allowed_pre_init(attr))) {
nit: !vgic_initialized(dev->kvm) && !reg_allowed_pre_init() is easier to
compute for me


Besides

Reviewed-by: Eric Auger <eric.auger@redhat.com>

Eric

>  		ret = -EBUSY;
>  		goto out;
>  	}

Re: [PATCH v4 3/6] KVM: arm64: vgic-v3: Allow access to GICD_IIDR prior to initialization

[Oliver Upton]

On Mon, Jul 14, 2025 at 04:41:45PM +0200, Eric Auger wrote:
> Hi Oliver,
> 
> On 7/9/25 11:14 PM, Oliver Upton wrote:
> > KVM allows userspace to write GICD_IIDR for backwards-compatibility with
> > older kernels, where new implementation revisions have new features.
> > Unfortunately this is allowed to happen at runtime, and ripping features
> > out from underneath a running guest is a terrible idea.
> > 
> > While we can't do anything about the ABI, prepare for more ID-like
> > registers by allowing access to GICD_IIDR prior to VGIC initialization.
> > Subsequent changes will allow the VMM to further provision the GIC
> > feature set, e.g. the presence of nASSGIcap.
> > 
> > Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
> > ---
> >  arch/arm64/kvm/vgic/vgic-init.c       |  9 +--------
> >  arch/arm64/kvm/vgic/vgic-kvm-device.c | 20 +++++++++++++++++++-
> >  2 files changed, 20 insertions(+), 9 deletions(-)
> > 
> > diff --git a/arch/arm64/kvm/vgic/vgic-init.c b/arch/arm64/kvm/vgic/vgic-init.c
> > index 5e0e4559004b..487e902b040c 100644
> > --- a/arch/arm64/kvm/vgic/vgic-init.c
> > +++ b/arch/arm64/kvm/vgic/vgic-init.c
> > @@ -157,6 +157,7 @@ int kvm_vgic_create(struct kvm *kvm, u32 type)
> >  
> >  	kvm->arch.vgic.in_kernel = true;
> >  	kvm->arch.vgic.vgic_model = type;
> > +	kvm->arch.vgic.implementation_rev = KVM_VGIC_IMP_REV_LATEST;
> >  
> >  	kvm->arch.vgic.vgic_dist_base = VGIC_ADDR_UNDEF;
> >  
> > @@ -409,15 +410,7 @@ int vgic_init(struct kvm *kvm)
> >  		goto out;
> >  
> >  	vgic_debug_init(kvm);
> > -
> > -	/*
> > -	 * If userspace didn't set the GIC implementation revision,
> > -	 * default to the latest and greatest. You know want it.
> > -	 */
> > -	if (!dist->implementation_rev)
> > -		dist->implementation_rev = KVM_VGIC_IMP_REV_LATEST;
> The change related to KVM_VGIC_IMP_REV_LATEST looks unrelated to the
> commit title/msg, isn't it?

It is deliberate here to make IIDR have a similar discovery model to the
CPU feature ID registers where the value after creation represents the
maximum feature set.

> > @@ -552,7 +570,7 @@ static int vgic_v3_attr_regs_access(struct kvm_device *dev,
> >  
> >  	mutex_lock(&dev->kvm->arch.config_lock);
> >  
> > -	if (!vgic_initialized(dev->kvm)) {
> > +	if (!(vgic_initialized(dev->kvm) || reg_allowed_pre_init(attr))) {
> nit: !vgic_initialized(dev->kvm) && !reg_allowed_pre_init() is easier to
> compute for me
> 
> 
> Besides
> 
> Reviewed-by: Eric Auger <eric.auger@redhat.com>

Thanks!

Oliver

Re: [PATCH v4 3/6] KVM: arm64: vgic-v3: Allow access to GICD_IIDR prior to initialization

[Zhou Wang]

On 2025/7/10 5:14, Oliver Upton wrote:
> KVM allows userspace to write GICD_IIDR for backwards-compatibility with
> older kernels, where new implementation revisions have new features.
> Unfortunately this is allowed to happen at runtime, and ripping features
> out from underneath a running guest is a terrible idea.
> 
> While we can't do anything about the ABI, prepare for more ID-like
> registers by allowing access to GICD_IIDR prior to VGIC initialization.
> Subsequent changes will allow the VMM to further provision the GIC
> feature set, e.g. the presence of nASSGIcap.
> 
> Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
> ---
>  arch/arm64/kvm/vgic/vgic-init.c       |  9 +--------
>  arch/arm64/kvm/vgic/vgic-kvm-device.c | 20 +++++++++++++++++++-
>  2 files changed, 20 insertions(+), 9 deletions(-)
> 
[...]
>  /*
>   * vgic_v3_attr_regs_access - allows user space to access VGIC v3 state
>   *
> @@ -552,7 +570,7 @@ static int vgic_v3_attr_regs_access(struct kvm_device *dev,
>  
>  	mutex_lock(&dev->kvm->arch.config_lock);
>  
> -	if (!vgic_initialized(dev->kvm)) {
> +	if (!(vgic_initialized(dev->kvm) || reg_allowed_pre_init(attr))) {

I am confused here, the logic is that:

1. For the ID registers, they can not be modified after vGIC init.
2. For other registers, the original logic is that they should be modified after
   vGIC init.

So code here should be:

if ((reg_allowed_pre_init(attr) && vgic_initialized(dev->kvm)) ||
    !vgic_initialized(dev->kvm)) {
         ret = -EBUSY;
         goto out;
}

Not sure my understanding is right.

Best,
Zhou

>  		ret = -EBUSY;
>  		goto out;
>  	}

Re: [PATCH v4 3/6] KVM: arm64: vgic-v3: Allow access to GICD_IIDR prior to initialization

[Oliver Upton]

On Thu, Aug 21, 2025 at 06:55:13PM +0800, Zhou Wang wrote:
> On 2025/7/10 5:14, Oliver Upton wrote:
> > KVM allows userspace to write GICD_IIDR for backwards-compatibility with
> > older kernels, where new implementation revisions have new features.
> > Unfortunately this is allowed to happen at runtime, and ripping features
> > out from underneath a running guest is a terrible idea.
> > 
> > While we can't do anything about the ABI, prepare for more ID-like
> > registers by allowing access to GICD_IIDR prior to VGIC initialization.
> > Subsequent changes will allow the VMM to further provision the GIC
> > feature set, e.g. the presence of nASSGIcap.
> > 
> > Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
> > ---
> >  arch/arm64/kvm/vgic/vgic-init.c       |  9 +--------
> >  arch/arm64/kvm/vgic/vgic-kvm-device.c | 20 +++++++++++++++++++-
> >  2 files changed, 20 insertions(+), 9 deletions(-)
> > 
> [...]
> >  /*
> >   * vgic_v3_attr_regs_access - allows user space to access VGIC v3 state
> >   *
> > @@ -552,7 +570,7 @@ static int vgic_v3_attr_regs_access(struct kvm_device *dev,
> >  
> >  	mutex_lock(&dev->kvm->arch.config_lock);
> >  
> > -	if (!vgic_initialized(dev->kvm)) {
> > +	if (!(vgic_initialized(dev->kvm) || reg_allowed_pre_init(attr))) {
> 
> I am confused here, the logic is that:
> 
> 1. For the ID registers, they can not be modified after vGIC init.
> 2. For other registers, the original logic is that they should be modified after
>    vGIC init.
> 
> So code here should be:
> 
> if ((reg_allowed_pre_init(attr) && vgic_initialized(dev->kvm)) ||
>     !vgic_initialized(dev->kvm)) {
>          ret = -EBUSY;
>          goto out;
> }
> 
> Not sure my understanding is right.

This would break ABI which I tried to allude to in the changelog.
GICD_IIDR is _already_ writable post-init, meaning that the only option
we have is to relax that register and allow access pre- and post-init.

The preferred semantics around ID registers is that they're RW prior to
init and RO post-init. GICD_TYPER2 enforces this.

Thanks,
Oliver

Re: [PATCH v4 3/6] KVM: arm64: vgic-v3: Allow access to GICD_IIDR prior to initialization

[Zhou Wang]

On 2025/8/22 2:43, Oliver Upton wrote:
> On Thu, Aug 21, 2025 at 06:55:13PM +0800, Zhou Wang wrote:
>> On 2025/7/10 5:14, Oliver Upton wrote:
>>> KVM allows userspace to write GICD_IIDR for backwards-compatibility with
>>> older kernels, where new implementation revisions have new features.
>>> Unfortunately this is allowed to happen at runtime, and ripping features
>>> out from underneath a running guest is a terrible idea.
>>>
>>> While we can't do anything about the ABI, prepare for more ID-like
>>> registers by allowing access to GICD_IIDR prior to VGIC initialization.
>>> Subsequent changes will allow the VMM to further provision the GIC
>>> feature set, e.g. the presence of nASSGIcap.
>>>
>>> Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
>>> ---
>>>  arch/arm64/kvm/vgic/vgic-init.c       |  9 +--------
>>>  arch/arm64/kvm/vgic/vgic-kvm-device.c | 20 +++++++++++++++++++-
>>>  2 files changed, 20 insertions(+), 9 deletions(-)
>>>
>> [...]
>>>  /*
>>>   * vgic_v3_attr_regs_access - allows user space to access VGIC v3 state
>>>   *
>>> @@ -552,7 +570,7 @@ static int vgic_v3_attr_regs_access(struct kvm_device *dev,
>>>  
>>>  	mutex_lock(&dev->kvm->arch.config_lock);
>>>  
>>> -	if (!vgic_initialized(dev->kvm)) {
>>> +	if (!(vgic_initialized(dev->kvm) || reg_allowed_pre_init(attr))) {
>>
>> I am confused here, the logic is that:
>>
>> 1. For the ID registers, they can not be modified after vGIC init.
>> 2. For other registers, the original logic is that they should be modified after
>>    vGIC init.
>>
>> So code here should be:
>>
>> if ((reg_allowed_pre_init(attr) && vgic_initialized(dev->kvm)) ||
>>     !vgic_initialized(dev->kvm)) {
>>          ret = -EBUSY;
>>          goto out;
>> }
>>
>> Not sure my understanding is right.
> 
> This would break ABI which I tried to allude to in the changelog.
> GICD_IIDR is _already_ writable post-init, meaning that the only option
> we have is to relax that register and allow access pre- and post-init.>
> The preferred semantics around ID registers is that they're RW prior to
> init and RO post-init. GICD_TYPER2 enforces this.

Hi Oliver,

I got your idea, many thanks for your help.

Best,
Zhou

> 
> Thanks,
> Oliver
> .

[PATCH v4 4/6] KVM: arm64: vgic-v3: Allow userspace to write GICD_TYPER2.nASSGIcap

[Oliver Upton]

From: Raghavendra Rao Ananta <rananta@google.com>

KVM unconditionally advertises GICD_TYPER2.nASSGIcap (which internally
implies vSGIs) on GICv4.1 systems. Allow userspace to change whether a
VM supports the feature. Only allow changes prior to VGIC initialization
as at that point vPEs need to be allocated for the VM.

For convenience, bundle support for vLPIs and vSGIs behind this feature,
allowing userspace to control vPE allocation for VMs in environments
that may be constrained on vPE IDs.

Signed-off-by: Raghavendra Rao Ananta <rananta@google.com>
Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
---
 arch/arm64/kvm/vgic/vgic-init.c       |  3 +++
 arch/arm64/kvm/vgic/vgic-kvm-device.c |  1 +
 arch/arm64/kvm/vgic/vgic-mmio-v3.c    | 19 +++++++++++++++++--
 arch/arm64/kvm/vgic/vgic.h            |  1 +
 include/kvm/arm_vgic.h                |  3 +++
 5 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/kvm/vgic/vgic-init.c b/arch/arm64/kvm/vgic/vgic-init.c
index 487e902b040c..80c07d5a7d7a 100644
--- a/arch/arm64/kvm/vgic/vgic-init.c
+++ b/arch/arm64/kvm/vgic/vgic-init.c
@@ -166,6 +166,9 @@ int kvm_vgic_create(struct kvm *kvm, u32 type)
 	else
 		INIT_LIST_HEAD(&kvm->arch.vgic.rd_regions);
 
+	if (type == KVM_DEV_TYPE_ARM_VGIC_V3)
+		kvm->arch.vgic.nassgicap = system_supports_direct_sgis();
+
 out_unlock:
 	mutex_unlock(&kvm->arch.config_lock);
 	kvm_unlock_all_vcpus(kvm);
diff --git a/arch/arm64/kvm/vgic/vgic-kvm-device.c b/arch/arm64/kvm/vgic/vgic-kvm-device.c
index 15d9772a53c8..37beb6f32b3d 100644
--- a/arch/arm64/kvm/vgic/vgic-kvm-device.c
+++ b/arch/arm64/kvm/vgic/vgic-kvm-device.c
@@ -515,6 +515,7 @@ static bool reg_allowed_pre_init(struct kvm_device_attr *attr)
 
 	switch (attr->attr & KVM_DEV_ARM_VGIC_OFFSET_MASK) {
 	case GICD_IIDR:
+	case GICD_TYPER2:
 		return true;
 	default:
 		return false;
diff --git a/arch/arm64/kvm/vgic/vgic-mmio-v3.c b/arch/arm64/kvm/vgic/vgic-mmio-v3.c
index 1a9c5b4418b2..1a24dde8ffdc 100644
--- a/arch/arm64/kvm/vgic/vgic-mmio-v3.c
+++ b/arch/arm64/kvm/vgic/vgic-mmio-v3.c
@@ -53,11 +53,16 @@ bool vgic_supports_direct_msis(struct kvm *kvm)
 	return kvm_vgic_global_state.has_gicv4 && vgic_has_its(kvm);
 }
 
-bool vgic_supports_direct_sgis(struct kvm *kvm)
+bool system_supports_direct_sgis(void)
 {
 	return kvm_vgic_global_state.has_gicv4_1 && gic_cpuif_has_vsgi();
 }
 
+bool vgic_supports_direct_sgis(struct kvm *kvm)
+{
+	return kvm->arch.vgic.nassgicap;
+}
+
 /*
  * The Revision field in the IIDR have the following meanings:
  *
@@ -163,8 +168,18 @@ static int vgic_mmio_uaccess_write_v3_misc(struct kvm_vcpu *vcpu,
 
 	switch (addr & 0x0c) {
 	case GICD_TYPER2:
-		if (val != vgic_mmio_read_v3_misc(vcpu, addr, len))
+		reg = vgic_mmio_read_v3_misc(vcpu, addr, len);
+
+		if (reg == val)
+			return 0;
+		if (vgic_initialized(vcpu->kvm))
+			return -EBUSY;
+		if ((reg ^ val) & ~GICD_TYPER2_nASSGIcap)
 			return -EINVAL;
+		if (!system_supports_direct_sgis() && val)
+			return -EINVAL;
+
+		dist->nassgicap = val & GICD_TYPER2_nASSGIcap;
 		return 0;
 	case GICD_IIDR:
 		reg = vgic_mmio_read_v3_misc(vcpu, addr, len);
diff --git a/arch/arm64/kvm/vgic/vgic.h b/arch/arm64/kvm/vgic/vgic.h
index ebf9ed6adeac..96048ec59d0d 100644
--- a/arch/arm64/kvm/vgic/vgic.h
+++ b/arch/arm64/kvm/vgic/vgic.h
@@ -369,6 +369,7 @@ void vgic_its_invalidate_all_caches(struct kvm *kvm);
 int vgic_its_inv_lpi(struct kvm *kvm, struct vgic_irq *irq);
 int vgic_its_invall(struct kvm_vcpu *vcpu);
 
+bool system_supports_direct_sgis(void);
 bool vgic_supports_direct_msis(struct kvm *kvm);
 bool vgic_supports_direct_sgis(struct kvm *kvm);
 
diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
index 4a34f7f0a864..1b4886f3fb20 100644
--- a/include/kvm/arm_vgic.h
+++ b/include/kvm/arm_vgic.h
@@ -264,6 +264,9 @@ struct vgic_dist {
 	/* distributor enabled */
 	bool			enabled;
 
+	/* Supports SGIs without active state */
+	bool			nassgicap;
+
 	/* Wants SGIs without active state */
 	bool			nassgireq;
 
-- 
2.39.5

Re: [PATCH v4 4/6] KVM: arm64: vgic-v3: Allow userspace to write GICD_TYPER2.nASSGIcap

[Eric Auger]

Hi Oliver,

On 7/9/25 11:14 PM, Oliver Upton wrote:
> From: Raghavendra Rao Ananta <rananta@google.com>
> 
> KVM unconditionally advertises GICD_TYPER2.nASSGIcap (which internally
> implies vSGIs) on GICv4.1 systems. Allow userspace to change whether a
> VM supports the feature. Only allow changes prior to VGIC initialization
> as at that point vPEs need to be allocated for the VM.
> 
> For convenience, bundle support for vLPIs and vSGIs behind this feature,
> allowing userspace to control vPE allocation for VMs in environments
> that may be constrained on vPE IDs.
> 
> Signed-off-by: Raghavendra Rao Ananta <rananta@google.com>
> Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
> ---
>  arch/arm64/kvm/vgic/vgic-init.c       |  3 +++
>  arch/arm64/kvm/vgic/vgic-kvm-device.c |  1 +
>  arch/arm64/kvm/vgic/vgic-mmio-v3.c    | 19 +++++++++++++++++--
>  arch/arm64/kvm/vgic/vgic.h            |  1 +
>  include/kvm/arm_vgic.h                |  3 +++
>  5 files changed, 25 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/kvm/vgic/vgic-init.c b/arch/arm64/kvm/vgic/vgic-init.c
> index 487e902b040c..80c07d5a7d7a 100644
> --- a/arch/arm64/kvm/vgic/vgic-init.c
> +++ b/arch/arm64/kvm/vgic/vgic-init.c
> @@ -166,6 +166,9 @@ int kvm_vgic_create(struct kvm *kvm, u32 type)
>  	else
>  		INIT_LIST_HEAD(&kvm->arch.vgic.rd_regions);
>  
> +	if (type == KVM_DEV_TYPE_ARM_VGIC_V3)
> +		kvm->arch.vgic.nassgicap = system_supports_direct_sgis();
> +
>  out_unlock:
>  	mutex_unlock(&kvm->arch.config_lock);
>  	kvm_unlock_all_vcpus(kvm);
> diff --git a/arch/arm64/kvm/vgic/vgic-kvm-device.c b/arch/arm64/kvm/vgic/vgic-kvm-device.c
> index 15d9772a53c8..37beb6f32b3d 100644
> --- a/arch/arm64/kvm/vgic/vgic-kvm-device.c
> +++ b/arch/arm64/kvm/vgic/vgic-kvm-device.c
> @@ -515,6 +515,7 @@ static bool reg_allowed_pre_init(struct kvm_device_attr *attr)
>  
>  	switch (attr->attr & KVM_DEV_ARM_VGIC_OFFSET_MASK) {
>  	case GICD_IIDR:
> +	case GICD_TYPER2:
>  		return true;
>  	default:
>  		return false;
> diff --git a/arch/arm64/kvm/vgic/vgic-mmio-v3.c b/arch/arm64/kvm/vgic/vgic-mmio-v3.c
> index 1a9c5b4418b2..1a24dde8ffdc 100644
> --- a/arch/arm64/kvm/vgic/vgic-mmio-v3.c
> +++ b/arch/arm64/kvm/vgic/vgic-mmio-v3.c
> @@ -53,11 +53,16 @@ bool vgic_supports_direct_msis(struct kvm *kvm)
>  	return kvm_vgic_global_state.has_gicv4 && vgic_has_its(kvm);
>  }
>  
> -bool vgic_supports_direct_sgis(struct kvm *kvm)
> +bool system_supports_direct_sgis(void)
>  {
>  	return kvm_vgic_global_state.has_gicv4_1 && gic_cpuif_has_vsgi();
>  }
>  
> +bool vgic_supports_direct_sgis(struct kvm *kvm)
> +{
> +	return kvm->arch.vgic.nassgicap;
> +}
> +
>  /*
>   * The Revision field in the IIDR have the following meanings:
>   *
> @@ -163,8 +168,18 @@ static int vgic_mmio_uaccess_write_v3_misc(struct kvm_vcpu *vcpu,
>  
>  	switch (addr & 0x0c) {
>  	case GICD_TYPER2:
> -		if (val != vgic_mmio_read_v3_misc(vcpu, addr, len))
> +		reg = vgic_mmio_read_v3_misc(vcpu, addr, len);
> +
> +		if (reg == val)
> +			return 0;
> +		if (vgic_initialized(vcpu->kvm))
> +			return -EBUSY;
> +		if ((reg ^ val) & ~GICD_TYPER2_nASSGIcap)
>  			return -EINVAL;
> +		if (!system_supports_direct_sgis() && val)
> +			return -EINVAL;
> +
> +		dist->nassgicap = val & GICD_TYPER2_nASSGIcap;
>  		return 0;
>  	case GICD_IIDR:
>  		reg = vgic_mmio_read_v3_misc(vcpu, addr, len);
> diff --git a/arch/arm64/kvm/vgic/vgic.h b/arch/arm64/kvm/vgic/vgic.h
> index ebf9ed6adeac..96048ec59d0d 100644
> --- a/arch/arm64/kvm/vgic/vgic.h
> +++ b/arch/arm64/kvm/vgic/vgic.h
> @@ -369,6 +369,7 @@ void vgic_its_invalidate_all_caches(struct kvm *kvm);
>  int vgic_its_inv_lpi(struct kvm *kvm, struct vgic_irq *irq);
>  int vgic_its_invall(struct kvm_vcpu *vcpu);Reviewed-by: Eric Auger <eric.auger@redhat.com>
>  
> +bool system_supports_direct_sgis(void);
>  bool vgic_supports_direct_msis(struct kvm *kvm);
>  bool vgic_supports_direct_sgis(struct kvm *kvm);
>  
> diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
> index 4a34f7f0a864..1b4886f3fb20 100644
> --- a/include/kvm/arm_vgic.h
> +++ b/include/kvm/arm_vgic.h
> @@ -264,6 +264,9 @@ struct vgic_dist {
>  	/* distributor enabled */
>  	bool			enabled;
>  
> +	/* Supports SGIs without active state */
> +	bool			nassgicap;
> +
>  	/* Wants SGIs without active state */
>  	bool			nassgireq;
>  
Reviewed-by: Eric Auger <eric.auger@redhat.com>

Eric

[PATCH v4 5/6] KVM: arm64: selftests: Add test for nASSGIcap attribute

[Oliver Upton]

From: Raghavendra Rao Ananta <rananta@google.com>

Extend vgic_init to test the nASSGIcap attribute, asserting that it is
configurable (within reason) prior to initializing the VGIC.
Additionally, check that userspace cannot set the attribute after the
VGIC has been initialized.

Signed-off-by: Raghavendra Rao Ananta <rananta@google.com>
Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
---
 tools/testing/selftests/kvm/arm64/vgic_init.c | 42 ++++++++++++++++++-
 1 file changed, 40 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/kvm/arm64/vgic_init.c b/tools/testing/selftests/kvm/arm64/vgic_init.c
index b3b5fb0ff0a9..743351aa998e 100644
--- a/tools/testing/selftests/kvm/arm64/vgic_init.c
+++ b/tools/testing/selftests/kvm/arm64/vgic_init.c
@@ -13,13 +13,12 @@
 #include "kvm_util.h"
 #include "processor.h"
 #include "vgic.h"
+#include "gic_v3.h"
 
 #define NR_VCPUS		4
 
 #define REG_OFFSET(vcpu, offset) (((uint64_t)vcpu << 32) | offset)
 
-#define GICR_TYPER 0x8
-
 #define VGIC_DEV_IS_V2(_d) ((_d) == KVM_DEV_TYPE_ARM_VGIC_V2)
 #define VGIC_DEV_IS_V3(_d) ((_d) == KVM_DEV_TYPE_ARM_VGIC_V3)
 
@@ -675,6 +674,44 @@ static void test_v3_its_region(void)
 	vm_gic_destroy(&v);
 }
 
+static void test_v3_nassgicap(void)
+{
+	struct kvm_vcpu *vcpus[NR_VCPUS];
+	bool has_nassgicap;
+	struct vm_gic vm;
+	u32 typer2;
+	int ret;
+
+	vm = vm_gic_create_with_vcpus(KVM_DEV_TYPE_ARM_VGIC_V3, NR_VCPUS, vcpus);
+	kvm_device_attr_get(vm.gic_fd, KVM_DEV_ARM_VGIC_GRP_DIST_REGS,
+			    GICD_TYPER2, &typer2);
+	has_nassgicap = typer2 & GICD_TYPER2_nASSGIcap;
+
+	typer2 |= GICD_TYPER2_nASSGIcap;
+	ret = __kvm_device_attr_set(vm.gic_fd, KVM_DEV_ARM_VGIC_GRP_DIST_REGS,
+				    GICD_TYPER2, &typer2);
+	if (has_nassgicap)
+		TEST_ASSERT(!ret, KVM_IOCTL_ERROR(KVM_DEVICE_ATTR_SET, ret));
+	else
+		TEST_ASSERT(ret && errno == EINVAL,
+			    "Enabled nASSGIcap even though it's unavailable");
+
+	typer2 &= ~GICD_TYPER2_nASSGIcap;
+	kvm_device_attr_set(vm.gic_fd, KVM_DEV_ARM_VGIC_GRP_DIST_REGS,
+			    GICD_TYPER2, &typer2);
+
+	kvm_device_attr_set(vm.gic_fd, KVM_DEV_ARM_VGIC_GRP_CTRL,
+			    KVM_DEV_ARM_VGIC_CTRL_INIT, NULL);
+
+	typer2 ^= GICD_TYPER2_nASSGIcap;
+	ret = __kvm_device_attr_set(vm.gic_fd, KVM_DEV_ARM_VGIC_GRP_DIST_REGS,
+				    GICD_TYPER2, &typer2);
+	TEST_ASSERT(ret && errno == EBUSY,
+		    "Changed nASSGIcap after initializing the VGIC");
+
+	vm_gic_destroy(&vm);
+}
+
 /*
  * Returns 0 if it's possible to create GIC device of a given type (V2 or V3).
  */
@@ -730,6 +767,7 @@ void run_tests(uint32_t gic_dev_type)
 		test_v3_last_bit_single_rdist();
 		test_v3_redist_ipa_range_check_at_vcpu_run();
 		test_v3_its_region();
+		test_v3_nassgicap();
 	}
 }
 
-- 
2.39.5

Re: [PATCH v4 5/6] KVM: arm64: selftests: Add test for nASSGIcap attribute

[Eric Auger]

Hi,

On 7/9/25 11:14 PM, Oliver Upton wrote:
> From: Raghavendra Rao Ananta <rananta@google.com>
> 
> Extend vgic_init to test the nASSGIcap attribute, asserting that it is
> configurable (within reason) prior to initializing the VGIC.
> Additionally, check that userspace cannot set the attribute after the
> VGIC has been initialized.
> 
> Signed-off-by: Raghavendra Rao Ananta <rananta@google.com>
> Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
Reviewed-by: Eric Auger <eric.auger@redhat.com>

Thanks

Eric
> ---
>  tools/testing/selftests/kvm/arm64/vgic_init.c | 42 ++++++++++++++++++-
>  1 file changed, 40 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/testing/selftests/kvm/arm64/vgic_init.c b/tools/testing/selftests/kvm/arm64/vgic_init.c
> index b3b5fb0ff0a9..743351aa998e 100644
> --- a/tools/testing/selftests/kvm/arm64/vgic_init.c
> +++ b/tools/testing/selftests/kvm/arm64/vgic_init.c
> @@ -13,13 +13,12 @@
>  #include "kvm_util.h"
>  #include "processor.h"
>  #include "vgic.h"
> +#include "gic_v3.h"
>  
>  #define NR_VCPUS		4
>  
>  #define REG_OFFSET(vcpu, offset) (((uint64_t)vcpu << 32) | offset)
>  
> -#define GICR_TYPER 0x8
> -
>  #define VGIC_DEV_IS_V2(_d) ((_d) == KVM_DEV_TYPE_ARM_VGIC_V2)
>  #define VGIC_DEV_IS_V3(_d) ((_d) == KVM_DEV_TYPE_ARM_VGIC_V3)
>  
> @@ -675,6 +674,44 @@ static void test_v3_its_region(void)
>  	vm_gic_destroy(&v);
>  }
>  
> +static void test_v3_nassgicap(void)
> +{
> +	struct kvm_vcpu *vcpus[NR_VCPUS];
> +	bool has_nassgicap;
> +	struct vm_gic vm;
> +	u32 typer2;
> +	int ret;
> +
> +	vm = vm_gic_create_with_vcpus(KVM_DEV_TYPE_ARM_VGIC_V3, NR_VCPUS, vcpus);
> +	kvm_device_attr_get(vm.gic_fd, KVM_DEV_ARM_VGIC_GRP_DIST_REGS,
> +			    GICD_TYPER2, &typer2);
> +	has_nassgicap = typer2 & GICD_TYPER2_nASSGIcap;
> +
> +	typer2 |= GICD_TYPER2_nASSGIcap;
> +	ret = __kvm_device_attr_set(vm.gic_fd, KVM_DEV_ARM_VGIC_GRP_DIST_REGS,
> +				    GICD_TYPER2, &typer2);
> +	if (has_nassgicap)
> +		TEST_ASSERT(!ret, KVM_IOCTL_ERROR(KVM_DEVICE_ATTR_SET, ret));
> +	else
> +		TEST_ASSERT(ret && errno == EINVAL,
> +			    "Enabled nASSGIcap even though it's unavailable");
> +
> +	typer2 &= ~GICD_TYPER2_nASSGIcap;
> +	kvm_device_attr_set(vm.gic_fd, KVM_DEV_ARM_VGIC_GRP_DIST_REGS,
> +			    GICD_TYPER2, &typer2);
> +
> +	kvm_device_attr_set(vm.gic_fd, KVM_DEV_ARM_VGIC_GRP_CTRL,
> +			    KVM_DEV_ARM_VGIC_CTRL_INIT, NULL);
> +
> +	typer2 ^= GICD_TYPER2_nASSGIcap;
> +	ret = __kvm_device_attr_set(vm.gic_fd, KVM_DEV_ARM_VGIC_GRP_DIST_REGS,
> +				    GICD_TYPER2, &typer2);
> +	TEST_ASSERT(ret && errno == EBUSY,
> +		    "Changed nASSGIcap after initializing the VGIC");
> +
> +	vm_gic_destroy(&vm);
> +}
> +
>  /*
>   * Returns 0 if it's possible to create GIC device of a given type (V2 or V3).
>   */
> @@ -730,6 +767,7 @@ void run_tests(uint32_t gic_dev_type)
>  		test_v3_last_bit_single_rdist();
>  		test_v3_redist_ipa_range_check_at_vcpu_run();
>  		test_v3_its_region();
> +		test_v3_nassgicap();
>  	}
>  }
>  

[PATCH v4 6/6] Documentation: KVM: arm64: Describe VGICv3 registers writable pre-init

[Oliver Upton]

KVM allows userspace to control GICD_IIDR.Revision and
GICD_TYPER2.nASSGIcap prior to initialization for the sake of
provisioning the guest-visible feature set. Document the userspace
expectations surrounding accesses to these registers.

Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
---
 Documentation/virt/kvm/devices/arm-vgic-v3.rst | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/Documentation/virt/kvm/devices/arm-vgic-v3.rst b/Documentation/virt/kvm/devices/arm-vgic-v3.rst
index e860498b1e35..c7a1cd22d814 100644
--- a/Documentation/virt/kvm/devices/arm-vgic-v3.rst
+++ b/Documentation/virt/kvm/devices/arm-vgic-v3.rst
@@ -78,6 +78,8 @@ Groups:
     -ENXIO   The group or attribute is unknown/unsupported for this device
              or hardware support is missing.
     -EFAULT  Invalid user pointer for attr->addr.
+    -EBUSY   Attempt to write a register that is read-only after
+             initialization
     =======  =============================================================
 
 
@@ -120,6 +122,15 @@ Groups:
     Note that distributor fields are not banked, but return the same value
     regardless of the mpidr used to access the register.
 
+    Userspace is allowed to write the following register fields prior to
+    initialization of the VGIC:
+
+      =====================
+      GICD_IIDR.Revision
+      GICD_TYPER2.nASSGIcap
+      =====================
+
+
     GICD_IIDR.Revision is updated when the KVM implementation is changed in a
     way directly observable by the guest or userspace.  Userspace should read
     GICD_IIDR from KVM and write back the read value to confirm its expected
@@ -128,6 +139,12 @@ Groups:
     behavior.
 
 
+    GICD_TYPER2.nASSGIcap allows userspace to control the support of SGIs
+    without an active state. At VGIC creation the field resets to the
+    maximum capability of the system. Userspace is expected to read the field
+    to determine the supported value(s) before writing to the field.
+
+
     The GICD_STATUSR and GICR_STATUSR registers are architecturally defined such
     that a write of a clear bit has no effect, whereas a write with a set bit
     clears that value.  To allow userspace to freely set the values of these two
-- 
2.39.5

Re: [PATCH v4 6/6] Documentation: KVM: arm64: Describe VGICv3 registers writable pre-init

[Eric Auger]

Hi,

On 7/9/25 11:14 PM, Oliver Upton wrote:
> KVM allows userspace to control GICD_IIDR.Revision and
> GICD_TYPER2.nASSGIcap prior to initialization for the sake of
> provisioning the guest-visible feature set. Document the userspace
> expectations surrounding accesses to these registers.
> 
> Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
> ---
>  Documentation/virt/kvm/devices/arm-vgic-v3.rst | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/Documentation/virt/kvm/devices/arm-vgic-v3.rst b/Documentation/virt/kvm/devices/arm-vgic-v3.rst
> index e860498b1e35..c7a1cd22d814 100644
> --- a/Documentation/virt/kvm/devices/arm-vgic-v3.rst
> +++ b/Documentation/virt/kvm/devices/arm-vgic-v3.rst
> @@ -78,6 +78,8 @@ Groups:
>      -ENXIO   The group or attribute is unknown/unsupported for this device
>               or hardware support is missing.
>      -EFAULT  Invalid user pointer for attr->addr.
> +    -EBUSY   Attempt to write a register that is read-only after
> +             initialization
>      =======  =============================================================
>  
>  
> @@ -120,6 +122,15 @@ Groups:
>      Note that distributor fields are not banked, but return the same value
>      regardless of the mpidr used to access the register.
>  
> +    Userspace is allowed to write the following register fields prior to
> +    initialization of the VGIC:
> +
> +      =====================
> +      GICD_IIDR.Revision
> +      GICD_TYPER2.nASSGIcap
> +      =====================
> +
> +
>      GICD_IIDR.Revision is updated when the KVM implementation is changed in a
>      way directly observable by the guest or userspace.  Userspace should read
>      GICD_IIDR from KVM and write back the read value to confirm its expected
> @@ -128,6 +139,12 @@ Groups:
>      behavior.
>  
>  
> +    GICD_TYPER2.nASSGIcap allows userspace to control the support of SGIs
> +    without an active state. At VGIC creation the field resets to the
In [PATCH v4 4/6] KVM: arm64: vgic-v3: Allow userspace to write
GICD_TYPER2.nASSGIcap commit message it was said:

"For convenience, bundle support for vLPIs and vSGIs behind this
feature,allowing userspace to control vPE allocation for VMs in
environments that may be constrained on vPE IDs."

which, I understand goes beyond the simple support of vSGIs. Maybe worth
a clarification.
> +    maximum capability of the system. Userspace is expected to read the field
> +    to determine the supported value(s) before writing to the field.
> +
> +
>      The GICD_STATUSR and GICR_STATUSR registers are architecturally defined such
>      that a write of a clear bit has no effect, whereas a write with a set bit
>      clears that value.  To allow userspace to freely set the values of these two

Thanks

Eric

Re: [PATCH v4 6/6] Documentation: KVM: arm64: Describe VGICv3 registers writable pre-init

[Oliver Upton]

On Mon, Jul 14, 2025 at 05:06:10PM +0200, Eric Auger wrote:
> Hi,
> 
> On 7/9/25 11:14 PM, Oliver Upton wrote:
> > KVM allows userspace to control GICD_IIDR.Revision and
> > GICD_TYPER2.nASSGIcap prior to initialization for the sake of
> > provisioning the guest-visible feature set. Document the userspace
> > expectations surrounding accesses to these registers.
> > 
> > Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
> > ---
> >  Documentation/virt/kvm/devices/arm-vgic-v3.rst | 17 +++++++++++++++++
> >  1 file changed, 17 insertions(+)
> > 
> > diff --git a/Documentation/virt/kvm/devices/arm-vgic-v3.rst b/Documentation/virt/kvm/devices/arm-vgic-v3.rst
> > index e860498b1e35..c7a1cd22d814 100644
> > --- a/Documentation/virt/kvm/devices/arm-vgic-v3.rst
> > +++ b/Documentation/virt/kvm/devices/arm-vgic-v3.rst
> > @@ -78,6 +78,8 @@ Groups:
> >      -ENXIO   The group or attribute is unknown/unsupported for this device
> >               or hardware support is missing.
> >      -EFAULT  Invalid user pointer for attr->addr.
> > +    -EBUSY   Attempt to write a register that is read-only after
> > +             initialization
> >      =======  =============================================================
> >  
> >  
> > @@ -120,6 +122,15 @@ Groups:
> >      Note that distributor fields are not banked, but return the same value
> >      regardless of the mpidr used to access the register.
> >  
> > +    Userspace is allowed to write the following register fields prior to
> > +    initialization of the VGIC:
> > +
> > +      =====================
> > +      GICD_IIDR.Revision
> > +      GICD_TYPER2.nASSGIcap
> > +      =====================
> > +
> > +
> >      GICD_IIDR.Revision is updated when the KVM implementation is changed in a
> >      way directly observable by the guest or userspace.  Userspace should read
> >      GICD_IIDR from KVM and write back the read value to confirm its expected
> > @@ -128,6 +139,12 @@ Groups:
> >      behavior.
> >  
> >  
> > +    GICD_TYPER2.nASSGIcap allows userspace to control the support of SGIs
> > +    without an active state. At VGIC creation the field resets to the
> In [PATCH v4 4/6] KVM: arm64: vgic-v3: Allow userspace to write
> GICD_TYPER2.nASSGIcap commit message it was said:
> 
> "For convenience, bundle support for vLPIs and vSGIs behind this
> feature,allowing userspace to control vPE allocation for VMs in
> environments that may be constrained on vPE IDs."
> 
> which, I understand goes beyond the simple support of vSGIs. Maybe worth
> a clarification.

I would like to omit that language from the documentation. Giving
userspace the impression of controlling the host allocation is a bit odd
and nothing stops us from implementing nASSGIcap for software-injected
SGIs.

vPE allocation should be a non-issue for correctly provisioned
implementations.

Thanks,
Oliver