[PATCH v10 0/2] dm: boot a mapped device without an initramfs

[Helen Koike]

As mentioned in the discussion from the previous version of this patch, Android
and Chrome OS do not use initramfs mostly due to boot time and size liability.
A practical example as mentioned by Kees is that Chrome OS has a limited amount
of storage available for the boot image as it is covered by the static root of
trust signature.

So instead of bringing up userspace to perform the required configuration for
mapped devices, this patchset allows them to be configured in the kernel command
line parameter for use early in the boot process, allowing booting from a mapped
device without an initramfs.

The syntax used in the boot param is based on the concise format from the 
dmsetup
tool as described in its man page 
http://man7.org/linux/man-pages/man8/dmsetup.8.html#CONCISE_FORMAT

Which is:

dm=[,+][;[,+]+]

Where,
  ::= The device name.
  ::= ---- | ""
 ::= The device minor number | ""
 ::= "ro" | "rw"
 ::=

   ::= "verity" | "linear" | ...

Example, the following could be added in the boot parameters.
dm="lroot,,,rw, 0 4096 linear 98:16 0, 4096 4096 linear 98:32 0" root=/dev/dm-0

Please check patch 2/2 with the documentation on the format.

The idea to make it compatible with the dmsetup concise format is to make it
easier for users, allowing just copy & paste from the output of the command:

sudo dmsetup table --concise /dev/mapper/lroot

The implementation consists basically in parsing the command line argument and
performing the ioctls that would be performed by userspace otherwise,
i.e. DM_DEV_CREATE, followed by DM_TABLE_LOAD then DM_DEV_SUSPEND.

Instead of performing the ioctls, we could by-pass it, calling the corresponding
functions directly, but the ioctls calls perform some checks and also the
implementation stays less invasive. Please let me know if you would prefer a
directly call instead of going thought the ioctls.

Changes since v9:
 - https://www.redhat.com/archives/linux-lvm/2018-September/msg00016.html
 - new file: drivers/md/dm-boot.c
 - most of the parsing code was moved from init/do_mounts_dm.c to 
drivers/md/dm-boot.c
 - parsing code was in essence replaced by the concise parser from dmsetup
 _create_concise function:
https://sourceware.org/git/?p=lvm2.git;a=blob;f=libdm/dm-tools/dmsetup.c;h=835fdcdc75e8f0f0f7c4ed46cc9788a6616f58b8;hb=7498f8383397a93db95655ca227257836cbcac82#l1265
 the main reason is that this code is already being used/tested by dmsetup, so
 we can have some level of confidence that it works as expected. Besides this,
 it also looks more efficient.
 - Not all targets are allowed to be used by dm=, as pointed previously, there
 are some risks in creating a mapped device without some validation from
 userspace (see documentation from the patch listing which targets are allowed).
 - Instead of using a simple singly linked list (for devices and tables), use
 the struct list_head. This occupies unnecessary space in the code, but it makes
 the code cleaner and easier to read and less prone to silly errors.
 - Documentation and comments were reviewed and refactored, e.g.:
* "is to possible" was removed
* s/specified as a simple string/specified as a string/
 - Added docs above __align function, make it clear that the second parameter @a
 must be a power of two.
 - Clean ups: removal of unnecessary includes, macros, variables, some redundant
 checks and warnings.
 - when calling ioctls, the code was allocating and freeing the same structure
 a couple of times. So instead of executing kzalloc/kfree 3 times, execute
 kmalloc once and reuse the structure after a memset, then finally kfree it 
once.
 - update commit message

Changes since v8:
 - https://www.redhat.com/archives/linux-lvm/2017-May/msg00055.html
 - Add minor number to make it compatible with dmsetup concise format

Changes since v7:
 - http://lkml.iu.edu/hypermail/linux/kernel/1705.2/02657.html
 - Fix build error due commit
e516db4f67 (dm ioctl: add a new DM_DEV_ARM_POLL ioctl)

Changes since v6:
 - https://www.redhat.com/archives/dm-devel/2017-April/msg00316.html
 - Add a new function to issue the equivalent of a DM ioctl programatically.
 - Use the new ioctl interface to create the devices.
 - Use a comma-delimited and semi-colon delimited dmsetup-like commands.

Changes since v5:
 - https://www.redhat.com/archives/dm-devel/2016-February/msg00112.html

Enric Balletbo i Serra (1):
  dm ioctl: add a device mapper ioctl function.

Will Drewry (1):
  init: add support to directly boot to a mapped device

 .../admin-guide/kernel-parameters.rst |   1 +
 .../admin-guide/kernel-parameters.txt |   3 +
 Documentation/device-mapper/dm-boot.txt   |  87 
 drivers/md/Makefile   |   2 +-
 drivers/md/dm-boot.c  | 433 ++
 drivers/md/dm-ioctl.c |  49 +

[PATCH v10 2/2] init: add support to directly boot to a mapped device

[Helen Koike]

From: Will Drewry 

Add a dm= kernel parameter.
It allows device-mapper targets to be configured at boot time for use early
in the boot process (as the root device or otherwise).

Signed-off-by: Will Drewry 
Signed-off-by: Kees Cook 
[rework to use dm_ioctl calls]
Signed-off-by: Enric Balletbo i Serra 
[rework to use concise format | rework for upstream]
Signed-off-by: Helen Koike 

---

Hello,

In this patch, I constrained the targets allowed to be used by dm=, but
I am not entirely familiar with all the targets. I blacklisted the ones
mentioned previously and some other that I think doesn't make much sense, but
please let me know if you think there are others I should blacklist.

Changes since v9:
 - https://www.redhat.com/archives/linux-lvm/2018-September/msg00016.html
 - new file: drivers/md/dm-boot.c
 - most of the parsing code was moved from init/do_mounts_dm.c to 
drivers/md/dm-boot.c
 - parsing code was in essence replaced by the concise parser from dmsetup
 _create_concise function:
https://sourceware.org/git/?p=lvm2.git;a=blob;f=libdm/dm-tools/dmsetup.c;h=835fdcdc75e8f0f0f7c4ed46cc9788a6616f58b8;hb=7498f8383397a93db95655ca227257836cbcac82#l1265
 the main reason is that this code is already being used/tested by dmsetup, so
 we can have some level of confidence that it works as expected. Besides this,
 it also looks more efficient.
 - Not all targets are allowed to be used by dm=, as pointed previously, there
 are some risks in creating a mapped device without some validation from
 userspace (see documentation from the patch listing which targets are allowed).
 - Instead of using a simple singly linked list (for devices and tables), use
 the struct list_head. This occupies unnecessary space in the code, but it makes
 the code cleaner and easier to read and less prone to silly errors.
 - Documentation and comments were reviewed and refactored, e.g.:
