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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Sun, 28 Apr 2024 12:28:03 +0100
From: Marc Zyngier <maz@...nel.org>
To: Jonathan Cameron <Jonathan.Cameron@...wei.com>
Cc: Thomas Gleixner <tglx@...utronix.de>,
	Peter Zijlstra
	<peterz@...radead.org>,
	<linux-pm@...r.kernel.org>,
	<loongarch@...ts.linux.dev>,
	<linux-acpi@...r.kernel.org>,
	<linux-arch@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>,
	<linux-arm-kernel@...ts.infradead.org>,
	<kvmarm@...ts.linux.dev>,
	<x86@...nel.org>,
	Russell King <linux@...linux.org.uk>,
	"Rafael J . Wysocki"
	<rafael@...nel.org>,
	Miguel Luis <miguel.luis@...cle.com>,
	"James Morse"
	<james.morse@....com>,
	Salil Mehta <salil.mehta@...wei.com>,
	Jean-Philippe
 Brucker <jean-philippe@...aro.org>,
	Catalin Marinas
	<catalin.marinas@....com>,
	Will Deacon <will@...nel.org>,
	Hanjun Guo
	<guohanjun@...wei.com>,
	Ingo Molnar <mingo@...hat.com>,
	Borislav Petkov
	<bp@...en8.de>,
	Dave Hansen <dave.hansen@...ux.intel.com>,
	<linuxarm@...wei.com>,
	<justin.he@....com>,
	<jianyong.wu@....com>,
	"Lorenzo\
 Pieralisi" <lpieralisi@...nel.org>,
	Sudeep Holla <sudeep.holla@....com>
Subject: Re: [PATCH v8 11/16] irqchip/gic-v3: Add support for ACPI's disabled but 'online capable' CPUs

On Fri, 26 Apr 2024 19:28:58 +0100,
Jonathan Cameron <Jonathan.Cameron@...wei.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.