2020-05-06 22:23:44

by Dave Hansen

[permalink] [raw]
Subject: Re: [PATCH 1/2] arch/x86: Rename config X86_INTEL_MEMORY_PROTECTION_KEYS to generic x86

> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 1197b5596d5a..8630b9fa06f5 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -1886,11 +1886,11 @@ config X86_UMIP
> specific cases in protected and virtual-8086 modes. Emulated
> results are dummy.
>
> -config X86_INTEL_MEMORY_PROTECTION_KEYS
> - prompt "Intel Memory Protection Keys"
> +config X86_MEMORY_PROTECTION_KEYS
> + prompt "Memory Protection Keys"
> def_bool y
> # Note: only available in 64-bit mode
> - depends on CPU_SUP_INTEL && X86_64
> + depends on X86_64 && (CPU_SUP_INTEL || CPU_SUP_AMD)
> select ARCH_USES_HIGH_VMA_FLAGS
> select ARCH_HAS_PKEYS
> ---help---

It's a bit of a bummer that we're going to prompt everybody doing
oldconfig's for this new option. But, I don't know any way for Kconfig
to suppress it if the name is changed. Also, I guess the def_bool=y
means that menuconfig and olddefconfig will tend to do the right thing.

Do we *really* need to change the Kconfig name? The text prompt, sure.
End users see that and having Intel in there is massively confusing.

If I have to put up with seeing 'amd64' all over my Debian package
names, you can put up with a Kconfig name. :P

I'm really just wondering what the point of the churn is.


2020-05-06 22:30:50

by Dave Hansen

[permalink] [raw]
Subject: Re: [PATCH 1/2] arch/x86: Rename config X86_INTEL_MEMORY_PROTECTION_KEYS to generic x86

On 5/6/20 3:21 PM, Dave Hansen wrote:
> I'm really just wondering what the point of the churn is.

The config option is also in the manpages, fwiw:

http://man7.org/linux/man-pages/man7/pkeys.7.html

By the way, I am regretting ever sticking "INTEL_" in there. Seems like
a good best practice would be to leave those things out in the future if
there's any credible opportunity for another vendor to add support.

2020-05-06 22:38:04

by Logan Gunthorpe

[permalink] [raw]
Subject: Re: [PATCH 1/2] arch/x86: Rename config X86_INTEL_MEMORY_PROTECTION_KEYS to generic x86



On 2020-05-06 4:21 p.m., Dave Hansen wrote:
>> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
>> index 1197b5596d5a..8630b9fa06f5 100644
>> --- a/arch/x86/Kconfig
>> +++ b/arch/x86/Kconfig
>> @@ -1886,11 +1886,11 @@ config X86_UMIP
>> specific cases in protected and virtual-8086 modes. Emulated
>> results are dummy.
>>
>> -config X86_INTEL_MEMORY_PROTECTION_KEYS
>> - prompt "Intel Memory Protection Keys"
>> +config X86_MEMORY_PROTECTION_KEYS
>> + prompt "Memory Protection Keys"
>> def_bool y
>> # Note: only available in 64-bit mode
>> - depends on CPU_SUP_INTEL && X86_64
>> + depends on X86_64 && (CPU_SUP_INTEL || CPU_SUP_AMD)
>> select ARCH_USES_HIGH_VMA_FLAGS
>> select ARCH_HAS_PKEYS
>> ---help---
>
> It's a bit of a bummer that we're going to prompt everybody doing
> oldconfig's for this new option. But, I don't know any way for Kconfig
> to suppress it if the name is changed. Also, I guess the def_bool=y
> means that menuconfig and olddefconfig will tend to do the right thing.
>
> Do we *really* need to change the Kconfig name? The text prompt, sure.
> End users see that and having Intel in there is massively confusing.
>
> If I have to put up with seeing 'amd64' all over my Debian package
> names, you can put up with a Kconfig name. :P

