[RFC tip/locking/lockdep v5 03/17] lockdep: Redefine LOCK_*_STATE* bits

[Boqun Feng]

There are three types of lock acquisitions: write, non-recursive read
and recursive read, among which write locks and non-recursive read locks
have no difference from a viewpoint for deadlock detections, because a
write acquisition of the corresponding lock on an independent CPU or
task makes a non-recursive read lock act as a write lock in the sense of
deadlock. So we could treat them as the same type(named as
"non-recursive lock") in lockdep.

As in the irq lock inversion detection(safe->unsafe deadlock detection),
we used to differ write lock with read lock(non-recursive and
recursive ones), such a classification could be improved as
non-recursive read lock behaves the same as write lock, so this patch
redefines the meanings of LOCK_{USED_IN, ENABLED}_STATE*.

old:
LOCK_* : stands for write lock
LOCK_*_READ: stands for read lock(non-recursive and recursive)
new:
LOCK_* : stands for non-recursive(write lock and non-recursive
read lock)
LOCK_*_RR: stands for recursive read lock

Such a change is needed for a future improvement on recursive read
related irq inversion deadlock detection.

Signed-off-by: Boqun Feng 
---
 Documentation/locking/lockdep-design.txt |  6 +++---
 kernel/locking/lockdep.c | 28 ++--
 kernel/locking/lockdep_internals.h   | 16 
 kernel/locking/lockdep_proc.c| 12 ++--
 4 files changed, 31 insertions(+), 31 deletions(-)

diff --git a/Documentation/locking/lockdep-design.txt 
b/Documentation/locking/lockdep-design.txt
index 9de1c158d44c..382bc25589c2 100644
--- a/Documentation/locking/lockdep-design.txt
+++ b/Documentation/locking/lockdep-design.txt
@@ -30,9 +30,9 @@ State
 The validator tracks lock-class usage history into 4n + 1 separate state bits:
 
 - 'ever held in STATE context'
-- 'ever held as readlock in STATE context'
+- 'ever held as recursive readlock in STATE context'
 - 'ever held with STATE enabled'
-- 'ever held as readlock with STATE enabled'
+- 'ever held as recurisve readlock with STATE enabled'
 
 Where STATE can be either one of (kernel/locking/lockdep_states.h)
  - hardirq
@@ -51,7 +51,7 @@ locking error messages, inside curlies. A contrived example:
 (_locks[i].lock){-.-...}, at: [] mutex_lock+0x21/0x24
 
 
-The bit position indicates STATE, STATE-read, for each of the states listed
+The bit position indicates STATE, STATE-RR, for each of the states listed
 above, and the character displayed in each indicates:
 
'.'  acquired while irqs disabled and not in irq context
diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index c80f8276ceaa..5e6bf8d6954d 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -448,10 +448,10 @@ DEFINE_PER_CPU(struct lockdep_stats, lockdep_stats);
  */
 
 #define __USAGE(__STATE)   \
