[PATCH, RFC] Create 'slot' sysfs attribute in /sys/devices/system/cpu/cpuN/topology/

[PATCH, RFC] Create 'slot' sysfs attribute in /sys/devices/system/cpu/cpuN/topology/

[Alex Chiang]

Hi Len, Tony,

A while back, I submitted a patch to expose the value of the _SUN
method for CPUs. On ia64, the "physical id" field in
/proc/cpuinfo isn't sufficient for some legacy HP ia64 platforms,
as they do not have a modern SAL or PAL.

	http://lkml.org/lkml/2007/10/30/529

That patch wasn't so great because as Len pointed out, /proc/acpi
is a deprecated interface.

Here is another try that attempts to present a generic "slot"
attribute in sysfs that leaves the various architectures free to
implement however they like.

Yes, I am aware of the physical_package_id attribute. However,
that doesn't necessarily map to a slot on the system board.
Additionally, it's broken on the same ia64 systems where
/proc/cpuinfo is broken, because it's trying to read the same
field. :-/

On ia64, I have a somewhat ugly implementation, where the ACPI
processor driver is filling in the answer. I chose this approach
because I couldn't figure out how to do it in the base CPU
driver, since calling the _SUN method requires the ACPI
interpreter to be initialized, and it didn't look like that was
the case when we did things like identify_siblings(), for
instance.

Finally, there is something that looks odd in struct
cpuinfo_ia64, where there are now two #ifdef CONFIG_SMP
statements. I did this because adding the new 'slot' member
created a 4-byte hole in the struct. It felt a little cleaner to
me to move the 'int cpu;' member upwards to fill in the hole, and
then my 'slot' could take the spot of 'cpu'.

I'd like to get feedback on both:

	- the concept in general of creating a new sysfs
	  attribute

	- the ia64 implementation, specifically

Thanks.

/ac

From: Alex Chiang <achiang@hp.com>

Physical slot information for CPUs is important for many reasons,
including managability. For instance, if a platform supports
physical CPU hotplug, it would be nice for the user to know which
physical CPU module to actually unplug after offlining it.

The "physical id" field in /proc/cpuinfo is broken/missing on
older ia64 platforms that do not have modern firmware.

/sys/devices/system/cpu/cpuN/topology/physical_package_id exists,
but is broken on those same platforms because it's simply reading
the same field that would have been presented in /proc/cpuinfo.

Create a 'slot' attribute and define an interface that may be
populated via alternate sources, without breaking existing
functionality.

Implement this interface on ia64 by invoking the _SUN method on a
per cpu basis.

Leave other archs alone for now.

Signed-off-by: Alex Chiang <achiang@hp.com>
---
 arch/ia64/kernel/setup.c      |    1 +
 arch/ia64/kernel/topology.c   |    6 ++++++
 drivers/acpi/processor_core.c |    5 +++++
 drivers/base/topology.c       |    9 +++++++++
 include/asm-ia64/cpu.h        |    1 +
 include/asm-ia64/processor.h  |    7 ++++++-
 include/asm-ia64/topology.h   |    1 +
 include/asm-x86/cpu.h         |    1 +
 8 files changed, 30 insertions(+), 1 deletions(-)

diff --git a/arch/ia64/kernel/setup.c b/arch/ia64/kernel/setup.c
index ebd1a09..34de68a 100644
--- a/arch/ia64/kernel/setup.c
+++ b/arch/ia64/kernel/setup.c
@@ -735,6 +735,7 @@ identify_cpu (struct cpuinfo_ia64 *c)
 	 */
 	c->threads_per_core = c->cores_per_socket = c->num_log = 1;
 	c->socket_id = -1;
+	c->slot = -1;
 
 	identify_siblings(c);
 
diff --git a/arch/ia64/kernel/topology.c b/arch/ia64/kernel/topology.c
index a2484fc..3810b48 100644
--- a/arch/ia64/kernel/topology.c
+++ b/arch/ia64/kernel/topology.c
@@ -27,6 +27,12 @@
 
 static struct ia64_cpu *sysfs_cpus;
 
+void arch_set_topology_slot(int num, u32 slot)
+{
+	cpu_data(num)->slot = slot;
+}
+EXPORT_SYMBOL(arch_set_topology_slot);
+
 int arch_register_cpu(int num)
 {
 #if defined (CONFIG_ACPI) && defined (CONFIG_HOTPLUG_CPU)
diff --git a/drivers/acpi/processor_core.c b/drivers/acpi/processor_core.c
index a3cc8a9..89a9ad5 100644
--- a/drivers/acpi/processor_core.c
+++ b/drivers/acpi/processor_core.c
@@ -590,6 +590,11 @@ static int acpi_processor_get_info(struct acpi_processor *pr, unsigned has_uid)
 	ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Processor [%d:%d]\n", pr->id,
 			  pr->acpi_id));
 
+	status = acpi_evaluate_object(pr->handle, "_SUN", NULL, &buffer);
+	if (ACPI_FAILURE(status))
+		object.integer.value = -1;
+	arch_set_topology_slot(pr->id, object.integer.value);
+
 	if (!object.processor.pblk_address)
 		ACPI_DEBUG_PRINT((ACPI_DB_INFO, "No PBLK (NULL address)\n"));
 	else if (object.processor.pblk_length != 6)
diff --git a/drivers/base/topology.c b/drivers/base/topology.c
index e1d3ad4..4fcffca 100644
--- a/drivers/base/topology.c
+++ b/drivers/base/topology.c
@@ -57,6 +57,14 @@ define_one_ro(physical_package_id);
 #define ref_physical_package_id_attr
 #endif
 
+#ifdef	topology_slot
+define_id_show_func(slot);
+define_one_ro(slot);
+#define ref_slot_attr			&attr_slot.attr,
+#else
+#define ref_slot_attr
+#endif
+
 #ifdef topology_core_id
 define_id_show_func(core_id);
 define_one_ro(core_id);
