* [PATCH V2 4/6] ARM: Make PID_IN_CONTEXTIDR incompatible with PID_NS
From: Will Deacon @ 2014-01-24 16:43 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1390581656-16372-5-git-send-email-adrienverge@gmail.com>
On Fri, Jan 24, 2014 at 04:40:54PM +0000, Adrien Verg? wrote:
> When using namespaces, different processes can have the same PID.
> It makes no sense to store a PID value in the Context ID register
> to track a specific process, when others share the same value.
>
> Consequently, PID_IN_CONTEXTIDR (which is used for tracing and
> debugging processes) should not be compatible with PID_NS.
>
> Signed-off-by: Adrien Verg? <adrienverge@gmail.com>
> ---
> arch/arm/Kconfig.debug | 2 +-
> arch/arm64/Kconfig.debug | 1 +
> 2 files changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug
> index 5765abf..ed46748 100644
> --- a/arch/arm/Kconfig.debug
> +++ b/arch/arm/Kconfig.debug
> @@ -1143,7 +1143,7 @@ config ARM_KPROBES_TEST
>
> config PID_IN_CONTEXTIDR
> bool "Write the current PID to the CONTEXTIDR register"
> - depends on CPU_COPY_V6
> + depends on CPU_COPY_V6 && !PID_NS
> help
> Enabling this option causes the kernel to write the current PID to
> the PROCID field of the CONTEXTIDR register, at the expense of some
> diff --git a/arch/arm64/Kconfig.debug b/arch/arm64/Kconfig.debug
> index 835c559..06b2633b 100644
> --- a/arch/arm64/Kconfig.debug
> +++ b/arch/arm64/Kconfig.debug
> @@ -17,6 +17,7 @@ config EARLY_PRINTK
>
> config PID_IN_CONTEXTIDR
> bool "Write the current PID to the CONTEXTIDR register"
> + depends on !PID_NS
> help
> Enabling this option causes the kernel to write the current PID to
> the CONTEXTIDR register, at the expense of some additional
Are you sure about this? The value we write is actually task_pid_nr, which I
believe to be globally unique.
Will
^ permalink raw reply
* [PATCH V2 6/6] ARM CoreSight: ETM: Allocate a trace buffer only when necessary
From: Adrien Vergé @ 2014-01-24 16:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1390581656-16372-1-git-send-email-adrienverge@gmail.com>
Some user applications try to retrieve trace data often, there
is no need to call vmalloc() when there is 0 byte to fetch.
Signed-off-by: Adrien Verg? <adrienverge@gmail.com>
---
arch/arm/kernel/etm.c | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/arch/arm/kernel/etm.c b/arch/arm/kernel/etm.c
index 54b5128..bc97240 100644
--- a/arch/arm/kernel/etm.c
+++ b/arch/arm/kernel/etm.c
@@ -309,7 +309,7 @@ static ssize_t etb_read(struct file *file, char __user *data,
long length;
struct tracectx *t = file->private_data;
u32 first = 0;
- u32 *buf;
+ u32 *buf = NULL;
mutex_lock(&t->mutex);
@@ -327,12 +327,14 @@ static ssize_t etb_read(struct file *file, char __user *data,
etb_writel(t, first, ETBR_READADDR);
length = min(total * 4, (int)len);
- buf = vmalloc(length);
+ if (length != 0)
+ buf = vmalloc(length);
dev_dbg(t->dev, "ETB buffer length: %d\n", total);
dev_dbg(t->dev, "ETB status reg: %x\n", etb_readl(t, ETBR_STATUS));
- for (i = 0; i < length / 4; i++)
- buf[i] = etb_readl(t, ETBR_READMEM);
+ if (buf)
+ for (i = 0; i < length / 4; i++)
+ buf[i] = etb_readl(t, ETBR_READMEM);
/* the only way to deassert overflow bit in ETB status is this */
etb_writel(t, 1, ETBR_CTRL);
@@ -345,7 +347,8 @@ static ssize_t etb_read(struct file *file, char __user *data,
etb_lock(t);
length -= copy_to_user(data, buf, length);
- vfree(buf);
+ if (buf)
+ vfree(buf);
out:
mutex_unlock(&t->mutex);
--
1.8.5.2
^ permalink raw reply related
* [PATCH V2 5/6] ARM CoreSight: ETM: Add PID control support
From: Adrien Vergé @ 2014-01-24 16:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1390581656-16372-1-git-send-email-adrienverge@gmail.com>
In the same manner as for enabling tracing, an entry is created in
sysfs to set the PID that triggers tracing. This change is effective
only if CONFIG_PID_IN_CONTEXTIDR is set, and thus is not compatible
with PID namespaces.
Signed-off-by: Adrien Verg? <adrienverge@gmail.com>
---
arch/arm/include/asm/hardware/coresight.h | 5 ++
arch/arm/kernel/etm.c | 82 ++++++++++++++++++++++++++++---
2 files changed, 80 insertions(+), 7 deletions(-)
diff --git a/arch/arm/include/asm/hardware/coresight.h b/arch/arm/include/asm/hardware/coresight.h
index 8c50cf6..2051af0 100644
--- a/arch/arm/include/asm/hardware/coresight.h
+++ b/arch/arm/include/asm/hardware/coresight.h
@@ -98,6 +98,11 @@
#define ETMR_ADDRCOMP_VAL(x) (0x40 + (x) * 4)
#define ETMR_ADDRCOMP_ACC_TYPE(x) (0x80 + (x) * 4)
+#ifdef CONFIG_PID_IN_CONTEXTIDR
+#define ETMR_CTXIDCOMP_VAL(x) (0x1b0 + (x) * 4)
+#define ETMR_CTXIDCOMP_MASK (0x1bc)
+#endif
+
/* ETM status register, "ETM Architecture", 3.3.2 */
#define ETMR_STATUS (0x10)
#define ETMST_OVERFLOW BIT(0)
diff --git a/arch/arm/kernel/etm.c b/arch/arm/kernel/etm.c
index 4442e5c..54b5128 100644
--- a/arch/arm/kernel/etm.c
+++ b/arch/arm/kernel/etm.c
@@ -40,12 +40,14 @@ struct tracectx {
void __iomem *etm_regs;
unsigned long flags;
int naddrcmppairs;
+ int nctxidcmp;
int etm_portsz;
struct device *dev;
struct clk *emu_clk;
struct mutex mutex;
unsigned long addrrange_start;
unsigned long addrrange_end;
+ int pid;
};
static struct tracectx tracer;
@@ -59,14 +61,18 @@ static inline bool trace_isrunning(struct tracectx *t)
* Setups ETM to trace only when:
* - address between start and end
* or address not between start and end (if exclude)
+ * - in user-space when process id equals pid,
+ * in kernel-space (if pid == 0),
+ * always (if pid == -1)
* - trace executed instructions
* or trace loads and stores (if data)
*/
-static int etm_setup_address_range(struct tracectx *t, int n,
- unsigned long start, unsigned long end, int exclude, int data)
+static int etm_setup(struct tracectx *t, int n,
+ unsigned long start, unsigned long end, int exclude,
+ int pid,
+ int data)
{
- u32 flags = ETMAAT_ARM | ETMAAT_IGNCONTEXTID | ETMAAT_NSONLY | \
- ETMAAT_NOVALCMP;
+ u32 flags = ETMAAT_ARM | ETMAAT_NSONLY | ETMAAT_NOVALCMP;
if (n < 1 || n > t->naddrcmppairs)
return -EINVAL;
@@ -75,6 +81,23 @@ static int etm_setup_address_range(struct tracectx *t, int n,
* to bits in a word */
n--;
+#ifdef CONFIG_PID_IN_CONTEXTIDR
+ if (pid < 0) {
+ /* ignore Context ID */
+ flags |= ETMAAT_IGNCONTEXTID;
+ } else {
+ flags |= ETMAAT_VALUE1;
+
+ /* Set up the first Context ID comparator.
+ Process ID is found in the 24 first bits of Context ID
+ (provided by CONFIG_PID_IN_CONTEXTIDR) */
+ etm_writel(t, pid << 8, ETMR_CTXIDCOMP_VAL(0));
+ etm_writel(t, 0xff, ETMR_CTXIDCOMP_MASK);
+ }
+#else
+ flags |= ETMAAT_IGNCONTEXTID;
+#endif
+
if (data)
flags |= ETMAAT_DLOADSTORE;
else
@@ -124,8 +147,10 @@ static int trace_start(struct tracectx *t)
return -EFAULT;
}
- etm_setup_address_range(t, 1, t->addrrange_start, t->addrrange_end,
- 0, 0);
+ etm_setup(t, 1,
+ t->addrrange_start, t->addrrange_end, 0,
+ t->pid,
+ 0);
etm_writel(t, 0, ETMR_TRACEENCTRL2);
etm_writel(t, 0, ETMR_TRACESSCTRL);
etm_writel(t, 0x6f, ETMR_TRACEENEVT);
@@ -486,6 +511,7 @@ static ssize_t trace_info_show(struct device *dev,
return sprintf(buf, "Trace buffer len: %d\n"
"Addr comparator pairs: %d\n"
+ "Ctx ID comparators: %d\n"
"ETBR_WRITEADDR:\t%08x\n"
"ETBR_READADDR:\t%08x\n"
"ETBR_STATUS:\t%08x\n"
@@ -494,6 +520,7 @@ static ssize_t trace_info_show(struct device *dev,
"ETMR_STATUS:\t%08x\n",
datalen,
tracer.naddrcmppairs,
+ tracer.nctxidcmp,
etb_wa,
etb_ra,
etb_st,
@@ -566,6 +593,36 @@ static ssize_t trace_addrrange_store(struct device *dev,
DEVICE_ATTR(trace_addrrange, S_IRUGO|S_IWUSR,
trace_addrrange_show, trace_addrrange_store);
+#ifdef CONFIG_PID_IN_CONTEXTIDR
+static ssize_t trace_pid_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ return sprintf(buf, "%d\n", tracer.pid);
+}
+
+static ssize_t trace_pid_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t n)
+{
+ int pid;
+
+ if (tracer.flags & TRACER_RUNNING)
+ return -EBUSY;
+
+ if (sscanf(buf, "%i", &pid) != 1)
+ return -EINVAL;
+
+ mutex_lock(&tracer.mutex);
+ tracer.pid = pid;
+ mutex_unlock(&tracer.mutex);
+
+ return n;
+}
+
+DEVICE_ATTR(trace_pid, S_IRUGO|S_IWUSR, trace_pid_show, trace_pid_store);
+#endif
+
static int etm_probe(struct amba_device *dev, const struct amba_id *id)
{
struct tracectx *t = &tracer;
@@ -595,6 +652,7 @@ static int etm_probe(struct amba_device *dev, const struct amba_id *id)
t->etm_portsz = 1;
t->addrrange_start = (unsigned long) _stext;
t->addrrange_end = (unsigned long) _etext;
+ t->pid = -1; /* trace everything */
etm_unlock(t);
(void)etm_readl(t, ETMMR_PDSR);
@@ -602,6 +660,7 @@ static int etm_probe(struct amba_device *dev, const struct amba_id *id)
(void)etm_readl(&tracer, ETMMR_OSSRR);
t->naddrcmppairs = etm_readl(t, ETMR_CONFCODE) & 0xf;
+ t->nctxidcmp = (etm_readl(t, ETMR_CONFCODE) >> 24) & 0x3;
etm_writel(t, 0x440, ETMR_CTRL);
etm_lock(t);
@@ -609,7 +668,7 @@ static int etm_probe(struct amba_device *dev, const struct amba_id *id)
if (ret)
goto out_unmap;
- /* failing to create any of these three is not fatal */
+ /* failing to create any of these four is not fatal */
ret = device_create_file(&dev->dev, &dev_attr_trace_info);
if (ret)
dev_dbg(&dev->dev, "Failed to create trace_info in sysfs\n");
@@ -622,6 +681,12 @@ static int etm_probe(struct amba_device *dev, const struct amba_id *id)
if (ret)
dev_dbg(&dev->dev, "Failed to create trace_addrrange in sysfs\n");
+#ifdef CONFIG_PID_IN_CONTEXTIDR
+ ret = device_create_file(&dev->dev, &dev_attr_trace_pid);
+ if (ret)
+ dev_dbg(&dev->dev, "Failed to create trace_pid in sysfs\n");
+#endif
+
dev_dbg(t->dev, "ETM AMBA driver initialized.\n");
out:
@@ -652,6 +717,9 @@ static int etm_remove(struct amba_device *dev)
device_remove_file(&dev->dev, &dev_attr_trace_info);
device_remove_file(&dev->dev, &dev_attr_trace_mode);
device_remove_file(&dev->dev, &dev_attr_trace_addrrange);
+#ifdef CONFIG_PID_IN_CONTEXTIDR
+ device_remove_file(&dev->dev, &dev_attr_trace_pid);
+#endif
return 0;
}
--
1.8.5.2
^ permalink raw reply related
* [PATCH V2 4/6] ARM: Make PID_IN_CONTEXTIDR incompatible with PID_NS
From: Adrien Vergé @ 2014-01-24 16:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1390581656-16372-1-git-send-email-adrienverge@gmail.com>
When using namespaces, different processes can have the same PID.
It makes no sense to store a PID value in the Context ID register
to track a specific process, when others share the same value.
Consequently, PID_IN_CONTEXTIDR (which is used for tracing and
debugging processes) should not be compatible with PID_NS.
Signed-off-by: Adrien Verg? <adrienverge@gmail.com>
---
arch/arm/Kconfig.debug | 2 +-
arch/arm64/Kconfig.debug | 1 +
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug
index 5765abf..ed46748 100644
--- a/arch/arm/Kconfig.debug
+++ b/arch/arm/Kconfig.debug
@@ -1143,7 +1143,7 @@ config ARM_KPROBES_TEST
config PID_IN_CONTEXTIDR
bool "Write the current PID to the CONTEXTIDR register"
- depends on CPU_COPY_V6
+ depends on CPU_COPY_V6 && !PID_NS
help
Enabling this option causes the kernel to write the current PID to
the PROCID field of the CONTEXTIDR register, at the expense of some
diff --git a/arch/arm64/Kconfig.debug b/arch/arm64/Kconfig.debug
index 835c559..06b2633b 100644
--- a/arch/arm64/Kconfig.debug
+++ b/arch/arm64/Kconfig.debug
@@ -17,6 +17,7 @@ config EARLY_PRINTK
config PID_IN_CONTEXTIDR
bool "Write the current PID to the CONTEXTIDR register"
+ depends on !PID_NS
help
Enabling this option causes the kernel to write the current PID to
the CONTEXTIDR register, at the expense of some additional
--
1.8.5.2
^ permalink raw reply related
* [PATCH V2 3/6] ARM CoreSight: ETM: Add address control support
From: Adrien Vergé @ 2014-01-24 16:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1390581656-16372-1-git-send-email-adrienverge@gmail.com>
In the same manner as for enabling tracing, an entry is created
in sysfs to set the address range that triggers tracing.
Signed-off-by: Adrien Verg? <adrienverge@gmail.com>
---
arch/arm/kernel/etm.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 49 insertions(+), 3 deletions(-)
diff --git a/arch/arm/kernel/etm.c b/arch/arm/kernel/etm.c
index 7f7a0ee..4442e5c 100644
--- a/arch/arm/kernel/etm.c
+++ b/arch/arm/kernel/etm.c
@@ -44,6 +44,8 @@ struct tracectx {
struct device *dev;
struct clk *emu_clk;
struct mutex mutex;
+ unsigned long addrrange_start;
+ unsigned long addrrange_end;
};
static struct tracectx tracer;
@@ -53,6 +55,13 @@ static inline bool trace_isrunning(struct tracectx *t)
return !!(t->flags & TRACER_RUNNING);
}
+/*
+ * Setups ETM to trace only when:
+ * - address between start and end
+ * or address not between start and end (if exclude)
+ * - trace executed instructions
+ * or trace loads and stores (if data)
+ */
static int etm_setup_address_range(struct tracectx *t, int n,
unsigned long start, unsigned long end, int exclude, int data)
{
@@ -115,8 +124,8 @@ static int trace_start(struct tracectx *t)
return -EFAULT;
}
- etm_setup_address_range(t, 1, (unsigned long)_stext,
- (unsigned long)_etext, 0, 0);
+ etm_setup_address_range(t, 1, t->addrrange_start, t->addrrange_end,
+ 0, 0);
etm_writel(t, 0, ETMR_TRACEENCTRL2);
etm_writel(t, 0, ETMR_TRACESSCTRL);
etm_writel(t, 0x6f, ETMR_TRACEENEVT);
@@ -527,6 +536,36 @@ static ssize_t trace_mode_store(struct device *dev,
DEVICE_ATTR(trace_mode, S_IRUGO|S_IWUSR, trace_mode_show, trace_mode_store);
+static ssize_t trace_addrrange_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ return sprintf(buf, "%08lx - %08lx\n", tracer.addrrange_start,
+ tracer.addrrange_end);
+}
+
+static ssize_t trace_addrrange_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t n)
+{
+ unsigned long start, end;
+
+ if (tracer.flags & TRACER_RUNNING)
+ return -EBUSY;
+
+ if (sscanf(buf, "%08lx - %08lx", &start, &end) != 2)
+ return -EINVAL;
+
+ mutex_lock(&tracer.mutex);
+ tracer.addrrange_start = start;
+ tracer.addrrange_end = end;
+ mutex_unlock(&tracer.mutex);
+
+ return n;
+}
+
+DEVICE_ATTR(trace_addrrange, S_IRUGO|S_IWUSR,
+ trace_addrrange_show, trace_addrrange_store);
+
static int etm_probe(struct amba_device *dev, const struct amba_id *id)
{
struct tracectx *t = &tracer;
@@ -554,6 +593,8 @@ static int etm_probe(struct amba_device *dev, const struct amba_id *id)
t->dev = &dev->dev;
t->flags = TRACER_CYCLE_ACC;
t->etm_portsz = 1;
+ t->addrrange_start = (unsigned long) _stext;
+ t->addrrange_end = (unsigned long) _etext;
etm_unlock(t);
(void)etm_readl(t, ETMMR_PDSR);
@@ -568,7 +609,7 @@ static int etm_probe(struct amba_device *dev, const struct amba_id *id)
if (ret)
goto out_unmap;
- /* failing to create any of these two is not fatal */
+ /* failing to create any of these three is not fatal */
ret = device_create_file(&dev->dev, &dev_attr_trace_info);
if (ret)
dev_dbg(&dev->dev, "Failed to create trace_info in sysfs\n");
@@ -577,6 +618,10 @@ static int etm_probe(struct amba_device *dev, const struct amba_id *id)
if (ret)
dev_dbg(&dev->dev, "Failed to create trace_mode in sysfs\n");
+ ret = device_create_file(&dev->dev, &dev_attr_trace_addrrange);
+ if (ret)
+ dev_dbg(&dev->dev, "Failed to create trace_addrrange in sysfs\n");
+
dev_dbg(t->dev, "ETM AMBA driver initialized.\n");
out:
@@ -606,6 +651,7 @@ static int etm_remove(struct amba_device *dev)
device_remove_file(&dev->dev, &dev_attr_trace_running);
device_remove_file(&dev->dev, &dev_attr_trace_info);
device_remove_file(&dev->dev, &dev_attr_trace_mode);
+ device_remove_file(&dev->dev, &dev_attr_trace_addrrange);
return 0;
}
--
1.8.5.2
^ permalink raw reply related
* [PATCH V2 2/6] ARM CoreSight: ETM: Rename 'comparator' to 'address comparator'
From: Adrien Vergé @ 2014-01-24 16:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1390581656-16372-1-git-send-email-adrienverge@gmail.com>
Since there are different types of comparators, and other kinds will
be used (such as Context ID comparators), rename them properly.
Signed-off-by: Adrien Verg? <adrienverge@gmail.com>
---
arch/arm/include/asm/hardware/coresight.h | 4 ++--
arch/arm/kernel/etm.c | 19 ++++++++++---------
2 files changed, 12 insertions(+), 11 deletions(-)
diff --git a/arch/arm/include/asm/hardware/coresight.h b/arch/arm/include/asm/hardware/coresight.h
index ad774f3..8c50cf6 100644
--- a/arch/arm/include/asm/hardware/coresight.h
+++ b/arch/arm/include/asm/hardware/coresight.h
@@ -95,8 +95,8 @@
#define ETMAAT_NSONLY (1 << 10)
#define ETMAAT_SONLY (2 << 10)
-#define ETMR_COMP_VAL(x) (0x40 + (x) * 4)
-#define ETMR_COMP_ACC_TYPE(x) (0x80 + (x) * 4)
+#define ETMR_ADDRCOMP_VAL(x) (0x40 + (x) * 4)
+#define ETMR_ADDRCOMP_ACC_TYPE(x) (0x80 + (x) * 4)
/* ETM status register, "ETM Architecture", 3.3.2 */
#define ETMR_STATUS (0x10)
diff --git a/arch/arm/kernel/etm.c b/arch/arm/kernel/etm.c
index 2b1a307..7f7a0ee 100644
--- a/arch/arm/kernel/etm.c
+++ b/arch/arm/kernel/etm.c
@@ -39,7 +39,7 @@ struct tracectx {
void __iomem *etb_regs;
void __iomem *etm_regs;
unsigned long flags;
- int ncmppairs;
+ int naddrcmppairs;
int etm_portsz;
struct device *dev;
struct clk *emu_clk;
@@ -59,7 +59,7 @@ static int etm_setup_address_range(struct tracectx *t, int n,
u32 flags = ETMAAT_ARM | ETMAAT_IGNCONTEXTID | ETMAAT_NSONLY | \
ETMAAT_NOVALCMP;
- if (n < 1 || n > t->ncmppairs)
+ if (n < 1 || n > t->naddrcmppairs)
return -EINVAL;
/* comparators and ranges are numbered starting with 1 as opposed
@@ -72,12 +72,12 @@ static int etm_setup_address_range(struct tracectx *t, int n,
flags |= ETMAAT_IEXEC;
/* first comparator for the range */
- etm_writel(t, flags, ETMR_COMP_ACC_TYPE(n * 2));
- etm_writel(t, start, ETMR_COMP_VAL(n * 2));
+ etm_writel(t, flags, ETMR_ADDRCOMP_ACC_TYPE(n * 2));
+ etm_writel(t, start, ETMR_ADDRCOMP_VAL(n * 2));
/* second comparator is right next to it */
- etm_writel(t, flags, ETMR_COMP_ACC_TYPE(n * 2 + 1));
- etm_writel(t, end, ETMR_COMP_VAL(n * 2 + 1));
+ etm_writel(t, flags, ETMR_ADDRCOMP_ACC_TYPE(n * 2 + 1));
+ etm_writel(t, end, ETMR_ADDRCOMP_VAL(n * 2 + 1));
flags = exclude ? ETMTE_INCLEXCL : 0;
etm_writel(t, flags | (1 << n), ETMR_TRACEENCTRL);
@@ -475,7 +475,8 @@ static ssize_t trace_info_show(struct device *dev,
etm_st = etm_readl(&tracer, ETMR_STATUS);
etm_lock(&tracer);
- return sprintf(buf, "Trace buffer len: %d\nComparator pairs: %d\n"
+ return sprintf(buf, "Trace buffer len: %d\n"
+ "Addr comparator pairs: %d\n"
"ETBR_WRITEADDR:\t%08x\n"
"ETBR_READADDR:\t%08x\n"
"ETBR_STATUS:\t%08x\n"
@@ -483,7 +484,7 @@ static ssize_t trace_info_show(struct device *dev,
"ETMR_CTRL:\t%08x\n"
"ETMR_STATUS:\t%08x\n",
datalen,
- tracer.ncmppairs,
+ tracer.naddrcmppairs,
etb_wa,
etb_ra,
etb_st,
@@ -559,7 +560,7 @@ static int etm_probe(struct amba_device *dev, const struct amba_id *id)
/* dummy first read */
(void)etm_readl(&tracer, ETMMR_OSSRR);
- t->ncmppairs = etm_readl(t, ETMR_CONFCODE) & 0xf;
+ t->naddrcmppairs = etm_readl(t, ETMR_CONFCODE) & 0xf;
etm_writel(t, 0x440, ETMR_CTRL);
etm_lock(t);
--
1.8.5.2
^ permalink raw reply related
* [PATCH V2 1/6] ARM CoreSight: ETM: Use device attributes
From: Adrien Vergé @ 2014-01-24 16:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1390581656-16372-1-git-send-email-adrienverge@gmail.com>
Replace all kobjects attributes with device attributes.
User experience isn't changed since the same files are created in sysfs.
Signed-off-by: Adrien Verg? <adrienverge@gmail.com>
---
arch/arm/kernel/etm.c | 48 +++++++++++++++++++++---------------------------
1 file changed, 21 insertions(+), 27 deletions(-)
diff --git a/arch/arm/kernel/etm.c b/arch/arm/kernel/etm.c
index 8ff0ecd..2b1a307 100644
--- a/arch/arm/kernel/etm.c
+++ b/arch/arm/kernel/etm.c
@@ -430,15 +430,14 @@ static struct amba_driver etb_driver = {
};
/* use a sysfs file "trace_running" to start/stop tracing */
-static ssize_t trace_running_show(struct kobject *kobj,
- struct kobj_attribute *attr,
- char *buf)
+static ssize_t trace_running_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
{
return sprintf(buf, "%x\n", trace_isrunning(&tracer));
}
-static ssize_t trace_running_store(struct kobject *kobj,
- struct kobj_attribute *attr,
+static ssize_t trace_running_store(struct device *dev,
+ struct device_attribute *attr,
const char *buf, size_t n)
{
unsigned int value;
@@ -454,12 +453,11 @@ static ssize_t trace_running_store(struct kobject *kobj,
return ret ? : n;
}
-static struct kobj_attribute trace_running_attr =
- __ATTR(trace_running, 0644, trace_running_show, trace_running_store);
+DEVICE_ATTR(trace_running, S_IRUGO|S_IWUSR,
+ trace_running_show, trace_running_store);
-static ssize_t trace_info_show(struct kobject *kobj,
- struct kobj_attribute *attr,
- char *buf)
+static ssize_t trace_info_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
{
u32 etb_wa, etb_ra, etb_st, etb_fc, etm_ctrl, etm_st;
int datalen;
@@ -495,21 +493,19 @@ static ssize_t trace_info_show(struct kobject *kobj,
);
}
-static struct kobj_attribute trace_info_attr =
- __ATTR(trace_info, 0444, trace_info_show, NULL);
+DEVICE_ATTR(trace_info, S_IRUGO, trace_info_show, NULL);
-static ssize_t trace_mode_show(struct kobject *kobj,
- struct kobj_attribute *attr,
- char *buf)
+static ssize_t trace_mode_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
{
return sprintf(buf, "%d %d\n",
!!(tracer.flags & TRACER_CYCLE_ACC),
tracer.etm_portsz);
}
-static ssize_t trace_mode_store(struct kobject *kobj,
- struct kobj_attribute *attr,
- const char *buf, size_t n)
+static ssize_t trace_mode_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t n)
{
unsigned int cycacc, portsz;
@@ -528,8 +524,7 @@ static ssize_t trace_mode_store(struct kobject *kobj,
return n;
}
-static struct kobj_attribute trace_mode_attr =
- __ATTR(trace_mode, 0644, trace_mode_show, trace_mode_store);
+DEVICE_ATTR(trace_mode, S_IRUGO|S_IWUSR, trace_mode_show, trace_mode_store);
static int etm_probe(struct amba_device *dev, const struct amba_id *id)
{
@@ -568,17 +563,16 @@ static int etm_probe(struct amba_device *dev, const struct amba_id *id)
etm_writel(t, 0x440, ETMR_CTRL);
etm_lock(t);
- ret = sysfs_create_file(&dev->dev.kobj,
- &trace_running_attr.attr);
+ ret = device_create_file(&dev->dev, &dev_attr_trace_running);
if (ret)
goto out_unmap;
/* failing to create any of these two is not fatal */
- ret = sysfs_create_file(&dev->dev.kobj, &trace_info_attr.attr);
+ ret = device_create_file(&dev->dev, &dev_attr_trace_info);
if (ret)
dev_dbg(&dev->dev, "Failed to create trace_info in sysfs\n");
- ret = sysfs_create_file(&dev->dev.kobj, &trace_mode_attr.attr);
+ ret = device_create_file(&dev->dev, &dev_attr_trace_mode);
if (ret)
dev_dbg(&dev->dev, "Failed to create trace_mode in sysfs\n");
@@ -608,9 +602,9 @@ static int etm_remove(struct amba_device *dev)
amba_release_regions(dev);
- sysfs_remove_file(&dev->dev.kobj, &trace_running_attr.attr);
- sysfs_remove_file(&dev->dev.kobj, &trace_info_attr.attr);
- sysfs_remove_file(&dev->dev.kobj, &trace_mode_attr.attr);
+ device_remove_file(&dev->dev, &dev_attr_trace_running);
+ device_remove_file(&dev->dev, &dev_attr_trace_info);
+ device_remove_file(&dev->dev, &dev_attr_trace_mode);
return 0;
}
--
1.8.5.2
^ permalink raw reply related
* [PATCH V2 0/6] ARM CoreSight: Enhance ETM tracing control
From: Adrien Vergé @ 2014-01-24 16:40 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
Different ARM users have shown their interest in this patch, so I made
a second version that corrects the first mistakes.
Mainly, it extends current support of CoreSight ETM, that is currently
very limited. ETM provides hardware-assisted program tracing, a
cycle-precise and low-overhead solution that software tools such as
'perf record' cannot provide.
Usage of ETM tracing facility is presently limited to start and stop
tracing. This set of patches enables management of address combinations
and PIDs that trigger tracing, thus allowing to trace specific
functions and processes.
ETM management was done via sysfs entries (trace_info,
trace_running...), this code adds trace_addrrange and trace_pid to
let the user read/write custom values.
Changes in V2:
- Use device attributes rather than raw kobjects
- Make PID_IN_CONTEXTIDR incompatible with PID_NS in Kconfig
- Use int for pid type (not long)
- During trace fetching, call vmalloc() only if really needed
This series of patches applies to v3.13.
Adrien Verg? (6):
ARM CoreSight: ETM: Use device attributes
ARM CoreSight: ETM: Rename 'comparator' to 'address comparator'
ARM CoreSight: ETM: Add address control support
ARM: Make PID_IN_CONTEXTIDR incompatible with PID_NS
ARM CoreSight: ETM: Add PID control support
ARM CoreSight: ETM: Allocate a trace buffer only when necessary
arch/arm/Kconfig.debug | 2 +-
arch/arm/include/asm/hardware/coresight.h | 9 +-
arch/arm/kernel/etm.c | 208 +++++++++++++++++++++++-------
arch/arm64/Kconfig.debug | 1 +
4 files changed, 169 insertions(+), 51 deletions(-)
--
1.8.5.2
^ permalink raw reply
* [PATCH 1/6] arm64: Add macros to manage processor debug state
From: Will Deacon @ 2014-01-24 16:39 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1390490948-25386-1-git-send-email-vijay.kilari@gmail.com>
Hello,
On Thu, Jan 23, 2014 at 03:29:07PM +0000, vijay.kilari at gmail.com wrote:
> From: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com>
>
> Add macros to enable and disable to manage PSTATE.D
> for debugging. The macros local_dbg_save and local_dbg_restore
> are moved to irqflags.h file
>
> KGDB boot tests fail because of PSTATE.D is masked.
> unmask it for debugging support
>
> Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com>
> ---
> arch/arm64/include/asm/debug-monitors.h | 17 -----------
> arch/arm64/include/asm/irqflags.h | 48 +++++++++++++++++++++++++++++++
> arch/arm64/kernel/debug-monitors.c | 1 +
> 3 files changed, 49 insertions(+), 17 deletions(-)
>
> diff --git a/arch/arm64/include/asm/debug-monitors.h b/arch/arm64/include/asm/debug-monitors.h
> index 6231479..ee9f28e 100644
> --- a/arch/arm64/include/asm/debug-monitors.h
> +++ b/arch/arm64/include/asm/debug-monitors.h
> @@ -43,23 +43,6 @@ enum debug_el {
> #ifndef __ASSEMBLY__
> struct task_struct;
>
> -#define local_dbg_save(flags) \
> - do { \
> - typecheck(unsigned long, flags); \
> - asm volatile( \
> - "mrs %0, daif // local_dbg_save\n" \
> - "msr daifset, #8" \
> - : "=r" (flags) : : "memory"); \
> - } while (0)
> -
> -#define local_dbg_restore(flags) \
> - do { \
> - typecheck(unsigned long, flags); \
> - asm volatile( \
> - "msr daif, %0 // local_dbg_restore\n" \
> - : : "r" (flags) : "memory"); \
> - } while (0)
> -
> #define DBG_ARCH_ID_RESERVED 0 /* In case of ptrace ABI updates. */
>
> #define DBG_HOOK_HANDLED 0
> diff --git a/arch/arm64/include/asm/irqflags.h b/arch/arm64/include/asm/irqflags.h
> index b2fcfbc..f9b013e 100644
> --- a/arch/arm64/include/asm/irqflags.h
> +++ b/arch/arm64/include/asm/irqflags.h
> @@ -90,5 +90,53 @@ static inline int arch_irqs_disabled_flags(unsigned long flags)
> return flags & PSR_I_BIT;
> }
>
> +/*
> + * save and restore debug state
> + */
> +static inline unsigned long arch_local_dbg_save(void)
> +{
> + unsigned long flags;
> + asm volatile(
> + "mrs %0, daif // arch_local_dbg_save"
> + "msr daifset, #8"
> + : "=r" (flags) : : "memory");
> + return flags;
> +}
> +
> +static inline void arch_local_dbg_restore(unsigned long flags)
> +{
> + asm volatile(
> + "msr daif, %0 // arch_local_dbg_restore"
> + :
> + : "r" (flags)
> + : "memory");
> +}
> +
> +#define raw_local_dbg_save(flags) \
> + do { \
> + typecheck(unsigned long, flags); \
> + flags = arch_local_dbg_save(); \
> + } while (0)
> +
> +#define raw_local_dbg_restore(flags) \
> + do { \
> + typecheck(unsigned long, flags); \
> + arch_local_dbg_restore(flags); \
> + } while (0)
> +
> +#define local_dbg_save(flags) \
> + do { \
> + raw_local_dbg_save(flags); \
> + } while (0)
> +
> +#define local_dbg_restore(flags) \
> + do { \
> + typecheck(unsigned long, flags); \
> + raw_local_dbg_restore(flags); \
> + } while (0)
Hehe, I think you took me a bit too literally when I said to follow what we
do for irqs. This code is arm64-specific, so you don't need to construct it
in the same way. All you need to do is *move* the existing code from
debug-monitors.h to irqflags.h. That's it!
> +#define local_dbg_enable() asm("msr daifclr, #8" : : : "memory")
> +#define local_dbg_disable() asm("msr daifset, #8" : : : "memory")
I'm also fine with adding these two.
Will
^ permalink raw reply
* [PATCH v5 0/4] perf: AARCH64 arch support
From: Will Deacon @ 2014-01-24 16:35 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1390399879-5109-1-git-send-email-jean.pihet@linaro.org>
Hi Jean,
On Wed, Jan 22, 2014 at 02:11:15PM +0000, Jean Pihet wrote:
> Add AARCH64 specific support. This includes the following:
> - AARCH64 perf registers definition and hooks,
> - compat mode registers use, i.e. profiling a 32-bit binary on
> a 64-bit system,
> - unwinding using the dwarf information from the .debug_frame
> section of the ELF binary,
> - unwinding using the frame pointer information; in 64-bit and
> compat modes.
>
> Note: support for unwinding using the dwarf information in compat
> mode requires some changes to the libunwind code. Those changes
> have been submitted on the libunwind ML and are in discussion.
>
> Tested on ARMv7, ARMv8 and x86_64 platforms. The compat mode has been
> tested on ARMv8 using statically built 32-bit binaries.
This is look alright to me now, but there are still two blockers:
(1) Getting the libunwind code merged
(2) Splitting this into an arch/arm64 series and a tools/perf series, so
that acme can merge the latter independently (and avoid the mess we had
last time).
Anyway, for the series:
Acked-by: Will Deacon <will.deacon@arm.com>
Will
^ permalink raw reply
* [PATCH v5 4/4] [media] exynos-scaler: Add DT bindings for SCALER driver
From: Tomasz Figa @ 2014-01-24 16:09 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1389238094-19386-5-git-send-email-shaik.ameer@samsung.com>
Hi Shaik,
On 09.01.2014 04:28, Shaik Ameer Basha wrote:
> This patch adds the DT binding documentation for the
> Exynos5420/5410 based SCALER device driver.
>
> Signed-off-by: Shaik Ameer Basha <shaik.ameer@samsung.com>
> Reviewed-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> ---
> .../devicetree/bindings/media/exynos5-scaler.txt | 22 ++++++++++++++++++++
> 1 file changed, 22 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/media/exynos5-scaler.txt
>
> diff --git a/Documentation/devicetree/bindings/media/exynos5-scaler.txt b/Documentation/devicetree/bindings/media/exynos5-scaler.txt
> new file mode 100644
> index 0000000..9328e7d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/exynos5-scaler.txt
> @@ -0,0 +1,22 @@
> +* Samsung Exynos5 SCALER device
> +
> +SCALER is used for scaling, blending, color fill and color space
> +conversion on EXYNOS[5420/5410] SoCs.
> +
> +Required properties:
> +- compatible: should be "samsung,exynos5420-scaler" or
> + "samsung,exynos5410-scaler"
> +- reg: should contain SCALER physical address location and length
> +- interrupts: should contain SCALER interrupt number
s/number/specifier/
> +- clocks: should contain the SCALER clock specifier, from the
> + common clock bindings
s/specifier/phandle and specifier pair for each clock listed in
clock-names property/
s/from/according to/
> +- clock-names: should be "scaler"
should contain exactly one entry:
- "scaler" - IP bus clock.
Also this patch should be first in the series to let the driver added in
further patches use already present bindings.
Best regards,
Tomasz
^ permalink raw reply
* [PATCH 10/20] ARM64 / ACPI: Enumerate possible/present CPU set and map logical cpu id to APIC id
From: Hanjun Guo @ 2014-01-24 16:02 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140124153557.GB23274@e102568-lin.cambridge.arm.com>
On 2014?01?24? 23:35, Lorenzo Pieralisi wrote:
> On Fri, Jan 24, 2014 at 02:37:28PM +0000, Hanjun Guo wrote:
>> Hi Lorenzo,
>>
>> On 2014?01?22? 23:53, Lorenzo Pieralisi wrote:
>>> On Fri, Jan 17, 2014 at 12:25:04PM +0000, Hanjun Guo wrote:
>>>
>>> [...]
>>>
>>>> +/* map logic cpu id to physical GIC id */
>>>> +extern int arm_cpu_to_apicid[NR_CPUS];
>>>> +#define cpu_physical_id(cpu) arm_cpu_to_apicid[cpu]
>>> Sudeep already commented on this, please update it accordingly.
>> Actually after some careful review of the ACPI code, I can't
>> update it as MPIDR here.
>>
>> MPIDR can be the ACPI uid and NOT the GIC id, the mapping
>> of them are something like this in ACPI driver now:
>>
>> logic cpu id <---> APIC Id (GIC ID) <---> ACPI uid (MPIDR on ARM)
>> but not logic cpu id <---> ACPI uid directly, you can refer to
>> the code of processor_core.c
>>
>> So here I can only map GIC id to logical cpu id.
> On ARM platforms GIC CPU IF id is probeable, you do not need to parse
> it (ie it is not information that you will find in DT). Please have a look
> at drivers/irqchip/irq-gic.c.
>
> We have to understand what's really required and when in ACPI, or to put it
> differently, why cpu_physical_id(cpu) is required and at what time at
> boot, I will have a look on my side too.
Me too :)
>
>>>> +
>>>> #else /* !CONFIG_ACPI */
>>>> #define acpi_disabled 1 /* ACPI sometimes enabled on ARM */
>>>> #define acpi_noirq 1 /* ACPI sometimes enabled on ARM */
>>>> diff --git a/drivers/acpi/plat/arm-core.c b/drivers/acpi/plat/arm-core.c
>>>> index 8ba3e6f..1d9b789 100644
>>>> --- a/drivers/acpi/plat/arm-core.c
>>>> +++ b/drivers/acpi/plat/arm-core.c
>>>> @@ -31,6 +31,7 @@
>>>> #include <linux/smp.h>
>>>>
>>>> #include <asm/pgtable.h>
>>>> +#include <asm/cputype.h>
>>>>
>>>> /*
>>>> * We never plan to use RSDT on arm/arm64 as its deprecated in spec but this
>>>> @@ -52,6 +53,13 @@ EXPORT_SYMBOL(acpi_pci_disabled);
>>>> */
>>>> static u64 acpi_lapic_addr __initdata;
>>> Is this variable actually needed ?
>> Yes, needed for GIC initialization.
>>
>>>>
>>>> +/* available_cpus here means enabled cpu in MADT */
>>>> +static int available_cpus;
>>> Ditto.
>>>
>>>> +
>>>> +/* Map logic cpu id to physical GIC id (physical CPU id). */
>>>> +int arm_cpu_to_apicid[NR_CPUS] = { [0 ... NR_CPUS-1] = -1 };
>>>> +static int boot_cpu_apic_id = -1;
>>> Do we need all these variables ? I think we should reuse cpu_logical_map
>>> data structures for that, it looks suspiciously familiar.
>> MPIDR is the different part. if we use MPIDR as GIC id, i think
>> we can reuse cpu_logical_map, but Sudeep suggested not
>> use MPIDR as GIC id.
> It is not about *reusing* cpu_logical_map, it is about setting it up
> properly. cpu_logical_map must be initialized by ACPI for the spin table
> method to work properly (and PSCI too).
>
> And yes, cpu_physical_id(cpu) is expected to be the GIC CPU IF id on
> ARM, at least it looks like, I had a look too. But this does not change
> anything as far as cpu_logical_map is concerned, it must contain a list
> of MPIDRs in the system and must be retrieved via ACPI, not DT CPU nodes
> when ACPI is used for booting.
>
> I will have a further look, since this discrepancy is annoying.
Thank you for doing this, I will look that too.
>
> [...]
>
>>>> +
>>>> + available_cpus++;
>>>> +
>>> Is available_cpus != num_possible_cpus() ? It does not look like hence
>>> available_cpus can go.
>> No, possible cpus include available cpus and disabled cpus
>> this is useful for ACPI based CPU hot-plug features.
>>
>>>> + /* allocate a logic cpu id for the new comer */
>>>> + if (boot_cpu_apic_id == id) {
>>>> + /*
>>>> + * boot_cpu_init() already hold bit 0 in cpu_present_mask
>>>> + * for BSP, no need to allocte again.
>>>> + */
>>>> + cpu = 0;
>>>> + } else {
>>>> + cpu = cpumask_next_zero(-1, cpu_present_mask);
>>>> + }
>>>> +
>>>> + /* map the logic cpu id to APIC id */
>>>> + arm_cpu_to_apicid[cpu] = id;
>>>> +
>>>> + set_cpu_present(cpu, true);
>>>> + set_cpu_possible(cpu, true);
>>> This is getting nasty. Before adding this patch and previous ones we
>>> need to put in place a method for the kernel to make a definite choice between
>>> ACPI and DT and stick to that. We can't initialize the logical map twice
>>> (which will happen if your DT has valid cpu nodes and a chosen node pointing
>>> to proper ACPI tables) or even having some entries initialized from DT and
>>> others by ACPI. It is a big fat no-no, please update the series accordingly.
>> really good catch here :)
>> so the problem here is that should we use both ACPI and DT in one system?
I think Mark had a clear answer about this :) (Answer for my self)
>>
>>
>>>> +
>>>> + return cpu;
>>>> +}
>>>> +
>>>> static int __init
>>>> acpi_parse_gic(struct acpi_subtable_header *header, const unsigned long end)
>>>> {
>>>> @@ -144,6 +201,16 @@ acpi_parse_gic(struct acpi_subtable_header *header, const unsigned long end)
>>>>
>>>> acpi_table_print_madt_entry(header);
>>>>
>>>> + /*
>>>> + * We need to register disabled CPU as well to permit
>>>> + * counting disabled CPUs. This allows us to size
>>>> + * cpus_possible_map more accurately, to permit
>>>> + * to not preallocating memory for all NR_CPUS
>>>> + * when we use CPU hotplug.
>>>> + */
>>>> + acpi_register_gic_cpu_interface(processor->gic_id,
>>>> + processor->flags & ACPI_MADT_ENABLED);
>>>> +
>>>> return 0;
>>>> }
>>>>
>>>> @@ -186,6 +253,19 @@ static int __init acpi_parse_madt_gic_entries(void)
>>>> return count;
>>>> }
>>>>
>>>> +#ifdef CONFIG_SMP
>>>> + if (available_cpus == 0) {
>>>> + pr_info(PREFIX "Found 0 CPUs; assuming 1\n");
>>>> + arm_cpu_to_apicid[available_cpus] =
>>>> + read_cpuid_mpidr() & MPIDR_HWID_BITMASK;
>>>> + available_cpus = 1; /* We've got at least one of these */
>>>> + }
>>> I'd rather check the MADT for at least the boot cpu to present, if it is
>>> not ACPI tables are horribly buggy and the kernel should barf on that.
>>>
>>>> +#endif
>>>> +
>>>> + /* Make boot-up look pretty */
>>>> + pr_info("%d CPUs available, %d CPUs total\n", available_cpus,
>>>> + total_cpus);
>>> Ok, now, how can we use the "disabled" CPUs == (total_cpus - available_cpus) ?
>> For cpus can be hot-added later when system is running.
> I do not see any usage in the patchset and certainly those variables are
> not used in this patch, apart from printing messages whose usefulness is
> debatable. If, as you say, you are using those variables for something
> else, please add code in the patch where they are introduced for it to be
> self-contained and to simplify the review.
ah, yes. although my ACPI based CPU hot-plug patch is ready, but need
this patch set goes in first and then send them out.
I agree with you, will try to update this patch.
Thanks
Hanjun
^ permalink raw reply
* [PATCH] arm64: Add CONFIG_CC_STACKPROTECTOR
From: Will Deacon @ 2014-01-24 15:59 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <52E16E42.5040000@codeaurora.org>
On Thu, Jan 23, 2014 at 07:32:18PM +0000, Laura Abbott wrote:
> On 1/23/2014 11:23 AM, Nicolas Pitre wrote:
> > Did gcc for Aarch64 replicate the same global variable arrangement?
> > That would be unfortunate...
> >
>
> Based on my experiments they did unfortunately do so.
Yes, and speaking to the tools guys, it would require an new ABI to fix
this. Given that I don't expect to see any UP AArch64 cores, I'd say drop
the #ifndef SMP part from this patch.
Will
^ permalink raw reply
* [PATCH] ARM: hw_breakpoint: Add ARMv8 support
From: Will Deacon @ 2014-01-24 15:57 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1390415365-17230-1-git-send-email-cov@codeaurora.org>
Hi Christopher,
On Wed, Jan 22, 2014 at 06:29:25PM +0000, Christopher Covington wrote:
> Add the trivial support necessary to get hardware breakpoints
> working for GDB on ARMv8 simulators running in AArch32 mode.
>
> Signed-off-by: Christopher Covington <cov@codeaurora.org>
> ---
> arch/arm/include/asm/hw_breakpoint.h | 1 +
> arch/arm/kernel/hw_breakpoint.c | 1 +
> 2 files changed, 2 insertions(+)
>
> diff --git a/arch/arm/include/asm/hw_breakpoint.h b/arch/arm/include/asm/hw_breakpoint.h
> index eef55ea..8e427c7 100644
> --- a/arch/arm/include/asm/hw_breakpoint.h
> +++ b/arch/arm/include/asm/hw_breakpoint.h
> @@ -51,6 +51,7 @@ static inline void decode_ctrl_reg(u32 reg,
> #define ARM_DEBUG_ARCH_V7_ECP14 3
> #define ARM_DEBUG_ARCH_V7_MM 4
> #define ARM_DEBUG_ARCH_V7_1 5
> +#define ARM_DEBUG_ARCH_V8 6
>
> /* Breakpoint */
> #define ARM_BREAKPOINT_EXECUTE 0
> diff --git a/arch/arm/kernel/hw_breakpoint.c b/arch/arm/kernel/hw_breakpoint.c
> index 3d44660..45fbcaf 100644
> --- a/arch/arm/kernel/hw_breakpoint.c
> +++ b/arch/arm/kernel/hw_breakpoint.c
> @@ -257,6 +257,7 @@ static int enable_monitor_mode(void)
> break;
> case ARM_DEBUG_ARCH_V7_ECP14:
> case ARM_DEBUG_ARCH_V7_1:
> + case ARM_DEBUG_ARCH_V8:
> ARM_DBG_WRITE(c0, c2, 2, (dscr | ARM_DSCR_MDBGEN));
> isb();
> break;
I'm slightly uneasy about this, mainly because I've not spent much time
looking at the AArch32 side of ARMv8 debug and I know that a fair amount
has changed since 7.1. Furthermore, the arch/arm/ kernel hasn't grown any v8
features yet and it's not clear whether that's a road down which people wish
to take it.
Anyway, assuming this works, can you please update debug_exception_updates_fsr
to return true for ARMv8 cores?
Will
^ permalink raw reply
* [PATCH 18/20] clocksource / acpi: Add macro CLOCKSOURCE_ACPI_DECLARE
From: Hanjun Guo @ 2014-01-24 15:53 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140124151513.GD19052@arm.com>
On 2014?01?24? 23:15, Catalin Marinas wrote:
> On Fri, Jan 24, 2014 at 12:08:15PM +0000, Mark Rutland wrote:
>> On Fri, Jan 24, 2014 at 12:20:46AM +0000, Hanjun Guo wrote:
>>> On 2014?01?22? 19:45, Mark Rutland wrote:
>>>> On Wed, Jan 22, 2014 at 08:26:50AM +0000, Linus Walleij wrote:
>>>>> On Fri, Jan 17, 2014 at 1:25 PM, Hanjun Guo <hanjun.guo@linaro.org> wrote:
>>>>>
>>>>>> From: Amit Daniel Kachhap <amit.daniel@samsung.com>
>>>>>>
>>>>>> This macro does the same job as CLOCKSOURCE_OF_DECLARE. The device
>>>>>> name from the ACPI timer table is matched with all the registered
>>>>>> timer controllers and matching initialisation routine is invoked.
>>>>>>
>>>>>> Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
>>>>>> Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
>>>>> Actually I have a fat patch renaming CLOCKSOURCE_OF_DECLARE()
>>>>> to TIMER_OF_DECLARE() and I think this macro, if needed, should
>>>>> be named TIMER_ACPI_DECLARE().
>>>>>
>>>>> The reason is that "clocksource" is a Linux-internal name and this
>>>>> macro pertains to the hardware name in respective system
>>>>> description type.
>>>>>
>>>>>> +#ifdef CONFIG_ACPI
>>>>>> +#define CLOCKSOURCE_ACPI_DECLARE(name, compat, fn) \
>>>>>> + static const struct acpi_device_id __clksrc_acpi_table_##name \
>>>>>> + __used __section(__clksrc_acpi_table) \
>>>>>> + = { .id = compat, \
>>>>>> + .driver_data = (kernel_ulong_t)fn }
>>>>>> +#else
>>>>>> +#define CLOCKSOURCE_ACPI_DECLARE(name, compat, fn)
>>>>>> +#endif
>>>>> This hammers down the world to compile one binary for ACPI
>>>>> and one binary for device tree. Maybe that's fine, I don't know.
>>>> How does it do that?
>>>>
>>>> As far as I could tell CONFIG_ACPI and CONFIG_OF are not mutually
>>>> exclusive, and this just means that we only build the datastructures for
>>>> matching from ACPI when CONFIG_ACPI is enabled.
>>>>
>>>> Have I missed something?
>>>>
>>>> I definitely don't want to see mutually exclusive ACPI and DT support.
>>> ACPI and DT did the same job so I think they should mutually exclusive.
>>> if we enable both DT and ACPI in one system, this will leading confusions.
>> ACPI and DT do similar jobs, and we should be mutually exclusive at
>> runtime. However, they should not be mutually exclusive at compile-time.
>>
>> Being mutually exclusive at compile-time is just broken. It creates more
>> work for distributions (who need to ship double the number of kernels),
>> it increases the number of configurations requiring testing, and it
>> makes it easier for bugs to be introduced. It's just painful, and
>> there's no reason for it.
> I fully agree (IOW, I'll NAK patches that break this assumption; we want
> single kernel image whether it uses DT or ACPI).
I will not break this in next version, because I totally agree with Mark
too :)
>
>> At boot time the kernel needs to decide which to use for hardware
>> description, and completely ignore the other (which should not be
>> present, but lets not assume that or inevitably someone will break that
>> assumption for a quick hack).
>>
>> The same kernel should boot on a system that has a DTB or a system that
>> has ACPI tables. On a system that's provided both it should use one or
>> the other, but not both.
> Do we still need the chosen node to be passed via DT for command line,
> even if the kernel uses ACPI?
It depends, but I would prefer not. I prefer UEFI+ACPI and then we don't
need
the chosen node to be passed via DT.
Thanks
Hanjun
^ permalink raw reply
* [PATCH 18/20] clocksource / acpi: Add macro CLOCKSOURCE_ACPI_DECLARE
From: Hanjun Guo @ 2014-01-24 15:45 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140124123259.GI814@e106331-lin.cambridge.arm.com>
Hi Mark,
On 2014?01?24? 20:32, Mark Rutland wrote:
> On Fri, Jan 24, 2014 at 12:12:24AM +0000, Hanjun Guo wrote:
>> Hi Linus,
>>
>> Sorry for the late reply.
>>
>> On 2014?01?22? 16:26, Linus Walleij wrote:
>>> On Fri, Jan 17, 2014 at 1:25 PM, Hanjun Guo <hanjun.guo@linaro.org> wrote:
>>>
>>>> From: Amit Daniel Kachhap <amit.daniel@samsung.com>
>>>>
>>>> This macro does the same job as CLOCKSOURCE_OF_DECLARE. The device
>>>> name from the ACPI timer table is matched with all the registered
>>>> timer controllers and matching initialisation routine is invoked.
>>>>
>>>> Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
>>>> Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
>>> Actually I have a fat patch renaming CLOCKSOURCE_OF_DECLARE()
>>> to TIMER_OF_DECLARE() and I think this macro, if needed, should
>>> be named TIMER_ACPI_DECLARE().
>>>
>>> The reason is that "clocksource" is a Linux-internal name and this
>>> macro pertains to the hardware name in respective system
>>> description type.
>> That make sense to me too, I will update in next version if
>> this patch is still needed.
>>
>>>> +#ifdef CONFIG_ACPI
>>>> +#define CLOCKSOURCE_ACPI_DECLARE(name, compat, fn) \
>>>> + static const struct acpi_device_id __clksrc_acpi_table_##name \
>>>> + __used __section(__clksrc_acpi_table) \
>>>> + = { .id = compat, \
>>>> + .driver_data = (kernel_ulong_t)fn }
>>>> +#else
>>>> +#define CLOCKSOURCE_ACPI_DECLARE(name, compat, fn)
>>>> +#endif
>>> This hammers down the world to compile one binary for ACPI
>>> and one binary for device tree. Maybe that's fine, I don't know.
>> This is a problem we can have some discussion on it.
>> I prefer mutually exclusive ACPI and DT support.
> A lot of work has been put into making a single kernel boot everywhere.
> It's forced duplicated code to be factored out, and it's made the kernel
> more flexible. While it has been painful, it's forced a far higher
> quality standard across the board(s).
>
> Having a separate ACPI-capable or DT-capable kernels goes completely
> against that, and it's completely broken:
>
> * It doubles the testing effort required for a particular kernel. I can
> guarantee that we will miss bugs (even amazingly bad build bugs)
> because no-one will be able to test a full suite of kernels.
>
> * It introduces the possibility of completely pointles arbitrary
> differences between the two. How long until we see the first bug-fix
> that only works in one configuration?
>
> * It creates additional work for distributions, which need to build more
> kernels test them, distribute them, and document which platforms which
> kernels are supported on. This creates more pain for end-users too.
>
> Eventually we _will_ get fed up with all of those, and we'll have to do
> painful invasive work to make the kernel decide at runtime.
>
> Having separate kernels is a lazy shortcut. It's painful for everyone,
> leads to a greater maintenance overhead, it's not what we want now and
> not what we want in future.
>
> No thanks.
>
> Either the kernel figures out whether or not to deal with ACPI at
> runtime, or it doesn't deal with it at all.
I fully agree with you for the single kernel image, I didn't notice this
before, sorry for
my noise about the exclusive ACPI and DT support.
Thank you very much to let things much more clearer :)
Hanjun
^ permalink raw reply
* [PATCH 18/20] clocksource / acpi: Add macro CLOCKSOURCE_ACPI_DECLARE
From: Mark Rutland @ 2014-01-24 15:44 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140124151513.GD19052@arm.com>
On Fri, Jan 24, 2014 at 03:15:13PM +0000, Catalin Marinas wrote:
> Do we still need the chosen node to be passed via DT for command line,
> even if the kernel uses ACPI?
It depends on how the kernel is booted.
If not booted as a UEFI application, then the user would have to provide
a dtb to pass the command line.
If booted as a UEFI application, the kernel can retrieve the commandline
from UEFI and handle it appropriately (this _might_ involve the EFI stub
building a dtb with a chosen node and pasing that to the kernel, but
that could change as it's within the kernel). In that case, the user
need not build a dtb manually.
However, I'd expect that we use a restricted dtb with only a /chosen
node for passing additional data as an equivalent to x86's bootargs
(which would contain the location of ACPI tables and possibly other
stuff like initrd location if not passed on the command line). That
makes the very early logic in the kernel much simpler, and it gives us
an extensible format that should limit headaches in future.
So if booted with ACPI we _might_ have a restricted chosen-only dtb,
which _might_ be passed to the kernel explicitly by the user.
Thanks,
Mark.
^ permalink raw reply
* [Q] L1_CACHE_BYTES on flush_pfn_alias function.
From: Catalin Marinas @ 2014-01-24 15:43 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <00d501cf136a$24ec49c0$6ec4dd40$@samsung.com>
On Fri, Jan 17, 2014 at 09:54:42AM +0000, ?????? wrote:
> Follow the mailing-list
> http://comments.gmane.org/gmane.linux.ports.arm.omap/31686
>
> >>Setting the L1 cache line size larger than it actually is should be safe.
>
> the written code expected as L1_CACHE_BYTES should be real cache line size
> has bug.
> It looks like that flush_pfn_alias function should be fixed.
Did you actually notice any problem with flushing some more bytes? It's
a clean+invalidate rather than invalidate, I don't see any problem.
--
Catalin
^ permalink raw reply
* [PATCH] arm64: delete non-required instances of <linux/init.h>
From: Paul Gortmaker @ 2014-01-24 15:43 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140124152308.GE19052@arm.com>
On 14-01-24 10:23 AM, Catalin Marinas wrote:
> On Thu, Jan 23, 2014 at 06:26:51PM +0000, Paul Gortmaker wrote:
>> None of these files are actually using any __init type directives
>> and hence don't need to include <linux/init.h>. Most are just a
>> left over from __devinit and __cpuinit removal, or simply due to
>> code getting copied from one place to the next.
>>
>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>> Cc: Will Deacon <will.deacon@arm.com>
>> Cc: linux-arm-kernel at lists.infradead.org
>> Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
>> ---
>>
>> [Build tested on today's linux-next ; thanks to Will for the pointer
>> to a prebuilt toolchain. Patch to be added to init cleanup series:
>> http://git.kernel.org/cgit/linux/kernel/git/paulg/init.git ]
>
> I assume you are pushing this upstream as part of your cleanup series.
Yes, that is the plan -- ack added, thanks.
P.
--
>
> Acked-by: Catalin Marinas <catalin.marinas@arm.com>
>
^ permalink raw reply
* [PATCH] sunxi: dts: add a note that memory size is adjusted by boot loader.
From: Ian Campbell @ 2014-01-24 15:41 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <52E2874F.10703@redhat.com>
On Fri, 2014-01-24 at 16:31 +0100, Hans de Goede wrote:
> Hi,
>
> On 01/24/2014 03:14 PM, Ian Campbell wrote:
> > I had to spend a couple of minutes proving to myself that this was the case on
> > cubietruck, so add a comment to save the next guy some effort.
>
> Seems like a good idea to me, one small nitpick though see comments inline.
>
> > Signed-off-by: Ian Campbell <ijc@hellion.org.uk>
> > Cc: Maxime Ripard <maxime.ripard@free-electrons.com>
> > Cc: Hans de Goede <hdegoede@redhat.com>
> > Cc: linux-arm-kernel at lists.infradead.org
> > Cc: linux-sunxi at googlegroups.com
> > ---
> > This patch applies cleanly against v3.13 and Hans' sunxi-devel branch.
> >
> > A plausible alternative would be to pull the memory node out of the dtsi files
> > and into the board specific files. I didn't go straight to that since I'd have
> > to research all the various boards ;-)
>
> That won't help, some boards ie the original cubieboard and the mele-a1000 come
> in both 512 MB and 1024 MB versions, and I don't think we want to start maintaining
> 2 different dts files just for that.
I didn't mean to suggest that u-boot should stop updating the dtb, so
e.g. I would have put a 512MB reg in the cubieboard dts, with a similar
comment.
But in cases where there is only one variant (e.g. the cubietruck) I'd
have put a 2GB reg and no comment, even though u-boot would in reality
rewrite the 2GB there.
> > diff --git a/arch/arm/boot/dts/sun4i-a10.dtsi b/arch/arm/boot/dts/sun4i-a10.dtsi
> > index 3e60883..9ba0beb 100644
> > --- a/arch/arm/boot/dts/sun4i-a10.dtsi
> > +++ b/arch/arm/boot/dts/sun4i-a10.dtsi
> > @@ -30,6 +30,7 @@
> > };
> >
> > memory {
> > + /* 1GB by default, will be updated by U-Boot */
> > reg = <0x40000000 0x80000000>;
> > };
> >
>
> The comment says 1GB, but the range says 2GB, note 2GB is consistent with what the
> datasheet claims as max RAM.
Oops, I must have missed that.
>
> > diff --git a/arch/arm/boot/dts/sun5i-a10s.dtsi b/arch/arm/boot/dts/sun5i-a10s.dtsi
> > index 0376c50..d12ed7e 100644
> > --- a/arch/arm/boot/dts/sun5i-a10s.dtsi
> > +++ b/arch/arm/boot/dts/sun5i-a10s.dtsi
> > @@ -27,6 +27,7 @@
> > };
> >
> > memory {
> > + /* 512MB by default, will be updated by U-Boot */
> > reg = <0x40000000 0x20000000>;
> > };
> >
>
> This seems wrong (copy paste from A13 error) I've a10s boards with 1G, and the data
> sheet claims 2GB max RAM.
I had expected these values to contain the minimum -- i.e. the one which
would work everywhere, so it would work if for some reason u-boot wasn't
updating correctly. Better to boot with only some of the RAM than to
crash randomly.
> > diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi
> > index 907cfcc..658e74b 100644
> > --- a/arch/arm/boot/dts/sun7i-a20.dtsi
> > +++ b/arch/arm/boot/dts/sun7i-a20.dtsi
> > @@ -38,6 +38,7 @@
> > };
> >
> > memory {
> > + /* 1GB by default, will be updated by U-Boot */
> > reg = <0x40000000 0x80000000>;
> > };
>
> The comment says 1GB, but the range says 2GB, note 2GB is consistent with what the
> datasheet claims as max RAM.
Huh, that's the second time I got the wrong one, and I swear I was
paying attention to it as well!
Ian.
^ permalink raw reply
* [PATCH] arm64: fix build error if DMA_CMA is enabled
From: Catalin Marinas @ 2014-01-24 15:37 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1390551788-6138-1-git-send-email-pankaj.dubey@samsung.com>
On Fri, Jan 24, 2014 at 08:23:08AM +0000, Pankaj Dubey wrote:
> arm64/include/asm/dma-contiguous.h is trying to include
> <asm-genric/dma-contiguous.h> which does not exist, and thus failing
> build for arm64 if we enable CONFIG_DMA_CMA. This patch fixes build
> error by removing unwanted header inclusion from arm64's dma-contiguous.h.
>
> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
> Signed-off-by: Somraj Mani <somraj.mani@samsung.com>
> ---
> arch/arm64/include/asm/dma-contiguous.h | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/arch/arm64/include/asm/dma-contiguous.h b/arch/arm64/include/asm/dma-contiguous.h
> index d6aacb6..14c4c0c 100644
> --- a/arch/arm64/include/asm/dma-contiguous.h
> +++ b/arch/arm64/include/asm/dma-contiguous.h
> @@ -18,7 +18,6 @@
> #ifdef CONFIG_DMA_CMA
>
> #include <linux/types.h>
> -#include <asm-generic/dma-contiguous.h>
Thanks for this.
Laura, did you have additional patches adding
asm-generic/dma-contiguous.h?
--
Catalin
^ permalink raw reply
* [PATCH 10/20] ARM64 / ACPI: Enumerate possible/present CPU set and map logical cpu id to APIC id
From: Lorenzo Pieralisi @ 2014-01-24 15:35 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <52E27AA8.8040105@linaro.org>
On Fri, Jan 24, 2014 at 02:37:28PM +0000, Hanjun Guo wrote:
> Hi Lorenzo,
>
> On 2014?01?22? 23:53, Lorenzo Pieralisi wrote:
> > On Fri, Jan 17, 2014 at 12:25:04PM +0000, Hanjun Guo wrote:
> >
> > [...]
> >
> >> +/* map logic cpu id to physical GIC id */
> >> +extern int arm_cpu_to_apicid[NR_CPUS];
> >> +#define cpu_physical_id(cpu) arm_cpu_to_apicid[cpu]
> > Sudeep already commented on this, please update it accordingly.
>
> Actually after some careful review of the ACPI code, I can't
> update it as MPIDR here.
>
> MPIDR can be the ACPI uid and NOT the GIC id, the mapping
> of them are something like this in ACPI driver now:
>
> logic cpu id <---> APIC Id (GIC ID) <---> ACPI uid (MPIDR on ARM)
> but not logic cpu id <---> ACPI uid directly, you can refer to
> the code of processor_core.c
>
> So here I can only map GIC id to logical cpu id.
On ARM platforms GIC CPU IF id is probeable, you do not need to parse
it (ie it is not information that you will find in DT). Please have a look
at drivers/irqchip/irq-gic.c.
We have to understand what's really required and when in ACPI, or to put it
differently, why cpu_physical_id(cpu) is required and at what time at
boot, I will have a look on my side too.
> >
> >> +
> >> #else /* !CONFIG_ACPI */
> >> #define acpi_disabled 1 /* ACPI sometimes enabled on ARM */
> >> #define acpi_noirq 1 /* ACPI sometimes enabled on ARM */
> >> diff --git a/drivers/acpi/plat/arm-core.c b/drivers/acpi/plat/arm-core.c
> >> index 8ba3e6f..1d9b789 100644
> >> --- a/drivers/acpi/plat/arm-core.c
> >> +++ b/drivers/acpi/plat/arm-core.c
> >> @@ -31,6 +31,7 @@
> >> #include <linux/smp.h>
> >>
> >> #include <asm/pgtable.h>
> >> +#include <asm/cputype.h>
> >>
> >> /*
> >> * We never plan to use RSDT on arm/arm64 as its deprecated in spec but this
> >> @@ -52,6 +53,13 @@ EXPORT_SYMBOL(acpi_pci_disabled);
> >> */
> >> static u64 acpi_lapic_addr __initdata;
> > Is this variable actually needed ?
>
> Yes, needed for GIC initialization.
>
> >
> >>
> >> +/* available_cpus here means enabled cpu in MADT */
> >> +static int available_cpus;
> > Ditto.
> >
> >> +
> >> +/* Map logic cpu id to physical GIC id (physical CPU id). */
> >> +int arm_cpu_to_apicid[NR_CPUS] = { [0 ... NR_CPUS-1] = -1 };
> >> +static int boot_cpu_apic_id = -1;
> > Do we need all these variables ? I think we should reuse cpu_logical_map
> > data structures for that, it looks suspiciously familiar.
>
> MPIDR is the different part. if we use MPIDR as GIC id, i think
> we can reuse cpu_logical_map, but Sudeep suggested not
> use MPIDR as GIC id.
It is not about *reusing* cpu_logical_map, it is about setting it up
properly. cpu_logical_map must be initialized by ACPI for the spin table
method to work properly (and PSCI too).
And yes, cpu_physical_id(cpu) is expected to be the GIC CPU IF id on
ARM, at least it looks like, I had a look too. But this does not change
anything as far as cpu_logical_map is concerned, it must contain a list
of MPIDRs in the system and must be retrieved via ACPI, not DT CPU nodes
when ACPI is used for booting.
I will have a further look, since this discrepancy is annoying.
[...]
> >> +
> >> + available_cpus++;
> >> +
> > Is available_cpus != num_possible_cpus() ? It does not look like hence
> > available_cpus can go.
>
> No, possible cpus include available cpus and disabled cpus
> this is useful for ACPI based CPU hot-plug features.
>
> >
> >> + /* allocate a logic cpu id for the new comer */
> >> + if (boot_cpu_apic_id == id) {
> >> + /*
> >> + * boot_cpu_init() already hold bit 0 in cpu_present_mask
> >> + * for BSP, no need to allocte again.
> >> + */
> >> + cpu = 0;
> >> + } else {
> >> + cpu = cpumask_next_zero(-1, cpu_present_mask);
> >> + }
> >> +
> >> + /* map the logic cpu id to APIC id */
> >> + arm_cpu_to_apicid[cpu] = id;
> >> +
> >> + set_cpu_present(cpu, true);
> >> + set_cpu_possible(cpu, true);
> > This is getting nasty. Before adding this patch and previous ones we
> > need to put in place a method for the kernel to make a definite choice between
> > ACPI and DT and stick to that. We can't initialize the logical map twice
> > (which will happen if your DT has valid cpu nodes and a chosen node pointing
> > to proper ACPI tables) or even having some entries initialized from DT and
> > others by ACPI. It is a big fat no-no, please update the series accordingly.
>
> really good catch here :)
> so the problem here is that should we use both ACPI and DT in one system?
>
>
> >
> >> +
> >> + return cpu;
> >> +}
> >> +
> >> static int __init
> >> acpi_parse_gic(struct acpi_subtable_header *header, const unsigned long end)
> >> {
> >> @@ -144,6 +201,16 @@ acpi_parse_gic(struct acpi_subtable_header *header, const unsigned long end)
> >>
> >> acpi_table_print_madt_entry(header);
> >>
> >> + /*
> >> + * We need to register disabled CPU as well to permit
> >> + * counting disabled CPUs. This allows us to size
> >> + * cpus_possible_map more accurately, to permit
> >> + * to not preallocating memory for all NR_CPUS
> >> + * when we use CPU hotplug.
> >> + */
> >> + acpi_register_gic_cpu_interface(processor->gic_id,
> >> + processor->flags & ACPI_MADT_ENABLED);
> >> +
> >> return 0;
> >> }
> >>
> >> @@ -186,6 +253,19 @@ static int __init acpi_parse_madt_gic_entries(void)
> >> return count;
> >> }
> >>
> >> +#ifdef CONFIG_SMP
> >> + if (available_cpus == 0) {
> >> + pr_info(PREFIX "Found 0 CPUs; assuming 1\n");
> >> + arm_cpu_to_apicid[available_cpus] =
> >> + read_cpuid_mpidr() & MPIDR_HWID_BITMASK;
> >> + available_cpus = 1; /* We've got at least one of these */
> >> + }
> > I'd rather check the MADT for at least the boot cpu to present, if it is
> > not ACPI tables are horribly buggy and the kernel should barf on that.
> >
> >> +#endif
> >> +
> >> + /* Make boot-up look pretty */
> >> + pr_info("%d CPUs available, %d CPUs total\n", available_cpus,
> >> + total_cpus);
> > Ok, now, how can we use the "disabled" CPUs == (total_cpus - available_cpus) ?
>
> For cpus can be hot-added later when system is running.
I do not see any usage in the patchset and certainly those variables are
not used in this patch, apart from printing messages whose usefulness is
debatable. If, as you say, you are using those variables for something
else, please add code in the patch where they are introduced for it to be
self-contained and to simplify the review.
Thanks,
Lorenzo
^ permalink raw reply
* [PATCH 03/20] ARM64 / ACPI: Introduce the skeleton of _PDC related for ARM64
From: Hanjun Guo @ 2014-01-24 15:35 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140123180335.GK27520@arm.com>
Hi Catalin,
On 2014?01?24? 02:03, Catalin Marinas wrote:
> On Fri, Jan 17, 2014 at 12:24:57PM +0000, Hanjun Guo wrote:
>> --- /dev/null
>> +++ b/arch/arm64/include/asm/acpi.h
>> @@ -0,0 +1,32 @@
>> +/*
>> + * Copyright (C) 2013, Al Stone <al.stone@linaro.org>
>> + *
>> + * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + *
>> + * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> + */
>> +
>> +#ifndef _ASM_ARM64_ACPI_H
>> +#define _ASM_ARM64_ACPI_H
> Nitpick: please use __ASM_ACPI_H as a guard for consistency (the same
> for all the other patches introducing new header files).
ok, will update this patch.
Hanjun
>
^ permalink raw reply
* [PATCH] ARM-i.MX6Q-dts : Added USB_OTG Support
From: Mark Rutland @ 2014-01-24 15:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAOMZO5B=2mYWeoj1snSMpDzxHK8H3_DKevuaA4T4bXiV=64vkg@mail.gmail.com>
On Fri, Jan 24, 2014 at 12:15:08PM +0000, Fabio Estevam wrote:
> Hi Mark,
>
> On Fri, Jan 24, 2014 at 9:50 AM, Mark Rutland <mark.rutland@arm.com> wrote:
>
> >> +
> >> + regulators {
> >> + compatible = "simple-bus";
> >
> > This is _not_ a simple bus. It doesn't have the required ranges
> > property.
> >
> > Why do these need to be in a regulators container node? We don't group
> > dma controllers under a dmas node, or uarts under a uarts node.
>
> It seems we have this same issue on several imx6 dts files.
>
> Would the below address your suggestion?
The below patch looks good to me.
In general there seems to be a misunderstanding of the purpose of
simple-bus, as an annotation that children of a node should be probed,
rather than as a representation of a simple bus (which is of the same
type as the parent-bus, which requires no programming, where children
can be used without knowledge of the simple-bus).
There seem to be a lot of cases where simple-bus is used as a fallback
compatible string, when in reality the node's children make no sense
without information from and/or programming of the node with the
simple-bus property. Those cases are completely wrong, and a complete
abuse of simple-bus.
There are other cases where simple-bus nodes are used to group nodes
from a commonly reused block. As long as these have the requisite
ranges, #address-cells, and #size-cells, then they are valid, and make
sense for translating / widening addresses for common groups of
peripherals even if they're not on a different physical bus.
I don't see the point in grouping together nodes in an artificial
container because of their functionality rather than their topology, it
seems to breed confusion.
Cheers,
Mark.
>
> diff --git a/arch/arm/boot/dts/imx6qdl-sabresd.dtsi b/arch/arm/boot/dts/imx6qdl-
> index e75e11b..ba35560 100644
> --- a/arch/arm/boot/dts/imx6qdl-sabresd.dtsi
> +++ b/arch/arm/boot/dts/imx6qdl-sabresd.dtsi
> @@ -15,33 +15,29 @@
> reg = <0x10000000 0x40000000>;
> };
>
> - regulators {
> - compatible = "simple-bus";
> -
> - reg_usb_otg_vbus: usb_otg_vbus {
> - compatible = "regulator-fixed";
> - regulator-name = "usb_otg_vbus";
> - regulator-min-microvolt = <5000000>;
> - regulator-max-microvolt = <5000000>;
> - gpio = <&gpio3 22 0>;
> - enable-active-high;
> - };
> + reg_usb_otg_vbus: regulator at 0 {
> + compatible = "regulator-fixed";
> + regulator-name = "usb_otg_vbus";
> + regulator-min-microvolt = <5000000>;
> + regulator-max-microvolt = <5000000>;
> + gpio = <&gpio3 22 0>;
> + enable-active-high;
> + };
>
> - reg_usb_h1_vbus: usb_h1_vbus {
> - compatible = "regulator-fixed";
> - regulator-name = "usb_h1_vbus";
> - regulator-min-microvolt = <5000000>;
> - regulator-max-microvolt = <5000000>;
> - gpio = <&gpio1 29 0>;
> - enable-active-high;
> - };
> + reg_usb_h1_vbus: regulator at 1 {
> + compatible = "regulator-fixed";
> + regulator-name = "usb_h1_vbus";
> + regulator-min-microvolt = <5000000>;
> + regulator-max-microvolt = <5000000>;
> + gpio = <&gpio1 29 0>;
> + enable-active-high;
> + };
>
> - reg_audio: wm8962_supply {
> - compatible = "regulator-fixed";
> - regulator-name = "wm8962-supply";
> - gpio = <&gpio4 10 0>;
> - enable-active-high;
> - };
> + reg_audio: regulator at 2 {
> + compatible = "regulator-fixed";
> + regulator-name = "wm8962-supply";
> + gpio = <&gpio4 10 0>;
> + enable-active-high;
> };
>
> gpio-keys {
>
> If so, I will prepare some patches to update other dts files.
>
> Thanks,
>
> Fabio Estevam
>
^ permalink raw reply
* [PATCH 09/20] ARM64 / ACPI: Implement core functions for parsing MADT table
From: Hanjun Guo @ 2014-01-24 15:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <52E15756.4060705@arm.com>
Hi Marc,
On 2014?01?24? 01:54, Marc Zyngier wrote:
> Hi Hanjun,
>
> On 17/01/14 12:25, Hanjun Guo wrote:
>> Implement core functions for parsing MADT table to get the information
>> about GIC cpu interface and GIC distributor to prepare for SMP and GIC
>> initialization.
>>
>> Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
>> ---
>> arch/arm64/include/asm/acpi.h | 3 +
>> drivers/acpi/plat/arm-core.c | 139 ++++++++++++++++++++++++++++++++++++++++-
>> drivers/acpi/tables.c | 21 +++++++
>> 3 files changed, 162 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/include/asm/acpi.h b/arch/arm64/include/asm/acpi.h
>> index e108d9c..c335c6d 100644
>> --- a/arch/arm64/include/asm/acpi.h
>> +++ b/arch/arm64/include/asm/acpi.h
>> @@ -83,6 +83,9 @@ void arch_fix_phys_package_id(int num, u32 slot);
>> extern int (*acpi_suspend_lowlevel)(void);
>> #define acpi_wakeup_address (0)
>>
>> +#define MAX_GIC_CPU_INTERFACE 256
> I'll bite. Where on Earth is this value coming from?
I just thought 256 is big enough for now :(
Yes, should be a larger number for GICv3.
> If that's for
> GICv2, 8 is the maximum. For GICv3+, that's incredibly low, and should
> be probed probed at runtime anyway.
I would prefer to do that, but this value is used to
probe CPUs in MADT :)
>
>> +#define MAX_GIC_DISTRIBUTOR 1 /* should be the same as MAX_GIC_NR */
> No support for cascaded GICs?
Yes, no cascade GICs in ACPI at now.
>
>> +
>> #else /* !CONFIG_ACPI */
>> #define acpi_disabled 1 /* ACPI sometimes enabled on ARM */
>> #define acpi_noirq 1 /* ACPI sometimes enabled on ARM */
>> diff --git a/drivers/acpi/plat/arm-core.c b/drivers/acpi/plat/arm-core.c
>> index 1835b21..8ba3e6f 100644
>> --- a/drivers/acpi/plat/arm-core.c
>> +++ b/drivers/acpi/plat/arm-core.c
>> @@ -46,6 +46,16 @@ EXPORT_SYMBOL(acpi_disabled);
>> int acpi_pci_disabled; /* skip ACPI PCI scan and IRQ initialization */
>> EXPORT_SYMBOL(acpi_pci_disabled);
>>
>> +/*
>> + * Local interrupt controller address,
>> + * GIC cpu interface base address on ARM/ARM64
>> + */
>> +static u64 acpi_lapic_addr __initdata;
> If that's a GIC address, why not call it as such?
thanks for the suggesting, I will update.
>
>> +#define BAD_MADT_ENTRY(entry, end) ( \
>> + (!entry) || (unsigned long)entry + sizeof(*entry) > end || \
>> + ((struct acpi_subtable_header *)entry)->length < sizeof(*entry))
>> +
>> #define PREFIX "ACPI: "
> Just do:
> #define pr_fmt(fmt) "ACPI: " fmt
>
> and remove all the occurrences of PREFIX.
>
>> /* FIXME: this function should be moved to topology.c when it is ready */
>> @@ -92,6 +102,115 @@ void __init __acpi_unmap_table(char *map, unsigned long size)
>> return;
>> }
>>
>> +static int __init acpi_parse_madt(struct acpi_table_header *table)
>> +{
>> + struct acpi_table_madt *madt = NULL;
> No need to initialize this to NULL, you're doing an assignment at the
> next line...
>
>> +
>> + madt = (struct acpi_table_madt *)table;
>> + if (!madt) {
>> + pr_warn(PREFIX "Unable to map MADT\n");
> There is no mapping here, please fix the message accordingly.
Ok, I will address your comments above in next version.
>
>> + return -ENODEV;
>> + }
>> +
>> + if (madt->address) {
>> + acpi_lapic_addr = (u64) madt->address;
> So you're updating this static variable, for the distributor and each
> CPU interface? /me puzzled...
Good catch. So I have a question: do we really have some SoCs
without banked registers on ARM64? if not , I think we can use
a single static variable is ok.
>
>> + pr_info(PREFIX "Local APIC address 0x%08x\n", madt->address);
> Away with this APIC madness. GICC and GICD are the concepts we're all
> familiar with here, and using the proper terminology would certainly
> help reviewing these patches...
That make sense to me too, will update.
>
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +/*
>> + * GIC structures on ARM are somthing like Local APIC structures on x86,
>> + * which means GIC cpu interfaces for GICv2/v3. Every GIC structure in
>> + * MADT table represents a cpu in the system.
> And what do you do when your GICv3 doesn't have a memory-mapped
> interface, but only uses system registers?
>
>> + * GIC distributor structures are somthing like IOAPIC on x86. GIC can
>> + * be initialized with information in this structure.
>> + *
>> + * Please refer to chapter5.2.12.14/15 of ACPI 5.0
> A pointer to that documentation?
Please refer to http://www.acpi.info/
>
>> + */
>> +
>> +static int __init
>> +acpi_parse_gic(struct acpi_subtable_header *header, const unsigned long end)
>> +{
>> + struct acpi_madt_generic_interrupt *processor = NULL;
>> +
>> + processor = (struct acpi_madt_generic_interrupt *)header;
>> +
>> + if (BAD_MADT_ENTRY(processor, end))
>> + return -EINVAL;
>> +
>> + acpi_table_print_madt_entry(header);
>> +
>> + return 0;
>> +}
>> +
>> +static int __init
>> +acpi_parse_gic_distributor(struct acpi_subtable_header *header,
>> + const unsigned long end)
>> +{
>> + struct acpi_madt_generic_distributor *distributor = NULL;
>> +
>> + distributor = (struct acpi_madt_generic_distributor *)header;
>> +
>> + if (BAD_MADT_ENTRY(distributor, end))
>> + return -EINVAL;
>> +
>> + acpi_table_print_madt_entry(header);
>> +
>> + return 0;
>> +}
>> +
>> +/*
>> + * Parse GIC cpu interface related entries in MADT
>> + * returns 0 on success, < 0 on error
>> + */
>> +static int __init acpi_parse_madt_gic_entries(void)
>> +{
>> + int count;
>> +
>> + /*
>> + * do a partial walk of MADT to determine how many CPUs
>> + * we have including disabled CPUs
>> + */
>> + count = acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_INTERRUPT,
>> + acpi_parse_gic, MAX_GIC_CPU_INTERFACE);
>> +
>> + if (!count) {
>> + pr_err(PREFIX "No GIC entries present\n");
>> + return -ENODEV;
>> + } else if (count < 0) {
>> + pr_err(PREFIX "Error parsing GIC entry\n");
>> + return count;
>> + }
> So you do a lot of parsing to count stuff, and then discard the number
> of counted objects... You might as well check that there is at least one
> valid object and stop there.
>
>> + return 0;
>> +}
>> +
>> +/*
>> + * Parse GIC distributor related entries in MADT
>> + * returns 0 on success, < 0 on error
>> + */
>> +static int __init acpi_parse_madt_gic_distributor_entries(void)
>> +{
>> + int count;
>> +
>> + count = acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_DISTRIBUTOR,
>> + acpi_parse_gic_distributor, MAX_GIC_DISTRIBUTOR);
>> +
>> + if (!count) {
>> + pr_err(PREFIX "No GIC distributor entries present\n");
>> + return -ENODEV;
>> + } else if (count < 0) {
>> + pr_err(PREFIX "Error parsing GIC distributor entry\n");
>> + return count;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> int acpi_gsi_to_irq(u32 gsi, unsigned int *irq)
>> {
>> *irq = gsi_to_irq(gsi);
>> @@ -141,11 +260,29 @@ static int __init acpi_parse_fadt(struct acpi_table_header *table)
>>
>> static void __init early_acpi_process_madt(void)
>> {
>> - return;
>> + acpi_table_parse(ACPI_SIG_MADT, acpi_parse_madt);
>> }
>>
>> static void __init acpi_process_madt(void)
>> {
>> + int error;
>> +
>> + if (!acpi_table_parse(ACPI_SIG_MADT, acpi_parse_madt)) {
> How many times are you going to parse the same table? Surely you can
> stash whatever information you need and be done with it?
good catch, we already addressed this problem, and will
update in next version.
>
>> + /*
>> + * Parse MADT GIC cpu interface entries
>> + */
>> + error = acpi_parse_madt_gic_entries();
>> + if (!error) {
>> + /*
>> + * Parse MADT GIC distributor entries
>> + */
>> + acpi_parse_madt_gic_distributor_entries();
>> + }
>> + }
>> +
>> + pr_info("Using ACPI for processor (GIC) configuration information\n");
>> +
>> return;
>> }
>>
>> diff --git a/drivers/acpi/tables.c b/drivers/acpi/tables.c
>> index d67a1fe..b3e4615 100644
>> --- a/drivers/acpi/tables.c
>> +++ b/drivers/acpi/tables.c
>> @@ -191,6 +191,27 @@ void acpi_table_print_madt_entry(struct acpi_subtable_header *header)
>> }
>> break;
>>
>> + case ACPI_MADT_TYPE_GENERIC_INTERRUPT:
>> + {
>> + struct acpi_madt_generic_interrupt *p =
>> + (struct acpi_madt_generic_interrupt *)header;
>> + printk(KERN_INFO PREFIX
> Use pr_info
this function use printk always, should change all of them?
>
>> + "GIC (acpi_id[0x%04x] gic_id[0x%04x] %s)\n",
>> + p->uid, p->gic_id,
>> + (p->flags & ACPI_MADT_ENABLED) ? "enabled" : "disabled");
>> + }
>> + break;
>> +
>> + case ACPI_MADT_TYPE_GENERIC_DISTRIBUTOR:
>> + {
>> + struct acpi_madt_generic_distributor *p =
>> + (struct acpi_madt_generic_distributor *)header;
>> + printk(KERN_INFO PREFIX
>> + "GIC Distributor (id[0x%04x] address[0x%08llx] gsi_base[%d])\n",
>> + p->gic_id, p->base_address, p->global_irq_base);
>> + }
>> + break;
>> +
>> default:
>> printk(KERN_WARNING PREFIX
>> "Found unsupported MADT entry (type = 0x%x)\n",
>>
> Most of that code seems to be repeatedly parsing and printing stuff, and
> I fail to see what it actually does.
yes, just print some information when booting.
Thank you very much for the comments.
Hanjun
^ permalink raw reply
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