* [PULL 1/3] hv-balloon: avoid alloca() usage
2024-03-08 17:02 [PULL 0/3] Hyper-V Dynamic Memory and VMBus misc small patches Maciej S. Szmigiero
@ 2024-03-08 17:02 ` Maciej S. Szmigiero
2024-03-08 17:02 ` [PULL 2/3] hv-balloon: define dm_hot_add_with_region to avoid Coverity warning Maciej S. Szmigiero
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Maciej S. Szmigiero @ 2024-03-08 17:02 UTC (permalink / raw)
To: qemu-devel
From: "Maciej S. Szmigiero" <maciej.szmigiero@oracle.com>
alloca() is frowned upon, replace it with g_malloc0() + g_autofree.
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: David Hildenbrand <david@redhat.com>
Signed-off-by: Maciej S. Szmigiero <maciej.szmigiero@oracle.com>
---
hw/hyperv/hv-balloon.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/hw/hyperv/hv-balloon.c b/hw/hyperv/hv-balloon.c
index ade283335a68..35333dab2434 100644
--- a/hw/hyperv/hv-balloon.c
+++ b/hw/hyperv/hv-balloon.c
@@ -366,7 +366,7 @@ static void hv_balloon_unballoon_posting(HvBalloon *balloon, StateDesc *stdesc)
PageRangeTree dtree;
uint64_t *dctr;
bool our_range;
- struct dm_unballoon_request *ur;
+ g_autofree struct dm_unballoon_request *ur = NULL;
size_t ur_size = sizeof(*ur) + sizeof(ur->range_array[0]);
PageRange range;
bool bret;
@@ -388,8 +388,7 @@ static void hv_balloon_unballoon_posting(HvBalloon *balloon, StateDesc *stdesc)
assert(dtree.t);
assert(dctr);
- ur = alloca(ur_size);
- memset(ur, 0, ur_size);
+ ur = g_malloc0(ur_size);
ur->hdr.type = DM_UNBALLOON_REQUEST;
ur->hdr.size = ur_size;
ur->hdr.trans_id = balloon->trans_id;
@@ -531,7 +530,7 @@ static void hv_balloon_hot_add_posting(HvBalloon *balloon, StateDesc *stdesc)
PageRange *hot_add_range = &balloon->hot_add_range;
uint64_t *current_count = &balloon->ha_current_count;
VMBusChannel *chan = hv_balloon_get_channel(balloon);
- struct dm_hot_add *ha;
+ g_autofree struct dm_hot_add *ha = NULL;
size_t ha_size = sizeof(*ha) + sizeof(ha->range);
union dm_mem_page_range *ha_region;
uint64_t align, chunk_max_size;
@@ -560,9 +559,8 @@ static void hv_balloon_hot_add_posting(HvBalloon *balloon, StateDesc *stdesc)
*/
*current_count = MIN(hot_add_range->count, chunk_max_size);
- ha = alloca(ha_size);
+ ha = g_malloc0(ha_size);
ha_region = &(&ha->range)[1];
- memset(ha, 0, ha_size);
ha->hdr.type = DM_MEM_HOT_ADD_REQUEST;
ha->hdr.size = ha_size;
ha->hdr.trans_id = balloon->trans_id;
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PULL 2/3] hv-balloon: define dm_hot_add_with_region to avoid Coverity warning
2024-03-08 17:02 [PULL 0/3] Hyper-V Dynamic Memory and VMBus misc small patches Maciej S. Szmigiero
2024-03-08 17:02 ` [PULL 1/3] hv-balloon: avoid alloca() usage Maciej S. Szmigiero
@ 2024-03-08 17:02 ` Maciej S. Szmigiero
2024-03-08 17:02 ` [PULL 3/3] vmbus: Print a warning when enabled without the recommended set of features Maciej S. Szmigiero
2024-03-09 20:11 ` [PULL 0/3] Hyper-V Dynamic Memory and VMBus misc small patches Peter Maydell
3 siblings, 0 replies; 5+ messages in thread
From: Maciej S. Szmigiero @ 2024-03-08 17:02 UTC (permalink / raw)
To: qemu-devel
From: "Maciej S. Szmigiero" <maciej.szmigiero@oracle.com>
Since the presence of a hot add memory region is optional in hot add
request message it wasn't part of this message declaration
(struct dm_hot_add).
Instead, the code allocated such enlarged message by simply adding the
necessary size for this extra field to the size of basic hot add message
struct.
However, Coverity considers accessing this extra member to be
an out-of-bounds access, even thought the memory is actually there.
Fix this by adding an extended variant of this message that explicitly has
an additional union dm_mem_page_range at its end.
CID: #1523903
Signed-off-by: Maciej S. Szmigiero <maciej.szmigiero@oracle.com>
---
hw/hyperv/hv-balloon.c | 10 +++++-----
include/hw/hyperv/dynmem-proto.h | 9 ++++++++-
2 files changed, 13 insertions(+), 6 deletions(-)
diff --git a/hw/hyperv/hv-balloon.c b/hw/hyperv/hv-balloon.c
index 35333dab2434..3a9ef0769103 100644
--- a/hw/hyperv/hv-balloon.c
+++ b/hw/hyperv/hv-balloon.c
@@ -513,8 +513,8 @@ ret_idle:
static void hv_balloon_hot_add_rb_wait(HvBalloon *balloon, StateDesc *stdesc)
{
VMBusChannel *chan = hv_balloon_get_channel(balloon);
- struct dm_hot_add *ha;
- size_t ha_size = sizeof(*ha) + sizeof(ha->range);
+ struct dm_hot_add_with_region *ha;
+ size_t ha_size = sizeof(*ha);
assert(balloon->state == S_HOT_ADD_RB_WAIT);
@@ -530,8 +530,8 @@ static void hv_balloon_hot_add_posting(HvBalloon *balloon, StateDesc *stdesc)
PageRange *hot_add_range = &balloon->hot_add_range;
uint64_t *current_count = &balloon->ha_current_count;
VMBusChannel *chan = hv_balloon_get_channel(balloon);
- g_autofree struct dm_hot_add *ha = NULL;
- size_t ha_size = sizeof(*ha) + sizeof(ha->range);
+ g_autofree struct dm_hot_add_with_region *ha = NULL;
+ size_t ha_size = sizeof(*ha);
union dm_mem_page_range *ha_region;
uint64_t align, chunk_max_size;
ssize_t ret;
@@ -560,7 +560,7 @@ static void hv_balloon_hot_add_posting(HvBalloon *balloon, StateDesc *stdesc)
*current_count = MIN(hot_add_range->count, chunk_max_size);
ha = g_malloc0(ha_size);
- ha_region = &(&ha->range)[1];
+ ha_region = &ha->region;
ha->hdr.type = DM_MEM_HOT_ADD_REQUEST;
ha->hdr.size = ha_size;
ha->hdr.trans_id = balloon->trans_id;
diff --git a/include/hw/hyperv/dynmem-proto.h b/include/hw/hyperv/dynmem-proto.h
index a657786a94b1..68b8b606f268 100644
--- a/include/hw/hyperv/dynmem-proto.h
+++ b/include/hw/hyperv/dynmem-proto.h
@@ -328,7 +328,8 @@ struct dm_unballoon_response {
/*
* Hot add request message. Message sent from the host to the guest.
*
- * mem_range: Memory range to hot add.
+ * range: Memory range to hot add.
+ * region: Explicit hot add memory region for guest to use. Optional.
*
*/
@@ -337,6 +338,12 @@ struct dm_hot_add {
union dm_mem_page_range range;
} QEMU_PACKED;
+struct dm_hot_add_with_region {
+ struct dm_header hdr;
+ union dm_mem_page_range range;
+ union dm_mem_page_range region;
+} QEMU_PACKED;
+
/*
* Hot add response message.
* This message is sent by the guest to report the status of a hot add request.
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PULL 3/3] vmbus: Print a warning when enabled without the recommended set of features
2024-03-08 17:02 [PULL 0/3] Hyper-V Dynamic Memory and VMBus misc small patches Maciej S. Szmigiero
2024-03-08 17:02 ` [PULL 1/3] hv-balloon: avoid alloca() usage Maciej S. Szmigiero
2024-03-08 17:02 ` [PULL 2/3] hv-balloon: define dm_hot_add_with_region to avoid Coverity warning Maciej S. Szmigiero
@ 2024-03-08 17:02 ` Maciej S. Szmigiero
2024-03-09 20:11 ` [PULL 0/3] Hyper-V Dynamic Memory and VMBus misc small patches Peter Maydell
3 siblings, 0 replies; 5+ messages in thread
From: Maciej S. Szmigiero @ 2024-03-08 17:02 UTC (permalink / raw)
To: qemu-devel
From: "Maciej S. Szmigiero" <maciej.szmigiero@oracle.com>
Some Windows versions crash at boot or fail to enable the VMBus device if
they don't see the expected set of Hyper-V features (enlightenments).
Since this provides poor user experience let's warn user if the VMBus
device is enabled without the recommended set of Hyper-V features.
The recommended set is the minimum set of Hyper-V features required to make
the VMBus device work properly in Windows Server versions 2016, 2019 and
2022.
Acked-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Maciej S. Szmigiero <maciej.szmigiero@oracle.com>
---
hw/hyperv/hyperv.c | 12 ++++++++++++
hw/hyperv/vmbus.c | 6 ++++++
include/hw/hyperv/hyperv.h | 4 ++++
target/i386/kvm/hyperv-stub.c | 4 ++++
target/i386/kvm/hyperv.c | 5 +++++
target/i386/kvm/hyperv.h | 2 ++
target/i386/kvm/kvm.c | 7 +++++++
7 files changed, 40 insertions(+)
diff --git a/hw/hyperv/hyperv.c b/hw/hyperv/hyperv.c
index 6c4a18dd0e2a..3ea54ba818b2 100644
--- a/hw/hyperv/hyperv.c
+++ b/hw/hyperv/hyperv.c
@@ -951,3 +951,15 @@ uint64_t hyperv_syndbg_query_options(void)
return msg.u.query_options.options;
}
+
+static bool vmbus_recommended_features_enabled;
+
+bool hyperv_are_vmbus_recommended_features_enabled(void)
+{
+ return vmbus_recommended_features_enabled;
+}
+
+void hyperv_set_vmbus_recommended_features_enabled(void)
+{
+ vmbus_recommended_features_enabled = true;
+}
diff --git a/hw/hyperv/vmbus.c b/hw/hyperv/vmbus.c
index 380239af2c7b..f33afeeea27d 100644
--- a/hw/hyperv/vmbus.c
+++ b/hw/hyperv/vmbus.c
@@ -2631,6 +2631,12 @@ static void vmbus_bridge_realize(DeviceState *dev, Error **errp)
return;
}
+ if (!hyperv_are_vmbus_recommended_features_enabled()) {
+ warn_report("VMBus enabled without the recommended set of Hyper-V features: "
+ "hv-stimer, hv-vapic and hv-runtime. "
+ "Some Windows versions might not boot or enable the VMBus device");
+ }
+
bridge->bus = VMBUS(qbus_new(TYPE_VMBUS, dev, "vmbus"));
}
diff --git a/include/hw/hyperv/hyperv.h b/include/hw/hyperv/hyperv.h
index 015c3524b1c2..d717b4e13d40 100644
--- a/include/hw/hyperv/hyperv.h
+++ b/include/hw/hyperv/hyperv.h
@@ -139,4 +139,8 @@ typedef struct HvSynDbgMsg {
} HvSynDbgMsg;
typedef uint16_t (*HvSynDbgHandler)(void *context, HvSynDbgMsg *msg);
void hyperv_set_syndbg_handler(HvSynDbgHandler handler, void *context);
+
+bool hyperv_are_vmbus_recommended_features_enabled(void);
+void hyperv_set_vmbus_recommended_features_enabled(void);
+
#endif
diff --git a/target/i386/kvm/hyperv-stub.c b/target/i386/kvm/hyperv-stub.c
index 778ed782e6fc..3263dcf05d31 100644
--- a/target/i386/kvm/hyperv-stub.c
+++ b/target/i386/kvm/hyperv-stub.c
@@ -52,3 +52,7 @@ void hyperv_x86_synic_reset(X86CPU *cpu)
void hyperv_x86_synic_update(X86CPU *cpu)
{
}
+
+void hyperv_x86_set_vmbus_recommended_features_enabled(void)
+{
+}
diff --git a/target/i386/kvm/hyperv.c b/target/i386/kvm/hyperv.c
index 6825c89af374..f2a3fe650a18 100644
--- a/target/i386/kvm/hyperv.c
+++ b/target/i386/kvm/hyperv.c
@@ -149,3 +149,8 @@ int kvm_hv_handle_exit(X86CPU *cpu, struct kvm_hyperv_exit *exit)
return -1;
}
}
+
+void hyperv_x86_set_vmbus_recommended_features_enabled(void)
+{
+ hyperv_set_vmbus_recommended_features_enabled();
+}
diff --git a/target/i386/kvm/hyperv.h b/target/i386/kvm/hyperv.h
index 67543296c3a4..e3982c8f4dd1 100644
--- a/target/i386/kvm/hyperv.h
+++ b/target/i386/kvm/hyperv.h
@@ -26,4 +26,6 @@ int hyperv_x86_synic_add(X86CPU *cpu);
void hyperv_x86_synic_reset(X86CPU *cpu);
void hyperv_x86_synic_update(X86CPU *cpu);
+void hyperv_x86_set_vmbus_recommended_features_enabled(void);
+
#endif
diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index 42970ab046fa..e68cbe929302 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -1650,6 +1650,13 @@ static int hyperv_init_vcpu(X86CPU *cpu)
}
}
+ /* Skip SynIC and VP_INDEX since they are hard deps already */
+ if (hyperv_feat_enabled(cpu, HYPERV_FEAT_STIMER) &&
+ hyperv_feat_enabled(cpu, HYPERV_FEAT_VAPIC) &&
+ hyperv_feat_enabled(cpu, HYPERV_FEAT_RUNTIME)) {
+ hyperv_x86_set_vmbus_recommended_features_enabled();
+ }
+
return 0;
}
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PULL 0/3] Hyper-V Dynamic Memory and VMBus misc small patches
2024-03-08 17:02 [PULL 0/3] Hyper-V Dynamic Memory and VMBus misc small patches Maciej S. Szmigiero
` (2 preceding siblings ...)
2024-03-08 17:02 ` [PULL 3/3] vmbus: Print a warning when enabled without the recommended set of features Maciej S. Szmigiero
@ 2024-03-09 20:11 ` Peter Maydell
3 siblings, 0 replies; 5+ messages in thread
From: Peter Maydell @ 2024-03-09 20:11 UTC (permalink / raw)
To: Maciej S. Szmigiero; +Cc: qemu-devel
On Fri, 8 Mar 2024 at 17:03, Maciej S. Szmigiero
<mail@maciej.szmigiero.name> wrote:
>
> From: "Maciej S. Szmigiero" <maciej.szmigiero@oracle.com>
>
> The following changes since commit 8f6330a807f2642dc2a3cdf33347aa28a4c00a87:
>
> Merge tag 'pull-maintainer-updates-060324-1' of https://gitlab.com/stsquad/qemu into staging (2024-03-06 16:56:20 +0000)
>
> are available in the Git repository at:
>
> https://github.com/maciejsszmigiero/qemu.git tags/pull-hv-balloon-20240308
>
> for you to fetch changes up to 6093637b4d32875f98cd59696ffc5f26884aa0b4:
>
> vmbus: Print a warning when enabled without the recommended set of features (2024-03-08 14:18:56 +0100)
>
> ----------------------------------------------------------------
> Hyper-V Dynamic Memory and VMBus misc small patches
>
> This pull request contains two small patches to hv-balloon:
> the first one replacing alloca() usage with g_malloc0() + g_autofree
> and the second one adding additional declaration of a protocol message
> struct with an optional field explicitly defined to avoid a Coverity
> warning.
>
> Also included is a VMBus patch to print a warning when it is enabled
> without the recommended set of Hyper-V features (enlightenments) since
> some Windows versions crash at boot in this case.
>
> ----------------------------------------------------------------
> Maciej S. Szmigiero (3):
> hv-balloon: avoid alloca() usage
> hv-balloon: define dm_hot_add_with_region to avoid Coverity warning
> vmbus: Print a warning when enabled without the recommended set of features
>
Applied, thanks.
Please update the changelog at https://wiki.qemu.org/ChangeLog/9.0
for any user-visible changes.
-- PMM
^ permalink raw reply [flat|nested] 5+ messages in thread