|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v5 2/2] PCI/TPH: Fix get cpu steer-tag fail on ARM64 platform
On Tue, Mar 10, 2026 at 10:58:49AM -0500, Jeremy Linton wrote:
> On 3/9/26 10:20 PM, Chengwen Feng wrote:
> > pcie_tph_get_cpu_st() is broken on ARM64:
> > 1. pcie_tph_get_cpu_st() passes cpu_uid to the PCI ACPI DSM method.
> > cpu_uid should be the ACPI Processor UID [1].
> > 2. In BNXT, pcie_tph_get_cpu_st() is passed a cpu_uid obtained via
> > cpumask_first(irq->cpu_mask) - the logical CPU ID of a CPU core,
> > generated and managed by kernel (e.g., [0,255] for a system with 256
> > logical CPU cores).
> > 3. On ARM64 platforms, ACPI assigns Processor UID to cores listed in the
> > MADT table, and this UID may not match the kernel's logical CPU ID.
> > When this occurs, the mismatch results in the wrong CPU steer-tag.
> > 4. On AMD x86 the logical CPU ID is identical to the ACPI Processor UID
> > so the mismatch is not seen.
> > int pcie_tph_get_cpu_st(struct pci_dev *pdev, enum tph_mem_type mem_type,
> > - unsigned int cpu_uid, u16 *tag)
> > + unsigned int cpu, u16 *tag)
> > {
> > #ifdef CONFIG_ACPI
> > + u32 cpu_uid = acpi_get_cpu_acpi_id(cpu);
>From AI review (gemini/gemini-3.1-pro-preview):
Does this code need to validate that `cpu` is within bounds before
using it? Before this change, the `cpu_uid` parameter was passed
opaquely to the ACPI firmware via `tph_invoke_dsm()`, which would
gracefully handle invalid values.
Now, `cpu` is treated as a logical CPU index and passed to
`acpi_get_cpu_acpi_id(cpu)`. On architectures like arm64 and riscv,
`acpi_get_cpu_acpi_id()` uses `cpu` directly as an array index
(`&cpu_madt_gicc[cpu]` and `&cpu_madt_rintc[cpu]`). On x86, it uses
`per_cpu(x86_cpu_to_acpiid, cpu)`.
If a caller passes an out-of-bounds `cpu` index (for example, if an
IRQ affinity mask is empty and `cpumask_first()` returns
`nr_cpu_ids`, or if userspace passes an arbitrary ID via
`mlx5_st_alloc_index()`), this will result in an out-of-bounds
memory read.
Consider adding a bounds check:
if (cpu >= nr_cpu_ids)
return -EINVAL;
I agree that this is an issue, and I think implementations of
acpi_get_cpu_acpi_id() should validate their inputs.
I don't know if there's a value that can never be a valid ACPI CPU UID
and could be used as an error value from acpi_get_cpu_acpi_id(). I do
see a few mentions of a ~0 value meaning "all processors" (ACPI r6.6,
sec 5.2.12.13).
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |