diff mbox series

[10/14] prctl.2: Add PR_SPEC_INDIRECT_BRANCH for SPECULATION_CTRL prctls

Message ID 1589301419-24459-11-git-send-email-Dave.Martin@arm.com (mailing list archive)
State New, archived
Headers show
Series prctl.2 man page updates for Linux 5.6 | expand

Commit Message

Dave Martin May 12, 2020, 4:36 p.m. UTC 
Add the PR_SPEC_INDIRECT_BRANCH "misfeature" added in Linux 4.20
for PR_SET_SPECULATION_CTRL and PR_GET_SPECULATION_CTRL.

Signed-off-by: Dave Martin <Dave.Martin@arm.com>
Cc: Tim Chen <tim.c.chen@linux.intel.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
---
 man2/prctl.2 | 24 ++++++++++++++++++------
 1 file changed, 18 insertions(+), 6 deletions(-)

Comments

Michael Kerrisk (man-pages) May 13, 2020, 11:21 a.m. UTC | #1 
Hello Dave,

On 5/12/20 6:36 PM, Dave Martin wrote:
> Add the PR_SPEC_INDIRECT_BRANCH "misfeature" added in Linux 4.20
> for PR_SET_SPECULATION_CTRL and PR_GET_SPECULATION_CTRL.
> 
> Signed-off-by: Dave Martin <Dave.Martin@arm.com>
> Cc: Tim Chen <tim.c.chen@linux.intel.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>

Thanks. Patch applied, but not yet pushed while I wait to see if any
Review/Ack arrives.

Also, could you please check the tweaks I note below.

