Re: [PATCH] doc: tracing: Fix a number of typos

[Will Korteland]

On 2018-10-31 14:16, Amir Livneh wrote:
> @@ -2978,7 +2978,7 @@ The following commands are supported:
>When the function is hit, it will dump the contents of the ftrace
>ring buffer to the console. This is useful if you need to debug
>something, and want to dump the trace when a certain function
> -  is hit. Perhaps its a function that is called before a tripple
> +  is hit. Perhaps it's a function that is called before a tripple
>fault happens and does not allow you to get a regular dump.
>  
>  - cpudump:

"tripple" -> "triple", while you're at it?

- Will

[PATCH v2] doc: tracing: Fix a number of typos

[Amir Livneh]

Trivial fixes to spelling mistakes in ftrace.rst

v2: tripple -> triple

Signed-off-by: Amir Livneh 
---
 Documentation/trace/ftrace.rst | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/Documentation/trace/ftrace.rst b/Documentation/trace/ftrace.rst
index 7ea16a0ceffc..f25c50ea64ca 100644
--- a/Documentation/trace/ftrace.rst
+++ b/Documentation/trace/ftrace.rst
@@ -24,13 +24,13 @@ It can be used for debugging or analyzing latencies and
 performance issues that take place outside of user-space.
 
 Although ftrace is typically considered the function tracer, it
-is really a frame work of several assorted tracing utilities.
+is really a framework of several assorted tracing utilities.
 There's latency tracing to examine what occurs between interrupts
 disabled and enabled, as well as for preemption and from a time
 a task is woken to the task is actually scheduled in.
 
 One of the most common uses of ftrace is the event tracing.
-Through out the kernel is hundreds of static event points that
+Throughout the kernel is hundreds of static event points that
 can be enabled via the tracefs file system to see what is
 going on in certain parts of the kernel.
 
@@ -462,7 +462,7 @@ of ftrace. Here is a list of some of the key files:
 
mono_raw:
This is the raw monotonic clock (CLOCK_MONOTONIC_RAW)
-   which is montonic but is not subject to any rate adjustments
+   which is monotonic but is not subject to any rate adjustments
and ticks at the same rate as the hardware clocksource.
 
boot:
@@ -914,8 +914,8 @@ The above is mostly meaningful for kernel developers.
current trace and the next trace.
 
  - '$' - greater than 1 second
- - '@' - greater than 100 milisecond
- - '*' - greater than 10 milisecond
+ - '@' - greater than 100 millisecond
+ - '*' - greater than 10 millisecond
  - '#' - greater than 1000 microsecond
  - '!' - greater than 100 microsecond
  - '+' - greater than 10 microsecond
@@ -2541,7 +2541,7 @@ At compile time every C file object is run through the
 recordmcount program (located in the scripts directory). This
 program will parse the ELF headers in the C object to find all
 the locations in the .text section that call mcount. Starting
-with gcc verson 4.6, the -mfentry has been added for x86, which
+with gcc version 4.6, the -mfentry has been added for x86, which
 calls "__fentry__" instead of "mcount". Which is called before
 the creation of the stack frame.
 
@@ -2978,7 +2978,7 @@ The following commands are supported:
   When the function is hit, it will dump the contents of the ftrace
   ring buffer to the console. This is useful if you need to debug
   something, and want to dump the trace when a certain function
-  is hit. Perhaps its a function that is called before a tripple
+  is hit. Perhaps it's a function that is called before a triple
   fault happens and does not allow you to get a regular dump.
 
 - cpudump:
-- 
2.13.5

Re: [GIT PULL] Compiler Attributes for v4.20-rc1

[Linus Torvalds]

On Mon, Oct 22, 2018 at 3:59 AM Miguel Ojeda
 wrote:
>
> Here it is the Compiler Attributes series/tree, which tries to disentangle
> the include/linux/compiler*.h headers and bring them up to date.

I've finally emptied the "normal" pull queues, and am looking at the
odd ones that I felt I needed to review more in-depth.

This looked fine to me, and I already pulled, but when looking at the
conflict for __no_sanitize_address_or_inline, I also noticed that you
actually changed the gcc version logic.

The logic for using __no_sanitize_address *used* to be

#if GCC_VERSION >= 40902

but you have changed it to

  # define __GCC4_has_attribute___no_sanitize_address__ (__GNUC_MINOR__ >= 8)

so now it's gcc-4.8+ rather than gcc-4.9.2+.

I'm not sure how much that matters (maybe the original check for 4.9.2
was just a random pick by Andrey? Added to cc), but together with the
movement to  that looks like it also
wouldn't want the CONFIG_KASAN tests, I wonder what the right merge
resolution would be.

Yes, I see the resolution in linux-next, and I think that one is odd
and dubious, and now it *mixes* that old test of gcc-4.9.2 with the
different test in compiler-attributes.

I'm _inclined_ to just do

#ifdef CONFIG_KASAN
 #define __no_sanitize_address_or_inline \
  __no_sanitize_address __maybe_unused notrace
#else
 #define __no_sanitize_address_or_inline inline
#endif

without any compiler versions at all, because I don't think it matters
(maybe we won't get address sanitation, and we will not get inlining
either, but do we care?).

But I'm also unsure whether you meant for the "__has_attribute()" test
to be usable outside the linux/compiler_attributes.h file, in which
case I could just do

  #if defined(CONFIG_KASAN) && __has_attribute(__no_sanitize_address__)

instead.

End result: it's not the merge resolution itself that raises
questions, it's just semantics in general.

So I've dropped this for now, in the hope that you/Andrey can answer
these questions.

   Linus

Re: [GIT PULL] Compiler Attributes for v4.20-rc1

[Miguel Ojeda]

On Thu, Nov 1, 2018 at 6:06 PM Linus Torvalds
 wrote:
>
> I'm not sure how much that matters (maybe the original check for 4.9.2
> was just a random pick by Andrey? Added to cc), but together with the
> movement to  that looks like it also
> wouldn't want the CONFIG_KASAN tests, I wonder what the right merge
> resolution would be.

Good catch. I don't recall any special logic when I did that change,
so most likely I simply did like for the rest of the attributes and
took a look at when it was first supported (and documentation in gcc's
docs) in order to implement __has_attribute by hand.

But indeed, it *may* be that there is an (undocumented) problem
between 4.8 <= gcc < 4.9.2 with it. If so, we should document it down
and fix it. Andrey?

> Yes, I see the resolution in linux-next, and I think that one is odd
> and dubious, and now it *mixes* that old test of gcc-4.9.2 with the
> different test in compiler-attributes.

I missed that conflict completely, my bad (I did not miss all of them,
at least; one required fixing).

Hm at a quick look, why is it only on compiler-gcc.h? It should
either have a corresponding #define elsewhere or just be put directly
in another common header, no? (Adding Vasily & Martin to CC.)

> But I'm also unsure whether you meant for the "__has_attribute()" test
> to be usable outside the linux/compiler_attributes.h file, in which
> case I could just do
>
>   #if defined(CONFIG_KASAN) && __has_attribute(__no_sanitize_address__)
>
> instead.

I think that (using __has_attribute() outside) may be a good idea: I
wanted to keep compiler_attributes.h as simple as possible by avoiding
#ifdefs inside that header (except for __has_attribute itself), as an
attempt to avoid going back to the mess of #ifdefs we had previously.
Basically, keeping the attributes in compiler_attributes.h that do not
depend on complex logic. So using __has_attribute *outside* the header
actually goes well with that principle, because it helps keeping stuff
out of it if they depend on other config options; without having to
rely on GCC_VERSION either.

[By the way, in case it clarifies: note that "optional" in that file
actually is a bit of a misnomer. I meant to say "optional" as in "not
supported by all compilers, so conditionally defined" ("optionally"
defined); rather than "optional" in the sense of "code still works
without the attribute". It caught Rasmus in one of his patches sent a
few days ago on top of this tree, so I want to change it or explain it
to avoid confusion.]

Cheers,
Miguel

Re: [GIT PULL] Compiler Attributes for v4.20-rc1

[Linus Torvalds]

On Thu, Nov 1, 2018 at 10:06 AM Linus Torvalds
 wrote:
>
> The logic for using __no_sanitize_address *used* to be
>
> #if GCC_VERSION >= 40902

Ok, looking around, I think this has less to do with the attribute
being recognized, and simply just being because KASAN itself wants
gcc-4.9.2.

I'm actually not seeing that KASAN dependency in the Kconfig scripts
(and it probably _should_ be now that we can just add compiler version
dependencies there), but that explains why the gcc version check is
different from "gcc supports the attribute".

Anyway, I decided to do the merge by just getting rid of the
GCC_VERSION check around __no_sanitize_address_or_inline entirely. If
you enable KASAN, then a function with that marking just won't be
marked inline.

End result: pulled. I'm as confused as you are as to why
__no_sanitize_address_or_inline is in the gcc header, but I guess it
ends up being the same issue: KASAN depends on gcc even if that
dependency doesn't seem to be spelled out in lib/Kconfig.kasan.

So I _think_ the KASAN config should have a

depends on CC_IS_GCC && GCC_VERSION >= 40902

on it, but maybe there is something I'm missing.

But from a pull standpoint, I don't want to mess with those
(unrelated) issues, so I just kept the merge resolution as simple and
straightforward as possible.

Miguel, please do double-check the merge (it's not pushed out yet, I'm
doing the usual build tests etc first).

Linus

[PATCH V5 2/8] misc/pvpanic: Remove one extra semicolon

[Peng Hao]

There is an extra semicolon in pvpanic_device_ids, remove it.

Signed-off-by: Peng Hao 
---
 drivers/misc/pvpanic.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/misc/pvpanic.c b/drivers/misc/pvpanic.c
index fd86dab..059005c 100644
--- a/drivers/misc/pvpanic.c
+++ b/drivers/misc/pvpanic.c
@@ -35,7 +35,7 @@
 
 static const struct acpi_device_id pvpanic_device_ids[] = {
{ "QEMU0001", 0 },
-   { "", 0 },
+   { "", 0 }
 };
 MODULE_DEVICE_TABLE(acpi, pvpanic_device_ids);
 
-- 
1.8.3.1

[PATCH V5 1/8] pvpanic: move pvpanic to misc as common driver

[Peng Hao]

move pvpanic.c from drivers/platform/x86 to drivers/misc.
following patches will use pvpanic device in arm64.

Signed-off-by: Peng Hao 
---
 drivers/misc/Kconfig | 8 +++
 drivers/misc/Makefile| 1 +
 drivers/{platform/x86 => misc}/pvpanic.c | 0
 drivers/platform/x86/Kconfig | 8 
 drivers/platform/x86/Makefile| 1 -
 5 files changed, 9 insertions(+), 9 deletions(-)
 rename drivers/{platform/x86 => misc}/pvpanic.c (100%)

diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index 3726eac..1ba0f62 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -513,6 +513,14 @@ config MISC_RTSX
tristate
default MISC_RTSX_PCI || MISC_RTSX_USB
 
+config PVPANIC
+   tristate "pvpanic device support"
+   depends on ACPI || OF
+   help
+ This driver provides support for the pvpanic device.  pvpanic is
+ a paravirtualized device provided by QEMU; it lets a virtual machine
+ (guest) communicate panic events to the host.
+
 source "drivers/misc/c2port/Kconfig"
 source "drivers/misc/eeprom/Kconfig"
 source "drivers/misc/cb710/Kconfig"
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index af22bbc..df951f4 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -58,3 +58,4 @@ obj-$(CONFIG_ASPEED_LPC_SNOOP)+= aspeed-lpc-snoop.o
 obj-$(CONFIG_PCI_ENDPOINT_TEST)+= pci_endpoint_test.o
 obj-$(CONFIG_OCXL) += ocxl/
 obj-$(CONFIG_MISC_RTSX)+= cardreader/
+obj-$(CONFIG_PVPANIC)  += pvpanic.o
diff --git a/drivers/platform/x86/pvpanic.c b/drivers/misc/pvpanic.c
similarity index 100%
rename from drivers/platform/x86/pvpanic.c
rename to drivers/misc/pvpanic.c
diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 0c1aa6c..4ac276c 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -1121,14 +1121,6 @@ config INTEL_SMARTCONNECT
  This driver checks to determine whether the device has Intel Smart
  Connect enabled, and if so disables it.
 
-config PVPANIC
-   tristate "pvpanic device support"
-   depends on ACPI
-   ---help---
- This driver provides support for the pvpanic device.  pvpanic is
- a paravirtualized device provided by QEMU; it lets a virtual machine
- (guest) communicate panic events to the host.
-
 config INTEL_PMC_IPC
tristate "Intel PMC IPC Driver"
depends on ACPI
diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
index e6d1bec..e88f44e 100644
--- a/drivers/platform/x86/Makefile
+++ b/drivers/platform/x86/Makefile
@@ -75,7 +75,6 @@ obj-$(CONFIG_APPLE_GMUX)  += apple-gmux.o
 obj-$(CONFIG_INTEL_RST)+= intel-rst.o
 obj-$(CONFIG_INTEL_SMARTCONNECT)   += intel-smartconnect.o
 
-obj-$(CONFIG_PVPANIC)   += pvpanic.o
 obj-$(CONFIG_ALIENWARE_WMI)+= alienware-wmi.o
 obj-$(CONFIG_INTEL_PMC_IPC)+= intel_pmc_ipc.o
 obj-$(CONFIG_TOUCHSCREEN_DMI)  += touchscreen_dmi.o
-- 
1.8.3.1

[PATCH V5 5/8] misc/pvpanic: simplify the code using acpi_dev_resource_io

[Peng Hao]

use acpi_dev_resource_io API.