* "is to possible" was removed
* s/specified as a simple string/specified as a string/
 - Added docs above __align function, make it clear that the second parameter @a
 must be a power of two.
 - Clean ups: removal of unnecessary includes, macros, variables, some redundant
 checks and warnings.
 - when calling ioctls, the code was allocating and freeing the same structure
 a couple of times. So instead of executing kzalloc/kfree 3 times, execute
 kmalloc once and reuse the structure after a memset, then finally kfree it 
once.
 - update commit message

Thanks
---
 .../admin-guide/kernel-parameters.rst |   1 +
 .../admin-guide/kernel-parameters.txt |   3 +
 Documentation/device-mapper/dm-boot.txt   |  87 
 drivers/md/Makefile   |   2 +-
 drivers/md/dm-boot.c  | 433 ++
 include/linux/device-mapper.h |   6 +
 init/Makefile |   1 +
 init/do_mounts.c  |   1 +
 init/do_mounts.h  |  10 +
 init/do_mounts_dm.c   |  46 ++
 10 files changed, 589 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/device-mapper/dm-boot.txt
 create mode 100644 drivers/md/dm-boot.c
 create mode 100644 init/do_mounts_dm.c

diff --git a/Documentation/admin-guide/kernel-parameters.rst 
b/Documentation/admin-guide/kernel-parameters.rst
index b8d0bc07ed0a..bd628865f66f 100644
--- a/Documentation/admin-guide/kernel-parameters.rst
+++ b/Documentation/admin-guide/kernel-parameters.rst
@@ -91,6 +91,7 @@ parameter is applicable::
AX25Appropriate AX.25 support is enabled.
CLK Common clock infrastructure is enabled.
CMA Contiguous Memory Area support is enabled.
+   DM  Device mapper support is enabled.
DRM Direct Rendering Management support is enabled.
DYNAMIC_DEBUG Build in debug messages and enable them at runtime
EDD BIOS Enhanced Disk Drive Services (EDD) is enabled
diff --git a/Documentation/admin-guide/kernel-parameters.txt 
b/Documentation/admin-guide/kernel-parameters.txt
index 92eb1f42240d..a3ff7192b980 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -880,6 +880,9 @@
 
dis_ucode_ldr   [X86] Disable the microcode loader.
 
+   dm= [DM] Allows early creation of a device-mapper device.
+   See Documentation/device-mapper/dm-boot.txt.
+
dma_debug=off   If the kernel is compiled with DMA_API_DEBUG support,
this option disables the debugging code at boot.
 
diff --git a/Documentation/device-mapper/dm-boot.txt 
b/Documentation/device-mapper/dm-boot.txt
new file mode 100644
index ..14f756b34328
--- /dev/null
+++ b/Documentation/device-mapper/dm-boot.txt
@@ -0,0 +1,87 @@
+Boot time creation of mapped devices
+
+
+It is possible to configure a device-mapper device to act as the root device 
f

[PATCH v10 1/2] dm ioctl: add a device mapper ioctl function.

[Helen Koike]

From: Enric Balletbo i Serra 

Add a dm_ioctl_cmd to issue the equivalent of a DM ioctl call in kernel.

Signed-off-by: Enric Balletbo i Serra 

---

Changes since v9:
 - https://www.redhat.com/archives/linux-lvm/2018-September/msg00016.html
 - Reorganize variables
---
 drivers/md/dm-ioctl.c | 49 +++
 include/linux/device-mapper.h |  6 +
 2 files changed, 55 insertions(+)

diff --git a/drivers/md/dm-ioctl.c b/drivers/md/dm-ioctl.c
index f666778ad237..ae34c2030a9c 100644
--- a/drivers/md/dm-ioctl.c
+++ b/drivers/md/dm-ioctl.c
@@ -2018,3 +2018,52 @@ int dm_copy_name_and_uuid(struct mapped_device *md, char 
*name, char *uuid)
 
return r;
 }
