Re: [Qemu-devel] [PATCH v3 7/9] target/i386: Support multi-dies when host doesn't support CPUID.1F

[Like Xu <like.xu@linux.intel.com>]

On 2019/6/20 7:36, Eduardo Habkost wrote:
> On Wed, Jun 19, 2019 at 04:15:46PM -0300, Eduardo Habkost wrote:
>> On Wed, Jun 12, 2019 at 04:41:02PM +0800, Like Xu wrote:
>>> In guest CPUID generation process, the cpuid_min_level would be adjusted to
>>> the maximum passed value for basic CPUID configuration and it should not be
>>> restricted by the limited value returned from cpu_x86_cpuid(). After the basic
>>> cpu_x86_cpuid() loop is finished, the cpuid_0_entry.eax needs to be configured
>>> again by the last adjusted cpuid_min_level value.
>>>
>>> If a user wants to expose CPUID.1F by passing dies > 1 for any reason without
>>> host support, a per-cpu smp topology warning will appear but it's not blocked.
>>>
>>> Signed-off-by: Like Xu <like.xu@linux.intel.com>
>>
>> This code doesn't look at host CPUID at all, as far as I can see.
>> Isn't it simpler to just make cpuid_x86_cpuid() return the
>> correct data?
> 
> I suggest the following change instead.
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>

Hi Eduardo,

Your code is more reasonable and concise than mine on this
so let's not break cpuid_x86_cpuid().

I'll remove the use of enable_cpuid_0x1f in next version, and should I 
resend the patch series "Refactor cpu topo into machine properties" 
because rebase-fix may distract you ?

> ---
>   target/i386/cpu.c | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 6db38e145b..d05a224092 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -5152,6 +5152,10 @@ static void x86_cpu_expand_features(X86CPU *cpu, Error **errp)
>               x86_cpu_adjust_level(cpu, &cpu->env.cpuid_min_level, 0x14);
>           }
>   
> +        if (env->nr_dies > 1) {
> +            x86_cpu_adjust_level(cpu, &env->cpuid_min_level, 0x1F);
> +        }
> +
>           /* SVM requires CPUID[0x8000000A] */
>           if (env->features[FEAT_8000_0001_ECX] & CPUID_EXT3_SVM) {
>               x86_cpu_adjust_level(cpu, &env->cpuid_min_xlevel, 0x8000000A);
>