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

[Oliver Upton <oliver.upton@linux.dev>]

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