Re: [PATCH v11 7/9] coresight: add support for CPU debug module

[Leo Yan]

Hi Mathieu,

On Tue, May 23, 2017 at 11:39:16AM -0600, Mathieu Poirier wrote:

[...]

> > +static bool debug_enable;
> > +module_param_named(enable, debug_enable, bool, 0600);
> > +MODULE_PARM_DESC(enable, "Knob to enable debug functionality "
> > +"(default is 0, which means is disabled by default)");
> 
> Checkpatch complains about a quoted string split across multiple
> lines.  You can probably removes the second one since information
> about the default value is already detailed in patch 03/09.  In this
> case I suggest modifiying the description to:
> 
> "Control to enable coresight CPU debug functionality"

It's shame for checkpatch.pl warning. Although everytime I used script
before sending out patch set, should think a bit more to handle them
better. Have sent new patch set to dismiss them.

Thanks,
Leo Yan
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v12 1/9] coresight: bindings for CPU debug module

[Leo Yan]

According to ARMv8 architecture reference manual (ARM DDI 0487A.k)
Chapter 'Part H: External debug', the CPU can integrate debug module
and it can support self-hosted debug and external debug. Especially
for supporting self-hosted debug, this means the program can access
the debug module from mmio region; and usually the mmio region is
integrated with coresight.

So add document for binding debug component, includes binding to APB
clock; and also need specify the CPU node which the debug module is
dedicated to specific CPU.

Suggested-by: Mike Leach 
Reviewed-by: Mathieu Poirier 
Reviewed-by: Suzuki K Poulose 
Acked-by: Rob Herring 
Signed-off-by: Leo Yan 
---
 .../bindings/arm/coresight-cpu-debug.txt   | 49 ++
 1 file changed, 49 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/arm/coresight-cpu-debug.txt

diff --git a/Documentation/devicetree/bindings/arm/coresight-cpu-debug.txt 
b/Documentation/devicetree/bindings/arm/coresight-cpu-debug.txt
new file mode 100644
index 000..2982912
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/coresight-cpu-debug.txt
@@ -0,0 +1,49 @@
+* CoreSight CPU Debug Component:
+
+CoreSight CPU debug component are compliant with the ARMv8 architecture
+reference manual (ARM DDI 0487A.k) Chapter 'Part H: External debug'. The
+external debug module is mainly used for two modes: self-hosted debug and
+external debug, and it can be accessed from mmio region from Coresight
+and eventually the debug module connects with CPU for debugging. And the
+debug module provides sample-based profiling extension, which can be used
+to sample CPU program counter, secure state and exception level, etc;
+usually every CPU has one dedicated debug module to be connected.
+
+Required properties:
+
+- compatible : should be "arm,coresight-cpu-debug"; supplemented with
+   "arm,primecell" since this driver is using the AMBA bus
+  interface.
+
+- reg : physical base address and length of the register set.
+
+- clocks : the clock associated to this component.
+
+- clock-names : the name of the clock referenced by the code. Since we are
+using the AMBA framework, the name of the clock providing
+   the interconnect should be "apb_pclk" and the clock is
+   mandatory. The interface between the debug logic and the
+   processor core is clocked by the internal CPU clock, so it
+   is enabled with CPU clock by default.
+
+- cpu : the CPU phandle the debug module is affined to. When omitted
+   the module is considered to belong to CPU0.
+
+Optional properties:
+
+- power-domains: a phandle to the debug power domain. We use "power-domains"
+ binding to turn on the debug logic if it has own dedicated
+power domain and if necessary to use "cpuidle.off=1" or
+"nohlt" in the kernel command line or sysfs node to
+constrain idle states to ensure registers in the CPU power
+domain are accessible.
+
+Example:
+
+   debug@f659 {
+   compatible = "arm,coresight-cpu-debug","arm,primecell";
+   reg = <0 0xf659 0 0x1000>;
+   clocks = <_ctrl HI6220_DAPB_CLK>;
+   clock-names = "apb_pclk";
+   cpu = <>;
+   };
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v12 2/9] doc: Add documentation for Coresight CPU debug

[Leo Yan]

Add detailed documentation for Coresight CPU debug driver, which
contains the info for driver implementation, Mike Leach excellent
summary for "clock and power domain". At the end some examples on how
to enable the debugging functionality are provided.

Suggested-by: Mike Leach 
Reviewed-by: Mathieu Poirier 
Signed-off-by: Leo Yan 
---
 Documentation/trace/coresight-cpu-debug.txt | 175 
 1 file changed, 175 insertions(+)
 create mode 100644 Documentation/trace/coresight-cpu-debug.txt

diff --git a/Documentation/trace/coresight-cpu-debug.txt 
b/Documentation/trace/coresight-cpu-debug.txt
new file mode 100644
index 000..b3da1f9
--- /dev/null
+++ b/Documentation/trace/coresight-cpu-debug.txt
@@ -0,0 +1,175 @@
+   Coresight CPU Debug Module
+   ==
+
+   Author:   Leo Yan 
+   Date: April 5th, 2017
+
+Introduction
+
+
+Coresight CPU debug module is defined in ARMv8-a architecture reference manual
+(ARM DDI 0487A.k) Chapter 'Part H: External debug', the CPU can integrate
+debug module and it is mainly used for two modes: self-hosted debug and
+external debug. Usually the external debug mode is well known as the external
+debugger connects with SoC from JTAG port; on the other hand the program can
+explore debugging method which rely on self-hosted debug mode, this document
+is to focus on this part.
+
+The debug module provides sample-based profiling extension, which can be used
+to sample CPU program counter, secure state and exception level, etc; usually
+every CPU has one dedicated debug module to be connected. Based on self-hosted
+debug mechanism, Linux kernel can access these related registers from mmio
+region when the kernel panic happens. The callback notifier for kernel panic
+will dump related registers for every CPU; finally this is good for assistant
+analysis for panic.
+
+
+Implementation
+--
+
+- During driver registration, it uses EDDEVID and EDDEVID1 - two device ID
+  registers to decide if sample-based profiling is implemented or not. On some
+  platforms this hardware feature is fully or partially implemented; and if
+  this feature is not supported then registration will fail.
+
+- At the time this documentation was written, the debug driver mainly relies on
+  information gathered by the kernel panic callback notifier from three
+  sampling registers: EDPCSR, EDVIDSR and EDCIDSR: from EDPCSR we can get
+  program counter; EDVIDSR has information for secure state, exception level,
+  bit width, etc; EDCIDSR is context ID value which contains the sampled value
+  of CONTEXTIDR_EL1.
+
+- The driver supports a CPU running in either AArch64 or AArch32 mode. The
+  registers naming convention is a bit different between them, AArch64 uses
+  'ED' for register prefix (ARM DDI 0487A.k, chapter H9.1) and AArch32 uses
+  'DBG' as prefix (ARM DDI 0487A.k, chapter G5.1). The driver is unified to
+  use AArch64 naming convention.
+
+- ARMv8-a (ARM DDI 0487A.k) and ARMv7-a (ARM DDI 0406C.b) have different
+  register bits definition. So the driver consolidates two difference:
+
+  If PCSROffset=0b, on ARMv8-a the feature of EDPCSR is not implemented;
+  but ARMv7-a defines "PCSR samples are offset by a value that depends on the
+  instruction set state". For ARMv7-a, the driver checks furthermore if CPU
+  runs with ARM or thumb instruction set and calibrate PCSR value, the
+  detailed description for offset is in ARMv7-a ARM (ARM DDI 0406C.b) chapter
+  C11.11.34 "DBGPCSR, Program Counter Sampling Register".
+
+  If PCSROffset=0b0010, ARMv8-a defines "EDPCSR implemented, and samples have
+  no offset applied and do not sample the instruction set state in AArch32
+  state". So on ARMv8 if EDDEVID1.PCSROffset is 0b0010 and the CPU operates
+  in AArch32 state, EDPCSR is not sampled; when the CPU operates in AArch64
+  state EDPCSR is sampled and no offset are applied.
+
+
+Clock and power domain
+--
+
+Before accessing debug registers, we should ensure the clock and power domain
+have been enabled properly. In ARMv8-a ARM (ARM DDI 0487A.k) chapter 'H9.1
+Debug registers', the debug registers are spread into two domains: the debug
+domain and the CPU domain.
+
++---+
+|   |
+|   |
+ +--+--+|
+dbg_clock -->|  |**||<-- cpu_clock
+ |Debug |**|   CPU  |
+ dbg_power_domain -->|  |**||<-- cpu_power_domain
+ +--+--+|
+|   |
+|   |
++---+
+
+For debug domain, the user uses DT 

[PATCH v12 3/9] doc: Add coresight_cpu_debug.enable to kernel-parameters.txt

[Leo Yan]

Add coresight_cpu_debug.enable to kernel-parameters.txt, this flag is
used to enable/disable the CPU sampling based debugging.

Reviewed-by: Mathieu Poirier 
Signed-off-by: Leo Yan 
---
 Documentation/admin-guide/kernel-parameters.txt | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/admin-guide/kernel-parameters.txt 
b/Documentation/admin-guide/kernel-parameters.txt
index 15f79c2..ff67ad7 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -649,6 +649,13 @@
/proc//coredump_filter.
See also Documentation/filesystems/proc.txt.
 
+   coresight_cpu_debug.enable
+   [ARM,ARM64]
+   Format: 
+   Enable/disable the CPU sampling based debugging.
+   0: default value, disable debugging
+   1: enable debugging at boot time
+
cpuidle.off=1   [CPU_IDLE]
disable the cpuidle sub-system
 
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v12 5/9] coresight: of_get_coresight_platform_data: Add missing of_node_put

[Leo Yan]

From: Suzuki K Poulose 

The of_get_coresight_platform_data iterates over the possible CPU nodes
to find a given cpu phandle. However it does not drop the reference
to the node pointer returned by the of_get_coresight_platform_data.

This patch also introduces another minor fix is to use
of_cpu_device_node_get() to replace of_get_cpu_node().

Cc: Mathieu Poirier 
Reviewed-by: Suzuki K Poulose 
Signed-off-by: Suzuki K Poulose 
[Leo: minor tweaks for of_get_coresight_platform_data]
Signed-off-by: Leo Yan 
---
 drivers/hwtracing/coresight/of_coresight.c | 17 ++---
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/drivers/hwtracing/coresight/of_coresight.c 
b/drivers/hwtracing/coresight/of_coresight.c
index 2749853..225b2dd 100644
--- a/drivers/hwtracing/coresight/of_coresight.c
+++ b/drivers/hwtracing/coresight/of_coresight.c
@@ -109,7 +109,8 @@ of_get_coresight_platform_data(struct device *dev,
struct coresight_platform_data *pdata;
struct of_endpoint endpoint, rendpoint;
struct device *rdev;
-   struct device_node *dn;
+   bool found;
+   struct device_node *dn, *np;
struct device_node *ep = NULL;
struct device_node *rparent = NULL;
struct device_node *rport = NULL;
@@ -176,17 +177,19 @@ of_get_coresight_platform_data(struct device *dev,
} while (ep);
}
 
-   /* Affinity defaults to CPU0 */
-   pdata->cpu = 0;
dn = of_parse_phandle(node, "cpu", 0);
-   for (cpu = 0; dn && cpu < nr_cpu_ids; cpu++) {
-   if (dn == of_get_cpu_node(cpu, NULL)) {
-   pdata->cpu = cpu;
+   for_each_possible_cpu(cpu) {
+   np = of_cpu_device_node_get(cpu);
+   found = (dn == np);
+   of_node_put(np);
+   if (found)
break;
-   }
}
of_node_put(dn);
 
+   /* Affinity to CPU0 if no cpu nodes are found */
+   pdata->cpu = found ? cpu : 0;
+
return pdata;
 }
 EXPORT_SYMBOL_GPL(of_get_coresight_platform_data);
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v12 9/9] arm64: dts: qcom: msm8916: Add debug unit

[Leo Yan]

Add debug unit on Qualcomm msm8916 based platforms, including the
DragonBoard 410c board.

Reviewed-by: Mathieu Poirier 
Signed-off-by: Leo Yan 
---
 arch/arm64/boot/dts/qcom/msm8916.dtsi | 32 
 1 file changed, 32 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/msm8916.dtsi 
b/arch/arm64/boot/dts/qcom/msm8916.dtsi
index ab30939..17691ab 100644
--- a/arch/arm64/boot/dts/qcom/msm8916.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8916.dtsi
@@ -1116,6 +1116,38 @@
};
};
 
+   debug@85 {
+   compatible = "arm,coresight-cpu-debug","arm,primecell";
+   reg = <0x85 0x1000>;
+   clocks = < RPM_QDSS_CLK>;
+   clock-names = "apb_pclk";
+   cpu = <>;
+   };
+
+   debug@852000 {
+   compatible = "arm,coresight-cpu-debug","arm,primecell";
+   reg = <0x852000 0x1000>;
+   clocks = < RPM_QDSS_CLK>;
+   clock-names = "apb_pclk";
+   cpu = <>;
+   };
+
+   debug@854000 {
+   compatible = "arm,coresight-cpu-debug","arm,primecell";
+   reg = <0x854000 0x1000>;
+   clocks = < RPM_QDSS_CLK>;
+   clock-names = "apb_pclk";
+   cpu = <>;
+   };
+
+   debug@856000 {
+   compatible = "arm,coresight-cpu-debug","arm,primecell";
+   reg = <0x856000 0x1000>;
+   clocks = < RPM_QDSS_CLK>;
+   clock-names = "apb_pclk";
+   cpu = <>;
+   };
+
etm@85c000 {
compatible = "arm,coresight-etm4x", "arm,primecell";
reg = <0x85c000 0x1000>;
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v12 7/9] coresight: add support for CPU debug module

