* [PATCH v1 0/4] HyperV: Synthetic Debugging device
@ 2022-02-04 10:07 Jon Doron
2022-02-04 10:07 ` [PATCH v1 1/4] hyperv: SControl is optional to enable SynIc Jon Doron
` (4 more replies)
0 siblings, 5 replies; 14+ messages in thread
From: Jon Doron @ 2022-02-04 10:07 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, vkuznets, Jon Doron
This patchset adds support for the synthetic debugging device.
HyperV supports a special transport layer for the kernel debugger when
running in HyperV.
This patchset add supports for this device so you could have a setup
fast windows kernel debugging.
At this point of time, DHCP is not implmeneted so to set this up few
things need to be noted.
The scenario I used to test is having 2 VMs in the same virtual network
i.e a Debugger VM with the NIC:
-nic tap,model=virtio,mac=02:ca:01:01:01:01,script=/etc/qemu-ifup
And it's IP is going to be static 192.168.53.12
And the VM we want to debug, to which we need to have the englightments
and vmbus configured:
-cpu host,hv-relaxed,hv_spinlocks=0x1fff,hv_time,+vmx,invtsc,hv-vapic,hv-vpindex,hv-synic,hv-syndbg \
-device vmbus-bridge \
-device hv-syndbg,host_ip=192.168.53.12,host_port=50000,use_hcalls=false \
-nic tap,model=virtio,mac=02:ca:01:01:01:02,script=/etc/qemu-ifup \
Then in the debuggee VM we would setup the kernel debugging in the
following way:
If the VM is older than Win8:
* Copy the proper platform kdvm.dll (make sure it's called kdvm.dll even if platform is 32bit)
bcdedit /set {GUID} dbgtransport kdvm.dll
bcdedit /set {GUID} loadoptions host_ip="1.2.3.4",host_port="50000",nodhcp
bcdedit /set {GUID} debug on
bcdedit /set {GUID} halbreakpoint on
Win8 and late:
bcdedit /dbgsettings net hostip:7.7.7.7 port:50000 nodhcp
This is all the setup that is required to get the synthetic debugger
configured correctly.
Jon Doron (4):
hyperv: SControl is optional to enable SynIc
hyperv: Add definitions for syndbg
hyperv: Add support to process syndbg commands
hw: hyperv: Initial commit for Synthetic Debugging device
docs/hyperv.txt | 15 +
hw/hyperv/Kconfig | 5 +
hw/hyperv/hyperv.c | 475 +++++++++++++++++++++++++------
hw/hyperv/meson.build | 1 +
hw/hyperv/syndbg.c | 407 ++++++++++++++++++++++++++
include/hw/hyperv/hyperv-proto.h | 52 ++++
include/hw/hyperv/hyperv.h | 60 ++++
target/i386/cpu.c | 2 +
target/i386/cpu.h | 7 +
target/i386/kvm/hyperv-proto.h | 37 +++
target/i386/kvm/hyperv-stub.c | 6 +
target/i386/kvm/hyperv.c | 52 +++-
target/i386/kvm/kvm.c | 76 ++++-
13 files changed, 1105 insertions(+), 90 deletions(-)
create mode 100644 hw/hyperv/syndbg.c
--
2.34.1
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v1 1/4] hyperv: SControl is optional to enable SynIc
2022-02-04 10:07 [PATCH v1 0/4] HyperV: Synthetic Debugging device Jon Doron
@ 2022-02-04 10:07 ` Jon Doron
2022-02-16 9:10 ` Emanuele Giuseppe Esposito
2022-02-04 10:07 ` [PATCH v1 2/4] hyperv: Add definitions for syndbg Jon Doron
` (3 subsequent siblings)
4 siblings, 1 reply; 14+ messages in thread
From: Jon Doron @ 2022-02-04 10:07 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, vkuznets, Jon Doron
SynIc can be enabled regardless of the SControl mechanisim which can
register a GSI for a given SintRoute.
This behaviour can achived by setting enabling SIMP and then the guest
will poll on the message slot.
Once there is another message pending the host will set the message slot
with the pending flag.
When the guest polls from the message slot, incase the pending flag is
set it will write to the HV_X64_MSR_EOM indicating it has cleared the
slow and we can try and push our message again.
Signed-off-by: Jon Doron <arilou@gmail.com>
---
hw/hyperv/hyperv.c | 233 ++++++++++++++++++++++++-------------
include/hw/hyperv/hyperv.h | 2 +
2 files changed, 153 insertions(+), 82 deletions(-)
diff --git a/hw/hyperv/hyperv.c b/hw/hyperv/hyperv.c
index cb1074f234..88c9cc1334 100644
--- a/hw/hyperv/hyperv.c
+++ b/hw/hyperv/hyperv.c
@@ -27,18 +27,70 @@ struct SynICState {
CPUState *cs;
- bool enabled;
+ bool sctl_enabled;
hwaddr msg_page_addr;
hwaddr event_page_addr;
MemoryRegion msg_page_mr;
MemoryRegion event_page_mr;
struct hyperv_message_page *msg_page;
struct hyperv_event_flags_page *event_page;
+
+ QemuMutex sint_routes_mutex;
+ QLIST_HEAD(, HvSintRoute) sint_routes;
};
#define TYPE_SYNIC "hyperv-synic"
OBJECT_DECLARE_SIMPLE_TYPE(SynICState, SYNIC)
+/*
+ * KVM has its own message producers (SynIC timers). To guarantee
+ * serialization with both KVM vcpu and the guest cpu, the messages are first
+ * staged in an intermediate area and then posted to the SynIC message page in
+ * the vcpu thread.
+ */
+typedef struct HvSintStagedMessage {
+ /* message content staged by hyperv_post_msg */
+ struct hyperv_message msg;
+ /* callback + data (r/o) to complete the processing in a BH */
+ HvSintMsgCb cb;
+ void *cb_data;
+ /* message posting status filled by cpu_post_msg */
+ int status;
+ /* passing the buck: */
+ enum {
+ /* initial state */
+ HV_STAGED_MSG_FREE,
+ /*
+ * hyperv_post_msg (e.g. in main loop) grabs the staged area (FREE ->
+ * BUSY), copies msg, and schedules cpu_post_msg on the assigned cpu
+ */
+ HV_STAGED_MSG_BUSY,
+ /*
+ * cpu_post_msg (vcpu thread) tries to copy staged msg to msg slot,
+ * notify the guest, records the status, marks the posting done (BUSY
+ * -> POSTED), and schedules sint_msg_bh BH
+ */
+ HV_STAGED_MSG_POSTED,
+ /*
+ * sint_msg_bh (BH) verifies that the posting is done, runs the
+ * callback, and starts over (POSTED -> FREE)
+ */
+ } state;
+} HvSintStagedMessage;
+
+struct HvSintRoute {
+ uint32_t sint;
+ SynICState *synic;
+ int gsi;
+ EventNotifier sint_set_notifier;
+ EventNotifier sint_ack_notifier;
+
+ HvSintStagedMessage *staged_msg;
+
+ unsigned refcount;
+ QLIST_ENTRY(HvSintRoute) link;
+};
+
static bool synic_enabled;
bool hyperv_is_synic_enabled(void)
@@ -51,11 +103,11 @@ static SynICState *get_synic(CPUState *cs)
return SYNIC(object_resolve_path_component(OBJECT(cs), "synic"));
}
-static void synic_update(SynICState *synic, bool enable,
+static void synic_update(SynICState *synic, bool sctl_enable,
hwaddr msg_page_addr, hwaddr event_page_addr)
{
- synic->enabled = enable;
+ synic->sctl_enabled = sctl_enable;
if (synic->msg_page_addr != msg_page_addr) {
if (synic->msg_page_addr) {
memory_region_del_subregion(get_system_memory(),
@@ -80,7 +132,7 @@ static void synic_update(SynICState *synic, bool enable,
}
}
-void hyperv_synic_update(CPUState *cs, bool enable,
+void hyperv_synic_update(CPUState *cs, bool sctl_enable,
hwaddr msg_page_addr, hwaddr event_page_addr)
{
SynICState *synic = get_synic(cs);
@@ -89,7 +141,7 @@ void hyperv_synic_update(CPUState *cs, bool enable,
return;
}
- synic_update(synic, enable, msg_page_addr, event_page_addr);
+ synic_update(synic, sctl_enable, msg_page_addr, event_page_addr);
}
static void synic_realize(DeviceState *dev, Error **errp)
@@ -110,16 +162,20 @@ static void synic_realize(DeviceState *dev, Error **errp)
sizeof(*synic->event_page), &error_abort);
synic->msg_page = memory_region_get_ram_ptr(&synic->msg_page_mr);
synic->event_page = memory_region_get_ram_ptr(&synic->event_page_mr);
+ qemu_mutex_init(&synic->sint_routes_mutex);
+ QLIST_INIT(&synic->sint_routes);
g_free(msgp_name);
g_free(eventp_name);
}
+
static void synic_reset(DeviceState *dev)
{
SynICState *synic = SYNIC(dev);
memset(synic->msg_page, 0, sizeof(*synic->msg_page));
memset(synic->event_page, 0, sizeof(*synic->event_page));
synic_update(synic, false, 0, 0);
+ assert(QLIST_EMPTY(&synic->sint_routes));
}
static void synic_class_init(ObjectClass *klass, void *data)
@@ -168,54 +224,6 @@ static void synic_register_types(void)
type_init(synic_register_types)
-/*
- * KVM has its own message producers (SynIC timers). To guarantee
- * serialization with both KVM vcpu and the guest cpu, the messages are first
- * staged in an intermediate area and then posted to the SynIC message page in
- * the vcpu thread.
- */
-typedef struct HvSintStagedMessage {
- /* message content staged by hyperv_post_msg */
- struct hyperv_message msg;
- /* callback + data (r/o) to complete the processing in a BH */
- HvSintMsgCb cb;
- void *cb_data;
- /* message posting status filled by cpu_post_msg */
- int status;
- /* passing the buck: */
- enum {
- /* initial state */
- HV_STAGED_MSG_FREE,
- /*
- * hyperv_post_msg (e.g. in main loop) grabs the staged area (FREE ->
- * BUSY), copies msg, and schedules cpu_post_msg on the assigned cpu
- */
- HV_STAGED_MSG_BUSY,
- /*
- * cpu_post_msg (vcpu thread) tries to copy staged msg to msg slot,
- * notify the guest, records the status, marks the posting done (BUSY
- * -> POSTED), and schedules sint_msg_bh BH
- */
- HV_STAGED_MSG_POSTED,
- /*
- * sint_msg_bh (BH) verifies that the posting is done, runs the
- * callback, and starts over (POSTED -> FREE)
- */
- } state;
-} HvSintStagedMessage;
-
-struct HvSintRoute {
- uint32_t sint;
- SynICState *synic;
- int gsi;
- EventNotifier sint_set_notifier;
- EventNotifier sint_ack_notifier;
-
- HvSintStagedMessage *staged_msg;
-
- unsigned refcount;
-};
-
static CPUState *hyperv_find_vcpu(uint32_t vp_index)
{
CPUState *cs = qemu_get_cpu(vp_index);
@@ -259,7 +267,7 @@ static void cpu_post_msg(CPUState *cs, run_on_cpu_data data)
assert(staged_msg->state == HV_STAGED_MSG_BUSY);
- if (!synic->enabled || !synic->msg_page_addr) {
+ if (!synic->msg_page_addr) {
staged_msg->status = -ENXIO;
goto posted;
}
@@ -343,7 +351,7 @@ int hyperv_set_event_flag(HvSintRoute *sint_route, unsigned eventno)
if (eventno > HV_EVENT_FLAGS_COUNT) {
return -EINVAL;
}
- if (!synic->enabled || !synic->event_page_addr) {
+ if (!synic->sctl_enabled || !synic->event_page_addr) {
return -ENXIO;
}
@@ -364,11 +372,13 @@ int hyperv_set_event_flag(HvSintRoute *sint_route, unsigned eventno)
HvSintRoute *hyperv_sint_route_new(uint32_t vp_index, uint32_t sint,
HvSintMsgCb cb, void *cb_data)
{
- HvSintRoute *sint_route;
- EventNotifier *ack_notifier;
+ HvSintRoute *sint_route = NULL;
+ EventNotifier *ack_notifier = NULL;
int r, gsi;
CPUState *cs;
SynICState *synic;
+ bool ack_event_initialized = false, sint_notifier_initialized = false,
+ irqfd_initialized = false;
cs = hyperv_find_vcpu(vp_index);
if (!cs) {
@@ -381,57 +391,82 @@ HvSintRoute *hyperv_sint_route_new(uint32_t vp_index, uint32_t sint,
}
sint_route = g_new0(HvSintRoute, 1);
- r = event_notifier_init(&sint_route->sint_set_notifier, false);
- if (r) {
- goto err;
+ if (!sint_route) {
+ goto cleanup_err;
}
+ sint_route->gsi = 0;
+ sint_route->synic = synic;
+ sint_route->sint = sint;
+ sint_route->refcount = 1;
ack_notifier = cb ? &sint_route->sint_ack_notifier : NULL;
if (ack_notifier) {
sint_route->staged_msg = g_new0(HvSintStagedMessage, 1);
+ if (!sint_route->staged_msg) {
+ goto cleanup_err;
+ }
sint_route->staged_msg->cb = cb;
sint_route->staged_msg->cb_data = cb_data;
r = event_notifier_init(ack_notifier, false);
if (r) {
- goto err_sint_set_notifier;
+ goto cleanup_err;
}
-
event_notifier_set_handler(ack_notifier, sint_ack_handler);
+ ack_event_initialized = true;
+ }
+
+ /* See if we are done or we need to setup a GSI for this SintRoute */
+ if (!synic->sctl_enabled) {
+ goto cleanup;
}
+ /* We need to setup a GSI for this SintRoute */
+ r = event_notifier_init(&sint_route->sint_set_notifier, false);
+ if (r) {
+ goto cleanup_err;
+ }
+ sint_notifier_initialized = true;
+
gsi = kvm_irqchip_add_hv_sint_route(kvm_state, vp_index, sint);
if (gsi < 0) {
- goto err_gsi;
+ goto cleanup_err;
}
+ irqfd_initialized = true;
r = kvm_irqchip_add_irqfd_notifier_gsi(kvm_state,
&sint_route->sint_set_notifier,
ack_notifier, gsi);
if (r) {
- goto err_irqfd;
+ goto cleanup_err;
}
sint_route->gsi = gsi;
- sint_route->synic = synic;
- sint_route->sint = sint;
- sint_route->refcount = 1;
-
+cleanup:
+ qemu_mutex_lock(&synic->sint_routes_mutex);
+ QLIST_INSERT_HEAD_RCU(&synic->sint_routes, sint_route, link);
+ qemu_mutex_unlock(&synic->sint_routes_mutex);
return sint_route;
-err_irqfd:
- kvm_irqchip_release_virq(kvm_state, gsi);
-err_gsi:
+cleanup_err:
+ if (irqfd_initialized) {
+ kvm_irqchip_release_virq(kvm_state, gsi);
+ }
+
+ if (sint_notifier_initialized) {
+ event_notifier_cleanup(&sint_route->sint_set_notifier);
+ }
+
if (ack_notifier) {
- event_notifier_set_handler(ack_notifier, NULL);
- event_notifier_cleanup(ack_notifier);
+ if (ack_event_initialized) {
+ event_notifier_set_handler(ack_notifier, NULL);
+ event_notifier_cleanup(ack_notifier);
+ }
+
g_free(sint_route->staged_msg);
}
-err_sint_set_notifier:
- event_notifier_cleanup(&sint_route->sint_set_notifier);
-err:
- g_free(sint_route);
+ g_free(sint_route);
return NULL;
}
@@ -442,6 +477,8 @@ void hyperv_sint_route_ref(HvSintRoute *sint_route)
void hyperv_sint_route_unref(HvSintRoute *sint_route)
{
+ SynICState *synic;
+
if (!sint_route) {
return;
}
@@ -452,21 +489,33 @@ void hyperv_sint_route_unref(HvSintRoute *sint_route)
return;
}
- kvm_irqchip_remove_irqfd_notifier_gsi(kvm_state,
- &sint_route->sint_set_notifier,
- sint_route->gsi);
- kvm_irqchip_release_virq(kvm_state, sint_route->gsi);
+ synic = sint_route->synic;
+ qemu_mutex_lock(&synic->sint_routes_mutex);
+ QLIST_REMOVE_RCU(sint_route, link);
+ qemu_mutex_unlock(&synic->sint_routes_mutex);
+
+ if (sint_route->gsi) {
+ kvm_irqchip_remove_irqfd_notifier_gsi(kvm_state,
+ &sint_route->sint_set_notifier,
+ sint_route->gsi);
+ kvm_irqchip_release_virq(kvm_state, sint_route->gsi);
+ event_notifier_cleanup(&sint_route->sint_set_notifier);
+ }
+
if (sint_route->staged_msg) {
event_notifier_set_handler(&sint_route->sint_ack_notifier, NULL);
event_notifier_cleanup(&sint_route->sint_ack_notifier);
g_free(sint_route->staged_msg);
}
- event_notifier_cleanup(&sint_route->sint_set_notifier);
g_free(sint_route);
}
int hyperv_sint_route_set_sint(HvSintRoute *sint_route)
{
+ if (!sint_route->gsi) {
+ return 0;
+ }
+
return event_notifier_set(&sint_route->sint_set_notifier);
}
@@ -529,6 +578,26 @@ int hyperv_set_msg_handler(uint32_t conn_id, HvMsgHandler handler, void *data)
return ret;
}
+int hyperv_synic_eom(CPUState *cs)
+{
+ SynICState *synic = get_synic(cs);
+ HvSintRoute *sint_route;
+
+ if (!synic) {
+ return -1;
+ }
+
+ qemu_mutex_lock(&synic->sint_routes_mutex);
+ QLIST_FOREACH(sint_route, &synic->sint_routes, link) {
+ /* Try to complete every SintRoute */
+ aio_bh_schedule_oneshot(qemu_get_aio_context(), sint_msg_bh,
+ sint_route);
+ }
+ qemu_mutex_unlock(&synic->sint_routes_mutex);
+
+ return 0;
+}
+
uint16_t hyperv_hcall_post_message(uint64_t param, bool fast)
{
uint16_t ret;
diff --git a/include/hw/hyperv/hyperv.h b/include/hw/hyperv/hyperv.h
index a63ee0003c..ef9f6b6c09 100644
--- a/include/hw/hyperv/hyperv.h
+++ b/include/hw/hyperv/hyperv.h
@@ -28,6 +28,8 @@ void hyperv_sint_route_unref(HvSintRoute *sint_route);
int hyperv_sint_route_set_sint(HvSintRoute *sint_route);
+int hyperv_synic_eom(CPUState *cs);
+
/*
* Submit a message to be posted in vcpu context. If the submission succeeds,
* the status of posting the message is reported via the callback associated
--
2.34.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v1 2/4] hyperv: Add definitions for syndbg
2022-02-04 10:07 [PATCH v1 0/4] HyperV: Synthetic Debugging device Jon Doron
2022-02-04 10:07 ` [PATCH v1 1/4] hyperv: SControl is optional to enable SynIc Jon Doron
@ 2022-02-04 10:07 ` Jon Doron
2022-02-16 9:10 ` Emanuele Giuseppe Esposito
2022-02-04 10:07 ` [PATCH v1 3/4] hyperv: Add support to process syndbg commands Jon Doron
` (2 subsequent siblings)
4 siblings, 1 reply; 14+ messages in thread
From: Jon Doron @ 2022-02-04 10:07 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, vkuznets, Jon Doron
Add all required definitions for hyperv synthetic debugger interface.
Signed-off-by: Jon Doron <arilou@gmail.com>
---
include/hw/hyperv/hyperv-proto.h | 52 ++++++++++++++++++++++++++++++++
target/i386/kvm/hyperv-proto.h | 37 +++++++++++++++++++++++
2 files changed, 89 insertions(+)
diff --git a/include/hw/hyperv/hyperv-proto.h b/include/hw/hyperv/hyperv-proto.h
index 21dc28aee9..94c9658eb0 100644
--- a/include/hw/hyperv/hyperv-proto.h
+++ b/include/hw/hyperv/hyperv-proto.h
@@ -24,12 +24,17 @@
#define HV_STATUS_INVALID_PORT_ID 17
#define HV_STATUS_INVALID_CONNECTION_ID 18
#define HV_STATUS_INSUFFICIENT_BUFFERS 19
+#define HV_STATUS_NOT_ACKNOWLEDGED 20
+#define HV_STATUS_NO_DATA 27
/*
* Hypercall numbers
*/
#define HV_POST_MESSAGE 0x005c
#define HV_SIGNAL_EVENT 0x005d
+#define HV_POST_DEBUG_DATA 0x0069
+#define HV_RETREIVE_DEBUG_DATA 0x006a
+#define HV_RESET_DEBUG_SESSION 0x006b
#define HV_HYPERCALL_FAST (1u << 16)
/*
@@ -127,4 +132,51 @@ struct hyperv_event_flags_page {
struct hyperv_event_flags slot[HV_SINT_COUNT];
};
+/*
+ * Kernel debugger structures
+ */
+
+/* Options flags for hyperv_reset_debug_session */
+#define HV_DEBUG_PURGE_INCOMING_DATA 0x00000001
+#define HV_DEBUG_PURGE_OUTGOING_DATA 0x00000002
+struct hyperv_reset_debug_session_input {
+ uint32_t options;
+} __attribute__ ((__packed__));
+
+struct hyperv_reset_debug_session_output {
+ uint32_t host_ip;
+ uint32_t target_ip;
+ uint16_t host_port;
+ uint16_t target_port;
+ uint8_t host_mac[6];
+ uint8_t target_mac[6];
+} __attribute__ ((__packed__));
+
+/* Options for hyperv_post_debug_data */
+#define HV_DEBUG_POST_LOOP 0x00000001
+
+struct hyperv_post_debug_data_input {
+ uint32_t count;
+ uint32_t options;
+ /*uint8_t data[HV_HYP_PAGE_SIZE - 2 * sizeof(uint32_t)];*/
+} __attribute__ ((__packed__));
+
+struct hyperv_post_debug_data_output {
+ uint32_t pending_count;
+} __attribute__ ((__packed__));
+
+/* Options for hyperv_retrieve_debug_data */
+#define HV_DEBUG_RETRIEVE_LOOP 0x00000001
+#define HV_DEBUG_RETRIEVE_TEST_ACTIVITY 0x00000002
+
+struct hyperv_retrieve_debug_data_input {
+ uint32_t count;
+ uint32_t options;
+ uint64_t timeout;
+} __attribute__ ((__packed__));
+
+struct hyperv_retrieve_debug_data_output {
+ uint32_t retrieved_count;
+ uint32_t remaining_count;
+} __attribute__ ((__packed__));
#endif
diff --git a/target/i386/kvm/hyperv-proto.h b/target/i386/kvm/hyperv-proto.h
index 89f81afda7..9480bcdf04 100644
--- a/target/i386/kvm/hyperv-proto.h
+++ b/target/i386/kvm/hyperv-proto.h
@@ -19,6 +19,9 @@
#define HV_CPUID_ENLIGHTMENT_INFO 0x40000004
#define HV_CPUID_IMPLEMENT_LIMITS 0x40000005
#define HV_CPUID_NESTED_FEATURES 0x4000000A
+#define HV_CPUID_SYNDBG_VENDOR_AND_MAX_FUNCTIONS 0x40000080
+#define HV_CPUID_SYNDBG_INTERFACE 0x40000081
+#define HV_CPUID_SYNDBG_PLATFORM_CAPABILITIES 0x40000082
#define HV_CPUID_MIN 0x40000005
#define HV_CPUID_MAX 0x4000ffff
#define HV_HYPERVISOR_PRESENT_BIT 0x80000000
@@ -55,8 +58,14 @@
#define HV_GUEST_IDLE_STATE_AVAILABLE (1u << 5)
#define HV_FREQUENCY_MSRS_AVAILABLE (1u << 8)
#define HV_GUEST_CRASH_MSR_AVAILABLE (1u << 10)
+#define HV_FEATURE_DEBUG_MSRS_AVAILABLE (1u << 11)
#define HV_STIMER_DIRECT_MODE_AVAILABLE (1u << 19)
+/*
+ * HV_CPUID_FEATURES.EBX bits
+ */
+#define HV_PARTITION_DEUBGGING_ALLOWED (1u << 12)
+
/*
* HV_CPUID_ENLIGHTMENT_INFO.EAX bits
*/
@@ -72,6 +81,11 @@
#define HV_ENLIGHTENED_VMCS_RECOMMENDED (1u << 14)
#define HV_NO_NONARCH_CORESHARING (1u << 18)
+/*
+ * HV_CPUID_SYNDBG_PLATFORM_CAPABILITIES.EAX bits
+ */
+#define HV_SYNDBG_CAP_ALLOW_KERNEL_DEBUGGING (1u << 1)
+
/*
* Basic virtualized MSRs
*/
@@ -130,6 +144,18 @@
#define HV_X64_MSR_STIMER3_CONFIG 0x400000B6
#define HV_X64_MSR_STIMER3_COUNT 0x400000B7
+/*
+ * Hyper-V Synthetic debug options MSR
+ */
+#define HV_X64_MSR_SYNDBG_CONTROL 0x400000F1
+#define HV_X64_MSR_SYNDBG_STATUS 0x400000F2
+#define HV_X64_MSR_SYNDBG_SEND_BUFFER 0x400000F3
+#define HV_X64_MSR_SYNDBG_RECV_BUFFER 0x400000F4
+#define HV_X64_MSR_SYNDBG_PENDING_BUFFER 0x400000F5
+#define HV_X64_MSR_SYNDBG_OPTIONS 0x400000FF
+
+#define HV_X64_SYNDBG_OPTION_USE_HCALLS BIT(2)
+
/*
* Guest crash notification MSRs
*/
@@ -168,5 +194,16 @@
#define HV_STIMER_COUNT 4
+/*
+ * Synthetic debugger control definitions
+ */
+#define HV_SYNDBG_CONTROL_SEND (1u << 0)
+#define HV_SYNDBG_CONTROL_RECV (1u << 1)
+#define HV_SYNDBG_CONTROL_SEND_SIZE(ctl) ((ctl >> 16) & 0xffff)
+#define HV_SYNDBG_STATUS_INVALID (0)
+#define HV_SYNDBG_STATUS_SEND_SUCCESS (1u << 0)
+#define HV_SYNDBG_STATUS_RECV_SUCCESS (1u << 2)
+#define HV_SYNDBG_STATUS_RESET (1u << 3)
+#define HV_SYNDBG_STATUS_SET_SIZE(st, sz) (st | (sz << 16))
#endif
--
2.34.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v1 3/4] hyperv: Add support to process syndbg commands
2022-02-04 10:07 [PATCH v1 0/4] HyperV: Synthetic Debugging device Jon Doron
2022-02-04 10:07 ` [PATCH v1 1/4] hyperv: SControl is optional to enable SynIc Jon Doron
2022-02-04 10:07 ` [PATCH v1 2/4] hyperv: Add definitions for syndbg Jon Doron
@ 2022-02-04 10:07 ` Jon Doron
2022-02-16 9:10 ` Emanuele Giuseppe Esposito
2022-02-04 10:07 ` [PATCH v1 4/4] hw: hyperv: Initial commit for Synthetic Debugging device Jon Doron
2022-02-13 7:49 ` [PATCH v1 0/4] HyperV: " Jon Doron
4 siblings, 1 reply; 14+ messages in thread
From: Jon Doron @ 2022-02-04 10:07 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, vkuznets, Jon Doron
SynDbg commands can come from two different flows:
1. Hypercalls, in this mode the data being sent is fully
encapsulated network packets.
2. SynDbg specific MSRs, in this mode only the data that needs to be
transfered is passed.
Signed-off-by: Jon Doron <arilou@gmail.com>
---
docs/hyperv.txt | 15 +++
hw/hyperv/hyperv.c | 242 ++++++++++++++++++++++++++++++++++
include/hw/hyperv/hyperv.h | 58 ++++++++
target/i386/cpu.c | 2 +
target/i386/cpu.h | 7 +
target/i386/kvm/hyperv-stub.c | 6 +
target/i386/kvm/hyperv.c | 52 +++++++-
target/i386/kvm/kvm.c | 76 ++++++++++-
8 files changed, 450 insertions(+), 8 deletions(-)
diff --git a/docs/hyperv.txt b/docs/hyperv.txt
index 0417c183a3..7abc1b2d89 100644
--- a/docs/hyperv.txt
+++ b/docs/hyperv.txt
@@ -225,6 +225,21 @@ default (WS2016).
Note: hv-version-id-* are not enlightenments and thus don't enable Hyper-V
identification when specified without any other enlightenments.
+3.21. hv-syndbg
+===============
+Enables Hyper-V synthetic debugger interface, this is a special interface used
+by Windows Kernel debugger to send the packets through, rather than sending
+them via serial/network .
+Whe enabled, this enlightenment provides additional communication facilities
+to the guest: SynDbg messages.
+This new communication is used by Windows Kernel debugger rather than sending
+packets via serial/network, adding significant performance boost over the other
+comm channels.
+This enlightenment requires a VMBus device (-device vmbus-bridge,irq=15)
+and the follow enlightenments to work:
+hv-relaxed,hv_time,hv-vapic,hv-vpindex,hv-synic,hv-runtime,hv-stimer
+
+
4. Supplementary features
=========================
diff --git a/hw/hyperv/hyperv.c b/hw/hyperv/hyperv.c
index 88c9cc1334..c86e2aa02e 100644
--- a/hw/hyperv/hyperv.c
+++ b/hw/hyperv/hyperv.c
@@ -730,3 +730,245 @@ uint16_t hyperv_hcall_signal_event(uint64_t param, bool fast)
}
return HV_STATUS_INVALID_CONNECTION_ID;
}
+
+static HvSynDbgHandler hv_syndbg_handler;
+static void *hv_syndbg_context;
+void hyperv_set_syndbg_handler(HvSynDbgHandler handler, void *context)
+{
+ assert(!hv_syndbg_handler);
+ hv_syndbg_handler = handler;
+ hv_syndbg_context = context;
+}
+
+uint16_t hyperv_hcall_reset_dbg_session(uint64_t outgpa)
+{
+ uint16_t ret;
+ HvSynDbgMsg msg;
+ struct hyperv_reset_debug_session_output *reset_dbg_session = NULL;
+ hwaddr len;
+
+ if (!hv_syndbg_handler) {
+ ret = HV_STATUS_INVALID_HYPERCALL_CODE;
+ goto cleanup;
+ }
+
+ len = sizeof(*reset_dbg_session);
+ reset_dbg_session = cpu_physical_memory_map(outgpa, &len, 1);
+ if (!reset_dbg_session || len < sizeof(*reset_dbg_session)) {
+ ret = HV_STATUS_INSUFFICIENT_MEMORY;
+ goto cleanup;
+ }
+
+ msg.type = HV_SYNDBG_MSG_CONNECTION_INFO;
+ ret = hv_syndbg_handler(hv_syndbg_context, &msg);
+ if (ret) {
+ goto cleanup;
+ }
+
+ reset_dbg_session->host_ip = msg.u.connection_info.host_ip;
+ reset_dbg_session->host_port = msg.u.connection_info.host_port;
+ /* The following fields are only used as validation for KDVM */
+ memset(&reset_dbg_session->host_mac, 0,
+ sizeof(reset_dbg_session->host_mac));
+ reset_dbg_session->target_ip = msg.u.connection_info.host_ip;
+ reset_dbg_session->target_port = msg.u.connection_info.host_port;
+ memset(&reset_dbg_session->target_mac, 0,
+ sizeof(reset_dbg_session->target_mac));
+cleanup:
+ if (reset_dbg_session) {
+ cpu_physical_memory_unmap(reset_dbg_session,
+ sizeof(*reset_dbg_session), 1, len);
+ }
+
+ return ret;
+}
+
+uint16_t hyperv_hcall_retreive_dbg_data(uint64_t ingpa, uint64_t outgpa,
+ bool fast)
+{
+ uint16_t ret;
+ struct hyperv_retrieve_debug_data_input *debug_data_in = NULL;
+ struct hyperv_retrieve_debug_data_output *debug_data_out = NULL;
+ hwaddr in_len, out_len;
+ HvSynDbgMsg msg;
+
+ if (fast || !hv_syndbg_handler) {
+ ret = HV_STATUS_INVALID_HYPERCALL_CODE;
+ goto cleanup;
+ }
+
+ in_len = sizeof(*debug_data_in);
+ debug_data_in = cpu_physical_memory_map(ingpa, &in_len, 0);
+ if (!debug_data_in || in_len < sizeof(*debug_data_in)) {
+ ret = HV_STATUS_INSUFFICIENT_MEMORY;
+ goto cleanup;
+ }
+
+ out_len = sizeof(*debug_data_out);
+ debug_data_out = cpu_physical_memory_map(outgpa, &out_len, 1);
+ if (!debug_data_out || out_len < sizeof(*debug_data_out)) {
+ ret = HV_STATUS_INSUFFICIENT_MEMORY;
+ goto cleanup;
+ }
+
+ msg.type = HV_SYNDBG_MSG_RECV;
+ msg.u.recv.buf_gpa = outgpa + sizeof(*debug_data_out);
+ msg.u.recv.count = TARGET_PAGE_SIZE - sizeof(*debug_data_out);
+ msg.u.recv.options = debug_data_in->options;
+ msg.u.recv.timeout = debug_data_in->timeout;
+ msg.u.recv.is_raw = true;
+ ret = hv_syndbg_handler(hv_syndbg_context, &msg);
+ if (ret == HV_STATUS_NO_DATA) {
+ debug_data_out->retrieved_count = 0;
+ debug_data_out->remaining_count = debug_data_in->count;
+ goto cleanup;
+ } else if (ret != HV_STATUS_SUCCESS) {
+ goto cleanup;
+ }
+
+ debug_data_out->retrieved_count = msg.u.recv.retrieved_count;
+ debug_data_out->remaining_count =
+ debug_data_in->count - msg.u.recv.retrieved_count;
+cleanup:
+ if (debug_data_out) {
+ cpu_physical_memory_unmap(debug_data_out, sizeof(*debug_data_out), 1,
+ out_len);
+ }
+
+ if (debug_data_in) {
+ cpu_physical_memory_unmap(debug_data_in, sizeof(*debug_data_in), 0,
+ in_len);
+ }
+
+ return ret;
+}
+
+uint16_t hyperv_hcall_post_dbg_data(uint64_t ingpa, uint64_t outgpa, bool fast)
+{
+ uint16_t ret;
+ struct hyperv_post_debug_data_input *post_data_in = NULL;
+ struct hyperv_post_debug_data_output *post_data_out = NULL;
+ hwaddr in_len, out_len;
+ HvSynDbgMsg msg;
+
+ if (fast || !hv_syndbg_handler) {
+ ret = HV_STATUS_INVALID_HYPERCALL_CODE;
+ goto cleanup;
+ }
+
+ in_len = sizeof(*post_data_in);
+ post_data_in = cpu_physical_memory_map(ingpa, &in_len, 0);
+ if (!post_data_in || in_len < sizeof(*post_data_in)) {
+ ret = HV_STATUS_INSUFFICIENT_MEMORY;
+ goto cleanup;
+ }
+
+ if (post_data_in->count > TARGET_PAGE_SIZE - sizeof(*post_data_in)) {
+ ret = HV_STATUS_INVALID_PARAMETER;
+ goto cleanup;
+ }
+
+ out_len = sizeof(*post_data_out);
+ post_data_out = cpu_physical_memory_map(outgpa, &out_len, 1);
+ if (!post_data_out || out_len < sizeof(*post_data_out)) {
+ ret = HV_STATUS_INSUFFICIENT_MEMORY;
+ goto cleanup;
+ }
+
+ msg.type = HV_SYNDBG_MSG_SEND;
+ msg.u.send.buf_gpa = ingpa + sizeof(*post_data_in);
+ msg.u.send.count = post_data_in->count;
+ msg.u.send.is_raw = true;
+ ret = hv_syndbg_handler(hv_syndbg_context, &msg);
+ if (ret != HV_STATUS_SUCCESS) {
+ goto cleanup;
+ }
+
+ post_data_out->pending_count = msg.u.send.pending_count;
+ ret = post_data_out->pending_count ? HV_STATUS_INSUFFICIENT_BUFFERS :
+ HV_STATUS_SUCCESS;
+cleanup:
+ if (post_data_out) {
+ cpu_physical_memory_unmap(post_data_out,
+ sizeof(*post_data_out), 1, out_len);
+ }
+
+ if (post_data_in) {
+ cpu_physical_memory_unmap(post_data_in,
+ sizeof(*post_data_in), 0, in_len);
+ }
+
+ return ret;
+}
+
+uint32_t hyperv_syndbg_send(uint64_t ingpa, uint32_t count)
+{
+ HvSynDbgMsg msg;
+
+ if (!hv_syndbg_handler) {
+ return HV_SYNDBG_STATUS_INVALID;
+ }
+
+ msg.type = HV_SYNDBG_MSG_SEND;
+ msg.u.send.buf_gpa = ingpa;
+ msg.u.send.count = count;
+ msg.u.send.is_raw = false;
+ if (hv_syndbg_handler(hv_syndbg_context, &msg)) {
+ return HV_SYNDBG_STATUS_INVALID;
+ }
+
+ return HV_SYNDBG_STATUS_SEND_SUCCESS;
+}
+
+uint32_t hyperv_syndbg_recv(uint64_t ingpa, uint32_t count)
+{
+ uint16_t ret;
+ HvSynDbgMsg msg;
+
+ if (!hv_syndbg_handler) {
+ return HV_SYNDBG_STATUS_INVALID;
+ }
+
+ msg.type = HV_SYNDBG_MSG_RECV;
+ msg.u.recv.buf_gpa = ingpa;
+ msg.u.recv.count = count;
+ msg.u.recv.options = 0;
+ msg.u.recv.timeout = 0;
+ msg.u.recv.is_raw = false;
+ ret = hv_syndbg_handler(hv_syndbg_context, &msg);
+ if (ret != HV_STATUS_SUCCESS) {
+ return 0;
+ }
+
+ return HV_SYNDBG_STATUS_SET_SIZE(HV_SYNDBG_STATUS_RECV_SUCCESS,
+ msg.u.recv.retrieved_count);
+}
+
+void hyperv_syndbg_set_pending_page(uint64_t ingpa)
+{
+ HvSynDbgMsg msg;
+
+ if (!hv_syndbg_handler) {
+ return;
+ }
+
+ msg.type = HV_SYNDBG_MSG_SET_PENDING_PAGE;
+ msg.u.pending_page.buf_gpa = ingpa;
+ hv_syndbg_handler(hv_syndbg_context, &msg);
+}
+
+uint64_t hyperv_syndbg_query_options(void)
+{
+ HvSynDbgMsg msg;
+
+ if (!hv_syndbg_handler) {
+ return 0;
+ }
+
+ msg.type = HV_SYNDBG_MSG_QUERY_OPTIONS;
+ if (hv_syndbg_handler(hv_syndbg_context, &msg) != HV_STATUS_SUCCESS) {
+ return 0;
+ }
+
+ return msg.u.query_options.options;
+}
diff --git a/include/hw/hyperv/hyperv.h b/include/hw/hyperv/hyperv.h
index ef9f6b6c09..e7a85156b0 100644
--- a/include/hw/hyperv/hyperv.h
+++ b/include/hw/hyperv/hyperv.h
@@ -83,4 +83,62 @@ void hyperv_synic_update(CPUState *cs, bool enable,
hwaddr msg_page_addr, hwaddr event_page_addr);
bool hyperv_is_synic_enabled(void);
+/*
+ * Process HVCALL_RESET_DEBUG_SESSION hypercall.
+ */
+uint16_t hyperv_hcall_reset_dbg_session(uint64_t outgpa);
+/*
+ * Process HVCALL_RETREIVE_DEBUG_DATA hypercall.
+ */
+uint16_t hyperv_hcall_retreive_dbg_data(uint64_t ingpa, uint64_t outgpa,
+ bool fast);
+/*
+ * Process HVCALL_POST_DEBUG_DATA hypercall.
+ */
+uint16_t hyperv_hcall_post_dbg_data(uint64_t ingpa, uint64_t outgpa, bool fast);
+
+uint32_t hyperv_syndbg_send(uint64_t ingpa, uint32_t count);
+uint32_t hyperv_syndbg_recv(uint64_t ingpa, uint32_t count);
+void hyperv_syndbg_set_pending_page(uint64_t ingpa);
+uint64_t hyperv_syndbg_query_options(void);
+
+typedef enum HvSynthDbgMsgType {
+ HV_SYNDBG_MSG_CONNECTION_INFO,
+ HV_SYNDBG_MSG_SEND,
+ HV_SYNDBG_MSG_RECV,
+ HV_SYNDBG_MSG_SET_PENDING_PAGE,
+ HV_SYNDBG_MSG_QUERY_OPTIONS
+} HvDbgSynthMsgType;
+
+typedef struct HvSynDbgMsg {
+ HvDbgSynthMsgType type;
+ union {
+ struct {
+ uint32_t host_ip;
+ uint16_t host_port;
+ } connection_info;
+ struct {
+ uint64_t buf_gpa;
+ uint32_t count;
+ uint32_t pending_count;
+ bool is_raw;
+ } send;
+ struct {
+ uint64_t buf_gpa;
+ uint32_t count;
+ uint32_t options;
+ uint64_t timeout;
+ uint32_t retrieved_count;
+ bool is_raw;
+ } recv;
+ struct {
+ uint64_t buf_gpa;
+ } pending_page;
+ struct {
+ uint64_t options;
+ } query_options;
+ } u;
+} HvSynDbgMsg;
+typedef uint16_t (*HvSynDbgHandler)(void *context, HvSynDbgMsg *msg);
+void hyperv_set_syndbg_handler(HvSynDbgHandler handler, void *context);
#endif
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index aa9e636800..9529a6389a 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -6841,6 +6841,8 @@ static Property x86_cpu_properties[] = {
HYPERV_FEAT_AVIC, 0),
DEFINE_PROP_ON_OFF_AUTO("hv-no-nonarch-coresharing", X86CPU,
hyperv_no_nonarch_cs, ON_OFF_AUTO_OFF),
+ DEFINE_PROP_BIT64("hv-syndbg", X86CPU, hyperv_features,
+ HYPERV_FEAT_SYNDBG, 0),
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/cpu.h b/target/i386/cpu.h
index 9911d7c871..56e0317924 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -1060,6 +1060,7 @@ typedef uint64_t FeatureWordArray[FEATURE_WORDS];
#define HYPERV_FEAT_IPI 13
#define HYPERV_FEAT_STIMER_DIRECT 14
#define HYPERV_FEAT_AVIC 15
+#define HYPERV_FEAT_SYNDBG 16
#ifndef HYPERV_SPINLOCK_NEVER_NOTIFY
#define HYPERV_SPINLOCK_NEVER_NOTIFY 0xFFFFFFFF
@@ -1560,6 +1561,12 @@ typedef struct CPUX86State {
uint64_t msr_hv_hypercall;
uint64_t msr_hv_guest_os_id;
uint64_t msr_hv_tsc;
+ uint64_t msr_hv_syndbg_control;
+ uint64_t msr_hv_syndbg_status;
+ uint64_t msr_hv_syndbg_send_page;
+ uint64_t msr_hv_syndbg_recv_page;
+ uint64_t msr_hv_syndbg_pending_page;
+ uint64_t msr_hv_syndbg_options;
/* Per-VCPU HV MSRs */
uint64_t msr_hv_vapic;
diff --git a/target/i386/kvm/hyperv-stub.c b/target/i386/kvm/hyperv-stub.c
index 0028527e79..778ed782e6 100644
--- a/target/i386/kvm/hyperv-stub.c
+++ b/target/i386/kvm/hyperv-stub.c
@@ -27,6 +27,12 @@ int kvm_hv_handle_exit(X86CPU *cpu, struct kvm_hyperv_exit *exit)
return 0;
case KVM_EXIT_HYPERV_HCALL:
exit->u.hcall.result = HV_STATUS_INVALID_HYPERCALL_CODE;
+ return 0;
+ case KVM_EXIT_HYPERV_SYNDBG:
+ if (!hyperv_feat_enabled(cpu, HYPERV_FEAT_SYNDBG)) {
+ return -1;
+ }
+
return 0;
default:
return -1;
diff --git a/target/i386/kvm/hyperv.c b/target/i386/kvm/hyperv.c
index 26efc1e0e6..a70f695205 100644
--- a/target/i386/kvm/hyperv.c
+++ b/target/i386/kvm/hyperv.c
@@ -81,20 +81,66 @@ int kvm_hv_handle_exit(X86CPU *cpu, struct kvm_hyperv_exit *exit)
case KVM_EXIT_HYPERV_HCALL: {
uint16_t code = exit->u.hcall.input & 0xffff;
bool fast = exit->u.hcall.input & HV_HYPERCALL_FAST;
- uint64_t param = exit->u.hcall.params[0];
+ uint64_t in_param = exit->u.hcall.params[0];
+ uint64_t out_param = exit->u.hcall.params[1];
switch (code) {
case HV_POST_MESSAGE:
- exit->u.hcall.result = hyperv_hcall_post_message(param, fast);
+ exit->u.hcall.result = hyperv_hcall_post_message(in_param, fast);
break;
case HV_SIGNAL_EVENT:
- exit->u.hcall.result = hyperv_hcall_signal_event(param, fast);
+ exit->u.hcall.result = hyperv_hcall_signal_event(in_param, fast);
+ break;
+ case HV_POST_DEBUG_DATA:
+ exit->u.hcall.result =
+ hyperv_hcall_post_dbg_data(in_param, out_param, fast);
+ break;
+ case HV_RETREIVE_DEBUG_DATA:
+ exit->u.hcall.result =
+ hyperv_hcall_retreive_dbg_data(in_param, out_param, fast);
+ break;
+ case HV_RESET_DEBUG_SESSION:
+ exit->u.hcall.result =
+ hyperv_hcall_reset_dbg_session(out_param);
break;
default:
exit->u.hcall.result = HV_STATUS_INVALID_HYPERCALL_CODE;
}
return 0;
}
+
+ case KVM_EXIT_HYPERV_SYNDBG:
+ if (!hyperv_feat_enabled(cpu, HYPERV_FEAT_SYNDBG)) {
+ return -1;
+ }
+
+ switch (exit->u.syndbg.msr) {
+ case HV_X64_MSR_SYNDBG_CONTROL: {
+ uint64_t control = exit->u.syndbg.control;
+ env->msr_hv_syndbg_control = control;
+ env->msr_hv_syndbg_send_page = exit->u.syndbg.send_page;
+ env->msr_hv_syndbg_recv_page = exit->u.syndbg.recv_page;
+ exit->u.syndbg.status = HV_STATUS_SUCCESS;
+ if (control & HV_SYNDBG_CONTROL_SEND) {
+ exit->u.syndbg.status =
+ hyperv_syndbg_send(env->msr_hv_syndbg_send_page,
+ HV_SYNDBG_CONTROL_SEND_SIZE(control));
+ } else if (control & HV_SYNDBG_CONTROL_RECV) {
+ exit->u.syndbg.status =
+ hyperv_syndbg_recv(env->msr_hv_syndbg_recv_page,
+ TARGET_PAGE_SIZE);
+ }
+ break;
+ }
+ case HV_X64_MSR_SYNDBG_PENDING_BUFFER:
+ env->msr_hv_syndbg_pending_page = exit->u.syndbg.pending_page;
+ hyperv_syndbg_set_pending_page(env->msr_hv_syndbg_pending_page);
+ break;
+ default:
+ return -1;
+ }
+
+ return 0;
default:
return -1;
}
diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index 2c8feb4a6f..ecabb332d7 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -102,6 +102,7 @@ static bool has_msr_hv_synic;
static bool has_msr_hv_stimer;
static bool has_msr_hv_frequencies;
static bool has_msr_hv_reenlightenment;
+static bool has_msr_hv_syndbg_options;
static bool has_msr_xss;
static bool has_msr_umwait;
static bool has_msr_spec_ctrl;
@@ -932,6 +933,14 @@ static struct {
.bits = HV_DEPRECATING_AEOI_RECOMMENDED}
}
},
+ [HYPERV_FEAT_SYNDBG] = {
+ .desc = "Enable synthetic kernel debugger channel (hv-syndbg)",
+ .flags = {
+ {.func = HV_CPUID_FEATURES, .reg = R_EDX,
+ .bits = HV_FEATURE_DEBUG_MSRS_AVAILABLE}
+ },
+ .dependencies = BIT(HYPERV_FEAT_SYNIC) | BIT(HYPERV_FEAT_RELAXED)
+ },
};
static struct kvm_cpuid2 *try_get_hv_cpuid(CPUState *cs, int max,
@@ -972,8 +981,8 @@ static struct kvm_cpuid2 *try_get_hv_cpuid(CPUState *cs, int max,
static struct kvm_cpuid2 *get_supported_hv_cpuid(CPUState *cs)
{
struct kvm_cpuid2 *cpuid;
- /* 0x40000000..0x40000005, 0x4000000A, 0x40000080..0x40000080 leaves */
- int max = 10;
+ /* 0x40000000..0x40000005, 0x4000000A, 0x40000080..0x40000082 leaves */
+ int max = 11;
int i;
bool do_sys_ioctl;
@@ -1086,6 +1095,12 @@ static struct kvm_cpuid2 *get_supported_hv_cpuid_legacy(CPUState *cs)
entry_feat->eax |= HV_SYNTIMERS_AVAILABLE;
}
+ if (has_msr_hv_syndbg_options) {
+ entry_feat->edx |= HV_GUEST_DEBUGGING_AVAILABLE;
+ entry_feat->edx |= HV_FEATURE_DEBUG_MSRS_AVAILABLE;
+ entry_feat->ebx |= HV_PARTITION_DEUBGGING_ALLOWED;
+ }
+
if (kvm_check_extension(cs->kvm_state,
KVM_CAP_HYPERV_TLBFLUSH) > 0) {
entry_recomm->eax |= HV_REMOTE_TLB_FLUSH_RECOMMENDED;
@@ -1337,12 +1352,22 @@ static int hyperv_fill_cpuids(CPUState *cs,
{
X86CPU *cpu = X86_CPU(cs);
struct kvm_cpuid_entry2 *c;
- uint32_t cpuid_i = 0;
+ uint32_t signature[3];
+ uint32_t cpuid_i = 0, max_cpuid_leaf = 0;
+
+ max_cpuid_leaf = HV_CPUID_IMPLEMENT_LIMITS;
+ if (hyperv_feat_enabled(cpu, HYPERV_FEAT_EVMCS)) {
+ max_cpuid_leaf = MAX(max_cpuid_leaf, HV_CPUID_NESTED_FEATURES);
+ }
+
+ if (hyperv_feat_enabled(cpu, HYPERV_FEAT_SYNDBG)) {
+ max_cpuid_leaf =
+ MAX(max_cpuid_leaf, HV_CPUID_SYNDBG_PLATFORM_CAPABILITIES);
+ }
c = &cpuid_ent[cpuid_i++];
c->function = HV_CPUID_VENDOR_AND_MAX_FUNCTIONS;
- c->eax = hyperv_feat_enabled(cpu, HYPERV_FEAT_EVMCS) ?
- HV_CPUID_NESTED_FEATURES : HV_CPUID_IMPLEMENT_LIMITS;
+ c->eax = max_cpuid_leaf;
c->ebx = cpu->hyperv_vendor_id[0];
c->ecx = cpu->hyperv_vendor_id[1];
c->edx = cpu->hyperv_vendor_id[2];
@@ -1421,6 +1446,33 @@ static int hyperv_fill_cpuids(CPUState *cs,
c->eax = cpu->hyperv_nested[0];
}
+ if (hyperv_feat_enabled(cpu, HYPERV_FEAT_SYNDBG)) {
+ c = &cpuid_ent[cpuid_i++];
+ c->function = HV_CPUID_SYNDBG_VENDOR_AND_MAX_FUNCTIONS;
+ c->eax = hyperv_feat_enabled(cpu, HYPERV_FEAT_EVMCS) ?
+ HV_CPUID_NESTED_FEATURES : HV_CPUID_IMPLEMENT_LIMITS;
+ memcpy(signature, "Microsoft VS", 12);
+ c->eax = 0;
+ c->ebx = signature[0];
+ c->ecx = signature[1];
+ c->edx = signature[2];
+
+ c = &cpuid_ent[cpuid_i++];
+ c->function = HV_CPUID_SYNDBG_INTERFACE;
+ memcpy(signature, "VS#1\0\0\0\0\0\0\0\0", 12);
+ c->eax = signature[0];
+ c->ebx = 0;
+ c->ecx = 0;
+ c->edx = 0;
+
+ c = &cpuid_ent[cpuid_i++];
+ c->function = HV_CPUID_SYNDBG_PLATFORM_CAPABILITIES;
+ c->eax = HV_SYNDBG_CAP_ALLOW_KERNEL_DEBUGGING;
+ c->ebx = 0;
+ c->ecx = 0;
+ c->edx = 0;
+ }
+
return cpuid_i;
}
@@ -2215,6 +2267,9 @@ static int kvm_get_supported_msrs(KVMState *s)
case HV_X64_MSR_REENLIGHTENMENT_CONTROL:
has_msr_hv_reenlightenment = true;
break;
+ case HV_X64_MSR_SYNDBG_OPTIONS:
+ has_msr_hv_syndbg_options = true;
+ break;
case MSR_IA32_SPEC_CTRL:
has_msr_spec_ctrl = true;
break;
@@ -3132,6 +3187,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);
}
+ 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());
+ }
}
if (hyperv_feat_enabled(cpu, HYPERV_FEAT_VAPIC)) {
kvm_msr_entry_add(cpu, HV_X64_MSR_APIC_ASSIST_PAGE,
@@ -3565,6 +3625,9 @@ static int kvm_get_msrs(X86CPU *cpu)
kvm_msr_entry_add(cpu, HV_X64_MSR_TSC_EMULATION_CONTROL, 0);
kvm_msr_entry_add(cpu, HV_X64_MSR_TSC_EMULATION_STATUS, 0);
}
+ if (has_msr_hv_syndbg_options) {
+ kvm_msr_entry_add(cpu, HV_X64_MSR_SYNDBG_OPTIONS, 0);
+ }
if (has_msr_hv_crash) {
int j;
@@ -3851,6 +3914,9 @@ static int kvm_get_msrs(X86CPU *cpu)
case HV_X64_MSR_TSC_EMULATION_STATUS:
env->msr_hv_tsc_emulation_status = msrs[i].data;
break;
+ case HV_X64_MSR_SYNDBG_OPTIONS:
+ env->msr_hv_syndbg_options = msrs[i].data;
+ break;
case MSR_MTRRdefType:
env->mtrr_deftype = msrs[i].data;
break;
--
2.34.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v1 4/4] hw: hyperv: Initial commit for Synthetic Debugging device
2022-02-04 10:07 [PATCH v1 0/4] HyperV: Synthetic Debugging device Jon Doron
` (2 preceding siblings ...)
2022-02-04 10:07 ` [PATCH v1 3/4] hyperv: Add support to process syndbg commands Jon Doron
@ 2022-02-04 10:07 ` Jon Doron
2022-02-16 9:11 ` Emanuele Giuseppe Esposito
2022-02-13 7:49 ` [PATCH v1 0/4] HyperV: " Jon Doron
4 siblings, 1 reply; 14+ messages in thread
From: Jon Doron @ 2022-02-04 10:07 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, vkuznets, Jon Doron
Signed-off-by: Jon Doron <arilou@gmail.com>
---
hw/hyperv/Kconfig | 5 +
hw/hyperv/meson.build | 1 +
hw/hyperv/syndbg.c | 407 ++++++++++++++++++++++++++++++++++++++++++
3 files changed, 413 insertions(+)
create mode 100644 hw/hyperv/syndbg.c
diff --git a/hw/hyperv/Kconfig b/hw/hyperv/Kconfig
index 3fbfe41c9e..fcf65903bd 100644
--- a/hw/hyperv/Kconfig
+++ b/hw/hyperv/Kconfig
@@ -11,3 +11,8 @@ config VMBUS
bool
default y
depends on HYPERV
+
+config SYNDBG
+ bool
+ default y
+ depends on VMBUS
diff --git a/hw/hyperv/meson.build b/hw/hyperv/meson.build
index 1367e2994f..b43f119ea5 100644
--- a/hw/hyperv/meson.build
+++ b/hw/hyperv/meson.build
@@ -1,3 +1,4 @@
specific_ss.add(when: 'CONFIG_HYPERV', if_true: files('hyperv.c'))
specific_ss.add(when: 'CONFIG_HYPERV_TESTDEV', if_true: files('hyperv_testdev.c'))
specific_ss.add(when: 'CONFIG_VMBUS', if_true: files('vmbus.c'))
+specific_ss.add(when: 'CONFIG_SYNDBG', if_true: files('syndbg.c'))
diff --git a/hw/hyperv/syndbg.c b/hw/hyperv/syndbg.c
new file mode 100644
index 0000000000..837eb33458
--- /dev/null
+++ b/hw/hyperv/syndbg.c
@@ -0,0 +1,407 @@
+/*
+ * QEMU Hyper-V Synthetic Debugging device
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/ctype.h"
+#include "qemu/osdep.h"
+#include "qemu/error-report.h"
+#include "qemu/main-loop.h"
+#include "qemu/sockets.h"
+#include "qemu-common.h"
+#include "qapi/error.h"
+#include "migration/vmstate.h"
+#include "hw/qdev-properties.h"
+#include "hw/loader.h"
+#include "cpu.h"
+#include "hw/hyperv/hyperv.h"
+#include "hw/hyperv/vmbus-bridge.h"
+#include "hw/hyperv/hyperv-proto.h"
+#include "net/net.h"
+#include "net/eth.h"
+#include "net/checksum.h"
+#include "trace.h"
+
+#define TYPE_HV_SYNDBG "hv-syndbg"
+
+typedef struct HvSynDbg {
+ DeviceState parent_obj;
+
+ char *host_ip;
+ uint16_t host_port;
+ bool use_hcalls;
+
+ uint32_t target_ip;
+ struct sockaddr_in servaddr;
+ int socket;
+ bool has_data_pending;
+ uint64_t pending_page_gpa;
+} HvSynDbg;
+
+#define HVSYNDBG(obj) OBJECT_CHECK(HvSynDbg, (obj), TYPE_HV_SYNDBG)
+
+/* returns NULL unless there is exactly one HV Synth debug device */
+static HvSynDbg *hv_syndbg_find(void)
+{
+ /* Returns NULL unless there is exactly one hvsd device */
+ return HVSYNDBG(object_resolve_path_type("", TYPE_HV_SYNDBG, NULL));
+}
+
+static void set_pending_state(HvSynDbg *syndbg, bool has_pending)
+{
+ hwaddr out_len;
+ void *out_data;
+
+ syndbg->has_data_pending = has_pending;
+
+ if (!syndbg->pending_page_gpa) {
+ return;
+ }
+
+ out_len = 1;
+ out_data = cpu_physical_memory_map(syndbg->pending_page_gpa, &out_len, 1);
+ if (out_data) {
+ *(uint8_t *)out_data = !!has_pending;
+ cpu_physical_memory_unmap(out_data, out_len, 1, out_len);
+ }
+}
+
+static bool get_udb_pkt_data(void *p, uint32_t len, uint32_t *data_ofs,
+ uint32_t *src_ip)
+{
+ uint32_t offset, curr_len = len;
+
+ if (curr_len < sizeof(struct eth_header) ||
+ (be16_to_cpu(PKT_GET_ETH_HDR(p)->h_proto) != ETH_P_IP)) {
+ return false;
+ }
+ offset = sizeof(struct eth_header);
+ curr_len -= sizeof(struct eth_header);
+
+ if (curr_len < sizeof(struct ip_header) ||
+ PKT_GET_IP_HDR(p)->ip_p != IP_PROTO_UDP) {
+ return false;
+ }
+ offset += PKT_GET_IP_HDR_LEN(p);
+ curr_len -= PKT_GET_IP_HDR_LEN(p);
+
+ if (curr_len < sizeof(struct udp_header)) {
+ return false;
+ }
+
+ offset += sizeof(struct udp_header);
+ *data_ofs = offset;
+ *src_ip = PKT_GET_IP_HDR(p)->ip_src;
+ return true;
+}
+
+static uint16_t handle_send_msg(HvSynDbg *syndbg, uint64_t ingpa,
+ uint32_t count, bool is_raw,
+ uint32_t *pending_count)
+{
+ uint16_t ret;
+ hwaddr data_len;
+ void *debug_data = NULL;
+ uint32_t udp_data_ofs = 0;
+ const void *pkt_data;
+ int sent_count;
+
+ data_len = count;
+ debug_data = cpu_physical_memory_map(ingpa, &data_len, 0);
+ if (!debug_data || data_len < count) {
+ ret = HV_STATUS_INSUFFICIENT_MEMORY;
+ goto cleanup;
+ }
+
+ if (is_raw &&
+ !get_udb_pkt_data(debug_data, count, &udp_data_ofs,
+ &syndbg->target_ip)) {
+ ret = HV_STATUS_SUCCESS;
+ goto cleanup;
+ }
+
+ pkt_data = (const void *)((uintptr_t)debug_data + udp_data_ofs);
+ sent_count = qemu_sendto(syndbg->socket, pkt_data, count - udp_data_ofs,
+ MSG_NOSIGNAL, NULL, 0);
+ if (sent_count == -1) {
+ ret = HV_STATUS_INSUFFICIENT_MEMORY;
+ goto cleanup;
+ }
+
+ *pending_count = count - (sent_count + udp_data_ofs);
+ ret = HV_STATUS_SUCCESS;
+cleanup:
+ if (debug_data) {
+ cpu_physical_memory_unmap(debug_data, count, 0, data_len);
+ }
+
+ return ret;
+}
+
+#define UDP_PKT_HEADER_SIZE \
+ (sizeof(struct eth_header) + sizeof(struct ip_header) +\
+ sizeof(struct udp_header))
+
+static bool create_udp_pkt(HvSynDbg *syndbg, void *pkt, uint32_t pkt_len,
+ void *udp_data, uint32_t udp_data_len)
+{
+ struct udp_header *udp_part;
+
+ if (pkt_len < (UDP_PKT_HEADER_SIZE + udp_data_len)) {
+ return false;
+ }
+
+ /* Setup the eth */
+ memset(&PKT_GET_ETH_HDR(pkt)->h_source, 0, ETH_ALEN);
+ memset(&PKT_GET_ETH_HDR(pkt)->h_dest, 0, ETH_ALEN);
+ PKT_GET_ETH_HDR(pkt)->h_proto = cpu_to_be16(ETH_P_IP);
+
+ /* Setup the ip */
+ PKT_GET_IP_HDR(pkt)->ip_ver_len =
+ (4 << 4) | (sizeof(struct ip_header) >> 2);
+ PKT_GET_IP_HDR(pkt)->ip_tos = 0;
+ PKT_GET_IP_HDR(pkt)->ip_id = 0;
+ PKT_GET_IP_HDR(pkt)->ip_off = 0;
+ PKT_GET_IP_HDR(pkt)->ip_ttl = 64; /* IPDEFTTL */
+ PKT_GET_IP_HDR(pkt)->ip_p = IP_PROTO_UDP;
+ PKT_GET_IP_HDR(pkt)->ip_src = syndbg->servaddr.sin_addr.s_addr;
+ PKT_GET_IP_HDR(pkt)->ip_dst = syndbg->target_ip;
+ PKT_GET_IP_HDR(pkt)->ip_len =
+ cpu_to_be16(sizeof(struct ip_header) + sizeof(struct udp_header) +
+ udp_data_len);
+ eth_fix_ip4_checksum(PKT_GET_IP_HDR(pkt), PKT_GET_IP_HDR_LEN(pkt));
+
+ udp_part = (struct udp_header *)((uintptr_t)pkt +
+ sizeof(struct eth_header) +
+ PKT_GET_IP_HDR_LEN(pkt));
+ udp_part->uh_sport = syndbg->servaddr.sin_port;
+ udp_part->uh_dport = syndbg->servaddr.sin_port;
+ udp_part->uh_ulen = cpu_to_be16(sizeof(struct udp_header) + udp_data_len);
+ memcpy(udp_part + 1, udp_data, udp_data_len);
+ net_checksum_calculate(pkt, UDP_PKT_HEADER_SIZE + udp_data_len, CSUM_ALL);
+ return true;
+}
+
+static uint16_t handle_recv_msg(HvSynDbg *syndbg, uint64_t outgpa,
+ uint32_t count, bool is_raw, uint32_t options,
+ uint64_t timeout, uint32_t *retrieved_count)
+{
+ uint16_t ret;
+ uint8_t data_buf[TARGET_PAGE_SIZE - UDP_PKT_HEADER_SIZE];
+ hwaddr out_len;
+ void *out_data = NULL;
+ ssize_t recv_byte_count;
+
+ /* TODO: Handle options and timeout */
+ (void)options;
+ (void)timeout;
+
+ if (!syndbg->has_data_pending) {
+ recv_byte_count = 0;
+ } else {
+ recv_byte_count = qemu_recv(syndbg->socket, data_buf,
+ MIN(sizeof(data_buf), count), MSG_WAITALL);
+ if (recv_byte_count == -1) {
+ ret = HV_STATUS_INVALID_PARAMETER;
+ goto cleanup;
+ }
+ }
+
+ if (!recv_byte_count) {
+ *retrieved_count = 0;
+ ret = HV_STATUS_NO_DATA;
+ goto cleanup;
+ }
+
+ set_pending_state(syndbg, false);
+
+ out_len = recv_byte_count;
+ if (is_raw) {
+ out_len += UDP_PKT_HEADER_SIZE;
+ }
+ out_data = cpu_physical_memory_map(outgpa, &out_len, 1);
+ if (!out_data) {
+ ret = HV_STATUS_INSUFFICIENT_MEMORY;
+ goto cleanup;
+ }
+
+ if (is_raw &&
+ !create_udp_pkt(syndbg, out_data,
+ recv_byte_count + UDP_PKT_HEADER_SIZE,
+ data_buf, recv_byte_count)) {
+ ret = HV_STATUS_INSUFFICIENT_MEMORY;
+ goto cleanup;
+ } else if (!is_raw) {
+ memcpy(out_data, data_buf, recv_byte_count);
+ }
+
+ *retrieved_count = recv_byte_count;
+ if (is_raw) {
+ *retrieved_count += UDP_PKT_HEADER_SIZE;
+ }
+ ret = HV_STATUS_SUCCESS;
+cleanup:
+ if (out_data) {
+ cpu_physical_memory_unmap(out_data, out_len, 1, out_len);
+ }
+
+ return ret;
+}
+
+static uint16_t hv_syndbg_handler(void *context, HvSynDbgMsg *msg)
+{
+ HvSynDbg *syndbg = context;
+ uint16_t ret = HV_STATUS_INVALID_HYPERCALL_CODE;
+
+ switch (msg->type) {
+ case HV_SYNDBG_MSG_CONNECTION_INFO:
+ msg->u.connection_info.host_ip =
+ ntohl(syndbg->servaddr.sin_addr.s_addr);
+ msg->u.connection_info.host_port =
+ ntohs(syndbg->servaddr.sin_port);
+ ret = HV_STATUS_SUCCESS;
+ break;
+ case HV_SYNDBG_MSG_SEND:
+ ret = handle_send_msg(syndbg, msg->u.send.buf_gpa, msg->u.send.count,
+ msg->u.send.is_raw, &msg->u.send.pending_count);
+ break;
+ case HV_SYNDBG_MSG_RECV:
+ ret = handle_recv_msg(syndbg, msg->u.recv.buf_gpa, msg->u.recv.count,
+ msg->u.recv.is_raw, msg->u.recv.options,
+ msg->u.recv.timeout,
+ &msg->u.recv.retrieved_count);
+ break;
+ case HV_SYNDBG_MSG_SET_PENDING_PAGE:
+ syndbg->pending_page_gpa = msg->u.pending_page.buf_gpa;
+ ret = HV_STATUS_SUCCESS;
+ break;
+ case HV_SYNDBG_MSG_QUERY_OPTIONS:
+ msg->u.query_options.options = 0;
+ if (syndbg->use_hcalls) {
+ msg->u.query_options.options = HV_X64_SYNDBG_OPTION_USE_HCALLS;
+ }
+ ret = HV_STATUS_SUCCESS;
+ break;
+ default:
+ break;
+ }
+
+ return ret;
+}
+
+static void hv_syndbg_recv_event(void *opaque)
+{
+ HvSynDbg *syndbg = opaque;
+ struct timeval tv;
+ fd_set rfds;
+
+ tv.tv_sec = 0;
+ tv.tv_usec = 0;
+ FD_ZERO(&rfds);
+ FD_SET(syndbg->socket, &rfds);
+ if (select(syndbg->socket + 1, &rfds, NULL, NULL, &tv) > 0) {
+ set_pending_state(syndbg, true);
+ }
+}
+
+static void hv_syndbg_realize(DeviceState *dev, Error **errp)
+{
+ HvSynDbg *syndbg = HVSYNDBG(dev);
+
+ if (!hv_syndbg_find()) {
+ error_setg(errp, "at most one %s device is permitted", TYPE_HV_SYNDBG);
+ return;
+ }
+
+ if (!vmbus_bridge_find()) {
+ error_setg(errp, "%s device requires vmbus-bridge device",
+ TYPE_HV_SYNDBG);
+ return;
+ }
+
+ /* Parse and host_ip */
+ if (qemu_isdigit(syndbg->host_ip[0])) {
+ syndbg->servaddr.sin_addr.s_addr = inet_addr(syndbg->host_ip);
+ } else {
+ struct hostent *he = gethostbyname(syndbg->host_ip);
+ if (!he) {
+ error_setg(errp, "%s failed to resolve host name %s",
+ TYPE_HV_SYNDBG, syndbg->host_ip);
+ return;
+ }
+ syndbg->servaddr.sin_addr = *(struct in_addr *)he->h_addr;
+ }
+
+ syndbg->socket = socket(AF_INET, SOCK_DGRAM, 0);
+ if (syndbg->socket < 0) {
+ error_setg(errp, "%s failed to create socket", TYPE_HV_SYNDBG);
+ return;
+ }
+
+ qemu_set_nonblock(syndbg->socket);
+
+ syndbg->servaddr.sin_port = htons(syndbg->host_port);
+ syndbg->servaddr.sin_family = AF_INET;
+ if (connect(syndbg->socket, (struct sockaddr *)&syndbg->servaddr,
+ sizeof(syndbg->servaddr)) < 0) {
+ closesocket(syndbg->socket);
+ error_setg(errp, "%s failed to connect to socket", TYPE_HV_SYNDBG);
+ return;
+ }
+
+ syndbg->pending_page_gpa = 0;
+ syndbg->has_data_pending = false;
+ hyperv_set_syndbg_handler(hv_syndbg_handler, syndbg);
+ qemu_set_fd_handler(syndbg->socket, hv_syndbg_recv_event, NULL, syndbg);
+}
+
+static void hv_syndbg_unrealize(DeviceState *dev)
+{
+ HvSynDbg *syndbg = HVSYNDBG(dev);
+
+ if (syndbg->socket > 0) {
+ qemu_set_fd_handler(syndbg->socket, NULL, NULL, NULL);
+ closesocket(syndbg->socket);
+ }
+}
+
+static const VMStateDescription vmstate_hv_syndbg = {
+ .name = TYPE_HV_SYNDBG,
+ .unmigratable = 1,
+};
+
+static Property hv_syndbg_properties[] = {
+ DEFINE_PROP_STRING("host_ip", HvSynDbg, host_ip),
+ DEFINE_PROP_UINT16("host_port", HvSynDbg, host_port, 50000),
+ DEFINE_PROP_BOOL("use_hcalls", HvSynDbg, use_hcalls, false),
+ DEFINE_PROP_END_OF_LIST(),
+};
+
+static void hv_syndbg_class_init(ObjectClass *klass, void *data)
+{
+ DeviceClass *dc = DEVICE_CLASS(klass);
+
+ device_class_set_props(dc, hv_syndbg_properties);
+ dc->fw_name = TYPE_HV_SYNDBG;
+ dc->vmsd = &vmstate_hv_syndbg;
+ dc->realize = hv_syndbg_realize;
+ dc->unrealize = hv_syndbg_unrealize;
+ dc->user_creatable = true;
+ set_bit(DEVICE_CATEGORY_MISC, dc->categories);
+}
+
+static const TypeInfo hv_syndbg_type_info = {
+ .name = TYPE_HV_SYNDBG,
+ .parent = TYPE_DEVICE,
+ .instance_size = sizeof(HvSynDbg),
+ .class_init = hv_syndbg_class_init,
+};
+
+static void hv_syndbg_register_types(void)
+{
+ type_register_static(&hv_syndbg_type_info);
+}
+
+type_init(hv_syndbg_register_types)
--
2.34.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v1 0/4] HyperV: Synthetic Debugging device
2022-02-04 10:07 [PATCH v1 0/4] HyperV: Synthetic Debugging device Jon Doron
` (3 preceding siblings ...)
2022-02-04 10:07 ` [PATCH v1 4/4] hw: hyperv: Initial commit for Synthetic Debugging device Jon Doron
@ 2022-02-13 7:49 ` Jon Doron
4 siblings, 0 replies; 14+ messages in thread
From: Jon Doron @ 2022-02-13 7:49 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, vkuznets
On 04/02/2022, Jon Doron wrote:
Ping
>This patchset adds support for the synthetic debugging device.
>
>HyperV supports a special transport layer for the kernel debugger when
>running in HyperV.
>
>This patchset add supports for this device so you could have a setup
>fast windows kernel debugging.
>
>At this point of time, DHCP is not implmeneted so to set this up few
>things need to be noted.
>
>The scenario I used to test is having 2 VMs in the same virtual network
>i.e a Debugger VM with the NIC:
>-nic tap,model=virtio,mac=02:ca:01:01:01:01,script=/etc/qemu-ifup
>And it's IP is going to be static 192.168.53.12
>And the VM we want to debug, to which we need to have the englightments
>and vmbus configured:
> -cpu host,hv-relaxed,hv_spinlocks=0x1fff,hv_time,+vmx,invtsc,hv-vapic,hv-vpindex,hv-synic,hv-syndbg \
> -device vmbus-bridge \
> -device hv-syndbg,host_ip=192.168.53.12,host_port=50000,use_hcalls=false \
> -nic tap,model=virtio,mac=02:ca:01:01:01:02,script=/etc/qemu-ifup \
>
>Then in the debuggee VM we would setup the kernel debugging in the
>following way:
>
>If the VM is older than Win8:
>* Copy the proper platform kdvm.dll (make sure it's called kdvm.dll even if platform is 32bit)
>bcdedit /set {GUID} dbgtransport kdvm.dll
>bcdedit /set {GUID} loadoptions host_ip="1.2.3.4",host_port="50000",nodhcp
>bcdedit /set {GUID} debug on
>bcdedit /set {GUID} halbreakpoint on
>
>Win8 and late:
>bcdedit /dbgsettings net hostip:7.7.7.7 port:50000 nodhcp
>
>This is all the setup that is required to get the synthetic debugger
>configured correctly.
>
>Jon Doron (4):
> hyperv: SControl is optional to enable SynIc
> hyperv: Add definitions for syndbg
> hyperv: Add support to process syndbg commands
> hw: hyperv: Initial commit for Synthetic Debugging device
>
> docs/hyperv.txt | 15 +
> hw/hyperv/Kconfig | 5 +
> hw/hyperv/hyperv.c | 475 +++++++++++++++++++++++++------
> hw/hyperv/meson.build | 1 +
> hw/hyperv/syndbg.c | 407 ++++++++++++++++++++++++++
> include/hw/hyperv/hyperv-proto.h | 52 ++++
> include/hw/hyperv/hyperv.h | 60 ++++
> target/i386/cpu.c | 2 +
> target/i386/cpu.h | 7 +
> target/i386/kvm/hyperv-proto.h | 37 +++
> target/i386/kvm/hyperv-stub.c | 6 +
> target/i386/kvm/hyperv.c | 52 +++-
> target/i386/kvm/kvm.c | 76 ++++-
> 13 files changed, 1105 insertions(+), 90 deletions(-)
> create mode 100644 hw/hyperv/syndbg.c
>
>--
>2.34.1
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v1 1/4] hyperv: SControl is optional to enable SynIc
2022-02-04 10:07 ` [PATCH v1 1/4] hyperv: SControl is optional to enable SynIc Jon Doron
@ 2022-02-16 9:10 ` Emanuele Giuseppe Esposito
2022-02-16 10:27 ` Jon Doron
0 siblings, 1 reply; 14+ messages in thread
From: Emanuele Giuseppe Esposito @ 2022-02-16 9:10 UTC (permalink / raw)
To: qemu-devel, Jon Doron; +Cc: Paolo Bonzini, Vitaly Kuznetsov
On 04/02/2022 11:07, Jon Doron wrote:
> SynIc can be enabled regardless of the SControl mechanisim which can
> register a GSI for a given SintRoute.
>
> This behaviour can achived by setting enabling SIMP and then the guest
> will poll on the message slot.
>
> Once there is another message pending the host will set the message slot
> with the pending flag.
> When the guest polls from the message slot, incase the pending flag is
s/incase/in case
> set it will write to the HV_X64_MSR_EOM indicating it has cleared the
> slow and we can try and push our message again.
what do you mean by "the slow"?
>
> Signed-off-by: Jon Doron <arilou@gmail.com>
> ---
> hw/hyperv/hyperv.c | 233 ++++++++++++++++++++++++-------------
> include/hw/hyperv/hyperv.h | 2 +
> 2 files changed, 153 insertions(+), 82 deletions(-)
>
> diff --git a/hw/hyperv/hyperv.c b/hw/hyperv/hyperv.c
> index cb1074f234..88c9cc1334 100644
> --- a/hw/hyperv/hyperv.c
> +++ b/hw/hyperv/hyperv.c
> @@ -27,18 +27,70 @@ struct SynICState {
>
> CPUState *cs;
>
> - bool enabled;
> + bool sctl_enabled;
> hwaddr msg_page_addr;
> hwaddr event_page_addr;
> MemoryRegion msg_page_mr;
> MemoryRegion event_page_mr;
> struct hyperv_message_page *msg_page;
> struct hyperv_event_flags_page *event_page;
> +
> + QemuMutex sint_routes_mutex;
> + QLIST_HEAD(, HvSintRoute) sint_routes;
> };
>
> #define TYPE_SYNIC "hyperv-synic"
> OBJECT_DECLARE_SIMPLE_TYPE(SynICState, SYNIC)
>
> +/*
> + * KVM has its own message producers (SynIC timers). To guarantee
> + * serialization with both KVM vcpu and the guest cpu, the messages are first
> + * staged in an intermediate area and then posted to the SynIC message page in
> + * the vcpu thread.
> + */
> +typedef struct HvSintStagedMessage {
> + /* message content staged by hyperv_post_msg */
> + struct hyperv_message msg;
> + /* callback + data (r/o) to complete the processing in a BH */
> + HvSintMsgCb cb;
> + void *cb_data;
> + /* message posting status filled by cpu_post_msg */
> + int status;
> + /* passing the buck: */
> + enum {
> + /* initial state */
> + HV_STAGED_MSG_FREE,
> + /*
> + * hyperv_post_msg (e.g. in main loop) grabs the staged area (FREE ->
> + * BUSY), copies msg, and schedules cpu_post_msg on the assigned cpu
> + */
> + HV_STAGED_MSG_BUSY,
> + /*
> + * cpu_post_msg (vcpu thread) tries to copy staged msg to msg slot,
> + * notify the guest, records the status, marks the posting done (BUSY
> + * -> POSTED), and schedules sint_msg_bh BH
> + */
> + HV_STAGED_MSG_POSTED,
> + /*
> + * sint_msg_bh (BH) verifies that the posting is done, runs the
> + * callback, and starts over (POSTED -> FREE)
> + */
> + } state;
> +} HvSintStagedMessage;
> +
> +struct HvSintRoute {
> + uint32_t sint;
> + SynICState *synic;
> + int gsi;
> + EventNotifier sint_set_notifier;
> + EventNotifier sint_ack_notifier;
> +
> + HvSintStagedMessage *staged_msg;
> +
> + unsigned refcount;
> + QLIST_ENTRY(HvSintRoute) link;
> +};
> +
> static bool synic_enabled;
Why did you move this struct above?
I think it was done purposefully to separate synic_* functions from the
others below (sint_*).
>
> bool hyperv_is_synic_enabled(void)
> @@ -51,11 +103,11 @@ static SynICState *get_synic(CPUState *cs)
> return SYNIC(object_resolve_path_component(OBJECT(cs), "synic"));
> }
>
> -static void synic_update(SynICState *synic, bool enable,
> +static void synic_update(SynICState *synic, bool sctl_enable,
> hwaddr msg_page_addr, hwaddr event_page_addr)
> {
>
> - synic->enabled = enable;
> + synic->sctl_enabled = sctl_enable;
> if (synic->msg_page_addr != msg_page_addr) {
> if (synic->msg_page_addr) {
> memory_region_del_subregion(get_system_memory(),
> @@ -80,7 +132,7 @@ static void synic_update(SynICState *synic, bool enable,
> }
> }
>
> -void hyperv_synic_update(CPUState *cs, bool enable,
> +void hyperv_synic_update(CPUState *cs, bool sctl_enable,
> hwaddr msg_page_addr, hwaddr event_page_addr)
> {
> SynICState *synic = get_synic(cs);
> @@ -89,7 +141,7 @@ void hyperv_synic_update(CPUState *cs, bool enable,
> return;
> }
>
> - synic_update(synic, enable, msg_page_addr, event_page_addr);
> + synic_update(synic, sctl_enable, msg_page_addr, event_page_addr);
> }
>
> static void synic_realize(DeviceState *dev, Error **errp)
> @@ -110,16 +162,20 @@ static void synic_realize(DeviceState *dev, Error **errp)
> sizeof(*synic->event_page), &error_abort);
> synic->msg_page = memory_region_get_ram_ptr(&synic->msg_page_mr);
> synic->event_page = memory_region_get_ram_ptr(&synic->event_page_mr);
> + qemu_mutex_init(&synic->sint_routes_mutex);
> + QLIST_INIT(&synic->sint_routes);
>
> g_free(msgp_name);
> g_free(eventp_name);
> }
> +
> static void synic_reset(DeviceState *dev)
> {
> SynICState *synic = SYNIC(dev);
> memset(synic->msg_page, 0, sizeof(*synic->msg_page));
> memset(synic->event_page, 0, sizeof(*synic->event_page));
> synic_update(synic, false, 0, 0);
> + assert(QLIST_EMPTY(&synic->sint_routes));
> }
>
> static void synic_class_init(ObjectClass *klass, void *data)
> @@ -168,54 +224,6 @@ static void synic_register_types(void)
>
> type_init(synic_register_types)
>
> -/*
> - * KVM has its own message producers (SynIC timers). To guarantee
> - * serialization with both KVM vcpu and the guest cpu, the messages are first
> - * staged in an intermediate area and then posted to the SynIC message page in
> - * the vcpu thread.
> - */
> -typedef struct HvSintStagedMessage {
> - /* message content staged by hyperv_post_msg */
> - struct hyperv_message msg;
> - /* callback + data (r/o) to complete the processing in a BH */
> - HvSintMsgCb cb;
> - void *cb_data;
> - /* message posting status filled by cpu_post_msg */
> - int status;
> - /* passing the buck: */
> - enum {
> - /* initial state */
> - HV_STAGED_MSG_FREE,
> - /*
> - * hyperv_post_msg (e.g. in main loop) grabs the staged area (FREE ->
> - * BUSY), copies msg, and schedules cpu_post_msg on the assigned cpu
> - */
> - HV_STAGED_MSG_BUSY,
> - /*
> - * cpu_post_msg (vcpu thread) tries to copy staged msg to msg slot,
> - * notify the guest, records the status, marks the posting done (BUSY
> - * -> POSTED), and schedules sint_msg_bh BH
> - */
> - HV_STAGED_MSG_POSTED,
> - /*
> - * sint_msg_bh (BH) verifies that the posting is done, runs the
> - * callback, and starts over (POSTED -> FREE)
> - */
> - } state;
> -} HvSintStagedMessage;
> -
> -struct HvSintRoute {
> - uint32_t sint;
> - SynICState *synic;
> - int gsi;
> - EventNotifier sint_set_notifier;
> - EventNotifier sint_ack_notifier;
> -
> - HvSintStagedMessage *staged_msg;
> -
> - unsigned refcount;
> -};
> -
> static CPUState *hyperv_find_vcpu(uint32_t vp_index)
> {
> CPUState *cs = qemu_get_cpu(vp_index);
> @@ -259,7 +267,7 @@ static void cpu_post_msg(CPUState *cs, run_on_cpu_data data)
>
> assert(staged_msg->state == HV_STAGED_MSG_BUSY);
>
> - if (!synic->enabled || !synic->msg_page_addr) {
> + if (!synic->msg_page_addr) {
Not sure if this is important, but why don't you check for
!synic->sctl_enabled anymore here? You do it below.
> staged_msg->status = -ENXIO;
> goto posted;
> }
> @@ -343,7 +351,7 @@ int hyperv_set_event_flag(HvSintRoute *sint_route, unsigned eventno)
> if (eventno > HV_EVENT_FLAGS_COUNT) {
> return -EINVAL;
> }
> - if (!synic->enabled || !synic->event_page_addr) {
> + if (!synic->sctl_enabled || !synic->event_page_addr) {
> return -ENXIO;
> }
>
> @@ -364,11 +372,13 @@ int hyperv_set_event_flag(HvSintRoute *sint_route, unsigned eventno)
> HvSintRoute *hyperv_sint_route_new(uint32_t vp_index, uint32_t sint,
> HvSintMsgCb cb, void *cb_data)
> {
> - HvSintRoute *sint_route;
> - EventNotifier *ack_notifier;
> + HvSintRoute *sint_route = NULL;
> + EventNotifier *ack_notifier = NULL;
> int r, gsi;
> CPUState *cs;
> SynICState *synic;
> + bool ack_event_initialized = false, sint_notifier_initialized = false,
> + irqfd_initialized = false;
>
> cs = hyperv_find_vcpu(vp_index);
> if (!cs) {
> @@ -381,57 +391,82 @@ HvSintRoute *hyperv_sint_route_new(uint32_t vp_index, uint32_t sint,
> }
>
> sint_route = g_new0(HvSintRoute, 1);
> - r = event_notifier_init(&sint_route->sint_set_notifier, false);
> - if (r) {
> - goto err;
> + if (!sint_route) {
> + goto cleanup_err;
> }
>
> + sint_route->gsi = 0;
useless, as g_new0 zeroes all fields
> + sint_route->synic = synic;
> + sint_route->sint = sint;
> + sint_route->refcount = 1;
>
> ack_notifier = cb ? &sint_route->sint_ack_notifier : NULL;
> if (ack_notifier) {
> sint_route->staged_msg = g_new0(HvSintStagedMessage, 1);
> + if (!sint_route->staged_msg) {
> + goto cleanup_err;
> + }
> sint_route->staged_msg->cb = cb;
> sint_route->staged_msg->cb_data = cb_data;
>
> r = event_notifier_init(ack_notifier, false);
> if (r) {
> - goto err_sint_set_notifier;
> + goto cleanup_err;
> }
> -
> event_notifier_set_handler(ack_notifier, sint_ack_handler);
> + ack_event_initialized = true;
> + }
> +
> + /* See if we are done or we need to setup a GSI for this SintRoute */
> + if (!synic->sctl_enabled) {
> + goto cleanup;
> }
>
> + /* We need to setup a GSI for this SintRoute */
> + r = event_notifier_init(&sint_route->sint_set_notifier, false);
> + if (r) {
> + goto cleanup_err;
> + }
> + sint_notifier_initialized = true;
> +
> gsi = kvm_irqchip_add_hv_sint_route(kvm_state, vp_index, sint);
> if (gsi < 0) {
> - goto err_gsi;
> + goto cleanup_err;
> }
> + irqfd_initialized = true;
>
> r = kvm_irqchip_add_irqfd_notifier_gsi(kvm_state,
> &sint_route->sint_set_notifier,
> ack_notifier, gsi);
> if (r) {
> - goto err_irqfd;
> + goto cleanup_err;
> }
> sint_route->gsi = gsi;
> - sint_route->synic = synic;
> - sint_route->sint = sint;
> - sint_route->refcount = 1;
> -
> +cleanup:
> + qemu_mutex_lock(&synic->sint_routes_mutex);
> + QLIST_INSERT_HEAD_RCU(&synic->sint_routes, sint_route, link);
> + qemu_mutex_unlock(&synic->sint_routes_mutex);
> return sint_route;
>
> -err_irqfd:
> - kvm_irqchip_release_virq(kvm_state, gsi);
> -err_gsi:
> +cleanup_err:
> + if (irqfd_initialized) {
> + kvm_irqchip_release_virq(kvm_state, gsi);
> + }
> +
> + if (sint_notifier_initialized) {
> + event_notifier_cleanup(&sint_route->sint_set_notifier);
> + }
> +
> if (ack_notifier) {
> - event_notifier_set_handler(ack_notifier, NULL);
> - event_notifier_cleanup(ack_notifier);
> + if (ack_event_initialized) {
> + event_notifier_set_handler(ack_notifier, NULL);
> + event_notifier_cleanup(ack_notifier);
> + }
> +
> g_free(sint_route->staged_msg);
> }
> -err_sint_set_notifier:
> - event_notifier_cleanup(&sint_route->sint_set_notifier);
> -err:
> - g_free(sint_route);
>
> + g_free(sint_route);
> return NULL;
> }
It is good that you check that sint_route is not NULL, but I don't find
it easy to read nor a common practice to have one single goto label and
multiple booleans to distinguish various error cases.
I think you should do as it was done before, with a specific label for
each type of error, and not always use cleanup_err.
>
> @@ -442,6 +477,8 @@ void hyperv_sint_route_ref(HvSintRoute *sint_route)
>
> void hyperv_sint_route_unref(HvSintRoute *sint_route)
> {
> + SynICState *synic;
> +
> if (!sint_route) {
> return;
> }
> @@ -452,21 +489,33 @@ void hyperv_sint_route_unref(HvSintRoute *sint_route)
> return;
> }
>
> - kvm_irqchip_remove_irqfd_notifier_gsi(kvm_state,
> - &sint_route->sint_set_notifier,
> - sint_route->gsi);
> - kvm_irqchip_release_virq(kvm_state, sint_route->gsi);
> + synic = sint_route->synic;
> + qemu_mutex_lock(&synic->sint_routes_mutex);
> + QLIST_REMOVE_RCU(sint_route, link);
> + qemu_mutex_unlock(&synic->sint_routes_mutex);
Not really important, but you can use WITH_QEMU_LOCK_GUARD instead of
doing lock/unlock.
> +
> + if (sint_route->gsi) {
> + kvm_irqchip_remove_irqfd_notifier_gsi(kvm_state,
> + &sint_route->sint_set_notifier,
> + sint_route->gsi);
> + kvm_irqchip_release_virq(kvm_state, sint_route->gsi);
> + event_notifier_cleanup(&sint_route->sint_set_notifier);
> + }
> +
> if (sint_route->staged_msg) {
> event_notifier_set_handler(&sint_route->sint_ack_notifier, NULL);
> event_notifier_cleanup(&sint_route->sint_ack_notifier);
> g_free(sint_route->staged_msg);
> }
> - event_notifier_cleanup(&sint_route->sint_set_notifier);
> g_free(sint_route);
> }
>
> int hyperv_sint_route_set_sint(HvSintRoute *sint_route)
> {
> + if (!sint_route->gsi) {
> + return 0;
> + }
> +
> return event_notifier_set(&sint_route->sint_set_notifier);
> }
>
> @@ -529,6 +578,26 @@ int hyperv_set_msg_handler(uint32_t conn_id, HvMsgHandler handler, void *data)
> return ret;
> }
>
> +int hyperv_synic_eom(CPUState *cs)
> +{
> + SynICState *synic = get_synic(cs);
> + HvSintRoute *sint_route;
> +
> + if (!synic) {
> + return -1;
> + }
use here
QEMU_LOCK_GUARD(&synic->sint_routes_mutex);
instead of lock/unlock
> +
> + qemu_mutex_lock(&synic->sint_routes_mutex);
> + QLIST_FOREACH(sint_route, &synic->sint_routes, link) {
> + /* Try to complete every SintRoute */
> + aio_bh_schedule_oneshot(qemu_get_aio_context(), sint_msg_bh,
> + sint_route);
> + }
> + qemu_mutex_unlock(&synic->sint_routes_mutex);> + return 0;
> +}
> +
> uint16_t hyperv_hcall_post_message(uint64_t param, bool fast)
> {
> uint16_t ret;
> diff --git a/include/hw/hyperv/hyperv.h b/include/hw/hyperv/hyperv.h
> index a63ee0003c..ef9f6b6c09 100644
> --- a/include/hw/hyperv/hyperv.h
> +++ b/include/hw/hyperv/hyperv.h
> @@ -28,6 +28,8 @@ void hyperv_sint_route_unref(HvSintRoute *sint_route);
>
> int hyperv_sint_route_set_sint(HvSintRoute *sint_route);
>
> +int hyperv_synic_eom(CPUState *cs);
> +
Documentation here? Where is this function used?
> /*
> * Submit a message to be posted in vcpu context. If the submission succeeds,
> * the status of posting the message is reported via the callback associated
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v1 2/4] hyperv: Add definitions for syndbg
2022-02-04 10:07 ` [PATCH v1 2/4] hyperv: Add definitions for syndbg Jon Doron
@ 2022-02-16 9:10 ` Emanuele Giuseppe Esposito
2022-02-16 10:27 ` Jon Doron
0 siblings, 1 reply; 14+ messages in thread
From: Emanuele Giuseppe Esposito @ 2022-02-16 9:10 UTC (permalink / raw)
To: Jon Doron, qemu-devel; +Cc: pbonzini, vkuznets
On 04/02/2022 11:07, Jon Doron wrote:
> Add all required definitions for hyperv synthetic debugger interface.
>
> Signed-off-by: Jon Doron <arilou@gmail.com>
> ---
> include/hw/hyperv/hyperv-proto.h | 52 ++++++++++++++++++++++++++++++++
> target/i386/kvm/hyperv-proto.h | 37 +++++++++++++++++++++++
> 2 files changed, 89 insertions(+)
>
> diff --git a/include/hw/hyperv/hyperv-proto.h b/include/hw/hyperv/hyperv-proto.h
> index 21dc28aee9..94c9658eb0 100644
> --- a/include/hw/hyperv/hyperv-proto.h
> +++ b/include/hw/hyperv/hyperv-proto.h
> @@ -24,12 +24,17 @@
> #define HV_STATUS_INVALID_PORT_ID 17
> #define HV_STATUS_INVALID_CONNECTION_ID 18
> #define HV_STATUS_INSUFFICIENT_BUFFERS 19
> +#define HV_STATUS_NOT_ACKNOWLEDGED 20
> +#define HV_STATUS_NO_DATA 27
>
> /*
> * Hypercall numbers
> */
> #define HV_POST_MESSAGE 0x005c
> #define HV_SIGNAL_EVENT 0x005d
> +#define HV_POST_DEBUG_DATA 0x0069
> +#define HV_RETREIVE_DEBUG_DATA 0x006a
s/RETREIVE/RETRIEVE?
> +#define HV_RESET_DEBUG_SESSION 0x006b
> #define HV_HYPERCALL_FAST (1u << 16)
>
> /*
> @@ -127,4 +132,51 @@ struct hyperv_event_flags_page {
> struct hyperv_event_flags slot[HV_SINT_COUNT];
> };
>
> +/*
> + * Kernel debugger structures
> + */
> +
> +/* Options flags for hyperv_reset_debug_session */
> +#define HV_DEBUG_PURGE_INCOMING_DATA 0x00000001
> +#define HV_DEBUG_PURGE_OUTGOING_DATA 0x00000002
> +struct hyperv_reset_debug_session_input {
> + uint32_t options;
> +} __attribute__ ((__packed__));
> +
> +struct hyperv_reset_debug_session_output {
> + uint32_t host_ip;
> + uint32_t target_ip;
> + uint16_t host_port;
> + uint16_t target_port;
> + uint8_t host_mac[6];
> + uint8_t target_mac[6];
> +} __attribute__ ((__packed__));
> +
> +/* Options for hyperv_post_debug_data */
> +#define HV_DEBUG_POST_LOOP 0x00000001
> +
> +struct hyperv_post_debug_data_input {
> + uint32_t count;
> + uint32_t options;
> + /*uint8_t data[HV_HYP_PAGE_SIZE - 2 * sizeof(uint32_t)];*/
What is this comment for?
> +} __attribute__ ((__packed__));
> +
> +struct hyperv_post_debug_data_output {
> + uint32_t pending_count;
> +} __attribute__ ((__packed__));
> +
> +/* Options for hyperv_retrieve_debug_data */
> +#define HV_DEBUG_RETRIEVE_LOOP 0x00000001
> +#define HV_DEBUG_RETRIEVE_TEST_ACTIVITY 0x00000002
> +
> +struct hyperv_retrieve_debug_data_input {
> + uint32_t count;
> + uint32_t options;
> + uint64_t timeout;
> +} __attribute__ ((__packed__));
> +
> +struct hyperv_retrieve_debug_data_output {
> + uint32_t retrieved_count;
> + uint32_t remaining_count;
> +} __attribute__ ((__packed__));
> #endif
> diff --git a/target/i386/kvm/hyperv-proto.h b/target/i386/kvm/hyperv-proto.h
> index 89f81afda7..9480bcdf04 100644
> --- a/target/i386/kvm/hyperv-proto.h
> +++ b/target/i386/kvm/hyperv-proto.h
> @@ -19,6 +19,9 @@
> #define HV_CPUID_ENLIGHTMENT_INFO 0x40000004
> #define HV_CPUID_IMPLEMENT_LIMITS 0x40000005
> #define HV_CPUID_NESTED_FEATURES 0x4000000A
> +#define HV_CPUID_SYNDBG_VENDOR_AND_MAX_FUNCTIONS 0x40000080
> +#define HV_CPUID_SYNDBG_INTERFACE 0x40000081
> +#define HV_CPUID_SYNDBG_PLATFORM_CAPABILITIES 0x40000082
> #define HV_CPUID_MIN 0x40000005
> #define HV_CPUID_MAX 0x4000ffff
> #define HV_HYPERVISOR_PRESENT_BIT 0x80000000
> @@ -55,8 +58,14 @@
> #define HV_GUEST_IDLE_STATE_AVAILABLE (1u << 5)
> #define HV_FREQUENCY_MSRS_AVAILABLE (1u << 8)
> #define HV_GUEST_CRASH_MSR_AVAILABLE (1u << 10)
> +#define HV_FEATURE_DEBUG_MSRS_AVAILABLE (1u << 11)
> #define HV_STIMER_DIRECT_MODE_AVAILABLE (1u << 19)
>
> +/*
> + * HV_CPUID_FEATURES.EBX bits
> + */
> +#define HV_PARTITION_DEUBGGING_ALLOWED (1u << 12)
s/DEUBGGING/DEBUGGING
> +
> /*
> * HV_CPUID_ENLIGHTMENT_INFO.EAX bits
> */
> @@ -72,6 +81,11 @@
> #define HV_ENLIGHTENED_VMCS_RECOMMENDED (1u << 14)
> #define HV_NO_NONARCH_CORESHARING (1u << 18)
>
> +/*
> + * HV_CPUID_SYNDBG_PLATFORM_CAPABILITIES.EAX bits
> + */
> +#define HV_SYNDBG_CAP_ALLOW_KERNEL_DEBUGGING (1u << 1)
> +
> /*
> * Basic virtualized MSRs
> */
> @@ -130,6 +144,18 @@
> #define HV_X64_MSR_STIMER3_CONFIG 0x400000B6
> #define HV_X64_MSR_STIMER3_COUNT 0x400000B7
>
> +/*
> + * Hyper-V Synthetic debug options MSR
> + */
> +#define HV_X64_MSR_SYNDBG_CONTROL 0x400000F1
> +#define HV_X64_MSR_SYNDBG_STATUS 0x400000F2
> +#define HV_X64_MSR_SYNDBG_SEND_BUFFER 0x400000F3
> +#define HV_X64_MSR_SYNDBG_RECV_BUFFER 0x400000F4
> +#define HV_X64_MSR_SYNDBG_PENDING_BUFFER 0x400000F5
> +#define HV_X64_MSR_SYNDBG_OPTIONS 0x400000FF
> +
> +#define HV_X64_SYNDBG_OPTION_USE_HCALLS BIT(2)
> +
> /*
> * Guest crash notification MSRs
> */
> @@ -168,5 +194,16 @@
>
> #define HV_STIMER_COUNT 4
>
> +/*
> + * Synthetic debugger control definitions
> + */
> +#define HV_SYNDBG_CONTROL_SEND (1u << 0)
> +#define HV_SYNDBG_CONTROL_RECV (1u << 1)
> +#define HV_SYNDBG_CONTROL_SEND_SIZE(ctl) ((ctl >> 16) & 0xffff)
> +#define HV_SYNDBG_STATUS_INVALID (0)
> +#define HV_SYNDBG_STATUS_SEND_SUCCESS (1u << 0)
> +#define HV_SYNDBG_STATUS_RECV_SUCCESS (1u << 2)
> +#define HV_SYNDBG_STATUS_RESET (1u << 3)
> +#define HV_SYNDBG_STATUS_SET_SIZE(st, sz) (st | (sz << 16))
>
> #endif
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v1 3/4] hyperv: Add support to process syndbg commands
2022-02-04 10:07 ` [PATCH v1 3/4] hyperv: Add support to process syndbg commands Jon Doron
@ 2022-02-16 9:10 ` Emanuele Giuseppe Esposito
2022-02-16 10:28 ` Jon Doron
0 siblings, 1 reply; 14+ messages in thread
From: Emanuele Giuseppe Esposito @ 2022-02-16 9:10 UTC (permalink / raw)
To: qemu-devel, Jon Doron; +Cc: Paolo Bonzini, Vitaly Kuznetsov
On 04/02/2022 11:07, Jon Doron wrote:
> SynDbg commands can come from two different flows:
> 1. Hypercalls, in this mode the data being sent is fully
> encapsulated network packets.
> 2. SynDbg specific MSRs, in this mode only the data that needs to be
> transfered is passed.
>
> Signed-off-by: Jon Doron <arilou@gmail.com>
> ---
> docs/hyperv.txt | 15 +++
> hw/hyperv/hyperv.c | 242 ++++++++++++++++++++++++++++++++++
> include/hw/hyperv/hyperv.h | 58 ++++++++
> target/i386/cpu.c | 2 +
> target/i386/cpu.h | 7 +
> target/i386/kvm/hyperv-stub.c | 6 +
> target/i386/kvm/hyperv.c | 52 +++++++-
> target/i386/kvm/kvm.c | 76 ++++++++++-
> 8 files changed, 450 insertions(+), 8 deletions(-)
>
> diff --git a/docs/hyperv.txt b/docs/hyperv.txt
> index 0417c183a3..7abc1b2d89 100644
> --- a/docs/hyperv.txt
> +++ b/docs/hyperv.txt
> @@ -225,6 +225,21 @@ default (WS2016).
> Note: hv-version-id-* are not enlightenments and thus don't enable Hyper-V
> identification when specified without any other enlightenments.
>
> +3.21. hv-syndbg
> +===============
> +Enables Hyper-V synthetic debugger interface, this is a special interface used
> +by Windows Kernel debugger to send the packets through, rather than sending
> +them via serial/network .
> +Whe enabled, this enlightenment provides additional communication facilities
When
> +to the guest: SynDbg messages.
> +This new communication is used by Windows Kernel debugger rather than sending
> +packets via serial/network, adding significant performance boost over the other
> +comm channels.
> +This enlightenment requires a VMBus device (-device vmbus-bridge,irq=15)
> +and the follow enlightenments to work:
> +hv-relaxed,hv_time,hv-vapic,hv-vpindex,hv-synic,hv-runtime,hv-stimer
> +
> +
> 4. Supplementary features
> =========================
>
> diff --git a/hw/hyperv/hyperv.c b/hw/hyperv/hyperv.c
> index 88c9cc1334..c86e2aa02e 100644
> --- a/hw/hyperv/hyperv.c
> +++ b/hw/hyperv/hyperv.c
> @@ -730,3 +730,245 @@ uint16_t hyperv_hcall_signal_event(uint64_t param, bool fast)
> }
> return HV_STATUS_INVALID_CONNECTION_ID;
> }
> +
> +static HvSynDbgHandler hv_syndbg_handler;
> +static void *hv_syndbg_context;
Add a line here between field and function definition.
> +void hyperv_set_syndbg_handler(HvSynDbgHandler handler, void *context)
> +{
> + assert(!hv_syndbg_handler);
> + hv_syndbg_handler = handler;
> + hv_syndbg_context = context;
> +}
> +
> +uint16_t hyperv_hcall_reset_dbg_session(uint64_t outgpa)
> +{
> + uint16_t ret;
> + HvSynDbgMsg msg;
> + struct hyperv_reset_debug_session_output *reset_dbg_session = NULL;
> + hwaddr len;
> +
> + if (!hv_syndbg_handler) {
> + ret = HV_STATUS_INVALID_HYPERCALL_CODE;
> + goto cleanup;
> + }
> +
> + len = sizeof(*reset_dbg_session);
> + reset_dbg_session = cpu_physical_memory_map(outgpa, &len, 1);
> + if (!reset_dbg_session || len < sizeof(*reset_dbg_session)) {
> + ret = HV_STATUS_INSUFFICIENT_MEMORY;
> + goto cleanup;
> + }
> +
> + msg.type = HV_SYNDBG_MSG_CONNECTION_INFO;
> + ret = hv_syndbg_handler(hv_syndbg_context, &msg);
> + if (ret) {
> + goto cleanup;
> + }
> +
> + reset_dbg_session->host_ip = msg.u.connection_info.host_ip;
> + reset_dbg_session->host_port = msg.u.connection_info.host_port;
> + /* The following fields are only used as validation for KDVM */
> + memset(&reset_dbg_session->host_mac, 0,
> + sizeof(reset_dbg_session->host_mac));
> + reset_dbg_session->target_ip = msg.u.connection_info.host_ip;
> + reset_dbg_session->target_port = msg.u.connection_info.host_port;
> + memset(&reset_dbg_session->target_mac, 0,
> + sizeof(reset_dbg_session->target_mac));
> +cleanup:
> + if (reset_dbg_session) {
> + cpu_physical_memory_unmap(reset_dbg_session,
> + sizeof(*reset_dbg_session), 1, len);
> + }
> +
> + return ret;
> +}
> +
> +uint16_t hyperv_hcall_retreive_dbg_data(uint64_t ingpa, uint64_t outgpa,
> + bool fast)
> +{
> + uint16_t ret;
> + struct hyperv_retrieve_debug_data_input *debug_data_in = NULL;
> + struct hyperv_retrieve_debug_data_output *debug_data_out = NULL;
> + hwaddr in_len, out_len;
> + HvSynDbgMsg msg;
> +
> + if (fast || !hv_syndbg_handler) {
> + ret = HV_STATUS_INVALID_HYPERCALL_CODE;
> + goto cleanup;
> + }
> +
> + in_len = sizeof(*debug_data_in);
> + debug_data_in = cpu_physical_memory_map(ingpa, &in_len, 0);
> + if (!debug_data_in || in_len < sizeof(*debug_data_in)) {
> + ret = HV_STATUS_INSUFFICIENT_MEMORY;
> + goto cleanup;
> + }
> +
> + out_len = sizeof(*debug_data_out);
> + debug_data_out = cpu_physical_memory_map(outgpa, &out_len, 1);
> + if (!debug_data_out || out_len < sizeof(*debug_data_out)) {
> + ret = HV_STATUS_INSUFFICIENT_MEMORY;
> + goto cleanup;
> + }
> +
> + msg.type = HV_SYNDBG_MSG_RECV;
> + msg.u.recv.buf_gpa = outgpa + sizeof(*debug_data_out);
> + msg.u.recv.count = TARGET_PAGE_SIZE - sizeof(*debug_data_out);
> + msg.u.recv.options = debug_data_in->options;
> + msg.u.recv.timeout = debug_data_in->timeout;
> + msg.u.recv.is_raw = true;
> + ret = hv_syndbg_handler(hv_syndbg_context, &msg);
> + if (ret == HV_STATUS_NO_DATA) {
> + debug_data_out->retrieved_count = 0;
> + debug_data_out->remaining_count = debug_data_in->count;
> + goto cleanup;
> + } else if (ret != HV_STATUS_SUCCESS) {
> + goto cleanup;
> + }
> +
> + debug_data_out->retrieved_count = msg.u.recv.retrieved_count;
> + debug_data_out->remaining_count =
> + debug_data_in->count - msg.u.recv.retrieved_count;
> +cleanup:
> + if (debug_data_out) {
> + cpu_physical_memory_unmap(debug_data_out, sizeof(*debug_data_out), 1,
> + out_len);
> + }
> +
> + if (debug_data_in) {
> + cpu_physical_memory_unmap(debug_data_in, sizeof(*debug_data_in), 0,
> + in_len);
> + }
> +
> + return ret;
> +}
> +
> +uint16_t hyperv_hcall_post_dbg_data(uint64_t ingpa, uint64_t outgpa, bool fast)
> +{
> + uint16_t ret;
> + struct hyperv_post_debug_data_input *post_data_in = NULL;
> + struct hyperv_post_debug_data_output *post_data_out = NULL;
> + hwaddr in_len, out_len;
> + HvSynDbgMsg msg;
> +
> + if (fast || !hv_syndbg_handler) {
> + ret = HV_STATUS_INVALID_HYPERCALL_CODE;
> + goto cleanup;
> + }
> +
> + in_len = sizeof(*post_data_in);
> + post_data_in = cpu_physical_memory_map(ingpa, &in_len, 0);
> + if (!post_data_in || in_len < sizeof(*post_data_in)) {
> + ret = HV_STATUS_INSUFFICIENT_MEMORY;
> + goto cleanup;
> + }
> +
> + if (post_data_in->count > TARGET_PAGE_SIZE - sizeof(*post_data_in)) {
> + ret = HV_STATUS_INVALID_PARAMETER;
> + goto cleanup;
> + }
> +
> + out_len = sizeof(*post_data_out);
> + post_data_out = cpu_physical_memory_map(outgpa, &out_len, 1);
> + if (!post_data_out || out_len < sizeof(*post_data_out)) {
> + ret = HV_STATUS_INSUFFICIENT_MEMORY;
> + goto cleanup;
> + }
> +
> + msg.type = HV_SYNDBG_MSG_SEND;
> + msg.u.send.buf_gpa = ingpa + sizeof(*post_data_in);
> + msg.u.send.count = post_data_in->count;
> + msg.u.send.is_raw = true;
> + ret = hv_syndbg_handler(hv_syndbg_context, &msg);
> + if (ret != HV_STATUS_SUCCESS) {
> + goto cleanup;
> + }
> +
> + post_data_out->pending_count = msg.u.send.pending_count;
> + ret = post_data_out->pending_count ? HV_STATUS_INSUFFICIENT_BUFFERS :
> + HV_STATUS_SUCCESS;
> +cleanup:
> + if (post_data_out) {
> + cpu_physical_memory_unmap(post_data_out,
> + sizeof(*post_data_out), 1, out_len);
> + }
> +
> + if (post_data_in) {
> + cpu_physical_memory_unmap(post_data_in,
> + sizeof(*post_data_in), 0, in_len);
> + }
> +
> + return ret;
> +}
> +
> +uint32_t hyperv_syndbg_send(uint64_t ingpa, uint32_t count)
> +{
> + HvSynDbgMsg msg;
> +
> + if (!hv_syndbg_handler) {
> + return HV_SYNDBG_STATUS_INVALID;
> + }
> +
> + msg.type = HV_SYNDBG_MSG_SEND;
> + msg.u.send.buf_gpa = ingpa;
> + msg.u.send.count = count;
> + msg.u.send.is_raw = false;
> + if (hv_syndbg_handler(hv_syndbg_context, &msg)) {
> + return HV_SYNDBG_STATUS_INVALID;
> + }
> +
> + return HV_SYNDBG_STATUS_SEND_SUCCESS;
> +}
> +
> +uint32_t hyperv_syndbg_recv(uint64_t ingpa, uint32_t count)
> +{
> + uint16_t ret;
> + HvSynDbgMsg msg;
> +
> + if (!hv_syndbg_handler) {
> + return HV_SYNDBG_STATUS_INVALID;
> + }
> +
> + msg.type = HV_SYNDBG_MSG_RECV;
> + msg.u.recv.buf_gpa = ingpa;
> + msg.u.recv.count = count;
> + msg.u.recv.options = 0;
> + msg.u.recv.timeout = 0;
> + msg.u.recv.is_raw = false;
> + ret = hv_syndbg_handler(hv_syndbg_context, &msg);
> + if (ret != HV_STATUS_SUCCESS) {
> + return 0;
> + }
> +
> + return HV_SYNDBG_STATUS_SET_SIZE(HV_SYNDBG_STATUS_RECV_SUCCESS,
> + msg.u.recv.retrieved_count);
> +}
> +
> +void hyperv_syndbg_set_pending_page(uint64_t ingpa)
> +{
> + HvSynDbgMsg msg;
> +
> + if (!hv_syndbg_handler) {
> + return;
> + }
> +
> + msg.type = HV_SYNDBG_MSG_SET_PENDING_PAGE;
> + msg.u.pending_page.buf_gpa = ingpa;
> + hv_syndbg_handler(hv_syndbg_context, &msg);
> +}
> +
> +uint64_t hyperv_syndbg_query_options(void)
> +{
> + HvSynDbgMsg msg;
> +
> + if (!hv_syndbg_handler) {
> + return 0;
> + }
> +
> + msg.type = HV_SYNDBG_MSG_QUERY_OPTIONS;
> + if (hv_syndbg_handler(hv_syndbg_context, &msg) != HV_STATUS_SUCCESS) {
> + return 0;
> + }
> +
> + return msg.u.query_options.options;
> +}
> diff --git a/include/hw/hyperv/hyperv.h b/include/hw/hyperv/hyperv.h
> index ef9f6b6c09..e7a85156b0 100644
> --- a/include/hw/hyperv/hyperv.h
> +++ b/include/hw/hyperv/hyperv.h
> @@ -83,4 +83,62 @@ void hyperv_synic_update(CPUState *cs, bool enable,
> hwaddr msg_page_addr, hwaddr event_page_addr);
> bool hyperv_is_synic_enabled(void);
>
> +/*
> + * Process HVCALL_RESET_DEBUG_SESSION hypercall.
> + */
> +uint16_t hyperv_hcall_reset_dbg_session(uint64_t outgpa);
> +/*
> + * Process HVCALL_RETREIVE_DEBUG_DATA hypercall.
> + */
> +uint16_t hyperv_hcall_retreive_dbg_data(uint64_t ingpa, uint64_t outgpa,
> + bool fast);
> +/*
> + * Process HVCALL_POST_DEBUG_DATA hypercall.
> + */
> +uint16_t hyperv_hcall_post_dbg_data(uint64_t ingpa, uint64_t outgpa, bool fast);
> +
> +uint32_t hyperv_syndbg_send(uint64_t ingpa, uint32_t count);
> +uint32_t hyperv_syndbg_recv(uint64_t ingpa, uint32_t count);
> +void hyperv_syndbg_set_pending_page(uint64_t ingpa);
> +uint64_t hyperv_syndbg_query_options(void);
> +
> +typedef enum HvSynthDbgMsgType {
> + HV_SYNDBG_MSG_CONNECTION_INFO,
> + HV_SYNDBG_MSG_SEND,
> + HV_SYNDBG_MSG_RECV,
> + HV_SYNDBG_MSG_SET_PENDING_PAGE,
> + HV_SYNDBG_MSG_QUERY_OPTIONS
> +} HvDbgSynthMsgType;
> +
> +typedef struct HvSynDbgMsg {
> + HvDbgSynthMsgType type;
> + union {
> + struct {
> + uint32_t host_ip;
> + uint16_t host_port;
> + } connection_info;
> + struct {
> + uint64_t buf_gpa;
> + uint32_t count;
> + uint32_t pending_count;
> + bool is_raw;
> + } send;
> + struct {
> + uint64_t buf_gpa;
> + uint32_t count;
> + uint32_t options;
> + uint64_t timeout;
> + uint32_t retrieved_count;
> + bool is_raw;
> + } recv;
> + struct {
> + uint64_t buf_gpa;
> + } pending_page;
> + struct {
> + uint64_t options;
> + } query_options;
> + } u;
> +} HvSynDbgMsg;
> +typedef uint16_t (*HvSynDbgHandler)(void *context, HvSynDbgMsg *msg);
> +void hyperv_set_syndbg_handler(HvSynDbgHandler handler, void *context);
> #endif
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index aa9e636800..9529a6389a 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -6841,6 +6841,8 @@ static Property x86_cpu_properties[] = {
> HYPERV_FEAT_AVIC, 0),
> DEFINE_PROP_ON_OFF_AUTO("hv-no-nonarch-coresharing", X86CPU,
> hyperv_no_nonarch_cs, ON_OFF_AUTO_OFF),
> + DEFINE_PROP_BIT64("hv-syndbg", X86CPU, hyperv_features,
> + HYPERV_FEAT_SYNDBG, 0),
> 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/cpu.h b/target/i386/cpu.h
> index 9911d7c871..56e0317924 100644
> --- a/target/i386/cpu.h
> +++ b/target/i386/cpu.h
> @@ -1060,6 +1060,7 @@ typedef uint64_t FeatureWordArray[FEATURE_WORDS];
> #define HYPERV_FEAT_IPI 13
> #define HYPERV_FEAT_STIMER_DIRECT 14
> #define HYPERV_FEAT_AVIC 15
> +#define HYPERV_FEAT_SYNDBG 16
>
> #ifndef HYPERV_SPINLOCK_NEVER_NOTIFY
> #define HYPERV_SPINLOCK_NEVER_NOTIFY 0xFFFFFFFF
> @@ -1560,6 +1561,12 @@ typedef struct CPUX86State {
> uint64_t msr_hv_hypercall;
> uint64_t msr_hv_guest_os_id;
> uint64_t msr_hv_tsc;
> + uint64_t msr_hv_syndbg_control;
> + uint64_t msr_hv_syndbg_status;
> + uint64_t msr_hv_syndbg_send_page;
> + uint64_t msr_hv_syndbg_recv_page;
> + uint64_t msr_hv_syndbg_pending_page;
> + uint64_t msr_hv_syndbg_options;
>
> /* Per-VCPU HV MSRs */
> uint64_t msr_hv_vapic;
> diff --git a/target/i386/kvm/hyperv-stub.c b/target/i386/kvm/hyperv-stub.c
> index 0028527e79..778ed782e6 100644
> --- a/target/i386/kvm/hyperv-stub.c
> +++ b/target/i386/kvm/hyperv-stub.c
> @@ -27,6 +27,12 @@ int kvm_hv_handle_exit(X86CPU *cpu, struct kvm_hyperv_exit *exit)
> return 0;
> case KVM_EXIT_HYPERV_HCALL:
> exit->u.hcall.result = HV_STATUS_INVALID_HYPERCALL_CODE;
> + return 0;
> + case KVM_EXIT_HYPERV_SYNDBG:
> + if (!hyperv_feat_enabled(cpu, HYPERV_FEAT_SYNDBG)) {
> + return -1;
> + }
> +
> return 0;
> default:
> return -1;
> diff --git a/target/i386/kvm/hyperv.c b/target/i386/kvm/hyperv.c
> index 26efc1e0e6..a70f695205 100644
> --- a/target/i386/kvm/hyperv.c
> +++ b/target/i386/kvm/hyperv.c
> @@ -81,20 +81,66 @@ int kvm_hv_handle_exit(X86CPU *cpu, struct kvm_hyperv_exit *exit)
> case KVM_EXIT_HYPERV_HCALL: {
> uint16_t code = exit->u.hcall.input & 0xffff;
> bool fast = exit->u.hcall.input & HV_HYPERCALL_FAST;
> - uint64_t param = exit->u.hcall.params[0];
> + uint64_t in_param = exit->u.hcall.params[0];
> + uint64_t out_param = exit->u.hcall.params[1];
>
> switch (code) {
> case HV_POST_MESSAGE:
> - exit->u.hcall.result = hyperv_hcall_post_message(param, fast);
> + exit->u.hcall.result = hyperv_hcall_post_message(in_param, fast);
> break;
> case HV_SIGNAL_EVENT:
> - exit->u.hcall.result = hyperv_hcall_signal_event(param, fast);
> + exit->u.hcall.result = hyperv_hcall_signal_event(in_param, fast);
> + break;
> + case HV_POST_DEBUG_DATA:
> + exit->u.hcall.result =
> + hyperv_hcall_post_dbg_data(in_param, out_param, fast);
> + break;
> + case HV_RETREIVE_DEBUG_DATA:
> + exit->u.hcall.result =
> + hyperv_hcall_retreive_dbg_data(in_param, out_param, fast);
> + break;
> + case HV_RESET_DEBUG_SESSION:
> + exit->u.hcall.result =
> + hyperv_hcall_reset_dbg_session(out_param);
> break;
> default:
> exit->u.hcall.result = HV_STATUS_INVALID_HYPERCALL_CODE;
> }
> return 0;
> }
> +
> + case KVM_EXIT_HYPERV_SYNDBG:
> + if (!hyperv_feat_enabled(cpu, HYPERV_FEAT_SYNDBG)) {
> + return -1;
> + }
> +
> + switch (exit->u.syndbg.msr) {
> + case HV_X64_MSR_SYNDBG_CONTROL: {
> + uint64_t control = exit->u.syndbg.control;
> + env->msr_hv_syndbg_control = control;
> + env->msr_hv_syndbg_send_page = exit->u.syndbg.send_page;
> + env->msr_hv_syndbg_recv_page = exit->u.syndbg.recv_page;
> + exit->u.syndbg.status = HV_STATUS_SUCCESS;
> + if (control & HV_SYNDBG_CONTROL_SEND) {
> + exit->u.syndbg.status =
> + hyperv_syndbg_send(env->msr_hv_syndbg_send_page,
> + HV_SYNDBG_CONTROL_SEND_SIZE(control));
> + } else if (control & HV_SYNDBG_CONTROL_RECV) {
> + exit->u.syndbg.status =
> + hyperv_syndbg_recv(env->msr_hv_syndbg_recv_page,
> + TARGET_PAGE_SIZE);
> + }
> + break;
> + }
> + case HV_X64_MSR_SYNDBG_PENDING_BUFFER:
> + env->msr_hv_syndbg_pending_page = exit->u.syndbg.pending_page;
> + hyperv_syndbg_set_pending_page(env->msr_hv_syndbg_pending_page);
> + break;
> + default:
> + return -1;
> + }
> +
> + return 0;
> default:
> return -1;
> }
> diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
> index 2c8feb4a6f..ecabb332d7 100644
> --- a/target/i386/kvm/kvm.c
> +++ b/target/i386/kvm/kvm.c
> @@ -102,6 +102,7 @@ static bool has_msr_hv_synic;
> static bool has_msr_hv_stimer;
> static bool has_msr_hv_frequencies;
> static bool has_msr_hv_reenlightenment;
> +static bool has_msr_hv_syndbg_options;
> static bool has_msr_xss;
> static bool has_msr_umwait;
> static bool has_msr_spec_ctrl;
> @@ -932,6 +933,14 @@ static struct {
> .bits = HV_DEPRECATING_AEOI_RECOMMENDED}
> }
> },
> + [HYPERV_FEAT_SYNDBG] = {
> + .desc = "Enable synthetic kernel debugger channel (hv-syndbg)",
> + .flags = {
> + {.func = HV_CPUID_FEATURES, .reg = R_EDX,
> + .bits = HV_FEATURE_DEBUG_MSRS_AVAILABLE}
> + },
> + .dependencies = BIT(HYPERV_FEAT_SYNIC) | BIT(HYPERV_FEAT_RELAXED)
> + },
> };
>
> static struct kvm_cpuid2 *try_get_hv_cpuid(CPUState *cs, int max,
> @@ -972,8 +981,8 @@ static struct kvm_cpuid2 *try_get_hv_cpuid(CPUState *cs, int max,
> static struct kvm_cpuid2 *get_supported_hv_cpuid(CPUState *cs)
> {
> struct kvm_cpuid2 *cpuid;
> - /* 0x40000000..0x40000005, 0x4000000A, 0x40000080..0x40000080 leaves */
> - int max = 10;
> + /* 0x40000000..0x40000005, 0x4000000A, 0x40000080..0x40000082 leaves */
> + int max = 11;
> int i;
> bool do_sys_ioctl;
>
> @@ -1086,6 +1095,12 @@ static struct kvm_cpuid2 *get_supported_hv_cpuid_legacy(CPUState *cs)
> entry_feat->eax |= HV_SYNTIMERS_AVAILABLE;
> }
>
> + if (has_msr_hv_syndbg_options) {
> + entry_feat->edx |= HV_GUEST_DEBUGGING_AVAILABLE;
> + entry_feat->edx |= HV_FEATURE_DEBUG_MSRS_AVAILABLE;
> + entry_feat->ebx |= HV_PARTITION_DEUBGGING_ALLOWED;
> + }
> +
> if (kvm_check_extension(cs->kvm_state,
> KVM_CAP_HYPERV_TLBFLUSH) > 0) {
> entry_recomm->eax |= HV_REMOTE_TLB_FLUSH_RECOMMENDED;
> @@ -1337,12 +1352,22 @@ static int hyperv_fill_cpuids(CPUState *cs,
> {
> X86CPU *cpu = X86_CPU(cs);
> struct kvm_cpuid_entry2 *c;
> - uint32_t cpuid_i = 0;
> + uint32_t signature[3];
> + uint32_t cpuid_i = 0, max_cpuid_leaf = 0;
> +
> + max_cpuid_leaf = HV_CPUID_IMPLEMENT_LIMITS;
> + if (hyperv_feat_enabled(cpu, HYPERV_FEAT_EVMCS)) {
> + max_cpuid_leaf = MAX(max_cpuid_leaf, HV_CPUID_NESTED_FEATURES);
> + }
> +
> + if (hyperv_feat_enabled(cpu, HYPERV_FEAT_SYNDBG)) {
> + max_cpuid_leaf =
> + MAX(max_cpuid_leaf, HV_CPUID_SYNDBG_PLATFORM_CAPABILITIES);
> + }
>
> c = &cpuid_ent[cpuid_i++];
> c->function = HV_CPUID_VENDOR_AND_MAX_FUNCTIONS;
> - c->eax = hyperv_feat_enabled(cpu, HYPERV_FEAT_EVMCS) ?
> - HV_CPUID_NESTED_FEATURES : HV_CPUID_IMPLEMENT_LIMITS;
> + c->eax = max_cpuid_leaf;
> c->ebx = cpu->hyperv_vendor_id[0];
> c->ecx = cpu->hyperv_vendor_id[1];
> c->edx = cpu->hyperv_vendor_id[2];
> @@ -1421,6 +1446,33 @@ static int hyperv_fill_cpuids(CPUState *cs,
> c->eax = cpu->hyperv_nested[0];
> }
>
> + if (hyperv_feat_enabled(cpu, HYPERV_FEAT_SYNDBG)) {
> + c = &cpuid_ent[cpuid_i++];
> + c->function = HV_CPUID_SYNDBG_VENDOR_AND_MAX_FUNCTIONS;
> + c->eax = hyperv_feat_enabled(cpu, HYPERV_FEAT_EVMCS) ?
> + HV_CPUID_NESTED_FEATURES : HV_CPUID_IMPLEMENT_LIMITS;
> + memcpy(signature, "Microsoft VS", 12);
> + c->eax = 0;
> + c->ebx = signature[0];
> + c->ecx = signature[1];
> + c->edx = signature[2];
> +
> + c = &cpuid_ent[cpuid_i++];
> + c->function = HV_CPUID_SYNDBG_INTERFACE;
> + memcpy(signature, "VS#1\0\0\0\0\0\0\0\0", 12);
> + c->eax = signature[0];
> + c->ebx = 0;
> + c->ecx = 0;
> + c->edx = 0;
> +
> + c = &cpuid_ent[cpuid_i++];
> + c->function = HV_CPUID_SYNDBG_PLATFORM_CAPABILITIES;
> + c->eax = HV_SYNDBG_CAP_ALLOW_KERNEL_DEBUGGING;
> + c->ebx = 0;
> + c->ecx = 0;
> + c->edx = 0;
> + }
> +
> return cpuid_i;
> }
>
> @@ -2215,6 +2267,9 @@ static int kvm_get_supported_msrs(KVMState *s)
> case HV_X64_MSR_REENLIGHTENMENT_CONTROL:
> has_msr_hv_reenlightenment = true;
> break;
> + case HV_X64_MSR_SYNDBG_OPTIONS:
> + has_msr_hv_syndbg_options = true;
> + break;
> case MSR_IA32_SPEC_CTRL:
> has_msr_spec_ctrl = true;
> break;
> @@ -3132,6 +3187,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);
> }
> + 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());
> + }
> }
> if (hyperv_feat_enabled(cpu, HYPERV_FEAT_VAPIC)) {
> kvm_msr_entry_add(cpu, HV_X64_MSR_APIC_ASSIST_PAGE,
> @@ -3565,6 +3625,9 @@ static int kvm_get_msrs(X86CPU *cpu)
> kvm_msr_entry_add(cpu, HV_X64_MSR_TSC_EMULATION_CONTROL, 0);
> kvm_msr_entry_add(cpu, HV_X64_MSR_TSC_EMULATION_STATUS, 0);
> }
> + if (has_msr_hv_syndbg_options) {
> + kvm_msr_entry_add(cpu, HV_X64_MSR_SYNDBG_OPTIONS, 0);
> + }
> if (has_msr_hv_crash) {
> int j;
>
> @@ -3851,6 +3914,9 @@ static int kvm_get_msrs(X86CPU *cpu)
> case HV_X64_MSR_TSC_EMULATION_STATUS:
> env->msr_hv_tsc_emulation_status = msrs[i].data;
> break;
> + case HV_X64_MSR_SYNDBG_OPTIONS:
> + env->msr_hv_syndbg_options = msrs[i].data;
> + break;
> case MSR_MTRRdefType:
> env->mtrr_deftype = msrs[i].data;
> break;
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v1 4/4] hw: hyperv: Initial commit for Synthetic Debugging device
2022-02-04 10:07 ` [PATCH v1 4/4] hw: hyperv: Initial commit for Synthetic Debugging device Jon Doron
@ 2022-02-16 9:11 ` Emanuele Giuseppe Esposito
2022-02-16 10:28 ` Jon Doron
0 siblings, 1 reply; 14+ messages in thread
From: Emanuele Giuseppe Esposito @ 2022-02-16 9:11 UTC (permalink / raw)
To: Jon Doron, qemu-devel; +Cc: pbonzini, vkuznets
> +
> +static uint16_t handle_recv_msg(HvSynDbg *syndbg, uint64_t outgpa,
> + uint32_t count, bool is_raw, uint32_t options,
> + uint64_t timeout, uint32_t *retrieved_count)
> +{
> + uint16_t ret;
> + uint8_t data_buf[TARGET_PAGE_SIZE - UDP_PKT_HEADER_SIZE];
> + hwaddr out_len;
> + void *out_data = NULL;
> + ssize_t recv_byte_count;
> +
> + /* TODO: Handle options and timeout */
> + (void)options;
> + (void)timeout;
> +
> + if (!syndbg->has_data_pending) {
> + recv_byte_count = 0;
> + } else {
> + recv_byte_count = qemu_recv(syndbg->socket, data_buf,
> + MIN(sizeof(data_buf), count), MSG_WAITALL);
> + if (recv_byte_count == -1) {
> + ret = HV_STATUS_INVALID_PARAMETER;
> + goto cleanup;
> + }
> + }
> +
> + if (!recv_byte_count) {
> + *retrieved_count = 0;
> + ret = HV_STATUS_NO_DATA;
> + goto cleanup;
> + }
> +
> + set_pending_state(syndbg, false);
> +
> + out_len = recv_byte_count;
> + if (is_raw) {
> + out_len += UDP_PKT_HEADER_SIZE;
> + }
> + out_data = cpu_physical_memory_map(outgpa, &out_len, 1);
> + if (!out_data) {
> + ret = HV_STATUS_INSUFFICIENT_MEMORY;
> + goto cleanup;
> + }
> +
> + if (is_raw &&
> + !create_udp_pkt(syndbg, out_data,
> + recv_byte_count + UDP_PKT_HEADER_SIZE,
> + data_buf, recv_byte_count)) {
> + ret = HV_STATUS_INSUFFICIENT_MEMORY;
> + goto cleanup;
> + } else if (!is_raw) {
> + memcpy(out_data, data_buf, recv_byte_count);
> + }
> +
> + *retrieved_count = recv_byte_count;
> + if (is_raw) {
> + *retrieved_count += UDP_PKT_HEADER_SIZE;
> + }
> + ret = HV_STATUS_SUCCESS;
> +cleanup:
> + if (out_data) {
> + cpu_physical_memory_unmap(out_data, out_len, 1, out_len);
> + }
Same nitpick as done in patch 1, I think you can use more gotos labels
instead of adding if statements.
> +
> + return ret;
> +}
> +
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v1 1/4] hyperv: SControl is optional to enable SynIc
2022-02-16 9:10 ` Emanuele Giuseppe Esposito
@ 2022-02-16 10:27 ` Jon Doron
0 siblings, 0 replies; 14+ messages in thread
From: Jon Doron @ 2022-02-16 10:27 UTC (permalink / raw)
To: Emanuele Giuseppe Esposito; +Cc: Paolo Bonzini, Vitaly Kuznetsov, qemu-devel
On 16/02/2022, Emanuele Giuseppe Esposito wrote:
>
>
>On 04/02/2022 11:07, Jon Doron wrote:
>> SynIc can be enabled regardless of the SControl mechanisim which can
>> register a GSI for a given SintRoute.
>>
>> This behaviour can achived by setting enabling SIMP and then the guest
>> will poll on the message slot.
>>
>> Once there is another message pending the host will set the message slot
>> with the pending flag.
>> When the guest polls from the message slot, incase the pending flag is
>
>s/incase/in case
Done
>> set it will write to the HV_X64_MSR_EOM indicating it has cleared the
>> slow and we can try and push our message again.
>
>what do you mean by "the slow"?
Just a typo to slot :) fixed
>>
>> Signed-off-by: Jon Doron <arilou@gmail.com>
>> ---
>> hw/hyperv/hyperv.c | 233 ++++++++++++++++++++++++-------------
>> include/hw/hyperv/hyperv.h | 2 +
>> 2 files changed, 153 insertions(+), 82 deletions(-)
>>
>> diff --git a/hw/hyperv/hyperv.c b/hw/hyperv/hyperv.c
>> index cb1074f234..88c9cc1334 100644
>> --- a/hw/hyperv/hyperv.c
>> +++ b/hw/hyperv/hyperv.c
>> @@ -27,18 +27,70 @@ struct SynICState {
>>
>> CPUState *cs;
>>
>> - bool enabled;
>> + bool sctl_enabled;
>> hwaddr msg_page_addr;
>> hwaddr event_page_addr;
>> MemoryRegion msg_page_mr;
>> MemoryRegion event_page_mr;
>> struct hyperv_message_page *msg_page;
>> struct hyperv_event_flags_page *event_page;
>> +
>> + QemuMutex sint_routes_mutex;
>> + QLIST_HEAD(, HvSintRoute) sint_routes;
>> };
>>
>> #define TYPE_SYNIC "hyperv-synic"
>> OBJECT_DECLARE_SIMPLE_TYPE(SynICState, SYNIC)
>>
>> +/*
>> + * KVM has its own message producers (SynIC timers). To guarantee
>> + * serialization with both KVM vcpu and the guest cpu, the messages are first
>> + * staged in an intermediate area and then posted to the SynIC message page in
>> + * the vcpu thread.
>> + */
>> +typedef struct HvSintStagedMessage {
>> + /* message content staged by hyperv_post_msg */
>> + struct hyperv_message msg;
>> + /* callback + data (r/o) to complete the processing in a BH */
>> + HvSintMsgCb cb;
>> + void *cb_data;
>> + /* message posting status filled by cpu_post_msg */
>> + int status;
>> + /* passing the buck: */
>> + enum {
>> + /* initial state */
>> + HV_STAGED_MSG_FREE,
>> + /*
>> + * hyperv_post_msg (e.g. in main loop) grabs the staged area (FREE ->
>> + * BUSY), copies msg, and schedules cpu_post_msg on the assigned cpu
>> + */
>> + HV_STAGED_MSG_BUSY,
>> + /*
>> + * cpu_post_msg (vcpu thread) tries to copy staged msg to msg slot,
>> + * notify the guest, records the status, marks the posting done (BUSY
>> + * -> POSTED), and schedules sint_msg_bh BH
>> + */
>> + HV_STAGED_MSG_POSTED,
>> + /*
>> + * sint_msg_bh (BH) verifies that the posting is done, runs the
>> + * callback, and starts over (POSTED -> FREE)
>> + */
>> + } state;
>> +} HvSintStagedMessage;
>> +
>> +struct HvSintRoute {
>> + uint32_t sint;
>> + SynICState *synic;
>> + int gsi;
>> + EventNotifier sint_set_notifier;
>> + EventNotifier sint_ack_notifier;
>> +
>> + HvSintStagedMessage *staged_msg;
>> +
>> + unsigned refcount;
>> + QLIST_ENTRY(HvSintRoute) link;
>> +};
>> +
>> static bool synic_enabled;
>
>Why did you move this struct above?
>I think it was done purposefully to separate synic_* functions from the
>others below (sint_*).
Done
>>
>> bool hyperv_is_synic_enabled(void)
>> @@ -51,11 +103,11 @@ static SynICState *get_synic(CPUState *cs)
>> return SYNIC(object_resolve_path_component(OBJECT(cs), "synic"));
>> }
>>
>> -static void synic_update(SynICState *synic, bool enable,
>> +static void synic_update(SynICState *synic, bool sctl_enable,
>> hwaddr msg_page_addr, hwaddr event_page_addr)
>> {
>>
>> - synic->enabled = enable;
>> + synic->sctl_enabled = sctl_enable;
>> if (synic->msg_page_addr != msg_page_addr) {
>> if (synic->msg_page_addr) {
>> memory_region_del_subregion(get_system_memory(),
>> @@ -80,7 +132,7 @@ static void synic_update(SynICState *synic, bool enable,
>> }
>> }
>>
>> -void hyperv_synic_update(CPUState *cs, bool enable,
>> +void hyperv_synic_update(CPUState *cs, bool sctl_enable,
>> hwaddr msg_page_addr, hwaddr event_page_addr)
>> {
>> SynICState *synic = get_synic(cs);
>> @@ -89,7 +141,7 @@ void hyperv_synic_update(CPUState *cs, bool enable,
>> return;
>> }
>>
>> - synic_update(synic, enable, msg_page_addr, event_page_addr);
>> + synic_update(synic, sctl_enable, msg_page_addr, event_page_addr);
>> }
>>
>> static void synic_realize(DeviceState *dev, Error **errp)
>> @@ -110,16 +162,20 @@ static void synic_realize(DeviceState *dev, Error **errp)
>> sizeof(*synic->event_page), &error_abort);
>> synic->msg_page = memory_region_get_ram_ptr(&synic->msg_page_mr);
>> synic->event_page = memory_region_get_ram_ptr(&synic->event_page_mr);
>> + qemu_mutex_init(&synic->sint_routes_mutex);
>> + QLIST_INIT(&synic->sint_routes);
>>
>> g_free(msgp_name);
>> g_free(eventp_name);
>> }
>> +
>> static void synic_reset(DeviceState *dev)
>> {
>> SynICState *synic = SYNIC(dev);
>> memset(synic->msg_page, 0, sizeof(*synic->msg_page));
>> memset(synic->event_page, 0, sizeof(*synic->event_page));
>> synic_update(synic, false, 0, 0);
>> + assert(QLIST_EMPTY(&synic->sint_routes));
>> }
>>
>> static void synic_class_init(ObjectClass *klass, void *data)
>> @@ -168,54 +224,6 @@ static void synic_register_types(void)
>>
>> type_init(synic_register_types)
>>
>> -/*
>> - * KVM has its own message producers (SynIC timers). To guarantee
>> - * serialization with both KVM vcpu and the guest cpu, the messages are first
>> - * staged in an intermediate area and then posted to the SynIC message page in
>> - * the vcpu thread.
>> - */
>> -typedef struct HvSintStagedMessage {
>> - /* message content staged by hyperv_post_msg */
>> - struct hyperv_message msg;
>> - /* callback + data (r/o) to complete the processing in a BH */
>> - HvSintMsgCb cb;
>> - void *cb_data;
>> - /* message posting status filled by cpu_post_msg */
>> - int status;
>> - /* passing the buck: */
>> - enum {
>> - /* initial state */
>> - HV_STAGED_MSG_FREE,
>> - /*
>> - * hyperv_post_msg (e.g. in main loop) grabs the staged area (FREE ->
>> - * BUSY), copies msg, and schedules cpu_post_msg on the assigned cpu
>> - */
>> - HV_STAGED_MSG_BUSY,
>> - /*
>> - * cpu_post_msg (vcpu thread) tries to copy staged msg to msg slot,
>> - * notify the guest, records the status, marks the posting done (BUSY
>> - * -> POSTED), and schedules sint_msg_bh BH
>> - */
>> - HV_STAGED_MSG_POSTED,
>> - /*
>> - * sint_msg_bh (BH) verifies that the posting is done, runs the
>> - * callback, and starts over (POSTED -> FREE)
>> - */
>> - } state;
>> -} HvSintStagedMessage;
>> -
>> -struct HvSintRoute {
>> - uint32_t sint;
>> - SynICState *synic;
>> - int gsi;
>> - EventNotifier sint_set_notifier;
>> - EventNotifier sint_ack_notifier;
>> -
>> - HvSintStagedMessage *staged_msg;
>> -
>> - unsigned refcount;
>> -};
>> -
>> static CPUState *hyperv_find_vcpu(uint32_t vp_index)
>> {
>> CPUState *cs = qemu_get_cpu(vp_index);
>> @@ -259,7 +267,7 @@ static void cpu_post_msg(CPUState *cs, run_on_cpu_data data)
>>
>> assert(staged_msg->state == HV_STAGED_MSG_BUSY);
>>
>> - if (!synic->enabled || !synic->msg_page_addr) {
>> + if (!synic->msg_page_addr) {
>
>Not sure if this is important, but why don't you check for
>!synic->sctl_enabled anymore here? You do it below.
>
I cannot check for sctl_enabled, there is a mode in which SCTL is not
set but the SynIC interface is still functioning simply with polling.
>> staged_msg->status = -ENXIO;
>> goto posted;
>> }
>> @@ -343,7 +351,7 @@ int hyperv_set_event_flag(HvSintRoute *sint_route, unsigned eventno)
>> if (eventno > HV_EVENT_FLAGS_COUNT) {
>> return -EINVAL;
>> }
>> - if (!synic->enabled || !synic->event_page_addr) {
>> + if (!synic->sctl_enabled || !synic->event_page_addr) {
>> return -ENXIO;
>> }
>>
>> @@ -364,11 +372,13 @@ int hyperv_set_event_flag(HvSintRoute *sint_route, unsigned eventno)
>> HvSintRoute *hyperv_sint_route_new(uint32_t vp_index, uint32_t sint,
>> HvSintMsgCb cb, void *cb_data)
>> {
>> - HvSintRoute *sint_route;
>> - EventNotifier *ack_notifier;
>> + HvSintRoute *sint_route = NULL;
>> + EventNotifier *ack_notifier = NULL;
>> int r, gsi;
>> CPUState *cs;
>> SynICState *synic;
>> + bool ack_event_initialized = false, sint_notifier_initialized = false,
>> + irqfd_initialized = false;
>>
>> cs = hyperv_find_vcpu(vp_index);
>> if (!cs) {
>> @@ -381,57 +391,82 @@ HvSintRoute *hyperv_sint_route_new(uint32_t vp_index, uint32_t sint,
>> }
>>
>> sint_route = g_new0(HvSintRoute, 1);
>> - r = event_notifier_init(&sint_route->sint_set_notifier, false);
>> - if (r) {
>> - goto err;
>> + if (!sint_route) {
>> + goto cleanup_err;
>> }
>>
>> + sint_route->gsi = 0;
>
>useless, as g_new0 zeroes all fields
>
Done
>> + sint_route->synic = synic;
>> + sint_route->sint = sint;
>> + sint_route->refcount = 1;
>>
>> ack_notifier = cb ? &sint_route->sint_ack_notifier : NULL;
>> if (ack_notifier) {
>> sint_route->staged_msg = g_new0(HvSintStagedMessage, 1);
>> + if (!sint_route->staged_msg) {
>> + goto cleanup_err;
>> + }
>> sint_route->staged_msg->cb = cb;
>> sint_route->staged_msg->cb_data = cb_data;
>>
>> r = event_notifier_init(ack_notifier, false);
>> if (r) {
>> - goto err_sint_set_notifier;
>> + goto cleanup_err;
>> }
>> -
>> event_notifier_set_handler(ack_notifier, sint_ack_handler);
>> + ack_event_initialized = true;
>> + }
>> +
>> + /* See if we are done or we need to setup a GSI for this SintRoute */
>> + if (!synic->sctl_enabled) {
>> + goto cleanup;
>> }
>>
>> + /* We need to setup a GSI for this SintRoute */
>> + r = event_notifier_init(&sint_route->sint_set_notifier, false);
>> + if (r) {
>> + goto cleanup_err;
>> + }
>> + sint_notifier_initialized = true;
>> +
>> gsi = kvm_irqchip_add_hv_sint_route(kvm_state, vp_index, sint);
>> if (gsi < 0) {
>> - goto err_gsi;
>> + goto cleanup_err;
>> }
>> + irqfd_initialized = true;
>>
>> r = kvm_irqchip_add_irqfd_notifier_gsi(kvm_state,
>> &sint_route->sint_set_notifier,
>> ack_notifier, gsi);
>> if (r) {
>> - goto err_irqfd;
>> + goto cleanup_err;
>> }
>> sint_route->gsi = gsi;
>> - sint_route->synic = synic;
>> - sint_route->sint = sint;
>> - sint_route->refcount = 1;
>> -
>> +cleanup:
>> + qemu_mutex_lock(&synic->sint_routes_mutex);
>> + QLIST_INSERT_HEAD_RCU(&synic->sint_routes, sint_route, link);
>> + qemu_mutex_unlock(&synic->sint_routes_mutex);
>> return sint_route;
>>
>> -err_irqfd:
>> - kvm_irqchip_release_virq(kvm_state, gsi);
>> -err_gsi:
>> +cleanup_err:
>> + if (irqfd_initialized) {
>> + kvm_irqchip_release_virq(kvm_state, gsi);
>> + }
>> +
>> + if (sint_notifier_initialized) {
>> + event_notifier_cleanup(&sint_route->sint_set_notifier);
>> + }
>> +
>> if (ack_notifier) {
>> - event_notifier_set_handler(ack_notifier, NULL);
>> - event_notifier_cleanup(ack_notifier);
>> + if (ack_event_initialized) {
>> + event_notifier_set_handler(ack_notifier, NULL);
>> + event_notifier_cleanup(ack_notifier);
>> + }
>> +
>> g_free(sint_route->staged_msg);
>> }
>> -err_sint_set_notifier:
>> - event_notifier_cleanup(&sint_route->sint_set_notifier);
>> -err:
>> - g_free(sint_route);
>>
>> + g_free(sint_route);
>> return NULL;
>> }
>
>It is good that you check that sint_route is not NULL, but I don't find
>it easy to read nor a common practice to have one single goto label and
>multiple booleans to distinguish various error cases.
>
>I think you should do as it was done before, with a specific label for
>each type of error, and not always use cleanup_err.
>
Done
>>
>> @@ -442,6 +477,8 @@ void hyperv_sint_route_ref(HvSintRoute *sint_route)
>>
>> void hyperv_sint_route_unref(HvSintRoute *sint_route)
>> {
>> + SynICState *synic;
>> +
>> if (!sint_route) {
>> return;
>> }
>> @@ -452,21 +489,33 @@ void hyperv_sint_route_unref(HvSintRoute *sint_route)
>> return;
>> }
>>
>> - kvm_irqchip_remove_irqfd_notifier_gsi(kvm_state,
>> - &sint_route->sint_set_notifier,
>> - sint_route->gsi);
>> - kvm_irqchip_release_virq(kvm_state, sint_route->gsi);
>> + synic = sint_route->synic;
>> + qemu_mutex_lock(&synic->sint_routes_mutex);
>> + QLIST_REMOVE_RCU(sint_route, link);
>> + qemu_mutex_unlock(&synic->sint_routes_mutex);
>
>Not really important, but you can use WITH_QEMU_LOCK_GUARD instead of
>doing lock/unlock.
>
Kept as is
>> +
>> + if (sint_route->gsi) {
>> + kvm_irqchip_remove_irqfd_notifier_gsi(kvm_state,
>> + &sint_route->sint_set_notifier,
>> + sint_route->gsi);
>> + kvm_irqchip_release_virq(kvm_state, sint_route->gsi);
>> + event_notifier_cleanup(&sint_route->sint_set_notifier);
>> + }
>> +
>> if (sint_route->staged_msg) {
>> event_notifier_set_handler(&sint_route->sint_ack_notifier, NULL);
>> event_notifier_cleanup(&sint_route->sint_ack_notifier);
>> g_free(sint_route->staged_msg);
>> }
>> - event_notifier_cleanup(&sint_route->sint_set_notifier);
>> g_free(sint_route);
>> }
>>
>> int hyperv_sint_route_set_sint(HvSintRoute *sint_route)
>> {
>> + if (!sint_route->gsi) {
>> + return 0;
>> + }
>> +
>> return event_notifier_set(&sint_route->sint_set_notifier);
>> }
>>
>> @@ -529,6 +578,26 @@ int hyperv_set_msg_handler(uint32_t conn_id, HvMsgHandler handler, void *data)
>> return ret;
>> }
>>
>> +int hyperv_synic_eom(CPUState *cs)
>> +{
>> + SynICState *synic = get_synic(cs);
>> + HvSintRoute *sint_route;
>> +
>> + if (!synic) {
>> + return -1;
>> + }
>
>use here
>QEMU_LOCK_GUARD(&synic->sint_routes_mutex);
>instead of lock/unlock
Removed the entire function, now that I realize that I dont need it.
>> +
>> + qemu_mutex_lock(&synic->sint_routes_mutex);
>> + QLIST_FOREACH(sint_route, &synic->sint_routes, link) {
>> + /* Try to complete every SintRoute */
>> + aio_bh_schedule_oneshot(qemu_get_aio_context(), sint_msg_bh,
>> + sint_route);
>> + }
>> + qemu_mutex_unlock(&synic->sint_routes_mutex);> + return 0;
>> +}
>> +
>> uint16_t hyperv_hcall_post_message(uint64_t param, bool fast)
>> {
>> uint16_t ret;
>> diff --git a/include/hw/hyperv/hyperv.h b/include/hw/hyperv/hyperv.h
>> index a63ee0003c..ef9f6b6c09 100644
>> --- a/include/hw/hyperv/hyperv.h
>> +++ b/include/hw/hyperv/hyperv.h
>> @@ -28,6 +28,8 @@ void hyperv_sint_route_unref(HvSintRoute *sint_route);
>>
>> int hyperv_sint_route_set_sint(HvSintRoute *sint_route);
>>
>> +int hyperv_synic_eom(CPUState *cs);
>> +
>
>Documentation here? Where is this function used?
>
Removed the entire function.
>> /*
>> * Submit a message to be posted in vcpu context. If the submission succeeds,
>> * the status of posting the message is reported via the callback associated
>>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v1 2/4] hyperv: Add definitions for syndbg
2022-02-16 9:10 ` Emanuele Giuseppe Esposito
@ 2022-02-16 10:27 ` Jon Doron
0 siblings, 0 replies; 14+ messages in thread
From: Jon Doron @ 2022-02-16 10:27 UTC (permalink / raw)
To: Emanuele Giuseppe Esposito; +Cc: pbonzini, vkuznets, qemu-devel
On 16/02/2022, Emanuele Giuseppe Esposito wrote:
>
>
>On 04/02/2022 11:07, Jon Doron wrote:
>> Add all required definitions for hyperv synthetic debugger interface.
>>
>> Signed-off-by: Jon Doron <arilou@gmail.com>
>> ---
>> include/hw/hyperv/hyperv-proto.h | 52 ++++++++++++++++++++++++++++++++
>> target/i386/kvm/hyperv-proto.h | 37 +++++++++++++++++++++++
>> 2 files changed, 89 insertions(+)
>>
>> diff --git a/include/hw/hyperv/hyperv-proto.h b/include/hw/hyperv/hyperv-proto.h
>> index 21dc28aee9..94c9658eb0 100644
>> --- a/include/hw/hyperv/hyperv-proto.h
>> +++ b/include/hw/hyperv/hyperv-proto.h
>> @@ -24,12 +24,17 @@
>> #define HV_STATUS_INVALID_PORT_ID 17
>> #define HV_STATUS_INVALID_CONNECTION_ID 18
>> #define HV_STATUS_INSUFFICIENT_BUFFERS 19
>> +#define HV_STATUS_NOT_ACKNOWLEDGED 20
>> +#define HV_STATUS_NO_DATA 27
>>
>> /*
>> * Hypercall numbers
>> */
>> #define HV_POST_MESSAGE 0x005c
>> #define HV_SIGNAL_EVENT 0x005d
>> +#define HV_POST_DEBUG_DATA 0x0069
>> +#define HV_RETREIVE_DEBUG_DATA 0x006a
>
>s/RETREIVE/RETRIEVE?
>
Done
>> +#define HV_RESET_DEBUG_SESSION 0x006b
>> #define HV_HYPERCALL_FAST (1u << 16)
>>
>> /*
>> @@ -127,4 +132,51 @@ struct hyperv_event_flags_page {
>> struct hyperv_event_flags slot[HV_SINT_COUNT];
>> };
>>
>> +/*
>> + * Kernel debugger structures
>> + */
>> +
>> +/* Options flags for hyperv_reset_debug_session */
>> +#define HV_DEBUG_PURGE_INCOMING_DATA 0x00000001
>> +#define HV_DEBUG_PURGE_OUTGOING_DATA 0x00000002
>> +struct hyperv_reset_debug_session_input {
>> + uint32_t options;
>> +} __attribute__ ((__packed__));
>> +
>> +struct hyperv_reset_debug_session_output {
>> + uint32_t host_ip;
>> + uint32_t target_ip;
>> + uint16_t host_port;
>> + uint16_t target_port;
>> + uint8_t host_mac[6];
>> + uint8_t target_mac[6];
>> +} __attribute__ ((__packed__));
>> +
>> +/* Options for hyperv_post_debug_data */
>> +#define HV_DEBUG_POST_LOOP 0x00000001
>> +
>> +struct hyperv_post_debug_data_input {
>> + uint32_t count;
>> + uint32_t options;
>
>> + /*uint8_t data[HV_HYP_PAGE_SIZE - 2 * sizeof(uint32_t)];*/
>
>What is this comment for?
>
It's a reference how the data really looks like.
>> +} __attribute__ ((__packed__));
>> +
>> +struct hyperv_post_debug_data_output {
>> + uint32_t pending_count;
>> +} __attribute__ ((__packed__));
>> +
>> +/* Options for hyperv_retrieve_debug_data */
>> +#define HV_DEBUG_RETRIEVE_LOOP 0x00000001
>> +#define HV_DEBUG_RETRIEVE_TEST_ACTIVITY 0x00000002
>> +
>> +struct hyperv_retrieve_debug_data_input {
>> + uint32_t count;
>> + uint32_t options;
>> + uint64_t timeout;
>> +} __attribute__ ((__packed__));
>> +
>> +struct hyperv_retrieve_debug_data_output {
>> + uint32_t retrieved_count;
>> + uint32_t remaining_count;
>> +} __attribute__ ((__packed__));
>> #endif
>> diff --git a/target/i386/kvm/hyperv-proto.h b/target/i386/kvm/hyperv-proto.h
>> index 89f81afda7..9480bcdf04 100644
>> --- a/target/i386/kvm/hyperv-proto.h
>> +++ b/target/i386/kvm/hyperv-proto.h
>> @@ -19,6 +19,9 @@
>> #define HV_CPUID_ENLIGHTMENT_INFO 0x40000004
>> #define HV_CPUID_IMPLEMENT_LIMITS 0x40000005
>> #define HV_CPUID_NESTED_FEATURES 0x4000000A
>> +#define HV_CPUID_SYNDBG_VENDOR_AND_MAX_FUNCTIONS 0x40000080
>> +#define HV_CPUID_SYNDBG_INTERFACE 0x40000081
>> +#define HV_CPUID_SYNDBG_PLATFORM_CAPABILITIES 0x40000082
>> #define HV_CPUID_MIN 0x40000005
>> #define HV_CPUID_MAX 0x4000ffff
>> #define HV_HYPERVISOR_PRESENT_BIT 0x80000000
>> @@ -55,8 +58,14 @@
>> #define HV_GUEST_IDLE_STATE_AVAILABLE (1u << 5)
>> #define HV_FREQUENCY_MSRS_AVAILABLE (1u << 8)
>> #define HV_GUEST_CRASH_MSR_AVAILABLE (1u << 10)
>> +#define HV_FEATURE_DEBUG_MSRS_AVAILABLE (1u << 11)
>> #define HV_STIMER_DIRECT_MODE_AVAILABLE (1u << 19)
>>
>> +/*
>> + * HV_CPUID_FEATURES.EBX bits
>> + */
>> +#define HV_PARTITION_DEUBGGING_ALLOWED (1u << 12)
>s/DEUBGGING/DEBUGGING
Done
>> +
>> /*
>> * HV_CPUID_ENLIGHTMENT_INFO.EAX bits
>> */
>> @@ -72,6 +81,11 @@
>> #define HV_ENLIGHTENED_VMCS_RECOMMENDED (1u << 14)
>> #define HV_NO_NONARCH_CORESHARING (1u << 18)
>>
>> +/*
>> + * HV_CPUID_SYNDBG_PLATFORM_CAPABILITIES.EAX bits
>> + */
>> +#define HV_SYNDBG_CAP_ALLOW_KERNEL_DEBUGGING (1u << 1)
>> +
>> /*
>> * Basic virtualized MSRs
>> */
>> @@ -130,6 +144,18 @@
>> #define HV_X64_MSR_STIMER3_CONFIG 0x400000B6
>> #define HV_X64_MSR_STIMER3_COUNT 0x400000B7
>>
>> +/*
>> + * Hyper-V Synthetic debug options MSR
>> + */
>> +#define HV_X64_MSR_SYNDBG_CONTROL 0x400000F1
>> +#define HV_X64_MSR_SYNDBG_STATUS 0x400000F2
>> +#define HV_X64_MSR_SYNDBG_SEND_BUFFER 0x400000F3
>> +#define HV_X64_MSR_SYNDBG_RECV_BUFFER 0x400000F4
>> +#define HV_X64_MSR_SYNDBG_PENDING_BUFFER 0x400000F5
>> +#define HV_X64_MSR_SYNDBG_OPTIONS 0x400000FF
>> +
>> +#define HV_X64_SYNDBG_OPTION_USE_HCALLS BIT(2)
>> +
>> /*
>> * Guest crash notification MSRs
>> */
>> @@ -168,5 +194,16 @@
>>
>> #define HV_STIMER_COUNT 4
>>
>> +/*
>> + * Synthetic debugger control definitions
>> + */
>> +#define HV_SYNDBG_CONTROL_SEND (1u << 0)
>> +#define HV_SYNDBG_CONTROL_RECV (1u << 1)
>> +#define HV_SYNDBG_CONTROL_SEND_SIZE(ctl) ((ctl >> 16) & 0xffff)
>> +#define HV_SYNDBG_STATUS_INVALID (0)
>> +#define HV_SYNDBG_STATUS_SEND_SUCCESS (1u << 0)
>> +#define HV_SYNDBG_STATUS_RECV_SUCCESS (1u << 2)
>> +#define HV_SYNDBG_STATUS_RESET (1u << 3)
>> +#define HV_SYNDBG_STATUS_SET_SIZE(st, sz) (st | (sz << 16))
>>
>> #endif
>>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v1 3/4] hyperv: Add support to process syndbg commands
2022-02-16 9:10 ` Emanuele Giuseppe Esposito
@ 2022-02-16 10:28 ` Jon Doron
0 siblings, 0 replies; 14+ messages in thread
From: Jon Doron @ 2022-02-16 10:28 UTC (permalink / raw)
To: Emanuele Giuseppe Esposito; +Cc: Paolo Bonzini, Vitaly Kuznetsov, qemu-devel
On 16/02/2022, Emanuele Giuseppe Esposito wrote:
>
>
>On 04/02/2022 11:07, Jon Doron wrote:
>> SynDbg commands can come from two different flows:
>> 1. Hypercalls, in this mode the data being sent is fully
>> encapsulated network packets.
>> 2. SynDbg specific MSRs, in this mode only the data that needs to be
>> transfered is passed.
>>
>> Signed-off-by: Jon Doron <arilou@gmail.com>
>> ---
>> docs/hyperv.txt | 15 +++
>> hw/hyperv/hyperv.c | 242 ++++++++++++++++++++++++++++++++++
>> include/hw/hyperv/hyperv.h | 58 ++++++++
>> target/i386/cpu.c | 2 +
>> target/i386/cpu.h | 7 +
>> target/i386/kvm/hyperv-stub.c | 6 +
>> target/i386/kvm/hyperv.c | 52 +++++++-
>> target/i386/kvm/kvm.c | 76 ++++++++++-
>> 8 files changed, 450 insertions(+), 8 deletions(-)
>>
>> diff --git a/docs/hyperv.txt b/docs/hyperv.txt
>> index 0417c183a3..7abc1b2d89 100644
>> --- a/docs/hyperv.txt
>> +++ b/docs/hyperv.txt
>> @@ -225,6 +225,21 @@ default (WS2016).
>> Note: hv-version-id-* are not enlightenments and thus don't enable Hyper-V
>> identification when specified without any other enlightenments.
>>
>> +3.21. hv-syndbg
>> +===============
>> +Enables Hyper-V synthetic debugger interface, this is a special interface used
>> +by Windows Kernel debugger to send the packets through, rather than sending
>> +them via serial/network .
>> +Whe enabled, this enlightenment provides additional communication facilities
>
>When
>
Done
>> +to the guest: SynDbg messages.
>> +This new communication is used by Windows Kernel debugger rather than sending
>> +packets via serial/network, adding significant performance boost over the other
>> +comm channels.
>> +This enlightenment requires a VMBus device (-device vmbus-bridge,irq=15)
>> +and the follow enlightenments to work:
>> +hv-relaxed,hv_time,hv-vapic,hv-vpindex,hv-synic,hv-runtime,hv-stimer
>> +
>> +
>> 4. Supplementary features
>> =========================
>>
>> diff --git a/hw/hyperv/hyperv.c b/hw/hyperv/hyperv.c
>> index 88c9cc1334..c86e2aa02e 100644
>> --- a/hw/hyperv/hyperv.c
>> +++ b/hw/hyperv/hyperv.c
>> @@ -730,3 +730,245 @@ uint16_t hyperv_hcall_signal_event(uint64_t param, bool fast)
>> }
>> return HV_STATUS_INVALID_CONNECTION_ID;
>> }
>> +
>> +static HvSynDbgHandler hv_syndbg_handler;
>> +static void *hv_syndbg_context;
>
>Add a line here between field and function definition.
>
Done
>> +void hyperv_set_syndbg_handler(HvSynDbgHandler handler, void *context)
>> +{
>> + assert(!hv_syndbg_handler);
>> + hv_syndbg_handler = handler;
>> + hv_syndbg_context = context;
>> +}
>> +
>> +uint16_t hyperv_hcall_reset_dbg_session(uint64_t outgpa)
>> +{
>> + uint16_t ret;
>> + HvSynDbgMsg msg;
>> + struct hyperv_reset_debug_session_output *reset_dbg_session = NULL;
>> + hwaddr len;
>> +
>> + if (!hv_syndbg_handler) {
>> + ret = HV_STATUS_INVALID_HYPERCALL_CODE;
>> + goto cleanup;
>> + }
>> +
>> + len = sizeof(*reset_dbg_session);
>> + reset_dbg_session = cpu_physical_memory_map(outgpa, &len, 1);
>> + if (!reset_dbg_session || len < sizeof(*reset_dbg_session)) {
>> + ret = HV_STATUS_INSUFFICIENT_MEMORY;
>> + goto cleanup;
>> + }
>> +
>> + msg.type = HV_SYNDBG_MSG_CONNECTION_INFO;
>> + ret = hv_syndbg_handler(hv_syndbg_context, &msg);
>> + if (ret) {
>> + goto cleanup;
>> + }
>> +
>> + reset_dbg_session->host_ip = msg.u.connection_info.host_ip;
>> + reset_dbg_session->host_port = msg.u.connection_info.host_port;
>> + /* The following fields are only used as validation for KDVM */
>> + memset(&reset_dbg_session->host_mac, 0,
>> + sizeof(reset_dbg_session->host_mac));
>> + reset_dbg_session->target_ip = msg.u.connection_info.host_ip;
>> + reset_dbg_session->target_port = msg.u.connection_info.host_port;
>> + memset(&reset_dbg_session->target_mac, 0,
>> + sizeof(reset_dbg_session->target_mac));
>> +cleanup:
>> + if (reset_dbg_session) {
>> + cpu_physical_memory_unmap(reset_dbg_session,
>> + sizeof(*reset_dbg_session), 1, len);
>> + }
>> +
>> + return ret;
>> +}
>> +
>> +uint16_t hyperv_hcall_retreive_dbg_data(uint64_t ingpa, uint64_t outgpa,
>> + bool fast)
>> +{
>> + uint16_t ret;
>> + struct hyperv_retrieve_debug_data_input *debug_data_in = NULL;
>> + struct hyperv_retrieve_debug_data_output *debug_data_out = NULL;
>> + hwaddr in_len, out_len;
>> + HvSynDbgMsg msg;
>> +
>> + if (fast || !hv_syndbg_handler) {
>> + ret = HV_STATUS_INVALID_HYPERCALL_CODE;
>> + goto cleanup;
>> + }
>> +
>> + in_len = sizeof(*debug_data_in);
>> + debug_data_in = cpu_physical_memory_map(ingpa, &in_len, 0);
>> + if (!debug_data_in || in_len < sizeof(*debug_data_in)) {
>> + ret = HV_STATUS_INSUFFICIENT_MEMORY;
>> + goto cleanup;
>> + }
>> +
>> + out_len = sizeof(*debug_data_out);
>> + debug_data_out = cpu_physical_memory_map(outgpa, &out_len, 1);
>> + if (!debug_data_out || out_len < sizeof(*debug_data_out)) {
>> + ret = HV_STATUS_INSUFFICIENT_MEMORY;
>> + goto cleanup;
>> + }
>> +
>> + msg.type = HV_SYNDBG_MSG_RECV;
>> + msg.u.recv.buf_gpa = outgpa + sizeof(*debug_data_out);
>> + msg.u.recv.count = TARGET_PAGE_SIZE - sizeof(*debug_data_out);
>> + msg.u.recv.options = debug_data_in->options;
>> + msg.u.recv.timeout = debug_data_in->timeout;
>> + msg.u.recv.is_raw = true;
>> + ret = hv_syndbg_handler(hv_syndbg_context, &msg);
>> + if (ret == HV_STATUS_NO_DATA) {
>> + debug_data_out->retrieved_count = 0;
>> + debug_data_out->remaining_count = debug_data_in->count;
>> + goto cleanup;
>> + } else if (ret != HV_STATUS_SUCCESS) {
>> + goto cleanup;
>> + }
>> +
>> + debug_data_out->retrieved_count = msg.u.recv.retrieved_count;
>> + debug_data_out->remaining_count =
>> + debug_data_in->count - msg.u.recv.retrieved_count;
>> +cleanup:
>> + if (debug_data_out) {
>> + cpu_physical_memory_unmap(debug_data_out, sizeof(*debug_data_out), 1,
>> + out_len);
>> + }
>> +
>> + if (debug_data_in) {
>> + cpu_physical_memory_unmap(debug_data_in, sizeof(*debug_data_in), 0,
>> + in_len);
>> + }
>> +
>> + return ret;
>> +}
>> +
>> +uint16_t hyperv_hcall_post_dbg_data(uint64_t ingpa, uint64_t outgpa, bool fast)
>> +{
>> + uint16_t ret;
>> + struct hyperv_post_debug_data_input *post_data_in = NULL;
>> + struct hyperv_post_debug_data_output *post_data_out = NULL;
>> + hwaddr in_len, out_len;
>> + HvSynDbgMsg msg;
>> +
>> + if (fast || !hv_syndbg_handler) {
>> + ret = HV_STATUS_INVALID_HYPERCALL_CODE;
>> + goto cleanup;
>> + }
>> +
>> + in_len = sizeof(*post_data_in);
>> + post_data_in = cpu_physical_memory_map(ingpa, &in_len, 0);
>> + if (!post_data_in || in_len < sizeof(*post_data_in)) {
>> + ret = HV_STATUS_INSUFFICIENT_MEMORY;
>> + goto cleanup;
>> + }
>> +
>> + if (post_data_in->count > TARGET_PAGE_SIZE - sizeof(*post_data_in)) {
>> + ret = HV_STATUS_INVALID_PARAMETER;
>> + goto cleanup;
>> + }
>> +
>> + out_len = sizeof(*post_data_out);
>> + post_data_out = cpu_physical_memory_map(outgpa, &out_len, 1);
>> + if (!post_data_out || out_len < sizeof(*post_data_out)) {
>> + ret = HV_STATUS_INSUFFICIENT_MEMORY;
>> + goto cleanup;
>> + }
>> +
>> + msg.type = HV_SYNDBG_MSG_SEND;
>> + msg.u.send.buf_gpa = ingpa + sizeof(*post_data_in);
>> + msg.u.send.count = post_data_in->count;
>> + msg.u.send.is_raw = true;
>> + ret = hv_syndbg_handler(hv_syndbg_context, &msg);
>> + if (ret != HV_STATUS_SUCCESS) {
>> + goto cleanup;
>> + }
>> +
>> + post_data_out->pending_count = msg.u.send.pending_count;
>> + ret = post_data_out->pending_count ? HV_STATUS_INSUFFICIENT_BUFFERS :
>> + HV_STATUS_SUCCESS;
>> +cleanup:
>> + if (post_data_out) {
>> + cpu_physical_memory_unmap(post_data_out,
>> + sizeof(*post_data_out), 1, out_len);
>> + }
>> +
>> + if (post_data_in) {
>> + cpu_physical_memory_unmap(post_data_in,
>> + sizeof(*post_data_in), 0, in_len);
>> + }
>> +
>> + return ret;
>> +}
>> +
>> +uint32_t hyperv_syndbg_send(uint64_t ingpa, uint32_t count)
>> +{
>> + HvSynDbgMsg msg;
>> +
>> + if (!hv_syndbg_handler) {
>> + return HV_SYNDBG_STATUS_INVALID;
>> + }
>> +
>> + msg.type = HV_SYNDBG_MSG_SEND;
>> + msg.u.send.buf_gpa = ingpa;
>> + msg.u.send.count = count;
>> + msg.u.send.is_raw = false;
>> + if (hv_syndbg_handler(hv_syndbg_context, &msg)) {
>> + return HV_SYNDBG_STATUS_INVALID;
>> + }
>> +
>> + return HV_SYNDBG_STATUS_SEND_SUCCESS;
>> +}
>> +
>> +uint32_t hyperv_syndbg_recv(uint64_t ingpa, uint32_t count)
>> +{
>> + uint16_t ret;
>> + HvSynDbgMsg msg;
>> +
>> + if (!hv_syndbg_handler) {
>> + return HV_SYNDBG_STATUS_INVALID;
>> + }
>> +
>> + msg.type = HV_SYNDBG_MSG_RECV;
>> + msg.u.recv.buf_gpa = ingpa;
>> + msg.u.recv.count = count;
>> + msg.u.recv.options = 0;
>> + msg.u.recv.timeout = 0;
>> + msg.u.recv.is_raw = false;
>> + ret = hv_syndbg_handler(hv_syndbg_context, &msg);
>> + if (ret != HV_STATUS_SUCCESS) {
>> + return 0;
>> + }
>> +
>> + return HV_SYNDBG_STATUS_SET_SIZE(HV_SYNDBG_STATUS_RECV_SUCCESS,
>> + msg.u.recv.retrieved_count);
>> +}
>> +
>> +void hyperv_syndbg_set_pending_page(uint64_t ingpa)
>> +{
>> + HvSynDbgMsg msg;
>> +
>> + if (!hv_syndbg_handler) {
>> + return;
>> + }
>> +
>> + msg.type = HV_SYNDBG_MSG_SET_PENDING_PAGE;
>> + msg.u.pending_page.buf_gpa = ingpa;
>> + hv_syndbg_handler(hv_syndbg_context, &msg);
>> +}
>> +
>> +uint64_t hyperv_syndbg_query_options(void)
>> +{
>> + HvSynDbgMsg msg;
>> +
>> + if (!hv_syndbg_handler) {
>> + return 0;
>> + }
>> +
>> + msg.type = HV_SYNDBG_MSG_QUERY_OPTIONS;
>> + if (hv_syndbg_handler(hv_syndbg_context, &msg) != HV_STATUS_SUCCESS) {
>> + return 0;
>> + }
>> +
>> + return msg.u.query_options.options;
>> +}
>> diff --git a/include/hw/hyperv/hyperv.h b/include/hw/hyperv/hyperv.h
>> index ef9f6b6c09..e7a85156b0 100644
>> --- a/include/hw/hyperv/hyperv.h
>> +++ b/include/hw/hyperv/hyperv.h
>> @@ -83,4 +83,62 @@ void hyperv_synic_update(CPUState *cs, bool enable,
>> hwaddr msg_page_addr, hwaddr event_page_addr);
>> bool hyperv_is_synic_enabled(void);
>>
>> +/*
>> + * Process HVCALL_RESET_DEBUG_SESSION hypercall.
>> + */
>> +uint16_t hyperv_hcall_reset_dbg_session(uint64_t outgpa);
>> +/*
>> + * Process HVCALL_RETREIVE_DEBUG_DATA hypercall.
>> + */
>> +uint16_t hyperv_hcall_retreive_dbg_data(uint64_t ingpa, uint64_t outgpa,
>> + bool fast);
>> +/*
>> + * Process HVCALL_POST_DEBUG_DATA hypercall.
>> + */
>> +uint16_t hyperv_hcall_post_dbg_data(uint64_t ingpa, uint64_t outgpa, bool fast);
>> +
>> +uint32_t hyperv_syndbg_send(uint64_t ingpa, uint32_t count);
>> +uint32_t hyperv_syndbg_recv(uint64_t ingpa, uint32_t count);
>> +void hyperv_syndbg_set_pending_page(uint64_t ingpa);
>> +uint64_t hyperv_syndbg_query_options(void);
>> +
>> +typedef enum HvSynthDbgMsgType {
>> + HV_SYNDBG_MSG_CONNECTION_INFO,
>> + HV_SYNDBG_MSG_SEND,
>> + HV_SYNDBG_MSG_RECV,
>> + HV_SYNDBG_MSG_SET_PENDING_PAGE,
>> + HV_SYNDBG_MSG_QUERY_OPTIONS
>> +} HvDbgSynthMsgType;
>> +
>> +typedef struct HvSynDbgMsg {
>> + HvDbgSynthMsgType type;
>> + union {
>> + struct {
>> + uint32_t host_ip;
>> + uint16_t host_port;
>> + } connection_info;
>> + struct {
>> + uint64_t buf_gpa;
>> + uint32_t count;
>> + uint32_t pending_count;
>> + bool is_raw;
>> + } send;
>> + struct {
>> + uint64_t buf_gpa;
>> + uint32_t count;
>> + uint32_t options;
>> + uint64_t timeout;
>> + uint32_t retrieved_count;
>> + bool is_raw;
>> + } recv;
>> + struct {
>> + uint64_t buf_gpa;
>> + } pending_page;
>> + struct {
>> + uint64_t options;
>> + } query_options;
>> + } u;
>> +} HvSynDbgMsg;
>> +typedef uint16_t (*HvSynDbgHandler)(void *context, HvSynDbgMsg *msg);
>> +void hyperv_set_syndbg_handler(HvSynDbgHandler handler, void *context);
>> #endif
>> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
>> index aa9e636800..9529a6389a 100644
>> --- a/target/i386/cpu.c
>> +++ b/target/i386/cpu.c
>> @@ -6841,6 +6841,8 @@ static Property x86_cpu_properties[] = {
>> HYPERV_FEAT_AVIC, 0),
>> DEFINE_PROP_ON_OFF_AUTO("hv-no-nonarch-coresharing", X86CPU,
>> hyperv_no_nonarch_cs, ON_OFF_AUTO_OFF),
>> + DEFINE_PROP_BIT64("hv-syndbg", X86CPU, hyperv_features,
>> + HYPERV_FEAT_SYNDBG, 0),
>> 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/cpu.h b/target/i386/cpu.h
>> index 9911d7c871..56e0317924 100644
>> --- a/target/i386/cpu.h
>> +++ b/target/i386/cpu.h
>> @@ -1060,6 +1060,7 @@ typedef uint64_t FeatureWordArray[FEATURE_WORDS];
>> #define HYPERV_FEAT_IPI 13
>> #define HYPERV_FEAT_STIMER_DIRECT 14
>> #define HYPERV_FEAT_AVIC 15
>> +#define HYPERV_FEAT_SYNDBG 16
>>
>> #ifndef HYPERV_SPINLOCK_NEVER_NOTIFY
>> #define HYPERV_SPINLOCK_NEVER_NOTIFY 0xFFFFFFFF
>> @@ -1560,6 +1561,12 @@ typedef struct CPUX86State {
>> uint64_t msr_hv_hypercall;
>> uint64_t msr_hv_guest_os_id;
>> uint64_t msr_hv_tsc;
>> + uint64_t msr_hv_syndbg_control;
>> + uint64_t msr_hv_syndbg_status;
>> + uint64_t msr_hv_syndbg_send_page;
>> + uint64_t msr_hv_syndbg_recv_page;
>> + uint64_t msr_hv_syndbg_pending_page;
>> + uint64_t msr_hv_syndbg_options;
>>
>> /* Per-VCPU HV MSRs */
>> uint64_t msr_hv_vapic;
>> diff --git a/target/i386/kvm/hyperv-stub.c b/target/i386/kvm/hyperv-stub.c
>> index 0028527e79..778ed782e6 100644
>> --- a/target/i386/kvm/hyperv-stub.c
>> +++ b/target/i386/kvm/hyperv-stub.c
>> @@ -27,6 +27,12 @@ int kvm_hv_handle_exit(X86CPU *cpu, struct kvm_hyperv_exit *exit)
>> return 0;
>> case KVM_EXIT_HYPERV_HCALL:
>> exit->u.hcall.result = HV_STATUS_INVALID_HYPERCALL_CODE;
>> + return 0;
>> + case KVM_EXIT_HYPERV_SYNDBG:
>> + if (!hyperv_feat_enabled(cpu, HYPERV_FEAT_SYNDBG)) {
>> + return -1;
>> + }
>> +
>> return 0;
>> default:
>> return -1;
>> diff --git a/target/i386/kvm/hyperv.c b/target/i386/kvm/hyperv.c
>> index 26efc1e0e6..a70f695205 100644
>> --- a/target/i386/kvm/hyperv.c
>> +++ b/target/i386/kvm/hyperv.c
>> @@ -81,20 +81,66 @@ int kvm_hv_handle_exit(X86CPU *cpu, struct kvm_hyperv_exit *exit)
>> case KVM_EXIT_HYPERV_HCALL: {
>> uint16_t code = exit->u.hcall.input & 0xffff;
>> bool fast = exit->u.hcall.input & HV_HYPERCALL_FAST;
>> - uint64_t param = exit->u.hcall.params[0];
>> + uint64_t in_param = exit->u.hcall.params[0];
>> + uint64_t out_param = exit->u.hcall.params[1];
>>
>> switch (code) {
>> case HV_POST_MESSAGE:
>> - exit->u.hcall.result = hyperv_hcall_post_message(param, fast);
>> + exit->u.hcall.result = hyperv_hcall_post_message(in_param, fast);
>> break;
>> case HV_SIGNAL_EVENT:
>> - exit->u.hcall.result = hyperv_hcall_signal_event(param, fast);
>> + exit->u.hcall.result = hyperv_hcall_signal_event(in_param, fast);
>> + break;
>> + case HV_POST_DEBUG_DATA:
>> + exit->u.hcall.result =
>> + hyperv_hcall_post_dbg_data(in_param, out_param, fast);
>> + break;
>> + case HV_RETREIVE_DEBUG_DATA:
>> + exit->u.hcall.result =
>> + hyperv_hcall_retreive_dbg_data(in_param, out_param, fast);
>> + break;
>> + case HV_RESET_DEBUG_SESSION:
>> + exit->u.hcall.result =
>> + hyperv_hcall_reset_dbg_session(out_param);
>> break;
>> default:
>> exit->u.hcall.result = HV_STATUS_INVALID_HYPERCALL_CODE;
>> }
>> return 0;
>> }
>> +
>> + case KVM_EXIT_HYPERV_SYNDBG:
>> + if (!hyperv_feat_enabled(cpu, HYPERV_FEAT_SYNDBG)) {
>> + return -1;
>> + }
>> +
>> + switch (exit->u.syndbg.msr) {
>> + case HV_X64_MSR_SYNDBG_CONTROL: {
>> + uint64_t control = exit->u.syndbg.control;
>> + env->msr_hv_syndbg_control = control;
>> + env->msr_hv_syndbg_send_page = exit->u.syndbg.send_page;
>> + env->msr_hv_syndbg_recv_page = exit->u.syndbg.recv_page;
>> + exit->u.syndbg.status = HV_STATUS_SUCCESS;
>> + if (control & HV_SYNDBG_CONTROL_SEND) {
>> + exit->u.syndbg.status =
>> + hyperv_syndbg_send(env->msr_hv_syndbg_send_page,
>> + HV_SYNDBG_CONTROL_SEND_SIZE(control));
>> + } else if (control & HV_SYNDBG_CONTROL_RECV) {
>> + exit->u.syndbg.status =
>> + hyperv_syndbg_recv(env->msr_hv_syndbg_recv_page,
>> + TARGET_PAGE_SIZE);
>> + }
>> + break;
>> + }
>> + case HV_X64_MSR_SYNDBG_PENDING_BUFFER:
>> + env->msr_hv_syndbg_pending_page = exit->u.syndbg.pending_page;
>> + hyperv_syndbg_set_pending_page(env->msr_hv_syndbg_pending_page);
>> + break;
>> + default:
>> + return -1;
>> + }
>> +
>> + return 0;
>> default:
>> return -1;
>> }
>> diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
>> index 2c8feb4a6f..ecabb332d7 100644
>> --- a/target/i386/kvm/kvm.c
>> +++ b/target/i386/kvm/kvm.c
>> @@ -102,6 +102,7 @@ static bool has_msr_hv_synic;
>> static bool has_msr_hv_stimer;
>> static bool has_msr_hv_frequencies;
>> static bool has_msr_hv_reenlightenment;
>> +static bool has_msr_hv_syndbg_options;
>> static bool has_msr_xss;
>> static bool has_msr_umwait;
>> static bool has_msr_spec_ctrl;
>> @@ -932,6 +933,14 @@ static struct {
>> .bits = HV_DEPRECATING_AEOI_RECOMMENDED}
>> }
>> },
>> + [HYPERV_FEAT_SYNDBG] = {
>> + .desc = "Enable synthetic kernel debugger channel (hv-syndbg)",
>> + .flags = {
>> + {.func = HV_CPUID_FEATURES, .reg = R_EDX,
>> + .bits = HV_FEATURE_DEBUG_MSRS_AVAILABLE}
>> + },
>> + .dependencies = BIT(HYPERV_FEAT_SYNIC) | BIT(HYPERV_FEAT_RELAXED)
>> + },
>> };
>>
>> static struct kvm_cpuid2 *try_get_hv_cpuid(CPUState *cs, int max,
>> @@ -972,8 +981,8 @@ static struct kvm_cpuid2 *try_get_hv_cpuid(CPUState *cs, int max,
>> static struct kvm_cpuid2 *get_supported_hv_cpuid(CPUState *cs)
>> {
>> struct kvm_cpuid2 *cpuid;
>> - /* 0x40000000..0x40000005, 0x4000000A, 0x40000080..0x40000080 leaves */
>> - int max = 10;
>> + /* 0x40000000..0x40000005, 0x4000000A, 0x40000080..0x40000082 leaves */
>> + int max = 11;
>> int i;
>> bool do_sys_ioctl;
>>
>> @@ -1086,6 +1095,12 @@ static struct kvm_cpuid2 *get_supported_hv_cpuid_legacy(CPUState *cs)
>> entry_feat->eax |= HV_SYNTIMERS_AVAILABLE;
>> }
>>
>> + if (has_msr_hv_syndbg_options) {
>> + entry_feat->edx |= HV_GUEST_DEBUGGING_AVAILABLE;
>> + entry_feat->edx |= HV_FEATURE_DEBUG_MSRS_AVAILABLE;
>> + entry_feat->ebx |= HV_PARTITION_DEUBGGING_ALLOWED;
>> + }
>> +
>> if (kvm_check_extension(cs->kvm_state,
>> KVM_CAP_HYPERV_TLBFLUSH) > 0) {
>> entry_recomm->eax |= HV_REMOTE_TLB_FLUSH_RECOMMENDED;
>> @@ -1337,12 +1352,22 @@ static int hyperv_fill_cpuids(CPUState *cs,
>> {
>> X86CPU *cpu = X86_CPU(cs);
>> struct kvm_cpuid_entry2 *c;
>> - uint32_t cpuid_i = 0;
>> + uint32_t signature[3];
>> + uint32_t cpuid_i = 0, max_cpuid_leaf = 0;
>> +
>> + max_cpuid_leaf = HV_CPUID_IMPLEMENT_LIMITS;
>> + if (hyperv_feat_enabled(cpu, HYPERV_FEAT_EVMCS)) {
>> + max_cpuid_leaf = MAX(max_cpuid_leaf, HV_CPUID_NESTED_FEATURES);
>> + }
>> +
>> + if (hyperv_feat_enabled(cpu, HYPERV_FEAT_SYNDBG)) {
>> + max_cpuid_leaf =
>> + MAX(max_cpuid_leaf, HV_CPUID_SYNDBG_PLATFORM_CAPABILITIES);
>> + }
>>
>> c = &cpuid_ent[cpuid_i++];
>> c->function = HV_CPUID_VENDOR_AND_MAX_FUNCTIONS;
>> - c->eax = hyperv_feat_enabled(cpu, HYPERV_FEAT_EVMCS) ?
>> - HV_CPUID_NESTED_FEATURES : HV_CPUID_IMPLEMENT_LIMITS;
>> + c->eax = max_cpuid_leaf;
>> c->ebx = cpu->hyperv_vendor_id[0];
>> c->ecx = cpu->hyperv_vendor_id[1];
>> c->edx = cpu->hyperv_vendor_id[2];
>> @@ -1421,6 +1446,33 @@ static int hyperv_fill_cpuids(CPUState *cs,
>> c->eax = cpu->hyperv_nested[0];
>> }
>>
>> + if (hyperv_feat_enabled(cpu, HYPERV_FEAT_SYNDBG)) {
>> + c = &cpuid_ent[cpuid_i++];
>> + c->function = HV_CPUID_SYNDBG_VENDOR_AND_MAX_FUNCTIONS;
>> + c->eax = hyperv_feat_enabled(cpu, HYPERV_FEAT_EVMCS) ?
>> + HV_CPUID_NESTED_FEATURES : HV_CPUID_IMPLEMENT_LIMITS;
>> + memcpy(signature, "Microsoft VS", 12);
>> + c->eax = 0;
>> + c->ebx = signature[0];
>> + c->ecx = signature[1];
>> + c->edx = signature[2];
>> +
>> + c = &cpuid_ent[cpuid_i++];
>> + c->function = HV_CPUID_SYNDBG_INTERFACE;
>> + memcpy(signature, "VS#1\0\0\0\0\0\0\0\0", 12);
>> + c->eax = signature[0];
>> + c->ebx = 0;
>> + c->ecx = 0;
>> + c->edx = 0;
>> +
>> + c = &cpuid_ent[cpuid_i++];
>> + c->function = HV_CPUID_SYNDBG_PLATFORM_CAPABILITIES;
>> + c->eax = HV_SYNDBG_CAP_ALLOW_KERNEL_DEBUGGING;
>> + c->ebx = 0;
>> + c->ecx = 0;
>> + c->edx = 0;
>> + }
>> +
>> return cpuid_i;
>> }
>>
>> @@ -2215,6 +2267,9 @@ static int kvm_get_supported_msrs(KVMState *s)
>> case HV_X64_MSR_REENLIGHTENMENT_CONTROL:
>> has_msr_hv_reenlightenment = true;
>> break;
>> + case HV_X64_MSR_SYNDBG_OPTIONS:
>> + has_msr_hv_syndbg_options = true;
>> + break;
>> case MSR_IA32_SPEC_CTRL:
>> has_msr_spec_ctrl = true;
>> break;
>> @@ -3132,6 +3187,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);
>> }
>> + 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());
>> + }
>> }
>> if (hyperv_feat_enabled(cpu, HYPERV_FEAT_VAPIC)) {
>> kvm_msr_entry_add(cpu, HV_X64_MSR_APIC_ASSIST_PAGE,
>> @@ -3565,6 +3625,9 @@ static int kvm_get_msrs(X86CPU *cpu)
>> kvm_msr_entry_add(cpu, HV_X64_MSR_TSC_EMULATION_CONTROL, 0);
>> kvm_msr_entry_add(cpu, HV_X64_MSR_TSC_EMULATION_STATUS, 0);
>> }
>> + if (has_msr_hv_syndbg_options) {
>> + kvm_msr_entry_add(cpu, HV_X64_MSR_SYNDBG_OPTIONS, 0);
>> + }
>> if (has_msr_hv_crash) {
>> int j;
>>
>> @@ -3851,6 +3914,9 @@ static int kvm_get_msrs(X86CPU *cpu)
>> case HV_X64_MSR_TSC_EMULATION_STATUS:
>> env->msr_hv_tsc_emulation_status = msrs[i].data;
>> break;
>> + case HV_X64_MSR_SYNDBG_OPTIONS:
>> + env->msr_hv_syndbg_options = msrs[i].data;
>> + break;
>> case MSR_MTRRdefType:
>> env->mtrr_deftype = msrs[i].data;
>> break;
>>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v1 4/4] hw: hyperv: Initial commit for Synthetic Debugging device
2022-02-16 9:11 ` Emanuele Giuseppe Esposito
@ 2022-02-16 10:28 ` Jon Doron
0 siblings, 0 replies; 14+ messages in thread
From: Jon Doron @ 2022-02-16 10:28 UTC (permalink / raw)
To: Emanuele Giuseppe Esposito; +Cc: pbonzini, vkuznets, qemu-devel
On 16/02/2022, Emanuele Giuseppe Esposito wrote:
>
>> +
>> +static uint16_t handle_recv_msg(HvSynDbg *syndbg, uint64_t outgpa,
>> + uint32_t count, bool is_raw, uint32_t options,
>> + uint64_t timeout, uint32_t *retrieved_count)
>> +{
>> + uint16_t ret;
>> + uint8_t data_buf[TARGET_PAGE_SIZE - UDP_PKT_HEADER_SIZE];
>> + hwaddr out_len;
>> + void *out_data = NULL;
>> + ssize_t recv_byte_count;
>> +
>> + /* TODO: Handle options and timeout */
>> + (void)options;
>> + (void)timeout;
>> +
>> + if (!syndbg->has_data_pending) {
>> + recv_byte_count = 0;
>> + } else {
>> + recv_byte_count = qemu_recv(syndbg->socket, data_buf,
>> + MIN(sizeof(data_buf), count), MSG_WAITALL);
>> + if (recv_byte_count == -1) {
>> + ret = HV_STATUS_INVALID_PARAMETER;
>> + goto cleanup;
>> + }
>> + }
>> +
>> + if (!recv_byte_count) {
>> + *retrieved_count = 0;
>> + ret = HV_STATUS_NO_DATA;
>> + goto cleanup;
>> + }
>> +
>> + set_pending_state(syndbg, false);
>> +
>> + out_len = recv_byte_count;
>> + if (is_raw) {
>> + out_len += UDP_PKT_HEADER_SIZE;
>> + }
>> + out_data = cpu_physical_memory_map(outgpa, &out_len, 1);
>> + if (!out_data) {
>> + ret = HV_STATUS_INSUFFICIENT_MEMORY;
>> + goto cleanup;
>> + }
>> +
>> + if (is_raw &&
>> + !create_udp_pkt(syndbg, out_data,
>> + recv_byte_count + UDP_PKT_HEADER_SIZE,
>> + data_buf, recv_byte_count)) {
>> + ret = HV_STATUS_INSUFFICIENT_MEMORY;
>> + goto cleanup;
>> + } else if (!is_raw) {
>> + memcpy(out_data, data_buf, recv_byte_count);
>> + }
>> +
>> + *retrieved_count = recv_byte_count;
>> + if (is_raw) {
>> + *retrieved_count += UDP_PKT_HEADER_SIZE;
>> + }
>> + ret = HV_STATUS_SUCCESS;
>> +cleanup:
>> + if (out_data) {
>> + cpu_physical_memory_unmap(out_data, out_len, 1, out_len);
>> + }
>
>Same nitpick as done in patch 1, I think you can use more gotos labels
>instead of adding if statements.
>
Done
>> +
>> + return ret;
>> +}
>> +
>
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2022-02-16 10:48 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-04 10:07 [PATCH v1 0/4] HyperV: Synthetic Debugging device Jon Doron
2022-02-04 10:07 ` [PATCH v1 1/4] hyperv: SControl is optional to enable SynIc Jon Doron
2022-02-16 9:10 ` Emanuele Giuseppe Esposito
2022-02-16 10:27 ` Jon Doron
2022-02-04 10:07 ` [PATCH v1 2/4] hyperv: Add definitions for syndbg Jon Doron
2022-02-16 9:10 ` Emanuele Giuseppe Esposito
2022-02-16 10:27 ` Jon Doron
2022-02-04 10:07 ` [PATCH v1 3/4] hyperv: Add support to process syndbg commands Jon Doron
2022-02-16 9:10 ` Emanuele Giuseppe Esposito
2022-02-16 10:28 ` Jon Doron
2022-02-04 10:07 ` [PATCH v1 4/4] hw: hyperv: Initial commit for Synthetic Debugging device Jon Doron
2022-02-16 9:11 ` Emanuele Giuseppe Esposito
2022-02-16 10:28 ` Jon Doron
2022-02-13 7:49 ` [PATCH v1 0/4] HyperV: " Jon Doron
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.