[PATCH 0/2] i386: use better matching family/model/stepping for generic CPUs

[PATCH 0/2] i386: use better matching family/model/stepping for generic CPUs

[Daniel P. Berrangé]

This is in response to this bug report:

   https://gitlab.com/qemu-project/qemu/-/issues/191

The generic 'qemu64' and 'max' CPUs currently defined to report a
family/model/stepping that approximately corresponds to an AMD K7
vintage architecture. The K7 series predates the introduction of
64-bit support by AMD in the K8 series. This has been reported to
lead to LLVM complaints about generating 64-bit code for a 32-bit
CPU target

  LLVM ERROR: 64-bit code requested on a subtarget that doesn't support it!

The bug report is fairly limited, but it suggests LLVM looks at the
family/model/stepping and decides it to be 32-bit, despite qemu64
reporting it is 64-bit capable. I've not reproduced this myself,
however, so I'm largely trusting the original reporter's diagnosis

Of course interpreting the family/model/stepping only makes sense
with scoped to the reported vendor ID.  Under TCG, the vendor is
honoured, but AFAICT, under KVM the vendor defined by the QEMU
model model is ignored and the real host vendor passed through.
This will make the chosen family/model/stepping non-sensical when
run under KVM on an Intel host.

None the less these patches change to report a CPUID with the
family, model and stepping taken from a

 AMD Athlon(tm) 64 X2 Dual Core Processor 4000+

which is one of the first 64-bit AMD CPUs. This is at least more
accurate in terms of the static CPU model definition, even if it
is still nonsense in the case where KVM overrides the vendor to
be non-AMD.

Daniel P. Berrangé (2):
  i386: use better matching family/model/stepping for 'qemu64' CPU
  i386: use better matching family/model/stepping for 'max' CPU

 hw/i386/pc.c      |  6 +++++-
 target/i386/cpu.c | 12 +++++++++---
 2 files changed, 14 insertions(+), 4 deletions(-)

-- 
2.31.1

[PATCH 1/2] i386: use better matching family/model/stepping for 'qemu64' CPU

[Daniel P. Berrangé]

The 'qemu64' CPUID currently reports a family/model/stepping that
approximately corresponds to an AMD K7 vintage architecture.
The K7 series predates the introduction of 64-bit support by AMD
in the K8 series. This has been reported to lead to LLVM complaints
about generating 64-bit code for a 32-bit CPU target

  LLVM ERROR: 64-bit code requested on a subtarget that doesn't support it!

It appears LLVM looks at the family/model/stepping, despite qemu64
reporting it is 64-bit capable.

This patch changes 'qemu64' to report a CPUID with the family, model
and stepping taken from a

 AMD Athlon(tm) 64 X2 Dual Core Processor 4000+

which is one of the first 64-bit AMD CPUs.

Closes https://gitlab.com/qemu-project/qemu/-/issues/191
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 hw/i386/pc.c      | 6 +++++-
 target/i386/cpu.c | 6 +++---
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 364816efc9..35d7a8122a 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -96,7 +96,11 @@
 #include "trace.h"
 #include CONFIG_DEVICES
 
-GlobalProperty pc_compat_6_0[] = {};
+GlobalProperty pc_compat_6_0[] = {
+    { "qemu64" "-" TYPE_X86_CPU, "family", "6" },
+    { "qemu64" "-" TYPE_X86_CPU, "model", "6" },
+    { "qemu64" "-" TYPE_X86_CPU, "stepping", "3" },
+};
 const size_t pc_compat_6_0_len = G_N_ELEMENTS(pc_compat_6_0);
 
 GlobalProperty pc_compat_5_2[] = {
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index ad99cad0e7..99caa3deae 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -1881,9 +1881,9 @@ static X86CPUDefinition builtin_x86_defs[] = {
         .name = "qemu64",
         .level = 0xd,
         .vendor = CPUID_VENDOR_AMD,
-        .family = 6,
-        .model = 6,
-        .stepping = 3,
+        .family = 15,
+        .model = 107,
+        .stepping = 1,
         .features[FEAT_1_EDX] =
             PPRO_FEATURES |
             CPUID_MTRR | CPUID_CLFLUSH | CPUID_MCA |
-- 
2.31.1

[PATCH 2/2] i386: use better matching family/model/stepping for 'max' CPU

[Daniel P. Berrangé]

The 'max' CPU under TCG currently reports a family/model/stepping that
approximately corresponds to an AMD K7 vintage architecture.
The K7 series predates the introduction of 64-bit support by AMD
in the K8 series. This has been reported to lead to LLVM complaints
about generating 64-bit code for a 32-bit CPU target

  LLVM ERROR: 64-bit code requested on a subtarget that doesn't support it!

It appears LLVM looks at the family/model/stepping, despite qemu64
reporting it is 64-bit capable.

This patch changes 'max' to report a CPUID with the family, model
and stepping taken from a

 AMD Athlon(tm) 64 X2 Dual Core Processor 4000+

which is one of the first 64-bit AMD CPUs.

Closes https://gitlab.com/qemu-project/qemu/-/issues/191
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 target/i386/cpu.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 99caa3deae..80de5b04eb 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -4440,9 +4440,15 @@ static void max_x86_cpu_initfn(Object *obj)
     } else {
         object_property_set_str(OBJECT(cpu), "vendor", CPUID_VENDOR_AMD,
                                 &error_abort);
+#ifdef TARGET_X86_64
+        object_property_set_int(OBJECT(cpu), "family", 15, &error_abort);
+        object_property_set_int(OBJECT(cpu), "model", 107, &error_abort);
+        object_property_set_int(OBJECT(cpu), "stepping", 1, &error_abort);
+#else
         object_property_set_int(OBJECT(cpu), "family", 6, &error_abort);
         object_property_set_int(OBJECT(cpu), "model", 6, &error_abort);
         object_property_set_int(OBJECT(cpu), "stepping", 3, &error_abort);
+#endif
         object_property_set_str(OBJECT(cpu), "model-id",
                                 "QEMU TCG CPU version " QEMU_HW_VERSION,
                                 &error_abort);
-- 
2.31.1

Re: [PATCH 0/2] i386: use better matching family/model/stepping for generic CPUs

[Daniel P. Berrangé]

ping

On Fri, May 07, 2021 at 02:36:48PM +0100, Daniel P. Berrangé wrote:
> This is in response to this bug report:
> 
>    https://gitlab.com/qemu-project/qemu/-/issues/191
> 
> The generic 'qemu64' and 'max' CPUs currently defined to report a
> family/model/stepping that approximately corresponds to an AMD K7
> vintage architecture. The K7 series predates the introduction of
> 64-bit support by AMD in the K8 series. This has been reported to
> lead to LLVM complaints about generating 64-bit code for a 32-bit
> CPU target
> 
>   LLVM ERROR: 64-bit code requested on a subtarget that doesn't support it!
> 
> The bug report is fairly limited, but it suggests LLVM looks at the
> family/model/stepping and decides it to be 32-bit, despite qemu64
> reporting it is 64-bit capable. I've not reproduced this myself,
> however, so I'm largely trusting the original reporter's diagnosis
> 
> Of course interpreting the family/model/stepping only makes sense
> with scoped to the reported vendor ID.  Under TCG, the vendor is
> honoured, but AFAICT, under KVM the vendor defined by the QEMU
> model model is ignored and the real host vendor passed through.
> This will make the chosen family/model/stepping non-sensical when
> run under KVM on an Intel host.
> 
> None the less these patches change to report a CPUID with the
> family, model and stepping taken from a
> 
>  AMD Athlon(tm) 64 X2 Dual Core Processor 4000+
> 
> which is one of the first 64-bit AMD CPUs. This is at least more
> accurate in terms of the static CPU model definition, even if it
> is still nonsense in the case where KVM overrides the vendor to
> be non-AMD.
> 
> Daniel P. Berrangé (2):
>   i386: use better matching family/model/stepping for 'qemu64' CPU
>   i386: use better matching family/model/stepping for 'max' CPU
> 
>  hw/i386/pc.c      |  6 +++++-
>  target/i386/cpu.c | 12 +++++++++---
>  2 files changed, 14 insertions(+), 4 deletions(-)
> 
> -- 
> 2.31.1
> 
> 

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH 1/2] i386: use better matching family/model/stepping for 'qemu64' CPU

[Eduardo Habkost]

On Fri, May 07, 2021 at 02:36:49PM +0100, Daniel P. Berrangé wrote:
> The 'qemu64' CPUID currently reports a family/model/stepping that
> approximately corresponds to an AMD K7 vintage architecture.
> The K7 series predates the introduction of 64-bit support by AMD
> in the K8 series. This has been reported to lead to LLVM complaints
> about generating 64-bit code for a 32-bit CPU target
> 
>   LLVM ERROR: 64-bit code requested on a subtarget that doesn't support it!
> 
> It appears LLVM looks at the family/model/stepping, despite qemu64
> reporting it is 64-bit capable.
> 
> This patch changes 'qemu64' to report a CPUID with the family, model
> and stepping taken from a
> 
>  AMD Athlon(tm) 64 X2 Dual Core Processor 4000+
> 
> which is one of the first 64-bit AMD CPUs.
> 
> Closes https://gitlab.com/qemu-project/qemu/-/issues/191
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  hw/i386/pc.c      | 6 +++++-
>  target/i386/cpu.c | 6 +++---
>  2 files changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 364816efc9..35d7a8122a 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -96,7 +96,11 @@
>  #include "trace.h"
>  #include CONFIG_DEVICES
>  
> -GlobalProperty pc_compat_6_0[] = {};
> +GlobalProperty pc_compat_6_0[] = {
> +    { "qemu64" "-" TYPE_X86_CPU, "family", "6" },
> +    { "qemu64" "-" TYPE_X86_CPU, "model", "6" },
> +    { "qemu64" "-" TYPE_X86_CPU, "stepping", "3" },

I wish we could have done this using a new CPU model version, but
the new version wouldn't be very useful unless the default CPU
model version to CPU_VERSION_LATEST[1], so:

Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>

[1] We still can't do that without breaking libvirt expectations
    about machine types.

> +};
>  const size_t pc_compat_6_0_len = G_N_ELEMENTS(pc_compat_6_0);
>  
>  GlobalProperty pc_compat_5_2[] = {
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index ad99cad0e7..99caa3deae 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -1881,9 +1881,9 @@ static X86CPUDefinition builtin_x86_defs[] = {
>          .name = "qemu64",
>          .level = 0xd,
>          .vendor = CPUID_VENDOR_AMD,
> -        .family = 6,
> -        .model = 6,
> -        .stepping = 3,
> +        .family = 15,
> +        .model = 107,
> +        .stepping = 1,
>          .features[FEAT_1_EDX] =
>              PPRO_FEATURES |
>              CPUID_MTRR | CPUID_CLFLUSH | CPUID_MCA |
> -- 
> 2.31.1
> 

-- 
Eduardo

Re: [PATCH 2/2] i386: use better matching family/model/stepping for 'max' CPU

[Eduardo Habkost]

On Fri, May 07, 2021 at 02:36:50PM +0100, Daniel P. Berrangé wrote:
> The 'max' CPU under TCG currently reports a family/model/stepping that
> approximately corresponds to an AMD K7 vintage architecture.
> The K7 series predates the introduction of 64-bit support by AMD
> in the K8 series. This has been reported to lead to LLVM complaints
> about generating 64-bit code for a 32-bit CPU target
> 
>   LLVM ERROR: 64-bit code requested on a subtarget that doesn't support it!
> 
> It appears LLVM looks at the family/model/stepping, despite qemu64
> reporting it is 64-bit capable.
> 
> This patch changes 'max' to report a CPUID with the family, model
> and stepping taken from a
> 
>  AMD Athlon(tm) 64 X2 Dual Core Processor 4000+
> 
> which is one of the first 64-bit AMD CPUs.
> 
> Closes https://gitlab.com/qemu-project/qemu/-/issues/191
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>

Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>

-- 
Eduardo

Re: [PATCH 0/2] i386: use better matching family/model/stepping for generic CPUs

[Eduardo Habkost]

On Fri, May 07, 2021 at 02:36:48PM +0100, Daniel P. Berrangé wrote:
[...]
> Daniel P. Berrangé (2):
>   i386: use better matching family/model/stepping for 'qemu64' CPU
>   i386: use better matching family/model/stepping for 'max' CPU

Queued, thanks!

-- 
Eduardo