* [PATCH 1/2] i386: Fix conditional CONFIG_SYNDBG enablement
2023-06-12 8:41 [PATCH 0/2] i386: Fix Hyper-V Gen1 guests stuck on boot with 'hv-passthrough' Vitaly Kuznetsov
@ 2023-06-12 8:42 ` Vitaly Kuznetsov
2023-06-12 8:42 ` [PATCH 2/2] i386: Exclude 'hv-syndbg' from 'hv-passthrough' Vitaly Kuznetsov
2023-06-27 8:31 ` [PATCH 0/2] i386: Fix Hyper-V Gen1 guests stuck on boot with 'hv-passthrough' Vitaly Kuznetsov
2 siblings, 0 replies; 6+ messages in thread
From: Vitaly Kuznetsov @ 2023-06-12 8:42 UTC (permalink / raw)
To: qemu-devel, Paolo Bonzini; +Cc: Marcelo Tosatti
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 1242bd541a53..caa207849e9a 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -7564,8 +7564,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 de531842f6b1..88c75f58f0a6 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -989,7 +989,6 @@ static struct {
.bits = HV_DEPRECATING_AEOI_RECOMMENDED}
}
},
-#ifdef CONFIG_SYNDBG
[HYPERV_FEAT_SYNDBG] = {
.desc = "Enable synthetic kernel debugger channel (hv-syndbg)",
.flags = {
@@ -998,7 +997,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 = {
@@ -1250,6 +1248,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;
@@ -3474,13 +3479,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.40.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 2/2] i386: Exclude 'hv-syndbg' from 'hv-passthrough'
2023-06-12 8:41 [PATCH 0/2] i386: Fix Hyper-V Gen1 guests stuck on boot with 'hv-passthrough' Vitaly Kuznetsov
2023-06-12 8:42 ` [PATCH 1/2] i386: Fix conditional CONFIG_SYNDBG enablement Vitaly Kuznetsov
@ 2023-06-12 8:42 ` Vitaly Kuznetsov
2023-06-27 8:31 ` [PATCH 0/2] i386: Fix Hyper-V Gen1 guests stuck on boot with 'hv-passthrough' Vitaly Kuznetsov
2 siblings, 0 replies; 6+ messages in thread
From: Vitaly Kuznetsov @ 2023-06-12 8:42 UTC (permalink / raw)
To: qemu-devel, Paolo Bonzini; +Cc: Marcelo Tosatti
Windows with Hyper-V role enabled doesn't boot with 'hv-passthrough' when
no debugger is configured, this significantly limits the usefulness of the
feature as there's no support for subtracting Hyper-V features from CPU
flags at this moment (e.g. "-cpu host,hv-passthrough,-hv-syndbg" does not
work). While this is also theoretically fixable, 'hv-syndbg' is likely
very special and unneeded in the default set. Genuine Hyper-V doesn't seem
to enable it either.
Introduce 'skip_passthrough' flag to 'kvm_hyperv_properties' and use it as
one-off to skip 'hv-syndbg' when enabling features in 'hv-passthrough'
mode. Note, "-cpu host,hv-passthrough,hv-syndbg" can still be used if
needed.
As both 'hv-passthrough' and 'hv-syndbg' are debug features, the change
should not have any effect on production environments.
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
docs/system/i386/hyperv.rst | 13 +++++++++----
target/i386/kvm/kvm.c | 7 +++++--
2 files changed, 14 insertions(+), 6 deletions(-)
diff --git a/docs/system/i386/hyperv.rst b/docs/system/i386/hyperv.rst
index 2505dc4c86e0..009947e39141 100644
--- a/docs/system/i386/hyperv.rst
+++ b/docs/system/i386/hyperv.rst
@@ -262,14 +262,19 @@ Supplementary features
``hv-passthrough``
In some cases (e.g. during development) it may make sense to use QEMU in
'pass-through' mode and give Windows guests all enlightenments currently
- supported by KVM. This pass-through mode is enabled by "hv-passthrough" CPU
- flag.
+ supported by KVM.
Note: ``hv-passthrough`` flag only enables enlightenments which are known to QEMU
(have corresponding 'hv-' flag) and copies ``hv-spinlocks`` and ``hv-vendor-id``
values from KVM to QEMU. ``hv-passthrough`` overrides all other 'hv-' settings on
- the command line. Also, enabling this flag effectively prevents migration as the
- list of enabled enlightenments may differ between target and destination hosts.
+ the command line.
+
+ Note: ``hv-passthrough`` does not enable ``hv-syndbg`` which can prevent certain
+ Windows guests from booting when used without proper configuration. If needed,
+ ``hv-syndbg`` can be enabled additionally.
+
+ Note: ``hv-passthrough`` effectively prevents migration as the list of enabled
+ enlightenments may differ between target and destination hosts.
``hv-enforce-cpuid``
By default, KVM allows the guest to use all currently supported Hyper-V
diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index 88c75f58f0a6..fbaaacf9877c 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -867,6 +867,7 @@ static struct {
uint32_t bits;
} flags[2];
uint64_t dependencies;
+ bool skip_passthrough;
} kvm_hyperv_properties[] = {
[HYPERV_FEAT_RELAXED] = {
.desc = "relaxed timing (hv-relaxed)",
@@ -995,7 +996,8 @@ static struct {
{.func = HV_CPUID_FEATURES, .reg = R_EDX,
.bits = HV_FEATURE_DEBUG_MSRS_AVAILABLE}
},
- .dependencies = BIT(HYPERV_FEAT_SYNIC) | BIT(HYPERV_FEAT_RELAXED)
+ .dependencies = BIT(HYPERV_FEAT_SYNIC) | BIT(HYPERV_FEAT_RELAXED),
+ .skip_passthrough = true,
},
[HYPERV_FEAT_MSR_BITMAP] = {
.desc = "enlightened MSR-Bitmap (hv-emsr-bitmap)",
@@ -1404,7 +1406,8 @@ bool kvm_hyperv_expand_features(X86CPU *cpu, Error **errp)
* hv_build_cpuid_leaf() uses this info to build guest CPUIDs.
*/
for (feat = 0; feat < ARRAY_SIZE(kvm_hyperv_properties); feat++) {
- if (hyperv_feature_supported(cs, feat)) {
+ if (hyperv_feature_supported(cs, feat) &&
+ !kvm_hyperv_properties[feat].skip_passthrough) {
cpu->hyperv_features |= BIT(feat);
}
}
--
2.40.1
^ permalink raw reply related [flat|nested] 6+ messages in thread