+
+/**
+ * dm_ioctl_cmd - Device mapper ioctl's
+ * @command: ioctl command
+ * @param: Pointer to device mapped ioctl struct
+ */
+int dm_ioctl_cmd(uint command, struct dm_ioctl *param)
+{
+   struct file *filp = NULL;
+   size_t input_param_size;
+   int ioctl_flags, r;
+   unsigned int cmd;
+   ioctl_fn fn;
+
+   if (_IOC_TYPE(command) != DM_IOCTL)
+   return -ENOTTY;
+
+   /* DM_DEV_ARM_POLL is not supported */
+   if (command == DM_DEV_ARM_POLL)
+   return -EINVAL;
+
+   cmd = _IOC_NR(command);
+
+   /*
+* Nothing more to do for the version command.
+*/
+   if (cmd == DM_VERSION_CMD)
+   return 0;
+
+   fn = lookup_ioctl(cmd, &ioctl_flags);
+   if (!fn) {
+   DMWARN("dm_ioctl: unknown command 0x%x", command);
+   return -ENOTTY;
+   }
+
+   input_param_size = param->data_size;
+   r = validate_params(cmd, param);
+   if (r)
+   return r;
+
+   param->data_size = sizeof(*param);
+   r = fn(filp, param, input_param_size);
+
+   if (unlikely(param->flags & DM_BUFFER_FULL_FLAG) &&
+   unlikely(ioctl_flags & IOCTL_FLAGS_NO_PARAMS))
+   DMERR("ioctl %d tried to output some data but has 
IOCTL_FLAGS_NO_PARAMS set", cmd);
+
+   return r;
+}
diff --git a/include/linux/device-mapper.h b/include/linux/device-mapper.h
index d7bee8669f10..8b2e4ae6a498 100644
--- a/include/linux/device-mapper.h
+++ b/include/linux/device-mapper.h
@@ -12,6 +12,7 @@
 #include 
 #include 
 #include 
+#include 
 
 struct dm_dev;
 struct dm_target;
@@ -423,6 +424,11 @@ void dm_remap_zone_report(struct dm_target *ti, struct bio 
*bio,
  sector_t start);
 union map_info *dm_get_rq_mapinfo(struct request *rq);
 
+/*
+ * Device mapper ioctl function.
+ */
+int dm_ioctl_cmd(unsigned int command, struct dm_ioctl *param);
+
 struct queue_limits *dm_get_queue_limits(struct mapped_device *md);
 
 /*
-- 
2.19.1

[PATCH v15 23/23] x86/sgx: Driver documentation

[Jarkko Sakkinen]

Documentation of the features of the Software Guard eXtensions used
by the Linux kernel and basic design choices for the core and driver
and functionality.

Signed-off-by: Jarkko Sakkinen 
---
 Documentation/index.rst |   1 +
 Documentation/x86/intel_sgx.rst | 185 
 2 files changed, 186 insertions(+)
 create mode 100644 Documentation/x86/intel_sgx.rst

diff --git a/Documentation/index.rst b/Documentation/index.rst
index 5db7e87c7cb1..1cdc139adb40 100644
--- a/Documentation/index.rst
+++ b/Documentation/index.rst
@@ -104,6 +104,7 @@ implementation.
:maxdepth: 2
 
sh/index
+   x86/index
 
 Filesystem Documentation
 
diff --git a/Documentation/x86/intel_sgx.rst b/Documentation/x86/intel_sgx.rst
new file mode 100644
index ..f6b7979c41f2
--- /dev/null
+++ b/Documentation/x86/intel_sgx.rst
@@ -0,0 +1,185 @@
+===
+Intel(R) SGX driver
+===
+
+Introduction
+
+
+Intel(R) SGX is a set of CPU instructions that can be used by applications to
+set aside private regions of code and data. The code outside the enclave is
+disallowed to access the memory inside the enclave by the CPU access control.
+In a way you can think that SGX provides inverted sandbox. It protects the
+application from a malicious host.
+
+You can tell if your CPU supports SGX by looking into ``/proc/cpuinfo``:
+
+   ``cat /proc/cpuinfo  | grep sgx``
+
+Overview of SGX
+===
+
+SGX has a set of data structures to maintain information about the enclaves and
+their security properties. BIOS reserves a fixed size region of physical memory
+for these structures by setting Processor Reserved Memory Range Registers
+(PRMRR).
+
+This memory range is protected from outside access by the CPU and all the data
+coming in and out of the CPU package is encrypted by a key that is generated 
for
+each boot cycle.
+
+Enclaves execute in ring-3 in a special enclave submode using pages from the
+reserved memory range. A fixed logical address range for the enclave is 
reserved
+by ENCLS(ECREATE), a leaf instruction used to create enclaves. It is referred 
in
+the documentation commonly as the ELRANGE.
+
+Every memory access to the ELRANGE is asserted by the CPU. If the CPU is not
+executing in the enclave mode inside the enclave, #GP is raised. On the other
+hand enclave code can make memory accesses both inside and outside of the
+ELRANGE.
+
+Enclave can only execute code inside the ELRANGE. Instructions that may cause
+VMEXIT, IO instructions and instructions that require a privilege change are
+prohibited inside the enclave. Interrupts and exceptions always cause enclave
+to exit and jump to an address outside the enclave given when the enclave is
+entered by using the leaf instruction ENCLS(EENTER).
+
+Data types
+--
+
+The protected memory range contains the following data:
+
+* **Enclave Page Cache (EPC):** protected pages
+* **Enclave Page Cache Map (EPCM):** a database that describes the state of the
+  pages and link them to an enclave.
+
+EPC has a number of different types of pages:
+
+* **SGX Enclave Control Structure (SECS)**: describes the global
+  properties of an enclave.
+* **Regular (REG):** code and data pages in the ELRANGE.
+* **Thread Control Structure (TCS):** pages that define entry points inside an
+  enclave. The enclave can only be entered through these entry points and each
+  can host a single hardware thread at a time.
+* **Version Array (VA)**: 64-bit version numbers for pages that have been
+  swapped outside the enclave. Each page contains 512 version numbers.
+
+Launch control
+--
+
+To launch an enclave, two structures must be provided for ENCLS(EINIT):
+
+1. **SIGSTRUCT:** signed measurement of the enclave binary.
+2. **EINITTOKEN:** a cryptographic token CMAC-signed with a AES256-key called
+   *launch key*, which is re-generated for each boot cycle.
+
+The CPU holds a SHA256 hash of a 3072-bit RSA public key inside
+IA32_SGXLEPUBKEYHASHn MSRs. Enclaves with a SIGSTRUCT that is signed with this
+key do not require a valid EINITTOKEN and can be authorized with special
+privileges. One of those privileges is ability to acquire the launch key with
+ENCLS(EGETKEY).
+
+**IA32_FEATURE_CONTROL[17]** is used by the BIOS configure whether
+IA32_SGXLEPUBKEYHASH MSRs are read-only or read-write before locking the
+feature control register and handing over control to the operating system.
+
+Enclave construction
+
+
+The construction is started by filling out the SECS that contains enclave
+address range, privileged attributes and measurement of TCS and REG pages 
(pages
+that will be mapped to the address range) among the other things. This 
structure
+is passed out to the ENCLS(ECREATE) together with a physical address of a page
+in EPC that will hold the SECS.
+
+The pages are added with ENCLS(EADD) and measured with ENCLS(EEXTEND) i.e.
+SHA256 hash MRENCLAVE re

[PATCH v15 00/23] Intel SGX1

[Jarkko Sakkinen]

Intel(R) SGX is a set of CPU instructions that can be used by applications
to set aside private regions of code and data. The code outside the enclave
is disallowed to access the memory inside the enclave by the CPU access
control.  In a way you can think that SGX provides inverted sandbox. It
protects the application from a malicious host.

There is a new hardware unit in the processor called Memory Encryption
Engine (MEE) starting from the Skylake microacrhitecture. BIOS can define
one or many MEE regions that can hold enclave data by configuring them with
PRMRR registers.

The MEE automatically encrypts the data leaving the processor package to
the MEE regions. The data is encrypted using a random key whose life-time
is exactly one power cycle.

The current implementation requires that the firmware sets
IA32_SGXLEPUBKEYHASH* MSRs as writable so that ultimately the kernel can
decide what enclaves it wants run. The implementation does not create
any bottlenecks to support read-only MSRs later on.

You can tell if your CPU supports SGX by looking into /proc/cpuinfo:

cat /proc/cpuinfo  | grep sgx

v15:
* Split into more digestable size patches.
* Lots of small fixes and clean ups.
* Signal a "plain" SIGSEGV on an EPCM violation.

v14:
* Change the comment about X86_FEATURE_SGX_LC from “SGX launch
  configuration” to “SGX launch control”.
* Move the SGX-related CPU feature flags as part of the Linux defined
  virtual leaf 8.
* Add SGX_ prefix to the constants defining the ENCLS leaf functions.
* Use GENMASK*() and BIT*() in sgx_arch.h instead of raw hex numbers.
* Refine the long description for CONFIG_INTEL_SGX_CORE.
* Do not use pr_*_ratelimited()  in the driver. The use of the rate limited
  versions is legacy cruft from the prototyping phase.
* Detect sleep with SGX_INVALID_EINIT_TOKEN instead of counting power
  cycles.
* Manually prefix with “sgx:” in the core SGX code instead of redefining
  pr_fmt.
* Report if IA32_SGXLEPUBKEYHASHx MSRs are not writable in the driver
  instead of core because it is a driver requirement.
* Change prompt to bool in the entry for CONFIG_INTEL_SGX_CORE because the
  default is ‘n’.
* Rename struct sgx_epc_bank as struct sgx_epc_section in order to match
  the SDM.
* Allocate struct sgx_epc_page instances one at a time.
* Use “__iomem void *” pointers for the mapped EPC memory consistently.
* Retry once on SGX_INVALID_TOKEN in sgx_einit() instead of counting power
  cycles.
* Call enclave swapping operations directly from the driver instead of
  calling them .indirectly through struct sgx_epc_page_ops because indirect
  calls are not required yet as the patch set does not contain the KVM
  support.
* Added special signal SEGV_SGXERR to notify about SGX EPCM violation
  errors.

v13:
* Always use SGX_CPUID constant instead of a hardcoded value.
* Simplified and documented the macros and functions for ENCLS leaves.
* Enable sgx_free_page() to free active enclave pages on demand
  in order to allow sgx_invalidate() to delete enclave pages.
  It no longer performs EREMOVE if a page is in the process of
  being reclaimed.
* Use PM notifier per enclave so that we don't have to traverse
  the global list of active EPC pages to find enclaves.
* Removed unused SGX_LE_ROLLBACK constant from uapi/asm/sgx.h
* Always use ioremap() to map EPC banks as we only support 64-bit kernel.
* Invalidate IA32_SGXLEPUBKEYHASH cache used by sgx_einit() when going
  to sleep.

v12:
* Split to more narrow scoped commits in order to ease the review process and
  use co-developed-by tag for co-authors of commits instead of listing them in
  the source files.
* Removed cruft EXPORT_SYMBOL() declarations and converted to static variables.
* Removed in-kernel LE i.e. this version of the SGX software stack only
  supports unlocked IA32_SGXLEPUBKEYHASHx MSRs.
* Refined documentation on launching enclaves, swapping and enclave
  construction.
* Refined sgx_arch.h to include alignment information for every struct that
  requires it and removed structs that are not needed without an LE.
* Got rid of SGX_CPUID.
* SGX detection now prints log messages about firmware configuration issues.

v11:
* Polished ENCLS wrappers with refined exception handling.
* ksgxswapd was not stopped (regression in v5) in
  sgx_page_cache_teardown(), which causes a leaked kthread after driver
  deinitialization.
* Shutdown sgx_le_proxy when going to suspend because its EPC pages will be
  invalidated when resuming, which will cause it not function properly
  anymore.
* Set EINITTOKEN.VALID to zero for a token that is passed when
  SGXLEPUBKEYHASH matches MRSIGNER as alloc_page() does not give a zero
  page.
* Fixed the check in sgx_edbgrd() for a TCS page. Allowed to read offsets
  around the flags field, which causes a #GP. Only flags read is readable.
* On read access memcpy() call inside sgx_vma_access() had src and dest
  parameters in wrong order.
* The build issue with CONFIG_KASAN is now fixed. Added undefined symbol

Re: [PATCH 10/17] prmem: documentation

[Nadav Amit]

From: Nadav Amit
Sent: November 1, 2018 at 4:31:59 PM GMT
> To: Peter Zijlstra 
> 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
> 
> 
> 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.

My patches had several embarrassing bugs. I’m fixing and will resend later,
hopefully today.

Re: [PATCH security-next v5 12/30] LSM: Provide separate ordered initialization

[Kees Cook]

On Fri, Nov 2, 2018 at 11:13 AM, Mimi Zohar  wrote:
> I don't recall why "integrity" is on the security_initcall, while both
> IMA and EVM are on the late_initcall().

It's because integrity needs to have a VFS buffer allocated extremely
early, so it used the security init to do it. While it's not an LSM,
it does use this part of LSM infrastructure. I didn't see an obvious
alternative at the time, but now that I think about it, maybe just a
simple postcore_initcall() would work?

-Kees

-- 
Kees Cook

Re: [PATCH security-next v5 12/30] LSM: Provide separate ordered initialization

[Mimi Zohar]

Hi Kees,

On Wed, 2018-10-10 at 17:18 -0700, Kees Cook wrote:
> This provides a place for ordered LSMs to be initialized, separate from
> the "major" LSMs. This is mainly a copy/paste from major_lsm_init() to
> ordered_lsm_init(), but it will change drastically in later patches.
> 
> What is not obvious in the patch is that this change moves the integrity
> LSM from major_lsm_init() into ordered_lsm_init(), since it is not marked
> with the LSM_FLAG_LEGACY_MAJOR. As it is the only LSM in the "ordered"
> list, there is no reordering yet created.

I'm so sorry for not reviewing these patches earlier.  Both IMA and
EVM are dependent on "integrity", but "integrity", at least by itself,
should not be considered an LSM.

I don't recall why "integrity" is on the security_initcall, while both
IMA and EVM are on the late_initcall().

Mimi

> 
> Signed-off-by: Kees Cook 
> Reviewed-by: Casey Schaufler 
> Reviewed-by: John Johansen 
> ---
>  security/security.c | 21 +
>  1 file changed, 21 insertions(+)
> 
> diff --git a/security/security.c b/security/security.c
> index 2055af907eba..e672ced5 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -52,12 +52,30 @@ static __initdata bool debug;
>   pr_info(__VA_ARGS__);   \
>   } while (0)
> 
> +static void __init ordered_lsm_init(void)
> +{
> + struct lsm_info *lsm;
> + int ret;
> +
> + for (lsm = __start_lsm_info; lsm < __end_lsm_info; lsm++) {
> + if ((lsm->flags & LSM_FLAG_LEGACY_MAJOR) != 0)
> + continue;
> +
> + init_debug("initializing %s\n", lsm->name);
> + ret = lsm->init();
> + WARN(ret, "%s failed to initialize: %d\n", lsm->name, ret);
> + }
> +}
> +
>  static void __init major_lsm_init(void)
>  {
>   struct lsm_info *lsm;
>   int ret;
> 
>   for (lsm = __start_lsm_info; lsm < __end_lsm_info; lsm++) {
> + if ((lsm->flags & LSM_FLAG_LEGACY_MAJOR) == 0)
> + continue;
> +
>   init_debug("initializing %s\n", lsm->name);
>   ret = lsm->init();
>   WARN(ret, "%s failed to initialize: %d\n", lsm->name, ret);
> @@ -87,6 +105,9 @@ int __init security_init(void)
>   yama_add_hooks();
>   loadpin_add_hooks();
> 
> + /* Load LSMs in specified order. */
> + ordered_lsm_init();
> +
>   /*
>* Load all the remaining security modules.
>*/

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

[Andrey Ryabinin]

On 11/02/2018 07:11 PM, Linus Torvalds wrote:
> On Fri, Nov 2, 2018 at 6:16 AM Andrey Ryabinin  
> wrote:
>>
>> On 11/02/2018 04:46 AM, Linus Torvalds wrote:
>>>
>>> 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.
>>
>> I'd rather use cc-option instead of version check, since we also support 
>> clang.
> 
> That would be even better, but I thought the requirement for 4.9.2
> came not from the option existing, but because of some bugs getting
> fixed?

It looks unusual but -fsanitize=kernel-address really showed up only in 4.9.2.
It was actually backported from 5.0 branch to 4.9 for whatever reason.

> 
> But if we can do it without version checks, that would be lovely.
> 
> Linus
> 

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

[Linus Torvalds]

On Fri, Nov 2, 2018 at 6:16 AM Andrey Ryabinin  wrote:
>
> On 11/02/2018 04:46 AM, Linus Torvalds wrote:
> >
> > 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.
>
> I'd rather use cc-option instead of version check, since we also support 
> clang.

That would be even better, but I thought the requirement for 4.9.2
came not from the option existing, but because of some bugs getting
fixed?

But if we can do it without version checks, that would be lovely.

Linus

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

[Linus Torvalds]

On Fri, Nov 2, 2018 at 2:43 AM Andrey Ryabinin  wrote:
>
> You're right, version checks shouldn't matter here. But 
> __no_sanitize_address_or_inline
> shouldn't have been added in the first place, because we already have almost 
> the same
>__no_kasan_or_inline:

Ahh, very good.

Vasily, Martin - since __no_sanitize_address_or_inline was added just
for s390, would you mind replacing it with __no_kasan_or_inline
instead, and testing that in whatever failed before?

Then we can just remove that unnecessary symbol #define entirely..

Thanks,

 Linus

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

[Steven Rostedt]

On Fri, 2 Nov 2018 10:43:26 -0500
Josh Poimboeuf  wrote:

> > I'll hopefully have a prototype ready by plumbers.  
> 
> Why do we need multiple users?  It would be a lot simpler if we could
> just enforce a single user per fgraphed/kretprobed function (and return
> -EBUSY if it's already being traced/probed).

Because that means if function graph tracer is active, then you can't
do a kretprobe, and vice versa. I'd really like to have it working for
multiple users, then we could trace different graph functions and store
them in different buffers. It would also allow for perf to use function
graph tracer too.

> 
> > And this too will require each architecture to probably change. As a
> > side project to this, I'm going to try to consolidate the function
> > graph code among all the architectures as well. Not an easy task.  
> 
> Do you mean implementing HAVE_FUNCTION_GRAPH_RET_ADDR_PTR for all the
> arches?  If so, I think have an old crusty patch which attempted to
> that.  I could try to dig it up if you're interested.
> 

I'd like to have that, but it still requires some work. But I'd just
the truly architecture dependent code be in the architecture (basically
the asm code), and have the ability to move most of the duplicate code
out of the archs.

-- Steve

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

[Josh Poimboeuf]

On Fri, Nov 02, 2018 at 09:16:58AM -0400, Steven Rostedt wrote:
> On Fri, 2 Nov 2018 17:59:32 +1100
> Aleksa Sarai  wrote:
> 
> > As an aside, I just tested with the frame unwinder and it isn't thrown
> > off-course by kretprobe_trampoline (though obviously the stack is still
> > wrong). So I think we just need to hook into the ORC unwinder to get it
> > to continue skipping up the stack, as well as add the rewriting code for
> > the stack traces (for all unwinders I guess -- though ideally we should
> 
> I agree that this is the right solution.

Sounds good to me.

However, it would be *really* nice if function graph and kretprobes
shared the same infrastructure, like they do for function entry.
There's a lot of duplicated effort there.

> > do this without having to add the same code to every architecture).
> 
> True, and there's an art to consolidating the code between
> architectures.
> 
> I'm currently looking at function graph and seeing if I can consolidate
> it too. And I'm also trying to get multiple uses to hook into its
> infrastructure. I think I finally figured out a way to do so.
> 
> The reason it is difficult, is that you need to maintain state between
> the entry of a function and the exit for each task and callback that is
> registered. Hence, it's a 3x tuple (function stack, task, callbacks).
> And this must be maintained with preemption. A task may sleep for
> minutes, and the state needs to be retained.
> 
> The only state that must be retained is the function stack with the
> task, because if that gets out of sync, the system crashes. But the
> callback state can be removed.
> 
> Here's what is there now:
> 
>  When something is registered with the function graph tracer, every
>  task gets a shadowed stack. A hook is added to fork to add shadow
>  stacks to new tasks. Once a shadow stack is added to a task, that
>  shadow stack is never removed until the task exits.
> 
>  When the function is entered, the real return code is stored in the
>  shadow stack and the trampoline address is put in its place.
> 
>  On return, the trampoline is called, and it will pop off the return
>  code from the shadow stack and return to that.
> 
> The issue with multiple users, is that different users may want to
> trace different functions. On entry, the user could say it doesn't want
> to trace the current function, and the return part must not be called
> on exit. Keeping track of which user needs the return called is the
> tricky part.
> 
> Here's what I plan on implementing:
> 
>  Along with a shadow stack, I was going to add a 4096 byte (one page)
>  array that holds 64 8 byte masks to every task as well. This will allow
>  64 simultaneous users (which is rather extreme). If we need to support
>  more, we could allocate another page for all tasks. The 8 byte mask
>  will represent each depth (allowing to do this for 64 function call
>  stack depth, which should also be enough).
> 
>  Each user will be assigned one of the masks. Each bit in the mask
>  represents the depth of the shadow stack. When a function is called,
>  each user registered with the function graph tracer will get called
>  (if they asked to be called for this function, via the ftrace_ops
>  hashes) and if they want to trace the function, then the bit is set in
>  the mask for that stack depth.
> 
>  When the function exits the function and we pop off the return code
>  from the shadow stack, we then look at all the bits set for the
>  corresponding users, and call their return callbacks, and ignore
>  anything that is not set.
> 
> 
> When a user is unregistered, it the corresponding bits that represent
> it are cleared, and it the return callback will not be called. But the
> tasks being traced will still have their shadow stack to allow it to
> get back to normal.
> 
> I'll hopefully have a prototype ready by plumbers.

Why do we need multiple users?  It would be a lot simpler if we could
just enforce a single user per fgraphed/kretprobed function (and return
-EBUSY if it's already being traced/probed).

> And this too will require each architecture to probably change. As a
> side project to this, I'm going to try to consolidate the function
> graph code among all the architectures as well. Not an easy task.

Do you mean implementing HAVE_FUNCTION_GRAPH_RET_ADDR_PTR for all the
arches?  If so, I think have an old crusty patch which attempted to
that.  I could try to dig it up if you're interested.

-- 
Josh

Re: [PATCH v14 12/12] cpuset: Show descriptive text when reading cpuset.sched.partition

[Waiman Long]

On 10/19/2018 03:24 PM, Tejun Heo wrote:
> On Fri, Oct 19, 2018 at 02:56:13PM -0400, Waiman Long wrote:
>> On 10/17/2018 11:08 AM, Tejun Heo wrote:
>>> On Mon, Oct 15, 2018 at 04:29:37PM -0400, Waiman Long wrote:
 Currently, cpuset.sched.partition returns the values, 0, 1 or -1 on
 read. A person who is not familiar with the partition code may not
 understand what they mean.

 In order to make cpuset.sched.partition more user-friendly, it will
 now display the following descriptive text on read:

   "normal" - A normal cpuset, not a partition root
   "partition" - A partition root
   "partition invalid" - An invalid partition root

 Suggested-by: Tejun Heo 
 Signed-off-by: Waiman Long 
>>> Can you also make this consistent when writing to the file?
>>>
>>> Thanks.
>>>
>> How about the attached patch instead?
> Looks good to me.  Once Peter is okay with it, I'll roll it into
> cgroup devel branch.  As v4.19 is almost done, I think it makes more
> sense to target the next merge window (v4.21).
>
> Thanks.
>
Peter, are you OK with the current cpuset v2 patch which does allow
hierarchical partitions as you have requested?

Cheers,
Longman

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

[Steven Rostedt]

On Fri, 2 Nov 2018 17:59:32 +1100
Aleksa Sarai  wrote:

> As an aside, I just tested with the frame unwinder and it isn't thrown
> off-course by kretprobe_trampoline (though obviously the stack is still
> wrong). So I think we just need to hook into the ORC unwinder to get it
> to continue skipping up the stack, as well as add the rewriting code for
> the stack traces (for all unwinders I guess -- though ideally we should

I agree that this is the right solution.

> do this without having to add the same code to every architecture).

True, and there's an art to consolidating the code between
architectures.

I'm currently looking at function graph and seeing if I can consolidate
it too. And I'm also trying to get multiple uses to hook into its
infrastructure. I think I finally figured out a way to do so.

The reason it is difficult, is that you need to maintain state between
the entry of a function and the exit for each task and callback that is
registered. Hence, it's a 3x tuple (function stack, task, callbacks).
And this must be maintained with preemption. A task may sleep for
minutes, and the state needs to be retained.

The only state that must be retained is the function stack with the
task, because if that gets out of sync, the system crashes. But the
callback state can be removed.

Here's what is there now:

 When something is registered with the function graph tracer, every
 task gets a shadowed stack. A hook is added to fork to add shadow
 stacks to new tasks. Once a shadow stack is added to a task, that
 shadow stack is never removed until the task exits.

 When the function is entered, the real return code is stored in the
 shadow stack and the trampoline address is put in its place.

 On return, the trampoline is called, and it will pop off the return
 code from the shadow stack and return to that.

The issue with multiple users, is that different users may want to
trace different functions. On entry, the user could say it doesn't want
to trace the current function, and the return part must not be called
on exit. Keeping track of which user needs the return called is the
tricky part.

Here's what I plan on implementing:

 Along with a shadow stack, I was going to add a 4096 byte (one page)
 array that holds 64 8 byte masks to every task as well. This will allow
 64 simultaneous users (which is rather extreme). If we need to support
 more, we could allocate another page for all tasks. The 8 byte mask
 will represent each depth (allowing to do this for 64 function call
 stack depth, which should also be enough).

 Each user will be assigned one of the masks. Each bit in the mask
 represents the depth of the shadow stack. When a function is called,
 each user registered with the function graph tracer will get called
 (if they asked to be called for this function, via the ftrace_ops
 hashes) and if they want to trace the function, then the bit is set in
 the mask for that stack depth.

 When the function exits the function and we pop off the return code
 from the shadow stack, we then look at all the bits set for the
 corresponding users, and call their return callbacks, and ignore
 anything that is not set.


When a user is unregistered, it the corresponding bits that represent
it are cleared, and it the return callback will not be called. But the
tasks being traced will still have their shadow stack to allow it to
get back to normal.

I'll hopefully have a prototype ready by plumbers.

And this too will require each architecture to probably change. As a
side project to this, I'm going to try to consolidate the function
graph code among all the architectures as well. Not an easy task.

-- Steve

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

[Andrey Ryabinin]

On 11/02/2018 04:46 AM, Linus Torvalds wrote:
> 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.
> 

I'd rather use cc-option instead of version check, since we also support clang.

It should be possible to express compiler requirements for subfeatures
like stack/inline instrumentation in Kconfig. But that would be not that 
trivial, 
see the scripts/Makefile.kasan

> 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
> 

Re: [PATCH v3 6/7] clk: clk-twl6040: Free of_provider at remove

[Peter Ujfalusi]

On 11/2/18 2:37 PM, Matti Vaittinen wrote:
> use devm variant for of_provider registration so provider is freed
> at exit.

Acked-by: Peter Ujfalusi 

> 
> 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 = {
> 

- Peter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

[PATCH v3 7/7] 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 v3 6/7] 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 v3 5/7] 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


-- 
Matti Vaittinen
ROHM Semiconductors

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

[PATCH v3 4/7] 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


-- 
Matti Vaittinen
ROHM Semiconductors

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

[PATCH v3 3/7] 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


-- 
Matti Vaittinen
ROHM Semiconductors

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

[PATCH v3 2/7] 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 
Reviewed-by: Krzysztof Kozlowski 
---
 drivers/clk/clk-max77686.c | 29 +++--
 1 file changed, 7 insertions(+), 22 deletions(-)

diff --git a/drivers/clk/clk-max77686.c b/drivers/clk/clk-max77686.c
index 02551fe4b87c..51a46179f6f8 100644
--- a/drivers/clk/clk-max77686.c
+++ b/drivers/clk/clk-max77686.c
@@ -235,8 +235,9 @@ static int max77686_clk_probe(struct platform_device *pdev)
return ret;
}
 
-   ret = clk_hw_register_clkdev(&max_clk_data->hw,
-max_clk_data->clk_idata.name, 
NULL);
+   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);
return ret;
@@ -244,8 +245,9 @@ 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 +263,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 +283,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


