Re: [PATCH v1] arch_topology: Introduce nr_possible_packages

[Feng Tang]

Hi Sudeep,

Many thanks for the review!

On Wed, May 27, 2026 at 02:53:24PM +0100, Sudeep Holla wrote:
> On Fri, May 15, 2026 at 10:44:35PM +0800, Feng Tang wrote:
> > In multi-sockets platform, kernel or driver code may need the number
> > of packages for chosing different code directions. Some architecture
> > already provide such kind of interface like x86, which is being used
> > in some architecture code and drivers.
> > 
> > Add similar interface 'nr_possible_packages' for platforms which can
> > get package topology information by parsing ACPI tables in boot phase,
> > which was verified to show the correct number of packages on some
> > 1-socket and 2-sockets production arm64 servers from different vendors.
> > 
> 
> Sorry, but without a clear demonstration of how and where you plan to use
> this, it is very difficult to provide meaningful feedback or review this patch
> in isolation.

Yes, Greg asked similar thing. We had 2 main use cases locally: 
* some arm64 PMU driver need this info to program register accordingly.
* We met some arch_timer un-synced bug cross socket, and need it to
  judge whether to start a timer sanity check in boot phase (very
  similar as the TSC timer check for x86)

Patch for first item hasn't been posted as the HW is not public yet,
while the patch for 2nd is tested and WIP for upstream posting.

> I am concerned that this approach may be based on another architecture, while
> arm64 might have a better alternative due to how the firmware description is
> structured.

My bad that the commit log is confusing. I mentioned x86 just to show
other architecture has similar requirement, while this patch's
implementation has nothing to do with x86.

> Please provide more details so I can review this properly.
> 
> > Signed-off-by: Feng Tang <feng.tang@xxxxxxxxxxxxxxxxx>
> > ---
> > Changelog:
> > 
> >   since RFC:
> >       * use EXPORT_SYMBOL_GPL instead of EXPORT_SYMBOL (Greg)
> > 	  * change the possible max package ID to 2047 (Greg)
> > 	  * remove the CONFIG_ARM64/RISCV limit for 'nr_possible_packages' (Greg)
> > 	
> >   RFC: https://lore.kernel.org/lkml/20260512150505.43871-1-feng.tang@xxxxxxxxxxxxxxxxx/ 
> > 
> >  drivers/base/arch_topology.c  | 21 +++++++++++++++++++++
> >  include/linux/arch_topology.h |  1 +
> >  2 files changed, 22 insertions(+)
[...]
> > +/*
> > + * Assuming silicon has a sane package ID decoding method to not have
> > + * an ID bigger than 2047
> > + */
> > +#define MAX_PACKAGE_ID	2047
> > +DECLARE_BITMAP(package_id_mask, MAX_PACKAGE_ID + 1);
> > +
> >  #if defined(CONFIG_ARM64) || defined(CONFIG_RISCV)
> >  struct cpu_smt_info {
> >  	unsigned int thread_num;
> > @@ -912,6 +922,13 @@ __weak int __init parse_acpi_topology(void)
> >  		cpu_topology[cpu].cluster_id = topology_id;
> >  		topology_id = find_acpi_cpu_topology_package(cpu);
> >  		cpu_topology[cpu].package_id = topology_id;
> > +
> > +
> > +		if (topology_id >= 0 && topology_id <= MAX_PACKAGE_ID)
> > +			bitmap_set(package_id_mask, topology_id, 1);
> > +		else
> > +			pr_warn("ACPI: abnormal package ID: %d !\n",
> > +				topology_id);
> >  	}
> >
> 
> MAX_PACKAGE_ID is set to 2047 yet it reads from PPTT which has no such
> restriction. So it completely falls apart there as UID is just 32-bit
> number IIUC. If UID is not present we compute offset which is sparse
> and even that can be > 2047.

Yes. In theory, the package ID could be 0 or 0xffffffff (though I
don't think such package ID is reasonable :)).

And for this, we may give up the bitmap way, and use other way to
count number of packages, like create a package ID array

	static u32 fresh_pkg_id[1 << CONFIG_NODES_SHIFT];

to save fresh IDs during the PPTT table parsing, and then count
the number of valid ones.

> >  	/*
> > @@ -927,6 +944,10 @@ __weak int __init parse_acpi_topology(void)
> >  
> >  	cpu_smt_set_num_threads(max_smt_thread_num, max_smt_thread_num);
> >  	xa_destroy(&hetero_cpu);
> > +
> > +	/* Count the number of possible packages in system */
> > +	nr_possible_packages = bitmap_weight(package_id_mask, MAX_PACKAGE_ID + 1);
> > +	pr_info("ACPI: System has %u package(s) detected\n", nr_possible_packages);
> >  	return 0;
> >  }
> 
> IIUC x86 does not expose raw firmware package IDs as an array bound.  It builds
> topology domain bitmaps and derives dense logical package IDs from them.

You are right. IIUC, x86 uses its own way to build up the topology mainly by
parsing MADT table in early boot phase.

And arch_topology.c of this patch is under CONFIG_GENRIC_ARCH_TOPOLOGY, and
was selected by arm64/arm/riscv/parisc architectures.

> >  
> > diff --git a/include/linux/arch_topology.h b/include/linux/arch_topology.h
> > index ebd7f8935f96..960045e6ac05 100644
> > --- a/include/linux/arch_topology.h
> > +++ b/include/linux/arch_topology.h
> > @@ -105,6 +105,7 @@ static inline bool topology_core_has_smt(int cpu)
> >  	return cpu_topology[cpu].thread_id != -1;
> >  }
> >  
> > +extern unsigned int nr_possible_packages;
> 
> It does not provide a logical package ID mapping corresponding to the count.
> A consumer that uses nr_possible_packages for allocation and
> topology_physical_package_id(cpu) for indexing can still index out of bounds
> on sparse ACPI Processor IDs or on PPTT offset-based IDs.

Yes. And for the user cases I mentioned earlier, users don't need the
logical/physical package ID, but the number of packages. 

Thanks,
Feng