@@ -83,6 +91,7 @@ define_one_ro(core_siblings);
 
 static struct attribute *default_attrs[] = {
 	ref_physical_package_id_attr
+	ref_slot_attr
 	ref_core_id_attr
 	ref_thread_siblings_attr
 	ref_core_siblings_attr
diff --git a/include/asm-ia64/cpu.h b/include/asm-ia64/cpu.h
index e87fa32..393e545 100644
--- a/include/asm-ia64/cpu.h
+++ b/include/asm-ia64/cpu.h
@@ -14,6 +14,7 @@ DECLARE_PER_CPU(struct ia64_cpu, cpu_devices);
 
 DECLARE_PER_CPU(int, cpu_state);
 
+extern void arch_set_topology_slot(int num, u32 slot);
 extern int arch_register_cpu(int num);
 #ifdef CONFIG_HOTPLUG_CPU
 extern void arch_unregister_cpu(int);
diff --git a/include/asm-ia64/processor.h b/include/asm-ia64/processor.h
index 741f7ec..066553c 100644
--- a/include/asm-ia64/processor.h
+++ b/include/asm-ia64/processor.h
@@ -125,6 +125,10 @@ struct ia64_psr {
  */
 struct cpuinfo_ia64 {
 	__u32 softirq_pending;
+
+#ifdef CONFIG_SMP
+	int cpu;
+#endif
 	__u64 itm_delta;	/* # of clock cycles between clock ticks */
 	__u64 itm_next;		/* interval timer mask value to use for next clock tick */
 	__u64 nsec_per_cyc;	/* (1000000000<<IA64_NSEC_PER_CYC_SHIFT)/itc_freq */
@@ -140,7 +144,6 @@ struct cpuinfo_ia64 {
 
 #ifdef CONFIG_SMP
 	__u64 loops_per_jiffy;
-	int cpu;
 	__u32 socket_id;	/* physical processor socket id */
 	__u16 core_id;		/* core id */
 	__u16 thread_id;	/* thread id */
@@ -150,6 +153,8 @@ struct cpuinfo_ia64 {
 	__u8  threads_per_core;	/* Threads per core */
 #endif
 
+	__u32 slot;		/* physical slot; can differ from socket_id */
+
 	/* CPUID-derived information: */
 	__u64 ppn;
 	__u64 features;
diff --git a/include/asm-ia64/topology.h b/include/asm-ia64/topology.h
index 2d67b72..3b0ab1c 100644
--- a/include/asm-ia64/topology.h
+++ b/include/asm-ia64/topology.h
@@ -110,6 +110,7 @@ void build_cpu_to_node_map(void);
 
 #ifdef CONFIG_SMP
 #define topology_physical_package_id(cpu)	(cpu_data(cpu)->socket_id)
+#define topology_slot(cpu)			(cpu_data(cpu)->slot)
 #define topology_core_id(cpu)			(cpu_data(cpu)->core_id)
 #define topology_core_siblings(cpu)		(cpu_core_map[cpu])
 #define topology_thread_siblings(cpu)		(per_cpu(cpu_sibling_map, cpu))
diff --git a/include/asm-x86/cpu.h b/include/asm-x86/cpu.h
index 73f2ea8..43132a2 100644
--- a/include/asm-x86/cpu.h
+++ b/include/asm-x86/cpu.h
@@ -11,6 +11,7 @@ struct x86_cpu {
 	struct cpu cpu;
 };
 
+extern void arch_set_topology_slot(int num, u32 slot);
 #ifdef CONFIG_HOTPLUG_CPU
 extern int arch_register_cpu(int num);
 extern void arch_unregister_cpu(int);
-- 
1.5.3.1.g1e61

RE: [PATCH, RFC] Create 'slot' sysfs attribute in/sys/devices/system/cpu/cpuN/topology/

[Luck, Tony]

> A while back, I submitted a patch to expose the value of the _SUN
> method for CPUs. On ia64, the "physical id" field in
> /proc/cpuinfo isn't sufficient for some legacy HP ia64 platforms,
> as they do not have a modern SAL or PAL.

Do these legacy systems with ancient SAL/PAL actually support
processor hot plug?

Perhaps this next rant is beyond the scope of Linux architecture
and more on overall systems useabilty.  It is just asking for
trouble to print a message on the console telling the user to go
pull out the board in "slot 2".  Do the slot numbers count from
"0" or from "1"?  Are they numbered left-to-right or right-to-left?
What if the rack is non-standard so the system was rotated 90
degrees?  Now are they numbered top-to-bottom or bottom-to-top.

Best idea is to have a (blinking) (red) light on the board and
instruct the operator to pull out the board with the fail-light
(but even then a red-green colour blind operator will still mess
it up for you and pull the wrong board :-)

-Tony

Re: [PATCH, RFC] Create 'slot' sysfs attribute in/sys/devices/system/cpu/cpuN/topology/

[Matthew Wilcox]

On Tue, Mar 11, 2008 at 10:31:43AM -0700, Luck, Tony wrote:
> Perhaps this next rant is beyond the scope of Linux architecture
> and more on overall systems useabilty.  It is just asking for
> trouble to print a message on the console telling the user to go
> pull out the board in "slot 2".  Do the slot numbers count from
> "0" or from "1"?  Are they numbered left-to-right or right-to-left?
> What if the rack is non-standard so the system was rotated 90
> degrees?  Now are they numbered top-to-bottom or bottom-to-top.

Some systems (particularly those designed for hotplug) have the numbers
silk-screened onto the case.  It's like PCI slots ... my beige-box
homebrew PCs don't have slot numbers written on them, but the machines I
have from HP are very clearly labelled with slot numbers beside each PCI
slot.

Admittedly, the flashing red LED is a better UI, but it's not
necessarily as confusing as you're painting it.

-- 
Intel are signing my paycheques ... these opinions are still mine
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."

Re: [PATCH, RFC] Create 'slot' sysfs attribute in/sys/devices/system/cpu/cpuN/topology/

[Alex Chiang]

* Luck, Tony <tony.luck@intel.com>:
> > A while back, I submitted a patch to expose the value of the _SUN
> > method for CPUs. On ia64, the "physical id" field in
> > /proc/cpuinfo isn't sufficient for some legacy HP ia64 platforms,
> > as they do not have a modern SAL or PAL.
> 
> Do these legacy systems with ancient SAL/PAL actually support
> processor hot plug?

No, they do not; that was simply the most convenient / obvious
example I could think of for someone reading changelogs a few
years later and wondering why we created this sysfs attribute.

A real world example would be HP managability software keeping
track of CMC/CPE data for CPUs over a long period of time for the
purpose of doing long-term failure analysis.

This software can already grab a bunch of info from IPMI, such as
CPU serial number, date code, physical location, but cannot
correlate it to the kernel's idea of logical CPU number and
physloc.

Having the kernel expose the physical slot ties it all together.

This example is something any vendor selling higher level
managability software might want to do, and specifically HP wants
to do it to take care of our customers who have large deployments
of these legacy systems.

Hopefully that is enough justification to warrant at least
looking at the patch and telling me why it sucks. ;)

> Perhaps this next rant is beyond the scope of Linux architecture
> and more on overall systems useabilty.  It is just asking for
> trouble to print a message on the console telling the user to go
> pull out the board in "slot 2".  Do the slot numbers count from
> "0" or from "1"?  Are they numbered left-to-right or right-to-left?
> What if the rack is non-standard so the system was rotated 90
> degrees?  Now are they numbered top-to-bottom or bottom-to-top.

Well, at least for HP ia64 machines, we have lots of
silk-screening on the backplane, as well as pretty diagrams on
chassis covers, and we took care in making sure our what our
firmware matches up with those pictures.

But in general, I like the idea of das blinkenlights too. :)

> Best idea is to have a (blinking) (red) light on the board and
> instruct the operator to pull out the board with the fail-light
> (but even then a red-green colour blind operator will still
> mess it up for you and pull the wrong board :-)

Heh, then just make the failure light either blink (for failure)
or not blink (default). Then as long as the operator can detect
lumens, he/she would just be able to pull the blinky board. :)

Thanks.

/ac

RE: [PATCH, RFC] Create 'slot' sysfs attribute in/sys/devices/system/cpu/cpuN/topology/

[Luck, Tony]

> include/asm-x86/cpu.h         |    1 +

I assume this slipped in accidentally (though it would
eventually be needed for an x86 version of this patch).

> +#ifdef	topology_slot
> +define_id_show_func(slot);
> +define_one_ro(slot);
> +#define ref_slot_attr			&attr_slot.attr,
> +#else
> +#define ref_slot_attr
> +#endif

This is a bit ugly ... I don't see how to make it better (but
I'm sure that akpm will say "You could just ...", and then
I'll wonder why I didn't think of that).

> +EXPORT_SYMBOL(arch_set_topology_slot);

As the author you get to choose the license ... but there is
strong movement to make new exported symbols GPL only.

When I apply this patch and run it on my tiger, I see a slew
of

	ACPI: Invalid PBLK length [0]

messages on my console during boot and the see

	$ cat /sys/devices/system/cpu/cpu0/topology/slot
	-1

So I guess that the _SUN method isn't there on my machine.

-Tony

Re: [PATCH, RFC] Create 'slot' sysfs attribute in/sys/devices/system/cpu/cpuN/topology/

[Alex Chiang]

Hi Tony,

* Luck, Tony <tony.luck@intel.com>:
> > include/asm-x86/cpu.h         |    1 +
> 
> I assume this slipped in accidentally (though it would
> eventually be needed for an x86 version of this patch).

Well, it wasn't an accident, but I think I did screw up my
attempt at making this patch a nop on x86. The patch below should
have a better attempt at that.

> > +#ifdef	topology_slot
> > +define_id_show_func(slot);
> > +define_one_ro(slot);
> > +#define ref_slot_attr			&attr_slot.attr,
> > +#else
> > +#define ref_slot_attr
> > +#endif
> 
> This is a bit ugly ... I don't see how to make it better (but
> I'm sure that akpm will say "You could just ...", and then
> I'll wonder why I didn't think of that).

Hrm, I was just following a precedent here. I guess I'll wait for
enlightenment from akpm too. ;)

> > +EXPORT_SYMBOL(arch_set_topology_slot);
> 
> As the author you get to choose the license ... but there is
> strong movement to make new exported symbols GPL only.

Changed.

> When I apply this patch and run it on my tiger, I see a slew
> of
> 
> 	ACPI: Invalid PBLK length [0]
> 
> messages on my console during boot and the see

I don't think this should be coming from my patch. However, I did
update so that we're checking for a _SUN method underneath the
CPU before blinding stabbing around and trying to call it.

> 	$ cat /sys/devices/system/cpu/cpu0/topology/slot
> 	-1
> 
> So I guess that the _SUN method isn't there on my machine.

If you don't have _SUN on your machine, this version of the patch
should prevent those annoying messages from appearing.

However, you'll still see the sysfs attribute with a value of -1.
Personally, I don't think this is so bad, since on my legacy HP
machines, I get a -1 when I cat the 'physical_package_id'
attribute...

I'll defer to your judgement on this though.

/ac

From: Alex Chiang <achiang@hp.com>
Subject: [PATCH] Create 'slot' sysfs attribute in /sys/devices/system/cpu/cpuN/topology/

/sys/devices/system/cpu/cpuN/topology/physical_package_id exists,
but may contain incorrect information, especially on older ia64
platforms that do not have modern firmware.

Create a 'slot' attribute and define an interface that may be
populated via alternate sources, without breaking existing
functionality.

Implement this interface on ia64 by invoking the _SUN method on a
per cpu basis.

Leave other archs alone for now.

v1 -> v2:
	- Check for _SUN object on CPU before blindly attempting
	  to call it

Signed-off-by: Alex Chiang <achiang@hp.com>
---
 arch/ia64/kernel/setup.c      |    1 +
 arch/ia64/kernel/topology.c   |    6 ++++++
 drivers/acpi/processor_core.c |   11 +++++++++++
 drivers/base/topology.c       |    9 +++++++++
 include/asm-ia64/cpu.h        |    1 +
 include/asm-ia64/processor.h  |    7 ++++++-
 include/asm-ia64/topology.h   |    3 +++
 include/asm-x86/topology.h    |    2 ++
 8 files changed, 39 insertions(+), 1 deletions(-)

diff --git a/arch/ia64/kernel/setup.c b/arch/ia64/kernel/setup.c
index 4aa9eae..228d960 100644
--- a/arch/ia64/kernel/setup.c
+++ b/arch/ia64/kernel/setup.c
@@ -735,6 +735,7 @@ identify_cpu (struct cpuinfo_ia64 *c)
 	 */
 	c->threads_per_core = c->cores_per_socket = c->num_log = 1;
 	c->socket_id = -1;
+	c->slot = -1;
 
 	identify_siblings(c);
 
diff --git a/arch/ia64/kernel/topology.c b/arch/ia64/kernel/topology.c
index a2484fc..512de14 100644
--- a/arch/ia64/kernel/topology.c
+++ b/arch/ia64/kernel/topology.c
@@ -27,6 +27,12 @@
 
 static struct ia64_cpu *sysfs_cpus;
 
+void ia64_set_topology_slot(int num, u32 slot)
+{
+	cpu_data(num)->slot = slot;
+}
+EXPORT_SYMBOL_GPL(ia64_set_topology_slot);
+
 int arch_register_cpu(int num)
 {
 #if defined (CONFIG_ACPI) && defined (CONFIG_HOTPLUG_CPU)
diff --git a/drivers/acpi/processor_core.c b/drivers/acpi/processor_core.c
index 36a68fa..49fb2a0 100644
--- a/drivers/acpi/processor_core.c
+++ b/drivers/acpi/processor_core.c
@@ -509,6 +509,7 @@ static int get_cpu_id(acpi_handle handle, u32 acpi_id)
 static int acpi_processor_get_info(struct acpi_processor *pr, unsigned has_uid)
 {
 	acpi_status status = 0;
+	acpi_handle handle;
 	union acpi_object object = { 0 };
 	struct acpi_buffer buffer = { sizeof(union acpi_object), &object };
 	int cpu_index;
@@ -590,6 +591,16 @@ static int acpi_processor_get_info(struct acpi_processor *pr, unsigned has_uid)
 	ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Processor [%d:%d]\n", pr->id,
 			  pr->acpi_id));
 
+	if (ACPI_SUCCESS(acpi_get_handle(pr->handle, "_SUN", &handle))) {
+		status = acpi_evaluate_object(pr->handle,
+						"_SUN", NULL, &buffer);
+		if (ACPI_FAILURE(status))
+			object.integer.value = -1;
+	} else {
+		object.integer.value = -1;
+	}
+	arch_set_topology_slot(pr->id, object.integer.value);
+
 	if (!object.processor.pblk_address)
 		ACPI_DEBUG_PRINT((ACPI_DB_INFO, "No PBLK (NULL address)\n"));
 	else if (object.processor.pblk_length != 6)