Signed-off-by: Peng Hao 
---
 drivers/misc/pvpanic.c | 14 ++
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/misc/pvpanic.c b/drivers/misc/pvpanic.c
index 0bcf1cd..c20fdff 100644
--- a/drivers/misc/pvpanic.c
+++ b/drivers/misc/pvpanic.c
@@ -64,17 +64,15 @@
 static acpi_status
 pvpanic_walk_resources(struct acpi_resource *res, void *context)
 {
-   switch (res->type) {
-   case ACPI_RESOURCE_TYPE_END_TAG:
-   return AE_OK;
+   struct resource r;
 
-   case ACPI_RESOURCE_TYPE_IO:
-   port = res->data.io.minimum;
+   if (acpi_dev_resource_io(res, &r)) {
+   port = r.start;
return AE_OK;
-
-   default:
-   return AE_ERROR;
}
+
+   return AE_ERROR;
+
 }
 
 static int pvpanic_add(struct acpi_device *device)
-- 
1.8.3.1

[PATCH V5 6/8] misc/pvpanic: add MMIO support

[Peng Hao]

On some architectures (e.g. arm64), it's preferable to use MMIO, since
this can be used standalone. Add MMIO support to the pvpanic driver.

Signed-off-by: Peng Hao 
---
 drivers/misc/pvpanic.c | 18 +-
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/drivers/misc/pvpanic.c b/drivers/misc/pvpanic.c
index c20fdff..c0b787e 100644
--- a/drivers/misc/pvpanic.c
+++ b/drivers/misc/pvpanic.c
@@ -28,7 +28,7 @@
 
 #define PVPANIC_PANICKED   (1 << 0)
 
-static u16 port;
+static void __iomem *base;
 
 static struct acpi_driver pvpanic_driver = {
.name = "pvpanic",
@@ -44,7 +44,7 @@
 static void
 pvpanic_send_event(unsigned int event)
 {
-   outb(event, port);
+   iowrite8(event, base);
 }
 
 static int
@@ -66,8 +66,14 @@
 {
struct resource r;
 
-   if (acpi_dev_resource_io(res, &r) {
-   port = r.start;
+   if (acpi_dev_resource_io(res, &r) ||
+   acpi_dev_resource_memory(res, &r)) {
+   if (r.flags & IORESOURCE_IO)
+   base = (void __iomem *) ioport_map(r.start,
+   r.end - r.start + 1);
+   else
+   base = ioremap(r.start, r.end - r.start + 1);
+
return AE_OK;
}
 
@@ -89,7 +95,7 @@ static int pvpanic_add(struct acpi_device *device)
acpi_walk_resources(device->handle, METHOD_NAME__CRS,
pvpanic_walk_resources, NULL);
 
-   if (!port)
+   if (!base)
return -ENODEV;
 
atomic_notifier_chain_register(&panic_notifier_list,
@@ -103,6 +109,8 @@ static int pvpanic_remove(struct acpi_device *device)
 
atomic_notifier_chain_unregister(&panic_notifier_list,
 &pvpanic_panic_nb);
+
+   iounmap(base);
return 0;
 }
 
-- 
1.8.3.1

[PATCH V5 3/8] misc/pvpanic: revmove unnecessary header file

[Peng Hao]

remove unnecessary header file init.h.

Signed-off-by: Peng Hao 
---
 drivers/misc/pvpanic.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/misc/pvpanic.c b/drivers/misc/pvpanic.c
index 059005c..66534d4 100644
--- a/drivers/misc/pvpanic.c
+++ b/drivers/misc/pvpanic.c
@@ -22,7 +22,6 @@
 
 #include 
 #include 
-#include 
 #include 
 #include 
 
-- 
1.8.3.1

[PATCH V5 4/8] misc/pvpanic :convert to SPDX license tags

[Peng Hao]

This patch updates license to use SPDX-License-Identifier
instead of verbose license text.

Signed-off-by: Peng Hao 
---
 drivers/misc/pvpanic.c | 16 ++--
 1 file changed, 2 insertions(+), 14 deletions(-)

diff --git a/drivers/misc/pvpanic.c b/drivers/misc/pvpanic.c
index 66534d4..0bcf1cd 100644
--- a/drivers/misc/pvpanic.c
+++ b/drivers/misc/pvpanic.c
@@ -1,21 +1,9 @@
+// SPDX-License-Identifier: GPL-2.0+
 /*
- *  pvpanic.c - pvpanic Device Support
+ *  Pvpanic Device Support
  *
  *  Copyright (C) 2013 Fujitsu.
  *
- *  This program is free software; you can redistribute it and/or modify
- *  it under the terms of the GNU General Public License as published by
- *  the Free Software Foundation; either version 2 of the License, or
- *  (at your option) any later version.
- *
- *  This program is distributed in the hope that it will be useful,
- *  but WITHOUT ANY WARRANTY; without even the implied warranty of
- *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- *  GNU General Public License for more details.
- *
- *  You should have received a copy of the GNU General Public License
- *  along with this program; if not, write to the Free Software
- *  Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301  
USA
  */
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
-- 
1.8.3.1

[PATCH V5 7/8] misc/pvpanic: add support to get pvpanic device info by FDT

[Peng Hao]

By default, when ACPI tables and FDT coexist for ARM64,
current kernel takes precedence over FDT to get device information.
Virt machine in qemu provides both FDT and ACPI table. This patch
increases the way to get information through FDT.

Signed-off-by: Peng Hao 
---
 drivers/misc/pvpanic.c | 66 +++---
 1 file changed, 63 insertions(+), 3 deletions(-)

diff --git a/drivers/misc/pvpanic.c b/drivers/misc/pvpanic.c
index c0b787e..9d9f0d7 100644
--- a/drivers/misc/pvpanic.c
+++ b/drivers/misc/pvpanic.c
@@ -3,15 +3,18 @@
  *  Pvpanic Device Support
  *
  *  Copyright (C) 2013 Fujitsu.
- *
+ *  Copyright (C) 2018 ZTE.
  */
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
+#include 
 #include 
 #include 
+#include 
+#include 
+#include 
 #include 
-#include 
 
 MODULE_AUTHOR("Hu Tao ");
 MODULE_DESCRIPTION("pvpanic device driver");
@@ -60,6 +63,32 @@
.priority = 1, /* let this called before broken drm_fb_helper */
 };
 
+static int pvpanic_mmio_probe(struct platform_device *pdev)
+{
+   struct resource *mem;
+
+   mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+   if (!mem)
+   return -EINVAL;
+
+   base = devm_ioremap_resource(&pdev->dev, mem);
+   if (base == NULL)
+   return -EFAULT;
+
+   atomic_notifier_chain_register(&panic_notifier_list,
+  &pvpanic_panic_nb);
+
+   return 0;
+}
+
+static int pvpanic_mmio_remove(struct platform_device *pdev)
+{
+
+   atomic_notifier_chain_unregister(&panic_notifier_list,
+&pvpanic_panic_nb);
+
+   return 0;
+}
 
 static acpi_status
 pvpanic_walk_resources(struct acpi_resource *res, void *context)
@@ -114,4 +143,35 @@ static int pvpanic_remove(struct acpi_device *device)
return 0;
 }
 
-module_acpi_driver(pvpanic_driver);
+static const struct of_device_id pvpanic_mmio_match[] = {
+   { .compatible = "qemu,pvpanic-mmio", },
+   {}
+};
+
+static struct platform_driver pvpanic_mmio_driver = {
+   .driver = {
+   .name = "pvpanic-mmio",
+   .of_match_table = pvpanic_mmio_match,
+   },
+   .probe = pvpanic_mmio_probe,
+   .remove = pvpanic_mmio_remove,
+};
+
+static int __init pvpanic_mmio_init(void)
+{
+   if (acpi_disabled)
+   return platform_driver_register(&pvpanic_mmio_driver);
+   else
+   return acpi_bus_register_driver(&pvpanic_driver);
+}
+
+static void __exit pvpanic_mmio_exit(void)
+{
+   if (acpi_disabled)
+   platform_driver_unregister(&pvpanic_mmio_driver);
+   else
+   acpi_bus_unregister_driver(&pvpanic_driver);
+}
+
+module_init(pvpanic_mmio_init);
+module_exit(pvpanic_mmio_exit);
-- 
1.8.3.1

[PATCH V5 8/8] dt-bindings/misc/pvpanic :add document for pvpanic-mmio

[Peng Hao]

Add dt-bindings document for "qemu:pvpanic-mmio".

Signed-off-by: Peng Hao 
---
 .../devicetree/bindings/misc/pvpanic-mmio.txt  | 29 ++
 1 file changed, 29 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/misc/pvpanic-mmio.txt

diff --git a/Documentation/devicetree/bindings/misc/pvpanic-mmio.txt 
b/Documentation/devicetree/bindings/misc/pvpanic-mmio.txt
new file mode 100644
index 000..985e907
--- /dev/null
+++ b/Documentation/devicetree/bindings/misc/pvpanic-mmio.txt
@@ -0,0 +1,29 @@
+* QEMU PVPANIC MMIO Configuration bindings
+
+QEMU's emulation / virtualization targets provide the following PVPANIC
+MMIO Configuration interface on the "virt" machine.
+type:
+
+- a read-write, 16-bit wide data register.
+
+QEMU exposes the data register to guests as memory mapped registers.
+
+Required properties:
+
+- compatible: "qemu,pvpanic-mmio".
+- reg: the MMIO region used by the device.
+  * Bytes 0x0  Write panic event to the reg when guest OS panics.
+  * Bytes 0x1  Reserved.
+
+Example:
+
+/ {
+#size-cells = <0x2>;
+#address-cells = <0x2>;
+
+pvpanic-mmio@906 {
+compatible = "qemu,pvpanic-mmio";
+reg = <0x0 0x906 0x0 0x2>;
+};
+};
+
-- 
1.8.3.1

Re: [PATCH] Document /proc/pid PID reuse behavior

[Mike Rapoport]

On Wed, Oct 31, 2018 at 03:06:22PM +, Daniel Colascione wrote:
> State explicitly that holding a /proc/pid file descriptor open does
> not reserve the PID. Also note that in the event of PID reuse, these
> open file descriptors refer to the old, now-dead process, and not the
> new one that happens to be named the same numeric PID.

Signed-off is missing.

> ---
>  Documentation/filesystems/proc.txt | 8 
>  1 file changed, 8 insertions(+)
> 
> diff --git a/Documentation/filesystems/proc.txt 
> b/Documentation/filesystems/proc.txt
> index 12a5e6e693b6..567f66a8a23c 100644
> --- a/Documentation/filesystems/proc.txt
> +++ b/Documentation/filesystems/proc.txt
> @@ -214,6 +214,14 @@ asynchronous manner and the value may not be very 
> precise. To see a precise
>  snapshot of a moment, you can see /proc//smaps file and scan page table.
>  It's slow but very precise.
> 
> +Note that an open a file descriptor to /proc/ or to any of its
> +contained files or subdirectories does not prevent  being reused
> +for some other process in the event that  exits. Operations on
> +open /proc/ file descriptors corresponding to dead processes
> +never act on any new process that the kernel may, through chance, have
> +also assigned the process ID . Instead, operations on these FDs
> +usually fail with ESRCH.
> +

I'd put this text in the beginning of the section, just before table 1-1.
Otherwise looks good.

It maybe also useful to update 'man 5 proc' (1) as well

[1] https://git.kernel.org/pub/scm/docs/man-pages/man-pages.git/tree/man5/proc.5

>  Table 1-2: Contents of the status files (as of 4.8)
>  
> ..
>   Field   Content
> -- 
> 2.19.1.568.g152ad8e336-goog
> 

-- 
Sincerely yours,
Mike.

[PATCH v2 0/8] clk: clkdev: managed clk lookup and provider registrations

[Matti Vaittinen]

Patch series adding managed clkdev and of_provider registrations

Few clk drivers appear to be leaking clkdev lookup registrations at
driver remove. The patch series adds devm versions of lookup
registrations and cleans up few drivers. Driver clean-up patches have
not been tested as I lack the HW. All testing and comments if
driver/device removal is even possible for changed drivers is highly
appreciated. If removal is not possible I will gladly drop the patches
from series - although leaking lookups may serve as bad example for new
developers =)

Changed drivers are:
clk-max77686, clk-s3c2410-dclk, clk-st, clk-hi655x, rk808, clk-twl6040
and apcs-msm8916.

New devm registration variants have been tested on BeagleBoneBlack
using ROHM BD71837 PMIC driver.

Same devm variants were earlier proposed together with BD71837/BD71847
PMIC clk driver in this series:
https://lore.kernel.org/linux-clk/cover.1535630942.git.matti.vaitti...@fi.rohmeurope.com/

The BD71837/BD71847 work is currently pending for related MFD commits to
get merged in clk-tree and the devm functions are now submitted in this
series.

Changelog v2
Issue spotted by 0-Day test suite
- Add a stub function 'devm_of_clk_add_parent_hw_provider' for no OF config.
- patches 2-8 are unchanged.

This patch series is based on clk-next

---

Matti Vaittinen (8):
  clk: clkdev/of_clk - add managed lookup and provider registrations
  clk: clk-max77686: Clean clkdev lookup leak and use devm
  clk: clk-s3c2410-dclk: clean up clkdev lookup leak
  clk: clk-st: avoid clkdev lookup leak at remove
  clk: clk-hi655x: Free of_provider at remove
  clk: rk808: use managed version of of_provider registration
  clk: clk-twl6040: Free of_provider at remove
  clk: apcs-msm8916: simplify probe cleanup by using devm

 Documentation/driver-model/devres.txt  |   3 +
 drivers/clk/clk-hi655x.c   |   4 +-
 drivers/clk/clk-max77686.c |  25 ++-
 drivers/clk/clk-rk808.c|  15 +---
 drivers/clk/clk-twl6040.c  |   5 +-
 drivers/clk/clk.c  |  28 ++--
 drivers/clk/clkdev.c   | 122 ++---
 drivers/clk/qcom/apcs-msm8916.c|   5 +-
 drivers/clk/samsung/clk-s3c2410-dclk.c |  15 ++--
 drivers/clk/x86/clk-st.c   |   3 +-
 include/linux/clk-provider.h   |  11 +++
 include/linux/clkdev.h |   4 ++
 12 files changed, 158 insertions(+), 82 deletions(-)

-- 
2.14.3


-- 
Matti Vaittinen
ROHM Semiconductors

~~~ "I don't think so," said Rene Descartes.  Just then, he vanished ~~~

[PATCH v2 1/8] clk: clkdev/of_clk - add managed lookup and provider registrations

[Matti Vaittinen]

With MFD devices the clk properties may be contained in MFD (parent) DT
node. Current devm_of_clk_add_hw_provider assumes the clk is bound to MFD
subdevice not to MFD device (parent). Add
devm_of_clk_add_hw_provider_parent to tackle this issue.

Also clkdev registration lacks of managed registration functions and it
seems few drivers do not drop clkdev lookups at exit. Add
devm_clk_hw_register_clkdev and devm_clk_release_clkdev to ease lookup
releasing at exit.

Signed-off-by: Matti Vaittinen 
---
 Documentation/driver-model/devres.txt |   3 +
 drivers/clk/clk.c |  28 ++--
 drivers/clk/clkdev.c  | 122 ++
 include/linux/clk-provider.h  |  11 +++
 include/linux/clkdev.h|   4 ++
 5 files changed, 136 insertions(+), 32 deletions(-)

diff --git a/Documentation/driver-model/devres.txt 
b/Documentation/driver-model/devres.txt
index 43681ca0837f..fac63760b01c 100644
--- a/Documentation/driver-model/devres.txt
+++ b/Documentation/driver-model/devres.txt
@@ -238,6 +238,9 @@ CLOCK
   devm_clk_put()
   devm_clk_hw_register()
   devm_of_clk_add_hw_provider()
+  devm_of_clk_add_parent_hw_provider()
+  devm_clk_hw_register_clkdev()
+  devm_clk_release_clkdev()
 
 DMA
   dmaenginem_async_device_register()
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index af011974d4ec..9bb921eb90f6 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -3893,12 +3893,12 @@ static void devm_of_clk_release_provider(struct device 
*dev, void *res)
of_clk_del_provider(*(struct device_node **)res);
 }
 
-int devm_of_clk_add_hw_provider(struct device *dev,
+static int __devm_of_clk_add_hw_provider(struct device *dev,
struct clk_hw *(*get)(struct of_phandle_args *clkspec,
  void *data),
-   void *data)
+   struct device_node *of_node, void *data)
 {
-   struct device_node **ptr, *np;
+   struct device_node **ptr;
int ret;
 
ptr = devres_alloc(devm_of_clk_release_provider, sizeof(*ptr),
@@ -3906,10 +3906,9 @@ int devm_of_clk_add_hw_provider(struct device *dev,
if (!ptr)
return -ENOMEM;
 
-   np = dev->of_node;
-   ret = of_clk_add_hw_provider(np, get, data);
+   *ptr = of_node;
+   ret = of_clk_add_hw_provider(of_node, get, data);
if (!ret) {
-   *ptr = np;
devres_add(dev, ptr);
} else {
devres_free(ptr);
@@ -3917,8 +3916,25 @@ int devm_of_clk_add_hw_provider(struct device *dev,
 
return ret;
 }
+int devm_of_clk_add_hw_provider(struct device *dev,
+   struct clk_hw *(*get)(struct of_phandle_args *clkspec,
+ void *data),
+   void *data)
+{
+   return __devm_of_clk_add_hw_provider(dev, get, dev->of_node, data);
+}
 EXPORT_SYMBOL_GPL(devm_of_clk_add_hw_provider);
 
+int devm_of_clk_add_parent_hw_provider(struct device *dev,
+   struct clk_hw *(*get)(struct of_phandle_args *clkspec,
+ void *data),
+   void *data)
+{
+   return __devm_of_clk_add_hw_provider(dev, get, dev->parent->of_node,
+data);
+}
+EXPORT_SYMBOL_GPL(devm_of_clk_add_parent_hw_provider);
+
 /**
  * of_clk_del_provider() - Remove a previously registered clock provider
  * @np: Device node pointer associated with clock provider
diff --git a/drivers/clk/clkdev.c b/drivers/clk/clkdev.c
index 9ab3db8b3988..f6100b6e06fd 100644
--- a/drivers/clk/clkdev.c
+++ b/drivers/clk/clkdev.c
@@ -401,6 +401,25 @@ static struct clk_lookup *__clk_register_clkdev(struct 
clk_hw *hw,
return cl;
 }
 
+static int do_clk_register_clkdev(struct clk_hw *hw,
+   struct clk_lookup **cl, const char *con_id, const char *dev_id)
+{
+
+   if (IS_ERR(hw))
+   return PTR_ERR(hw);
+   /*
+* Since dev_id can be NULL, and NULL is handled specially, we must
+* pass it as either a NULL format string, or with "%s".
+*/
+   if (dev_id)
+   *cl = __clk_register_clkdev(hw, con_id, "%s",
+  dev_id);
+   else
+   *cl = __clk_register_clkdev(hw, con_id, NULL);
+
+   return *cl ? 0 : -ENOMEM;
+}
+
 /**
  * clk_register_clkdev - register one clock lookup for a struct clk
  * @clk: struct clk to associate with all clk_lookups
@@ -420,20 +439,10 @@ int clk_register_clkdev(struct clk *clk, const char 
*con_id,
 {
struct clk_lookup *cl;
 
-   if (IS_ERR(clk))
-   return PTR_ERR(clk);
-
-   /*
-* Since dev_id can be NULL, and NULL is handled specially, we must
-* pass it as either a NULL format string, or with "%s".
-*/
-   if (dev_id)
-   cl = __clk_register_clkdev(__clk_get_hw(c

[PATCH v2 2/8] clk: clk-max77686: Clean clkdev lookup leak and use devm

[Matti Vaittinen]

clk-max77686 never clean clkdev lookup at remove. This can cause
oops if clk-max77686 is removed and inserted again. Fix leak by
using new devm clkdev lookup registration. Simplify also error
path by using new devm_of_clk_add_parent_hw_provider.

Signed-off-by: Matti Vaittinen 
---
 drivers/clk/clk-max77686.c | 25 -
 1 file changed, 4 insertions(+), 21 deletions(-)

diff --git a/drivers/clk/clk-max77686.c b/drivers/clk/clk-max77686.c
index 02551fe4b87c..b1920c1d9b76 100644
--- a/drivers/clk/clk-max77686.c
+++ b/drivers/clk/clk-max77686.c
@@ -235,7 +235,7 @@ static int max77686_clk_probe(struct platform_device *pdev)
return ret;
}
 
-   ret = clk_hw_register_clkdev(&max_clk_data->hw,
+   ret = devm_clk_hw_register_clkdev(dev, &max_clk_data->hw,
 max_clk_data->clk_idata.name, 
NULL);
if (ret < 0) {
dev_err(dev, "Failed to clkdev register: %d\n", ret);
@@ -244,8 +244,8 @@ static int max77686_clk_probe(struct platform_device *pdev)
}
 
if (parent->of_node) {
-   ret = of_clk_add_hw_provider(parent->of_node, 
of_clk_max77686_get,
-drv_data);
+   ret = devm_of_clk_add_parent_hw_provider(dev,
+   of_clk_max77686_get, drv_data);
 
if (ret < 0) {
dev_err(dev, "Failed to register OF clock provider: 
%d\n",
@@ -261,27 +261,11 @@ static int max77686_clk_probe(struct platform_device 
*pdev)
 1 << MAX77802_CLOCK_LOW_JITTER_SHIFT);
if (ret < 0) {
dev_err(dev, "Failed to config low-jitter: %d\n", ret);
-   goto remove_of_clk_provider;
+   return ret;
}
}
 
return 0;
-
-remove_of_clk_provider:
-   if (parent->of_node)
-   of_clk_del_provider(parent->of_node);
-
-   return ret;
-}
-
-static int max77686_clk_remove(struct platform_device *pdev)
-{
-   struct device *parent = pdev->dev.parent;
-
-   if (parent->of_node)
-   of_clk_del_provider(parent->of_node);
-
-   return 0;
 }
 
 static const struct platform_device_id max77686_clk_id[] = {
@@ -297,7 +281,6 @@ static struct platform_driver max77686_clk_driver = {
.name  = "max77686-clk",
},
.probe = max77686_clk_probe,
-   .remove = max77686_clk_remove,
.id_table = max77686_clk_id,
 };
 
-- 
2.14.3

[PATCH v2 3/8] clk: clk-s3c2410-dclk: clean up clkdev lookup leak

[Matti Vaittinen]

Use devm variant of clkdev lookup registration in order to avoid
clkdev lookup leak at device remove.

Signed-off-by: Matti Vaittinen 
---
 drivers/clk/samsung/clk-s3c2410-dclk.c | 15 ---
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/clk/samsung/clk-s3c2410-dclk.c 
b/drivers/clk/samsung/clk-s3c2410-dclk.c
index 0d92f3e5e3d9..07798debfec4 100644
--- a/drivers/clk/samsung/clk-s3c2410-dclk.c
+++ b/drivers/clk/samsung/clk-s3c2410-dclk.c
@@ -309,16 +309,17 @@ static int s3c24xx_dclk_probe(struct platform_device 
*pdev)
goto err_clk_register;
}
 
-   ret = clk_hw_register_clkdev(clk_table[MUX_DCLK0], "dclk0", NULL);
+   ret = devm_clk_hw_register_clkdev(&pdev->dev, clk_table[MUX_DCLK0],
+ "dclk0", NULL);
if (!ret)
-   ret = clk_hw_register_clkdev(clk_table[MUX_DCLK1], "dclk1",
-NULL);
+   ret = devm_clk_hw_register_clkdev(&pdev->dev,
+   clk_table[MUX_DCLK1], "dclk1", NULL);
if (!ret)
-   ret = clk_hw_register_clkdev(clk_table[MUX_CLKOUT0],
-"clkout0", NULL);
+   ret = devm_clk_hw_register_clkdev(&pdev->dev,
+   clk_table[MUX_CLKOUT0], "clkout0", NULL);
if (!ret)
-   ret = clk_hw_register_clkdev(clk_table[MUX_CLKOUT1],
-"clkout1", NULL);
+   ret = devm_clk_hw_register_clkdev(&pdev->dev,
+   clk_table[MUX_CLKOUT1], "clkout1", NULL);
if (ret) {
dev_err(&pdev->dev, "failed to register aliases, %d\n", ret);
goto err_clk_register;
-- 
2.14.3

[PATCH v2 4/8] clk: clk-st: avoid clkdev lookup leak at remove

[Matti Vaittinen]

Use devm based clkdev lookup registration to avoid leaking lookup
structures.

Signed-off-by: Matti Vaittinen 
---
 drivers/clk/x86/clk-st.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/x86/clk-st.c b/drivers/clk/x86/clk-st.c
index fb62f3938008..32d8df9bd853 100644
--- a/drivers/clk/x86/clk-st.c
+++ b/drivers/clk/x86/clk-st.c
@@ -52,7 +52,8 @@ static int st_clk_probe(struct platform_device *pdev)
0, st_data->base + MISCCLKCNTL1, OSCCLKENB,
CLK_GATE_SET_TO_DISABLE, NULL);
 
-   clk_hw_register_clkdev(hws[ST_CLK_GATE], "oscout1", NULL);
+   devm_clk_hw_register_clkdev(&pdev->dev, hws[ST_CLK_GATE], "oscout1",
+   NULL);
 
return 0;
 }
-- 
2.14.3

[PATCH v2 5/8] clk: clk-hi655x: Free of_provider at remove

[Matti Vaittinen]

use devm variant for of_provider registration so provider is freed
at exit.

Signed-off-by: Matti Vaittinen 
---
 drivers/clk/clk-hi655x.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/clk-hi655x.c b/drivers/clk/clk-hi655x.c
index 403a0188634a..394d0109104d 100644
--- a/drivers/clk/clk-hi655x.c
+++ b/drivers/clk/clk-hi655x.c
@@ -107,8 +107,8 @@ static int hi655x_clk_probe(struct platform_device *pdev)
if (ret)
return ret;
 
-   return of_clk_add_hw_provider(parent->of_node, of_clk_hw_simple_get,
-&hi655x_clk->clk_hw);
+   return devm_of_clk_add_parent_hw_provider(&pdev->dev,
+   of_clk_hw_simple_get, &hi655x_clk->clk_hw);
 }
 
 static struct platform_driver hi655x_clk_driver = {
-- 
2.14.3

[PATCH v2 6/8] clk: rk808: use managed version of of_provider registration

[Matti Vaittinen]

Simplify clean-up for rk808 by using managed version of of_provider
registration.

Signed-off-by: Matti Vaittinen 
---
 drivers/clk/clk-rk808.c | 15 ++-
 1 file changed, 2 insertions(+), 13 deletions(-)

diff --git a/drivers/clk/clk-rk808.c b/drivers/clk/clk-rk808.c
index 6461f2820a5b..177340edaae5 100644
--- a/drivers/clk/clk-rk808.c
+++ b/drivers/clk/clk-rk808.c
@@ -138,23 +138,12 @@ static int rk808_clkout_probe(struct platform_device 
*pdev)
if (ret)
return ret;
 
-   return of_clk_add_hw_provider(node, of_clk_rk808_get, rk808_clkout);
-}
-
-static int rk808_clkout_remove(struct platform_device *pdev)
-{
-   struct rk808 *rk808 = dev_get_drvdata(pdev->dev.parent);
-   struct i2c_client *client = rk808->i2c;
-   struct device_node *node = client->dev.of_node;
-
-   of_clk_del_provider(node);
-
-   return 0;
+   return devm_of_clk_add_parent_hw_provider(&pdev->dev,
+   of_clk_rk808_get, rk808_clkout);
 }
 
 static struct platform_driver rk808_clkout_driver = {
.probe = rk808_clkout_probe,
-   .remove = rk808_clkout_remove,
.driver = {
.name   = "rk808-clkout",
},
-- 
2.14.3

[PATCH v2 7/8] clk: clk-twl6040: Free of_provider at remove

[Matti Vaittinen]

use devm variant for of_provider registration so provider is freed
at exit.

Signed-off-by: Matti Vaittinen 
---
 drivers/clk/clk-twl6040.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/clk-twl6040.c b/drivers/clk/clk-twl6040.c
index 25dfe050ae9f..e9da09453eb2 100644
--- a/drivers/clk/clk-twl6040.c
+++ b/drivers/clk/clk-twl6040.c
@@ -108,9 +108,8 @@ static int twl6040_pdmclk_probe(struct platform_device 
*pdev)
 
platform_set_drvdata(pdev, clkdata);
 
-   return of_clk_add_hw_provider(pdev->dev.parent->of_node,
- of_clk_hw_simple_get,
- &clkdata->pdmclk_hw);
+   return devm_of_clk_add_parent_hw_provider(&pdev->dev,
+   of_clk_hw_simple_get, &clkdata->pdmclk_hw);
 }
 
 static struct platform_driver twl6040_pdmclk_driver = {
-- 
2.14.3

[PATCH v2 8/8] clk: apcs-msm8916: simplify probe cleanup by using devm

[Matti Vaittinen]

use devm variant for of_provider registration.

Signed-off-by: Matti Vaittinen 
---
 drivers/clk/qcom/apcs-msm8916.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/qcom/apcs-msm8916.c b/drivers/clk/qcom/apcs-msm8916.c
index b1cc8dbcd327..f4e0c136ab1a 100644
--- a/drivers/clk/qcom/apcs-msm8916.c
+++ b/drivers/clk/qcom/apcs-msm8916.c
@@ -96,8 +96,8 @@ static int qcom_apcs_msm8916_clk_probe(struct platform_device 
*pdev)
goto err;
}
 
-   ret = of_clk_add_hw_provider(parent->of_node, of_clk_hw_simple_get,
-&a53cc->clkr.hw);
+   ret = devm_of_clk_add_parent_hw_provider(dev, of_clk_hw_simple_get,
+&a53cc->clkr.hw);
if (ret) {
dev_err(dev, "failed to add clock provider: %d\n", ret);
goto err;
@@ -118,7 +118,6 @@ static int qcom_apcs_msm8916_clk_remove(struct 
platform_device *pdev)
struct device *parent = pdev->dev.parent;
 
clk_notifier_unregister(a53cc->pclk, &a53cc->clk_nb);
-   of_clk_del_provider(parent->of_node);
 
return 0;
 }
-- 
2.14.3

[PATCH V5 2/8] misc/pvpanic: Remove one extra comma

[Peng Hao]

There is an extra comma in pvpanic_device_ids, remove it.

Signed-off-by: Peng Hao 
---
 drivers/misc/pvpanic.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/misc/pvpanic.c b/drivers/misc/pvpanic.c
index fd86dab..059005c 100644
--- a/drivers/misc/pvpanic.c
+++ b/drivers/misc/pvpanic.c
@@ -35,7 +35,7 @@
 
 static const struct acpi_device_id pvpanic_device_ids[] = {
{ "QEMU0001", 0 },
-   { "", 0 },
+   { "", 0 }
 };
 MODULE_DEVICE_TABLE(acpi, pvpanic_device_ids);
 
-- 
1.8.3.1

Re: [PATCH 10/17] prmem: documentation

[Thomas Gleixner]

Igor,

On Thu, 1 Nov 2018, Igor Stoppa wrote:
> On 01/11/2018 01:19, Andy Lutomirski wrote:
> 
> > ISTM you don't need that atomic operation -- you could take a spinlock
> > and then just add one directly to the variable.
> 
> It was my intention to provide a 1:1 conversion of existing code, as it should
> be easier to verify the correctness of the conversion, as long as there isn't
> any significant degradation in performance.
> 
> The rework could be done afterward.

Please don't go there. The usual approach is to

  1) Rework existing code in a way that the new functionality can be added
 with minimal effort afterwards and without creating API horrors.

  2) Verify correctness of the rework

  3) Add the new functionality
  
That avoids creation of odd functionality and APIs in the first place, so
they won't be used in other places and does not leave half cleaned up code
around which will stick for a long time.

Thanks,

tglx

[PATCH v3 1/2] kretprobe: produce sane stack traces

[Aleksa Sarai]

Historically, kretprobe has always produced unusable stack traces
(kretprobe_trampoline is the only entry in most cases, because of the
funky stack pointer overwriting). This has caused quite a few annoyances
when using tracing to debug problems[1] -- since return values are only
available with kretprobes but stack traces were only usable for kprobes,
users had to probe both and then manually associate them.

With the advent of bpf_trace, users would have been able to do this
association in bpf, but this was less than ideal (because
bpf_get_stackid would still produce rubbish and programs that didn't
know better would get silly results). The main usecase for stack traces
(at least with bpf_trace) is for DTrace-style aggregation on stack
traces (both entry and exit). Therefore we cannot simply correct the
stack trace on exit -- we must stash away the stack trace and return the
entry stack trace when it is requested.

[1]: https://github.com/iovisor/bpftrace/issues/101

Cc: Brendan Gregg 
Cc: Christian Brauner 
Signed-off-by: Aleksa Sarai 
---
 Documentation/kprobes.txt |   6 +-
 include/linux/kprobes.h   |  27 +
 kernel/events/callchain.c |   8 +-
 kernel/kprobes.c  | 101 +-
 kernel/trace/trace.c  |  11 +-
 .../test.d/kprobe/kretprobe_stacktrace.tc |  25 +
 6 files changed, 173 insertions(+), 5 deletions(-)
 create mode 100644 
tools/testing/selftests/ftrace/test.d/kprobe/kretprobe_stacktrace.tc

diff --git a/Documentation/kprobes.txt b/Documentation/kprobes.txt
index 10f4499e677c..1965585848f4 100644
--- a/Documentation/kprobes.txt
+++ b/Documentation/kprobes.txt
@@ -597,7 +597,11 @@ address with the trampoline's address, stack backtraces 
and calls
 to __builtin_return_address() will typically yield the trampoline's
 address instead of the real return address for kretprobed functions.
 (As far as we can tell, __builtin_return_address() is used only
-for instrumentation and error reporting.)
+for instrumentation and error reporting.) However, since return probes
+are used extensively in tracing (where stack backtraces are useful),
+return probes will stash away the stack backtrace during function entry
+so that return probe handlers can use the entry backtrace instead of
+having a trace with just kretprobe_trampoline.
 
 If the number of times a function is called does not match the number
 of times it returns, registering a return probe on that function may
diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h
index e909413e4e38..1a1629544e56 100644
--- a/include/linux/kprobes.h
+++ b/include/linux/kprobes.h
@@ -40,6 +40,8 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 #include 
 
 #ifdef CONFIG_KPROBES
