Re: [Qemu-devel] [PATCH 1/1] target-i386: prevent users from setting threads>1 for AMD CPUs

[Wei Huang]

On 10/08/2014 02:47 AM, Paolo Bonzini wrote:
> Il 08/10/2014 02:41, Wei Huang ha scritto:
> > I am OK with either way. The key question is: should QEMU presents
> > CPUIDs strictly as specified by the command line or QEMU can tweak a
> > little bit on behalf of end-users? For instance, if end-users say "-smp
> > 8,cores=2,threads=2,sockets=2", they meant "two socket, each has two
> > 2-hyperthread cores". Current QEMU will convert CPUID as "two socket,
> > each has 4 cores". My patch will forbid the tweaking...
>
> Understood---it actually looks like it was intentional:
>
> commit 400281af34e5ee6aa9f5496b53d8f82c6fef9319
> Author: Andre Przywara <address@hidden>
> Date:   Wed Aug 19 15:42:42 2009 +0200
>
>      set CPUID bits to present cores and threads topology
>
>      Controlled by the enhanced -smp option set the CPUID bits to present the
>      guest the desired topology. This is vendor specific, but (with the 
> exception
>      of the CMP_LEGACY bit) not conflicting, so we set all bits everytime.
>      There is no real multithreading support for AMD CPUs, so report cores
>      instead.
>
>      Signed-off-by: Andre Przywara <address@hidden>
>      Signed-off-by: Anthony Liguori <address@hidden>

Given that back-ward compatibility is a concern, will the following work?

1. Instead of bailing out, print a warning message (e.g. to log file via 
error_report) in QEMU.
2. [optional] Eduardo Habkost suggested that we can create a new machine 
model which more strictly checks threads=n option for AMD. For any 
existing machine config, we don't force it; but warning message still 
applies. This is optional because it is a bit over-killed IMO.
3. Gives out a warning in virt-manager as well. This is similar to 
"Overcomming CPUs will slow down performance" in current virt-manager 
screen. The message will read "Chosen CPU model doesn't support 
hyperthreading" or something similar.

-Wei

> Paolo