On Fri, Jul 07, 2023 at 04:25:22PM +0200, Paolo Bonzini wrote:
> On 7/6/23 21:40, John Allen wrote:
> > case 0x80000007:
> > *eax = 0;
> > - *ebx = 0;
> > + *ebx = env->features[FEAT_8000_0007_EBX] |
> > CPUID_8000_0007_EBX_SUCCOR;
> > *ecx = 0;
> > *edx = env->features[FEAT_8000_0007_EDX];
> > break;
>
> I agree that it needs no hypervisor support, but Babu is right that you
> cannot add it unconditionally (especially not on Intel processors).
>
> You can special case CPUID_8000_0007_EBX_SUCCOR in
> kvm_arch_get_supported_cpuid() so that it is added even on old kernels.
> There are already several such cases. Adding it to KVM is nice to have
> anyway, so please send a patch for that.
By adding it to KVM do you mean adding a patch to the kernel to expose
the cpuid bit? Or do you mean just adding the special case to
kvm_arch_get_supported_cpuid?
For the kvm_arch_get_supported_cpuid case, I don't understand how this
would be different from unconditionally exposing the bit as done above.
Can you help me understand what you have in mind for this?
I might add a case like below:
...
} else if (function == 0x80000007 && reg == R_EBX) {
ret |= CPUID_8000_0007_EBX_SUCCOR;
...
If we wanted to only expose the bit for AMD cpus, we would then need to
call IS_AMD_CPU with the CPUX86State as a paramter which would mean that
kvm_arch_get_supported_cpuid and all of its callers would need to take
the CPUX86State as a parameter. Is there another way to differentiate
between AMD and Intel cpus in this case?
>
> Also, the patch does not compile (probably you missed a prerequisite) as it
> lacks all the rigamarole that is needed to add FEAT_8000_0007_EBX.
I'm not encountering any compilation issues. What are the errors that
you are seeing?
Thanks,
John