[Leo Yan]

Coresight includes debug module and usually the module connects with CPU
debug logic. ARMv8 architecture reference manual (ARM DDI 0487A.k) has
description for related info in "Part H: External Debug".

Chapter H7 "The Sample-based Profiling Extension" introduces several
sampling registers, e.g. we can check program counter value with
combined CPU exception level, secure state, etc. So this is helpful for
analysis CPU lockup scenarios, e.g. if one CPU has run into infinite
loop with IRQ disabled. In this case the CPU cannot switch context and
handle any interrupt (including IPIs), as the result it cannot handle
SMP call for stack dump.

This patch is to enable coresight debug module, so firstly this driver
is to bind apb clock for debug module and this is to ensure the debug
module can be accessed from program or external debugger. And the driver
uses sample-based registers for debug purpose, e.g. when system triggers
panic, the driver will dump program counter and combined context
registers (EDCIDSR, EDVIDSR); by parsing context registers so can
quickly get to know CPU secure state, exception level, etc.

Some of the debug module registers are located in CPU power domain, so
this requires the CPU power domain stays on when access related debug
registers, but the power management for CPU power domain is quite
dependent on SoC integration for power management. For the platforms
which with sane power controller implementations, this driver follows
the method to set EDPRCR to try to pull the CPU out of low power state
and then set 'no power down request' bit so the CPU has no chance to
lose power.

If the SoC has not followed up this design well for power management
controller, the user should use the command line parameter or sysfs
to constrain all or partial idle states to ensure the CPU power
domain is enabled and access coresight CPU debug component safely.

Reviewed-by: Suzuki K Poulose 
Signed-off-by: Leo Yan 
---
 drivers/hwtracing/coresight/Kconfig   |  14 +
 drivers/hwtracing/coresight/Makefile  |   1 +
 drivers/hwtracing/coresight/coresight-cpu-debug.c | 696 ++
 3 files changed, 711 insertions(+)
 create mode 100644 drivers/hwtracing/coresight/coresight-cpu-debug.c

diff --git a/drivers/hwtracing/coresight/Kconfig 
b/drivers/hwtracing/coresight/Kconfig
index 130cb21..8d55d6d 100644
--- a/drivers/hwtracing/coresight/Kconfig
+++ b/drivers/hwtracing/coresight/Kconfig
@@ -89,4 +89,18 @@ config CORESIGHT_STM
  logging useful software events or data coming from various entities
  in the system, possibly running different OSs
 
+config CORESIGHT_CPU_DEBUG
+   tristate "CoreSight CPU Debug driver"
+   depends on ARM || ARM64
+   depends on DEBUG_FS
+   help
+ This driver provides support for coresight debugging module. This
+ is primarily used to dump sample-based profiling registers when
+ system triggers panic, the driver will parse context registers so
+ can quickly get to know program counter (PC), secure state,
+ exception level, etc. Before use debugging functionality, platform
+ needs to ensure the clock domain and power domain are enabled
+ properly, please refer Documentation/trace/coresight-cpu-debug.txt
+ for detailed description and the example for usage.
+
 endif
diff --git a/drivers/hwtracing/coresight/Makefile 
b/drivers/hwtracing/coresight/Makefile
index af480d9..433d590 100644
--- a/drivers/hwtracing/coresight/Makefile
+++ b/drivers/hwtracing/coresight/Makefile
@@ -16,3 +16,4 @@ obj-$(CONFIG_CORESIGHT_SOURCE_ETM4X) += coresight-etm4x.o \
coresight-etm4x-sysfs.o
 obj-$(CONFIG_CORESIGHT_QCOM_REPLICATOR) += coresight-replicator-qcom.o
 obj-$(CONFIG_CORESIGHT_STM) += coresight-stm.o
+obj-$(CONFIG_CORESIGHT_CPU_DEBUG) += coresight-cpu-debug.o
diff --git a/drivers/hwtracing/coresight/coresight-cpu-debug.c 
b/drivers/hwtracing/coresight/coresight-cpu-debug.c
new file mode 100644
index 000..f5c1235
--- /dev/null
+++ b/drivers/hwtracing/coresight/coresight-cpu-debug.c
@@ -0,0 +1,696 @@
+/*
+ * Copyright (c) 2017 Linaro Limited. All rights reserved.
+ *
+ * Author: Leo Yan 
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ *
+ * 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.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see .
+ *
+ */
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 

[PATCH v12 8/9] arm64: dts: hi6220: register debug module

[Leo Yan]

Bind debug module driver for Hi6220.

Reviewed-by: Mathieu Poirier 
Signed-off-by: Leo Yan 
---
 arch/arm64/boot/dts/hisilicon/hi6220.dtsi | 64 +++
 1 file changed, 64 insertions(+)

diff --git a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi 
b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
index 1e5129b..21805b9 100644
--- a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
+++ b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
@@ -916,5 +916,69 @@
};
};
};
+
+   debug@f659 {
+   compatible = "arm,coresight-cpu-debug","arm,primecell";
+   reg = <0 0xf659 0 0x1000>;
+   clocks = <_ctrl HI6220_DAPB_CLK>;
+   clock-names = "apb_pclk";
+   cpu = <>;
+   };
+
+   debug@f6592000 {
+   compatible = "arm,coresight-cpu-debug","arm,primecell";
+   reg = <0 0xf6592000 0 0x1000>;
+   clocks = <_ctrl HI6220_DAPB_CLK>;
+   clock-names = "apb_pclk";
+   cpu = <>;
+   };
+
+   debug@f6594000 {
+   compatible = "arm,coresight-cpu-debug","arm,primecell";
+   reg = <0 0xf6594000 0 0x1000>;
+   clocks = <_ctrl HI6220_DAPB_CLK>;
+   clock-names = "apb_pclk";
+   cpu = <>;
+   };
+
+   debug@f6596000 {
+   compatible = "arm,coresight-cpu-debug","arm,primecell";
+   reg = <0 0xf6596000 0 0x1000>;
+   clocks = <_ctrl HI6220_DAPB_CLK>;
+   clock-names = "apb_pclk";
+   cpu = <>;
+   };
+
+   debug@f65d {
+   compatible = "arm,coresight-cpu-debug","arm,primecell";
+   reg = <0 0xf65d 0 0x1000>;
+   clocks = <_ctrl HI6220_DAPB_CLK>;
+   clock-names = "apb_pclk";
+   cpu = <>;
+   };
+
+   debug@f65d2000 {
+   compatible = "arm,coresight-cpu-debug","arm,primecell";
+   reg = <0 0xf65d2000 0 0x1000>;
+   clocks = <_ctrl HI6220_DAPB_CLK>;
+   clock-names = "apb_pclk";
+   cpu = <>;
+   };
+
+   debug@f65d4000 {
+   compatible = "arm,coresight-cpu-debug","arm,primecell";
+   reg = <0 0xf65d4000 0 0x1000>;
+   clocks = <_ctrl HI6220_DAPB_CLK>;
+   clock-names = "apb_pclk";
+   cpu = <>;
+   };
+
+   debug@f65d6000 {
+   compatible = "arm,coresight-cpu-debug","arm,primecell";
+   reg = <0 0xf65d6000 0 0x1000>;
+   clocks = <_ctrl HI6220_DAPB_CLK>;
+   clock-names = "apb_pclk";
+   cpu = <>;
+   };
};
 };
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v12 6/9] coresight: refactor with function of_coresight_get_cpu

[Leo Yan]

This is refactor to add function of_coresight_get_cpu(), so it's used to
retrieve CPU id for coresight component. Finally can use it as a common
function for multiple places.

Suggested-by: Mathieu Poirier 
Reviewed-by: Suzuki K Poulose 
Signed-off-by: Leo Yan 
---
 drivers/hwtracing/coresight/of_coresight.c | 43 +++---
 include/linux/coresight.h  |  3 +++
 2 files changed, 31 insertions(+), 15 deletions(-)

diff --git a/drivers/hwtracing/coresight/of_coresight.c 
b/drivers/hwtracing/coresight/of_coresight.c
index 225b2dd..a187941 100644
--- a/drivers/hwtracing/coresight/of_coresight.c
+++ b/drivers/hwtracing/coresight/of_coresight.c
@@ -101,16 +101,40 @@ static int of_coresight_alloc_memory(struct device *dev,
return 0;
 }
 
+int of_coresight_get_cpu(const struct device_node *node)
+{
+   int cpu;
+   bool found;
+   struct device_node *dn, *np;
+
+   dn = of_parse_phandle(node, "cpu", 0);
+
+   /* Affinity defaults to CPU0 */
+   if (!dn)
+   return 0;
+
+   for_each_possible_cpu(cpu) {
+   np = of_cpu_device_node_get(cpu);
+   found = (dn == np);
+   of_node_put(np);
+   if (found)
+   break;
+   }
+   of_node_put(dn);
+
+   /* Affinity to CPU0 if no cpu nodes are found */
+   return found ? cpu : 0;
+}
+EXPORT_SYMBOL_GPL(of_coresight_get_cpu);
+
 struct coresight_platform_data *
 of_get_coresight_platform_data(struct device *dev,
   const struct device_node *node)
 {
-   int i = 0, ret = 0, cpu;
+   int i = 0, ret = 0;
struct coresight_platform_data *pdata;
struct of_endpoint endpoint, rendpoint;
struct device *rdev;
-   bool found;
-   struct device_node *dn, *np;
struct device_node *ep = NULL;
struct device_node *rparent = NULL;
struct device_node *rport = NULL;
@@ -177,18 +201,7 @@ of_get_coresight_platform_data(struct device *dev,
} while (ep);
}
 
-   dn = of_parse_phandle(node, "cpu", 0);
-   for_each_possible_cpu(cpu) {
-   np = of_cpu_device_node_get(cpu);
-   found = (dn == np);
-   of_node_put(np);
-   if (found)
-   break;
-   }
-   of_node_put(dn);
-
-   /* Affinity to CPU0 if no cpu nodes are found */
-   pdata->cpu = found ? cpu : 0;
+   pdata->cpu = of_coresight_get_cpu(node);
 
return pdata;
 }
diff --git a/include/linux/coresight.h b/include/linux/coresight.h
index bf0aa50..d950dad 100644
--- a/include/linux/coresight.h
+++ b/include/linux/coresight.h
@@ -263,10 +263,13 @@ static inline int coresight_timeout(void __iomem *addr, 
u32 offset,
 #endif
 
 #ifdef CONFIG_OF
+extern int of_coresight_get_cpu(const struct device_node *node);
 extern struct coresight_platform_data *
 of_get_coresight_platform_data(struct device *dev,
   const struct device_node *node);
 #else
+static inline int of_coresight_get_cpu(const struct device_node *node)
+{ return 0; }
 static inline struct coresight_platform_data *of_get_coresight_platform_data(
struct device *dev, const struct device_node *node) { return NULL; }
 #endif
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v12 0/9] coresight: enable debug module

[Leo Yan]

ARMv8 architecture reference manual (ARM DDI 0487A.k) Chapter H7 "The
Sample-based Profiling Extension" has description for sampling
registers, we can utilize these registers to check program counter
value with combined CPU exception level, secure state, etc. So this is
helpful for CPU lockup bugs, e.g. if one CPU has run into infinite loop
with IRQ disabled; the 'hang' CPU cannot switch context and handle any
interrupt, so it cannot handle SMP call for stack dump, etc.

This patch series is to enable coresight debug module with sample-based
registers and register call back notifier for PCSR register dumping
when panic happens, so we can see below dumping info for panic; and
this patch series has considered the conditions for access permission
for debug registers self, so this can avoid access debug registers when
CPU power domain is off; the driver also try to figure out the CPU is
in secure or non-secure state.

Patch 0001 is to document the dt binding; patch 0002 adds one detailed
document to describe the Coresight debug module implementation, the
clock and power domain impaction on the driver, some examples for usage.

Patch 0003 is to document boot parameters used in kernel command line.

Patch 0004 is to add file entries for MAINTAINERS.

Patch 0005 is used to fix the func of_get_coresight_platform_data()
doesn't properly drop the reference to the CPU node pointer; and
patch 0006 is refactor to add new function of_coresight_get_cpu().

Patch 0007 is the driver for CPU debug module.

Patch 0008 in this series are to enable debug unit on 96boards Hikey,
Patch 0009 is to enable debug on 96boards DB410c. Have verified on both
two boards.

We can enable debugging with two methods, adding parameters into kernel
command line for build-in module:
  coresight_cpu_debug.enable=1

Or we can wait the system has booted up to use debugfs nodes to enable
debugging:
  # echo 1 > /sys/kernel/debug/coresight_cpu_debug/enable

As result we can get below log after input command:
echo c > /proc/sysrq-trigger:

