* Re: [linux-pm] [PATCH v2 0/8] RFC: CPU frequency min/max as PM QoS params
From: Rafael J. Wysocki @ 2012-01-18 23:10 UTC (permalink / raw)
To: Antti P Miettinen; +Cc: linux-pm, cpufreq, mark gross
In-Reply-To: <87ipk9qwff.fsf@amiettinen-lnx.nvidia.com>
On Wednesday, January 18, 2012, Antti P Miettinen wrote:
> "Rafael J. Wysocki" <rjw@sisk.pl> writes:
> [..]
> > If I understand you correctly, you want to have an iterface for specifying
> > min and max frequencies from user space. I can understand that. At least
> > I can see some use cases for that.
> >
> > Now, the question is if using the PM QoS framework is the right way to do
> > that.
> >
> >> The change to cpufreq core just adds two read-only files to be able to
> >> inspect user_policy.min/max in addition to the currently enforced
> >> policy->min/max. Yes - there has been the possibility of using the sysfs
> >> min for setting a frequency floor but this is problematic when there are
> >> multiple clients. You'd need some kind of arbitration and book keeping
> >> to set/restore the minimum. And PM QoS provides exactly this mechanism.
> >
> > Just as I suspected. :-)
> >
> > OK, so what's your anticipated usage model of this?
>
> We've been experimenting with hooking the frequency boost to e.g. touch
> screen UI events and application launch. I believe there are also some
> specific applications that would want to either set a frequency floor or
> ceiling in some situations.
>
> >> I think the kernel needs to be extended to handle more PM constraints
> >> and PM QoS is the closest thing I know for this kind of
> >> functionality. However, I'm open to suggestions about alternative
> >> approaches. I think we need e.g. more than just min/max "reduction
> >> operators". Ideas, anyone?
> >
> > I first need to know who those multiple clients are going to be.
> >
> > Thanks,
> > Rafael
>
> For the touch screen UI, the boost could be implemented as a kernel
> module hooked with suitable event filter to the input event
> stream. Another possibility is the UI framework in user space. Some
> specific user space applications would also be clients.
I see.
However, if you use PM QoS for that as proposed, there will be only one
misc device for the minimum CPU frequency and one for the maximum, so
user space will only be able to specify those numbers globally, ie. for
all CPUs in the system. Is that your intention?
Rafael
^ permalink raw reply
* Re: [PATCH v2 0/8] RFC: CPU frequency min/max as PM QoS params
From: Antti P Miettinen @ 2012-01-18 20:22 UTC (permalink / raw)
To: linux-pm
In-Reply-To: <20120118034406.GD27153@mgross-G62>
mark gross <markgross@thegnar.org> writes:
> Would it be possible for you to split this set up where you only have
> the cpu_min_freq enabling in pos.c and cpufreq and make the rest of the
> set a separate patch set?
Would that be patches 1-5 vs the rest?
> As you have it currently the parts I care about are intermixed with
> controversial things (max freq and the new cpufreq exports).
I think the cpufreq changes, patches 3-5, are necessary. Well, patch 3
is not necessary for functionality but would be helpful for
debugging/diagnosis and IMHO makes sense as a companion to patch 4. The
max freq is in patches 7 and 8.
> Also could you make the entire set git am friendly? Only the first
> patch am's the reset don't.
Could you help me reproduce the problem? For me the mails apply fine.
Also a colleague tested them and encountered no problems. When I know
how to fix the set I'll post a split version.
--Antti
^ permalink raw reply
* [linux-pm][RFC PATCH] thermal: Add support to report cooling statistics achieved by cooling devices
From: Amit Daniel Kachhap @ 2012-01-18 9:21 UTC (permalink / raw)
To: linux-pm
Cc: linux-kernel, mjg59, linux-acpi, lenb, linaro-dev, amit.kachhap,
vincent.guittot, rob.lee, patches
Add a sysfs node code to report effective cooling of all cooling devices
attached to each trip points of a thermal zone. The cooling data reported
will be absolute if the higher temperature trip points are arranged first
otherwise the cooling stats is the cumulative effect of the earlier
invoked cooling handlers.
The basic assumption is that cooling devices will bring down the temperature
in a symmetric manner and those statistics can be stored back and used for
further tuning of the system.
Signed-off-by: Amit Daniel Kachhap <amit.kachhap@linaro.org>
---
Documentation/thermal/sysfs-api.txt | 10 ++++
drivers/thermal/thermal_sys.c | 96 +++++++++++++++++++++++++++++++++++
include/linux/thermal.h | 8 +++
3 files changed, 114 insertions(+), 0 deletions(-)
diff --git a/Documentation/thermal/sysfs-api.txt b/Documentation/thermal/sysfs-api.txt
index b61e46f..1db9a9d 100644
--- a/Documentation/thermal/sysfs-api.txt
+++ b/Documentation/thermal/sysfs-api.txt
@@ -209,6 +209,13 @@ passive
Valid values: 0 (disabled) or greater than 1000
RW, Optional
+trip_stats
+ This attribute presents the effective cooling generated from all the
+ cooling devices attached to a trip point. The output is a pair of value
+ for each trip point. Each pair represents
+ (trip index, cooling temperature difference in millidegree Celsius)
+ RO, Optional
+
*****************************
* Cooling device attributes *
*****************************
@@ -261,6 +268,9 @@ method, the sys I/F structure will be built like this:
|---cdev0_trip_point: 1 /* cdev0 can be used for passive */
|---cdev1: --->/sys/class/thermal/cooling_device3
|---cdev1_trip_point: 2 /* cdev1 can be used for active[0]*/
+ |---trip_stats 0 0
+ 1 8000 /*trip 1 causes 8 degree Celsius drop*/
+ 2 3000 /*trip 2 causes 3 degree Celsius drop*/
|cooling_device0:
|---type: Processor
diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c
index dd9a574..47e7b6e 100644
--- a/drivers/thermal/thermal_sys.c
+++ b/drivers/thermal/thermal_sys.c
@@ -92,6 +92,64 @@ static void release_idr(struct idr *idr, struct mutex *lock, int id)
if (lock)
mutex_unlock(lock);
}
+static void update_cooling_stats(struct thermal_zone_device *tz, long cur_temp)
+{
+ int count, max_index, cur_interval;
+ long trip_temp, max_temp = 0, cool_temp;
+ static int last_trip_level = -1;
+
+ if (cur_temp >= tz->last_temperature)
+ return;
+
+ /* find the trip according to last temperature */
+ for (count = 0; count < tz->trips; count++) {
+ tz->ops->get_trip_temp(tz, count, &trip_temp);
+ if (tz->last_temperature >= trip_temp) {
+ if (max_temp < trip_temp) {
+ max_temp = trip_temp;
+ max_index = count;
+ }
+ }
+ }
+
+ if (!max_temp) {
+ last_trip_level = -1;
+ return;
+ }
+
+ cur_interval = tz->stat[max_index].interval_ptr;
+ cool_temp = tz->last_temperature - cur_temp;
+
+ if (last_trip_level != max_index) {
+ if (++cur_interval == INTERVAL_HISTORY)
+ cur_interval = 0;
+ tz->stat[max_index].cool_temp[cur_interval] = cool_temp;
+ tz->stat[max_index].interval_ptr = cur_interval;
+ last_trip_level = max_index;
+ } else {
+ tz->stat[max_index].cool_temp[cur_interval] += cool_temp;
+ }
+}
+
+static int calculate_cooling_temp_avg(struct thermal_zone_device *tz, int trip,
+ int *avg_cool)
+{
+ int result = 0, count = 0, used_data = 0;
+
+ if (trip > THERMAL_MAX_TRIPS)
+ return -EINVAL;
+
+ *avg_cool = 0;
+ for (count = 0; count < INTERVAL_HISTORY; count++) {
+ if (tz->stat[trip].cool_temp[count] > 0) {
+ *avg_cool += tz->stat[trip].cool_temp[count];
+ used_data++;
+ }
+ }
+ if (used_data > 1)
+ *avg_cool = (*avg_cool)/used_data;
+ return result;
+}
/* sys I/F for thermal zone */
@@ -493,6 +551,26 @@ temp_crit_show(struct device *dev, struct device_attribute *attr,
return sprintf(buf, "%ld\n", temperature);
}
+static ssize_t
+thermal_cooling_trip_stats_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct thermal_zone_device *tz = to_thermal_zone(dev);
+ int avg_cool = 0, result, trip_index;
+ ssize_t len = 0;
+
+ for (trip_index = 0; trip_index < tz->trips; trip_index++) {
+ result = calculate_cooling_temp_avg(tz,
+ trip_index, &avg_cool);
+ if (!result)
+ len += sprintf(buf + len, "%d %d\n",
+ trip_index, avg_cool);
+ }
+ return len;
+}
+static DEVICE_ATTR(trip_stats, 0444,
+ thermal_cooling_trip_stats_show, NULL);
static struct thermal_hwmon_device *
thermal_hwmon_lookup_by_type(const struct thermal_zone_device *tz)
@@ -1049,6 +1127,8 @@ void thermal_zone_device_update(struct thermal_zone_device *tz)
goto leave;
}
+ update_cooling_stats(tz, temp);
+
for (count = 0; count < tz->trips; count++) {
tz->ops->get_trip_type(tz, count, &trip_type);
tz->ops->get_trip_temp(tz, count, &trip_temp);
@@ -1181,6 +1261,13 @@ struct thermal_zone_device *thermal_zone_device_register(char *type,
return ERR_PTR(result);
}
+ /*Allocate variables for cooling stats*/
+ tz->stat = devm_kzalloc(&tz->device,
+ sizeof(struct thermal_cooling_stats) * trips,
+ GFP_KERNEL);
+ if (!tz->stat)
+ goto unregister;
+
/* sys I/F */
if (type) {
result = device_create_file(&tz->device, &dev_attr_type);
@@ -1207,6 +1294,12 @@ struct thermal_zone_device *thermal_zone_device_register(char *type,
passive = 1;
}
+ if (trips > 0) {
+ result = device_create_file(&tz->device, &dev_attr_trip_stats);
+ if (result)
+ goto unregister;
+ }
+
if (!passive)
result = device_create_file(&tz->device,
&dev_attr_passive);
@@ -1282,6 +1375,9 @@ void thermal_zone_device_unregister(struct thermal_zone_device *tz)
for (count = 0; count < tz->trips; count++)
TRIP_POINT_ATTR_REMOVE(&tz->device, count);
+ if (tz->trips > 0)
+ device_remove_file(&tz->device, &dev_attr_trip_stats);
+
thermal_remove_hwmon_sysfs(tz);
release_idr(&thermal_tz_idr, &thermal_idr_lock, tz->id);
idr_destroy(&tz->idr);
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index 47b4a27..47504c7 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -72,6 +72,13 @@ struct thermal_cooling_device_ops {
#define THERMAL_TRIPS_NONE -1
#define THERMAL_MAX_TRIPS 12
#define THERMAL_NAME_LENGTH 20
+#define INTERVAL_HISTORY 12
+
+struct thermal_cooling_stats {
+ int cool_temp[INTERVAL_HISTORY];
+ int interval_ptr;
+};
+
struct thermal_cooling_device {
int id;
char type[THERMAL_NAME_LENGTH];
@@ -102,6 +109,7 @@ struct thermal_zone_device {
struct list_head cooling_devices;
struct idr idr;
struct mutex lock; /* protect cooling devices list */
+ struct thermal_cooling_stats *stat;
struct list_head node;
struct delayed_work poll_queue;
};
--
1.7.1
^ permalink raw reply related
* Re: [PATCH v2 0/8] RFC: CPU frequency min/max as PM QoS params
From: Antti P Miettinen @ 2012-01-18 8:15 UTC (permalink / raw)
To: linux-pm; +Cc: cpufreq
In-Reply-To: <20120118031319.GB27153@mgross-G62>
mark gross <markgross@thegnar.org> writes:
> I'm not a big fan of the cpufreq seamanly redundant export either.
> Doesn't the equivalent data get exported under
> /sys/devices/system/cpu/cpu?/cpufreq/ ?
The added sysfs nodes are under /sys/devices/system/cpu/cpu?/cpufreq.
They do no not duplicate functionality, they are just an
addition. Currently you can request a new minimum by writing to
scaling_min_freq and you can view the currently enforced policy->min via
the same file. Patch 3 adds read-only policy_{min,max}_freq nodes for
being able to inspect the user_policy.min/max. This is related to patch
4 which preserves the requested min/max in user_policy instead of
storing the enforced min/max to user_policy. This is in turn related to
patch 5. We need to be able to revert back to requested min/max when PM
QoS constraints get lifted. I think we do not want to overwrite
user_policy min/max with policy->min/max as those values can be affected
by temporary constraints.
I would welcome more comments on patches 3 and 4.
--Antti
^ permalink raw reply
* Re: [PATCH v2 0/8] RFC: CPU frequency min/max as PM QoS params
From: Antti P Miettinen @ 2012-01-18 7:52 UTC (permalink / raw)
To: linux-pm; +Cc: cpufreq
In-Reply-To: <201201172227.59080.rjw@sisk.pl>
"Rafael J. Wysocki" <rjw@sisk.pl> writes:
[..]
> If I understand you correctly, you want to have an iterface for specifying
> min and max frequencies from user space. I can understand that. At least
> I can see some use cases for that.
>
> Now, the question is if using the PM QoS framework is the right way to do
> that.
>
>> The change to cpufreq core just adds two read-only files to be able to
>> inspect user_policy.min/max in addition to the currently enforced
>> policy->min/max. Yes - there has been the possibility of using the sysfs
>> min for setting a frequency floor but this is problematic when there are
>> multiple clients. You'd need some kind of arbitration and book keeping
>> to set/restore the minimum. And PM QoS provides exactly this mechanism.
>
> Just as I suspected. :-)
>
> OK, so what's your anticipated usage model of this?
We've been experimenting with hooking the frequency boost to e.g. touch
screen UI events and application launch. I believe there are also some
specific applications that would want to either set a frequency floor or
ceiling in some situations.
>> I think the kernel needs to be extended to handle more PM constraints
>> and PM QoS is the closest thing I know for this kind of
>> functionality. However, I'm open to suggestions about alternative
>> approaches. I think we need e.g. more than just min/max "reduction
>> operators". Ideas, anyone?
>
> I first need to know who those multiple clients are going to be.
>
> Thanks,
> Rafael
For the touch screen UI, the boost could be implemented as a kernel
module hooked with suitable event filter to the input event
stream. Another possibility is the UI framework in user space. Some
specific user space applications would also be clients.
--Antti
^ permalink raw reply
* Re: [PATCH v2 0/8] RFC: CPU frequency min/max as PM QoS params
From: mark gross @ 2012-01-18 3:44 UTC (permalink / raw)
To: Antti P Miettinen; +Cc: linux-pm
In-Reply-To: <1326697201-32406-1-git-send-email-amiettinen@nvidia.com>
Would it be possible for you to split this set up where you only have
the cpu_min_freq enabling in pos.c and cpufreq and make the rest of the
set a separate patch set?
As you have it currently the parts I care about are intermixed with
controversial things (max freq and the new cpufreq exports).
Also could you make the entire set git am friendly? Only the first
patch am's the reset don't.
--mark
On Mon, Jan 16, 2012 at 08:59:53AM +0200, Antti P Miettinen wrote:
> [did not reach linux-pm as I sent to wrong address, sorry for
> duplicates]
>
> The inspiration for this patch series is the N9 CPU frequency boost
> upon input events:
>
> http://www.spinics.net/lists/cpufreq/msg00667.html
>
> and the related changes in git://codeaurora.org/kernel/msm.git tree.
> Those patches modify the ondemand cpufreq governor. This patch series
> adds minimum and maximum CPU frequency as PM QoS parameters and
> modifies the cpufreq core to enforce the PM QoS limits. There is also
> an example module for boosting the frequency upon input events.
>
> I've been testing these changes against Ubuntu 3.2 kernel on a Dell
> E6420 with the ACPI cpufreq driver. The patches are against
> linux-next/master, compile tested against it.
>
> V2:
> * split min and max to separate commits
> * handle PM QoS min above max as max
> * handle PM QoS max below min as min
>
> --Antti
>
> Alex Frid (1):
> PM QoS: Simplify PM QoS expansion/merge
>
> Antti P Miettinen (7):
> PM QoS: Add CPU frequency minimum as PM QoS param
> cpufreq: Export user_policy min/max
> cpufreq: Preserve sysfs min/max request
> cpufreq: Enforce PM QoS minimum limit
> input: CPU frequency booster
> PM QoS: Add CPU frequency maximum as PM QoS param
> cpufreq: Enforce PM QoS maximum frequency
>
> drivers/cpufreq/cpufreq.c | 59 +++++++++++++-
> drivers/input/Kconfig | 9 ++
> drivers/input/Makefile | 1 +
> drivers/input/input-cfboost.c | 174 +++++++++++++++++++++++++++++++++++++++++
> include/linux/pm_qos.h | 19 ++++-
> kernel/power/qos.c | 55 ++++++++++----
> 6 files changed, 295 insertions(+), 22 deletions(-)
> create mode 100644 drivers/input/input-cfboost.c
>
> --
> 1.7.4.1
>
> _______________________________________________
> linux-pm mailing list
> linux-pm@lists.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/linux-pm
^ permalink raw reply
* Re: [PATCH v2 0/8] RFC: CPU frequency min/max as PM QoS params
From: mark gross @ 2012-01-18 3:13 UTC (permalink / raw)
To: Rafael J. Wysocki; +Cc: linux-pm, Antti P Miettinen, cpufreq, mark gross
In-Reply-To: <201201162238.57556.rjw@sisk.pl>
On Mon, Jan 16, 2012 at 10:38:57PM +0100, Rafael J. Wysocki wrote:
> Hi,
>
> On Monday, January 16, 2012, Antti P Miettinen wrote:
> > [did not reach linux-pm as I sent to wrong address, sorry for
> > duplicates]
> >
> > The inspiration for this patch series is the N9 CPU frequency boost
> > upon input events:
> >
> > http://www.spinics.net/lists/cpufreq/msg00667.html
> >
> > and the related changes in git://codeaurora.org/kernel/msm.git tree.
> > Those patches modify the ondemand cpufreq governor. This patch series
> > adds minimum and maximum CPU frequency as PM QoS parameters and
> > modifies the cpufreq core to enforce the PM QoS limits.
>
> If that hasn't been clear enough so far, I'm still not convinced that using
> PM QoS for that is a good idea.
>
> First off, frequency as a unit of throughput is questionable to say the least,
> because it isn't portable from one system to another. Moreover, even on a
> given system it isn't particularly clear what the exact correspondence
> between frequency and throughput actually is.
You are right. The notion of throughput of a CPU is really hard to
quantify. Perhaps not using the term "throughput" would help?
The base issue I see, the Intel platform, is needing is that sometimes
we need to block the lowest P-states that the ondemand governor goes for
because those P-states result in media / graphics workloads dropping
frames. However; GPU intensive workloads do not stress the CPU so the
ondemand governor goes for the low p-state.
I could use some way of constraining the PM-throttling of the
cpu-freq that can be hit from kernel or user mode. So the graphics
driver can dynamically adjust the constraint request on the cpufreq
subsystem.
It is problematic that any driver requesting a given frequency request
is not portable across ISA's or even processor families in the same ISA.
But, maybe such a driver should use a module parameter to work around
this lack of portability?
> Second, it's not particularly clear what the meaning of the "min" frequency
> is supposed to be in terms of throughput.
It should mean "please cpufreq do not put the cpu into a state where its
clock runs slower than min". I don't think we should talk about it as
throughput because thats not what the cpufreq controls.
>
> Moreover, you make cpufreq export user_policy.min and user_policy.max
> regardless of the new PM QoS parameters, so it looks like you could use those
> new attributes to set the min/max as well.
I'm not a big fan of the cpufreq seamanly redundant export either.
Doesn't the equivalent data get exported under
/sys/devices/system/cpu/cpu?/cpufreq/ ?
my 2 cents.
--mark
^ permalink raw reply
* Re: [PATCH v2 1/8] PM QoS: Simplify PM QoS expansion/merge
From: mark gross @ 2012-01-18 2:50 UTC (permalink / raw)
To: Rafael J. Wysocki; +Cc: linux-pm, Antti P Miettinen, mark gross
In-Reply-To: <201201162222.09828.rjw@sisk.pl>
On Mon, Jan 16, 2012 at 10:22:09PM +0100, Rafael J. Wysocki wrote:
> On Monday, January 16, 2012, Antti P Miettinen wrote:
> > From: Alex Frid <afrid@nvidia.com>
> >
> > - Replace class ID #define with enumeration
> > - Loop through PM QoS objects during initialization (rather than
> > initializing them one-by-one)
> >
> > Signed-off-by: Alex Frid <afrid@nvidia.com>
> > Reviewed-by: Antti Miettinen <amiettinen@nvidia.com>
> > Reviewed-by: Diwakar Tundlam <dtundlam@nvidia.com>
> > Reviewed-by: Scott Williams <scwilliams@nvidia.com>
> > Reviewed-by: Yu-Huan Hsu <yhsu@nvidia.com>
>
> This one has been ACKed by Mark IIRC.
yes it was. I sent responces to a few other patches that I can't seem
to find in the list server archives. Am I doing something dumb?
--mark
> Thanks,
> Rafael
>
>
> > ---
> > include/linux/pm_qos.h | 14 +++++++++-----
> > kernel/power/qos.c | 23 ++++++++++-------------
> > 2 files changed, 19 insertions(+), 18 deletions(-)
> >
> > diff --git a/include/linux/pm_qos.h b/include/linux/pm_qos.h
> > index e5bbcba..5ac91d8 100644
> > --- a/include/linux/pm_qos.h
> > +++ b/include/linux/pm_qos.h
> > @@ -9,12 +9,16 @@
> > #include <linux/miscdevice.h>
> > #include <linux/device.h>
> >
> > -#define PM_QOS_RESERVED 0
> > -#define PM_QOS_CPU_DMA_LATENCY 1
> > -#define PM_QOS_NETWORK_LATENCY 2
> > -#define PM_QOS_NETWORK_THROUGHPUT 3
> > +enum {
> > + PM_QOS_RESERVED = 0,
> > + PM_QOS_CPU_DMA_LATENCY,
> > + PM_QOS_NETWORK_LATENCY,
> > + PM_QOS_NETWORK_THROUGHPUT,
> > +
> > + /* insert new class ID */
> > + PM_QOS_NUM_CLASSES,
> > +};
> >
> > -#define PM_QOS_NUM_CLASSES 4
> > #define PM_QOS_DEFAULT_VALUE -1
> >
> > #define PM_QOS_CPU_DMA_LAT_DEFAULT_VALUE (2000 * USEC_PER_SEC)
> > diff --git a/kernel/power/qos.c b/kernel/power/qos.c
> > index 995e3bd..d6d6dbd 100644
> > --- a/kernel/power/qos.c
> > +++ b/kernel/power/qos.c
> > @@ -469,21 +469,18 @@ static ssize_t pm_qos_power_write(struct file *filp, const char __user *buf,
> > static int __init pm_qos_power_init(void)
> > {
> > int ret = 0;
> > + int i;
> >
> > - ret = register_pm_qos_misc(&cpu_dma_pm_qos);
> > - if (ret < 0) {
> > - printk(KERN_ERR "pm_qos_param: cpu_dma_latency setup failed\n");
> > - return ret;
> > - }
> > - ret = register_pm_qos_misc(&network_lat_pm_qos);
> > - if (ret < 0) {
> > - printk(KERN_ERR "pm_qos_param: network_latency setup failed\n");
> > - return ret;
> > + BUILD_BUG_ON(ARRAY_SIZE(pm_qos_array) != PM_QOS_NUM_CLASSES);
> > +
> > + for (i = 1; i < PM_QOS_NUM_CLASSES; i++) {
> > + ret = register_pm_qos_misc(pm_qos_array[i]);
> > + if (ret < 0) {
> > + printk(KERN_ERR "pm_qos_param: %s setup failed\n",
> > + pm_qos_array[i]->name);
> > + return ret;
> > + }
> > }
> > - ret = register_pm_qos_misc(&network_throughput_pm_qos);
> > - if (ret < 0)
> > - printk(KERN_ERR
> > - "pm_qos_param: network_throughput setup failed\n");
> >
> > return ret;
> > }
> >
>
^ permalink raw reply
* Re: [linux-pm] [PATCH v2 0/8] RFC: CPU frequency min/max as PM QoS params
From: Rafael J. Wysocki @ 2012-01-17 21:27 UTC (permalink / raw)
To: linux-pm; +Cc: Antti P Miettinen, cpufreq
In-Reply-To: <87r4yyrh2e.fsf@amiettinen-lnx.nvidia.com>
On Tuesday, January 17, 2012, Antti P Miettinen wrote:
> "Rafael J. Wysocki" <rjw@sisk.pl> writes:
> > If that hasn't been clear enough so far, I'm still not convinced that using
> > PM QoS for that is a good idea.
> >
> > First off, frequency as a unit of throughput is questionable to say the least,
> > because it isn't portable from one system to another. Moreover, even on a
> > given system it isn't particularly clear what the exact correspondence
> > between frequency and throughput actually is.
> >
> > Second, it's not particularly clear what the meaning of the "min" frequency
> > is supposed to be in terms of throughput.
> >
> > Moreover, you make cpufreq export user_policy.min and user_policy.max
> > regardless of the new PM QoS parameters, so it looks like you could use those
> > new attributes to set the min/max as well.
> >
> > Thanks,
> > Rafael
>
> Thanks - yes - I've understood you are not convinced :-)
>
> Is there any reason why the mapping from application oriented
> performance requirement metric to hardware oriented performance setting
> metric would need to be inside kernel? As I've said (and Mark Gross
> seems to agree) the performance requirements are likely to be system
> specific and probably obtained via trial and error or some kind of
> adaptive iteration. Wouldn't it be better to leave this complexity
> outside PM QoS core or even outside kernel if possible?
If I understand you correctly, you want to have an iterface for specifying
min and max frequencies from user space. I can understand that. At least
I can see some use cases for that.
Now, the question is if using the PM QoS framework is the right way to do
that.
> The change to cpufreq core just adds two read-only files to be able to
> inspect user_policy.min/max in addition to the currently enforced
> policy->min/max. Yes - there has been the possibility of using the sysfs
> min for setting a frequency floor but this is problematic when there are
> multiple clients. You'd need some kind of arbitration and book keeping
> to set/restore the minimum. And PM QoS provides exactly this mechanism.
Just as I suspected. :-)
OK, so what's your anticipated usage model of this?
> I think the kernel needs to be extended to handle more PM constraints
> and PM QoS is the closest thing I know for this kind of
> functionality. However, I'm open to suggestions about alternative
> approaches. I think we need e.g. more than just min/max "reduction
> operators". Ideas, anyone?
I first need to know who those multiple clients are going to be.
Thanks,
Rafael
^ permalink raw reply
* [PATCH 50/50] ACPI processor: Fix error path, also remove sysdev link
From: Len Brown @ 2012-01-17 12:21 UTC (permalink / raw)
To: linux-acpi, linux-pm, linux-kernel; +Cc: Thomas Renninger, Len Brown
In-Reply-To: <553575f1ae048aa44682b46b3c51929a0b3ad337.1326802228.git.len.brown@intel.com>
From: Thomas Renninger <trenn@suse.de>
Signed-off-by: Thomas Renninger <trenn@suse.de>
Signed-off-by: Len Brown <len.brown@intel.com>
---
drivers/acpi/processor_driver.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/drivers/acpi/processor_driver.c b/drivers/acpi/processor_driver.c
index 9d7bc9f..90719d1 100644
--- a/drivers/acpi/processor_driver.c
+++ b/drivers/acpi/processor_driver.c
@@ -539,6 +539,7 @@ err_thermal_unregister:
thermal_cooling_device_unregister(pr->cdev);
err_power_exit:
acpi_processor_power_exit(pr, device);
+ sysfs_remove_link(&device->dev.kobj, "sysdev");
err_free_cpumask:
free_cpumask_var(pr->throttling.shared_cpu_map);
--
1.7.9.rc1
^ permalink raw reply related
* [PATCH 49/50] intel idle: Make idle driver more robust
From: Len Brown @ 2012-01-17 12:21 UTC (permalink / raw)
To: linux-acpi, linux-pm, linux-kernel
Cc: Thomas Renninger, stable, Bruno Friedmann, Len Brown
In-Reply-To: <553575f1ae048aa44682b46b3c51929a0b3ad337.1326802228.git.len.brown@intel.com>
From: Thomas Renninger <trenn@suse.de>
kvm -cpu host passes the original cpuid info to the guest.
Latest kvm version seem to return true for mwait_leaf cpuid
function on recent Intel CPUs. But it does not return mwait
C-states (mwait_substates), instead zero is returned.
While real CPUs seem to always return non-zero values, the intel
idle driver should not get active in kvm (mwait_substates == 0)
case and bail out.
Otherwise a Null pointer exception will happen later when the
cpuidle subsystem tries to get active:
[0.984807] BUG: unable to handle kernel NULL pointer dereference at (null)
[0.984807] IP: [<(null)>] (null)
...
[0.984807][<ffffffff8143cf34>] ? cpuidle_idle_call+0xb4/0x340
[0.984807][<ffffffff8159e7bc>] ? __atomic_notifier_call_chain+0x4c/0x70
[0.984807][<ffffffff81001198>] ? cpu_idle+0x78/0xd0
Reference:
https://bugzilla.novell.com/show_bug.cgi?id=726296
Cc: stable@vger.kernel.org
Signed-off-by: Thomas Renninger <trenn@suse.de>
CC: Bruno Friedmann <bruno@ioda-net.ch>
Signed-off-by: Len Brown <len.brown@intel.com>
---
drivers/idle/intel_idle.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c
index 6a3d91d..e56ae0b 100644
--- a/drivers/idle/intel_idle.c
+++ b/drivers/idle/intel_idle.c
@@ -356,7 +356,8 @@ static int intel_idle_probe(void)
cpuid(CPUID_MWAIT_LEAF, &eax, &ebx, &ecx, &mwait_substates);
if (!(ecx & CPUID5_ECX_EXTENSIONS_SUPPORTED) ||
- !(ecx & CPUID5_ECX_INTERRUPT_BREAK))
+ !(ecx & CPUID5_ECX_INTERRUPT_BREAK) ||
+ !mwait_substates)
return -ENODEV;
pr_debug(PREFIX "MWAIT substates: 0x%x\n", mwait_substates);
--
1.7.9.rc1
^ permalink raw reply related
* [PATCH 48/50] intel_idle: Fix a cast to pointer from integer of different size warning in intel_idle
From: Len Brown @ 2012-01-17 12:21 UTC (permalink / raw)
To: linux-acpi, linux-pm, linux-kernel; +Cc: David Howells, Len Brown
In-Reply-To: <553575f1ae048aa44682b46b3c51929a0b3ad337.1326802228.git.len.brown@intel.com>
From: David Howells <dhowells@redhat.com>
Fix the following warning:
drivers/idle/intel_idle.c: In function 'intel_idle_cpuidle_devices_init':
drivers/idle/intel_idle.c:518:5: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
By making get_driver_data() return a long instead of an int.
Signed-off-by: David Howells <dhowells@redhat.com>
Signed-off-by: Len Brown <len.brown@intel.com>
---
drivers/idle/intel_idle.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c
index 27e41cc..6a3d91d 100644
--- a/drivers/idle/intel_idle.c
+++ b/drivers/idle/intel_idle.c
@@ -197,7 +197,7 @@ static struct cpuidle_state atom_cstates[MWAIT_MAX_NUM_CSTATES] = {
.enter = &intel_idle },
};
-static int get_driver_data(int cstate)
+static long get_driver_data(int cstate)
{
int driver_data;
switch (cstate) {
--
1.7.9.rc1
^ permalink raw reply related
* [PATCH 47/50] ACPI: processor: fix acpi_get_cpuid for UP processor
From: Len Brown @ 2012-01-17 12:21 UTC (permalink / raw)
To: linux-acpi, linux-pm, linux-kernel; +Cc: Lin Ming, stable, Len Brown
In-Reply-To: <553575f1ae048aa44682b46b3c51929a0b3ad337.1326802228.git.len.brown@intel.com>
From: Lin Ming <ming.m.lin@intel.com>
For UP processor, it is likely that no _MAT method or MADT table defined.
So currently acpi_get_cpuid(...) always return -1 for UP processor.
This is wrong. It should return valid value for CPU0.
In the other hand, BIOS may define multiple CPU handles even for UP
processor, for example
Scope (_PR)
{
Processor (CPU0, 0x00, 0x00000410, 0x06) {}
Processor (CPU1, 0x01, 0x00000410, 0x06) {}
Processor (CPU2, 0x02, 0x00000410, 0x06) {}
Processor (CPU3, 0x03, 0x00000410, 0x06) {}
}
We should only return valid value for CPU0's acpi handle.
And return invalid value for others.
http://marc.info/?t=132329819900003&r=1&w=2
Cc: stable@vger.kernel.org
Reported-and-tested-by: wallak@free.fr
Signed-off-by: Lin Ming <ming.m.lin@intel.com>
Signed-off-by: Len Brown <len.brown@intel.com>
---
drivers/acpi/processor_core.c | 26 ++++++++++++++++++++++++--
1 files changed, 24 insertions(+), 2 deletions(-)
diff --git a/drivers/acpi/processor_core.c b/drivers/acpi/processor_core.c
index 3a0428e..c850de4 100644
--- a/drivers/acpi/processor_core.c
+++ b/drivers/acpi/processor_core.c
@@ -173,8 +173,30 @@ int acpi_get_cpuid(acpi_handle handle, int type, u32 acpi_id)
apic_id = map_mat_entry(handle, type, acpi_id);
if (apic_id == -1)
apic_id = map_madt_entry(type, acpi_id);
- if (apic_id == -1)
- return apic_id;
+ if (apic_id == -1) {
+ /*
+ * On UP processor, there is no _MAT or MADT table.
+ * So above apic_id is always set to -1.
+ *
+ * BIOS may define multiple CPU handles even for UP processor.
+ * For example,
+ *
+ * Scope (_PR)
+ * {
+ * Processor (CPU0, 0x00, 0x00000410, 0x06) {}
+ * Processor (CPU1, 0x01, 0x00000410, 0x06) {}
+ * Processor (CPU2, 0x02, 0x00000410, 0x06) {}
+ * Processor (CPU3, 0x03, 0x00000410, 0x06) {}
+ * }
+ *
+ * Ignores apic_id and always return 0 for CPU0's handle.
+ * Return -1 for other CPU's handle.
+ */
+ if (acpi_id == 0)
+ return acpi_id;
+ else
+ return apic_id;
+ }
#ifdef CONFIG_SMP
for_each_possible_cpu(i) {
--
1.7.9.rc1
^ permalink raw reply related
* [PATCH 46/50] ACPI: kernel-parameters.txt : Add intel_idle.max_cstate
From: Len Brown @ 2012-01-17 12:21 UTC (permalink / raw)
To: linux-acpi, linux-pm, linux-kernel; +Cc: Masanari Iida, Len Brown
In-Reply-To: <553575f1ae048aa44682b46b3c51929a0b3ad337.1326802228.git.len.brown@intel.com>
From: Masanari Iida <standby24x7@gmail.com>
Add missing intel_idle.max_cstate in kernel-parameters.txt
Signed-off-by Masanari Iida <standby24x7@gmail.com>
Signed-off-by: Len Brown <len.brown@intel.com>
---
Documentation/kernel-parameters.txt | 5 +++++
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index 81c287f..c6a56d8 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -1035,6 +1035,11 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
By default, super page will be supported if Intel IOMMU
has the capability. With this option, super page will
not be supported.
+
+ intel_idle.max_cstate= [KNL,HW,ACPI,X86]
+ 0 disables intel_idle and fall back on acpi_idle.
+ 1 to 6 specify maximum depth of C-state.
+
intremap= [X86-64, Intel-IOMMU]
on enable Interrupt Remapping (default)
off disable Interrupt Remapping
--
1.7.9.rc1
^ permalink raw reply related
* [PATCH 45/50] intel_idle: remove redundant local_irq_disable() call
From: Len Brown @ 2012-01-17 12:21 UTC (permalink / raw)
To: linux-acpi, linux-pm, linux-kernel
Cc: Len Brown, Yanmin Zhang, Mingming Zhang, Andrew Morton
In-Reply-To: <553575f1ae048aa44682b46b3c51929a0b3ad337.1326802228.git.len.brown@intel.com>
From: Yanmin Zhang <yanmin_zhang@linux.intel.com>
irq disabling happens earlier in process_32.c:cpu_idle. Basically,
cpuidle_state->enter is called, cpu irq is disabled. cpuidle_state->enter
would turn on irq when exiting.
intel_idle doesn't follow this assumption. Although it doesn't cause real
issue, it misleads developers. Remove the call to local_irq_disable() at
entry.
[akpm@linux-foundation.org: add comment]
Signed-off-by: Mingming Zhang <mingmingx.zhang@intel.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Len Brown <len.brown@intel.com>
---
drivers/idle/intel_idle.c | 3 +--
1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c
index 38da9f8..27e41cc 100644
--- a/drivers/idle/intel_idle.c
+++ b/drivers/idle/intel_idle.c
@@ -232,6 +232,7 @@ static int get_driver_data(int cstate)
* @drv: cpuidle driver
* @index: index of cpuidle state
*
+ * Must be called under local_irq_disable().
*/
static int intel_idle(struct cpuidle_device *dev,
struct cpuidle_driver *drv, int index)
@@ -247,8 +248,6 @@ static int intel_idle(struct cpuidle_device *dev,
cstate = (((eax) >> MWAIT_SUBSTATE_SIZE) & MWAIT_CSTATE_MASK) + 1;
- local_irq_disable();
-
/*
* leave_mm() to avoid costly and often unnecessary wakeups
* for flushing the user TLB's associated with the active mm.
--
1.7.9.rc1
^ permalink raw reply related
* [PATCH 44/50] intel_idle: disable auto_demotion for hotplugged CPUs
From: Len Brown @ 2012-01-17 12:21 UTC (permalink / raw)
To: linux-acpi, linux-pm, linux-kernel; +Cc: Shaohua Li, Andrew Morton, Len Brown
In-Reply-To: <553575f1ae048aa44682b46b3c51929a0b3ad337.1326802228.git.len.brown@intel.com>
From: Shaohua Li <shaohua.li@intel.com>
auto_demotion_disable is called only for online CPUs. For hotplugged
CPUs, we should disable it too.
Signed-off-by: Shaohua Li <shaohua.li@intel.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Len Brown <len.brown@intel.com>
---
drivers/idle/intel_idle.c | 53 ++++++++++++++++++++++++++++-----------------
1 files changed, 33 insertions(+), 20 deletions(-)
diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c
index af0b4a5..38da9f8 100644
--- a/drivers/idle/intel_idle.c
+++ b/drivers/idle/intel_idle.c
@@ -297,33 +297,42 @@ static void __setup_broadcast_timer(void *arg)
clockevents_notify(reason, &cpu);
}
-static int setup_broadcast_cpuhp_notify(struct notifier_block *n,
+static void auto_demotion_disable(void *dummy)
+{
+ unsigned long long msr_bits;
+
+ rdmsrl(MSR_NHM_SNB_PKG_CST_CFG_CTL, msr_bits);
+ msr_bits &= ~auto_demotion_disable_flags;
+ wrmsrl(MSR_NHM_SNB_PKG_CST_CFG_CTL, msr_bits);
+}
+
+static void __intel_idle_notify_handler(void *arg)
+{
+ if (auto_demotion_disable_flags)
+ auto_demotion_disable(NULL);
+
+ if (lapic_timer_reliable_states != LAPIC_TIMER_ALWAYS_RELIABLE)
+ __setup_broadcast_timer((void *)true);
+}
+
+static int setup_intelidle_cpuhp_notify(struct notifier_block *n,
unsigned long action, void *hcpu)
{
int hotcpu = (unsigned long)hcpu;
switch (action & 0xf) {
case CPU_ONLINE:
- smp_call_function_single(hotcpu, __setup_broadcast_timer,
- (void *)true, 1);
+ smp_call_function_single(hotcpu, __intel_idle_notify_handler,
+ NULL, 1);
break;
}
return NOTIFY_OK;
}
-static struct notifier_block setup_broadcast_notifier = {
- .notifier_call = setup_broadcast_cpuhp_notify,
+static struct notifier_block setup_intelidle_notifier = {
+ .notifier_call = setup_intelidle_cpuhp_notify,
};
-static void auto_demotion_disable(void *dummy)
-{
- unsigned long long msr_bits;
-
- rdmsrl(MSR_NHM_SNB_PKG_CST_CFG_CTL, msr_bits);
- msr_bits &= ~auto_demotion_disable_flags;
- wrmsrl(MSR_NHM_SNB_PKG_CST_CFG_CTL, msr_bits);
-}
-
/*
* intel_idle_probe()
*/
@@ -393,10 +402,8 @@ static int intel_idle_probe(void)
if (boot_cpu_has(X86_FEATURE_ARAT)) /* Always Reliable APIC Timer */
lapic_timer_reliable_states = LAPIC_TIMER_ALWAYS_RELIABLE;
- else {
+ else
on_each_cpu(__setup_broadcast_timer, (void *)true, 1);
- register_cpu_notifier(&setup_broadcast_notifier);
- }
pr_debug(PREFIX "v" INTEL_IDLE_VERSION
" model 0x%X\n", boot_cpu_data.x86_model);
@@ -559,6 +566,10 @@ static int __init intel_idle_init(void)
return retval;
}
+ if (auto_demotion_disable_flags || lapic_timer_reliable_states !=
+ LAPIC_TIMER_ALWAYS_RELIABLE)
+ register_cpu_notifier(&setup_intelidle_notifier);
+
return 0;
}
@@ -567,10 +578,12 @@ static void __exit intel_idle_exit(void)
intel_idle_cpuidle_devices_uninit();
cpuidle_unregister_driver(&intel_idle_driver);
- if (lapic_timer_reliable_states != LAPIC_TIMER_ALWAYS_RELIABLE) {
+ if (lapic_timer_reliable_states != LAPIC_TIMER_ALWAYS_RELIABLE)
on_each_cpu(__setup_broadcast_timer, (void *)false, 1);
- unregister_cpu_notifier(&setup_broadcast_notifier);
- }
+
+ if (auto_demotion_disable_flags || lapic_timer_reliable_states !=
+ LAPIC_TIMER_ALWAYS_RELIABLE)
+ unregister_cpu_notifier(&setup_intelidle_notifier);
return;
}
--
1.7.9.rc1
^ permalink raw reply related
* [PATCH 43/50] intel_idle: fix API misuse
From: Len Brown @ 2012-01-17 12:21 UTC (permalink / raw)
To: linux-acpi, linux-pm, linux-kernel
Cc: Shaohua Li, stable, Andrew Morton, Len Brown
In-Reply-To: <553575f1ae048aa44682b46b3c51929a0b3ad337.1326802228.git.len.brown@intel.com>
From: Shaohua Li <shaohua.li@intel.com>
smp_call_function() only lets all other CPUs execute a specific function,
while we expect all CPUs do in intel_idle. Without the fix, we could have
one cpu which has auto_demotion enabled or has no broadcast timer setup.
Usually we don't see impact because auto demotion just harms power and the
intel_idle init is called in CPU 0, where boradcast timer delivers
interrupt, but this still could be a problem.
Cc: stable@vger.kernel.org
Signed-off-by: Shaohua Li <shaohua.li@intel.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Len Brown <len.brown@intel.com>
---
drivers/idle/intel_idle.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c
index 5d2f8e1..af0b4a5 100644
--- a/drivers/idle/intel_idle.c
+++ b/drivers/idle/intel_idle.c
@@ -394,7 +394,7 @@ static int intel_idle_probe(void)
if (boot_cpu_has(X86_FEATURE_ARAT)) /* Always Reliable APIC Timer */
lapic_timer_reliable_states = LAPIC_TIMER_ALWAYS_RELIABLE;
else {
- smp_call_function(__setup_broadcast_timer, (void *)true, 1);
+ on_each_cpu(__setup_broadcast_timer, (void *)true, 1);
register_cpu_notifier(&setup_broadcast_notifier);
}
@@ -471,7 +471,7 @@ static int intel_idle_cpuidle_driver_init(void)
}
if (auto_demotion_disable_flags)
- smp_call_function(auto_demotion_disable, NULL, 1);
+ on_each_cpu(auto_demotion_disable, NULL, 1);
return 0;
}
@@ -568,7 +568,7 @@ static void __exit intel_idle_exit(void)
cpuidle_unregister_driver(&intel_idle_driver);
if (lapic_timer_reliable_states != LAPIC_TIMER_ALWAYS_RELIABLE) {
- smp_call_function(__setup_broadcast_timer, (void *)false, 1);
+ on_each_cpu(__setup_broadcast_timer, (void *)false, 1);
unregister_cpu_notifier(&setup_broadcast_notifier);
}
--
1.7.9.rc1
^ permalink raw reply related
* [PATCH 42/50] acpi/apei/einj: Add extensions to EINJ from rev 5.0 of acpi spec
From: Len Brown @ 2012-01-17 12:21 UTC (permalink / raw)
To: linux-acpi, linux-pm, linux-kernel; +Cc: Len Brown, Tony Luck
In-Reply-To: <553575f1ae048aa44682b46b3c51929a0b3ad337.1326802228.git.len.brown@intel.com>
From: Tony Luck <tony.luck@intel.com>
ACPI 5.0 provides extensions to the EINJ mechanism to specify the
target for the error injection - by APICID for cpu related errors,
by address for memory related errors, and by segment/bus/device/function
for PCIe related errors. Also extensions for vendor specific error
injections.
Tested-by: Chen Gong <gong.chen@linux.intel.com>
Signed-off-by: Tony Luck <tony.luck@intel.com>
Signed-off-by: Len Brown <len.brown@intel.com>
---
Documentation/acpi/apei/einj.txt | 43 ++++++--
drivers/acpi/apei/einj.c | 217 +++++++++++++++++++++++++++++++------
include/acpi/actbl1.h | 3 +-
3 files changed, 215 insertions(+), 48 deletions(-)
diff --git a/Documentation/acpi/apei/einj.txt b/Documentation/acpi/apei/einj.txt
index 5cc699b..95a2b22 100644
--- a/Documentation/acpi/apei/einj.txt
+++ b/Documentation/acpi/apei/einj.txt
@@ -47,20 +47,41 @@ directory apei/einj. The following files are provided.
- param1
This file is used to set the first error parameter value. Effect of
- parameter depends on error_type specified. For memory error, this is
- physical memory address. Only available if param_extension module
- parameter is specified.
+ parameter depends on error_type specified.
- param2
This file is used to set the second error parameter value. Effect of
- parameter depends on error_type specified. For memory error, this is
- physical memory address mask. Only available if param_extension
- module parameter is specified.
+ parameter depends on error_type specified.
-Injecting parameter support is a BIOS version specific extension, that
-is, it only works on some BIOS version. If you want to use it, please
-make sure your BIOS version has the proper support and specify
-"param_extension=y" in module parameter.
+BIOS versions based in the ACPI 4.0 specification have limited options
+to control where the errors are injected. Your BIOS may support an
+extension (enabled with the param_extension=1 module parameter, or
+boot command line einj.param_extension=1). This allows the address
+and mask for memory injections to be specified by the param1 and
+param2 files in apei/einj.
+
+BIOS versions using the ACPI 5.0 specification have more control over
+the target of the injection. For processor related errors (type 0x1,
+0x2 and 0x4) the APICID of the target should be provided using the
+param1 file in apei/einj. For memory errors (type 0x8, 0x10 and 0x20)
+the address is set using param1 with a mask in param2 (0x0 is equivalent
+to all ones). For PCI express errors (type 0x80, 0x80 and 0x100) the
+segment, bus, device and function are specified using param1:
+
+ 31 24 23 16 15 11 10 8 7 0
+ +-------------------------------------------------+
+ | segment | bus | device | function | reserved |
+ +-------------------------------------------------+
+
+An ACPI 5.0 BIOS may also allow vendor specific errors to be injected.
+In this case a file named vendor will contain identifying information
+from the BIOS that hopefully will allow an application wishing to use
+the vendor specific extension to tell that they are running on a BIOS
+that supports it. All vendor extensions have the 0x80000000 bit set in
+error_type. A file vendor_flags controls the interpretation of param1
+and param2 (1 = PROCESSOR, 2 = MEMORY, 4 = PCI). See your BIOS vendor
+documentation for details (and expect changes to this API if vendors
+creativity in using this feature expands beyond our expectations).
For more information about EINJ, please refer to ACPI specification
-version 4.0, section 17.5.
+version 4.0, section 17.5 and ACPI 5.0, section 18.6.
diff --git a/drivers/acpi/apei/einj.c b/drivers/acpi/apei/einj.c
index 589b96c..1bc1f309 100644
--- a/drivers/acpi/apei/einj.c
+++ b/drivers/acpi/apei/einj.c
@@ -43,6 +43,42 @@
#define FIRMWARE_TIMEOUT (1 * NSEC_PER_MSEC)
/*
+ * ACPI version 5 provides a SET_ERROR_TYPE_WITH_ADDRESS action.
+ */
+static int acpi5;
+
+struct set_error_type_with_address {
+ u32 type;
+ u32 vendor_extension;
+ u32 flags;
+ u32 apicid;
+ u64 memory_address;
+ u64 memory_address_range;
+ u32 pcie_sbdf;
+};
+enum {
+ SETWA_FLAGS_APICID = 1,
+ SETWA_FLAGS_MEM = 2,
+ SETWA_FLAGS_PCIE_SBDF = 4,
+};
+
+/*
+ * Vendor extensions for platform specific operations
+ */
+struct vendor_error_type_extension {
+ u32 length;
+ u32 pcie_sbdf;
+ u16 vendor_id;
+ u16 device_id;
+ u8 rev_id;
+ u8 reserved[3];
+};
+
+static u32 vendor_flags;
+static struct debugfs_blob_wrapper vendor_blob;
+static char vendor_dev[64];
+
+/*
* Some BIOSes allow parameters to the SET_ERROR_TYPE entries in the
* EINJ table through an unpublished extension. Use with caution as
* most will ignore the parameter and make their own choice of address
@@ -103,7 +139,7 @@ static struct apei_exec_ins_type einj_ins_type[] = {
*/
static DEFINE_MUTEX(einj_mutex);
-static struct einj_parameter *einj_param;
+static void *einj_param;
#ifndef writeq
static inline void writeq(__u64 val, volatile void __iomem *addr)
@@ -158,10 +194,31 @@ static int einj_timedout(u64 *t)
return 0;
}
-static u64 einj_get_parameter_address(void)
+static void check_vendor_extension(u64 paddr,
+ struct set_error_type_with_address *v5param)
+{
+ int offset = readl(&v5param->vendor_extension);
+ struct vendor_error_type_extension *v;
+ u32 sbdf;
+
+ if (!offset)
+ return;
+ v = ioremap(paddr + offset, sizeof(*v));
+ if (!v)
+ return;
+ sbdf = readl(&v->pcie_sbdf);
+ sprintf(vendor_dev, "%x:%x:%x.%x vendor_id=%x device_id=%x rev_id=%x\n",
+ sbdf >> 24, (sbdf >> 16) & 0xff, (sbdf >> 11) & 0x1f, (sbdf >> 8) & 0x7,
+ readw(&v->vendor_id), readw(&v->device_id),
+ readb(&v->rev_id));
+ iounmap(v);
+}
+
+static void *einj_get_parameter_address(void)
{
int i;
- u64 paddr = 0;
+ u64 paddrv4 = 0, paddrv5 = 0;
+ void *param;
struct acpi_whea_header *entry;
entry = EINJ_TAB_ENTRY(einj_tab);
@@ -170,12 +227,40 @@ static u64 einj_get_parameter_address(void)
entry->instruction == ACPI_EINJ_WRITE_REGISTER &&
entry->register_region.space_id ==
ACPI_ADR_SPACE_SYSTEM_MEMORY)
- memcpy(&paddr, &entry->register_region.address,
- sizeof(paddr));
+ memcpy(&paddrv4, &entry->register_region.address,
+ sizeof(paddrv4));
+ if (entry->action == ACPI_EINJ_SET_ERROR_TYPE_WITH_ADDRESS &&
+ entry->instruction == ACPI_EINJ_WRITE_REGISTER &&
+ entry->register_region.space_id ==
+ ACPI_ADR_SPACE_SYSTEM_MEMORY)
+ memcpy(&paddrv5, &entry->register_region.address,
+ sizeof(paddrv5));
entry++;
}
+ if (paddrv5) {
+ struct set_error_type_with_address *v5param;
+
+ v5param = ioremap(paddrv5, sizeof(*v5param));
+ if (v5param) {
+ acpi5 = 1;
+ check_vendor_extension(paddrv5, v5param);
+ return v5param;
+ }
+ }
+ if (paddrv4) {
+ struct einj_parameter *v4param;
+
+ v4param = ioremap(paddrv4, sizeof(*v4param));
+ if (!v4param)
+ return 0;
+ if (readq(&v4param->reserved1) || readq(&v4param->reserved2)) {
+ iounmap(param);
+ return 0;
+ }
+ return v4param;
+ }
- return paddr;
+ return 0;
}
/* do sanity check to trigger table */
@@ -293,12 +378,56 @@ static int __einj_error_inject(u32 type, u64 param1, u64 param2)
if (rc)
return rc;
apei_exec_ctx_set_input(&ctx, type);
- rc = apei_exec_run(&ctx, ACPI_EINJ_SET_ERROR_TYPE);
- if (rc)
- return rc;
- if (einj_param) {
- writeq(param1, &einj_param->param1);
- writeq(param2, &einj_param->param2);
+ if (acpi5) {
+ struct set_error_type_with_address *v5param = einj_param;
+
+ writel(type, &v5param->type);
+ if (type & 0x80000000) {
+ switch (vendor_flags) {
+ case SETWA_FLAGS_APICID:
+ writel(param1, &v5param->apicid);
+ break;
+ case SETWA_FLAGS_MEM:
+ writeq(param1, &v5param->memory_address);
+ writeq(param2, &v5param->memory_address_range);
+ break;
+ case SETWA_FLAGS_PCIE_SBDF:
+ writel(param1, &v5param->pcie_sbdf);
+ break;
+ }
+ writel(vendor_flags, &v5param->flags);
+ } else {
+ switch (type) {
+ case ACPI_EINJ_PROCESSOR_CORRECTABLE:
+ case ACPI_EINJ_PROCESSOR_UNCORRECTABLE:
+ case ACPI_EINJ_PROCESSOR_FATAL:
+ writel(param1, &v5param->apicid);
+ writel(SETWA_FLAGS_APICID, &v5param->flags);
+ break;
+ case ACPI_EINJ_MEMORY_CORRECTABLE:
+ case ACPI_EINJ_MEMORY_UNCORRECTABLE:
+ case ACPI_EINJ_MEMORY_FATAL:
+ writeq(param1, &v5param->memory_address);
+ writeq(param2, &v5param->memory_address_range);
+ writel(SETWA_FLAGS_MEM, &v5param->flags);
+ break;
+ case ACPI_EINJ_PCIX_CORRECTABLE:
+ case ACPI_EINJ_PCIX_UNCORRECTABLE:
+ case ACPI_EINJ_PCIX_FATAL:
+ writel(param1, &v5param->pcie_sbdf);
+ writel(SETWA_FLAGS_PCIE_SBDF, &v5param->flags);
+ break;
+ }
+ }
+ } else {
+ rc = apei_exec_run(&ctx, ACPI_EINJ_SET_ERROR_TYPE);
+ if (rc)
+ return rc;
+ if (einj_param) {
+ struct einj_parameter *v4param = einj_param;
+ writeq(param1, &v4param->param1);
+ writeq(param2, &v4param->param2);
+ }
}
rc = apei_exec_run(&ctx, ACPI_EINJ_EXECUTE_OPERATION);
if (rc)
@@ -408,15 +537,25 @@ static int error_type_set(void *data, u64 val)
{
int rc;
u32 available_error_type = 0;
+ u32 tval, vendor;
+
+ /*
+ * Vendor defined types have 0x80000000 bit set, and
+ * are not enumerated by ACPI_EINJ_GET_ERROR_TYPE
+ */
+ vendor = val & 0x80000000;
+ tval = val & 0x7fffffff;
/* Only one error type can be specified */
- if (val & (val - 1))
- return -EINVAL;
- rc = einj_get_available_error_type(&available_error_type);
- if (rc)
- return rc;
- if (!(val & available_error_type))
+ if (tval & (tval - 1))
return -EINVAL;
+ if (!vendor) {
+ rc = einj_get_available_error_type(&available_error_type);
+ if (rc)
+ return rc;
+ if (!(val & available_error_type))
+ return -EINVAL;
+ }
error_type = val;
return 0;
@@ -455,7 +594,6 @@ static int einj_check_table(struct acpi_table_einj *einj_tab)
static int __init einj_init(void)
{
int rc;
- u64 param_paddr;
acpi_status status;
struct dentry *fentry;
struct apei_exec_context ctx;
@@ -509,23 +647,30 @@ static int __init einj_init(void)
rc = apei_exec_pre_map_gars(&ctx);
if (rc)
goto err_release;
- if (param_extension) {
- param_paddr = einj_get_parameter_address();
- if (param_paddr) {
- einj_param = ioremap(param_paddr, sizeof(*einj_param));
- rc = -ENOMEM;
- if (!einj_param)
- goto err_unmap;
- fentry = debugfs_create_x64("param1", S_IRUSR | S_IWUSR,
- einj_debug_dir, &error_param1);
- if (!fentry)
- goto err_unmap;
- fentry = debugfs_create_x64("param2", S_IRUSR | S_IWUSR,
- einj_debug_dir, &error_param2);
- if (!fentry)
- goto err_unmap;
- } else
- pr_warn(EINJ_PFX "Parameter extension is not supported.\n");
+
+ einj_param = einj_get_parameter_address();
+ if ((param_extension || acpi5) && einj_param) {
+ fentry = debugfs_create_x64("param1", S_IRUSR | S_IWUSR,
+ einj_debug_dir, &error_param1);
+ if (!fentry)
+ goto err_unmap;
+ fentry = debugfs_create_x64("param2", S_IRUSR | S_IWUSR,
+ einj_debug_dir, &error_param2);
+ if (!fentry)
+ goto err_unmap;
+ }
+
+ if (vendor_dev[0]) {
+ vendor_blob.data = vendor_dev;
+ vendor_blob.size = strlen(vendor_dev);
+ fentry = debugfs_create_blob("vendor", S_IRUSR,
+ einj_debug_dir, &vendor_blob);
+ if (!fentry)
+ goto err_unmap;
+ fentry = debugfs_create_x32("vendor_flags", S_IRUSR | S_IWUSR,
+ einj_debug_dir, &vendor_flags);
+ if (!fentry)
+ goto err_unmap;
}
pr_info(EINJ_PFX "Error INJection is initialized.\n");
diff --git a/include/acpi/actbl1.h b/include/acpi/actbl1.h
index 7504bc9..f25d7ef 100644
--- a/include/acpi/actbl1.h
+++ b/include/acpi/actbl1.h
@@ -228,7 +228,8 @@ enum acpi_einj_actions {
ACPI_EINJ_EXECUTE_OPERATION = 5,
ACPI_EINJ_CHECK_BUSY_STATUS = 6,
ACPI_EINJ_GET_COMMAND_STATUS = 7,
- ACPI_EINJ_ACTION_RESERVED = 8, /* 8 and greater are reserved */
+ ACPI_EINJ_SET_ERROR_TYPE_WITH_ADDRESS = 8,
+ ACPI_EINJ_ACTION_RESERVED = 9, /* 9 and greater are reserved */
ACPI_EINJ_TRIGGER_ERROR = 0xFF /* Except for this value */
};
--
1.7.9.rc1
^ permalink raw reply related
* [PATCH 41/50] ACPI APEI: Convert atomicio routines
From: Len Brown @ 2012-01-17 12:21 UTC (permalink / raw)
To: linux-acpi, linux-pm, linux-kernel; +Cc: Myron Stowe, Myron Stowe, Len Brown
In-Reply-To: <553575f1ae048aa44682b46b3c51929a0b3ad337.1326802228.git.len.brown@intel.com>
From: Myron Stowe <mstowe@redhat.com>
APEI needs memory access in interrupt context. The obvious choice is
acpi_read(), but originally it couldn't be used in interrupt context
because it makes temporary mappings with ioremap(). Therefore, we added
drivers/acpi/atomicio.c, which provides:
acpi_pre_map_gar() -- ioremap in process context
acpi_atomic_read() -- memory access in interrupt context
acpi_post_unmap_gar() -- iounmap
Later we added acpi_os_map_generic_address() (2971852) and enhanced
acpi_read() so it works in interrupt context as long as the address has
been previously mapped (620242a). Now this sequence:
acpi_os_map_generic_address() -- ioremap in process context
acpi_read()/apei_read() -- now OK in interrupt context
acpi_os_unmap_generic_address()
is equivalent to what atomicio.c provides.
This patch introduces apei_read() and apei_write(), which currently are
functional equivalents of acpi_read() and acpi_write(). This is mainly
proactive, to prevent APEI breakages if acpi_read() and acpi_write()
are ever augmented to support the 'bit_offset' field of GAS, as APEI's
__apei_exec_write_register() precludes splitting up functionality
related to 'bit_offset' and APEI's 'mask' (see its
APEI_EXEC_PRESERVE_REGISTER block).
With apei_read() and apei_write() in place, usages of atomicio routines
are converted to apei_read()/apei_write() and existing calls within
osl.c and the CA, based on the re-factoring that was done in an earlier
patch series - http://marc.info/?l=linux-acpi&m=128769263327206&w=2:
acpi_pre_map_gar() --> acpi_os_map_generic_address()
acpi_post_unmap_gar() --> acpi_os_unmap_generic_address()
acpi_atomic_read() --> apei_read()
acpi_atomic_write() --> apei_write()
Note that acpi_read() and acpi_write() currently use 'bit_width'
for accessing GARs which seems incorrect. 'bit_width' is the size of
the register, while 'access_width' is the size of the access the
processor must generate on the bus. The 'access_width' may be larger,
for example, if the hardware only supports 32-bit or 64-bit reads. I
wanted to minimize any possible impacts with this patch series so I
did *not* change this behavior.
Signed-off-by: Myron Stowe <myron.stowe@redhat.com>
Signed-off-by: Len Brown <len.brown@intel.com>
---
drivers/acpi/apei/apei-base.c | 102 ++++++++++++++++++++++++++++++++++--
drivers/acpi/apei/apei-internal.h | 3 +
drivers/acpi/apei/ghes.c | 10 ++--
3 files changed, 104 insertions(+), 11 deletions(-)
diff --git a/drivers/acpi/apei/apei-base.c b/drivers/acpi/apei/apei-base.c
index 4abb6c7..e45350c 100644
--- a/drivers/acpi/apei/apei-base.c
+++ b/drivers/acpi/apei/apei-base.c
@@ -34,13 +34,13 @@
#include <linux/module.h>
#include <linux/init.h>
#include <linux/acpi.h>
+#include <linux/acpi_io.h>
#include <linux/slab.h>
#include <linux/io.h>
#include <linux/kref.h>
#include <linux/rculist.h>
#include <linux/interrupt.h>
#include <linux/debugfs.h>
-#include <acpi/atomicio.h>
#include "apei-internal.h"
@@ -70,7 +70,7 @@ int __apei_exec_read_register(struct acpi_whea_header *entry, u64 *val)
{
int rc;
- rc = acpi_atomic_read(val, &entry->register_region);
+ rc = apei_read(val, &entry->register_region);
if (rc)
return rc;
*val >>= entry->register_region.bit_offset;
@@ -116,13 +116,13 @@ int __apei_exec_write_register(struct acpi_whea_header *entry, u64 val)
val <<= entry->register_region.bit_offset;
if (entry->flags & APEI_EXEC_PRESERVE_REGISTER) {
u64 valr = 0;
- rc = acpi_atomic_read(&valr, &entry->register_region);
+ rc = apei_read(&valr, &entry->register_region);
if (rc)
return rc;
valr &= ~(entry->mask << entry->register_region.bit_offset);
val |= valr;
}
- rc = acpi_atomic_write(val, &entry->register_region);
+ rc = apei_write(val, &entry->register_region);
return rc;
}
@@ -243,7 +243,7 @@ static int pre_map_gar_callback(struct apei_exec_context *ctx,
u8 ins = entry->instruction;
if (ctx->ins_table[ins].flags & APEI_EXEC_INS_ACCESS_REGISTER)
- return acpi_pre_map_gar(&entry->register_region);
+ return acpi_os_map_generic_address(&entry->register_region);
return 0;
}
@@ -276,7 +276,7 @@ static int post_unmap_gar_callback(struct apei_exec_context *ctx,
u8 ins = entry->instruction;
if (ctx->ins_table[ins].flags & APEI_EXEC_INS_ACCESS_REGISTER)
- acpi_post_unmap_gar(&entry->register_region);
+ acpi_os_unmap_generic_address(&entry->register_region);
return 0;
}
@@ -591,6 +591,96 @@ static int apei_check_gar(struct acpi_generic_address *reg, u64 *paddr)
return 0;
}
+/* read GAR in interrupt (including NMI) or process context */
+int apei_read(u64 *val, struct acpi_generic_address *reg)
+{
+ int rc;
+ u64 address;
+ u32 tmp, width = reg->bit_width;
+ acpi_status status;
+
+ rc = apei_check_gar(reg, &address);
+ if (rc)
+ return rc;
+
+ if (width == 64)
+ width = 32; /* Break into two 32-bit transfers */
+
+ *val = 0;
+ switch(reg->space_id) {
+ case ACPI_ADR_SPACE_SYSTEM_MEMORY:
+ status = acpi_os_read_memory((acpi_physical_address)
+ address, &tmp, width);
+ if (ACPI_FAILURE(status))
+ return -EIO;
+ *val = tmp;
+
+ if (reg->bit_width == 64) {
+ /* Read the top 32 bits */
+ status = acpi_os_read_memory((acpi_physical_address)
+ (address + 4), &tmp, 32);
+ if (ACPI_FAILURE(status))
+ return -EIO;
+ *val |= ((u64)tmp << 32);
+ }
+ break;
+ case ACPI_ADR_SPACE_SYSTEM_IO:
+ status = acpi_os_read_port(address, (u32 *)val, reg->bit_width);
+ if (ACPI_FAILURE(status))
+ return -EIO;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(apei_read);
+
+/* write GAR in interrupt (including NMI) or process context */
+int apei_write(u64 val, struct acpi_generic_address *reg)
+{
+ int rc;
+ u64 address;
+ u32 width = reg->bit_width;
+ acpi_status status;
+
+ rc = apei_check_gar(reg, &address);
+ if (rc)
+ return rc;
+
+ if (width == 64)
+ width = 32; /* Break into two 32-bit transfers */
+
+ switch (reg->space_id) {
+ case ACPI_ADR_SPACE_SYSTEM_MEMORY:
+ status = acpi_os_write_memory((acpi_physical_address)
+ address, ACPI_LODWORD(val),
+ width);
+ if (ACPI_FAILURE(status))
+ return -EIO;
+
+ if (reg->bit_width == 64) {
+ status = acpi_os_write_memory((acpi_physical_address)
+ (address + 4),
+ ACPI_HIDWORD(val), 32);
+ if (ACPI_FAILURE(status))
+ return -EIO;
+ }
+ break;
+ case ACPI_ADR_SPACE_SYSTEM_IO:
+ status = acpi_os_write_port(address, val, reg->bit_width);
+ if (ACPI_FAILURE(status))
+ return -EIO;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(apei_write);
+
static int collect_res_callback(struct apei_exec_context *ctx,
struct acpi_whea_header *entry,
void *data)
diff --git a/drivers/acpi/apei/apei-internal.h b/drivers/acpi/apei/apei-internal.h
index d778edd..cca240a 100644
--- a/drivers/acpi/apei/apei-internal.h
+++ b/drivers/acpi/apei/apei-internal.h
@@ -68,6 +68,9 @@ static inline int apei_exec_run_optional(struct apei_exec_context *ctx, u8 actio
/* IP has been set in instruction function */
#define APEI_EXEC_SET_IP 1
+int apei_read(u64 *val, struct acpi_generic_address *reg);
+int apei_write(u64 val, struct acpi_generic_address *reg);
+
int __apei_exec_read_register(struct acpi_whea_header *entry, u64 *val);
int __apei_exec_write_register(struct acpi_whea_header *entry, u64 val);
int apei_exec_read_register(struct apei_exec_context *ctx,
diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index aaf3609..b3207e1 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -33,6 +33,7 @@
#include <linux/module.h>
#include <linux/init.h>
#include <linux/acpi.h>
+#include <linux/acpi_io.h>
#include <linux/io.h>
#include <linux/interrupt.h>
#include <linux/timer.h>
@@ -48,7 +49,6 @@
#include <linux/pci.h>
#include <linux/aer.h>
#include <acpi/apei.h>
-#include <acpi/atomicio.h>
#include <acpi/hed.h>
#include <asm/mce.h>
#include <asm/tlbflush.h>
@@ -301,7 +301,7 @@ static struct ghes *ghes_new(struct acpi_hest_generic *generic)
if (!ghes)
return ERR_PTR(-ENOMEM);
ghes->generic = generic;
- rc = acpi_pre_map_gar(&generic->error_status_address);
+ rc = acpi_os_map_generic_address(&generic->error_status_address);
if (rc)
goto err_free;
error_block_length = generic->error_block_length;
@@ -321,7 +321,7 @@ static struct ghes *ghes_new(struct acpi_hest_generic *generic)
return ghes;
err_unmap:
- acpi_post_unmap_gar(&generic->error_status_address);
+ acpi_os_unmap_generic_address(&generic->error_status_address);
err_free:
kfree(ghes);
return ERR_PTR(rc);
@@ -330,7 +330,7 @@ err_free:
static void ghes_fini(struct ghes *ghes)
{
kfree(ghes->estatus);
- acpi_post_unmap_gar(&ghes->generic->error_status_address);
+ acpi_os_unmap_generic_address(&ghes->generic->error_status_address);
}
enum {
@@ -401,7 +401,7 @@ static int ghes_read_estatus(struct ghes *ghes, int silent)
u32 len;
int rc;
- rc = acpi_atomic_read(&buf_paddr, &g->error_status_address);
+ rc = apei_read(&buf_paddr, &g->error_status_address);
if (rc) {
if (!silent && printk_ratelimit())
pr_warning(FW_WARN GHES_PFX
--
1.7.9.rc1
^ permalink raw reply related
* [PATCH 40/50] ACPI: Export interfaces for ioremapping/iounmapping ACPI registers
From: Len Brown @ 2012-01-17 12:21 UTC (permalink / raw)
To: linux-acpi, linux-pm, linux-kernel; +Cc: Myron Stowe, Myron Stowe, Len Brown
In-Reply-To: <553575f1ae048aa44682b46b3c51929a0b3ad337.1326802228.git.len.brown@intel.com>
From: Myron Stowe <mstowe@redhat.com>
Export remapping and unmapping interfaces - acpi_os_map_generic_address()
and acpi_os_unmap_generic_address() - for ACPI generic registers that are
backed by memory mapped I/O (MMIO).
The acpi_os_map_generic_address() and acpi_os_unmap_generic_address()
declarations may more properly belong in include/acpi/acpiosxf.h next to
acpi_os_read_memory() but I believe that would require the ACPI CA making
them an official part of the ACPI CA - OS interface.
ACPI Generic Address Structure (GAS) reference (ACPI's fixed/generic
hardware registers use the GAS format):
ACPI Specification, Revision 4.0, Section 5.2.3.1, "Generic Address
Structure"
Signed-off-by: Myron Stowe <myron.stowe@redhat.com>
Acked-by: Rafael J. Wysocki <rjw@sisk.pl>
Signed-off-by: Len Brown <len.brown@intel.com>
---
drivers/acpi/osl.c | 6 ++++--
include/linux/acpi_io.h | 3 +++
2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
index 2e285cd..b11f267 100644
--- a/drivers/acpi/osl.c
+++ b/drivers/acpi/osl.c
@@ -431,7 +431,7 @@ void __init early_acpi_os_unmap_memory(void __iomem *virt, acpi_size size)
__acpi_unmap_table(virt, size);
}
-static int acpi_os_map_generic_address(struct acpi_generic_address *gas)
+int acpi_os_map_generic_address(struct acpi_generic_address *gas)
{
u64 addr;
void __iomem *virt;
@@ -450,8 +450,9 @@ static int acpi_os_map_generic_address(struct acpi_generic_address *gas)
return 0;
}
+EXPORT_SYMBOL(acpi_os_map_generic_address);
-static void acpi_os_unmap_generic_address(struct acpi_generic_address *gas)
+void acpi_os_unmap_generic_address(struct acpi_generic_address *gas)
{
u64 addr;
struct acpi_ioremap *map;
@@ -475,6 +476,7 @@ static void acpi_os_unmap_generic_address(struct acpi_generic_address *gas)
acpi_os_map_cleanup(map);
}
+EXPORT_SYMBOL(acpi_os_unmap_generic_address);
#ifdef ACPI_FUTURE_USAGE
acpi_status
diff --git a/include/linux/acpi_io.h b/include/linux/acpi_io.h
index 4afd710..b0ffa21 100644
--- a/include/linux/acpi_io.h
+++ b/include/linux/acpi_io.h
@@ -12,4 +12,7 @@ static inline void __iomem *acpi_os_ioremap(acpi_physical_address phys,
void __iomem *acpi_os_get_iomem(acpi_physical_address phys, unsigned int size);
+int acpi_os_map_generic_address(struct acpi_generic_address *addr);
+void acpi_os_unmap_generic_address(struct acpi_generic_address *addr);
+
#endif
--
1.7.9.rc1
^ permalink raw reply related
* [PATCH 39/50] ACPI: Fix possible alignment issues with GAS 'address' references
From: Len Brown @ 2012-01-17 12:21 UTC (permalink / raw)
To: linux-acpi, linux-pm, linux-kernel; +Cc: Myron Stowe, Myron Stowe, Len Brown
In-Reply-To: <553575f1ae048aa44682b46b3c51929a0b3ad337.1326802228.git.len.brown@intel.com>
From: Myron Stowe <mstowe@redhat.com>
Generic Address Structures (GAS) may reside within ACPI tables which
are byte aligned. This patch copies GAS 'address' references to a local
variable, which will be naturally aligned, to be used going forward.
ACPI Generic Address Structure (GAS) reference:
ACPI Specification, Revision 4.0, Section 5.2.3.1, "Generic Address
Structure"
Signed-off-by: Myron Stowe <myron.stowe@redhat.com>
Signed-off-by: Len Brown <len.brown@intel.com>
---
drivers/acpi/osl.c | 38 ++++++++++++++++++++++++--------------
1 files changed, 24 insertions(+), 14 deletions(-)
diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
index f31c5c5..2e285cd 100644
--- a/drivers/acpi/osl.c
+++ b/drivers/acpi/osl.c
@@ -166,17 +166,21 @@ static u32 acpi_osi_handler(acpi_string interface, u32 supported)
return supported;
}
-static void __init acpi_request_region (struct acpi_generic_address *addr,
+static void __init acpi_request_region (struct acpi_generic_address *gas,
unsigned int length, char *desc)
{
- if (!addr->address || !length)
+ u64 addr;
+
+ /* Handle possible alignment issues */
+ memcpy(&addr, &gas->address, sizeof(addr));
+ if (!addr || !length)
return;
/* Resources are never freed */
- if (addr->space_id == ACPI_ADR_SPACE_SYSTEM_IO)
- request_region(addr->address, length, desc);
- else if (addr->space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY)
- request_mem_region(addr->address, length, desc);
+ if (gas->space_id == ACPI_ADR_SPACE_SYSTEM_IO)
+ request_region(addr, length, desc);
+ else if (gas->space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY)
+ request_mem_region(addr, length, desc);
}
static int __init acpi_reserve_resources(void)
@@ -427,35 +431,41 @@ void __init early_acpi_os_unmap_memory(void __iomem *virt, acpi_size size)
__acpi_unmap_table(virt, size);
}
-static int acpi_os_map_generic_address(struct acpi_generic_address *addr)
+static int acpi_os_map_generic_address(struct acpi_generic_address *gas)
{
+ u64 addr;
void __iomem *virt;
- if (addr->space_id != ACPI_ADR_SPACE_SYSTEM_MEMORY)
+ if (gas->space_id != ACPI_ADR_SPACE_SYSTEM_MEMORY)
return 0;
- if (!addr->address || !addr->bit_width)
+ /* Handle possible alignment issues */
+ memcpy(&addr, &gas->address, sizeof(addr));
+ if (!addr || !gas->bit_width)
return -EINVAL;
- virt = acpi_os_map_memory(addr->address, addr->bit_width / 8);
+ virt = acpi_os_map_memory(addr, gas->bit_width / 8);
if (!virt)
return -EIO;
return 0;
}
-static void acpi_os_unmap_generic_address(struct acpi_generic_address *addr)
+static void acpi_os_unmap_generic_address(struct acpi_generic_address *gas)
{
+ u64 addr;
struct acpi_ioremap *map;
- if (addr->space_id != ACPI_ADR_SPACE_SYSTEM_MEMORY)
+ if (gas->space_id != ACPI_ADR_SPACE_SYSTEM_MEMORY)
return;
- if (!addr->address || !addr->bit_width)
+ /* Handle possible alignment issues */
+ memcpy(&addr, &gas->address, sizeof(addr));
+ if (!addr || !gas->bit_width)
return;
mutex_lock(&acpi_ioremap_lock);
- map = acpi_map_lookup(addr->address, addr->bit_width / 8);
+ map = acpi_map_lookup(addr, gas->bit_width / 8);
if (!map) {
mutex_unlock(&acpi_ioremap_lock);
return;
--
1.7.9.rc1
^ permalink raw reply related
* [PATCH 38/50] ACPI, ia64: Use SRAT table rev to use 8bit or 16/32bit PXM fields (ia64)
From: Len Brown @ 2012-01-17 12:21 UTC (permalink / raw)
To: linux-acpi, linux-pm, linux-kernel; +Cc: Kurt Garloff, Len Brown
In-Reply-To: <553575f1ae048aa44682b46b3c51929a0b3ad337.1326802228.git.len.brown@intel.com>
From: Kurt Garloff <kurt@garloff.de>
In SRAT v1, we had 8bit proximity domain (PXM) fields; SRAT v2 provides
32bits for these. The new fields were reserved before.
According to the ACPI spec, the OS must disregrard reserved fields.
ia64 did handle the PXM fields almost consistently, but depending on
sgi's sn2 platform. This patch leaves the sn2 logic in, but does also
use 16/32 bits for PXM if the SRAT has rev 2 or higher.
The patch also adds __init to the two pxm accessor functions, as they
access __initdata now and are called from an __init function only anyway.
Note that the code only uses 16 bits for the PXM field in the processor
proximity field; the patch does not address this as 16 bits are more than
enough.
Signed-off-by: Kurt Garloff <kurt@garloff.de>
Signed-off-by: Len Brown <len.brown@intel.com>
---
arch/ia64/kernel/acpi.c | 10 ++++++----
1 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/arch/ia64/kernel/acpi.c b/arch/ia64/kernel/acpi.c
index bfb4d01..5207035 100644
--- a/arch/ia64/kernel/acpi.c
+++ b/arch/ia64/kernel/acpi.c
@@ -429,22 +429,24 @@ static u32 __devinitdata pxm_flag[PXM_FLAG_LEN];
static struct acpi_table_slit __initdata *slit_table;
cpumask_t early_cpu_possible_map = CPU_MASK_NONE;
-static int get_processor_proximity_domain(struct acpi_srat_cpu_affinity *pa)
+static int __init
+get_processor_proximity_domain(struct acpi_srat_cpu_affinity *pa)
{
int pxm;
pxm = pa->proximity_domain_lo;
- if (ia64_platform_is("sn2"))
+ if (ia64_platform_is("sn2") || acpi_srat_revision >= 2)
pxm += pa->proximity_domain_hi[0] << 8;
return pxm;
}
-static int get_memory_proximity_domain(struct acpi_srat_mem_affinity *ma)
+static int __init
+get_memory_proximity_domain(struct acpi_srat_mem_affinity *ma)
{
int pxm;
pxm = ma->proximity_domain;
- if (!ia64_platform_is("sn2"))
+ if (!ia64_platform_is("sn2") && acpi_srat_revision <= 1)
pxm &= 0xff;
return pxm;
--
1.7.9.rc1
^ permalink raw reply related
* [PATCH 37/50] ACPI, x86: Use SRAT table rev to use 8bit or 32bit PXM fields (x86/x86-64)
From: Len Brown @ 2012-01-17 12:21 UTC (permalink / raw)
To: linux-acpi, linux-pm, linux-kernel; +Cc: Len Brown, x86, Kurt Garloff
In-Reply-To: <553575f1ae048aa44682b46b3c51929a0b3ad337.1326802228.git.len.brown@intel.com>
From: Kurt Garloff <kurt@garloff.de>
In SRAT v1, we had 8bit proximity domain (PXM) fields; SRAT v2 provides
32bits for these. The new fields were reserved before.
According to the ACPI spec, the OS must disregrard reserved fields.
x86/x86-64 was rather inconsistent prior to this patch; it used 8 bits
for the pxm field in cpu_affinity, but 32 bits in mem_affinity.
This patch makes it consistent: Either use 8 bits consistently (SRAT
rev 1 or lower) or 32 bits (SRAT rev 2 or higher).
cc: x86@kernel.org
Signed-off-by: Kurt Garloff <kurt@garloff.de>
Signed-off-by: Len Brown <len.brown@intel.com>
---
arch/x86/mm/srat.c | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/arch/x86/mm/srat.c b/arch/x86/mm/srat.c
index 81dbfde..7efd0c6 100644
--- a/arch/x86/mm/srat.c
+++ b/arch/x86/mm/srat.c
@@ -104,6 +104,8 @@ acpi_numa_processor_affinity_init(struct acpi_srat_cpu_affinity *pa)
if ((pa->flags & ACPI_SRAT_CPU_ENABLED) == 0)
return;
pxm = pa->proximity_domain_lo;
+ if (acpi_srat_revision >= 2)
+ pxm |= *((unsigned int*)pa->proximity_domain_hi) << 8;
node = setup_node(pxm);
if (node < 0) {
printk(KERN_ERR "SRAT: Too many proximity domains %x\n", pxm);
@@ -155,6 +157,8 @@ acpi_numa_memory_affinity_init(struct acpi_srat_mem_affinity *ma)
start = ma->base_address;
end = start + ma->length;
pxm = ma->proximity_domain;
+ if (acpi_srat_revision <= 1)
+ pxm &= 0xff;
node = setup_node(pxm);
if (node < 0) {
printk(KERN_ERR "SRAT: Too many proximity domains.\n");
--
1.7.9.rc1
^ permalink raw reply related
* [PATCH 36/50] ACPI: Store SRAT table revision
From: Len Brown @ 2012-01-17 12:21 UTC (permalink / raw)
To: linux-acpi, linux-pm, linux-kernel; +Cc: Kurt Garloff, Len Brown
In-Reply-To: <553575f1ae048aa44682b46b3c51929a0b3ad337.1326802228.git.len.brown@intel.com>
From: Kurt Garloff <kurt@garloff.de>
In SRAT v1, we had 8bit proximity domain (PXM) fields; SRAT v2 provides
32bits for these. The new fields were reserved before.
According to the ACPI spec, the OS must disregrard reserved fields.
In order to know whether or not, we must know what version the SRAT
table has.
This patch stores the SRAT table revision for later consumption
by arch specific __init functions.
Signed-off-by: Kurt Garloff <kurt@garloff.de>
Signed-off-by: Len Brown <len.brown@intel.com>
---
drivers/acpi/numa.c | 6 ++++++
include/acpi/acpi_numa.h | 1 +
2 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/drivers/acpi/numa.c b/drivers/acpi/numa.c
index 3b5c318..e56f3be 100644
--- a/drivers/acpi/numa.c
+++ b/drivers/acpi/numa.c
@@ -45,6 +45,8 @@ static int pxm_to_node_map[MAX_PXM_DOMAINS]
static int node_to_pxm_map[MAX_NUMNODES]
= { [0 ... MAX_NUMNODES - 1] = PXM_INVAL };
+unsigned char acpi_srat_revision __initdata;
+
int pxm_to_node(int pxm)
{
if (pxm < 0)
@@ -255,9 +257,13 @@ acpi_parse_memory_affinity(struct acpi_subtable_header * header,
static int __init acpi_parse_srat(struct acpi_table_header *table)
{
+ struct acpi_table_srat *srat;
if (!table)
return -EINVAL;
+ srat = (struct acpi_table_srat *)table;
+ acpi_srat_revision = srat->header.revision;
+
/* Real work done in acpi_table_parse_srat below. */
return 0;
diff --git a/include/acpi/acpi_numa.h b/include/acpi/acpi_numa.h
index 1739726..451823c 100644
--- a/include/acpi/acpi_numa.h
+++ b/include/acpi/acpi_numa.h
@@ -15,6 +15,7 @@ extern int pxm_to_node(int);
extern int node_to_pxm(int);
extern void __acpi_map_pxm_to_node(int, int);
extern int acpi_map_pxm_to_node(int);
+extern unsigned char acpi_srat_revision;
#endif /* CONFIG_ACPI_NUMA */
#endif /* __ACP_NUMA_H */
--
1.7.9.rc1
^ permalink raw reply related
* [PATCH 35/50] ACPI, APEI, Resolve false conflict between ACPI NVS and APEI
From: Len Brown @ 2012-01-17 12:21 UTC (permalink / raw)
To: linux-acpi, linux-pm, linux-kernel; +Cc: Huang Ying, Len Brown
In-Reply-To: <553575f1ae048aa44682b46b3c51929a0b3ad337.1326802228.git.len.brown@intel.com>
From: Huang Ying <ying.huang@intel.com>
Some firmware will access memory in ACPI NVS region via APEI. That
is, instructions in APEI ERST/EINJ table will read/write ACPI NVS
region. The original resource conflict checking in APEI code will
check memory/ioport accessed by APEI via general resource management
mech. But ACPI NVS region is marked as busy already, so that the
false resource conflict will prevent APEI ERST/EINJ to work.
To fix this, this patch excludes ACPI NVS regions when APEI components
request resources. So that they will not conflict with ACPI NVS
regions.
Reported-and-tested-by: Pavel Ivanov <paivanof@gmail.com>
Signed-off-by: Huang Ying <ying.huang@intel.com>
Signed-off-by: Len Brown <len.brown@intel.com>
---
drivers/acpi/apei/apei-base.c | 29 ++++++++++++++++++++++++++++-
1 files changed, 28 insertions(+), 1 deletions(-)
diff --git a/drivers/acpi/apei/apei-base.c b/drivers/acpi/apei/apei-base.c
index f2c5062..4abb6c7 100644
--- a/drivers/acpi/apei/apei-base.c
+++ b/drivers/acpi/apei/apei-base.c
@@ -449,8 +449,19 @@ int apei_resources_sub(struct apei_resources *resources1,
}
EXPORT_SYMBOL_GPL(apei_resources_sub);
+static int apei_get_nvs_callback(__u64 start, __u64 size, void *data)
+{
+ struct apei_resources *resources = data;
+ return apei_res_add(&resources->iomem, start, size);
+}
+
+static int apei_get_nvs_resources(struct apei_resources *resources)
+{
+ return acpi_nvs_for_each_region(apei_get_nvs_callback, resources);
+}
+
/*
- * IO memory/port rersource management mechanism is used to check
+ * IO memory/port resource management mechanism is used to check
* whether memory/port area used by GARs conflicts with normal memory
* or IO memory/port of devices.
*/
@@ -459,12 +470,26 @@ int apei_resources_request(struct apei_resources *resources,
{
struct apei_res *res, *res_bak = NULL;
struct resource *r;
+ struct apei_resources nvs_resources;
int rc;
rc = apei_resources_sub(resources, &apei_resources_all);
if (rc)
return rc;
+ /*
+ * Some firmware uses ACPI NVS region, that has been marked as
+ * busy, so exclude it from APEI resources to avoid false
+ * conflict.
+ */
+ apei_resources_init(&nvs_resources);
+ rc = apei_get_nvs_resources(&nvs_resources);
+ if (rc)
+ goto res_fini;
+ rc = apei_resources_sub(resources, &nvs_resources);
+ if (rc)
+ goto res_fini;
+
rc = -EINVAL;
list_for_each_entry(res, &resources->iomem, list) {
r = request_mem_region(res->start, res->end - res->start,
@@ -511,6 +536,8 @@ err_unmap_iomem:
break;
release_mem_region(res->start, res->end - res->start);
}
+res_fini:
+ apei_resources_fini(&nvs_resources);
return rc;
}
EXPORT_SYMBOL_GPL(apei_resources_request);
--
1.7.9.rc1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox