Re: [RFC PATCH v4 2/3] target/i386: Restrict system-specific features from user emulation

[Paolo Bonzini <pbonzini@redhat.com>]

On 9/11/23 23:13, Philippe Mathieu-Daudé wrote:
>   /*
>    * Only for builtin_x86_defs models initialized with x86_register_cpudef_types.
>    */
> @@ -6163,6 +6195,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
>               }
>               *edx = env->features[FEAT_7_0_EDX]; /* Feature flags */
>   
> +#ifndef CONFIG_USER_ONLY
>               /*
>                * SGX cannot be emulated in software.  If hardware does not
>                * support enabling SGX and/or SGX flexible launch control,
> @@ -6181,6 +6214,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
>                       CPUID_7_0_ECX_SGX_LC))) {
>                   *ecx &= ~CPUID_7_0_ECX_SGX_LC;
>               }
> +#endif

This can use a variant of x86_cpu_get_supported_cpuid that returns a 
single register; or it can be rewritten to use x86_cpu_get_supported_cpuid.

In general, a lot of checks for accel_uses_host_cpuid() are unnecessary, 
and the code can be modified to not depend on either KVM or HVF.

>           } else if (count == 1) {
>               *eax = env->features[FEAT_7_1_EAX];
>               *edx = env->features[FEAT_7_1_EDX];
> @@ -6876,6 +6910,8 @@ static void mce_init(X86CPU *cpu)
>       }
>   }
>   
> +#ifndef CONFIG_USER_ONLY
> +
>   static void x86_cpu_adjust_level(X86CPU *cpu, uint32_t *min, uint32_t value)
>   {
>       if (*min < value) {
> @@ -6948,6 +6984,8 @@ static void x86_cpu_enable_xsave_components(X86CPU *cpu)
>       env->features[FEAT_XSAVE_XSS_HI] = mask >> 32;
>   }
>   
> +#endif /* !CONFIG_USER_ONLY */

These functions should all be used in user-mode emulation as well.

>   /***** Steps involved on loading and filtering CPUID data
>    *
>    * When initializing and realizing a CPU object, the steps
> @@ -7040,6 +7078,7 @@ void x86_cpu_expand_features(X86CPU *cpu, Error **errp)
>           }
>       }
>   
> +#ifndef CONFIG_USER_ONLY
>       if (!kvm_enabled() || !cpu->expose_kvm) {
>           env->features[FEAT_KVM] = 0;
>       }

This is "!kvm_enabled()" so it should be kept for user-mode emulation.

> @@ -7111,6 +7150,8 @@ void x86_cpu_expand_features(X86CPU *cpu, Error **errp)
>           return;
>       }
>   
> +#endif /* !CONFIG_USER_ONLY */
>       /* Set cpuid_*level* based on cpuid_min_*level, if not explicitly set */
>       if (env->cpuid_level_func7 == UINT32_MAX) {
>           env->cpuid_level_func7 = env->cpuid_min_level_func7;
> @@ -7152,6 +7193,7 @@ static void x86_cpu_filter_features(X86CPU *cpu, bool verbose)
>           mark_unavailable_features(cpu, w, unavailable_features, prefix);
>       }
>   
> +#ifndef CONFIG_USER_ONLY
>       if ((env->features[FEAT_7_0_EBX] & CPUID_7_0_EBX_INTEL_PT) &&
>           kvm_enabled()) {
>           KVMState *s = CPU(cpu)->kvm_state;
> @@ -7179,6 +7221,7 @@ static void x86_cpu_filter_features(X86CPU *cpu, bool verbose)
>               mark_unavailable_features(cpu, FEAT_7_0_EBX, CPUID_7_0_EBX_INTEL_PT, prefix);
>           }
>       }
> +#endif

This need not be limited to KVM, it can likewise use 
x86_cpu_get_supported_cpuid.

Paolo

>   }
>   
>   static void x86_cpu_hyperv_realize(X86CPU *cpu)