ARM external debug module:
coresight-cpu-debug 85.debug: CPU[0]:
coresight-cpu-debug 85.debug:  EDPRSR:  0001 (Power:On DLK:Unlock)
coresight-cpu-debug 85.debug:  EDPCSR:  [] 
handle_IPI+0x174/0x1d8
coresight-cpu-debug 85.debug:  EDCIDSR: 
coresight-cpu-debug 85.debug:  EDVIDSR: 9000 (State:Non-secure 
Mode:EL1/0 Width:64bits VMID:0)
coresight-cpu-debug 852000.debug: CPU[1]:
coresight-cpu-debug 852000.debug:  EDPRSR:  0001 (Power:On DLK:Unlock)
coresight-cpu-debug 852000.debug:  EDPCSR:  [] 
debug_notifier_call+0x23c/0x358
coresight-cpu-debug 852000.debug:  EDCIDSR: 
coresight-cpu-debug 852000.debug:  EDVIDSR: 9000 (State:Non-secure 
Mode:EL1/0 Width:64bits VMID:0)

[...]

Changes from v11:
* Dismissed checkpatch.pl warning about "quoted string split across
  multiple lines" and "line over 80 characters".

Changes from v10:
* Followed Liviu suggestion to improve readability of the documentation.
* ARM Juno DTS binding patch has been picked by Sudeep, so this patch
  set has not included anymore. Great!

Changes from v9:
* Used dev_xyz() to replace pr_xyz() for print log.
* Added DT binding patch for Juno shared by Suzuki.

Changes from v8:
* According to Mathieu suggestions to split the doc into two patches,
  one is for kernel parameter and another is for driver documentation.
* Add file entries to MAINTAINERS.
* According to Mathieu suggestions, refined functions
  debug_enable_func()/debug_disable_func().

Changes from v7:
* Fix operator priority bug.
* Minor sequence adjustment for function debug_func_exit().

Changes from v6:
* According to Suzuki and Mathieu suggestions, refined debug module
  driver to install panic notifier when insmod module; refined function
  debug_force_cpu_powered_up() for CPU power state checking; some minor
  fixing for output log, adding comments for memory barrier, code
  alignment.

Changes from v5:
* According to Suzuki and Mathieu suggestions, refined debug module
  driver to drop unused structure members, refactored initialization
  code to distinguish hardware implementation features, refactored
  flow for forcing CPU powered up, supported pm_runtime operations.
* Added one new doc file: Documentation/trace/coresight-cpu-debug.txt,
  which is used to describe detailed info for implementation, clock
  and power domain impaction on debug module, and exmaples for common
  usage.
* Removed "idle constraints" from debug driver.

Changes from v4:
* This version is mainly credit to ARM colleagues many contribution
  ideas for better quality (Thanks a lot Suzuki, Mike and Sudeep!).
* According to Suzuki suggestion, refined debug module driver to avoid
  memory leak for drvdata struct, handle PCSAMPLE_MODE=1, use flag
  drvdata.pc_has_offset to indicate if PCSR has offset, minor fixes.
* According to Mathieu suggestion, refined dt binding description.
* Changed driver to support module mode;
* According to Mike suggestion and very 

Re: [kernel-hardening] [PATCH v4 next 0/3] modules: automatic module loading restrictions

[Andy Lutomirski]

On Tue, May 23, 2017 at 11:36 AM, Kees Cook  wrote:
> On Tue, May 23, 2017 at 12:48 AM, Solar Designer  wrote:
>> For modules_autoload_mode=2, we already seem to have the equivalent of
>> modprobe=/bin/true (or does it differ subtly, maybe in return values?),
>> which I already use at startup on a GPU box like this (preloading
>> modules so that the OpenCL backends wouldn't need the autoloading):
>>
>> nvidia-smi
>> nvidia-modprobe -u -c=0
>> #modprobe nvidia_uvm
>> #modprobe fglrx
>>
>> sysctl -w kernel.modprobe=/bin/true
>> sysctl -w kernel.hotplug=/bin/true
>>
>> but it's good to also have this supported more explicitly and more
>> consistently through modules_autoload_mode=2 while we're at it.  So I
>> support having this mode as well.  I just question the need to have it
>> non-resettable.
>
> I agree it's useful to have the explicit =2 state just to avoid
> confusion when more systems start implementing
> CONFIG_STATIC_USERMODEHELPER and kernel.modprobe becomes read-only
> (though the userspace implementation may allow for some way to disable
> it, etc). I just like avoiding the upcall to modprobe at all.

I fully support =2 to mean "no automatic loading at all".  I dislike
making it non-resettable.  If you can write to sysctls, then, most
likely you can either call init_module() directly or the system has
module loading disabled entirely.
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 next 1/3] modules:capabilities: allow __request_module() to take a capability argument

[Kees Cook]

On Tue, May 23, 2017 at 3:29 AM, Djalal Harouni  wrote:
> On Tue, May 23, 2017 at 12:20 AM, Kees Cook  wrote:
>> On Mon, May 22, 2017 at 4:57 AM, Djalal Harouni  wrote:
>>> This is a preparation patch for the module auto-load restriction feature.
>>>
>>> In order to restrict module auto-load operations we need to check if the
>>> caller has CAP_SYS_MODULE capability. This allows to align security
>>> checks of automatic module loading with the checks of the explicit 
>>> operations.
>>>
>>> However for "netdev-%s" modules, they are allowed to be loaded if
>>> CAP_NET_ADMIN is set. Therefore, in order to not break this assumption,
>>> and allow userspace to only load "netdev-%s" modules with CAP_NET_ADMIN
>>> capability which is considered a privileged operation, we have two
>>> choices: 1) parse "netdev-%s" alias and check the capability or 2) hand
>>> the capability form request_module() to security_kernel_module_request()
>>> hook and let the capability subsystem decide.
>>>
>>> After a discussion with Rusty Russell [1], the suggestion was to pass
>>> the capability from request_module() to security_kernel_module_request()
>>> for 'netdev-%s' modules that need CAP_NET_ADMIN.
>>>
>>> The patch does not update request_module(), it updates the internal
>>> __request_module() that will take an extra "allow_cap" argument. If
>>> positive, then automatic module load operation can be allowed.
>>
>> I find this refactor slightly confusing. I would expect to collapse
>> the existing caps checks in net/core/dev_ioctl.c and
>> net/ipv4/tcp_cong.c, and make this a "required cap" argument, and to
>> add a new non-__ function instead of requiring callers use
>> __request_module.
>>
>> request_module_capable(int cap_required, fmt, args);
>>
>> adjust __request_module() for the new arg, and when cap_required !=
>> -1, perform a cap check.
>>
>> Then make request_module pass -1 to __request_module(), and change
>> dev_ioctl.c (and tcp_cong.c) from:
>>
>> if (no_module && capable(CAP_NET_ADMIN))
>> no_module = request_module("netdev-%s", name);
>> if (no_module && capable(CAP_SYS_MODULE))
>> request_module("%s", name);
>>
>> to:
>>
>> if (no_module)
>> no_module = request_module_capable(CAP_NET_ADMIN,
>> "netdev-%s", name);
>> if (no_module)
>> no_module = request_module_capable(CAP_SYS_MODULE, "%s", 
>> name);
>>
>> that'll make the code cleaner, too.
>
> The refactoring in the patch is more for backward compatibility with
> CAP_NET_ADMIN,
> as discussed here: https://lkml.org/lkml/2017/4/26/147

I think Rusty and I are saying the same thing here, and I must be not
understanding something you're trying to explain. Apologies for being
dense.

> I think if there is an interface request_module_capable() , then code
> will use it. The DCCP code path did not check capabilities at all and
> called request_module(), other code does the same.
>
> A new interface can be abused, the result of this: we may break
> "modules_autoload_mode" in mode 0 and 1. In the long term code will
> want to change may_autoload_module() to also allow mode 1 to load a
> module with CAP_NET_ADMIN or other caps in its own userns, resulting
> in "modules_autoload_mode == 0 == 1". Without userns in the game we
> may just see request_module_capable(CAP_SYS_ADMIN, ...)  . There is
> already some code maybe phonet sockets ? that require CAP_SYS_ADMIN to
> get the appropriate protocol and no one will be able to review all
> this code or track new patches with request_module_capable() callers.

I'm having some trouble following what you're saying here, but if I
understand, you're worried about getting the kernel into a state where
autoload state 0 == 1. Autoload 0 is "business as usual", and autoload
1 is "CAP_SYS_MODULE required to be able to trigger a module auto-load
operation, or CAP_NET_ADMIN for modules with a 'netdev-%s' alias."

In the v4 patch, under autoload==1, CAP_NET_ADMIN is needed to load
netdev- modules:

if (no_module && capable(CAP_NET_ADMIN))
   no_module = __request_module(true, CAP_NET_ADMIN,
"netdev-%s", name);

and in the LSM hook, CAP_NET_ADMIN is passed as an allowable "alias"
for the CAP_SYS_MODULE requirement:

   else if (modules_autoload_mode == MODULES_AUTOLOAD_PRIVILEGED) {
   /* Check CAP_SYS_MODULE then allow_cap if valid */
   if (capable(CAP_SYS_MODULE) ||
   (allow_cap > 0 && capable(allow_cap)))
  return 0;
   }

What I see is some needless double-checking. Since you're making
changes to the request_module() API, it would be possible to have
request_module_cap(), which could be checked instead of open-coding
it:

 if (no_module)
no_module = request_module_cap(CAP_NET_ADMIN, "netdev-%s", name);

If I'm understanding your 

Re: [kernel-hardening] [PATCH v4 next 0/3] modules: automatic module loading restrictions

[Kees Cook]

On Tue, May 23, 2017 at 12:48 AM, Solar Designer  wrote:
> For modules_autoload_mode=2, we already seem to have the equivalent of
> modprobe=/bin/true (or does it differ subtly, maybe in return values?),
> which I already use at startup on a GPU box like this (preloading
> modules so that the OpenCL backends wouldn't need the autoloading):
>
> nvidia-smi
> nvidia-modprobe -u -c=0
> #modprobe nvidia_uvm
> #modprobe fglrx
>
> sysctl -w kernel.modprobe=/bin/true
> sysctl -w kernel.hotplug=/bin/true
>
> but it's good to also have this supported more explicitly and more
> consistently through modules_autoload_mode=2 while we're at it.  So I
> support having this mode as well.  I just question the need to have it
> non-resettable.

I agree it's useful to have the explicit =2 state just to avoid
confusion when more systems start implementing
CONFIG_STATIC_USERMODEHELPER and kernel.modprobe becomes read-only
(though the userspace implementation may allow for some way to disable
it, etc). I just like avoiding the upcall to modprobe at all.

-Kees

-- 
Kees Cook
Pixel Security
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v11 7/9] coresight: add support for CPU debug module

[Mathieu Poirier]

