2017-03-03 08:11:38

by Dou Liyang

[permalink] [raw]
Subject: [PATCH v3 0/5] Do repair works for the mapping of cpuid <-> nodeid

[Summary]:

1, Revert two commits
2, Fix the order of Logical CPU IDs
3, Move the validation of processor IDs to hot-plug time.

The mapping of "cpuid <-> nodeid" is established at boot time via ACPI
tables to keep associations of workqueues and other node related items
consistent across cpu hotplug as following:

Step 1. Make the "Logical CPU ID <-> Processor ID/UID" fixed Using MADT:
We generate the logical CPU IDs by the Local APIC/x2APIC IDs orderly and
get the mapping of Processor ID/UID <-> Local Apic ID directly in MADT.
So, we get the mapping of
*Processor ID/UID <-> Local Apic ID <-> Logical CPU ID*

Step 2. Make the "Processor ID/UID <-> Node ID(_PXM)" fixed Using DSDT:
The maaping of "Processor ID/UID <-> Node ID(_PXM)" is ready-made in
each entities. we just use it directly.

But, ACPI tables are unreliable and failures with that boot time mapping
have been reported on machines where the ACPI table and the physical
information which is retrieved at actual hotplug is inconsistent. Here
has already two bugs we found:

1. Duplicated Processor IDs in DSDT.
It has been fixed by commits:
'8e089eaa1999 ("acpi: Provide mechanism to validate processors
in the ACPI tables")' and 'fd74da217df7 ("acpi: Validate processor id
when mapping the processor")'

2. The _PXM in DSDT is inconsistent with the one in MADT.
It may cause the bug, which is shown in:
https://lkml.org/lkml/2017/2/12/200

And one phenomenon is happened in some specific boxes:

1. The logical CPU IDs is discrete. Such as:
Node2: 64-69, 72-77, 80-85, 88-93,...

There may be more strange things happened in the futher. We shouldn't just
only fix them everytime, we should solve this problem from the source to
avoid such problems happened again and again.

Find a simple and easy way:

1. Do the step 1 when the CPU flag is enabled
2. Do the step 2 at hot-plug time, not at boot time when we did some
useless work.

It also can make the mapping of "cpuid <-> nodeid" fixed and avoid
excessive using of the ACPI tables.

Change log:
v2 -> v3: 1. rewirte the changelogs
copy the changelogs Thomas Gleixner <[email protected]>
rewrite for the patch 1,2,4,5.
2. s/duplicate_processor_id()/acpi_duplicate_processor_id().
by Thomas Gleixner <[email protected]>'s advice.
3. modify the error handle in acpi_processor_ids_walk()
by Thomas Gleixner <[email protected]>'s advice.
4. add a new patch for restoring the order of CPU IDs

v1 -> v2: 1. fix some comments.
2. add the verification of duplicate processor id.

Dou Liyang (5):
Revert"x86/acpi: Set persistent cpuid <-> nodeid mapping when booting"
Revert"x86/acpi: Enable MADT APIs to return disabled apicids"
x86/acpi: Restore the order of CPU IDs
acpi/processor: Implement DEVICE operator for processor enumeration
acpi/processor: Check for duplicate processor ids at hotplug time

arch/x86/kernel/acpi/boot.c | 9 ++-
arch/x86/kernel/apic/apic.c | 26 +++------
drivers/acpi/acpi_processor.c | 57 +++++++++++++-----
drivers/acpi/bus.c | 1 -
drivers/acpi/processor_core.c | 133 +++++++-----------------------------------
include/linux/acpi.h | 5 +-
6 files changed, 79 insertions(+), 152 deletions(-)

--
2.5.5




2017-03-03 08:11:04

by Dou Liyang

[permalink] [raw]
Subject: [PATCH v3 2/5] Revert"x86/acpi: Enable MADT APIs to return disabled apicids"

Remove the leftovers of the boot time 'cpuid <-> nodeid' mapping approach.

Signed-off-by: Dou Liyang <[email protected]>
Tested-by: Xiaolong Ye <[email protected]>
---
drivers/acpi/processor_core.c | 60 ++++++++++++++++---------------------------
1 file changed, 22 insertions(+), 38 deletions(-)

diff --git a/drivers/acpi/processor_core.c b/drivers/acpi/processor_core.c
index a843862..b933061 100644
--- a/drivers/acpi/processor_core.c
+++ b/drivers/acpi/processor_core.c
@@ -32,12 +32,12 @@ static struct acpi_table_madt *get_madt_table(void)
}

