[8/9] x86/cpuid: Introduce missing feature adjustment in calculate_pv_def_policy()
diff mbox series

Message ID 20200615141532.1927-9-andrew.cooper3@citrix.com
State New
Headers show
Series
  • XSA-320 follow for IvyBridge
Related show

Commit Message

Andrew Cooper June 15, 2020, 2:15 p.m. UTC
This was an accidental asymmetry with the HVM side.

No change in behaviour at this point.

Fixes: 83b387382 ("x86/cpuid: Introduce and use default CPUID policies")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Wei Liu <wl@xen.org>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Paul Durrant <paul@xen.org>
---
 xen/arch/x86/cpuid.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Jan Beulich June 16, 2020, 9:40 a.m. UTC | #1
On 15.06.2020 16:15, Andrew Cooper wrote:
> This was an accidental asymmetry with the HVM side.
> 
> No change in behaviour at this point.
> 
> Fixes: 83b387382 ("x86/cpuid: Introduce and use default CPUID policies")
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>
with a remark:

> --- a/xen/arch/x86/cpuid.c
> +++ b/xen/arch/x86/cpuid.c
> @@ -402,6 +402,8 @@ static void __init calculate_pv_def_policy(void)
>      for ( i = 0; i < ARRAY_SIZE(pv_featureset); ++i )
>          pv_featureset[i] &= pv_def_featuremask[i];
>  
> +    guest_common_feature_adjustments(pv_featureset);
> +
>      sanitise_featureset(pv_featureset);
>      cpuid_featureset_to_policy(pv_featureset, p);
>      recalculate_xstate(p);

These four calls are common to all three callers of the function.
Perhaps them going out of sync would be less likely if all four
called the same helper to carry out these four steps?

Jan
Andrew Cooper June 16, 2020, 4:17 p.m. UTC | #2
On 16/06/2020 10:40, Jan Beulich wrote:
> On 15.06.2020 16:15, Andrew Cooper wrote:
>> This was an accidental asymmetry with the HVM side.
>>
>> No change in behaviour at this point.
>>
>> Fixes: 83b387382 ("x86/cpuid: Introduce and use default CPUID policies")
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Reviewed-by: Jan Beulich <jbeulich@suse.com>

Thanks.

> with a remark:
>
>> --- a/xen/arch/x86/cpuid.c
>> +++ b/xen/arch/x86/cpuid.c
>> @@ -402,6 +402,8 @@ static void __init calculate_pv_def_policy(void)
>>      for ( i = 0; i < ARRAY_SIZE(pv_featureset); ++i )
>>          pv_featureset[i] &= pv_def_featuremask[i];
>>  
>> +    guest_common_feature_adjustments(pv_featureset);
>> +
>>      sanitise_featureset(pv_featureset);
>>      cpuid_featureset_to_policy(pv_featureset, p);
>>      recalculate_xstate(p);
> These four calls are common to all three callers of the function.
> Perhaps them going out of sync would be less likely if all four
> called the same helper to carry out these four steps?

I'm not sure how many of them are going to survive the transformation to
a fully libx86 based world.

I expect it not to look exactly like this.

~Andrew

Patch
diff mbox series

diff --git a/xen/arch/x86/cpuid.c b/xen/arch/x86/cpuid.c
index ee11087626..f2fc0aa895 100644
--- a/xen/arch/x86/cpuid.c
+++ b/xen/arch/x86/cpuid.c
@@ -402,6 +402,8 @@  static void __init calculate_pv_def_policy(void)
     for ( i = 0; i < ARRAY_SIZE(pv_featureset); ++i )
         pv_featureset[i] &= pv_def_featuremask[i];
 
+    guest_common_feature_adjustments(pv_featureset);
+
     sanitise_featureset(pv_featureset);
     cpuid_featureset_to_policy(pv_featureset, p);
     recalculate_xstate(p);