On 23 May 2017 at 08:16, Leo Yan  wrote:
> Coresight includes debug module and usually the module connects with CPU
> debug logic. ARMv8 architecture reference manual (ARM DDI 0487A.k) has
> description for related info in "Part H: External Debug".
>
> Chapter H7 "The Sample-based Profiling Extension" introduces several
> sampling registers, e.g. we can check program counter value with
> combined CPU exception level, secure state, etc. So this is helpful for
> analysis CPU lockup scenarios, e.g. if one CPU has run into infinite
> loop with IRQ disabled. In this case the CPU cannot switch context and
> handle any interrupt (including IPIs), as the result it cannot handle
> SMP call for stack dump.
>
> This patch is to enable coresight debug module, so firstly this driver
> is to bind apb clock for debug module and this is to ensure the debug
> module can be accessed from program or external debugger. And the driver
> uses sample-based registers for debug purpose, e.g. when system triggers
> panic, the driver will dump program counter and combined context
> registers (EDCIDSR, EDVIDSR); by parsing context registers so can
> quickly get to know CPU secure state, exception level, etc.
>
> Some of the debug module registers are located in CPU power domain, so
> this requires the CPU power domain stays on when access related debug
> registers, but the power management for CPU power domain is quite
> dependent on SoC integration for power management. For the platforms
> which with sane power controller implementations, this driver follows
> the method to set EDPRCR to try to pull the CPU out of low power state
> and then set 'no power down request' bit so the CPU has no chance to
> lose power.
>
> If the SoC has not followed up this design well for power management
> controller, the user should use the command line parameter or sysfs
> to constrain all or partial idle states to ensure the CPU power
> domain is enabled and access coresight CPU debug component safely.
>
> Reviewed-by: Suzuki K Poulose 
> Signed-off-by: Leo Yan 
> ---
>  drivers/hwtracing/coresight/Kconfig   |  14 +
>  drivers/hwtracing/coresight/Makefile  |   1 +
>  drivers/hwtracing/coresight/coresight-cpu-debug.c | 696 
> ++
>  3 files changed, 711 insertions(+)
>  create mode 100644 drivers/hwtracing/coresight/coresight-cpu-debug.c
>
> diff --git a/drivers/hwtracing/coresight/Kconfig 
> b/drivers/hwtracing/coresight/Kconfig
> index 130cb21..8d55d6d 100644
> --- a/drivers/hwtracing/coresight/Kconfig
> +++ b/drivers/hwtracing/coresight/Kconfig
> @@ -89,4 +89,18 @@ config CORESIGHT_STM
>   logging useful software events or data coming from various entities
>   in the system, possibly running different OSs
>
> +config CORESIGHT_CPU_DEBUG
> +   tristate "CoreSight CPU Debug driver"
> +   depends on ARM || ARM64
> +   depends on DEBUG_FS
> +   help
> + This driver provides support for coresight debugging module. This
> + is primarily used to dump sample-based profiling registers when
> + system triggers panic, the driver will parse context registers so
> + can quickly get to know program counter (PC), secure state,
> + exception level, etc. Before use debugging functionality, platform
> + needs to ensure the clock domain and power domain are enabled
> + properly, please refer Documentation/trace/coresight-cpu-debug.txt
> + for detailed description and the example for usage.
> +
>  endif
> diff --git a/drivers/hwtracing/coresight/Makefile 
> b/drivers/hwtracing/coresight/Makefile
> index af480d9..433d590 100644
> --- a/drivers/hwtracing/coresight/Makefile
> +++ b/drivers/hwtracing/coresight/Makefile
> @@ -16,3 +16,4 @@ obj-$(CONFIG_CORESIGHT_SOURCE_ETM4X) += coresight-etm4x.o \
> coresight-etm4x-sysfs.o
>  obj-$(CONFIG_CORESIGHT_QCOM_REPLICATOR) += coresight-replicator-qcom.o
>  obj-$(CONFIG_CORESIGHT_STM) += coresight-stm.o
> +obj-$(CONFIG_CORESIGHT_CPU_DEBUG) += coresight-cpu-debug.o
> diff --git a/drivers/hwtracing/coresight/coresight-cpu-debug.c 
> b/drivers/hwtracing/coresight/coresight-cpu-debug.c
> new file mode 100644
> index 000..3589174
> --- /dev/null
> +++ b/drivers/hwtracing/coresight/coresight-cpu-debug.c
> @@ -0,0 +1,696 @@
> +/*
> + * Copyright (c) 2017 Linaro Limited. All rights reserved.
> + *
> + * Author: Leo Yan 
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License version 2 as published 
> by
> + * the Free Software Foundation.
> + *
> + * 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 

Re: [PATCH v11 9/9] arm64: dts: qcom: msm8916: Add debug unit

[Mathieu Poirier]

On 23 May 2017 at 08:16, Leo Yan  wrote:
> Add debug unit on Qualcomm msm8916 based platforms, including the
> DragonBoard 410c board.
>
> Reviewed-by: Mathieu Poirier 
> Signed-off-by: Leo Yan 

Andy and David,

Would you mind taking a look a this when you have a minute?  If it's
fine with you we can make this go through my tree with the rest of the
driver.

Regards,
Mathieu


> ---
>  arch/arm64/boot/dts/qcom/msm8916.dtsi | 32 
>  1 file changed, 32 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/msm8916.dtsi 
> b/arch/arm64/boot/dts/qcom/msm8916.dtsi
> index ab30939..17691ab 100644
> --- a/arch/arm64/boot/dts/qcom/msm8916.dtsi
> +++ b/arch/arm64/boot/dts/qcom/msm8916.dtsi
> @@ -1116,6 +1116,38 @@
> };
> };
>
> +   debug@85 {
> +   compatible = 
> "arm,coresight-cpu-debug","arm,primecell";
> +   reg = <0x85 0x1000>;
> +   clocks = < RPM_QDSS_CLK>;
> +   clock-names = "apb_pclk";
> +   cpu = <>;
> +   };
> +
> +   debug@852000 {
> +   compatible = 
> "arm,coresight-cpu-debug","arm,primecell";
> +   reg = <0x852000 0x1000>;
> +   clocks = < RPM_QDSS_CLK>;
> +   clock-names = "apb_pclk";
> +   cpu = <>;
> +   };
> +
> +   debug@854000 {
> +   compatible = 
> "arm,coresight-cpu-debug","arm,primecell";
> +   reg = <0x854000 0x1000>;
> +   clocks = < RPM_QDSS_CLK>;
> +   clock-names = "apb_pclk";
> +   cpu = <>;
> +   };
> +
> +   debug@856000 {
> +   compatible = 
> "arm,coresight-cpu-debug","arm,primecell";
> +   reg = <0x856000 0x1000>;
> +   clocks = < RPM_QDSS_CLK>;
> +   clock-names = "apb_pclk";
> +   cpu = <>;
> +   };
> +
> etm@85c000 {
> compatible = "arm,coresight-etm4x", "arm,primecell";
> reg = <0x85c000 0x1000>;
> --
> 2.7.4
>
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v11 8/9] arm64: dts: hi6220: register debug module

[Mathieu Poirier]

On 23 May 2017 at 08:16, Leo Yan  wrote:
> Bind debug module driver for Hi6220.
>
> Reviewed-by: Mathieu Poirier 
> Signed-off-by: Leo Yan 

Wei and Mark,

I think it makes sense to have this go through the coresight tree
along with the rest of the driver.  Please have a look and let me
know.

Thanks,
Mathieu

> ---
>  arch/arm64/boot/dts/hisilicon/hi6220.dtsi | 64 
> +++
>  1 file changed, 64 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi 
> b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
> index 1e5129b..21805b9 100644
> --- a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
> +++ b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
> @@ -916,5 +916,69 @@
> };
> };
> };
> +
> +   debug@f659 {
> +   compatible = 
> "arm,coresight-cpu-debug","arm,primecell";
> +   reg = <0 0xf659 0 0x1000>;
> +   clocks = <_ctrl HI6220_DAPB_CLK>;
> +   clock-names = "apb_pclk";
> +   cpu = <>;
> +   };
> +
> +   debug@f6592000 {
> +   compatible = 
> "arm,coresight-cpu-debug","arm,primecell";
> +   reg = <0 0xf6592000 0 0x1000>;
> +   clocks = <_ctrl HI6220_DAPB_CLK>;
> +   clock-names = "apb_pclk";
> +   cpu = <>;
> +   };
> +
> +   debug@f6594000 {
> +   compatible = 
> "arm,coresight-cpu-debug","arm,primecell";
> +   reg = <0 0xf6594000 0 0x1000>;
> +   clocks = <_ctrl HI6220_DAPB_CLK>;
> +   clock-names = "apb_pclk";
> +   cpu = <>;
> +   };
> +
> +   debug@f6596000 {
> +   compatible = 
> "arm,coresight-cpu-debug","arm,primecell";
> +   reg = <0 0xf6596000 0 0x1000>;
> +   clocks = <_ctrl HI6220_DAPB_CLK>;
> +   clock-names = "apb_pclk";
> +   cpu = <>;
> +   };
> +
> +   debug@f65d {
> +   compatible = 
> "arm,coresight-cpu-debug","arm,primecell";
> +   reg = <0 0xf65d 0 0x1000>;
> +   clocks = <_ctrl HI6220_DAPB_CLK>;
> +   clock-names = "apb_pclk";
> +   cpu = <>;
> +   };
> +
> +   debug@f65d2000 {
> +   compatible = 
> "arm,coresight-cpu-debug","arm,primecell";
> +   reg = <0 0xf65d2000 0 0x1000>;
> +   clocks = <_ctrl HI6220_DAPB_CLK>;
> +   clock-names = "apb_pclk";
> +   cpu = <>;
> +   };
> +
> +   debug@f65d4000 {
> +   compatible = 
> "arm,coresight-cpu-debug","arm,primecell";
> +   reg = <0 0xf65d4000 0 0x1000>;
> +   clocks = <_ctrl HI6220_DAPB_CLK>;
> +   clock-names = "apb_pclk";
> +   cpu = <>;
> +   };
> +
> +   debug@f65d6000 {
> +   compatible = 
> "arm,coresight-cpu-debug","arm,primecell";
> +   reg = <0 0xf65d6000 0 0x1000>;
> +   clocks = <_ctrl HI6220_DAPB_CLK>;
> +   clock-names = "apb_pclk";
> +   cpu = <>;
> +   };
> };
>  };
> --
> 2.7.4
>
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 13/41] mmc: host: omap_hsmmc: Add support to set IODELAY values

[Rob Herring]

On Fri, May 19, 2017 at 01:45:13PM +0530, Kishon Vijay Abraham I wrote:
> The data manual of J6/J6 Eco recommends to set different IODELAY values
> depending on the mode in which the MMC/SD is enumerated in order to
> ensure IO timings are met.
> 
> Add support to set the IODELAY values depending on the various MMC
> modes using the pinctrl APIs.
> 
> Signed-off-by: Kishon Vijay Abraham I 
> [nsek...@ti.com: introduce OMAP_HSMMC_SETUP_PINCTRL()]
> Signed-off-by: Sekhar Nori 
> ---
>  .../devicetree/bindings/mmc/ti-omap-hsmmc.txt  |   5 +
>  drivers/mmc/host/omap_hsmmc.c  | 124 
> -
>  include/linux/platform_data/hsmmc-omap.h   |   3 +
>  3 files changed, 129 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt 
> b/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt
> index 258e25af10f7..dcf0b777c031 100644
> --- a/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt
> +++ b/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt
> @@ -20,6 +20,11 @@ Optional properties:
>  ti,dual-volt: boolean, supports dual voltage cards
>  -supply: phandle to the regulator device tree node
>  "supply-name" examples are "vmmc", "vmmc_aux" etc
> +pinctrl-names: Should be a list of pinctrl state names and can be "sdr104",
> +"hs200_1_8v", "ddr50", "sdr50", "sdr25", "sdr12", "hs", "ddr_1_8v" or
> +"default".
> +pinctrl-: Phandle referencing pin configuration of the sd/emmc 
> controller.
> +See: Documentation/devicetree/bindings/pinctrl/pinctrl-binding.txt
>  ti,non-removable: non-removable slot (like eMMC)
>  ti,needs-special-reset: Requires a special softreset sequence
>  ti,needs-special-hs-handling: HSMMC IP needs special setting for handling 
> High Speed

Binding looks fine, but...

> @@ -2388,6 +2427,73 @@ static inline struct omap_hsmmc_platform_data
>  }
>  #endif
>  
> +#define OMAP_HSMMC_SETUP_PINCTRL(capvar, capmask, mode)  
> \
> + do {\

This is kind of ugly. Why not make it a function. Just return 
pinctrl_state ptr.

> + struct pinctrl_state *s = ERR_PTR(-ENODEV); \
> + char str[20];   \
> + char *version = host->pdata->version;   \
> + \
> + if (!(mmc->capvar & (capmask))) \
> + break;  \
> + \
> + if (host->pdata->version) { \
> + sprintf(str, "%s-%s", #mode, version);  \
> + s = pinctrl_lookup_state(host->pinctrl, str);   \
> + }   \
> + \
> + if (IS_ERR(s)) {\
> + sprintf(str, "%s", #mode);  \
> + s = pinctrl_lookup_state(host->pinctrl, str);   \
> + }   \
> + \
> + if (IS_ERR(s)) {\
> + dev_err(host->dev, "no pinctrl state for %s "   \
> + "mode\n", #mode);   \
> + mmc->capvar &= ~(capmask);  \
> + } else {\
> + host->mode##_pinctrl_state = s; \
> + }   \
> + \
> + } while (0)
> +
> +static int omap_hsmmc_get_iodelay_pinctrl_state(struct omap_hsmmc_host *host)
> +{
> + struct mmc_host *mmc = host->mmc;
> +
> + if (!(host->pdata->controller_flags & OMAP_HSMMC_REQUIRE_IODELAY))
> + return 0;
> +
> + host->pinctrl = devm_pinctrl_get(host->dev);
> + if (IS_ERR(host->pinctrl)) {
> + dev_err(host->dev, "Cannot get pinctrl\n");
> + return PTR_ERR(host->pinctrl);
> + }
> +
> + host->default_pinctrl_state = pinctrl_lookup_state(host->pinctrl,
> +"default");
> + if (IS_ERR(host->default_pinctrl_state)) {
> + dev_err(host->dev,
> + "no pinctrl state for default mode\n");
> + return PTR_ERR(host->default_pinctrl_state);
> + }
> +
> + OMAP_HSMMC_SETUP_PINCTRL(caps,  MMC_CAP_UHS_SDR104, sdr104);
> + OMAP_HSMMC_SETUP_PINCTRL(caps,  MMC_CAP_UHS_DDR50, 

Re: [PATCH 2/6] module: use list_for_each_entry_rcu() on find_module_all()

[Miroslav Benes]

On Thu, 18 May 2017, Luis R. Rodriguez wrote:

> The module list has been using RCU in a lot of other calls
> for a while now, we just overlooked changing this one over to
> use RCU.
> 
> Signed-off-by: Luis R. Rodriguez 
> ---
>  kernel/module.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/module.c b/kernel/module.c
> index 4a3665f8f837..70f494638974 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -602,7 +602,7 @@ static struct module *find_module_all(const char *name, 
> size_t len,
>  
>   module_assert_mutex_or_preempt();
>  
> - list_for_each_entry(mod, , list) {
> + list_for_each_entry_rcu(mod, , list) {
>   if (!even_unformed && mod->state == MODULE_STATE_UNFORMED)
>   continue;
>   if (strlen(mod->name) == len && !memcmp(mod->name, name, len))

This makes sense. It would not be an issue if all callers of 
find_module_all() held module_mutex, but module_kallsyms_lookup_name() 
does not.

There is one more occurrence of just list_for_each_entry(mod, , 
list) in kernel/module.c -- in module_kallsyms_on_each_symbol(). But that 
one is safe because we have to hold the mutex there.

Miroslav
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 11/41] mmc: host: omap_hsmmc: Add new compatible string to support dra7

[Rob Herring]

On Fri, May 19, 2017 at 01:45:11PM +0530, Kishon Vijay Abraham I wrote:
> Add a new compatible string "ti,dra7-hsmmc" to support
> dra7 and dra72 controllers. Also create a new controller flag
> "OMAP_HSMMC_REQUIRE_IODELAY" to specify all controllers that use
> "ti,dra7-hsmmc" require iodealy configuration to be set.
> 
> Signed-off-by: Kishon Vijay Abraham I 
> Signed-off-by: Sekhar Nori 
> ---
>  Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt |  1 +
>  drivers/mmc/host/omap_hsmmc.c   | 10 ++
>  include/linux/platform_data/hsmmc-omap.h|  1 +
>  3 files changed, 12 insertions(+)

Acked-by: Rob Herring 

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v11 0/9] coresight: enable debug module

[Leo Yan]

ARMv8 architecture reference manual (ARM DDI 0487A.k) Chapter H7 "The
Sample-based Profiling Extension" has description for sampling
registers, we can utilize these registers to check program counter
value with combined CPU exception level, secure state, etc. So this is
helpful for CPU lockup bugs, e.g. if one CPU has run into infinite loop
with IRQ disabled; the 'hang' CPU cannot switch context and handle any
interrupt, so it cannot handle SMP call for stack dump, etc.

This patch series is to enable coresight debug module with sample-based
registers and register call back notifier for PCSR register dumping
when panic happens, so we can see below dumping info for panic; and
this patch series has considered the conditions for access permission
for debug registers self, so this can avoid access debug registers when
CPU power domain is off; the driver also try to figure out the CPU is
in secure or non-secure state.

Patch 0001 is to document the dt binding; patch 0002 adds one detailed
document to describe the Coresight debug module implementation, the
clock and power domain impaction on the driver, some examples for usage.

Patch 0003 is to document boot parameters used in kernel command line.

Patch 0004 is to add file entries for MAINTAINERS.

Patch 0005 is used to fix the func of_get_coresight_platform_data()
doesn't properly drop the reference to the CPU node pointer; and
patch 0006 is refactor to add new function of_coresight_get_cpu().

Patch 0007 is the driver for CPU debug module.

Patch 0008 in this series are to enable debug unit on 96boards Hikey,
Patch 0009 is to enable debug on 96boards DB410c. Have verified on both
two boards.

We can enable debugging with two methods, adding parameters into kernel
command line for build-in module:
  coresight_cpu_debug.enable=1

Or we can wait the system has booted up to use debugfs nodes to enable
debugging:
  # echo 1 > /sys/kernel/debug/coresight_cpu_debug/enable

As result we can get below log after input command:
echo c > /proc/sysrq-trigger:

ARM external debug module:
coresight-cpu-debug 85.debug: CPU[0]:
coresight-cpu-debug 85.debug:  EDPRSR:  0001 (Power:On DLK:Unlock)
coresight-cpu-debug 85.debug:  EDPCSR:  [] 
handle_IPI+0x174/0x1d8
coresight-cpu-debug 85.debug:  EDCIDSR: 
coresight-cpu-debug 85.debug:  EDVIDSR: 9000 (State:Non-secure 
Mode:EL1/0 Width:64bits VMID:0)
coresight-cpu-debug 852000.debug: CPU[1]:
coresight-cpu-debug 852000.debug:  EDPRSR:  0001 (Power:On DLK:Unlock)
coresight-cpu-debug 852000.debug:  EDPCSR:  [] 
debug_notifier_call+0x23c/0x358
coresight-cpu-debug 852000.debug:  EDCIDSR: 
coresight-cpu-debug 852000.debug:  EDVIDSR: 9000 (State:Non-secure 
Mode:EL1/0 Width:64bits VMID:0)

[...]

Changes from v10:
* Followed Liviu suggestion to improve readability of the documentation.
* ARM Juno DTS binding patch has been picked by Sudeep, so this patch
  set has not included anymore. Great!

Changes from v9:
* Used dev_xyz() to replace pr_xyz() for print log.
* Added DT binding patch for Juno shared by Suzuki.

Changes from v8:
* According to Mathieu suggestions to split the doc into two patches,
  one is for kernel parameter and another is for driver documentation.
* Add file entries to MAINTAINERS.
* According to Mathieu suggestions, refined functions
  debug_enable_func()/debug_disable_func().

Changes from v7:
* Fix operator priority bug.
* Minor sequence adjustment for function debug_func_exit().

Changes from v6:
* According to Suzuki and Mathieu suggestions, refined debug module
  driver to install panic notifier when insmod module; refined function
  debug_force_cpu_powered_up() for CPU power state checking; some minor
  fixing for output log, adding comments for memory barrier, code
  alignment.

Changes from v5:
* According to Suzuki and Mathieu suggestions, refined debug module
  driver to drop unused structure members, refactored initialization
  code to distinguish hardware implementation features, refactored
  flow for forcing CPU powered up, supported pm_runtime operations.
* Added one new doc file: Documentation/trace/coresight-cpu-debug.txt,
  which is used to describe detailed info for implementation, clock
  and power domain impaction on debug module, and exmaples for common
  usage.
* Removed "idle constraints" from debug driver.

Changes from v4:
* This version is mainly credit to ARM colleagues many contribution
  ideas for better quality (Thanks a lot Suzuki, Mike and Sudeep!).
* According to Suzuki suggestion, refined debug module driver to avoid
  memory leak for drvdata struct, handle PCSAMPLE_MODE=1, use flag
  drvdata.pc_has_offset to indicate if PCSR has offset, minor fixes.
* According to Mathieu suggestion, refined dt binding description.
* Changed driver to support module mode;
* According to Mike suggestion and very appreciate the pseudo code,
  added support to force CPU powered up with register EDPRCR;
* According to discussions, added command line 

[PATCH v11 5/9] coresight: of_get_coresight_platform_data: Add missing of_node_put

[Leo Yan]

From: Suzuki K Poulose 

The of_get_coresight_platform_data iterates over the possible CPU nodes
to find a given cpu phandle. However it does not drop the reference
to the node pointer returned by the of_get_coresight_platform_data.

This patch also introduces another minor fix is to use
of_cpu_device_node_get() to replace of_get_cpu_node().

Cc: Mathieu Poirier 
Reviewed-by: Suzuki K Poulose 
Signed-off-by: Suzuki K Poulose 
[Leo: minor tweaks for of_get_coresight_platform_data]
Signed-off-by: Leo Yan 
---
 drivers/hwtracing/coresight/of_coresight.c | 17 ++---
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/drivers/hwtracing/coresight/of_coresight.c 
b/drivers/hwtracing/coresight/of_coresight.c
index 2749853..225b2dd 100644
--- a/drivers/hwtracing/coresight/of_coresight.c
+++ b/drivers/hwtracing/coresight/of_coresight.c
@@ -109,7 +109,8 @@ of_get_coresight_platform_data(struct device *dev,
struct coresight_platform_data *pdata;
struct of_endpoint endpoint, rendpoint;
struct device *rdev;
-   struct device_node *dn;
+   bool found;
+   struct device_node *dn, *np;
struct device_node *ep = NULL;
struct device_node *rparent = NULL;
struct device_node *rport = NULL;
@@ -176,17 +177,19 @@ of_get_coresight_platform_data(struct device *dev,
} while (ep);
}
 
-   /* Affinity defaults to CPU0 */
-   pdata->cpu = 0;
dn = of_parse_phandle(node, "cpu", 0);
-   for (cpu = 0; dn && cpu < nr_cpu_ids; cpu++) {
-   if (dn == of_get_cpu_node(cpu, NULL)) {
-   pdata->cpu = cpu;
+   for_each_possible_cpu(cpu) {
+   np = of_cpu_device_node_get(cpu);
+   found = (dn == np);
+   of_node_put(np);
+   if (found)
break;
-   }
}
of_node_put(dn);
 
+   /* Affinity to CPU0 if no cpu nodes are found */
+   pdata->cpu = found ? cpu : 0;
+
return pdata;
 }
 EXPORT_SYMBOL_GPL(of_get_coresight_platform_data);
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v11 6/9] coresight: refactor with function of_coresight_get_cpu

