Re: [PATCH 10/14] prctl.2: Add PR_SPEC_INDIRECT_BRANCH for SPECULATION_CTRL prctls - Michael Kerrisk (man-pages)

From: "Michael Kerrisk (man-pages)" <mtk.manpages@gmail.com>
To: Dave Martin <Dave.Martin@arm.com>
Cc: linux-arch@vger.kernel.org, linux-man@vger.kernel.org,
	mtk.manpages@gmail.com, Thomas Gleixner <tglx@linutronix.de>,
	Tim Chen <tim.c.chen@linux.intel.com>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 10/14] prctl.2: Add PR_SPEC_INDIRECT_BRANCH for SPECULATION_CTRL prctls
Date: Wed, 13 May 2020 14:06:38 +0200	[thread overview]
Message-ID: <604879eb-1c7e-d08b-a6b8-165e4259b60c@gmail.com> (raw)
In-Reply-To: <20200513114915.GL21779@arm.com>

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

-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel