Re: [PATCH v1 4/4] hw: hyperv: Initial commit for Synthetic Debugging device

2022-02-16 Thread Jon Doron 

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;
+}
+

Re: [PATCH v1 4/4] hw: hyperv: Initial commit for Synthetic Debugging device

2022-02-16 Thread Emanuele Giuseppe Esposito 


> +
> +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;
> +}
> +

[PATCH v1 4/4] hw: hyperv: Initial commit for Synthetic Debugging device

2022-02-04 Thread Jon Doron 
Signed-off-by: Jon Doron 
---
 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 00..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)