Discussion:
ACPI "_PDC" - acpi_processor_set_pdc() - execution regression - Linux-3.x
w***@public.gmane.org
2011-12-08 00:00:51 UTC
Permalink
We have a regression on the ACPI stack of the last linux kernel line 3.x (3.1.4,
3.2-rc4...). The ACPI "_PDC" chunk is not executed on some computers (e.g. Dell
X300; the function acpi_processor_set_pdc() is not called). This issue yields to
an uninitialized state of some ACPI variables.

A patch is available below. This patch come back to the previous linux behavior,
and works fine.

Best Regards,
Wallak.

--- linux-3.1.4-mdf/drivers/acpi/processor_core.c.orig 2011-12-07
23:12:57.000000000 +0100
+++ linux-3.1.4-mdf/drivers/acpi/processor_core.c 2011-12-07
23:13:39.000000000 +0100
@@ -223,8 +223,8 @@
type = (acpi_type == ACPI_TYPE_DEVICE) ? 1 : 0;
cpuid = acpi_get_cpuid(handle, type, acpi_id);

- if (cpuid == -1)
- return false;
+ if ((cpuid == -1) && (num_possible_cpus() > 1))
+ return false;

return true;
}
Kok, Auke-jan H
2011-12-08 01:01:47 UTC
Permalink
Post by w***@public.gmane.org
We have a regression on the ACPI stack of the last linux kernel line 3.x (3.1.4,
3.2-rc4...). The ACPI "_PDC" chunk is not executed on some computers (e.g. Dell
X300; the function acpi_processor_set_pdc() is not called). This issue yields to
an uninitialized state of some ACPI variables.
A patch is available below. This patch come back to the previous linux behavior,
and works fine.
Do you have a reference to the commit that broke it? It would be smart
to include that. Also, a signed-off-by line will help, as well as
including Len on the Cc.

Auke
Post by w***@public.gmane.org
Best Regards,
Wallak.
--- linux-3.1.4-mdf/drivers/acpi/processor_core.c.orig  2011-12-07
23:12:57.000000000 +0100
+++ linux-3.1.4-mdf/drivers/acpi/processor_core.c       2011-12-07
23:13:39.000000000 +0100
@@ -223,8 +223,8 @@
       type = (acpi_type == ACPI_TYPE_DEVICE) ? 1 : 0;
       cpuid = acpi_get_cpuid(handle, type, acpi_id);
-       if (cpuid == -1)
-               return false;
+       if ((cpuid == -1) && (num_possible_cpus() > 1))
+               return false;
       return true;
 }
_______________________________________________
Discuss mailing list
http://lists.lesswatts.org/listinfo/discuss
w***@public.gmane.org
2011-12-08 20:13:19 UTC
Permalink
The patch that removes the condition is available below, as well, as the patch
that adds it.

If processor_physically_present() returns false, acpi_processor_set_pdc() is not
executed; It seems, this is always the case on UP kernel, if we use the
following condition: if (cpuid == -1) return false; This last statement yields
to an ACPI layer not fully initialized. As far as I know this is a mistake.

Best Regards,
Wallak.


#Previous linux change to this code:
----------------------------
commit 932df7414336a00f45e5aec62724cf736b0bcfd4
Author: Lin Ming <ming.m.lin-***@public.gmane.org>
AuthorDate: Mon May 16 09:11:00 2011 +0800
Commit: Len Brown <len.brown-***@public.gmane.org>
CommitDate: Sun May 29 02:17:56 2011 -0400

ACPI: processor: fix processor_physically_present in UP kernel

Usually, there are multiple processors defined in ACPI table, for
example

Scope (_PR)
{
Processor (CPU0, 0x00, 0x00000410, 0x06) {}
Processor (CPU1, 0x01, 0x00000410, 0x06) {}
Processor (CPU2, 0x02, 0x00000410, 0x06) {}
Processor (CPU3, 0x03, 0x00000410, 0x06) {}
}

processor_physically_present(...) will be called to check whether those
processors are physically present.

Currently we have below codes in processor_physically_present,

cpuid = acpi_get_cpuid(...);
if ((cpuid == -1) && (num_possible_cpus() > 1))
return false;
return true;

In UP kernel, acpi_get_cpuid(...) always return -1 and
num_possible_cpus() always return 1, so
processor_physically_present(...) always returns true for all passed in
processor handles.

This is wrong for UP processor or SMP processor running UP kernel.

This patch removes the !SMP version of acpi_get_cpuid(), so both UP and
SMP kernel use the same acpi_get_cpuid function.

And for UP kernel, only processor 0 is valid.

https://bugzilla.kernel.org/show_bug.cgi?id=16548
https://bugzilla.kernel.org/show_bug.cgi?id=16357

Tested-by: Anton Kochkov <anton.kochkov-***@public.gmane.org>
Tested-by: Ambroz Bizjak <ambrop7-***@public.gmane.org>
Signed-off-by: Lin Ming <ming.m.lin-***@public.gmane.org>
Signed-off-by: Len Brown <len.brown-***@public.gmane.org>

diff --git a/drivers/acpi/processor_core.c b/drivers/acpi/processor_core.c
index 25bf17d..02d2a4c 100644
--- a/drivers/acpi/processor_core.c
+++ b/drivers/acpi/processor_core.c
@@ -37,7 +37,6 @@ static struct dmi_system_id __initdata
processor_idle_dmi_table[] = {
{},
};