@@ -168,11 +170,18 @@ struct kretprobe {
raw_spinlock_t lock;
 };
 
+#define KRETPROBE_TRACE_SIZE 127
+struct kretprobe_trace {
+   int nr_entries;
+   unsigned long entries[KRETPROBE_TRACE_SIZE];
+};
+
 struct kretprobe_instance {
struct hlist_node hlist;
struct kretprobe *rp;
kprobe_opcode_t *ret_addr;
struct task_struct *task;
+   struct kretprobe_trace entry;
char data[0];
 };
 
@@ -371,6 +380,12 @@ void unregister_kretprobe(struct kretprobe *rp);
 int register_kretprobes(struct kretprobe **rps, int num);
 void unregister_kretprobes(struct kretprobe **rps, int num);
 
+struct kretprobe_instance *current_kretprobe_instance(void);
+void kretprobe_save_stack_trace(struct kretprobe_instance *ri,
+   struct stack_trace *trace);
+void kretprobe_perf_callchain_kernel(struct kretprobe_instance *ri,
+struct perf_callchain_entry_ctx *ctx);
+
 void kprobe_flush_task(struct task_struct *tk);
 void recycle_rp_inst(struct kretprobe_instance *ri, struct hlist_head *head);
 
@@ -397,6 +412,18 @@ static inline struct kprobe *kprobe_running(void)
 {
return NULL;
 }
