[4/4] x86/speculation: Add PSF mitigation kernel parameters
diff mbox series

Message ID 20210421090117.22315-5-rsaripal@amd.com
State New
Headers show
Series
  • Introduce support for PSF mitigation
Related show

Commit Message

Ramakrishna Saripalli April 21, 2021, 9:01 a.m. UTC
From: Ramakrishna Saripalli <rk.saripalli@amd.com>

PSF mitigation introduces a new kernel parameter called
	predict_store_fwd.

Signed-off-by: Ramakrishna Saripalli<rk.saripalli@amd.com>
---
 Documentation/admin-guide/kernel-parameters.txt | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Randy Dunlap April 21, 2021, 4:04 p.m. UTC | #1
Hi,

On 4/21/21 2:01 AM, Ramakrishna Saripalli wrote:
> From: Ramakrishna Saripalli <rk.saripalli@amd.com>
> 
> PSF mitigation introduces a new kernel parameter called
> 	predict_store_fwd.
> 
> Signed-off-by: Ramakrishna Saripalli<rk.saripalli@amd.com>
> ---
>  Documentation/admin-guide/kernel-parameters.txt | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 04545725f187..58f6bd02385b 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -3940,6 +3940,11 @@
>  			Format: {"off"}
>  			Disable Hardware Transactional Memory
>  
> +	predict_store_fwd	[X86] This option controls PSF mitigation
> +			off - Turns on PSF mitigation.
> +			on  - Turns off PSF mitigation.
> +			default : on.

This should be formatted more like:

+	predict_store_fwd=	[X86] This option controls PSF mitigation
+			off - Turns on PSF mitigation.
+			on  - Turns off PSF mitigation.
+			default: on.

But why does "off" turn it on and "on" turn it off?


> +
>  	preempt=	[KNL]
>  			Select preemption mode if you have CONFIG_PREEMPT_DYNAMIC
>  			none - Limited to cond_resched() calls
> 

thanks.
Bandan Das April 21, 2021, 6:32 p.m. UTC | #2
Randy Dunlap <rdunlap@infradead.org> writes:

> Hi,
>
> On 4/21/21 2:01 AM, Ramakrishna Saripalli wrote:
>> From: Ramakrishna Saripalli <rk.saripalli@amd.com>
>> 
>> PSF mitigation introduces a new kernel parameter called
>> 	predict_store_fwd.
>> 
>> Signed-off-by: Ramakrishna Saripalli<rk.saripalli@amd.com>
>> ---
>>  Documentation/admin-guide/kernel-parameters.txt | 5 +++++
>>  1 file changed, 5 insertions(+)
>> 
>> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
>> index 04545725f187..58f6bd02385b 100644
>> --- a/Documentation/admin-guide/kernel-parameters.txt
>> +++ b/Documentation/admin-guide/kernel-parameters.txt
>> @@ -3940,6 +3940,11 @@
>>  			Format: {"off"}
>>  			Disable Hardware Transactional Memory
>>  
>> +	predict_store_fwd	[X86] This option controls PSF mitigation
>> +			off - Turns on PSF mitigation.
>> +			on  - Turns off PSF mitigation.
>> +			default : on.
>
> This should be formatted more like:
>
> +	predict_store_fwd=	[X86] This option controls PSF mitigation
> +			off - Turns on PSF mitigation.
> +			on  - Turns off PSF mitigation.
> +			default: on.
>
> But why does "off" turn it on and "on" turn it off?
>
Maybe, rename the parameter to something like psfd_disable, then off -> disables mitigation and on -> enables it.
Or just rewriting this to off -> turns off predictive store forwarding is probably ok too.

Bandan

>
>> +
>>  	preempt=	[KNL]
>>  			Select preemption mode if you have CONFIG_PREEMPT_DYNAMIC
>>  			none - Limited to cond_resched() calls
>> 
>
> thanks.
Ramakrishna Saripalli April 21, 2021, 6:36 p.m. UTC | #3
Bandan / Randy, this convention was chosen based on review of the earlier patches from Boris.

For now, the functionality will be just on and off.
Later based on interest from community and other factors, I will online the prctl and seccomp variants of the 
mitigation.

Thanks,
RK

