diff mbox

[v2,13/30] xen/x86: Clear dependent features when clearing a cpu cap

Message ID 1454679743-18133-14-git-send-email-andrew.cooper3@citrix.com (mailing list archive)
State New, archived
Headers show

Commit Message

Andrew Cooper Feb. 5, 2016, 1:42 p.m. UTC
When clearing a cpu cap, clear all dependent features.  This avoids having a
featureset with intermediate features disabled, but leaf features enabled.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
---
 xen/arch/x86/cpu/common.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

Comments

Jan Beulich Feb. 15, 2016, 2:53 p.m. UTC | #1
>>> On 05.02.16 at 14:42, <andrew.cooper3@citrix.com> wrote:
> --- a/xen/arch/x86/cpu/common.c
> +++ b/xen/arch/x86/cpu/common.c
> @@ -53,8 +53,24 @@ static unsigned int cleared_caps[NCAPINTS];
>  
>  void __init setup_clear_cpu_cap(unsigned int cap)
>  {
> +	const uint32_t *dfs;
> +	unsigned int i;
> +
> +	if ( test_bit(cap, cleared_caps) )
> +		return;
> +
>  	__clear_bit(cap, boot_cpu_data.x86_capability);
>  	__set_bit(cap, cleared_caps);

Perhaps __test_and_set_bit() above?

> +	dfs = lookup_deep_deps(cap);

Ah, this is a use I can see as valid outside of cpuid.c, but it's still
(as I had expected) one from an __init function.

Jan
Jan Beulich Feb. 15, 2016, 2:56 p.m. UTC | #2
>>> On 05.02.16 at 14:42, <andrew.cooper3@citrix.com> wrote:
> --- a/xen/arch/x86/cpu/common.c
> +++ b/xen/arch/x86/cpu/common.c
> @@ -53,8 +53,24 @@ static unsigned int cleared_caps[NCAPINTS];
>  
>  void __init setup_clear_cpu_cap(unsigned int cap)
>  {
> +	const uint32_t *dfs;
> +	unsigned int i;
> +
> +	if ( test_bit(cap, cleared_caps) )
> +		return;
> +
>  	__clear_bit(cap, boot_cpu_data.x86_capability);
>  	__set_bit(cap, cleared_caps);
> +
> +	dfs = lookup_deep_deps(cap);
> +
> +	if ( !dfs )
> +		return;
> +
> +	for ( i = 0; i < FSCAPINTS; ++i ) {
> +		cleared_caps[i] |= dfs[i];
> +		boot_cpu_data.x86_capability[i] &= ~dfs[i];
> +	}
>  }

Oh, also - you're mixing styles here (note the stray blanks inside
for() and if()) ...

Jan
Andrew Cooper Feb. 15, 2016, 3:33 p.m. UTC | #3
On 15/02/16 14:53, Jan Beulich wrote:
>>>> On 05.02.16 at 14:42, <andrew.cooper3@citrix.com> wrote:
>> --- a/xen/arch/x86/cpu/common.c
>> +++ b/xen/arch/x86/cpu/common.c
>> @@ -53,8 +53,24 @@ static unsigned int cleared_caps[NCAPINTS];
>>  
>>  void __init setup_clear_cpu_cap(unsigned int cap)
>>  {
>> +	const uint32_t *dfs;
>> +	unsigned int i;
>> +
>> +	if ( test_bit(cap, cleared_caps) )
>> +		return;
>> +
>>  	__clear_bit(cap, boot_cpu_data.x86_capability);
>>  	__set_bit(cap, cleared_caps);
> Perhaps __test_and_set_bit() above?

Hmm yes - that won't make it atomic.

And I will fix up the style issues.

~Andrew
diff mbox

Patch

diff --git a/xen/arch/x86/cpu/common.c b/xen/arch/x86/cpu/common.c
index 39c340b..e205565 100644
--- a/xen/arch/x86/cpu/common.c
+++ b/xen/arch/x86/cpu/common.c
@@ -53,8 +53,24 @@  static unsigned int cleared_caps[NCAPINTS];
 
 void __init setup_clear_cpu_cap(unsigned int cap)
 {
+	const uint32_t *dfs;
+	unsigned int i;
+
+	if ( test_bit(cap, cleared_caps) )
+		return;
+
 	__clear_bit(cap, boot_cpu_data.x86_capability);
 	__set_bit(cap, cleared_caps);
+
+	dfs = lookup_deep_deps(cap);
+
+	if ( !dfs )
+		return;
+
+	for ( i = 0; i < FSCAPINTS; ++i ) {
+		cleared_caps[i] |= dfs[i];
+		boot_cpu_data.x86_capability[i] &= ~dfs[i];
+	}
 }
 
 static void default_init(struct cpuinfo_x86 * c)