Lol, isn't that just Intel's penance for Itanium?

Logan

Subject: Re: [PATCH 1/2] arch/x86: Rename config X86_INTEL_MEMORY_PROTECTION_KEYS to generic x86

On 2020-05-06 15:21:29 [-0700], Dave Hansen wrote:
> > diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> > index 1197b5596d5a..8630b9fa06f5 100644
> > --- a/arch/x86/Kconfig
> > +++ b/arch/x86/Kconfig
> > @@ -1886,11 +1886,11 @@ config X86_UMIP
> > specific cases in protected and virtual-8086 modes. Emulated
> > results are dummy.
> >
> > -config X86_INTEL_MEMORY_PROTECTION_KEYS
> > - prompt "Intel Memory Protection Keys"
> > +config X86_MEMORY_PROTECTION_KEYS
> > + prompt "Memory Protection Keys"
> > def_bool y
> > # Note: only available in 64-bit mode
> > - depends on CPU_SUP_INTEL && X86_64
> > + depends on X86_64 && (CPU_SUP_INTEL || CPU_SUP_AMD)
> > select ARCH_USES_HIGH_VMA_FLAGS
> > select ARCH_HAS_PKEYS
> > ---help---
>
> It's a bit of a bummer that we're going to prompt everybody doing
> oldconfig's for this new option. But, I don't know any way for Kconfig
> to suppress it if the name is changed. Also, I guess the def_bool=y
> means that menuconfig and olddefconfig will tend to do the right thing.

You could add a new option (X86_MEMORY_PROTECTION_KEYS) which is
def_bool X86_INTEL_MEMORY_PROTECTION_KEYS and avoiding the prompt line.
Soo it is selected based on the old option and the user isn't bother. A
few cycles later you could remove intel option and add prompt to other.
But still little work for…

> Do we *really* need to change the Kconfig name? The text prompt, sure.
> End users see that and having Intel in there is massively confusing.
>
> If I have to put up with seeing 'amd64' all over my Debian package
> names, you can put up with a Kconfig name. :P

:) Right. On AMD you also use the crc32c-intel (if possible) and I
haven't seen people complain about this one.

> I'm really just wondering what the point of the churn is.

Sebastian

2020-05-07 14:49:23

by Dave Hansen

[permalink] [raw]
Subject: Re: [PATCH 1/2] arch/x86: Rename config X86_INTEL_MEMORY_PROTECTION_KEYS to generic x86

On 5/7/20 12:29 AM, Sebastian Andrzej Siewior wrote:
>>> -config X86_INTEL_MEMORY_PROTECTION_KEYS
>>> - prompt "Intel Memory Protection Keys"
>>> +config X86_MEMORY_PROTECTION_KEYS
>>> + prompt "Memory Protection Keys"
>>> def_bool y
>>> # Note: only available in 64-bit mode
>>> - depends on CPU_SUP_INTEL && X86_64
>>> + depends on X86_64 && (CPU_SUP_INTEL || CPU_SUP_AMD)
>>> select ARCH_USES_HIGH_VMA_FLAGS
>>> select ARCH_HAS_PKEYS
>>> ---help---
>> It's a bit of a bummer that we're going to prompt everybody doing
>> oldconfig's for this new option. But, I don't know any way for Kconfig
>> to suppress it if the name is changed. Also, I guess the def_bool=y
>> means that menuconfig and olddefconfig will tend to do the right thing.
> You could add a new option (X86_MEMORY_PROTECTION_KEYS) which is
> def_bool X86_INTEL_MEMORY_PROTECTION_KEYS and avoiding the prompt line.
> Soo it is selected based on the old option and the user isn't bother. A
> few cycles later you could remove intel option and add prompt to other.
> But still little work for…

That does sound viable, if we decide it's all worth it.

