[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).  



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.