From: Vitaly Kuznetsov <vkuznets@redhat.com>
To: qemu-devel@nongnu.org, Paolo Bonzini <pbonzini@redhat.com>
Cc: Marcelo Tosatti <mtosatti@redhat.com>
Subject: [PATCH RESEND 1/2] i386: Fix conditional CONFIG_SYNDBG enablement
Date: Fri, 22 Sep 2023 14:19:14 +0200 [thread overview]
Message-ID: <20230922121915.566591-2-vkuznets@redhat.com> (raw)
In-Reply-To: <20230922121915.566591-1-vkuznets@redhat.com>
Putting HYPERV_FEAT_SYNDBG entry under "#ifdef CONFIG_SYNDBG" in
'kvm_hyperv_properties' array is wrong: as HYPERV_FEAT_SYNDBG is not
the highest feature number, the result is an empty (zeroed) entry in
the array (and not a skipped entry!). hyperv_feature_supported() is
designed to check that all CPUID bits are set but for a zeroed
feature in 'kvm_hyperv_properties' it returns 'true' so QEMU considers
HYPERV_FEAT_SYNDBG as always supported, regardless of whether KVM host
actually supports it.
To fix the issue, leave HYPERV_FEAT_SYNDBG's definition in
'kvm_hyperv_properties' array, there's nothing wrong in having it defined
even when 'CONFIG_SYNDBG' is not set. Instead, put "hv-syndbg" CPU property
under '#ifdef CONFIG_SYNDBG' to alter the existing behavior when the flag
is silently skipped in !CONFIG_SYNDBG builds.
Leave an 'assert' sentinel in hyperv_feature_supported() making sure there
are no 'holes' or improperly defined features in 'kvm_hyperv_properties'.
Fixes: d8701185f40c ("hw: hyperv: Initial commit for Synthetic Debugging device")
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
target/i386/cpu.c | 2 ++
target/i386/kvm/kvm.c | 11 +++++++----
2 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 2589c8e9294a..01c7e8414408 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -7840,8 +7840,10 @@ static Property x86_cpu_properties[] = {
HYPERV_FEAT_TLBFLUSH_DIRECT, 0),
DEFINE_PROP_ON_OFF_AUTO("hv-no-nonarch-coresharing", X86CPU,
hyperv_no_nonarch_cs, ON_OFF_AUTO_OFF),
+#ifdef CONFIG_SYNDBG
DEFINE_PROP_BIT64("hv-syndbg", X86CPU, hyperv_features,
HYPERV_FEAT_SYNDBG, 0),
+#endif
DEFINE_PROP_BOOL("hv-passthrough", X86CPU, hyperv_passthrough, false),
DEFINE_PROP_BOOL("hv-enforce-cpuid", X86CPU, hyperv_enforce_cpuid, false),
diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index af101fcdf6ff..51b381a2fbbc 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -993,7 +993,6 @@ static struct {
.bits = HV_DEPRECATING_AEOI_RECOMMENDED}
}
},
-#ifdef CONFIG_SYNDBG
[HYPERV_FEAT_SYNDBG] = {
.desc = "Enable synthetic kernel debugger channel (hv-syndbg)",
.flags = {
@@ -1002,7 +1001,6 @@ static struct {
},
.dependencies = BIT(HYPERV_FEAT_SYNIC) | BIT(HYPERV_FEAT_RELAXED)
},
-#endif
[HYPERV_FEAT_MSR_BITMAP] = {
.desc = "enlightened MSR-Bitmap (hv-emsr-bitmap)",
.flags = {
@@ -1254,6 +1252,13 @@ static bool hyperv_feature_supported(CPUState *cs, int feature)
uint32_t func, bits;
int i, reg;
+ /*
+ * kvm_hyperv_properties needs to define at least one CPUID flag which
+ * must be used to detect the feature, it's hard to say whether it is
+ * supported or not otherwise.
+ */
+ assert(kvm_hyperv_properties[feature].flags[0].func);
+
for (i = 0; i < ARRAY_SIZE(kvm_hyperv_properties[feature].flags); i++) {
func = kvm_hyperv_properties[feature].flags[i].func;
@@ -3483,13 +3488,11 @@ static int kvm_put_msrs(X86CPU *cpu, int level)
kvm_msr_entry_add(cpu, HV_X64_MSR_TSC_EMULATION_STATUS,
env->msr_hv_tsc_emulation_status);
}
-#ifdef CONFIG_SYNDBG
if (hyperv_feat_enabled(cpu, HYPERV_FEAT_SYNDBG) &&
has_msr_hv_syndbg_options) {
kvm_msr_entry_add(cpu, HV_X64_MSR_SYNDBG_OPTIONS,
hyperv_syndbg_query_options());
}
-#endif
}
if (hyperv_feat_enabled(cpu, HYPERV_FEAT_VAPIC)) {
kvm_msr_entry_add(cpu, HV_X64_MSR_APIC_ASSIST_PAGE,
--
2.41.0
next prev parent reply other threads:[~2023-09-22 12:20 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-09-22 12:19 [PATCH RESEND 0/2] i386: Fix Hyper-V Gen1 guests stuck on boot with 'hv-passthrough' Vitaly Kuznetsov
2023-09-22 12:19 ` Vitaly Kuznetsov [this message]
2023-09-22 12:19 ` [PATCH RESEND 2/2] i386: Exclude 'hv-syndbg' from 'hv-passthrough' Vitaly Kuznetsov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230922121915.566591-2-vkuznets@redhat.com \
--to=vkuznets@redhat.com \
--cc=mtosatti@redhat.com \
--cc=pbonzini@redhat.com \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).