+static inline struct kretprobe_instance *current_kretprobe_instance(void)
+{
+   return NULL;
+}
+static inline void kretprobe_save_stack_trace(struct kretprobe_instance *ri,
+ struct stack_trace *trace)
+{
+}
+static inline void kretprobe_perf_callchain_kernel(struct kretprobe_instance 
*ri,
+  struct 
perf_callchain_entry_ctx *ctx)
+{
+}
 static inline int register_kprobe(struct kprobe *p)
 {
return -ENOSYS;
diff --git a/kernel/events/callchain.c b/kernel/events/callchain.c
index 24a77c34e9ad..98edcd8a6987 100644
--- a/kernel/events/callchain.c
+++ b/kernel/events/callchain.c
@@ -12,6 +12,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "internal.h"
 
@@ -197,9 +198,14 @@ get_perf_callchain(struct pt_regs *regs, u32 init_nr, bool 
kernel, bool user,
ctx.contexts_maxed = false;
 
if (kernel && !user_mode(regs)) {
+   struct kret

[PATCH v3 2/2] trace: remove kretprobed checks

[Aleksa Sarai]

This is effectively a reversion of commit 76094a2cf46e ("ftrace:
distinguish kretprobe'd functions in trace logs"), as the checking of
kretprobe_trampoline *for tracing* is no longer necessary with the new
kretprobe stack trace changes.

Signed-off-by: Aleksa Sarai 
---
 kernel/trace/trace_output.c | 34 --
 1 file changed, 4 insertions(+), 30 deletions(-)

diff --git a/kernel/trace/trace_output.c b/kernel/trace/trace_output.c
index 6e6cc64faa38..951de16bd4fd 100644
--- a/kernel/trace/trace_output.c
+++ b/kernel/trace/trace_output.c
@@ -321,36 +321,14 @@ int trace_output_call(struct trace_iterator *iter, char 
*name, char *fmt, ...)
 }
 EXPORT_SYMBOL_GPL(trace_output_call);
 
-#ifdef CONFIG_KRETPROBES
-static inline const char *kretprobed(const char *name)
-{
-   static const char tramp_name[] = "kretprobe_trampoline";
-   int size = sizeof(tramp_name);
-
-   if (strncmp(tramp_name, name, size) == 0)
-   return "[unknown/kretprobe'd]";
-   return name;
-}
-#else
-static inline const char *kretprobed(const char *name)
-{
-   return name;
-}
-#endif /* CONFIG_KRETPROBES */
-
 static void
 seq_print_sym_short(struct trace_seq *s, const char *fmt, unsigned long 
address)
 {
char str[KSYM_SYMBOL_LEN];
 #ifdef CONFIG_KALLSYMS
-   const char *name;
-
kallsyms_lookup(address, NULL, NULL, NULL, str);
-
-   name = kretprobed(str);
-
-   if (name && strlen(name)) {
-   trace_seq_printf(s, fmt, name);
+   if (strlen(str)) {
+   trace_seq_printf(s, fmt, str);
return;
}
 #endif
@@ -364,13 +342,9 @@ seq_print_sym_offset(struct trace_seq *s, const char *fmt,
 {
char str[KSYM_SYMBOL_LEN];
 #ifdef CONFIG_KALLSYMS
-   const char *name;
-
sprint_symbol(str, address);
-   name = kretprobed(str);
-
-   if (name && strlen(name)) {
-   trace_seq_printf(s, fmt, name);
+   if (strlen(str)) {
+   trace_seq_printf(s, fmt, str);
return;
}
 #endif
-- 
2.19.1

[PATCH v3 0/2] kretprobe: produce sane stack traces

[Aleksa Sarai]

Historically, kretprobe has always produced unusable stack traces
(kretprobe_trampoline is the only entry in most cases, because of the
funky stack pointer overwriting). This has caused quite a few annoyances
when using tracing to debug problems[1] -- since return values are only
available with kretprobes but stack traces were only usable for kprobes,
users had to probe both and then manually associate them.

This patch series stores the stack trace within kretprobe_instance on
the kprobe entry used to set up the kretprobe. This allows for
DTrace-style stack aggregation between function entry and exit with
tools like BPFtrace -- which would not really be doable if the stack
unwinder understood kretprobe_trampoline.

We also revert commit 76094a2cf46e ("ftrace: distinguish kretprobe'd
functions in trace logs") and any follow-up changes because that code is
no longer necessary now that stack traces are sane. *However* this patch
might be a bit contentious since the original usecase (that ftrace
returns shouldn't show kretprobe_trampoline) is arguably still an
issue. Feel free to drop it if you think it is wrong.

Patch changelog:
 v3:
   * kprobe: fix build on !CONFIG_KPROBES
 v2:
   * documentation: mention kretprobe stack-stashing
   * ftrace: add self-test for fixed kretprobe stacktraces
   * ftrace: remove [unknown/kretprobe'd] handling
   * kprobe: remove needless EXPORT statements
   * kprobe: minor corrections to current_kretprobe_instance (switch
 away from hlist_for_each_entry_safe)
   * kprobe: make maximum stack size 127, which is the ftrace default

Aleksa Sarai (2):
  kretprobe: produce sane stack traces
  trace: remove kretprobed checks

 Documentation/kprobes.txt |   6 +-
 include/linux/kprobes.h   |  27 +
 kernel/events/callchain.c |   8 +-
 kernel/kprobes.c  | 101 +-
 kernel/trace/trace.c  |  11 +-
 kernel/trace/trace_output.c   |  34 +-
 .../test.d/kprobe/kretprobe_stacktrace.tc |  25 +
 7 files changed, 177 insertions(+), 35 deletions(-)
 create mode 100644 
tools/testing/selftests/ftrace/test.d/kprobe/kretprobe_stacktrace.tc

-- 
2.19.1

Re: [PATCH V5 2/8] misc/pvpanic: Remove one extra semicolon

[Mark Rutland]

On Thu, Nov 01, 2018 at 11:09:56PM +0800, Peng Hao wrote:
> There is an extra semicolon in pvpanic_device_ids, remove it.

This is a comma, not a semicolon.

How about:

  Since there should be nothing after the sentinel entry in an
  acpi_device_id list, it doesn't make sense for it to have a trailing
  comma.

  This patch removes the redundant comma.

... with that commit message (and the title fixed to say 'comma' rather
than 'semicolon':

Acked-by: Mark Rutland 

Either that, or simply drop this patch entirely.

Thanks,
Mark.

> 
> Signed-off-by: Peng Hao 
> ---
>  drivers/misc/pvpanic.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/misc/pvpanic.c b/drivers/misc/pvpanic.c
> index fd86dab..059005c 100644
> --- a/drivers/misc/pvpanic.c
> +++ b/drivers/misc/pvpanic.c
> @@ -35,7 +35,7 @@
>  
>  static const struct acpi_device_id pvpanic_device_ids[] = {
>   { "QEMU0001", 0 },
> - { "", 0 },
> + { "", 0 }
>  };
>  MODULE_DEVICE_TABLE(acpi, pvpanic_device_ids);
>  
> -- 
> 1.8.3.1
> 

Re: [PATCH V5 3/8] misc/pvpanic: revmove unnecessary header file

[Mark Rutland]

On Thu, Nov 01, 2018 at 11:09:57PM +0800, Peng Hao wrote:
> remove unnecessary header file init.h.
> 
> Signed-off-by: Peng Hao 

Acked-by: Mark Rutland 

Mark.

> ---
>  drivers/misc/pvpanic.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/misc/pvpanic.c b/drivers/misc/pvpanic.c
> index 059005c..66534d4 100644
> --- a/drivers/misc/pvpanic.c
> +++ b/drivers/misc/pvpanic.c
> @@ -22,7 +22,6 @@
>  
>  #include 
>  #include 
> -#include 
>  #include 
>  #include 
>  
> -- 
> 1.8.3.1
> 

Re: [PATCH V5 3/8] misc/pvpanic: revmove unnecessary header file

[Mark Rutland]

On Thu, Nov 01, 2018 at 11:34:29AM +, Mark Rutland wrote:
> On Thu, Nov 01, 2018 at 11:09:57PM +0800, Peng Hao wrote:
> > remove unnecessary header file init.h.
> > 
> > Signed-off-by: Peng Hao 
> 
> Acked-by: Mark Rutland 

... though please fix the title:

s/revmove/remove/

Mark.

Re: [PATCH V5 4/8] misc/pvpanic :convert to SPDX license tags

[Mark Rutland]

Hi,

Please fix the spacing for the colon in the commit title. It should be
formatted as:

misc/pvpanic: convert to SPDX license tags

On Thu, Nov 01, 2018 at 11:09:58PM +0800, Peng Hao wrote:
> This patch updates license to use SPDX-License-Identifier
> instead of verbose license text.
> 
> Signed-off-by: Peng Hao 

Other than the commit title, this looks fine to me:

Acked-by: Mark Rutland 

Mark.

> ---
>  drivers/misc/pvpanic.c | 16 ++--
>  1 file changed, 2 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/misc/pvpanic.c b/drivers/misc/pvpanic.c
> index 66534d4..0bcf1cd 100644
> --- a/drivers/misc/pvpanic.c
> +++ b/drivers/misc/pvpanic.c
> @@ -1,21 +1,9 @@
> +// SPDX-License-Identifier: GPL-2.0+
>  /*
> - *  pvpanic.c - pvpanic Device Support
> + *  Pvpanic Device Support
>   *
>   *  Copyright (C) 2013 Fujitsu.
>   *
> - *  This program is free software; you can redistribute it and/or modify
> - *  it under the terms of the GNU General Public License as published by
> - *  the Free Software Foundation; either version 2 of the License, or
> - *  (at your option) any later version.
> - *
> - *  This program is distributed in the hope that it will be useful,
> - *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> - *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> - *  GNU General Public License for more details.
> - *
> - *  You should have received a copy of the GNU General Public License
> - *  along with this program; if not, write to the Free Software
> - *  Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  
> 02110-1301  USA
>   */
>  
>  #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> -- 
> 1.8.3.1
> 

Re: [PATCH V5 6/8] misc/pvpanic: add MMIO support

[Mark Rutland]

On Thu, Nov 01, 2018 at 11:10:00PM +0800, Peng Hao wrote:
> On some architectures (e.g. arm64), it's preferable to use MMIO, since
> this can be used standalone. Add MMIO support to the pvpanic driver.
> 
> Signed-off-by: Peng Hao 

Acked-by: Mark Rutland 

Mark.

> ---
>  drivers/misc/pvpanic.c | 18 +-
>  1 file changed, 13 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/misc/pvpanic.c b/drivers/misc/pvpanic.c
> index c20fdff..c0b787e 100644
> --- a/drivers/misc/pvpanic.c
> +++ b/drivers/misc/pvpanic.c
> @@ -28,7 +28,7 @@
>  
>  #define PVPANIC_PANICKED (1 << 0)
>  
> -static u16 port;
> +static void __iomem *base;
>  
>  static struct acpi_driver pvpanic_driver = {
>   .name = "pvpanic",
> @@ -44,7 +44,7 @@
>  static void
>  pvpanic_send_event(unsigned int event)
>  {
> - outb(event, port);
> + iowrite8(event, base);
>  }
>  
>  static int
> @@ -66,8 +66,14 @@
>  {
>   struct resource r;
>  
> - if (acpi_dev_resource_io(res, &r) {
> - port = r.start;
> + if (acpi_dev_resource_io(res, &r) ||
> + acpi_dev_resource_memory(res, &r)) {
> + if (r.flags & IORESOURCE_IO)
> + base = (void __iomem *) ioport_map(r.start,
> + r.end - r.start + 1);
> + else
> + base = ioremap(r.start, r.end - r.start + 1);
> +
>   return AE_OK;
>   }
>  
> @@ -89,7 +95,7 @@ static int pvpanic_add(struct acpi_device *device)
>   acpi_walk_resources(device->handle, METHOD_NAME__CRS,
>   pvpanic_walk_resources, NULL);
>  
> - if (!port)
> + if (!base)
>   return -ENODEV;
>  
>   atomic_notifier_chain_register(&panic_notifier_list,
> @@ -103,6 +109,8 @@ static int pvpanic_remove(struct acpi_device *device)
>  
>   atomic_notifier_chain_unregister(&panic_notifier_list,
>&pvpanic_panic_nb);
> +
> + iounmap(base);
>   return 0;
>  }
>  
> -- 
> 1.8.3.1
> 

Re: [PATCH V5 8/8] dt-bindings/misc/pvpanic :add document for pvpanic-mmio

[Mark Rutland]

Please fix the spacing in the commit title. It should be formatted:

  dt-bindings: misc/pvpanic: add document for pvpanic-mmio

On Thu, Nov 01, 2018 at 11:10:02PM +0800, Peng Hao wrote:
> Add dt-bindings document for "qemu:pvpanic-mmio".
> 
> Signed-off-by: Peng Hao 

With the title fixed:

Acked-by: Mark Rutland 

When sending DT bindings in future, please follow the process in:

  Documentation/devicetree/bindings/submitting-patches.txt

... e.g., send the binding before the code using the binding.

Thanks,
Mark.

> ---
>  .../devicetree/bindings/misc/pvpanic-mmio.txt  | 29 
> ++
>  1 file changed, 29 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/misc/pvpanic-mmio.txt
> 
> diff --git a/Documentation/devicetree/bindings/misc/pvpanic-mmio.txt 
> b/Documentation/devicetree/bindings/misc/pvpanic-mmio.txt
> new file mode 100644
> index 000..985e907
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/misc/pvpanic-mmio.txt
> @@ -0,0 +1,29 @@
> +* QEMU PVPANIC MMIO Configuration bindings
> +
> +QEMU's emulation / virtualization targets provide the following PVPANIC
> +MMIO Configuration interface on the "virt" machine.
> +type:
> +
> +- a read-write, 16-bit wide data register.
> +
> +QEMU exposes the data register to guests as memory mapped registers.
> +
> +Required properties:
> +
> +- compatible: "qemu,pvpanic-mmio".
> +- reg: the MMIO region used by the device.
> +  * Bytes 0x0  Write panic event to the reg when guest OS panics.
> +  * Bytes 0x1  Reserved.
> +
> +Example:
> +
> +/ {
> +#size-cells = <0x2>;
> +#address-cells = <0x2>;
> +
> +pvpanic-mmio@906 {
> +compatible = "qemu,pvpanic-mmio";
> +reg = <0x0 0x906 0x0 0x2>;
> +};
> +};
> +
> -- 
> 1.8.3.1
> 

Re: [PATCH V5 7/8] misc/pvpanic: add support to get pvpanic device info by FDT

[Mark Rutland]

On Thu, Nov 01, 2018 at 11:10:01PM +0800, Peng Hao wrote:
> By default, when ACPI tables and FDT coexist for ARM64,
> current kernel takes precedence over FDT to get device information.
> Virt machine in qemu provides both FDT and ACPI table. This patch
> increases the way to get information through FDT.
> 
> Signed-off-by: Peng Hao 

Acked-by: Mark Rutland 

Mark.

> ---
>  drivers/misc/pvpanic.c | 66 
> +++---
>  1 file changed, 63 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/misc/pvpanic.c b/drivers/misc/pvpanic.c
> index c0b787e..9d9f0d7 100644
> --- a/drivers/misc/pvpanic.c
> +++ b/drivers/misc/pvpanic.c
> @@ -3,15 +3,18 @@
>   *  Pvpanic Device Support
>   *
>   *  Copyright (C) 2013 Fujitsu.
> - *
> + *  Copyright (C) 2018 ZTE.
>   */
>  
>  #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>  
> +#include 
>  #include 
>  #include 
> +#include 
> +#include 
> +#include 
>  #include 
> -#include 
>  
>  MODULE_AUTHOR("Hu Tao ");
>  MODULE_DESCRIPTION("pvpanic device driver");
> @@ -60,6 +63,32 @@
>   .priority = 1, /* let this called before broken drm_fb_helper */
>  };
>  
> +static int pvpanic_mmio_probe(struct platform_device *pdev)
> +{
> + struct resource *mem;
> +
> + mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (!mem)
> + return -EINVAL;
> +
> + base = devm_ioremap_resource(&pdev->dev, mem);
> + if (base == NULL)
> + return -EFAULT;
> +
> + atomic_notifier_chain_register(&panic_notifier_list,
> +&pvpanic_panic_nb);
> +
> + return 0;
> +}
> +
> +static int pvpanic_mmio_remove(struct platform_device *pdev)
> +{
> +
> + atomic_notifier_chain_unregister(&panic_notifier_list,
> +  &pvpanic_panic_nb);
> +
> + return 0;
> +}
>  
>  static acpi_status
>  pvpanic_walk_resources(struct acpi_resource *res, void *context)
> @@ -114,4 +143,35 @@ static int pvpanic_remove(struct acpi_device *device)
>   return 0;
>  }
>  
> -module_acpi_driver(pvpanic_driver);
> +static const struct of_device_id pvpanic_mmio_match[] = {
> + { .compatible = "qemu,pvpanic-mmio", },
> + {}
> +};
> +
> +static struct platform_driver pvpanic_mmio_driver = {
> + .driver = {
> + .name = "pvpanic-mmio",
> + .of_match_table = pvpanic_mmio_match,
> + },
> + .probe = pvpanic_mmio_probe,
> + .remove = pvpanic_mmio_remove,
> +};
> +
> +static int __init pvpanic_mmio_init(void)
> +{
> + if (acpi_disabled)
> + return platform_driver_register(&pvpanic_mmio_driver);
> + else
> + return acpi_bus_register_driver(&pvpanic_driver);
> +}
> +
> +static void __exit pvpanic_mmio_exit(void)
> +{
> + if (acpi_disabled)
> + platform_driver_unregister(&pvpanic_mmio_driver);
> + else
> + acpi_bus_unregister_driver(&pvpanic_driver);
> +}
> +
> +module_init(pvpanic_mmio_init);
> +module_exit(pvpanic_mmio_exit);
> -- 
> 1.8.3.1
> 

Re: [PATCH V5 1/8] pvpanic: move pvpanic to misc as common driver

[Andy Shevchenko]

On Thu, Nov 1, 2018 at 8:59 AM Peng Hao  wrote:
>
> move pvpanic.c from drivers/platform/x86 to drivers/misc.
> following patches will use pvpanic device in arm64.

There are few items you need to address in the next revision:
- use proper English grammar, i.e. Capitalize beginning of sentenses
- don't forget the tags people gave you in case you didn't (much) change a patch

Other than that, this one looks good to me

Reviewed-by: Andy Shevchenko 

P.S. WRT tags this one in v6 should have something like:
...

Reviewed-by: me
Acked-by: Mark
Signed-off-by: you

at the end of commit message

>
> Signed-off-by: Peng Hao 
> ---
>  drivers/misc/Kconfig | 8 +++
>  drivers/misc/Makefile| 1 +
>  drivers/{platform/x86 => misc}/pvpanic.c | 0
>  drivers/platform/x86/Kconfig | 8 
>  drivers/platform/x86/Makefile| 1 -
>  5 files changed, 9 insertions(+), 9 deletions(-)
>  rename drivers/{platform/x86 => misc}/pvpanic.c (100%)
>
> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
> index 3726eac..1ba0f62 100644
> --- a/drivers/misc/Kconfig
> +++ b/drivers/misc/Kconfig
> @@ -513,6 +513,14 @@ config MISC_RTSX
> tristate
> default MISC_RTSX_PCI || MISC_RTSX_USB
>
> +config PVPANIC
> +   tristate "pvpanic device support"
> +   depends on ACPI || OF
> +   help
> + This driver provides support for the pvpanic device.  pvpanic is
> + a paravirtualized device provided by QEMU; it lets a virtual machine
> + (guest) communicate panic events to the host.
> +
>  source "drivers/misc/c2port/Kconfig"
>  source "drivers/misc/eeprom/Kconfig"
>  source "drivers/misc/cb710/Kconfig"
> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
> index af22bbc..df951f4 100644
> --- a/drivers/misc/Makefile
> +++ b/drivers/misc/Makefile
> @@ -58,3 +58,4 @@ obj-$(CONFIG_ASPEED_LPC_SNOOP)+= aspeed-lpc-snoop.o
>  obj-$(CONFIG_PCI_ENDPOINT_TEST)+= pci_endpoint_test.o
>  obj-$(CONFIG_OCXL) += ocxl/
>  obj-$(CONFIG_MISC_RTSX)+= cardreader/
> +obj-$(CONFIG_PVPANIC)  += pvpanic.o
> diff --git a/drivers/platform/x86/pvpanic.c b/drivers/misc/pvpanic.c
> similarity index 100%
> rename from drivers/platform/x86/pvpanic.c
> rename to drivers/misc/pvpanic.c
> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> index 0c1aa6c..4ac276c 100644
> --- a/drivers/platform/x86/Kconfig
> +++ b/drivers/platform/x86/Kconfig
> @@ -1121,14 +1121,6 @@ config INTEL_SMARTCONNECT
>   This driver checks to determine whether the device has Intel Smart
>   Connect enabled, and if so disables it.
>
> -config PVPANIC
> -   tristate "pvpanic device support"
> -   depends on ACPI
> -   ---help---
> - This driver provides support for the pvpanic device.  pvpanic is
> - a paravirtualized device provided by QEMU; it lets a virtual machine
> - (guest) communicate panic events to the host.
> -
>  config INTEL_PMC_IPC
> tristate "Intel PMC IPC Driver"
> depends on ACPI
> diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
> index e6d1bec..e88f44e 100644
> --- a/drivers/platform/x86/Makefile
> +++ b/drivers/platform/x86/Makefile
> @@ -75,7 +75,6 @@ obj-$(CONFIG_APPLE_GMUX)  += apple-gmux.o
>  obj-$(CONFIG_INTEL_RST)+= intel-rst.o
>  obj-$(CONFIG_INTEL_SMARTCONNECT)   += intel-smartconnect.o
>
> -obj-$(CONFIG_PVPANIC)   += pvpanic.o
>  obj-$(CONFIG_ALIENWARE_WMI)+= alienware-wmi.o
>  obj-$(CONFIG_INTEL_PMC_IPC)+= intel_pmc_ipc.o
>  obj-$(CONFIG_TOUCHSCREEN_DMI)  += touchscreen_dmi.o
> --
> 1.8.3.1
>


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH V5 2/8] misc/pvpanic: Remove one extra semicolon

[Andy Shevchenko]

On Thu, Nov 1, 2018 at 1:34 PM Mark Rutland  wrote:
>
> On Thu, Nov 01, 2018 at 11:09:56PM +0800, Peng Hao wrote:
> > There is an extra semicolon in pvpanic_device_ids, remove it.
>
> This is a comma, not a semicolon.
>
> How about:
>
>   Since there should be nothing after the sentinel entry in an
>   acpi_device_id list, it doesn't make sense for it to have a trailing
>   comma.
>
>   This patch removes the redundant comma.
>
> ... with that commit message (and the title fixed to say 'comma' rather
> than 'semicolon':

Agree on this, but...

>
> Acked-by: Mark Rutland 
>
> Either that, or simply drop this patch entirely.

...point was to gather style changes here, like moving linux/acpi.h.

And definitely the style changes should go last in the series.
If you decide to leave this patch, it would be 8/8.

>
> Thanks,
> Mark.
>
> >
> > Signed-off-by: Peng Hao 
> > ---
> >  drivers/misc/pvpanic.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/misc/pvpanic.c b/drivers/misc/pvpanic.c
> > index fd86dab..059005c 100644
> > --- a/drivers/misc/pvpanic.c
> > +++ b/drivers/misc/pvpanic.c
> > @@ -35,7 +35,7 @@
> >
> >  static const struct acpi_device_id pvpanic_device_ids[] = {
> >   { "QEMU0001", 0 },
> > - { "", 0 },
> > + { "", 0 }
> >  };
> >  MODULE_DEVICE_TABLE(acpi, pvpanic_device_ids);
> >
> > --
> > 1.8.3.1
> >



-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH V5 3/8] misc/pvpanic: revmove unnecessary header file

[Andy Shevchenko]

On Thu, Nov 1, 2018 at 9:00 AM Peng Hao  wrote:
>
> remove unnecessary header file init.h.
>

Same comments as per patch 1, on top of this is ordering issue. This
patch should be 6/8.
Also, address Mark's comments, please.

Otherwise,
Reviewed-by: Andy Shevchenko 

> Signed-off-by: Peng Hao 
> ---
>  drivers/misc/pvpanic.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/drivers/misc/pvpanic.c b/drivers/misc/pvpanic.c
> index 059005c..66534d4 100644
> --- a/drivers/misc/pvpanic.c
> +++ b/drivers/misc/pvpanic.c
> @@ -22,7 +22,6 @@
>
>  #include 
>  #include 
> -#include 
>  #include 
>  #include 
>
> --
> 1.8.3.1
>


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH V5 4/8] misc/pvpanic :convert to SPDX license tags

[Andy Shevchenko]

On Thu, Nov 1, 2018 at 9:00 AM Peng Hao  wrote:
>
> This patch updates license to use SPDX-License-Identifier
> instead of verbose license text.
>

After addressing Mark's comment consider to move this to be patch 7/8
in the series.
Otherwise,
Reviewed-by: Andy Shevchenko 

> Signed-off-by: Peng Hao 
> ---
>  drivers/misc/pvpanic.c | 16 ++--
>  1 file changed, 2 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/misc/pvpanic.c b/drivers/misc/pvpanic.c
> index 66534d4..0bcf1cd 100644
> --- a/drivers/misc/pvpanic.c
> +++ b/drivers/misc/pvpanic.c
> @@ -1,21 +1,9 @@
> +// SPDX-License-Identifier: GPL-2.0+
>  /*
> - *  pvpanic.c - pvpanic Device Support
> + *  Pvpanic Device Support
>   *
>   *  Copyright (C) 2013 Fujitsu.
>   *
> - *  This program is free software; you can redistribute it and/or modify
> - *  it under the terms of the GNU General Public License as published by
> - *  the Free Software Foundation; either version 2 of the License, or
> - *  (at your option) any later version.
> - *
> - *  This program is distributed in the hope that it will be useful,
> - *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> - *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> - *  GNU General Public License for more details.
> - *
> - *  You should have received a copy of the GNU General Public License
> - *  along with this program; if not, write to the Free Software
> - *  Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  
> 02110-1301  USA
>   */
>
>  #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> --
> 1.8.3.1
>


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH V5 5/8] misc/pvpanic: simplify the code using acpi_dev_resource_io

[Andy Shevchenko]

On Thu, Nov 1, 2018 at 9:00 AM Peng Hao  wrote:
>
> use acpi_dev_resource_io API.
>

Same comments as per patch 1.
This should be patch 2/8 in the series.

Otherwise,
Reviewed-by: Andy Shevchenko 

Also, you might consider to use Suggested-by tag here.

> Signed-off-by: Peng Hao 
> ---
>  drivers/misc/pvpanic.c | 14 ++
>  1 file changed, 6 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/misc/pvpanic.c b/drivers/misc/pvpanic.c
> index 0bcf1cd..c20fdff 100644
> --- a/drivers/misc/pvpanic.c
> +++ b/drivers/misc/pvpanic.c
> @@ -64,17 +64,15 @@
>  static acpi_status
>  pvpanic_walk_resources(struct acpi_resource *res, void *context)
>  {
> -   switch (res->type) {
> -   case ACPI_RESOURCE_TYPE_END_TAG:
> -   return AE_OK;
> +   struct resource r;
>
> -   case ACPI_RESOURCE_TYPE_IO:
> -   port = res->data.io.minimum;
> +   if (acpi_dev_resource_io(res, &r)) {
> +   port = r.start;
> return AE_OK;
> -
> -   default:
> -   return AE_ERROR;
> }
> +
> +   return AE_ERROR;
> +
>  }
>
>  static int pvpanic_add(struct acpi_device *device)
> --
> 1.8.3.1
>


-- 
With Best Regards,
Andy Shevchenko

RE: [PATCH v5 00/13] arch/x86: AMD QoS support

[Moger, Babu]

Fenghua,

> -Original Message-
> From: linux-kernel-ow...@vger.kernel.org  ow...@vger.kernel.org> On Behalf Of Fenghua Yu
> Sent: Wednesday, October 31, 2018 4:55 PM
> To: Moger, Babu 
> Cc: t...@linutronix.de; mi...@redhat.com; b...@alien8.de; cor...@lwn.net;
> reinette.cha...@intel.com; pet...@infradead.org;
> gre...@linuxfoundation.org; da...@davemloft.net; akpm@linux-
> foundation.org; h...@zytor.com; x...@kernel.org;
> mchehab+sams...@kernel.org; a...@arndb.de;
> kstew...@linuxfoundation.org; pombreda...@nexb.com;
> raf...@kernel.org; kirill.shute...@linux.intel.com; tony.l...@intel.com;
> qianyue...@alibaba-inc.com; xiaochen.s...@intel.com;
> pbonz...@redhat.com; Singh, Brijesh ; Hurwitz,
> Sherry ; dw...@infradead.org; Lendacky,
> Thomas ; l...@kernel.org; j...@8bytes.org;
> ja...@google.com; vkuzn...@redhat.com; r...@alum.mit.edu;
> jpoim...@redhat.com; linux-ker...@vger.kernel.org; linux-
> d...@vger.kernel.org
> Subject: Re: [PATCH v5 00/13] arch/x86: AMD QoS support
> 
> On Thu, Oct 18, 2018 at 10:52:00PM +, Moger, Babu wrote:
> > Changes from v4 -> v5:
> >  b. The functions update_mba_bw and set_mba_sc is not required for
> AMD.
> > Removed all the changes related to these functions.
> 
> Hi, Babu,
> 
> In v4, you says AMD won't support MBA software controller.
> 
> In v5, does AMD support MBA software controller or not? The v5 patches

No, AMD does not support MBA software controller. set_mba_sc will always
return -EINVAL on AMD(because delay_linear is set false). That is the
reason, I did not add this check. I will add this check in next refresh.
Will add Suggested-by you. Thanks

> show the feature is supported now. If that's a code bug, you may need
> the following patch?
> 
> x86/resctrl: Only Intel RDT supports MBA software controller
> 
> AMD doesn't support the feature.
> 
> Signed-off-by: Fenghua Yu 
> 
> diff --git a/arch/x86/kernel/cpu/resctrl_rdtgroup.c
> b/arch/x86/kernel/cpu/resctrl_rdtgroup.c
> index 8b6b4a8bb7ca..89dd9b7c9dd7 100644
> --- a/arch/x86/kernel/cpu/resctrl_rdtgroup.c
> +++ b/arch/x86/kernel/cpu/resctrl_rdtgroup.c
> @@ -1878,9 +1878,11 @@ static int parse_rdtgroupfs_options(char *data)
>   if (ret)
>   goto out;
>   } else if (!strcmp(token, "mba_MBps")) {
> - ret = set_mba_sc(true);
> - if (ret)
> - goto out;
> + if (boot_cpu_data.x86_vendor ==
> X86_VENDOR_INTEL) {
> + ret = set_mba_sc(true);
> + if (ret)
> + goto out;
> + }
>   } else {
>   ret = -EINVAL;
>   goto out;

Re: [PATCH v3 1/2] kretprobe: produce sane stack traces

[Masami Hiramatsu]

Hi Aleksa,

On Thu,  1 Nov 2018 19:35:50 +1100
Aleksa Sarai  wrote:
[...]
> diff --git 
> a/tools/testing/selftests/ftrace/test.d/kprobe/kretprobe_stacktrace.tc 
> b/tools/testing/selftests/ftrace/test.d/kprobe/kretprobe_stacktrace.tc

Please split the test case as an independent patch.

> new file mode 100644
> index ..03146c6a1a3c
> --- /dev/null
> +++ b/tools/testing/selftests/ftrace/test.d/kprobe/kretprobe_stacktrace.tc
> @@ -0,0 +1,25 @@
> +#!/bin/sh
> +# SPDX-License-Identifier: GPL-2.0+
> +# description: Kretprobe dynamic event with a stacktrace
> +
> +[ -f kprobe_events ] || exit_unsupported # this is configurable
> +
> +echo 0 > events/enable
> +echo 1 > options/stacktrace
> +
> +echo 'r:teststackprobe sched_fork $retval' > kprobe_events
> +grep teststackprobe kprobe_events
> +test -d events/kprobes/teststackprobe

Hmm, what happen if we have 2 or more kretprobes on same stack?
It seems you just save stack in pre_handler, but that stack can already
includes another kretprobe's trampline address...

Thank you,

> +
> +clear_trace
> +echo 1 > events/kprobes/teststackprobe/enable
> +( echo "forked")
> +echo 0 > events/kprobes/teststackprobe/enable
> +
> +# Make sure we don't see kretprobe_trampoline and we see _do_fork.
> +! grep 'kretprobe' trace
> +grep '_do_fork' trace
> +
> +echo '-:teststackprobe' >> kprobe_events
> +clear_trace
> +test -d events/kprobes/teststackprobe && exit_fail || exit_pass
> -- 
> 2.19.1
> 


-- 
Masami Hiramatsu 

Re: [PATCH V5 0/5] KVM: X86: Introducing ROE Protection Kernel Hardening

[Igor Stoppa]

Hello Ahmed,

On 01/11/2018 01:21, Ahmed Soliman wrote:

Hello Igor,

This is very interesting, because it seems a very good match to the work
I'm doing, for supporting the creation of more targets for protection:

https://www.openwall.com/lists/kernel-hardening/2018/10/23/3

In my case the protection would extend also to write-rate type of data.
There is an open problem of identifying legitimate write-rare
operations, however it should be possible to provide at least a certain
degree of confidence.


I have checked your patch set. In our work we were originally planning to do
something similar to write_rare just so we can differentiate between memory
chunks that may be modified and those that will be set once and never modify.
I see you are planning to do a white paper too, actually we are doing
an academic
paper based on our work. If you would like to collaborate, so that ROE
and write_rare
would integrate well from the beginning, we will be glad to do so.


The offer is very kind, thanks a lot.
I will contact you in private.

--
igor

Re: [PATCH 10/17] prmem: documentation

[Igor Stoppa]

On 01/11/2018 10:21, Thomas Gleixner wrote:

On Thu, 1 Nov 2018, Igor Stoppa wrote:



The rework could be done afterward.


Please don't go there. The usual approach is to

   1) Rework existing code in a way that the new functionality can be added
  with minimal effort afterwards and without creating API horrors.



Thanks a lot for the advice.
It makes things even easier for me, as I can start the rework, while the 
discussion on the actual write-rare mechanism continues.


--
igor

Re: [PATCH 10/17] prmem: documentation

[Nadav Amit]

From: Peter Zijlstra
Sent: October 31, 2018 at 9:08:35 AM GMT
> To: Nadav Amit 
> Cc: Andy Lutomirski , Matthew Wilcox 
> , Kees Cook , Igor Stoppa 
> , Mimi Zohar , Dave Chinner 
> , James Morris , Michal Hocko 
> , Kernel Hardening , 
> linux-integrity , linux-security-module 
> , Igor Stoppa 
> , Dave Hansen , Jonathan 
> Corbet , Laura Abbott , Randy Dunlap 
> , Mike Rapoport , open 
> list:DOCUMENTATION , LKML 
> , Thomas Gleixner 
> Subject: Re: [PATCH 10/17] prmem: documentation
> 
> 
> On Tue, Oct 30, 2018 at 04:18:39PM -0700, Nadav Amit wrote:
>>> Nadav, want to resubmit your series? IIRC the only thing wrong with it was
>>> that it was a big change and we wanted a simpler fix to backport. But
>>> that’s all done now, and I, at least, rather liked your code. :)
>> 
>> I guess since it was based on your ideas…
>> 
>> Anyhow, the only open issue that I have with v2 is Peter’s wish that I would
>> make kgdb use of poke_text() less disgusting. I still don’t know exactly
>> how to deal with it.
>> 
>> Perhaps it (fixing kgdb) can be postponed? In that case I can just resend
>> v2.
> 
> Oh man, I completely forgot about kgdb :/
> 
> Also, would it be possible to entirely remove that kmap fallback path?

Let me see what I can do about it.

Re: [PATCH V5 6/8] misc/pvpanic: add MMIO support

[Andy Shevchenko]

On Thu, Nov 1, 2018 at 9:00 AM Peng Hao  wrote:
>
> On some architectures (e.g. arm64), it's preferable to use MMIO, since
> this can be used standalone. Add MMIO support to the pvpanic driver.

> -   if (acpi_dev_resource_io(res, &r) {
> -   port = r.start;
> +   if (acpi_dev_resource_io(res, &r) ||
> +   acpi_dev_resource_memory(res, &r)) {


> +   if (r.flags & IORESOURCE_IO)

You need spend some time to understand better how acpi_dev_resource_*() works.
Hint: this condition is redundant.

> +   base = (void __iomem *) ioport_map(r.start,
> +   r.end - r.start + 1);
> +   else
> +   base = ioremap(r.start, r.end - r.start + 1);
> +


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 10/17] prmem: documentation

[Peter Zijlstra]

On Wed, Oct 31, 2018 at 03:57:21PM -0700, Andy Lutomirski wrote:
> > On Oct 31, 2018, at 2:00 PM, Peter Zijlstra  wrote:
> >> On Wed, Oct 31, 2018 at 01:36:48PM -0700, Andy Lutomirski wrote:

> >> This stuff is called *rare* write for a reason. Do we really want to
> >> allow atomics beyond just store-release?  Taking a big lock and then
> >> writing in the right order should cover everything, no?
> > 
> > Ah, so no. That naming is very misleading.
> > 
> > We modify page-tables a _lot_. The point is that only a few sanctioned
> > sites are allowed writing to it, not everybody.
> > 
> > I _think_ the use-case for atomics is updating the reference counts of
> > objects that are in this write-rare domain. But I'm not entirely clear
> > on that myself either. I just really want to avoid duplicating that
> > stuff.
> 
> Sounds nuts. Doing a rare-write is many hundreds of cycles at best.

Yes, which is why I'm somewhat sceptical of the whole endeavour.

Re: [PATCH v3 1/2] kretprobe: produce sane stack traces

[Aleksa Sarai]

On 2018-11-02, Masami Hiramatsu  wrote:
> Please split the test case as an independent patch.

Will do. Should the Documentation/ change also be a separate patch?

> > new file mode 100644
> > index ..03146c6a1a3c
> > --- /dev/null
> > +++ b/tools/testing/selftests/ftrace/test.d/kprobe/kretprobe_stacktrace.tc
> > @@ -0,0 +1,25 @@
> > +#!/bin/sh
> > +# SPDX-License-Identifier: GPL-2.0+
> > +# description: Kretprobe dynamic event with a stacktrace
> > +
> > +[ -f kprobe_events ] || exit_unsupported # this is configurable
> > +
> > +echo 0 > events/enable
> > +echo 1 > options/stacktrace
> > +
> > +echo 'r:teststackprobe sched_fork $retval' > kprobe_events
> > +grep teststackprobe kprobe_events
> > +test -d events/kprobes/teststackprobe
> 
> Hmm, what happen if we have 2 or more kretprobes on same stack?
> It seems you just save stack in pre_handler, but that stack can already
> includes another kretprobe's trampline address...

Yeah, you're quite right...

My first instinct was to do something awful like iterate over the set of
"kretprobe_instance"s with ->task == current, and then correct
kretprobe_trampoline entries using ->ret_addr. (I think this would be
correct because each task can only be in one context at once, and in
order to get to a particular kretprobe all of your caller kretprobes
were inserted before you and any sibling or later kretprobe_instances
will have been removed. But I might be insanely wrong on this front.)

However (as I noted in the other thread), there is a problem where
kretprobe_trampoline actually stops the unwinder in its tracks and thus
you only get the first kretprobe_trampoline. This is something I'm going
to look into some more (despite not having made progress on it last
time) since now it's something that actually needs to be fixed (and
as I mentioned in the other thread, show_stack() actually works on x86
in this context unlike the other stack_trace users).

-- 
Aleksa Sarai
Senior Software Engineer (Containers)
SUSE Linux GmbH



signature.asc
Description: PGP signature

Re: [PATCH v3 1/2] kretprobe: produce sane stack traces

[Steven Rostedt]

On Thu,  1 Nov 2018 19:35:50 +1100
Aleksa Sarai  wrote:

> Historically, kretprobe has always produced unusable stack traces
> (kretprobe_trampoline is the only entry in most cases, because of the
> funky stack pointer overwriting). This has caused quite a few annoyances
> when using tracing to debug problems[1] -- since return values are only
> available with kretprobes but stack traces were only usable for kprobes,
> users had to probe both and then manually associate them.
> 
> With the advent of bpf_trace, users would have been able to do this
> association in bpf, but this was less than ideal (because
> bpf_get_stackid would still produce rubbish and programs that didn't
> know better would get silly results). The main usecase for stack traces
> (at least with bpf_trace) is for DTrace-style aggregation on stack
> traces (both entry and exit). Therefore we cannot simply correct the
> stack trace on exit -- we must stash away the stack trace and return the
> entry stack trace when it is requested.
> 
> [1]: https://github.com/iovisor/bpftrace/issues/101
> 
> Cc: Brendan Gregg 
> Cc: Christian Brauner 
> Signed-off-by: Aleksa Sarai 
> ---
>  Documentation/kprobes.txt |   6 +-
>  include/linux/kprobes.h   |  27 +
>  kernel/events/callchain.c |   8 +-
>  kernel/kprobes.c  | 101 +-
>  kernel/trace/trace.c  |  11 +-
>  .../test.d/kprobe/kretprobe_stacktrace.tc |  25 +
>  6 files changed, 173 insertions(+), 5 deletions(-)
>  create mode 100644 
> tools/testing/selftests/ftrace/test.d/kprobe/kretprobe_stacktrace.tc
> 
> diff --git a/Documentation/kprobes.txt b/Documentation/kprobes.txt
> index 10f4499e677c..1965585848f4 100644
> --- a/Documentation/kprobes.txt
> +++ b/Documentation/kprobes.txt
> @@ -597,7 +597,11 @@ address with the trampoline's address, stack backtraces 
> and calls
>  to __builtin_return_address() will typically yield the trampoline's
>  address instead of the real return address for kretprobed functions.
>  (As far as we can tell, __builtin_return_address() is used only
> -for instrumentation and error reporting.)
> +for instrumentation and error reporting.) However, since return probes
> +are used extensively in tracing (where stack backtraces are useful),
> +return probes will stash away the stack backtrace during function entry
> +so that return probe handlers can use the entry backtrace instead of
> +having a trace with just kretprobe_trampoline.
>  
>  If the number of times a function is called does not match the number
>  of times it returns, registering a return probe on that function may
> diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h
> index e909413e4e38..1a1629544e56 100644
> --- a/include/linux/kprobes.h
> +++ b/include/linux/kprobes.h
> @@ -40,6 +40,8 @@
>  #include 
>  #include 
>  #include 
> +#include 
> +#include 
>  #include 
>  
>  #ifdef CONFIG_KPROBES
> @@ -168,11 +170,18 @@ struct kretprobe {
>   raw_spinlock_t lock;
>  };
>  
> +#define KRETPROBE_TRACE_SIZE 127
> +struct kretprobe_trace {
> + int nr_entries;
> + unsigned long entries[KRETPROBE_TRACE_SIZE];
> +};
> +
>  struct kretprobe_instance {
>   struct hlist_node hlist;
>   struct kretprobe *rp;
>   kprobe_opcode_t *ret_addr;
>   struct task_struct *task;
> + struct kretprobe_trace entry;
>   char data[0];
>  };
>  
> @@ -371,6 +380,12 @@ void unregister_kretprobe(struct kretprobe *rp);
>  int register_kretprobes(struct kretprobe **rps, int num);
>  void unregister_kretprobes(struct kretprobe **rps, int num);
>  
> +struct kretprobe_instance *current_kretprobe_instance(void);
> +void kretprobe_save_stack_trace(struct kretprobe_instance *ri,
> + struct stack_trace *trace);
> +void kretprobe_perf_callchain_kernel(struct kretprobe_instance *ri,
> +  struct perf_callchain_entry_ctx *ctx);
> +
>  void kprobe_flush_task(struct task_struct *tk);
>  void recycle_rp_inst(struct kretprobe_instance *ri, struct hlist_head *head);
>  
> @@ -397,6 +412,18 @@ static inline struct kprobe *kprobe_running(void)
>  {
>   return NULL;
>  }
> +static inline struct kretprobe_instance *current_kretprobe_instance(void)
> +{
> + return NULL;
> +}
> +static inline void kretprobe_save_stack_trace(struct kretprobe_instance *ri,
> +   struct stack_trace *trace)
> +{
> +}
> +static inline void kretprobe_perf_callchain_kernel(struct kretprobe_instance 
> *ri,
> +struct 
> perf_callchain_entry_ctx *ctx)
> +{
> +}
>  static inline int register_kprobe(struct kprobe *p)
>  {
>   return -ENOSYS;
> diff --git a/kernel/events/callchain.c b/kernel/events/callchain.c
> index 24a77c34e9ad..98edcd8a6987 100644
> --- a/kernel/events/callchain.c
> +++ b/kernel/events/callchain.c
> @@ -12,6 +12,7 @@
>  #include

Re: [PATCH v3 1/2] kretprobe: produce sane stack traces

[Masami Hiramatsu]

On Fri, 2 Nov 2018 08:13:43 +1100
Aleksa Sarai  wrote:

> On 2018-11-02, Masami Hiramatsu  wrote:
> > Please split the test case as an independent patch.
> 
> Will do. Should the Documentation/ change also be a separate patch?

I think the Documentation change can be coupled with code change
if the change is small. But selftests is different, that can be
backported soon for testing the stable kernels.


> > > new file mode 100644
> > > index ..03146c6a1a3c
> > > --- /dev/null
> > > +++ b/tools/testing/selftests/ftrace/test.d/kprobe/kretprobe_stacktrace.tc
> > > @@ -0,0 +1,25 @@
> > > +#!/bin/sh
> > > +# SPDX-License-Identifier: GPL-2.0+
> > > +# description: Kretprobe dynamic event with a stacktrace
> > > +
> > > +[ -f kprobe_events ] || exit_unsupported # this is configurable
> > > +
> > > +echo 0 > events/enable
> > > +echo 1 > options/stacktrace
> > > +
> > > +echo 'r:teststackprobe sched_fork $retval' > kprobe_events
> > > +grep teststackprobe kprobe_events
> > > +test -d events/kprobes/teststackprobe
> > 
> > Hmm, what happen if we have 2 or more kretprobes on same stack?
> > It seems you just save stack in pre_handler, but that stack can already
> > includes another kretprobe's trampline address...
> 
> Yeah, you're quite right...
> 
> My first instinct was to do something awful like iterate over the set of
> "kretprobe_instance"s with ->task == current, and then correct
> kretprobe_trampoline entries using ->ret_addr. (I think this would be
> correct because each task can only be in one context at once, and in
> order to get to a particular kretprobe all of your caller kretprobes
> were inserted before you and any sibling or later kretprobe_instances
> will have been removed. But I might be insanely wrong on this front.)

yeah, you are right. 

> 
> However (as I noted in the other thread), there is a problem where
> kretprobe_trampoline actually stops the unwinder in its tracks and thus
> you only get the first kretprobe_trampoline. This is something I'm going
> to look into some more (despite not having made progress on it last
> time) since now it's something that actually needs to be fixed (and
> as I mentioned in the other thread, show_stack() actually works on x86
> in this context unlike the other stack_trace users).

I should read the unwinder code, but anyway, fixing it up in kretprobe
handler context is not hard. Since each instance is on an hlist, so when
we hit the kretprobe_trampoline, we can search it. However, the problem
is the case where the out of kretprobe handler context. In that context
we need to try to lock the hlist and search the list, which will be a
costful operation.

On the other hand, func-graph tracer introduces a shadow return address
stack for each task (thread), and when we hit its trampoline on the stack,
we can easily search the entry from "current" task without locking the
shadow stack (and we already did it). This may need to pay a cost (1 page)
for each task, but smarter than kretprobe, which makes a system-wide 
hash-table and need to search from hlist which has return addresses
of other context coexist.

Thank you,


-- 
Masami Hiramatsu 

Re: [PATCH v3 1/2] kretprobe: produce sane stack traces

[kbuild test robot]

Hi Aleksa,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on tip/perf/core]
[also build test ERROR on v4.19 next-20181101]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Aleksa-Sarai/kretprobe-produce-sane-stack-traces/20181102-034111
config: i386-randconfig-h1-11021006 (attached as .config)
compiler: gcc-4.9 (Debian 4.9.4-2) 4.9.4
reproduce:
# save the attached .config to linux build tree
make ARCH=i386 

All errors (new ones prefixed by >>):

   kernel/kprobes.c: In function 'pre_handler_kretprobe':
   kernel/kprobes.c:1846:10: error: variable 'trace' has initializer but 
incomplete type
  struct stack_trace trace = {};
 ^
   kernel/kprobes.c:1846:22: error: storage size of 'trace' isn't known
  struct stack_trace trace = {};
 ^
>> kernel/kprobes.c:1858:3: error: implicit declaration of function 
>> 'save_stack_trace_regs' [-Werror=implicit-function-declaration]
  save_stack_trace_regs(regs, &trace);
  ^
   kernel/kprobes.c:1846:22: warning: unused variable 'trace' 
[-Wunused-variable]
  struct stack_trace trace = {};
 ^
   kernel/kprobes.c: In function 'kretprobe_save_stack_trace':
>> kernel/kprobes.c:1922:16: error: dereferencing pointer to incomplete type
 for (i = trace->skip; i < krt->nr_entries; i++) {
   ^
   kernel/kprobes.c:1923:12: error: dereferencing pointer to incomplete type
  if (trace->nr_entries >= trace->max_entries)
   ^
   kernel/kprobes.c:1923:33: error: dereferencing pointer to incomplete type
  if (trace->nr_entries >= trace->max_entries)
^
   kernel/kprobes.c:1925:8: error: dereferencing pointer to incomplete type
  trace->entries[trace->nr_entries++] = krt->entries[i];
   ^
   kernel/kprobes.c:1925:23: error: dereferencing pointer to incomplete type
  trace->entries[trace->nr_entries++] = krt->entries[i];
  ^
   cc1: some warnings being treated as errors

vim +/save_stack_trace_regs +1858 kernel/kprobes.c

  1819  
  1820  #ifdef CONFIG_KRETPROBES
  1821  /*
  1822   * This kprobe pre_handler is registered with every kretprobe. When 
probe
  1823   * hits it will set up the return probe.
  1824   */
  1825  static int pre_handler_kretprobe(struct kprobe *p, struct pt_regs *regs)
  1826  {
  1827  struct kretprobe *rp = container_of(p, struct kretprobe, kp);
  1828  unsigned long hash, flags = 0;
  1829  struct kretprobe_instance *ri;
  1830  
  1831  /*
  1832   * To avoid deadlocks, prohibit return probing in NMI contexts,
  1833   * just skip the probe and increase the (inexact) 'nmissed'
  1834   * statistical counter, so that the user is informed that
  1835   * something happened:
  1836   */
  1837  if (unlikely(in_nmi())) {
  1838  rp->nmissed++;
  1839  return 0;
  1840  }
  1841  
  1842  /* TODO: consider to only swap the RA after the last 
pre_handler fired */
  1843  hash = hash_ptr(current, KPROBE_HASH_BITS);
  1844  raw_spin_lock_irqsave(&rp->lock, flags);
  1845  if (!hlist_empty(&rp->free_instances)) {
> 1846  struct stack_trace trace = {};
  1847  
  1848  ri = hlist_entry(rp->free_instances.first,
  1849  struct kretprobe_instance, hlist);
  1850  hlist_del(&ri->hlist);
  1851  raw_spin_unlock_irqrestore(&rp->lock, flags);
  1852  
  1853  ri->rp = rp;
  1854  ri->task = current;
  1855  
  1856  trace.entries = &ri->entry.entries[0];
  1857  trace.max_entries = KRETPROBE_TRACE_SIZE;
> 1858  save_stack_trace_regs(regs, &trace);
  1859  ri->entry.nr_entries = trace.nr_entries;
  1860  
  1861  if (rp->entry_handler && rp->entry_handler(ri, regs)) {
  1862  raw_spin_lock_irqsave(&rp->lock, flags);
  1863  hlist_add_head(&ri->hlist, &rp->free_instances);
  1864  raw_spin_unlock_irqrestore(&rp->lock, flags);
  1865  return 0;
  1866  }
  1867  
  1868  arch_prepare_kretprobe(ri, regs);
  1869  
  1870  /* XXX(hch): why is there no hlist_move_head? */
  1871  INIT_HLIST_NODE(&ri->hlist);
  1872  kretprobe_table_lock(hash, &flags);
  1873   

Re: [PATCH v3 1/2] kretprobe: produce sane stack traces

[Aleksa Sarai]

On 2018-11-02, Masami Hiramatsu  wrote:
> On Fri, 2 Nov 2018 08:13:43 +1100
> Aleksa Sarai  wrote:
> 
> > On 2018-11-02, Masami Hiramatsu  wrote:
> > > Please split the test case as an independent patch.
> > 
> > Will do. Should the Documentation/ change also be a separate patch?
> 
> I think the Documentation change can be coupled with code change
> if the change is small. But selftests is different, that can be
> backported soon for testing the stable kernels.
> 
> 
> > > > new file mode 100644
> > > > index ..03146c6a1a3c
> > > > --- /dev/null
> > > > +++ 
> > > > b/tools/testing/selftests/ftrace/test.d/kprobe/kretprobe_stacktrace.tc
> > > > @@ -0,0 +1,25 @@
> > > > +#!/bin/sh
> > > > +# SPDX-License-Identifier: GPL-2.0+
> > > > +# description: Kretprobe dynamic event with a stacktrace
> > > > +
> > > > +[ -f kprobe_events ] || exit_unsupported # this is configurable
> > > > +
> > > > +echo 0 > events/enable
> > > > +echo 1 > options/stacktrace
> > > > +
> > > > +echo 'r:teststackprobe sched_fork $retval' > kprobe_events
> > > > +grep teststackprobe kprobe_events
> > > > +test -d events/kprobes/teststackprobe
> > > 
> > > Hmm, what happen if we have 2 or more kretprobes on same stack?
> > > It seems you just save stack in pre_handler, but that stack can already
> > > includes another kretprobe's trampline address...
> > 
> > Yeah, you're quite right...
> > 
> > My first instinct was to do something awful like iterate over the set of
> > "kretprobe_instance"s with ->task == current, and then correct
> > kretprobe_trampoline entries using ->ret_addr. (I think this would be
> > correct because each task can only be in one context at once, and in
> > order to get to a particular kretprobe all of your caller kretprobes
> > were inserted before you and any sibling or later kretprobe_instances
> > will have been removed. But I might be insanely wrong on this front.)
> 
> yeah, you are right. 
> 
> > 
> > However (as I noted in the other thread), there is a problem where
> > kretprobe_trampoline actually stops the unwinder in its tracks and thus
> > you only get the first kretprobe_trampoline. This is something I'm going
> > to look into some more (despite not having made progress on it last
> > time) since now it's something that actually needs to be fixed (and
> > as I mentioned in the other thread, show_stack() actually works on x86
> > in this context unlike the other stack_trace users).
> 
> I should read the unwinder code, but anyway, fixing it up in kretprobe
> handler context is not hard. Since each instance is on an hlist, so when
> we hit the kretprobe_trampoline, we can search it.

As in, find the stored stack and merge the two? Interesting idea, though
Steven has shot this down because of the associated cost (I was
considering making it a kprobe flag, but that felt far too dirty).

> However, the problem is the case where the out of kretprobe handler
> context. In that context we need to try to lock the hlist and search
> the list, which will be a costful operation.

I think the best solution would be to unify all of the kretprobe-like
things so that we don't need to handle non-kretprobe contexts for
basically the same underlying idea. If we wanted to do it like this.

I think a potentially better option would be to just fix the unwinder to
correctly handle kretprobes (like it handles ftrace today).

> On the other hand, func-graph tracer introduces a shadow return address
> stack for each task (thread), and when we hit its trampoline on the stack,
> we can easily search the entry from "current" task without locking the
> shadow stack (and we already did it). This may need to pay a cost (1 page)
> for each task, but smarter than kretprobe, which makes a system-wide 
> hash-table and need to search from hlist which has return addresses
> of other context coexist.

Probably a silly question (I've been staring at the function_graph code
trying to understand how it handles return addresses -- and I'm really
struggling), is this what ->ret_stack (and ->curr_ret_stack) do?

Can you explain how the .retp handling works, because I'm really missing
how the core arch/ hooks know to pass the correct retp value.

-- 
Aleksa Sarai
Senior Software Engineer (Containers)
SUSE Linux GmbH



signature.asc
Description: PGP signature

[PATCH V6 7/8] misc/pvpanic: convert to SPDX license tags

[Peng Hao]

Updates license to use SPDX-License-Identifier instead of
verbose license text.

Reviewed-by: Andy Shevchenko 
Acked-by: Mark Rutland 
Signed-off-by: Peng Hao 
---
 drivers/misc/pvpanic.c | 15 +--
 1 file changed, 1 insertion(+), 14 deletions(-)

diff --git a/drivers/misc/pvpanic.c b/drivers/misc/pvpanic.c
index 9c9d9f8..1826cdf 100644
--- a/drivers/misc/pvpanic.c
+++ b/drivers/misc/pvpanic.c
@@ -1,22 +1,9 @@
+// SPDX-License-Identifier: GPL-2.0+
 /*
  *  pvpanic.c - pvpanic Device Support
  *
  *  Copyright (C) 2013 Fujitsu.
  *  Copyright (C) 2018 ZTE.
- *
- *  This program is free software; you can redistribute it and/or modify
- *  it under the terms of the GNU General Public License as published by
- *  the Free Software Foundation; either version 2 of the License, or
- *  (at your option) any later version.
- *
- *  This program is distributed in the hope that it will be useful,
- *  but WITHOUT ANY WARRANTY; without even the implied warranty of
- *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- *  GNU General Public License for more details.
- *
- *  You should have received a copy of the GNU General Public License
- *  along with this program; if not, write to the Free Software
- *  Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301  
USA
  */
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
-- 
1.8.3.1

[PATCH V6 1/8] pvpanic: move pvpanic to misc as common driver

[Peng Hao]

Move pvpanic.c from drivers/platform/x86 to drivers/misc.
Following patches will use pvpanic device as common driver.

Reviewed-by: Andy Shevchenko 
Acked-by: Mark Rutland 
Signed-off-by: Peng Hao 
---
 drivers/misc/Kconfig | 8 
 drivers/misc/Makefile| 1 +
 drivers/{platform/x86 => misc}/pvpanic.c | 0
 drivers/platform/x86/Kconfig | 8 
 drivers/platform/x86/Makefile| 1 -
 5 files changed, 9 insertions(+), 9 deletions(-)
 rename drivers/{platform/x86 => misc}/pvpanic.c (100%)

diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index 3726eac..642626a 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -513,6 +513,14 @@ config MISC_RTSX
tristate
default MISC_RTSX_PCI || MISC_RTSX_USB
 
+config PVPANIC
+   tristate "pvpanic device support"
+   depends on ACPI || OF
+   help
+ This driver provides support for the pvpanic device.  pvpanic is
+ a paravirtualized device provided by QEMU; it lets a virtual machine
+ (guest) communicate panic events to the host.
+
 source "drivers/misc/c2port/Kconfig"
 source "drivers/misc/eeprom/Kconfig"
 source "drivers/misc/cb710/Kconfig"
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index af22bbc..cd4b2f0 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -58,3 +58,4 @@ obj-$(CONFIG_ASPEED_LPC_SNOOP)+= aspeed-lpc-snoop.o
 obj-$(CONFIG_PCI_ENDPOINT_TEST)+= pci_endpoint_test.o
 obj-$(CONFIG_OCXL) += ocxl/
 obj-$(CONFIG_MISC_RTSX)+= cardreader/
+obj-$(CONFIG_PVPANIC)  += pvpanic.o
diff --git a/drivers/platform/x86/pvpanic.c b/drivers/misc/pvpanic.c
similarity index 100%
rename from drivers/platform/x86/pvpanic.c
rename to drivers/misc/pvpanic.c
diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 0c1aa6c..4ac276c 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -1121,14 +1121,6 @@ config INTEL_SMARTCONNECT
  This driver checks to determine whether the device has Intel Smart
  Connect enabled, and if so disables it.
 
-config PVPANIC
-   tristate "pvpanic device support"
-   depends on ACPI
-   ---help---
- This driver provides support for the pvpanic device.  pvpanic is
- a paravirtualized device provided by QEMU; it lets a virtual machine
- (guest) communicate panic events to the host.
-
 config INTEL_PMC_IPC
tristate "Intel PMC IPC Driver"
depends on ACPI
diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
index e6d1bec..e88f44e 100644
--- a/drivers/platform/x86/Makefile
+++ b/drivers/platform/x86/Makefile
@@ -75,7 +75,6 @@ obj-$(CONFIG_APPLE_GMUX)  += apple-gmux.o
 obj-$(CONFIG_INTEL_RST)+= intel-rst.o
 obj-$(CONFIG_INTEL_SMARTCONNECT)   += intel-smartconnect.o
 
-obj-$(CONFIG_PVPANIC)   += pvpanic.o
 obj-$(CONFIG_ALIENWARE_WMI)+= alienware-wmi.o
 obj-$(CONFIG_INTEL_PMC_IPC)+= intel_pmc_ipc.o
 obj-$(CONFIG_TOUCHSCREEN_DMI)  += touchscreen_dmi.o
-- 
1.8.3.1

[PATCH V6 3/8] misc/pvpanic: add MMIO support

[Peng Hao]

On some architectures (e.g. arm64), it's preferable to use MMIO, since
this can be used standalone. Add MMIO support to the pvpanic driver.

Suggested-by: Andy Shevchenko 
Acked-by: Mark Rutland 
Signed-off-by: Peng Hao 
---
 drivers/misc/pvpanic.c | 14 ++
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/misc/pvpanic.c b/drivers/misc/pvpanic.c
index 49c59e1..22fb487 100644
--- a/drivers/misc/pvpanic.c
+++ b/drivers/misc/pvpanic.c
@@ -41,7 +41,7 @@
 
 #define PVPANIC_PANICKED   (1 << 0)
 
-static u16 port;
+static void __iomem *base;
 
 static struct acpi_driver pvpanic_driver = {
.name = "pvpanic",
@@ -57,7 +57,7 @@
 static void
 pvpanic_send_event(unsigned int event)
 {
-   outb(event, port);
+   iowrite8(event, base);
 }
 
 static int
@@ -80,7 +80,11 @@
struct resource r;
 
if (acpi_dev_resource_io(res, &r)) {
-   port = r.start;
+   base = (void __iomem *) ioport_map(r.start,
+   r.end - r.start + 1);
+   return AE_OK;
+   } else if (acpi_dev_resource_memory(res, &r)) {
+   base = ioremap(r.start, r.end - r.start + 1);
return AE_OK;
}
 
@@ -101,7 +105,7 @@ static int pvpanic_add(struct acpi_device *device)
acpi_walk_resources(device->handle, METHOD_NAME__CRS,
pvpanic_walk_resources, NULL);
 
-   if (!port)
+   if (!base)
return -ENODEV;
 
atomic_notifier_chain_register(&panic_notifier_list,
@@ -115,6 +119,8 @@ static int pvpanic_remove(struct acpi_device *device)
 
atomic_notifier_chain_unregister(&panic_notifier_list,
 &pvpanic_panic_nb);
+   iounmap(base);
+
return 0;
 }
 
-- 
1.8.3.1

[PATCH V6 5/8] misc/pvpanic: add support to get pvpanic device info by FDT

[Peng Hao]

By default, when ACPI tables and FDT coexist for ARM64,
current kernel takes precedence over FDT to get device information.
Virt machine in qemu provides both FDT and ACPI table. Increases the
way to get information through FDT.

Acked-by: Mark Rutland 
Signed-off-by: Peng Hao 
---
 drivers/misc/pvpanic.c | 67 +++---
 1 file changed, 64 insertions(+), 3 deletions(-)

diff --git a/drivers/misc/pvpanic.c b/drivers/misc/pvpanic.c
index 22fb487..5378d5f 100644
--- a/drivers/misc/pvpanic.c
+++ b/drivers/misc/pvpanic.c
@@ -2,6 +2,7 @@
  *  pvpanic.c - pvpanic Device Support
  *
  *  Copyright (C) 2013 Fujitsu.
+ *  Copyright (C) 2018 ZTE.
  *
  *  This program is free software; you can redistribute it and/or modify
  *  it under the terms of the GNU General Public License as published by
@@ -20,11 +21,14 @@
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
+#include 
+#include 
 #include 
 #include 
-#include 
+#include 
+#include 
+#include 
 #include 
-#include 
 
 MODULE_AUTHOR("Hu Tao ");
 MODULE_DESCRIPTION("pvpanic device driver");
@@ -73,6 +77,32 @@
.priority = 1, /* let this called before broken drm_fb_helper */
 };
 
+static int pvpanic_mmio_probe(struct platform_device *pdev)
+{
+   struct resource *mem;
+
+   mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+   if (!mem)
+   return -EINVAL;
+
+   base = devm_ioremap_resource(&pdev->dev, mem);
+   if (base == NULL)
+   return -EFAULT;
+
+   atomic_notifier_chain_register(&panic_notifier_list,
+  &pvpanic_panic_nb);
+
+   return 0;
+}
+
+static int pvpanic_mmio_remove(struct platform_device *pdev)
+{
+
+   atomic_notifier_chain_unregister(&panic_notifier_list,
+&pvpanic_panic_nb);
+
+   return 0;
+}
 
 static acpi_status
 pvpanic_walk_resources(struct acpi_resource *res, void *context)
@@ -124,4 +154,35 @@ static int pvpanic_remove(struct acpi_device *device)
return 0;
 }
 
-module_acpi_driver(pvpanic_driver);
+static const struct of_device_id pvpanic_mmio_match[] = {
+   { .compatible = "qemu,pvpanic-mmio", },
+   {}
+};
+
+static struct platform_driver pvpanic_mmio_driver = {
+   .driver = {
+   .name = "pvpanic-mmio",
+   .of_match_table = pvpanic_mmio_match,
+   },
+   .probe = pvpanic_mmio_probe,
+   .remove = pvpanic_mmio_remove,
+};
+
+static int __init pvpanic_mmio_init(void)
+{
+   if (acpi_disabled)
+   return platform_driver_register(&pvpanic_mmio_driver);
+   else
+   return acpi_bus_register_driver(&pvpanic_driver);
+}
+
+static void __exit pvpanic_mmio_exit(void)
+{
+   if (acpi_disabled)
+   platform_driver_unregister(&pvpanic_mmio_driver);
+   else
+   acpi_bus_unregister_driver(&pvpanic_driver);
+}
+
+module_init(pvpanic_mmio_init);
+module_exit(pvpanic_mmio_exit);
-- 
1.8.3.1

[PATCH V6 2/8] misc/pvpanic: simplify the code using acpi_dev_resource_io

[Peng Hao]

Use acpi_dev_resource_io API.

Suggested-by: Andy Shevchenko 
Reviewed-by: Andy Shevchenko 
Acked-by: Mark Rutland 
Signed-off-by: Peng Hao 
---
 drivers/misc/pvpanic.c | 13 +
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/drivers/misc/pvpanic.c b/drivers/misc/pvpanic.c
index fd86dab..49c59e1 100644
--- a/drivers/misc/pvpanic.c
+++ b/drivers/misc/pvpanic.c
@@ -77,17 +77,14 @@
 static acpi_status
 pvpanic_walk_resources(struct acpi_resource *res, void *context)
 {
-   switch (res->type) {
-   case ACPI_RESOURCE_TYPE_END_TAG:
-   return AE_OK;
+   struct resource r;
 
-   case ACPI_RESOURCE_TYPE_IO:
-   port = res->data.io.minimum;
+   if (acpi_dev_resource_io(res, &r)) {
+   port = r.start;
return AE_OK;
-
-   default:
-   return AE_ERROR;
}
+
+   return AE_ERROR;
 }
 
 static int pvpanic_add(struct acpi_device *device)
-- 
1.8.3.1

[PATCH V6 8/8] misc/pvpanic: remove a redundant comma

[Peng Hao]

Remove a redundant comma in pvpanic_device_ids.

Acked-by: Mark Rutland 
Signed-off-by: Peng Hao 
---
 drivers/misc/pvpanic.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/misc/pvpanic.c b/drivers/misc/pvpanic.c
index 1826cdf..07149db 100644
--- a/drivers/misc/pvpanic.c
+++ b/drivers/misc/pvpanic.c
@@ -25,7 +25,7 @@
 
 static const struct acpi_device_id pvpanic_device_ids[] = {
{ "QEMU0001", 0 },
-   { "", 0 },
+   { "", 0 }
 };
 MODULE_DEVICE_TABLE(acpi, pvpanic_device_ids);
 
-- 
1.8.3.1

[PATCH V6 4/8] dt-bindings: misc/pvpanic: add document for pvpanic-mmio

[Peng Hao]

Add dt-bindings document for "qemu:pvpanic-mmio".

Acked-by: Mark Rutland 
Signed-off-by: Peng Hao 
---
 .../devicetree/bindings/misc/pvpanic-mmio.txt  | 29 ++
 1 file changed, 29 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/misc/pvpanic-mmio.txt

diff --git a/Documentation/devicetree/bindings/misc/pvpanic-mmio.txt 
b/Documentation/devicetree/bindings/misc/pvpanic-mmio.txt
new file mode 100644
index 000..985e907
--- /dev/null
+++ b/Documentation/devicetree/bindings/misc/pvpanic-mmio.txt
@@ -0,0 +1,29 @@
+* QEMU PVPANIC MMIO Configuration bindings
+
+QEMU's emulation / virtualization targets provide the following PVPANIC
+MMIO Configuration interface on the "virt" machine.
+type:
+
+- a read-write, 16-bit wide data register.
+
+QEMU exposes the data register to guests as memory mapped registers.
+
+Required properties:
+
+- compatible: "qemu,pvpanic-mmio".
+- reg: the MMIO region used by the device.
+  * Bytes 0x0  Write panic event to the reg when guest OS panics.
+  * Bytes 0x1  Reserved.
+
+Example:
+
+/ {
+#size-cells = <0x2>;
+#address-cells = <0x2>;
+
+pvpanic-mmio@906 {
+compatible = "qemu,pvpanic-mmio";
+reg = <0x0 0x906 0x0 0x2>;
+};
+};
+
-- 
1.8.3.1

[PATCH V6 6/8] misc/pvpanic: remove unnecessary header file

[Peng Hao]

Remove unnecessary header file init.h.

Reviewed-by: Andy Shevchenko 
Acked-by: Mark Rutland 
Signed-off-by: Peng Hao 
---
 drivers/misc/pvpanic.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/misc/pvpanic.c b/drivers/misc/pvpanic.c
index 5378d5f..9c9d9f8 100644
--- a/drivers/misc/pvpanic.c
+++ b/drivers/misc/pvpanic.c
@@ -22,7 +22,6 @@
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
 #include 
-#include 
 #include 
 #include 
 #include 
-- 
1.8.3.1

Re: [PATCH v3 1/2] kretprobe: produce sane stack traces

[Aleksa Sarai]

On 2018-11-01, Steven Rostedt  wrote:
> On Thu,  1 Nov 2018 19:35:50 +1100
> Aleksa Sarai  wrote:
> > @@ -1834,6 +1853,11 @@ static int pre_handler_kretprobe(struct kprobe *p, 
> > struct pt_regs *regs)
> > ri->rp = rp;
> > ri->task = current;
> >  
> > +   trace.entries = &ri->entry.entries[0];
> > +   trace.max_entries = KRETPROBE_TRACE_SIZE;
> > +   save_stack_trace_regs(regs, &trace);
> > +   ri->entry.nr_entries = trace.nr_entries;
> > +
> 
> So basically your saving a copy of the stack trace for each probe, for
> something that may not ever be used?
> 
> This adds quite a lot of overhead for something not used often.
> 
> I think the real answer is to fix kretprobes (and I just checked, the
> return call of function graph tracer stack trace doesn't go past the
> return_to_handler function either. It's just that a stack trace was
> never done on the return side).
> 
> The more I look at this patch, the less I like it.

That's more than fair.

There's also an issue that Masami noted -- nested kretprobes don't give
you the full stack trace with this solution since the stack was already
overwritten. I think that the only real option is to fix the unwinder to
work in a kretprobe context -- which is what I'm looking at now.

Unfortunately, I'm having a lot of trouble understanding how the current
ftrace hooking works -- ORC has a couple of ftrace hooks that seem
reasonable on the surface but I don't understand (for instance) how
HAVE_FUNCTION_GRAPH_RET_ADDR_PTR *actually* works. Though your comment
appears to indicate that it doesn't work for stack traces?

For kretprobes I think it would be fairly easy to reconstruct what
landed you into a kretprobe_trampoline by walking the set of
kretprobe_instances (since all new ones are added to the head, you can
get the real return address in-order).

But I still have to figure out what is actually stopping the
save_stack_trace() unwinder that isn't stopping the show_stacks()
unwinder (though the show_stacks() code is more ... liberal with the
degree of certainty it has about the unwind).

Am I barking up the wrong tree?

-- 
Aleksa Sarai
Senior Software Engineer (Containers)
SUSE Linux GmbH



signature.asc
Description: PGP signature

Re: [PATCH V6 1/8] pvpanic: move pvpanic to misc as common driver

[kbuild test robot]

Hi Peng,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on char-misc/char-misc-testing]
[also build test ERROR on v4.19 next-20181102]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Peng-Hao/pvpanic-move-pvpanic-to-misc-as-common-driver/20181102-124907
config: openrisc-allyesconfig (attached as .config)
compiler: or1k-linux-gcc (GCC) 6.0.0 20160327 (experimental)
reproduce:
wget 
https://raw.githubusercontent.com/intel/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=openrisc 

All errors (new ones prefixed by >>):

   drivers/misc/pvpanic.c:36:36: error: array type has incomplete element type 
'struct acpi_device_id'
static const struct acpi_device_id pvpanic_device_ids[] = {
   ^~
   drivers/misc/pvpanic.c:46:15: error: variable 'pvpanic_driver' has 
initializer but incomplete type
static struct acpi_driver pvpanic_driver = {
  ^~~
>> drivers/misc/pvpanic.c:47:2: error: unknown field 'name' specified in 
>> initializer
 .name =  "pvpanic",
 ^
   drivers/misc/pvpanic.c:47:11: warning: excess elements in struct initializer
 .name =  "pvpanic",
  ^
   drivers/misc/pvpanic.c:47:11: note: (near initialization for 
'pvpanic_driver')
>> drivers/misc/pvpanic.c:48:2: error: unknown field 'class' specified in 
>> initializer
 .class = "QEMU",
 ^
   drivers/misc/pvpanic.c:48:11: warning: excess elements in struct initializer
 .class = "QEMU",
  ^~
   drivers/misc/pvpanic.c:48:11: note: (near initialization for 
'pvpanic_driver')
>> drivers/misc/pvpanic.c:49:2: error: unknown field 'ids' specified in 
>> initializer
 .ids =  pvpanic_device_ids,
 ^
   drivers/misc/pvpanic.c:49:10: warning: excess elements in struct initializer
 .ids =  pvpanic_device_ids,
 ^~
   drivers/misc/pvpanic.c:49:10: note: (near initialization for 
'pvpanic_driver')
>> drivers/misc/pvpanic.c:50:2: error: unknown field 'ops' specified in 
>> initializer
 .ops =  {
 ^
   drivers/misc/pvpanic.c:50:10: error: extra brace group at end of initializer
 .ops =  {
 ^
   drivers/misc/pvpanic.c:50:10: note: (near initialization for 
'pvpanic_driver')
   drivers/misc/pvpanic.c:50:10: warning: excess elements in struct initializer
   drivers/misc/pvpanic.c:50:10: note: (near initialization for 
'pvpanic_driver')
>> drivers/misc/pvpanic.c:54:2: error: unknown field 'owner' specified in 
>> initializer
 .owner = THIS_MODULE,
 ^
   In file included from include/linux/linkage.h:7:0,
from include/linux/kernel.h:7,
from drivers/misc/pvpanic.c:23:
   include/linux/export.h:18:21: warning: excess elements in struct initializer
#define THIS_MODULE ((struct module *)0)
^
   drivers/misc/pvpanic.c:54:11: note: in expansion of macro 'THIS_MODULE'
 .owner = THIS_MODULE,
  ^~~
   include/linux/export.h:18:21: note: (near initialization for 
'pvpanic_driver')
#define THIS_MODULE ((struct module *)0)
^
   drivers/misc/pvpanic.c:54:11: note: in expansion of macro 'THIS_MODULE'
 .owner = THIS_MODULE,
  ^~~
   drivers/misc/pvpanic.c: In function 'pvpanic_send_event':
   drivers/misc/pvpanic.c:60:2: error: implicit declaration of function 'outb' 
[-Werror=implicit-function-declaration]
 outb(event, port);
 ^~~~
   drivers/misc/pvpanic.c: In function 'pvpanic_add':
>> drivers/misc/pvpanic.c:97:8: error: implicit declaration of function 
>> 'acpi_bus_get_status' [-Werror=implicit-function-declaration]
 ret = acpi_bus_get_status(device);
   ^~~
   drivers/misc/pvpanic.c:101:13: error: dereferencing pointer to incomplete 
type 'struct acpi_device'
 if (!device->status.enabled || !device->status.functional)
^~
   drivers/misc/pvpanic.c: At top level:
   drivers/misc/pvpanic.c:124:1: warning: data definition has no type or 
storage class
module_acpi_driver(pvpanic_driver);
^~
   drivers/misc/pvpanic.c:124:1: error: type defaults to 'int' in declaration 
of 'module_acpi_driver' [-Werror=implicit-int]
   drivers/misc/pvpanic.c:124:1: warning: parameter names (without types) in 
function declaration
   drivers/misc/pvpanic.c:46:27: error: storage size of 'pvpanic_driver' isn't 
known
static struct acpi_driver pvpanic_driver = {
  ^~
   drivers/misc/pvpanic.c:46:27: warning: 'pvpanic_driver' defined but not used 
[-Wunused-variable]
   drivers/misc/pvpanic.c:36:36: warning: 'pvpanic_device_ids' defined but not 
used [-Wunused-variable]
static const st