-   [LOCK_USED_IN_##__STATE] = "IN-"__stringify(__STATE)"-W",   \
-   [LOCK_ENABLED_##__STATE] = __stringify(__STATE)"-ON-W", \
-   [LOCK_USED_IN_##__STATE##_READ] = "IN-"__stringify(__STATE)"-R",\
-   [LOCK_ENABLED_##__STATE##_READ] = __stringify(__STATE)"-ON-R",
+   [LOCK_USED_IN_##__STATE] = "IN-"__stringify(__STATE),   \
+   [LOCK_ENABLED_##__STATE] = __stringify(__STATE)"-ON",   \
+   [LOCK_USED_IN_##__STATE##_RR] = "IN-"__stringify(__STATE)"-RR", \
+   [LOCK_ENABLED_##__STATE##_RR] = __stringify(__STATE)"-ON-RR",
 
 static const char *usage_str[] =
 {
@@ -492,7 +492,7 @@ void get_usage_chars(struct lock_class *class, char 
usage[LOCK_USAGE_CHARS])
 
 #define LOCKDEP_STATE(__STATE) 
\
usage[i++] = get_usage_char(class, LOCK_USED_IN_##__STATE); \
-   usage[i++] = get_usage_char(class, LOCK_USED_IN_##__STATE##_READ);
+   usage[i++] = get_usage_char(class, LOCK_USED_IN_##__STATE##_RR);
 #include "lockdep_states.h"
 #undef LOCKDEP_STATE
 
@@ -1645,7 +1645,7 @@ static const char *state_names[] = {
 
 static const char *state_rnames[] = {
 #define LOCKDEP_STATE(__STATE) \
-   __stringify(__STATE)"-READ",
+   __stringify(__STATE)"-RR",
 #include "lockdep_states.h"
 #undef LOCKDEP_STATE
 };
@@ -3039,14 +3039,14 @@ static int mark_irqflags(struct task_struct *curr, 
struct held_lock *hlock)
 * mark the lock as used in these contexts:
 */
if (!hlock->trylock) {
-   if (hlock->read) {
+   if (hlock->read == 2) {
if (curr->hardirq_context)
if (!mark_lock(curr, hlock,
-   LOCK_USED_IN_HARDIRQ_READ))
+   LOCK_USED_IN_HARDIRQ_RR))
return 0;
if (curr->softirq_context)
if (!mark_lock(curr, hlock,
-

[RFC tip/locking/lockdep v5 16/17] lockdep: Documention for recursive read lock detection reasoning

[Boqun Feng]

As now we support recursive read lock deadlock detection, add related
explanation in the Documentation/lockdep/lockdep-desgin.txt:

*   Definition of recursive read locks, non-recursive locks, strong
dependency path and notions of -(**)->.

*   Lockdep's assumption.

*   Informal proof of recursive read lock deadlock detection.

Signed-off-by: Boqun Feng 
Cc: Randy Dunlap 
---
 Documentation/locking/lockdep-design.txt | 170 +++
 1 file changed, 170 insertions(+)

diff --git a/Documentation/locking/lockdep-design.txt 
b/Documentation/locking/lockdep-design.txt
index 382bc25589c2..fd8a8d97ce58 100644
--- a/Documentation/locking/lockdep-design.txt
+++ b/Documentation/locking/lockdep-design.txt
@@ -284,3 +284,173 @@ Run the command and save the output, then compare against 
the output from
 a later run of this command to identify the leakers.  This same output
 can also help you find situations where runtime lock initialization has
 been omitted.
+
+Recursive Read Deadlock Detection:
+--
+Lockdep now is equipped with deadlock detection for recursive read locks.
+
+Recursive read locks, as their name indicates, are the locks able to be
+acquired recursively. Unlike non-recursive read locks, recursive read locks
+only get blocked by current write lock *holders* other than write lock
+*waiters*, for example:
+
+   TASK A: TASK B:
+
+   read_lock(X);
+
+   write_lock(X);
+
+   read_lock(X);
+
+is not a deadlock for recursive read locks, as while the task B is waiting for
+the lock X, the second read_lock() doesn't need to wait because it's a 
recursive
+read lock.
+
+Note that a lock can be a write lock(exclusive lock), a non-recursive read lock
+(non-recursive shared lock) or a recursive read lock(recursive shared lock),
+depending on the API used to acquire it(more specifically, the value of the
+'read' parameter for lock_acquire(...)). In other words, a single lock instance
+has three types of acquisition depending on the acquisition functions:
+exclusive, non-recursive read, and recursive read.
+
+That said, recursive read locks could introduce deadlocks too, considering the
+following:
+
+   TASK A: TASK B:
+
+   read_lock(X);
+   read_lock(Y);
+   write_lock(Y);
+   write_lock(X);
+
+, neither task could get the write locks because the corresponding read locks
+are held by each other.
+
+Lockdep could detect recursive read lock related deadlocks. The 
dependencies(edges)
+in the lockdep graph are classified into four categories:
+
+1) -(NN)->: non-recursive to non-recursive dependency, non-recursive locks 
include
+non-recursive read locks, write locks and exclusive locks(e.g. 
spinlock_t).
+   They are treated equally in deadlock detection. "X -(NN)-> Y" means
+X -> Y and both X and Y are non-recursive locks.
+
+2) -(RN)->: recursive to non-recursive dependency, recursive locks means 
recursive read
+   locks. "X -(RN)-> Y" means X -> Y and X is recursive read lock and
+Y is non-recursive lock.
+
+3) -(NR)->: non-recursive to recursive dependency, "X -(NR)-> Y" means X -> Y 
and X is
+non-recursive lock and Y is recursive lock.
+
+4) -(RR)->: recursive to recursive dependency, "X -(RR)-> Y" means X -> Y and 
both X
+and Y are recursive locks.
+
+Note that given two locks, they may have multiple dependencies between them, 
for example:
+
+   TASK A:
+
+   read_lock(X);
+   write_lock(Y);
+   ...
+
+   TASK B:
+
+   write_lock(X);
+   write_lock(Y);
+
+, we have both X -(RN)-> Y and X -(NN)-> Y in the dependency graph.
+
+And obviously a non-recursive lock can block the corresponding recursive lock,
+and vice versa. Besides a non-recursive lock may block the other non-recursive
+lock of the same instance(e.g. a write lock may block a corresponding
+non-recursive read lock and vice versa).
+
+We use -(*N)-> for edges that is either -(RN)-> or -(NN)->, the similar for 
-(N*)->,
+-(*R)-> and -(R*)->
+
+A "path" is a series of conjunct dependency edges in the graph. And we define a
+"strong" path, which indicates the strong dependency throughout each dependency
+in the path, as the path that doesn't have two conjunct edges(dependencies) as
+-(*R)-> and -(R*)->. IOW, a "strong" path is a path from a lock walking to 
another
+through the lock dependencies, and if X -> Y -> Z in the path(where X, Y, Z are
+locks), if the walk from X to Y is through a -(NR)-> or -(RR)-> dependency, the
+walk from Y to Z must not be through a -(RN)-> or -(RR)-> dependency, otherwise
+it's not a strong path.
+
+We now prove that if a strong path forms a circle, then we have a potential 
deadlock.
+By "forms a circle", it means for a set of locks A0,A1...An, there is a path 

[RFC PATCH] drivers/peci: peci_match_id() can be static

[kbuild test robot]

Fixes: 99f5d2b99ecd ("drivers/peci: Add support for PECI bus driver core")
Signed-off-by: Fengguang Wu 
---
 peci-core.c |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/peci/peci-core.c b/drivers/peci/peci-core.c
index d976c73..4709b8c 100644
--- a/drivers/peci/peci-core.c
+++ b/drivers/peci/peci-core.c
@@ -770,8 +770,8 @@ peci_of_match_device(const struct of_device_id *matches,
 }
 #endif
 
-const struct peci_device_id *peci_match_id(const struct peci_device_id *id,
-  struct peci_client *client)
+static const struct peci_device_id *peci_match_id(const struct peci_device_id 
*id,
+ struct peci_client *client)
 {
if (!(id && client))
return NULL;
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/8] [PATCH 1/8] drivers/peci: Add support for PECI bus driver core

[kbuild test robot]

Hi Jae,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[also build test WARNING on v4.16-rc2 next-20180221]
[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/Jae-Hyun-Yoo/PECI-device-driver-introduction/20180222-054545
reproduce:
# apt-get install sparse
make ARCH=x86_64 allmodconfig
make C=1 CF=-D__CHECK_ENDIAN__


sparse warnings: (new ones prefixed by >>)

>> drivers/peci/peci-core.c:773:29: sparse: symbol 'peci_match_id' was not 
>> declared. Should it be

Please review and possibly fold the followup patch.

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/8] [PATCH 1/8] drivers/peci: Add support for PECI bus driver core

[Greg KH]

On Wed, Feb 21, 2018 at 12:42:30PM -0800, Jae Hyun Yoo wrote:
> On 2/21/2018 9:58 AM, Greg KH wrote:
> > On Wed, Feb 21, 2018 at 08:15:59AM -0800, Jae Hyun Yoo wrote:
> > > This commit adds driver implementation for PECI bus into linux
> > > driver framework.
> > > 
> > > Signed-off-by: Jae Hyun Yoo 
> > > ---
> > 
> > Why is there no other Intel developers willing to review and sign off on
> > this patch?  Please get their review first before asking us to do their
> > work for them :)
> > 
> > thanks,
> > 
> > greg k-h
> > 
> 
> Hi Greg,
> 
> This patch set got our internal review process. Sorry if it's code quality
> is under your expectation but it's the reason why I'm asking you to review
> the code. Could you please share your time to review it?

Nope.  If no other Intel developer thinks it is good enough to put their
name on it as part of their review process, why should I?

Again, please use the resources you have, to fix the obvious problems in
your code, BEFORE asking the community to do that work for you.

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

Re: [PATCH] doc: process: Add "Root-caused-by" and "Suggested-by"

[Kees Cook]

On Wed, Feb 21, 2018 at 8:43 PM, Randy Dunlap  wrote:
> On 02/21/2018 04:37 PM, Kees Cook wrote:
>> As recently pointed out by Linus, "Root-caused-by" is a good tag to include
>> since it can indicate significantly more work than "just" a Reported-by.
>> This adds it and "Suggested-by" (which was also missing) to the documented
>> list of tags. Additionally updates checkpatch.pl to match the process docs.
>>
>> Signed-off-by: Kees Cook 
>> ---
>>  Documentation/process/5.Posting.rst | 7 +++
>>  scripts/checkpatch.pl   | 2 ++
>>  2 files changed, 9 insertions(+)
>
> I would still rather see Co-developed-by: in both the docs and in checkpatch. 
> :(

Hm? It is in docs. This syncs the process doc to checkpatch...

-Kees

>
>> diff --git a/Documentation/process/5.Posting.rst 
>> b/Documentation/process/5.Posting.rst
>> index 645fa9c7388a..2ff01f76f02a 100644
>> --- a/Documentation/process/5.Posting.rst
>> +++ b/Documentation/process/5.Posting.rst
>> @@ -234,6 +234,13 @@ The tags in common use are:
>> people who test our code and let us know when things do not work
>> correctly.
>>
>> + - Suggested-by: names a person who suggested the solution, but may not
>> +   have constructed the full patch. A weaker version of `Co-Developed-by`.
>> +
>> + - Root-caused-by: names a person who diagnosed the root cause of a
>> +   problem. This usually indicates significantly more work than a simple
>> +   `Reported-by`.
>> +
>>   - Cc: the named person received a copy of the patch and had the
>> opportunity to comment on it.
>>
>> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
>> index 3d4040322ae1..a1ab82e70b54 100755
>> --- a/scripts/checkpatch.pl
>> +++ b/scripts/checkpatch.pl
>> @@ -464,9 +464,11 @@ our $logFunctions = qr{(?x:
>>  our $signature_tags = qr{(?xi:
>>   Signed-off-by:|
>>   Acked-by:|
>> + Co-Developed-by:|
>>   Tested-by:|
>>   Reviewed-by:|
>>   Reported-by:|
>> + Root-caused-by:|
>>   Suggested-by:|
>>   To:|
>>   Cc:
>>
>
>
> --
> ~Randy



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

Re: [PATCH v12 01/22] selftests/x86: Move protecton key selftest to arch neutral directory

[Ingo Molnar]

* Ram Pai  wrote:

> cc: Dave Hansen 
> cc: Florian Weimer 
> Signed-off-by: Ram Pai 
> ---
>  tools/testing/selftests/vm/Makefile   |1 +
>  tools/testing/selftests/vm/pkey-helpers.h |  223 
>  tools/testing/selftests/vm/protection_keys.c  | 1407 
> +
>  tools/testing/selftests/x86/Makefile  |2 +-
>  tools/testing/selftests/x86/pkey-helpers.h|  223 
>  tools/testing/selftests/x86/protection_keys.c | 1407 
> -
>  6 files changed, 1632 insertions(+), 1631 deletions(-)
>  create mode 100644 tools/testing/selftests/vm/pkey-helpers.h
>  create mode 100644 tools/testing/selftests/vm/protection_keys.c
>  delete mode 100644 tools/testing/selftests/x86/pkey-helpers.h
>  delete mode 100644 tools/testing/selftests/x86/protection_keys.c

Acked-by: Ingo Molnar 

Thanks,

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

Re: [PATCH] doc: process: Add "Root-caused-by" and "Suggested-by"

[Randy Dunlap]

On 02/21/2018 04:37 PM, Kees Cook wrote:
> As recently pointed out by Linus, "Root-caused-by" is a good tag to include
> since it can indicate significantly more work than "just" a Reported-by.
> This adds it and "Suggested-by" (which was also missing) to the documented
> list of tags. Additionally updates checkpatch.pl to match the process docs.
> 
> Signed-off-by: Kees Cook 
> ---
>  Documentation/process/5.Posting.rst | 7 +++
>  scripts/checkpatch.pl   | 2 ++
>  2 files changed, 9 insertions(+)

I would still rather see Co-developed-by: in both the docs and in checkpatch. :(

> diff --git a/Documentation/process/5.Posting.rst 
> b/Documentation/process/5.Posting.rst
> index 645fa9c7388a..2ff01f76f02a 100644
> --- a/Documentation/process/5.Posting.rst
> +++ b/Documentation/process/5.Posting.rst
> @@ -234,6 +234,13 @@ The tags in common use are:
> people who test our code and let us know when things do not work
> correctly.
>  
> + - Suggested-by: names a person who suggested the solution, but may not
> +   have constructed the full patch. A weaker version of `Co-Developed-by`.
> +
> + - Root-caused-by: names a person who diagnosed the root cause of a
> +   problem. This usually indicates significantly more work than a simple
> +   `Reported-by`.
> +
>   - Cc: the named person received a copy of the patch and had the
> opportunity to comment on it.
>  
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 3d4040322ae1..a1ab82e70b54 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -464,9 +464,11 @@ our $logFunctions = qr{(?x:
>  our $signature_tags = qr{(?xi:
>   Signed-off-by:|
>   Acked-by:|
> + Co-Developed-by:|
>   Tested-by:|
>   Reviewed-by:|
>   Reported-by:|
> + Root-caused-by:|
>   Suggested-by:|
>   To:|
>   Cc:
> 


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

Re: [PATCH 00/23] kconfig: move compiler capability tests to Kconfig

[Michael Ellerman]

Masahiro Yamada  writes:
>

>
> (Case 3)
> Compiler flag -foo is sensitive to endian-ness.
>
>
> config CC_NEEDS_BIG_ENDIAN
>   def_bool $(cc-option -mbig-endian) && CPU_BIG_ENDIAN
>
> config CC_NEEDS_LITTLE_ENDIAN
>   def_bool $(cc-option -mlittle-endian) && CPU_LITTLE_ENDIAN
>
> config CC_HAS_FOO
>  bool
>  default $(cc-option -mbig-endian -foo) if CC_NEEDS_BIG_ENDIAN
>  default $(cc-option -mlittle-endian -foo) if CC_NEEDS_LITTLE_ENDIAN
>  default $(cc-option -foo)

We may do something like this on powerpc, where we have 32/64-bit and
big/little endian (on 64-bit) and then some ABI options that we
set/unset depending on endian.

The above looks like it could work though.

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

Re: [PATCH] doc: process: Add "Root-caused-by" and "Suggested-by"

[Joe Perches]

On Wed, 2018-02-21 at 16:37 -0800, Kees Cook wrote:
> As recently pointed out by Linus, "Root-caused-by" is a good tag to include
> since it can indicate significantly more work than "just" a Reported-by.
> This adds it and "Suggested-by" (which was also missing) to the documented
> list of tags. Additionally updates checkpatch.pl to match the process docs.
[]
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
[]
> @@ -464,9 +464,11 @@ our $logFunctions = qr{(?x:
>  our $signature_tags = qr{(?xi:
>   Signed-off-by:|
>   Acked-by:|
> + Co-Developed-by:|
>   Tested-by:|
>   Reviewed-by:|
>   Reported-by:|
> + Root-caused-by:|
>   Suggested-by:|
>   To:|
>   Cc:

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

Re: [PATCH] doc: process: Add "Root-caused-by" and "Suggested-by"

[Kees Cook]

On Wed, Feb 21, 2018 at 6:13 PM, Joe Perches  wrote:
> On Wed, 2018-02-21 at 16:37 -0800, Kees Cook wrote:
>> As recently pointed out by Linus, "Root-caused-by" is a good tag to include
>> since it can indicate significantly more work than "just" a Reported-by.
>> This adds it and "Suggested-by" (which was also missing) to the documented
>> list of tags. Additionally updates checkpatch.pl to match the process docs.
> []
>> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> []
>> @@ -464,9 +464,11 @@ our $logFunctions = qr{(?x:
>>  our $signature_tags = qr{(?xi:
>>   Signed-off-by:|
>>   Acked-by:|
>> + Co-Developed-by:|
>>   Tested-by:|
>>   Reviewed-by:|
>>   Reported-by:|
>> + Root-caused-by:|
>>   Suggested-by:|
>>   To:|
>>   Cc:
>
> Patch does not match commit description

Hm? Why not? It's updating checkpatch.pl to match the process docs.
checkpatch.pl was missing Co-Developed-by and Root-caused-by, relative
to the process docs.

-Kees

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

Re: [PATCH 4/5 resend] documentation: firewire: add basic firewire.rst to driver-api

[Takashi Sakamoto]

Hi,

On Feb 22 2018 11:01, Randy Dunlap wrote:

On 02/21/2018 05:57 PM, Takashi Sakamoto wrote:

Hi,

Furthermore, 'scripts/checkpatch.pl' generates three warnings.

```
$ ./scripts/checkpatch.pl /tmp/patches/*
...

/tmp/patches/0004-documentation-firewire-add-basic-firewire.rst-to-dri.patch

WARNING: Use a single space after Cc:
#11:
Cc:    Stefan Richter 

WARNING: Use a single space after Cc:
#12:
Cc:    linux1394-de...@lists.sourceforge.net

WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#20:
new file mode 100644

total: 0 errors, 3 warnings, 40 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
   mechanically convert to the typical style using --fix or --fix-inplace.

/tmp/patches/0004-documentation-firewire-add-basic-firewire.rst-to-dri.patch 
has style problems, please review.
```

One of them can be ignored (adding a new file by a developer who is not in 
MAINTAINERS)

On Feb 22 2018 10:07, Randy Dunlap wrote:

From: Randy Dunlap 

Add a basic Firewire/IEEE 1394 driver API chapter to the Linux
kernel documentation.

Signed-off-by: Randy Dunlap 
Cc:    Stefan Richter 
Cc:    linux1394-de...@lists.sourceforge.net


However, the rest can be improved. I have never seen any case with multiple 
spaces between any tag and name. It's better to modify them to follow 
undocumented convention in Linux kernel development.


It's a tab, but I'll fix it.


Aha, I overlooked it.

Anyway, in next time, please send a cover letter together to aggregate 
each of your patch. It helps me to ask maintainers to add one 
'Reviewed-by' tag from me.



Thanks

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

Re: [PATCH 4/5 resend] documentation: firewire: add basic firewire.rst to driver-api

[Randy Dunlap]

On 02/21/2018 05:47 PM, Takashi Sakamoto wrote:
> Hi,
> 
> On Feb 22 2018 10:07, Randy Dunlap wrote:
>> From: Randy Dunlap 
>>
>> Add a basic Firewire/IEEE 1394 driver API chapter to the Linux
>> kernel documentation.
>>
>> Signed-off-by: Randy Dunlap 
>> Cc:    Stefan Richter 
>> Cc:    linux1394-de...@lists.sourceforge.net
>> ---
>>   Documentation/driver-api/firewire.rst |   33 
>>   Documentation/driver-api/index.rst    |    1
>>   2 files changed, 34 insertions(+)
>>
>> --- /dev/null
>> +++ linux-next-20180220/Documentation/driver-api/firewire.rst
>> @@ -0,0 +1,33 @@
>> +===
>> +Firewire (IEEE 1394) driver Interface Guide
>> +===
>> +
>> +Introduction and Overview
>> +=
>> +
>> +TBD
>> +
>> +Firewire char device data structures
>> +
>> +
>> +.. kernel-doc:: include/uapi/linux/firewire-cdev.h
>> +    :internal:
>> +
>> +Firewire device probing and sysfs interfaces
>> +
>> +
>> +.. kernel-doc:: drivers/firewire/core-device.c
>> +    :export:
>> +
>> +Firewire core transaction interfaces
>> +
>> +
>> +.. kernel-doc:: drivers/firewire/core-transaction.c
>> +    :export:
>> +
>> +Firewire Isochronous I/O interfaces
>> +===
>> +
>> +.. kernel-doc:: drivers/firewire/core-iso.c
>> +   :export:
>> +
> 
> A command of 'git am' generates below warning when applying this patch.
> 
> ```
> $ git am /tmp/patches/*
> Applying: documentation: firewire: add basic firewire.rst to driver-api
> .git/rebase-apply/patch:41: new blank line at EOF.
> +
> ```
> 
> It's better to remove the blank line.

OK, I fixed that one also (but git am shouldn't be so picky).


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

Re: [PATCH 5/5] documentation: firewire: add introduction/overview text

[Takashi Sakamoto]

Hi,

On Feb 22 2018 10:08, Randy Dunlap wrote:

From: Takashi Sakamoto 

Replace the Introduction section's TBD with some useful overview text.

Acked-by: Randy Dunlap 
Tested-by: Randy Dunlap 


- Not-yet-Signed-off-by: Takashi Sakamoto 
+ Signed-off-by: Takashi Sakamoto 


Signed-off-by: Randy Dunlap 
---
  Documentation/driver-api/firewire.rst |   18 +-
  1 file changed, 17 insertions(+), 1 deletion(-)

--- linux-next-20180220.orig/Documentation/driver-api/firewire.rst
+++ linux-next-20180220/Documentation/driver-api/firewire.rst
@@ -5,17 +5,33 @@ Firewire (IEEE 1394) driver Interface Gu
  Introduction and Overview
  =
  
-TBD

+The Linux FireWire subsystem adds some interfaces into the Linux system
+to use/maintain any resource on the IEEE 1394 bus.
+
+The main purpose of these interfaces is to access address space on each node
+on the IEEE 1394 bus by ISO/IEC 13213 (IEEE 1212) procedure, and to control
+isochronous resources on the bus by IEEE 1394 procedure.
+
+Two types of interfaces are added, according to consumers of the interface. A
+set of userspace interfaces is available via `firewire character devices`. A 
set
+of kernel interfaces is available via exported symbols in the `firewire-core`
+module.
  
  Firewire char device data structures

  
  
+.. include:: /ABI/stable/firewire-cdev

+:literal:
+
  .. kernel-doc:: include/uapi/linux/firewire-cdev.h
  :internal:
  
  Firewire device probing and sysfs interfaces

  
  
+.. include:: /ABI/stable/sysfs-bus-firewire

+:literal:
+
  .. kernel-doc:: drivers/firewire/core-device.c
  :export:


Thanks

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

[PATCH v12 04/22] selftests/vm: typecast the pkey register

[Ram Pai]

This is in preparation to accomadate a differing size register
across architectures.

cc: Dave Hansen 
cc: Florian Weimer 
Signed-off-by: Ram Pai 
---
 tools/testing/selftests/vm/pkey-helpers.h|   27 +-
 tools/testing/selftests/vm/protection_keys.c |   69 ++
 2 files changed, 51 insertions(+), 45 deletions(-)

diff --git a/tools/testing/selftests/vm/pkey-helpers.h 
b/tools/testing/selftests/vm/pkey-helpers.h
index c1bc761..b6c2133 100644
--- a/tools/testing/selftests/vm/pkey-helpers.h
+++ b/tools/testing/selftests/vm/pkey-helpers.h
@@ -18,6 +18,7 @@
 #define u16 uint16_t
 #define u32 uint32_t
 #define u64 uint64_t
+#define pkey_reg_t u32
 
 #ifdef __i386__
 #define SYS_mprotect_key 380
@@ -80,12 +81,12 @@ static inline void sigsafe_printf(const char *format, ...)
 #define dprintf3(args...) dprintf_level(3, args)
 #define dprintf4(args...) dprintf_level(4, args)
 
-extern unsigned int shadow_pkey_reg;
-static inline unsigned int __rdpkey_reg(void)
+extern pkey_reg_t shadow_pkey_reg;
+static inline pkey_reg_t __rdpkey_reg(void)
 {
unsigned int eax, edx;
unsigned int ecx = 0;
-   unsigned int pkey_reg;
+   pkey_reg_t pkey_reg;
 
asm volatile(".byte 0x0f,0x01,0xee\n\t"
 : "=a" (eax), "=d" (edx)
@@ -94,11 +95,11 @@ static inline unsigned int __rdpkey_reg(void)
return pkey_reg;
 }
 
-static inline unsigned int _rdpkey_reg(int line)
+static inline pkey_reg_t _rdpkey_reg(int line)
 {
-   unsigned int pkey_reg = __rdpkey_reg();
+   pkey_reg_t pkey_reg = __rdpkey_reg();
 
-   dprintf4("rdpkey_reg(line=%d) pkey_reg: %x shadow: %x\n",
+   dprintf4("rdpkey_reg(line=%d) pkey_reg: %016lx shadow: %016lx\n",
line, pkey_reg, shadow_pkey_reg);
assert(pkey_reg == shadow_pkey_reg);
 
@@ -107,11 +108,11 @@ static inline unsigned int _rdpkey_reg(int line)
 
 #define rdpkey_reg() _rdpkey_reg(__LINE__)
 
-static inline void __wrpkey_reg(unsigned int pkey_reg)
+static inline void __wrpkey_reg(pkey_reg_t pkey_reg)
 {
-   unsigned int eax = pkey_reg;
-   unsigned int ecx = 0;
-   unsigned int edx = 0;
+   pkey_reg_t eax = pkey_reg;
+   pkey_reg_t ecx = 0;
+   pkey_reg_t edx = 0;
 
dprintf4("%s() changing %08x to %08x\n", __func__,
__rdpkey_reg(), pkey_reg);
@@ -120,7 +121,7 @@ static inline void __wrpkey_reg(unsigned int pkey_reg)
assert(pkey_reg == __rdpkey_reg());
 }
 
-static inline void wrpkey_reg(unsigned int pkey_reg)
+static inline void wrpkey_reg(pkey_reg_t pkey_reg)
 {
dprintf4("%s() changing %08x to %08x\n", __func__,
__rdpkey_reg(), pkey_reg);
@@ -138,7 +139,7 @@ static inline void wrpkey_reg(unsigned int pkey_reg)
  */
 static inline void __pkey_access_allow(int pkey, int do_allow)
 {
-   unsigned int pkey_reg = rdpkey_reg();
+   pkey_reg_t pkey_reg = rdpkey_reg();
int bit = pkey * 2;
 
if (do_allow)
@@ -152,7 +153,7 @@ static inline void __pkey_access_allow(int pkey, int 
do_allow)
 
 static inline void __pkey_write_allow(int pkey, int do_allow_write)
 {
-   long pkey_reg = rdpkey_reg();
+   pkey_reg_t pkey_reg = rdpkey_reg();
int bit = pkey * 2 + 1;
 
if (do_allow_write)
diff --git a/tools/testing/selftests/vm/protection_keys.c 
b/tools/testing/selftests/vm/protection_keys.c
index 91bade4..3ef2569 100644
--- a/tools/testing/selftests/vm/protection_keys.c
+++ b/tools/testing/selftests/vm/protection_keys.c
@@ -48,7 +48,7 @@
 int iteration_nr = 1;
 int test_nr;
 
-unsigned int shadow_pkey_reg;
+pkey_reg_t shadow_pkey_reg;
 int dprint_in_signal;
 char dprint_in_signal_buffer[DPRINT_IN_SIGNAL_BUF_SIZE];
 
@@ -158,7 +158,7 @@ void dump_mem(void *dumpme, int len_bytes)
 
for (i = 0; i < len_bytes; i += sizeof(u64)) {
u64 *ptr = (u64 *)(c + i);
-   dprintf1("dump[%03d][@%p]: %016jx\n", i, ptr, *ptr);
+   dprintf1("dump[%03d][@%p]: %016lx\n", i, ptr, *ptr);
}
 }
 
@@ -186,7 +186,7 @@ void signal_handler(int signum, siginfo_t *si, void 
*vucontext)
int trapno;
unsigned long ip;
char *fpregs;
-   u32 *pkey_reg_ptr;
+   pkey_reg_t *pkey_reg_ptr;
u64 siginfo_pkey;
u32 *si_pkey_ptr;
int pkey_reg_offset;
@@ -194,7 +194,8 @@ void signal_handler(int signum, siginfo_t *si, void 
*vucontext)
 
dprint_in_signal = 1;
dprintf1("===SIGSEGV\n");
-   dprintf1("%s()::%d, pkey_reg: 0x%x shadow: %x\n", __func__, __LINE__,
+   dprintf1("%s()::%d, pkey_reg: 0x%016lx shadow: %016lx\n",
+   __func__, __LINE__,
__rdpkey_reg(), shadow_pkey_reg);
 
trapno = uctxt->uc_mcontext.gregs[REG_TRAPNO];
@@ -202,8 +203,9 @@ void signal_handler(int signum, siginfo_t *si, void 
*vucontext)
   

[PATCH v12 02/22] selftests/vm: rename all references to pkru to a generic name

[Ram Pai]

some pkru references are named to pkey_reg
and some prku references are renamed to pkey

cc: Dave Hansen 
cc: Florian Weimer 
Signed-off-by: Ram Pai 
---
 tools/testing/selftests/vm/pkey-helpers.h|   85 +-
 tools/testing/selftests/vm/protection_keys.c |  227 ++
 2 files changed, 164 insertions(+), 148 deletions(-)

diff --git a/tools/testing/selftests/vm/pkey-helpers.h 
b/tools/testing/selftests/vm/pkey-helpers.h
index b3cb767..a568166 100644
--- a/tools/testing/selftests/vm/pkey-helpers.h
+++ b/tools/testing/selftests/vm/pkey-helpers.h
@@ -14,7 +14,7 @@
 #include 
 
 #define NR_PKEYS 16
-#define PKRU_BITS_PER_PKEY 2
+#define PKEY_BITS_PER_PKEY 2
 
 #ifndef DEBUG_LEVEL
 #define DEBUG_LEVEL 0
@@ -57,85 +57,88 @@ static inline void sigsafe_printf(const char *format, ...)
 #define dprintf3(args...) dprintf_level(3, args)
 #define dprintf4(args...) dprintf_level(4, args)
 
-extern unsigned int shadow_pkru;
-static inline unsigned int __rdpkru(void)
+extern unsigned int shadow_pkey_reg;
+static inline unsigned int __rdpkey_reg(void)
 {
unsigned int eax, edx;
unsigned int ecx = 0;
-   unsigned int pkru;
+   unsigned int pkey_reg;
 
asm volatile(".byte 0x0f,0x01,0xee\n\t"
 : "=a" (eax), "=d" (edx)
 : "c" (ecx));
-   pkru = eax;
-   return pkru;
+   pkey_reg = eax;
+   return pkey_reg;
 }
 
-static inline unsigned int _rdpkru(int line)
+static inline unsigned int _rdpkey_reg(int line)
 {
-   unsigned int pkru = __rdpkru();
+   unsigned int pkey_reg = __rdpkey_reg();
 
-   dprintf4("rdpkru(line=%d) pkru: %x shadow: %x\n",
-   line, pkru, shadow_pkru);
-   assert(pkru == shadow_pkru);
+   dprintf4("rdpkey_reg(line=%d) pkey_reg: %x shadow: %x\n",
+   line, pkey_reg, shadow_pkey_reg);
+   assert(pkey_reg == shadow_pkey_reg);
 
-   return pkru;
+   return pkey_reg;
 }
 
-#define rdpkru() _rdpkru(__LINE__)
+#define rdpkey_reg() _rdpkey_reg(__LINE__)
 
-static inline void __wrpkru(unsigned int pkru)
+static inline void __wrpkey_reg(unsigned int pkey_reg)
 {
-   unsigned int eax = pkru;
+   unsigned int eax = pkey_reg;
unsigned int ecx = 0;
unsigned int edx = 0;
 
-   dprintf4("%s() changing %08x to %08x\n", __func__, __rdpkru(), pkru);
+   dprintf4("%s() changing %08x to %08x\n", __func__,
+   __rdpkey_reg(), pkey_reg);
asm volatile(".byte 0x0f,0x01,0xef\n\t"
 : : "a" (eax), "c" (ecx), "d" (edx));
-   assert(pkru == __rdpkru());
+   assert(pkey_reg == __rdpkey_reg());
 }
 
-static inline void wrpkru(unsigned int pkru)
+static inline void wrpkey_reg(unsigned int pkey_reg)
 {
-   dprintf4("%s() changing %08x to %08x\n", __func__, __rdpkru(), pkru);
+   dprintf4("%s() changing %08x to %08x\n", __func__,
+   __rdpkey_reg(), pkey_reg);
/* will do the shadow check for us: */
-   rdpkru();
-   __wrpkru(pkru);
-   shadow_pkru = pkru;
-   dprintf4("%s(%08x) pkru: %08x\n", __func__, pkru, __rdpkru());
+   rdpkey_reg();
+   __wrpkey_reg(pkey_reg);
+   shadow_pkey_reg = pkey_reg;
+   dprintf4("%s(%08x) pkey_reg: %08x\n", __func__,
+   pkey_reg, __rdpkey_reg());
 }
 
 /*
  * These are technically racy. since something could
- * change PKRU between the read and the write.
+ * change PKEY register between the read and the write.
  */
 static inline void __pkey_access_allow(int pkey, int do_allow)
 {
-   unsigned int pkru = rdpkru();
+   unsigned int pkey_reg = rdpkey_reg();
int bit = pkey * 2;
 
if (do_allow)
-   pkru &= (1<

[PATCH v12 05/22] selftests/vm: generic function to handle shadow key register

[Ram Pai]

helper functions to handler shadow pkey register

cc: Dave Hansen 
cc: Florian Weimer 
Signed-off-by: Ram Pai 
---
 tools/testing/selftests/vm/pkey-helpers.h|   27 
 tools/testing/selftests/vm/protection_keys.c |   34 -
 2 files changed, 49 insertions(+), 12 deletions(-)

diff --git a/tools/testing/selftests/vm/pkey-helpers.h 
b/tools/testing/selftests/vm/pkey-helpers.h
index b6c2133..7c979ad 100644
--- a/tools/testing/selftests/vm/pkey-helpers.h
+++ b/tools/testing/selftests/vm/pkey-helpers.h
@@ -44,6 +44,33 @@
 #define DEBUG_LEVEL 0
 #endif
 #define DPRINT_IN_SIGNAL_BUF_SIZE 4096
+
+static inline u32 pkey_to_shift(int pkey)
+{
+   return pkey * PKEY_BITS_PER_PKEY;
+}
+
+static inline pkey_reg_t reset_bits(int pkey, pkey_reg_t bits)
+{
+   u32 shift = pkey_to_shift(pkey);
+
+   return ~(bits << shift);
+}
+
+static inline pkey_reg_t left_shift_bits(int pkey, pkey_reg_t bits)
+{
+   u32 shift = pkey_to_shift(pkey);
+
+   return (bits << shift);
+}
+
+static inline pkey_reg_t right_shift_bits(int pkey, pkey_reg_t bits)
+{
+   u32 shift = pkey_to_shift(pkey);
+
+   return (bits >> shift);
+}
+
 extern int dprint_in_signal;
 extern char dprint_in_signal_buffer[DPRINT_IN_SIGNAL_BUF_SIZE];
 static inline void sigsafe_printf(const char *format, ...)
diff --git a/tools/testing/selftests/vm/protection_keys.c 
b/tools/testing/selftests/vm/protection_keys.c
index 3ef2569..83216c5 100644
--- a/tools/testing/selftests/vm/protection_keys.c
+++ b/tools/testing/selftests/vm/protection_keys.c
@@ -374,7 +374,7 @@ u32 pkey_get(int pkey, unsigned long flags)
__func__, pkey, flags, 0, 0);
dprintf2("%s() raw pkey_reg: %x\n", __func__, pkey_reg);
 
-   shifted_pkey_reg = (pkey_reg >> (pkey * PKEY_BITS_PER_PKEY));
+   shifted_pkey_reg = right_shift_bits(pkey, pkey_reg);
dprintf2("%s() shifted_pkey_reg: %x\n", __func__, shifted_pkey_reg);
masked_pkey_reg = shifted_pkey_reg & mask;
dprintf2("%s() masked  pkey_reg: %x\n", __func__, masked_pkey_reg);
@@ -397,9 +397,9 @@ int pkey_set(int pkey, unsigned long rights, unsigned long 
flags)
/* copy old pkey_reg */
new_pkey_reg = old_pkey_reg;
/* mask out bits from pkey in old value: */
-   new_pkey_reg &= ~(mask << (pkey * PKEY_BITS_PER_PKEY));
+   new_pkey_reg &= reset_bits(pkey, mask);
/* OR in new bits for pkey: */
-   new_pkey_reg |= (rights << (pkey * PKEY_BITS_PER_PKEY));
+   new_pkey_reg |= left_shift_bits(pkey, rights);
 
__wrpkey_reg(new_pkey_reg);
 
@@ -430,7 +430,7 @@ void pkey_disable_set(int pkey, int flags)
ret = pkey_set(pkey, pkey_rights, syscall_flags);
assert(!ret);
/*pkey_reg and flags have the same format */
-   shadow_pkey_reg |= flags << (pkey * 2);
+   shadow_pkey_reg |= left_shift_bits(pkey, flags);
dprintf1("%s(%d) shadow: 0x%016lx\n",
__func__, pkey, shadow_pkey_reg);
 
@@ -465,7 +465,7 @@ void pkey_disable_clear(int pkey, int flags)
 
ret = pkey_set(pkey, pkey_rights, 0);
/* pkey_reg and flags have the same format */
-   shadow_pkey_reg &= ~(flags << (pkey * 2));
+   shadow_pkey_reg &= reset_bits(pkey, flags);
pkey_assert(ret >= 0);
 
pkey_rights = pkey_get(pkey, syscall_flags);
@@ -523,6 +523,21 @@ int sys_pkey_alloc(unsigned long flags, unsigned long 
init_val)
return ret;
 }
 
+void pkey_setup_shadow(void)
+{
+   shadow_pkey_reg = __rdpkey_reg();
+}
+
+void pkey_reset_shadow(u32 key)
+{
+   shadow_pkey_reg &= reset_bits(key, 0x3);
+}
+
+void pkey_set_shadow(u32 key, u64 init_val)
+{
+   shadow_pkey_reg |=  left_shift_bits(key, init_val);
+}
+
 int alloc_pkey(void)
 {
int ret;
@@ -540,7 +555,7 @@ int alloc_pkey(void)
shadow_pkey_reg);
if (ret) {
/* clear both the bits: */
-   shadow_pkey_reg &= ~(0x3  << (ret * 2));
+   pkey_reset_shadow(ret);
dprintf4("%s()::%d, ret: %d pkey_reg: 0x%016lx "
"shadow: 0x%016lx\n",
__func__,
@@ -550,7 +565,7 @@ int alloc_pkey(void)
 * move the new state in from init_val
 * (remember, we cheated and init_val == pkey_reg format)
 */
-   shadow_pkey_reg |=  (init_val << (ret * 2));
+   pkey_set_shadow(ret, init_val);
}
dprintf4("%s()::%d, ret: %d pkey_reg: 0x%016lx shadow: 0x%016lx\n",
__func__, __LINE__, ret, __rdpkey_reg(),
@@ -1322,11 +1337,6 @@ void run_tests_once(void)
iteration_nr++;
 }
 
-void pkey_setup_shadow(void)
-{
-   shadow_pkey_reg = __rdpkey_reg();
-}
-
 int main(void)
 {
int nr_iterations = 22;
-- 
1.7.1

--
To unsubscribe from this list: send the line 

Re: [PATCH 4/5 resend] documentation: firewire: add basic firewire.rst to driver-api

[Randy Dunlap]

On 02/21/2018 05:57 PM, Takashi Sakamoto wrote:
> Hi,
> 
> Furthermore, 'scripts/checkpatch.pl' generates three warnings.
> 
> ```
> $ ./scripts/checkpatch.pl /tmp/patches/*
> ...
> 
> /tmp/patches/0004-documentation-firewire-add-basic-firewire.rst-to-dri.patch
> 
> WARNING: Use a single space after Cc:
> #11:
> Cc:    Stefan Richter 
> 
> WARNING: Use a single space after Cc:
> #12:
> Cc:    linux1394-de...@lists.sourceforge.net
> 
> WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
> #20:
> new file mode 100644
> 
> total: 0 errors, 3 warnings, 40 lines checked
> 
> NOTE: For some of the reported defects, checkpatch may be able to
>   mechanically convert to the typical style using --fix or --fix-inplace.
> 
> /tmp/patches/0004-documentation-firewire-add-basic-firewire.rst-to-dri.patch 
> has style problems, please review.
> ```
> 
> One of them can be ignored (adding a new file by a developer who is not in 
> MAINTAINERS)
> 
> On Feb 22 2018 10:07, Randy Dunlap wrote:
>> From: Randy Dunlap 
>>
>> Add a basic Firewire/IEEE 1394 driver API chapter to the Linux
>> kernel documentation.
>>
>> Signed-off-by: Randy Dunlap 
>> Cc:    Stefan Richter 
>> Cc:    linux1394-de...@lists.sourceforge.net
> 
> However, the rest can be improved. I have never seen any case with multiple 
> spaces between any tag and name. It's better to modify them to follow 
> undocumented convention in Linux kernel development.

It's a tab, but I'll fix it.

Thanks.


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

[PATCH v12 01/22] selftests/x86: Move protecton key selftest to arch neutral directory

[Ram Pai]

cc: Dave Hansen 
cc: Florian Weimer 
Signed-off-by: Ram Pai 
---
 tools/testing/selftests/vm/Makefile   |1 +
 tools/testing/selftests/vm/pkey-helpers.h |  223 
 tools/testing/selftests/vm/protection_keys.c  | 1407 +
 tools/testing/selftests/x86/Makefile  |2 +-
 tools/testing/selftests/x86/pkey-helpers.h|  223 
 tools/testing/selftests/x86/protection_keys.c | 1407 -
 6 files changed, 1632 insertions(+), 1631 deletions(-)
 create mode 100644 tools/testing/selftests/vm/pkey-helpers.h
 create mode 100644 tools/testing/selftests/vm/protection_keys.c
 delete mode 100644 tools/testing/selftests/x86/pkey-helpers.h
 delete mode 100644 tools/testing/selftests/x86/protection_keys.c

diff --git a/tools/testing/selftests/vm/Makefile 
b/tools/testing/selftests/vm/Makefile
index fdefa22..9788a58 100644
--- a/tools/testing/selftests/vm/Makefile
+++ b/tools/testing/selftests/vm/Makefile
@@ -20,6 +20,7 @@ TEST_GEN_FILES += transhuge-stress
 TEST_GEN_FILES += userfaultfd
 TEST_GEN_FILES += va_128TBswitch
 TEST_GEN_FILES += virtual_address_range
+TEST_GEN_FILES += protection_keys
 
 TEST_PROGS := run_vmtests
 
diff --git a/tools/testing/selftests/vm/pkey-helpers.h 
b/tools/testing/selftests/vm/pkey-helpers.h
new file mode 100644
index 000..b3cb767
--- /dev/null
+++ b/tools/testing/selftests/vm/pkey-helpers.h
@@ -0,0 +1,223 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _PKEYS_HELPER_H
+#define _PKEYS_HELPER_H
+#define _GNU_SOURCE
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define NR_PKEYS 16
+#define PKRU_BITS_PER_PKEY 2
+
+#ifndef DEBUG_LEVEL
+#define DEBUG_LEVEL 0
+#endif
+#define DPRINT_IN_SIGNAL_BUF_SIZE 4096
+extern int dprint_in_signal;
+extern char dprint_in_signal_buffer[DPRINT_IN_SIGNAL_BUF_SIZE];
+static inline void sigsafe_printf(const char *format, ...)
+{
+   va_list ap;
+
+   va_start(ap, format);
+   if (!dprint_in_signal) {
+   vprintf(format, ap);
+   } else {
+   int ret;
+   int len = vsnprintf(dprint_in_signal_buffer,
+   DPRINT_IN_SIGNAL_BUF_SIZE,
+   format, ap);
+   /*
+* len is amount that would have been printed,
+* but actual write is truncated at BUF_SIZE.
+*/
+   if (len > DPRINT_IN_SIGNAL_BUF_SIZE)
+   len = DPRINT_IN_SIGNAL_BUF_SIZE;
+   ret = write(1, dprint_in_signal_buffer, len);
+   if (ret < 0)
+   abort();
+   }
+   va_end(ap);
+}
+#define dprintf_level(level, args...) do { \
+   if (level <= DEBUG_LEVEL)   \
+   sigsafe_printf(args);   \
+   fflush(NULL);   \
+} while (0)
+#define dprintf0(args...) dprintf_level(0, args)
+#define dprintf1(args...) dprintf_level(1, args)
+#define dprintf2(args...) dprintf_level(2, args)
+#define dprintf3(args...) dprintf_level(3, args)
+#define dprintf4(args...) dprintf_level(4, args)
+
+extern unsigned int shadow_pkru;
+static inline unsigned int __rdpkru(void)
+{
+   unsigned int eax, edx;
+   unsigned int ecx = 0;
+   unsigned int pkru;
+
+   asm volatile(".byte 0x0f,0x01,0xee\n\t"
+: "=a" (eax), "=d" (edx)
+: "c" (ecx));
+   pkru = eax;
+   return pkru;
+}
+
+static inline unsigned int _rdpkru(int line)
+{
+   unsigned int pkru = __rdpkru();
+
+   dprintf4("rdpkru(line=%d) pkru: %x shadow: %x\n",
+   line, pkru, shadow_pkru);
+   assert(pkru == shadow_pkru);
+
+   return pkru;
+}
+
+#define rdpkru() _rdpkru(__LINE__)
+
+static inline void __wrpkru(unsigned int pkru)
+{
+   unsigned int eax = pkru;
+   unsigned int ecx = 0;
+   unsigned int edx = 0;
+
+   dprintf4("%s() changing %08x to %08x\n", __func__, __rdpkru(), pkru);
+   asm volatile(".byte 0x0f,0x01,0xef\n\t"
+: : "a" (eax), "c" (ecx), "d" (edx));
+   assert(pkru == __rdpkru());
+}
+
+static inline void wrpkru(unsigned int pkru)
+{
+   dprintf4("%s() changing %08x to %08x\n", __func__, __rdpkru(), pkru);
+   /* will do the shadow check for us: */
+   rdpkru();
+   __wrpkru(pkru);
+   shadow_pkru = pkru;
+   dprintf4("%s(%08x) pkru: %08x\n", __func__, pkru, __rdpkru());
+}
+
+/*
+ * These are technically racy. since something could
+ * change PKRU between the read and the write.
+ */
+static inline void __pkey_access_allow(int pkey, int do_allow)
+{
+   unsigned int pkru = rdpkru();
+   int bit = pkey * 2;
+
+   if (do_allow)
+   pkru &= (1<

[PATCH v12 07/22] selftests/vm: fixed bugs in pkey_disable_clear()

[Ram Pai]

instead of clearing the bits, pkey_disable_clear() was setting
the bits. Fixed it.

Also fixed a wrong assertion in that function. When bits are
cleared, the resulting bit value will be less than the original.

cc: Dave Hansen 
cc: Florian Weimer 
Signed-off-by: Ram Pai 
---
 tools/testing/selftests/vm/protection_keys.c |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/vm/protection_keys.c 
b/tools/testing/selftests/vm/protection_keys.c
index 0109388..ca54a95 100644
--- a/tools/testing/selftests/vm/protection_keys.c
+++ b/tools/testing/selftests/vm/protection_keys.c
@@ -461,7 +461,7 @@ void pkey_disable_clear(int pkey, int flags)
pkey, pkey, pkey_rights);
pkey_assert(pkey_rights >= 0);
 
-   pkey_rights |= flags;
+   pkey_rights &= ~flags;
 
ret = pkey_set(pkey, pkey_rights, 0);
/* pkey_reg and flags have the same format */
@@ -475,7 +475,7 @@ void pkey_disable_clear(int pkey, int flags)
dprintf1("%s(%d) pkey_reg: 0x%016lx\n", __func__,
pkey, rdpkey_reg());
if (flags)
-   assert(rdpkey_reg() > orig_pkey_reg);
+   assert(rdpkey_reg() < orig_pkey_reg);
 }
 
 void pkey_write_allow(int pkey)
-- 
1.7.1

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

[PATCH v12 06/22] selftests/vm: fix the wrong assert in pkey_disable_set()

[Ram Pai]

If the flag is 0, no bits will be set. Hence we cant expect
the resulting bitmap to have a higher value than what it
was earlier.

cc: Dave Hansen 
cc: Florian Weimer 
Signed-off-by: Ram Pai 
---
 tools/testing/selftests/vm/protection_keys.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/tools/testing/selftests/vm/protection_keys.c 
b/tools/testing/selftests/vm/protection_keys.c
index 83216c5..0109388 100644
--- a/tools/testing/selftests/vm/protection_keys.c
+++ b/tools/testing/selftests/vm/protection_keys.c
@@ -443,7 +443,7 @@ void pkey_disable_set(int pkey, int flags)
dprintf1("%s(%d) pkey_reg: 0x%lx\n",
__func__, pkey, rdpkey_reg());
if (flags)
-   pkey_assert(rdpkey_reg() > orig_pkey_reg);
+   pkey_assert(rdpkey_reg() >= orig_pkey_reg);
dprintf1("END<---%s(%d, 0x%x)\n", __func__,
pkey, flags);
 }
-- 
1.7.1

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

[PATCH v12 11/22] selftests/vm: pkey register should match shadow pkey

[Ram Pai]

expected_pkey_fault() is comparing the contents of pkey
register with 0. This may not be true all the time. There
could be bits set by default by the architecture
which can never be changed. Hence compare the value against
shadow pkey register, which is supposed to track the bits
accurately all throughout

cc: Dave Hansen 
cc: Florian Weimer 
Signed-off-by: Ram Pai 
---
 tools/testing/selftests/vm/protection_keys.c |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/vm/protection_keys.c 
b/tools/testing/selftests/vm/protection_keys.c
index 254b66d..6054093 100644
--- a/tools/testing/selftests/vm/protection_keys.c
+++ b/tools/testing/selftests/vm/protection_keys.c
@@ -926,10 +926,10 @@ void expected_pkey_fault(int pkey)
pkey_assert(last_pkey_faults + 1 == pkey_faults);
pkey_assert(last_si_pkey == pkey);
/*
-* The signal handler shold have cleared out PKEY register to let the
+* The signal handler shold have cleared out pkey-register to let the
 * test program continue.  We now have to restore it.
 */
-   if (__rdpkey_reg() != 0)
+   if (__rdpkey_reg() != shadow_pkey_reg)
pkey_assert(0);
 
__wrpkey_reg(shadow_pkey_reg);
-- 
1.7.1

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

[PATCH v12 09/22] selftests/vm: fix alloc_random_pkey() to make it really random

[Ram Pai]

alloc_random_pkey() was allocating the same pkey every time.
Not all pkeys were geting tested. fixed it.

cc: Dave Hansen 
cc: Florian Weimer 
Signed-off-by: Ram Pai 
---
 tools/testing/selftests/vm/protection_keys.c |   10 +++---
 1 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/vm/protection_keys.c 
b/tools/testing/selftests/vm/protection_keys.c
index aaf9f09..2e4b636 100644
--- a/tools/testing/selftests/vm/protection_keys.c
+++ b/tools/testing/selftests/vm/protection_keys.c
@@ -24,6 +24,7 @@
 #define _GNU_SOURCE
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -602,13 +603,15 @@ int alloc_random_pkey(void)
int alloced_pkeys[NR_PKEYS];
int nr_alloced = 0;
int random_index;
+
memset(alloced_pkeys, 0, sizeof(alloced_pkeys));
+   srand((unsigned int)time(NULL));
 
/* allocate every possible key and make a note of which ones we got */
max_nr_pkey_allocs = NR_PKEYS;
-   max_nr_pkey_allocs = 1;
for (i = 0; i < max_nr_pkey_allocs; i++) {
int new_pkey = alloc_pkey();
+
if (new_pkey < 0)
break;
alloced_pkeys[nr_alloced++] = new_pkey;
@@ -624,13 +627,14 @@ int alloc_random_pkey(void)
/* go through the allocated ones that we did not want and free them */
for (i = 0; i < nr_alloced; i++) {
int free_ret;
+
if (!alloced_pkeys[i])
continue;
free_ret = sys_pkey_free(alloced_pkeys[i]);
pkey_assert(!free_ret);
}
-   dprintf1("%s()::%d, ret: %d pkey_reg: 0x%x shadow: 0x%x\n", __func__,
-   __LINE__, ret, __rdpkey_reg(), shadow_pkey_reg);
+   dprintf1("%s()::%d, ret: %d pkey_reg: 0x%x shadow: 0x%016lx\n",
+   __func__, __LINE__, ret, __rdpkey_reg(), shadow_pkey_reg);
return ret;
 }
 
-- 
1.7.1

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

Re: [PATCH 4/5 resend] documentation: firewire: add basic firewire.rst to driver-api

[Takashi Sakamoto]

Hi,

Furthermore, 'scripts/checkpatch.pl' generates three warnings.

```
$ ./scripts/checkpatch.pl /tmp/patches/*
...

/tmp/patches/0004-documentation-firewire-add-basic-firewire.rst-to-dri.patch

WARNING: Use a single space after Cc:
#11:
Cc: Stefan Richter 

WARNING: Use a single space after Cc:
#12:
Cc: linux1394-de...@lists.sourceforge.net

WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#20:
new file mode 100644

total: 0 errors, 3 warnings, 40 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
  mechanically convert to the typical style using --fix or 
--fix-inplace.


/tmp/patches/0004-documentation-firewire-add-basic-firewire.rst-to-dri.patch 
has style problems, please review.

```

One of them can be ignored (adding a new file by a developer who is not 
in MAINTAINERS)


On Feb 22 2018 10:07, Randy Dunlap wrote:

From: Randy Dunlap 

Add a basic Firewire/IEEE 1394 driver API chapter to the Linux
kernel documentation.

Signed-off-by: Randy Dunlap 
Cc: Stefan Richter 
Cc: linux1394-de...@lists.sourceforge.net


However, the rest can be improved. I have never seen any case with 
multiple spaces between any tag and name. It's better to modify them to 
follow undocumented convention in Linux kernel development.



---
  Documentation/driver-api/firewire.rst |   33 
  Documentation/driver-api/index.rst|1
  2 files changed, 34 insertions(+)

--- /dev/null
+++ linux-next-20180220/Documentation/driver-api/firewire.rst
@@ -0,0 +1,33 @@
+===
+Firewire (IEEE 1394) driver Interface Guide
+===
+
+Introduction and Overview
+=
+
+TBD
+
+Firewire char device data structures
+
+
+.. kernel-doc:: include/uapi/linux/firewire-cdev.h
+:internal:
+
+Firewire device probing and sysfs interfaces
+
+
+.. kernel-doc:: drivers/firewire/core-device.c
+:export:
+
+Firewire core transaction interfaces
+
+
+.. kernel-doc:: drivers/firewire/core-transaction.c
+:export:
+
+Firewire Isochronous I/O interfaces
+===
+
+.. kernel-doc:: drivers/firewire/core-iso.c
+   :export:
+
--- linux-next-20180220.orig/Documentation/driver-api/index.rst
+++ linux-next-20180220/Documentation/driver-api/index.rst
@@ -27,6 +27,7 @@ available subsections can be seen below.
 iio/index
 input
 usb/index
+   firewire
 pci
 spi
 i2c



Thanks

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

[PATCH v12 13/22] selftests/vm: powerpc implementation for generic abstraction

[Ram Pai]

Introduce powerpc implementation for the various
abstractions.

cc: Dave Hansen 
cc: Florian Weimer 
Signed-off-by: Ram Pai 
---
 tools/testing/selftests/vm/pkey-helpers.h|  128 ++
 tools/testing/selftests/vm/protection_keys.c |   42 +---
 2 files changed, 136 insertions(+), 34 deletions(-)

diff --git a/tools/testing/selftests/vm/pkey-helpers.h 
b/tools/testing/selftests/vm/pkey-helpers.h
index c8f5739..c47aead 100644
--- a/tools/testing/selftests/vm/pkey-helpers.h
+++ b/tools/testing/selftests/vm/pkey-helpers.h
@@ -18,27 +18,51 @@
 #define u16 uint16_t
 #define u32 uint32_t
 #define u64 uint64_t
-#define pkey_reg_t u32
 
-#ifdef __i386__
+#if defined(__i386__) || defined(__x86_64__) /* arch */
+
+#ifdef __i386__ /* arch */
 #define SYS_mprotect_key 380
-#define SYS_pkey_alloc  381
-#define SYS_pkey_free   382
+#define SYS_pkey_alloc   381
+#define SYS_pkey_free382
 #define REG_IP_IDX REG_EIP
-#define si_pkey_offset 0x14
-#else
+#elif __x86_64__
 #define SYS_mprotect_key 329
-#define SYS_pkey_alloc  330
-#define SYS_pkey_free   331
+#define SYS_pkey_alloc   330
+#define SYS_pkey_free331
 #define REG_IP_IDX REG_RIP
-#define si_pkey_offset 0x20
-#endif
+#endif /* __x86_64__ */
+
+#define NR_PKEYS   16
+#define NR_RESERVED_PKEYS  1
+#define PKEY_BITS_PER_PKEY 2
+#define PKEY_DISABLE_ACCESS0x1
+#define PKEY_DISABLE_WRITE 0x2
+#define HPAGE_SIZE (1UL<<21)
+#define pkey_reg_t u32
 
-#define NR_PKEYS 16
-#define PKEY_BITS_PER_PKEY 2
-#define PKEY_DISABLE_ACCESS0x1
-#define PKEY_DISABLE_WRITE 0x2
-#define HPAGE_SIZE (1UL<<21)
+#elif __powerpc64__ /* arch */
+
+#define SYS_mprotect_key 386
+#define SYS_pkey_alloc  384
+#define SYS_pkey_free   385
+#define REG_IP_IDX PT_NIP
+#define REG_TRAPNO PT_TRAP
+#define gregs gp_regs
+#define fpregs fp_regs
+
+#define NR_PKEYS   32
+#define NR_RESERVED_PKEYS_4K   26
+#define NR_RESERVED_PKEYS_64K  3
+#define PKEY_BITS_PER_PKEY 2
+#define PKEY_DISABLE_ACCESS0x3  /* disable read and write */
+#define PKEY_DISABLE_WRITE 0x2
+#define HPAGE_SIZE (1UL<<24)
+#define pkey_reg_t u64
+
+#else /* arch */
+   NOT SUPPORTED
+#endif /* arch */
 
 #ifndef DEBUG_LEVEL
 #define DEBUG_LEVEL 0
@@ -47,7 +71,11 @@
 
 static inline u32 pkey_to_shift(int pkey)
 {
+#if defined(__i386__) || defined(__x86_64__) /* arch */
return pkey * PKEY_BITS_PER_PKEY;
+#elif __powerpc64__ /* arch */
+   return (NR_PKEYS - pkey - 1) * PKEY_BITS_PER_PKEY;
+#endif /* arch */
 }
 
 static inline pkey_reg_t reset_bits(int pkey, pkey_reg_t bits)
@@ -111,6 +139,7 @@ static inline void sigsafe_printf(const char *format, ...)
 extern pkey_reg_t shadow_pkey_reg;
 static inline pkey_reg_t __rdpkey_reg(void)
 {
+#if defined(__i386__) || defined(__x86_64__) /* arch */
unsigned int eax, edx;
unsigned int ecx = 0;
pkey_reg_t pkey_reg;
@@ -118,7 +147,13 @@ static inline pkey_reg_t __rdpkey_reg(void)
asm volatile(".byte 0x0f,0x01,0xee\n\t"
 : "=a" (eax), "=d" (edx)
 : "c" (ecx));
-   pkey_reg = eax;
+#elif __powerpc64__ /* arch */
+   pkey_reg_t eax;
+   pkey_reg_t pkey_reg;
+
+   asm volatile("mfspr %0, 0xd" : "=r" ((pkey_reg_t)(eax)));
+#endif /* arch */
+   pkey_reg = (pkey_reg_t)eax;
return pkey_reg;
 }
 
@@ -138,6 +173,7 @@ static inline pkey_reg_t _rdpkey_reg(int line)
 static inline void __wrpkey_reg(pkey_reg_t pkey_reg)
 {
pkey_reg_t eax = pkey_reg;
+#if defined(__i386__) || defined(__x86_64__) /* arch */
pkey_reg_t ecx = 0;
pkey_reg_t edx = 0;
 
@@ -146,6 +182,14 @@ static inline void __wrpkey_reg(pkey_reg_t pkey_reg)
asm volatile(".byte 0x0f,0x01,0xef\n\t"
 : : "a" (eax), "c" (ecx), "d" (edx));
assert(pkey_reg == __rdpkey_reg());
+
+#elif __powerpc64__ /* arch */
+   dprintf4("%s() changing %llx to %llx\n",
+__func__, __rdpkey_reg(), pkey_reg);
+   asm volatile("mtspr 0xd, %0" : : "r" ((unsigned long)(eax)) : "memory");
+#endif /* arch */
+   dprintf4("%s() pkey register after changing %016lx to %016lx\n",
+__func__, __rdpkey_reg(), pkey_reg);
 }
 
 static inline void wrpkey_reg(pkey_reg_t pkey_reg)
@@ -192,6 +236,8 @@ static inline void __pkey_write_allow(int pkey, int 
do_allow_write)
dprintf4("pkey_reg now: %08x\n", rdpkey_reg());
 }
 
+#if defined(__i386__) || defined(__x86_64__) /* arch */
+
 #define PAGE_SIZE 4096
 #define MB (1<<20)
 
@@ -274,8 +320,18 @@ static inline void __page_o_noops(void)
/* 8-bytes of instruction * 512 bytes = 1 page */
asm(".rept 512 ; nopl 0x7eee(%eax) ; .endr");
 }
+#elif __powerpc64__ /* arch */
 
-#endif /* _PKEYS_HELPER_H */
+#define PAGE_SIZE (0x1UL << 16)
+static inline int cpu_has_pku(void)
+{
+   return 1;
+}
+
+/* 8-bytes 

[PATCH v12 14/22] selftests/vm: clear the bits in shadow reg when a pkey is freed.

[Ram Pai]

When a key is freed, the  key  is  no  more  effective.
Clear the bits corresponding to the pkey in the shadow
register. Otherwise  it  will carry some spurious bits
which can trigger false-positive asserts.

cc: Dave Hansen 
cc: Florian Weimer 
Signed-off-by: Ram Pai 
---
 tools/testing/selftests/vm/protection_keys.c |3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/tools/testing/selftests/vm/protection_keys.c 
b/tools/testing/selftests/vm/protection_keys.c
index c4c73e6..e82bd88 100644
--- a/tools/testing/selftests/vm/protection_keys.c
+++ b/tools/testing/selftests/vm/protection_keys.c
@@ -586,7 +586,8 @@ int sys_pkey_free(unsigned long pkey)
int ret = syscall(SYS_pkey_free, pkey);
 
if (!ret)
-   shadow_pkey_reg &= reset_bits(pkey, PKEY_DISABLE_ACCESS);
+   shadow_pkey_reg &= reset_bits(pkey,
+   PKEY_DISABLE_ACCESS | PKEY_DISABLE_WRITE);
dprintf1("%s(pkey=%ld) syscall ret: %d\n", __func__, pkey, ret);
return ret;
 }
-- 
1.7.1

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

[PATCH v12 15/22] selftests/vm: powerpc implementation to check support for pkey

[Ram Pai]

pkey subsystem is supported if the hardware and kernel has support.
We determine that by checking if allocation of a key succeeds or not.

cc: Dave Hansen 
cc: Florian Weimer 
Signed-off-by: Ram Pai 
---
 tools/testing/selftests/vm/pkey-helpers.h|   22 --
 tools/testing/selftests/vm/protection_keys.c |9 +
 2 files changed, 21 insertions(+), 10 deletions(-)

diff --git a/tools/testing/selftests/vm/pkey-helpers.h 
b/tools/testing/selftests/vm/pkey-helpers.h
index c47aead..88ef58f 100644
--- a/tools/testing/selftests/vm/pkey-helpers.h
+++ b/tools/testing/selftests/vm/pkey-helpers.h
@@ -258,7 +258,7 @@ static inline void __cpuid(unsigned int *eax, unsigned int 
*ebx,
 #define X86_FEATURE_PKU(1<<3) /* Protection Keys for Userspace */
 #define X86_FEATURE_OSPKE  (1<<4) /* OS Protection Keys Enable */
 
-static inline int cpu_has_pku(void)
+static inline bool is_pkey_supported(void)
 {
unsigned int eax;
unsigned int ebx;
@@ -271,13 +271,13 @@ static inline int cpu_has_pku(void)
 
if (!(ecx & X86_FEATURE_PKU)) {
dprintf2("cpu does not have PKU\n");
-   return 0;
+   return false;
}
if (!(ecx & X86_FEATURE_OSPKE)) {
dprintf2("cpu does not have OSPKE\n");
-   return 0;
+   return false;
}
-   return 1;
+   return true;
 }
 
 #define XSTATE_PKEY_BIT(9)
@@ -323,9 +323,19 @@ static inline void __page_o_noops(void)
 #elif __powerpc64__ /* arch */
 
 #define PAGE_SIZE (0x1UL << 16)
-static inline int cpu_has_pku(void)
+static inline bool is_pkey_supported(void)
 {
-   return 1;
+   /*
+* No simple way to determine this.
+* lets try allocating a key and see if it succeeds.
+*/
+   int ret = sys_pkey_alloc(0, 0);
+
+   if (ret > 0) {
+   sys_pkey_free(ret);
+   return true;
+   }
+   return false;
 }
 
 /* 8-bytes of instruction * 16384bytes = 1 page */
diff --git a/tools/testing/selftests/vm/protection_keys.c 
b/tools/testing/selftests/vm/protection_keys.c
index e82bd88..58da5a0 100644
--- a/tools/testing/selftests/vm/protection_keys.c
+++ b/tools/testing/selftests/vm/protection_keys.c
@@ -1299,8 +1299,8 @@ void test_mprotect_pkey_on_unsupported_cpu(int *ptr, u16 
pkey)
int size = PAGE_SIZE;
int sret;
 
-   if (cpu_has_pku()) {
-   dprintf1("SKIP: %s: no CPU support\n", __func__);
+   if (is_pkey_supported()) {
+   dprintf1("SKIP: %s: no CPU/kernel support\n", __func__);
return;
}
 
@@ -1362,12 +1362,13 @@ void run_tests_once(void)
 int main(void)
 {
int nr_iterations = 22;
+   int pkey_supported = is_pkey_supported();
 
setup_handlers();
 
-   printf("has pkey: %d\n", cpu_has_pku());
+   printf("has pkey: %s\n", pkey_supported ? "Yes" : "No");
 
-   if (!cpu_has_pku()) {
+   if (!pkey_supported) {
int size = PAGE_SIZE;
int *ptr;
 
-- 
1.7.1

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

[PATCH v12 18/22] selftests/vm: associate key on a mapped page and detect write violation

[Ram Pai]

detect write-violation on a page to which write-disabled
key is associated much after the page is mapped.

cc: Dave Hansen 
cc: Florian Weimer 
Signed-off-by: Ram Pai 
---
 tools/testing/selftests/vm/protection_keys.c |   12 
 1 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/tools/testing/selftests/vm/protection_keys.c 
b/tools/testing/selftests/vm/protection_keys.c
index dba9162..b685e26 100644
--- a/tools/testing/selftests/vm/protection_keys.c
+++ b/tools/testing/selftests/vm/protection_keys.c
@@ -1034,6 +1034,17 @@ void 
test_read_of_access_disabled_region_with_page_already_mapped(int *ptr,
expected_pkey_fault(pkey);
 }
 
+void test_write_of_write_disabled_region_with_page_already_mapped(int *ptr,
+   u16 pkey)
+{
+   *ptr = __LINE__;
+   dprintf1("disabling write access; after accessing the page, "
+   "to PKEY[%02d], doing write\n", pkey);
+   pkey_write_deny(pkey);
+   *ptr = __LINE__;
+   expected_pkey_fault(pkey);
+}
+
 void test_write_of_write_disabled_region(int *ptr, u16 pkey)
 {
dprintf1("disabling write access to PKEY[%02d], doing write\n", pkey);
@@ -1330,6 +1341,7 @@ void test_mprotect_pkey_on_unsupported_cpu(int *ptr, u16 
pkey)
test_read_of_access_disabled_region,
test_read_of_access_disabled_region_with_page_already_mapped,
test_write_of_write_disabled_region,
+   test_write_of_write_disabled_region_with_page_already_mapped,
test_write_of_access_disabled_region,
test_kernel_write_of_access_disabled_region,
test_kernel_write_of_write_disabled_region,
-- 
1.7.1

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

[PATCH v12 16/22] selftests/vm: fix an assertion in test_pkey_alloc_exhaust()

[Ram Pai]

The maximum number of keys that can be allocated has to
take into consideration, that some keys are reserved by
the architecture for   specific   purpose. Hence cannot
be allocated.

Fix the assertion in test_pkey_alloc_exhaust()

cc: Dave Hansen 
cc: Florian Weimer 
Signed-off-by: Ram Pai 
---
 tools/testing/selftests/vm/pkey-helpers.h|   14 ++
 tools/testing/selftests/vm/protection_keys.c |9 -
 2 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/tools/testing/selftests/vm/pkey-helpers.h 
b/tools/testing/selftests/vm/pkey-helpers.h
index 88ef58f..67f9b0f 100644
--- a/tools/testing/selftests/vm/pkey-helpers.h
+++ b/tools/testing/selftests/vm/pkey-helpers.h
@@ -416,4 +416,18 @@ static inline int get_start_key(void)
 #endif
 }
 
+static inline int arch_reserved_keys(void)
+{
+#if defined(__i386__) || defined(__x86_64__) /* arch */
+   return NR_RESERVED_PKEYS;
+#elif __powerpc64__ /* arch */
+   if (sysconf(_SC_PAGESIZE) == 4096)
+   return NR_RESERVED_PKEYS_4K;
+   else
+   return NR_RESERVED_PKEYS_64K;
+#else /* arch */
+   NOT SUPPORTED
+#endif /* arch */
+}
+
 #endif /* _PKEYS_HELPER_H */
diff --git a/tools/testing/selftests/vm/protection_keys.c 
b/tools/testing/selftests/vm/protection_keys.c
index 58da5a0..aae6771 100644
--- a/tools/testing/selftests/vm/protection_keys.c
+++ b/tools/testing/selftests/vm/protection_keys.c
@@ -1167,12 +1167,11 @@ void test_pkey_alloc_exhaust(int *ptr, u16 pkey)
pkey_assert(i < NR_PKEYS*2);
 
/*
-* There are 16 pkeys supported in hardware.  One is taken
-* up for the default (0) and another can be taken up by
-* an execute-only mapping.  Ensure that we can allocate
-* at least 14 (16-2).
+* There are NR_PKEYS pkeys supported in hardware. arch_reserved_keys()
+* are reserved. One   can   be   taken   up by an execute-only mapping.
+* Ensure that we can allocate at least the remaining.
 */
-   pkey_assert(i >= NR_PKEYS-2);
+   pkey_assert(i >= (NR_PKEYS-arch_reserved_keys()-1));
 
for (i = 0; i < nr_allocated_pkeys; i++) {
err = sys_pkey_free(allocated_pkeys[i]);
-- 
1.7.1

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

[PATCH v12 17/22] selftests/vm: associate key on a mapped page and detect access violation

[Ram Pai]

detect access-violation on a page to which access-disabled
key is associated much after the page is mapped.

cc: Dave Hansen 
cc: Florian Weimer 
Signed-off-by: Ram Pai 
---
 tools/testing/selftests/vm/protection_keys.c |   19 +++
 1 files changed, 19 insertions(+), 0 deletions(-)

diff --git a/tools/testing/selftests/vm/protection_keys.c 
b/tools/testing/selftests/vm/protection_keys.c
index aae6771..dba9162 100644
--- a/tools/testing/selftests/vm/protection_keys.c
+++ b/tools/testing/selftests/vm/protection_keys.c
@@ -1016,6 +1016,24 @@ void test_read_of_access_disabled_region(int *ptr, u16 
pkey)
dprintf1("*ptr: %d\n", ptr_contents);
expected_pkey_fault(pkey);
 }
+
+void test_read_of_access_disabled_region_with_page_already_mapped(int *ptr,
+   u16 pkey)
+{
+   int ptr_contents;
+
+   dprintf1("disabling access to PKEY[%02d], doing read @ %p\n",
+   pkey, ptr);
+   ptr_contents = read_ptr(ptr);
+   dprintf1("reading ptr before disabling the read : %d\n",
+   ptr_contents);
+   rdpkey_reg();
+   pkey_access_deny(pkey);
+   ptr_contents = read_ptr(ptr);
+   dprintf1("*ptr: %d\n", ptr_contents);
+   expected_pkey_fault(pkey);
+}
+
 void test_write_of_write_disabled_region(int *ptr, u16 pkey)
 {
dprintf1("disabling write access to PKEY[%02d], doing write\n", pkey);
@@ -1310,6 +1328,7 @@ void test_mprotect_pkey_on_unsupported_cpu(int *ptr, u16 
pkey)
 void (*pkey_tests[])(int *ptr, u16 pkey) = {
test_read_of_write_disabled_region,
test_read_of_access_disabled_region,
+   test_read_of_access_disabled_region_with_page_already_mapped,
test_write_of_write_disabled_region,
test_write_of_access_disabled_region,
test_kernel_write_of_access_disabled_region,
-- 
1.7.1

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

[PATCH v12 19/22] selftests/vm: detect write violation on a mapped access-denied-key page

[Ram Pai]

detect write-violation on a page to which access-disabled
key is associated much after the page is mapped.

cc: Dave Hansen 
cc: Florian Weimer 
Signed-off-by: Ram Pai 
---
 tools/testing/selftests/vm/protection_keys.c |   13 +
 1 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/tools/testing/selftests/vm/protection_keys.c 
b/tools/testing/selftests/vm/protection_keys.c
index b685e26..437ee74 100644
--- a/tools/testing/selftests/vm/protection_keys.c
+++ b/tools/testing/selftests/vm/protection_keys.c
@@ -1059,6 +1059,18 @@ void test_write_of_access_disabled_region(int *ptr, u16 
pkey)
*ptr = __LINE__;
expected_pkey_fault(pkey);
 }
+
+void test_write_of_access_disabled_region_with_page_already_mapped(int *ptr,
+   u16 pkey)
+{
+   *ptr = __LINE__;
+   dprintf1("disabling access; after accessing the page, "
+   " to PKEY[%02d], doing write\n", pkey);
+   pkey_access_deny(pkey);
+   *ptr = __LINE__;
+   expected_pkey_fault(pkey);
+}
+
 void test_kernel_write_of_access_disabled_region(int *ptr, u16 pkey)
 {
int ret;
@@ -1343,6 +1355,7 @@ void test_mprotect_pkey_on_unsupported_cpu(int *ptr, u16 
pkey)
test_write_of_write_disabled_region,
test_write_of_write_disabled_region_with_page_already_mapped,
test_write_of_access_disabled_region,
+   test_write_of_access_disabled_region_with_page_already_mapped,
test_kernel_write_of_access_disabled_region,
test_kernel_write_of_write_disabled_region,
test_kernel_gup_of_access_disabled_region,
-- 
1.7.1

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

[PATCH v12 21/22] selftests/vm: sub-page allocator

[Ram Pai]

introduce a new allocator that allocates 4k hardware-pages to back
64k linux-page. This allocator is only applicable on powerpc.

cc: Dave Hansen 
cc: Florian Weimer 
Signed-off-by: Ram Pai 
---
 tools/testing/selftests/vm/protection_keys.c |   30 ++
 1 files changed, 30 insertions(+), 0 deletions(-)

diff --git a/tools/testing/selftests/vm/protection_keys.c 
b/tools/testing/selftests/vm/protection_keys.c
index 42c068a..1b06e59 100644
--- a/tools/testing/selftests/vm/protection_keys.c
+++ b/tools/testing/selftests/vm/protection_keys.c
@@ -766,6 +766,35 @@ void free_pkey_malloc(void *ptr)
return ptr;
 }
 
+void *malloc_pkey_with_mprotect_subpage(long size, int prot, u16 pkey)
+{
+#ifdef __powerpc64__
+   void *ptr;
+   int ret;
+
+   dprintf1("doing %s(size=%ld, prot=0x%x, pkey=%d)\n", __func__,
+   size, prot, pkey);
+   pkey_assert(pkey < NR_PKEYS);
+   ptr = mmap(NULL, size, prot, MAP_ANONYMOUS|MAP_PRIVATE, -1, 0);
+   pkey_assert(ptr != (void *)-1);
+
+   ret = syscall(__NR_subpage_prot, ptr, size, NULL);
+   if (ret) {
+   perror("subpage_perm");
+   return PTR_ERR_ENOTSUP;
+   }
+
+   ret = mprotect_pkey((void *)ptr, PAGE_SIZE, prot, pkey);
+   pkey_assert(!ret);
+   record_pkey_malloc(ptr, size);
+
+   dprintf1("%s() for pkey %d @ %p\n", __func__, pkey, ptr);
+   return ptr;
+#else /*  __powerpc64__ */
+   return PTR_ERR_ENOTSUP;
+#endif /*  __powerpc64__ */
+}
+
 void *malloc_pkey_anon_huge(long size, int prot, u16 pkey)
 {
int ret;
@@ -888,6 +917,7 @@ void setup_hugetlbfs(void)
 void *(*pkey_malloc[])(long size, int prot, u16 pkey) = {
 
malloc_pkey_with_mprotect,
+   malloc_pkey_with_mprotect_subpage,
malloc_pkey_anon_huge,
malloc_pkey_hugetlb
 /* can not do direct with the pkey_mprotect() API:
-- 
1.7.1

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

[PATCH v12 22/22] selftests/vm: Fix deadlock in protection_keys.c

[Ram Pai]

From: Thiago Jung Bauermann 

The sig_chld() handler calls dprintf2() taking care of setting
dprint_in_signal so that sigsafe_printf() won't call printf().
Unfortunately, this precaution is is negated by dprintf_level(), which
has a call to fflush().

This function acquires a lock, which means that if the signal interrupts an
ongoing fflush() the process will deadlock. At least on powerpc this is
easy to trigger, resulting in the following backtrace when attaching to the
frozen process:

  (gdb) bt
  #0  0x7fff9f96c7d8 in __lll_lock_wait_private () from 
/lib64/power8/libc.so.6
  #1  0x7fff9f8cba4c in _IO_flush_all_lockp () from /lib64/power8/libc.so.6
  #2  0x7fff9f8cbd1c in __GI__IO_flush_all () from /lib64/power8/libc.so.6
  #3  0x7fff9f8b7424 in fflush () from /lib64/power8/libc.so.6
  #4  0x100504f8 in sig_chld (x=17) at protection_keys.c:283
  #5  
  #6  0x7fff9f8cb8ac in _IO_flush_all_lockp () from /lib64/power8/libc.so.6
  #7  0x7fff9f8cbd1c in __GI__IO_flush_all () from /lib64/power8/libc.so.6
  #8  0x7fff9f8b7424 in fflush () from /lib64/power8/libc.so.6
  #9  0x10050b50 in pkey_get (pkey=7, flags=0) at protection_keys.c:379
  #10 0x10050dc0 in pkey_disable_set (pkey=7, flags=2) at 
protection_keys.c:423
  #11 0x10051414 in pkey_write_deny (pkey=7) at protection_keys.c:486
  #12 0x100556bc in test_ptrace_of_child (ptr=0x7fff9f7f, pkey=7) 
at protection_keys.c:1288
  #13 0x10055f60 in run_tests_once () at protection_keys.c:1414
  #14 0x100561a4 in main () at protection_keys.c:1459

The fix is to refrain from calling fflush() when inside a signal handler.
The output may not be as pretty but at least the testcase will be able to
move on.

cc: Dave Hansen 
cc: Florian Weimer 
Signed-off-by: Ram Pai 
Signed-off-by: Thiago Jung Bauermann 

 tools/testing/selftests/vm/pkey-helpers.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
---
 tools/testing/selftests/vm/pkey-helpers.h |3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/tools/testing/selftests/vm/pkey-helpers.h 
b/tools/testing/selftests/vm/pkey-helpers.h
index 67f9b0f..7240598 100644
--- a/tools/testing/selftests/vm/pkey-helpers.h
+++ b/tools/testing/selftests/vm/pkey-helpers.h
@@ -128,7 +128,8 @@ static inline void sigsafe_printf(const char *format, ...)
 #define dprintf_level(level, args...) do { \
if (level <= DEBUG_LEVEL)   \
sigsafe_printf(args);   \
-   fflush(NULL);   \
+   if (!dprint_in_signal)  \
+   fflush(NULL);   \
 } while (0)
 #define dprintf0(args...) dprintf_level(0, args)
 #define dprintf1(args...) dprintf_level(1, args)
-- 
1.7.1

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

[PATCH v12 12/22] selftests/vm: generic cleanup

[Ram Pai]

cleanup the code to satisfy coding styles.

cc: Dave Hansen 
cc: Florian Weimer 
Signed-off-by: Ram Pai 
---
 tools/testing/selftests/vm/protection_keys.c |   81 ++
 1 files changed, 43 insertions(+), 38 deletions(-)

diff --git a/tools/testing/selftests/vm/protection_keys.c 
b/tools/testing/selftests/vm/protection_keys.c
index 6054093..6fdd8f5 100644
--- a/tools/testing/selftests/vm/protection_keys.c
+++ b/tools/testing/selftests/vm/protection_keys.c
@@ -4,7 +4,7 @@
  *
  * There are examples in here of:
  *  * how to set protection keys on memory
- *  * how to set/clear bits in pkey registers (the rights register)
+ *  * how to set/clear bits in Protection Key registers (the rights register)
  *  * how to handle SEGV_PKUERR signals and extract pkey-relevant
  *information from the siginfo
  *
@@ -13,13 +13,18 @@
  * prefault pages in at malloc, or not
  * protect MPX bounds tables with protection keys?
  * make sure VMA splitting/merging is working correctly
- * OOMs can destroy mm->mmap (see exit_mmap()), so make sure it is immune 
to pkeys
- * look for pkey "leaks" where it is still set on a VMA but "freed" back 
to the kernel
- * do a plain mprotect() to a mprotect_pkey() area and make sure the pkey 
sticks
+ * OOMs can destroy mm->mmap (see exit_mmap()),
+ * so make sure it is immune to pkeys
+ * look for pkey "leaks" where it is still set on a VMA
+ *  but "freed" back to the kernel
+ * do a plain mprotect() to a mprotect_pkey() area and make
+ *  sure the pkey sticks
  *
  * Compile like this:
- * gcc  -o protection_keys-O2 -g -std=gnu99 -pthread -Wall 
protection_keys.c -lrt -ldl -lm
- * gcc -m32 -o protection_keys_32 -O2 -g -std=gnu99 -pthread -Wall 
protection_keys.c -lrt -ldl -lm
+ * gcc  -o protection_keys-O2 -g -std=gnu99
+ *  -pthread -Wall protection_keys.c -lrt -ldl -lm
+ * gcc -m32 -o protection_keys_32 -O2 -g -std=gnu99
+ *  -pthread -Wall protection_keys.c -lrt -ldl -lm
  */
 #define _GNU_SOURCE
 #include 
@@ -251,26 +256,11 @@ void signal_handler(int signum, siginfo_t *si, void 
*vucontext)
dprintf1("signal pkey_reg from  pkey_reg: %016lx\n", __rdpkey_reg());
dprintf1("pkey from siginfo: %jx\n", siginfo_pkey);
*(u64 *)pkey_reg_ptr = 0x;
-   dprintf1("WARNING: set PRKU=0 to allow faulting instruction to 
continue\n");
+   dprintf1("WARNING: set PKEY_REG=0 to allow faulting instruction "
+   "to continue\n");
pkey_faults++;
dprintf1("==\n");
return;
-   if (trapno == 14) {
-   fprintf(stderr,
-   "ERROR: In signal handler, page fault, trapno = %d, ip 
= %016lx\n",
-   trapno, ip);
-   fprintf(stderr, "si_addr %p\n", si->si_addr);
-   fprintf(stderr, "REG_ERR: %lx\n",
-   (unsigned 
long)uctxt->uc_mcontext.gregs[REG_ERR]);
-   exit(1);
-   } else {
-   fprintf(stderr, "unexpected trap %d! at 0x%lx\n", trapno, ip);
-   fprintf(stderr, "si_addr %p\n", si->si_addr);
-   fprintf(stderr, "REG_ERR: %lx\n",
-   (unsigned 
long)uctxt->uc_mcontext.gregs[REG_ERR]);
-   exit(2);
-   }
-   dprint_in_signal = 0;
 }
 
 int wait_all_children(void)
@@ -415,7 +405,7 @@ void pkey_disable_set(int pkey, int flags)
 {
unsigned long syscall_flags = 0;
int ret;
-   int pkey_rights;
+   u32 pkey_rights;
pkey_reg_t orig_pkey_reg = rdpkey_reg();
 
dprintf1("START->%s(%d, 0x%x)\n", __func__,
@@ -453,7 +443,7 @@ void pkey_disable_clear(int pkey, int flags)
 {
unsigned long syscall_flags = 0;
int ret;
-   int pkey_rights = pkey_get(pkey, syscall_flags);
+   u32 pkey_rights = pkey_get(pkey, syscall_flags);
pkey_reg_t orig_pkey_reg = rdpkey_reg();
 
pkey_assert(flags & (PKEY_DISABLE_ACCESS | PKEY_DISABLE_WRITE));
@@ -516,9 +506,10 @@ int sys_mprotect_pkey(void *ptr, size_t size, unsigned 
long orig_prot,
return sret;
 }
 
-int sys_pkey_alloc(unsigned long flags, unsigned long init_val)
+int sys_pkey_alloc(unsigned long flags, u64 init_val)
 {
int ret = syscall(SYS_pkey_alloc, flags, init_val);
+
dprintf1("%s(flags=%lx, init_val=%lx) syscall ret: %d errno: %d\n",
__func__, flags, init_val, ret, errno);
return ret;
@@ -542,7 +533,7 @@ void pkey_set_shadow(u32 key, u64 init_val)
 int alloc_pkey(void)
 {
int ret;
-   unsigned long init_val = 0x0;
+   u64 init_val = 0x0;
 
dprintf1("%s()::%d, pkey_reg: 0x%016lx shadow: %016lx\n", __func__,
__LINE__, 

[PATCH v12 08/22] selftests/vm: clear the bits in shadow reg when a pkey is freed.

[Ram Pai]

When a key is freed, the  key  is  no  more  effective.
Clear the bits corresponding to the pkey in the shadow
register. Otherwise  it  will carry some spurious bits
which can trigger false-positive asserts.

cc: Dave Hansen 
cc: Florian Weimer 
Signed-off-by: Ram Pai 
---
 tools/testing/selftests/vm/protection_keys.c |3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/tools/testing/selftests/vm/protection_keys.c 
b/tools/testing/selftests/vm/protection_keys.c
index ca54a95..aaf9f09 100644
--- a/tools/testing/selftests/vm/protection_keys.c
+++ b/tools/testing/selftests/vm/protection_keys.c
@@ -582,6 +582,9 @@ int alloc_pkey(void)
 int sys_pkey_free(unsigned long pkey)
 {
int ret = syscall(SYS_pkey_free, pkey);
+
+   if (!ret)
+   shadow_pkey_reg &= reset_bits(pkey, PKEY_DISABLE_ACCESS);
dprintf1("%s(pkey=%ld) syscall ret: %d\n", __func__, pkey, ret);
return ret;
 }
-- 
1.7.1

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

[PATCH v12 10/22] selftests/vm: introduce two arch independent abstraction

[Ram Pai]

open_hugepage_file() <- opens the huge page file
get_start_key() <--  provides the first non-reserved key.

cc: Dave Hansen 
cc: Florian Weimer 
Signed-off-by: Ram Pai 
---
 tools/testing/selftests/vm/pkey-helpers.h|   11 +++
 tools/testing/selftests/vm/protection_keys.c |6 +++---
 2 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/vm/pkey-helpers.h 
b/tools/testing/selftests/vm/pkey-helpers.h
index 7c979ad..c8f5739 100644
--- a/tools/testing/selftests/vm/pkey-helpers.h
+++ b/tools/testing/selftests/vm/pkey-helpers.h
@@ -304,3 +304,14 @@ static inline void __page_o_noops(void)
}   \
 } while (0)
 #define raw_assert(cond) assert(cond)
+
+static inline int open_hugepage_file(int flag)
+{
+   return open("/sys/kernel/mm/hugepages/hugepages-2048kB/nr_hugepages",
+O_RDONLY);
+}
+
+static inline int get_start_key(void)
+{
+   return 1;
+}
diff --git a/tools/testing/selftests/vm/protection_keys.c 
b/tools/testing/selftests/vm/protection_keys.c
index 2e4b636..254b66d 100644
--- a/tools/testing/selftests/vm/protection_keys.c
+++ b/tools/testing/selftests/vm/protection_keys.c
@@ -809,7 +809,7 @@ void setup_hugetlbfs(void)
 * Now go make sure that we got the pages and that they
 * are 2M pages.  Someone might have made 1G the default.
 */
-   fd = open("/sys/kernel/mm/hugepages/hugepages-2048kB/nr_hugepages", 
O_RDONLY);
+   fd = open_hugepage_file(O_RDONLY);
if (fd < 0) {
perror("opening sysfs 2M hugetlb config");
return;
@@ -1087,10 +1087,10 @@ void test_kernel_gup_write_to_write_disabled_region(int 
*ptr, u16 pkey)
 void test_pkey_syscalls_on_non_allocated_pkey(int *ptr, u16 pkey)
 {
int err;
-   int i;
+   int i = get_start_key();
 
/* Note: 0 is the default pkey, so don't mess with it */
-   for (i = 1; i < NR_PKEYS; i++) {
+   for (; i < NR_PKEYS; i++) {
if (pkey == i)
continue;
 
-- 
1.7.1

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

[PATCH v12 00/22] selftests, powerpc, x86 : Memory Protection Keys

[Ram Pai]

Memory protection keys enables an application to protect its address space from
inadvertent access by its own code.

This feature is now enabled on powerpc architecture and integrated in
4.16-rc1.  The patches move the selftests to arch neutral directory
and enhance their test coverage. 

Test

Verified for correctness on powerpc
and on x86 architectures(using EC2 ubuntu VM instance).

History:
---

version 12: 
(1) fixed the offset of pkey field in the siginfo structure for
x86_64 and powerpc. And tries to use the actual field
if the headers have it defined.
version 11:
(1) fixed a deadlock in the ptrace testcase.

version 10 and prior:
(1) moved the testcase to arch neutral directory
(2) split the changes into incremental patches.

Ram Pai (21):
  selftests/x86: Move protecton key selftest to arch neutral directory
  selftests/vm: rename all references to pkru to a generic name
  selftests/vm: move generic definitions to header file
  selftests/vm: typecast the pkey register
  selftests/vm: generic function to handle shadow key register
  selftests/vm: fix the wrong assert in pkey_disable_set()
  selftests/vm: fixed bugs in pkey_disable_clear()
  selftests/vm: clear the bits in shadow reg when a pkey is freed.
  selftests/vm: fix alloc_random_pkey() to make it really random
  selftests/vm: introduce two arch independent abstraction
  selftests/vm: pkey register should match shadow pkey
  selftests/vm: generic cleanup
  selftests/vm: powerpc implementation for generic abstraction
  selftests/vm: clear the bits in shadow reg when a pkey is freed.
  selftests/vm: powerpc implementation to check support for pkey
  selftests/vm: fix an assertion in test_pkey_alloc_exhaust()
  selftests/vm: associate key on a mapped page and detect access
violation
  selftests/vm: associate key on a mapped page and detect write
violation
  selftests/vm: detect write violation on a mapped access-denied-key
page
  selftests/vm: testcases must restore pkey-permissions
  selftests/vm: sub-page allocator

Thiago Jung Bauermann (1):
  selftests/vm: Fix deadlock in protection_keys.c

 tools/testing/selftests/vm/Makefile   |1 +
 tools/testing/selftests/vm/pkey-helpers.h |  434 
 tools/testing/selftests/vm/protection_keys.c  | 1471 +
 tools/testing/selftests/x86/Makefile  |2 +-
 tools/testing/selftests/x86/pkey-helpers.h|  223 
 tools/testing/selftests/x86/protection_keys.c | 1407 ---
 6 files changed, 1907 insertions(+), 1631 deletions(-)
 create mode 100644 tools/testing/selftests/vm/pkey-helpers.h
 create mode 100644 tools/testing/selftests/vm/protection_keys.c
 delete mode 100644 tools/testing/selftests/x86/pkey-helpers.h
 delete mode 100644 tools/testing/selftests/x86/protection_keys.c

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

[PATCH v12 03/22] selftests/vm: move generic definitions to header file

[Ram Pai]

Moved all the generic definition and helper functions to the
header file

cc: Dave Hansen 
cc: Florian Weimer 
Signed-off-by: Ram Pai 
---
 tools/testing/selftests/vm/pkey-helpers.h|   62 ++--
 tools/testing/selftests/vm/protection_keys.c |   66 --
 2 files changed, 57 insertions(+), 71 deletions(-)

diff --git a/tools/testing/selftests/vm/pkey-helpers.h 
b/tools/testing/selftests/vm/pkey-helpers.h
index a568166..c1bc761 100644
--- a/tools/testing/selftests/vm/pkey-helpers.h
+++ b/tools/testing/selftests/vm/pkey-helpers.h
@@ -13,8 +13,31 @@
 #include 
 #include 
 
+/* Define some kernel-like types */
+#define  u8 uint8_t
+#define u16 uint16_t
+#define u32 uint32_t
+#define u64 uint64_t
+
+#ifdef __i386__
+#define SYS_mprotect_key 380
+#define SYS_pkey_alloc  381
+#define SYS_pkey_free   382
+#define REG_IP_IDX REG_EIP
+#define si_pkey_offset 0x14
+#else
+#define SYS_mprotect_key 329
+#define SYS_pkey_alloc  330
+#define SYS_pkey_free   331
+#define REG_IP_IDX REG_RIP
+#define si_pkey_offset 0x20
+#endif
+
 #define NR_PKEYS 16
 #define PKEY_BITS_PER_PKEY 2
+#define PKEY_DISABLE_ACCESS0x1
+#define PKEY_DISABLE_WRITE 0x2
+#define HPAGE_SIZE (1UL<<21)
 
 #ifndef DEBUG_LEVEL
 #define DEBUG_LEVEL 0
@@ -141,11 +164,6 @@ static inline void __pkey_write_allow(int pkey, int 
do_allow_write)
dprintf4("pkey_reg now: %08x\n", rdpkey_reg());
 }
 
-#define PROT_PKEY0 0x10/* protection key value (bit 0) */
-#define PROT_PKEY1 0x20/* protection key value (bit 1) */
-#define PROT_PKEY2 0x40/* protection key value (bit 2) */
-#define PROT_PKEY3 0x80/* protection key value (bit 3) */
-
 #define PAGE_SIZE 4096
 #define MB (1<<20)
 
@@ -223,4 +241,38 @@ int pkey_reg_xstate_offset(void)
return xstate_offset;
 }
 
+static inline void __page_o_noops(void)
+{
+   /* 8-bytes of instruction * 512 bytes = 1 page */
+   asm(".rept 512 ; nopl 0x7eee(%eax) ; .endr");
+}
+
 #endif /* _PKEYS_HELPER_H */
+
+#define ARRAY_SIZE(x) (sizeof(x) / sizeof(*(x)))
+#define ALIGN_UP(x, align_to)  (((x) + ((align_to)-1)) & ~((align_to)-1))
+#define ALIGN_DOWN(x, align_to) ((x) & ~((align_to)-1))
+#define ALIGN_PTR_UP(p, ptr_align_to)  \
+   ((typeof(p))ALIGN_UP((unsigned long)(p), ptr_align_to))
+#define ALIGN_PTR_DOWN(p, ptr_align_to) \
+   ((typeof(p))ALIGN_DOWN((unsigned long)(p), ptr_align_to))
+#define __stringify_1(x...) #x
+#define __stringify(x...)   __stringify_1(x)
+
+#define PTR_ERR_ENOTSUP ((void *)-ENOTSUP)
+
+int dprint_in_signal;
+char dprint_in_signal_buffer[DPRINT_IN_SIGNAL_BUF_SIZE];
+
+extern void abort_hooks(void);
+#define pkey_assert(condition) do {\
+   if (!(condition)) { \
+   dprintf0("assert() at %s::%d test_nr: %d iteration: %d\n", \
+   __FILE__, __LINE__, \
+   test_nr, iteration_nr); \
+   dprintf0("errno at assert: %d", errno); \
+   abort_hooks();  \
+   assert(condition);  \
+   }   \
+} while (0)
+#define raw_assert(cond) assert(cond)
diff --git a/tools/testing/selftests/vm/protection_keys.c 
b/tools/testing/selftests/vm/protection_keys.c
index 4aebf12..91bade4 100644
--- a/tools/testing/selftests/vm/protection_keys.c
+++ b/tools/testing/selftests/vm/protection_keys.c
@@ -49,34 +49,9 @@
 int test_nr;
 
 unsigned int shadow_pkey_reg;
-
-#define HPAGE_SIZE (1UL<<21)
-#define ARRAY_SIZE(x) (sizeof(x) / sizeof(*(x)))
-#define ALIGN_UP(x, align_to)  (((x) + ((align_to)-1)) & ~((align_to)-1))
-#define ALIGN_DOWN(x, align_to) ((x) & ~((align_to)-1))
-#define ALIGN_PTR_UP(p, ptr_align_to)  ((typeof(p))ALIGN_UP((unsigned 
long)(p),ptr_align_to))
-#define ALIGN_PTR_DOWN(p, ptr_align_to)
((typeof(p))ALIGN_DOWN((unsigned long)(p),  ptr_align_to))
-#define __stringify_1(x...) #x
-#define __stringify(x...)   __stringify_1(x)
-
-#define PTR_ERR_ENOTSUP ((void *)-ENOTSUP)
-
 int dprint_in_signal;
 char dprint_in_signal_buffer[DPRINT_IN_SIGNAL_BUF_SIZE];
 
-extern void abort_hooks(void);
-#define pkey_assert(condition) do {\
-   if (!(condition)) { \
-   dprintf0("assert() at %s::%d test_nr: %d iteration: %d\n", \
-   __FILE__, __LINE__, \
-   test_nr, iteration_nr); \
-   dprintf0("errno at assert: %d", errno); \
-   abort_hooks();  \
-   assert(condition);  \
-   }   \
-} while (0)
-#define raw_assert(cond) assert(cond)
-
 void cat_into_file(char *str, char *file)
 {
int fd = open(file, O_RDWR);
@@ -154,12 +129,6 @@ void 

Re: [PATCH 4/5 resend] documentation: firewire: add basic firewire.rst to driver-api

[Takashi Sakamoto]

Hi,

On Feb 22 2018 10:07, Randy Dunlap wrote:

From: Randy Dunlap 

Add a basic Firewire/IEEE 1394 driver API chapter to the Linux
kernel documentation.

Signed-off-by: Randy Dunlap 
Cc: Stefan Richter 
Cc: linux1394-de...@lists.sourceforge.net
---
  Documentation/driver-api/firewire.rst |   33 
  Documentation/driver-api/index.rst|1
  2 files changed, 34 insertions(+)

--- /dev/null
+++ linux-next-20180220/Documentation/driver-api/firewire.rst
@@ -0,0 +1,33 @@
+===
+Firewire (IEEE 1394) driver Interface Guide
+===
+
+Introduction and Overview
+=
+
+TBD
+
+Firewire char device data structures
+
+
+.. kernel-doc:: include/uapi/linux/firewire-cdev.h
+:internal:
+
+Firewire device probing and sysfs interfaces
+
+
+.. kernel-doc:: drivers/firewire/core-device.c
+:export:
+
+Firewire core transaction interfaces
+
+
+.. kernel-doc:: drivers/firewire/core-transaction.c
+:export:
+
+Firewire Isochronous I/O interfaces
+===
+
+.. kernel-doc:: drivers/firewire/core-iso.c
+   :export:
+


A command of 'git am' generates below warning when applying this patch.

```
$ git am /tmp/patches/*
Applying: documentation: firewire: add basic firewire.rst to driver-api
.git/rebase-apply/patch:41: new blank line at EOF.
+
```

It's better to remove the blank line.


--- linux-next-20180220.orig/Documentation/driver-api/index.rst
+++ linux-next-20180220/Documentation/driver-api/index.rst
@@ -27,6 +27,7 @@ available subsections can be seen below.
 iio/index
 input
 usb/index
+   firewire
 pci
 spi
 i2c



Thanks

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

Re: [PATCH v2 7/8] [PATCH 7/8] drivers/hwmon: Add a generic PECI hwmon client driver

[Jae Hyun Yoo]

On 2/21/2018 4:37 PM, Andrew Lunn wrote:

But even with this change, it still needs to use delayed creation
because BMC side kernel doesn't know how many DIMMs are populated on
a remote server before the remote server completes its memory
training and testing in BIOS, but it needs to check the remote
server's CPU temperature as immediate as possible to make
appropriate thermal control based on the remote CPU's temperature to
avoid any critical thermal issue. What would be a better solution in
this case?


You could change this driver so that it supports one DIMM.  Move the
'hotplug' part into another driver which creates and destroys
instances of the hwmon DIMM device as the DIMMS come and go.

Also, do you need to handle CPU hotplug? You could split the CPU
temperature part into a separate hwmon driver? And again create and
destroy devices as CPUs come and go?

Andrew



That seems like a possible option. I'll rewrite the hwmon driver again 
like that.


Thanks for the good idea. :)

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

[PATCH 3/5 resend] firewire: clean up core-transaction.c kernel-doc

[Randy Dunlap]

From: Randy Dunlap 

Clean up kernel-doc warnings in 
so that it can be added to a Firewire/IEEE 1394 driver-api chapter
without adding lots of noisy warnings to the documentation build.

Signed-off-by: Randy Dunlap 
---
 drivers/firewire/core-transaction.c |   10 ++
 1 file changed, 10 insertions(+)

--- linux-next-20180220.orig/drivers/firewire/core-transaction.c
+++ linux-next-20180220/drivers/firewire/core-transaction.c
@@ -410,6 +410,14 @@ static void transaction_callback(struct
 
 /**
  * fw_run_transaction() - send request and sleep until transaction is completed
+ * @card:  card interface for this request
+ * @tcode: transaction code
+ * @destination_id:destination node ID, consisting of bus_ID and phy_ID
+ * @generation:bus generation in which request and response 
are valid
+ * @speed: transmission speed
+ * @offset:48bit wide offset into destination's address space
+ * @payload:   data payload for the request subaction
+ * @length:length of the payload, in bytes
  *
  * Returns the RCODE.  See fw_send_request() for parameter documentation.
  * Unlike fw_send_request(), @data points to the payload of the request or/and
@@ -604,6 +612,7 @@ EXPORT_SYMBOL(fw_core_add_address_handle
 
 /**
  * fw_core_remove_address_handler() - unregister an address handler
+ * @handler: callback
  *
  * To be called in process context.
  *
@@ -828,6 +837,7 @@ EXPORT_SYMBOL(fw_send_response);
 
 /**
  * fw_get_request_speed() - returns speed at which the @request was received
+ * @request: firewire request data
  */
 int fw_get_request_speed(struct fw_request *request)
 {


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

[PATCH 4/5 resend] documentation: firewire: add basic firewire.rst to driver-api

[Randy Dunlap]

From: Randy Dunlap 

Add a basic Firewire/IEEE 1394 driver API chapter to the Linux
kernel documentation.

Signed-off-by: Randy Dunlap 
Cc: Stefan Richter 
Cc: linux1394-de...@lists.sourceforge.net
---
 Documentation/driver-api/firewire.rst |   33 
 Documentation/driver-api/index.rst|1 
 2 files changed, 34 insertions(+)

--- /dev/null
+++ linux-next-20180220/Documentation/driver-api/firewire.rst
@@ -0,0 +1,33 @@
+===
+Firewire (IEEE 1394) driver Interface Guide
+===
+
+Introduction and Overview
+=
+
+TBD
+
+Firewire char device data structures
+
+
+.. kernel-doc:: include/uapi/linux/firewire-cdev.h
+:internal:
+
+Firewire device probing and sysfs interfaces
+
+
+.. kernel-doc:: drivers/firewire/core-device.c
+:export:
+
+Firewire core transaction interfaces
+
+
+.. kernel-doc:: drivers/firewire/core-transaction.c
+:export:
+
+Firewire Isochronous I/O interfaces
+===
+
+.. kernel-doc:: drivers/firewire/core-iso.c
+   :export:
+
--- linux-next-20180220.orig/Documentation/driver-api/index.rst
+++ linux-next-20180220/Documentation/driver-api/index.rst
@@ -27,6 +27,7 @@ available subsections can be seen below.
iio/index
input
usb/index
+   firewire
pci
spi
i2c


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

[PATCH 5/5] documentation: firewire: add introduction/overview text

[Randy Dunlap]

From: Takashi Sakamoto 

Replace the Introduction section's TBD with some useful overview text.

Acked-by: Randy Dunlap 
Tested-by: Randy Dunlap 

Not-yet-Signed-off-by: Takashi Sakamoto 
Signed-off-by: Randy Dunlap 
---
 Documentation/driver-api/firewire.rst |   18 +-
 1 file changed, 17 insertions(+), 1 deletion(-)

--- linux-next-20180220.orig/Documentation/driver-api/firewire.rst
+++ linux-next-20180220/Documentation/driver-api/firewire.rst
@@ -5,17 +5,33 @@ Firewire (IEEE 1394) driver Interface Gu
 Introduction and Overview
 =
 
-TBD
+The Linux FireWire subsystem adds some interfaces into the Linux system
+to use/maintain any resource on the IEEE 1394 bus.
+
+The main purpose of these interfaces is to access address space on each node
+on the IEEE 1394 bus by ISO/IEC 13213 (IEEE 1212) procedure, and to control
+isochronous resources on the bus by IEEE 1394 procedure.
+
+Two types of interfaces are added, according to consumers of the interface. A
+set of userspace interfaces is available via `firewire character devices`. A 
set
+of kernel interfaces is available via exported symbols in the `firewire-core`
+module.
 
 Firewire char device data structures
 
 
+.. include:: /ABI/stable/firewire-cdev
+:literal:
+
 .. kernel-doc:: include/uapi/linux/firewire-cdev.h
 :internal:
 
 Firewire device probing and sysfs interfaces
 
 
+.. include:: /ABI/stable/sysfs-bus-firewire
+:literal:
+
 .. kernel-doc:: drivers/firewire/core-device.c
 :export:
 

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

[PATCH 2/5 resend] firewire: clean up core-iso.c kernel-doc

[Randy Dunlap]

From: Randy Dunlap 

Clean up kernel-doc warnings in  so that
it can be added to a Firewire/IEEE 1394 driver-api chapter
without adding lots of noisy warnings to the documentation build.

Signed-off-by: Randy Dunlap 
---
 drivers/firewire/core-iso.c |7 +++
 1 file changed, 7 insertions(+)

--- linux-next-20180220.orig/drivers/firewire/core-iso.c
+++ linux-next-20180220/drivers/firewire/core-iso.c
@@ -337,9 +337,16 @@ static void deallocate_channel(struct fw
 
 /**
  * fw_iso_resource_manage() - Allocate or deallocate a channel and/or bandwidth
+ * @card: card interface for this action
+ * @generation: bus generation
+ * @channels_mask: bitmask for channel allocation
+ * @channel: pointer for returning channel allocation result
+ * @bandwidth: pointer for returning bandwidth allocation result
+ * @allocate: whether to allocate (true) or deallocate (false)
  *
  * In parameters: card, generation, channels_mask, bandwidth, allocate
  * Out parameters: channel, bandwidth
+ *
  * This function blocks (sleeps) during communication with the IRM.
  *
  * Allocates or deallocates at most one channel out of channels_mask.


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

[PATCH 1/5 resend] firewire: clean up firewire-cdev.h kernel-doc

[Randy Dunlap]

From: Randy Dunlap 

Clean up kernel-doc warnings in  so that
it can be added to a Firewire/IEEE 1394 driver-api chapter
without adding lots of noisy warnings to the documentation build.

Signed-off-by: Randy Dunlap 
---
 include/uapi/linux/firewire-cdev.h |   22 ++
 1 file changed, 14 insertions(+), 8 deletions(-)

--- linux-next-20180220.orig/include/uapi/linux/firewire-cdev.h
+++ linux-next-20180220/include/uapi/linux/firewire-cdev.h
@@ -47,11 +47,11 @@
 #define FW_CDEV_EVENT_ISO_INTERRUPT_MULTICHANNEL   0x09
 
 /**
- * struct fw_cdev_event_common - Common part of all fw_cdev_event_ types
+ * struct fw_cdev_event_common - Common part of all fw_cdev_event_* types
  * @closure:   For arbitrary use by userspace
- * @type:  Discriminates the fw_cdev_event_ types
+ * @type:  Discriminates the fw_cdev_event_* types
  *
- * This struct may be used to access generic members of all fw_cdev_event_
+ * This struct may be used to access generic members of all fw_cdev_event_*
  * types regardless of the specific type.
  *
  * Data passed in the @closure field for a request will be returned in the
@@ -123,7 +123,13 @@ struct fw_cdev_event_response {
 
 /**
  * struct fw_cdev_event_request - Old version of _cdev_event_request2
+ * @closure:   See _cdev_event_common; set by %FW_CDEV_IOC_ALLOCATE ioctl
  * @type:  See _cdev_event_common; always %FW_CDEV_EVENT_REQUEST
+ * @tcode: Transaction code of the incoming request
+ * @offset:The offset into the 48-bit per-node address space
+ * @handle:Reference to the kernel-side pending request
+ * @length:Data length, i.e. the request's payload size in bytes
+ * @data:  Incoming data, if any
  *
  * This event is sent instead of _cdev_event_request2 if the kernel or
  * the client implements ABI version <= 3.  _cdev_event_request lacks
@@ -353,7 +359,7 @@ struct fw_cdev_event_phy_packet {
 };
 
 /**
- * union fw_cdev_event - Convenience union of fw_cdev_event_ types
+ * union fw_cdev_event - Convenience union of fw_cdev_event_* types
  * @common:Valid for all types
  * @bus_reset: Valid if @common.type == %FW_CDEV_EVENT_BUS_RESET
  * @response:  Valid if @common.type == %FW_CDEV_EVENT_RESPONSE
@@ -735,7 +741,7 @@ struct fw_cdev_set_iso_channels {
  * @header:Header and payload in case of a transmit context.
  *
  *  fw_cdev_iso_packet is used to describe isochronous packet queues.
- * Use the FW_CDEV_ISO_ macros to fill in @control.
+ * Use the FW_CDEV_ISO_* macros to fill in @control.
  * The @header array is empty in case of receive contexts.
  *
  * Context type %FW_CDEV_ISO_CONTEXT_TRANSMIT:
@@ -842,7 +848,7 @@ struct fw_cdev_queue_iso {
  * the %FW_CDEV_ISO_SYNC bit set
  * @tags:  Tag filter bit mask.  Only valid for isochronous reception.
  * Determines the tag values for which packets will be accepted.
- * Use FW_CDEV_ISO_CONTEXT_MATCH_ macros to set @tags.
+ * Use FW_CDEV_ISO_CONTEXT_MATCH_* macros to set @tags.
  * @handle:Isochronous context handle within which to transmit or receive
  */
 struct fw_cdev_start_iso {
@@ -1009,8 +1015,8 @@ struct fw_cdev_send_stream_packet {
  * on the same card as this device.  After transmission, an
  * %FW_CDEV_EVENT_PHY_PACKET_SENT event is generated.
  *
- * The payload @data[] shall be specified in host byte order.  Usually,
- * @data[1] needs to be the bitwise inverse of @data[0].  VersaPHY packets
+ * The payload @data\[\] shall be specified in host byte order.  Usually,
+ * @data\[1\] needs to be the bitwise inverse of @data\[0\].  VersaPHY packets
  * are an exception to this rule.
  *
  * The ioctl is only permitted on device files which represent a local node.


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

[PATCH] doc: process: Add "Root-caused-by" and "Suggested-by"

[Kees Cook]

As recently pointed out by Linus, "Root-caused-by" is a good tag to include
since it can indicate significantly more work than "just" a Reported-by.
This adds it and "Suggested-by" (which was also missing) to the documented
list of tags. Additionally updates checkpatch.pl to match the process docs.

Signed-off-by: Kees Cook 
---
 Documentation/process/5.Posting.rst | 7 +++
 scripts/checkpatch.pl   | 2 ++
 2 files changed, 9 insertions(+)

diff --git a/Documentation/process/5.Posting.rst 
b/Documentation/process/5.Posting.rst
index 645fa9c7388a..2ff01f76f02a 100644
--- a/Documentation/process/5.Posting.rst
+++ b/Documentation/process/5.Posting.rst
@@ -234,6 +234,13 @@ The tags in common use are:
people who test our code and let us know when things do not work
correctly.
 
+ - Suggested-by: names a person who suggested the solution, but may not
+   have constructed the full patch. A weaker version of `Co-Developed-by`.
+
+ - Root-caused-by: names a person who diagnosed the root cause of a
+   problem. This usually indicates significantly more work than a simple
+   `Reported-by`.
+
  - Cc: the named person received a copy of the patch and had the
opportunity to comment on it.
 
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 3d4040322ae1..a1ab82e70b54 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -464,9 +464,11 @@ our $logFunctions = qr{(?x:
 our $signature_tags = qr{(?xi:
Signed-off-by:|
Acked-by:|
+   Co-Developed-by:|
Tested-by:|
Reviewed-by:|
Reported-by:|
+   Root-caused-by:|
Suggested-by:|
To:|
Cc:
-- 
2.7.4


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

Re: [PATCH v2 7/8] [PATCH 7/8] drivers/hwmon: Add a generic PECI hwmon client driver

[Andrew Lunn]

> But even with this change, it still needs to use delayed creation
> because BMC side kernel doesn't know how many DIMMs are populated on
> a remote server before the remote server completes its memory
> training and testing in BIOS, but it needs to check the remote
> server's CPU temperature as immediate as possible to make
> appropriate thermal control based on the remote CPU's temperature to
> avoid any critical thermal issue. What would be a better solution in
> this case?

You could change this driver so that it supports one DIMM.  Move the
'hotplug' part into another driver which creates and destroys
instances of the hwmon DIMM device as the DIMMS come and go.

Also, do you need to handle CPU hotplug? You could split the CPU
temperature part into a separate hwmon driver? And again create and
destroy devices as CPUs come and go?

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

[PATCH 02/13] docs: Remove metag docs

[James Hogan]

Now that arch/metag/ has been removed, remove Meta architecture specific
documentation from the Documentation/ directory.

Signed-off-by: James Hogan 
Cc: Jonathan Corbet 
Cc: linux-me...@vger.kernel.org
Cc: linux-doc@vger.kernel.org
---
 Documentation/00-INDEX   |   2 -
 Documentation/admin-guide/kernel-parameters.txt  |   4 -
 Documentation/devicetree/bindings/metag/meta.txt |  30 ---
 Documentation/metag/00-INDEX |   4 -
 Documentation/metag/kernel-ABI.txt   | 256 ---
 5 files changed, 296 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/metag/meta.txt
 delete mode 100644 Documentation/metag/00-INDEX
 delete mode 100644 Documentation/metag/kernel-ABI.txt

diff --git a/Documentation/00-INDEX b/Documentation/00-INDEX
index 7f3a0728ccf2..eae1e7193f50 100644
--- a/Documentation/00-INDEX
+++ b/Documentation/00-INDEX
@@ -276,8 +276,6 @@ memory-hotplug.txt
- Hotpluggable memory support, how to use and current status.
 men-chameleon-bus.txt
- info on MEN chameleon bus.
-metag/
-   - directory with info about Linux on Meta architecture.
 mic/
- Intel Many Integrated Core (MIC) architecture device driver.
 mips/
diff --git a/Documentation/admin-guide/kernel-parameters.txt 
b/Documentation/admin-guide/kernel-parameters.txt
index 1d1d53f85ddd..30a8d0635898 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -1347,10 +1347,6 @@
   If specified, z/VM IUCV HVC accepts connections
   from listed z/VM user IDs only.
 
-   hwthread_map=   [METAG] Comma-separated list of Linux cpu id to
-   hardware thread id mappings.
-   Format: :
-
keep_bootcon[KNL]
Do not unregister boot console at start. This is only
useful for debugging when something happens in the 
window
diff --git a/Documentation/devicetree/bindings/metag/meta.txt 
b/Documentation/devicetree/bindings/metag/meta.txt
deleted file mode 100644
index f4457f57ab08..
diff --git a/Documentation/metag/00-INDEX b/Documentation/metag/00-INDEX
deleted file mode 100644
index db11c513bd5c..
diff --git a/Documentation/metag/kernel-ABI.txt 
b/Documentation/metag/kernel-ABI.txt
deleted file mode 100644
index 628216603198..
-- 
2.13.6

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

[PATCH 03/13] docs: Remove remaining references to metag

[James Hogan]

Remove any remaining references to the Meta architecture in
Documentation/, primarily from Documentation/features/.

Signed-off-by: James Hogan 
Cc: Jonathan Corbet 
Cc: linux-me...@vger.kernel.org
Cc: linux-doc@vger.kernel.org
---
 Documentation/dev-tools/kmemleak.rst   | 2 +-
 Documentation/features/core/BPF-JIT/arch-support.txt   | 1 -
 Documentation/features/core/generic-idle-thread/arch-support.txt   | 1 -
 Documentation/features/core/jump-labels/arch-support.txt   | 1 -
 Documentation/features/core/tracehook/arch-support.txt | 1 -
 Documentation/features/debug/KASAN/arch-support.txt| 1 -
 Documentation/features/debug/gcov-profile-all/arch-support.txt | 1 -
 Documentation/features/debug/kgdb/arch-support.txt | 1 -
 Documentation/features/debug/kprobes-on-ftrace/arch-support.txt| 1 -
 Documentation/features/debug/kprobes/arch-support.txt  | 1 -
 Documentation/features/debug/kretprobes/arch-support.txt   | 1 -
 Documentation/features/debug/optprobes/arch-support.txt| 1 -
 Documentation/features/debug/stackprotector/arch-support.txt   | 1 -
 Documentation/features/debug/uprobes/arch-support.txt  | 1 -
 Documentation/features/debug/user-ret-profiler/arch-support.txt| 1 -
 Documentation/features/io/dma-api-debug/arch-support.txt   | 1 -
 Documentation/features/io/dma-contiguous/arch-support.txt  | 1 -
 Documentation/features/io/sg-chain/arch-support.txt| 1 -
 Documentation/features/lib/strncasecmp/arch-support.txt| 1 -
 Documentation/features/locking/cmpxchg-local/arch-support.txt  | 1 -
 Documentation/features/locking/lockdep/arch-support.txt| 1 -
 Documentation/features/locking/queued-rwlocks/arch-support.txt | 1 -
 Documentation/features/locking/queued-spinlocks/arch-support.txt   | 1 -
 Documentation/features/locking/rwsem-optimized/arch-support.txt| 1 -
 Documentation/features/perf/kprobes-event/arch-support.txt | 1 -
 Documentation/features/perf/perf-regs/arch-support.txt | 1 -
 Documentation/features/perf/perf-stackdump/arch-support.txt| 1 -
 Documentation/features/sched/membarrier-sync-core/arch-support.txt | 1 -
 Documentation/features/sched/numa-balancing/arch-support.txt   | 1 -
 Documentation/features/seccomp/seccomp-filter/arch-support.txt | 1 -
 Documentation/features/time/arch-tick-broadcast/arch-support.txt   | 1 -
 Documentation/features/time/clockevents/arch-support.txt   | 1 -
 Documentation/features/time/context-tracking/arch-support.txt  | 1 -
 Documentation/features/time/irq-time-acct/arch-support.txt | 1 -
 Documentation/features/time/modern-timekeeping/arch-support.txt| 1 -
 Documentation/features/time/virt-cpuacct/arch-support.txt  | 1 -
 Documentation/features/vm/ELF-ASLR/arch-support.txt| 1 -
 Documentation/features/vm/PG_uncached/arch-support.txt | 1 -
 Documentation/features/vm/THP/arch-support.txt | 1 -
 Documentation/features/vm/TLB/arch-support.txt | 1 -
 Documentation/features/vm/huge-vmap/arch-support.txt   | 1 -
 Documentation/features/vm/ioremap_prot/arch-support.txt| 1 -
 Documentation/features/vm/numa-memblock/arch-support.txt   | 1 -
 Documentation/features/vm/pte_special/arch-support.txt | 1 -
 44 files changed, 1 insertion(+), 44 deletions(-)

diff --git a/Documentation/dev-tools/kmemleak.rst 
b/Documentation/dev-tools/kmemleak.rst
index cb8862659178..e6f51260ff32 100644
--- a/Documentation/dev-tools/kmemleak.rst
+++ b/Documentation/dev-tools/kmemleak.rst
@@ -8,7 +8,7 @@ with the difference that the orphan objects are not freed but 
only
 reported via /sys/kernel/debug/kmemleak. A similar method is used by the
 Valgrind tool (``memcheck --leak-check``) to detect the memory leaks in
 user-space applications.
-Kmemleak is supported on x86, arm, powerpc, sparc, sh, microblaze, ppc, mips, 
s390, metag and tile.
+Kmemleak is supported on x86, arm, powerpc, sparc, sh, microblaze, ppc, mips, 
s390 and tile.
 
 Usage
 -
diff --git a/Documentation/features/core/BPF-JIT/arch-support.txt 
b/Documentation/features/core/BPF-JIT/arch-support.txt
index 5575d2d09625..b0634ec01881 100644
--- a/Documentation/features/core/BPF-JIT/arch-support.txt
+++ b/Documentation/features/core/BPF-JIT/arch-support.txt
@@ -19,7 +19,6 @@
 |ia64: | TODO |
 |m32r: | TODO |
 |m68k: | TODO |
-|   metag: | TODO |
 |  microblaze: | TODO |
 |mips: |  ok  |
 | mn10300: | TODO |
diff --git a/Documentation/features/core/generic-idle-thread/arch-support.txt 
b/Documentation/features/core/generic-idle-thread/arch-support.txt
index abb5f271a792..e2a1a385efd3 100644
--- 

Re: [PATCH v2 7/8] [PATCH 7/8] drivers/hwmon: Add a generic PECI hwmon client driver

[Jae Hyun Yoo]

On 2/21/2018 1:48 PM, Guenter Roeck wrote:

On Wed, Feb 21, 2018 at 01:24:48PM -0800, Jae Hyun Yoo wrote:

Hi Guenter,

Thanks for sharing your time to review this code. Please check my answers
inline.

On 2/21/2018 10:26 AM, Guenter Roeck wrote:

On Wed, Feb 21, 2018 at 08:16:05AM -0800, Jae Hyun Yoo wrote:

This commit adds a generic PECI hwmon client driver implementation.

Signed-off-by: Jae Hyun Yoo 
---
  drivers/hwmon/Kconfig  |  10 +
  drivers/hwmon/Makefile |   1 +
  drivers/hwmon/peci-hwmon.c | 928 +
  3 files changed, 939 insertions(+)
  create mode 100644 drivers/hwmon/peci-hwmon.c

diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index ef23553ff5cb..f22e0c31f597 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -1246,6 +1246,16 @@ config SENSORS_NCT7904
  This driver can also be built as a module.  If so, the module
  will be called nct7904.
+config SENSORS_PECI_HWMON
+   tristate "PECI hwmon support"
+   depends on PECI
+   help
+ If you say yes here you get support for the generic PECI hwmon
+ driver.
+
+ This driver can also be built as a module.  If so, the module
+ will be called peci-hwmon.
+
  config SENSORS_NSA320
tristate "ZyXEL NSA320 and compatible fan speed and temperature sensors"
depends on GPIOLIB && OF
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index f814b4ace138..946f54b168e5 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -135,6 +135,7 @@ obj-$(CONFIG_SENSORS_NCT7802)   += nct7802.o
  obj-$(CONFIG_SENSORS_NCT7904) += nct7904.o
  obj-$(CONFIG_SENSORS_NSA320)  += nsa320-hwmon.o
  obj-$(CONFIG_SENSORS_NTC_THERMISTOR)  += ntc_thermistor.o
+obj-$(CONFIG_SENSORS_PECI_HWMON)   += peci-hwmon.o
  obj-$(CONFIG_SENSORS_PC87360) += pc87360.o
  obj-$(CONFIG_SENSORS_PC87427) += pc87427.o
  obj-$(CONFIG_SENSORS_PCF8591) += pcf8591.o
diff --git a/drivers/hwmon/peci-hwmon.c b/drivers/hwmon/peci-hwmon.c
new file mode 100644
index ..edd27744adcb
--- /dev/null
+++ b/drivers/hwmon/peci-hwmon.c
@@ -0,0 +1,928 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (c) 2018 Intel Corporation
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define DIMM_SLOT_NUMS_MAX12  /* Max DIMM numbers (channel ranks x 2) */
+#define CORE_NUMS_MAX 28  /* Max core numbers (max on SKX Platinum) */
+#define TEMP_TYPE_PECI6   /* Sensor type 6: Intel PECI */
+
+#define CORE_TEMP_ATTRS   5
+#define DIMM_TEMP_ATTRS   2
+#define ATTR_NAME_LEN 24
+
+#define DEFAULT_ATTR_GRP_NUMS 5
+
+#define UPDATE_INTERVAL_MIN   HZ
+#define DIMM_MASK_CHECK_DELAY msecs_to_jiffies(5000)
+
+enum sign {
+   POS,
+   NEG
+};
+
+struct temp_data {
+   bool valid;
+   s32  value;
+   unsigned long last_updated;
+};
+
+struct temp_group {
+   struct temp_data tjmax;
+   struct temp_data tcontrol;
+   struct temp_data tthrottle;
+   struct temp_data dts_margin;
+   struct temp_data die;
+   struct temp_data core[CORE_NUMS_MAX];
+   struct temp_data dimm[DIMM_SLOT_NUMS_MAX];
+};
+
+struct core_temp_group {
+   struct sensor_device_attribute sd_attrs[CORE_TEMP_ATTRS];
+   char attr_name[CORE_TEMP_ATTRS][ATTR_NAME_LEN];
+   struct attribute *attrs[CORE_TEMP_ATTRS + 1];
+   struct attribute_group attr_group;
+};
+
+struct dimm_temp_group {
+   struct sensor_device_attribute sd_attrs[DIMM_TEMP_ATTRS];
+   char attr_name[DIMM_TEMP_ATTRS][ATTR_NAME_LEN];
+   struct attribute *attrs[DIMM_TEMP_ATTRS + 1];
+   struct attribute_group attr_group;
+};
+
+struct peci_hwmon {
+   struct peci_client *client;
+   struct device *dev;
+   struct device *hwmon_dev;
+   struct workqueue_struct *work_queue;
+   struct delayed_work work_handler;
+   char name[PECI_NAME_SIZE];
+   struct temp_group temp;
+   u8 addr;
+   uint cpu_no;
+   u32 core_mask;
+   u32 dimm_mask;
+   const struct attribute_group *core_attr_groups[CORE_NUMS_MAX + 1];
+   const struct attribute_group *dimm_attr_groups[DIMM_SLOT_NUMS_MAX + 1];
+   uint global_idx;
+   uint core_idx;
+   uint dimm_idx;
+};
+
+enum label {
+   L_DIE,
+   L_DTS,
+   L_TCONTROL,
+   L_TTHROTTLE,
+   L_TJMAX,
+   L_MAX
+};
+
+static const char *peci_label[L_MAX] = {
+   "Die\n",
+   "DTS margin to Tcontrol\n",
+   "Tcontrol\n",
+   "Tthrottle\n",
+   "Tjmax\n",
+};
+
+static int send_peci_cmd(struct peci_hwmon *priv, enum peci_cmd cmd, void *msg)
+{
+   return peci_command(priv->client->adapter, cmd, msg);
+}
+
+static int need_update(struct temp_data *temp)
+{
+   if (temp->valid &&
+   time_before(jiffies, temp->last_updated + UPDATE_INTERVAL_MIN))
+   return 0;
+
+   return 1;
+}
+

Re: [PATCH v2 7/8] [PATCH 7/8] drivers/hwmon: Add a generic PECI hwmon client driver

[Guenter Roeck]

On Wed, Feb 21, 2018 at 08:16:05AM -0800, Jae Hyun Yoo wrote:
> This commit adds a generic PECI hwmon client driver implementation.
> 
> Signed-off-by: Jae Hyun Yoo 
> ---
>  drivers/hwmon/Kconfig  |  10 +
>  drivers/hwmon/Makefile |   1 +
>  drivers/hwmon/peci-hwmon.c | 928 
> +
>  3 files changed, 939 insertions(+)
>  create mode 100644 drivers/hwmon/peci-hwmon.c
> 
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index ef23553ff5cb..f22e0c31f597 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -1246,6 +1246,16 @@ config SENSORS_NCT7904
> This driver can also be built as a module.  If so, the module
> will be called nct7904.
>  
> +config SENSORS_PECI_HWMON
> + tristate "PECI hwmon support"
> + depends on PECI
> + help
> +   If you say yes here you get support for the generic PECI hwmon
> +   driver.
> +
> +   This driver can also be built as a module.  If so, the module
> +   will be called peci-hwmon.
> +
>  config SENSORS_NSA320
>   tristate "ZyXEL NSA320 and compatible fan speed and temperature sensors"
>   depends on GPIOLIB && OF
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index f814b4ace138..946f54b168e5 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -135,6 +135,7 @@ obj-$(CONFIG_SENSORS_NCT7802) += nct7802.o
>  obj-$(CONFIG_SENSORS_NCT7904)+= nct7904.o
>  obj-$(CONFIG_SENSORS_NSA320) += nsa320-hwmon.o
>  obj-$(CONFIG_SENSORS_NTC_THERMISTOR) += ntc_thermistor.o
> +obj-$(CONFIG_SENSORS_PECI_HWMON) += peci-hwmon.o
>  obj-$(CONFIG_SENSORS_PC87360)+= pc87360.o
>  obj-$(CONFIG_SENSORS_PC87427)+= pc87427.o
>  obj-$(CONFIG_SENSORS_PCF8591)+= pcf8591.o
> diff --git a/drivers/hwmon/peci-hwmon.c b/drivers/hwmon/peci-hwmon.c
> new file mode 100644
> index ..edd27744adcb
> --- /dev/null
> +++ b/drivers/hwmon/peci-hwmon.c
> @@ -0,0 +1,928 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (c) 2018 Intel Corporation
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define DIMM_SLOT_NUMS_MAX12  /* Max DIMM numbers (channel ranks x 2) */
> +#define CORE_NUMS_MAX 28  /* Max core numbers (max on SKX Platinum) 
> */
> +#define TEMP_TYPE_PECI6   /* Sensor type 6: Intel PECI */
> +
> +#define CORE_TEMP_ATTRS   5
> +#define DIMM_TEMP_ATTRS   2
> +#define ATTR_NAME_LEN 24
> +
> +#define DEFAULT_ATTR_GRP_NUMS 5
> +
> +#define UPDATE_INTERVAL_MIN   HZ
> +#define DIMM_MASK_CHECK_DELAY msecs_to_jiffies(5000)
> +
> +enum sign {
> + POS,
> + NEG
> +};
> +
> +struct temp_data {
> + bool valid;
> + s32  value;
> + unsigned long last_updated;
> +};
> +
> +struct temp_group {
> + struct temp_data tjmax;
> + struct temp_data tcontrol;
> + struct temp_data tthrottle;
> + struct temp_data dts_margin;
> + struct temp_data die;
> + struct temp_data core[CORE_NUMS_MAX];
> + struct temp_data dimm[DIMM_SLOT_NUMS_MAX];
> +};
> +
> +struct core_temp_group {
> + struct sensor_device_attribute sd_attrs[CORE_TEMP_ATTRS];
> + char attr_name[CORE_TEMP_ATTRS][ATTR_NAME_LEN];
> + struct attribute *attrs[CORE_TEMP_ATTRS + 1];
> + struct attribute_group attr_group;
> +};
> +
> +struct dimm_temp_group {
> + struct sensor_device_attribute sd_attrs[DIMM_TEMP_ATTRS];
> + char attr_name[DIMM_TEMP_ATTRS][ATTR_NAME_LEN];
> + struct attribute *attrs[DIMM_TEMP_ATTRS + 1];
> + struct attribute_group attr_group;
> +};
> +
> +struct peci_hwmon {
> + struct peci_client *client;
> + struct device *dev;
> + struct device *hwmon_dev;
> + struct workqueue_struct *work_queue;
> + struct delayed_work work_handler;
> + char name[PECI_NAME_SIZE];
> + struct temp_group temp;
> + u8 addr;
> + uint cpu_no;
> + u32 core_mask;
> + u32 dimm_mask;
> + const struct attribute_group *core_attr_groups[CORE_NUMS_MAX + 1];
> + const struct attribute_group *dimm_attr_groups[DIMM_SLOT_NUMS_MAX + 1];
> + uint global_idx;
> + uint core_idx;
> + uint dimm_idx;
> +};
> +
> +enum label {
> + L_DIE,
> + L_DTS,
> + L_TCONTROL,
> + L_TTHROTTLE,
> + L_TJMAX,
> + L_MAX
> +};
> +
> +static const char *peci_label[L_MAX] = {
> + "Die\n",
> + "DTS margin to Tcontrol\n",
> + "Tcontrol\n",
> + "Tthrottle\n",
> + "Tjmax\n",
> +};
> +
> +static int send_peci_cmd(struct peci_hwmon *priv, enum peci_cmd cmd, void 
> *msg)
> +{
> + return peci_command(priv->client->adapter, cmd, msg);
> +}
> +
> +static int need_update(struct temp_data *temp)
> +{
> + if (temp->valid &&
> + time_before(jiffies, temp->last_updated + UPDATE_INTERVAL_MIN))
> + return 0;
> +
> + return 1;
> +}
> +
> +static s32 

Re: [PATCH v2 1/8] [PATCH 1/8] drivers/peci: Add support for PECI bus driver core

[Jae Hyun Yoo]

On 2/21/2018 1:51 PM, Andrew Lunn wrote:

Is there a real need to do transfers in atomic context, or with
interrupts disabled?



Actually, no. Generally, this function will be called in sleep-able context
so this code is for an exceptional case handling.

I'll rewrite this code like below:
if (in_atomic() || irqs_disabled()) {
dev_dbg(>dev,
"xfer in non-sleepable context is not supported\n");
return -EWOULDBLOCK;
}


I would not even do that. Just add a call to
might_sleep(). CONFIG_DEBUG_ATOMIC_SLEEP will then find bad calls.



Thanks for the suggestion. I've learned one thing. :)


+static int peci_ioctl_get_temp(struct peci_adapter *adapter, void *vmsg)
+{
+   struct peci_get_temp_msg *umsg = vmsg;
+   struct peci_xfer_msg msg;
+   int rc;
+


Is this getting the temperature?



Yes, this is getting the 'die' temperature of a processor package.
  
So the hwmon driver provides this. No need to have both.




This this common API in core driver of PECI bus. The hwmon is also uses 
it through peci_command call.



+static long peci_ioctl(struct file *file, unsigned int iocmd, unsigned long 
arg)
+{
+   struct peci_adapter *adapter = file->private_data;
+   void __user *argp = (void __user *)arg;
+   unsigned int msg_len;
+   enum peci_cmd cmd;
+   u8 *msg;
+   int rc = 0;
+
+   dev_dbg(>dev, "ioctl, cmd=0x%x, arg=0x%lx\n", iocmd, arg);
+
+   switch (iocmd) {
+   case PECI_IOC_PING:
+   case PECI_IOC_GET_DIB:
+   case PECI_IOC_GET_TEMP:
+   case PECI_IOC_RD_PKG_CFG:
+   case PECI_IOC_WR_PKG_CFG:
+   case PECI_IOC_RD_IA_MSR:
+   case PECI_IOC_RD_PCI_CFG:
+   case PECI_IOC_RD_PCI_CFG_LOCAL:
+   case PECI_IOC_WR_PCI_CFG_LOCAL:
+   cmd = _IOC_TYPE(iocmd) - PECI_IOC_BASE;
+   msg_len = _IOC_SIZE(iocmd);
+   break;


Adding new ioctl calls is pretty frowned up. Can you export this info
via /sysfs?



Most of these are not simple IOs so ioctl is better suited, I think.


Lets see what other reviewers say, but i think ioctls are
wrong.

  Andrew


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

Re: [PATCH v2 1/8] [PATCH 1/8] drivers/peci: Add support for PECI bus driver core

[Andrew Lunn]

> >Is there a real need to do transfers in atomic context, or with
> >interrupts disabled?
> >
> 
> Actually, no. Generally, this function will be called in sleep-able context
> so this code is for an exceptional case handling.
> 
> I'll rewrite this code like below:
>   if (in_atomic() || irqs_disabled()) {
>   dev_dbg(>dev,
>   "xfer in non-sleepable context is not supported\n");
>   return -EWOULDBLOCK;
>   }

I would not even do that. Just add a call to
might_sleep(). CONFIG_DEBUG_ATOMIC_SLEEP will then find bad calls.

> >>+static int peci_ioctl_get_temp(struct peci_adapter *adapter, void *vmsg)
> >>+{
> >>+   struct peci_get_temp_msg *umsg = vmsg;
> >>+   struct peci_xfer_msg msg;
> >>+   int rc;
> >>+
> >
> >Is this getting the temperature?
> >
> 
> Yes, this is getting the 'die' temperature of a processor package.
 
So the hwmon driver provides this. No need to have both.

> >>+static long peci_ioctl(struct file *file, unsigned int iocmd, unsigned 
> >>long arg)
> >>+{
> >>+   struct peci_adapter *adapter = file->private_data;
> >>+   void __user *argp = (void __user *)arg;
> >>+   unsigned int msg_len;
> >>+   enum peci_cmd cmd;
> >>+   u8 *msg;
> >>+   int rc = 0;
> >>+
> >>+   dev_dbg(>dev, "ioctl, cmd=0x%x, arg=0x%lx\n", iocmd, arg);
> >>+
> >>+   switch (iocmd) {
> >>+   case PECI_IOC_PING:
> >>+   case PECI_IOC_GET_DIB:
> >>+   case PECI_IOC_GET_TEMP:
> >>+   case PECI_IOC_RD_PKG_CFG:
> >>+   case PECI_IOC_WR_PKG_CFG:
> >>+   case PECI_IOC_RD_IA_MSR:
> >>+   case PECI_IOC_RD_PCI_CFG:
> >>+   case PECI_IOC_RD_PCI_CFG_LOCAL:
> >>+   case PECI_IOC_WR_PCI_CFG_LOCAL:
> >>+   cmd = _IOC_TYPE(iocmd) - PECI_IOC_BASE;
> >>+   msg_len = _IOC_SIZE(iocmd);
> >>+   break;
> >
> >Adding new ioctl calls is pretty frowned up. Can you export this info
> >via /sysfs?
> >
> 
> Most of these are not simple IOs so ioctl is better suited, I think.

Lets see what other reviewers say, but i think ioctls are
wrong.

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

Re: [PATCH v2 7/8] [PATCH 7/8] drivers/hwmon: Add a generic PECI hwmon client driver

[Guenter Roeck]

On Wed, Feb 21, 2018 at 01:24:48PM -0800, Jae Hyun Yoo wrote:
> Hi Guenter,
> 
> Thanks for sharing your time to review this code. Please check my answers
> inline.
> 
> On 2/21/2018 10:26 AM, Guenter Roeck wrote:
> >On Wed, Feb 21, 2018 at 08:16:05AM -0800, Jae Hyun Yoo wrote:
> >>This commit adds a generic PECI hwmon client driver implementation.
> >>
> >>Signed-off-by: Jae Hyun Yoo 
> >>---
> >>  drivers/hwmon/Kconfig  |  10 +
> >>  drivers/hwmon/Makefile |   1 +
> >>  drivers/hwmon/peci-hwmon.c | 928 
> >> +
> >>  3 files changed, 939 insertions(+)
> >>  create mode 100644 drivers/hwmon/peci-hwmon.c
> >>
> >>diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> >>index ef23553ff5cb..f22e0c31f597 100644
> >>--- a/drivers/hwmon/Kconfig
> >>+++ b/drivers/hwmon/Kconfig
> >>@@ -1246,6 +1246,16 @@ config SENSORS_NCT7904
> >>  This driver can also be built as a module.  If so, the module
> >>  will be called nct7904.
> >>+config SENSORS_PECI_HWMON
> >>+   tristate "PECI hwmon support"
> >>+   depends on PECI
> >>+   help
> >>+ If you say yes here you get support for the generic PECI hwmon
> >>+ driver.
> >>+
> >>+ This driver can also be built as a module.  If so, the module
> >>+ will be called peci-hwmon.
> >>+
> >>  config SENSORS_NSA320
> >>tristate "ZyXEL NSA320 and compatible fan speed and temperature sensors"
> >>depends on GPIOLIB && OF
> >>diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> >>index f814b4ace138..946f54b168e5 100644
> >>--- a/drivers/hwmon/Makefile
> >>+++ b/drivers/hwmon/Makefile
> >>@@ -135,6 +135,7 @@ obj-$(CONFIG_SENSORS_NCT7802)   += nct7802.o
> >>  obj-$(CONFIG_SENSORS_NCT7904) += nct7904.o
> >>  obj-$(CONFIG_SENSORS_NSA320)  += nsa320-hwmon.o
> >>  obj-$(CONFIG_SENSORS_NTC_THERMISTOR)  += ntc_thermistor.o
> >>+obj-$(CONFIG_SENSORS_PECI_HWMON)   += peci-hwmon.o
> >>  obj-$(CONFIG_SENSORS_PC87360) += pc87360.o
> >>  obj-$(CONFIG_SENSORS_PC87427) += pc87427.o
> >>  obj-$(CONFIG_SENSORS_PCF8591) += pcf8591.o
> >>diff --git a/drivers/hwmon/peci-hwmon.c b/drivers/hwmon/peci-hwmon.c
> >>new file mode 100644
> >>index ..edd27744adcb
> >>--- /dev/null
> >>+++ b/drivers/hwmon/peci-hwmon.c
> >>@@ -0,0 +1,928 @@
> >>+// SPDX-License-Identifier: GPL-2.0
> >>+// Copyright (c) 2018 Intel Corporation
> >>+
> >>+#include 
> >>+#include 
> >>+#include 
> >>+#include 
> >>+#include 
> >>+#include 
> >>+#include 
> >>+#include 
> >>+
> >>+#define DIMM_SLOT_NUMS_MAX12  /* Max DIMM numbers (channel ranks x 2) 
> >>*/
> >>+#define CORE_NUMS_MAX 28  /* Max core numbers (max on SKX 
> >>Platinum) */
> >>+#define TEMP_TYPE_PECI6   /* Sensor type 6: Intel PECI */
> >>+
> >>+#define CORE_TEMP_ATTRS   5
> >>+#define DIMM_TEMP_ATTRS   2
> >>+#define ATTR_NAME_LEN 24
> >>+
> >>+#define DEFAULT_ATTR_GRP_NUMS 5
> >>+
> >>+#define UPDATE_INTERVAL_MIN   HZ
> >>+#define DIMM_MASK_CHECK_DELAY msecs_to_jiffies(5000)
> >>+
> >>+enum sign {
> >>+   POS,
> >>+   NEG
> >>+};
> >>+
> >>+struct temp_data {
> >>+   bool valid;
> >>+   s32  value;
> >>+   unsigned long last_updated;
> >>+};
> >>+
> >>+struct temp_group {
> >>+   struct temp_data tjmax;
> >>+   struct temp_data tcontrol;
> >>+   struct temp_data tthrottle;
> >>+   struct temp_data dts_margin;
> >>+   struct temp_data die;
> >>+   struct temp_data core[CORE_NUMS_MAX];
> >>+   struct temp_data dimm[DIMM_SLOT_NUMS_MAX];
> >>+};
> >>+
> >>+struct core_temp_group {
> >>+   struct sensor_device_attribute sd_attrs[CORE_TEMP_ATTRS];
> >>+   char attr_name[CORE_TEMP_ATTRS][ATTR_NAME_LEN];
> >>+   struct attribute *attrs[CORE_TEMP_ATTRS + 1];
> >>+   struct attribute_group attr_group;
> >>+};
> >>+
> >>+struct dimm_temp_group {
> >>+   struct sensor_device_attribute sd_attrs[DIMM_TEMP_ATTRS];
> >>+   char attr_name[DIMM_TEMP_ATTRS][ATTR_NAME_LEN];
> >>+   struct attribute *attrs[DIMM_TEMP_ATTRS + 1];
> >>+   struct attribute_group attr_group;
> >>+};
> >>+
> >>+struct peci_hwmon {
> >>+   struct peci_client *client;
> >>+   struct device *dev;
> >>+   struct device *hwmon_dev;
> >>+   struct workqueue_struct *work_queue;
> >>+   struct delayed_work work_handler;
> >>+   char name[PECI_NAME_SIZE];
> >>+   struct temp_group temp;
> >>+   u8 addr;
> >>+   uint cpu_no;
> >>+   u32 core_mask;
> >>+   u32 dimm_mask;
> >>+   const struct attribute_group *core_attr_groups[CORE_NUMS_MAX + 1];
> >>+   const struct attribute_group *dimm_attr_groups[DIMM_SLOT_NUMS_MAX + 1];
> >>+   uint global_idx;
> >>+   uint core_idx;
> >>+   uint dimm_idx;
> >>+};
> >>+
> >>+enum label {
> >>+   L_DIE,
> >>+   L_DTS,
> >>+   L_TCONTROL,
> >>+   L_TTHROTTLE,
> >>+   L_TJMAX,
> >>+   L_MAX
> >>+};
> >>+
> >>+static const char *peci_label[L_MAX] = {
> >>+   "Die\n",
> >>+   "DTS margin to Tcontrol\n",
> >>+   "Tcontrol\n",
> >>+   "Tthrottle\n",
> >>+   "Tjmax\n",
> >>+};
> >>+
> >>+static int 

Re: [PATCH 00/23] kconfig: move compiler capability tests to Kconfig

[Ulf Magnusson]

On Wed, Feb 21, 2018 at 09:57:03PM +0900, Masahiro Yamada wrote:
> 2018-02-21 19:52 GMT+09:00 Arnd Bergmann :
> > On Wed, Feb 21, 2018 at 11:20 AM, Masahiro Yamada
> >  wrote:
> >> 2018-02-21 18:56 GMT+09:00 Arnd Bergmann :
> >>> On Wed, Feb 21, 2018 at 8:38 AM, Masahiro Yamada
> >>>  wrote:
>  2018-02-20 0:18 GMT+09:00 Ulf Magnusson :
> >>
> >> Let me clarify my concern.
> >>
> >> When we test the compiler flag, is there a case
> >> where a particular flag depends on -m{32,64} ?
> >>
> >> For example, is there a compiler that supports -fstack-protector
> >> for 64bit mode, but unsupports it for 32bit mode?
> >>
> >>   $(cc-option -m32) ->  y
> >>   $(cc-option -m64) ->  y
> >>   $(cc-option -fstack-protector)->  y
> >>   $(cc-option -m32 -fstack-protector)   ->  n
> >>   $(cc-option -m64 -fstack-protector)   ->  y
> >>
> >> I guess this is unlikely to happen,
> >> but I am not whether it is zero possibility.
> >>
> >> If this could happen,
> >> $(cc-option ) must be evaluated together with
> >> correct bi-arch option (either -m32 or -m64).
> >>
> >>
> >> Currently, -m32/-m64 is specified in Makefile,
> >> but we are moving compiler tests to Kconfig
> >> and, CONFIG_64BIT can be dynamically toggled in Kconfig.
> >
> > I don't think it can happen for this particular combination (stack protector
> > and word size), but I'm sure we'll eventually run into options that
> > need to be tested in combination. For the current CFLAGS_KERNEL
> > setting, we definitely have the case of needing the variables to be
> > evaluated in a specific order.
> >
> 
> 
> 
> 
> I was thinking of how we can handle complex cases
> in the current approach.
> 
> 
> 
> (Case 1)
> 
> Compiler flag -foo and -bar interacts, so
> we also need to check the combination of the two.
> 
> 
> config CC_HAS_FOO
> def_bool $(cc-option -foo)
> 
> config CC_HAS_BAR
> def_bool $(cc-option -bar)
> 
> config CC_HAS_FOO_WITH_BAR
> def_bool $(cc-option -foo -bar)
> 
> 
> 
> (Case 2)
> Compiler flag -foo is sensitive to word-size.
> So, we need to test this option together with -m32/-m64.
> User can toggle CONFIG_64BIT, like i386/x86_64.
> 
> 
> config CC_NEEDS_M64
>   def_bool $(cc-option -m64) && 64BIT
> 
> config CC_NEEDS_M32
>   def_bool $(cc-option -m32) && !64BIT
> 
> config CC_HAS_FOO
>  bool
>  default $(cc-option -m64 -foo) if CC_NEEDS_M64
>  default $(cc-option -m32 -foo) if CC_NEEDS_M32
>  default $(cc-option -foo)
> 
> 
> 
> (Case 3)
> Compiler flag -foo is sensitive to endian-ness.
> 
> 
> config CC_NEEDS_BIG_ENDIAN
>   def_bool $(cc-option -mbig-endian) && CPU_BIG_ENDIAN
> 
> config CC_NEEDS_LITTLE_ENDIAN
>   def_bool $(cc-option -mlittle-endian) && CPU_LITTLE_ENDIAN
> 
> config CC_HAS_FOO
>  bool
>  default $(cc-option -mbig-endian -foo) if CC_NEEDS_BIG_ENDIAN
>  default $(cc-option -mlittle-endian -foo) if CC_NEEDS_LITTLE_ENDIAN
>  default $(cc-option -foo)
> 
> 
> 
> 
> Hmm, I think I can implement those somehow.
> But, I hope we do not have many instances like this...
> 
> 
> If you know more naive cases, please share your knowledge.
> 
> Thanks!
> 
> 
> -- 
> Best Regards
> Masahiro Yamada

Would get pretty bad if a test needs to consider multiple symbols.
Exponential explosion there...


I thought some more about the implementation of dynamic (post-parsing)
functions to see how bad it would get with the current implementation.

Some background on how things work now:

  1. All expression operands in Kconfig are symbols.

  2. Returning '$ENV' or '$(fn foo)' as a T_WORD during parsing gets
 you symbols with those strings as names and S_UNKNOWN type (because
 they act like references to undefined symbols).

  3. For "foo-$(fn foo)", you also get a symbol with that string as its
 name and S_UNKNOWN type (stored among the SYMBOL_CONST symbols)

  4. Symbols with S_UNKNOWN type get their name as their string value,
 and the tristate value n.

So, if you do string expansion on the names of symbols with S_UNKNOWN
type in sym_calc_value(), you're almost there with the current
implementation, except for the tristate case.

Maybe you could set the tristate value of S_UNKNOWN symbols depending on
the string value you end up with. Things are getting pretty confusing at
that point.

Could have something like S_DYNAMIC as well. More Kconfig complexity...

Then there's other complications:

  1. SYMBOL_CONST is no longer constant.

  2. Dependency loop detection needs to consider symbol references
 within strings.

  3. Dependency loop detection relies on static knowledge of what
 symbols a symbol depends on. That might get messy for certain
 expansions, though it might be things you wouldn't do in practice.

  4. Symbols still need to be 

Re: [PATCH v2 7/8] [PATCH 7/8] drivers/hwmon: Add a generic PECI hwmon client driver

[Jae Hyun Yoo]

Hi Guenter,

Thanks for sharing your time to review this code. Please check my 
answers inline.


On 2/21/2018 10:26 AM, Guenter Roeck wrote:

On Wed, Feb 21, 2018 at 08:16:05AM -0800, Jae Hyun Yoo wrote:

This commit adds a generic PECI hwmon client driver implementation.

Signed-off-by: Jae Hyun Yoo 
---
  drivers/hwmon/Kconfig  |  10 +
  drivers/hwmon/Makefile |   1 +
  drivers/hwmon/peci-hwmon.c | 928 +
  3 files changed, 939 insertions(+)
  create mode 100644 drivers/hwmon/peci-hwmon.c

diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index ef23553ff5cb..f22e0c31f597 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -1246,6 +1246,16 @@ config SENSORS_NCT7904
  This driver can also be built as a module.  If so, the module
  will be called nct7904.
  
+config SENSORS_PECI_HWMON

+   tristate "PECI hwmon support"
+   depends on PECI
+   help
+ If you say yes here you get support for the generic PECI hwmon
+ driver.
+
+ This driver can also be built as a module.  If so, the module
+ will be called peci-hwmon.
+
  config SENSORS_NSA320
tristate "ZyXEL NSA320 and compatible fan speed and temperature sensors"
depends on GPIOLIB && OF
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index f814b4ace138..946f54b168e5 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -135,6 +135,7 @@ obj-$(CONFIG_SENSORS_NCT7802)   += nct7802.o
  obj-$(CONFIG_SENSORS_NCT7904) += nct7904.o
  obj-$(CONFIG_SENSORS_NSA320)  += nsa320-hwmon.o
  obj-$(CONFIG_SENSORS_NTC_THERMISTOR)  += ntc_thermistor.o
+obj-$(CONFIG_SENSORS_PECI_HWMON)   += peci-hwmon.o
  obj-$(CONFIG_SENSORS_PC87360) += pc87360.o
  obj-$(CONFIG_SENSORS_PC87427) += pc87427.o
  obj-$(CONFIG_SENSORS_PCF8591) += pcf8591.o
diff --git a/drivers/hwmon/peci-hwmon.c b/drivers/hwmon/peci-hwmon.c
new file mode 100644
index ..edd27744adcb
--- /dev/null
+++ b/drivers/hwmon/peci-hwmon.c
@@ -0,0 +1,928 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (c) 2018 Intel Corporation
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define DIMM_SLOT_NUMS_MAX12  /* Max DIMM numbers (channel ranks x 2) */
+#define CORE_NUMS_MAX 28  /* Max core numbers (max on SKX Platinum) */
+#define TEMP_TYPE_PECI6   /* Sensor type 6: Intel PECI */
+
+#define CORE_TEMP_ATTRS   5
+#define DIMM_TEMP_ATTRS   2
+#define ATTR_NAME_LEN 24
+
+#define DEFAULT_ATTR_GRP_NUMS 5
+
+#define UPDATE_INTERVAL_MIN   HZ
+#define DIMM_MASK_CHECK_DELAY msecs_to_jiffies(5000)
+
+enum sign {
+   POS,
+   NEG
+};
+
+struct temp_data {
+   bool valid;
+   s32  value;
+   unsigned long last_updated;
+};
+
+struct temp_group {
+   struct temp_data tjmax;
+   struct temp_data tcontrol;
+   struct temp_data tthrottle;
+   struct temp_data dts_margin;
+   struct temp_data die;
+   struct temp_data core[CORE_NUMS_MAX];
+   struct temp_data dimm[DIMM_SLOT_NUMS_MAX];
+};
+
+struct core_temp_group {
+   struct sensor_device_attribute sd_attrs[CORE_TEMP_ATTRS];
+   char attr_name[CORE_TEMP_ATTRS][ATTR_NAME_LEN];
+   struct attribute *attrs[CORE_TEMP_ATTRS + 1];
+   struct attribute_group attr_group;
+};
+
+struct dimm_temp_group {
+   struct sensor_device_attribute sd_attrs[DIMM_TEMP_ATTRS];
+   char attr_name[DIMM_TEMP_ATTRS][ATTR_NAME_LEN];
+   struct attribute *attrs[DIMM_TEMP_ATTRS + 1];
+   struct attribute_group attr_group;
+};
+
+struct peci_hwmon {
+   struct peci_client *client;
+   struct device *dev;
+   struct device *hwmon_dev;
+   struct workqueue_struct *work_queue;
+   struct delayed_work work_handler;
+   char name[PECI_NAME_SIZE];
+   struct temp_group temp;
+   u8 addr;
+   uint cpu_no;
+   u32 core_mask;
+   u32 dimm_mask;
+   const struct attribute_group *core_attr_groups[CORE_NUMS_MAX + 1];
+   const struct attribute_group *dimm_attr_groups[DIMM_SLOT_NUMS_MAX + 1];
+   uint global_idx;
+   uint core_idx;
+   uint dimm_idx;
+};
+
+enum label {
+   L_DIE,
+   L_DTS,
+   L_TCONTROL,
+   L_TTHROTTLE,
+   L_TJMAX,
+   L_MAX
+};
+
+static const char *peci_label[L_MAX] = {
+   "Die\n",
+   "DTS margin to Tcontrol\n",
+   "Tcontrol\n",
+   "Tthrottle\n",
+   "Tjmax\n",
+};
+
+static int send_peci_cmd(struct peci_hwmon *priv, enum peci_cmd cmd, void *msg)
+{
+   return peci_command(priv->client->adapter, cmd, msg);
+}
+
+static int need_update(struct temp_data *temp)
+{
+   if (temp->valid &&
+   time_before(jiffies, temp->last_updated + UPDATE_INTERVAL_MIN))
+   return 0;
+
+   return 1;
+}
+
+static s32 ten_dot_six_to_millidegree(s32 x)
+{
+   return x) ^ 0x8000) - 0x8000) * 1000 / 

Re: [PATCH v2 1/8] [PATCH 1/8] drivers/peci: Add support for PECI bus driver core

[Jae Hyun Yoo]

On 2/21/2018 9:58 AM, Greg KH wrote:

On Wed, Feb 21, 2018 at 08:15:59AM -0800, Jae Hyun Yoo wrote:

This commit adds driver implementation for PECI bus into linux
driver framework.

Signed-off-by: Jae Hyun Yoo 
---


Why is there no other Intel developers willing to review and sign off on
this patch?  Please get their review first before asking us to do their
work for them :)

thanks,

greg k-h



Hi Greg,

This patch set got our internal review process. Sorry if it's code 
quality is under your expectation but it's the reason why I'm asking you 
to review the code. Could you please share your time to review it?


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

Re: [PATCH v2 2/8] [PATCH 2/8] Documentations: dt-bindings: Add a document of PECI adapter driver for Aspeed AST24xx/25xx SoCs

[Jae Hyun Yoo]

On 2/21/2018 9:13 AM, Andrew Lunn wrote:

On Wed, Feb 21, 2018 at 08:16:00AM -0800, Jae Hyun Yoo wrote:

This commit adds a dt-bindings document of PECI adapter driver for Aspeed
AST24xx/25xx SoCs.


Hi Jae

It would be good to separate this into two. One binding document for a
generic adaptor, with a generic PECI bus, and generic client
devices. List all the properties you expect at the generic level.

Then have an aspeed specific binding for those properties which are
specific to the Aspeed adaptor.



That makes sense. I'll add generic PECI bus/adapter/client and Aspeed 
specific documents as separated.



 Andrew
  



Thanks again for sharing your time to review it. I really appreciate it.

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

Re: [PATCH v2 1/8] [PATCH 1/8] drivers/peci: Add support for PECI bus driver core

[Jae Hyun Yoo]

Hi Andrew,

Thanks for sharing your time to review it. Please check my answers inline.

On 2/21/2018 9:04 AM, Andrew Lunn wrote:

+static int peci_locked_xfer(struct peci_adapter *adapter,
+   struct peci_xfer_msg *msg,
+   bool do_retry,
+   bool has_aw_fcs)
+{
+   ktime_t start, end;
+   s64 elapsed_ms;
+   int rc = 0;
+
+   if (!adapter->xfer) {
+   dev_dbg(>dev, "PECI level transfers not supported\n");
+   return -ENODEV;
+   }
+
+   if (in_atomic() || irqs_disabled()) {


Hi Jae

Is there a real need to do transfers in atomic context, or with
interrupts disabled?



Actually, no. Generally, this function will be called in sleep-able 
context so this code is for an exceptional case handling.


I'll rewrite this code like below:
if (in_atomic() || irqs_disabled()) {
dev_dbg(>dev,
"xfer in non-sleepable context is not supported\n");
return -EWOULDBLOCK;
}

And then, will add a sleep call into the below loop.

I know that in_atomic() call is not recommended in driver code but some 
driver codes still use it since there is no alternative way at this 
time, AFAIK. Please tell me if there is a better solution.



+   rt_mutex_trylock(>bus_lock);
+   if (!rc)
+   return -EAGAIN; /* PECI activity is ongoing */
+   } else {
+   rt_mutex_lock(>bus_lock);
+   }
+
+   if (do_retry)
+   start = ktime_get();
+
+   do {
+   rc = adapter->xfer(adapter, msg);
+
+   if (!do_retry)
+   break;
+
+   /* Per the PECI spec, need to retry commands that return 0x8x */
+   if (!(!rc && ((msg->rx_buf[0] & DEV_PECI_CC_RETRY_ERR_MASK) ==
+ DEV_PECI_CC_TIMEOUT)))
+   break;
+
+   /* Set the retry bit to indicate a retry attempt */
+   msg->tx_buf[1] |= DEV_PECI_RETRY_BIT;
+
+   /* Recalculate the AW FCS if it has one */
+   if (has_aw_fcs)
+   msg->tx_buf[msg->tx_len - 1] = 0x80 ^
+   peci_aw_fcs((u8 *)msg,
+   2 + msg->tx_len);
+
+   /* Retry for at least 250ms before returning an error */
+   end = ktime_get();
+   elapsed_ms = ktime_to_ms(ktime_sub(end, start));
+   if (elapsed_ms >= DEV_PECI_RETRY_TIME_MS) {
+   dev_dbg(>dev, "Timeout retrying xfer!\n");
+   break;
+   }
+   } while (true);


So you busy loop to 1/4 second? How about putting a sleep in here so
other things can be done between each retry.

And should it not return -ETIMEDOUT after that 1/4 second?



Yes, you are right. I'll rewrite this code like below after adding the 
above change:


/**
 * Retry for at least 250ms before returning an error.
 * Retry interval guideline:
 *   No minimum < Retry Interval < No maximum
 *(recommend 10ms)
 */
end = ktime_get();
elapsed_ms = ktime_to_ms(ktime_sub(end, start));
if (elapsed_ms >= DEV_PECI_RETRY_TIME_MS) {
dev_dbg(>dev, "Timeout retrying xfer!\n");
rc = -ETIMEDOUT;
break;
}

usleep_range(DEV_PECI_RETRY_INTERVAL_MS * 1000,
 (DEV_PECI_RETRY_INTERVAL_MS * 1000) + 1000);


+static int peci_scan_cmd_mask(struct peci_adapter *adapter)
+{
+   struct peci_xfer_msg msg;
+   u32 dib;
+   int rc = 0;
+
+   /* Update command mask just once */
+   if (adapter->cmd_mask & BIT(PECI_CMD_PING))
+   return 0;
+
+   msg.addr  = PECI_BASE_ADDR;
+   msg.tx_len= GET_DIB_WR_LEN;
+   msg.rx_len= GET_DIB_RD_LEN;
+   msg.tx_buf[0] = GET_DIB_PECI_CMD;
+
+   rc = peci_xfer(adapter, );
+   if (rc < 0) {
+   dev_dbg(>dev, "PECI xfer error, rc : %d\n", rc);
+   return rc;
+   }
+
+   dib = msg.rx_buf[0] | (msg.rx_buf[1] << 8) |
+ (msg.rx_buf[2] << 16) | (msg.rx_buf[3] << 24);
+
+   /* Check special case for Get DIB command */
+   if (dib == 0x00) {
+   dev_dbg(>dev, "DIB read as 0x00\n");
+   return -1;
+   }
+
+   if (!rc) {
+   /**
+* setting up the supporting commands based on minor rev#
+* see PECI Spec Table 3-1
+*/
+   dib = (dib >> 8) & 0xF;
+
+   if (dib >= 0x1) {
+   adapter->cmd_mask |= BIT(PECI_CMD_RD_PKG_CFG);
+   

Re: [PATCH v2 1/8] [PATCH 1/8] drivers/peci: Add support for PECI bus driver core

[Greg KH]

On Wed, Feb 21, 2018 at 08:15:59AM -0800, Jae Hyun Yoo wrote:
> This commit adds driver implementation for PECI bus into linux
> driver framework.
> 
> Signed-off-by: Jae Hyun Yoo 
> ---

Why is there no other Intel developers willing to review and sign off on
this patch?  Please get their review first before asking us to do their
work for them :)

thanks,

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

[PATCH v12 10/11] sparc64: Add support for ADI (Application Data Integrity)

[Khalid Aziz]

ADI is a new feature supported on SPARC M7 and newer processors to allow
hardware to catch rogue accesses to memory. ADI is supported for data
fetches only and not instruction fetches. An app can enable ADI on its
data pages, set version tags on them and use versioned addresses to
access the data pages. Upper bits of the address contain the version
tag. On M7 processors, upper four bits (bits 63-60) contain the version
tag. If a rogue app attempts to access ADI enabled data pages, its
access is blocked and processor generates an exception. Please see
Documentation/sparc/adi.txt for further details.

This patch extends mprotect to enable ADI (TSTATE.mcde), enable/disable
MCD (Memory Corruption Detection) on selected memory ranges, enable
TTE.mcd in PTEs, return ADI parameters to userspace and save/restore ADI
version tags on page swap out/in or migration. ADI is not enabled by
default for any task. A task must explicitly enable ADI on a memory
range and set version tag for ADI to be effective for the task.

Signed-off-by: Khalid Aziz 
Cc: Khalid Aziz 
Reviewed-by: Anthony Yznaga 
---
v10:
- Added code to return from kernel path to set PSTATE.mcde if
  kernel continues execution in another thread (Suggested by
  Anthony Yznaga)
v9:
- Added code to migrate ADI tags to copy_highpage() to
  ensure tags get copied on page migration
- Improved code to detect underflow and overflow when allocating
  tag storage
v8: 
- Added note to doc about non-faulting loads not triggering
  ADI tag mismatch and more details on special tag values
  of 0x0 and 0xf, as suggested by Anthony Yznaga)
- Added an IPI on mprotect(...PROT_ADI...) call to set
  TSTATE.MCDE on threads running on other processors and
  restore of TSTATE.MCDE on context switch (suggested by
  David Miller)
- Removed restriction on enabling ADI on read-only memory
  (suggested by Anthony Yznaga)
- Changed kzalloc() for tag storage to use GFP_NOWAIT
- Added code to handle overflow and underflow when allocating
  tag storage, as suggested by Anthony Yznaga
- Replaced sun_m7_patch_1insn_range() with sun4v_patch_1insn_range()
  which is functionally identical (suggested by Anthony Yznaga)
- Added membar after restoring ADI tags in copy_user_highpage(),
  as suggested by David Miller

v7:
- Enhanced arch_validate_prot() to enable ADI only on writable
  addresses backed by physical RAM
- Added support for saving/restoring ADI tags for each ADI
  block size address range on a page on swap in/out
- Added code to copy ADI tags on COW
- Updated values for auxiliary vectors to not conflict with
  values on other architectures to avoid conflict in glibc. glibc
  consolidates all auxiliary vectors into its headers and
  duplicate values in consolidated header are problematic
- Disable same page merging on ADI enabled pages since ADI tags
  may not match on pages with identical data
- Broke the patch up further into smaller patches

v6:
- Eliminated instructions to read and write PSTATE as well as
  MCDPER and PMCDPER on every access to userspace addresses
  by setting PSTATE and PMCDPER correctly upon entry into
  kernel. PSTATE.mcde and PMCDPER are set upon entry into
  kernel when running on an M7 processor. PSTATE.mcde being
  set only affects memory accesses that have TTE.mcd set.
  PMCDPER being set only affects writes to memory addresses
  that have TTE.mcd set. This ensures any faults caused by
  ADI tag mismatch on a write are exposed before kernel returns
  to userspace.

v5:
- Fixed indentation issues and instrcuctions in assembly code
- Removed CONFIG_SPARC64 from mdesc.c
- Changed to maintain state of MCDPER register in thread info
  flags as opposed to in mm context. MCDPER is a per-thread
  state and belongs in thread info flag as opposed to mm context
  which is shared across threads. Added comments to clarify this
  is a lazily maintained state and must be updated on context
  switch and copy_process()
- Updated code to use the new arch_do_swap_page() and
  arch_unmap_one() functions

v4:
- Broke patch up into smaller patches

v3:
- Removed CONFIG_SPARC_ADI
- Replaced prctl commands with mprotect
- Added auxiliary vectors for ADI parameters
- Enabled ADI for swappable pages

v2:
- Fixed a build error

 Documentation/sparc/adi.txt | 278 +
 arch/sparc/include/asm/mman.h   |  84 -
 arch/sparc/include/asm/mmu_64.h |  17 ++
 

[PATCH v12 00/11] Application Data Integrity feature introduced by SPARC M7

[Khalid Aziz]

V12 changes:
This series is same as v10 and v11 and was simply rebased on 4.16-rc2
kernel and patch 11 was added to update signal delivery code to use the
new helper functions added by Eric Biederman. Can mm maintainers please
review patches 2, 7, 8 and 9 which are arch independent, and
include/linux/mm.h and mm/ksm.c changes in patch 10 and ack these if
everything looks good? 


SPARC M7 processor adds additional metadata for memory address space
that can be used to secure access to regions of memory. This additional
metadata is implemented as a 4-bit tag attached to each cacheline size
block of memory. A task can set a tag on any number of such blocks.
Access to such block is granted only if the virtual address used to
access that block of memory has the tag encoded in the uppermost 4 bits
of VA. Since sparc processor does not implement all 64 bits of VA, top 4
bits are available for ADI tags. Any mismatch between tag encoded in VA
and tag set on the memory block results in a trap. Tags are verified in
the VA presented to the MMU and tags are associated with the physical
page VA maps on to. If a memory page is swapped out and page frame gets
reused for another task, the tags are lost and hence must be saved when
swapping or migrating the page.

A userspace task enables ADI through mprotect(). This patch series adds
a page protection bit PROT_ADI and a corresponding VMA flag
VM_SPARC_ADI. VM_SPARC_ADI is used to trigger setting TTE.mcd bit in the
sparc pte that enables ADI checking on the corresponding page. MMU
validates the tag embedded in VA for every page that has TTE.mcd bit set
in its pte. After enabling ADI on a memory range, the userspace task can
set ADI version tags using stxa instruction with ASI_MCD_PRIMARY or
ASI_MCD_ST_BLKINIT_PRIMARY ASI.

Once userspace task calls mprotect() with PROT_ADI, kernel takes
following overall steps:

1. Find the VMAs covering the address range passed in to mprotect and
set VM_SPARC_ADI flag. If address range covers a subset of a VMA, the
VMA will be split.

2. When a page is allocated for a VA and the VMA covering this VA has
VM_SPARC_ADI flag set, set the TTE.mcd bit so MMU will check the
vwersion tag.

3. Userspace can now set version tags on the memory it has enabled ADI
on. Userspace accesses ADI enabled memory using a virtual address that
has the version tag embedded in the high bits. MMU validates this
version tag against the actual tag set on the memory. If tag matches,
MMU performs the VA->PA translation and access is granted. If there is a
mismatch, hypervisor sends a data access exception or precise memory
corruption detected exception depending upon whether precise exceptions
are enabled or not (controlled by MCDPERR register). Kernel sends
SIGSEGV to the task with appropriate si_code.

4. If a page is being swapped out or migrated, kernel must save any ADI
tags set on the page. Kernel maintains a page worth of tag storage
descriptors. Each descriptors pointsto a tag storage space and the
address range it covers. If the page being swapped out or migrated has
ADI enabled on it, kernel finds a tag storage descriptor that covers the
address range for the page or allocates a new descriptor if none of the
existing descriptors cover the address range. Kernel saves tags from the
page into the tag storage space descriptor points to.

5. When the page is swapped back in or reinstantiated after migration,
kernel restores the version tags on the new physical page by retrieving
the original tag from tag storage pointed to by a tag storage descriptor
for the virtual address range for new page.

User task can disable ADI by calling mprotect() again on the memory
range with PROT_ADI bit unset. Kernel clears the VM_SPARC_ADI flag in
VMAs, merges adjacent VMAs if necessary, and clears TTE.mcd bit in the
corresponding ptes.

IOMMU does not support ADI checking. Any version tags embedded in the
top bits of VA meant for IOMMU, are cleared and replaced with sign
extension of the first non-version tag bit (bit 59 for SPARC M7) for
IOMMU addresses.

This patch series adds support for this feature in 11 patches:

Patch 1/11
  Tag mismatch on access by a task results in a trap from hypervisor as
  data access exception or a precide memory corruption detected
  exception. As part of handling these exceptions, kernel sends a
  SIGSEGV to user process with special si_code to indicate which fault
  occurred. This patch adds three new si_codes to differentiate between
  various mismatch errors.

Patch 2/11
  When a page is swapped or migrated, metadata associated with the page
  must be saved so it can be restored later. This patch adds a new
  function that saves/restores this metadata when updating pte upon a
  swap/migration.

Patch 3/11
  SPARC M7 processor adds new fields to control registers to support ADI
  feature. It also adds a new exception for precise traps on tag
  mismatch. This patch adds definitions for the new control register
  fields, new ASIs for ADI and an 

Re: [PATCH v2 2/8] [PATCH 2/8] Documentations: dt-bindings: Add a document of PECI adapter driver for Aspeed AST24xx/25xx SoCs

[Andrew Lunn]

On Wed, Feb 21, 2018 at 08:16:00AM -0800, Jae Hyun Yoo wrote:
> This commit adds a dt-bindings document of PECI adapter driver for Aspeed
> AST24xx/25xx SoCs.

Hi Jae

It would be good to separate this into two. One binding document for a
generic adaptor, with a generic PECI bus, and generic client
devices. List all the properties you expect at the generic level.

Then have an aspeed specific binding for those properties which are
specific to the Aspeed adaptor.

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

Re: [PATCH v2 1/8] [PATCH 1/8] drivers/peci: Add support for PECI bus driver core

[Andrew Lunn]

> +static int peci_locked_xfer(struct peci_adapter *adapter,
> + struct peci_xfer_msg *msg,
> + bool do_retry,
> + bool has_aw_fcs)
> +{
> + ktime_t start, end;
> + s64 elapsed_ms;
> + int rc = 0;
> +
> + if (!adapter->xfer) {
> + dev_dbg(>dev, "PECI level transfers not supported\n");
> + return -ENODEV;
> + }
> +
> + if (in_atomic() || irqs_disabled()) {

Hi Jae

Is there a real need to do transfers in atomic context, or with
interrupts disabled? 

> + rt_mutex_trylock(>bus_lock);
> + if (!rc)
> + return -EAGAIN; /* PECI activity is ongoing */
> + } else {
> + rt_mutex_lock(>bus_lock);
> + }
> +
> + if (do_retry)
> + start = ktime_get();
> +
> + do {
> + rc = adapter->xfer(adapter, msg);
> +
> + if (!do_retry)
> + break;
> +
> + /* Per the PECI spec, need to retry commands that return 0x8x */
> + if (!(!rc && ((msg->rx_buf[0] & DEV_PECI_CC_RETRY_ERR_MASK) ==
> +   DEV_PECI_CC_TIMEOUT)))
> + break;
> +
> + /* Set the retry bit to indicate a retry attempt */
> + msg->tx_buf[1] |= DEV_PECI_RETRY_BIT;
> +
> + /* Recalculate the AW FCS if it has one */
> + if (has_aw_fcs)
> + msg->tx_buf[msg->tx_len - 1] = 0x80 ^
> + peci_aw_fcs((u8 *)msg,
> + 2 + msg->tx_len);
> +
> + /* Retry for at least 250ms before returning an error */
> + end = ktime_get();
> + elapsed_ms = ktime_to_ms(ktime_sub(end, start));
> + if (elapsed_ms >= DEV_PECI_RETRY_TIME_MS) {
> + dev_dbg(>dev, "Timeout retrying xfer!\n");
> + break;
> + }
> + } while (true);

So you busy loop to 1/4 second? How about putting a sleep in here so
other things can be done between each retry.

And should it not return -ETIMEDOUT after that 1/4 second?

> +static int peci_scan_cmd_mask(struct peci_adapter *adapter)
> +{
> + struct peci_xfer_msg msg;
> + u32 dib;
> + int rc = 0;
> +
> + /* Update command mask just once */
> + if (adapter->cmd_mask & BIT(PECI_CMD_PING))
> + return 0;
> +
> + msg.addr  = PECI_BASE_ADDR;
> + msg.tx_len= GET_DIB_WR_LEN;
> + msg.rx_len= GET_DIB_RD_LEN;
> + msg.tx_buf[0] = GET_DIB_PECI_CMD;
> +
> + rc = peci_xfer(adapter, );
> + if (rc < 0) {
> + dev_dbg(>dev, "PECI xfer error, rc : %d\n", rc);
> + return rc;
> + }
> +
> + dib = msg.rx_buf[0] | (msg.rx_buf[1] << 8) |
> +   (msg.rx_buf[2] << 16) | (msg.rx_buf[3] << 24);
> +
> + /* Check special case for Get DIB command */
> + if (dib == 0x00) {
> + dev_dbg(>dev, "DIB read as 0x00\n");
> + return -1;
> + }
> +
> + if (!rc) {
> + /**
> +  * setting up the supporting commands based on minor rev#
> +  * see PECI Spec Table 3-1
> +  */
> + dib = (dib >> 8) & 0xF;
> +
> + if (dib >= 0x1) {
> + adapter->cmd_mask |= BIT(PECI_CMD_RD_PKG_CFG);
> + adapter->cmd_mask |= BIT(PECI_CMD_WR_PKG_CFG);
> + }
> +
> + if (dib >= 0x2)
> + adapter->cmd_mask |= BIT(PECI_CMD_RD_IA_MSR);
> +
> + if (dib >= 0x3) {
> + adapter->cmd_mask |= BIT(PECI_CMD_RD_PCI_CFG_LOCAL);
> + adapter->cmd_mask |= BIT(PECI_CMD_WR_PCI_CFG_LOCAL);
> + }
> +
> + if (dib >= 0x4)
> + adapter->cmd_mask |= BIT(PECI_CMD_RD_PCI_CFG);
> +
> + if (dib >= 0x5)
> + adapter->cmd_mask |= BIT(PECI_CMD_WR_PCI_CFG);
> +
> + if (dib >= 0x6)
> + adapter->cmd_mask |= BIT(PECI_CMD_WR_IA_MSR);

Lots of magic numbers here. Can they be replaced with #defines.  Also,
it looks like a switch statement could be used, with fall through.

> +
> + adapter->cmd_mask |= BIT(PECI_CMD_GET_TEMP);
> + adapter->cmd_mask |= BIT(PECI_CMD_GET_DIB);
> + adapter->cmd_mask |= BIT(PECI_CMD_PING);
> + } else {
> + dev_dbg(>dev, "Error reading DIB, rc : %d\n", rc);
> + }
> +
> + return rc;
> +}
> +

> +static int peci_ioctl_get_temp(struct peci_adapter *adapter, void *vmsg)
> +{
> + struct peci_get_temp_msg *umsg = vmsg;
> + struct peci_xfer_msg msg;
> + int rc;
> +

Is this getting the temperature?

> + rc = peci_cmd_support(adapter, PECI_CMD_GET_TEMP);
> + if (rc < 0)
> + return rc;
> +
> + msg.addr  = umsg->addr;
> + msg.tx_len= 

[PATCH v2 1/8] [PATCH 1/8] drivers/peci: Add support for PECI bus driver core

[Jae Hyun Yoo]

This commit adds driver implementation for PECI bus into linux
driver framework.

Signed-off-by: Jae Hyun Yoo 
---
 drivers/Kconfig |2 +
 drivers/Makefile|1 +
 drivers/peci/Kconfig|   20 +
 drivers/peci/Makefile   |6 +
 drivers/peci/peci-core.c| 1337 +++
 include/linux/peci.h|   97 +++
 include/uapi/linux/peci-ioctl.h |  207 ++
 7 files changed, 1670 insertions(+)
 create mode 100644 drivers/peci/Kconfig
 create mode 100644 drivers/peci/Makefile
 create mode 100644 drivers/peci/peci-core.c
 create mode 100644 include/linux/peci.h
 create mode 100644 include/uapi/linux/peci-ioctl.h

diff --git a/drivers/Kconfig b/drivers/Kconfig
index 879dc0604cba..031bed5bbe7b 100644
--- a/drivers/Kconfig
+++ b/drivers/Kconfig
@@ -219,4 +219,6 @@ source "drivers/siox/Kconfig"
 
 source "drivers/slimbus/Kconfig"
 
+source "drivers/peci/Kconfig"
+
 endmenu
diff --git a/drivers/Makefile b/drivers/Makefile
index 24cd47014657..250fe3d0fa7e 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -185,3 +185,4 @@ obj-$(CONFIG_TEE)   += tee/
 obj-$(CONFIG_MULTIPLEXER)  += mux/
 obj-$(CONFIG_UNISYS_VISORBUS)  += visorbus/
 obj-$(CONFIG_SIOX) += siox/
+obj-$(CONFIG_PECI) += peci/
diff --git a/drivers/peci/Kconfig b/drivers/peci/Kconfig
new file mode 100644
index ..1cd2cb4b2298
--- /dev/null
+++ b/drivers/peci/Kconfig
@@ -0,0 +1,20 @@
+#
+# Platform Environment Control Interface (PECI) subsystem configuration
+#
+
+menu "PECI support"
+
+config PECI
+   tristate "PECI support"
+   select RT_MUTEXES
+   select CRC8
+   help
+ The Platform Environment Control Interface (PECI) is a one-wire bus
+ interface that provides a communication channel between Intel
+ processor and chipset components to external monitoring or control
+ devices.
+
+ This PECI support can also be built as a module.  If so, the module
+ will be called peci-core.
+
+endmenu
diff --git a/drivers/peci/Makefile b/drivers/peci/Makefile
new file mode 100644
index ..9e8615e0d3ff
--- /dev/null
+++ b/drivers/peci/Makefile
@@ -0,0 +1,6 @@
+#
+# Makefile for the PECI core and bus drivers.
+#
+
+# Core functionality
+obj-$(CONFIG_PECI) += peci-core.o
diff --git a/drivers/peci/peci-core.c b/drivers/peci/peci-core.c
new file mode 100644
index ..d976c7317801
--- /dev/null
+++ b/drivers/peci/peci-core.c
@@ -0,0 +1,1337 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (c) 2018 Intel Corporation
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/* Device Specific Completion Code (CC) Definition */
+#define DEV_PECI_CC_RETRY_ERR_MASK  0xf0
+#define DEV_PECI_CC_SUCCESS 0x40
+#define DEV_PECI_CC_TIMEOUT 0x80
+#define DEV_PECI_CC_OUT_OF_RESOURCE 0x81
+#define DEV_PECI_CC_INVALID_REQ 0x90
+
+/* Skylake EDS says to retry for 250ms */
+#define DEV_PECI_RETRY_TIME_MS 250
+#define DEV_PECI_RETRY_BIT 0x01
+
+#define GET_TEMP_WR_LEN   1
+#define GET_TEMP_RD_LEN   2
+#define GET_TEMP_PECI_CMD 0x01
+
+#define GET_DIB_WR_LEN   1
+#define GET_DIB_RD_LEN   8
+#define GET_DIB_PECI_CMD 0xf7
+
+#define RDPKGCFG_WRITE_LEN 5
+#define RDPKGCFG_READ_LEN_BASE 1
+#define RDPKGCFG_PECI_CMD  0xa1
+
+#define WRPKGCFG_WRITE_LEN_BASE 6
+#define WRPKGCFG_READ_LEN   1
+#define WRPKGCFG_PECI_CMD   0xa5
+
+#define RDIAMSR_WRITE_LEN 5
+#define RDIAMSR_READ_LEN  9
+#define RDIAMSR_PECI_CMD  0xb1
+
+#define WRIAMSR_PECI_CMD  0xb5
+
+#define RDPCICFG_WRITE_LEN 6
+#define RDPCICFG_READ_LEN  5
+#define RDPCICFG_PECI_CMD  0x61
+
+#define WRPCICFG_PECI_CMD  0x65
+
+#define RDPCICFGLOCAL_WRITE_LEN 5
+#define RDPCICFGLOCAL_READ_LEN_BASE 1
+#define RDPCICFGLOCAL_PECI_CMD  0xe1
+
+#define WRPCICFGLOCAL_WRITE_LEN_BASE 6
+#define WRPCICFGLOCAL_READ_LEN   1
+#define WRPCICFGLOCAL_PECI_CMD   0xe5
+
+/* CRC8 table for Assure Write Frame Check */
+#define PECI_CRC8_POLYNOMIAL 0x07
+DECLARE_CRC8_TABLE(peci_crc8_table);
+
+static struct device_type peci_adapter_type;
+static struct device_type peci_client_type;
+
+#define PECI_CDEV_MAX 16
+static dev_t peci_devt;
+static bool is_registered;
+
+static DEFINE_MUTEX(core_lock);
+static DEFINE_IDR(peci_adapter_idr);
+
+static ssize_t name_show(struct device *dev,
+struct device_attribute *attr,
+char *buf)
+{
+   return sprintf(buf, "%s\n", dev->type == _client_type ?
+  to_peci_client(dev)->name : to_peci_adapter(dev)->name);
+}
+static DEVICE_ATTR_RO(name);
+
+static void peci_client_dev_release(struct device *dev)
+{
+   kfree(to_peci_client(dev));
+}
+
+static struct attribute *peci_device_attrs[] = {
+   _attr_name.attr,
+   NULL
+};
+ATTRIBUTE_GROUPS(peci_device);
+
+static struct device_type 

[PATCH v2 2/8] [PATCH 2/8] Documentations: dt-bindings: Add a document of PECI adapter driver for Aspeed AST24xx/25xx SoCs

[Jae Hyun Yoo]

This commit adds a dt-bindings document of PECI adapter driver for Aspeed
AST24xx/25xx SoCs.

Signed-off-by: Jae Hyun Yoo 
---
 .../devicetree/bindings/peci/peci-aspeed.txt   | 73 ++
 1 file changed, 73 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/peci/peci-aspeed.txt

diff --git a/Documentation/devicetree/bindings/peci/peci-aspeed.txt 
b/Documentation/devicetree/bindings/peci/peci-aspeed.txt
new file mode 100644
index ..8a86f346d550
--- /dev/null
+++ b/Documentation/devicetree/bindings/peci/peci-aspeed.txt
@@ -0,0 +1,73 @@
+Device tree configuration for PECI buses on the AST24XX and AST25XX SoCs.
+
+Required properties:
+- compatible
+   "aspeed,ast2400-peci" or "aspeed,ast2500-peci"
+   - aspeed,ast2400-peci: Aspeed AST2400 family PECI controller
+   - aspeed,ast2500-peci: Aspeed AST2500 family PECI controller
+
+- reg
+   Should contain PECI registers location and length.
+
+- #address-cells
+   Should be <1>.
+
+- #size-cells
+   Should be <0>.
+
+- interrupts
+   Should contain PECI interrupt.
+
+- clocks
+   Should contain clock source for PECI hardware module. Should reference
+   clkin clock.
+
+- clock_frequency
+   Should contain the operation frequency of PECI hardware module.
+   187500 ~ 2400
+
+Optional properties:
+- msg-timing-nego
+   Message timing negotiation period. This value will determine the period
+   of message timing negotiation to be issued by PECI controller. The unit
+   of the programmed value is four times of PECI clock period.
+   0 ~ 255 (default: 1)
+
+- addr-timing-nego
+   Address timing negotiation period. This value will determine the period
+   of address timing negotiation to be issued by PECI controller. The unit
+   of the programmed value is four times of PECI clock period.
+   0 ~ 255 (default: 1)
+
+- rd-sampling-point
+   Read sampling point selection. The whole period of a bit time will be
+   divided into 16 time frames. This value will determine which time frame
+   this controller will sample PECI signal for data read back. Usually in
+   the middle of a bit time is the best.
+   0 ~ 15 (default: 8)
+
+- cmd_timeout_ms
+   Command timeout in units of ms.
+   1 ~ 6 (default: 1000)
+
+Example:
+   peci: peci@1e78b000 {
+   compatible = "simple-bus";
+   #address-cells = <1>;
+   #size-cells = <1>;
+   ranges = <0x0 0x1e78b000 0x60>;
+
+   peci0: peci-bus@0 {
+   compatible = "aspeed,ast2500-peci";
+   reg = <0x0 0x60>;
+   #address-cells = <1>;
+   #size-cells = <0>;
+   interrupts = <15>;
+   clocks = <_clkin>;
+   clock-frequency = <2400>;
+   msg-timing-nego = <1>;
+   addr-timing-nego = <1>;
+   rd-sampling-point = <8>;
+   cmd-timeout-ms = <1000>;
+   };
+   };
\ No newline at end of file
-- 
2.16.1

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

[PATCH v2 3/8] [PATCH 3/8] ARM: dts: aspeed: peci: Add PECI node

[Jae Hyun Yoo]

This commit adds PECI bus/adapter node of AST24xx/AST25xx into
aspeed-g4 and aspeed-g5.

Signed-off-by: Jae Hyun Yoo 
---
 arch/arm/boot/dts/aspeed-g4.dtsi | 25 +
 arch/arm/boot/dts/aspeed-g5.dtsi | 25 +
 2 files changed, 50 insertions(+)

diff --git a/arch/arm/boot/dts/aspeed-g4.dtsi b/arch/arm/boot/dts/aspeed-g4.dtsi
index b0d8431a3700..077b4d6795b8 100644
--- a/arch/arm/boot/dts/aspeed-g4.dtsi
+++ b/arch/arm/boot/dts/aspeed-g4.dtsi
@@ -29,6 +29,7 @@
serial3 = 
serial4 = 
serial5 = 
+   peci0 = 
};
 
cpus {
@@ -250,6 +251,13 @@
};
};
 
+   peci: peci@1e78b000 {
+   compatible = "simple-bus";
+   #address-cells = <1>;
+   #size-cells = <1>;
+   ranges = <0x0 0x1e78b000 0x60>;
+   };
+
uart2: serial@1e78d000 {
compatible = "ns16550a";
reg = <0x1e78d000 0x20>;
@@ -290,6 +298,23 @@
};
 };
 
+ {
+   peci0: peci-bus@0 {
+   compatible = "aspeed,ast2400-peci";
+   reg = <0x0 0x60>;
+   #address-cells = <1>;
+   #size-cells = <0>;
+   interrupts = <15>;
+   clocks = < ASPEED_CLK_GATE_REFCLK>;
+   clock-frequency = <2400>;
+   msg-timing-nego = <1>;
+   addr-timing-nego = <1>;
+   rd-sampling-point = <8>;
+   cmd-timeout-ms = <1000>;
+   status = "disabled";
+   };
+};
+
  {
i2c_ic: interrupt-controller@0 {
#interrupt-cells = <1>;
diff --git a/arch/arm/boot/dts/aspeed-g5.dtsi b/arch/arm/boot/dts/aspeed-g5.dtsi
index 40de3b66c33f..5d3b5e177a32 100644
--- a/arch/arm/boot/dts/aspeed-g5.dtsi
+++ b/arch/arm/boot/dts/aspeed-g5.dtsi
@@ -29,6 +29,7 @@
serial3 = 
serial4 = 
serial5 = 
+   peci0 = 
};
 
cpus {
@@ -301,6 +302,13 @@
};
};
 
+   peci: peci@1e78b000 {
+   compatible = "simple-bus";
+   #address-cells = <1>;
+   #size-cells = <1>;
+   ranges = <0x0 0x1e78b000 0x60>;
+   };
+
uart2: serial@1e78d000 {
compatible = "ns16550a";
reg = <0x1e78d000 0x20>;
@@ -341,6 +349,23 @@
};
 };
 
+ {
+   peci0: peci-bus@0 {
+   compatible = "aspeed,ast2500-peci";
+   reg = <0x0 0x60>;
+   #address-cells = <1>;
+   #size-cells = <0>;
+   interrupts = <15>;
+   clocks = < ASPEED_CLK_GATE_REFCLK>;
+   clock-frequency = <2400>;
+   msg-timing-nego = <1>;
+   addr-timing-nego = <1>;
+   rd-sampling-point = <8>;
+   cmd-timeout-ms = <1000>;
+   status = "disabled";
+   };
+};
+
  {
i2c_ic: interrupt-controller@0 {
#interrupt-cells = <1>;
-- 
2.16.1

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

[PATCH v2 6/8] [PATCH 6/8] Documentation: hwmon: Add a document for PECI hwmon client driver

[Jae Hyun Yoo]

This commit adds a hwmon document for a generic PECI hwmon client driver.

Signed-off-by: Jae Hyun Yoo 
---
 Documentation/hwmon/peci-hwmon | 73 ++
 1 file changed, 73 insertions(+)
 create mode 100644 Documentation/hwmon/peci-hwmon

diff --git a/Documentation/hwmon/peci-hwmon b/Documentation/hwmon/peci-hwmon
new file mode 100644
index ..93e587498536
--- /dev/null
+++ b/Documentation/hwmon/peci-hwmon
@@ -0,0 +1,73 @@
+Kernel driver peci-hwmon
+===
+
+Supported chips:
+   Any recent Intel CPU which is connected through a PECI bus.
+   Addresses scanned: PECI client address 0x30 - 0x37
+   Datasheet: Available from http://www.intel.com/design/literature.htm
+
+Author:
+   Jae Hyun Yoo 
+
+Description
+---
+
+This driver implements a generic PECI hwmon feature which provides Digital
+Thermal Sensor (DTS) thermal readings of the CPU package, CPU cores and DIMM
+components that are accessible using the PECI Client Command Suite via the
+processor PECI client.
+
+All temperature values are given in millidegree Celsius and will be measurable
+only when the target CPU is powered on.
+
+sysfs attributes
+
+
+temp1_inputProvides current die temperature of the CPU package.
+temp1_max  Provides thermal control temperature of the CPU package
+   which is also known as Tcontrol.
+temp1_crit Provides shutdown temperature of the CPU package which
+   is also known as the maximum processor junction
+   temperature, Tjmax or Tprochot.
+temp1_crit_hystProvides the hysteresis value from Tcontrol to 
Tjmax of
+   the CPU package.
+
+temp2_inputProvides current DTS thermal margin to Tcontrol of the
+   CPU package. Value 0 means it reaches to Tcontrol
+   temperature. Sub-zero value means the die temperature
+   goes across Tconrtol to Tjmax.
+temp2_min  Provides the minimum DTS thermal margin to Tcontrol of
+   the CPU package.
+temp2_lcritProvides the value when the CPU package temperature
+   reaches to Tjmax.
+
+temp3_inputProvides current Tcontrol temperature of the CPU
+   package which is also known as Fan Temperature target.
+   Indicates the relative value from thermal monitor trip
+   temperature at which fans should be engaged.
+temp3_crit Provides Tcontrol critical value of the CPU package
+   which is same to Tjmax.
+
+temp4_inputProvides current Tthrottle temperature of the CPU
+   package. Used for throttling temperature. If this value
+   is allowed and lower than Tjmax - the throttle will
+   occur and reported at lower than Tjmax.
+
+temp5_inputProvides the maximum junction temperature, Tjmax of the
+   CPU package.
+
+temp_label  Provides core temperature if this label indicates
+   'Core #'.
+temp[n]_input  Provides current temperature of each core.
+temp[n]_maxProvides thermal control temperature of the core.
+temp[n]_crit   Provides shutdown temperature of the core.
+temp[n]_crit_hyst  Provides the hysteresis value from Tcontrol to Tjmax of
+   the core.
+
+temp_label  Provides DDR DIMM temperature if this label indicates
+   'DIMM #'.
+temp_input  Provides current temperature of the DDR DIMM.
+
+Note:
+   DIMM temperature group will be appeared when the client CPU's BIOS
+   completes memory training and testing.
-- 
2.16.1

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

[PATCH v2 7/8] [PATCH 7/8] drivers/hwmon: Add a generic PECI hwmon client driver

[Jae Hyun Yoo]

This commit adds a generic PECI hwmon client driver implementation.

Signed-off-by: Jae Hyun Yoo 
---
 drivers/hwmon/Kconfig  |  10 +
 drivers/hwmon/Makefile |   1 +
 drivers/hwmon/peci-hwmon.c | 928 +
 3 files changed, 939 insertions(+)
 create mode 100644 drivers/hwmon/peci-hwmon.c

diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index ef23553ff5cb..f22e0c31f597 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -1246,6 +1246,16 @@ config SENSORS_NCT7904
  This driver can also be built as a module.  If so, the module
  will be called nct7904.
 
+config SENSORS_PECI_HWMON
+   tristate "PECI hwmon support"
+   depends on PECI
+   help
+ If you say yes here you get support for the generic PECI hwmon
+ driver.
+
+ This driver can also be built as a module.  If so, the module
+ will be called peci-hwmon.
+
 config SENSORS_NSA320
tristate "ZyXEL NSA320 and compatible fan speed and temperature sensors"
depends on GPIOLIB && OF
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index f814b4ace138..946f54b168e5 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -135,6 +135,7 @@ obj-$(CONFIG_SENSORS_NCT7802)   += nct7802.o
 obj-$(CONFIG_SENSORS_NCT7904)  += nct7904.o
 obj-$(CONFIG_SENSORS_NSA320)   += nsa320-hwmon.o
 obj-$(CONFIG_SENSORS_NTC_THERMISTOR)   += ntc_thermistor.o
+obj-$(CONFIG_SENSORS_PECI_HWMON)   += peci-hwmon.o
 obj-$(CONFIG_SENSORS_PC87360)  += pc87360.o
 obj-$(CONFIG_SENSORS_PC87427)  += pc87427.o
 obj-$(CONFIG_SENSORS_PCF8591)  += pcf8591.o
diff --git a/drivers/hwmon/peci-hwmon.c b/drivers/hwmon/peci-hwmon.c
new file mode 100644
index ..edd27744adcb
--- /dev/null
+++ b/drivers/hwmon/peci-hwmon.c
@@ -0,0 +1,928 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (c) 2018 Intel Corporation
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define DIMM_SLOT_NUMS_MAX12  /* Max DIMM numbers (channel ranks x 2) */
+#define CORE_NUMS_MAX 28  /* Max core numbers (max on SKX Platinum) */
+#define TEMP_TYPE_PECI6   /* Sensor type 6: Intel PECI */
+
+#define CORE_TEMP_ATTRS   5
+#define DIMM_TEMP_ATTRS   2
+#define ATTR_NAME_LEN 24
+
+#define DEFAULT_ATTR_GRP_NUMS 5
+
+#define UPDATE_INTERVAL_MIN   HZ
+#define DIMM_MASK_CHECK_DELAY msecs_to_jiffies(5000)
+
+enum sign {
+   POS,
+   NEG
+};
+
+struct temp_data {
+   bool valid;
+   s32  value;
+   unsigned long last_updated;
+};
+
+struct temp_group {
+   struct temp_data tjmax;
+   struct temp_data tcontrol;
+   struct temp_data tthrottle;
+   struct temp_data dts_margin;
+   struct temp_data die;
+   struct temp_data core[CORE_NUMS_MAX];
+   struct temp_data dimm[DIMM_SLOT_NUMS_MAX];
+};
+
+struct core_temp_group {
+   struct sensor_device_attribute sd_attrs[CORE_TEMP_ATTRS];
+   char attr_name[CORE_TEMP_ATTRS][ATTR_NAME_LEN];
+   struct attribute *attrs[CORE_TEMP_ATTRS + 1];
+   struct attribute_group attr_group;
+};
+
+struct dimm_temp_group {
+   struct sensor_device_attribute sd_attrs[DIMM_TEMP_ATTRS];
+   char attr_name[DIMM_TEMP_ATTRS][ATTR_NAME_LEN];
+   struct attribute *attrs[DIMM_TEMP_ATTRS + 1];
+   struct attribute_group attr_group;
+};
+
+struct peci_hwmon {
+   struct peci_client *client;
+   struct device *dev;
+   struct device *hwmon_dev;
+   struct workqueue_struct *work_queue;
+   struct delayed_work work_handler;
+   char name[PECI_NAME_SIZE];
+   struct temp_group temp;
+   u8 addr;
+   uint cpu_no;
+   u32 core_mask;
+   u32 dimm_mask;
+   const struct attribute_group *core_attr_groups[CORE_NUMS_MAX + 1];
+   const struct attribute_group *dimm_attr_groups[DIMM_SLOT_NUMS_MAX + 1];
+   uint global_idx;
+   uint core_idx;
+   uint dimm_idx;
+};
+
+enum label {
+   L_DIE,
+   L_DTS,
+   L_TCONTROL,
+   L_TTHROTTLE,
+   L_TJMAX,
+   L_MAX
+};
+
+static const char *peci_label[L_MAX] = {
+   "Die\n",
+   "DTS margin to Tcontrol\n",
+   "Tcontrol\n",
+   "Tthrottle\n",
+   "Tjmax\n",
+};
+
+static int send_peci_cmd(struct peci_hwmon *priv, enum peci_cmd cmd, void *msg)
+{
+   return peci_command(priv->client->adapter, cmd, msg);
+}
+
+static int need_update(struct temp_data *temp)
+{
+   if (temp->valid &&
+   time_before(jiffies, temp->last_updated + UPDATE_INTERVAL_MIN))
+   return 0;
+
+   return 1;
+}
+
+static s32 ten_dot_six_to_millidegree(s32 x)
+{
+   return x) ^ 0x8000) - 0x8000) * 1000 / 64);
+}
+
+static int get_tjmax(struct peci_hwmon *priv)
+{
+   struct peci_rd_pkg_cfg_msg msg;
+   int rc;
+
+   if (!priv->temp.tjmax.valid) {
+   msg.addr = priv->addr;
+   

[PATCH v2 8/8] [PATCH 8/8] Add a maintainer for the PECI subsystem

[Jae Hyun Yoo]

This commit adds a maintainer information for the PECI subsystem.

Signed-off-by: Jae Hyun Yoo 
---
 MAINTAINERS | 9 +
 1 file changed, 9 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 93a12af4f180..f9c302cbb76b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10830,6 +10830,15 @@ L: platform-driver-...@vger.kernel.org
 S: Maintained
 F: drivers/platform/x86/peaq-wmi.c
 
+PECI SUBSYSTEM
+M: Jae Hyun Yoo 
+S: Maintained
+F: Documentation/devicetree/bindings/peci/
+F: drivers/peci/
+F: drivers/hwmon/peci-*.c
+F: include/linux/peci.h
+F: include/uapi/linux/peci-ioctl.h
+
 PER-CPU MEMORY ALLOCATOR
 M: Tejun Heo 
 M: Christoph Lameter 
-- 
2.16.1

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

[PATCH v2 0/8] PECI device driver introduction

[Jae Hyun Yoo]

Introduction of the Platform Environment Control Interface (PECI) bus
device driver. PECI is a one-wire bus interface that provides a
communication channel between Intel processor and chipset components to
external monitoring or control devices. PECI is designed to support the
following sideband functions:

* Processor and DRAM thermal management
  - Processor fan speed control is managed by comparing Digital Thermal
Sensor (DTS) thermal readings acquired via PECI against the
processor-specific fan speed control reference point, or TCONTROL.
Both TCONTROL and DTS thermal readings are accessible via the processor
PECI client. These variables are referenced to a common temperature,
the TCC activation point, and are both defined as negative offsets from
that reference.
  - PECI based access to the processor package configuration space provides
a means for Baseboard Management Controllers (BMC) or other platform
management devices to actively manage the processor and memory power
and thermal features.

* Platform Manageability
  - Platform manageability functions including thermal, power, and error
monitoring. Note that platform 'power' management includes monitoring
and control for both the processor and DRAM subsystem to assist with
data center power limiting.
  - PECI allows read access to certain error registers in the processor MSR
space and status monitoring registers in the PCI configuration space
within the processor and downstream devices.
  - PECI permits writes to certain registers in the processor PCI
configuration space.

* Processor Interface Tuning and Diagnostics
  - Processor interface tuning and diagnostics capabilities
(Intel(c) Interconnect BIST). The processors Intel(c) Interconnect
Built In Self Test (Intel(c) IBIST) allows for infield diagnostic
capabilities in the Intel UPI and memory controller interfaces. PECI
provides a port to execute these diagnostics via its PCI Configuration
read and write capabilities.

* Failure Analysis
  - Output the state of the processor after a failure for analysis via
Crashdump.

PECI uses a single wire for self-clocking and data transfer. The bus
requires no additional control lines. The physical layer is a self-clocked
one-wire bus that begins each bit with a driven, rising edge from an idle
level near zero volts. The duration of the signal driven high depends on
whether the bit value is a logic '0' or logic '1'. PECI also includes
variable data transfer rate established with every message. In this way,
it is highly flexible even though underlying logic is simple.

The interface design was optimized for interfacing to Intel processor and
chipset components in both single processor and multiple processor
environments. The single wire interface provides low board routing
overhead for the multiple load connections in the congested routing area
near the processor and chipset components. Bus speed, error checking, and
low protocol overhead provides adequate link bandwidth and reliability to
transfer critical device operating conditions and configuration
information.

This implementation provides the basic framework to add PECI extensions
to the Linux bus and device models. A hardware specific 'Adapter' driver
can be attached to the PECI bus to provide sideband functions described
above. It is also possible to access all devices on an adapter from
userspace through the /dev interface. A device specific 'Client' driver
also can be attached to the PECI bus so each processor client's features
can be supported by the 'Client' driver through an adapter connection in
the bus. This patch set includes Aspeed 24xx/25xx PECI driver and a generic
PECI hwmon driver as the first implementation for both adapter and client
drivers on the PECI bus framework.

v1 -> v2
- Additionally implemented a core driver to support PECI linux bus driver
  model.
- Modified Aspeed PECI driver to make that to be an adapter driver in PECI
  bus.
- Modified PECI hwmon driver to make that to be a client driver in PECI
  bus.
- Simplified hwmon driver attribute labels and removed redundant strings.
- Removed core_nums from device tree setting of hwmon driver and modified
  core number detection logic to check the resolved_core register in
  client CPU's local PCI configuration area.
- Removed dimm_nums from device tree setting of hwmon driver and added
  populated DIMM detection logic to support dynamic creation.
- Removed indexing gap on core temperature and DIMM temperature attributes.
- Improved hwmon registration and dynamic attribute creation logic.
- Fixed structure definitions in PECI uapi header to make that use __u8,
  __u16 and etc.
- Modified wait_for_completion_interruptible_timeout error handling logic
  in Aspeed PECI driver to deliver errors correctly.
- Removed low-level xfer command from ioctl and kept only high-level PECI
  command suite as ioctls.
- Fixed I/O timeout logic in Aspeed PECI 

[PATCH v2 4/8] [PATCH 4/8] drivers/peci: Add a PECI adapter driver for Aspeed AST24xx/AST25xx

[Jae Hyun Yoo]

This commit adds PECI adapter driver implementation for Aspeed
AST24xx/AST25xx.

Signed-off-by: Jae Hyun Yoo 
---
 drivers/peci/Kconfig   |  19 ++
 drivers/peci/Makefile  |   3 +
 drivers/peci/peci-aspeed.c | 510 +
 3 files changed, 532 insertions(+)
 create mode 100644 drivers/peci/peci-aspeed.c

diff --git a/drivers/peci/Kconfig b/drivers/peci/Kconfig
index 1cd2cb4b2298..f9875a0d0bc7 100644
--- a/drivers/peci/Kconfig
+++ b/drivers/peci/Kconfig
@@ -14,7 +14,26 @@ config PECI
  processor and chipset components to external monitoring or control
  devices.
 
+ If you want PECI support, you should say Y here and also to the
+ specific driver for your bus adapter(s) below.
+
  This PECI support can also be built as a module.  If so, the module
  will be called peci-core.
 
+if PECI
+
+config PECI_ASPEED
+   tristate "Aspeed AST24xx/AST25xx PECI support"
+   select REGMAP_MMIO
+   depends on OF && (ARCH_ASPEED || COMPILE_TEST)
+   help
+ Say Y here if you want support for the Platform Environment Control
+ Interface (PECI) bus adapter driver on the Aspeed AST24XX and AST25XX
+ SoCs.
+
+ This support is also available as a module.  If so, the module
+ will be called peci-aspeed.
+
+endif # PECI
+
 endmenu
diff --git a/drivers/peci/Makefile b/drivers/peci/Makefile
index 9e8615e0d3ff..886285e69765 100644
--- a/drivers/peci/Makefile
+++ b/drivers/peci/Makefile
@@ -4,3 +4,6 @@
 
 # Core functionality
 obj-$(CONFIG_PECI) += peci-core.o
+
+# Hardware specific bus drivers
+obj-$(CONFIG_PECI_ASPEED)  += peci-aspeed.o
diff --git a/drivers/peci/peci-aspeed.c b/drivers/peci/peci-aspeed.c
new file mode 100644
index ..2b7800e96805
--- /dev/null
+++ b/drivers/peci/peci-aspeed.c
@@ -0,0 +1,510 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2012-2020 ASPEED Technology Inc.
+// Copyright (c) 2018 Intel Corporation
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define DUMP_DEBUG 0
+
+/* Aspeed PECI Registers */
+#define AST_PECI_CTRL 0x00
+#define AST_PECI_TIMING   0x04
+#define AST_PECI_CMD  0x08
+#define AST_PECI_CMD_CTRL 0x0c
+#define AST_PECI_EXP_FCS  0x10
+#define AST_PECI_CAP_FCS  0x14
+#define AST_PECI_INT_CTRL 0x18
+#define AST_PECI_INT_STS  0x1c
+#define AST_PECI_W_DATA0  0x20
+#define AST_PECI_W_DATA1  0x24
+#define AST_PECI_W_DATA2  0x28
+#define AST_PECI_W_DATA3  0x2c
+#define AST_PECI_R_DATA0  0x30
+#define AST_PECI_R_DATA1  0x34
+#define AST_PECI_R_DATA2  0x38
+#define AST_PECI_R_DATA3  0x3c
+#define AST_PECI_W_DATA4  0x40
+#define AST_PECI_W_DATA5  0x44
+#define AST_PECI_W_DATA6  0x48
+#define AST_PECI_W_DATA7  0x4c
+#define AST_PECI_R_DATA4  0x50
+#define AST_PECI_R_DATA5  0x54
+#define AST_PECI_R_DATA6  0x58
+#define AST_PECI_R_DATA7  0x5c
+
+/* AST_PECI_CTRL - 0x00 : Control Register */
+#define PECI_CTRL_SAMPLING_MASK GENMASK(19, 16)
+#define PECI_CTRL_SAMPLING(x)   (((x) << 16) & PECI_CTRL_SAMPLING_MASK)
+#define PECI_CTRL_SAMPLING_GET(x)   (((x) & PECI_CTRL_SAMPLING_MASK) >> 16)
+#define PECI_CTRL_READ_MODE_MASKGENMASK(13, 12)
+#define PECI_CTRL_READ_MODE(x)  (((x) << 12) & PECI_CTRL_READ_MODE_MASK)
+#define PECI_CTRL_READ_MODE_GET(x)  (((x) & PECI_CTRL_READ_MODE_MASK) >> 12)
+#define PECI_CTRL_READ_MODE_COUNT   BIT(12)
+#define PECI_CTRL_READ_MODE_DBG BIT(13)
+#define PECI_CTRL_CLK_SOURCE_MASK   BIT(11)
+#define PECI_CTRL_CLK_SOURCE(x) (((x) << 11) & PECI_CTRL_CLK_SOURCE_MASK)
+#define PECI_CTRL_CLK_SOURCE_GET(x) (((x) & PECI_CTRL_CLK_SOURCE_MASK) >> 11)
+#define PECI_CTRL_CLK_DIV_MASK  GENMASK(10, 8)
+#define PECI_CTRL_CLK_DIV(x)(((x) << 8) & PECI_CTRL_CLK_DIV_MASK)
+#define PECI_CTRL_CLK_DIV_GET(x)(((x) & PECI_CTRL_CLK_DIV_MASK) >> 8)
+#define PECI_CTRL_INVERT_OUTBIT(7)
+#define PECI_CTRL_INVERT_IN BIT(6)
+#define PECI_CTRL_BUS_CONTENT_ENBIT(5)
+#define PECI_CTRL_PECI_EN   BIT(4)
+#define PECI_CTRL_PECI_CLK_EN   BIT(0)
+
+/* AST_PECI_TIMING - 0x04 : Timing Negotiation Register */
+#define PECI_TIMING_MESSAGE_MASK   GENMASK(15, 8)
+#define PECI_TIMING_MESSAGE(x) (((x) << 8) & PECI_TIMING_MESSAGE_MASK)
+#define PECI_TIMING_MESSAGE_GET(x) (((x) & PECI_TIMING_MESSAGE_MASK) >> 8)
+#define PECI_TIMING_ADDRESS_MASK   GENMASK(7, 0)
+#define PECI_TIMING_ADDRESS(x) ((x) & PECI_TIMING_ADDRESS_MASK)
+#define PECI_TIMING_ADDRESS_GET(x) ((x) & PECI_TIMING_ADDRESS_MASK)
+
+/* AST_PECI_CMD - 0x08 : Command Register */
+#define PECI_CMD_PIN_MONBIT(31)
+#define PECI_CMD_STS_MASK   GENMASK(27, 24)
+#define PECI_CMD_STS_GET(x) (((x) & PECI_CMD_STS_MASK) >> 24)
+#define PECI_CMD_FIRE   BIT(0)
+
+/* AST_PECI_LEN - 0x0C : Read/Write Length Register */
+#define PECI_AW_FCS_EN   BIT(31)
+#define PECI_READ_LEN_MASK   GENMASK(23, 16)
+#define 

[PATCH v2 5/8] [PATCH [5/8] Documentation: dt-bindings: Add a document for PECI hwmon client driver

[Jae Hyun Yoo]

This commit adds a dt-bindings document for a generic PECI hwmon client
driver.

Signed-off-by: Jae Hyun Yoo 
---
 .../devicetree/bindings/hwmon/peci-hwmon.txt   | 27 ++
 1 file changed, 27 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/hwmon/peci-hwmon.txt

diff --git a/Documentation/devicetree/bindings/hwmon/peci-hwmon.txt 
b/Documentation/devicetree/bindings/hwmon/peci-hwmon.txt
new file mode 100644
index ..831813158884
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwmon/peci-hwmon.txt
@@ -0,0 +1,27 @@
+Bindings for Intel PECI (Platform Environment Control Interface) hwmon driver.
+
+Required properties:
+- compatible
+   Should be "intel,peci-hwmon".
+
+- reg
+   Should contain address of a client CPU. Address range of CPU clients is
+   starting from 0x30 based on PECI specification.
+   <0x30> .. <0x37> (depends on the PECI_OFFSET_MAX definition)
+
+Example:
+   peci-bus@0 {
+   #address-cells = <1>;
+   #size-cells = <0>;
+   < more properties >
+
+   peci-hwmon@cpu0 {
+   compatible = "intel,peci-hwmon";
+   reg = <0x30>;
+   };
+
+   peci-hwmon@cpu1 {
+   compatible = "intel,peci-hwmon";
+   reg = <0x31>;
+   };
+   };
-- 
2.16.1

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

Re: [PATCH 05/10] hwmon: generic-pwm-tachometer: Add generic PWM based tachometer

[Guenter Roeck]

On Wed, Feb 21, 2018 at 05:20:29PM +0200, Mikko Perttunen wrote:
> On 21.02.2018 16:46, Guenter Roeck wrote:
> >On 02/20/2018 11:15 PM, Mikko Perttunen wrote:
> >>AIUI, the PWM framework already exposes a sysfs node with period
> >>information. We should just use that instead of adding a new driver
> >>for this.
> >>
> >
> >I am kind of lost. Please explain.
> >
> >Are you saying that we should drop the pwm-fan driver as well (which goes
> >the opposite way), as well as any other drivers doing anything with pwm
> >signals,
> >because after all those signals are already exposed to userspace a sysfs
> >attributes,
> >and a kernel driver to abstract those values is thus not needed ?
> 
> The only thing this driver does is do a constant division in kernelspace.
> I'm not really seeing why that couldn't be done in userspace. But if you
> think it's appropriate to do the RPM conversion in kernelspace then I'm not
> greatly opposed to that.
> 
Isn't that true for any conversion from and to pwm values ? Voltages,
for example ? Should those be handled in userspace as well ?

Note that I am not entirely convinced that the approach suggested in this
patch series makes sense. Patch 4 seems to move the notion of a tachometer
into the pwm subsystem. I am not really convinced that this makes sense
(maybe all that code should be in the hwmon driver instead, or in a thermal
driver if the author prefers). But that is a different issue. The question
you raised is if there should be any abstraction to or from raw pwm values
in the kernel.

> >
> >>In any case, we cannot add something like this to device tree since
> >>it's not a hardware device.
> >>
> >
> >So you are saying there is no means to express in devicetree that
> >a pwm input is connected to a fan ? How is that not hardware ?
> >
> >If so, how do you express in devicetree that a pwm signal is connected
> >to anything ?
> 
> If we want to describe that the tachometer is connected to a fan, then we
> should have a fan node in the board's device tree. We don't have a chip that
> has a thing called "generic-pwm-tachometer" attached to it. (We have chips
> that have a "nvidia,tegra186-tachometer", so it's proper to have that.)
> 
So you are concerned about the property name ? I don't really care how
it is called.

Guenter

> Thanks,
> Mikko
> 
> >
> >Guenter
> >
> >>Mikko
> >>
> >>On 21.02.2018 08:58, Rajkumar Rampelli wrote:
> >>>Add generic PWM based tachometer driver via HWMON interface
> >>>to report the RPM of motor. This drivers get the period/duty
> >>>cycle from PWM IP which captures the motor PWM output.
> >>>
> >>>This driver implements a simple interface for monitoring the speed of
> >>>a fan and exposes it in roatations per minute (RPM) to the user space
> >>>by using the hwmon's sysfs interface
> >>>
> >>>Signed-off-by: Rajkumar Rampelli 
> >>>---
> >>> Documentation/hwmon/generic-pwm-tachometer |  17 +
> >>> drivers/hwmon/Kconfig  |  10 +++
> >>> drivers/hwmon/Makefile |   1 +
> >>> drivers/hwmon/generic-pwm-tachometer.c | 112
> >>>+
> >>> 4 files changed, 140 insertions(+)
> >>> create mode 100644 Documentation/hwmon/generic-pwm-tachometer
> >>> create mode 100644 drivers/hwmon/generic-pwm-tachometer.c
> >>>
> >>>diff --git a/Documentation/hwmon/generic-pwm-tachometer
> >>>b/Documentation/hwmon/generic-pwm-tachometer
> >>>new file mode 100644
> >>>index 000..e0713ee
> >>>--- /dev/null
> >>>+++ b/Documentation/hwmon/generic-pwm-tachometer
> >>>@@ -0,0 +1,17 @@
> >>>+Kernel driver generic-pwm-tachometer
> >>>+
> >>>+
> >>>+This driver enables the use of a PWM module to monitor a fan. It
> >>>uses the
> >>>+generic PWM interface and can be used on SoCs as along as the SoC
> >>>supports
> >>>+Tachometer controller that moniors the Fan speed in periods.
> >>>+
> >>>+Author: Rajkumar Rampelli 
> >>>+
> >>>+Description
> >>>+---
> >>>+
> >>>+The driver implements a simple interface for monitoring the Fan
> >>>speed using
> >>>+PWM module and Tachometer controller. It requests period value
> >>>through PWM
> >>>+capture interface to Tachometer and measures the Rotations per
> >>>minute using
> >>>+received period value. It exposes the Fan speed in RPM to the user
> >>>space by
> >>>+using the hwmon's sysfs interface.
> >>>diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> >>>index ef23553..8912dcb 100644
> >>>--- a/drivers/hwmon/Kconfig
> >>>+++ b/drivers/hwmon/Kconfig
> >>>@@ -1878,6 +1878,16 @@ config SENSORS_XGENE
> >>>   If you say yes here you get support for the temperature
> >>>   and power sensors for APM X-Gene SoC.
> >>>
> >>>+config GENERIC_PWM_TACHOMETER
> >>>+tristate "Generic PWM based tachometer driver"
> >>>+depends on PWM
> >>>+help
> >>>+  Enables a driver to use PWM signal from motor to use
> >>>+  for measuring the motor speed. The RPM is captured 

Re: [PATCH 00/23] kconfig: move compiler capability tests to Kconfig

[Arnd Bergmann]

On Wed, Feb 21, 2018 at 1:57 PM, Masahiro Yamada
 wrote:
> 2018-02-21 19:52 GMT+09:00 Arnd Bergmann :
>> On Wed, Feb 21, 2018 at 11:20 AM, Masahiro Yamada
>>  wrote:
>>> 2018-02-21 18:56 GMT+09:00 Arnd Bergmann :
 On Wed, Feb 21, 2018 at 8:38 AM, Masahiro Yamada
  wrote:
> 2018-02-20 0:18 GMT+09:00 Ulf Magnusson :
>
> Hmm, I think I can implement those somehow.
> But, I hope we do not have many instances like this...
>
>
> If you know more naive cases, please share your knowledge.
>

One case that comes to mind would be architecture level selection on 32-bit
ARM, which is roughly this (I probably have some details wrong, but you
get the idea):

- older compilers don't support the latest architecture setting (-march=armv8
  or -march=armv7ve)
- newer compilers no longer support really old architectures (-march=armv4)
- setting -mthumb requires setting one of  -march=armv7-a, armv7ve, armv7-m or
  armv8 if the compiler doesn't default to those
- on a compiler that defaults to -marm, setting -march=armv7-m requires
  setting -mthumb (IIRC)
- really old compilers only support OABI, but not EABI
- newer compilers no longer support OABI
- mthumb requires EABI
- armv6 and higher are subtly broken with OABI, but only when using
  certain inline assembly with 64-bit arguments in register pairs.

I think we just shouldn't try to capture all of the above correctly in Kconfig
conditionals.

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

[PATCH V2 5/7] thermal/drivers/cpu_cooling: Add idle cooling device documentation

[Daniel Lezcano]

Provide some documentation for the idle injection cooling effect in
order to let people to understand the rational of the approach for the
idle injection CPU cooling device.

Signed-off-by: Daniel Lezcano 
---
 Documentation/thermal/cpu-idle-cooling.txt | 165 +
 1 file changed, 165 insertions(+)
 create mode 100644 Documentation/thermal/cpu-idle-cooling.txt

diff --git a/Documentation/thermal/cpu-idle-cooling.txt 
b/Documentation/thermal/cpu-idle-cooling.txt
new file mode 100644
index 000..29fc651
--- /dev/null
+++ b/Documentation/thermal/cpu-idle-cooling.txt
@@ -0,0 +1,165 @@
+
+Situation:
+--
+
+Under certain circumstances, the SoC reaches a temperature exceeding
+the allocated power budget or the maximum temperature limit. The
+former must be mitigated to stabilize the SoC temperature around the
+temperature control using the defined cooling devices, the latter is a
+catastrophic situation where a radical decision must be taken to
+reduce the temperature under the critical threshold, that can impact
+the performances.
+
+Another situation is when the silicon reaches a certain temperature
+which continues to increase even if the dynamic leakage is reduced to
+its minimum by clock gating the component. The runaway phenomena will
+continue with the static leakage and only powering down the component,
+thus dropping the dynamic and static leakage will allow the component
+to cool down. This situation is critical.
+
+Last but not least, the system can ask for a specific power budget but
+because of the OPP density, we can only choose an OPP with a power
+budget lower than the requested one and underuse the CPU, thus losing
+performances. In other words, one OPP under uses the CPU with a power
+lesser than the power budget and the next OPP exceed the power budget,
+an intermediate OPP could have been used if it were present.
+
+Solutions:
+--
+
+If we can remove the static and the dynamic leakage for a specific
+duration in a controlled period, the SoC temperature will
+decrease. Acting at the idle state duration or the idle cycle
+injection period, we can mitigate the temperature by modulating the
+power budget.
+
+The Operating Performance Point (OPP) density has a great influence on
+the control precision of cpufreq, however different vendors have a
+plethora of OPP density, and some have large power gap between OPPs,
+that will result in loss of performance during thermal control and
+loss of power in other scenes.
+
+At a specific OPP, we can assume injecting idle cycle on all CPUs,
+belonging to the same cluster, with a duration greater than the
+cluster idle state target residency, we drop the static and the
+dynamic leakage for this period (modulo the energy needed to enter
+this state). So the sustainable power with idle cycles has a linear
+relation with the OPP’s sustainable power and can be computed with a
+coefficient similar to:
+
+   Power(IdleCycle) = Coef x Power(OPP)
+
+Idle Injection:
+---
+
+The base concept of the idle injection is to force the CPU to go to an
+idle state for a specified time each control cycle, it provides
+another way to control CPU power and heat in addition to
+cpufreq. Ideally, if all CPUs of a cluster inject idle synchronously,
+this cluster can get into the deepest idle state and achieve minimum
+power consumption, but that will also increase system response latency
+if we inject less than cpuidle latency.
+
+ ^
+ |
+ |
+ |---   ---   ---
+ |___|_|___|_|___|___
+
+  <->
+   idle  <>
+  running
+
+With the fixed idle injection duration, we can give a value which is
+an acceptable performance drop off or latency when we reach a specific
+temperature and we begin to mitigate by varying the Idle injection
+period.
+
+The mitigation begins with a maximum period value which decrease when
+more cooling effect is requested. When the period duration is equal to
+the idle duration, then we are in a situation the platform can’t
+dissipate the heat enough and the mitigation fails. In this case the
+situation is considered critical and there is nothing to do. The idle
+injection duration must be changed by configuration and until we reach
+the cooling effect, otherwise an additionnal cooling device must be
+used or ultimately decrease the SoC performance by dropping the
+highest OPP point of the SoC.
+
+The idle injection duration value must comply with the constraints:
+
+- It is lesser or equal to the latency we tolerate when the mitigation
+  begins. It is platform dependent and will depend on the user
+  experience, reactivity vs performance trade off we want. This value
+  should be specified.
+
+- It is greater than the idle state’s target residency we want to go
+  for thermal mitigation, otherwise we end up consuming more energy.
+
+Minimum period
+--
+
+The idle 

[PATCH] tracing: Remove redundant subject

[Xiongwei Song]

There are two consecutive 'we' to represent subject, remove one of the two.

Signed-off-by: Xiongwei Song 
---
 Documentation/trace/events.txt | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/Documentation/trace/events.txt b/Documentation/trace/events.txt
index 1d486660b40f..813b140cfe2c 100644
--- a/Documentation/trace/events.txt
+++ b/Documentation/trace/events.txt
@@ -878,10 +878,10 @@ The following commands are supported:
   Because the default sort key above is 'hitcount', the above shows a
   the list of call_sites by increasing hitcount, so that at the bottom
   we see the functions that made the most kmalloc calls during the
-  run.  If instead we we wanted to see the top kmalloc callers in
-  terms of the number of bytes requested rather than the number of
-  calls, and we wanted the top caller to appear at the top, we can use
-  the 'sort' parameter, along with the 'descending' modifier:
+  run.  If instead we wanted to see the top kmalloc callers in terms
+  of the number of bytes requested rather than the number of calls,
+  and we wanted the top caller to appear at the top, we can use the
+  'sort' parameter, along with the 'descending' modifier:
 
 # echo 'hist:key=call_site.sym:val=bytes_req:sort=bytes_req.descending' > \
/sys/kernel/debug/tracing/events/kmem/kmalloc/trigger
-- 
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

Re: [PATCH 05/10] hwmon: generic-pwm-tachometer: Add generic PWM based tachometer

[Mikko Perttunen]

On 21.02.2018 16:46, Guenter Roeck wrote:

On 02/20/2018 11:15 PM, Mikko Perttunen wrote:

AIUI, the PWM framework already exposes a sysfs node with period
information. We should just use that instead of adding a new driver
for this.



I am kind of lost. Please explain.

Are you saying that we should drop the pwm-fan driver as well (which goes
the opposite way), as well as any other drivers doing anything with pwm
signals,
because after all those signals are already exposed to userspace a sysfs
attributes,
and a kernel driver to abstract those values is thus not needed ?


The only thing this driver does is do a constant division in 
kernelspace. I'm not really seeing why that couldn't be done in 
userspace. But if you think it's appropriate to do the RPM conversion in 
kernelspace then I'm not greatly opposed to that.





In any case, we cannot add something like this to device tree since
it's not a hardware device.



So you are saying there is no means to express in devicetree that
a pwm input is connected to a fan ? How is that not hardware ?

If so, how do you express in devicetree that a pwm signal is connected
to anything ?


If we want to describe that the tachometer is connected to a fan, then 
we should have a fan node in the board's device tree. We don't have a 
chip that has a thing called "generic-pwm-tachometer" attached to it. 
(We have chips that have a "nvidia,tegra186-tachometer", so it's proper 
to have that.)


Thanks,
Mikko



Guenter


Mikko

On 21.02.2018 08:58, Rajkumar Rampelli wrote:

Add generic PWM based tachometer driver via HWMON interface
to report the RPM of motor. This drivers get the period/duty
cycle from PWM IP which captures the motor PWM output.

This driver implements a simple interface for monitoring the speed of
a fan and exposes it in roatations per minute (RPM) to the user space
by using the hwmon's sysfs interface

Signed-off-by: Rajkumar Rampelli 
---
 Documentation/hwmon/generic-pwm-tachometer |  17 +
 drivers/hwmon/Kconfig  |  10 +++
 drivers/hwmon/Makefile |   1 +
 drivers/hwmon/generic-pwm-tachometer.c | 112
+
 4 files changed, 140 insertions(+)
 create mode 100644 Documentation/hwmon/generic-pwm-tachometer
 create mode 100644 drivers/hwmon/generic-pwm-tachometer.c

diff --git a/Documentation/hwmon/generic-pwm-tachometer
b/Documentation/hwmon/generic-pwm-tachometer
new file mode 100644
index 000..e0713ee
--- /dev/null
+++ b/Documentation/hwmon/generic-pwm-tachometer
@@ -0,0 +1,17 @@
+Kernel driver generic-pwm-tachometer
+
+
+This driver enables the use of a PWM module to monitor a fan. It
uses the
+generic PWM interface and can be used on SoCs as along as the SoC
supports
+Tachometer controller that moniors the Fan speed in periods.
+
+Author: Rajkumar Rampelli 
+
+Description
+---
+
+The driver implements a simple interface for monitoring the Fan
speed using
+PWM module and Tachometer controller. It requests period value
through PWM
+capture interface to Tachometer and measures the Rotations per
minute using
+received period value. It exposes the Fan speed in RPM to the user
space by
+using the hwmon's sysfs interface.
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index ef23553..8912dcb 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -1878,6 +1878,16 @@ config SENSORS_XGENE
   If you say yes here you get support for the temperature
   and power sensors for APM X-Gene SoC.

+config GENERIC_PWM_TACHOMETER
+tristate "Generic PWM based tachometer driver"
+depends on PWM
+help
+  Enables a driver to use PWM signal from motor to use
+  for measuring the motor speed. The RPM is captured by
+  PWM modules which has PWM capture capability and this
+  drivers reads the captured data from PWM IP to convert
+  it to speed in RPM.
+
 if ACPI

 comment "ACPI drivers"
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index f814b4a..9dcc374 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -175,6 +175,7 @@ obj-$(CONFIG_SENSORS_WM8350)+= wm8350-hwmon.o
 obj-$(CONFIG_SENSORS_XGENE)+= xgene-hwmon.o

 obj-$(CONFIG_PMBUS)+= pmbus/
+obj-$(CONFIG_GENERIC_PWM_TACHOMETER) += generic-pwm-tachometer.o

 ccflags-$(CONFIG_HWMON_DEBUG_CHIP) := -DDEBUG

diff --git a/drivers/hwmon/generic-pwm-tachometer.c
b/drivers/hwmon/generic-pwm-tachometer.c
new file mode 100644
index 000..9354d43
--- /dev/null
+++ b/drivers/hwmon/generic-pwm-tachometer.c
@@ -0,0 +1,112 @@
+/*
+ * Copyright (c) 2017-2018, NVIDIA CORPORATION.  All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or
modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but

Re: [PATCH 05/10] hwmon: generic-pwm-tachometer: Add generic PWM based tachometer

[Guenter Roeck]

On 02/20/2018 10:58 PM, Rajkumar Rampelli wrote:

Add generic PWM based tachometer driver via HWMON interface
to report the RPM of motor. This drivers get the period/duty
cycle from PWM IP which captures the motor PWM output.

This driver implements a simple interface for monitoring the speed of
a fan and exposes it in roatations per minute (RPM) to the user space
by using the hwmon's sysfs interface

Signed-off-by: Rajkumar Rampelli 
---
  Documentation/hwmon/generic-pwm-tachometer |  17 +
  drivers/hwmon/Kconfig  |  10 +++
  drivers/hwmon/Makefile |   1 +
  drivers/hwmon/generic-pwm-tachometer.c | 112 +
  4 files changed, 140 insertions(+)
  create mode 100644 Documentation/hwmon/generic-pwm-tachometer
  create mode 100644 drivers/hwmon/generic-pwm-tachometer.c

diff --git a/Documentation/hwmon/generic-pwm-tachometer 
b/Documentation/hwmon/generic-pwm-tachometer
new file mode 100644
index 000..e0713ee
--- /dev/null
+++ b/Documentation/hwmon/generic-pwm-tachometer
@@ -0,0 +1,17 @@
+Kernel driver generic-pwm-tachometer
+
+
+This driver enables the use of a PWM module to monitor a fan. It uses the
+generic PWM interface and can be used on SoCs as along as the SoC supports
+Tachometer controller that moniors the Fan speed in periods.
+
+Author: Rajkumar Rampelli 
+
+Description
+---
+
+The driver implements a simple interface for monitoring the Fan speed using
+PWM module and Tachometer controller. It requests period value through PWM
+capture interface to Tachometer and measures the Rotations per minute using
+received period value. It exposes the Fan speed in RPM to the user space by
+using the hwmon's sysfs interface.
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index ef23553..8912dcb 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -1878,6 +1878,16 @@ config SENSORS_XGENE
  If you say yes here you get support for the temperature
  and power sensors for APM X-Gene SoC.
  
+config GENERIC_PWM_TACHOMETER

+   tristate "Generic PWM based tachometer driver"
+   depends on PWM
+   help
+ Enables a driver to use PWM signal from motor to use
+ for measuring the motor speed. The RPM is captured by
+ PWM modules which has PWM capture capability and this
+ drivers reads the captured data from PWM IP to convert
+ it to speed in RPM.
+
  if ACPI
  
  comment "ACPI drivers"

diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index f814b4a..9dcc374 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -175,6 +175,7 @@ obj-$(CONFIG_SENSORS_WM8350)+= wm8350-hwmon.o
  obj-$(CONFIG_SENSORS_XGENE)   += xgene-hwmon.o
  
  obj-$(CONFIG_PMBUS)		+= pmbus/

+obj-$(CONFIG_GENERIC_PWM_TACHOMETER) += generic-pwm-tachometer.o
  
  ccflags-$(CONFIG_HWMON_DEBUG_CHIP) := -DDEBUG
  
diff --git a/drivers/hwmon/generic-pwm-tachometer.c b/drivers/hwmon/generic-pwm-tachometer.c

new file mode 100644
index 000..9354d43
--- /dev/null
+++ b/drivers/hwmon/generic-pwm-tachometer.c
@@ -0,0 +1,112 @@
+/*
+ * Copyright (c) 2017-2018, NVIDIA CORPORATION.  All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope 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.
+ *
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+struct pwm_hwmon_tach {
+   struct device   *dev;
+   struct pwm_device   *pwm;
+   struct device   *hwmon;
+};
+
+static ssize_t show_rpm(struct device *dev, struct device_attribute *attr,
+   char *buf)
+{
+   struct pwm_hwmon_tach *ptt = dev_get_drvdata(dev);
+   struct pwm_device *pwm = ptt->pwm;
+   struct pwm_capture result;
+   int err;
+   unsigned int rpm = 0;
+
+   err = pwm_capture(pwm, , 0);
+   if (err < 0) {
+   dev_err(ptt->dev, "Failed to capture PWM: %d\n", err);
+   return err;
+   }
+
+   if (result.period)
+   rpm = DIV_ROUND_CLOSEST_ULL(60ULL * NSEC_PER_SEC,
+   result.period);
+
+   return sprintf(buf, "%u\n", rpm);
+}
+
+static SENSOR_DEVICE_ATTR(rpm, 0444, show_rpm, NULL, 0);
+
+static struct attribute *pwm_tach_attrs[] = {
+   _dev_attr_rpm.dev_attr.attr,
+   NULL,
+};


"rpm" is not a standard hwmon sysfs attribute. If you don't provide
a single standard hwmon sysfs attribute, having a hwmon driver is pointless.


+
+ATTRIBUTE_GROUPS(pwm_tach);
+
+static int 

Re: [PATCH 05/10] hwmon: generic-pwm-tachometer: Add generic PWM based tachometer

[Guenter Roeck]

On 02/20/2018 11:15 PM, Mikko Perttunen wrote:

AIUI, the PWM framework already exposes a sysfs node with period information. 
We should just use that instead of adding a new driver for this.



I am kind of lost. Please explain.

Are you saying that we should drop the pwm-fan driver as well (which goes
the opposite way), as well as any other drivers doing anything with pwm signals,
because after all those signals are already exposed to userspace a sysfs 
attributes,
and a kernel driver to abstract those values is thus not needed ?


In any case, we cannot add something like this to device tree since it's not a 
hardware device.



So you are saying there is no means to express in devicetree that
a pwm input is connected to a fan ? How is that not hardware ?

If so, how do you express in devicetree that a pwm signal is connected
to anything ?

Guenter


Mikko

On 21.02.2018 08:58, Rajkumar Rampelli wrote:

Add generic PWM based tachometer driver via HWMON interface
to report the RPM of motor. This drivers get the period/duty
cycle from PWM IP which captures the motor PWM output.

This driver implements a simple interface for monitoring the speed of
a fan and exposes it in roatations per minute (RPM) to the user space
by using the hwmon's sysfs interface

Signed-off-by: Rajkumar Rampelli 
---
 Documentation/hwmon/generic-pwm-tachometer |  17 +
 drivers/hwmon/Kconfig  |  10 +++
 drivers/hwmon/Makefile |   1 +
 drivers/hwmon/generic-pwm-tachometer.c | 112 +
 4 files changed, 140 insertions(+)
 create mode 100644 Documentation/hwmon/generic-pwm-tachometer
 create mode 100644 drivers/hwmon/generic-pwm-tachometer.c

diff --git a/Documentation/hwmon/generic-pwm-tachometer 
b/Documentation/hwmon/generic-pwm-tachometer
new file mode 100644
index 000..e0713ee
--- /dev/null
+++ b/Documentation/hwmon/generic-pwm-tachometer
@@ -0,0 +1,17 @@
+Kernel driver generic-pwm-tachometer
+
+
+This driver enables the use of a PWM module to monitor a fan. It uses the
+generic PWM interface and can be used on SoCs as along as the SoC supports
+Tachometer controller that moniors the Fan speed in periods.
+
+Author: Rajkumar Rampelli 
+
+Description
+---
+
+The driver implements a simple interface for monitoring the Fan speed using
+PWM module and Tachometer controller. It requests period value through PWM
+capture interface to Tachometer and measures the Rotations per minute using
+received period value. It exposes the Fan speed in RPM to the user space by
+using the hwmon's sysfs interface.
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index ef23553..8912dcb 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -1878,6 +1878,16 @@ config SENSORS_XGENE
   If you say yes here you get support for the temperature
   and power sensors for APM X-Gene SoC.

+config GENERIC_PWM_TACHOMETER
+    tristate "Generic PWM based tachometer driver"
+    depends on PWM
+    help
+  Enables a driver to use PWM signal from motor to use
+  for measuring the motor speed. The RPM is captured by
+  PWM modules which has PWM capture capability and this
+  drivers reads the captured data from PWM IP to convert
+  it to speed in RPM.
+
 if ACPI

 comment "ACPI drivers"
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index f814b4a..9dcc374 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -175,6 +175,7 @@ obj-$(CONFIG_SENSORS_WM8350)    += wm8350-hwmon.o
 obj-$(CONFIG_SENSORS_XGENE)    += xgene-hwmon.o

 obj-$(CONFIG_PMBUS)    += pmbus/
+obj-$(CONFIG_GENERIC_PWM_TACHOMETER) += generic-pwm-tachometer.o

 ccflags-$(CONFIG_HWMON_DEBUG_CHIP) := -DDEBUG

diff --git a/drivers/hwmon/generic-pwm-tachometer.c 
b/drivers/hwmon/generic-pwm-tachometer.c
new file mode 100644
index 000..9354d43
--- /dev/null
+++ b/drivers/hwmon/generic-pwm-tachometer.c
@@ -0,0 +1,112 @@
+/*
+ * Copyright (c) 2017-2018, NVIDIA CORPORATION.  All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope 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.
+ *
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+struct pwm_hwmon_tach {
+    struct device    *dev;
+    struct pwm_device    *pwm;
+    struct device    *hwmon;
+};
+
+static ssize_t show_rpm(struct device *dev, struct device_attribute *attr,
+    char *buf)
+{
+    struct pwm_hwmon_tach *ptt = dev_get_drvdata(dev);
+    struct pwm_device *pwm = ptt->pwm;
+    struct pwm_capture 

Re: [PATCH v2 2/7] i3c: Add core I3C infrastructure

[Greg Kroah-Hartman]

On Wed, Feb 21, 2018 at 03:22:48PM +0100, Boris Brezillon wrote:
> Hi Greg,
> 
> On Tue, 19 Dec 2017 10:36:43 +0100
> Greg Kroah-Hartman  wrote:
> 
> > On Tue, Dec 19, 2017 at 10:28:58AM +0100, Boris Brezillon wrote:
> > > On Tue, 19 Dec 2017 10:21:19 +0100
> > > Greg Kroah-Hartman  wrote:
> > >   
> > > > On Tue, Dec 19, 2017 at 10:13:36AM +0100, Boris Brezillon wrote:  
> > > > > On Tue, 19 Dec 2017 10:09:00 +0100
> > > > > Boris Brezillon  wrote:
> > > > > 
> > > > > > On Tue, 19 Dec 2017 09:52:50 +0100
> > > > > > Greg Kroah-Hartman  wrote:
> > > > > > 
> > > > > > > On Thu, Dec 14, 2017 at 04:16:05PM +0100, Boris Brezillon wrote:  
> > > > > > > 
> > > > > > > > +/**
> > > > > > > > + * i3c_device_match_id() - Find the I3C device ID entry 
> > > > > > > > matching an I3C dev
> > > > > > > > + * @i3cdev: the I3C device we're searching a match for
> > > > > > > > + * @id_table: the I3C device ID table
> > > > > > > > + *
> > > > > > > > + * Return: a pointer to the first entry matching @i3cdev, or 
> > > > > > > > NULL if there's
> > > > > > > > + *no match.
> > > > > > > > + */
> > > > > > > > +const struct i3c_device_id *
> > > > > > > > +i3c_device_match_id(struct i3c_device *i3cdev,
> > > > > > > > +   const struct i3c_device_id *id_table)
> > > > > > > > +{
> > > > > > > > +   const struct i3c_device_id *id;
> > > > > > > > +
> > > > > > > > +   /*
> > > > > > > > +* The lower 32bits of the provisional ID is just 
> > > > > > > > filled with a random
> > > > > > > > +* value, try to match using DCR info.
> > > > > > > > +*/
> > > > > > > > +   if (!I3C_PID_RND_LOWER_32BITS(i3cdev->info.pid)) {
> > > > > > > > +   u16 manuf = I3C_PID_MANUF_ID(i3cdev->info.pid);
> > > > > > > > +   u16 part = I3C_PID_PART_ID(i3cdev->info.pid);
> > > > > > > > +   u16 ext_info = 
> > > > > > > > I3C_PID_EXTRA_INFO(i3cdev->info.pid);
> > > > > > > > +
> > > > > > > > +   /* First try to match by manufacturer/part ID. 
> > > > > > > > */
> > > > > > > > +   for (id = id_table; id->match_flags != 0; id++) 
> > > > > > > > {
> > > > > > > > +   if ((id->match_flags & 
> > > > > > > > I3C_MATCH_MANUF_AND_PART) !=
> > > > > > > > +   I3C_MATCH_MANUF_AND_PART)
> > > > > > > > +   continue;
> > > > > > > > +
> > > > > > > > +   if (manuf != id->manuf_id || part != 
> > > > > > > > id->part_id)
> > > > > > > > +   continue;
> > > > > > > > +
> > > > > > > > +   if ((id->match_flags & 
> > > > > > > > I3C_MATCH_EXTRA_INFO) &&
> > > > > > > > +   ext_info != id->extra_info)
> > > > > > > > +   continue;
> > > > > > > > +
> > > > > > > > +   return id;
> > > > > > > > +   }
> > > > > > > > +   }
> > > > > > > > +
> > > > > > > > +   /* Fallback to DCR match. */
> > > > > > > > +   for (id = id_table; id->match_flags != 0; id++) {
> > > > > > > > +   if ((id->match_flags & I3C_MATCH_DCR) &&
> > > > > > > > +   id->dcr == i3cdev->info.dcr)
> > > > > > > > +   return id;
> > > > > > > > +   }
> > > > > > > > +
> > > > > > > > +   return NULL;
> > > > > > > > +}
> > > > > > > > +EXPORT_SYMBOL_GPL(i3c_device_match_id);
> > > > > > > 
> > > > > > > I just picked one random export here, but it feels like you are
> > > > > > > exporting a bunch of symbols you don't need to.  Why would 
> > > > > > > something
> > > > > > > outside of the i3c "core" need to call this function?  
> > > > > > 
> > > > > > Because I'm not passing the i3c_device_id to the ->probe() method, 
> > > > > > and
> > > > > > if the driver is supporting different variants of the device, it may
> > > > > > want to know which one is being probed.
> > > > > > 
> > > > > > I considered retrieving this information in the core just before 
> > > > > > probing
> > > > > > the driver and passing it to the ->probe() function, but it means
> > > > > > having an extra i3c_device_match_id() call for everyone even those 
> > > > > > who
> > > > > > don't care about the device_id information, so I thought exporting 
> > > > > > this
> > > > > > function was a good alternative (device drivers can use it when they
> > > > > > actually need to retrieve the device_id).
> > > > > > 
> > > > > > Anyway, that's something I can change if you think passing the
> > > > > > i3c_device_id to the ->probe() method is preferable.
> > > > > > 
> > > > > > > Have you looked
> > > > > > > to see if you really have callers for everything you are 
> > > > > > > exporting?  
> > > > > > 
> > > > > > Yes, I tried to only export functions that I 

Re: [PATCH v2 2/7] i3c: Add core I3C infrastructure

[Boris Brezillon]

Hi Greg,

On Tue, 19 Dec 2017 10:36:43 +0100
Greg Kroah-Hartman  wrote:

> On Tue, Dec 19, 2017 at 10:28:58AM +0100, Boris Brezillon wrote:
> > On Tue, 19 Dec 2017 10:21:19 +0100
> > Greg Kroah-Hartman  wrote:
> >   
> > > On Tue, Dec 19, 2017 at 10:13:36AM +0100, Boris Brezillon wrote:  
> > > > On Tue, 19 Dec 2017 10:09:00 +0100
> > > > Boris Brezillon  wrote:
> > > > 
> > > > > On Tue, 19 Dec 2017 09:52:50 +0100
> > > > > Greg Kroah-Hartman  wrote:
> > > > > 
> > > > > > On Thu, Dec 14, 2017 at 04:16:05PM +0100, Boris Brezillon wrote:
> > > > > >   
> > > > > > > +/**
> > > > > > > + * i3c_device_match_id() - Find the I3C device ID entry matching 
> > > > > > > an I3C dev
> > > > > > > + * @i3cdev: the I3C device we're searching a match for
> > > > > > > + * @id_table: the I3C device ID table
> > > > > > > + *
> > > > > > > + * Return: a pointer to the first entry matching @i3cdev, or 
> > > > > > > NULL if there's
> > > > > > > + *  no match.
> > > > > > > + */
> > > > > > > +const struct i3c_device_id *
> > > > > > > +i3c_device_match_id(struct i3c_device *i3cdev,
> > > > > > > + const struct i3c_device_id *id_table)
> > > > > > > +{
> > > > > > > + const struct i3c_device_id *id;
> > > > > > > +
> > > > > > > + /*
> > > > > > > +  * The lower 32bits of the provisional ID is just filled with a 
> > > > > > > random
> > > > > > > +  * value, try to match using DCR info.
> > > > > > > +  */
> > > > > > > + if (!I3C_PID_RND_LOWER_32BITS(i3cdev->info.pid)) {
> > > > > > > + u16 manuf = I3C_PID_MANUF_ID(i3cdev->info.pid);
> > > > > > > + u16 part = I3C_PID_PART_ID(i3cdev->info.pid);
> > > > > > > + u16 ext_info = I3C_PID_EXTRA_INFO(i3cdev->info.pid);
> > > > > > > +
> > > > > > > + /* First try to match by manufacturer/part ID. */
> > > > > > > + for (id = id_table; id->match_flags != 0; id++) {
> > > > > > > + if ((id->match_flags & 
> > > > > > > I3C_MATCH_MANUF_AND_PART) !=
> > > > > > > + I3C_MATCH_MANUF_AND_PART)
> > > > > > > + continue;
> > > > > > > +
> > > > > > > + if (manuf != id->manuf_id || part != 
> > > > > > > id->part_id)
> > > > > > > + continue;
> > > > > > > +
> > > > > > > + if ((id->match_flags & I3C_MATCH_EXTRA_INFO) &&
> > > > > > > + ext_info != id->extra_info)
> > > > > > > + continue;
> > > > > > > +
> > > > > > > + return id;
> > > > > > > + }
> > > > > > > + }
> > > > > > > +
> > > > > > > + /* Fallback to DCR match. */
> > > > > > > + for (id = id_table; id->match_flags != 0; id++) {
> > > > > > > + if ((id->match_flags & I3C_MATCH_DCR) &&
> > > > > > > + id->dcr == i3cdev->info.dcr)
> > > > > > > + return id;
> > > > > > > + }
> > > > > > > +
> > > > > > > + return NULL;
> > > > > > > +}
> > > > > > > +EXPORT_SYMBOL_GPL(i3c_device_match_id);
> > > > > > 
> > > > > > I just picked one random export here, but it feels like you are
> > > > > > exporting a bunch of symbols you don't need to.  Why would something
> > > > > > outside of the i3c "core" need to call this function?  
> > > > > 
> > > > > Because I'm not passing the i3c_device_id to the ->probe() method, and
> > > > > if the driver is supporting different variants of the device, it may
> > > > > want to know which one is being probed.
> > > > > 
> > > > > I considered retrieving this information in the core just before 
> > > > > probing
> > > > > the driver and passing it to the ->probe() function, but it means
> > > > > having an extra i3c_device_match_id() call for everyone even those who
> > > > > don't care about the device_id information, so I thought exporting 
> > > > > this
> > > > > function was a good alternative (device drivers can use it when they
> > > > > actually need to retrieve the device_id).
> > > > > 
> > > > > Anyway, that's something I can change if you think passing the
> > > > > i3c_device_id to the ->probe() method is preferable.
> > > > > 
> > > > > > Have you looked
> > > > > > to see if you really have callers for everything you are exporting? 
> > > > > >  
> > > > > 
> > > > > Yes, I tried to only export functions that I think will be needed by
> > > > > I3C device drivers and I3C master drivers. Note that I didn't post the
> > > > > dummy device driver I developed to test the framework (partly because
> > > > > this is 
> > > > 
> > > > Sorry, I hit the send button before finishing my sentence :-).
> > > > 
> > > > "
> > > > Note that I didn't post the dummy device driver [1] I developed to test
> > > > the framework (partly because the quality of the code does not meet
> > > > mainline standards and I was ashamed of posting it publicly :-)), but
> > > > this 

Re: [PATCH 00/23] kconfig: move compiler capability tests to Kconfig

[Masahiro Yamada]

2018-02-21 19:52 GMT+09:00 Arnd Bergmann :
> On Wed, Feb 21, 2018 at 11:20 AM, Masahiro Yamada
>  wrote:
>> 2018-02-21 18:56 GMT+09:00 Arnd Bergmann :
>>> On Wed, Feb 21, 2018 at 8:38 AM, Masahiro Yamada
>>>  wrote:
 2018-02-20 0:18 GMT+09:00 Ulf Magnusson :
>>
>> Let me clarify my concern.
>>
>> When we test the compiler flag, is there a case
>> where a particular flag depends on -m{32,64} ?
>>
>> For example, is there a compiler that supports -fstack-protector
>> for 64bit mode, but unsupports it for 32bit mode?
>>
>>   $(cc-option -m32) ->  y
>>   $(cc-option -m64) ->  y
>>   $(cc-option -fstack-protector)->  y
>>   $(cc-option -m32 -fstack-protector)   ->  n
>>   $(cc-option -m64 -fstack-protector)   ->  y
>>
>> I guess this is unlikely to happen,
>> but I am not whether it is zero possibility.
>>
>> If this could happen,
>> $(cc-option ) must be evaluated together with
>> correct bi-arch option (either -m32 or -m64).
>>
>>
>> Currently, -m32/-m64 is specified in Makefile,
>> but we are moving compiler tests to Kconfig
>> and, CONFIG_64BIT can be dynamically toggled in Kconfig.
>
> I don't think it can happen for this particular combination (stack protector
> and word size), but I'm sure we'll eventually run into options that
> need to be tested in combination. For the current CFLAGS_KERNEL
> setting, we definitely have the case of needing the variables to be
> evaluated in a specific order.
>




I was thinking of how we can handle complex cases
in the current approach.



(Case 1)

Compiler flag -foo and -bar interacts, so
we also need to check the combination of the two.


config CC_HAS_FOO
def_bool $(cc-option -foo)

config CC_HAS_BAR
def_bool $(cc-option -bar)

config CC_HAS_FOO_WITH_BAR
def_bool $(cc-option -foo -bar)



(Case 2)
Compiler flag -foo is sensitive to word-size.
So, we need to test this option together with -m32/-m64.
User can toggle CONFIG_64BIT, like i386/x86_64.


config CC_NEEDS_M64
  def_bool $(cc-option -m64) && 64BIT

config CC_NEEDS_M32
  def_bool $(cc-option -m32) && !64BIT

config CC_HAS_FOO
 bool
 default $(cc-option -m64 -foo) if CC_NEEDS_M64
 default $(cc-option -m32 -foo) if CC_NEEDS_M32
 default $(cc-option -foo)



(Case 3)
Compiler flag -foo is sensitive to endian-ness.


config CC_NEEDS_BIG_ENDIAN
  def_bool $(cc-option -mbig-endian) && CPU_BIG_ENDIAN

config CC_NEEDS_LITTLE_ENDIAN
  def_bool $(cc-option -mlittle-endian) && CPU_LITTLE_ENDIAN

config CC_HAS_FOO
 bool
 default $(cc-option -mbig-endian -foo) if CC_NEEDS_BIG_ENDIAN
 default $(cc-option -mlittle-endian -foo) if CC_NEEDS_LITTLE_ENDIAN
 default $(cc-option -foo)




Hmm, I think I can implement those somehow.
But, I hope we do not have many instances like this...


If you know more naive cases, please share your knowledge.

Thanks!


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

Re: [PATCH 00/23] kconfig: move compiler capability tests to Kconfig

[Arnd Bergmann]

On Wed, Feb 21, 2018 at 11:20 AM, Masahiro Yamada
 wrote:
> 2018-02-21 18:56 GMT+09:00 Arnd Bergmann :
>> On Wed, Feb 21, 2018 at 8:38 AM, Masahiro Yamada
>>  wrote:
>>> 2018-02-20 0:18 GMT+09:00 Ulf Magnusson :
>
> Let me clarify my concern.
>
> When we test the compiler flag, is there a case
> where a particular flag depends on -m{32,64} ?
>
> For example, is there a compiler that supports -fstack-protector
> for 64bit mode, but unsupports it for 32bit mode?
>
>   $(cc-option -m32) ->  y
>   $(cc-option -m64) ->  y
>   $(cc-option -fstack-protector)->  y
>   $(cc-option -m32 -fstack-protector)   ->  n
>   $(cc-option -m64 -fstack-protector)   ->  y
>
> I guess this is unlikely to happen,
> but I am not whether it is zero possibility.
>
> If this could happen,
> $(cc-option ) must be evaluated together with
> correct bi-arch option (either -m32 or -m64).
>
>
> Currently, -m32/-m64 is specified in Makefile,
> but we are moving compiler tests to Kconfig
> and, CONFIG_64BIT can be dynamically toggled in Kconfig.

I don't think it can happen for this particular combination (stack protector
and word size), but I'm sure we'll eventually run into options that
need to be tested in combination. For the current CFLAGS_KERNEL
setting, we definitely have the case of needing the variables to be
evaluated in a specific order.

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

Re: [PATCH 00/23] kconfig: move compiler capability tests to Kconfig

[Masahiro Yamada]

2018-02-21 18:56 GMT+09:00 Arnd Bergmann :
> On Wed, Feb 21, 2018 at 8:38 AM, Masahiro Yamada
>  wrote:
>> 2018-02-20 0:18 GMT+09:00 Ulf Magnusson :
>>

 I'm not happy that we in one context can reference CONFIG variables
 directly, but inside the $(call ...) and $(shell ...) needs the $ prefix.
 But I could not come up with something un-ambigious where this could be 
 avoided.
>>>
>>> I think we should be careful about allowing references to config
>>> symbols. It mixes up the parsing and evaluation phases, since $() is
>>> expanded during parsing (which I consider a feature and think is
>>> needed to retain sanity).
>>>
>>> Patch 06/23 removes the last existing instance of symbol references in
>>> strings by getting rid of 'option env'. That's an improvement to me.
>>> We shouldn't add it back.
>>
>>
>> This is really important design decision,
>> so I'd like to hear a little more from experts.
>>
>>
>> For example, x86 allows users to choose sub-arch, either 'i386' or 'x86_64'.
>>
>> https://github.com/torvalds/linux/blob/v4.16-rc2/arch/x86/Kconfig#L4
>>
>>
>>
>> If the user toggles CONFIG_64BIT,
>> the bi-arch compiler will work in a slightly different mode
>> (at least, back-end parts)
>>
>> So, my question is, is there a case,
>>
>> $(cc-option, -m32 -foo) is y, but
>> $(cc-option, -m64 -foo) is n  ?
>> (or vice versa)
>>
>>
>> If the answer is yes, $(cc-option -foo) would have to be re-calculated
>> every time CONFIG_64BIT is toggled.
>>
>> This is what I'd like to avoid, though.
>
> The -m32/-m64 trick (and -mbig-endian/-mlittle-endian on other architectures
> as well as a couple of other flags) only works if the compiler is configured 
> to
> support it. In other cases (e.g. big-endian xtensa), the kernel always
> detects what the compiler does and silently configures itself to match
> using Makefile logic.
>
> On x86, compilers are usually built as bi-arch, but you can build one that
> only allows one of them.
>
> I can see two reasonable ways out:
>
> - we don't use  $(cc-option -foo) in a case like this, and instead require the
>   user to have a matching toolchain.
> - we could make the 32/64 selection on x86 a 'choice' statement where
>   each option depends on both the ARCH= variable and the
>   $(cc-option, -m32)/ $(cc-option, -m64) output.
>
>Arnd



Let me clarify my concern.

When we test the compiler flag, is there a case
where a particular flag depends on -m{32,64} ?

For example, is there a compiler that supports -fstack-protector
for 64bit mode, but unsupports it for 32bit mode?

  $(cc-option -m32) ->  y
  $(cc-option -m64) ->  y
  $(cc-option -fstack-protector)->  y
  $(cc-option -m32 -fstack-protector)   ->  n
  $(cc-option -m64 -fstack-protector)   ->  y

I guess this is unlikely to happen,
but I am not whether it is zero possibility.

If this could happen,
$(cc-option ) must be evaluated together with
correct bi-arch option (either -m32 or -m64).


Currently, -m32/-m64 is specified in Makefile,
but we are moving compiler tests to Kconfig
and, CONFIG_64BIT can be dynamically toggled in Kconfig.




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

Re: [PATCH 00/23] kconfig: move compiler capability tests to Kconfig

[Arnd Bergmann]

On Wed, Feb 21, 2018 at 8:38 AM, Masahiro Yamada
 wrote:
> 2018-02-20 0:18 GMT+09:00 Ulf Magnusson :
>
>>>
>>> I'm not happy that we in one context can reference CONFIG variables
>>> directly, but inside the $(call ...) and $(shell ...) needs the $ prefix.
>>> But I could not come up with something un-ambigious where this could be 
>>> avoided.
>>
>> I think we should be careful about allowing references to config
>> symbols. It mixes up the parsing and evaluation phases, since $() is
>> expanded during parsing (which I consider a feature and think is
>> needed to retain sanity).
>>
>> Patch 06/23 removes the last existing instance of symbol references in
>> strings by getting rid of 'option env'. That's an improvement to me.
>> We shouldn't add it back.
>
>
> This is really important design decision,
> so I'd like to hear a little more from experts.
>
>
> For example, x86 allows users to choose sub-arch, either 'i386' or 'x86_64'.
>
> https://github.com/torvalds/linux/blob/v4.16-rc2/arch/x86/Kconfig#L4
>
>
>
> If the user toggles CONFIG_64BIT,
> the bi-arch compiler will work in a slightly different mode
> (at least, back-end parts)
>
> So, my question is, is there a case,
>
> $(cc-option, -m32 -foo) is y, but
> $(cc-option, -m64 -foo) is n  ?
> (or vice versa)
>
>
> If the answer is yes, $(cc-option -foo) would have to be re-calculated
> every time CONFIG_64BIT is toggled.
>
> This is what I'd like to avoid, though.

The -m32/-m64 trick (and -mbig-endian/-mlittle-endian on other architectures
as well as a couple of other flags) only works if the compiler is configured to
support it. In other cases (e.g. big-endian xtensa), the kernel always
detects what the compiler does and silently configures itself to match
using Makefile logic.

On x86, compilers are usually built as bi-arch, but you can build one that
only allows one of them.

I can see two reasonable ways out:

- we don't use  $(cc-option -foo) in a case like this, and instead require the
  user to have a matching toolchain.
- we could make the 32/64 selection on x86 a 'choice' statement where
  each option depends on both the ARCH= variable and the
  $(cc-option, -m32)/ $(cc-option, -m64) output.

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