Re: [PATCH 2/4] target/i386: nvmm, whpx: add accel/CPU class that sets host vendor

[Zhao Liu <zhao1.liu@intel.com>]

On Fri, Jul 11, 2025 at 02:06:01AM +0200, Paolo Bonzini wrote:
> Date: Fri, 11 Jul 2025 02:06:01 +0200
> From: Paolo Bonzini <pbonzini@redhat.com>
> Subject: [PATCH 2/4] target/i386: nvmm, whpx: add accel/CPU class that sets
>  host vendor
> X-Mailer: git-send-email 2.50.0
> 
> NVMM and WHPX are virtualizers, and therefore they need to use
> (at least by default) the host vendor for the guest CPUID.

Here's a comment of about why the vendor needs to be overridden in KVM:

(in x86_cpu_load_model())

    /* sysenter isn't supported in compatibility mode on AMD,
     * syscall isn't supported in compatibility mode on Intel.
     * Normally we advertise the actual CPU vendor, but you can
     * override this using the 'vendor' property if you want to use
     * KVM's sysenter/syscall emulation in compatibility mode and
     * when doing cross vendor migration
     */

This is a KVM default-vendor hack since the 1st KVM commit [*].

I guess that this hack might have been related to the immaturity of
vDSO at the time (it's been so long, I just took a quick look at the
general time, maybe linux v2.6), or just to reduce overhead.

Now, both KVM's emulation and vDSO seem to be quite stable. Do you
think QEMU KVM still needs to keep this hack today?

Maybe it's difficult to change for QEMU KVM because it's been a
long-time practice, but other accels don't seem to need to inherit KVM's
history. What do you think?

[*]: 1201818980-27534-7-git-send-email-aliguori@us.ibm.com

> Add a cpu_instance_init implementation to these accelerators.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  target/i386/cpu.c           |  8 +++++++-
>  target/i386/nvmm/nvmm-all.c | 23 +++++++++++++++++++++++
>  target/i386/whpx/whpx-all.c | 23 +++++++++++++++++++++++
>  3 files changed, 53 insertions(+), 1 deletion(-)
> 
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 624cebc3ff7..69bdffbfe46 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -43,6 +43,7 @@
>  #include "hw/boards.h"
>  #include "hw/i386/sgx-epc.h"
>  #endif
> +#include "system/qtest.h"
>  #include "tcg/tcg-cpu.h"
>  
>  #include "disas/capstone.h"
> @@ -1943,7 +1944,7 @@ uint32_t xsave_area_size(uint64_t mask, bool compacted)
>  
>  static inline bool accel_uses_host_cpuid(void)
>  {
> -    return kvm_enabled() || hvf_enabled();
> +    return !tcg_enabled() && !qtest_enabled();
>  }

I was considerreing whether we could check this helper and call
host_cpu_instance_init(cpu) directly in x86_cpu_load_model().

However, this goes against the original intent of moving this hack to
the KVM-specific code. But when it can cover almost all accels, it
becomes a general case...

...

So in summary, the benefits of having all accels override the vendor now
include:

the behavior of -cpu NAMED_CPU is consistent across all accels (except
TCG), and all showing the same vendor as the Host.

The possible issue would be: 
 * This changes the previous behavior of these accels, which might not
   have required setting the vendor before, but now the vendor has
   changed... (I'm unsure if these accels are used in migration
   scenarios, but it's better to add a compat option?)
 * expand the scope of historical KVM hack (if it's still a hack?)

Thanks,
Zhao