-#ifdef CONFIG_SMP
static int map_lapic_id(struct acpi_subtable_header *entry,
u32 acpi_id, int *apic_id)
{
@@ -165,7 +164,9 @@ exit:

int acpi_get_cpuid(acpi_handle handle, int type, u32 acpi_id)
{
+#ifdef CONFIG_SMP
int i;
+#endif
int apic_id = -1;

apic_id = map_mat_entry(handle, type, acpi_id);
@@ -174,14 +175,19 @@ int acpi_get_cpuid(acpi_handle handle, int type, u32
acpi_id)
if (apic_id == -1)
return apic_id;

+#ifdef CONFIG_SMP
for_each_possible_cpu(i) {
if (cpu_physical_id(i) == apic_id)
return i;
}
+#else
+ /* In UP kernel, only processor 0 is valid */
+ if (apic_id == 0)
+ return apic_id;
+#endif
return -1;
}
EXPORT_SYMBOL_GPL(acpi_get_cpuid);
-#endif

static bool __init processor_physically_present(acpi_handle handle)
{
@@ -217,7 +223,7 @@ static bool __init processor_physically_present(acpi_handle
handle)
type = (acpi_type == ACPI_TYPE_DEVICE) ? 1 : 0;
cpuid = acpi_get_cpuid(handle, type, acpi_id);

- if ((cpuid == -1) && (num_possible_cpus() > 1))
+ if (cpuid == -1)
return false;

return true;
diff --git a/include/acpi/processor.h b/include/acpi/processor.h
index 55192ac..ba4928c 100644
--- a/include/acpi/processor.h
+++ b/include/acpi/processor.h
@@ -310,14 +310,7 @@ static inline int acpi_processor_get_bios_limit(int cpu,
unsigned int *limit)

/* in processor_core.c */
void acpi_processor_set_pdc(acpi_handle handle);
-#ifdef CONFIG_SMP
int acpi_get_cpuid(acpi_handle, int type, u32 acpi_id);
-#else
-static inline int acpi_get_cpuid(acpi_handle handle, int type, u32 acpi_id)
-{
- return -1;
-}
-#endif

/* in processor_throttling.c */
int acpi_processor_tstate_has_changed(struct acpi_processor *pr);


-------------------------------------------------------------------------
commit 856b185dd23da39e562983fbf28860f54e661b41
Author: Alex Chiang <achiang-Z7WLFzj8eWMS+***@public.gmane.org>
AuthorDate: Thu Jun 17 09:08:54 2010 -0600
Commit: Len Brown <len.brown-***@public.gmane.org>
CommitDate: Mon Jul 12 13:28:34 2010 -0400

ACPI: processor: fix processor_physically_present on UP

The commit 5d554a7bb06 (ACPI: processor: add internal
processor_physically_present()) is broken on uniprocessor (UP)
configurations, as acpi_get_cpuid() will always return -1.

We use the value of num_possible_cpus() to tell us whether we got
an invalid cpuid from acpi_get_cpuid() in the SMP case, or if
instead, we are UP, in which case num_possible_cpus() is #defined
as 1.

We use num_possible_cpus() instead of num_online_cpus() to
protect ourselves against the scenario of CPU hotplug, and we've
taken down all the CPUs except one.

Thanks to Jan Pogadl for initial report and analysis and Chen
Gong for review.

https://bugzilla.kernel.org/show_bug.cgi?id=16357

Reported-by: Jan Pogadl <pogadl.jan-gM/Ye1E23mwN+***@public.gmane.org>:
Reviewed-by: Chen Gong <gong.chen-VuQAYsv1563Yd54FQh9/***@public.gmane.org>
Signed-off-by: Alex Chiang <achiang-Z7WLFzj8eWMS+***@public.gmane.org>
Signed-off-by: Len Brown <len.brown-***@public.gmane.org>

diff --git a/drivers/acpi/processor_core.c b/drivers/acpi/processor_core.c
index 5128435..e9699aa 100644
--- a/drivers/acpi/processor_core.c
+++ b/drivers/acpi/processor_core.c
@@ -223,7 +223,7 @@ static bool processor_physically_present(acpi_handle handle)
type = (acpi_type == ACPI_TYPE_DEVICE) ? 1 : 0;
cpuid = acpi_get_cpuid(handle, type, acpi_id);

- if (cpuid == -1)
+ if ((cpuid == -1) && (num_possible_cpus() > 1))
return false;

return true;

--------------------------------------------------
Post by w***@public.gmane.org
Post by w***@public.gmane.org
We have a regression on the ACPI stack of the last linux kernel line 3.x
(3.1.4,
Post by w***@public.gmane.org
3.2-rc4...). The ACPI "_PDC" chunk is not executed on some computers (e.g.
Dell
Post by w***@public.gmane.org
X300; the function acpi_processor_set_pdc() is not called). This issue
yields to
Post by w***@public.gmane.org
an uninitialized state of some ACPI variables.
A patch is available below. This patch come back to the previous linux
behavior,
Post by w***@public.gmane.org
and works fine.
Do you have a reference to the commit that broke it? It would be smart
to include that. Also, a signed-off-by line will help, as well as
including Len on the Cc.
Auke
Post by w***@public.gmane.org
Best Regards,
Wallak.
--- linux-3.1.4-mdf/drivers/acpi/processor_core.c.orig  2011-12-07
23:12:57.000000000 +0100
+++ linux-3.1.4-mdf/drivers/acpi/processor_core.c       2011-12-07
23:13:39.000000000 +0100
@@ -223,8 +223,8 @@
       type = (acpi_type == ACPI_TYPE_DEVICE) ? 1 : 0;
       cpuid = acpi_get_cpuid(handle, type, acpi_id);
-       if (cpuid == -1)
-               return false;
+       if ((cpuid == -1) && (num_possible_cpus() > 1))
+               return false;
       return true;
 }
_______________________________________________
Discuss mailing list
http://lists.lesswatts.org/listinfo/discuss
Loading...