Re: [PATCH 2/2] i386/cpu: Mask off unsupported XSAVE components

[Eduardo Habkost]

On Thu, Jul 16, 2020 at 04:20:19PM +0800, Xiaoyao Li wrote:
> When setting up XSAVE components, it needs to mask off those unsupported
> by KVM.
> 
> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>

We must never disable CPUID features silently based on host
capabilities, otherwise we can't guarantee guest ABI stability
when migrating to another host.  Filtering of features should
involve a call to mark_unavailable_features() (or some equivalent
mechanism) so we can report the missing features properly through
QMP.

Could you explain what's the bug you are trying to fix?  The loop
at x86_cpu_filter_features() is already supposed to disable
features unsupported by the host.

> ---
>  target/i386/cpu.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index f5f11603e805..efc92334b7b1 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -6274,8 +6274,10 @@ static void x86_cpu_enable_xsave_components(X86CPU 
> *cpu)
>          }
>      }
>  
> -    env->features[FEAT_XSAVE_COMP_LO] = mask;
> -    env->features[FEAT_XSAVE_COMP_HI] = mask >> 32;
> +    env->features[FEAT_XSAVE_COMP_LO] = mask &
> +        x86_cpu_get_supported_feature_word(FEAT_XSAVE_COMP_LO, 
> cpu->migratable);
> +    env->features[FEAT_XSAVE_COMP_HI] = (mask >> 32) &
> +        x86_cpu_get_supported_feature_word(FEAT_XSAVE_COMP_HI, 
> cpu->migratable);
>  }
>  
>  /***** Steps involved on loading and filtering CPUID data
> -- 
> 2.18.4
> 

-- 
Eduardo