[PULL 00/10] i386, tests/vm changes for QEMU 9.1-rc

[PULL 00/10] i386, tests/vm changes for QEMU 9.1-rc

[Paolo Bonzini]

The following changes since commit 4e56e89d6c81589cc47cf5811f570c67889bd18a:

  Update version for v9.1.0-rc0 release (2024-07-31 16:21:21 +1000)

are available in the Git repository at:

  https://gitlab.com/bonzini/qemu.git tags/for-upstream

for you to fetch changes up to 2a99c2ba822ef9758d739ffdefbe6252520c1719:

  qemu-vmsr-helper: implement --verbose/-v (2024-07-31 13:15:06 +0200)

----------------------------------------------------------------
* target/i386: qemu-vmsr-helper fixes
* target/i386: mask off SGX/SGX_LC feature words for non-PC machine
* tests/vm/openbsd: Install tomli
* fix issue with 64-bit features (vmx kvm-unit-tests)

----------------------------------------------------------------
Anthony Harivel (2):
      target/i386: Fix typo that assign same value twice
      target/i386: Clean up error cases for vmsr_read_thread_stat()

Paolo Bonzini (2):
      qemu-vmsr-helper: fix socket loop breakage
      qemu-vmsr-helper: implement --verbose/-v

Richard Henderson (1):
      tests/vm/openbsd: Install tomli

Xiong Zhang (1):
      target/i386: Change unavail from u32 to u64

Zhao Liu (4):
      target/i386/cpu: Remove unnecessary SGX feature words checks
      target/i386/cpu: Explicitly express SGX_LC and SGX feature words dependency
      target/i386/cpu: Add dependencies of CPUID 0x12 leaves
      target/i386/cpu: Mask off SGX/SGX_LC feature words for non-PC machine

 include/hw/i386/sgx-epc.h     |  1 +
 hw/i386/sgx-stub.c            |  5 +++++
 hw/i386/sgx.c                 |  8 +++++++
 target/i386/cpu.c             | 52 ++++++++++++++++++++++++++-----------------
 target/i386/kvm/kvm.c         |  4 ++--
 target/i386/kvm/vmsr_energy.c |  5 +++--
 tools/i386/qemu-vmsr-helper.c | 24 ++++++++++++++------
 tests/vm/openbsd              |  1 +
 8 files changed, 69 insertions(+), 31 deletions(-)
-- 
2.45.2

[PULL 01/10] tests/vm/openbsd: Install tomli

[Paolo Bonzini]

From: Richard Henderson <richard.henderson@linaro.org>

OpenBSD still defaults to python 3.10, therefore tomli is now required by configure.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Link: https://lore.kernel.org/r/20240729051244.436851-1-richard.henderson@linaro.org
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 tests/vm/openbsd | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/vm/openbsd b/tests/vm/openbsd
index 5e646f7c51e..49cab087825 100755
--- a/tests/vm/openbsd
+++ b/tests/vm/openbsd
@@ -32,6 +32,7 @@ class OpenBSDVM(basevm.BaseVM):
         "pkgconf",
         "bzip2", "xz",
         "ninja",
+        "py3-tomli",
 
         # gnu tools
         "bash",
-- 
2.45.2

[PULL 02/10] target/i386: Change unavail from u32 to u64

[Paolo Bonzini]

From: Xiong Zhang <xiong.y.zhang@linux.intel.com>