-- 
Matti Vaittinen
ROHM Semiconductors

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

[PATCH v3 1/7] 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 v3 0/7] 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-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 v3
Address issues spotted by Krzysztof Kozlowski
- Drop patch 3 for clk-s3c2410-dclk as this device can never be removed
- Fix indentiation for clk-max77686
- Rest  of the patches are unchanged.

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 (7):
  clk: clkdev/of_clk - add managed lookup and provider registrations
  clk: clk-max77686: Clean clkdev lookup leak and use devm
  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|  29 ++--
 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/x86/clk-st.c  |   3 +-
 include/linux/clk-provider.h  |  11 +++
 include/linux/clkdev.h|   4 ++
 11 files changed, 153 insertions(+), 76 deletions(-)

-- 
2.14.3


-- 
Matti Vaittinen
ROHM Semiconductors

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

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

[Matti Vaittinen]

Thanks for taking the time and reviewing this!

On Fri, Nov 02, 2018 at 09:15:17AM +0100, Krzysztof Kozlowski wrote:
> On Thu, 1 Nov 2018 at 08:19, Matti Vaittinen
>  wrote:
> >
> > 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);
> 
> You need to re-align the next line.

I'll change this to
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);
> 
> The same, please.

I will change this to 
ret = devm_of_clk_add_parent_hw_provider(dev,
 of_clk_max77686_get,
 drv_data);