diff --git a/drivers/base/topology.c b/drivers/base/topology.c
index e1d3ad4..4fcffca 100644
--- a/drivers/base/topology.c
+++ b/drivers/base/topology.c
@@ -57,6 +57,14 @@ define_one_ro(physical_package_id);
 #define ref_physical_package_id_attr
 #endif
 
+#ifdef	topology_slot
+define_id_show_func(slot);
+define_one_ro(slot);
+#define ref_slot_attr			&attr_slot.attr,
+#else
+#define ref_slot_attr
+#endif
+
 #ifdef topology_core_id
 define_id_show_func(core_id);
 define_one_ro(core_id);
@@ -83,6 +91,7 @@ define_one_ro(core_siblings);
 
 static struct attribute *default_attrs[] = {
 	ref_physical_package_id_attr
+	ref_slot_attr
 	ref_core_id_attr
 	ref_thread_siblings_attr
 	ref_core_siblings_attr
diff --git a/include/asm-ia64/cpu.h b/include/asm-ia64/cpu.h
index e87fa32..393e545 100644
--- a/include/asm-ia64/cpu.h
+++ b/include/asm-ia64/cpu.h
@@ -14,6 +14,7 @@ DECLARE_PER_CPU(struct ia64_cpu, cpu_devices);
 
 DECLARE_PER_CPU(int, cpu_state);
 
+extern void arch_set_topology_slot(int num, u32 slot);
 extern int arch_register_cpu(int num);
 #ifdef CONFIG_HOTPLUG_CPU
 extern void arch_unregister_cpu(int);
diff --git a/include/asm-ia64/processor.h b/include/asm-ia64/processor.h
index 741f7ec..066553c 100644
--- a/include/asm-ia64/processor.h
+++ b/include/asm-ia64/processor.h
@@ -125,6 +125,10 @@ struct ia64_psr {
  */
 struct cpuinfo_ia64 {
 	__u32 softirq_pending;
+
+#ifdef CONFIG_SMP
+	int cpu;
+#endif
 	__u64 itm_delta;	/* # of clock cycles between clock ticks */
 	__u64 itm_next;		/* interval timer mask value to use for next clock tick */
 	__u64 nsec_per_cyc;	/* (1000000000<<IA64_NSEC_PER_CYC_SHIFT)/itc_freq */
@@ -140,7 +144,6 @@ struct cpuinfo_ia64 {
 
 #ifdef CONFIG_SMP
 	__u64 loops_per_jiffy;
-	int cpu;
 	__u32 socket_id;	/* physical processor socket id */
 	__u16 core_id;		/* core id */
 	__u16 thread_id;	/* thread id */
@@ -150,6 +153,8 @@ struct cpuinfo_ia64 {
 	__u8  threads_per_core;	/* Threads per core */
 #endif
 
+	__u32 slot;		/* physical slot; can differ from socket_id */
+
 	/* CPUID-derived information: */
 	__u64 ppn;
 	__u64 features;
diff --git a/include/asm-ia64/topology.h b/include/asm-ia64/topology.h
index 2d67b72..f986693 100644
--- a/include/asm-ia64/topology.h
+++ b/include/asm-ia64/topology.h
@@ -110,12 +110,15 @@ void build_cpu_to_node_map(void);
 
 #ifdef CONFIG_SMP
 #define topology_physical_package_id(cpu)	(cpu_data(cpu)->socket_id)
+#define topology_slot(cpu)			(cpu_data(cpu)->slot)
 #define topology_core_id(cpu)			(cpu_data(cpu)->core_id)
 #define topology_core_siblings(cpu)		(cpu_core_map[cpu])
 #define topology_thread_siblings(cpu)		(per_cpu(cpu_sibling_map, cpu))
 #define smt_capable() 				(smp_num_siblings > 1)
 #endif
 
+#define arch_set_topology_slot(num, slot)	ia64_set_topology_slot(num,slot)
+
 #include <asm-generic/topology.h>
 
 #endif /* _ASM_IA64_TOPOLOGY_H */
diff --git a/include/asm-x86/topology.h b/include/asm-x86/topology.h
index 8af05a9..d4c922c 100644
--- a/include/asm-x86/topology.h
+++ b/include/asm-x86/topology.h
@@ -180,6 +180,8 @@ extern cpumask_t cpu_coregroup_map(int cpu);
 #define topology_thread_siblings(cpu)		(per_cpu(cpu_sibling_map, cpu))
 #endif
 
+#define arch_set_topology_slot(num, slot)
+
 #ifdef CONFIG_SMP
 #define mc_capable()			(boot_cpu_data.x86_max_cores > 1)
 #define smt_capable()			(smp_num_siblings > 1)
-- 
1.5.3.1.g1e61

RE: [PATCH, RFC] Create 'slot' sysfs attributein/sys/devices/system/cpu/cpuN/topology/

[Luck, Tony]

+	if (ACPI_SUCCESS(acpi_get_handle(pr->handle, "_SUN", &handle))) {
+		status = acpi_evaluate_object(pr->handle,
+						"_SUN", NULL, &buffer);
+		if (ACPI_FAILURE(status))
+			object.integer.value = -1;
+	} else {
+		object.integer.value = -1;
+	}
+	arch_set_topology_slot(pr->id, object.integer.value);
+
 	if (!object.processor.pblk_address)
 		ACPI_DEBUG_PRINT((ACPI_DB_INFO, "No PBLK (NULL address)\n"));
 	else if (object.processor.pblk_length != 6)

I'm still seeing these messages:

 "ACPI: Invalid PBLK length [0]"

for every cpu.  Presumably because "object" is a union and when
you did the "object.integer.value = -1;" you scribbled on some bits
that were needed in the subsequent code.

-Tony

Re: [PATCH, RFC] Create 'slot' sysfs attributein/sys/devices/system/cpu/cpuN/topology/

[Alex Chiang]

Hi Tony,

* Luck, Tony <tony.luck@intel.com>:
> +	if (ACPI_SUCCESS(acpi_get_handle(pr->handle, "_SUN", &handle))) {
> +		status = acpi_evaluate_object(pr->handle,
> +						"_SUN", NULL, &buffer);
> +		if (ACPI_FAILURE(status))
> +			object.integer.value = -1;
> +	} else {
> +		object.integer.value = -1;
> +	}
> +	arch_set_topology_slot(pr->id, object.integer.value);
> +
>  	if (!object.processor.pblk_address)
>  		ACPI_DEBUG_PRINT((ACPI_DB_INFO, "No PBLK (NULL address)\n"));
>  	else if (object.processor.pblk_length != 6)
> 
> I'm still seeing these messages:
> 
>  "ACPI: Invalid PBLK length [0]"
> 
> for every cpu.  Presumably because "object" is a union and when
> you did the "object.integer.value = -1;" you scribbled on some bits
> that were needed in the subsequent code.

Yes, you're right. Sorry about the sloppy code.

Here is v3. I've verified that:

	- it's benign on tiger (does not create errors/console
	  noise). You'll still see the -1 if you cat
	  /sys/devices/system/cpu/cpuN/topology/slot, but you
	  would also get the -1 if you cat physical_package_id

	- it does the right thing on platforms that do supply
	  _SUN for processor objects. On my rx2600,
	  physical_package_id is -1, but I get valid values for
	  'slot' now.

Thanks.

/ac

Subject: [PATCH] Create 'slot' sysfs attribute in /sys/devices/system/cpu/cpuN/topology/
From: Alex Chiang <achiang@hp.com>

/sys/devices/system/cpu/cpuN/topology/physical_package_id exists,
but may contain incorrect information, especially on older ia64
platforms that do not have modern firmware.

Create a 'slot' attribute and define an interface that may be
populated via alternate sources, without breaking existing
functionality.

Implement this interface on ia64 by invoking the _SUN method on a
per cpu basis.

Leave other archs alone for now.

v2 -> v3:
	- Stop stomping on existing PBLK in the event of _SUN failure

v1 -> v2:
	- Check for _SUN object on CPU before blindly attempting
	  to call it

Signed-off-by: Alex Chiang <achiang@hp.com>
---
 arch/ia64/kernel/setup.c      |    1 +
 arch/ia64/kernel/topology.c   |    6 ++++++
 drivers/acpi/processor_core.c |    8 ++++++++
 drivers/base/topology.c       |    9 +++++++++
 include/asm-ia64/cpu.h        |    1 +
 include/asm-ia64/processor.h  |    7 ++++++-
 include/asm-ia64/topology.h   |    3 +++
 include/asm-x86/topology.h    |    2 ++
 8 files changed, 36 insertions(+), 1 deletions(-)

diff --git a/arch/ia64/kernel/setup.c b/arch/ia64/kernel/setup.c
index 4aa9eae..228d960 100644
--- a/arch/ia64/kernel/setup.c
+++ b/arch/ia64/kernel/setup.c
@@ -735,6 +735,7 @@ identify_cpu (struct cpuinfo_ia64 *c)
 	 */
 	c->threads_per_core = c->cores_per_socket = c->num_log = 1;
 	c->socket_id = -1;
+	c->slot = -1;
 
 	identify_siblings(c);
 
diff --git a/arch/ia64/kernel/topology.c b/arch/ia64/kernel/topology.c
index a2484fc..512de14 100644
--- a/arch/ia64/kernel/topology.c
+++ b/arch/ia64/kernel/topology.c
@@ -27,6 +27,12 @@
 
 static struct ia64_cpu *sysfs_cpus;
 
+void ia64_set_topology_slot(int num, u32 slot)
+{
+	cpu_data(num)->slot = slot;
+}
+EXPORT_SYMBOL_GPL(ia64_set_topology_slot);
+
 int arch_register_cpu(int num)
 {
 #if defined (CONFIG_ACPI) && defined (CONFIG_HOTPLUG_CPU)
diff --git a/drivers/acpi/processor_core.c b/drivers/acpi/processor_core.c
index 36a68fa..a3a740d 100644
--- a/drivers/acpi/processor_core.c
+++ b/drivers/acpi/processor_core.c
@@ -612,6 +612,14 @@ static int acpi_processor_get_info(struct acpi_processor *pr, unsigned has_uid)
 		request_region(pr->throttling.address, 6, "ACPI CPU throttle");
 	}
 
+	/*
+	 * If ACPI describes a slot number for this CPU, let's fill it in
+	 */
+	status = acpi_evaluate_object(pr->handle, "_SUN", NULL, &buffer);
+	if (ACPI_FAILURE(status))
+		object.integer.value = -1;
+	arch_set_topology_slot(pr->id, object.integer.value);
+
 	return 0;
 }
 
diff --git a/drivers/base/topology.c b/drivers/base/topology.c
index e1d3ad4..4fcffca 100644
--- a/drivers/base/topology.c
+++ b/drivers/base/topology.c
@@ -57,6 +57,14 @@ define_one_ro(physical_package_id);
 #define ref_physical_package_id_attr
 #endif
 
+#ifdef	topology_slot
+define_id_show_func(slot);
+define_one_ro(slot);
+#define ref_slot_attr			&attr_slot.attr,
+#else
+#define ref_slot_attr
+#endif
+
 #ifdef topology_core_id
 define_id_show_func(core_id);
 define_one_ro(core_id);
@@ -83,6 +91,7 @@ define_one_ro(core_siblings);
 
 static struct attribute *default_attrs[] = {
 	ref_physical_package_id_attr
+	ref_slot_attr
 	ref_core_id_attr
 	ref_thread_siblings_attr
 	ref_core_siblings_attr
diff --git a/include/asm-ia64/cpu.h b/include/asm-ia64/cpu.h
index e87fa32..393e545 100644
--- a/include/asm-ia64/cpu.h
+++ b/include/asm-ia64/cpu.h
@@ -14,6 +14,7 @@ DECLARE_PER_CPU(struct ia64_cpu, cpu_devices);
 
 DECLARE_PER_CPU(int, cpu_state);
 
+extern void arch_set_topology_slot(int num, u32 slot);
 extern int arch_register_cpu(int num);
 #ifdef CONFIG_HOTPLUG_CPU
 extern void arch_unregister_cpu(int);
diff --git a/include/asm-ia64/processor.h b/include/asm-ia64/processor.h
index 741f7ec..066553c 100644
--- a/include/asm-ia64/processor.h
+++ b/include/asm-ia64/processor.h
@@ -125,6 +125,10 @@ struct ia64_psr {
  */
 struct cpuinfo_ia64 {
 	__u32 softirq_pending;
+
+#ifdef CONFIG_SMP
+	int cpu;
+#endif
 	__u64 itm_delta;	/* # of clock cycles between clock ticks */
 	__u64 itm_next;		/* interval timer mask value to use for next clock tick */
 	__u64 nsec_per_cyc;	/* (1000000000<<IA64_NSEC_PER_CYC_SHIFT)/itc_freq */
@@ -140,7 +144,6 @@ struct cpuinfo_ia64 {
 
 #ifdef CONFIG_SMP
 	__u64 loops_per_jiffy;
-	int cpu;
 	__u32 socket_id;	/* physical processor socket id */
 	__u16 core_id;		/* core id */
 	__u16 thread_id;	/* thread id */
@@ -150,6 +153,8 @@ struct cpuinfo_ia64 {
 	__u8  threads_per_core;	/* Threads per core */
 #endif
 
+	__u32 slot;		/* physical slot; can differ from socket_id */
+
 	/* CPUID-derived information: */
 	__u64 ppn;
 	__u64 features;
diff --git a/include/asm-ia64/topology.h b/include/asm-ia64/topology.h
index 2d67b72..f986693 100644
--- a/include/asm-ia64/topology.h
+++ b/include/asm-ia64/topology.h
@@ -110,12 +110,15 @@ void build_cpu_to_node_map(void);
 
 #ifdef CONFIG_SMP
 #define topology_physical_package_id(cpu)	(cpu_data(cpu)->socket_id)
+#define topology_slot(cpu)			(cpu_data(cpu)->slot)
 #define topology_core_id(cpu)			(cpu_data(cpu)->core_id)
 #define topology_core_siblings(cpu)		(cpu_core_map[cpu])
 #define topology_thread_siblings(cpu)		(per_cpu(cpu_sibling_map, cpu))
 #define smt_capable() 				(smp_num_siblings > 1)
 #endif
 
+#define arch_set_topology_slot(num, slot)	ia64_set_topology_slot(num,slot)
+
 #include <asm-generic/topology.h>
 
 #endif /* _ASM_IA64_TOPOLOGY_H */
diff --git a/include/asm-x86/topology.h b/include/asm-x86/topology.h
index 8af05a9..d4c922c 100644
--- a/include/asm-x86/topology.h
+++ b/include/asm-x86/topology.h
@@ -180,6 +180,8 @@ extern cpumask_t cpu_coregroup_map(int cpu);
 #define topology_thread_siblings(cpu)		(per_cpu(cpu_sibling_map, cpu))
 #endif
 
+#define arch_set_topology_slot(num, slot)
+
 #ifdef CONFIG_SMP
 #define mc_capable()			(boot_cpu_data.x86_max_cores > 1)
 #define smt_capable()			(smp_num_siblings > 1)
-- 
1.5.3.1.g1e61

Re: [PATCH, RFC] Create 'slot' sysfs attributein/sys/devices/system/cpu/cpuN/topology/

[Alex Chiang]

Hi Tony,

I noticed that this patch wasn't in the git tree you sent to
Linus for 2.6.26. I don't remember seeing a NACK though -- is
there something that I could rework to make it more acceptable?

Thanks.

/ac

* Alex Chiang <achiang@hp.com>:
> Hi Tony,
> 
> * Luck, Tony <tony.luck@intel.com>:
> > +	if (ACPI_SUCCESS(acpi_get_handle(pr->handle, "_SUN", &handle))) {
> > +		status = acpi_evaluate_object(pr->handle,
> > +						"_SUN", NULL, &buffer);
> > +		if (ACPI_FAILURE(status))
> > +			object.integer.value = -1;
> > +	} else {
> > +		object.integer.value = -1;
> > +	}
> > +	arch_set_topology_slot(pr->id, object.integer.value);
> > +
> >  	if (!object.processor.pblk_address)
> >  		ACPI_DEBUG_PRINT((ACPI_DB_INFO, "No PBLK (NULL address)\n"));
> >  	else if (object.processor.pblk_length != 6)
> > 
> > I'm still seeing these messages:
> > 
> >  "ACPI: Invalid PBLK length [0]"
> > 
> > for every cpu.  Presumably because "object" is a union and when
> > you did the "object.integer.value = -1;" you scribbled on some bits
> > that were needed in the subsequent code.
> 
> Yes, you're right. Sorry about the sloppy code.
> 
> Here is v3. I've verified that:
> 
> 	- it's benign on tiger (does not create errors/console
> 	  noise). You'll still see the -1 if you cat
> 	  /sys/devices/system/cpu/cpuN/topology/slot, but you
> 	  would also get the -1 if you cat physical_package_id
> 
> 	- it does the right thing on platforms that do supply
> 	  _SUN for processor objects. On my rx2600,
> 	  physical_package_id is -1, but I get valid values for
> 	  'slot' now.
> 
> Thanks.
> 
> /ac
> 
> Subject: [PATCH] Create 'slot' sysfs attribute in /sys/devices/system/cpu/cpuN/topology/
> From: Alex Chiang <achiang@hp.com>
> 
> /sys/devices/system/cpu/cpuN/topology/physical_package_id exists,
> but may contain incorrect information, especially on older ia64
> platforms that do not have modern firmware.
> 
> Create a 'slot' attribute and define an interface that may be
> populated via alternate sources, without breaking existing
> functionality.
> 
> Implement this interface on ia64 by invoking the _SUN method on a
> per cpu basis.
> 
> Leave other archs alone for now.
> 
> v2 -> v3:
> 	- Stop stomping on existing PBLK in the event of _SUN failure
> 
> v1 -> v2:
> 	- Check for _SUN object on CPU before blindly attempting
> 	  to call it
> 
> Signed-off-by: Alex Chiang <achiang@hp.com>
> ---
>  arch/ia64/kernel/setup.c      |    1 +
>  arch/ia64/kernel/topology.c   |    6 ++++++
>  drivers/acpi/processor_core.c |    8 ++++++++
>  drivers/base/topology.c       |    9 +++++++++
>  include/asm-ia64/cpu.h        |    1 +
>  include/asm-ia64/processor.h  |    7 ++++++-
>  include/asm-ia64/topology.h   |    3 +++
>  include/asm-x86/topology.h    |    2 ++
>  8 files changed, 36 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/ia64/kernel/setup.c b/arch/ia64/kernel/setup.c
> index 4aa9eae..228d960 100644
> --- a/arch/ia64/kernel/setup.c
> +++ b/arch/ia64/kernel/setup.c
> @@ -735,6 +735,7 @@ identify_cpu (struct cpuinfo_ia64 *c)
>  	 */
>  	c->threads_per_core = c->cores_per_socket = c->num_log = 1;
>  	c->socket_id = -1;
> +	c->slot = -1;
>  
>  	identify_siblings(c);
>  
> diff --git a/arch/ia64/kernel/topology.c b/arch/ia64/kernel/topology.c
> index a2484fc..512de14 100644
> --- a/arch/ia64/kernel/topology.c
> +++ b/arch/ia64/kernel/topology.c
> @@ -27,6 +27,12 @@
>  
>  static struct ia64_cpu *sysfs_cpus;
>  
> +void ia64_set_topology_slot(int num, u32 slot)
> +{
> +	cpu_data(num)->slot = slot;
> +}
> +EXPORT_SYMBOL_GPL(ia64_set_topology_slot);
> +
>  int arch_register_cpu(int num)
>  {
>  #if defined (CONFIG_ACPI) && defined (CONFIG_HOTPLUG_CPU)
> diff --git a/drivers/acpi/processor_core.c b/drivers/acpi/processor_core.c
> index 36a68fa..a3a740d 100644
> --- a/drivers/acpi/processor_core.c
> +++ b/drivers/acpi/processor_core.c
> @@ -612,6 +612,14 @@ static int acpi_processor_get_info(struct acpi_processor *pr, unsigned has_uid)
>  		request_region(pr->throttling.address, 6, "ACPI CPU throttle");
>  	}
>  
> +	/*
> +	 * If ACPI describes a slot number for this CPU, let's fill it in
> +	 */
> +	status = acpi_evaluate_object(pr->handle, "_SUN", NULL, &buffer);
> +	if (ACPI_FAILURE(status))
> +		object.integer.value = -1;
> +	arch_set_topology_slot(pr->id, object.integer.value);
> +
>  	return 0;
>  }
>  
> diff --git a/drivers/base/topology.c b/drivers/base/topology.c
> index e1d3ad4..4fcffca 100644
> --- a/drivers/base/topology.c
> +++ b/drivers/base/topology.c
> @@ -57,6 +57,14 @@ define_one_ro(physical_package_id);
>  #define ref_physical_package_id_attr
>  #endif
>  
> +#ifdef	topology_slot
> +define_id_show_func(slot);
> +define_one_ro(slot);
> +#define ref_slot_attr			&attr_slot.attr,
> +#else
> +#define ref_slot_attr
> +#endif
> +
>  #ifdef topology_core_id
>  define_id_show_func(core_id);
>  define_one_ro(core_id);
> @@ -83,6 +91,7 @@ define_one_ro(core_siblings);
>  
>  static struct attribute *default_attrs[] = {
>  	ref_physical_package_id_attr
> +	ref_slot_attr
>  	ref_core_id_attr
>  	ref_thread_siblings_attr
>  	ref_core_siblings_attr
> diff --git a/include/asm-ia64/cpu.h b/include/asm-ia64/cpu.h
> index e87fa32..393e545 100644
> --- a/include/asm-ia64/cpu.h
> +++ b/include/asm-ia64/cpu.h
> @@ -14,6 +14,7 @@ DECLARE_PER_CPU(struct ia64_cpu, cpu_devices);
>  
>  DECLARE_PER_CPU(int, cpu_state);
>  
> +extern void arch_set_topology_slot(int num, u32 slot);
>  extern int arch_register_cpu(int num);
>  #ifdef CONFIG_HOTPLUG_CPU
>  extern void arch_unregister_cpu(int);
> diff --git a/include/asm-ia64/processor.h b/include/asm-ia64/processor.h
> index 741f7ec..066553c 100644
> --- a/include/asm-ia64/processor.h
> +++ b/include/asm-ia64/processor.h
> @@ -125,6 +125,10 @@ struct ia64_psr {
>   */
>  struct cpuinfo_ia64 {
>  	__u32 softirq_pending;
> +
> +#ifdef CONFIG_SMP
> +	int cpu;
> +#endif
>  	__u64 itm_delta;	/* # of clock cycles between clock ticks */
>  	__u64 itm_next;		/* interval timer mask value to use for next clock tick */
>  	__u64 nsec_per_cyc;	/* (1000000000<<IA64_NSEC_PER_CYC_SHIFT)/itc_freq */
> @@ -140,7 +144,6 @@ struct cpuinfo_ia64 {
>  
>  #ifdef CONFIG_SMP
>  	__u64 loops_per_jiffy;
> -	int cpu;
>  	__u32 socket_id;	/* physical processor socket id */
>  	__u16 core_id;		/* core id */
>  	__u16 thread_id;	/* thread id */
> @@ -150,6 +153,8 @@ struct cpuinfo_ia64 {
>  	__u8  threads_per_core;	/* Threads per core */
>  #endif
>  
> +	__u32 slot;		/* physical slot; can differ from socket_id */
> +
>  	/* CPUID-derived information: */
>  	__u64 ppn;
>  	__u64 features;
> diff --git a/include/asm-ia64/topology.h b/include/asm-ia64/topology.h
> index 2d67b72..f986693 100644
> --- a/include/asm-ia64/topology.h
> +++ b/include/asm-ia64/topology.h
> @@ -110,12 +110,15 @@ void build_cpu_to_node_map(void);
>  
>  #ifdef CONFIG_SMP
>  #define topology_physical_package_id(cpu)	(cpu_data(cpu)->socket_id)
> +#define topology_slot(cpu)			(cpu_data(cpu)->slot)
>  #define topology_core_id(cpu)			(cpu_data(cpu)->core_id)
>  #define topology_core_siblings(cpu)		(cpu_core_map[cpu])
>  #define topology_thread_siblings(cpu)		(per_cpu(cpu_sibling_map, cpu))
>  #define smt_capable() 				(smp_num_siblings > 1)
>  #endif
>  
> +#define arch_set_topology_slot(num, slot)	ia64_set_topology_slot(num,slot)
> +
>  #include <asm-generic/topology.h>
>  
>  #endif /* _ASM_IA64_TOPOLOGY_H */
> diff --git a/include/asm-x86/topology.h b/include/asm-x86/topology.h
> index 8af05a9..d4c922c 100644
> --- a/include/asm-x86/topology.h
> +++ b/include/asm-x86/topology.h
> @@ -180,6 +180,8 @@ extern cpumask_t cpu_coregroup_map(int cpu);
>  #define topology_thread_siblings(cpu)		(per_cpu(cpu_sibling_map, cpu))
>  #endif
>  
> +#define arch_set_topology_slot(num, slot)
> +
>  #ifdef CONFIG_SMP
>  #define mc_capable()			(boot_cpu_data.x86_max_cores > 1)
>  #define smt_capable()			(smp_num_siblings > 1)
> -- 
> 1.5.3.1.g1e61
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

RE: [PATCH, RFC] Create 'slot' sysfsattributein/sys/devices/system/cpu/cpuN/topology/

[Luck, Tony]

> I noticed that this patch wasn't in the git tree you sent to
> Linus for 2.6.26. I don't remember seeing a NACK though -- is
> there something that I could rework to make it more acceptable?

I'm mostly ok with this version of the patch.  I didn't see any
comments from the linux-kernel crowd on the whether they are fond
of the new API ("slot" file in cpu/cpuN/topology/) and hate to make
the presumption that because they are silent that they agree.

The "mostly ok" part would transform to "fully ok" if there were a way
to make sure the "slot" files only appeared on systems where they are
meaningful (i.e. have a value other then -1 in them).  If there is
an easy way to make this happen, then it would make me happier (less
clutter in /sys) and perhaps others too (since this API is only useful
on large systems where "slot" is meaningful, and there is generally
some bias from the community at large about adding interfaces that
aren't needed for normal desktop/laptop systems).

-Tony

Patch follows to save people digging in the archives to see what we
are talking about.


> Subject: [PATCH] Create 'slot' sysfs attribute in /sys/devices/system/cpu/cpuN/topology/
> From: Alex Chiang <achiang@hp.com>
> 
> /sys/devices/system/cpu/cpuN/topology/physical_package_id exists,
> but may contain incorrect information, especially on older ia64
> platforms that do not have modern firmware.
> 
> Create a 'slot' attribute and define an interface that may be
> populated via alternate sources, without breaking existing
> functionality.
> 
> Implement this interface on ia64 by invoking the _SUN method on a
> per cpu basis.
> 
> Leave other archs alone for now.
> 
> v2 -> v3:
> 	- Stop stomping on existing PBLK in the event of _SUN failure
> 
> v1 -> v2:
> 	- Check for _SUN object on CPU before blindly attempting
> 	  to call it
> 
> Signed-off-by: Alex Chiang <achiang@hp.com>
> ---
>  arch/ia64/kernel/setup.c      |    1 +
>  arch/ia64/kernel/topology.c   |    6 ++++++
>  drivers/acpi/processor_core.c |    8 ++++++++
>  drivers/base/topology.c       |    9 +++++++++
>  include/asm-ia64/cpu.h        |    1 +
>  include/asm-ia64/processor.h  |    7 ++++++-
>  include/asm-ia64/topology.h   |    3 +++
>  include/asm-x86/topology.h    |    2 ++
>  8 files changed, 36 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/ia64/kernel/setup.c b/arch/ia64/kernel/setup.c
> index 4aa9eae..228d960 100644
> --- a/arch/ia64/kernel/setup.c
> +++ b/arch/ia64/kernel/setup.c
> @@ -735,6 +735,7 @@ identify_cpu (struct cpuinfo_ia64 *c)
>  	 */
>  	c->threads_per_core = c->cores_per_socket = c->num_log = 1;
>  	c->socket_id = -1;
> +	c->slot = -1;
>  
>  	identify_siblings(c);
>  
> diff --git a/arch/ia64/kernel/topology.c b/arch/ia64/kernel/topology.c
> index a2484fc..512de14 100644
> --- a/arch/ia64/kernel/topology.c
> +++ b/arch/ia64/kernel/topology.c
> @@ -27,6 +27,12 @@
>  
>  static struct ia64_cpu *sysfs_cpus;
>  
> +void ia64_set_topology_slot(int num, u32 slot)
> +{
> +	cpu_data(num)->slot = slot;
> +}
> +EXPORT_SYMBOL_GPL(ia64_set_topology_slot);
> +
>  int arch_register_cpu(int num)
>  {
>  #if defined (CONFIG_ACPI) && defined (CONFIG_HOTPLUG_CPU)
> diff --git a/drivers/acpi/processor_core.c b/drivers/acpi/processor_core.c
> index 36a68fa..a3a740d 100644
> --- a/drivers/acpi/processor_core.c
> +++ b/drivers/acpi/processor_core.c
> @@ -612,6 +612,14 @@ static int acpi_processor_get_info(struct acpi_processor *pr, unsigned has_uid)
>  		request_region(pr->throttling.address, 6, "ACPI CPU throttle");
>  	}
>  
> +	/*
> +	 * If ACPI describes a slot number for this CPU, let's fill it in
> +	 */
> +	status = acpi_evaluate_object(pr->handle, "_SUN", NULL, &buffer);
> +	if (ACPI_FAILURE(status))
> +		object.integer.value = -1;
> +	arch_set_topology_slot(pr->id, object.integer.value);
> +
>  	return 0;
>  }
>  
> diff --git a/drivers/base/topology.c b/drivers/base/topology.c
> index e1d3ad4..4fcffca 100644
> --- a/drivers/base/topology.c
> +++ b/drivers/base/topology.c
> @@ -57,6 +57,14 @@ define_one_ro(physical_package_id);
>  #define ref_physical_package_id_attr
>  #endif
>  
> +#ifdef	topology_slot
> +define_id_show_func(slot);
> +define_one_ro(slot);
> +#define ref_slot_attr			&attr_slot.attr,
> +#else
> +#define ref_slot_attr
> +#endif
> +
>  #ifdef topology_core_id
>  define_id_show_func(core_id);
>  define_one_ro(core_id);
> @@ -83,6 +91,7 @@ define_one_ro(core_siblings);
>  
>  static struct attribute *default_attrs[] = {
>  	ref_physical_package_id_attr
> +	ref_slot_attr
>  	ref_core_id_attr
>  	ref_thread_siblings_attr
>  	ref_core_siblings_attr
> diff --git a/include/asm-ia64/cpu.h b/include/asm-ia64/cpu.h
> index e87fa32..393e545 100644
> --- a/include/asm-ia64/cpu.h
> +++ b/include/asm-ia64/cpu.h
> @@ -14,6 +14,7 @@ DECLARE_PER_CPU(struct ia64_cpu, cpu_devices);
>  
>  DECLARE_PER_CPU(int, cpu_state);
>  
> +extern void arch_set_topology_slot(int num, u32 slot);
>  extern int arch_register_cpu(int num);
>  #ifdef CONFIG_HOTPLUG_CPU
>  extern void arch_unregister_cpu(int);
> diff --git a/include/asm-ia64/processor.h b/include/asm-ia64/processor.h
> index 741f7ec..066553c 100644
> --- a/include/asm-ia64/processor.h
> +++ b/include/asm-ia64/processor.h
> @@ -125,6 +125,10 @@ struct ia64_psr {
>   */
>  struct cpuinfo_ia64 {
>  	__u32 softirq_pending;
> +
> +#ifdef CONFIG_SMP
> +	int cpu;
> +#endif
>  	__u64 itm_delta;	/* # of clock cycles between clock ticks */
>  	__u64 itm_next;		/* interval timer mask value to use for next clock tick */
>  	__u64 nsec_per_cyc;	/* (1000000000<<IA64_NSEC_PER_CYC_SHIFT)/itc_freq */
> @@ -140,7 +144,6 @@ struct cpuinfo_ia64 {
>  
>  #ifdef CONFIG_SMP
>  	__u64 loops_per_jiffy;
> -	int cpu;
>  	__u32 socket_id;	/* physical processor socket id */
>  	__u16 core_id;		/* core id */
>  	__u16 thread_id;	/* thread id */
> @@ -150,6 +153,8 @@ struct cpuinfo_ia64 {
>  	__u8  threads_per_core;	/* Threads per core */
>  #endif
>  
> +	__u32 slot;		/* physical slot; can differ from socket_id */
> +
>  	/* CPUID-derived information: */
>  	__u64 ppn;
>  	__u64 features;
> diff --git a/include/asm-ia64/topology.h b/include/asm-ia64/topology.h
> index 2d67b72..f986693 100644
> --- a/include/asm-ia64/topology.h
> +++ b/include/asm-ia64/topology.h
> @@ -110,12 +110,15 @@ void build_cpu_to_node_map(void);
>  
>  #ifdef CONFIG_SMP
>  #define topology_physical_package_id(cpu)	(cpu_data(cpu)->socket_id)
> +#define topology_slot(cpu)			(cpu_data(cpu)->slot)
>  #define topology_core_id(cpu)			(cpu_data(cpu)->core_id)
>  #define topology_core_siblings(cpu)		(cpu_core_map[cpu])
>  #define topology_thread_siblings(cpu)		(per_cpu(cpu_sibling_map, cpu))
>  #define smt_capable() 				(smp_num_siblings > 1)
>  #endif
>  
> +#define arch_set_topology_slot(num, slot)	ia64_set_topology_slot(num,slot)
> +
>  #include <asm-generic/topology.h>
>  
>  #endif /* _ASM_IA64_TOPOLOGY_H */
> diff --git a/include/asm-x86/topology.h b/include/asm-x86/topology.h
> index 8af05a9..d4c922c 100644
> --- a/include/asm-x86/topology.h
> +++ b/include/asm-x86/topology.h
> @@ -180,6 +180,8 @@ extern cpumask_t cpu_coregroup_map(int cpu);
>  #define topology_thread_siblings(cpu)		(per_cpu(cpu_sibling_map, cpu))
>  #endif
>  
> +#define arch_set_topology_slot(num, slot)
> +
>  #ifdef CONFIG_SMP
>  #define mc_capable()			(boot_cpu_data.x86_max_cores > 1)
>  #define smt_capable()			(smp_num_siblings > 1)
> -- 
> 1.5.3.1.g1e61
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH, RFC] Create 'slot' sysfsattributein/sys/devices/system/cpu/cpuN/topology/

[Alex Chiang]

Hi Tony,

* Luck, Tony <tony.luck@intel.com>:
> > I noticed that this patch wasn't in the git tree you sent to
> > Linus for 2.6.26. I don't remember seeing a NACK though -- is
> > there something that I could rework to make it more acceptable?
> 
> I'm mostly ok with this version of the patch.  I didn't see any
> comments from the linux-kernel crowd on the whether they are fond
> of the new API ("slot" file in cpu/cpuN/topology/) and hate to make
> the presumption that because they are silent that they agree.
> 
> The "mostly ok" part would transform to "fully ok" if there were a way
> to make sure the "slot" files only appeared on systems where they are
> meaningful (i.e. have a value other then -1 in them).  If there is
> an easy way to make this happen, then it would make me happier (less
> clutter in /sys) and perhaps others too (since this API is only useful
> on large systems where "slot" is meaningful, and there is generally
> some bias from the community at large about adding interfaces that
> aren't needed for normal desktop/laptop systems).

Yeah, I agree that the first few attempts weren't so great. I
don't really like the idea of adding a new 'slot' file to sysfs
either.

I reworked this patch to play nicer with the current topology
stuff.

I've also included a cleanup patch that you might be happy to see
(removes the noisy ia64_sal_pltid failed with -1 printk).

Tested on an hp rx4640 with Madisons and verified that I actually
get sane output in /proc/cpuinfo:

[root@max ~]# cat /proc/cpuinfo | grep processor
processor  : 0
processor  : 1
processor  : 2
processor  : 3
[root@max ~]# cat /proc/cpuinfo | grep physical
physical id: 0
physical id: 1
physical id: 2
physical id: 3

Also tested on a Tiger and verified that /proc/cpuinfo remains
unchanged, and no useless printk's appear during boot.

Thanks.

/ac

[PATCH 1/2] ia64: Remove printk noise on unimplemented SAL_PHYSICAL_ID_INFO

[Alex Chiang]

Commit 113134fcbca83619be4c68d0ca66db6093777b5d changed the flow of
control when calling PAL_LOGICAL_TO_PHYSICAL and SAL_PHYSICAL_ID_INFO.
With the change, if a platform did not implement the latter, a useless
printk would appear in the boot log:

	ia64_sal_pltid failed with -1

So let's check the return code and only printk on a true error, and do
not print anything in the unimplemented case. While we're in there,
clean up some stylistic issues too.

Signed-off-by: Alex Chiang <achiang@hp.com>
---
 arch/ia64/kernel/smpboot.c |   12 +++++++++---
 1 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/arch/ia64/kernel/smpboot.c b/arch/ia64/kernel/smpboot.c
index 16483be..8d86e1f 100644
--- a/arch/ia64/kernel/smpboot.c
+++ b/arch/ia64/kernel/smpboot.c
@@ -873,7 +873,8 @@ identify_siblings(struct cpuinfo_ia64 *c)
 	u16 pltid;
 	pal_logical_to_physical_t info;
 
-	if ((status = ia64_pal_logical_to_phys(-1, &info)) != PAL_STATUS_SUCCESS) {
+	status = ia64_pal_logical_to_phys(-1, &info);
+	if (status != PAL_STATUS_SUCCESS) {
 		if (status != PAL_STATUS_UNIMPLEMENTED) {
 			printk(KERN_ERR
 				"ia64_pal_logical_to_phys failed with %ld\n",
@@ -885,8 +886,13 @@ identify_siblings(struct cpuinfo_ia64 *c)
 		info.overview_cpp  = 1;
 		info.overview_tpc  = 1;
 	}
-	if ((status = ia64_sal_physical_id_info(&pltid)) != PAL_STATUS_SUCCESS) {
-		printk(KERN_ERR "ia64_sal_pltid failed with %ld\n", status);
+
+	status = ia64_sal_physical_id_info(&pltid);
+	if (status != PAL_STATUS_SUCCESS) {
+		if (status != PAL_STATUS_UNIMPLEMENTED)
+			printk(KERN_ERR 
+				"ia64_sal_pltid failed with %ld\n", 
+				status);
 		return;
 	}
 
-- 
1.5.3.1.g1e61

[PATCH 2/2] ia64: Provide ACPI fixup for /proc/cpuinfo/physical_id

[Alex Chiang]

Legacy HP ia64 platforms currently cannot provide
/proc/cpuinfo/physical_id due to legacy SAL/PAL implementations.
However, that physical topology information can be obtained
via ACPI.

Provide an interface that gives ACPI one last chance to provide
physical_id for these legacy platforms. This logic only comes
into play iff:

	- ACPI actually provides slot information for the CPU
	- we lack a valid socket_id

Otherwise, we don't do anything. Since x86 uses the ACPI processor
driver as well, we provide an empty #define.

Signed-off-by: Alex Chiang <achiang@hp.com>
---
 arch/ia64/kernel/topology.c   |    7 +++++++
 drivers/acpi/processor_core.c |    8 ++++++++
 include/asm-ia64/cpu.h        |    1 +
 include/asm-ia64/topology.h   |    1 +
 include/asm-x86/topology.h    |    2 ++
 5 files changed, 19 insertions(+), 0 deletions(-)

diff --git a/arch/ia64/kernel/topology.c b/arch/ia64/kernel/topology.c
index a2484fc..bb584b4 100644
--- a/arch/ia64/kernel/topology.c
+++ b/arch/ia64/kernel/topology.c
@@ -27,6 +27,13 @@
 
 static struct ia64_cpu *sysfs_cpus;
 
+void ia64_fix_socket_id(int num, u32 slot)
+{
+	if (cpu_data(num)->socket_id == -1)
+		cpu_data(num)->socket_id= slot;
+}
+EXPORT_SYMBOL_GPL(ia64_fix_socket_id);
+
 int arch_register_cpu(int num)
 {
 #if defined (CONFIG_ACPI) && defined (CONFIG_HOTPLUG_CPU)
diff --git a/drivers/acpi/processor_core.c b/drivers/acpi/processor_core.c
index a825b43..89578d4 100644
--- a/drivers/acpi/processor_core.c
+++ b/drivers/acpi/processor_core.c
@@ -612,6 +612,14 @@ static int acpi_processor_get_info(struct acpi_processor *pr, unsigned has_uid)
 		request_region(pr->throttling.address, 6, "ACPI CPU throttle");
 	}
 
+	/*
+	 * If ACPI describes a slot number for this CPU, we can use it
+	 * to fix a broken topology->physical_package_id.
+	 */
+	status = acpi_evaluate_object(pr->handle, "_SUN", NULL, &buffer);
+	if (ACPI_SUCCESS(status))
+		arch_fix_phys_package_id(pr->id, object.integer.value);
+
 	return 0;
 }
 
diff --git a/include/asm-ia64/cpu.h b/include/asm-ia64/cpu.h
index e87fa32..9ba5155 100644
--- a/include/asm-ia64/cpu.h
+++ b/include/asm-ia64/cpu.h
@@ -14,6 +14,7 @@ DECLARE_PER_CPU(struct ia64_cpu, cpu_devices);
 
 DECLARE_PER_CPU(int, cpu_state);
 
+extern void arch_fix_phys_package_id(int num, u32 slot);
 extern int arch_register_cpu(int num);
 #ifdef CONFIG_HOTPLUG_CPU
 extern void arch_unregister_cpu(int);
diff --git a/include/asm-ia64/topology.h b/include/asm-ia64/topology.h
index f2f72ef..8e68f0f 100644
--- a/include/asm-ia64/topology.h
+++ b/include/asm-ia64/topology.h
@@ -116,6 +116,7 @@ void build_cpu_to_node_map(void);
 #define smt_capable() 				(smp_num_siblings > 1)
 #endif
 
+#define arch_fix_phys_package_id(num, slot)	ia64_fix_socket_id(num, slot)
 #define pcibus_to_cpumask(bus)	(pcibus_to_node(bus) == -1 ? \
 					CPU_MASK_ALL : \
 					node_to_cpumask(pcibus_to_node(bus)) \
diff --git a/include/asm-x86/topology.h b/include/asm-x86/topology.h
index 2207326..efa3108 100644
--- a/include/asm-x86/topology.h
+++ b/include/asm-x86/topology.h
@@ -193,6 +193,8 @@ extern cpumask_t cpu_coregroup_map(int cpu);
 #define topology_thread_siblings(cpu)		(per_cpu(cpu_sibling_map, cpu))
 #endif
 
+#define arch_fix_phys_package_id(num, slot)
+
 #ifdef CONFIG_SMP
 #define mc_capable()			(boot_cpu_data.x86_max_cores > 1)
 #define smt_capable()			(smp_num_siblings > 1)
-- 
1.5.3.1.g1e61

Re: [PATCH 1/2] ia64: Remove printk noise on unimplemented SAL_PHYSICAL_ID_INFO

[Alex Chiang]

Ugh, next time, run checkpatch *before* sending out the patches.
Sorry for the noise.

/ac

From: Alex Chiang <achiang@hp.com>
Subject: [PATCH 1/2] ia64: Remove printk noise on unimplemented SAL_PHYSICAL_ID_INFO

Commit 113134fcbca83619be4c68d0ca66db6093777b5d changed the flow of
control when calling PAL_LOGICAL_TO_PHYSICAL and SAL_PHYSICAL_ID_INFO.
With the change, if a platform did not implement the latter, a useless
printk would appear in the boot log:

	ia64_sal_pltid failed with -1

So let's check the return code and only printk on a true error, and do
not print anything in the unimplemented case. While we're in there,
clean up some stylistic issues too.

Signed-off-by: Alex Chiang <achiang@hp.com>
---
 arch/ia64/kernel/smpboot.c |   12 +++++++++---
 1 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/arch/ia64/kernel/smpboot.c b/arch/ia64/kernel/smpboot.c
index 16483be..8d86e1f 100644
--- a/arch/ia64/kernel/smpboot.c
+++ b/arch/ia64/kernel/smpboot.c
@@ -873,7 +873,8 @@ identify_siblings(struct cpuinfo_ia64 *c)
 	u16 pltid;
 	pal_logical_to_physical_t info;
 
-	if ((status = ia64_pal_logical_to_phys(-1, &info)) != PAL_STATUS_SUCCESS) {
+	status = ia64_pal_logical_to_phys(-1, &info);
+	if (status != PAL_STATUS_SUCCESS) {
 		if (status != PAL_STATUS_UNIMPLEMENTED) {
 			printk(KERN_ERR
 				"ia64_pal_logical_to_phys failed with %ld\n",
@@ -885,8 +886,13 @@ identify_siblings(struct cpuinfo_ia64 *c)
 		info.overview_cpp  = 1;
 		info.overview_tpc  = 1;
 	}
-	if ((status = ia64_sal_physical_id_info(&pltid)) != PAL_STATUS_SUCCESS) {
-		printk(KERN_ERR "ia64_sal_pltid failed with %ld\n", status);
+
+	status = ia64_sal_physical_id_info(&pltid);
+	if (status != PAL_STATUS_SUCCESS) {
+		if (status != PAL_STATUS_UNIMPLEMENTED)
+			printk(KERN_ERR
+				"ia64_sal_pltid failed with %ld\n",
+				status);
 		return;
 	}
 
-- 
1.5.3.1.g1e61

Re: [PATCH 2/2] ia64: Provide ACPI fixup for /proc/cpuinfo/physical_id

[Alex Chiang]

Fix the checkpatch issues in this patch as well.

/ac

From: Alex Chiang <achiang@hp.com>
Subject: [PATCH 2/2] ia64: Provide ACPI fixup for /proc/cpuinfo/physical_id

Legacy HP ia64 platforms currently cannot provide
/proc/cpuinfo/physical_id due to legacy SAL/PAL implementations.
However, that physical topology information can be obtained
via ACPI.

Provide an interface that gives ACPI one last chance to provide
physical_id for these legacy platforms. This logic only comes
into play iff:

	- ACPI actually provides slot information for the CPU
	- we lack a valid socket_id

Otherwise, we don't do anything. Since x86 uses the ACPI processor
driver as well, we provide an empty #define.

Signed-off-by: Alex Chiang <achiang@hp.com>
---
 arch/ia64/kernel/topology.c   |    7 +++++++
 drivers/acpi/processor_core.c |    8 ++++++++
 include/asm-ia64/cpu.h        |    1 +
 include/asm-ia64/topology.h   |    1 +
 include/asm-x86/topology.h    |    2 ++
 5 files changed, 19 insertions(+), 0 deletions(-)

diff --git a/arch/ia64/kernel/topology.c b/arch/ia64/kernel/topology.c
index a2484fc..bb584b4 100644
--- a/arch/ia64/kernel/topology.c
+++ b/arch/ia64/kernel/topology.c
@@ -27,6 +27,13 @@
 
 static struct ia64_cpu *sysfs_cpus;
 
+void ia64_fix_socket_id(int num, u32 slot)
+{
+	if (cpu_data(num)->socket_id == -1)
+		cpu_data(num)->socket_id = slot;
+}
+EXPORT_SYMBOL_GPL(ia64_fix_socket_id);
+
 int arch_register_cpu(int num)
 {
 #if defined (CONFIG_ACPI) && defined (CONFIG_HOTPLUG_CPU)
diff --git a/drivers/acpi/processor_core.c b/drivers/acpi/processor_core.c
index a825b43..89578d4 100644
--- a/drivers/acpi/processor_core.c
+++ b/drivers/acpi/processor_core.c
@@ -612,6 +612,14 @@ static int acpi_processor_get_info(struct acpi_processor *pr, unsigned has_uid)
 		request_region(pr->throttling.address, 6, "ACPI CPU throttle");
 	}
 
+	/*
+	 * If ACPI describes a slot number for this CPU, we can use it
+	 * to fix a broken topology->physical_package_id.
+	 */
+	status = acpi_evaluate_object(pr->handle, "_SUN", NULL, &buffer);
+	if (ACPI_SUCCESS(status))
+		arch_fix_phys_package_id(pr->id, object.integer.value);
+
 	return 0;
 }
 
diff --git a/include/asm-ia64/cpu.h b/include/asm-ia64/cpu.h
index e87fa32..9ba5155 100644
--- a/include/asm-ia64/cpu.h
+++ b/include/asm-ia64/cpu.h
@@ -14,6 +14,7 @@ DECLARE_PER_CPU(struct ia64_cpu, cpu_devices);
 
 DECLARE_PER_CPU(int, cpu_state);
 
+extern void arch_fix_phys_package_id(int num, u32 slot);
 extern int arch_register_cpu(int num);
 #ifdef CONFIG_HOTPLUG_CPU
 extern void arch_unregister_cpu(int);
diff --git a/include/asm-ia64/topology.h b/include/asm-ia64/topology.h
index f2f72ef..8e68f0f 100644
--- a/include/asm-ia64/topology.h
+++ b/include/asm-ia64/topology.h
@@ -116,6 +116,7 @@ void build_cpu_to_node_map(void);
 #define smt_capable() 				(smp_num_siblings > 1)
 #endif
 
+#define arch_fix_phys_package_id(num, slot)	ia64_fix_socket_id(num, slot)
 #define pcibus_to_cpumask(bus)	(pcibus_to_node(bus) == -1 ? \
 					CPU_MASK_ALL : \
 					node_to_cpumask(pcibus_to_node(bus)) \
diff --git a/include/asm-x86/topology.h b/include/asm-x86/topology.h
index 2207326..efa3108 100644
--- a/include/asm-x86/topology.h
+++ b/include/asm-x86/topology.h
@@ -193,6 +193,8 @@ extern cpumask_t cpu_coregroup_map(int cpu);
 #define topology_thread_siblings(cpu)		(per_cpu(cpu_sibling_map, cpu))
 #endif
 
+#define arch_fix_phys_package_id(num, slot)
+
 #ifdef CONFIG_SMP
 #define mc_capable()			(boot_cpu_data.x86_max_cores > 1)
 #define smt_capable()			(smp_num_siblings > 1)
-- 
1.5.3.1.g1e61

RE: [PATCH 2/2] ia64: Provide ACPI fixup for /proc/cpuinfo/physical_id

[Luck, Tony]

Alex,

I chewed on this a bit ... 
 
+void ia64_fix_socket_id(int num, u32 slot)
+{
+	if (cpu_data(num)->socket_id == -1)
+		cpu_data(num)->socket_id= slot;
+}
+EXPORT_SYMBOL_GPL(ia64_fix_socket_id);

1) socket_id is only defined when CONFIG_SMP=y, so this breaks UP
   build.  Fix: Wrap body of this function inside #ifdef
2) should really just name this function "arch_fix_phys_package_id"
   and avoid the #define rename in topology.h


diff --git a/drivers/acpi/processor_core.c b/drivers/acpi/processor_core.c
+	/*
+	 * If ACPI describes a slot number for this CPU, we can use it
+	 * to fix a broken topology->physical_package_id.
+	 */
+	status = acpi_evaluate_object(pr->handle, "_SUN", NULL, &buffer);
+	if (ACPI_SUCCESS(status))
+		arch_fix_phys_package_id(pr->id, object.integer.value);
+

Comment here still refers to what the previous patch was doing. Should
now say that we are fixing /proc/cpuinfo "physical id".
 
diff --git a/include/asm-ia64/topology.h b/include/asm-ia64/topology.h
+#define arch_fix_phys_package_id(num, slot)	ia64_fix_socket_id(num, slot)

Not needed if we name ia64_fix_socket_id() appropriately. Turn this
into an extern definition for arch_fix_phys_package_id() and drop the
extern from cpu.h

diff --git a/include/asm-x86/topology.h b/include/asm-x86/topology.h
+#define arch_fix_phys_package_id(num, slot)

Style preference is to now use a "static inline void" function for
this to make sure args are correctly typed, and to avoid warnings about
unused variables.

updated version of the patch looks like this ... ok???

-Tony

commit fe086a7bea7ab714930bd48addba961ceeef7634
Author: Alex Chiang <achiang@hp.com>
Date:   Tue Apr 29 15:05:29 2008 -0700

    [IA64] Provide ACPI fixup for /proc/cpuinfo/physical_id
    
    Legacy HP ia64 platforms currently cannot provide
    /proc/cpuinfo/physical_id due to legacy SAL/PAL implementations.
    However, that physical topology information can be obtained
    via ACPI.
    
    Provide an interface that gives ACPI one last chance to provide
    physical_id for these legacy platforms. This logic only comes
    into play iff:
    
    - ACPI actually provides slot information for the CPU
    - we lack a valid socket_id
    
    Otherwise, we don't do anything.
    
    Since x86 uses the ACPI processor driver as well, we provide a nop
    stub function for arch_fix_phys_package_id() in asm-x86/topology.h
    
    Signed-off-by: Alex Chiang <achiang@hp.com>
    Signed-off-by: Tony Luck <tony.luck@intel.com>

diff --git a/arch/ia64/kernel/topology.c b/arch/ia64/kernel/topology.c
index a2484fc..abb17a6 100644
--- a/arch/ia64/kernel/topology.c
+++ b/arch/ia64/kernel/topology.c
@@ -27,6 +27,15 @@
 
 static struct ia64_cpu *sysfs_cpus;
 
+void arch_fix_phys_package_id(int num, u32 slot)
+{
+#ifdef CONFIG_SMP
+	if (cpu_data(num)->socket_id == -1)
+		cpu_data(num)->socket_id = slot;
+#endif
+}
+EXPORT_SYMBOL_GPL(arch_fix_phys_package_id);
+
 int arch_register_cpu(int num)
 {
 #if defined (CONFIG_ACPI) && defined (CONFIG_HOTPLUG_CPU)
diff --git a/drivers/acpi/processor_core.c b/drivers/acpi/processor_core.c
index dd28c91..5241e3f 100644
--- a/drivers/acpi/processor_core.c
+++ b/drivers/acpi/processor_core.c
@@ -603,6 +603,15 @@ static int acpi_processor_get_info(struct acpi_processor *pr, unsigned has_uid)
 		request_region(pr->throttling.address, 6, "ACPI CPU throttle");
 	}
 
+	/*
+	 * If ACPI describes a slot number for this CPU, we can use it
+	 * ensure we get the right value in the "physical id" field
+	 * of /proc/cpuinfo
+	 */
+	status = acpi_evaluate_object(pr->handle, "_SUN", NULL, &buffer);
+	if (ACPI_SUCCESS(status))
+		arch_fix_phys_package_id(pr->id, object.integer.value);
+
 	return 0;
 }
 
diff --git a/include/asm-ia64/topology.h b/include/asm-ia64/topology.h
index f2f72ef..32863b3 100644
--- a/include/asm-ia64/topology.h
+++ b/include/asm-ia64/topology.h
@@ -116,6 +116,8 @@ void build_cpu_to_node_map(void);
 #define smt_capable() 				(smp_num_siblings > 1)
 #endif
 
+extern void arch_fix_phys_package_id(int num, u32 slot);
+
 #define pcibus_to_cpumask(bus)	(pcibus_to_node(bus) == -1 ? \
 					CPU_MASK_ALL : \
 					node_to_cpumask(pcibus_to_node(bus)) \
diff --git a/include/asm-x86/topology.h b/include/asm-x86/topology.h
index 0e6d6b0..4f35a0f 100644
--- a/include/asm-x86/topology.h
+++ b/include/asm-x86/topology.h
@@ -193,6 +193,10 @@ extern cpumask_t cpu_coregroup_map(int cpu);
 #define topology_thread_siblings(cpu)		(per_cpu(cpu_sibling_map, cpu))
 #endif
 
+static inline void arch_fix_phys_package_id(int num, u32 slot)
+{
+}
+
 struct pci_bus;
 void set_pci_bus_resources_arch_default(struct pci_bus *b);
 

Re: [PATCH 2/2] ia64: Provide ACPI fixup for /proc/cpuinfo/physical_id

[Alex Chiang]

Hi Tony,

* Luck, Tony <tony.luck@intel.com>:
> Alex,
> 
> I chewed on this a bit ... 

Thanks for all the fixups -- your version looks a lot nicer!

> diff --git a/include/asm-x86/topology.h b/include/asm-x86/topology.h
> +#define arch_fix_phys_package_id(num, slot)
> 
> Style preference is to now use a "static inline void" function for
> this to make sure args are correctly typed, and to avoid warnings about
> unused variables.

*nod*

I'll use that convention in the future, thanks.

> updated version of the patch looks like this ... ok???

Looks great to me.

/ac

> 
> -Tony
> 
> commit fe086a7bea7ab714930bd48addba961ceeef7634
> Author: Alex Chiang <achiang@hp.com>
> Date:   Tue Apr 29 15:05:29 2008 -0700
> 
>     [IA64] Provide ACPI fixup for /proc/cpuinfo/physical_id
>     
>     Legacy HP ia64 platforms currently cannot provide
>     /proc/cpuinfo/physical_id due to legacy SAL/PAL implementations.
>     However, that physical topology information can be obtained
>     via ACPI.
>     
>     Provide an interface that gives ACPI one last chance to provide
>     physical_id for these legacy platforms. This logic only comes
>     into play iff:
>     
>     - ACPI actually provides slot information for the CPU
>     - we lack a valid socket_id
>     
>     Otherwise, we don't do anything.
>     
>     Since x86 uses the ACPI processor driver as well, we provide a nop
>     stub function for arch_fix_phys_package_id() in asm-x86/topology.h
>     
>     Signed-off-by: Alex Chiang <achiang@hp.com>
>     Signed-off-by: Tony Luck <tony.luck@intel.com>
> 
> diff --git a/arch/ia64/kernel/topology.c b/arch/ia64/kernel/topology.c
> index a2484fc..abb17a6 100644
> --- a/arch/ia64/kernel/topology.c
> +++ b/arch/ia64/kernel/topology.c
> @@ -27,6 +27,15 @@
>  
>  static struct ia64_cpu *sysfs_cpus;
>  
> +void arch_fix_phys_package_id(int num, u32 slot)
> +{
> +#ifdef CONFIG_SMP
> +	if (cpu_data(num)->socket_id == -1)
> +		cpu_data(num)->socket_id = slot;
> +#endif
> +}
> +EXPORT_SYMBOL_GPL(arch_fix_phys_package_id);
> +
>  int arch_register_cpu(int num)
>  {
>  #if defined (CONFIG_ACPI) && defined (CONFIG_HOTPLUG_CPU)
> diff --git a/drivers/acpi/processor_core.c b/drivers/acpi/processor_core.c
> index dd28c91..5241e3f 100644
> --- a/drivers/acpi/processor_core.c
> +++ b/drivers/acpi/processor_core.c
> @@ -603,6 +603,15 @@ static int acpi_processor_get_info(struct acpi_processor *pr, unsigned has_uid)
>  		request_region(pr->throttling.address, 6, "ACPI CPU throttle");
>  	}
>  
> +	/*
> +	 * If ACPI describes a slot number for this CPU, we can use it
> +	 * ensure we get the right value in the "physical id" field
> +	 * of /proc/cpuinfo
> +	 */
> +	status = acpi_evaluate_object(pr->handle, "_SUN", NULL, &buffer);
> +	if (ACPI_SUCCESS(status))
> +		arch_fix_phys_package_id(pr->id, object.integer.value);
> +
>  	return 0;
>  }
>  
> diff --git a/include/asm-ia64/topology.h b/include/asm-ia64/topology.h
> index f2f72ef..32863b3 100644
> --- a/include/asm-ia64/topology.h
> +++ b/include/asm-ia64/topology.h
> @@ -116,6 +116,8 @@ void build_cpu_to_node_map(void);
>  #define smt_capable() 				(smp_num_siblings > 1)
>  #endif
>  
> +extern void arch_fix_phys_package_id(int num, u32 slot);
> +
>  #define pcibus_to_cpumask(bus)	(pcibus_to_node(bus) == -1 ? \
>  					CPU_MASK_ALL : \
>  					node_to_cpumask(pcibus_to_node(bus)) \
> diff --git a/include/asm-x86/topology.h b/include/asm-x86/topology.h
> index 0e6d6b0..4f35a0f 100644
> --- a/include/asm-x86/topology.h
> +++ b/include/asm-x86/topology.h
> @@ -193,6 +193,10 @@ extern cpumask_t cpu_coregroup_map(int cpu);
>  #define topology_thread_siblings(cpu)		(per_cpu(cpu_sibling_map, cpu))
>  #endif
>  
> +static inline void arch_fix_phys_package_id(int num, u32 slot)
> +{
> +}
> +
>  struct pci_bus;
>  void set_pci_bus_resources_arch_default(struct pci_bus *b);
>  
> 
>