[Leo Yan]

This is refactor to add function of_coresight_get_cpu(), so it's used to
retrieve CPU id for coresight component. Finally can use it as a common
function for multiple places.

Suggested-by: Mathieu Poirier 
Reviewed-by: Suzuki K Poulose 
Signed-off-by: Leo Yan 
---
 drivers/hwtracing/coresight/of_coresight.c | 43 +++---
 include/linux/coresight.h  |  3 +++
 2 files changed, 31 insertions(+), 15 deletions(-)

diff --git a/drivers/hwtracing/coresight/of_coresight.c 
b/drivers/hwtracing/coresight/of_coresight.c
index 225b2dd..a187941 100644
--- a/drivers/hwtracing/coresight/of_coresight.c
+++ b/drivers/hwtracing/coresight/of_coresight.c
@@ -101,16 +101,40 @@ static int of_coresight_alloc_memory(struct device *dev,
return 0;
 }
 
+int of_coresight_get_cpu(const struct device_node *node)
+{
+   int cpu;
+   bool found;
+   struct device_node *dn, *np;
+
+   dn = of_parse_phandle(node, "cpu", 0);
+
+   /* Affinity defaults to CPU0 */
+   if (!dn)
+   return 0;
+
+   for_each_possible_cpu(cpu) {
+   np = of_cpu_device_node_get(cpu);
+   found = (dn == np);
+   of_node_put(np);
+   if (found)
+   break;
+   }
+   of_node_put(dn);
+
+   /* Affinity to CPU0 if no cpu nodes are found */
+   return found ? cpu : 0;
+}
+EXPORT_SYMBOL_GPL(of_coresight_get_cpu);
+
 struct coresight_platform_data *
 of_get_coresight_platform_data(struct device *dev,
   const struct device_node *node)
 {
-   int i = 0, ret = 0, cpu;
+   int i = 0, ret = 0;
struct coresight_platform_data *pdata;
struct of_endpoint endpoint, rendpoint;
struct device *rdev;
-   bool found;
-   struct device_node *dn, *np;
struct device_node *ep = NULL;
struct device_node *rparent = NULL;
struct device_node *rport = NULL;
@@ -177,18 +201,7 @@ of_get_coresight_platform_data(struct device *dev,
} while (ep);
}
 
-   dn = of_parse_phandle(node, "cpu", 0);
-   for_each_possible_cpu(cpu) {
-   np = of_cpu_device_node_get(cpu);
-   found = (dn == np);
-   of_node_put(np);
-   if (found)
-   break;
-   }
-   of_node_put(dn);
-
-   /* Affinity to CPU0 if no cpu nodes are found */
-   pdata->cpu = found ? cpu : 0;
+   pdata->cpu = of_coresight_get_cpu(node);
 
return pdata;
 }
diff --git a/include/linux/coresight.h b/include/linux/coresight.h
index bf0aa50..d950dad 100644
--- a/include/linux/coresight.h
+++ b/include/linux/coresight.h
@@ -263,10 +263,13 @@ static inline int coresight_timeout(void __iomem *addr, 
u32 offset,
 #endif
 
 #ifdef CONFIG_OF
+extern int of_coresight_get_cpu(const struct device_node *node);
 extern struct coresight_platform_data *
 of_get_coresight_platform_data(struct device *dev,
   const struct device_node *node);
 #else
+static inline int of_coresight_get_cpu(const struct device_node *node)
+{ return 0; }
 static inline struct coresight_platform_data *of_get_coresight_platform_data(
struct device *dev, const struct device_node *node) { return NULL; }
 #endif
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 next 3/3] modules:capabilities: add a per-task modules auto-load mode

[kbuild test robot]

Hi Djalal,

[auto build test ERROR on next-20170522]

url:
https://github.com/0day-ci/linux/commits/Djalal-Harouni/modules-automatic-module-loading-restrictions/20170523-193128
config: sparc64-allnoconfig (attached as .config)
compiler: sparc64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
wget 
https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=sparc64 

All errors (new ones prefixed by >>):

   In file included from arch/sparc/kernel/setup_64.c:32:0:
   include/linux/module.h:683:30: error: storage class specified for parameter 
'__module_address'
static inline struct module *__module_address(unsigned long addr)
 ^~~~
   include/linux/module.h:683:30: error: parameter '__module_address' declared 