> 
> Rest looks good, so with these changes:
> Reviewed-by: Krzysztof Kozlowski 
> 
> Best regards,
> Krzysztof

-- 
Matti Vaittinen
ROHM Semiconductors

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

Re: [PATCH 2/2] Compiler Attributes: auxdisplay: panel: use __fallthrough

[Miguel Ojeda]

On Fri, Nov 2, 2018 at 11:49 AM Miguel Ojeda
 wrote:
>
> Thanks for checking! Let's wait then a few months and see if we can
> get cppcheck/Eclipse to support it.

In the meantime, saved here too:

  https://github.com/ojeda/linux/tree/compiler-attributes-fallthrough

rebased on top of e468f5c06b5e ("Merge tag
'compiler-attributes-for-linus-4.20-rc1' of
https://github.com/ojeda/linux";).

Cheers,
Miguel

Re: [PATCH 2/2] Compiler Attributes: auxdisplay: panel: use __fallthrough

[Miguel Ojeda]

Hi Dan,

On Tue, Oct 23, 2018 at 7:37 AM Dan Carpenter  wrote:
>
> On Mon, Oct 22, 2018 at 07:10:02AM -0700, Kees Cook wrote:
> > I would prefer we continue to use the comment style until we've got
> > confirmed support for (at least) Clang, Coverity, CPPcheck, smatch,
> > and eclipse.
>
> Clang and Smatch don't support fall throught comments.  Coverity
> supports both.  CPPcheck is unknown.
>
> Eclipse doesn't support attributes.  So it's just Eclipse and maybe
> CPP check.

Thanks for checking! Let's wait then a few months and see if we can
get cppcheck/Eclipse to support it.

Cheers,
Miguel

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

[Miguel Ojeda]

On Fri, Nov 2, 2018 at 2:52 AM Linus Torvalds
 wrote:
>
> 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.

I was a bit confused when reading the gcc bug reports, i.e. why gcc
did *not* complain in 4.9 but it did in 5.1 (when it was supposed to
complain also in 4.9). It turns out that gcc 5.1 takes into account
who is the actual caller due to this change:

+  cgraph_node *caller = e->caller->global.inlined_to
+   ? e->caller->global.inlined_to : e->caller;
...
-  else if (!sanitize_attrs_match_for_inline_p (e->caller->decl, callee->decl))
+  else if (!sanitize_attrs_match_for_inline_p (caller->decl, callee->decl))

change; e.g. this:

#define really_inline inline __attribute__((always_inline))
#define no_sanitize __attribute__((no_sanitize_address))

really_inline void f() {}
really_inline void g() { f(); }
no_sanitize void h() { g(); }

Complains in gcc 4.9 -O0, 5.1 -O0 and 5.1 -O2; but *not* in 4.9 -O2.
https://godbolt.org/z/kNApqk

Anyway, this is orthogonal but in case it clarifies that for someone else...

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

I was sleeping, didn't manage to see it (in your GitHub, I guess?).

I did the merge myself, and arrived at the same thing as you. I
quickly inspected the rest and seems fine. By the way, I spotted an
extra space at:

+ * we do one or the other.

Cheers,
Miguel

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

[Andrey Ryabinin]

On 11/01/2018 08:06 PM, Linus Torvalds wrote:
> 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+.

As you said in other email - "this has less to do with the attribute
being recognized, and simply just being because KASAN itself wants
gcc-4.9.2."

gcc < 4.9.2 simply doesn't have -fsanitize=kernel-address.
But no_sanitize attribute originally comes with user space ASAN 
(-fsanitize=address)
which exist in earlier GCCs like 4.8.
Checking against 4.8 gcc should be fine. If we compile the kernel with gcc 4.8
it will be compiled without instrumentation, so the attribute won't have any 
effect.

> 
> 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?).

