Hi Dave,

On 03/10/2016 08:43 PM, Dave Hansen wrote:

Michael, thanks again for the detailed review.  I've tried to respond to
all your comments.  Here's an incremental patch from the last one.  I'll
also resend the entire thing shortly.

Okay. (I'll wait for the resend before further review.)

	https://www.sr71.net/~dave/intel/pkey-man-20160310.0.patch

Just a few questions to clarify some of your review comments below...

On 03/10/2016 09:07 AM, Michael Kerrisk (man-pages) wrote:

quoted

On 03/09/2016 10:40 PM, Dave Hansen wrote:

quoted

+.PP
+.I flags
+may contain zero or more disable operations:
+.TP
+.B PKEY_DISABLE_ACCESS
+Disable all data access to memory covered by the returned protection key.
+.TP
+.B PKEY_DISABLE_WRITE
+Disable write access to memory covered by the returned protection key.
+.SH RETURN VALUE
+On success,
+.BR pkey_alloc ()
+returns a positive protection key value.
+.BR pkey_free ()
+returns zero.
+On error, \-1 is returned, and
+.I errno
+is set appropriately.
+.SH ERRORS
+.TP
+.B EINVAL
+.IR pkey ,
+.IR flags ,
+or
+.I access_rights
+is invalid.
+.TP
+.B ENOSPC

At the start of the following paragraph, add

.(RB pkey_alloc ())

so that the reader knows that this error applies only for that syscall.

I'm not seeing that actually affect the formatting in the resulting
manpage as I view it.  Is that really the syntax you want?  Is there
another example bit of syntax I should be looking for?

Sorry!

.RB ( pkey_alloc ())

...

quoted

In a previous review of the pages, I asked:

[[
And I have a question (and the answer probably should 
be documented in the manual page).  What happens when 
one signal handler interrupts the execution of another? 
Do pkey_set() calls in the first handler persist into the 
second handler? I presume not, but it would be good to 
be a little more explicit about this.
]]

I think this point does need to be covered in the man page.

I did reword some things in response to this review comment, but not
thoroughly enough. :)

How about the following as a change to the existing second paragraph?

The effects of a call to
.BR pkey_set ()
from a signal handler will not persist when the signal handler
returns.  This is true whether the return is to a normal,
non-signal context, or to another signal handler.

How about this:

The effects of a call to
.BR pkey_set ()
from a signal handler will not persist when control passes out of
the signal handler.
This is true both when the handler returns to a normal,
nonsignal context, and when the signal handler is interrupted
by another signal handler.

...

quoted

quoted

+
+To use this feature, the processor must support it, and Linux
+must contain support for the feature on a given processor.  As of
+early 2016 only future Intel x86 processors are supported, and this
+hardware supports 16 protection keys in each process.  However,
+pkey 0 is used as the default key, so a maximum of 15 are available
+for actual application use.

Is there a recommended way for an application to discover whether the
system supports pkeys? If so, that should be documented here.

I've added the following text:

Any application wanting to use protection keys needs to be able
to function without them.  They might be unavailable because the
hardware the application runs on does not support them, the

s/hardware/hardware that/
(for easier parsing)

kernel code does not contain support, the kernel support has been
disabled, or because the keys have all been allocated, perhaps by
a library the application is using.  It is recommended that
applications wanting to use protection keys should simply call
.BR pkey_alloc()

s/()/ ()/

instead of attempting to detect support for the
feature in any other way.

Hardware support for protection keys may be enumerated with
the cpuid instruction.  Details on how to do this can be
found in the Intel Software Developers Manual.  The kernel
performs this enumeration and exposes the information in
/proc/cpuinfo under the "flags" field.  "pku" in this field
indicates hardware support for protection keys and "ospke"
indicates that the kernel contains and has enabled protection
keys support,.

...

quoted

quoted

+.BR pkey_mprotect (2),
+.BR pkey_alloc (2),
+.BR pkey_free (2),
+.BR pkey_set (2),
+.BR pkey_get (2),

Would it be possible to get a small, complete working example program
in one of these pages? The axample could show how pkeys override
traditional memory protections. I appreciate that the rest of us do
not yet have suitable hardware, but presumably you do.

Sure, I'll include one in pkey(7).

Thanks.

Cheers,

Michael



-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/
--
To unsubscribe from this list: send the line "unsubscribe linux-man" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html