'inline' [-Werror]
>> include/linux/module.h:684:1: error: 'always_inline' attribute ignored 
>> [-Werror=attributes]
{
^
   include/linux/module.h:683:30: error: 'no_instrument_function' attribute 
applies only to functions
static inline struct module *__module_address(unsigned long addr)
 ^~~~
   include/linux/module.h:684:1: error: expected ';', ',' or ')' before '{' 
token
{
^
   cc1: all warnings being treated as errors

vim +/always_inline +684 include/linux/module.h

98702500 Djalal Harouni 2017-05-22  677  static inline int 
task_modules_autoload_mode(struct task_struct *task)
98702500 Djalal Harouni 2017-05-22  678  {
98702500 Djalal Harouni 2017-05-22  679 return -ENOSYS;
98702500 Djalal Harouni 2017-05-22  680  }
98702500 Djalal Harouni 2017-05-22  681  
98702500 Djalal Harouni 2017-05-22  682  static inline bool 
within_module_core(unsigned long addr,
e610499e Rusty Russell  2009-03-31 @683  static inline struct module 
*__module_address(unsigned long addr)
e610499e Rusty Russell  2009-03-31 @684  {
e610499e Rusty Russell  2009-03-31  685 return NULL;
e610499e Rusty Russell  2009-03-31  686  }
e610499e Rusty Russell  2009-03-31  687  

:: The code at line 684 was first introduced by commit
:: e610499e2656e61975affd0af56b26eb73964c84 module: __module_address

:: TO: Rusty Russell <ru...@rustcorp.com.au>
:: CC: Rusty Russell <ru...@rustcorp.com.au>

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

[PATCH v11 7/9] coresight: add support for CPU debug module

[Leo Yan]

Coresight includes debug module and usually the module connects with CPU
debug logic. ARMv8 architecture reference manual (ARM DDI 0487A.k) has
description for related info in "Part H: External Debug".

Chapter H7 "The Sample-based Profiling Extension" introduces several
sampling registers, e.g. we can check program counter value with
combined CPU exception level, secure state, etc. So this is helpful for
analysis CPU lockup scenarios, e.g. if one CPU has run into infinite
loop with IRQ disabled. In this case the CPU cannot switch context and
handle any interrupt (including IPIs), as the result it cannot handle
SMP call for stack dump.

This patch is to enable coresight debug module, so firstly this driver
is to bind apb clock for debug module and this is to ensure the debug
module can be accessed from program or external debugger. And the driver
uses sample-based registers for debug purpose, e.g. when system triggers
panic, the driver will dump program counter and combined context
registers (EDCIDSR, EDVIDSR); by parsing context registers so can
quickly get to know CPU secure state, exception level, etc.

Some of the debug module registers are located in CPU power domain, so
this requires the CPU power domain stays on when access related debug
registers, but the power management for CPU power domain is quite
dependent on SoC integration for power management. For the platforms
which with sane power controller implementations, this driver follows
the method to set EDPRCR to try to pull the CPU out of low power state
and then set 'no power down request' bit so the CPU has no chance to
lose power.

If the SoC has not followed up this design well for power management
controller, the user should use the command line parameter or sysfs
to constrain all or partial idle states to ensure the CPU power
domain is enabled and access coresight CPU debug component safely.

Reviewed-by: Suzuki K Poulose 
Signed-off-by: Leo Yan 
---
 drivers/hwtracing/coresight/Kconfig   |  14 +
 drivers/hwtracing/coresight/Makefile  |   1 +
 drivers/hwtracing/coresight/coresight-cpu-debug.c | 696 ++
 3 files changed, 711 insertions(+)
 create mode 100644 drivers/hwtracing/coresight/coresight-cpu-debug.c

diff --git a/drivers/hwtracing/coresight/Kconfig 
b/drivers/hwtracing/coresight/Kconfig
index 130cb21..8d55d6d 100644
--- a/drivers/hwtracing/coresight/Kconfig
+++ b/drivers/hwtracing/coresight/Kconfig
@@ -89,4 +89,18 @@ config CORESIGHT_STM
  logging useful software events or data coming from various entities
  in the system, possibly running different OSs
 
+config CORESIGHT_CPU_DEBUG
+   tristate "CoreSight CPU Debug driver"
+   depends on ARM || ARM64
+   depends on DEBUG_FS
+   help
+ This driver provides support for coresight debugging module. This
+ is primarily used to dump sample-based profiling registers when
+ system triggers panic, the driver will parse context registers so
+ can quickly get to know program counter (PC), secure state,
+ exception level, etc. Before use debugging functionality, platform
+ needs to ensure the clock domain and power domain are enabled
+ properly, please refer Documentation/trace/coresight-cpu-debug.txt
+ for detailed description and the example for usage.
+
 endif
diff --git a/drivers/hwtracing/coresight/Makefile 
b/drivers/hwtracing/coresight/Makefile
index af480d9..433d590 100644
--- a/drivers/hwtracing/coresight/Makefile
+++ b/drivers/hwtracing/coresight/Makefile
@@ -16,3 +16,4 @@ obj-$(CONFIG_CORESIGHT_SOURCE_ETM4X) += coresight-etm4x.o \
coresight-etm4x-sysfs.o
 obj-$(CONFIG_CORESIGHT_QCOM_REPLICATOR) += coresight-replicator-qcom.o
 obj-$(CONFIG_CORESIGHT_STM) += coresight-stm.o
+obj-$(CONFIG_CORESIGHT_CPU_DEBUG) += coresight-cpu-debug.o
diff --git a/drivers/hwtracing/coresight/coresight-cpu-debug.c 
b/drivers/hwtracing/coresight/coresight-cpu-debug.c
new file mode 100644
index 000..3589174
--- /dev/null
+++ b/drivers/hwtracing/coresight/coresight-cpu-debug.c
@@ -0,0 +1,696 @@
+/*
+ * Copyright (c) 2017 Linaro Limited. All rights reserved.
+ *
+ * Author: Leo Yan 
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ *
+ * 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.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see .
+ *
+ */
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 

[PATCH v11 8/9] arm64: dts: hi6220: register debug module

[Leo Yan]

Bind debug module driver for Hi6220.

Reviewed-by: Mathieu Poirier 
Signed-off-by: Leo Yan 
---
 arch/arm64/boot/dts/hisilicon/hi6220.dtsi | 64 +++
 1 file changed, 64 insertions(+)

diff --git a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi 
b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
index 1e5129b..21805b9 100644
--- a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
+++ b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
@@ -916,5 +916,69 @@
};
};
};
+
+   debug@f659 {
+   compatible = "arm,coresight-cpu-debug","arm,primecell";
+   reg = <0 0xf659 0 0x1000>;
+   clocks = <_ctrl HI6220_DAPB_CLK>;
+   clock-names = "apb_pclk";
+   cpu = <>;
+   };
+
+   debug@f6592000 {
+   compatible = "arm,coresight-cpu-debug","arm,primecell";
+   reg = <0 0xf6592000 0 0x1000>;
+   clocks = <_ctrl HI6220_DAPB_CLK>;
+   clock-names = "apb_pclk";
+   cpu = <>;
+   };
+
+   debug@f6594000 {
+   compatible = "arm,coresight-cpu-debug","arm,primecell";
+   reg = <0 0xf6594000 0 0x1000>;
+   clocks = <_ctrl HI6220_DAPB_CLK>;
+   clock-names = "apb_pclk";
+   cpu = <>;
+   };
+
+   debug@f6596000 {
+   compatible = "arm,coresight-cpu-debug","arm,primecell";
+   reg = <0 0xf6596000 0 0x1000>;
+   clocks = <_ctrl HI6220_DAPB_CLK>;
+   clock-names = "apb_pclk";
+   cpu = <>;
+   };
+
+   debug@f65d {
+   compatible = "arm,coresight-cpu-debug","arm,primecell";
+   reg = <0 0xf65d 0 0x1000>;
+   clocks = <_ctrl HI6220_DAPB_CLK>;
+   clock-names = "apb_pclk";
+   cpu = <>;
+   };
+
+   debug@f65d2000 {
+   compatible = "arm,coresight-cpu-debug","arm,primecell";
+   reg = <0 0xf65d2000 0 0x1000>;
+   clocks = <_ctrl HI6220_DAPB_CLK>;
+   clock-names = "apb_pclk";
+   cpu = <>;
+   };
+
+   debug@f65d4000 {
+   compatible = "arm,coresight-cpu-debug","arm,primecell";
+   reg = <0 0xf65d4000 0 0x1000>;
+   clocks = <_ctrl HI6220_DAPB_CLK>;
+   clock-names = "apb_pclk";
+   cpu = <>;
+   };
+
+   debug@f65d6000 {
+   compatible = "arm,coresight-cpu-debug","arm,primecell";
+   reg = <0 0xf65d6000 0 0x1000>;
+   clocks = <_ctrl HI6220_DAPB_CLK>;
+   clock-names = "apb_pclk";
+   cpu = <>;
+   };
};
 };
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v11 9/9] arm64: dts: qcom: msm8916: Add debug unit

[Leo Yan]

Add debug unit on Qualcomm msm8916 based platforms, including the
DragonBoard 410c board.

Reviewed-by: Mathieu Poirier 
Signed-off-by: Leo Yan 
---
 arch/arm64/boot/dts/qcom/msm8916.dtsi | 32 
 1 file changed, 32 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/msm8916.dtsi 
b/arch/arm64/boot/dts/qcom/msm8916.dtsi
index ab30939..17691ab 100644
--- a/arch/arm64/boot/dts/qcom/msm8916.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8916.dtsi
@@ -1116,6 +1116,38 @@
};
};
 
+   debug@85 {
+   compatible = "arm,coresight-cpu-debug","arm,primecell";
+   reg = <0x85 0x1000>;
+   clocks = < RPM_QDSS_CLK>;
+   clock-names = "apb_pclk";
+   cpu = <>;
+   };
+
+   debug@852000 {
+   compatible = "arm,coresight-cpu-debug","arm,primecell";
+   reg = <0x852000 0x1000>;
+   clocks = < RPM_QDSS_CLK>;
+   clock-names = "apb_pclk";
+   cpu = <>;
+   };
+
+   debug@854000 {
+   compatible = "arm,coresight-cpu-debug","arm,primecell";
+   reg = <0x854000 0x1000>;
+   clocks = < RPM_QDSS_CLK>;
+   clock-names = "apb_pclk";
+   cpu = <>;
+   };
+
+   debug@856000 {
+   compatible = "arm,coresight-cpu-debug","arm,primecell";
+   reg = <0x856000 0x1000>;
+   clocks = < RPM_QDSS_CLK>;
+   clock-names = "apb_pclk";
+   cpu = <>;
+   };
+
etm@85c000 {
compatible = "arm,coresight-etm4x", "arm,primecell";
reg = <0x85c000 0x1000>;
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v11 1/9] coresight: bindings for CPU debug module

[Leo Yan]

According to ARMv8 architecture reference manual (ARM DDI 0487A.k)
Chapter 'Part H: External debug', the CPU can integrate debug module
and it can support self-hosted debug and external debug. Especially
for supporting self-hosted debug, this means the program can access
the debug module from mmio region; and usually the mmio region is
integrated with coresight.

So add document for binding debug component, includes binding to APB
clock; and also need specify the CPU node which the debug module is
dedicated to specific CPU.

Suggested-by: Mike Leach 
Reviewed-by: Mathieu Poirier 
Reviewed-by: Suzuki K Poulose 
Acked-by: Rob Herring 
Signed-off-by: Leo Yan 
---
 .../bindings/arm/coresight-cpu-debug.txt   | 49 ++
 1 file changed, 49 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/arm/coresight-cpu-debug.txt

diff --git a/Documentation/devicetree/bindings/arm/coresight-cpu-debug.txt 
b/Documentation/devicetree/bindings/arm/coresight-cpu-debug.txt
new file mode 100644
index 000..2982912
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/coresight-cpu-debug.txt
@@ -0,0 +1,49 @@
+* CoreSight CPU Debug Component:
+
+CoreSight CPU debug component are compliant with the ARMv8 architecture
+reference manual (ARM DDI 0487A.k) Chapter 'Part H: External debug'. The
+external debug module is mainly used for two modes: self-hosted debug and
+external debug, and it can be accessed from mmio region from Coresight
+and eventually the debug module connects with CPU for debugging. And the
+debug module provides sample-based profiling extension, which can be used
+to sample CPU program counter, secure state and exception level, etc;
+usually every CPU has one dedicated debug module to be connected.
+
+Required properties:
+
+- compatible : should be "arm,coresight-cpu-debug"; supplemented with
+   "arm,primecell" since this driver is using the AMBA bus
+  interface.
+
+- reg : physical base address and length of the register set.
+
+- clocks : the clock associated to this component.
+
+- clock-names : the name of the clock referenced by the code. Since we are
+using the AMBA framework, the name of the clock providing
+   the interconnect should be "apb_pclk" and the clock is
+   mandatory. The interface between the debug logic and the
+   processor core is clocked by the internal CPU clock, so it
+   is enabled with CPU clock by default.
+
+- cpu : the CPU phandle the debug module is affined to. When omitted
+   the module is considered to belong to CPU0.
+
+Optional properties:
+
+- power-domains: a phandle to the debug power domain. We use "power-domains"
+ binding to turn on the debug logic if it has own dedicated
+power domain and if necessary to use "cpuidle.off=1" or
+"nohlt" in the kernel command line or sysfs node to
+constrain idle states to ensure registers in the CPU power
+domain are accessible.
+
+Example:
+
+   debug@f659 {
+   compatible = "arm,coresight-cpu-debug","arm,primecell";
+   reg = <0 0xf659 0 0x1000>;
+   clocks = <_ctrl HI6220_DAPB_CLK>;
+   clock-names = "apb_pclk";
+   cpu = <>;
+   };
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v11 2/9] doc: Add documentation for Coresight CPU debug

