Re: [PATCH v4 03/48] irqchip/gic-v5: Set up gic_kvm_info on ACPI hosts

On Sat, 2026-07-25 at 10:35 +0100, Marc Zyngier wrote:
> On Fri, 24 Jul 2026 11:49:11 +0100,
> Sascha Bischoff <[email protected]> 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 protected]>
> > ---
> >  drivers/irqchip/irq-gic-v5.c | 100
> > +++++++++++++++++++++++++++++++++--
> >  1 file changed, 96 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/irqchip/irq-gic-v5.c b/drivers/irqchip/irq-
> > gic-v5.c
> > index e7a7aedcfaf78..0d90675fb319b 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();
> >  
> > @@ -1137,17 +1137,19 @@ static void __init
> > gic_of_setup_kvm_info(struct device_node *node)
> >  	 */
> >  	if (!gicv5_global_data.virt_capable) {
> >  		pr_info("GIC implementation is not virtualization
> > capable\n");
> > -		return;
> > +		goto out_dispose_maint_irq;
> >  	}
> >  
> > -	gic_v5_kvm_info.type = GIC_V5;
> > +	if (WARN_ON(!irs_data))
> > +		goto out_dispose_maint_irq;
> >  
> > +	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.no_maint_irq_mask = false;
> > -	gic_v5_kvm_info.maint_irq = irq_of_parse_and_map(node, 0);
> > +	gic_v5_kvm_info.maint_irq = maint_irq;
> >  
> >  	/*
> >  	 * We require an MI if we have legacy support, but don't,
> > otherwise.
> > @@ -1162,11 +1164,100 @@ static void __init
> > gic_of_setup_kvm_info(struct device_node *node)
> >  		gic_v5_kvm_info.no_maint_irq_mask = true;
> >  
> >  	vgic_set_kvm_info(&gic_v5_kvm_info);
> > +	return;
> > +
> > +out_dispose_maint_irq:
> > +	irq_dispose_mapping(maint_irq);
> > +}
> > +
> > +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;
> > +
> 
> This looks wrong, as the MI cannot be edge-triggered. This is
> cargo-culted from the GICv3 code, which is just as buggy (I'll go and
> clean it up).
> 
> This should simply be ignored, and the existence of the field shows
> how little understanding of the architecture the ACPI zealots have.

Have done that, and have broadly matched your GICv3 changes.

> 
> > +	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;
> 
> Similarly, this second check should be dropped. We're not in the
> business of validating ACPI tables. Print a FW_BUG warning if you
> want, but that's all that needs doing, and we don't need to screw the
> user over the FW's brokenness.

OK, done. I've left a warning, but otherwise continue.

> 
> > +
> > +	return 0;
> > +}
> > +
> > +static bool __init gic_acpi_collect_virt_info(void)
> > +{
> > +	int count;
> > +
> > +	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) {
> > +		irq = acpi_register_gsi(NULL,
> > acpi_v5_kvm_info.maint_irq,
> > +					acpi_v5_kvm_info.maint_irq
> > _mode,
> > +					ACPI_ACTIVE_HIGH);
> 
> Just hardcode the trigger mode here.

Done.

> 
> > +		if (irq > 0)
> > +			maint_irq = irq;
> > +		else
> > +			pr_warn("Failed to register GSI for GICv5
> > maintenance IRQ\n");
> > +	}
> > +
> > +	gic_setup_kvm_info(maint_irq);
> >  }
> > +#endif
> 
> nit: add '// CONFIG_ACPI' here, for consistency.

And done.

> 
> >  #else
> >  static inline void __init gic_of_setup_kvm_info(struct device_node
> > *node)
> >  {
> >  }
> > +
> > +#ifdef CONFIG_ACPI
> > +static inline void __init gic_acpi_setup_kvm_info(void)
> > +{
> > +}
> > +#endif
> >  #endif // CONFIG_KVM
> >  
> >  static int __init gicv5_init_common(struct fwnode_handle
> > *parent_domain)
> > @@ -1265,6 +1356,7 @@ static int __init gic_acpi_init(union
> > acpi_subtable_headers *header, const unsig
> >  		goto out_irs;
> >  
> >  	acpi_set_irq_model(ACPI_IRQ_MODEL_GIC_V5,
> > gic_v5_get_gsi_domain_id);
> > +	gic_acpi_setup_kvm_info();
> >  
> >  	return 0;
> >  
> 
> Thanks,
> 
> 	M.
> 
Thanks,
Sascha