* [PATCH 07/11] coresight-etm4x: Controls pertaining to the sequencer functions
From: Mathieu Poirier @ 2015-04-22 22:40 UTC (permalink / raw)
To: gregkh
Cc: linux-arm-kernel, linux-api, linux-kernel, kaixu.xia,
zhang.chunyan, mathieu.poirier
In-Reply-To: <1429742451-11465-1-git-send-email-mathieu.poirier@linaro.org>
From: Pratik Patel <pratikp@codeaurora.org>
Adding sysfs entries to access the sequencers related registers,
more specifically the sequencer state, the sequencer state
transition and the sequencer reset control registers.
Signed-off-by: Pratik Patel <pratikp@codeaurora.org>
Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
---
.../ABI/testing/sysfs-bus-coresight-devices-etm4x | 25 ++++
drivers/hwtracing/coresight/coresight-etm4x.c | 129 +++++++++++++++++++++
2 files changed, 154 insertions(+)
diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices-etm4x b/Documentation/ABI/testing/sysfs-bus-coresight-devices-etm4x
index 8cdc4ad10bd6..44723f2e107e 100644
--- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-etm4x
+++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-etm4x
@@ -191,3 +191,28 @@ Date: April 2015
KernelVersion: 4.01
Contact: Mathieu Poirier <mathieu.poirier@linaro.org>
Description: (RW) Used to setup address range comparator values.
+
+What: /sys/bus/coresight/devices/<memory_map>.etm/seq_idx
+Date: April 2015
+KernelVersion: 4.01
+Contact: Mathieu Poirier <mathieu.poirier@linaro.org>
+Description: (RW) Select which sequensor.
+
+What: /sys/bus/coresight/devices/<memory_map>.etm/seq_state
+Date: April 2015
+KernelVersion: 4.01
+Contact: Mathieu Poirier <mathieu.poirier@linaro.org>
+Description: (RW) Use this to set, or read, the sequencer state.
+
+What: /sys/bus/coresight/devices/<memory_map>.etm/seq_event
+Date: April 2015
+KernelVersion: 4.01
+Contact: Mathieu Poirier <mathieu.poirier@linaro.org>
+Description: (RW) Moves the sequencer state to a specific state.
+
+What: /sys/bus/coresight/devices/<memory_map>.etm/seq_reset_event
+Date: April 2015
+KernelVersion: 4.01
+Contact: Mathieu Poirier <mathieu.poirier@linaro.org>
+Description: (RW) Moves the sequencer to state 0 when a programmed event
+ occurs.
diff --git a/drivers/hwtracing/coresight/coresight-etm4x.c b/drivers/hwtracing/coresight/coresight-etm4x.c
index 439b37109279..7d2af2926967 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x.c
@@ -1442,6 +1442,131 @@ static ssize_t addr_context_store(struct device *dev,
}
static DEVICE_ATTR_RW(addr_context);
+static ssize_t seq_idx_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ unsigned long val;
+ struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+ val = drvdata->seq_idx;
+ return scnprintf(buf, PAGE_SIZE, "%#lx\n", val);
+}
+
+static ssize_t seq_idx_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t size)
+{
+ unsigned long val;
+ struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+ if (kstrtoul(buf, 16, &val))
+ return -EINVAL;
+ if (val >= drvdata->nrseqstate - 1)
+ return -EINVAL;
+
+ /*
+ * Use spinlock to ensure index doesn't change while it gets
+ * dereferenced multiple times within a spinlock block elsewhere.
+ */
+ spin_lock(&drvdata->spinlock);
+ drvdata->seq_idx = val;
+ spin_unlock(&drvdata->spinlock);
+ return size;
+}
+static DEVICE_ATTR_RW(seq_idx);
+
+static ssize_t seq_state_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ unsigned long val;
+ struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+ val = drvdata->seq_state;
+ return scnprintf(buf, PAGE_SIZE, "%#lx\n", val);
+}
+
+static ssize_t seq_state_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t size)
+{
+ unsigned long val;
+ struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+ if (kstrtoul(buf, 16, &val))
+ return -EINVAL;
+ if (val >= drvdata->nrseqstate)
+ return -EINVAL;
+
+ drvdata->seq_state = val;
+ return size;
+}
+static DEVICE_ATTR_RW(seq_state);
+
+static ssize_t seq_event_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ u8 idx;
+ unsigned long val;
+ struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+ spin_lock(&drvdata->spinlock);
+ idx = drvdata->seq_idx;
+ val = drvdata->seq_ctrl[idx];
+ spin_unlock(&drvdata->spinlock);
+ return scnprintf(buf, PAGE_SIZE, "%#lx\n", val);
+}
+
+static ssize_t seq_event_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t size)
+{
+ u8 idx;
+ unsigned long val;
+ struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+ if (kstrtoul(buf, 16, &val))
+ return -EINVAL;
+
+ spin_lock(&drvdata->spinlock);
+ idx = drvdata->seq_idx;
+ /* RST, bits[7:0] */
+ drvdata->seq_ctrl[idx] = val & 0xFF;
+ spin_unlock(&drvdata->spinlock);
+ return size;
+}
+static DEVICE_ATTR_RW(seq_event);
+
+static ssize_t seq_reset_event_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ unsigned long val;
+ struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+ val = drvdata->seq_rst;
+ return scnprintf(buf, PAGE_SIZE, "%#lx\n", val);
+}
+
+static ssize_t seq_reset_event_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t size)
+{
+ unsigned long val;
+ struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+ if (kstrtoul(buf, 16, &val))
+ return -EINVAL;
+ if (!(drvdata->nrseqstate))
+ return -EINVAL;
+
+ drvdata->seq_rst = val & ETMv4_EVENT_MASK;
+ return size;
+}
+static DEVICE_ATTR_RW(seq_reset_event);
+
static ssize_t status_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
@@ -1614,6 +1739,10 @@ static struct attribute *coresight_etmv4_attrs[] = {
&dev_attr_addr_stop.attr,
&dev_attr_addr_ctxtype.attr,
&dev_attr_addr_context.attr,
+ &dev_attr_seq_idx.attr,
+ &dev_attr_seq_state.attr,
+ &dev_attr_seq_event.attr,
+ &dev_attr_seq_reset_event.attr,
&dev_attr_status.attr,
&dev_attr_mgmt.attr,
&dev_attr_trcidr.attr,
--
1.9.1
^ permalink raw reply related
* [PATCH 08/11] coresight-etm4x: Controls pertaining to the counter functions
From: Mathieu Poirier @ 2015-04-22 22:40 UTC (permalink / raw)
To: gregkh
Cc: linux-arm-kernel, linux-api, linux-kernel, kaixu.xia,
zhang.chunyan, mathieu.poirier
In-Reply-To: <1429742451-11465-1-git-send-email-mathieu.poirier@linaro.org>
From: Pratik Patel <pratikp@codeaurora.org>
Adding sysfs entries related to the counter functionality, more specifically
to set, control and reload the counters.
Signed-off-by: Pratik Patel <pratikp@codeaurora.org>
Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
---
.../ABI/testing/sysfs-bus-coresight-devices-etm4x | 26 ++++
drivers/hwtracing/coresight/coresight-etm4x.c | 144 +++++++++++++++++++++
2 files changed, 170 insertions(+)
diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices-etm4x b/Documentation/ABI/testing/sysfs-bus-coresight-devices-etm4x
index 44723f2e107e..b4581c9426d3 100644
--- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-etm4x
+++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-etm4x
@@ -216,3 +216,29 @@ KernelVersion: 4.01
Contact: Mathieu Poirier <mathieu.poirier@linaro.org>
Description: (RW) Moves the sequencer to state 0 when a programmed event
occurs.
+
+What: /sys/bus/coresight/devices/<memory_map>.etm/cntr_idx
+Date: April 2015
+KernelVersion: 4.01
+Contact: Mathieu Poirier <mathieu.poirier@linaro.org>
+Description: (RW) Select which counter unit to work with.
+
+What: /sys/bus/coresight/devices/<memory_map>.etm/cntrldvr
+Date: April 2015
+KernelVersion: 4.01
+Contact: Mathieu Poirier <mathieu.poirier@linaro.org>
+Description: (RW) This sets or returns the reload count value of the
+ specific counter.
+
+What: /sys/bus/coresight/devices/<memory_map>.etm/cntr_val
+Date: April 2015
+KernelVersion: 4.01
+Contact: Mathieu Poirier <mathieu.poirier@linaro.org>
+Description: (RW) This sets or returns the current count value of the
+ specific counter.
+
+What: /sys/bus/coresight/devices/<memory_map>.etm/cntr_ctrl
+Date: April 2015
+KernelVersion: 4.01
+Contact: Mathieu Poirier <mathieu.poirier@linaro.org>
+Description: (RW) Controls the operation of the selected counter.
diff --git a/drivers/hwtracing/coresight/coresight-etm4x.c b/drivers/hwtracing/coresight/coresight-etm4x.c
index 7d2af2926967..309f0c282392 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x.c
@@ -1567,6 +1567,146 @@ static ssize_t seq_reset_event_store(struct device *dev,
}
static DEVICE_ATTR_RW(seq_reset_event);
+static ssize_t cntr_idx_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ unsigned long val;
+ struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+ val = drvdata->cntr_idx;
+ return scnprintf(buf, PAGE_SIZE, "%#lx\n", val);
+}
+
+static ssize_t cntr_idx_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t size)
+{
+ unsigned long val;
+ struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+ if (kstrtoul(buf, 16, &val))
+ return -EINVAL;
+ if (val >= drvdata->nr_cntr)
+ return -EINVAL;
+
+ /*
+ * Use spinlock to ensure index doesn't change while it gets
+ * dereferenced multiple times within a spinlock block elsewhere.
+ */
+ spin_lock(&drvdata->spinlock);
+ drvdata->cntr_idx = val;
+ spin_unlock(&drvdata->spinlock);
+ return size;
+}
+static DEVICE_ATTR_RW(cntr_idx);
+
+static ssize_t cntrldvr_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ u8 idx;
+ unsigned long val;
+ struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+ spin_lock(&drvdata->spinlock);
+ idx = drvdata->cntr_idx;
+ val = drvdata->cntrldvr[idx];
+ spin_unlock(&drvdata->spinlock);
+ return scnprintf(buf, PAGE_SIZE, "%#lx\n", val);
+}
+
+static ssize_t cntrldvr_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t size)
+{
+ u8 idx;
+ unsigned long val;
+ struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+ if (kstrtoul(buf, 16, &val))
+ return -EINVAL;
+ if (val > ETM_CNTR_MAX_VAL)
+ return -EINVAL;
+
+ spin_lock(&drvdata->spinlock);
+ idx = drvdata->cntr_idx;
+ drvdata->cntrldvr[idx] = val;
+ spin_unlock(&drvdata->spinlock);
+ return size;
+}
+static DEVICE_ATTR_RW(cntrldvr);
+
+static ssize_t cntr_val_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ u8 idx;
+ unsigned long val;
+ struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+ spin_lock(&drvdata->spinlock);
+ idx = drvdata->cntr_idx;
+ val = drvdata->cntr_val[idx];
+ spin_unlock(&drvdata->spinlock);
+ return scnprintf(buf, PAGE_SIZE, "%#lx\n", val);
+}
+
+static ssize_t cntr_val_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t size)
+{
+ u8 idx;
+ unsigned long val;
+ struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+ if (kstrtoul(buf, 16, &val))
+ return -EINVAL;
+ if (val > ETM_CNTR_MAX_VAL)
+ return -EINVAL;
+
+ spin_lock(&drvdata->spinlock);
+ idx = drvdata->cntr_idx;
+ drvdata->cntr_val[idx] = val;
+ spin_unlock(&drvdata->spinlock);
+ return size;
+}
+static DEVICE_ATTR_RW(cntr_val);
+
+static ssize_t cntr_ctrl_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ u8 idx;
+ unsigned long val;
+ struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+ spin_lock(&drvdata->spinlock);
+ idx = drvdata->cntr_idx;
+ val = drvdata->cntr_ctrl[idx];
+ spin_unlock(&drvdata->spinlock);
+ return scnprintf(buf, PAGE_SIZE, "%#lx\n", val);
+}
+
+static ssize_t cntr_ctrl_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t size)
+{
+ u8 idx;
+ unsigned long val;
+ struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+ if (kstrtoul(buf, 16, &val))
+ return -EINVAL;
+
+ spin_lock(&drvdata->spinlock);
+ idx = drvdata->cntr_idx;
+ drvdata->cntr_ctrl[idx] = val;
+ spin_unlock(&drvdata->spinlock);
+ return size;
+}
+static DEVICE_ATTR_RW(cntr_ctrl);
+
static ssize_t status_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
@@ -1743,6 +1883,10 @@ static struct attribute *coresight_etmv4_attrs[] = {
&dev_attr_seq_state.attr,
&dev_attr_seq_event.attr,
&dev_attr_seq_reset_event.attr,
+ &dev_attr_cntr_idx.attr,
+ &dev_attr_cntrldvr.attr,
+ &dev_attr_cntr_val.attr,
+ &dev_attr_cntr_ctrl.attr,
&dev_attr_status.attr,
&dev_attr_mgmt.attr,
&dev_attr_trcidr.attr,
--
1.9.1
^ permalink raw reply related
* [PATCH 09/11] coresight-etm4x: Controls pertaining to the selection of resources
From: Mathieu Poirier @ 2015-04-22 22:40 UTC (permalink / raw)
To: gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
kaixu.xia-QSEj5FYQhm4dnm+yROfE0A,
zhang.chunyan-QSEj5FYQhm4dnm+yROfE0A,
mathieu.poirier-QSEj5FYQhm4dnm+yROfE0A
In-Reply-To: <1429742451-11465-1-git-send-email-mathieu.poirier-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
From: Pratik Patel <pratikp-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
Adding sysfs entries to control the selection of the resources the
trace unit will use as triggers to perform a trace run.
Signed-off-by: Pratik Patel <pratikp-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
Signed-off-by: Mathieu Poirier <mathieu.poirier-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
.../ABI/testing/sysfs-bus-coresight-devices-etm4x | 12 ++++
drivers/hwtracing/coresight/coresight-etm4x.c | 75 ++++++++++++++++++++++
2 files changed, 87 insertions(+)
diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices-etm4x b/Documentation/ABI/testing/sysfs-bus-coresight-devices-etm4x
index b4581c9426d3..b5c0456290ab 100644
--- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-etm4x
+++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-etm4x
@@ -242,3 +242,15 @@ Date: April 2015
KernelVersion: 4.01
Contact: Mathieu Poirier <mathieu.poirier-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Description: (RW) Controls the operation of the selected counter.
+
+What: /sys/bus/coresight/devices/<memory_map>.etm/res_idx
+Date: April 2015
+KernelVersion: 4.01
+Contact: Mathieu Poirier <mathieu.poirier-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
+Description: (RW) Select which resource selection unit to work with.
+
+What: /sys/bus/coresight/devices/<memory_map>.etm/res_ctrl
+Date: April 2015
+KernelVersion: 4.01
+Contact: Mathieu Poirier <mathieu.poirier-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
+Description: (RW) Controls the selection of the resources in the trace unit.
diff --git a/drivers/hwtracing/coresight/coresight-etm4x.c b/drivers/hwtracing/coresight/coresight-etm4x.c
index 309f0c282392..67f3b71ea8dc 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x.c
@@ -1707,6 +1707,79 @@ static ssize_t cntr_ctrl_store(struct device *dev,
}
static DEVICE_ATTR_RW(cntr_ctrl);
+static ssize_t res_idx_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ unsigned long val;
+ struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+ val = drvdata->res_idx;
+ return scnprintf(buf, PAGE_SIZE, "%#lx\n", val);
+}
+
+static ssize_t res_idx_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t size)
+{
+ unsigned long val;
+ struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+ if (kstrtoul(buf, 16, &val))
+ return -EINVAL;
+ /* Resource selector pair 0 is always implemented and reserved */
+ if ((val == 0) || (val >= drvdata->nr_resource))
+ return -EINVAL;
+
+ /*
+ * Use spinlock to ensure index doesn't change while it gets
+ * dereferenced multiple times within a spinlock block elsewhere.
+ */
+ spin_lock(&drvdata->spinlock);
+ drvdata->res_idx = val;
+ spin_unlock(&drvdata->spinlock);
+ return size;
+}
+static DEVICE_ATTR_RW(res_idx);
+
+static ssize_t res_ctrl_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ u8 idx;
+ unsigned long val;
+ struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+ spin_lock(&drvdata->spinlock);
+ idx = drvdata->res_idx;
+ val = drvdata->res_ctrl[idx];
+ spin_unlock(&drvdata->spinlock);
+ return scnprintf(buf, PAGE_SIZE, "%#lx\n", val);
+}
+
+static ssize_t res_ctrl_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t size)
+{
+ u8 idx;
+ unsigned long val;
+ struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+ if (kstrtoul(buf, 16, &val))
+ return -EINVAL;
+
+ spin_lock(&drvdata->spinlock);
+ idx = drvdata->res_idx;
+ /* For odd idx pair inversal bit is RES0 */
+ if (idx % 2 != 0)
+ /* PAIRINV, bit[21] */
+ val &= ~BIT(21);
+ drvdata->res_ctrl[idx] = val;
+ spin_unlock(&drvdata->spinlock);
+ return size;
+}
+static DEVICE_ATTR_RW(res_ctrl);
+
static ssize_t status_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
@@ -1887,6 +1960,8 @@ static struct attribute *coresight_etmv4_attrs[] = {
&dev_attr_cntrldvr.attr,
&dev_attr_cntr_val.attr,
&dev_attr_cntr_ctrl.attr,
+ &dev_attr_res_idx.attr,
+ &dev_attr_res_ctrl.attr,
&dev_attr_status.attr,
&dev_attr_mgmt.attr,
&dev_attr_trcidr.attr,
--
1.9.1
^ permalink raw reply related
* [PATCH 10/11] coresight-etm4x: Controls pertaining to the context ID functions
From: Mathieu Poirier @ 2015-04-22 22:40 UTC (permalink / raw)
To: gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
kaixu.xia-QSEj5FYQhm4dnm+yROfE0A,
zhang.chunyan-QSEj5FYQhm4dnm+yROfE0A,
mathieu.poirier-QSEj5FYQhm4dnm+yROfE0A
In-Reply-To: <1429742451-11465-1-git-send-email-mathieu.poirier-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
From: Pratik Patel <pratikp-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
Adding sysfs entries to access and configure specifics about the
context ID comparator functions.
Signed-off-by: Pratik Patel <pratikp-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
Signed-off-by: Mathieu Poirier <mathieu.poirier-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
.../ABI/testing/sysfs-bus-coresight-devices-etm4x | 19 +++
drivers/hwtracing/coresight/coresight-etm4x.c | 187 +++++++++++++++++++++
2 files changed, 206 insertions(+)
diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices-etm4x b/Documentation/ABI/testing/sysfs-bus-coresight-devices-etm4x
index b5c0456290ab..8b32cb7b9723 100644
--- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-etm4x
+++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-etm4x
@@ -254,3 +254,22 @@ Date: April 2015
KernelVersion: 4.01
Contact: Mathieu Poirier <mathieu.poirier-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Description: (RW) Controls the selection of the resources in the trace unit.
+
+What: /sys/bus/coresight/devices/<memory_map>.etm/ctxid_idx
+Date: April 2015
+KernelVersion: 4.01
+Contact: Mathieu Poirier <mathieu.poirier-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
+Description: (RW) Select which context ID comparator to work with.
+
+What: /sys/bus/coresight/devices/<memory_map>.etm/ctxid_val
+Date: April 2015
+KernelVersion: 4.01
+Contact: Mathieu Poirier <mathieu.poirier-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
+Description: (RW) Get/Set the context ID comparator value to trigger on.
+
+What: /sys/bus/coresight/devices/<memory_map>.etm/ctxid_masks
+Date: April 2015
+KernelVersion: 4.01
+Contact: Mathieu Poirier <mathieu.poirier-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
+Description: (RW) Mask for all 8 context ID comparator value
+ registers (if implemented).
diff --git a/drivers/hwtracing/coresight/coresight-etm4x.c b/drivers/hwtracing/coresight/coresight-etm4x.c
index 67f3b71ea8dc..9bae7b158b20 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x.c
@@ -1780,6 +1780,190 @@ static ssize_t res_ctrl_store(struct device *dev,
}
static DEVICE_ATTR_RW(res_ctrl);
+static ssize_t ctxid_idx_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ unsigned long val;
+ struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+ val = drvdata->ctxid_idx;
+ return scnprintf(buf, PAGE_SIZE, "%#lx\n", val);
+}
+
+static ssize_t ctxid_idx_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t size)
+{
+ unsigned long val;
+ struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+ if (kstrtoul(buf, 16, &val))
+ return -EINVAL;
+ if (val >= drvdata->numcidc)
+ return -EINVAL;
+
+ /*
+ * Use spinlock to ensure index doesn't change while it gets
+ * dereferenced multiple times within a spinlock block elsewhere.
+ */
+ spin_lock(&drvdata->spinlock);
+ drvdata->ctxid_idx = val;
+ spin_unlock(&drvdata->spinlock);
+ return size;
+}
+static DEVICE_ATTR_RW(ctxid_idx);
+
+static ssize_t ctxid_val_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ u8 idx;
+ unsigned long val;
+ struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+ spin_lock(&drvdata->spinlock);
+ idx = drvdata->ctxid_idx;
+ val = (unsigned long)drvdata->ctxid_val[idx];
+ spin_unlock(&drvdata->spinlock);
+ return scnprintf(buf, PAGE_SIZE, "%#lx\n", val);
+}
+
+static ssize_t ctxid_val_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t size)
+{
+ u8 idx;
+ unsigned long val;
+ struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+ /*
+ * only implemented when ctxid tracing is enabled, i.e. at least one
+ * ctxid comparator is implemented and ctxid is greater than 0 bits
+ * in length
+ */
+ if (!drvdata->ctxid_size || !drvdata->numcidc)
+ return -EINVAL;
+ if (kstrtoul(buf, 16, &val))
+ return -EINVAL;
+
+ spin_lock(&drvdata->spinlock);
+ idx = drvdata->ctxid_idx;
+ drvdata->ctxid_val[idx] = (u64)val;
+ spin_unlock(&drvdata->spinlock);
+ return size;
+}
+static DEVICE_ATTR_RW(ctxid_val);
+
+static ssize_t ctxid_masks_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ unsigned long val1, val2;
+ struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+ spin_lock(&drvdata->spinlock);
+ val1 = drvdata->ctxid_mask0;
+ val2 = drvdata->ctxid_mask1;
+ spin_unlock(&drvdata->spinlock);
+ return scnprintf(buf, PAGE_SIZE, "%#lx %#lx\n", val1, val2);
+}
+
+static ssize_t ctxid_masks_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t size)
+{
+ u8 i, j, maskbyte;
+ unsigned long val1, val2, mask;
+ struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+ /*
+ * only implemented when ctxid tracing is enabled, i.e. at least one
+ * ctxid comparator is implemented and ctxid is greater than 0 bits
+ * in length
+ */
+ if (!drvdata->ctxid_size || !drvdata->numcidc)
+ return -EINVAL;
+ if (sscanf(buf, "%lx %lx", &val1, &val2) != 2)
+ return -EINVAL;
+
+ spin_lock(&drvdata->spinlock);
+ /*
+ * each byte[0..3] controls mask value applied to ctxid
+ * comparator[0..3]
+ */
+ switch (drvdata->numcidc) {
+ case 0x1:
+ /* COMP0, bits[7:0] */
+ drvdata->ctxid_mask0 = val1 & 0xFF;
+ break;
+ case 0x2:
+ /* COMP1, bits[15:8] */
+ drvdata->ctxid_mask0 = val1 & 0xFFFF;
+ break;
+ case 0x3:
+ /* COMP2, bits[23:16] */
+ drvdata->ctxid_mask0 = val1 & 0xFFFFFF;
+ break;
+ case 0x4:
+ /* COMP3, bits[31:24] */
+ drvdata->ctxid_mask0 = val1;
+ break;
+ case 0x5:
+ /* COMP4, bits[7:0] */
+ drvdata->ctxid_mask0 = val1;
+ drvdata->ctxid_mask1 = val2 & 0xFF;
+ break;
+ case 0x6:
+ /* COMP5, bits[15:8] */
+ drvdata->ctxid_mask0 = val1;
+ drvdata->ctxid_mask1 = val2 & 0xFFFF;
+ break;
+ case 0x7:
+ /* COMP6, bits[23:16] */
+ drvdata->ctxid_mask0 = val1;
+ drvdata->ctxid_mask1 = val2 & 0xFFFFFF;
+ break;
+ case 0x8:
+ /* COMP7, bits[31:24] */
+ drvdata->ctxid_mask0 = val1;
+ drvdata->ctxid_mask1 = val2;
+ break;
+ default:
+ break;
+ }
+ /*
+ * If software sets a mask bit to 1, it must program relevant byte
+ * of ctxid comparator value 0x0, otherwise behavior is unpredictable.
+ * For example, if bit[3] of ctxid_mask0 is 1, we must clear bits[31:24]
+ * of ctxid comparator0 value (corresponding to byte 0) register.
+ */
+ mask = drvdata->ctxid_mask0;
+ for (i = 0; i < drvdata->numcidc; i++) {
+ /* mask value of corresponding ctxid comparator */
+ maskbyte = mask & ETMv4_EVENT_MASK;
+ /*
+ * each bit corresponds to a byte of respective ctxid comparator
+ * value register
+ */
+ for (j = 0; j < 8; j++) {
+ if (maskbyte & 1)
+ drvdata->ctxid_val[i] &= ~(0xFF << (j * 8));
+ maskbyte >>= 1;
+ }
+ /* Select the next ctxid comparator mask value */
+ if (i == 3)
+ /* ctxid comparators[4-7] */
+ mask = drvdata->ctxid_mask1;
+ else
+ mask >>= 0x8;
+ }
+
+ spin_unlock(&drvdata->spinlock);
+ return size;
+}
+static DEVICE_ATTR_RW(ctxid_masks);
+
static ssize_t status_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
@@ -1962,6 +2146,9 @@ static struct attribute *coresight_etmv4_attrs[] = {
&dev_attr_cntr_ctrl.attr,
&dev_attr_res_idx.attr,
&dev_attr_res_ctrl.attr,
+ &dev_attr_ctxid_idx.attr,
+ &dev_attr_ctxid_val.attr,
+ &dev_attr_ctxid_masks.attr,
&dev_attr_status.attr,
&dev_attr_mgmt.attr,
&dev_attr_trcidr.attr,
--
1.9.1
^ permalink raw reply related
* [PATCH 11/11] coresight-etm4x: Controls pertaining to the VM ID functions
From: Mathieu Poirier @ 2015-04-22 22:40 UTC (permalink / raw)
To: gregkh
Cc: linux-arm-kernel, linux-api, linux-kernel, kaixu.xia,
zhang.chunyan, mathieu.poirier
In-Reply-To: <1429742451-11465-1-git-send-email-mathieu.poirier@linaro.org>
From: Pratik Patel <pratikp@codeaurora.org>
Adding sysfs entries to access and configure specifics about the
virtual machine ID comparator functions.
Signed-off-by: Pratik Patel <pratikp@codeaurora.org>
Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
---
.../ABI/testing/sysfs-bus-coresight-devices-etm4x | 20 +++
drivers/hwtracing/coresight/coresight-etm4x.c | 178 +++++++++++++++++++++
2 files changed, 198 insertions(+)
diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices-etm4x b/Documentation/ABI/testing/sysfs-bus-coresight-devices-etm4x
index 8b32cb7b9723..eb5153891919 100644
--- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-etm4x
+++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-etm4x
@@ -273,3 +273,23 @@ KernelVersion: 4.01
Contact: Mathieu Poirier <mathieu.poirier@linaro.org>
Description: (RW) Mask for all 8 context ID comparator value
registers (if implemented).
+
+What: /sys/bus/coresight/devices/<memory_map>.etm/vmid_idx
+Date: April 2015
+KernelVersion: 4.01
+Contact: Mathieu Poirier <mathieu.poirier@linaro.org>
+Description: (RW) Select which virtual machine ID comparator to work with.
+
+What: /sys/bus/coresight/devices/<memory_map>.etm/vmid_val
+Date: April 2015
+KernelVersion: 4.01
+Contact: Mathieu Poirier <mathieu.poirier@linaro.org>
+Description: (RW) Get/Set the virtual machine ID comparator value to
+ trigger on.
+
+What: /sys/bus/coresight/devices/<memory_map>.etm/vmid_masks
+Date: April 2015
+KernelVersion: 4.01
+Contact: Mathieu Poirier <mathieu.poirier@linaro.org>
+Description: (RW) Mask for all 8 virtual machine ID comparator value
+ registers (if implemented).
diff --git a/drivers/hwtracing/coresight/coresight-etm4x.c b/drivers/hwtracing/coresight/coresight-etm4x.c
index 9bae7b158b20..c3c03c72593e 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x.c
@@ -1964,6 +1964,181 @@ static ssize_t ctxid_masks_store(struct device *dev,
}
static DEVICE_ATTR_RW(ctxid_masks);
+static ssize_t vmid_idx_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ unsigned long val;
+ struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+ val = drvdata->vmid_idx;
+ return scnprintf(buf, PAGE_SIZE, "%#lx\n", val);
+}
+
+static ssize_t vmid_idx_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t size)
+{
+ unsigned long val;
+ struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+ if (kstrtoul(buf, 16, &val))
+ return -EINVAL;
+ if (val >= drvdata->numvmidc)
+ return -EINVAL;
+
+ /*
+ * Use spinlock to ensure index doesn't change while it gets
+ * dereferenced multiple times within a spinlock block elsewhere.
+ */
+ spin_lock(&drvdata->spinlock);
+ drvdata->vmid_idx = val;
+ spin_unlock(&drvdata->spinlock);
+ return size;
+}
+static DEVICE_ATTR_RW(vmid_idx);
+
+static ssize_t vmid_val_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ unsigned long val;
+ struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+ val = (unsigned long)drvdata->vmid_val[drvdata->vmid_idx];
+ return scnprintf(buf, PAGE_SIZE, "%#lx\n", val);
+}
+
+static ssize_t vmid_val_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t size)
+{
+ unsigned long val;
+ struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+ /*
+ * only implemented when vmid tracing is enabled, i.e. at least one
+ * vmid comparator is implemented and at least 8 bit vmid size
+ */
+ if (!drvdata->vmid_size || !drvdata->numvmidc)
+ return -EINVAL;
+ if (kstrtoul(buf, 16, &val))
+ return -EINVAL;
+
+ spin_lock(&drvdata->spinlock);
+ drvdata->vmid_val[drvdata->vmid_idx] = (u64)val;
+ spin_unlock(&drvdata->spinlock);
+ return size;
+}
+static DEVICE_ATTR_RW(vmid_val);
+
+static ssize_t vmid_masks_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ unsigned long val1, val2;
+ struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+ spin_lock(&drvdata->spinlock);
+ val1 = drvdata->vmid_mask0;
+ val2 = drvdata->vmid_mask1;
+ spin_unlock(&drvdata->spinlock);
+ return scnprintf(buf, PAGE_SIZE, "%#lx %#lx\n", val1, val2);
+}
+
+static ssize_t vmid_masks_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t size)
+{
+ u8 i, j, maskbyte;
+ unsigned long val1, val2, mask;
+ struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
+ /*
+ * only implemented when vmid tracing is enabled, i.e. at least one
+ * vmid comparator is implemented and at least 8 bit vmid size
+ */
+ if (!drvdata->vmid_size || !drvdata->numvmidc)
+ return -EINVAL;
+ if (sscanf(buf, "%lx %lx", &val1, &val2) != 2)
+ return -EINVAL;
+
+ spin_lock(&drvdata->spinlock);
+
+ /*
+ * each byte[0..3] controls mask value applied to vmid
+ * comparator[0..3]
+ */
+ switch (drvdata->numvmidc) {
+ case 0x1:
+ /* COMP0, bits[7:0] */
+ drvdata->vmid_mask0 = val1 & 0xFF;
+ break;
+ case 0x2:
+ /* COMP1, bits[15:8] */
+ drvdata->vmid_mask0 = val1 & 0xFFFF;
+ break;
+ case 0x3:
+ /* COMP2, bits[23:16] */
+ drvdata->vmid_mask0 = val1 & 0xFFFFFF;
+ break;
+ case 0x4:
+ /* COMP3, bits[31:24] */
+ drvdata->vmid_mask0 = val1;
+ break;
+ case 0x5:
+ /* COMP4, bits[7:0] */
+ drvdata->vmid_mask0 = val1;
+ drvdata->vmid_mask1 = val2 & 0xFF;
+ break;
+ case 0x6:
+ /* COMP5, bits[15:8] */
+ drvdata->vmid_mask0 = val1;
+ drvdata->vmid_mask1 = val2 & 0xFFFF;
+ break;
+ case 0x7:
+ /* COMP6, bits[23:16] */
+ drvdata->vmid_mask0 = val1;
+ drvdata->vmid_mask1 = val2 & 0xFFFFFF;
+ break;
+ case 0x8:
+ /* COMP7, bits[31:24] */
+ drvdata->vmid_mask0 = val1;
+ drvdata->vmid_mask1 = val2;
+ break;
+ default:
+ break;
+ }
+
+ /*
+ * If software sets a mask bit to 1, it must program relevant byte
+ * of vmid comparator value 0x0, otherwise behavior is unpredictable.
+ * For example, if bit[3] of vmid_mask0 is 1, we must clear bits[31:24]
+ * of vmid comparator0 value (corresponding to byte 0) register.
+ */
+ mask = drvdata->vmid_mask0;
+ for (i = 0; i < drvdata->numvmidc; i++) {
+ /* mask value of corresponding vmid comparator */
+ maskbyte = mask & ETMv4_EVENT_MASK;
+ /*
+ * each bit corresponds to a byte of respective vmid comparator
+ * value register
+ */
+ for (j = 0; j < 8; j++) {
+ if (maskbyte & 1)
+ drvdata->vmid_val[i] &= ~(0xFF << (j * 8));
+ maskbyte >>= 1;
+ }
+ /* Select the next vmid comparator mask value */
+ if (i == 3)
+ /* vmid comparators[4-7] */
+ mask = drvdata->vmid_mask1;
+ else
+ mask >>= 0x8;
+ }
+ spin_unlock(&drvdata->spinlock);
+ return size;
+}
+static DEVICE_ATTR_RW(vmid_masks);
+
static ssize_t status_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
@@ -2149,6 +2324,9 @@ static struct attribute *coresight_etmv4_attrs[] = {
&dev_attr_ctxid_idx.attr,
&dev_attr_ctxid_val.attr,
&dev_attr_ctxid_masks.attr,
+ &dev_attr_vmid_idx.attr,
+ &dev_attr_vmid_val.attr,
+ &dev_attr_vmid_masks.attr,
&dev_attr_status.attr,
&dev_attr_mgmt.attr,
&dev_attr_trcidr.attr,
--
1.9.1
^ permalink raw reply related
* Re: [PATCH 2/3] uffd: Introduce the v2 API
From: Pavel Emelyanov @ 2015-04-23 6:29 UTC (permalink / raw)
To: Andrea Arcangeli
Cc: Linux Kernel Mailing List, Linux MM, Linux API, Sanidhya Kashyap
In-Reply-To: <20150421121817.GD4481@redhat.com>
On 04/21/2015 03:18 PM, Andrea Arcangeli wrote:
> On Wed, Mar 18, 2015 at 10:35:17PM +0300, Pavel Emelyanov wrote:
>> + if (!(ctx->features & UFFD_FEATURE_LONGMSG)) {
>
> If we are to use different protocols, it'd be nicer to have two
> different methods to assign to userfaultfd_fops.read that calls an
> __always_inline function, so that the above check can be optimized
> away at build time when the inline is expanded. So the branch is
> converted to calling a different pointer to function which is zero
> additional cost.
OK :)
>> + /* careful to always initialize addr if ret == 0 */
>> + __u64 uninitialized_var(addr);
>> + __u64 uninitialized_var(mtype);
>> + if (count < sizeof(addr))
>> + return ret ? ret : -EINVAL;
>> + _ret = userfaultfd_ctx_read(ctx, no_wait, &mtype, &addr);
>> + if (_ret < 0)
>> + return ret ? ret : _ret;
>> + BUG_ON(mtype != UFFD_PAGEFAULT);
>> + if (put_user(addr, (__u64 __user *) buf))
>> + return ret ? ret : -EFAULT;
>> + _ret = sizeof(addr);
>> + } else {
>> + struct uffd_v2_msg msg;
>> + if (count < sizeof(msg))
>> + return ret ? ret : -EINVAL;
>> + _ret = userfaultfd_ctx_read(ctx, no_wait, &msg.type, &msg.arg);
>> + if (_ret < 0)
>> + return ret ? ret : _ret;
>> + if (copy_to_user(buf, &msg, sizeof(msg)))
>> + return ret ? ret : -EINVAL;
>> + _ret = sizeof(msg);
>
> Reading 16bytes instead of 8bytes for each fault, probably wouldn't
> move the needle much in terms of userfaultfd_read performance. Perhaps
> we could consider using the uffd_v2_msg unconditionally and then have
> a single protocol differentiated by the feature bits.
So your proposal is to always report 16 bytes per PF from read() and
let userspace decide itself how to handle the result?
> The only reason to have two different protocols would be to be able to
> read 8 bytes per userfault, in the cooperative usage (i.e. qemu
> postcopy). But if we do that we want to use the __always_inline trick
> to avoid branches and additional runtime costs (otherwise we may as
> well forget all microoptimizations and read 16bytes always).
>
>> @@ -992,6 +1013,12 @@ static int userfaultfd_api(struct userfaultfd_ctx *ctx,
>> /* careful not to leak info, we only read the first 8 bytes */
>> uffdio_api.bits = UFFD_API_BITS;
>> uffdio_api.ioctls = UFFD_API_IOCTLS;
>> +
>> + if (uffdio_api.api == UFFD_API_V2) {
>> + ctx->features |= UFFD_FEATURE_LONGMSG;
>> + uffdio_api.bits |= UFFD_API_V2_BITS;
>> + }
>> +
>> ret = -EFAULT;
>> if (copy_to_user(buf, &uffdio_api, sizeof(uffdio_api)))
>> goto out;
>
> The original meaning of the bits is:
>
> If UFFD_BIT_WRITE was set in api.bits, it means the
> !!(address&UFFD_BIT_WRITE) tells if it was a write fault (missing or
> WP).
>
> If UFFD_BIT_WP was set in api.bits, it means the
> !!(address&UFFD_BIT_WP) tells if it was a WP fault (if not set it
> means it was a missing fault).
>
> Currently api.bits sets only UFFD_BIT_WRITE, and UFFD_BIT_WP will be
> set later, after the WP tracking mode will be implemented.
>
> I'm uncertain how bits translated to features and if they should be
> unified or only have features.
>
>> +struct uffd_v2_msg {
>> + __u64 type;
>> + __u64 arg;
>> +};
>> +
>> +#define UFFD_PAGEFAULT 0x1
>> +
>> +#define UFFD_PAGEFAULT_BIT (1 << (UFFD_PAGEFAULT - 1))
>> +#define __UFFD_API_V2_BITS (UFFD_PAGEFAULT_BIT)
>> +
>> +/*
>> + * Lower PAGE_SHIFT bits are used to report those supported
>> + * by the pagefault message itself. Other bits are used to
>> + * report the message types v2 API supports
>> + */
>> +#define UFFD_API_V2_BITS (__UFFD_API_V2_BITS << 12)
>> +
>
> And why exactly is this 12 hardcoded?
Ah, it should have been the PAGE_SHIFT one, but I was unsure whether it
would be OK to have different shifts in different arches.
But taking into account your comment that bits field id bad for these
values, if we introduce the new .features one for api message, then this
12 will just go away.
> And which field should be masked
> with the bits? In the V1 protocol it was the "arg" (userfault address)
> not the "type". So this is a bit confusing and probably requires
> simplification.
I see. Actually I decided that since bits higher than 12th (for x86) is
always 0 in api message (no bits allowed there, since pfn sits in this
place), it would be OK to put non-PF bits there.
Should I better introduce another .features field in uffd API message?
-- Pavel
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH 0/3] UserfaultFD: Extension for non cooperative uffd usage
From: Pavel Emelyanov @ 2015-04-23 6:34 UTC (permalink / raw)
To: Andrea Arcangeli
Cc: Linux Kernel Mailing List, Linux MM, Linux API, Sanidhya Kashyap,
Dr. David Alan Gilbert, Dave Hansen
In-Reply-To: <20150421120222.GC4481-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
On 04/21/2015 03:02 PM, Andrea Arcangeli wrote:
> Hi Pavel,
>
> On Wed, Mar 18, 2015 at 10:34:26PM +0300, Pavel Emelyanov wrote:
>> Hi,
>>
>> On the recent LSF Andrea presented his userfault-fd patches and
>> I had shown some issues that appear in usage scenarios when the
>> monitor task and mm task do not cooperate to each other on VM
>> changes (and fork()-s).
>>
>> Here's the implementation of the extended uffd API that would help
>> us to address those issues.
>>
>> As proof of concept I've implemented only fix for fork() case, but
>> I also plan to add the mremap() and exit() notifications, both are
>> also required for such non-cooperative usage.
>>
>> More details about the extension itself is in patch #2 and the fork()
>> notification description is in patch #3.
>>
>> Comments and suggestion are warmly welcome :)
>
> This looks feasible.
>
>> Andrea, what's the best way to go on with the patches -- would you
>> prefer to include them in your git tree or should I instead continue
>> with them on my own, re-sending them when required? Either way would
>> be OK for me.
>
> Ok so various improvements happened in userfaultfd patchset over the
> last month so your incremental patchset likely requires a rebase
> sorry. When you posted it I was in the middle of the updates. Now
> things are working stable and I have no pending updates, so it would
> be a good time for a rebase.
OK, thanks for the heads up! I will rebase my patches.
> I can merge it if you like, it's your call if you prefer to maintain
> it incrementally or if I should merge it, but I wouldn't push it to
> Andrew for upstream integration in the first batch, as this
> complicates things further and it's not fully complete yet (at least
> the version posted only handled fork). I think it can be merged
> incrementally in a second stage.
Sure!
> The major updates of the userfaultfd patchset over the last month were:
>
> 1) Various mixed fixes thanks to the feedback from Dave Hansen and
> David Gilbert.
>
> The most notable one is the use of mm_users instead of mm_count to
> pin the mm to avoid crashes that assumed the vma still existed (in
> the userfaultfd_release method and in the various ioctl). exit_mmap
> doesn't even set mm->mmap to NULL, so unless I introduce a
> userfaultfd_exit to call in mmput, I have to pin the mm_users to be
> safe. This is mainly an issue for the non-cooperative usage you're
> implementing. Can you catch the exit somehow so you can close the
> fd? The memory won't be released until you do. Is this ok with you?
> I suppose you had to close the fd somehow anyway.
>
> 2) userfaults are waken immediately even if they're not been "read"
> yet, this can lead to POLLIN false positives (so I only allow poll
> if the fd is open in nonblocking mode to be sure it won't hang). Is
> it too paranoid to return POLLERR if the fd is not open in
> nonblocking mode?
>
> http://git.kernel.org/cgit/linux/kernel/git/andrea/aa.git/commit/?h=userfault&id=f222d9de0a5302dc8ac62d6fab53a84251098751
>
> 3) optimize read to return entries in O(1) and poll which was already
> O(1) becomes lockless. This required to split the waitqueue in two,
> one for pending faults and one for non pending faults, and the
> faults are refiled across the two waitqueues when they're
> read. Both waitqueues are protected by a single lock to be simpler
> and faster at runtime (the fault_pending_wqh one).
>
> http://git.kernel.org/cgit/linux/kernel/git/andrea/aa.git/commit/?h=userfault&id=9aa033ed43a1134c2223dac8c5d9e02e0100fca1
>
> 4) Allocate the ctx with kmem_cache_alloc. This is going to collide a
> bit with your cleanup patch sorry.
>
> http://git.kernel.org/cgit/linux/kernel/git/andrea/aa.git/commit/?h=userfault&id=f5a8db16d2876eed8906a4d36f1d0e06ca5490f6
>
> 5) Originally qemu had two bitflags for each page and kept 3 states
> (of the 4 possible with two bits) for each page in order to deal
> with the races that can happen if one thread is reading the
> userfaults and another thread is calling the UFFDIO_COPY ioctl in
> the background. This patch solves all races in the kernel so the
> two bits per page can be dropped from qemu codebase. I started
> documenting the races that can materialize by using 2 threads
> (instead of running the workload single threaded with a single poll
> event loop), and how userland had to solve them until I decided it
> was simpler to fix the race in the kernel by running an ad-hoc
> pagetable walk inside the wait_event()-kind-of-section. This
> simplified qemu significantly and it doesn't make the kernel much
> more complicated.
>
> I tried this before in much older versions but I use gup_fast for
> it and it didn't work well with gup_fast for various reasons.
>
> http://git.kernel.org/cgit/linux/kernel/git/andrea/aa.git/commit/?h=userfault&id=41efeae4e93f0296436f2a9fc6b28b6b0158512a
>
> After this patch the only reason to call UFFDIO_WAKE is to handle
> the userfaults in batches in combination with the DONT_WAKE flag of
> UFFDIO_COPY.
>
> 6) I removed the read recursion from mcopy_atomic. This avoids to
> depend on the write-starvation behavior of rwsem to be safe. After
> this change the rwsem is free to stop any further down_read if
> there's a down_write waiting on the lock.
>
> http://git.kernel.org/cgit/linux/kernel/git/andrea/aa.git/commit/?h=userfault&id=b1e3a08acc9e3f6c2614e89fc3b8e338daa58e18
>
> About other troubles for the non cooperative usage: MADV_DONTNEED
> likely needs to be trapped too or how do you know that you shall map a
> zero page instead of the old data at the faulting address?
>
> Thanks,
> Andrea
> .
>
^ permalink raw reply
* Re: [RFC] simple_char: New infrastructure to simplify chardev management
From: Greg Kroah-Hartman @ 2015-04-23 8:25 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Arnd Bergmann, Jiri Kosina, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <d3ab93173d51cebf00dd2263fd0ce9f8cd6258f7.1423609645.git.luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org>
On Tue, Feb 10, 2015 at 03:44:05PM -0800, Andy Lutomirski wrote:
> This isn't adequately tested, and I don't have a demonstration (yet).
> It's here for review for whether it's a good idea in the first place
> and for weather the fully_dynamic mechanism is a good idea.
Sorry for the long delay.
I looked at this, and at a first glance, this looks good. The existing
char interface is a mess, and needs to really be simplified. I think
this code can go a long way toward making that happen.
But I'm a bit confused as to how to use this. Can you pick some
in-kernel driver and convert it to this interface to see how it would
"work"?
Ideally, between an interface like this, and the miscdevice interface,
that should be the main way to create character devices, simplifying a
lot of "boilerplate" code we have in drivers today.
Some minor comments on the code:
> + ret = -ENOMEM;
> + major = kmalloc(sizeof(struct simple_char_major), GFP_KERNEL);
> + if (!major)
> + goto out_unregister;
> + cdev_init(&major->cdev, &simple_char_fops);
> + kobject_set_name(&major->cdev.kobj, "%s", name);
The kobject in a cdev isn't a "real" kobject in that the name doesn't
matter, and it's never "registered" with sysfs. It's only use for the
kobject map code, for looking up the cdev very quickly. I really would
like to just split the kmap code out from being related to a kobject as
it's something that confuses a lot of people, but never spent the time
to do the work.
So a line like this shouldn't do anything, you don't have to set the
name here.
> +void simple_char_major_free(struct simple_char_major *major)
> +{
> + BUG_ON(!idr_is_empty(&major->idr));
WARN_ON is best, we never want to add new BUG calls to the kernel. Or,
if this really can never happen, we don't need to test for it.
thanks,
greg k-h
^ permalink raw reply
* Re: [RFC] simple_char: New infrastructure to simplify chardev management
From: David Herrmann @ 2015-04-23 9:34 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Greg Kroah-Hartman, Arnd Bergmann, Jiri Kosina, linux-kernel,
Linux API
In-Reply-To: <d3ab93173d51cebf00dd2263fd0ce9f8cd6258f7.1423609645.git.luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org>
Hi
On Wed, Feb 11, 2015 at 12:44 AM, Andy Lutomirski <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org> wrote:
> This isn't adequately tested, and I don't have a demonstration (yet).
> It's here for review for whether it's a good idea in the first place
> and for weather the fully_dynamic mechanism is a good idea.
>
> The current character device interfaces are IMO awful. There's a
> reservation mechanism (alloc_chrdev_region, etc), a bizarre
> sort-of-hashtable lookup mechanism that character drivers need to
> interact with (cdev_add, etc), and number of lookup stages on open
> with various levels of optimization. Despite all the code, there's no
> core infrastructure to map from a dev_t back to a kobject, struct
> device, or any other useful device pointer.
>
> This means that lots of drivers need to implement all of this
> themselves. The drivers don't even all seem to do it right. For
> example, the UIO code seems to be missing any protection against
> chardev open racing against device removal.
>
> On top of the complexity of writing a chardev driver, the user
> interface is odd. We have /proc/devices, which nothing should use,
> since it conveys no information about minor numbers, and minors are
> mostly dynamic these days. Even the major numbers aren't terribly
> useful, since sysfs refers to (major, minor) pairs.
>
> This adds simple helpers simple_char_minor_create and
> simple_char_minor_free to create and destroy chardev minors. Common
> code handles minor allocation and lookup and provides a simple helper
> to allow (and even mostly require!) users to reference count their
> devices correctly.
>
> Users can either allocation a traditional dynamic major using
> simple_char_major_create, or they can use a global "fully_dynamic"
> major and avoid thinking about major numbers at all.
>
> This currently does not integrate with the driver core at all.
> Users are responsible for plugging the dev_t into their struct
> devices manually. I couldn't see a clean way to fix this without
> integrating all of this into the driver core.
>
> Thoughts? I want to use this for the u2f driver, which will either be
> a chardev driver in its own right or use a simple new iso7816 class.
>
> Ideally we could convert a bunch of drivers to use this, at least
> where there are no legacy minor number considerations.
>
> (Note: simple_char users will *not* have their devicename%d indices
> match their minor numbers unless they specifically arrange for this to
> be the case. For new drivers, this shouldn't be a problem at all. I
> don't know whether it matters for old drivers.)
>
> Signed-off-by: Andy Lutomirski <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org>
> ---
>
> drivers/base/Makefile | 2 +-
> drivers/base/simple_char.c | 231 ++++++++++++++++++++++++++++++++++++++++++++
> include/linux/simple_char.h | 38 ++++++++
> 3 files changed, 270 insertions(+), 1 deletion(-)
> create mode 100644 drivers/base/simple_char.c
> create mode 100644 include/linux/simple_char.h
>
> diff --git a/drivers/base/Makefile b/drivers/base/Makefile
> index 6922cd6850a2..d3832749f74c 100644
> --- a/drivers/base/Makefile
> +++ b/drivers/base/Makefile
> @@ -4,7 +4,7 @@ obj-y := component.o core.o bus.o dd.o syscore.o \
> driver.o class.o platform.o \
> cpu.o firmware.o init.o map.o devres.o \
> attribute_container.o transport_class.o \
> - topology.o container.o
> + topology.o container.o simple_char.o
> obj-$(CONFIG_DEVTMPFS) += devtmpfs.o
> obj-$(CONFIG_DMA_CMA) += dma-contiguous.o
> obj-y += power/
> diff --git a/drivers/base/simple_char.c b/drivers/base/simple_char.c
> new file mode 100644
> index 000000000000..f3205ef9e44b
> --- /dev/null
> +++ b/drivers/base/simple_char.c
> @@ -0,0 +1,231 @@
> +/*
> + * A simple way to create character devices
> + *
> + * Copyright (c) 2015 Andy Lutomirski <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org>
> + *
> + * Loosely based, somewhat arbitrarily, on the UIO driver, which is one
> + * of many copies of essentially identical boilerplate.
> + *
> + * Licensed under the GPLv2.
> + */
> +
> +#include <linux/simple_char.h>
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/poll.h>
> +#include <linux/device.h>
> +#include <linux/slab.h>
> +#include <linux/mm.h>
> +#include <linux/idr.h>
> +#include <linux/sched.h>
> +#include <linux/string.h>
> +#include <linux/kobject.h>
> +#include <linux/cdev.h>
> +
> +#define MAX_MINORS (1U << MINORBITS)
> +
> +struct simple_char_major {
> + struct cdev cdev;
> + unsigned majornum;
> + struct idr idr;
> + struct mutex lock;
> +};
> +
> +static struct simple_char_major *fully_dynamic_major;
> +static DEFINE_MUTEX(fully_dynamic_major_lock);
> +
> +static int simple_char_open(struct inode *inode, struct file *filep)
> +{
> + struct simple_char_major *major =
> + container_of(inode->i_cdev, struct simple_char_major,
> + cdev);
> + void *private;
> + const struct simple_char_ops *ops;
> + int ret = 0;
> +
> + mutex_lock(&major->lock);
> +
> + {
> + /*
> + * This is a separate block to make the locking entirely
> + * clear. The only thing keeping minor alive is major->lock.
> + * We need to be completely done with the simple_char_minor
> + * by the time we release the lock.
> + */
> + struct simple_char_minor *minor;
> + minor = idr_find(&major->idr, iminor(inode));
> + if (!minor || !minor->ops->reference(minor->private)) {
> + mutex_unlock(&major->lock);
> + return -ENODEV;
> + }
> + private = minor->private;
> + ops = minor->ops;
> + }
> +
> + mutex_unlock(&major->lock);
> +
> + replace_fops(filep, ops->fops);
> + filep->private_data = private;
> + if (ops->fops->open)
> + ret = ops->fops->open(inode, filep);
> +
> + return ret;
> +}
> +
> +static const struct file_operations simple_char_fops = {
> + .open = simple_char_open,
> + .llseek = noop_llseek,
> +};
> +
> +struct simple_char_major *simple_char_major_create(const char *name)
> +{
> + struct simple_char_major *major = NULL;
> + dev_t devt;
> + int ret;
> +
> + ret = alloc_chrdev_region(&devt, 0, MAX_MINORS, name);
> + if (ret)
> + goto out;
> +
> + ret = -ENOMEM;
> + major = kmalloc(sizeof(struct simple_char_major), GFP_KERNEL);
> + if (!major)
> + goto out_unregister;
> + cdev_init(&major->cdev, &simple_char_fops);
> + kobject_set_name(&major->cdev.kobj, "%s", name);
> +
> + ret = cdev_add(&major->cdev, devt, MAX_MINORS);
> + if (ret)
> + goto out_free;
> +
> + major->majornum = MAJOR(devt);
> + idr_init(&major->idr);
> + return major;
> +
> +out_free:
> + cdev_del(&major->cdev);
> + kfree(major);
> +out_unregister:
> + unregister_chrdev_region(devt, MAX_MINORS);
> +out:
> + return ERR_PTR(ret);
> +}
> +EXPORT_SYMBOL(simple_char_major_create);
> +
> +void simple_char_major_free(struct simple_char_major *major)
> +{
> + BUG_ON(!idr_is_empty(&major->idr));
> +
> + cdev_del(&major->cdev);
> + unregister_chrdev_region(MKDEV(major->majornum, 0), MAX_MINORS);
> + idr_destroy(&major->idr);
> + kfree(major);
cdevs are ref-counted, you cannot free them here. See fs/char_dev.c,
it holds a cdev-ref while calling ->open(). If you unregister the
major in between, this will free "major" while ->open() is running.
Sure, this is fine for your fully-dynamic major, as it is never freed.
But it breaks for fixed majors.
Currently, the only way to get called on cdev-destruction, is to hook
into the parent device, as the last cdev_put() drops its ref to the
parent. See for instance drivers/input/evdev.c for details.
> +}
> +
> +static struct simple_char_major *get_fully_dynamic_major(void)
> +{
> + struct simple_char_major *major =
> + smp_load_acquire(&fully_dynamic_major);
> + if (major)
> + return major;
> +
> + mutex_lock(&fully_dynamic_major_lock);
> +
> + if (fully_dynamic_major) {
> + major = fully_dynamic_major;
> + goto out;
> + }
> +
> + major = simple_char_major_create("fully_dynamic");
> + if (!IS_ERR(major))
> + smp_store_release(&fully_dynamic_major, major);
> +
> +out:
> + mutex_unlock(&fully_dynamic_major_lock);
> + return major;
> +
> +}
> +
> +/**
> + * simple_char_minor_create() - create a chardev minor
> + * @major: Major to use or NULL for a fully dynamic chardev.
> + * @ops: simple_char_ops to associate with the minor.
> + * @private: opaque pointer for @ops's use.
> + *
> + * simple_char_minor_create() creates a minor chardev. For new code,
> + * @major should be NULL; this will create a minor chardev with fully
> + * dynamic major and minor numbers and without a useful name in
> + * /proc/devices. (All recent user code should be using sysfs
> + * exclusively to map between devices and device numbers.) For legacy
> + * code, @major can come from simple_char_major_create().
> + *
> + * The chardev will use @ops->fops for its file operations. Before any
> + * of those operations are called, the struct file's private_data will
> + * be set to @private.
> + *
> + * To simplify reference counting, @ops->reference will be called before
> + * @ops->fops->open. @ops->reference should take any needed references
> + * and return true if the object being opened still exists, and it
> + * should return false without taking references if the object is dying.
> + * @ops->reference is called with locks held, so it should neither sleep
> + * nor take heavy locks.
> + *
> + * @ops->fops->release (and @ops->fops->open, if it exists and fails)
> + * are responsible for releasing any references takes by @ops->reference.
> + *
> + * The minor must be destroyed by @simple_char_minor_free. After
> + * @simple_char_minor_free returns, @ops->reference will not be called.
> + */
> +struct simple_char_minor *
> +simple_char_minor_create(struct simple_char_major *major,
> + const struct simple_char_ops *ops,
> + void *private)
> +{
> + int ret;
> + struct simple_char_minor *minor = NULL;
> +
> + if (!major) {
> + major = get_fully_dynamic_major();
> + if (IS_ERR(major))
> + return (void *)major;
> + }
> +
> + minor = kmalloc(sizeof(struct simple_char_minor), GFP_KERNEL);
> + if (!minor)
> + return ERR_PTR(-ENOMEM);
> +
> + mutex_lock(&major->lock);
> + ret = idr_alloc(&major->idr, minor, 0, MAX_MINORS, GFP_KERNEL);
> + if (ret >= 0) {
> + minor->devt = MKDEV(major->majornum, ret);
> + ret = 0;
> + }
> + /* Warn on ENOSPC? It's embarrassing if it ever happens. */
> + mutex_unlock(&major->lock);
> +
> + if (ret) {
> + kfree(minor);
> + return ERR_PTR(ret);
> + }
> +
> + minor->major = major;
> + minor->private = private;
> + minor->ops = ops;
These assignments need to be underneath major->lock (like the
minor->devt assignment). Otherwise, open() might run before this, and
fault on minor->ops->reference, as ops is uninitialized.
> + return minor;
> +}
> +
> +/**
> + * simple_char_minor_free() - Free a simple_char chardev minor
> + * @minor: the minor to free.
> + *
> + * This frees a chardev minor and prevents that minor's @ops->reference
> + * op from being called in the future.
> + */
> +void simple_char_minor_free(struct simple_char_minor *minor)
> +{
> + mutex_lock(&minor->major->lock);
> + idr_remove(&minor->major->idr, MINOR(minor->devt));
> + mutex_unlock(&minor->major->lock);
> + kfree(minor);
> +}
> +EXPORT_SYMBOL(simple_char_minor_free);
> diff --git a/include/linux/simple_char.h b/include/linux/simple_char.h
> new file mode 100644
> index 000000000000..8f391e7b50af
> --- /dev/null
> +++ b/include/linux/simple_char.h
> @@ -0,0 +1,38 @@
> +/*
> + * A simple way to create character devices
> + *
> + * Copyright (c) 2015 Andy Lutomirski <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org>
> + *
> + * Licensed under the GPLv2.
> + */
> +
> +#include <linux/types.h>
> +#include <linux/kobject.h>
> +#include <linux/file.h>
> +
> +struct simple_char_major;
> +
> +struct simple_char_ops {
> + bool (*reference)(void *private);
> + const struct file_operations *fops;
> +};
> +
> +struct simple_char_minor {
> + struct simple_char_major *major;
> + const struct simple_char_ops *ops;
> + void *private;
> + dev_t devt;
> +};
> +
> +extern struct simple_char_minor *
> +simple_char_minor_create(struct simple_char_major *major,
> + const struct simple_char_ops *ops,
> + void *private);
> +extern void simple_char_minor_free(struct simple_char_minor *minor);
> +
> +extern void simple_char_file_release(struct file *filep, struct kobject *kobj);
Leftover? I cannot see the definition of this function.
Thanks
David
> +
> +/* These exist only to support legacy classes that need their own major. */
> +extern struct simple_char_major *simple_char_major_create(const char *name);
> +extern void simple_char_major_free(struct simple_char_major *major);
> +
> --
> 2.1.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
^ permalink raw reply
* Re: [Patch v2 2/3] firmware: dmi_scan: add SBMIOS entry and DMI tables
From: Jean Delvare @ 2015-04-23 11:33 UTC (permalink / raw)
To: Ivan.khoronzhuk
Cc: linux-kernel, matt.fleming, ard.biesheuvel, grant.likely,
linux-api, linux-doc, mikew, dmidecode-devel, leif.lindholm,
msalter, roy.franz
In-Reply-To: <553674D2.10407@globallogic.com>
Le Tuesday 21 April 2015 à 19:03 +0300, Ivan.khoronzhuk a écrit :
> On 21.04.15 18:36, Jean Delvare wrote:
> > I just found that more work is needed here for the SMBIOS v3 entry
> > point case. These entry points do not specify the exact length of the
> > table, but only its maximum. The real world sample I have access to
> > indeed specifies a maximum length of 6419 bytes, but the actual table
> > only spans over 2373 bytes. It is properly terminated with a type 127
> > DMI structure, so the kernel table parser ignores the garbage after it.
> > The garbage is however exported to user-space above.
> >
> > I taught dmidecode to ignore the garbage, but there are two problem
> > left here. First problem is a waste of memory. Minor issue I suppose,
> > who cares about a few kilobytes these days.
> >
> > Second problem is a security problem. We are leaking the contents of
> > physical memory to user-space. In my case it's filled with 0xffs so no
> > big deal. But what if actual data happens to be stored there? It
> > definitely shouldn't go to user-space.
> >
> > So dmi_len needs to be trimmed to the actual table size before the
> > attribute above is created. I have an idea how this could be
> > implemented easily, let me give it a try.
> >
> > Maybe we should trim the length for previous implementations, too.
> > There is no reason to walk past a type 127 structure anyway, ever.
> >
>
> It can happen of-cause, I've also thought about that sometime ago,
> but forget...).
> I've sent the updated series already.
Got it, reviewed and ready to push upstream. I'll do so as soon as I'm
done with other duties.
> Let me know when your fix will be ready and I will re-base the
> series if it has conflicts.
Don't worry, I'm quite good at manual conflict resolution :)
Thanks,
--
Jean Delvare
SUSE L3 Support
^ permalink raw reply
* Re: [PATCH v2 3/4] of: overlay: Add sysfs attributes
From: Pantelis Antoniou @ 2015-04-23 12:00 UTC (permalink / raw)
To: Rob Herring
Cc: Grant Likely, Andrew Morton, Matt Porter, Koen Kooi,
Guenter Roeck, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <CAL_JsqLMVBfQpmSjg9qfOvtM3RXMY3Dm+knN8h54gDCzx513-g-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
Hi Rob,
> On Apr 15, 2015, at 04:27 , Rob Herring <robherring2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>
> On Tue, Apr 7, 2015 at 2:23 PM, Pantelis Antoniou
> <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org> wrote:
>> Implement a number of sysfs attributes for overlays.
>>
>> * A throw once master enable switch to protect against any
>> further overlay applications if the administrator desires so.
>
> This one should be a separate patch.
>
OK.
>> * A per overlay targets sysfs attribute listing the targets of
>> the installed overlay.
>
> What are targets? "targets lists targets" does not help me. The
> documentation doesn't help me either.
>
It lists the targets of the overlay that has been applied. What do
you need in order to be helped? I mean what do you want listed?
>> * A per overlay can_remove sysfs attribute that reports whether
>> the overlay can be removed or not due to another overlapping overlay.
>>
>> Signed-off-by: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
>> ---
>> drivers/of/overlay.c | 167 ++++++++++++++++++++++++++++++++++++++++++++++++++-
>> 1 file changed, 166 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
>> index f17f5ef..c54d097 100644
>> --- a/drivers/of/overlay.c
>> +++ b/drivers/of/overlay.c
>> @@ -21,6 +21,7 @@
>> #include <linux/err.h>
>> #include <linux/idr.h>
>> #include <linux/sysfs.h>
>> +#include <linux/atomic.h>
>>
>> #include "of_private.h"
>>
>> @@ -55,8 +56,12 @@ struct of_overlay {
>> struct kobject kobj;
>> };
>>
>> +/* master enable switch; once set to 0 can't be re-enabled */
>> +static atomic_t ov_enable = ATOMIC_INIT(1);
>> +
>> static int of_overlay_apply_one(struct of_overlay *ov,
>> struct device_node *target, const struct device_node *overlay);
>> +static int overlay_removal_is_ok(struct of_overlay *ov);
>>
>> static int of_overlay_apply_single_property(struct of_overlay *ov,
>> struct device_node *target, struct property *prop)
>> @@ -345,6 +350,144 @@ static struct kobj_type of_overlay_ktype = {
>>
>> static struct kset *ov_kset;
>>
>> +static ssize_t enable_read(struct file *filp, struct kobject *kobj,
>> + struct bin_attribute *bin_attr, char *buf,
>> + loff_t offset, size_t count)
>> +{
>> + char tbuf[3];
>> +
>> + if (offset < 0)
>> + return -EINVAL;
>> +
>> + if (offset >= sizeof(tbuf))
>> + return 0;
>> +
>> + if (count > sizeof(tbuf) - offset)
>> + count = sizeof(tbuf) - offset;
>> +
>> + /* fill in temp */
>> + tbuf[0] = '0' + atomic_read(&ov_enable);
>> + tbuf[1] = '\n';
>> + tbuf[2] = '\0';
>> +
>> + /* copy to buffer */
>> + memcpy(buf, tbuf + offset, count);
>> +
>> + return count;
>> +}
>> +
>> +static ssize_t enable_write(struct file *filp, struct kobject *kobj,
>> + struct bin_attribute *bin_attr, char *buf,
>> + loff_t off, size_t count)
>> +{
>> + unsigned int new_enable;
>> +
>> + if (off != 0 || (buf[0] != '0' && buf[0] != '1'))
>> + return -EINVAL;
>> +
>> + new_enable = (unsigned int)(buf[0] - '0');
>> + if (new_enable > 1)
>> + return -EINVAL;
>> +
>> + /* NOP for same value */
>> + if (new_enable == atomic_read(&ov_enable))
>> + return count;
>> +
>> + /* if we've disabled it, no going back */
>> + if (atomic_read(&ov_enable) == 0)
>> + return -EPERM;
>> +
>> + atomic_set(&ov_enable, new_enable);
>> + return count;
>> +}
>> +
>> +/* just a single char + '\n' + '\0' */
>> +static BIN_ATTR_RW(enable, 3);
>
> Why are you using bin attribute? You are complicating the
> implementation needlessly.
>
It’s the same reason that the device tree core is using it.
Believe it or not, this is the simplest way to do it.
If you take a look at the sysfs attribute implementation, the binary
implementation is the one that’s using the least amount of code.
To use a non-binary method we have to register per ktype sysfs_ops
and duplicate the way the non-binary attribute works.
For the gory details look at sysfs_add_file_mode_ns() in fs/sysfs/file.c
I can add the sysfs_ops but that’s going to be more complicated not less.
>> +
>> +static ssize_t targets_read(struct file *filp, struct kobject *kobj,
>> + struct bin_attribute *bin_attr, char *buf,
>> + loff_t offset, size_t count)
>> +{
>> + struct of_overlay *ov = kobj_to_overlay(kobj);
>> + struct of_overlay_info *ovinfo;
>> + char *tmpbuf, *s, *e;
>> + const char *name;
>> + ssize_t ret;
>> + int i, len;
>> +
>> + /* allocate work buffer; we know that PAGE_SIZE is enough */
>> + tmpbuf = kmalloc(PAGE_SIZE, GFP_KERNEL);
>> + if (tmpbuf == NULL)
>> + return -ENOMEM;
>> +
>> + s = tmpbuf;
>> + e = tmpbuf + PAGE_SIZE;
>> +
>> + mutex_lock(&of_mutex);
>> +
>> + /* targets */
>> + for (i = 0; i < ov->count; i++) {
>> + ovinfo = &ov->ovinfo_tab[i];
>> +
>> + name = of_node_full_name(ovinfo->target);
>> + len = strlen(name);
>> + if (s + len + 1 >= e)
>> + return -ENOMEM;
>
> Leaking memory here and holding the mutex.
>
OK
>> + memcpy(s, name, len);
>> + s += len;
>> + *s++ = '\n';
>> + }
>> + if (s + 1 >= e)
>> + return -ENOMEM;
> And here.
>
OK
>> + *s++ = '\0';
>> +
>> + /* the buffer is zero terminated */
>> + len = s - tmpbuf;
>> +
>> + mutex_unlock(&of_mutex);
>> +
>> + /* perform the read */
>> + ret = memory_read_from_buffer(buf, count, &offset, tmpbuf, len);
>> +
>> + /* free the temporary buffer */
>> + kfree(tmpbuf);
>> +
>> + return ret;
>> +}
>> +
>> +/* targets property */
>> +static BIN_ATTR_RO(targets, PAGE_SIZE);
>> +
>> +static ssize_t can_remove_read(struct file *filp, struct kobject *kobj,
>> + struct bin_attribute *bin_attr, char *buf,
>> + loff_t offset, size_t count)
>> +{
>> + struct of_overlay *ov = kobj_to_overlay(kobj);
>> + char tbuf[3];
>> +
>> + if (offset < 0)
>> + return -EINVAL;
>> +
>> + if (offset >= sizeof(tbuf))
>> + return 0;
>> +
>> + if (count > sizeof(tbuf) - offset)
>> + count = sizeof(tbuf) - offset;
>> +
>> + /* fill in temp */
>> + tbuf[0] = '0' + overlay_removal_is_ok(ov);
>> + tbuf[1] = '\n';
>> + tbuf[2] = '\0';
>> +
>> + /* copy to buffer */
>> + memcpy(buf, tbuf + offset, count);
>> +
>> + return count;
>> +}
>> +
>> +/* can_remove property */
>> +static BIN_ATTR_RO(can_remove, 3);
>
> Same question about bin attr here.
>
Same answer as above.
>> +
>> /**
>> * of_overlay_create() - Create and apply an overlay
>> * @tree: Device node containing all the overlays
>> @@ -360,6 +503,10 @@ int of_overlay_create(struct device_node *tree)
>> struct of_overlay *ov;
>> int err, id;
>>
>> + /* administratively disabled */
>> + if (!atomic_read(&ov_enable))
>> + return -EPERM;
>> +
>> /* allocate the overlay structure */
>> ov = kzalloc(sizeof(*ov), GFP_KERNEL);
>> if (ov == NULL)
>> @@ -416,6 +563,22 @@ int of_overlay_create(struct device_node *tree)
>> goto err_cancel_overlay;
>> }
>>
>> + /* create targets file */
>> + err = sysfs_create_bin_file(&ov->kobj, &bin_attr_targets);
>> + if (err != 0) {
>> + pr_err("%s: sysfs_create_bin_file() failed for tree@%s\n",
>> + __func__, tree->full_name);
>> + goto err_cancel_overlay;
>> + }
>> +
>> + /* create can_remove file */
>> + err = sysfs_create_bin_file(&ov->kobj, &bin_attr_can_remove);
>> + if (err != 0) {
>> + pr_err("%s: sysfs_create_bin_file() failed for tree@%s\n",
>> + __func__, tree->full_name);
>> + goto err_cancel_overlay;
>> + }
>> +
>> /* add to the tail of the overlay list */
>> list_add_tail(&ov->node, &ov_list);
>>
>> @@ -596,5 +759,7 @@ int of_overlay_init(void)
>> if (!ov_kset)
>> return -ENOMEM;
>>
>> - return 0;
>> + rc = sysfs_create_bin_file(&ov_kset->kobj, &bin_attr_enable);
>> + WARN(rc, "%s: error adding enable attribute\n", __func__);
>> + return rc;
>> }
>> --
>> 1.7.12
Regards
— Pantelis
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v2 3/4] of: overlay: Add sysfs attributes
From: Greg KH @ 2015-04-23 12:33 UTC (permalink / raw)
To: Pantelis Antoniou
Cc: Rob Herring, Grant Likely, Andrew Morton, Matt Porter, Koen Kooi,
Guenter Roeck, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <6E391B8F-936A-4FD1-B8CC-C9B5FCE0B291-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
On Thu, Apr 23, 2015 at 03:00:03PM +0300, Pantelis Antoniou wrote:
> Hi Rob,
>
> > On Apr 15, 2015, at 04:27 , Rob Herring <robherring2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> >
> > On Tue, Apr 7, 2015 at 2:23 PM, Pantelis Antoniou
> > <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org> wrote:
> >> Implement a number of sysfs attributes for overlays.
> >>
> >> * A throw once master enable switch to protect against any
> >> further overlay applications if the administrator desires so.
> >
> > This one should be a separate patch.
> >
>
> OK.
>
> >> * A per overlay targets sysfs attribute listing the targets of
> >> the installed overlay.
> >
> > What are targets? "targets lists targets" does not help me. The
> > documentation doesn't help me either.
> >
>
> It lists the targets of the overlay that has been applied. What do
> you need in order to be helped? I mean what do you want listed?
>
> >> * A per overlay can_remove sysfs attribute that reports whether
> >> the overlay can be removed or not due to another overlapping overlay.
> >>
> >> Signed-off-by: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
> >> ---
> >> drivers/of/overlay.c | 167 ++++++++++++++++++++++++++++++++++++++++++++++++++-
> >> 1 file changed, 166 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
> >> index f17f5ef..c54d097 100644
> >> --- a/drivers/of/overlay.c
> >> +++ b/drivers/of/overlay.c
> >> @@ -21,6 +21,7 @@
> >> #include <linux/err.h>
> >> #include <linux/idr.h>
> >> #include <linux/sysfs.h>
> >> +#include <linux/atomic.h>
> >>
> >> #include "of_private.h"
> >>
> >> @@ -55,8 +56,12 @@ struct of_overlay {
> >> struct kobject kobj;
> >> };
> >>
> >> +/* master enable switch; once set to 0 can't be re-enabled */
> >> +static atomic_t ov_enable = ATOMIC_INIT(1);
> >> +
> >> static int of_overlay_apply_one(struct of_overlay *ov,
> >> struct device_node *target, const struct device_node *overlay);
> >> +static int overlay_removal_is_ok(struct of_overlay *ov);
> >>
> >> static int of_overlay_apply_single_property(struct of_overlay *ov,
> >> struct device_node *target, struct property *prop)
> >> @@ -345,6 +350,144 @@ static struct kobj_type of_overlay_ktype = {
> >>
> >> static struct kset *ov_kset;
> >>
> >> +static ssize_t enable_read(struct file *filp, struct kobject *kobj,
> >> + struct bin_attribute *bin_attr, char *buf,
> >> + loff_t offset, size_t count)
> >> +{
> >> + char tbuf[3];
> >> +
> >> + if (offset < 0)
> >> + return -EINVAL;
> >> +
> >> + if (offset >= sizeof(tbuf))
> >> + return 0;
> >> +
> >> + if (count > sizeof(tbuf) - offset)
> >> + count = sizeof(tbuf) - offset;
> >> +
> >> + /* fill in temp */
> >> + tbuf[0] = '0' + atomic_read(&ov_enable);
> >> + tbuf[1] = '\n';
> >> + tbuf[2] = '\0';
> >> +
> >> + /* copy to buffer */
> >> + memcpy(buf, tbuf + offset, count);
> >> +
> >> + return count;
> >> +}
> >> +
> >> +static ssize_t enable_write(struct file *filp, struct kobject *kobj,
> >> + struct bin_attribute *bin_attr, char *buf,
> >> + loff_t off, size_t count)
> >> +{
> >> + unsigned int new_enable;
> >> +
> >> + if (off != 0 || (buf[0] != '0' && buf[0] != '1'))
> >> + return -EINVAL;
> >> +
> >> + new_enable = (unsigned int)(buf[0] - '0');
> >> + if (new_enable > 1)
> >> + return -EINVAL;
> >> +
> >> + /* NOP for same value */
> >> + if (new_enable == atomic_read(&ov_enable))
> >> + return count;
> >> +
> >> + /* if we've disabled it, no going back */
> >> + if (atomic_read(&ov_enable) == 0)
> >> + return -EPERM;
> >> +
> >> + atomic_set(&ov_enable, new_enable);
> >> + return count;
> >> +}
> >> +
> >> +/* just a single char + '\n' + '\0' */
> >> +static BIN_ATTR_RW(enable, 3);
> >
> > Why are you using bin attribute? You are complicating the
> > implementation needlessly.
> >
>
> It’s the same reason that the device tree core is using it.
It is doing that for "raw" device tree files, not individual attributes,
right?
> Believe it or not, this is the simplest way to do it.
> If you take a look at the sysfs attribute implementation, the binary
> implementation is the one that’s using the least amount of code.
Then something is really wrong here.
> To use a non-binary method we have to register per ktype sysfs_ops
> and duplicate the way the non-binary attribute works.
really? Again, something must be wrong.
> For the gory details look at sysfs_add_file_mode_ns() in fs/sysfs/file.c
>
> I can add the sysfs_ops but that’s going to be more complicated not less.
Only use binary sysfs files if you are accepting binary data directly
from userspace and using it as a "pass-through" to the kernel.
Otherwise just use a "normal" sysfs file. I don't understand the
problem here, sysfs shouldn't be hard to use for simple attributes, that
was not the goal here at all.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH v2 3/4] of: overlay: Add sysfs attributes
From: Pantelis Antoniou @ 2015-04-23 12:39 UTC (permalink / raw)
To: Greg KH
Cc: Rob Herring, Grant Likely, Andrew Morton, Matt Porter, Koen Kooi,
Guenter Roeck, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-api@vger.kernel.org
In-Reply-To: <20150423123346.GA19166@kroah.com>
Hi Greg,
> On Apr 23, 2015, at 15:33 , Greg KH <greg@kroah.com> wrote:
>
> On Thu, Apr 23, 2015 at 03:00:03PM +0300, Pantelis Antoniou wrote:
>> Hi Rob,
>>
>>> On Apr 15, 2015, at 04:27 , Rob Herring <robherring2@gmail.com> wrote:
>>>
>>> On Tue, Apr 7, 2015 at 2:23 PM, Pantelis Antoniou
>>> <pantelis.antoniou@konsulko.com> wrote:
>>>> Implement a number of sysfs attributes for overlays.
>>>>
>>>> * A throw once master enable switch to protect against any
>>>> further overlay applications if the administrator desires so.
>>>
>>> This one should be a separate patch.
>>>
>>
>> OK.
>>
>>>> * A per overlay targets sysfs attribute listing the targets of
>>>> the installed overlay.
>>>
>>> What are targets? "targets lists targets" does not help me. The
>>> documentation doesn't help me either.
>>>
>>
>> It lists the targets of the overlay that has been applied. What do
>> you need in order to be helped? I mean what do you want listed?
>>
>>>> * A per overlay can_remove sysfs attribute that reports whether
>>>> the overlay can be removed or not due to another overlapping overlay.
>>>>
>>>> Signed-off-by: Pantelis Antoniou <pantelis.antoniou@konsulko.com>
>>>> ---
>>>> drivers/of/overlay.c | 167 ++++++++++++++++++++++++++++++++++++++++++++++++++-
>>>> 1 file changed, 166 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
>>>> index f17f5ef..c54d097 100644
>>>> --- a/drivers/of/overlay.c
>>>> +++ b/drivers/of/overlay.c
>>>> @@ -21,6 +21,7 @@
>>>> #include <linux/err.h>
>>>> #include <linux/idr.h>
>>>> #include <linux/sysfs.h>
>>>> +#include <linux/atomic.h>
>>>>
>>>> #include "of_private.h"
>>>>
>>>> @@ -55,8 +56,12 @@ struct of_overlay {
>>>> struct kobject kobj;
>>>> };
>>>>
>>>> +/* master enable switch; once set to 0 can't be re-enabled */
>>>> +static atomic_t ov_enable = ATOMIC_INIT(1);
>>>> +
>>>> static int of_overlay_apply_one(struct of_overlay *ov,
>>>> struct device_node *target, const struct device_node *overlay);
>>>> +static int overlay_removal_is_ok(struct of_overlay *ov);
>>>>
>>>> static int of_overlay_apply_single_property(struct of_overlay *ov,
>>>> struct device_node *target, struct property *prop)
>>>> @@ -345,6 +350,144 @@ static struct kobj_type of_overlay_ktype = {
>>>>
>>>> static struct kset *ov_kset;
>>>>
>>>> +static ssize_t enable_read(struct file *filp, struct kobject *kobj,
>>>> + struct bin_attribute *bin_attr, char *buf,
>>>> + loff_t offset, size_t count)
>>>> +{
>>>> + char tbuf[3];
>>>> +
>>>> + if (offset < 0)
>>>> + return -EINVAL;
>>>> +
>>>> + if (offset >= sizeof(tbuf))
>>>> + return 0;
>>>> +
>>>> + if (count > sizeof(tbuf) - offset)
>>>> + count = sizeof(tbuf) - offset;
>>>> +
>>>> + /* fill in temp */
>>>> + tbuf[0] = '0' + atomic_read(&ov_enable);
>>>> + tbuf[1] = '\n';
>>>> + tbuf[2] = '\0';
>>>> +
>>>> + /* copy to buffer */
>>>> + memcpy(buf, tbuf + offset, count);
>>>> +
>>>> + return count;
>>>> +}
>>>> +
>>>> +static ssize_t enable_write(struct file *filp, struct kobject *kobj,
>>>> + struct bin_attribute *bin_attr, char *buf,
>>>> + loff_t off, size_t count)
>>>> +{
>>>> + unsigned int new_enable;
>>>> +
>>>> + if (off != 0 || (buf[0] != '0' && buf[0] != '1'))
>>>> + return -EINVAL;
>>>> +
>>>> + new_enable = (unsigned int)(buf[0] - '0');
>>>> + if (new_enable > 1)
>>>> + return -EINVAL;
>>>> +
>>>> + /* NOP for same value */
>>>> + if (new_enable == atomic_read(&ov_enable))
>>>> + return count;
>>>> +
>>>> + /* if we've disabled it, no going back */
>>>> + if (atomic_read(&ov_enable) == 0)
>>>> + return -EPERM;
>>>> +
>>>> + atomic_set(&ov_enable, new_enable);
>>>> + return count;
>>>> +}
>>>> +
>>>> +/* just a single char + '\n' + '\0' */
>>>> +static BIN_ATTR_RW(enable, 3);
>>>
>>> Why are you using bin attribute? You are complicating the
>>> implementation needlessly.
>>>
>>
>> It’s the same reason that the device tree core is using it.
>
> It is doing that for "raw" device tree files, not individual attributes,
> right?
>
Each property of a device tree is a binary attribute.
>> Believe it or not, this is the simplest way to do it.
>> If you take a look at the sysfs attribute implementation, the binary
>> implementation is the one that’s using the least amount of code.
>
> Then something is really wrong here.
>
>> To use a non-binary method we have to register per ktype sysfs_ops
>> and duplicate the way the non-binary attribute works.
>
> really? Again, something must be wrong.
>
>> For the gory details look at sysfs_add_file_mode_ns() in fs/sysfs/file.c
>>
>> I can add the sysfs_ops but that’s going to be more complicated not less.
>
Please take a look in linux/sysfs.h.
The non-binary sysfs accessors are all using some kind of other kobj;
for instance DEVICE_ATTR is using a device_attribute, etc.
For the overlay case, I’d have to create a of_overlay_attribute and work from
there.
> Only use binary sysfs files if you are accepting binary data directly
> from userspace and using it as a "pass-through" to the kernel.
>
> Otherwise just use a "normal" sysfs file. I don't understand the
> problem here, sysfs shouldn't be hard to use for simple attributes, that
> was not the goal here at all.
>
There is no generic (i.e. not kobj type specific), non-binary sysfs file interface right now.
I can add one for my case, but that’s more code.
It’s your call.
> thanks,
>
> greg k-h
Regards
— Pantelis
^ permalink raw reply
* Re: [PATCH v2 3/4] of: overlay: Add sysfs attributes
From: Greg KH @ 2015-04-23 12:54 UTC (permalink / raw)
To: Pantelis Antoniou
Cc: Rob Herring, Grant Likely, Andrew Morton, Matt Porter, Koen Kooi,
Guenter Roeck, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <98237E9E-04CB-4CD5-8B2F-63F070B54459-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
On Thu, Apr 23, 2015 at 03:39:21PM +0300, Pantelis Antoniou wrote:
> Hi Greg,
>
> > On Apr 23, 2015, at 15:33 , Greg KH <greg-U8xfFu+wG4EAvxtiuMwx3w@public.gmane.org> wrote:
> >
> > On Thu, Apr 23, 2015 at 03:00:03PM +0300, Pantelis Antoniou wrote:
> >> Hi Rob,
> >>
> >>> On Apr 15, 2015, at 04:27 , Rob Herring <robherring2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> >>>
> >>> On Tue, Apr 7, 2015 at 2:23 PM, Pantelis Antoniou
> >>> <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org> wrote:
> >>>> Implement a number of sysfs attributes for overlays.
> >>>>
> >>>> * A throw once master enable switch to protect against any
> >>>> further overlay applications if the administrator desires so.
> >>>
> >>> This one should be a separate patch.
> >>>
> >>
> >> OK.
> >>
> >>>> * A per overlay targets sysfs attribute listing the targets of
> >>>> the installed overlay.
> >>>
> >>> What are targets? "targets lists targets" does not help me. The
> >>> documentation doesn't help me either.
> >>>
> >>
> >> It lists the targets of the overlay that has been applied. What do
> >> you need in order to be helped? I mean what do you want listed?
> >>
> >>>> * A per overlay can_remove sysfs attribute that reports whether
> >>>> the overlay can be removed or not due to another overlapping overlay.
> >>>>
> >>>> Signed-off-by: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
> >>>> ---
> >>>> drivers/of/overlay.c | 167 ++++++++++++++++++++++++++++++++++++++++++++++++++-
> >>>> 1 file changed, 166 insertions(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
> >>>> index f17f5ef..c54d097 100644
> >>>> --- a/drivers/of/overlay.c
> >>>> +++ b/drivers/of/overlay.c
> >>>> @@ -21,6 +21,7 @@
> >>>> #include <linux/err.h>
> >>>> #include <linux/idr.h>
> >>>> #include <linux/sysfs.h>
> >>>> +#include <linux/atomic.h>
> >>>>
> >>>> #include "of_private.h"
> >>>>
> >>>> @@ -55,8 +56,12 @@ struct of_overlay {
> >>>> struct kobject kobj;
> >>>> };
> >>>>
> >>>> +/* master enable switch; once set to 0 can't be re-enabled */
> >>>> +static atomic_t ov_enable = ATOMIC_INIT(1);
> >>>> +
> >>>> static int of_overlay_apply_one(struct of_overlay *ov,
> >>>> struct device_node *target, const struct device_node *overlay);
> >>>> +static int overlay_removal_is_ok(struct of_overlay *ov);
> >>>>
> >>>> static int of_overlay_apply_single_property(struct of_overlay *ov,
> >>>> struct device_node *target, struct property *prop)
> >>>> @@ -345,6 +350,144 @@ static struct kobj_type of_overlay_ktype = {
> >>>>
> >>>> static struct kset *ov_kset;
> >>>>
> >>>> +static ssize_t enable_read(struct file *filp, struct kobject *kobj,
> >>>> + struct bin_attribute *bin_attr, char *buf,
> >>>> + loff_t offset, size_t count)
> >>>> +{
> >>>> + char tbuf[3];
> >>>> +
> >>>> + if (offset < 0)
> >>>> + return -EINVAL;
> >>>> +
> >>>> + if (offset >= sizeof(tbuf))
> >>>> + return 0;
> >>>> +
> >>>> + if (count > sizeof(tbuf) - offset)
> >>>> + count = sizeof(tbuf) - offset;
> >>>> +
> >>>> + /* fill in temp */
> >>>> + tbuf[0] = '0' + atomic_read(&ov_enable);
> >>>> + tbuf[1] = '\n';
> >>>> + tbuf[2] = '\0';
> >>>> +
> >>>> + /* copy to buffer */
> >>>> + memcpy(buf, tbuf + offset, count);
> >>>> +
> >>>> + return count;
> >>>> +}
> >>>> +
> >>>> +static ssize_t enable_write(struct file *filp, struct kobject *kobj,
> >>>> + struct bin_attribute *bin_attr, char *buf,
> >>>> + loff_t off, size_t count)
> >>>> +{
> >>>> + unsigned int new_enable;
> >>>> +
> >>>> + if (off != 0 || (buf[0] != '0' && buf[0] != '1'))
> >>>> + return -EINVAL;
> >>>> +
> >>>> + new_enable = (unsigned int)(buf[0] - '0');
> >>>> + if (new_enable > 1)
> >>>> + return -EINVAL;
> >>>> +
> >>>> + /* NOP for same value */
> >>>> + if (new_enable == atomic_read(&ov_enable))
> >>>> + return count;
> >>>> +
> >>>> + /* if we've disabled it, no going back */
> >>>> + if (atomic_read(&ov_enable) == 0)
> >>>> + return -EPERM;
> >>>> +
> >>>> + atomic_set(&ov_enable, new_enable);
> >>>> + return count;
> >>>> +}
> >>>> +
> >>>> +/* just a single char + '\n' + '\0' */
> >>>> +static BIN_ATTR_RW(enable, 3);
> >>>
> >>> Why are you using bin attribute? You are complicating the
> >>> implementation needlessly.
> >>>
> >>
> >> It’s the same reason that the device tree core is using it.
> >
> > It is doing that for "raw" device tree files, not individual attributes,
> > right?
> >
>
> Each property of a device tree is a binary attribute.
Because they export binary data, right? I don't have access to a
machine that uses device tree at the moment to check this...
Any specific file/function you are referring to?
> >> Believe it or not, this is the simplest way to do it.
> >> If you take a look at the sysfs attribute implementation, the binary
> >> implementation is the one that’s using the least amount of code.
> >
> > Then something is really wrong here.
> >
> >> To use a non-binary method we have to register per ktype sysfs_ops
> >> and duplicate the way the non-binary attribute works.
> >
> > really? Again, something must be wrong.
> >
> >> For the gory details look at sysfs_add_file_mode_ns() in fs/sysfs/file.c
> >>
> >> I can add the sysfs_ops but that’s going to be more complicated not less.
> >
>
> Please take a look in linux/sysfs.h.
> The non-binary sysfs accessors are all using some kind of other kobj;
> for instance DEVICE_ATTR is using a device_attribute, etc.
>
> For the overlay case, I’d have to create a of_overlay_attribute and work from
> there.
Yes, that is what you should be doing here as well.
That's just the model we have to work with, the uses of "raw" kobjects
are very limited, so it does take a bit more wrapper code to use them,
sorry.
You need access to the kobject anyway, which is why you need to provide
a type of attribute function, so that you get the right kobject.
Or just use an attribute group, would that be simpler? If you have more
than one sysfs file, that's usually the best way to do things.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH v2 3/4] of: overlay: Add sysfs attributes
From: Pantelis Antoniou @ 2015-04-23 12:59 UTC (permalink / raw)
To: Greg KH
Cc: Rob Herring, Grant Likely, Andrew Morton, Matt Porter, Koen Kooi,
Guenter Roeck, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <20150423125441.GA32303-U8xfFu+wG4EAvxtiuMwx3w@public.gmane.org>
Hi Greg,
> On Apr 23, 2015, at 15:54 , Greg KH <greg-U8xfFu+wG4EAvxtiuMwx3w@public.gmane.org> wrote:
>
> On Thu, Apr 23, 2015 at 03:39:21PM +0300, Pantelis Antoniou wrote:
>> Hi Greg,
>>
>>> On Apr 23, 2015, at 15:33 , Greg KH <greg-U8xfFu+wG4EAvxtiuMwx3w@public.gmane.org> wrote:
>>>
>>> On Thu, Apr 23, 2015 at 03:00:03PM +0300, Pantelis Antoniou wrote:
>>>> Hi Rob,
>>>>
>>>>> On Apr 15, 2015, at 04:27 , Rob Herring <robherring2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>>>>
>>>>> On Tue, Apr 7, 2015 at 2:23 PM, Pantelis Antoniou
>>>>> <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org> wrote:
>>>>>> Implement a number of sysfs attributes for overlays.
>>>>>>
>>>>>> * A throw once master enable switch to protect against any
>>>>>> further overlay applications if the administrator desires so.
>>>>>
>>>>> This one should be a separate patch.
>>>>>
>>>>
>>>> OK.
>>>>
>>>>>> * A per overlay targets sysfs attribute listing the targets of
>>>>>> the installed overlay.
>>>>>
>>>>> What are targets? "targets lists targets" does not help me. The
>>>>> documentation doesn't help me either.
>>>>>
>>>>
>>>> It lists the targets of the overlay that has been applied. What do
>>>> you need in order to be helped? I mean what do you want listed?
>>>>
>>>>>> * A per overlay can_remove sysfs attribute that reports whether
>>>>>> the overlay can be removed or not due to another overlapping overlay.
>>>>>>
>>>>>> Signed-off-by: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
>>>>>> ---
>>>>>> drivers/of/overlay.c | 167 ++++++++++++++++++++++++++++++++++++++++++++++++++-
>>>>>> 1 file changed, 166 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
>>>>>> index f17f5ef..c54d097 100644
>>>>>> --- a/drivers/of/overlay.c
>>>>>> +++ b/drivers/of/overlay.c
>>>>>> @@ -21,6 +21,7 @@
>>>>>> #include <linux/err.h>
>>>>>> #include <linux/idr.h>
>>>>>> #include <linux/sysfs.h>
>>>>>> +#include <linux/atomic.h>
>>>>>>
>>>>>> #include "of_private.h"
>>>>>>
>>>>>> @@ -55,8 +56,12 @@ struct of_overlay {
>>>>>> struct kobject kobj;
>>>>>> };
>>>>>>
>>>>>> +/* master enable switch; once set to 0 can't be re-enabled */
>>>>>> +static atomic_t ov_enable = ATOMIC_INIT(1);
>>>>>> +
>>>>>> static int of_overlay_apply_one(struct of_overlay *ov,
>>>>>> struct device_node *target, const struct device_node *overlay);
>>>>>> +static int overlay_removal_is_ok(struct of_overlay *ov);
>>>>>>
>>>>>> static int of_overlay_apply_single_property(struct of_overlay *ov,
>>>>>> struct device_node *target, struct property *prop)
>>>>>> @@ -345,6 +350,144 @@ static struct kobj_type of_overlay_ktype = {
>>>>>>
>>>>>> static struct kset *ov_kset;
>>>>>>
>>>>>> +static ssize_t enable_read(struct file *filp, struct kobject *kobj,
>>>>>> + struct bin_attribute *bin_attr, char *buf,
>>>>>> + loff_t offset, size_t count)
>>>>>> +{
>>>>>> + char tbuf[3];
>>>>>> +
>>>>>> + if (offset < 0)
>>>>>> + return -EINVAL;
>>>>>> +
>>>>>> + if (offset >= sizeof(tbuf))
>>>>>> + return 0;
>>>>>> +
>>>>>> + if (count > sizeof(tbuf) - offset)
>>>>>> + count = sizeof(tbuf) - offset;
>>>>>> +
>>>>>> + /* fill in temp */
>>>>>> + tbuf[0] = '0' + atomic_read(&ov_enable);
>>>>>> + tbuf[1] = '\n';
>>>>>> + tbuf[2] = '\0';
>>>>>> +
>>>>>> + /* copy to buffer */
>>>>>> + memcpy(buf, tbuf + offset, count);
>>>>>> +
>>>>>> + return count;
>>>>>> +}
>>>>>> +
>>>>>> +static ssize_t enable_write(struct file *filp, struct kobject *kobj,
>>>>>> + struct bin_attribute *bin_attr, char *buf,
>>>>>> + loff_t off, size_t count)
>>>>>> +{
>>>>>> + unsigned int new_enable;
>>>>>> +
>>>>>> + if (off != 0 || (buf[0] != '0' && buf[0] != '1'))
>>>>>> + return -EINVAL;
>>>>>> +
>>>>>> + new_enable = (unsigned int)(buf[0] - '0');
>>>>>> + if (new_enable > 1)
>>>>>> + return -EINVAL;
>>>>>> +
>>>>>> + /* NOP for same value */
>>>>>> + if (new_enable == atomic_read(&ov_enable))
>>>>>> + return count;
>>>>>> +
>>>>>> + /* if we've disabled it, no going back */
>>>>>> + if (atomic_read(&ov_enable) == 0)
>>>>>> + return -EPERM;
>>>>>> +
>>>>>> + atomic_set(&ov_enable, new_enable);
>>>>>> + return count;
>>>>>> +}
>>>>>> +
>>>>>> +/* just a single char + '\n' + '\0' */
>>>>>> +static BIN_ATTR_RW(enable, 3);
>>>>>
>>>>> Why are you using bin attribute? You are complicating the
>>>>> implementation needlessly.
>>>>>
>>>>
>>>> It’s the same reason that the device tree core is using it.
>>>
>>> It is doing that for "raw" device tree files, not individual attributes,
>>> right?
>>>
>>
>> Each property of a device tree is a binary attribute.
>
> Because they export binary data, right? I don't have access to a
> machine that uses device tree at the moment to check this...
>
> Any specific file/function you are referring to?
>
Yes, they export binary data. It works because the device tree nodes
are raw kobjs.
>>>> Believe it or not, this is the simplest way to do it.
>>>> If you take a look at the sysfs attribute implementation, the binary
>>>> implementation is the one that’s using the least amount of code.
>>>
>>> Then something is really wrong here.
>>>
>>>> To use a non-binary method we have to register per ktype sysfs_ops
>>>> and duplicate the way the non-binary attribute works.
>>>
>>> really? Again, something must be wrong.
>>>
>>>> For the gory details look at sysfs_add_file_mode_ns() in fs/sysfs/file.c
>>>>
>>>> I can add the sysfs_ops but that’s going to be more complicated not less.
>>>
>>
>> Please take a look in linux/sysfs.h.
>> The non-binary sysfs accessors are all using some kind of other kobj;
>> for instance DEVICE_ATTR is using a device_attribute, etc.
>>
>> For the overlay case, I’d have to create a of_overlay_attribute and work from
>> there.
>
> Yes, that is what you should be doing here as well.
>
> That's just the model we have to work with, the uses of "raw" kobjects
> are very limited, so it does take a bit more wrapper code to use them,
> sorry.
>
That’s fine, I can work with this. I was trying to avoid creating overlay
attributes but...
> You need access to the kobject anyway, which is why you need to provide
> a type of attribute function, so that you get the right kobject.
>
> Or just use an attribute group, would that be simpler? If you have more
> than one sysfs file, that's usually the best way to do things.
>
Attribute groups might work, but I have some more work to do to get them to
work.
Thanks for answering definitively this.
> thanks,
>
> greg k-h
Regards
— Pantelis
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v2 3/4] of: overlay: Add sysfs attributes
From: Greg KH @ 2015-04-23 13:04 UTC (permalink / raw)
To: Pantelis Antoniou
Cc: Rob Herring, Grant Likely, Andrew Morton, Matt Porter, Koen Kooi,
Guenter Roeck, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-api@vger.kernel.org
In-Reply-To: <5F0BA9FB-8345-4296-8595-0E4CD1C816AA@konsulko.com>
On Thu, Apr 23, 2015 at 03:59:02PM +0300, Pantelis Antoniou wrote:
> >>>> It’s the same reason that the device tree core is using it.
> >>>
> >>> It is doing that for "raw" device tree files, not individual attributes,
> >>> right?
> >>>
> >>
> >> Each property of a device tree is a binary attribute.
> >
> > Because they export binary data, right? I don't have access to a
> > machine that uses device tree at the moment to check this...
> >
> > Any specific file/function you are referring to?
> >
>
> Yes, they export binary data. It works because the device tree nodes
> are raw kobjs.
Great, then the device tree is doing this correctly :)
For text files, use real attribute files, that's the proper way to
handle sysfs from kobjects.
thanks,
greg k-h
^ permalink raw reply
* Re: [RFC] capabilities: Ambient capabilities
From: Christoph Lameter @ 2015-04-23 14:01 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Jarkko Sakkinen, Andrew Lutomirski, Ted Ts'o, Andrew Morton,
Andrew G. Morgan, Linux API, Mimi Zohar, Michael Kerrisk,
Austin S Hemmelgarn, linux-security-module, Aaron Jones,
Serge Hallyn, LKML, Markku Savela, Kees Cook, Jonathan Corbet
In-Reply-To: <alpine.DEB.2.11.1504091024540.13650-gkYfJU5Cukgdnm+yROfE0A@public.gmane.org>
On Thu, 9 Apr 2015, Christoph Lameter wrote:
> > I'll submit a new version this week with the securebits. Sorry for the delay.
> Are we going to get a new version?
Replying to my own here. Cant we simply use the SETPCAP approach as per
the patch I posted?
^ permalink raw reply
* Re: [PATCH 10/11] coresight-etm4x: Controls pertaining to the context ID functions
From: Christopher Covington @ 2015-04-23 15:08 UTC (permalink / raw)
To: Mathieu Poirier
Cc: gregkh, linux-arm-kernel, linux-api, linux-kernel, kaixu.xia,
zhang.chunyan
In-Reply-To: <1429742451-11465-11-git-send-email-mathieu.poirier@linaro.org>
Hi Mathieu,
On 04/22/2015 06:40 PM, Mathieu Poirier wrote:
> From: Pratik Patel <pratikp@codeaurora.org>
>
> Adding sysfs entries to access and configure specifics about the
> context ID comparator functions.
>
> Signed-off-by: Pratik Patel <pratikp@codeaurora.org>
> Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
> ---
> .../ABI/testing/sysfs-bus-coresight-devices-etm4x | 19 +++
> drivers/hwtracing/coresight/coresight-etm4x.c | 187 +++++++++++++++++++++
> 2 files changed, 206 insertions(+)
>
> diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices-etm4x b/Documentation/ABI/testing/sysfs-bus-coresight-devices-etm4x
> index b5c0456290ab..8b32cb7b9723 100644
> --- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-etm4x
> +++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-etm4x
> @@ -254,3 +254,22 @@ Date: April 2015
> KernelVersion: 4.01
> Contact: Mathieu Poirier <mathieu.poirier@linaro.org>
> Description: (RW) Controls the selection of the resources in the trace unit.
> +
> +What: /sys/bus/coresight/devices/<memory_map>.etm/ctxid_idx
> +Date: April 2015
> +KernelVersion: 4.01
> +Contact: Mathieu Poirier <mathieu.poirier@linaro.org>
> +Description: (RW) Select which context ID comparator to work with.
> +
> +What: /sys/bus/coresight/devices/<memory_map>.etm/ctxid_val
> +Date: April 2015
> +KernelVersion: 4.01
> +Contact: Mathieu Poirier <mathieu.poirier@linaro.org>
> +Description: (RW) Get/Set the context ID comparator value to trigger on.
If I'm understanding this correctly, ctxid_val is multiplexed using ctxid_idx.
Why not just have ctxid_val0, ctxid_val1, ...?
Thanks,
Chris
--
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* Re: [PATCH 10/11] coresight-etm4x: Controls pertaining to the context ID functions
From: Mathieu Poirier @ 2015-04-23 15:16 UTC (permalink / raw)
To: Christopher Covington
Cc: Greg KH,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Kaixu Xia,
Chunyan Zhang
In-Reply-To: <55390B05.1050801-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
On 23 April 2015 at 09:08, Christopher Covington <cov-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> wrote:
> Hi Mathieu,
>
> On 04/22/2015 06:40 PM, Mathieu Poirier wrote:
>> From: Pratik Patel <pratikp-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
>>
>> Adding sysfs entries to access and configure specifics about the
>> context ID comparator functions.
>>
>> Signed-off-by: Pratik Patel <pratikp-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
>> Signed-off-by: Mathieu Poirier <mathieu.poirier-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>> ---
>> .../ABI/testing/sysfs-bus-coresight-devices-etm4x | 19 +++
>> drivers/hwtracing/coresight/coresight-etm4x.c | 187 +++++++++++++++++++++
>> 2 files changed, 206 insertions(+)
>>
>> diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices-etm4x b/Documentation/ABI/testing/sysfs-bus-coresight-devices-etm4x
>> index b5c0456290ab..8b32cb7b9723 100644
>> --- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-etm4x
>> +++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-etm4x
>> @@ -254,3 +254,22 @@ Date: April 2015
>> KernelVersion: 4.01
>> Contact: Mathieu Poirier <mathieu.poirier-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>> Description: (RW) Controls the selection of the resources in the trace unit.
>> +
>> +What: /sys/bus/coresight/devices/<memory_map>.etm/ctxid_idx
>> +Date: April 2015
>> +KernelVersion: 4.01
>> +Contact: Mathieu Poirier <mathieu.poirier-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>> +Description: (RW) Select which context ID comparator to work with.
>> +
>> +What: /sys/bus/coresight/devices/<memory_map>.etm/ctxid_val
>> +Date: April 2015
>> +KernelVersion: 4.01
>> +Contact: Mathieu Poirier <mathieu.poirier-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>> +Description: (RW) Get/Set the context ID comparator value to trigger on.
>
> If I'm understanding this correctly, ctxid_val is multiplexed using ctxid_idx.
> Why not just have ctxid_val0, ctxid_val1, ...?
There can be up to 8 of them on the current implementation and who
knows how many more in the future. To me using and index to select
the context ID to work on scaled better and is introducing less
clutter in sysfs.
Thanks for the review,
Mathieu
>
> Thanks,
> Chris
>
> --
> Qualcomm Innovation Center, Inc.
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
^ permalink raw reply
* Re: [PATCH 01/11] coresight-etm4x: Adding CoreSight ETM4x driver
From: Christopher Covington @ 2015-04-23 15:17 UTC (permalink / raw)
To: Mathieu Poirier
Cc: gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
kaixu.xia-QSEj5FYQhm4dnm+yROfE0A,
zhang.chunyan-QSEj5FYQhm4dnm+yROfE0A
In-Reply-To: <1429742451-11465-2-git-send-email-mathieu.poirier-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
On 04/22/2015 06:40 PM, Mathieu Poirier wrote:
> From: Pratik Patel <pratikp-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
>
> This driver manages the CoreSight ETMv4 (Embedded Trace Macrocell) IP block
> to support HW assisted tracing on ARMv7 and ARMv8 architectures.
>
> Signed-off-by: Pratik Patel <pratikp-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> Signed-off-by: Kaixu Xia <xiakaixu-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
> Signed-off-by: Mathieu Poirier <mathieu.poirier-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
> .../ABI/testing/sysfs-bus-coresight-devices-etm4x | 28 +
> drivers/hwtracing/coresight/Kconfig | 10 +
> drivers/hwtracing/coresight/Makefile | 1 +
> drivers/hwtracing/coresight/coresight-etm4x.c | 833 +++++++++++++++++++++
> drivers/hwtracing/coresight/coresight-etm4x.h | 391 ++++++++++
> 5 files changed, 1263 insertions(+)
> create mode 100644 Documentation/ABI/testing/sysfs-bus-coresight-devices-etm4x
> create mode 100644 drivers/hwtracing/coresight/coresight-etm4x.c
> create mode 100644 drivers/hwtracing/coresight/coresight-etm4x.h
>
> diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices-etm4x b/Documentation/ABI/testing/sysfs-bus-coresight-devices-etm4x
> new file mode 100644
> index 000000000000..a4b623871ca0
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-etm4x
> @@ -0,0 +1,28 @@
> +What: /sys/bus/coresight/devices/<memory_map>.etm/enable_source
> +Date: April 2015
> +KernelVersion: 4.01
> +Contact: Mathieu Poirier <mathieu.poirier-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> +Description: (RW) Enable/disable tracing on this specific trace entiry.
^
entry
> + Enabling a source implies the source has been configured
> + properly and a sink has been identidifed for it. The path
^
identified
--
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* [PATCH v5 0/8] vhost: support for cross endian guests
From: Greg Kurz @ 2015-04-23 15:26 UTC (permalink / raw)
To: Rusty Russell, Michael S. Tsirkin
Cc: linux-api, linux-kernel, kvm, virtualization
Hi,
This patchset allows vhost to be used with legacy virtio when guest and host
have a different endianness. It is compatible with modern virtio and can be
fully compiled out through kernel config.
FWIW, I could flawlessly kexec/reboot guests from ppc64 to ppc64le and back.
I could also migrate from a ppc64 to a ppc64le host and back. No regressions
on x86 as expected. My experimental QEMU tree is here:
https://github.com/gkurz/qemu.git vhost/cross-endian
I'd be glad if this series could make it to 4.1.
Cheers.
---
Greg Kurz (8):
virtio: introduce virtio_is_little_endian() helper
tun: add tun_is_little_endian() helper
macvtap: introduce macvtap_is_little_endian() helper
vringh: introduce vringh_is_little_endian() helper
vhost: introduce vhost_is_little_endian() helper
virtio: add explicit big-endian support to memory accessors
vhost: cross-endian support for legacy devices
macvtap/tun: cross-endian support for little-endian hosts
drivers/net/Kconfig | 14 ++++++
drivers/net/macvtap.c | 68 +++++++++++++++++++++++++++++-
drivers/net/tun.c | 70 ++++++++++++++++++++++++++++++-
drivers/vhost/Kconfig | 15 +++++++
drivers/vhost/vhost.c | 86 ++++++++++++++++++++++++++++++++++++++
drivers/vhost/vhost.h | 25 ++++++++---
include/linux/virtio_byteorder.h | 24 ++++++-----
include/linux/virtio_config.h | 20 ++++++---
include/linux/vringh.h | 17 +++++---
include/uapi/linux/if_tun.h | 6 +++
include/uapi/linux/vhost.h | 12 +++++
11 files changed, 324 insertions(+), 33 deletions(-)
--
Greg
^ permalink raw reply
* [PATCH v5 1/8] virtio: introduce virtio_is_little_endian() helper
From: Greg Kurz @ 2015-04-23 15:26 UTC (permalink / raw)
To: Rusty Russell, Michael S. Tsirkin
Cc: linux-api, linux-kernel, kvm, virtualization
In-Reply-To: <20150423152608.11795.4373.stgit@bahia.local>
Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
---
include/linux/virtio_config.h | 17 +++++++++++------
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
index ca3ed78..bd1a582 100644
--- a/include/linux/virtio_config.h
+++ b/include/linux/virtio_config.h
@@ -205,35 +205,40 @@ int virtqueue_set_affinity(struct virtqueue *vq, int cpu)
return 0;
}
+static inline bool virtio_is_little_endian(struct virtio_device *vdev)
+{
+ return virtio_has_feature(vdev, VIRTIO_F_VERSION_1);
+}
+
/* Memory accessors */
static inline u16 virtio16_to_cpu(struct virtio_device *vdev, __virtio16 val)
{
- return __virtio16_to_cpu(virtio_has_feature(vdev, VIRTIO_F_VERSION_1), val);
+ return __virtio16_to_cpu(virtio_is_little_endian(vdev), val);
}
static inline __virtio16 cpu_to_virtio16(struct virtio_device *vdev, u16 val)
{
- return __cpu_to_virtio16(virtio_has_feature(vdev, VIRTIO_F_VERSION_1), val);
+ return __cpu_to_virtio16(virtio_is_little_endian(vdev), val);
}
static inline u32 virtio32_to_cpu(struct virtio_device *vdev, __virtio32 val)
{
- return __virtio32_to_cpu(virtio_has_feature(vdev, VIRTIO_F_VERSION_1), val);
+ return __virtio32_to_cpu(virtio_is_little_endian(vdev), val);
}
static inline __virtio32 cpu_to_virtio32(struct virtio_device *vdev, u32 val)
{
- return __cpu_to_virtio32(virtio_has_feature(vdev, VIRTIO_F_VERSION_1), val);
+ return __cpu_to_virtio32(virtio_is_little_endian(vdev), val);
}
static inline u64 virtio64_to_cpu(struct virtio_device *vdev, __virtio64 val)
{
- return __virtio64_to_cpu(virtio_has_feature(vdev, VIRTIO_F_VERSION_1), val);
+ return __virtio64_to_cpu(virtio_is_little_endian(vdev), val);
}
static inline __virtio64 cpu_to_virtio64(struct virtio_device *vdev, u64 val)
{
- return __cpu_to_virtio64(virtio_has_feature(vdev, VIRTIO_F_VERSION_1), val);
+ return __cpu_to_virtio64(virtio_is_little_endian(vdev), val);
}
/* Config space accessors. */
^ permalink raw reply related
* [PATCH v5 2/8] tun: add tun_is_little_endian() helper
From: Greg Kurz @ 2015-04-23 15:26 UTC (permalink / raw)
To: Rusty Russell, Michael S. Tsirkin
Cc: linux-api, linux-kernel, kvm, virtualization
In-Reply-To: <20150423152608.11795.4373.stgit@bahia.local>
Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
---
drivers/net/tun.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index 857dca4..3c3d6c0 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -206,14 +206,19 @@ struct tun_struct {
u32 flow_count;
};
+static inline bool tun_is_little_endian(struct tun_struct *tun)
+{
+ return tun->flags & TUN_VNET_LE;
+}
+
static inline u16 tun16_to_cpu(struct tun_struct *tun, __virtio16 val)
{
- return __virtio16_to_cpu(tun->flags & TUN_VNET_LE, val);
+ return __virtio16_to_cpu(tun_is_little_endian(tun), val);
}
static inline __virtio16 cpu_to_tun16(struct tun_struct *tun, u16 val)
{
- return __cpu_to_virtio16(tun->flags & TUN_VNET_LE, val);
+ return __cpu_to_virtio16(tun_is_little_endian(tun), val);
}
static inline u32 tun_hashfn(u32 rxhash)
^ permalink raw reply related
* [PATCH v5 3/8] macvtap: introduce macvtap_is_little_endian() helper
From: Greg Kurz @ 2015-04-23 15:26 UTC (permalink / raw)
To: Rusty Russell, Michael S. Tsirkin
Cc: linux-api, linux-kernel, kvm, virtualization
In-Reply-To: <20150423152608.11795.4373.stgit@bahia.local>
Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
---
drivers/net/macvtap.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
index 27ecc5c..a2f2958 100644
--- a/drivers/net/macvtap.c
+++ b/drivers/net/macvtap.c
@@ -49,14 +49,19 @@ struct macvtap_queue {
#define MACVTAP_VNET_LE 0x80000000
+static inline bool macvtap_is_little_endian(struct macvtap_queue *q)
+{
+ return q->flags & MACVTAP_VNET_LE;
+}
+
static inline u16 macvtap16_to_cpu(struct macvtap_queue *q, __virtio16 val)
{
- return __virtio16_to_cpu(q->flags & MACVTAP_VNET_LE, val);
+ return __virtio16_to_cpu(macvtap_is_little_endian(q), val);
}
static inline __virtio16 cpu_to_macvtap16(struct macvtap_queue *q, u16 val)
{
- return __cpu_to_virtio16(q->flags & MACVTAP_VNET_LE, val);
+ return __cpu_to_virtio16(macvtap_is_little_endian(q), val);
}
static struct proto macvtap_proto = {
^ permalink raw reply related
* [PATCH v5 4/8] vringh: introduce vringh_is_little_endian() helper
From: Greg Kurz @ 2015-04-23 15:26 UTC (permalink / raw)
To: Rusty Russell, Michael S. Tsirkin
Cc: linux-api, linux-kernel, kvm, virtualization
In-Reply-To: <20150423152608.11795.4373.stgit@bahia.local>
Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
---
include/linux/vringh.h | 17 +++++++++++------
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/include/linux/vringh.h b/include/linux/vringh.h
index a3fa537..3ed62ef 100644
--- a/include/linux/vringh.h
+++ b/include/linux/vringh.h
@@ -226,33 +226,38 @@ static inline void vringh_notify(struct vringh *vrh)
vrh->notify(vrh);
}
+static inline bool vringh_is_little_endian(const struct vringh *vrh)
+{
+ return vrh->little_endian;
+}
+
static inline u16 vringh16_to_cpu(const struct vringh *vrh, __virtio16 val)
{
- return __virtio16_to_cpu(vrh->little_endian, val);
+ return __virtio16_to_cpu(vringh_is_little_endian(vrh), val);
}
static inline __virtio16 cpu_to_vringh16(const struct vringh *vrh, u16 val)
{
- return __cpu_to_virtio16(vrh->little_endian, val);
+ return __cpu_to_virtio16(vringh_is_little_endian(vrh), val);
}
static inline u32 vringh32_to_cpu(const struct vringh *vrh, __virtio32 val)
{
- return __virtio32_to_cpu(vrh->little_endian, val);
+ return __virtio32_to_cpu(vringh_is_little_endian(vrh), val);
}
static inline __virtio32 cpu_to_vringh32(const struct vringh *vrh, u32 val)
{
- return __cpu_to_virtio32(vrh->little_endian, val);
+ return __cpu_to_virtio32(vringh_is_little_endian(vrh), val);
}
static inline u64 vringh64_to_cpu(const struct vringh *vrh, __virtio64 val)
{
- return __virtio64_to_cpu(vrh->little_endian, val);
+ return __virtio64_to_cpu(vringh_is_little_endian(vrh), val);
}
static inline __virtio64 cpu_to_vringh64(const struct vringh *vrh, u64 val)
{
- return __cpu_to_virtio64(vrh->little_endian, val);
+ return __cpu_to_virtio64(vringh_is_little_endian(vrh), val);
}
#endif /* _LINUX_VRINGH_H */
^ 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