qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Paolo Bonzini <pbonzini@redhat.com>
To: qemu-devel@nongnu.org
Cc: Vitaly Kuznetsov <vkuznets@redhat.com>
Subject: [PULL 07/25] target/i386: Fix conditional CONFIG_SYNDBG enablement
Date: Tue, 15 Oct 2024 16:16:53 +0200	[thread overview]
Message-ID: <20241015141711.528342-8-pbonzini@redhat.com> (raw)
In-Reply-To: <20241015141711.528342-1-pbonzini@redhat.com>

From: Vitaly Kuznetsov <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>
Link: https://lore.kernel.org/r/20240917160051.2637594-2-vkuznets@redhat.com
Signed-off-by: Paolo Bonzini <pbonzini@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 9a6b9e9e51b..565aad02ea6 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -8299,8 +8299,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 7c3fcb8698f..1ec4977a8e9 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -1056,7 +1056,6 @@ static struct {
              .bits = HV_DEPRECATING_AEOI_RECOMMENDED}
         }
     },
-#ifdef CONFIG_SYNDBG
     [HYPERV_FEAT_SYNDBG] = {
         .desc = "Enable synthetic kernel debugger channel (hv-syndbg)",
         .flags = {
@@ -1065,7 +1064,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 = {
@@ -1317,6 +1315,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;
@@ -4025,13 +4030,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.46.2



  parent reply	other threads:[~2024-10-15 14:20 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-10-15 14:16 [PULL 00/25] x86 and KVM patches for 2024-10-15 Paolo Bonzini
2024-10-15 14:16 ` [PULL 01/25] target/i386: Don't construct a all-zero entry for CPUID[0xD 0x3f] Paolo Bonzini
2024-10-15 14:16 ` [PULL 02/25] target/i386: Enable fdp-excptn-only and zero-fcs-fds Paolo Bonzini
2024-10-15 14:16 ` [PULL 03/25] target/i386: Construct CPUID 2 as stateful iff times > 1 Paolo Bonzini
2024-10-15 14:16 ` [PULL 04/25] target/i386: Make invtsc migratable when user sets tsc-khz explicitly Paolo Bonzini
2024-10-15 14:16 ` [PULL 05/25] target/i386: Add more features enumerated by CPUID.7.2.EDX Paolo Bonzini
2024-10-15 14:16 ` [PULL 06/25] target/i386: Add support save/load HWCR MSR Paolo Bonzini
2024-10-15 14:16 ` Paolo Bonzini [this message]
2024-10-15 14:16 ` [PULL 08/25] target/i386: Exclude 'hv-syndbg' from 'hv-passthrough' Paolo Bonzini
2024-10-15 14:16 ` [PULL 09/25] target/i386: Make sure SynIC state is really updated before KVM_RUN Paolo Bonzini
2024-10-15 14:16 ` [PULL 10/25] docs/system: Add recommendations to Hyper-V enlightenments doc Paolo Bonzini
2024-10-15 14:16 ` [PULL 11/25] target/i386: convert bit test instructions to new decoder Paolo Bonzini
2024-10-15 14:16 ` [PULL 12/25] target/i386: decode address before going back to translate.c Paolo Bonzini
2024-10-15 14:16 ` [PULL 13/25] target/i386: convert CMPXCHG8B/CMPXCHG16B to new decoder Paolo Bonzini
2024-10-16 16:37   ` Philippe Mathieu-Daudé
2024-10-17  9:14     ` Paolo Bonzini
2024-10-15 14:17 ` [PULL 14/25] target/i386: do not check PREFIX_LOCK in old-style decoder Paolo Bonzini
2024-10-15 14:17 ` [PULL 15/25] target/i386: list instructions still in translate.c Paolo Bonzini
2024-10-15 14:17 ` [PULL 16/25] target/i386: assert that cc_op* and pc_save are preserved Paolo Bonzini
2024-10-15 14:17 ` [PULL 17/25] KVM: Dynamic sized kvm memslots array Paolo Bonzini
2024-10-15 14:17 ` [PULL 18/25] KVM: Define KVM_MEMSLOTS_NUM_MAX_DEFAULT Paolo Bonzini
2024-10-15 14:17 ` [PULL 19/25] KVM: Rename KVMMemoryListener.nr_used_slots to nr_slots_used Paolo Bonzini
2024-10-15 14:17 ` [PULL 20/25] KVM: Rename KVMState->nr_slots to nr_slots_max Paolo Bonzini
2024-10-15 14:17 ` [PULL 21/25] target/i386/tcg: Use DPL-level accesses for interrupts and call gates Paolo Bonzini
2024-10-15 14:17 ` [PULL 22/25] accel/kvm: check for KVM_CAP_MULTI_ADDRESS_SPACE on vm Paolo Bonzini
2024-10-15 14:17 ` [PULL 23/25] accel/kvm: check for KVM_CAP_MEMORY_ATTRIBUTES " Paolo Bonzini
2024-10-15 14:17 ` [PULL 24/25] accel/kvm: check for KVM_CAP_READONLY_MEM on VM Paolo Bonzini
2024-10-15 14:17 ` [PULL 25/25] target/i386: Use only 16 and 32-bit operands for IN/OUT Paolo Bonzini
2024-10-17 10:32 ` [PULL 00/25] x86 and KVM patches for 2024-10-15 Peter Maydell

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=20241015141711.528342-8-pbonzini@redhat.com \
    --to=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=vkuznets@redhat.com \
    /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).