[PATCH v8 11/16] irqchip/gic-v3: Add support for ACPI's disabled but 'online capable' CPUs

[Marc Zyngier]

On Fri, 26 Apr 2024 19:28:58 +0100,
Jonathan Cameron <Jonathan.Cameron at Huawei.com> wrote:
> 
> 
> I'll not send a formal v9 until early next week, so here is the current state
> if you have time to take another look before then.

Don't bother resending this on my account -- you only sent it on
Friday and there hasn't been much response to it yet. There is still a
problem (see below), but looks otherwise OK.

[...]

> @@ -2363,11 +2381,25 @@ gic_acpi_parse_madt_gicc(union acpi_subtable_headers *header,
>  				(struct acpi_madt_generic_interrupt *)header;
>  	u32 reg = readl_relaxed(acpi_data.dist_base + GICD_PIDR2) & GIC_PIDR2_ARCH_MASK;
>  	u32 size = reg == GIC_PIDR2_ARCH_GICv4 ? SZ_64K * 4 : SZ_64K * 2;
> +	int cpu = get_cpu_for_acpi_id(gicc->uid);

I already commented that get_cpu_for_acpi_id() can...

>  	void __iomem *redist_base;
>  
> -	if (!acpi_gicc_is_usable(gicc))
> +	/* Neither enabled or online capable means it doesn't exist, skip it */
> +	if (!(gicc->flags & (ACPI_MADT_ENABLED | ACPI_MADT_GICC_ONLINE_CAPABLE)))
>  		return 0;
>  
> +	/*
> +	 * Capable but disabled CPUs can be brought online later. What about
> +	 * the redistributor? ACPI doesn't want to say!
> +	 * Virtual hotplug systems can use the MADT's "always-on" GICR entries.
> +	 * Otherwise, prevent such CPUs from being brought online.
> +	 */
> +	if (!(gicc->flags & ACPI_MADT_ENABLED)) {
> +		pr_warn("CPU %u's redistributor is inaccessible: this CPU can't be brought online\n", cpu);
> +		cpumask_set_cpu(cpu, &broken_rdists);

... return -EINVAL, and then be passed to cpumask_set_cpu(), with
interesting effects. It shouldn't happen, but I trust anything that
comes from firmware tables as much as I trust a campaigning
politician's promises. This should really result in the RD being
considered unusable, but without affecting any CPU (there is no valid
CPU the first place).

Another question is what get_cpu_for acpi_id() returns for a disabled
CPU. A valid CPU number? Or -EINVAL?

Thanks,

	M.

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