static int map_lapic_id(struct acpi_subtable_header *entry,
- u32 acpi_id, phys_cpuid_t *apic_id, bool ignore_disabled)
+ u32 acpi_id, phys_cpuid_t *apic_id)
{
struct acpi_madt_local_apic *lapic =
container_of(entry, struct acpi_madt_local_apic, header);

- if (ignore_disabled && !(lapic->lapic_flags & ACPI_MADT_ENABLED))
+ if (!(lapic->lapic_flags & ACPI_MADT_ENABLED))
return -ENODEV;

if (lapic->processor_id != acpi_id)
@@ -48,13 +48,12 @@ static int map_lapic_id(struct acpi_subtable_header *entry,
}

static int map_x2apic_id(struct acpi_subtable_header *entry,
- int device_declaration, u32 acpi_id, phys_cpuid_t *apic_id,
- bool ignore_disabled)
+ int device_declaration, u32 acpi_id, phys_cpuid_t *apic_id)
{
struct acpi_madt_local_x2apic *apic =
container_of(entry, struct acpi_madt_local_x2apic, header);

- if (ignore_disabled && !(apic->lapic_flags & ACPI_MADT_ENABLED))
+ if (!(apic->lapic_flags & ACPI_MADT_ENABLED))
return -ENODEV;

if (device_declaration && (apic->uid == acpi_id)) {
@@ -66,13 +65,12 @@ static int map_x2apic_id(struct acpi_subtable_header *entry,
}

static int map_lsapic_id(struct acpi_subtable_header *entry,
- int device_declaration, u32 acpi_id, phys_cpuid_t *apic_id,
- bool ignore_disabled)
+ int device_declaration, u32 acpi_id, phys_cpuid_t *apic_id)
{
struct acpi_madt_local_sapic *lsapic =
container_of(entry, struct acpi_madt_local_sapic, header);

- if (ignore_disabled && !(lsapic->lapic_flags & ACPI_MADT_ENABLED))
+ if (!(lsapic->lapic_flags & ACPI_MADT_ENABLED))
return -ENODEV;

if (device_declaration) {
@@ -89,13 +87,12 @@ static int map_lsapic_id(struct acpi_subtable_header *entry,
* Retrieve the ARM CPU physical identifier (MPIDR)
*/
static int map_gicc_mpidr(struct acpi_subtable_header *entry,
- int device_declaration, u32 acpi_id, phys_cpuid_t *mpidr,
- bool ignore_disabled)
+ int device_declaration, u32 acpi_id, phys_cpuid_t *mpidr)
{
struct acpi_madt_generic_interrupt *gicc =
container_of(entry, struct acpi_madt_generic_interrupt, header);

- if (ignore_disabled && !(gicc->flags & ACPI_MADT_ENABLED))
+ if (!(gicc->flags & ACPI_MADT_ENABLED))
return -ENODEV;

/* device_declaration means Device object in DSDT, in the
@@ -112,7 +109,7 @@ static int map_gicc_mpidr(struct acpi_subtable_header *entry,
}

static phys_cpuid_t map_madt_entry(struct acpi_table_madt *madt,
- int type, u32 acpi_id, bool ignore_disabled)
+ int type, u32 acpi_id)
{
unsigned long madt_end, entry;
phys_cpuid_t phys_id = PHYS_CPUID_INVALID; /* CPU hardware ID */
@@ -130,20 +127,16 @@ static phys_cpuid_t map_madt_entry(struct acpi_table_madt *madt,
struct acpi_subtable_header *header =
(struct acpi_subtable_header *)entry;
if (header->type == ACPI_MADT_TYPE_LOCAL_APIC) {
- if (!map_lapic_id(header, acpi_id, &phys_id,
- ignore_disabled))
+ if (!map_lapic_id(header, acpi_id, &phys_id))
break;
} else if (header->type == ACPI_MADT_TYPE_LOCAL_X2APIC) {
- if (!map_x2apic_id(header, type, acpi_id, &phys_id,
- ignore_disabled))
+ if (!map_x2apic_id(header, type, acpi_id, &phys_id))
break;
} else if (header->type == ACPI_MADT_TYPE_LOCAL_SAPIC) {
- if (!map_lsapic_id(header, type, acpi_id, &phys_id,
- ignore_disabled))
+ if (!map_lsapic_id(header, type, acpi_id, &phys_id))
break;
} else if (header->type == ACPI_MADT_TYPE_GENERIC_INTERRUPT) {
- if (!map_gicc_mpidr(header, type, acpi_id, &phys_id,
- ignore_disabled))
+ if (!map_gicc_mpidr(header, type, acpi_id, &phys_id))
break;
}
entry += header->length;
@@ -161,15 +154,14 @@ phys_cpuid_t __init acpi_map_madt_entry(u32 acpi_id)
if (!madt)
return PHYS_CPUID_INVALID;

- rv = map_madt_entry(madt, 1, acpi_id, true);
+ rv = map_madt_entry(madt, 1, acpi_id);

acpi_put_table((struct acpi_table_header *)madt);

return rv;
}

-static phys_cpuid_t map_mat_entry(acpi_handle handle, int type, u32 acpi_id,
- bool ignore_disabled)
+static phys_cpuid_t map_mat_entry(acpi_handle handle, int type, u32 acpi_id)
{
struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
union acpi_object *obj;
@@ -190,38 +182,30 @@ static phys_cpuid_t map_mat_entry(acpi_handle handle, int type, u32 acpi_id,

header = (struct acpi_subtable_header *)obj->buffer.pointer;
if (header->type == ACPI_MADT_TYPE_LOCAL_APIC)
- map_lapic_id(header, acpi_id, &phys_id, ignore_disabled);
+ map_lapic_id(header, acpi_id, &phys_id);
else if (header->type == ACPI_MADT_TYPE_LOCAL_SAPIC)
- map_lsapic_id(header, type, acpi_id, &phys_id, ignore_disabled);
+ map_lsapic_id(header, type, acpi_id, &phys_id);
else if (header->type == ACPI_MADT_TYPE_LOCAL_X2APIC)
- map_x2apic_id(header, type, acpi_id, &phys_id, ignore_disabled);
+ map_x2apic_id(header, type, acpi_id, &phys_id);
else if (header->type == ACPI_MADT_TYPE_GENERIC_INTERRUPT)
- map_gicc_mpidr(header, type, acpi_id, &phys_id,
- ignore_disabled);
+ map_gicc_mpidr(header, type, acpi_id, &phys_id);

exit:
kfree(buffer.pointer);
return phys_id;
}

-static phys_cpuid_t __acpi_get_phys_id(acpi_handle handle, int type,
- u32 acpi_id, bool ignore_disabled)
+phys_cpuid_t acpi_get_phys_id(acpi_handle handle, int type, u32 acpi_id)
{
phys_cpuid_t phys_id;

- phys_id = map_mat_entry(handle, type, acpi_id, ignore_disabled);
+ phys_id = map_mat_entry(handle, type, acpi_id);
if (invalid_phys_cpuid(phys_id))
- phys_id = map_madt_entry(get_madt_table(), type, acpi_id,
- ignore_disabled);
+ phys_id = map_madt_entry(get_madt_table(), type, acpi_id);

return phys_id;
}

-phys_cpuid_t acpi_get_phys_id(acpi_handle handle, int type, u32 acpi_id)
-{
- return __acpi_get_phys_id(handle, type, acpi_id, true);
-}
-
int acpi_map_cpuid(phys_cpuid_t phys_id, u32 acpi_id)
{
#ifdef CONFIG_SMP
--
2.5.5



2017-03-03 08:12:15

by Dou Liyang

[permalink] [raw]
Subject: [PATCH v3 3/5] x86/acpi: Restore the order of CPU IDs

The following commits:

f7c28833c2 ("x86/acpi: Enable acpi to register all possible cpus at
boot time") and 8f54969dc8 ("x86/acpi: Introduce persistent storage
for cpuid <-> apicid mapping")

... registered all the possible CPUs at boot time via ACPI tables to
make the mapping of cpuid <-> apicid fixed. Both enabled and disabled
CPUs could have a logical CPU ID after boot time.

But, ACPI tables are unreliable. the number amd order of Local APIC
entries which depends on the firmware is often inconsistent with the
physical devices. Even if they are consistent, The disabled CPUs which
take up some logical CPU IDs will also make the order discontinuous.

Revert the part of disabled CPUs registration, keep the allocation
logic of logical CPU IDs and also keep some code location changes.

Signed-off-by: Dou Liyang <[email protected]>
Tested-by: Xiaolong Ye <[email protected]>
---
arch/x86/kernel/acpi/boot.c | 7 ++++++-
arch/x86/kernel/apic/apic.c | 26 +++++++-------------------
2 files changed, 13 insertions(+), 20 deletions(-)

diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
index f6b0e87..b2879cc23 100644
--- a/arch/x86/kernel/acpi/boot.c
+++ b/arch/x86/kernel/acpi/boot.c
@@ -179,10 +179,15 @@ static int acpi_register_lapic(int id, u32 acpiid, u8 enabled)
return -EINVAL;
}

+ if (!enabled) {
+ ++disabled_cpus;
+ return -EINVAL;
+ }
+
if (boot_cpu_physical_apicid != -1U)
ver = boot_cpu_apic_version;

- cpu = __generic_processor_info(id, ver, enabled);
+ cpu = generic_processor_info(id, ver);
if (cpu >= 0)
early_per_cpu(x86_cpu_to_acpiid, cpu) = acpiid;

diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index 4261b32..26a2ff0 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -2072,7 +2072,7 @@ static int allocate_logical_cpuid(int apicid)
return nr_logical_cpuids++;
}

-int __generic_processor_info(int apicid, int version, bool enabled)
+int generic_processor_info(int apicid, int version)
{
int cpu, max = nr_cpu_ids;
bool boot_cpu_detected = physid_isset(boot_cpu_physical_apicid,
@@ -2130,11 +2130,9 @@ int __generic_processor_info(int apicid, int version, bool enabled)
if (num_processors >= nr_cpu_ids) {
int thiscpu = max + disabled_cpus;

- if (enabled) {
- pr_warning("APIC: NR_CPUS/possible_cpus limit of %i "
- "reached. Processor %d/0x%x ignored.\n",
- max, thiscpu, apicid);
- }
+ pr_warning("APIC: NR_CPUS/possible_cpus limit of %i "
+ "reached. Processor %d/0x%x ignored.\n",
+ max, thiscpu, apicid);

disabled_cpus++;
return -EINVAL;
@@ -2186,23 +2184,13 @@ int __generic_processor_info(int apicid, int version, bool enabled)
apic->x86_32_early_logical_apicid(cpu);
#endif
set_cpu_possible(cpu, true);
-
- if (enabled) {
- num_processors++;
- physid_set(apicid, phys_cpu_present_map);
- set_cpu_present(cpu, true);
- } else {
- disabled_cpus++;
- }
+ physid_set(apicid, phys_cpu_present_map);
+ set_cpu_present(cpu, true);
+ num_processors++;

return cpu;
}

-int generic_processor_info(int apicid, int version)
-{
- return __generic_processor_info(apicid, version, true);
-}
-
int hard_smp_processor_id(void)
{
return read_apic_id();
--
2.5.5



2017-03-03 08:20:07

by Dou Liyang

[permalink] [raw]
Subject: [PATCH v3 4/5] acpi/processor: Implement DEVICE operator for processor enumeration

ACPI allows to declare processors either with the PROCESSOR or with the
DEVICE operator. The current implementation handles only the PROCESSOR
operator.

On a system which uses the DEVICE operator for processor enumeration the
evaluation fails.

Check for the ACPI type of the ACPI handle and evaluate PROCESSOR and
DEVICE types separately.

Signed-off-by: Dou Liyang <[email protected]>
Tested-by: Xiaolong Ye <[email protected]>
---
drivers/acpi/acpi_processor.c | 39 ++++++++++++++++++++++++++++++++-------
1 file changed, 32 insertions(+), 7 deletions(-)

diff --git a/drivers/acpi/acpi_processor.c b/drivers/acpi/acpi_processor.c
index 5d208a9..9a98d7e 100644
--- a/drivers/acpi/acpi_processor.c
+++ b/drivers/acpi/acpi_processor.c
@@ -633,25 +633,50 @@ static acpi_status __init acpi_processor_ids_walk(acpi_handle handle,
void **rv)
{
acpi_status status;
+ acpi_object_type acpi_type;
+ unsigned long long uid;
union acpi_object object = { 0 };
struct acpi_buffer buffer = { sizeof(union acpi_object), &object };

- status = acpi_evaluate_object(handle, NULL, NULL, &buffer);
+ status = acpi_get_type(handle, &acpi_type);
if (ACPI_FAILURE(status))
- acpi_handle_info(handle, "Not get the processor object\n");
- else
- processor_validated_ids_update(object.processor.proc_id);
+ return false;
+
+ switch (acpi_type) {
+ case ACPI_TYPE_PROCESSOR:
+ status = acpi_evaluate_object(handle, NULL, NULL, &buffer);
+ if (ACPI_FAILURE(status))
+ goto err;
+ uid = object.processor.proc_id;
+ break;
+
+ case ACPI_TYPE_DEVICE:
+ status = acpi_evaluate_integer(handle, "_UID", NULL, &uid);
+ if (ACPI_FAILURE(status))
+ goto err;
+ break;
+ default:
+ goto err;
+ }
+
+ processor_validated_ids_update(uid);
+ return true;
+
+err:
+ acpi_handle_info(handle, "Invalid processor object\n");
+ return false;

- return AE_OK;
}

-static void __init acpi_processor_check_duplicates(void)
+void __init acpi_processor_check_duplicates(void)
{
- /* Search all processor nodes in ACPI namespace */
+ /* check the correctness for all processors in ACPI namespace */
acpi_walk_namespace(ACPI_TYPE_PROCESSOR, ACPI_ROOT_OBJECT,
ACPI_UINT32_MAX,
acpi_processor_ids_walk,
NULL, NULL, NULL);
+ acpi_get_devices(ACPI_PROCESSOR_DEVICE_HID, acpi_processor_ids_walk,
+ NULL, NULL);
}

bool __init acpi_processor_validate_proc_id(int proc_id)
--
2.5.5



2017-03-03 08:34:22

by Dou Liyang

[permalink] [raw]
Subject: Re: [PATCH v3 0/5] Do repair works for the mapping of cpuid <-> nodeid

Hi All,

My Simple Test Result:

In our box: Fujitsu PQ2000 with 1 nodes for hot-plug.

Before the patchset:

+-------------------------------------+
| |
| NUMA node0 CPU?? 0-23,256-279 +------+
| NUMA node1 CPU?? 24-47,280-303 | |
| | |
+-------------------------------------+ |
Hot-plug
+-------------------------------------+ +
| | |
| NUMA Node0: 0-23, 256-279 <------+
| NUMA Node1: 24-47, 280-303 |
| NUMA Node2: 64|69, 72-77, 80-85, 88-93...
| NUMA Node3: 96-101, 104-109, 112-117,...
| | |
+-------------------------------------+ |
Hot-remove
+-------------------------------------+ |
| | |
| NUMA node0 CPU?? 0-23,256-279 | |
| NUMA node1 CPU?? 24-47,280-303 +^-----+
| |
| |
+-------------------------------------+

After the patchset:

+-------------------------------------+
| |
| NUMA node0 CPU?? 0-23,48-71 +------+
| NUMA node1 CPU?? 24-47,72-95 | |
| | |
+-------------------------------------+ |
Hot-plug
+-------------------------------------+ +
| | |
| NUMA node0 CPU?? 0-23,48-71 <------+
| NUMA node1 CPU?? 24-47,72-95 |
| NUMA node2 CPU?? 96-143 +------+
| NUMA node3 CPU?? 144-191 | |
| | |
+-------------------------------------+ |
Hot-remove
+-------------------------------------+ |
| | |
| NUMA node0 CPU?? 0-23,48-71 | |
| NUMA node1 CPU?? 24-47,72-95 +^-----+
| |
| |
+-------------------------------------+

And I also test some cases in VMs with QEmu.

And When I get more nodes, I will test the whole
function.

Thanks,
Liyang.

At 03/03/2017 04:02 PM, Dou Liyang wrote:
> [Summary]:
>
> 1, Revert two commits
> 2, Fix the order of Logical CPU IDs
> 3, Move the validation of processor IDs to hot-plug time.
>
> The mapping of "cpuid <-> nodeid" is established at boot time via ACPI
> tables to keep associations of workqueues and other node related items
> consistent across cpu hotplug as following:
>
> Step 1. Make the "Logical CPU ID <-> Processor ID/UID" fixed Using MADT:
> We generate the logical CPU IDs by the Local APIC/x2APIC IDs orderly and
> get the mapping of Processor ID/UID <-> Local Apic ID directly in MADT.
> So, we get the mapping of
> *Processor ID/UID <-> Local Apic ID <-> Logical CPU ID*
>
> Step 2. Make the "Processor ID/UID <-> Node ID(_PXM)" fixed Using DSDT:
> The maaping of "Processor ID/UID <-> Node ID(_PXM)" is ready-made in
> each entities. we just use it directly.
>
> But, ACPI tables are unreliable and failures with that boot time mapping
> have been reported on machines where the ACPI table and the physical
> information which is retrieved at actual hotplug is inconsistent. Here
> has already two bugs we found:
>
> 1. Duplicated Processor IDs in DSDT.
> It has been fixed by commits:
> '8e089eaa1999 ("acpi: Provide mechanism to validate processors
> in the ACPI tables")' and 'fd74da217df7 ("acpi: Validate processor id
> when mapping the processor")'
>
> 2. The _PXM in DSDT is inconsistent with the one in MADT.
> It may cause the bug, which is shown in:
> https://lkml.org/lkml/2017/2/12/200
>
> And one phenomenon is happened in some specific boxes:
>
> 1. The logical CPU IDs is discrete. Such as:
> Node2: 64-69, 72-77, 80-85, 88-93,...
>
> There may be more strange things happened in the futher. We shouldn't just
> only fix them everytime, we should solve this problem from the source to
> avoid such problems happened again and again.
>
> Find a simple and easy way:
>
> 1. Do the step 1 when the CPU flag is enabled
> 2. Do the step 2 at hot-plug time, not at boot time when we did some
> useless work.
>
> It also can make the mapping of "cpuid <-> nodeid" fixed and avoid
> excessive using of the ACPI tables.
>
> Change log:
> v2 -> v3: 1. rewirte the changelogs
> copy the changelogs Thomas Gleixner <[email protected]>
> rewrite for the patch 1,2,4,5.
> 2. s/duplicate_processor_id()/acpi_duplicate_processor_id().
> by Thomas Gleixner <[email protected]>'s advice.
> 3. modify the error handle in acpi_processor_ids_walk()
> by Thomas Gleixner <[email protected]>'s advice.
> 4. add a new patch for restoring the order of CPU IDs
>
> v1 -> v2: 1. fix some comments.
> 2. add the verification of duplicate processor id.
>
> Dou Liyang (5):
> Revert"x86/acpi: Set persistent cpuid <-> nodeid mapping when booting"
> Revert"x86/acpi: Enable MADT APIs to return disabled apicids"
> x86/acpi: Restore the order of CPU IDs
> acpi/processor: Implement DEVICE operator for processor enumeration
> acpi/processor: Check for duplicate processor ids at hotplug time
>
> arch/x86/kernel/acpi/boot.c | 9 ++-
> arch/x86/kernel/apic/apic.c | 26 +++------
> drivers/acpi/acpi_processor.c | 57 +++++++++++++-----
> drivers/acpi/bus.c | 1 -
> drivers/acpi/processor_core.c | 133 +++++++-----------------------------------
> include/linux/acpi.h | 5 +-
> 6 files changed, 79 insertions(+), 152 deletions(-)
>


2017-03-03 09:17:36

by Dou Liyang

[permalink] [raw]
Subject: [PATCH v3 1/5] Revert"x86/acpi: Set persistent cpuid <-> nodeid mapping when booting"

The mapping of "cpuid <-> nodeid" is established at boot time via ACPI
tables to keep associations of workqueues and other node related items
consistent across cpu hotplug.

But, ACPI tables are unreliable and failures with that boot time mapping
have been reported on machines where the ACPI table and the physical
information which is retrieved at actual hotplug is inconsistent.

Revert the mapping implementation so it can be replaced with a less error
prone approach.

Signed-off-by: Dou Liyang <[email protected]>
Tested-by: Xiaolong Ye <[email protected]>
---
arch/x86/kernel/acpi/boot.c | 2 +-
drivers/acpi/acpi_processor.c | 5 ---
drivers/acpi/bus.c | 1 -
drivers/acpi/processor_core.c | 73 -------------------------------------------
include/linux/acpi.h | 3 --
5 files changed, 1 insertion(+), 83 deletions(-)

diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
index ae32838..f6b0e87 100644
--- a/arch/x86/kernel/acpi/boot.c
+++ b/arch/x86/kernel/acpi/boot.c
@@ -710,7 +710,7 @@ static void __init acpi_set_irq_model_ioapic(void)
#ifdef CONFIG_ACPI_HOTPLUG_CPU
#include <acpi/processor.h>

-int acpi_map_cpu2node(acpi_handle handle, int cpu, int physid)
+static int acpi_map_cpu2node(acpi_handle handle, int cpu, int physid)
{
#ifdef CONFIG_ACPI_NUMA
int nid;
diff --git a/drivers/acpi/acpi_processor.c b/drivers/acpi/acpi_processor.c
index 4467a80..5d208a9 100644
--- a/drivers/acpi/acpi_processor.c
+++ b/drivers/acpi/acpi_processor.c
@@ -182,11 +182,6 @@ int __weak arch_register_cpu(int cpu)

void __weak arch_unregister_cpu(int cpu) {}

-int __weak acpi_map_cpu2node(acpi_handle handle, int cpu, int physid)
-{
- return -ENODEV;
-}
-
static int acpi_processor_hotadd_init(struct acpi_processor *pr)
{
unsigned long long sta;
diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
index 80cb5eb..34fbe02 100644
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -1249,7 +1249,6 @@ static int __init acpi_init(void)
acpi_wakeup_device_init();
acpi_debugger_init();
acpi_setup_sb_notify_handler();
- acpi_set_processor_mapping();
return 0;
}

diff --git a/drivers/acpi/processor_core.c b/drivers/acpi/processor_core.c
index 611a558..a843862 100644
--- a/drivers/acpi/processor_core.c
+++ b/drivers/acpi/processor_core.c
@@ -278,79 +278,6 @@ int acpi_get_cpuid(acpi_handle handle, int type, u32 acpi_id)
}
EXPORT_SYMBOL_GPL(acpi_get_cpuid);

-#ifdef CONFIG_ACPI_HOTPLUG_CPU
-static bool __init
-map_processor(acpi_handle handle, phys_cpuid_t *phys_id, int *cpuid)
-{
- int type, id;
- u32 acpi_id;
- acpi_status status;
- acpi_object_type acpi_type;
- unsigned long long tmp;
- union acpi_object object = { 0 };
- struct acpi_buffer buffer = { sizeof(union acpi_object), &object };
-
- status = acpi_get_type(handle, &acpi_type);
- if (ACPI_FAILURE(status))
- return false;
-
- switch (acpi_type) {
- case ACPI_TYPE_PROCESSOR:
- status = acpi_evaluate_object(handle, NULL, NULL, &buffer);
- if (ACPI_FAILURE(status))
- return false;
- acpi_id = object.processor.proc_id;
-
- /* validate the acpi_id */
- if(acpi_processor_validate_proc_id(acpi_id))
- return false;
- break;
- case ACPI_TYPE_DEVICE:
- status = acpi_evaluate_integer(handle, "_UID", NULL, &tmp);
- if (ACPI_FAILURE(status))
- return false;
- acpi_id = tmp;
- break;
- default:
- return false;
- }
-
- type = (acpi_type == ACPI_TYPE_DEVICE) ? 1 : 0;
-
- *phys_id = __acpi_get_phys_id(handle, type, acpi_id, false);
- id = acpi_map_cpuid(*phys_id, acpi_id);
-
- if (id < 0)
- return false;
- *cpuid = id;
- return true;
-}
-
-static acpi_status __init
-set_processor_node_mapping(acpi_handle handle, u32 lvl, void *context,
- void **rv)
-{
- phys_cpuid_t phys_id;
- int cpu_id;
-
- if (!map_processor(handle, &phys_id, &cpu_id))
- return AE_ERROR;
-
- acpi_map_cpu2node(handle, cpu_id, phys_id);
- return AE_OK;
-}
-
-void __init acpi_set_processor_mapping(void)
-{
- /* Set persistent cpu <-> node mapping for all processors. */
- acpi_walk_namespace(ACPI_TYPE_PROCESSOR, ACPI_ROOT_OBJECT,
- ACPI_UINT32_MAX, set_processor_node_mapping,
- NULL, NULL, NULL);
-}
-#else
-void __init acpi_set_processor_mapping(void) {}
-#endif /* CONFIG_ACPI_HOTPLUG_CPU */
-
#ifdef CONFIG_ACPI_HOTPLUG_IOAPIC
static int get_ioapic_id(struct acpi_subtable_header *entry, u32 gsi_base,
u64 *phys_addr, int *ioapic_id)
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 673acda..63a7519 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -294,11 +294,8 @@ bool acpi_processor_validate_proc_id(int proc_id);
int acpi_map_cpu(acpi_handle handle, phys_cpuid_t physid, u32 acpi_id,
int *pcpu);
int acpi_unmap_cpu(int cpu);
-int acpi_map_cpu2node(acpi_handle handle, int cpu, int physid);
#endif /* CONFIG_ACPI_HOTPLUG_CPU */

-void acpi_set_processor_mapping(void);
-
#ifdef CONFIG_ACPI_HOTPLUG_IOAPIC
int acpi_get_ioapic_id(acpi_handle handle, u32 gsi_base, u64 *phys_addr);
#endif
--
2.5.5



2017-03-03 11:41:37

by Dou Liyang

[permalink] [raw]
Subject: [PATCH v3 5/5] acpi/processor: Check for duplicate processor ids at hotplug time

The check for duplicate processor ids happens at boot time based on the
ACPI table contents, but the final sanity checks for a processor happen
at hotplug time.

At hotplug time, where the physical information is available, which might
differ from the ACPI table information, a check for duplicate processor
ids is missing.

Add it to the hotplug checks and rename the function so it better
reflects its purpose.

Signed-off-by: Dou Liyang <[email protected]>
Tested-by: Xiaolong Ye <[email protected]>
---
drivers/acpi/acpi_processor.c | 13 ++++++++++---
include/linux/acpi.h | 2 +-
2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/acpi/acpi_processor.c b/drivers/acpi/acpi_processor.c
index 9a98d7e..3ce2e9e 100644
--- a/drivers/acpi/acpi_processor.c
+++ b/drivers/acpi/acpi_processor.c
@@ -280,6 +280,13 @@ static int acpi_processor_get_info(struct acpi_device *device)
pr->acpi_id = value;
}

+ if (acpi_duplicate_processor_id(pr->acpi_id)) {
+ dev_err(&device->dev,
+ "Failed to get unique processor _UID (0x%x)\n",
+ pr->acpi_id);
+ return -ENODEV;
+ }
+
pr->phys_id = acpi_get_phys_id(pr->handle, device_declaration,
pr->acpi_id);
if (invalid_phys_cpuid(pr->phys_id))
@@ -580,7 +587,7 @@ static struct acpi_scan_handler processor_container_handler = {
static int nr_unique_ids __initdata;

/* The number of the duplicate processor IDs */
-static int nr_duplicate_ids __initdata;
+static int nr_duplicate_ids;

/* Used to store the unique processor IDs */
static int unique_processor_ids[] __initdata = {
@@ -588,7 +595,7 @@ static int unique_processor_ids[] __initdata = {
};

/* Used to store the duplicate processor IDs */
-static int duplicate_processor_ids[] __initdata = {
+static int duplicate_processor_ids[] = {
[0 ... NR_CPUS - 1] = -1,
};

@@ -679,7 +686,7 @@ void __init acpi_processor_check_duplicates(void)
NULL, NULL);
}

-bool __init acpi_processor_validate_proc_id(int proc_id)
+bool acpi_duplicate_processor_id(int proc_id)
{
int i;

diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 63a7519..9b05886 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -287,7 +287,7 @@ static inline bool invalid_phys_cpuid(phys_cpuid_t phys_id)
}

/* Validate the processor object's proc_id */
-bool acpi_processor_validate_proc_id(int proc_id);
+bool acpi_duplicate_processor_id(int proc_id);

#ifdef CONFIG_ACPI_HOTPLUG_CPU
/* Arch dependent functions for cpu hotplug support */
--
2.5.5



2017-03-06 02:11:54

by Dou Liyang

[permalink] [raw]
Subject: Re: [PATCH v3 0/5] Do repair works for the mapping of cpuid <-> nodeid



At 03/03/2017 04:32 PM, Dou Liyang wrote:
> Hi All,
>
> My Simple Test Result:
>
> In our box: Fujitsu PQ2000 with 1 nodes for hot-plug.

s/1 nodes/2 nodes in 1 SB which contains CPU, Memory.../

Thanks,
Liyang


>
> Before the patchset:
>
> +-------------------------------------+
> | |
> | NUMA node0 CPU?? 0-23,256-279 +------+
> | NUMA node1 CPU?? 24-47,280-303 | |
> | | |
> +-------------------------------------+ |
> Hot-plug
> +-------------------------------------+ +
> | | |
> | NUMA Node0: 0-23, 256-279 <------+
> | NUMA Node1: 24-47, 280-303 |
> | NUMA Node2: 64|69, 72-77, 80-85, 88-93...
> | NUMA Node3: 96-101, 104-109, 112-117,...
> | | |
> +-------------------------------------+ |
> Hot-remove
> +-------------------------------------+ |
> | | |
> | NUMA node0 CPU?? 0-23,256-279 | |
> | NUMA node1 CPU?? 24-47,280-303 +^-----+
> | |
> | |
> +-------------------------------------+
>
> After the patchset:
>
> +-------------------------------------+
> | |
> | NUMA node0 CPU?? 0-23,48-71 +------+
> | NUMA node1 CPU?? 24-47,72-95 | |
> | | |
> +-------------------------------------+ |
> Hot-plug
> +-------------------------------------+ +
> | | |
> | NUMA node0 CPU?? 0-23,48-71 <------+
> | NUMA node1 CPU?? 24-47,72-95 |
> | NUMA node2 CPU?? 96-143 +------+
> | NUMA node3 CPU?? 144-191 | |
> | | |
> +-------------------------------------+ |
> Hot-remove
> +-------------------------------------+ |
> | | |
> | NUMA node0 CPU?? 0-23,48-71 | |
> | NUMA node1 CPU?? 24-47,72-95 +^-----+
> | |
> | |
> +-------------------------------------+
>
> And I also test some cases in VMs with QEmu.
>
> And When I get more nodes, I will test the whole
> function.
>
> Thanks,
> Liyang.
>
> At 03/03/2017 04:02 PM, Dou Liyang wrote:
>> [Summary]:
>>
>> 1, Revert two commits
>> 2, Fix the order of Logical CPU IDs
>> 3, Move the validation of processor IDs to hot-plug time.
>>
>> The mapping of "cpuid <-> nodeid" is established at boot time via ACPI
>> tables to keep associations of workqueues and other node related items
>> consistent across cpu hotplug as following:
>>
>> Step 1. Make the "Logical CPU ID <-> Processor ID/UID" fixed Using MADT:
>> We generate the logical CPU IDs by the Local APIC/x2APIC IDs orderly and
>> get the mapping of Processor ID/UID <-> Local Apic ID directly in MADT.
>> So, we get the mapping of
>> *Processor ID/UID <-> Local Apic ID <-> Logical CPU ID*
>>
>> Step 2. Make the "Processor ID/UID <-> Node ID(_PXM)" fixed Using DSDT:
>> The maaping of "Processor ID/UID <-> Node ID(_PXM)" is ready-made in
>> each entities. we just use it directly.
>>
>> But, ACPI tables are unreliable and failures with that boot time mapping
>> have been reported on machines where the ACPI table and the physical
>> information which is retrieved at actual hotplug is inconsistent. Here
>> has already two bugs we found:
>>
>> 1. Duplicated Processor IDs in DSDT.
>> It has been fixed by commits:
>> '8e089eaa1999 ("acpi: Provide mechanism to validate processors
>> in the ACPI tables")' and 'fd74da217df7 ("acpi: Validate processor id
>> when mapping the processor")'
>>
>> 2. The _PXM in DSDT is inconsistent with the one in MADT.
>> It may cause the bug, which is shown in:
>> https://lkml.org/lkml/2017/2/12/200
>>
>> And one phenomenon is happened in some specific boxes:
>>
>> 1. The logical CPU IDs is discrete. Such as:
>> Node2: 64-69, 72-77, 80-85, 88-93,...
>>
>> There may be more strange things happened in the futher. We shouldn't
>> just
>> only fix them everytime, we should solve this problem from the source to
>> avoid such problems happened again and again.
>>
>> Find a simple and easy way:
>>
>> 1. Do the step 1 when the CPU flag is enabled
>> 2. Do the step 2 at hot-plug time, not at boot time when we did some
>> useless work.
>>
>> It also can make the mapping of "cpuid <-> nodeid" fixed and avoid
>> excessive using of the ACPI tables.
>>
>> Change log:
>> v2 -> v3: 1. rewirte the changelogs
>> copy the changelogs Thomas Gleixner <[email protected]>
>> rewrite for the patch 1,2,4,5.
>> 2. s/duplicate_processor_id()/acpi_duplicate_processor_id().
>> by Thomas Gleixner <[email protected]>'s advice.
>> 3. modify the error handle in acpi_processor_ids_walk()
>> by Thomas Gleixner <[email protected]>'s advice.
>> 4. add a new patch for restoring the order of CPU IDs
>>
>> v1 -> v2: 1. fix some comments.
>> 2. add the verification of duplicate processor id.
>>
>> Dou Liyang (5):
>> Revert"x86/acpi: Set persistent cpuid <-> nodeid mapping when booting"
>> Revert"x86/acpi: Enable MADT APIs to return disabled apicids"
>> x86/acpi: Restore the order of CPU IDs
>> acpi/processor: Implement DEVICE operator for processor enumeration
>> acpi/processor: Check for duplicate processor ids at hotplug time
>>
>> arch/x86/kernel/acpi/boot.c | 9 ++-
>> arch/x86/kernel/apic/apic.c | 26 +++------
>> drivers/acpi/acpi_processor.c | 57 +++++++++++++-----
>> drivers/acpi/bus.c | 1 -
>> drivers/acpi/processor_core.c | 133
>> +++++++-----------------------------------
>> include/linux/acpi.h | 5 +-
>> 6 files changed, 79 insertions(+), 152 deletions(-)
>>


Subject: [tip:x86/acpi] Revert "x86/acpi: Set persistent cpuid <-> nodeid mapping when booting"

Commit-ID: c962cff17dfa11f4a8227ac16de2b28aea3312e4
Gitweb: http://git.kernel.org/tip/c962cff17dfa11f4a8227ac16de2b28aea3312e4
Author: Dou Liyang <[email protected]>
AuthorDate: Fri, 3 Mar 2017 16:02:23 +0800
Committer: Thomas Gleixner <[email protected]>
CommitDate: Sat, 11 Mar 2017 14:41:18 +0100

Revert "x86/acpi: Set persistent cpuid <-> nodeid mapping when booting"

Revert: dc6db24d2476 ("x86/acpi: Set persistent cpuid <-> nodeid mapping when booting")

The mapping of "cpuid <-> nodeid" is established at boot time via ACPI
tables to keep associations of workqueues and other node related items
consistent across cpu hotplug.

But, ACPI tables are unreliable and failures with that boot time mapping
have been reported on machines where the ACPI table and the physical
information which is retrieved at actual hotplug is inconsistent.

Revert the mapping implementation so it can be replaced with a less error
prone approach.

Signed-off-by: Dou Liyang <[email protected]>
Tested-by: Xiaolong Ye <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Thomas Gleixner <[email protected]>

---
arch/x86/kernel/acpi/boot.c | 2 +-
drivers/acpi/acpi_processor.c | 5 ---
drivers/acpi/bus.c | 1 -
drivers/acpi/processor_core.c | 73 -------------------------------------------
include/linux/acpi.h | 3 --
5 files changed, 1 insertion(+), 83 deletions(-)

diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
index ae32838..f6b0e87 100644
--- a/arch/x86/kernel/acpi/boot.c
+++ b/arch/x86/kernel/acpi/boot.c
@@ -710,7 +710,7 @@ static void __init acpi_set_irq_model_ioapic(void)
#ifdef CONFIG_ACPI_HOTPLUG_CPU
#include <acpi/processor.h>

-int acpi_map_cpu2node(acpi_handle handle, int cpu, int physid)
+static int acpi_map_cpu2node(acpi_handle handle, int cpu, int physid)
{
#ifdef CONFIG_ACPI_NUMA
int nid;
diff --git a/drivers/acpi/acpi_processor.c b/drivers/acpi/acpi_processor.c
index 4467a80..5d208a9 100644
--- a/drivers/acpi/acpi_processor.c
+++ b/drivers/acpi/acpi_processor.c
@@ -182,11 +182,6 @@ int __weak arch_register_cpu(int cpu)

void __weak arch_unregister_cpu(int cpu) {}

-int __weak acpi_map_cpu2node(acpi_handle handle, int cpu, int physid)
-{
- return -ENODEV;
-}
-
static int acpi_processor_hotadd_init(struct acpi_processor *pr)
{
unsigned long long sta;
diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
index 80cb5eb..34fbe02 100644
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -1249,7 +1249,6 @@ static int __init acpi_init(void)
acpi_wakeup_device_init();
acpi_debugger_init();
acpi_setup_sb_notify_handler();
- acpi_set_processor_mapping();
return 0;
}

diff --git a/drivers/acpi/processor_core.c b/drivers/acpi/processor_core.c
index 611a558..a843862 100644
--- a/drivers/acpi/processor_core.c
+++ b/drivers/acpi/processor_core.c
@@ -278,79 +278,6 @@ int acpi_get_cpuid(acpi_handle handle, int type, u32 acpi_id)
}
EXPORT_SYMBOL_GPL(acpi_get_cpuid);

-#ifdef CONFIG_ACPI_HOTPLUG_CPU
-static bool __init
-map_processor(acpi_handle handle, phys_cpuid_t *phys_id, int *cpuid)
-{
- int type, id;
- u32 acpi_id;
- acpi_status status;
- acpi_object_type acpi_type;
- unsigned long long tmp;
- union acpi_object object = { 0 };
- struct acpi_buffer buffer = { sizeof(union acpi_object), &object };
-
- status = acpi_get_type(handle, &acpi_type);
- if (ACPI_FAILURE(status))
- return false;
-
- switch (acpi_type) {
- case ACPI_TYPE_PROCESSOR:
- status = acpi_evaluate_object(handle, NULL, NULL, &buffer);
- if (ACPI_FAILURE(status))
- return false;
- acpi_id = object.processor.proc_id;
-
- /* validate the acpi_id */
- if(acpi_processor_validate_proc_id(acpi_id))
- return false;
- break;
- case ACPI_TYPE_DEVICE:
- status = acpi_evaluate_integer(handle, "_UID", NULL, &tmp);
- if (ACPI_FAILURE(status))
- return false;
- acpi_id = tmp;
- break;
- default:
- return false;
- }
-
- type = (acpi_type == ACPI_TYPE_DEVICE) ? 1 : 0;
-
- *phys_id = __acpi_get_phys_id(handle, type, acpi_id, false);
- id = acpi_map_cpuid(*phys_id, acpi_id);
-
- if (id < 0)
- return false;
- *cpuid = id;
- return true;
-}
-
-static acpi_status __init
-set_processor_node_mapping(acpi_handle handle, u32 lvl, void *context,
- void **rv)
-{
- phys_cpuid_t phys_id;
- int cpu_id;
-
- if (!map_processor(handle, &phys_id, &cpu_id))
- return AE_ERROR;
-
- acpi_map_cpu2node(handle, cpu_id, phys_id);
- return AE_OK;
-}
-
-void __init acpi_set_processor_mapping(void)
-{
- /* Set persistent cpu <-> node mapping for all processors. */
- acpi_walk_namespace(ACPI_TYPE_PROCESSOR, ACPI_ROOT_OBJECT,
- ACPI_UINT32_MAX, set_processor_node_mapping,
- NULL, NULL, NULL);
-}
-#else
-void __init acpi_set_processor_mapping(void) {}
-#endif /* CONFIG_ACPI_HOTPLUG_CPU */
-
#ifdef CONFIG_ACPI_HOTPLUG_IOAPIC
static int get_ioapic_id(struct acpi_subtable_header *entry, u32 gsi_base,
u64 *phys_addr, int *ioapic_id)
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 673acda..63a7519 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -294,11 +294,8 @@ bool acpi_processor_validate_proc_id(int proc_id);
int acpi_map_cpu(acpi_handle handle, phys_cpuid_t physid, u32 acpi_id,
int *pcpu);
int acpi_unmap_cpu(int cpu);
-int acpi_map_cpu2node(acpi_handle handle, int cpu, int physid);
#endif /* CONFIG_ACPI_HOTPLUG_CPU */

-void acpi_set_processor_mapping(void);
-
#ifdef CONFIG_ACPI_HOTPLUG_IOAPIC
int acpi_get_ioapic_id(acpi_handle handle, u32 gsi_base, u64 *phys_addr);
#endif

Subject: [tip:x86/acpi] x86/acpi: Restore the order of CPU IDs

Commit-ID: 2b85b3d22920db7473e5fed5719e7955c0ec323e
Gitweb: http://git.kernel.org/tip/2b85b3d22920db7473e5fed5719e7955c0ec323e
Author: Dou Liyang <[email protected]>
AuthorDate: Fri, 3 Mar 2017 16:02:25 +0800
Committer: Thomas Gleixner <[email protected]>
CommitDate: Sat, 11 Mar 2017 14:41:19 +0100

x86/acpi: Restore the order of CPU IDs

The following commits:

f7c28833c2 ("x86/acpi: Enable acpi to register all possible cpus at
boot time") and 8f54969dc8 ("x86/acpi: Introduce persistent storage
for cpuid <-> apicid mapping")

... registered all the possible CPUs at boot time via ACPI tables to
make the mapping of cpuid <-> apicid fixed. Both enabled and disabled
CPUs could have a logical CPU ID after boot time.

But, ACPI tables are unreliable. the number amd order of Local APIC
entries which depends on the firmware is often inconsistent with the
physical devices. Even if they are consistent, The disabled CPUs which
take up some logical CPU IDs will also make the order discontinuous.

Revert the part of disabled CPUs registration, keep the allocation
logic of logical CPU IDs and also keep some code location changes.

Signed-off-by: Dou Liyang <[email protected]>
Tested-by: Xiaolong Ye <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Thomas Gleixner <[email protected]>

---
arch/x86/kernel/acpi/boot.c | 7 ++++++-
arch/x86/kernel/apic/apic.c | 26 +++++++-------------------
2 files changed, 13 insertions(+), 20 deletions(-)

diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
index f6b0e87..b2879cc23 100644
--- a/arch/x86/kernel/acpi/boot.c
+++ b/arch/x86/kernel/acpi/boot.c
@@ -179,10 +179,15 @@ static int acpi_register_lapic(int id, u32 acpiid, u8 enabled)
return -EINVAL;
}

+ if (!enabled) {
+ ++disabled_cpus;
+ return -EINVAL;
+ }
+
if (boot_cpu_physical_apicid != -1U)
ver = boot_cpu_apic_version;

- cpu = __generic_processor_info(id, ver, enabled);
+ cpu = generic_processor_info(id, ver);
if (cpu >= 0)
early_per_cpu(x86_cpu_to_acpiid, cpu) = acpiid;

diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index aee7ded..8ccb7ef 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -2063,7 +2063,7 @@ static int allocate_logical_cpuid(int apicid)
return nr_logical_cpuids++;
}

-int __generic_processor_info(int apicid, int version, bool enabled)
+int generic_processor_info(int apicid, int version)
{
int cpu, max = nr_cpu_ids;
bool boot_cpu_detected = physid_isset(boot_cpu_physical_apicid,
@@ -2121,11 +2121,9 @@ int __generic_processor_info(int apicid, int version, bool enabled)
if (num_processors >= nr_cpu_ids) {
int thiscpu = max + disabled_cpus;

- if (enabled) {
- pr_warning("APIC: NR_CPUS/possible_cpus limit of %i "
- "reached. Processor %d/0x%x ignored.\n",
- max, thiscpu, apicid);
- }
+ pr_warning("APIC: NR_CPUS/possible_cpus limit of %i "
+ "reached. Processor %d/0x%x ignored.\n",
+ max, thiscpu, apicid);

disabled_cpus++;
return -EINVAL;
@@ -2177,23 +2175,13 @@ int __generic_processor_info(int apicid, int version, bool enabled)
apic->x86_32_early_logical_apicid(cpu);
#endif
set_cpu_possible(cpu, true);
-
- if (enabled) {
- num_processors++;
- physid_set(apicid, phys_cpu_present_map);
- set_cpu_present(cpu, true);
- } else {
- disabled_cpus++;
- }
+ physid_set(apicid, phys_cpu_present_map);
+ set_cpu_present(cpu, true);
+ num_processors++;

return cpu;
}

-int generic_processor_info(int apicid, int version)
-{
- return __generic_processor_info(apicid, version, true);
-}
-
int hard_smp_processor_id(void)
{
return read_apic_id();

Subject: [tip:x86/acpi] Revert"x86/acpi: Enable MADT APIs to return disabled apicids"

Commit-ID: 09c3f2bd5c7e5f18687663acb6adc6b167484ca5
Gitweb: http://git.kernel.org/tip/09c3f2bd5c7e5f18687663acb6adc6b167484ca5
Author: Dou Liyang <[email protected]>
AuthorDate: Fri, 3 Mar 2017 16:02:24 +0800
Committer: Thomas Gleixner <[email protected]>
CommitDate: Sat, 11 Mar 2017 14:41:18 +0100

Revert"x86/acpi: Enable MADT APIs to return disabled apicids"

Revert: 8ad893faf2ea ("x86/acpi: Enable MADT APIs to return disabled apicids")

Remove the leftovers of the boot time 'cpuid <-> nodeid' mapping approach.

Signed-off-by: Dou Liyang <[email protected]>
Tested-by: Xiaolong Ye <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Thomas Gleixner <[email protected]>

---
drivers/acpi/processor_core.c | 60 ++++++++++++++++---------------------------
1 file changed, 22 insertions(+), 38 deletions(-)

diff --git a/drivers/acpi/processor_core.c b/drivers/acpi/processor_core.c
index a843862..b933061 100644
--- a/drivers/acpi/processor_core.c
+++ b/drivers/acpi/processor_core.c
@@ -32,12 +32,12 @@ static struct acpi_table_madt *get_madt_table(void)
}

static int map_lapic_id(struct acpi_subtable_header *entry,
- u32 acpi_id, phys_cpuid_t *apic_id, bool ignore_disabled)
+ u32 acpi_id, phys_cpuid_t *apic_id)
{
struct acpi_madt_local_apic *lapic =
container_of(entry, struct acpi_madt_local_apic, header);

- if (ignore_disabled && !(lapic->lapic_flags & ACPI_MADT_ENABLED))
+ if (!(lapic->lapic_flags & ACPI_MADT_ENABLED))
return -ENODEV;

if (lapic->processor_id != acpi_id)
@@ -48,13 +48,12 @@ static int map_lapic_id(struct acpi_subtable_header *entry,
}

static int map_x2apic_id(struct acpi_subtable_header *entry,
- int device_declaration, u32 acpi_id, phys_cpuid_t *apic_id,
- bool ignore_disabled)
+ int device_declaration, u32 acpi_id, phys_cpuid_t *apic_id)
{
struct acpi_madt_local_x2apic *apic =
container_of(entry, struct acpi_madt_local_x2apic, header);

- if (ignore_disabled && !(apic->lapic_flags & ACPI_MADT_ENABLED))
+ if (!(apic->lapic_flags & ACPI_MADT_ENABLED))
return -ENODEV;

if (device_declaration && (apic->uid == acpi_id)) {
@@ -66,13 +65,12 @@ static int map_x2apic_id(struct acpi_subtable_header *entry,
}

static int map_lsapic_id(struct acpi_subtable_header *entry,
- int device_declaration, u32 acpi_id, phys_cpuid_t *apic_id,
- bool ignore_disabled)
+ int device_declaration, u32 acpi_id, phys_cpuid_t *apic_id)
{
struct acpi_madt_local_sapic *lsapic =
container_of(entry, struct acpi_madt_local_sapic, header);

- if (ignore_disabled && !(lsapic->lapic_flags & ACPI_MADT_ENABLED))
+ if (!(lsapic->lapic_flags & ACPI_MADT_ENABLED))
return -ENODEV;

if (device_declaration) {
@@ -89,13 +87,12 @@ static int map_lsapic_id(struct acpi_subtable_header *entry,
* Retrieve the ARM CPU physical identifier (MPIDR)
*/
static int map_gicc_mpidr(struct acpi_subtable_header *entry,
- int device_declaration, u32 acpi_id, phys_cpuid_t *mpidr,
- bool ignore_disabled)
+ int device_declaration, u32 acpi_id, phys_cpuid_t *mpidr)
{
struct acpi_madt_generic_interrupt *gicc =
container_of(entry, struct acpi_madt_generic_interrupt, header);

- if (ignore_disabled && !(gicc->flags & ACPI_MADT_ENABLED))
+ if (!(gicc->flags & ACPI_MADT_ENABLED))
return -ENODEV;

/* device_declaration means Device object in DSDT, in the
@@ -112,7 +109,7 @@ static int map_gicc_mpidr(struct acpi_subtable_header *entry,
}

static phys_cpuid_t map_madt_entry(struct acpi_table_madt *madt,
- int type, u32 acpi_id, bool ignore_disabled)
+ int type, u32 acpi_id)
{
unsigned long madt_end, entry;
phys_cpuid_t phys_id = PHYS_CPUID_INVALID; /* CPU hardware ID */
@@ -130,20 +127,16 @@ static phys_cpuid_t map_madt_entry(struct acpi_table_madt *madt,
struct acpi_subtable_header *header =
(struct acpi_subtable_header *)entry;
if (header->type == ACPI_MADT_TYPE_LOCAL_APIC) {
- if (!map_lapic_id(header, acpi_id, &phys_id,
- ignore_disabled))
+ if (!map_lapic_id(header, acpi_id, &phys_id))
break;
} else if (header->type == ACPI_MADT_TYPE_LOCAL_X2APIC) {
- if (!map_x2apic_id(header, type, acpi_id, &phys_id,
- ignore_disabled))
+ if (!map_x2apic_id(header, type, acpi_id, &phys_id))
break;
} else if (header->type == ACPI_MADT_TYPE_LOCAL_SAPIC) {
- if (!map_lsapic_id(header, type, acpi_id, &phys_id,
- ignore_disabled))
+ if (!map_lsapic_id(header, type, acpi_id, &phys_id))
break;
} else if (header->type == ACPI_MADT_TYPE_GENERIC_INTERRUPT) {
- if (!map_gicc_mpidr(header, type, acpi_id, &phys_id,
- ignore_disabled))
+ if (!map_gicc_mpidr(header, type, acpi_id, &phys_id))
break;
}
entry += header->length;
@@ -161,15 +154,14 @@ phys_cpuid_t __init acpi_map_madt_entry(u32 acpi_id)
if (!madt)
return PHYS_CPUID_INVALID;

- rv = map_madt_entry(madt, 1, acpi_id, true);
+ rv = map_madt_entry(madt, 1, acpi_id);

acpi_put_table((struct acpi_table_header *)madt);

return rv;
}

-static phys_cpuid_t map_mat_entry(acpi_handle handle, int type, u32 acpi_id,
- bool ignore_disabled)
+static phys_cpuid_t map_mat_entry(acpi_handle handle, int type, u32 acpi_id)
{
struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
union acpi_object *obj;
@@ -190,38 +182,30 @@ static phys_cpuid_t map_mat_entry(acpi_handle handle, int type, u32 acpi_id,

header = (struct acpi_subtable_header *)obj->buffer.pointer;
if (header->type == ACPI_MADT_TYPE_LOCAL_APIC)
- map_lapic_id(header, acpi_id, &phys_id, ignore_disabled);
+ map_lapic_id(header, acpi_id, &phys_id);
else if (header->type == ACPI_MADT_TYPE_LOCAL_SAPIC)
- map_lsapic_id(header, type, acpi_id, &phys_id, ignore_disabled);
+ map_lsapic_id(header, type, acpi_id, &phys_id);
else if (header->type == ACPI_MADT_TYPE_LOCAL_X2APIC)
- map_x2apic_id(header, type, acpi_id, &phys_id, ignore_disabled);
+ map_x2apic_id(header, type, acpi_id, &phys_id);
else if (header->type == ACPI_MADT_TYPE_GENERIC_INTERRUPT)
- map_gicc_mpidr(header, type, acpi_id, &phys_id,
- ignore_disabled);
+ map_gicc_mpidr(header, type, acpi_id, &phys_id);

exit:
kfree(buffer.pointer);
return phys_id;
}

-static phys_cpuid_t __acpi_get_phys_id(acpi_handle handle, int type,
- u32 acpi_id, bool ignore_disabled)
+phys_cpuid_t acpi_get_phys_id(acpi_handle handle, int type, u32 acpi_id)
{
phys_cpuid_t phys_id;

- phys_id = map_mat_entry(handle, type, acpi_id, ignore_disabled);
+ phys_id = map_mat_entry(handle, type, acpi_id);
if (invalid_phys_cpuid(phys_id))
- phys_id = map_madt_entry(get_madt_table(), type, acpi_id,
- ignore_disabled);
+ phys_id = map_madt_entry(get_madt_table(), type, acpi_id);

return phys_id;
}

-phys_cpuid_t acpi_get_phys_id(acpi_handle handle, int type, u32 acpi_id)
-{
- return __acpi_get_phys_id(handle, type, acpi_id, true);
-}
-
int acpi_map_cpuid(phys_cpuid_t phys_id, u32 acpi_id)
{
#ifdef CONFIG_SMP

Subject: [tip:x86/acpi] acpi/processor: Implement DEVICE operator for processor enumeration

Commit-ID: 8c8cb30f49b86333d8e036e1945cf1a78c03577e
Gitweb: http://git.kernel.org/tip/8c8cb30f49b86333d8e036e1945cf1a78c03577e
Author: Dou Liyang <[email protected]>
AuthorDate: Fri, 3 Mar 2017 16:02:26 +0800
Committer: Thomas Gleixner <[email protected]>
CommitDate: Sat, 11 Mar 2017 14:41:20 +0100

acpi/processor: Implement DEVICE operator for processor enumeration

ACPI allows to declare processors either with the PROCESSOR or with the
DEVICE operator. The current implementation handles only the PROCESSOR
operator.

On a system which uses the DEVICE operator for processor enumeration the
evaluation fails.

Check for the ACPI type of the ACPI handle and evaluate PROCESSOR and
DEVICE types separately.

Signed-off-by: Dou Liyang <[email protected]>
Tested-by: Xiaolong Ye <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Thomas Gleixner <[email protected]>

---
drivers/acpi/acpi_processor.c | 39 ++++++++++++++++++++++++++++++++-------
1 file changed, 32 insertions(+), 7 deletions(-)

diff --git a/drivers/acpi/acpi_processor.c b/drivers/acpi/acpi_processor.c
index 5d208a9..9a98d7e 100644
--- a/drivers/acpi/acpi_processor.c
+++ b/drivers/acpi/acpi_processor.c
@@ -633,25 +633,50 @@ static acpi_status __init acpi_processor_ids_walk(acpi_handle handle,
void **rv)
{
acpi_status status;
+ acpi_object_type acpi_type;
+ unsigned long long uid;
union acpi_object object = { 0 };
struct acpi_buffer buffer = { sizeof(union acpi_object), &object };

- status = acpi_evaluate_object(handle, NULL, NULL, &buffer);
+ status = acpi_get_type(handle, &acpi_type);
if (ACPI_FAILURE(status))
- acpi_handle_info(handle, "Not get the processor object\n");
- else
- processor_validated_ids_update(object.processor.proc_id);
+ return false;
+
+ switch (acpi_type) {
+ case ACPI_TYPE_PROCESSOR:
+ status = acpi_evaluate_object(handle, NULL, NULL, &buffer);
+ if (ACPI_FAILURE(status))
+ goto err;
+ uid = object.processor.proc_id;
+ break;
+
+ case ACPI_TYPE_DEVICE:
+ status = acpi_evaluate_integer(handle, "_UID", NULL, &uid);
+ if (ACPI_FAILURE(status))
+ goto err;
+ break;
+ default:
+ goto err;
+ }
+
+ processor_validated_ids_update(uid);
+ return true;
+
+err:
+ acpi_handle_info(handle, "Invalid processor object\n");
+ return false;

- return AE_OK;
}

-static void __init acpi_processor_check_duplicates(void)
+void __init acpi_processor_check_duplicates(void)
{
- /* Search all processor nodes in ACPI namespace */
+ /* check the correctness for all processors in ACPI namespace */
acpi_walk_namespace(ACPI_TYPE_PROCESSOR, ACPI_ROOT_OBJECT,
ACPI_UINT32_MAX,
acpi_processor_ids_walk,
NULL, NULL, NULL);
+ acpi_get_devices(ACPI_PROCESSOR_DEVICE_HID, acpi_processor_ids_walk,
+ NULL, NULL);
}

bool __init acpi_processor_validate_proc_id(int proc_id)

Subject: [tip:x86/acpi] acpi/processor: Check for duplicate processor ids at hotplug time

Commit-ID: a77d6cd968497792e072b74dff45b891ba778ddb
Gitweb: http://git.kernel.org/tip/a77d6cd968497792e072b74dff45b891ba778ddb
Author: Dou Liyang <[email protected]>
AuthorDate: Fri, 3 Mar 2017 16:02:27 +0800
Committer: Thomas Gleixner <[email protected]>
CommitDate: Sat, 11 Mar 2017 14:41:20 +0100

acpi/processor: Check for duplicate processor ids at hotplug time

The check for duplicate processor ids happens at boot time based on the
ACPI table contents, but the final sanity checks for a processor happen
at hotplug time.

At hotplug time, where the physical information is available, which might
differ from the ACPI table information, a check for duplicate processor
ids is missing.

Add it to the hotplug checks and rename the function so it better
reflects its purpose.

Signed-off-by: Dou Liyang <[email protected]>
Tested-by: Xiaolong Ye <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Thomas Gleixner <[email protected]>

---
drivers/acpi/acpi_processor.c | 13 ++++++++++---
include/linux/acpi.h | 2 +-
2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/acpi/acpi_processor.c b/drivers/acpi/acpi_processor.c
index 9a98d7e..0143135 100644
--- a/drivers/acpi/acpi_processor.c
+++ b/drivers/acpi/acpi_processor.c
@@ -280,6 +280,13 @@ static int acpi_processor_get_info(struct acpi_device *device)
pr->acpi_id = value;
}

+ if (acpi_duplicate_processor_id(pr->acpi_id)) {
+ dev_err(&device->dev,
+ "Failed to get unique processor _UID (0x%x)\n",
+ pr->acpi_id);
+ return -ENODEV;
+ }
+
pr->phys_id = acpi_get_phys_id(pr->handle, device_declaration,
pr->acpi_id);
if (invalid_phys_cpuid(pr->phys_id))
@@ -580,7 +587,7 @@ static struct acpi_scan_handler processor_container_handler = {
static int nr_unique_ids __initdata;

/* The number of the duplicate processor IDs */
-static int nr_duplicate_ids __initdata;
+static int nr_duplicate_ids;

/* Used to store the unique processor IDs */
static int unique_processor_ids[] __initdata = {
@@ -588,7 +595,7 @@ static int unique_processor_ids[] __initdata = {
};

/* Used to store the duplicate processor IDs */
-static int duplicate_processor_ids[] __initdata = {
+static int duplicate_processor_ids[] = {
[0 ... NR_CPUS - 1] = -1,
};

@@ -679,7 +686,7 @@ void __init acpi_processor_check_duplicates(void)
NULL, NULL);
}

-bool __init acpi_processor_validate_proc_id(int proc_id)
+bool acpi_duplicate_processor_id(int proc_id)
{
int i;

diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 63a7519..9b05886 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -287,7 +287,7 @@ static inline bool invalid_phys_cpuid(phys_cpuid_t phys_id)
}

/* Validate the processor object's proc_id */
-bool acpi_processor_validate_proc_id(int proc_id);
+bool acpi_duplicate_processor_id(int proc_id);

#ifdef CONFIG_ACPI_HOTPLUG_CPU
/* Arch dependent functions for cpu hotplug support */