[Leo Yan]

Add detailed documentation for Coresight CPU debug driver, which
contains the info for driver implementation, Mike Leach excellent
summary for "clock and power domain". At the end some examples on how
to enable the debugging functionality are provided.

Suggested-by: Mike Leach 
Reviewed-by: Mathieu Poirier 
Signed-off-by: Leo Yan 
---
 Documentation/trace/coresight-cpu-debug.txt | 175 
 1 file changed, 175 insertions(+)
 create mode 100644 Documentation/trace/coresight-cpu-debug.txt

diff --git a/Documentation/trace/coresight-cpu-debug.txt 
b/Documentation/trace/coresight-cpu-debug.txt
new file mode 100644
index 000..b3da1f9
--- /dev/null
+++ b/Documentation/trace/coresight-cpu-debug.txt
@@ -0,0 +1,175 @@
+   Coresight CPU Debug Module
+   ==
+
+   Author:   Leo Yan 
+   Date: April 5th, 2017
+
+Introduction
+
+
+Coresight CPU debug module is defined in ARMv8-a architecture reference manual
+(ARM DDI 0487A.k) Chapter 'Part H: External debug', the CPU can integrate
+debug module and it is mainly used for two modes: self-hosted debug and
+external debug. Usually the external debug mode is well known as the external
+debugger connects with SoC from JTAG port; on the other hand the program can
+explore debugging method which rely on self-hosted debug mode, this document
+is to focus on this part.
+
+The debug module provides sample-based profiling extension, which can be used
+to sample CPU program counter, secure state and exception level, etc; usually
+every CPU has one dedicated debug module to be connected. Based on self-hosted
+debug mechanism, Linux kernel can access these related registers from mmio
+region when the kernel panic happens. The callback notifier for kernel panic
+will dump related registers for every CPU; finally this is good for assistant
+analysis for panic.
+
+
+Implementation
+--
+
+- During driver registration, it uses EDDEVID and EDDEVID1 - two device ID
+  registers to decide if sample-based profiling is implemented or not. On some
+  platforms this hardware feature is fully or partially implemented; and if
+  this feature is not supported then registration will fail.
+
+- At the time this documentation was written, the debug driver mainly relies on
+  information gathered by the kernel panic callback notifier from three
+  sampling registers: EDPCSR, EDVIDSR and EDCIDSR: from EDPCSR we can get
+  program counter; EDVIDSR has information for secure state, exception level,
+  bit width, etc; EDCIDSR is context ID value which contains the sampled value
+  of CONTEXTIDR_EL1.
+
+- The driver supports a CPU running in either AArch64 or AArch32 mode. The
+  registers naming convention is a bit different between them, AArch64 uses
+  'ED' for register prefix (ARM DDI 0487A.k, chapter H9.1) and AArch32 uses
+  'DBG' as prefix (ARM DDI 0487A.k, chapter G5.1). The driver is unified to
+  use AArch64 naming convention.
+
+- ARMv8-a (ARM DDI 0487A.k) and ARMv7-a (ARM DDI 0406C.b) have different
+  register bits definition. So the driver consolidates two difference:
+
+  If PCSROffset=0b, on ARMv8-a the feature of EDPCSR is not implemented;
+  but ARMv7-a defines "PCSR samples are offset by a value that depends on the
+  instruction set state". For ARMv7-a, the driver checks furthermore if CPU
+  runs with ARM or thumb instruction set and calibrate PCSR value, the
+  detailed description for offset is in ARMv7-a ARM (ARM DDI 0406C.b) chapter
+  C11.11.34 "DBGPCSR, Program Counter Sampling Register".
+
+  If PCSROffset=0b0010, ARMv8-a defines "EDPCSR implemented, and samples have
+  no offset applied and do not sample the instruction set state in AArch32
+  state". So on ARMv8 if EDDEVID1.PCSROffset is 0b0010 and the CPU operates
+  in AArch32 state, EDPCSR is not sampled; when the CPU operates in AArch64
+  state EDPCSR is sampled and no offset are applied.
+
+
+Clock and power domain
+--
+
+Before accessing debug registers, we should ensure the clock and power domain
+have been enabled properly. In ARMv8-a ARM (ARM DDI 0487A.k) chapter 'H9.1
+Debug registers', the debug registers are spread into two domains: the debug
+domain and the CPU domain.
+
++---+
+|   |
+|   |
+ +--+--+|
+dbg_clock -->|  |**||<-- cpu_clock
+ |Debug |**|   CPU  |
+ dbg_power_domain -->|  |**||<-- cpu_power_domain
+ +--+--+|
+|   |
+|   |
++---+
+
+For debug domain, the user uses DT 

[PATCH] docs: driver-api: i2c: remove some outdated information

[Wolfram Sang]

a) Linux can be an I2C slave meanwhile
b) all drivers except one use the driver model currently

Update the documentation.

Signed-off-by: Wolfram Sang 
---
 Documentation/driver-api/i2c.rst | 9 -
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/Documentation/driver-api/i2c.rst b/Documentation/driver-api/i2c.rst
index f3939f7852bd59..0bf86a445d0135 100644
--- a/Documentation/driver-api/i2c.rst
+++ b/Documentation/driver-api/i2c.rst
@@ -13,8 +13,8 @@ I2C is a multi-master bus; open drain signaling is used to 
arbitrate
 between masters, as well as to handshake and to synchronize clocks from
 slower clients.
 
-The Linux I2C programming interfaces support only the master side of bus
-interactions, not the slave side. The programming interface is
+The Linux I2C programming interfaces support the master side of bus
+interactions and the slave side. The programming interface is
 structured around two kinds of driver, and two kinds of device. An I2C
 "Adapter Driver" abstracts the controller hardware; it binds to a
 physical device (perhaps a PCI device or platform_device) and exposes a