So, for now my preference would be to change the prompt, but leave the
CONFIG_ naming in place. If we decide that transitioning the config is
the right thing (I don't feel super strongly either way), let's use
Sebastian's trick to avoid the Kconfig prompts.

2020-05-07 15:21:17

by Paolo Bonzini

[permalink] [raw]
Subject: Re: [PATCH 1/2] arch/x86: Rename config X86_INTEL_MEMORY_PROTECTION_KEYS to generic x86

On 07/05/20 16:44, Dave Hansen wrote:
>> You could add a new option (X86_MEMORY_PROTECTION_KEYS) which is
>> def_bool X86_INTEL_MEMORY_PROTECTION_KEYS and avoiding the prompt line.
>> Soo it is selected based on the old option and the user isn't bother. A
>> few cycles later you could remove intel option and add prompt to other.
>> But still little work for…
> That does sound viable, if we decide it's all worth it.
>
> So, for now my preference would be to change the prompt, but leave the
> CONFIG_ naming in place.

I agree.

What's in a name? An Intel rose by any other name would smell as sweet.
Oh wait... :)

Paolo

> If we decide that transitioning the config is
> the right thing (I don't feel super strongly either way), let's use
> Sebastian's trick to avoid the Kconfig prompts.
>

2020-05-07 16:08:24

by Moger, Babu

[permalink] [raw]
Subject: Re: [PATCH 1/2] arch/x86: Rename config X86_INTEL_MEMORY_PROTECTION_KEYS to generic x86



On 5/7/20 10:16 AM, Paolo Bonzini wrote:
> On 07/05/20 16:44, Dave Hansen wrote:
>>> You could add a new option (X86_MEMORY_PROTECTION_KEYS) which is
>>> def_bool X86_INTEL_MEMORY_PROTECTION_KEYS and avoiding the prompt line.
>>> Soo it is selected based on the old option and the user isn't bother. A
>>> few cycles later you could remove intel option and add prompt to other.
>>> But still little work for…
>> That does sound viable, if we decide it's all worth it.
>>
>> So, for now my preference would be to change the prompt, but leave the
>> CONFIG_ naming in place.
>
> I agree.
>
> What's in a name? An Intel rose by any other name would smell as sweet.

How about X86_MPK? Or I will use already proposed name
X86_MEMORY_PROTECTION_KEYS.

> Oh wait... :)
>
> Paolo
>
>> If we decide that transitioning the config is
>> the right thing (I don't feel super strongly either way), let's use
>> Sebastian's trick to avoid the Kconfig prompts.
>>
>

2020-05-07 16:12:08

by Paolo Bonzini

[permalink] [raw]
Subject: Re: [PATCH 1/2] arch/x86: Rename config X86_INTEL_MEMORY_PROTECTION_KEYS to generic x86

On 07/05/20 18:06, Babu Moger wrote:
>>> So, for now my preference would be to change the prompt, but leave the
>>> CONFIG_ naming in place.
>> I agree.
>>
>> What's in a name? An Intel rose by any other name would smell as sweet.
>
> How about X86_MPK? Or I will use already proposed name
> X86_MEMORY_PROTECTION_KEYS.

Dave is proposing to keep the CONFIG_ as is and only change the prompt.

Paolo

2020-05-07 16:14:34

by Moger, Babu

[permalink] [raw]
Subject: Re: [PATCH 1/2] arch/x86: Rename config X86_INTEL_MEMORY_PROTECTION_KEYS to generic x86



On 5/7/20 11:07 AM, Paolo Bonzini wrote:
> On 07/05/20 18:06, Babu Moger wrote:
>>>> So, for now my preference would be to change the prompt, but leave the
>>>> CONFIG_ naming in place.
>>> I agree.
>>>
>>> What's in a name? An Intel rose by any other name would smell as sweet.
>>
>> How about X86_MPK? Or I will use already proposed name
>> X86_MEMORY_PROTECTION_KEYS.
>
> Dave is proposing to keep the CONFIG_ as is and only change the prompt.

Ok. Got it. thanks