[RFC tip/locking/lockdep v5 03/17] lockdep: Redefine LOCK_*_STATE* bits
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: (&sio_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, - LOCK_US
[RFC tip/locking/lockdep v5 16/17] lockdep: Documention for recursive read lock detection reasoning
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 from +A0 to An: + + A0 -> A1 -> ... ->
[RFC PATCH] drivers/peci: peci_match_id() can be static
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
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
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"
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
* 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"
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
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"
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"
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
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
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
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
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) fpregset = uctxt->uc_mcontext.fpregs; fpregs = (voi
[PATCH v12 02/22] selftests/vm: rename all references to pkru to a generic name
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<>>>===SIGSEGV\n"); - dprintf1("%s()::%d, pkru: 0x%x shadow: %x\n", __func__, __LINE__, - __rdpkru(), shadow_pkru); + dprintf1("%s()::%d, pkey_reg: 0x%x shadow: %x\n", __func__, __LINE__, + __rdpkey_reg(), shadow_pkey_reg); trapno = uctxt->uc_mcontext.gregs[REG_TRAPNO]; ip = uctxt->uc_mcontext.gregs[REG_IP_IDX]; @@ -275,19 +275,19 @@ void signal_handler(int signum, siginfo_t *si, void *vucontext) */ fpregs += 0x70; #endif - pkru_offset = pkru_xstate_offset(); - pkru_ptr = (void *)(&fpregs[pkru_offset]); + pkey_reg_offset = pkey_reg_xstate_offset(); + pkey_reg_ptr = (void *)(&fpregs[pkey_reg_offset]); dprintf1("siginfo: %p\n", si); dprintf1(" fpregs: %p\n", fpregs); /* -* If we got a PKRU fault, we *HAVE* to have at least one bit set in +* If we got a PKEY fault, we *HAVE* to have at least one bit set in * here. */ - dprintf1("pkru_xstate_offset: %d\n", pkru_xstate_offset()); + dprintf1("pkey_reg_xstate_offset: %d\n", pkey_reg_xstate_offset()); if (DEBUG_LEVEL > 4) - dump
[PATCH v12 05/22] selftests/vm: generic function to handle shadow key register
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 "unsubscribe linux-doc" in the body of a message to majord...@vger
Re: [PATCH 4/5 resend] documentation: firewire: add basic firewire.rst to driver-api
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
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()
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()
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
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
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
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
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 of instruction * 16384bytes = 1 page */ +#define __page_o_noops()
[PATCH v12 14/22] selftests/vm: clear the bits in shadow reg when a pkey is freed.
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
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
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()
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 20/22] selftests/vm: testcases must restore pkey-permissions
Generally the signal handler restores the state of the pkey register before returning. However there are times when the read/write operation can legitamely fail without invoking the signal handler. Eg: A sys_read() operaton to a write-protected page should be disallowed. In such a case the state of the pkey register is not restored to its original state. The test case is responsible for restoring the key register state to its original value. cc: Dave Hansen cc: Florian Weimer Signed-off-by: Ram Pai --- tools/testing/selftests/vm/protection_keys.c |5 + 1 files changed, 5 insertions(+), 0 deletions(-) diff --git a/tools/testing/selftests/vm/protection_keys.c b/tools/testing/selftests/vm/protection_keys.c index 437ee74..42c068a 100644 --- a/tools/testing/selftests/vm/protection_keys.c +++ b/tools/testing/selftests/vm/protection_keys.c @@ -1003,6 +1003,7 @@ void test_read_of_write_disabled_region(int *ptr, u16 pkey) ptr_contents = read_ptr(ptr); dprintf1("*ptr: %d\n", ptr_contents); dprintf1("\n"); + pkey_write_allow(pkey); } void test_read_of_access_disabled_region(int *ptr, u16 pkey) { @@ -1082,6 +1083,7 @@ void test_kernel_write_of_access_disabled_region(int *ptr, u16 pkey) ret = read(test_fd, ptr, 1); dprintf1("read ret: %d\n", ret); pkey_assert(ret); + pkey_access_allow(pkey); } void test_kernel_write_of_write_disabled_region(int *ptr, u16 pkey) { @@ -1094,6 +1096,7 @@ void test_kernel_write_of_write_disabled_region(int *ptr, u16 pkey) if (ret < 0 && (DEBUG_LEVEL > 0)) perror("verbose read result (OK for this to be bad)"); pkey_assert(ret); + pkey_write_allow(pkey); } void test_kernel_gup_of_access_disabled_region(int *ptr, u16 pkey) @@ -1113,6 +1116,7 @@ void test_kernel_gup_of_access_disabled_region(int *ptr, u16 pkey) vmsplice_ret = vmsplice(pipe_fds[1], &iov, 1, SPLICE_F_GIFT); dprintf1("vmsplice() ret: %d\n", vmsplice_ret); pkey_assert(vmsplice_ret == -1); + pkey_access_allow(pkey); close(pipe_fds[0]); close(pipe_fds[1]); @@ -1133,6 +1137,7 @@ void test_kernel_gup_write_to_write_disabled_region(int *ptr, u16 pkey) if (DEBUG_LEVEL > 0) perror("futex"); dprintf1("futex() ret: %d\n", futex_ret); + pkey_write_allow(pkey); } /* Assumes that all pkeys other than 'pkey' are unallocated */ -- 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
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
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
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
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
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__, __rdpkey_reg(), shadow_pkey_reg); @@ -692,7 +683,9 @@ void record
[PATCH v12 08/22] selftests/vm: clear the bits in shadow reg when a pkey is freed.
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
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 03/22] selftests/vm: move generic definitions to header file
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 abort_hooks(void) #endif } -static inline void __page_o_noops(void) -{ -
[PATCH v12 00/22] selftests, powerpc, x86 : Memory Protection Keys
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
Re: [PATCH 4/5 resend] documentation: firewire: add basic firewire.rst to driver-api
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
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
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
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
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
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
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 &fw_cdev_event_request2 + * @closure: See &fw_cdev_event_common; set by %FW_CDEV_IOC_ALLOCATE ioctl * @type: See &fw_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 &fw_cdev_event_request2 if the kernel or * the client implements ABI version <= 3. &fw_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. * * &struct 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"
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
> 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 00/13] Remove metag architecture
These patches remove the metag architecture and tightly dependent drivers from the kernel. With the 4.16 kernel the ancient gcc 4.2.4 based metag toolchain we have been using is hitting compiler bugs, so now seems a good time to drop it altogether. Quoting from patch 1: The earliest Meta architecture port of Linux I have a record of was an import of a Meta port of Linux v2.4.1 in February 2004, which was worked on significantly over the next few years by Graham Whaley, Will Newton, Matt Fleming, myself and others. Eventually the port was merged into mainline in v3.9 in March 2013, not long after Imagination Technologies bought MIPS Technologies and shifted its CPU focus over to the MIPS architecture. As a result, though the port was maintained for a while, kept on life support for a while longer, and useful for testing a few specific drivers for which I don't have ready access to the equivalent MIPS hardware, it is now essentially dead with no users. It is also stuck using an out-of-tree toolchain based on GCC 4.2.4 which is no longer maintained, now struggles to build modern kernels due to toolchain bugs, and doesn't itself build with a modern GCC. The latest buildroot port is still using an old uClibc snapshot which is no longer served, and the latest uClibc doesn't build with GCC 4.2.4. So lets call it a day and drop the Meta architecture port from the kernel. RIP Meta. Cc: Guenter Roeck Cc: Jonathan Corbet Cc: Steven Rostedt Cc: Ingo Molnar Cc: Peter Zijlstra Cc: Arnaldo Carvalho de Melo Cc: Alexander Shishkin Cc: Jiri Olsa Cc: Namhyung Kim Cc: Thomas Gleixner Cc: Jason Cooper Cc: Marc Zyngier Cc: Daniel Lezcano Cc: Greg Kroah-Hartman Cc: Jiri Slaby Cc: Linus Walleij Cc: Wim Van Sebroeck Cc: Mauro Carvalho Chehab Cc: Mauro Carvalho Chehab Cc: Wolfram Sang Cc: linux-me...@vger.kernel.org Cc: linux-doc@vger.kernel.org Cc: linux...@kvack.org Cc: linux-g...@vger.kernel.org Cc: linux-watch...@vger.kernel.org Cc: linux-me...@vger.kernel.org Cc: linux-...@vger.kernel.org James Hogan (13): metag: Remove arch/metag/ docs: Remove metag docs docs: Remove remaining references to metag Drop a bunch of metag references irqchip: Remove metag irqchip drivers clocksource: Remove metag generic timer driver tty: Remove metag DA TTY and console driver MAINTAINERS/CREDITS: Drop METAG ARCHITECTURE pinctrl: Drop TZ1090 drivers gpio: Drop TZ1090 drivers watchdog: imgpdc: Drop METAG dependency media: img-ir: Drop METAG dependency i2c: img-scb: Drop METAG dependency CREDITS|5 + Documentation/00-INDEX |2 - Documentation/admin-guide/kernel-parameters.txt|4 - Documentation/dev-tools/kmemleak.rst |2 +- .../devicetree/bindings/gpio/gpio-tz1090-pdc.txt | 45 - .../devicetree/bindings/gpio/gpio-tz1090.txt | 88 - Documentation/devicetree/bindings/metag/meta.txt | 30 - .../bindings/pinctrl/img,tz1090-pdc-pinctrl.txt| 127 -- .../bindings/pinctrl/img,tz1090-pinctrl.txt| 227 --- .../features/core/BPF-JIT/arch-support.txt |1 - .../core/generic-idle-thread/arch-support.txt |1 - .../features/core/jump-labels/arch-support.txt |1 - .../features/core/tracehook/arch-support.txt |1 - .../features/debug/KASAN/arch-support.txt |1 - .../debug/gcov-profile-all/arch-support.txt|1 - Documentation/features/debug/kgdb/arch-support.txt |1 - .../debug/kprobes-on-ftrace/arch-support.txt |1 - .../features/debug/kprobes/arch-support.txt|1 - .../features/debug/kretprobes/arch-support.txt |1 - .../features/debug/optprobes/arch-support.txt |1 - .../features/debug/stackprotector/arch-support.txt |1 - .../features/debug/uprobes/arch-support.txt|1 - .../debug/user-ret-profiler/arch-support.txt |1 - .../features/io/dma-api-debug/arch-support.txt |1 - .../features/io/dma-contiguous/arch-support.txt|1 - .../features/io/sg-chain/arch-support.txt |1 - .../features/lib/strncasecmp/arch-support.txt |1 - .../locking/cmpxchg-local/arch-support.txt |1 - .../features/locking/lockdep/arch-support.txt |1 - .../locking/queued-rwlocks/arch-support.txt|1 - .../locking/queued-spinlocks/arch-support.txt |1 - .../locking/rwsem-optimized/arch-support.txt |1 - .../features/perf/kprobes-event/arch-support.txt |1 - .../features/perf/perf-regs/arch-support.txt |1 - .../features/perf/perf-stackdump/arch-support.txt |1 - .../sched/membarrier-sync-core/arch-support.txt|1 - .../features/sched/numa-balancing/arch-support.txt |1 - .../seccomp/seccomp-filter/arch-support.txt|1 - .../time/arch-tick-broadcast/arch-support.txt |1 - .../features/time/clockevents/arch-support.txt |1 - .../time/c
[PATCH 02/13] docs: Remove metag docs
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
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 --- a/Documentation/features/core/generic-idle-thread/arch-support.txt +++ b/Document
Re: [PATCH v2 7/8] [PATCH 7/8] drivers/hwmon: Add a generic PECI hwmon client driver
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; +} + +static s32 ten_dot_six_to_mill
Re: [PATCH v2 7/8] [PATCH 7/8] drivers/hwmon: Add a generic PECI hwmon client driver
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 ((
Re: [PATCH v2 1/8] [PATCH 1/8] drivers/peci: Add support for PECI bus driver core
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(&adapter->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(&adapter->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
> >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(&adapter->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(&adapter->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
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_hwmo
Re: [PATCH 00/23] kconfig: move compiler capability tests to Kconfig
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 properly invalidated. It looks like at least menuconfig just does a dumb invalidate-everything whenever the
Re: [PATCH v2 7/8] [PATCH 7/8] drivers/hwmon: Add a generic PECI hwmon client driver
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 / 64); +} + +static int get_tjmax(st
Re: [PATCH v2 1/8] [PATCH 1/8] drivers/peci: Add support for PECI bus driver core
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
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
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(&adapter->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(&adapter->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(&adapter->bus_lock); + if (!rc) + return -EAGAIN; /* PECI activity is ongoing */ + } else { + rt_mutex_lock(&adapter->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(&adapter->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(&adapter->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, &msg); + if (rc < 0) { + dev_dbg(&adapter->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(&adapter->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-
Re: [PATCH v2 1/8] [PATCH 1/8] drivers/peci: Add support for PECI bus driver core
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)
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 ++ arch/sparc/include/asm/mmu_context_64.h | 50 ++ arch/sparc/include/asm/page_64.h|
[PATCH v12 00/11] Application Data Integrity feature introduced by SPARC M7
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 exc
Re: [PATCH v2 2/8] [PATCH 2/8] Documentations: dt-bindings: Add a document of PECI adapter driver for Aspeed AST24xx/25xx SoCs
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
> +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(&adapter->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(&adapter->bus_lock); > + if (!rc) > + return -EAGAIN; /* PECI activity is ongoing */ > + } else { > + rt_mutex_lock(&adapter->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(&adapter->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, &msg); > + if (rc < 0) { > + dev_dbg(&adapter->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(&adapter->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(&adapter->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; > + > +
[PATCH v2 1/8] [PATCH 1/8] drivers/peci: Add support for PECI bus driver core
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 == &peci_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[] = { + &dev_attr_name.attr, + NULL +}; +ATTRIBUTE_GROUPS(peci_device); + +static struct device_type peci_client_type = { + .grou
[PATCH v2 2/8] [PATCH 2/8] Documentations: dt-bindings: Add a document of PECI adapter driver for Aspeed AST24xx/25xx SoCs
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 = <&clk_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 6/8] [PATCH 6/8] Documentation: hwmon: Add a document for PECI hwmon client driver
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 3/8] [PATCH 3/8] ARM: dts: aspeed: peci: Add PECI node
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 = &uart4; serial4 = &uart5; serial5 = &vuart; + peci0 = &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 @@ }; }; +&peci { + peci0: peci-bus@0 { + compatible = "aspeed,ast2400-peci"; + reg = <0x0 0x60>; + #address-cells = <1>; + #size-cells = <0>; + interrupts = <15>; + clocks = <&syscon 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 { 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 = &uart4; serial4 = &uart5; serial5 = &vuart; + peci0 = &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 @@ }; }; +&peci { + peci0: peci-bus@0 { + compatible = "aspeed,ast2500-peci"; + reg = <0x0 0x60>; + #address-cells = <1>; + #size-cells = <0>; + interrupts = <15>; + clocks = <&syscon 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 { 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 7/8] [PATCH 7/8] drivers/hwmon: Add a generic PECI hwmon client driver
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; + msg.index = MBX_INDEX_TEMP_TARGET; +
[PATCH v2 8/8] [PATCH 8/8] Add a maintainer for the PECI subsystem
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
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 driv
[PATCH v2 4/8] [PATCH 4/8] drivers/peci: Add a PECI adapter driver for Aspeed AST24xx/AST25xx
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 PECI_READ_LEN(x) (((x) << 16) & PECI_REA
[PATCH v2 5/8] [PATCH [5/8] Documentation: dt-bindings: Add a document for PECI hwmon client driver
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
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 by > >>>+ PWM modules which has
Re: [PATCH 00/23] kconfig: move compiler capability tests to Kconfig
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
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 injection duration being fixed, it
[PATCH] tracing: Remove redundant subject
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
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 WITHOUT + * ANY WARRANTY; without ev
Re: [PATCH 05/10] hwmon: generic-pwm-tachometer: Add generic PWM based tachometer
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, &result, 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[] = { + &sensor_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 pwm_tach_probe(struct p
Re: [PATCH 05/10] hwmon: generic-pwm-tachometer: Add generic PWM based tachometer
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 result; + int err; + unsigned int rpm
Re: [PATCH v2 2/7] i3c: Add core I3C infrastructure
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 think will be needed by > > > > > > I3C device drivers and I3C master drivers. Note that I didn't post > > > > > > the > >
Re: [PATCH v2 2/7] i3c: Add core I3C infrastructure
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 driver is using some of the exported functions. > > > > " > > > > > > We don't export functions that has no in-kern
Re: [PATCH 00/23] kconfig: move compiler capability tests to Kconfig
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
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
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
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