@@ -22,9 +22,8 @@ physical device (perhaps a PCI device or platform_device) and 
exposes a
 I2C bus segment it manages. On each I2C bus segment will be I2C devices
 represented by a :c:type:`struct i2c_client `.
 Those devices will be bound to a :c:type:`struct i2c_driver
-`, which should follow the standard Linux driver
-model. (At this writing, a legacy model is more widely used.) There are
-functions to perform various I2C protocol operations; at this writing
+`, which should follow the standard Linux driver model. There
+are functions to perform various I2C protocol operations; at this writing
 all such functions are usable only from task context.
 
 The System Management Bus (SMBus) is a sibling protocol. Most SMBus
-- 
2.11.0

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH] mm, oom: cgroup-aware OOM-killer

[Johannes Weiner]

On Tue, May 23, 2017 at 09:07:47AM +0200, Michal Hocko wrote:
> On Mon 22-05-17 18:01:16, Roman Gushchin wrote:
> > On Sat, May 20, 2017 at 09:37:29PM +0300, Vladimir Davydov wrote:
> > > On Thu, May 18, 2017 at 05:28:04PM +0100, Roman Gushchin wrote:
> > > ...
> > > > +5-2-4. Cgroup-aware OOM Killer
> > > > +
> > > > +Cgroup v2 memory controller implements a cgroup-aware OOM killer.
> > > > +It means that it treats memory cgroups as memory consumers
> > > > +rather then individual processes. Under the OOM conditions it tries
> > > > +to find an elegible leaf memory cgroup, and kill all processes
> > > > +in this cgroup. If it's not possible (e.g. all processes belong
> > > > +to the root cgroup), it falls back to the traditional per-process
> > > > +behaviour.
> > > 
> > > I agree that the current OOM victim selection algorithm is totally
> > > unfair in a system using containers and it has been crying for rework
> > > for the last few years now, so it's great to see this finally coming.
> > > 
> > > However, I don't reckon that killing a whole leaf cgroup is always the
> > > best practice. It does make sense when cgroups are used for
> > > containerizing services or applications, because a service is unlikely
> > > to remain operational after one of its processes is gone, but one can
> > > also use cgroups to containerize processes started by a user. Kicking a
> > > user out for one of her process has gone mad doesn't sound right to me.
> > 
> > I agree, that it's not always a best practise, if you're not allowed
> > to change the cgroup configuration (e.g. create new cgroups).
> > IMHO, this case is mostly covered by using the v1 cgroup interface,
> > which remains unchanged.
> 
> But there are features which are v2 only and users might really want to
> use it. So I really do not buy this v2-only argument.

I have to agree here. We won't get around making the leaf killing
opt-in or opt-out in some fashion.

> > > Another example when the policy you're suggesting fails in my opinion is
> > > in case a service (cgroup) consists of sub-services (sub-cgroups) that
> > > run processes. The main service may stop working normally if one of its
> > > sub-services is killed. So it might make sense to kill not just an
> > > individual process or a leaf cgroup, but the whole main service with all
> > > its sub-services.
> > 
> > I agree, although I do not pretend for solving all possible
> > userspace problems caused by an OOM.
> > 
> > How to react on an OOM - is definitely a policy, which depends
> > on the workload. Nothing is changing here from how it's working now,
> > except now kernel will choose a victim cgroup, and kill the victim cgroup
> > rather than a process.
> 
> There is a _big_ difference. The current implementation just tries
> to recover from the OOM situation without carying much about the
> consequences on the workload. This is the last resort and a services for
> the _system_ to get back to sane state. You are trying to make it more
> clever and workload aware and that is inevitable going to depend on the
> specific workload. I really do think we cannot simply hardcode any
> policy into the kernel for this purpose and that is why I would like to
> see a discussion about how to do that in a more extensible way. This
> might be harder to implement now but it I believe it will turn out
> better longerm.

And that's where I still maintain that this isn't really a policy
change. Because what this code does ISN'T more clever, and the OOM
killer STILL IS a last-resort thing. We don't need any elaborate
just-in-time evaluation of what each entity is worth. We just want to
kill the biggest job, not the biggest MM. Just like you wouldn't want
just the biggest VMA unmapped and freed, since it leaves your process
incoherent, killing one process leaves a job incoherent.

I understand that making it fully configurable is a tempting thought,
because you'd offload all responsibility to userspace. But on the
other hand, this was brought up years ago and nothing has happened
since. And to me this is evidence that nobody really cares all that
much. Because it's still a rather rare event, and there isn't much you
cannot accomplish with periodic score adjustments.

> > > And both kinds of workloads (services/applications and individual
> > > processes run by users) can co-exist on the same host - consider the
> > > default systemd setup, for instance.
> > > 
> > > IMHO it would be better to give users a choice regarding what they
> > > really want for a particular cgroup in case of OOM - killing the whole
> > > cgroup or one of its descendants. For example, we could introduce a
> > > per-cgroup flag that would tell the kernel whether the cgroup can
> > > tolerate killing a descendant or not. If it can, the kernel will pick
> > > the fattest sub-cgroup or process and check it. If it cannot, it will
> > > kill the whole cgroup and all its processes and sub-cgroups.
> > 
> > The last thing we want to do, is to 

Re: [kernel-hardening] [PATCH v4 next 0/3] modules: automatic module loading restrictions

[Djalal Harouni]

On Tue, May 23, 2017 at 1:38 AM, Andy Lutomirski  wrote:
> On Mon, May 22, 2017 at 4:07 PM, Kees Cook  wrote:
>> On Mon, May 22, 2017 at 12:55 PM, Djalal Harouni  wrote:
>>> On Mon, May 22, 2017 at 6:43 PM, Solar Designer  wrote:
 On Mon, May 22, 2017 at 03:49:15PM +0200, Djalal Harouni wrote:
> On Mon, May 22, 2017 at 2:08 PM, Solar Designer  
> wrote:
> > On Mon, May 22, 2017 at 01:57:03PM +0200, Djalal Harouni wrote:
> >> *) When modules_autoload_mode is set to (2), automatic module loading 
> >> is
> >> disabled for all. Once set, this value can not be changed.
> >
> > What purpose does this securelevel-like property ("Once set, this value
> > can not be changed.") serve here?  I think this mode 2 is needed, but
> > without this extra property, which is bypassable by e.g. explicitly
> > loaded kernel modules anyway (and that's OK).
>
> My reasoning about "Once set, this value can not be changed" is mainly 
> for:
>
> If you have some systems where modules are not updated for any given
> reason, then the only one who will be able to load a module is an
> administrator, basically this is a shortcut for:
>
> * Apps/services can run with CAP_NET_ADMIN but they are not allowed to
> auto-load 'netdev' modules.
>
> * Explicitly loading modules can be guarded by seccomp filters *per*
> app, so even if these apps have
>   CAP_SYS_MODULE they won't be able to explicitly load modules, one
> has to remount some sysctl /proc/ entries read-only here and remove
> CAP_SYS_ADMIN for all apps anyway.
>
> This mainly serves the purpose of these systems that do not receive
> updates, if I don't want to expose those kernel interfaces what should
> I do ? then if I want to unload old versions and replace them with new
> ones what operation should be allowed ? and only real root of the
> system can do it. Hence, the "Once set, this value can not be changed"
> is more of a shortcut, also the idea was put in my mind based on how
> "modules_disabled" is disabled forever, and some other interfaces. I
> would say: it is easy to handle a transition from 1) "hey this system
> is still up to date, some features should be exposed" to 2) "this
> system is not up to date anymore, only root should expose some
> features..."
>
> Hmm, I am not sure if this answers your question ? :-)

 This answers my question, but in a way that I summarize as "there's no
 good reason to include this securelevel-like property".

>>>
>>> Hmm, sorry I did forget to add in my previous comment that with such
>>> systems, CAP_SYS_MODULE can be used to reset the
>>> "modules_autoload_mode" sysctl back from mode 2 to mode 1, even if we
>>> disable it privileged tasks can be triggered to overwrite the sysctl
>>> flag and get it back unless /proc is read-only... that's one of the
>>> points, it should not be so easy to relax it.
>>
>> I'm on the fence. For modules_disabled and Yama, it was tied to
>> CAP_SYS_ADMIN, basically designed to be a at-boot setting that could
>> not later be undone by an attacker gaining that privilege, keeping
>> them out of either kernel memory or existing user process memory.
>> Here, it's CAP_SYS_MODULE... it's hard to imagine the situation where
>> a CAP_SYS_MODULE-capable process could write to this sysctl but NOT
>> issue direct modprobe requests, but it's _possible_ via crazy symlink
>> games to trick capable processes into writing to sysctls. We've seen
>> this multiple times before, and it's a way for attackers to turn a
>> single privileged write into a privileged exec.
>>
>> I might turn the question around, though: why would we want to have it
>> changeable at this setting?
>>
>> I'm fine leaving that piece off, either way.
>
> I think that having the un-resettable mode is unnecessary.  We should
> have option that disables loading modules entirely and cannot be
> unset.  (That means no explicit loads and not implicit loads.)  Maybe
> we already have this.  Otherwise, tightening caps needed for implicit
> loads should just be a normal yes/no setting IMO.

Ok, so as requested by you and Alexander in the other email, I will
remove the un-resettable property, if there is no general agreement,
better leave it for future.

I would have preferred if it is "yes/no" (in systemd the module
protection and other directives are also only 'yes/no').  But 1)
backward compatibility for unprivileged loading, 2) capabilities:
CAP_SYS_MODULE, CAP_NET_ADMIN use cases and exploits against
CAP_NET_ADMIN modules, and finally 3) separate and disable implicit
operation from the explicit one for various reasons that are noted in
the commit log like explicitly load the good module version, concluded
that we need three modes!

Thanks!

-- 
tixxdz
--
To unsubscribe from this 

Re: [PATCH 01/29] pinctrl.txt: standardize document format

[Linus Walleij]

On Fri, May 19, 2017 at 3:25 AM, Mauro Carvalho Chehab
 wrote:

> Each text file under Documentation follows a different
> format. Some doesn't even have titles!
>
> Change its representation to follow the adopted standard,
> using ReST markups for it to be parseable by Sphinx.
>
> This document is almost following the standard stile.
>
> There are only two things to adjust on it:
>
> - promote the level of the document title;
> - mark literal blocks as such.
>
> Signed-off-by: Mauro Carvalho Chehab 

I applied the patch fixing some of the indentation mistakes pointed
out by Geert.

Should the file be renamed pinctrl.rst now?

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [kernel-hardening] [PATCH v4 next 0/3] modules: automatic module loading restrictions

[Solar Designer]

> >>> On Mon, May 22, 2017 at 2:08 PM, Solar Designer  
> >>> wrote:
> >>> > On Mon, May 22, 2017 at 01:57:03PM +0200, Djalal Harouni wrote:
> >>> >> *) When modules_autoload_mode is set to (2), automatic module loading 
> >>> >> is
> >>> >> disabled for all. Once set, this value can not be changed.
> >>> >
> >>> > What purpose does this securelevel-like property ("Once set, this value
> >>> > can not be changed.") serve here?  I think this mode 2 is needed, but
> >>> > without this extra property, which is bypassable by e.g. explicitly
> >>> > loaded kernel modules anyway (and that's OK).

On Mon, May 22, 2017 at 04:07:56PM -0700, Kees Cook wrote:
> I'm on the fence. For modules_disabled and Yama, it was tied to
> CAP_SYS_ADMIN, basically designed to be a at-boot setting that could
> not later be undone by an attacker gaining that privilege, keeping
> them out of either kernel memory or existing user process memory.
> Here, it's CAP_SYS_MODULE... it's hard to imagine the situation where
> a CAP_SYS_MODULE-capable process could write to this sysctl but NOT
> issue direct modprobe requests, but it's _possible_ via crazy symlink
> games to trick capable processes into writing to sysctls. We've seen
> this multiple times before, and it's a way for attackers to turn a
> single privileged write into a privileged exec.

OK, tricking a process via crazy symlink games is finally a potentially
valid reason.  The question then becomes: are there perhaps so many
other important sysctl's, disk files, etc. (which the vulnerable capable
process could similarly be tricked into writing) so that specifically
resetting modules_autoload_mode isn't particularly lucrative?  I think
that the answer to that is usually yes.  Another related question: do we
really want to inconsistently single out a handful of sysctl's for this
kind of extra protection?  I think not.

I agree there are some other settings where being unable to reset them
makes sense, but I think this isn't one of those.

> I might turn the question around, though: why would we want to have it
> changeable at this setting?

Convenience for the sysadmin - being able to correct one's error (e.g.,
wrong order of shell commands), respond to new findings (thought module
autoloading was unneeded after some point, then found out some software
relies on it), change one's mind, reuse a system differently than
originally intended without a forced reboot.

> I'm fine leaving that piece off, either way.

I'm also fine with either decision.  I just thought I'd point out what
looked weird to me.

I think this is an important patch that should get in, but primarily
for modules_autoload_mode=1, which many distros could make the default
(and maybe the kernel eventually should?)

For modules_autoload_mode=2, we already seem to have the equivalent of
modprobe=/bin/true (or does it differ subtly, maybe in return values?),
which I already use at startup on a GPU box like this (preloading
modules so that the OpenCL backends wouldn't need the autoloading):

nvidia-smi
nvidia-modprobe -u -c=0
#modprobe nvidia_uvm
#modprobe fglrx

sysctl -w kernel.modprobe=/bin/true
sysctl -w kernel.hotplug=/bin/true

but it's good to also have this supported more explicitly and more
consistently through modules_autoload_mode=2 while we're at it.  So I
support having this mode as well.  I just question the need to have it
non-resettable.

Alexander
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 0/3] watchdog: allow setting deadline for opening /dev/watchdogN

[Rasmus Villemoes]

On 2017-05-22 20:07, Alan Cox wrote:
> On Mon, 22 May 2017 16:06:36 +0200
> Rasmus Villemoes  wrote:
> 
>> If a watchdog driver tells the framework that the device is running,
>> the framework takes care of feeding the watchdog until userspace opens
>> the device. If the userspace application which is supposed to do that
>> never comes up properly, the watchdog is fed indefinitely by the
>> kernel. This can be especially problematic for embedded devices.
>>
>> These patches allow one to set a maximum time for which the kernel
>> will feed the watchdog, thus ensuring that either userspace has come
>> up, or the board gets reset. This allows fallback logic in the
>> bootloader to attempt some recovery (for example, if an automatic
>> update is in progress, it could roll back to the previous version).
> 
> 
> This makes sense except for being a CONFIG_ option not a boot parameter.
> If it's a boot parameter then the same kernel works for multiple systems
> and is general. If it's compile time then you have to build a custom
> kernel.

I don't understand. Patch 3 simply allows the default to be set in the
config, but it is very much still possible to set it via the boot
command line, overriding the default (whether that was a CONFIG or a
hardcoded 0).

> For some embedded stuff that might not matter (although I bet they'd
> prefer it command line/device tree too)

When building a full BSP, yes, one can cook it into the boot command
line in u-boot, but that means that to update it one has to edit one's
u-boot recipe. It also often makes testing-turnaround times longer,
since updating the kernel can often be done by just scp'ing a new image
to the board, while updating the bootloader is sometimes a much more
involved procedure (not to mention that that's sometimes not even
possible to do remotely). There's also the issue of the command line
getting rather crowded if every aspect of the kernel has to be
configured at boot time.

As an embedded developer, I'm all for being able to override this
timeout as well lots of other kernel behavior via the command line, but
at the same time, I also prefer being able to set the default values in
the kernel's .config.

 but for something like an x86
> platform where you are deploying a standard vendor supplied kernel it's
> bad to do it that way IMHO.

A "standard vendor supplied kernel" will never set that CONFIG to
anything but its default - how could it possibly decide how long
userspace should have to open /dev/watchdog0, or even if userspace will
ever do that? Again, patch 3 doesn't in any way remove the possiblity of
setting this on the command line, and the end user is no worse off just
because the vendor had to keep a default setting.

> In other words I think you should drop patch 3 but the rest is good.

For the life of me, I cannot see the downside of patch 3, or how it's
any different from CONSOLE_LOGLEVEL_DEFAULT, BLK_DEV_RAM_SIZE, or any
number of other parameters-with-defaults-from-Kconfig.

Rasmus

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH] mm, oom: cgroup-aware OOM-killer

[Michal Hocko]

On Mon 22-05-17 18:01:16, Roman Gushchin wrote:
> On Sat, May 20, 2017 at 09:37:29PM +0300, Vladimir Davydov wrote:
> > Hello Roman,
> 
> Hi Vladimir!
> 
> > 
> > On Thu, May 18, 2017 at 05:28:04PM +0100, Roman Gushchin wrote:
> > ...
> > > +5-2-4. Cgroup-aware OOM Killer
> > > +
> > > +Cgroup v2 memory controller implements a cgroup-aware OOM killer.
> > > +It means that it treats memory cgroups as memory consumers
> > > +rather then individual processes. Under the OOM conditions it tries
> > > +to find an elegible leaf memory cgroup, and kill all processes
> > > +in this cgroup. If it's not possible (e.g. all processes belong
> > > +to the root cgroup), it falls back to the traditional per-process
> > > +behaviour.
> > 
> > I agree that the current OOM victim selection algorithm is totally
> > unfair in a system using containers and it has been crying for rework
> > for the last few years now, so it's great to see this finally coming.
> > 
> > However, I don't reckon that killing a whole leaf cgroup is always the
> > best practice. It does make sense when cgroups are used for
> > containerizing services or applications, because a service is unlikely
> > to remain operational after one of its processes is gone, but one can
> > also use cgroups to containerize processes started by a user. Kicking a
> > user out for one of her process has gone mad doesn't sound right to me.
> 
> I agree, that it's not always a best practise, if you're not allowed
> to change the cgroup configuration (e.g. create new cgroups).
> IMHO, this case is mostly covered by using the v1 cgroup interface,
> which remains unchanged.

But there are features which are v2 only and users might really want to
use it. So I really do not buy this v2-only argument.

> If you do have control over cgroups, you can put processes into
> separate cgroups, and obtain control over OOM victim selection and killing.

Usually you do not have that control because there is a global daemon
doing the placement for you.

> > Another example when the policy you're suggesting fails in my opinion is
> > in case a service (cgroup) consists of sub-services (sub-cgroups) that
> > run processes. The main service may stop working normally if one of its
> > sub-services is killed. So it might make sense to kill not just an
> > individual process or a leaf cgroup, but the whole main service with all
> > its sub-services.
> 
> I agree, although I do not pretend for solving all possible
> userspace problems caused by an OOM.
> 
> How to react on an OOM - is definitely a policy, which depends
> on the workload. Nothing is changing here from how it's working now,
> except now kernel will choose a victim cgroup, and kill the victim cgroup
> rather than a process.

There is a _big_ difference. The current implementation just tries
to recover from the OOM situation without carying much about the
consequences on the workload. This is the last resort and a services for
the _system_ to get back to sane state. You are trying to make it more
clever and workload aware and that is inevitable going to depend on the
specific workload. I really do think we cannot simply hardcode any
policy into the kernel for this purpose and that is why I would like to
see a discussion about how to do that in a more extensible way. This
might be harder to implement now but it I believe it will turn out
better longerm.

> > And both kinds of workloads (services/applications and individual
> > processes run by users) can co-exist on the same host - consider the
> > default systemd setup, for instance.
> > 
> > IMHO it would be better to give users a choice regarding what they
> > really want for a particular cgroup in case of OOM - killing the whole
> > cgroup or one of its descendants. For example, we could introduce a
> > per-cgroup flag that would tell the kernel whether the cgroup can
> > tolerate killing a descendant or not. If it can, the kernel will pick
> > the fattest sub-cgroup or process and check it. If it cannot, it will
> > kill the whole cgroup and all its processes and sub-cgroups.
> 
> The last thing we want to do, is to compare processes with cgroups.
> I agree, that we can have some option to disable the cgroup-aware OOM at all,
> mostly for backward-compatibility. But I don't think it should be a
> per-cgroup configuration option, which we will support forever.

I can clearly see a demand for "this is definitely more important
container than others so do not kill" usecases. I can also see demand
for "do not kill this container running for X days". And more are likely
to pop out.
-- 
Michal Hocko
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html