You're right, version checks shouldn't matter here. But 
__no_sanitize_address_or_inline
shouldn't have been added in the first place, because we already have almost 
the same __no_kasan_or_inline:

#ifdef CONFIG_KASAN
/*
 * We can't declare function 'inline' because __no_sanitize_address confilcts
 * with inlining. Attempt to inline it may cause a build failure.
 *  https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67368
 * '__maybe_unused' allows us to avoid defined-but-not-used warnings.
 */
# define __no_kasan_or_inline __no_sanitize_address __maybe_unused
#else
# define __no_kasan_or_inline __always_inline
#endif

There are small differences like absence of notrace, but notrace could be added
to the function together with __no_kasan_or_inline.
And inline is *not* redefined to __always_inline  only on x86 when 
CONFIG_OPTIMIZE_INLINING=y

So there is absolutely no reason to have both __no_kasan_or_inline and 
__no_sanitize_address_or_inline.

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

[Andy Shevchenko]

On Fri, Nov 2, 2018 at 6:46 AM 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. Increases the
> way to get information through FDT.
>

so OF dependency in Kconfig is a matter of this patch.
Since 0day bot complain you might need to add

#include 

as well.

> 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
>


-- 
With Best Regards,
Andy Shevchenko

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

[Andy Shevchenko]