> ---
>  man2/prctl.2 | 24 ++++++++++++++++++------
>  1 file changed, 18 insertions(+), 6 deletions(-)
> 
> diff --git a/man2/prctl.2 b/man2/prctl.2
> index e8eaf95..66417cf 100644
> --- a/man2/prctl.2
> +++ b/man2/prctl.2
> @@ -1213,11 +1213,20 @@ arguments must be specified as 0; otherwise the call fails with the error
>  .\" commit 356e4bfff2c5489e016fdb925adbf12a1e3950ee
>  Sets the state of the speculation misfeature specified in
>  .IR arg2 .
> -Currently, the only permitted value for this argument is
> +Currently, this argument must be one of:
> +.RS
> +.TP
>  .B PR_SPEC_STORE_BYPASS
> -(otherwise the call fails with the error
> +speculative store bypass control, or

s/speculative/enable speculative/

> +.\" commit 9137bb27e60e554dab694eafa4cca241fa3a694f
> +.TP
> +.BR PR_SPEC_INDIRECT_BRANCH " (since Linux 4.20)"
> +indirect branch speculation control.

s/indirect/enable indirect/

> +.RE
> +.IP
> +(Otherwise the call fails with the error
>  .BR ENODEV ).
> -This setting is a per-thread attribute.
> +These settings are per-thread attributes.
>  The
>  .IR arg3
>  argument is used to hand in the control value,
> @@ -1235,13 +1244,16 @@ Same as
>  .BR PR_SPEC_DISABLE ,
>  but cannot be undone.
>  A subsequent
> -.B
> -prctl(..., PR_SPEC_ENABLE)
> +.BR prctl (\c
> +.IR arg2 ,
> +.BR PR_SPEC_ENABLE )
> +with the same value for
> +.I arg2
>  will fail with the error
>  .BR EPERM .
>  .RE
>  .IP
> -Any other value in
> +Any unsupported value in
>  .IR arg3
>  will result in the call failing with the error
>  .BR ERANGE .

Cheers,

Michael
Dave Martin May 13, 2020, 11:49 a.m. UTC | #2 
On Wed, May 13, 2020 at 01:21:12PM +0200, Michael Kerrisk (man-pages) wrote:
> Hello Dave,
> 
> On 5/12/20 6:36 PM, Dave Martin wrote:
> > Add the PR_SPEC_INDIRECT_BRANCH "misfeature" added in Linux 4.20
> > for PR_SET_SPECULATION_CTRL and PR_GET_SPECULATION_CTRL.
> > 
> > Signed-off-by: Dave Martin <Dave.Martin@arm.com>
> > Cc: Tim Chen <tim.c.chen@linux.intel.com>
> > Cc: Thomas Gleixner <tglx@linutronix.de>
> 
> Thanks. Patch applied, but not yet pushed while I wait to see if any
> Review/Ack arrives.
> 
> Also, could you please check the tweaks I note below.
> 
> > ---
> >  man2/prctl.2 | 24 ++++++++++++++++++------
> >  1 file changed, 18 insertions(+), 6 deletions(-)
> > 
> > diff --git a/man2/prctl.2 b/man2/prctl.2
> > index e8eaf95..66417cf 100644
> > --- a/man2/prctl.2
> > +++ b/man2/prctl.2
> > @@ -1213,11 +1213,20 @@ arguments must be specified as 0; otherwise the call fails with the error
> >  .\" commit 356e4bfff2c5489e016fdb925adbf12a1e3950ee
> >  Sets the state of the speculation misfeature specified in
> >  .IR arg2 .
> > -Currently, the only permitted value for this argument is
> > +Currently, this argument must be one of:
> > +.RS
> > +.TP
> >  .B PR_SPEC_STORE_BYPASS
> > -(otherwise the call fails with the error
> > +speculative store bypass control, or
> 
> s/speculative/enable speculative/
> 
> > +.\" commit 9137bb27e60e554dab694eafa4cca241fa3a694f
> > +.TP
> > +.BR PR_SPEC_INDIRECT_BRANCH " (since Linux 4.20)"
> > +indirect branch speculation control.
> 
> s/indirect/enable indirect/

That doesn't seem quite right.

arg2 just identifies what behaviour to configure.
It's arg3 that says whether to disable / enable it or whatever.


While editing this I did wonder whether the "control" was helpful.
Maybe just dropping that word from these entries would help.

[...]

Cheers
---Dave
Michael Kerrisk (man-pages) May 13, 2020, 12:06 p.m. UTC | #3 
On 5/13/20 1:49 PM, Dave Martin wrote:
> On Wed, May 13, 2020 at 01:21:12PM +0200, Michael Kerrisk (man-pages) wrote:
>> Hello Dave,
>>
>> On 5/12/20 6:36 PM, Dave Martin wrote:
>>> Add the PR_SPEC_INDIRECT_BRANCH "misfeature" added in Linux 4.20
>>> for PR_SET_SPECULATION_CTRL and PR_GET_SPECULATION_CTRL.
>>>
>>> Signed-off-by: Dave Martin <Dave.Martin@arm.com>
>>> Cc: Tim Chen <tim.c.chen@linux.intel.com>
>>> Cc: Thomas Gleixner <tglx@linutronix.de>
>>
>> Thanks. Patch applied, but not yet pushed while I wait to see if any
>> Review/Ack arrives.
>>
>> Also, could you please check the tweaks I note below.
>>
>>> ---
>>>  man2/prctl.2 | 24 ++++++++++++++++++------
>>>  1 file changed, 18 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/man2/prctl.2 b/man2/prctl.2
>>> index e8eaf95..66417cf 100644
>>> --- a/man2/prctl.2
>>> +++ b/man2/prctl.2
>>> @@ -1213,11 +1213,20 @@ arguments must be specified as 0; otherwise the call fails with the error
>>>  .\" commit 356e4bfff2c5489e016fdb925adbf12a1e3950ee
>>>  Sets the state of the speculation misfeature specified in
>>>  .IR arg2 .
>>> -Currently, the only permitted value for this argument is
>>> +Currently, this argument must be one of:
>>> +.RS
>>> +.TP
>>>  .B PR_SPEC_STORE_BYPASS
>>> -(otherwise the call fails with the error
>>> +speculative store bypass control, or
>>
>> s/speculative/enable speculative/
>>
>>> +.\" commit 9137bb27e60e554dab694eafa4cca241fa3a694f
>>> +.TP
>>> +.BR PR_SPEC_INDIRECT_BRANCH " (since Linux 4.20)"
>>> +indirect branch speculation control.
>>
>> s/indirect/enable indirect/
> 
> That doesn't seem quite right.

My goof, not looking at the bigger context of the patch.

> 
> arg2 just identifies what behaviour to configure.
> It's arg3 that says whether to disable / enable it or whatever.
> 
> 
> While editing this I did wonder whether the "control" was helpful.
> Maybe just dropping that word from these entries would help.

Okay I tried to fix things, and made also some other edits.
How does the following look to you?

      PR_SET_SPECULATION_CTRL (since Linux 4.17)
              Sets the state of the speculation misfeature  specified  in
              arg2.   The  speculation-misfeature settings are per-thread
              attributes.

              Currently, arg2 must be one of:

              PR_SPEC_STORE_BYPASS
                     Set the state of the speculative store  bypass  mis‐
                     feature.

              PR_SPEC_INDIRECT_BRANCH (since Linux 4.20)
                     Set  the  state  of  the indirect branch speculation
                     misfeature.

              If arg2 does not have one of the  above  values,  then  the
              call fails with the error ENODEV.

              The  arg3  argument  is  used to hand in the control value,
              which is one of the following:

              ...

Cheers,

Michael
Dave Martin May 13, 2020, 1:53 p.m. UTC | #4 
On Wed, May 13, 2020 at 02:06:38PM +0200, Michael Kerrisk (man-pages) wrote:
> On 5/13/20 1:49 PM, Dave Martin wrote:
> > On Wed, May 13, 2020 at 01:21:12PM +0200, Michael Kerrisk (man-pages) wrote:
> >> Hello Dave,
> >>
> >> On 5/12/20 6:36 PM, Dave Martin wrote:
> >>> Add the PR_SPEC_INDIRECT_BRANCH "misfeature" added in Linux 4.20
> >>> for PR_SET_SPECULATION_CTRL and PR_GET_SPECULATION_CTRL.
> >>>
> >>> Signed-off-by: Dave Martin <Dave.Martin@arm.com>
> >>> Cc: Tim Chen <tim.c.chen@linux.intel.com>
> >>> Cc: Thomas Gleixner <tglx@linutronix.de>
> >>
> >> Thanks. Patch applied, but not yet pushed while I wait to see if any
> >> Review/Ack arrives.
> >>
> >> Also, could you please check the tweaks I note below.
> >>
> >>> ---
> >>>  man2/prctl.2 | 24 ++++++++++++++++++------
> >>>  1 file changed, 18 insertions(+), 6 deletions(-)
> >>>
> >>> diff --git a/man2/prctl.2 b/man2/prctl.2
> >>> index e8eaf95..66417cf 100644
> >>> --- a/man2/prctl.2
> >>> +++ b/man2/prctl.2
> >>> @@ -1213,11 +1213,20 @@ arguments must be specified as 0; otherwise the call fails with the error
> >>>  .\" commit 356e4bfff2c5489e016fdb925adbf12a1e3950ee
> >>>  Sets the state of the speculation misfeature specified in
> >>>  .IR arg2 .
> >>> -Currently, the only permitted value for this argument is
> >>> +Currently, this argument must be one of:
> >>> +.RS
> >>> +.TP
> >>>  .B PR_SPEC_STORE_BYPASS
> >>> -(otherwise the call fails with the error
> >>> +speculative store bypass control, or
> >>
> >> s/speculative/enable speculative/
> >>
> >>> +.\" commit 9137bb27e60e554dab694eafa4cca241fa3a694f
> >>> +.TP
> >>> +.BR PR_SPEC_INDIRECT_BRANCH " (since Linux 4.20)"
> >>> +indirect branch speculation control.
> >>
> >> s/indirect/enable indirect/
> > 
> > That doesn't seem quite right.
> 
> My goof, not looking at the bigger context of the patch.
> 
> > 
> > arg2 just identifies what behaviour to configure.
> > It's arg3 that says whether to disable / enable it or whatever.
> > 
> > 
> > While editing this I did wonder whether the "control" was helpful.
> > Maybe just dropping that word from these entries would help.
> 
> Okay I tried to fix things, and made also some other edits.
> How does the following look to you?
> 
>       PR_SET_SPECULATION_CTRL (since Linux 4.17)
>               Sets the state of the speculation misfeature  specified  in
>               arg2.   The  speculation-misfeature settings are per-thread
>               attributes.
> 
>               Currently, arg2 must be one of:
> 
>               PR_SPEC_STORE_BYPASS
>                      Set the state of the speculative store  bypass  mis‐
>                      feature.
> 
>               PR_SPEC_INDIRECT_BRANCH (since Linux 4.20)
>                      Set  the  state  of  the indirect branch speculation
>                      misfeature.
> 
>               If arg2 does not have one of the  above  values,  then  the
>               call fails with the error ENODEV.
> 
>               The  arg3  argument  is  used to hand in the control value,
>               which is one of the following:
> 
>               ...

Look OK to me.

I thought it sounded odd to "set the state" of a silicon bug, but that
was a bogus concern.

This isn't about bugs, but intentional, often configurable behaviours in
the silicon that happen to have harmful side effects.

It could be more terse to say "configure" instead of "set the state of",
but either way works for me.

Cheers
---Dave
diff mbox series

Patch

diff --git a/man2/prctl.2 b/man2/prctl.2
index e8eaf95..66417cf 100644
--- a/man2/prctl.2
+++ b/man2/prctl.2
@@ -1213,11 +1213,20 @@  arguments must be specified as 0; otherwise the call fails with the error
 .\" commit 356e4bfff2c5489e016fdb925adbf12a1e3950ee
 Sets the state of the speculation misfeature specified in
 .IR arg2 .
-Currently, the only permitted value for this argument is
+Currently, this argument must be one of:
+.RS
+.TP
 .B PR_SPEC_STORE_BYPASS
-(otherwise the call fails with the error
+speculative store bypass control, or
+.\" commit 9137bb27e60e554dab694eafa4cca241fa3a694f
+.TP
+.BR PR_SPEC_INDIRECT_BRANCH " (since Linux 4.20)"
+indirect branch speculation control.
+.RE
+.IP
+(Otherwise the call fails with the error
 .BR ENODEV ).
-This setting is a per-thread attribute.
+These settings are per-thread attributes.
 The
 .IR arg3
 argument is used to hand in the control value,
@@ -1235,13 +1244,16 @@  Same as
 .BR PR_SPEC_DISABLE ,
 but cannot be undone.
 A subsequent
-.B
-prctl(..., PR_SPEC_ENABLE)
+.BR prctl (\c
+.IR arg2 ,
+.BR PR_SPEC_ENABLE )
+with the same value for
+.I arg2
 will fail with the error
 .BR EPERM .
 .RE
 .IP
-Any other value in
+Any unsupported value in
 .IR arg3
 will result in the call failing with the error
 .BR ERANGE .