From c4afcec90f117e703666e2436592cc4e825ef2a1 Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Mon, 29 Jul 2024 15:12:44 +1000 Subject: [PATCH 01/10] tests/vm/openbsd: Install tomli OpenBSD still defaults to python 3.10, therefore tomli is now required by configure. Signed-off-by: Richard Henderson Link: https://lore.kernel.org/r/20240729051244.436851-1-richard.henderson@linaro.org Signed-off-by: Paolo Bonzini --- tests/vm/openbsd | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/vm/openbsd b/tests/vm/openbsd index 5e646f7c51..49cab08782 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", From 39635ccd0b4935ecbf184cf4544fce92d5827de2 Mon Sep 17 00:00:00 2001 From: Xiong Zhang Date: Tue, 30 Jul 2024 16:29:27 +0800 Subject: [PATCH 02/10] target/i386: Change unavail from u32 to u64 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 Link: https://lore.kernel.org/r/20240730082927.250180-1-xiong.y.zhang@linux.intel.com Signed-off-by: Paolo Bonzini --- 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 4688d140c2..ef06da54c6 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) { From eee194dd7120f62896d5fbffeba8ab2ec39a09da Mon Sep 17 00:00:00 2001 From: Zhao Liu Date: Tue, 30 Jul 2024 12:55:41 +0800 Subject: [PATCH 03/10] target/i386/cpu: Remove unnecessary SGX feature words checks 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 Link: https://lore.kernel.org/r/20240730045544.2516284-2-zhao1.liu@intel.com Signed-off-by: Paolo Bonzini --- 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 ef06da54c6..a9535284aa 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) { From 4912d6990b806177733efc1365110cd2d92513fa Mon Sep 17 00:00:00 2001 From: Zhao Liu Date: Tue, 30 Jul 2024 12:55:42 +0800 Subject: [PATCH 04/10] target/i386/cpu: Explicitly express SGX_LC and SGX feature words dependency 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 Link: https://lore.kernel.org/r/20240730045544.2516284-3-zhao1.liu@intel.com Signed-off-by: Paolo Bonzini --- 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 a9535284aa..e864f55d4f 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]; From 3722a98948d4fedec7a8c4575f520b346b6bc923 Mon Sep 17 00:00:00 2001 From: Zhao Liu Date: Tue, 30 Jul 2024 12:55:43 +0800 Subject: [PATCH 05/10] target/i386/cpu: Add dependencies of CPUID 0x12 leaves 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 Link: https://lore.kernel.org/r/20240730045544.2516284-4-zhao1.liu@intel.com Signed-off-by: Paolo Bonzini --- target/i386/cpu.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/target/i386/cpu.c b/target/i386/cpu.c index e864f55d4f..28b46ef536 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 { From ada1f3cab32a4eded6a453c2e22fc897009da555 Mon Sep 17 00:00:00 2001 From: Zhao Liu Date: Tue, 30 Jul 2024 12:55:44 +0800 Subject: [PATCH 06/10] target/i386/cpu: Mask off SGX/SGX_LC feature words for non-PC machine Only PC machine supports SGX, so mask off SGX related feature words for non-PC machine (microvm). Signed-off-by: Zhao Liu Link: https://lore.kernel.org/r/20240730045544.2516284-5-zhao1.liu@intel.com Signed-off-by: Paolo Bonzini --- hw/i386/sgx-stub.c | 5 +++++ hw/i386/sgx.c | 8 ++++++++ include/hw/i386/sgx-epc.h | 1 + target/i386/cpu.c | 15 +++++++++++++++ 4 files changed, 29 insertions(+) diff --git a/hw/i386/sgx-stub.c b/hw/i386/sgx-stub.c index 16b1dfd90b..38ff75e9f3 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 849472a128..4900dd414a 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/include/hw/i386/sgx-epc.h b/include/hw/i386/sgx-epc.h index 3e00efd870..41d55da479 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/target/i386/cpu.c b/target/i386/cpu.c index 28b46ef536..85ef7452c0 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; } From 5997fbdfaca3e69062c2b706d6228c7c6e133c03 Mon Sep 17 00:00:00 2001 From: Anthony Harivel Date: Fri, 26 Jul 2024 12:26:31 +0200 Subject: [PATCH 07/10] target/i386: Fix typo that assign same value twice Should fix: CID 1558553 Signed-off-by: Anthony Harivel Link: https://lore.kernel.org/r/20240726102632.1324432-2-aharivel@redhat.com Signed-off-by: Paolo Bonzini --- 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 b4aab9a410..31f149c990 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; From 6e623af30130fce6e94c717176f3e3c9f2742b7d Mon Sep 17 00:00:00 2001 From: Anthony Harivel Date: Fri, 26 Jul 2024 12:26:32 +0200 Subject: [PATCH 08/10] target/i386: Clean up error cases for vmsr_read_thread_stat() 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 Link: https://lore.kernel.org/r/20240726102632.1324432-3-aharivel@redhat.com Signed-off-by: Paolo Bonzini --- 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 a1d78f2f2a..7e064c5aef 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; } From 768a28394c9412fe1cfdf48509713fd11779a658 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Tue, 30 Jul 2024 17:55:37 +0200 Subject: [PATCH 09/10] qemu-vmsr-helper: fix socket loop breakage 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 --- 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 ebf562c3ff..585eaf88b3 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); } From 2a99c2ba822ef9758d739ffdefbe6252520c1719 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Tue, 30 Jul 2024 18:00:01 +0200 Subject: [PATCH 10/10] qemu-vmsr-helper: implement --verbose/-v 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 --- 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 585eaf88b3..a35dcb88a3 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;