On 4/21/2021 1:32 PM, Bandan Das wrote:
> Randy Dunlap <rdunlap@infradead.org> writes:
> 
>> Hi,
>>
>> On 4/21/21 2:01 AM, Ramakrishna Saripalli wrote:
>>> From: Ramakrishna Saripalli <rk.saripalli@amd.com>
>>>
>>> PSF mitigation introduces a new kernel parameter called
>>> 	predict_store_fwd.
>>>
>>> Signed-off-by: Ramakrishna Saripalli<rk.saripalli@amd.com>
>>> ---
>>>  Documentation/admin-guide/kernel-parameters.txt | 5 +++++
>>>  1 file changed, 5 insertions(+)
>>>
>>> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
>>> index 04545725f187..58f6bd02385b 100644
>>> --- a/Documentation/admin-guide/kernel-parameters.txt
>>> +++ b/Documentation/admin-guide/kernel-parameters.txt
>>> @@ -3940,6 +3940,11 @@
>>>  			Format: {"off"}
>>>  			Disable Hardware Transactional Memory
>>>  
>>> +	predict_store_fwd	[X86] This option controls PSF mitigation
>>> +			off - Turns on PSF mitigation.
>>> +			on  - Turns off PSF mitigation.
>>> +			default : on.
>>
>> This should be formatted more like:
>>
>> +	predict_store_fwd=	[X86] This option controls PSF mitigation
>> +			off - Turns on PSF mitigation.
>> +			on  - Turns off PSF mitigation.
>> +			default: on.
>>
>> But why does "off" turn it on and "on" turn it off?
>>
> Maybe, rename the parameter to something like psfd_disable, then off -> disables mitigation and on -> enables it.
> Or just rewriting this to off -> turns off predictive store forwarding is probably ok too.
> 
> Bandan
> 
>>
>>> +
>>>  	preempt=	[KNL]
>>>  			Select preemption mode if you have CONFIG_PREEMPT_DYNAMIC
>>>  			none - Limited to cond_resched() calls
>>>
>>
>> thanks.
>
Borislav Petkov April 21, 2021, 6:48 p.m. UTC | #4
On Wed, Apr 21, 2021 at 02:32:13PM -0400, Bandan Das wrote:
> Maybe, rename the parameter to something like psfd_disable, then off
> -> disables mitigation and on -> enables it. Or just rewriting this to
> off -> turns off predictive store forwarding is probably ok too.

Yes:

	off - Turns off predictive store forwarding.
	on  - Turns on...

Ramakrishna, you don't have to call this a mitigation - this is a flag
which controls the feature.

Also, those 4 patches can be merged into a single one which simply adds
the feature along with the boot-time controls.

Thx.
Ramakrishna Saripalli April 21, 2021, 6:55 p.m. UTC | #5
On 4/21/2021 1:48 PM, Borislav Petkov wrote:
> On Wed, Apr 21, 2021 at 02:32:13PM -0400, Bandan Das wrote:
>> Maybe, rename the parameter to something like psfd_disable, then off
>> -> disables mitigation and on -> enables it. Or just rewriting this to
>> off -> turns off predictive store forwarding is probably ok too.
> 
> Yes:
> 
> 	off - Turns off predictive store forwarding.
> 	on  - Turns on...
> 
> Ramakrishna, you don't have to call this a mitigation - this is a flag
> which controls the feature.

Agreed. I will fix it.
> 
> Also, those 4 patches can be merged into a single one which simply adds
> the feature along with the boot-time controls.

I separated them into separate patches because the KVM patch depends on one of the patch.
The corresponding QEMU patch depends on another patch.

By separating them into 4 separate patches, my thinking was I could keep them logically separate.
Yes, I can combine all 4 patches into one patch but would like to get feedback before I do so.
> 
> Thx.
>
Borislav Petkov April 21, 2021, 6:57 p.m. UTC | #6
On Wed, Apr 21, 2021 at 01:55:03PM -0500, Saripalli, RK wrote:
> I separated them into separate patches because the KVM patch depends on one of the patch.
> The corresponding QEMU patch depends on another patch.
> 
> By separating them into 4 separate patches, my thinking was I could keep them logically separate.

Sure but they all go together through the same tree - why does that
matter for Qemu/KVM?
Ramakrishna Saripalli April 21, 2021, 7:05 p.m. UTC | #7
On 4/21/2021 1:57 PM, Borislav Petkov wrote:
> On Wed, Apr 21, 2021 at 01:55:03PM -0500, Saripalli, RK wrote:
>> I separated them into separate patches because the KVM patch depends on one of the patch.
>> The corresponding QEMU patch depends on another patch.
>>
>> By separating them into 4 separate patches, my thinking was I could keep them logically separate.
> 
> Sure but they all go together through the same tree - why does that
> matter for Qemu/KVM?

Ok. I will combine all the 4 patches into one patch and resend that one patch.

Thanks,
RK
>

Patch
diff mbox series

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 04545725f187..58f6bd02385b 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -3940,6 +3940,11 @@ 
 			Format: {"off"}
 			Disable Hardware Transactional Memory
 
+	predict_store_fwd	[X86] This option controls PSF mitigation
+			off - Turns on PSF mitigation.
+			on  - Turns off PSF mitigation.
+			default : on.
+
 	preempt=	[KNL]
 			Select preemption mode if you have CONFIG_PREEMPT_DYNAMIC
 			none - Limited to cond_resched() calls