The feature word 'r' is a u64, and "unavail" is a u32, the operation
'r &= ~unavail' clears the high 32 bits of 'r'. This causes many vmx cases
in kvm-unit-tests to fail. Changing 'unavail' from u32 to u64 fixes this
issue.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2442
Fixes: 0b2757412cb1 ("target/i386: drop AMD machine check bits from Intel CPUID")
Signed-off-by: Xiong Zhang <xiong.y.zhang@linux.intel.com>
Link: https://lore.kernel.org/r/20240730082927.250180-1-xiong.y.zhang@linux.intel.com
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 target/i386/cpu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 4688d140c2d..ef06da54c63 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -6039,7 +6039,7 @@ uint64_t x86_cpu_get_supported_feature_word(X86CPU *cpu, FeatureWord w)
 {
     FeatureWordInfo *wi = &feature_word_info[w];
     uint64_t r = 0;
-    uint32_t unavail = 0;
+    uint64_t unavail = 0;
 
     if (kvm_enabled()) {
         switch (wi->type) {
-- 
2.45.2

[PULL 03/10] target/i386/cpu: Remove unnecessary SGX feature words checks

[Paolo Bonzini]

From: Zhao Liu <zhao1.liu@intel.com>

CPUID.0x7.0.ebx and CPUID.0x7.0.ecx leaves have been expressed as the
feature word lists, and the Host capability support has been checked
in x86_cpu_filter_features().

Therefore, such checks on SGX feature "words" are redundant, and
the follow-up adjustments to those feature "words" will not actually
take effect.

Remove unnecessary SGX feature words related checks.

Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
Link: https://lore.kernel.org/r/20240730045544.2516284-2-zhao1.liu@intel.com
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 target/i386/cpu.c | 16 +---------------
 1 file changed, 1 insertion(+), 15 deletions(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index ef06da54c63..a9535284aa9 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -6537,8 +6537,6 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
     case 7:
         /* Structured Extended Feature Flags Enumeration Leaf */
         if (count == 0) {
-            uint32_t eax_0_unused, ebx_0, ecx_0, edx_0_unused;
-
             /* Maximum ECX value for sub-leaves */
             *eax = env->cpuid_level_func7;
             *ebx = env->features[FEAT_7_0_EBX]; /* Feature flags */
@@ -6548,20 +6546,8 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
             }
             *edx = env->features[FEAT_7_0_EDX]; /* Feature flags */
 
-            /*
-             * SGX cannot be emulated in software.  If hardware does not
-             * support enabling SGX and/or SGX flexible launch control,
-             * then we need to update the VM's CPUID values accordingly.
-             */
-            x86_cpu_get_supported_cpuid(0x7, 0,
-                                        &eax_0_unused, &ebx_0,
-                                        &ecx_0, &edx_0_unused);
-            if ((*ebx & CPUID_7_0_EBX_SGX) && !(ebx_0 & CPUID_7_0_EBX_SGX)) {
-                *ebx &= ~CPUID_7_0_EBX_SGX;
-            }
-
             if ((*ecx & CPUID_7_0_ECX_SGX_LC)
-                    && (!(*ebx & CPUID_7_0_EBX_SGX) || !(ecx_0 & CPUID_7_0_ECX_SGX_LC))) {
+                    && (!(*ebx & CPUID_7_0_EBX_SGX))) {
                 *ecx &= ~CPUID_7_0_ECX_SGX_LC;
             }
         } else if (count == 1) {
-- 
2.45.2

[PULL 04/10] target/i386/cpu: Explicitly express SGX_LC and SGX feature words dependency

[Paolo Bonzini]

From: Zhao Liu <zhao1.liu@intel.com>

At present, cpu_x86_cpuid() silently masks off SGX_LC if SGX is absent.

This is not proper because the user is not told about the dependency
between the two.

So explicitly define the dependency between SGX_LC and SGX feature
words, so that user could get a warning when SGX_LC is enabled but
SGX is absent.

Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
Link: https://lore.kernel.org/r/20240730045544.2516284-3-zhao1.liu@intel.com
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 target/i386/cpu.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index a9535284aa9..e864f55d4fc 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -1730,6 +1730,10 @@ static FeatureDep feature_dependencies[] = {
         .from = { FEAT_7_1_EAX,             CPUID_7_1_EAX_WRMSRNS },
         .to = { FEAT_7_1_EAX,               CPUID_7_1_EAX_FRED },
     },
+    {
+        .from = { FEAT_7_0_EBX,             CPUID_7_0_EBX_SGX },
+        .to = { FEAT_7_0_ECX,               CPUID_7_0_ECX_SGX_LC },
+    },
 };
 
 typedef struct X86RegisterInfo32 {
@@ -6545,11 +6549,6 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
                 *ecx |= CPUID_7_0_ECX_OSPKE;
             }
             *edx = env->features[FEAT_7_0_EDX]; /* Feature flags */
-
-            if ((*ecx & CPUID_7_0_ECX_SGX_LC)
-                    && (!(*ebx & CPUID_7_0_EBX_SGX))) {
-                *ecx &= ~CPUID_7_0_ECX_SGX_LC;
-            }
         } else if (count == 1) {
             *eax = env->features[FEAT_7_1_EAX];
             *edx = env->features[FEAT_7_1_EDX];
-- 
2.45.2

[PULL 05/10] target/i386/cpu: Add dependencies of CPUID 0x12 leaves

[Paolo Bonzini]

From: Zhao Liu <zhao1.liu@intel.com>

As SDM stated, CPUID 0x12 leaves depend on CPUID_7_0_EBX_SGX (SGX
feature word).

Since FEAT_SGX_12_0_EAX, FEAT_SGX_12_0_EBX and FEAT_SGX_12_1_EAX define
multiple feature words, add the dependencies of those registers to
report the warning to user if SGX is absent.

Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
Link: https://lore.kernel.org/r/20240730045544.2516284-4-zhao1.liu@intel.com
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 target/i386/cpu.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index e864f55d4fc..28b46ef536f 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -1734,6 +1734,18 @@ static FeatureDep feature_dependencies[] = {
         .from = { FEAT_7_0_EBX,             CPUID_7_0_EBX_SGX },
         .to = { FEAT_7_0_ECX,               CPUID_7_0_ECX_SGX_LC },
     },
+    {
+        .from = { FEAT_7_0_EBX,             CPUID_7_0_EBX_SGX },
+        .to = { FEAT_SGX_12_0_EAX,          ~0ull },
+    },
+    {
+        .from = { FEAT_7_0_EBX,             CPUID_7_0_EBX_SGX },
+        .to = { FEAT_SGX_12_0_EBX,          ~0ull },
+    },
+    {
+        .from = { FEAT_7_0_EBX,             CPUID_7_0_EBX_SGX },
+        .to = { FEAT_SGX_12_1_EAX,          ~0ull },
+    },
 };
 
 typedef struct X86RegisterInfo32 {
-- 
2.45.2

[PULL 06/10] target/i386/cpu: Mask off SGX/SGX_LC feature words for non-PC machine

[Paolo Bonzini]

From: Zhao Liu <zhao1.liu@intel.com>

Only PC machine supports SGX, so mask off SGX related feature words for
non-PC machine (microvm).

Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
Link: https://lore.kernel.org/r/20240730045544.2516284-5-zhao1.liu@intel.com
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 include/hw/i386/sgx-epc.h |  1 +
 hw/i386/sgx-stub.c        |  5 +++++
 hw/i386/sgx.c             |  8 ++++++++
 target/i386/cpu.c         | 15 +++++++++++++++
 4 files changed, 29 insertions(+)

diff --git a/include/hw/i386/sgx-epc.h b/include/hw/i386/sgx-epc.h
index 3e00efd870c..41d55da4799 100644
--- a/include/hw/i386/sgx-epc.h
+++ b/include/hw/i386/sgx-epc.h
@@ -58,6 +58,7 @@ typedef struct SGXEPCState {
     int nr_sections;
 } SGXEPCState;
 
+bool check_sgx_support(void);
 bool sgx_epc_get_section(int section_nr, uint64_t *addr, uint64_t *size);
 void sgx_epc_build_srat(GArray *table_data);
 
diff --git a/hw/i386/sgx-stub.c b/hw/i386/sgx-stub.c
index 16b1dfd90bb..38ff75e9f37 100644
--- a/hw/i386/sgx-stub.c
+++ b/hw/i386/sgx-stub.c
@@ -32,6 +32,11 @@ void pc_machine_init_sgx_epc(PCMachineState *pcms)
     memset(&pcms->sgx_epc, 0, sizeof(SGXEPCState));
 }
 
+bool check_sgx_support(void)
+{
+    return false;
+}
+
 bool sgx_epc_get_section(int section_nr, uint64_t *addr, uint64_t *size)
 {
     return true;
diff --git a/hw/i386/sgx.c b/hw/i386/sgx.c
index 849472a1286..4900dd414a1 100644
--- a/hw/i386/sgx.c
+++ b/hw/i386/sgx.c
@@ -266,6 +266,14 @@ void hmp_info_sgx(Monitor *mon, const QDict *qdict)
                    size);
 }
 
+bool check_sgx_support(void)
+{
+    if (!object_dynamic_cast(qdev_get_machine(), TYPE_PC_MACHINE)) {
+        return false;
+    }
+    return true;
+}
+
 bool sgx_epc_get_section(int section_nr, uint64_t *addr, uint64_t *size)
 {
     PCMachineState *pcms =
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 28b46ef536f..85ef7452c04 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -6103,6 +6103,21 @@ uint64_t x86_cpu_get_supported_feature_word(X86CPU *cpu, FeatureWord w)
         }
         break;
 
+    case FEAT_7_0_EBX:
+#ifndef CONFIG_USER_ONLY
+        if (!check_sgx_support()) {
+            unavail = CPUID_7_0_EBX_SGX;
+        }
+#endif
+        break;
+    case FEAT_7_0_ECX:
+#ifndef CONFIG_USER_ONLY
+        if (!check_sgx_support()) {
+            unavail = CPUID_7_0_ECX_SGX_LC;
+        }
+#endif
+        break;
+
     default:
         break;
     }