On Fri, Nov 2, 2018 at 6:46 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.
>

0day complains b/c of 1st patch in the series.

Reviewed-by: Andy Shevchenko 

> Suggested-by: Andy Shevchenko 

This wasn't suggested by me. It's your idea to support to MMIO.

> 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
>


-- 
With Best Regards,
Andy Shevchenko

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

[Andy Shevchenko]

On Fri, Nov 2, 2018 at 6:46 AM Peng Hao  wrote:
>
> 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

0day bot is rightful in its complain.
>From where OF comes here?

> +   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
>


-- 
With Best Regards,
Andy Shevchenko

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

[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: sh-allyesconfig (attached as .config)
compiler: sh4-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
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
GCC_VERSION=7.2.0 make.cross ARCH=sh 

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:3: error: 'struct acpi_driver' has no member 
named 'name'
 .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:3: error: 'struct acpi_driver' has no member 
named 'class'
 .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:3: error: 'struct acpi_driver' has no member 
named 'ids'
 .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:3: error: 'struct acpi_driver' has no member 
named 'ops'
 .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:3: error: 'struct acpi_driver' has no member 
named 'owner'
 .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_walk_resources':
>> drivers//misc/pvpanic.c:82:6: error: implicit declaration of function 
>> 'acpi_dev_resource_io'; did you mean 'acpi_walk_resources'? 
>> [-Werror=implicit-function-declaration]
 if (acpi_dev_resource_io(res, &r)) {
 ^~~~
 acpi_walk_resources
   drivers//misc/pvpanic.c: In function 'pvpanic_add':
   drivers//misc/pvpanic.c:94:8: error: implicit declaration of function 
'acpi_bus_get_status'; did you mean 'acpi_get_gpe_status'? 
[-Werror=implicit-function-declaration]
 ret = acpi_bus_get_status(device);
   ^~~
   acpi_get_gpe_status
   drivers//misc/pvpanic.c:98:13: error: dereferencing pointer to incomplete 
type 'struct acpi_device'
 if (!device->status.enabled || !device->status.functional)
^

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

[Matti Vaittinen]

Thanks Krzysztof!

On Fri, Nov 02, 2018 at 09:19:10AM +0100, Krzysztof Kozlowski wrote:
> On Thu, 1 Nov 2018 at 08:19, Matti Vaittinen
>  wrote:
> >
> > 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(-)
> 
> Driver has suppress_bind_attrs and it cannot be built as module so
> device removal should not be possible. There is no need to use devm.

This is exactly what I wanted to hear =) I will drop this patch from
series.

-- 
Matti Vaittinen
ROHM Semiconductors

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

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

[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 
'iowrite8' [-Werror=implicit-function-declaration]
 iowrite8(event, base);
 ^~~~
   drivers//misc/pvpanic.c: In function 'pvpanic_walk_resources':
   drivers//misc/pvpanic.c:82:6: error: implicit declaration of function 
'acpi_dev_resource_io' [-Werror=implicit-function-declaration]
 if (acpi_dev_resource_io(res, &r)) {
 ^~~~
>> drivers//misc/pvpanic.c:83:27: error: implicit declaration of function 
>> 'ioport_map' [-Werror=implicit-function-declaration]
  base = (void __iomem *) ioport_map(r.start,
  ^~
>> drivers//misc/pvpanic.c:86:13: error: implicit declaration of function 
>> 'acpi_dev_resource_memory' [-Werror=implicit-function-declaration]
 } else if (acpi_dev_resource_memory(res, &r)) {
^~~~
   drivers//misc/pvpanic.c:87:10: error: implicit declaration of function 
'ioremap' [-Werror=implicit-function-declaration]
  base = ioremap(r.start, r.end - r.start + 1);
 ^~~
   driver

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

[Krzysztof Kozlowski]

On Thu, 1 Nov 2018 at 08:19, Matti Vaittinen
 wrote:
>
> 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(-)

Driver has suppress_bind_attrs and it cannot be built as module so
device removal should not be possible. There is no need to use devm.

Best regards,
Krzysztof

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

[Krzysztof Kozlowski]

On Thu, 1 Nov 2018 at 08:19, Matti Vaittinen
 wrote:
>
> 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);

You need to re-align the next line.

> 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);

The same, please.

Rest looks good, so with these changes:
Reviewed-by: Krzysztof Kozlowski 

Best regards,
Krzysztof

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

[Aleksa Sarai]

On 2018-11-02, Aleksa Sarai  wrote:
> 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?

Sorry, I just figured out how it works! (To make sure I actually
understand -- retp is a pointer to the place in the stack where the
return address is stored, so it uniquely specifies what each trampoline
actually points to -- this trick could also be used for kretprobes).

-- 
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: sh-allyesconfig (attached as .config)
compiler: sh4-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
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
GCC_VERSION=7.2.0 make.cross ARCH=sh 

All error/warnings (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:3: error: 'struct acpi_driver' has no member named 
>> 'name'
 .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:3: error: 'struct acpi_driver' has no member named 
>> 'class'
 .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:3: error: 'struct acpi_driver' has no member named 
>> 'ids'
 .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:3: error: 'struct acpi_driver' has no member named 
>> 'ops'
 .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:3: error: 'struct acpi_driver' has no member named 
>> 'owner'
 .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'; did you mean 'acpi_get_gpe_status'? 
>> [-Werror=implicit-function-declaration]
 ret = acpi_bus_get_status(device);
   ^~~
   acpi_get_gpe_status
>> 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&#x

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

[Aleksa Sarai]

On 2018-11-02, Aleksa Sarai  wrote:
> 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).

As an aside, I just tested with the frame unwinder and it isn't thrown
off-course by kretprobe_trampoline (though obviously the stack is still
wrong). So I think we just need to hook into the ORC unwinder to get it
to continue skipping up the stack, as well as add the rewriting code for
the stack traces (for all unwinders I guess -- though ideally we should
do this without having to add the same code to every architecture).

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



signature.asc
Description: PGP signature