On Mon, Oct 28, 2024 at 11:04:07PM +0800, Xiaoyao Li wrote:
> On 10/28/2024 10:45 AM, Tao Su wrote:
> > + case 0x24: {
> > + *eax = 0;
> > + *ebx = 0;
> > + *ecx = 0;
> > + *edx = 0;
> > + if (!(env->features[FEAT_7_1_EDX] & CPUID_7_1_EDX_AVX10)) {
> > + break;
> > + }
> > +
> > + if (count == 0) {
> > + uint8_t v = kvm_arch_get_supported_cpuid(cs->kvm_state, 0x24,
> > + 0, R_EBX);
> > + if (env->avx10_version && env->avx10_version < v) {
> > + v = env->avx10_version;
> > + }
>
> Here, if user specified avx10_version is >= kvm reported value, it uses
> KVM's reported value silently.
>
> I think it's not good. It'd better to validate if user specified value can
> be satisfied or not, and emit a warning when not. e.g., in
> x86_cpu_filter_features() or in kvm_cpu_realizefn(). Also we can put the
> behavior along with it that "use KVM reported maximum value when
> avx10_version is 0 "
>
> then, here we can simply do
>
> *ebx = env->features[FEAT_24_0_EBX] | env->avx10_version;
Is it necessary to add enforce_cpuid for avx10_version too?
How about checking this in x86_cpu_realizefn, because I see this may be a
more suitable place to implement check_cpuid and enforce_cpuid.
@@ -7816,6 +7810,29 @@ static void x86_cpu_realizefn(DeviceState *dev, Error
**errp)
x86_cpu_filter_features(cpu, cpu->check_cpuid || cpu->enforce_cpuid);
+ if (env->features[FEAT_7_1_EDX] & CPUID_7_1_EDX_AVX10) {
+ uint8_t version =
+ kvm_arch_get_supported_cpuid(cs->kvm_state, 0x24, 0, R_EBX);
+
+ if (!env->avx10_version) {
+ env->avx10_version = version;
+ }
+
+ if (version < env->avx10_version) {
+ const char *msg = accel_uses_host_cpuid()
+ ? "Host doesn't support requested feature"
+ : "TCG doesn't support requested feature";
+ if (cpu->enforce_cpuid) {
+ error_setg(&local_err, "%s: avx10.%d", msg,
+ env->avx10_version);
+ goto out;
+ } else if (cpu->check_cpuid) {
+ warn_report("%s: avx10.%d", msg, env->avx10_version);
+ }
+ env->avx10_version = version;
+ }
+ }
+
if (cpu->enforce_cpuid && x86_cpu_have_filtered_features(cpu)) {
error_setg(&local_err,
accel_uses_host_cpuid() ?