-- 
2.45.2

[PULL 07/10] target/i386: Fix typo that assign same value twice

[Paolo Bonzini]

From: Anthony Harivel <aharivel@redhat.com>

Should fix: CID 1558553

Signed-off-by: Anthony Harivel <aharivel@redhat.com>
Link: https://lore.kernel.org/r/20240726102632.1324432-2-aharivel@redhat.com
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 target/i386/kvm/kvm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index b4aab9a410b..31f149c9902 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -2694,8 +2694,8 @@ static void *kvm_msr_energy_thread(void *data)
 
     while (true) {
         /* Get all qemu threads id */
-        g_autofree pid_t *thread_ids =
-            thread_ids = vmsr_get_thread_ids(vmsr->pid, &num_threads);
+        g_autofree pid_t *thread_ids
+            = vmsr_get_thread_ids(vmsr->pid, &num_threads);
 
         if (thread_ids == NULL) {
             goto clean;
-- 
2.45.2

[PULL 08/10] target/i386: Clean up error cases for vmsr_read_thread_stat()

[Paolo Bonzini]

From: Anthony Harivel <aharivel@redhat.com>

Fix leaking memory of file handle in case of error
Erase unused "pid = -1"
Add clearer error_report

Should fix Coverity CID 1558557.

Signed-off-by: Anthony Harivel <aharivel@redhat.com>
Link: https://lore.kernel.org/r/20240726102632.1324432-3-aharivel@redhat.com
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 target/i386/kvm/vmsr_energy.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/target/i386/kvm/vmsr_energy.c b/target/i386/kvm/vmsr_energy.c
index a1d78f2f2ae..7e064c5aef3 100644
--- a/target/i386/kvm/vmsr_energy.c
+++ b/target/i386/kvm/vmsr_energy.c
@@ -270,7 +270,7 @@ void vmsr_read_thread_stat(pid_t pid,
 
     FILE *file = fopen(path, "r");
     if (file == NULL) {
-        pid = -1;
+        error_report("Error opening %s", path_name);
         return;
     }
 
@@ -279,7 +279,8 @@ void vmsr_read_thread_stat(pid_t pid,
         " %*u %*u %*u %*u %*u %*u %*u %*u %*u %*d %*u %*u %u",
            utime, stime, cpu_id) != 3)
     {
-        pid = -1;
+        fclose(file);
+        error_report("Error fscanf did not report the right amount of items");
         return;
     }
 
-- 
2.45.2

[PULL 09/10] qemu-vmsr-helper: fix socket loop breakage

[Paolo Bonzini]

Between v5 and v6 of the series, the socket loop of qemu-vmsr-helper was changed to
allow sending multiple requests on the same socket.  Unfortunately, the condition
of the while loop is botched and the loop will never be entered.  Clean it up, and
also unify the handling of error reporting.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 tools/i386/qemu-vmsr-helper.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/tools/i386/qemu-vmsr-helper.c b/tools/i386/qemu-vmsr-helper.c
index ebf562c3ff8..585eaf88b37 100644
--- a/tools/i386/qemu-vmsr-helper.c
+++ b/tools/i386/qemu-vmsr-helper.c
@@ -227,19 +227,17 @@ static void coroutine_fn vh_co_entry(void *opaque)
                                 &peer_pid,
                                 &local_err);
     if (r < 0) {
-        error_report_err(local_err);
         goto out;
     }
 
-    while (r < 0) {
+    for (;;) {
         /*
          * Read the requested MSR
          * Only RAPL MSR in rapl-msr-index.h is allowed
          */
-        r = qio_channel_read_all(QIO_CHANNEL(client->ioc),
-                                (char *) &request, sizeof(request), &local_err);
-        if (r < 0) {
-            error_report_err(local_err);
+        r = qio_channel_read_all_eof(QIO_CHANNEL(client->ioc),
+                                     (char *) &request, sizeof(request), &local_err);
+        if (r <= 0) {
             break;
         }
 
@@ -261,11 +259,15 @@ static void coroutine_fn vh_co_entry(void *opaque)
                                   sizeof(vmsr),
                                   &local_err);
         if (r < 0) {
-            error_report_err(local_err);
             break;
         }
     }
+
 out:
+    if (local_err) {
+        error_report_err(local_err);
+    }
+
     object_unref(OBJECT(client->ioc));
     g_free(client);
 }
-- 
2.45.2

[PULL 10/10] qemu-vmsr-helper: implement --verbose/-v

[Paolo Bonzini]

Similar to qemu-pr-helper, do not print errors from the socket handling loop
unless a --verbose or -v option is provided explicitly on the command line.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 tools/i386/qemu-vmsr-helper.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/tools/i386/qemu-vmsr-helper.c b/tools/i386/qemu-vmsr-helper.c
index 585eaf88b37..a35dcb88a3f 100644
--- a/tools/i386/qemu-vmsr-helper.c
+++ b/tools/i386/qemu-vmsr-helper.c
@@ -54,6 +54,7 @@ static enum { RUNNING, TERMINATE, TERMINATING } state;
 static QIOChannelSocket *server_ioc;
 static int server_watch;
 static int num_active_sockets = 1;
+static bool verbose;
 
 #ifdef CONFIG_LIBCAP_NG
 static int uid = -1;
@@ -265,7 +266,11 @@ static void coroutine_fn vh_co_entry(void *opaque)
 
 out:
     if (local_err) {
-        error_report_err(local_err);
+        if (!verbose) {
+            error_free(local_err);
+        } else {
+            error_report_err(local_err);
+        }
     }
 
     object_unref(OBJECT(client->ioc));
@@ -431,6 +436,9 @@ int main(int argc, char **argv)
         case 'd':
             daemonize = true;
             break;
+        case 'v':
+            verbose = true;
+            break;
         case 'T':
             trace_opt_parse(optarg);
             break;
-- 
2.45.2

Re: [PULL 00/10] i386, tests/vm changes for QEMU 9.1-rc

[Richard Henderson]

On 7/31/24 21:17, Paolo Bonzini wrote:
> The following changes since commit 4e56e89d6c81589cc47cf5811f570c67889bd18a:
> 
>    Update version for v9.1.0-rc0 release (2024-07-31 16:21:21 +1000)
> 
> are available in the Git repository at:
> 
>    https://gitlab.com/bonzini/qemu.git tags/for-upstream
> 
> for you to fetch changes up to 2a99c2ba822ef9758d739ffdefbe6252520c1719:
> 
>    qemu-vmsr-helper: implement --verbose/-v (2024-07-31 13:15:06 +0200)
> 
> ----------------------------------------------------------------
> * target/i386: qemu-vmsr-helper fixes
> * target/i386: mask off SGX/SGX_LC feature words for non-PC machine
> * tests/vm/openbsd: Install tomli
> * fix issue with 64-bit features (vmx kvm-unit-tests)


Applied, thanks.  Please update https://wiki.qemu.org/ChangeLog/9.1 as appropriate.

r~