Subject: Re: [PATCH v2 03/39] irqchip/gic-v5: Setup gic_kvm_info on ACPI hosts

From: Sascha Bischoff <[email-masked]>
Date: Fri, 29 May 2026 14:33:57 +0000

On Wed, 2026-05-27 at 11:51 +0100, Marc Zyngier wrote:
> On Thu, 21 May 2026 15:50:09 +0100,
> Sascha Bischoff <[email-masked]> wrote:
> > 
> > Device-tree based GICv5 probing already passes the IRS details and
> > maintenance interrupt to KVM, but the ACPI path only initialises
> > the
> > irqchip and installs the ACPI IRQ model. As a result, KVM never
> > sees
> > the GICv5 host information required to probe the vGIC on ACPI
> > systems.
> > 
> > Add the ACPI equivalent of the DT KVM setup. Parse the MADT GICC
> > entries for the maintenance interrupt, require all relevant entries
> > to
> > agree, register the interrupt as a GICv5 PPI-encoded GSI, and pass
> > the
> > resulting IRQ together with the IRS base and coherency information
> > to
> > KVM. Native GICv5 does not require a maintenance interrupt unless
> > the
> > legacy GICv3-compatible CPU interface is present, so preserve the
> > existing no-maintenance-IRQ handling for that case.
> > 
> > Signed-off-by: Sascha Bischoff <[email-masked]>
> > ---
> >  drivers/irqchip/irq-gic-v5.c | 103
> > +++++++++++++++++++++++++++++++++--
> >  1 file changed, 98 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/irqchip/irq-gic-v5.c b/drivers/irqchip/irq-
> > gic-v5.c
> > index 707deabbf2f63..ccd1ec69a6ab2 100644
> > --- a/drivers/irqchip/irq-gic-v5.c
> > +++ b/drivers/irqchip/irq-gic-v5.c
> > @@ -1126,7 +1126,7 @@ static void gicv5_set_cpuif_idbits(void)
> >  #ifdef CONFIG_KVM
> >  static struct gic_kvm_info gic_v5_kvm_info __initdata;
> >  
> > -static void __init gic_of_setup_kvm_info(struct device_node *node)
> > +static void __init gic_setup_kvm_info(unsigned int maint_irq)
> >  {
> >  	struct gicv5_irs_chip_data *irs_data =
> > gicv5_irs_get_chip_data();
> >  
> > @@ -1140,13 +1140,14 @@ static void __init
> > gic_of_setup_kvm_info(struct device_node *node)
> >  		return;
> >  	}
> >  
> > -	gic_v5_kvm_info.type = GIC_V5;
> > +	if (WARN_ON(!irs_data))
> > +		return;
> >  
> > +	gic_v5_kvm_info.type = GIC_V5;
> >  	gic_v5_kvm_info.gicv5_irs.base = irs_data->irs_base;
> >  	gic_v5_kvm_info.gicv5_irs.non_coherent = !!(irs_data-
> > >flags & IRS_FLAGS_NON_COHERENT);
> > -
> > -	/* GIC Virtual CPU interface maintenance interrupt */
> > -	gic_v5_kvm_info.maint_irq = irq_of_parse_and_map(node, 0);
> > +	gic_v5_kvm_info.maint_irq = maint_irq;
> > +	gic_v5_kvm_info.no_maint_irq_mask = false;
> 
> 
> You remove this last line from patch #1, and reintroduce it here. My
> gut feeling is that it should never be removed the first place.

Agreed. I've cleaned that noise up.

> 
> > 
> >  	/*
> >  	 * We require an MI if we have legacy support, but don't,
> > otherwise.
> > @@ -1162,10 +1163,101 @@ static void __init
> > gic_of_setup_kvm_info(struct device_node *node)
> >  
> >  	vgic_set_kvm_info(&gic_v5_kvm_info);
> >  }
> > +
> > +static void __init gic_of_setup_kvm_info(struct device_node *node)
> > +{
> > +	/* GIC Virtual CPU interface maintenance interrupt */
> > +	gic_setup_kvm_info(irq_of_parse_and_map(node, 0));
> > +}
> > +
> > +#ifdef CONFIG_ACPI
> > +struct gicv5_acpi_kvm_info {
> > +	u32 maint_irq;
> > +	int maint_irq_mode;
> > +};
> > +
> > +static struct gicv5_acpi_kvm_info acpi_v5_kvm_info __initdata;
> > +
> > +static int __init gic_acpi_parse_virt_madt_gicc(union
> > acpi_subtable_headers *header,
> > +						const unsigned
> > long end)
> > +{
> > +	struct acpi_madt_generic_interrupt *gicc =
> > +		(struct acpi_madt_generic_interrupt *)header;
> > +	static int first_madt = true;
> > +	int maint_irq_mode;
> > +
> > +	if (!(gicc->flags &
> > +	      (ACPI_MADT_ENABLED |
> > ACPI_MADT_GICC_ONLINE_CAPABLE)))
> > +		return 0;
> > +
> > +	maint_irq_mode = (gicc->flags & ACPI_MADT_VGIC_IRQ_MODE) ?
> > +			 ACPI_EDGE_SENSITIVE :
> > ACPI_LEVEL_SENSITIVE;
> > +
> > +	if (first_madt) {
> > +		first_madt = false;
> > +
> > +		acpi_v5_kvm_info.maint_irq = gicc->vgic_interrupt;
> > +		acpi_v5_kvm_info.maint_irq_mode = maint_irq_mode;
> > +		return 0;
> > +	}
> > +
> > +	/* The maintenance interrupt must be the same for every
> > GICC entry. */
> > +	if (acpi_v5_kvm_info.maint_irq != gicc->vgic_interrupt ||
> > +	    acpi_v5_kvm_info.maint_irq_mode != maint_irq_mode)
> > +		return -EINVAL;
> > +
> > +	return 0;
> > +}
> > +
> > +static bool __init gic_acpi_collect_virt_info(void)
> > +{
> > +	int count;
> > +
> > +	acpi_v5_kvm_info.maint_irq = 0;
> > +	acpi_v5_kvm_info.maint_irq_mode = 0;
> > +
> > +	count =
> > acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_INTERRUPT,
> > +				     
> > gic_acpi_parse_virt_madt_gicc, 0);
> > +
> > +	return count > 0;
> > +}
> > +
> > +static void __init gic_acpi_setup_kvm_info(void)
> > +{
> > +	unsigned int maint_irq = 0;
> > +	int irq;
> > +
> > +	if (!gic_acpi_collect_virt_info()) {
> > +		pr_warn("Unable to get hardware information used
> > for virtualization\n");
> > +		return;
> > +	}
> > +
> > +	if (acpi_v5_kvm_info.maint_irq) {
> > +		u32 gsi = FIELD_PREP(GICV5_HWIRQ_TYPE,
> > GICV5_HWIRQ_TYPE_PPI) |
> > +			  FIELD_PREP(GICV5_HWIRQ_ID,
> > acpi_v5_kvm_info.maint_irq);
> > +
> > +		irq = acpi_register_gsi(NULL, gsi,
> > +					acpi_v5_kvm_info.maint_irq
> > _mode,
> > +					ACPI_ACTIVE_HIGH);
> > +		if (irq <= 0)
> > +			return;
> 
> This probably deserves a bit of a warning. And maybe not completely
> fail the registration with KVM?

That's a good point. At this stage we don't know if we have legacy
support or not, which means we also don't know if we need the MI in the
first place. I've changed to emitting a warning here, and let the
existing code in gic_setup_kvm_info() figure out if we can continue or
not.

> Thanks,
> 	M.

Thanks,
Sascha