linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/15] hyperv: more stuff to uapi + cleanup
@ 2016-12-20 15:55 Roman Kagan
  2016-12-20 15:55 ` [PATCH 01/15] hyperv: consolidate TSC ref page definitions Roman Kagan
                   ` (15 more replies)
  0 siblings, 16 replies; 54+ messages in thread
From: Roman Kagan @ 2016-12-20 15:55 UTC (permalink / raw)
  To: Paolo Bonzini, Radim Krčmář,
	K. Y. Srinivasan, Vitaly Kuznetsov
  Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, Haiyang Zhang,
	kvm, linux-kernel, devel, Denis V . Lunev, Roman Kagan

Expose more Hyper-V-related definitions in the uapi header for
consumption by userspace.

While doing so, get rid of a number of duplications between the KVM and
the guest driver code.  Also a few other cleanups are made which are not
strictly necessary for the main purpose of the series but appear
reasonable to do at the same time.

The most controversial is the last patch which modifies the stuff
already published in the uapi header, in the hope that no userspace
applications have started relying on it; I'm ok dropping it if this is
unacceptable.

Roman Kagan (15):
  hyperv: consolidate TSC ref page definitions
  hyperv: uapi-fy synic event flags definitions
  hyperv: use standard bitops
  hyperv: define VMBus message type
  hyperv: GFP_ATOMIC -> GFP_KERNEL
  hyperv: avoid unnecessary vmalloc
  hyperv: dedup cpuid definitions
  hyperv: dedup crash msr related definitions
  hyperv: unify Hyper-V msr definitions
  hyperv: uapi-fy PostMessage and SignalEvent hypercall structures
  hyperv: uapi-fy monitored notification structures
  hyperv: move VMBus connection ids to uapi
  hyperv: move function close to its only callsite
  hyperv_vmbus: drop unused definitions
  hyperv: redefine hv_message without bitfields

 arch/x86/include/asm/kvm_host.h    |   2 +-
 arch/x86/include/uapi/asm/hyperv.h | 101 +++++++---
 drivers/hv/hyperv_vmbus.h          | 399 +------------------------------------
 include/linux/hyperv.h             |  24 +--
 arch/x86/kvm/hyperv.c              |  14 +-
 drivers/hv/channel.c               |   8 +-
 drivers/hv/channel_mgmt.c          |  30 +--
 drivers/hv/connection.c            |  65 ++----
 drivers/hv/hv.c                    | 300 +++++++++++++---------------
 drivers/hv/vmbus_drv.c             |  67 +++----
 10 files changed, 288 insertions(+), 722 deletions(-)

-- 
2.9.3

^ permalink raw reply	[flat|nested] 54+ messages in thread

* [PATCH 01/15] hyperv: consolidate TSC ref page definitions
  2016-12-20 15:55 [PATCH 00/15] hyperv: more stuff to uapi + cleanup Roman Kagan
@ 2016-12-20 15:55 ` Roman Kagan
  2016-12-20 20:57   ` KY Srinivasan
  2016-12-20 15:55 ` [PATCH 02/15] hyperv: uapi-fy synic event flags definitions Roman Kagan
                   ` (14 subsequent siblings)
  15 siblings, 1 reply; 54+ messages in thread
From: Roman Kagan @ 2016-12-20 15:55 UTC (permalink / raw)
  To: Paolo Bonzini, Radim Krčmář,
	K. Y. Srinivasan, Vitaly Kuznetsov
  Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, Haiyang Zhang,
	kvm, linux-kernel, devel, Denis V . Lunev, Roman Kagan

Consolidate the guest-side and kvm-side definitions for Hyper-V TSC
reference page.

While at this, rewrite read_hv_clock_tsc using the existing helpers.

Signed-off-by: Roman Kagan <rkagan@virtuozzo.com>
---
 arch/x86/include/asm/kvm_host.h    |  2 +-
 arch/x86/include/uapi/asm/hyperv.h |  4 +--
 drivers/hv/hyperv_vmbus.h          |  8 ------
 arch/x86/kvm/hyperv.c              |  4 +--
 drivers/hv/hv.c                    | 54 +++++++++++++++-----------------------
 5 files changed, 26 insertions(+), 46 deletions(-)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 2e25038..2b85f49 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -713,7 +713,7 @@ struct kvm_hv {
 	u64 hv_crash_param[HV_X64_MSR_CRASH_PARAMS];
 	u64 hv_crash_ctl;
 
-	HV_REFERENCE_TSC_PAGE tsc_ref;
+	struct hv_ref_tsc_page tsc_ref;
 };
 
 struct kvm_arch {
diff --git a/arch/x86/include/uapi/asm/hyperv.h b/arch/x86/include/uapi/asm/hyperv.h
index 9b1a918..6098ab5 100644
--- a/arch/x86/include/uapi/asm/hyperv.h
+++ b/arch/x86/include/uapi/asm/hyperv.h
@@ -252,12 +252,12 @@
 #define HV_STATUS_INVALID_CONNECTION_ID		18
 #define HV_STATUS_INSUFFICIENT_BUFFERS		19
 
-typedef struct _HV_REFERENCE_TSC_PAGE {
+struct hv_ref_tsc_page {
 	__u32 tsc_sequence;
 	__u32 res1;
 	__u64 tsc_scale;
 	__s64 tsc_offset;
-} HV_REFERENCE_TSC_PAGE, *PHV_REFERENCE_TSC_PAGE;
+};
 
 /* Define the number of synthetic interrupt sources. */
 #define HV_SYNIC_SINT_COUNT		(16)
diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
index 0675b39..4516498 100644
--- a/drivers/hv/hyperv_vmbus.h
+++ b/drivers/hv/hyperv_vmbus.h
@@ -475,14 +475,6 @@ struct hv_context {
 
 extern struct hv_context hv_context;
 
-struct ms_hyperv_tsc_page {
-	volatile u32 tsc_sequence;
-	u32 reserved1;
-	volatile u64 tsc_scale;
-	volatile s64 tsc_offset;
-	u64 reserved2[509];
-};
-
 struct hv_ring_buffer_debug_info {
 	u32 current_interrupt_mask;
 	u32 current_read_index;
diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
index 1572c35..c7db112 100644
--- a/arch/x86/kvm/hyperv.c
+++ b/arch/x86/kvm/hyperv.c
@@ -806,7 +806,7 @@ static int kvm_hv_msr_set_crash_data(struct kvm_vcpu *vcpu,
  * These two equivalencies are implemented in this function.
  */
 static bool compute_tsc_page_parameters(struct pvclock_vcpu_time_info *hv_clock,
-					HV_REFERENCE_TSC_PAGE *tsc_ref)
+					struct hv_ref_tsc_page *tsc_ref)
 {
 	u64 max_mul;
 
@@ -847,7 +847,7 @@ void kvm_hv_setup_tsc_page(struct kvm *kvm,
 	u64 gfn;
 
 	BUILD_BUG_ON(sizeof(tsc_seq) != sizeof(hv->tsc_ref.tsc_sequence));
-	BUILD_BUG_ON(offsetof(HV_REFERENCE_TSC_PAGE, tsc_sequence) != 0);
+	BUILD_BUG_ON(offsetof(struct hv_ref_tsc_page, tsc_sequence) != 0);
 
 	if (!(hv->hv_tsc_page & HV_X64_MSR_TSC_REFERENCE_ENABLE))
 		return;
diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
index 446802a..a7256ec 100644
--- a/drivers/hv/hv.c
+++ b/drivers/hv/hv.c
@@ -137,41 +137,29 @@ EXPORT_SYMBOL_GPL(hv_do_hypercall);
 #ifdef CONFIG_X86_64
 static cycle_t read_hv_clock_tsc(struct clocksource *arg)
 {
-	cycle_t current_tick;
-	struct ms_hyperv_tsc_page *tsc_pg = hv_context.tsc_page;
+	struct hv_ref_tsc_page *tsc_pg = hv_context.tsc_page;
+	u32 sequence;
+	u64 scale;
+	s64 offset;
+
+	do {
+		sequence = tsc_pg->tsc_sequence;
+		virt_rmb();
+
+		if (!sequence) {
+			/* fallback to MSR */
+			cycle_t current_tick;
+			rdmsrl(HV_X64_MSR_TIME_REF_COUNT, current_tick);
+			return current_tick;
+		}
 
-	if (tsc_pg->tsc_sequence != 0) {
-		/*
-		 * Use the tsc page to compute the value.
-		 */
+		scale = tsc_pg->tsc_scale;
+		offset = tsc_pg->tsc_offset;
 
-		while (1) {
-			cycle_t tmp;
-			u32 sequence = tsc_pg->tsc_sequence;
-			u64 cur_tsc;
-			u64 scale = tsc_pg->tsc_scale;
-			s64 offset = tsc_pg->tsc_offset;
-
-			rdtscll(cur_tsc);
-			/* current_tick = ((cur_tsc *scale) >> 64) + offset */
-			asm("mulq %3"
-				: "=d" (current_tick), "=a" (tmp)
-				: "a" (cur_tsc), "r" (scale));
-
-			current_tick += offset;
-			if (tsc_pg->tsc_sequence == sequence)
-				return current_tick;
-
-			if (tsc_pg->tsc_sequence != 0)
-				continue;
-			/*
-			 * Fallback using MSR method.
-			 */
-			break;
-		}
-	}
-	rdmsrl(HV_X64_MSR_TIME_REF_COUNT, current_tick);
-	return current_tick;
+		virt_rmb();
+	} while (tsc_pg->tsc_sequence != sequence);
+
+	return mul_u64_u64_shr(rdtsc_ordered(), scale, 64) + offset;
 }
 
 static struct clocksource hyperv_cs_tsc = {
-- 
2.9.3

^ permalink raw reply	[flat|nested] 54+ messages in thread

* [PATCH 02/15] hyperv: uapi-fy synic event flags definitions
  2016-12-20 15:55 [PATCH 00/15] hyperv: more stuff to uapi + cleanup Roman Kagan
  2016-12-20 15:55 ` [PATCH 01/15] hyperv: consolidate TSC ref page definitions Roman Kagan
@ 2016-12-20 15:55 ` Roman Kagan
  2016-12-20 17:24   ` Stephen Hemminger
                     ` (2 more replies)
  2016-12-20 15:55 ` [PATCH 03/15] hyperv: use standard bitops Roman Kagan
                   ` (13 subsequent siblings)
  15 siblings, 3 replies; 54+ messages in thread
From: Roman Kagan @ 2016-12-20 15:55 UTC (permalink / raw)
  To: Paolo Bonzini, Radim Krčmář,
	K. Y. Srinivasan, Vitaly Kuznetsov
  Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, Haiyang Zhang,
	kvm, linux-kernel, devel, Denis V . Lunev, Roman Kagan

Move definitions related to the Hyper-V SynIC event flags to a header
where they can be consumed by userspace.

While doing so, also clean up their use by switching to standard bitops
and struct-based dereferencing.  The latter is also done for message
pages.

Signed-off-by: Roman Kagan <rkagan@virtuozzo.com>
---
 arch/x86/include/uapi/asm/hyperv.h | 13 +++++++++
 drivers/hv/hyperv_vmbus.h          | 24 ++--------------
 drivers/hv/channel_mgmt.c          | 10 +++----
 drivers/hv/connection.c            | 47 ++++++++++---------------------
 drivers/hv/vmbus_drv.c             | 57 ++++++++++++++------------------------
 5 files changed, 54 insertions(+), 97 deletions(-)

diff --git a/arch/x86/include/uapi/asm/hyperv.h b/arch/x86/include/uapi/asm/hyperv.h
index 6098ab5..af542a3 100644
--- a/arch/x86/include/uapi/asm/hyperv.h
+++ b/arch/x86/include/uapi/asm/hyperv.h
@@ -363,4 +363,17 @@ struct hv_timer_message_payload {
 #define HV_STIMER_AUTOENABLE		(1ULL << 3)
 #define HV_STIMER_SINT(config)		(__u8)(((config) >> 16) & 0x0F)
 
+/* Define synthetic interrupt controller flag constants. */
+#define HV_EVENT_FLAGS_COUNT		(256 * 8)
+
+/* Define the synthetic interrupt controller event flags format. */
+struct hv_synic_event_flags {
+	__u64 flags[HV_EVENT_FLAGS_COUNT / 64];
+};
+
+/* Define the synthetic interrupt flags page layout. */
+struct hv_synic_event_flags_page {
+	struct hv_synic_event_flags sintevent_flags[HV_SYNIC_SINT_COUNT];
+};
+
 #endif
diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
index 4516498..4fab154 100644
--- a/drivers/hv/hyperv_vmbus.h
+++ b/drivers/hv/hyperv_vmbus.h
@@ -26,7 +26,6 @@
 #define _HYPERV_VMBUS_H
 
 #include <linux/list.h>
-#include <asm/sync_bitops.h>
 #include <linux/atomic.h>
 #include <linux/hyperv.h>
 
@@ -75,11 +74,6 @@ enum hv_cpuid_function {
 
 #define HV_ANY_VP			(0xFFFFFFFF)
 
-/* Define synthetic interrupt controller flag constants. */
-#define HV_EVENT_FLAGS_COUNT		(256 * 8)
-#define HV_EVENT_FLAGS_BYTE_COUNT	(256)
-#define HV_EVENT_FLAGS_DWORD_COUNT	(256 / sizeof(u32))
-
 /* Define invalid partition identifier. */
 #define HV_PARTITION_ID_INVALID		((u64)0x0)
 
@@ -146,20 +140,6 @@ union hv_timer_config {
 	};
 };
 
-/* Define the number of message buffers associated with each port. */
-#define HV_PORT_MESSAGE_BUFFER_COUNT	(16)
-
-/* Define the synthetic interrupt controller event flags format. */
-union hv_synic_event_flags {
-	u8 flags8[HV_EVENT_FLAGS_BYTE_COUNT];
-	u32 flags32[HV_EVENT_FLAGS_DWORD_COUNT];
-};
-
-/* Define the synthetic interrupt flags page layout. */
-struct hv_synic_event_flags_page {
-	union hv_synic_event_flags sintevent_flags[HV_SYNIC_SINT_COUNT];
-};
-
 /* Define SynIC control register. */
 union hv_synic_scontrol {
 	u64 as_uint64;
@@ -434,8 +414,8 @@ struct hv_context {
 
 	bool synic_initialized;
 
-	void *synic_message_page[NR_CPUS];
-	void *synic_event_page[NR_CPUS];
+	struct hv_message_page *synic_message_page[NR_CPUS];
+	struct hv_synic_event_flags_page *synic_event_page[NR_CPUS];
 	/*
 	 * Hypervisor's notion of virtual processor ID is different from
 	 * Linux' notion of CPU ID. This information can only be retrieved
diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index 26b4192..49eaae2 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -654,7 +654,6 @@ static void init_vp_index(struct vmbus_channel *channel, u16 dev_type)
 static void vmbus_wait_for_unload(void)
 {
 	int cpu;
-	void *page_addr;
 	struct hv_message *msg;
 	struct vmbus_channel_message_header *hdr;
 	u32 message_type;
@@ -673,9 +672,8 @@ static void vmbus_wait_for_unload(void)
 			break;
 
 		for_each_online_cpu(cpu) {
-			page_addr = hv_context.synic_message_page[cpu];
-			msg = (struct hv_message *)page_addr +
-				VMBUS_MESSAGE_SINT;
+			msg = &hv_context.synic_message_page[cpu]->
+					sint_message[VMBUS_MESSAGE_SINT];
 
 			message_type = READ_ONCE(msg->header.message_type);
 			if (message_type == HVMSG_NONE)
@@ -699,8 +697,8 @@ static void vmbus_wait_for_unload(void)
 	 * messages after we reconnect.
 	 */
 	for_each_online_cpu(cpu) {
-		page_addr = hv_context.synic_message_page[cpu];
-		msg = (struct hv_message *)page_addr + VMBUS_MESSAGE_SINT;
+		msg = &hv_context.synic_message_page[cpu]->
+				sint_message[VMBUS_MESSAGE_SINT];
 		msg->header.message_type = HVMSG_NONE;
 	}
 }
diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c
index 6ce8b87..aaa2103 100644
--- a/drivers/hv/connection.c
+++ b/drivers/hv/connection.c
@@ -381,17 +381,12 @@ static void process_chn_event(u32 relid)
  */
 void vmbus_on_event(unsigned long data)
 {
-	u32 dword;
-	u32 maxdword;
-	int bit;
-	u32 relid;
-	u32 *recv_int_page = NULL;
-	void *page_addr;
+	u32 relid, max_relid;
+	unsigned long *recv_int_page;
 	int cpu = smp_processor_id();
-	union hv_synic_event_flags *event;
 
 	if (vmbus_proto_version < VERSION_WIN8) {
-		maxdword = MAX_NUM_CHANNELS_SUPPORTED >> 5;
+		max_relid = MAX_NUM_CHANNELS_SUPPORTED;
 		recv_int_page = vmbus_connection.recv_int_page;
 	} else {
 		/*
@@ -399,36 +394,22 @@ void vmbus_on_event(unsigned long data)
 		 * can be directly checked to get the id of the channel
 		 * that has the interrupt pending.
 		 */
-		maxdword = HV_EVENT_FLAGS_DWORD_COUNT;
-		page_addr = hv_context.synic_event_page[cpu];
-		event = (union hv_synic_event_flags *)page_addr +
-						 VMBUS_MESSAGE_SINT;
-		recv_int_page = event->flags32;
+		struct hv_synic_event_flags *event =
+			&hv_context.synic_event_page[cpu]->
+				sintevent_flags[VMBUS_MESSAGE_SINT];
+		max_relid = HV_EVENT_FLAGS_COUNT;
+		recv_int_page = (unsigned long *)event->flags;
 	}
 
-
-
 	/* Check events */
 	if (!recv_int_page)
 		return;
-	for (dword = 0; dword < maxdword; dword++) {
-		if (!recv_int_page[dword])
-			continue;
-		for (bit = 0; bit < 32; bit++) {
-			if (sync_test_and_clear_bit(bit,
-				(unsigned long *)&recv_int_page[dword])) {
-				relid = (dword << 5) + bit;
-
-				if (relid == 0)
-					/*
-					 * Special case - vmbus
-					 * channel protocol msg
-					 */
-					continue;
-
-				process_chn_event(relid);
-			}
-		}
+
+	/* relid == 0 is vmbus channel protocol msg */
+	relid = 1;
+	for_each_set_bit_from(relid, recv_int_page, max_relid) {
+		clear_bit(relid, recv_int_page);
+		process_chn_event(relid);
 	}
 }
 
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 230c62e..13dd210 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -872,9 +872,8 @@ static void hv_process_timer_expiration(struct hv_message *msg, int cpu)
 void vmbus_on_msg_dpc(unsigned long data)
 {
 	int cpu = smp_processor_id();
-	void *page_addr = hv_context.synic_message_page[cpu];
-	struct hv_message *msg = (struct hv_message *)page_addr +
-				  VMBUS_MESSAGE_SINT;
+	struct hv_message *msg = &hv_context.synic_message_page[cpu]->
+					sint_message[VMBUS_MESSAGE_SINT];
 	struct vmbus_channel_message_header *hdr;
 	struct vmbus_channel_message_table_entry *entry;
 	struct onmessage_work_context *ctx;
@@ -911,54 +910,40 @@ void vmbus_on_msg_dpc(unsigned long data)
 static void vmbus_isr(void)
 {
 	int cpu = smp_processor_id();
-	void *page_addr;
 	struct hv_message *msg;
-	union hv_synic_event_flags *event;
-	bool handled = false;
+	struct hv_synic_event_flags *event;
 
-	page_addr = hv_context.synic_event_page[cpu];
-	if (page_addr == NULL)
+	if (!hv_context.synic_event_page[cpu])
 		return;
 
-	event = (union hv_synic_event_flags *)page_addr +
-					 VMBUS_MESSAGE_SINT;
+	event = &hv_context.synic_event_page[cpu]->
+			sintevent_flags[VMBUS_MESSAGE_SINT];
 	/*
 	 * Check for events before checking for messages. This is the order
 	 * in which events and messages are checked in Windows guests on
 	 * Hyper-V, and the Windows team suggested we do the same.
 	 */
 
-	if ((vmbus_proto_version == VERSION_WS2008) ||
-		(vmbus_proto_version == VERSION_WIN7)) {
-
+	/* On win8 and above the channel interrupts are signaled directly in
+	 * the event page and will be checked in the .event_dpc
+	 */
+	if (vmbus_proto_version >= VERSION_WIN8 ||
 		/* Since we are a child, we only need to check bit 0 */
-		if (sync_test_and_clear_bit(0,
-			(unsigned long *) &event->flags32[0])) {
-			handled = true;
-		}
-	} else {
-		/*
-		 * Our host is win8 or above. The signaling mechanism
-		 * has changed and we can directly look at the event page.
-		 * If bit n is set then we have an interrup on the channel
-		 * whose id is n.
-		 */
-		handled = true;
-	}
-
-	if (handled)
+	    test_and_clear_bit(0, (unsigned long *)event->flags))
 		tasklet_schedule(hv_context.event_dpc[cpu]);
 
-
-	page_addr = hv_context.synic_message_page[cpu];
-	msg = (struct hv_message *)page_addr + VMBUS_MESSAGE_SINT;
+	msg = &hv_context.synic_message_page[cpu]->
+			sint_message[VMBUS_MESSAGE_SINT];
 
 	/* Check if there are actual msgs to be processed */
-	if (msg->header.message_type != HVMSG_NONE) {
-		if (msg->header.message_type == HVMSG_TIMER_EXPIRED)
-			hv_process_timer_expiration(msg, cpu);
-		else
-			tasklet_schedule(hv_context.msg_dpc[cpu]);
+	switch (READ_ONCE(msg->header.message_type)) {
+	case HVMSG_NONE:
+		break;
+	case HVMSG_TIMER_EXPIRED:
+		hv_process_timer_expiration(msg, cpu);
+		break;
+	default:
+		tasklet_schedule(hv_context.msg_dpc[cpu]);
 	}
 
 	add_interrupt_randomness(HYPERVISOR_CALLBACK_VECTOR, 0);
-- 
2.9.3

^ permalink raw reply	[flat|nested] 54+ messages in thread

* [PATCH 03/15] hyperv: use standard bitops
  2016-12-20 15:55 [PATCH 00/15] hyperv: more stuff to uapi + cleanup Roman Kagan
  2016-12-20 15:55 ` [PATCH 01/15] hyperv: consolidate TSC ref page definitions Roman Kagan
  2016-12-20 15:55 ` [PATCH 02/15] hyperv: uapi-fy synic event flags definitions Roman Kagan
@ 2016-12-20 15:55 ` Roman Kagan
  2016-12-21 12:00   ` Olaf Hering
  2016-12-21 19:08   ` KY Srinivasan
  2016-12-20 15:55 ` [PATCH 04/15] hyperv: define VMBus message type Roman Kagan
                   ` (12 subsequent siblings)
  15 siblings, 2 replies; 54+ messages in thread
From: Roman Kagan @ 2016-12-20 15:55 UTC (permalink / raw)
  To: Paolo Bonzini, Radim Krčmář,
	K. Y. Srinivasan, Vitaly Kuznetsov
  Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, Haiyang Zhang,
	kvm, linux-kernel, devel, Denis V . Lunev, Roman Kagan

Signed-off-by: Roman Kagan <rkagan@virtuozzo.com>
---
 drivers/hv/channel.c    | 8 +++-----
 drivers/hv/connection.c | 9 +++------
 2 files changed, 6 insertions(+), 11 deletions(-)

diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
index 5fb4c6d..f9df275 100644
--- a/drivers/hv/channel.c
+++ b/drivers/hv/channel.c
@@ -49,15 +49,13 @@ void vmbus_setevent(struct vmbus_channel *channel)
 	 */
 	if ((channel->offermsg.monitor_allocated) &&
 	    (!channel->low_latency)) {
-		/* Each u32 represents 32 channels */
-		sync_set_bit(channel->offermsg.child_relid & 31,
-			(unsigned long *) vmbus_connection.send_int_page +
-			(channel->offermsg.child_relid >> 5));
+		set_bit(channel->offermsg.child_relid,
+			(unsigned long *)vmbus_connection.send_int_page);
 
 		/* Get the child to parent monitor page */
 		monitorpage = vmbus_connection.monitor_pages[1];
 
-		sync_set_bit(channel->monitor_bit,
+		set_bit(channel->monitor_bit,
 			(unsigned long *)&monitorpage->trigger_group
 					[channel->monitor_grp].pending);
 
diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c
index aaa2103..139b33e 100644
--- a/drivers/hv/connection.c
+++ b/drivers/hv/connection.c
@@ -468,12 +468,9 @@ void vmbus_set_event(struct vmbus_channel *channel)
 {
 	u32 child_relid = channel->offermsg.child_relid;
 
-	if (!channel->is_dedicated_interrupt) {
-		/* Each u32 represents 32 channels */
-		sync_set_bit(child_relid & 31,
-			(unsigned long *)vmbus_connection.send_int_page +
-			(child_relid >> 5));
-	}
+	if (!channel->is_dedicated_interrupt)
+		set_bit(child_relid,
+			(unsigned long *)vmbus_connection.send_int_page);
 
 	hv_do_hypercall(HVCALL_SIGNAL_EVENT, channel->sig_event, NULL);
 }
-- 
2.9.3

^ permalink raw reply	[flat|nested] 54+ messages in thread

* [PATCH 04/15] hyperv: define VMBus message type
  2016-12-20 15:55 [PATCH 00/15] hyperv: more stuff to uapi + cleanup Roman Kagan
                   ` (2 preceding siblings ...)
  2016-12-20 15:55 ` [PATCH 03/15] hyperv: use standard bitops Roman Kagan
@ 2016-12-20 15:55 ` Roman Kagan
  2016-12-20 15:55 ` [PATCH 05/15] hyperv: GFP_ATOMIC -> GFP_KERNEL Roman Kagan
                   ` (11 subsequent siblings)
  15 siblings, 0 replies; 54+ messages in thread
From: Roman Kagan @ 2016-12-20 15:55 UTC (permalink / raw)
  To: Paolo Bonzini, Radim Krčmář,
	K. Y. Srinivasan, Vitaly Kuznetsov
  Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, Haiyang Zhang,
	kvm, linux-kernel, devel, Denis V . Lunev, Roman Kagan

Give a name to the constant (1) already used in the code.

Signed-off-by: Roman Kagan <rkagan@virtuozzo.com>
---
 arch/x86/include/uapi/asm/hyperv.h | 2 ++
 drivers/hv/connection.c            | 2 +-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/uapi/asm/hyperv.h b/arch/x86/include/uapi/asm/hyperv.h
index af542a3..749fbb25 100644
--- a/arch/x86/include/uapi/asm/hyperv.h
+++ b/arch/x86/include/uapi/asm/hyperv.h
@@ -282,6 +282,8 @@ struct hv_ref_tsc_page {
 enum hv_message_type {
 	HVMSG_NONE			= 0x00000000,
 
+	HVMSG_VMBUS			= 0x00000001,
+
 	/* Memory access messages. */
 	HVMSG_UNMAPPED_GPA		= 0x80000000,
 	HVMSG_GPA_INTERCEPT		= 0x80000001,
diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c
index 139b33e..d38b27f 100644
--- a/drivers/hv/connection.c
+++ b/drivers/hv/connection.c
@@ -432,7 +432,7 @@ int vmbus_post_msg(void *buffer, size_t buflen)
 	 * times before giving up.
 	 */
 	while (retries < 20) {
-		ret = hv_post_message(conn_id, 1, buffer, buflen);
+		ret = hv_post_message(conn_id, HVMSG_VMBUS, buffer, buflen);
 
 		switch (ret) {
 		case HV_STATUS_INVALID_CONNECTION_ID:
-- 
2.9.3

^ permalink raw reply	[flat|nested] 54+ messages in thread

* [PATCH 05/15] hyperv: GFP_ATOMIC -> GFP_KERNEL
  2016-12-20 15:55 [PATCH 00/15] hyperv: more stuff to uapi + cleanup Roman Kagan
                   ` (3 preceding siblings ...)
  2016-12-20 15:55 ` [PATCH 04/15] hyperv: define VMBus message type Roman Kagan
@ 2016-12-20 15:55 ` Roman Kagan
  2016-12-20 15:55 ` [PATCH 06/15] hyperv: avoid unnecessary vmalloc Roman Kagan
                   ` (10 subsequent siblings)
  15 siblings, 0 replies; 54+ messages in thread
From: Roman Kagan @ 2016-12-20 15:55 UTC (permalink / raw)
  To: Paolo Bonzini, Radim Krčmář,
	K. Y. Srinivasan, Vitaly Kuznetsov
  Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, Haiyang Zhang,
	kvm, linux-kernel, devel, Denis V . Lunev, Roman Kagan

There's no need in GFP_ATOMIC when initializing the driver state.

While at this, also rely on free_page() to take null argument.

Signed-off-by: Roman Kagan <rkagan@virtuozzo.com>
---
 drivers/hv/hv.c | 23 ++++++++++-------------
 1 file changed, 10 insertions(+), 13 deletions(-)

diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
index a7256ec..6bbc0b09 100644
--- a/drivers/hv/hv.c
+++ b/drivers/hv/hv.c
@@ -393,28 +393,28 @@ int hv_synic_alloc(void)
 	int cpu;
 
 	hv_context.hv_numa_map = kzalloc(sizeof(struct cpumask) * nr_node_ids,
-					 GFP_ATOMIC);
+					 GFP_KERNEL);
 	if (hv_context.hv_numa_map == NULL) {
 		pr_err("Unable to allocate NUMA map\n");
 		goto err;
 	}
 
 	for_each_online_cpu(cpu) {
-		hv_context.event_dpc[cpu] = kmalloc(size, GFP_ATOMIC);
+		hv_context.event_dpc[cpu] = kmalloc(size, GFP_KERNEL);
 		if (hv_context.event_dpc[cpu] == NULL) {
 			pr_err("Unable to allocate event dpc\n");
 			goto err;
 		}
 		tasklet_init(hv_context.event_dpc[cpu], vmbus_on_event, cpu);
 
-		hv_context.msg_dpc[cpu] = kmalloc(size, GFP_ATOMIC);
+		hv_context.msg_dpc[cpu] = kmalloc(size, GFP_KERNEL);
 		if (hv_context.msg_dpc[cpu] == NULL) {
 			pr_err("Unable to allocate event dpc\n");
 			goto err;
 		}
 		tasklet_init(hv_context.msg_dpc[cpu], vmbus_on_msg_dpc, cpu);
 
-		hv_context.clk_evt[cpu] = kzalloc(ced_size, GFP_ATOMIC);
+		hv_context.clk_evt[cpu] = kzalloc(ced_size, GFP_KERNEL);
 		if (hv_context.clk_evt[cpu] == NULL) {
 			pr_err("Unable to allocate clock event device\n");
 			goto err;
@@ -423,7 +423,7 @@ int hv_synic_alloc(void)
 		hv_init_clockevent_device(hv_context.clk_evt[cpu], cpu);
 
 		hv_context.synic_message_page[cpu] =
-			(void *)get_zeroed_page(GFP_ATOMIC);
+			(void *)get_zeroed_page(GFP_KERNEL);
 
 		if (hv_context.synic_message_page[cpu] == NULL) {
 			pr_err("Unable to allocate SYNIC message page\n");
@@ -431,7 +431,7 @@ int hv_synic_alloc(void)
 		}
 
 		hv_context.synic_event_page[cpu] =
-			(void *)get_zeroed_page(GFP_ATOMIC);
+			(void *)get_zeroed_page(GFP_KERNEL);
 
 		if (hv_context.synic_event_page[cpu] == NULL) {
 			pr_err("Unable to allocate SYNIC event page\n");
@@ -439,7 +439,7 @@ int hv_synic_alloc(void)
 		}
 
 		hv_context.post_msg_page[cpu] =
-			(void *)get_zeroed_page(GFP_ATOMIC);
+			(void *)get_zeroed_page(GFP_KERNEL);
 
 		if (hv_context.post_msg_page[cpu] == NULL) {
 			pr_err("Unable to allocate post msg page\n");
@@ -457,12 +457,9 @@ static void hv_synic_free_cpu(int cpu)
 	kfree(hv_context.event_dpc[cpu]);
 	kfree(hv_context.msg_dpc[cpu]);
 	kfree(hv_context.clk_evt[cpu]);
-	if (hv_context.synic_event_page[cpu])
-		free_page((unsigned long)hv_context.synic_event_page[cpu]);
-	if (hv_context.synic_message_page[cpu])
-		free_page((unsigned long)hv_context.synic_message_page[cpu]);
-	if (hv_context.post_msg_page[cpu])
-		free_page((unsigned long)hv_context.post_msg_page[cpu]);
+	free_page((unsigned long)hv_context.synic_event_page[cpu]);
+	free_page((unsigned long)hv_context.synic_message_page[cpu]);
+	free_page((unsigned long)hv_context.post_msg_page[cpu]);
 }
 
 void hv_synic_free(void)
-- 
2.9.3

^ permalink raw reply	[flat|nested] 54+ messages in thread

* [PATCH 06/15] hyperv: avoid unnecessary vmalloc
  2016-12-20 15:55 [PATCH 00/15] hyperv: more stuff to uapi + cleanup Roman Kagan
                   ` (4 preceding siblings ...)
  2016-12-20 15:55 ` [PATCH 05/15] hyperv: GFP_ATOMIC -> GFP_KERNEL Roman Kagan
@ 2016-12-20 15:55 ` Roman Kagan
  2016-12-21 19:19   ` KY Srinivasan
  2016-12-20 15:55 ` [PATCH 07/15] hyperv: dedup cpuid definitions Roman Kagan
                   ` (9 subsequent siblings)
  15 siblings, 1 reply; 54+ messages in thread
From: Roman Kagan @ 2016-12-20 15:55 UTC (permalink / raw)
  To: Paolo Bonzini, Radim Krčmář,
	K. Y. Srinivasan, Vitaly Kuznetsov
  Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, Haiyang Zhang,
	kvm, linux-kernel, devel, Denis V . Lunev, Roman Kagan

Make hypercall and tsc page allocation similar to the rest of the
Hyper-V shared memory stuff instead of vmalloc-ing them.

Also perform cleanup unconditionally which is safe.

TODO: the skipping of free in case of a crash is probably no longer
necessary, too.

Signed-off-by: Roman Kagan <rkagan@virtuozzo.com>
---
 drivers/hv/hv.c | 42 ++++++++++++++++++------------------------
 1 file changed, 18 insertions(+), 24 deletions(-)

diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
index 6bbc0b09..b40c7d9 100644
--- a/drivers/hv/hv.c
+++ b/drivers/hv/hv.c
@@ -29,6 +29,7 @@
 #include <linux/version.h>
 #include <linux/interrupt.h>
 #include <linux/clockchips.h>
+#include <asm/cacheflush.h>
 #include <asm/hyperv.h>
 #include <asm/mshyperv.h>
 #include "hyperv_vmbus.h"
@@ -208,14 +209,15 @@ int hv_init(void)
 	/* See if the hypercall page is already set */
 	rdmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64);
 
-	virtaddr = __vmalloc(PAGE_SIZE, GFP_KERNEL, PAGE_KERNEL_EXEC);
-
-	if (!virtaddr)
+	virtaddr = (void *)get_zeroed_page(GFP_KERNEL);
+	if (!virtaddr || set_memory_x((unsigned long)virtaddr, 1))
 		goto cleanup;
+	hv_context.hypercall_page = virtaddr;
 
 	hypercall_msr.enable = 1;
 
-	hypercall_msr.guest_physical_address = vmalloc_to_pfn(virtaddr);
+	hypercall_msr.guest_physical_address =
+				virt_to_phys(virtaddr) >> PAGE_SHIFT;
 	wrmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64);
 
 	/* Confirm that hypercall page did get setup. */
@@ -225,14 +227,12 @@ int hv_init(void)
 	if (!hypercall_msr.enable)
 		goto cleanup;
 
-	hv_context.hypercall_page = virtaddr;
-
 #ifdef CONFIG_X86_64
 	if (ms_hyperv.features & HV_X64_MSR_REFERENCE_TSC_AVAILABLE) {
 		union hv_x64_msr_hypercall_contents tsc_msr;
 		void *va_tsc;
 
-		va_tsc = __vmalloc(PAGE_SIZE, GFP_KERNEL, PAGE_KERNEL);
+		va_tsc = (void *)get_zeroed_page(GFP_KERNEL);
 		if (!va_tsc)
 			goto cleanup;
 		hv_context.tsc_page = va_tsc;
@@ -240,7 +240,8 @@ int hv_init(void)
 		rdmsrl(HV_X64_MSR_REFERENCE_TSC, tsc_msr.as_uint64);
 
 		tsc_msr.enable = 1;
-		tsc_msr.guest_physical_address = vmalloc_to_pfn(va_tsc);
+		tsc_msr.guest_physical_address =
+				virt_to_phys(va_tsc) >> PAGE_SHIFT;
 
 		wrmsrl(HV_X64_MSR_REFERENCE_TSC, tsc_msr.as_uint64);
 		clocksource_register_hz(&hyperv_cs_tsc, NSEC_PER_SEC/100);
@@ -249,14 +250,9 @@ int hv_init(void)
 	return 0;
 
 cleanup:
-	if (virtaddr) {
-		if (hypercall_msr.enable) {
-			hypercall_msr.as_uint64 = 0;
-			wrmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64);
-		}
-
-		vfree(virtaddr);
-	}
+	hypercall_msr.as_uint64 = 0;
+	wrmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64);
+	free_page((unsigned long)virtaddr);
 
 	return -ENOTSUPP;
 }
@@ -273,13 +269,11 @@ void hv_cleanup(bool crash)
 	/* Reset our OS id */
 	wrmsrl(HV_X64_MSR_GUEST_OS_ID, 0);
 
-	if (hv_context.hypercall_page) {
-		hypercall_msr.as_uint64 = 0;
-		wrmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64);
-		if (!crash)
-			vfree(hv_context.hypercall_page);
-		hv_context.hypercall_page = NULL;
-	}
+	hypercall_msr.as_uint64 = 0;
+	wrmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64);
+	if (!crash)
+		free_page((unsigned long)hv_context.hypercall_page);
+	hv_context.hypercall_page = NULL;
 
 #ifdef CONFIG_X86_64
 	/*
@@ -298,7 +292,7 @@ void hv_cleanup(bool crash)
 		hypercall_msr.as_uint64 = 0;
 		wrmsrl(HV_X64_MSR_REFERENCE_TSC, hypercall_msr.as_uint64);
 		if (!crash)
-			vfree(hv_context.tsc_page);
+			free_page((unsigned long)hv_context.tsc_page);
 		hv_context.tsc_page = NULL;
 	}
 #endif
-- 
2.9.3

^ permalink raw reply	[flat|nested] 54+ messages in thread

* [PATCH 07/15] hyperv: dedup cpuid definitions
  2016-12-20 15:55 [PATCH 00/15] hyperv: more stuff to uapi + cleanup Roman Kagan
                   ` (5 preceding siblings ...)
  2016-12-20 15:55 ` [PATCH 06/15] hyperv: avoid unnecessary vmalloc Roman Kagan
@ 2016-12-20 15:55 ` Roman Kagan
  2016-12-20 15:55 ` [PATCH 08/15] hyperv: dedup crash msr related definitions Roman Kagan
                   ` (8 subsequent siblings)
  15 siblings, 0 replies; 54+ messages in thread
From: Roman Kagan @ 2016-12-20 15:55 UTC (permalink / raw)
  To: Paolo Bonzini, Radim Krčmář,
	K. Y. Srinivasan, Vitaly Kuznetsov
  Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, Haiyang Zhang,
	kvm, linux-kernel, devel, Denis V . Lunev, Roman Kagan

Use the definitions already present in the uapi header throughout the
guest driver, too.

Signed-off-by: Roman Kagan <rkagan@virtuozzo.com>
---
 drivers/hv/hyperv_vmbus.h | 19 -------------------
 drivers/hv/hv.c           |  6 +++---
 2 files changed, 3 insertions(+), 22 deletions(-)

diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
index 4fab154..9b0f1c9 100644
--- a/drivers/hv/hyperv_vmbus.h
+++ b/drivers/hv/hyperv_vmbus.h
@@ -39,25 +39,6 @@
  */
 #define HV_UTIL_NEGO_TIMEOUT 55
 
-/*
- * The below CPUID leaves are present if VersionAndFeatures.HypervisorPresent
- * is set by CPUID(HVCPUID_VERSION_FEATURES).
- */
-enum hv_cpuid_function {
-	HVCPUID_VERSION_FEATURES		= 0x00000001,
-	HVCPUID_VENDOR_MAXFUNCTION		= 0x40000000,
-	HVCPUID_INTERFACE			= 0x40000001,
-
-	/*
-	 * The remaining functions depend on the value of
-	 * HVCPUID_INTERFACE
-	 */
-	HVCPUID_VERSION			= 0x40000002,
-	HVCPUID_FEATURES			= 0x40000003,
-	HVCPUID_ENLIGHTENMENT_INFO	= 0x40000004,
-	HVCPUID_IMPLEMENTATION_LIMITS		= 0x40000005,
-};
-
 #define  HV_FEATURE_GUEST_CRASH_MSR_AVAILABLE   0x400
 
 #define HV_X64_MSR_CRASH_P0   0x40000100
diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
index b40c7d9..dddba07 100644
--- a/drivers/hv/hv.c
+++ b/drivers/hv/hv.c
@@ -69,17 +69,17 @@ static int query_hypervisor_info(void)
 	ebx = 0;
 	ecx = 0;
 	edx = 0;
-	op = HVCPUID_VENDOR_MAXFUNCTION;
+	op = HYPERV_CPUID_VENDOR_AND_MAX_FUNCTIONS;
 	cpuid(op, &eax, &ebx, &ecx, &edx);
 
 	max_leaf = eax;
 
-	if (max_leaf >= HVCPUID_VERSION) {
+	if (max_leaf >= HYPERV_CPUID_VERSION) {
 		eax = 0;
 		ebx = 0;
 		ecx = 0;
 		edx = 0;
-		op = HVCPUID_VERSION;
+		op = HYPERV_CPUID_VERSION;
 		cpuid(op, &eax, &ebx, &ecx, &edx);
 		host_info_eax = eax;
 		host_info_ebx = ebx;
-- 
2.9.3

^ permalink raw reply	[flat|nested] 54+ messages in thread

* [PATCH 08/15] hyperv: dedup crash msr related definitions
  2016-12-20 15:55 [PATCH 00/15] hyperv: more stuff to uapi + cleanup Roman Kagan
                   ` (6 preceding siblings ...)
  2016-12-20 15:55 ` [PATCH 07/15] hyperv: dedup cpuid definitions Roman Kagan
@ 2016-12-20 15:55 ` Roman Kagan
  2016-12-20 15:55 ` [PATCH 09/15] hyperv: unify Hyper-V msr definitions Roman Kagan
                   ` (7 subsequent siblings)
  15 siblings, 0 replies; 54+ messages in thread
From: Roman Kagan @ 2016-12-20 15:55 UTC (permalink / raw)
  To: Paolo Bonzini, Radim Krčmář,
	K. Y. Srinivasan, Vitaly Kuznetsov
  Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, Haiyang Zhang,
	kvm, linux-kernel, devel, Denis V . Lunev, Roman Kagan

Use the definitions already present in the uapi header throughout the
guest driver, too.

Signed-off-by: Roman Kagan <rkagan@virtuozzo.com>
---
 drivers/hv/hyperv_vmbus.h | 11 -----------
 drivers/hv/vmbus_drv.c    |  6 +++---
 2 files changed, 3 insertions(+), 14 deletions(-)

diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
index 9b0f1c9..7bf1b10 100644
--- a/drivers/hv/hyperv_vmbus.h
+++ b/drivers/hv/hyperv_vmbus.h
@@ -39,17 +39,6 @@
  */
 #define HV_UTIL_NEGO_TIMEOUT 55
 
-#define  HV_FEATURE_GUEST_CRASH_MSR_AVAILABLE   0x400
-
-#define HV_X64_MSR_CRASH_P0   0x40000100
-#define HV_X64_MSR_CRASH_P1   0x40000101
-#define HV_X64_MSR_CRASH_P2   0x40000102
-#define HV_X64_MSR_CRASH_P3   0x40000103
-#define HV_X64_MSR_CRASH_P4   0x40000104
-#define HV_X64_MSR_CRASH_CTL  0x40000105
-
-#define HV_CRASH_CTL_CRASH_NOTIFY (1ULL << 63)
-
 /* Define version of the synthetic interrupt controller. */
 #define HV_SYNIC_VERSION		(1)
 
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 13dd210..7564a7b 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -77,7 +77,7 @@ static void hyperv_report_panic(struct pt_regs *regs)
 	/*
 	 * Let Hyper-V know there is crash data available
 	 */
-	wrmsrl(HV_X64_MSR_CRASH_CTL, HV_CRASH_CTL_CRASH_NOTIFY);
+	wrmsrl(HV_X64_MSR_CRASH_CTL, HV_X64_MSR_CRASH_CTL_NOTIFY);
 }
 
 static int hyperv_panic_event(struct notifier_block *nb, unsigned long val,
@@ -993,7 +993,7 @@ static int vmbus_bus_init(void)
 	/*
 	 * Only register if the crash MSRs are available
 	 */
-	if (ms_hyperv.misc_features & HV_FEATURE_GUEST_CRASH_MSR_AVAILABLE) {
+	if (ms_hyperv.misc_features & HV_X64_GUEST_CRASH_MSR_AVAILABLE) {
 		register_die_notifier(&hyperv_die_block);
 		atomic_notifier_chain_register(&panic_notifier_list,
 					       &hyperv_panic_block);
@@ -1535,7 +1535,7 @@ static void __exit vmbus_exit(void)
 	for_each_online_cpu(cpu)
 		tasklet_kill(hv_context.msg_dpc[cpu]);
 	vmbus_free_channels();
-	if (ms_hyperv.misc_features & HV_FEATURE_GUEST_CRASH_MSR_AVAILABLE) {
+	if (ms_hyperv.misc_features & HV_X64_GUEST_CRASH_MSR_AVAILABLE) {
 		unregister_die_notifier(&hyperv_die_block);
 		atomic_notifier_chain_unregister(&panic_notifier_list,
 						 &hyperv_panic_block);
-- 
2.9.3

^ permalink raw reply	[flat|nested] 54+ messages in thread

* [PATCH 09/15] hyperv: unify Hyper-V msr definitions
  2016-12-20 15:55 [PATCH 00/15] hyperv: more stuff to uapi + cleanup Roman Kagan
                   ` (7 preceding siblings ...)
  2016-12-20 15:55 ` [PATCH 08/15] hyperv: dedup crash msr related definitions Roman Kagan
@ 2016-12-20 15:55 ` Roman Kagan
  2016-12-20 15:55 ` [PATCH 10/15] hyperv: uapi-fy PostMessage and SignalEvent hypercall structures Roman Kagan
                   ` (6 subsequent siblings)
  15 siblings, 0 replies; 54+ messages in thread
From: Roman Kagan @ 2016-12-20 15:55 UTC (permalink / raw)
  To: Paolo Bonzini, Radim Krčmář,
	K. Y. Srinivasan, Vitaly Kuznetsov
  Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, Haiyang Zhang,
	kvm, linux-kernel, devel, Denis V . Lunev, Roman Kagan

Use the definitions already present in the uapi header.  Besides, drop
all bitfields for the msr values and use bitwise operations instead.

Signed-off-by: Roman Kagan <rkagan@virtuozzo.com>
---
 drivers/hv/hyperv_vmbus.h |  88 ---------------------------
 drivers/hv/hv.c           | 150 ++++++++++++++++++----------------------------
 2 files changed, 59 insertions(+), 179 deletions(-)

diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
index 7bf1b10..ac73832 100644
--- a/drivers/hv/hyperv_vmbus.h
+++ b/drivers/hv/hyperv_vmbus.h
@@ -94,63 +94,6 @@ struct hv_connection_info {
 	};
 };
 
-/*
- * Timer configuration register.
- */
-union hv_timer_config {
-	u64 as_uint64;
-	struct {
-		u64 enable:1;
-		u64 periodic:1;
-		u64 lazy:1;
-		u64 auto_enable:1;
-		u64 reserved_z0:12;
-		u64 sintx:4;
-		u64 reserved_z1:44;
-	};
-};
-
-/* Define SynIC control register. */
-union hv_synic_scontrol {
-	u64 as_uint64;
-	struct {
-		u64 enable:1;
-		u64 reserved:63;
-	};
-};
-
-/* Define synthetic interrupt source. */
-union hv_synic_sint {
-	u64 as_uint64;
-	struct {
-		u64 vector:8;
-		u64 reserved1:8;
-		u64 masked:1;
-		u64 auto_eoi:1;
-		u64 reserved2:46;
-	};
-};
-
-/* Define the format of the SIMP register */
-union hv_synic_simp {
-	u64 as_uint64;
-	struct {
-		u64 simp_enabled:1;
-		u64 preserved:11;
-		u64 base_simp_gpa:52;
-	};
-};
-
-/* Define the format of the SIEFP register */
-union hv_synic_siefp {
-	u64 as_uint64;
-	struct {
-		u64 siefp_enabled:1;
-		u64 preserved:11;
-		u64 base_siefp_gpa:52;
-	};
-};
-
 /* Definitions for the monitored notification facility */
 union hv_monitor_trigger_group {
 	u64 as_uint64;
@@ -239,37 +182,6 @@ enum hv_guest_os_microsoft_ids {
 	HVGUESTOS_MICROSOFT_WINDOWSCE	= 0x05
 };
 
-/*
- * Declare the MSR used to identify the guest OS.
- */
-#define HV_X64_MSR_GUEST_OS_ID	0x40000000
-
-union hv_x64_msr_guest_os_id_contents {
-	u64 as_uint64;
-	struct {
-		u64 build_number:16;
-		u64 service_version:8; /* Service Pack, etc. */
-		u64 minor_version:8;
-		u64 major_version:8;
-		u64 os_id:8; /* enum hv_guest_os_microsoft_ids (if Vendor=MS) */
-		u64 vendor_id:16; /* enum hv_guest_os_vendor */
-	};
-};
-
-/*
- * Declare the MSR used to setup pages used to communicate with the hypervisor.
- */
-#define HV_X64_MSR_HYPERCALL	0x40000001
-
-union hv_x64_msr_hypercall_contents {
-	u64 as_uint64;
-	struct {
-		u64 enable:1;
-		u64 reserved:11;
-		u64 guest_physical_address:52;
-	};
-};
-
 
 enum {
 	VMBUS_MESSAGE_CONNECTION_ID	= 1,
diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
index dddba07..7d2a3d1 100644
--- a/drivers/hv/hv.c
+++ b/drivers/hv/hv.c
@@ -181,7 +181,7 @@ static struct clocksource hyperv_cs_tsc = {
 int hv_init(void)
 {
 	int max_leaf;
-	union hv_x64_msr_hypercall_contents hypercall_msr;
+	u64 hypercall_msr = 0;
 	void *virtaddr = NULL;
 
 	memset(hv_context.synic_event_page, 0, sizeof(void *) * NR_CPUS);
@@ -206,30 +206,24 @@ int hv_init(void)
 	hv_context.guestid = generate_guest_id(0, LINUX_VERSION_CODE, 0);
 	wrmsrl(HV_X64_MSR_GUEST_OS_ID, hv_context.guestid);
 
-	/* See if the hypercall page is already set */
-	rdmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64);
-
 	virtaddr = (void *)get_zeroed_page(GFP_KERNEL);
 	if (!virtaddr || set_memory_x((unsigned long)virtaddr, 1))
 		goto cleanup;
 	hv_context.hypercall_page = virtaddr;
 
-	hypercall_msr.enable = 1;
-
-	hypercall_msr.guest_physical_address =
-				virt_to_phys(virtaddr) >> PAGE_SHIFT;
-	wrmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64);
+	rdmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr);
+	hypercall_msr &= PAGE_MASK;
+	hypercall_msr = HV_X64_MSR_HYPERCALL_ENABLE | virt_to_phys(virtaddr);
+	wrmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr);
 
 	/* Confirm that hypercall page did get setup. */
-	hypercall_msr.as_uint64 = 0;
-	rdmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64);
-
-	if (!hypercall_msr.enable)
+	rdmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr);
+	if (!(hypercall_msr & HV_X64_MSR_HYPERCALL_ENABLE))
 		goto cleanup;
 
 #ifdef CONFIG_X86_64
 	if (ms_hyperv.features & HV_X64_MSR_REFERENCE_TSC_AVAILABLE) {
-		union hv_x64_msr_hypercall_contents tsc_msr;
+		u64 tsc_msr;
 		void *va_tsc;
 
 		va_tsc = (void *)get_zeroed_page(GFP_KERNEL);
@@ -237,21 +231,19 @@ int hv_init(void)
 			goto cleanup;
 		hv_context.tsc_page = va_tsc;
 
-		rdmsrl(HV_X64_MSR_REFERENCE_TSC, tsc_msr.as_uint64);
-
-		tsc_msr.enable = 1;
-		tsc_msr.guest_physical_address =
-				virt_to_phys(va_tsc) >> PAGE_SHIFT;
-
-		wrmsrl(HV_X64_MSR_REFERENCE_TSC, tsc_msr.as_uint64);
+		rdmsrl(HV_X64_MSR_REFERENCE_TSC, tsc_msr);
+		tsc_msr &= PAGE_MASK;
+		tsc_msr |= HV_X64_MSR_TSC_REFERENCE_ENABLE |
+			virt_to_phys(va_tsc);
+		wrmsrl(HV_X64_MSR_REFERENCE_TSC, tsc_msr);
 		clocksource_register_hz(&hyperv_cs_tsc, NSEC_PER_SEC/100);
 	}
 #endif
 	return 0;
 
 cleanup:
-	hypercall_msr.as_uint64 = 0;
-	wrmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64);
+	hypercall_msr &= ~HV_X64_MSR_HYPERCALL_ENABLE;
+	wrmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr);
 	free_page((unsigned long)virtaddr);
 
 	return -ENOTSUPP;
@@ -264,13 +256,14 @@ int hv_init(void)
  */
 void hv_cleanup(bool crash)
 {
-	union hv_x64_msr_hypercall_contents hypercall_msr;
+	u64 msr;
 
 	/* Reset our OS id */
 	wrmsrl(HV_X64_MSR_GUEST_OS_ID, 0);
 
-	hypercall_msr.as_uint64 = 0;
-	wrmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64);
+	rdmsrl(HV_X64_MSR_HYPERCALL, msr);
+	msr &= ~HV_X64_MSR_HYPERCALL_ENABLE;
+	wrmsrl(HV_X64_MSR_HYPERCALL, msr);
 	if (!crash)
 		free_page((unsigned long)hv_context.hypercall_page);
 	hv_context.hypercall_page = NULL;
@@ -289,8 +282,9 @@ void hv_cleanup(bool crash)
 			clocksource_unregister(&hyperv_cs_tsc);
 		}
 
-		hypercall_msr.as_uint64 = 0;
-		wrmsrl(HV_X64_MSR_REFERENCE_TSC, hypercall_msr.as_uint64);
+		rdmsrl(HV_X64_MSR_REFERENCE_TSC, msr);
+		msr &= ~HV_X64_MSR_TSC_REFERENCE_ENABLE;
+		wrmsrl(HV_X64_MSR_REFERENCE_TSC, msr);
 		if (!crash)
 			free_page((unsigned long)hv_context.tsc_page);
 		hv_context.tsc_page = NULL;
@@ -352,12 +346,10 @@ static int hv_ce_shutdown(struct clock_event_device *evt)
 
 static int hv_ce_set_oneshot(struct clock_event_device *evt)
 {
-	union hv_timer_config timer_cfg;
+	u64 timer_cfg = HV_STIMER_ENABLE | HV_STIMER_AUTOENABLE |
+		(VMBUS_MESSAGE_SINT << 16);
 
-	timer_cfg.enable = 1;
-	timer_cfg.auto_enable = 1;
-	timer_cfg.sintx = VMBUS_MESSAGE_SINT;
-	wrmsrl(HV_X64_MSR_STIMER0_CONFIG, timer_cfg.as_uint64);
+	wrmsrl(HV_X64_MSR_STIMER0_CONFIG, timer_cfg);
 
 	return 0;
 }
@@ -474,52 +466,39 @@ void hv_synic_free(void)
  */
 void hv_synic_init(void *arg)
 {
-	u64 version;
-	union hv_synic_simp simp;
-	union hv_synic_siefp siefp;
-	union hv_synic_sint shared_sint;
-	union hv_synic_scontrol sctrl;
-	u64 vp_index;
-
+	u64 msr;
 	int cpu = smp_processor_id();
 
 	if (!hv_context.hypercall_page)
 		return;
 
 	/* Check the version */
-	rdmsrl(HV_X64_MSR_SVERSION, version);
+	rdmsrl(HV_X64_MSR_SVERSION, msr);
 
 	/* Setup the Synic's message page */
-	rdmsrl(HV_X64_MSR_SIMP, simp.as_uint64);
-	simp.simp_enabled = 1;
-	simp.base_simp_gpa = virt_to_phys(hv_context.synic_message_page[cpu])
-		>> PAGE_SHIFT;
-
-	wrmsrl(HV_X64_MSR_SIMP, simp.as_uint64);
+	rdmsrl(HV_X64_MSR_SIMP, msr);
+	msr &= PAGE_MASK;
+	msr |= virt_to_phys(hv_context.synic_message_page[cpu]) |
+		HV_SYNIC_SIMP_ENABLE;
+	wrmsrl(HV_X64_MSR_SIMP, msr);
 
 	/* Setup the Synic's event page */
-	rdmsrl(HV_X64_MSR_SIEFP, siefp.as_uint64);
-	siefp.siefp_enabled = 1;
-	siefp.base_siefp_gpa = virt_to_phys(hv_context.synic_event_page[cpu])
-		>> PAGE_SHIFT;
-
-	wrmsrl(HV_X64_MSR_SIEFP, siefp.as_uint64);
+	rdmsrl(HV_X64_MSR_SIEFP, msr);
+	msr &= PAGE_MASK;
+	msr |= virt_to_phys(hv_context.synic_event_page[cpu]) |
+		HV_SYNIC_SIEFP_ENABLE;
+	wrmsrl(HV_X64_MSR_SIEFP, msr);
 
 	/* Setup the shared SINT. */
-	rdmsrl(HV_X64_MSR_SINT0 + VMBUS_MESSAGE_SINT, shared_sint.as_uint64);
-
-	shared_sint.as_uint64 = 0;
-	shared_sint.vector = HYPERVISOR_CALLBACK_VECTOR;
-	shared_sint.masked = false;
-	shared_sint.auto_eoi = true;
-
-	wrmsrl(HV_X64_MSR_SINT0 + VMBUS_MESSAGE_SINT, shared_sint.as_uint64);
+	rdmsrl(HV_X64_MSR_SINT0 + VMBUS_MESSAGE_SINT, msr);
+	msr &= ~(HV_SYNIC_SINT_MASKED | HV_SYNIC_SINT_VECTOR_MASK);
+	msr |= HYPERVISOR_CALLBACK_VECTOR | HV_SYNIC_SINT_AUTO_EOI;
+	wrmsrl(HV_X64_MSR_SINT0 + VMBUS_MESSAGE_SINT, msr);
 
 	/* Enable the global synic bit */
-	rdmsrl(HV_X64_MSR_SCONTROL, sctrl.as_uint64);
-	sctrl.enable = 1;
-
-	wrmsrl(HV_X64_MSR_SCONTROL, sctrl.as_uint64);
+	rdmsrl(HV_X64_MSR_SCONTROL, msr);
+	msr |= HV_SYNIC_CONTROL_ENABLE;
+	wrmsrl(HV_X64_MSR_SCONTROL, msr);
 
 	hv_context.synic_initialized = true;
 
@@ -528,8 +507,8 @@ void hv_synic_init(void *arg)
 	 * of cpuid and Linux' notion of cpuid.
 	 * This array will be indexed using Linux cpuid.
 	 */
-	rdmsrl(HV_X64_MSR_VP_INDEX, vp_index);
-	hv_context.vp_index[cpu] = (u32)vp_index;
+	rdmsrl(HV_X64_MSR_VP_INDEX, msr);
+	hv_context.vp_index[cpu] = (u32)msr;
 
 	INIT_LIST_HEAD(&hv_context.percpu_list[cpu]);
 
@@ -563,10 +542,7 @@ void hv_synic_clockevents_cleanup(void)
  */
 void hv_synic_cleanup(void *arg)
 {
-	union hv_synic_sint shared_sint;
-	union hv_synic_simp simp;
-	union hv_synic_siefp siefp;
-	union hv_synic_scontrol sctrl;
+	u64 msr;
 	int cpu = smp_processor_id();
 
 	if (!hv_context.synic_initialized)
@@ -578,28 +554,20 @@ void hv_synic_cleanup(void *arg)
 		hv_ce_shutdown(hv_context.clk_evt[cpu]);
 	}
 
-	rdmsrl(HV_X64_MSR_SINT0 + VMBUS_MESSAGE_SINT, shared_sint.as_uint64);
-
-	shared_sint.masked = 1;
-
-	/* Need to correctly cleanup in the case of SMP!!! */
-	/* Disable the interrupt */
-	wrmsrl(HV_X64_MSR_SINT0 + VMBUS_MESSAGE_SINT, shared_sint.as_uint64);
-
-	rdmsrl(HV_X64_MSR_SIMP, simp.as_uint64);
-	simp.simp_enabled = 0;
-	simp.base_simp_gpa = 0;
-
-	wrmsrl(HV_X64_MSR_SIMP, simp.as_uint64);
+	rdmsrl(HV_X64_MSR_SINT0 + VMBUS_MESSAGE_SINT, msr);
+	msr |= HV_SYNIC_SINT_MASKED;
+	wrmsrl(HV_X64_MSR_SINT0 + VMBUS_MESSAGE_SINT, msr);
 
-	rdmsrl(HV_X64_MSR_SIEFP, siefp.as_uint64);
-	siefp.siefp_enabled = 0;
-	siefp.base_siefp_gpa = 0;
+	rdmsrl(HV_X64_MSR_SIMP, msr);
+	msr &= ~HV_SYNIC_SIMP_ENABLE;
+	wrmsrl(HV_X64_MSR_SIMP, msr);
 
-	wrmsrl(HV_X64_MSR_SIEFP, siefp.as_uint64);
+	rdmsrl(HV_X64_MSR_SIEFP, msr);
+	msr &= ~HV_SYNIC_SIEFP_ENABLE;
+	wrmsrl(HV_X64_MSR_SIEFP, msr);
 
 	/* Disable the global synic bit */
-	rdmsrl(HV_X64_MSR_SCONTROL, sctrl.as_uint64);
-	sctrl.enable = 0;
-	wrmsrl(HV_X64_MSR_SCONTROL, sctrl.as_uint64);
+	rdmsrl(HV_X64_MSR_SCONTROL, msr);
+	msr &= ~HV_SYNIC_CONTROL_ENABLE;
+	wrmsrl(HV_X64_MSR_SCONTROL, msr);
 }
-- 
2.9.3

^ permalink raw reply	[flat|nested] 54+ messages in thread

* [PATCH 10/15] hyperv: uapi-fy PostMessage and SignalEvent hypercall structures
  2016-12-20 15:55 [PATCH 00/15] hyperv: more stuff to uapi + cleanup Roman Kagan
                   ` (8 preceding siblings ...)
  2016-12-20 15:55 ` [PATCH 09/15] hyperv: unify Hyper-V msr definitions Roman Kagan
@ 2016-12-20 15:55 ` Roman Kagan
  2016-12-21 19:27   ` KY Srinivasan
  2016-12-20 15:55 ` [PATCH 11/15] hyperv: uapi-fy monitored notification structures Roman Kagan
                   ` (5 subsequent siblings)
  15 siblings, 1 reply; 54+ messages in thread
From: Roman Kagan @ 2016-12-20 15:55 UTC (permalink / raw)
  To: Paolo Bonzini, Radim Krčmář,
	K. Y. Srinivasan, Vitaly Kuznetsov
  Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, Haiyang Zhang,
	kvm, linux-kernel, devel, Denis V . Lunev, Roman Kagan

Expose structures used for PostMessage and SignalEvent hypercalls in a
uapi header.  While doing so, simplify alignment handling and drop
unnecessary complications in the connectionid field.

Signed-off-by: Roman Kagan <rkagan@virtuozzo.com>
---
 arch/x86/include/uapi/asm/hyperv.h | 18 ++++++++++++++++++
 drivers/hv/hyperv_vmbus.h          | 16 ++--------------
 include/linux/hyperv.h             | 24 +-----------------------
 drivers/hv/channel_mgmt.c          | 14 ++++----------
 drivers/hv/connection.c            |  9 +++------
 drivers/hv/hv.c                    |  2 +-
 drivers/hv/vmbus_drv.c             |  2 +-
 7 files changed, 30 insertions(+), 55 deletions(-)

diff --git a/arch/x86/include/uapi/asm/hyperv.h b/arch/x86/include/uapi/asm/hyperv.h
index 749fbb25..eb8d42a 100644
--- a/arch/x86/include/uapi/asm/hyperv.h
+++ b/arch/x86/include/uapi/asm/hyperv.h
@@ -378,4 +378,22 @@ struct hv_synic_event_flags_page {
 	struct hv_synic_event_flags sintevent_flags[HV_SYNIC_SINT_COUNT];
 };
 
+#define HV_HYPERCALL_PARAM_ALIGN	8
+
+/* Definition of the hv_post_message hypercall input structure. */
+struct hv_input_post_message {
+	__u32 connectionid;
+	__u32 reserved;
+	__u32 message_type;
+	__u32 payload_size;
+	__u64 payload[HV_MESSAGE_PAYLOAD_BYTE_COUNT];
+} __attribute__((aligned(HV_HYPERCALL_PARAM_ALIGN)));
+
+/* Definition of the hv_signal_event hypercall input structure. */
+struct hv_input_signal_event {
+	__u32 connectionid;
+	__u16 flag_number;
+	__u16 rsvdz;
+} __attribute__((aligned(HV_HYPERCALL_PARAM_ALIGN)));
+
 #endif
diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
index ac73832..a96f021 100644
--- a/drivers/hv/hyperv_vmbus.h
+++ b/drivers/hv/hyperv_vmbus.h
@@ -104,7 +104,7 @@ union hv_monitor_trigger_group {
 };
 
 struct hv_monitor_parameter {
-	union hv_connection_id connectionid;
+	u32 connectionid;
 	u16 flagnumber;
 	u16 rsvdz;
 };
@@ -154,15 +154,6 @@ struct hv_monitor_page {
 	u8 rsvdz4[1984];
 };
 
-/* Definition of the hv_post_message hypercall input structure. */
-struct hv_input_post_message {
-	union hv_connection_id connectionid;
-	u32 reserved;
-	u32 message_type;
-	u32 payload_size;
-	u64 payload[HV_MESSAGE_PAYLOAD_QWORD_COUNT];
-};
-
 /*
  * Versioning definitions used for guests reporting themselves to the
  * hypervisor, and visa versa.
@@ -248,9 +239,6 @@ static inline  __u64 generate_guest_id(__u8 d_info1, __u32 kernel_version,
 #define HV_CAPS_MAX			8
 
 
-#define HV_HYPERCALL_PARAM_ALIGN	sizeof(u64)
-
-
 /* Service definitions */
 
 #define HV_SERVICE_PARENT_PORT				(0)
@@ -351,7 +339,7 @@ extern int hv_init(void);
 
 extern void hv_cleanup(bool crash);
 
-extern int hv_post_message(union hv_connection_id connection_id,
+extern int hv_post_message(u32 connection_id,
 			 enum hv_message_type message_type,
 			 void *payload, size_t payload_size);
 
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index 42fe43f..e92446e 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -648,27 +648,6 @@ struct vmbus_close_msg {
 	struct vmbus_channel_close_channel msg;
 };
 
-/* Define connection identifier type. */
-union hv_connection_id {
-	u32 asu32;
-	struct {
-		u32 id:24;
-		u32 reserved:8;
-	} u;
-};
-
-/* Definition of the hv_signal_event hypercall input structure. */
-struct hv_input_signal_event {
-	union hv_connection_id connectionid;
-	u16 flag_number;
-	u16 rsvdz;
-};
-
-struct hv_input_signal_event_buffer {
-	u64 align8;
-	struct hv_input_signal_event event;
-};
-
 enum hv_signal_policy {
 	HV_SIGNAL_POLICY_DEFAULT = 0,
 	HV_SIGNAL_POLICY_EXPLICIT,
@@ -755,8 +734,7 @@ struct vmbus_channel {
 	bool batched_reading;
 
 	bool is_dedicated_interrupt;
-	struct hv_input_signal_event_buffer sig_buf;
-	struct hv_input_signal_event *sig_event;
+	struct hv_input_signal_event sig_event;
 
 	/*
 	 * Starting with win8, this field will be used to specify
diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index 49eaae2..4a5cc11 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -766,20 +766,14 @@ static void vmbus_onoffer(struct vmbus_channel_message_header *hdr)
 	/*
 	 * Setup state for signalling the host.
 	 */
-	newchannel->sig_event = (struct hv_input_signal_event *)
-				(ALIGN((unsigned long)
-				&newchannel->sig_buf,
-				HV_HYPERCALL_PARAM_ALIGN));
-
-	newchannel->sig_event->connectionid.asu32 = 0;
-	newchannel->sig_event->connectionid.u.id = VMBUS_EVENT_CONNECTION_ID;
-	newchannel->sig_event->flag_number = 0;
-	newchannel->sig_event->rsvdz = 0;
+	newchannel->sig_event.connectionid = VMBUS_EVENT_CONNECTION_ID;
+	newchannel->sig_event.flag_number = 0;
+	newchannel->sig_event.rsvdz = 0;
 
 	if (vmbus_proto_version != VERSION_WS2008) {
 		newchannel->is_dedicated_interrupt =
 				(offer->is_dedicated_interrupt != 0);
-		newchannel->sig_event->connectionid.u.id =
+		newchannel->sig_event.connectionid =
 				offer->connection_id;
 	}
 
diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c
index d38b27f..16ae977 100644
--- a/drivers/hv/connection.c
+++ b/drivers/hv/connection.c
@@ -418,21 +418,18 @@ void vmbus_on_event(unsigned long data)
  */
 int vmbus_post_msg(void *buffer, size_t buflen)
 {
-	union hv_connection_id conn_id;
 	int ret = 0;
 	int retries = 0;
 	u32 usec = 1;
 
-	conn_id.asu32 = 0;
-	conn_id.u.id = VMBUS_MESSAGE_CONNECTION_ID;
-
 	/*
 	 * hv_post_message() can have transient failures because of
 	 * insufficient resources. Retry the operation a couple of
 	 * times before giving up.
 	 */
 	while (retries < 20) {
-		ret = hv_post_message(conn_id, HVMSG_VMBUS, buffer, buflen);
+		ret = hv_post_message(VMBUS_MESSAGE_CONNECTION_ID, HVMSG_VMBUS,
+				      buffer, buflen);
 
 		switch (ret) {
 		case HV_STATUS_INVALID_CONNECTION_ID:
@@ -472,6 +469,6 @@ void vmbus_set_event(struct vmbus_channel *channel)
 		set_bit(child_relid,
 			(unsigned long *)vmbus_connection.send_int_page);
 
-	hv_do_hypercall(HVCALL_SIGNAL_EVENT, channel->sig_event, NULL);
+	hv_do_hypercall(HVCALL_SIGNAL_EVENT, &channel->sig_event, NULL);
 }
 EXPORT_SYMBOL_GPL(vmbus_set_event);
diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
index 7d2a3d1..b9f50de 100644
--- a/drivers/hv/hv.c
+++ b/drivers/hv/hv.c
@@ -297,7 +297,7 @@ void hv_cleanup(bool crash)
  *
  * This involves a hypercall.
  */
-int hv_post_message(union hv_connection_id connection_id,
+int hv_post_message(u32 connection_id,
 		  enum hv_message_type message_type,
 		  void *payload, size_t payload_size)
 {
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 7564a7b..f6b626b 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -159,7 +159,7 @@ static u32 channel_conn_id(struct vmbus_channel *channel,
 {
 	u8 monitor_group = channel_monitor_group(channel);
 	u8 monitor_offset = channel_monitor_offset(channel);
-	return monitor_page->parameter[monitor_group][monitor_offset].connectionid.u.id;
+	return monitor_page->parameter[monitor_group][monitor_offset].connectionid;
 }
 
 static ssize_t id_show(struct device *dev, struct device_attribute *dev_attr,
-- 
2.9.3

^ permalink raw reply	[flat|nested] 54+ messages in thread

* [PATCH 11/15] hyperv: uapi-fy monitored notification structures
  2016-12-20 15:55 [PATCH 00/15] hyperv: more stuff to uapi + cleanup Roman Kagan
                   ` (9 preceding siblings ...)
  2016-12-20 15:55 ` [PATCH 10/15] hyperv: uapi-fy PostMessage and SignalEvent hypercall structures Roman Kagan
@ 2016-12-20 15:55 ` Roman Kagan
  2016-12-20 15:55 ` [PATCH 12/15] hyperv: move VMBus connection ids to uapi Roman Kagan
                   ` (4 subsequent siblings)
  15 siblings, 0 replies; 54+ messages in thread
From: Roman Kagan @ 2016-12-20 15:55 UTC (permalink / raw)
  To: Paolo Bonzini, Radim Krčmář,
	K. Y. Srinivasan, Vitaly Kuznetsov
  Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, Haiyang Zhang,
	kvm, linux-kernel, devel, Denis V . Lunev, Roman Kagan

Move structures for monitored notifications to the uapi header for
userspace to be able to consume.  Also observe that hv_monitor_parameter
is by definition the same as hv_input_signal_event so use the latter and
nuke the former.

Signed-off-by: Roman Kagan <rkagan@virtuozzo.com>
---
 arch/x86/include/uapi/asm/hyperv.h | 23 +++++++++++++++
 drivers/hv/hyperv_vmbus.h          | 60 --------------------------------------
 2 files changed, 23 insertions(+), 60 deletions(-)

diff --git a/arch/x86/include/uapi/asm/hyperv.h b/arch/x86/include/uapi/asm/hyperv.h
index eb8d42a..e081615 100644
--- a/arch/x86/include/uapi/asm/hyperv.h
+++ b/arch/x86/include/uapi/asm/hyperv.h
@@ -396,4 +396,27 @@ struct hv_input_signal_event {
 	__u16 rsvdz;
 } __attribute__((aligned(HV_HYPERCALL_PARAM_ALIGN)));
 
+/* Definitions for the monitored notification facility */
+struct hv_monitor_trigger_group {
+	__u32 pending;
+	__u32 armed;
+};
+
+struct hv_monitor_page {
+	__u32 trigger_state;
+	__u32 rsvdz1;
+
+	struct hv_monitor_trigger_group trigger_group[4];
+	__u64 rsvdz2[3];
+
+	__s32 next_checktime[4][32];
+
+	__u16 latency[4][32];
+	__u64 rsvdz3[32];
+
+	struct hv_input_signal_event parameter[4][32];
+
+	__u8 rsvdz4[1984];
+};
+
 #endif
diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
index a96f021..7f247f2 100644
--- a/drivers/hv/hyperv_vmbus.h
+++ b/drivers/hv/hyperv_vmbus.h
@@ -94,66 +94,6 @@ struct hv_connection_info {
 	};
 };
 
-/* Definitions for the monitored notification facility */
-union hv_monitor_trigger_group {
-	u64 as_uint64;
-	struct {
-		u32 pending;
-		u32 armed;
-	};
-};
-
-struct hv_monitor_parameter {
-	u32 connectionid;
-	u16 flagnumber;
-	u16 rsvdz;
-};
-
-union hv_monitor_trigger_state {
-	u32 asu32;
-
-	struct {
-		u32 group_enable:4;
-		u32 rsvdz:28;
-	};
-};
-
-/* struct hv_monitor_page Layout */
-/* ------------------------------------------------------ */
-/* | 0   | TriggerState (4 bytes) | Rsvd1 (4 bytes)     | */
-/* | 8   | TriggerGroup[0]                              | */
-/* | 10  | TriggerGroup[1]                              | */
-/* | 18  | TriggerGroup[2]                              | */
-/* | 20  | TriggerGroup[3]                              | */
-/* | 28  | Rsvd2[0]                                     | */
-/* | 30  | Rsvd2[1]                                     | */
-/* | 38  | Rsvd2[2]                                     | */
-/* | 40  | NextCheckTime[0][0]    | NextCheckTime[0][1] | */
-/* | ...                                                | */
-/* | 240 | Latency[0][0..3]                             | */
-/* | 340 | Rsvz3[0]                                     | */
-/* | 440 | Parameter[0][0]                              | */
-/* | 448 | Parameter[0][1]                              | */
-/* | ...                                                | */
-/* | 840 | Rsvd4[0]                                     | */
-/* ------------------------------------------------------ */
-struct hv_monitor_page {
-	union hv_monitor_trigger_state trigger_state;
-	u32 rsvdz1;
-
-	union hv_monitor_trigger_group trigger_group[4];
-	u64 rsvdz2[3];
-
-	s32 next_checktime[4][32];
-
-	u16 latency[4][32];
-	u64 rsvdz3[32];
-
-	struct hv_monitor_parameter parameter[4][32];
-
-	u8 rsvdz4[1984];
-};
-
 /*
  * Versioning definitions used for guests reporting themselves to the
  * hypervisor, and visa versa.
-- 
2.9.3

^ permalink raw reply	[flat|nested] 54+ messages in thread

* [PATCH 12/15] hyperv: move VMBus connection ids to uapi
  2016-12-20 15:55 [PATCH 00/15] hyperv: more stuff to uapi + cleanup Roman Kagan
                   ` (10 preceding siblings ...)
  2016-12-20 15:55 ` [PATCH 11/15] hyperv: uapi-fy monitored notification structures Roman Kagan
@ 2016-12-20 15:55 ` Roman Kagan
  2016-12-20 17:25   ` Stephen Hemminger
  2016-12-20 15:56 ` [PATCH 13/15] hyperv: move function close to its only callsite Roman Kagan
                   ` (3 subsequent siblings)
  15 siblings, 1 reply; 54+ messages in thread
From: Roman Kagan @ 2016-12-20 15:55 UTC (permalink / raw)
  To: Paolo Bonzini, Radim Krčmář,
	K. Y. Srinivasan, Vitaly Kuznetsov
  Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, Haiyang Zhang,
	kvm, linux-kernel, devel, Denis V . Lunev, Roman Kagan

Userspace will need them too.

Signed-off-by: Roman Kagan <rkagan@virtuozzo.com>
---
 arch/x86/include/uapi/asm/hyperv.h |  9 +++++++++
 drivers/hv/hyperv_vmbus.h          | 10 ----------
 2 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/arch/x86/include/uapi/asm/hyperv.h b/arch/x86/include/uapi/asm/hyperv.h
index e081615..5d6e525 100644
--- a/arch/x86/include/uapi/asm/hyperv.h
+++ b/arch/x86/include/uapi/asm/hyperv.h
@@ -419,4 +419,13 @@ struct hv_monitor_page {
 	__u8 rsvdz4[1984];
 };
 
+/* VMBus expects pre-established communication with the following IDs */
+#define VMBUS_MESSAGE_CONNECTION_ID	1
+#define VMBUS_MESSAGE_PORT_ID		1
+#define VMBUS_EVENT_CONNECTION_ID	2
+#define VMBUS_EVENT_PORT_ID		2
+#define VMBUS_MONITOR_CONNECTION_ID	3
+#define VMBUS_MONITOR_PORT_ID		3
+#define VMBUS_MESSAGE_SINT		2
+
 #endif
diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
index 7f247f2..c0a65f7 100644
--- a/drivers/hv/hyperv_vmbus.h
+++ b/drivers/hv/hyperv_vmbus.h
@@ -114,16 +114,6 @@ enum hv_guest_os_microsoft_ids {
 };
 
 
-enum {
-	VMBUS_MESSAGE_CONNECTION_ID	= 1,
-	VMBUS_MESSAGE_PORT_ID		= 1,
-	VMBUS_EVENT_CONNECTION_ID	= 2,
-	VMBUS_EVENT_PORT_ID		= 2,
-	VMBUS_MONITOR_CONNECTION_ID	= 3,
-	VMBUS_MONITOR_PORT_ID		= 3,
-	VMBUS_MESSAGE_SINT		= 2,
-};
-
 /* #defines */
 
 #define HV_PRESENT_BIT			0x80000000
-- 
2.9.3

^ permalink raw reply	[flat|nested] 54+ messages in thread

* [PATCH 13/15] hyperv: move function close to its only callsite
  2016-12-20 15:55 [PATCH 00/15] hyperv: more stuff to uapi + cleanup Roman Kagan
                   ` (11 preceding siblings ...)
  2016-12-20 15:55 ` [PATCH 12/15] hyperv: move VMBus connection ids to uapi Roman Kagan
@ 2016-12-20 15:56 ` Roman Kagan
  2016-12-20 15:56 ` [PATCH 14/15] hyperv_vmbus: drop unused definitions Roman Kagan
                   ` (2 subsequent siblings)
  15 siblings, 0 replies; 54+ messages in thread
From: Roman Kagan @ 2016-12-20 15:56 UTC (permalink / raw)
  To: Paolo Bonzini, Radim Krčmář,
	K. Y. Srinivasan, Vitaly Kuznetsov
  Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, Haiyang Zhang,
	kvm, linux-kernel, devel, Denis V . Lunev, Roman Kagan

Signed-off-by: Roman Kagan <rkagan@virtuozzo.com>
---
 drivers/hv/hyperv_vmbus.h | 44 --------------------------------------------
 drivers/hv/hv.c           | 39 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 39 insertions(+), 44 deletions(-)

diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
index c0a65f7..fcb5d91 100644
--- a/drivers/hv/hyperv_vmbus.h
+++ b/drivers/hv/hyperv_vmbus.h
@@ -118,50 +118,6 @@ enum hv_guest_os_microsoft_ids {
 
 #define HV_PRESENT_BIT			0x80000000
 
-/*
- * The guest OS needs to register the guest ID with the hypervisor.
- * The guest ID is a 64 bit entity and the structure of this ID is
- * specified in the Hyper-V specification:
- *
- * http://msdn.microsoft.com/en-us/library/windows/hardware/ff542653%28v=vs.85%29.aspx
- *
- * While the current guideline does not specify how Linux guest ID(s)
- * need to be generated, our plan is to publish the guidelines for
- * Linux and other guest operating systems that currently are hosted
- * on Hyper-V. The implementation here conforms to this yet
- * unpublished guidelines.
- *
- *
- * Bit(s)
- * 63 - Indicates if the OS is Open Source or not; 1 is Open Source
- * 62:56 - Os Type; Linux is 0x100
- * 55:48 - Distro specific identification
- * 47:16 - Linux kernel version number
- * 15:0  - Distro specific identification
- *
- *
- */
-
-#define HV_LINUX_VENDOR_ID		0x8100
-
-/*
- * Generate the guest ID based on the guideline described above.
- */
-
-static inline  __u64 generate_guest_id(__u8 d_info1, __u32 kernel_version,
-					__u16 d_info2)
-{
-	__u64 guest_id = 0;
-
-	guest_id = (((__u64)HV_LINUX_VENDOR_ID) << 48);
-	guest_id |= (((__u64)(d_info1)) << 48);
-	guest_id |= (((__u64)(kernel_version)) << 16);
-	guest_id |= ((__u64)(d_info2));
-
-	return guest_id;
-}
-
-
 #define HV_CPU_POWER_MANAGEMENT		(1 << 0)
 #define HV_RECOMMENDATIONS_MAX		4
 
diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
index b9f50de..3598090 100644
--- a/drivers/hv/hv.c
+++ b/drivers/hv/hv.c
@@ -45,6 +45,45 @@ struct hv_context hv_context = {
 #define HV_MIN_DELTA_TICKS 1
 
 /*
+ * The guest OS needs to register the guest ID with the hypervisor.
+ * The guest ID is a 64 bit entity and the structure of this ID is
+ * specified in the Hyper-V specification:
+ *
+ * http://msdn.microsoft.com/en-us/library/windows/hardware/ff542653%28v=vs.85%29.aspx
+ *
+ * While the current guideline does not specify how Linux guest ID(s)
+ * need to be generated, our plan is to publish the guidelines for
+ * Linux and other guest operating systems that currently are hosted
+ * on Hyper-V. The implementation here conforms to this yet
+ * unpublished guidelines.
+ *
+ * Bit(s)
+ * 63 - Indicates if the OS is Open Source or not; 1 is Open Source
+ * 62:56 - Os Type; Linux is 0x100
+ * 55:48 - Distro specific identification
+ * 47:16 - Linux kernel version number
+ * 15:0  - Distro specific identification
+ */
+
+#define HV_LINUX_VENDOR_ID		0x8100
+
+/*
+ * Generate the guest ID based on the guideline described above.
+ */
+
+static u64 generate_guest_id(u8 d_info1, u32 kernel_version, u16 d_info2)
+{
+	u64 guest_id;
+
+	guest_id = ((u64)HV_LINUX_VENDOR_ID) << 48;
+	guest_id |= ((u64)d_info1) << 48;
+	guest_id |= ((u64)kernel_version) << 16;
+	guest_id |= (u64)d_info2;
+
+	return guest_id;
+}
+
+/*
  * query_hypervisor_info - Get version info of the windows hypervisor
  */
 unsigned int host_info_eax;
-- 
2.9.3

^ permalink raw reply	[flat|nested] 54+ messages in thread

* [PATCH 14/15] hyperv_vmbus: drop unused definitions
  2016-12-20 15:55 [PATCH 00/15] hyperv: more stuff to uapi + cleanup Roman Kagan
                   ` (12 preceding siblings ...)
  2016-12-20 15:56 ` [PATCH 13/15] hyperv: move function close to its only callsite Roman Kagan
@ 2016-12-20 15:56 ` Roman Kagan
  2016-12-20 15:56 ` [PATCH 15/15] hyperv: redefine hv_message without bitfields Roman Kagan
  2016-12-21 18:00 ` [PATCH 00/15] hyperv: more stuff to uapi + cleanup KY Srinivasan
  15 siblings, 0 replies; 54+ messages in thread
From: Roman Kagan @ 2016-12-20 15:56 UTC (permalink / raw)
  To: Paolo Bonzini, Radim Krčmář,
	K. Y. Srinivasan, Vitaly Kuznetsov
  Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, Haiyang Zhang,
	kvm, linux-kernel, devel, Denis V . Lunev, Roman Kagan

None of these is used in the kernel.

Signed-off-by: Roman Kagan <rkagan@virtuozzo.com>
---
 drivers/hv/hyperv_vmbus.h | 119 ----------------------------------------------
 1 file changed, 119 deletions(-)

diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
index fcb5d91..8ce6d64 100644
--- a/drivers/hv/hyperv_vmbus.h
+++ b/drivers/hv/hyperv_vmbus.h
@@ -39,125 +39,6 @@
  */
 #define HV_UTIL_NEGO_TIMEOUT 55
 
-/* Define version of the synthetic interrupt controller. */
-#define HV_SYNIC_VERSION		(1)
-
-#define HV_ANY_VP			(0xFFFFFFFF)
-
-/* Define invalid partition identifier. */
-#define HV_PARTITION_ID_INVALID		((u64)0x0)
-
-/* Define port type. */
-enum hv_port_type {
-	HVPORT_MSG	= 1,
-	HVPORT_EVENT		= 2,
-	HVPORT_MONITOR	= 3
-};
-
-/* Define port information structure. */
-struct hv_port_info {
-	enum hv_port_type port_type;
-	u32 padding;
-	union {
-		struct {
-			u32 target_sint;
-			u32 target_vp;
-			u64 rsvdz;
-		} message_port_info;
-		struct {
-			u32 target_sint;
-			u32 target_vp;
-			u16 base_flag_number;
-			u16 flag_count;
-			u32 rsvdz;
-		} event_port_info;
-		struct {
-			u64 monitor_address;
-			u64 rsvdz;
-		} monitor_port_info;
-	};
-};
-
-struct hv_connection_info {
-	enum hv_port_type port_type;
-	u32 padding;
-	union {
-		struct {
-			u64 rsvdz;
-		} message_connection_info;
-		struct {
-			u64 rsvdz;
-		} event_connection_info;
-		struct {
-			u64 monitor_address;
-		} monitor_connection_info;
-	};
-};
-
-/*
- * Versioning definitions used for guests reporting themselves to the
- * hypervisor, and visa versa.
- */
-
-/* Version info reported by guest OS's */
-enum hv_guest_os_vendor {
-	HVGUESTOS_VENDOR_MICROSOFT	= 0x0001
-};
-
-enum hv_guest_os_microsoft_ids {
-	HVGUESTOS_MICROSOFT_UNDEFINED	= 0x00,
-	HVGUESTOS_MICROSOFT_MSDOS		= 0x01,
-	HVGUESTOS_MICROSOFT_WINDOWS3X	= 0x02,
-	HVGUESTOS_MICROSOFT_WINDOWS9X	= 0x03,
-	HVGUESTOS_MICROSOFT_WINDOWSNT	= 0x04,
-	HVGUESTOS_MICROSOFT_WINDOWSCE	= 0x05
-};
-
-
-/* #defines */
-
-#define HV_PRESENT_BIT			0x80000000
-
-#define HV_CPU_POWER_MANAGEMENT		(1 << 0)
-#define HV_RECOMMENDATIONS_MAX		4
-
-#define HV_X64_MAX			5
-#define HV_CAPS_MAX			8
-
-
-/* Service definitions */
-
-#define HV_SERVICE_PARENT_PORT				(0)
-#define HV_SERVICE_PARENT_CONNECTION			(0)
-
-#define HV_SERVICE_CONNECT_RESPONSE_SUCCESS		(0)
-#define HV_SERVICE_CONNECT_RESPONSE_INVALID_PARAMETER	(1)
-#define HV_SERVICE_CONNECT_RESPONSE_UNKNOWN_SERVICE	(2)
-#define HV_SERVICE_CONNECT_RESPONSE_CONNECTION_REJECTED	(3)
-
-#define HV_SERVICE_CONNECT_REQUEST_MESSAGE_ID		(1)
-#define HV_SERVICE_CONNECT_RESPONSE_MESSAGE_ID		(2)
-#define HV_SERVICE_DISCONNECT_REQUEST_MESSAGE_ID	(3)
-#define HV_SERVICE_DISCONNECT_RESPONSE_MESSAGE_ID	(4)
-#define HV_SERVICE_MAX_MESSAGE_ID				(4)
-
-#define HV_SERVICE_PROTOCOL_VERSION (0x0010)
-#define HV_CONNECT_PAYLOAD_BYTE_COUNT 64
-
-/* #define VMBUS_REVISION_NUMBER	6 */
-
-/* Our local vmbus's port and connection id. Anything >0 is fine */
-/* #define VMBUS_PORT_ID		11 */
-
-/* 628180B8-308D-4c5e-B7DB-1BEB62E62EF4 */
-static const uuid_le VMBUS_SERVICE_ID = {
-	.b = {
-		0xb8, 0x80, 0x81, 0x62, 0x8d, 0x30, 0x5e, 0x4c,
-		0xb7, 0xdb, 0x1b, 0xeb, 0x62, 0xe6, 0x2e, 0xf4
-	},
-};
-
-
 
 struct hv_context {
 	/* We only support running on top of Hyper-V
-- 
2.9.3

^ permalink raw reply	[flat|nested] 54+ messages in thread

* [PATCH 15/15] hyperv: redefine hv_message without bitfields
  2016-12-20 15:55 [PATCH 00/15] hyperv: more stuff to uapi + cleanup Roman Kagan
                   ` (13 preceding siblings ...)
  2016-12-20 15:56 ` [PATCH 14/15] hyperv_vmbus: drop unused definitions Roman Kagan
@ 2016-12-20 15:56 ` Roman Kagan
  2016-12-21 18:00 ` [PATCH 00/15] hyperv: more stuff to uapi + cleanup KY Srinivasan
  15 siblings, 0 replies; 54+ messages in thread
From: Roman Kagan @ 2016-12-20 15:56 UTC (permalink / raw)
  To: Paolo Bonzini, Radim Krčmář,
	K. Y. Srinivasan, Vitaly Kuznetsov
  Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, Haiyang Zhang,
	kvm, linux-kernel, devel, Denis V . Lunev, Roman Kagan

This brings more symmetry in the API.

The downside is that this changes the userspace-visible structure.
Hopefully no userspace code had a chance to use it yet.

Signed-off-by: Roman Kagan <rkagan@virtuozzo.com>
---
 arch/x86/include/uapi/asm/hyperv.h | 32 +++++---------------------------
 drivers/hv/hyperv_vmbus.h          |  2 +-
 arch/x86/kvm/hyperv.c              | 10 +++++-----
 drivers/hv/channel_mgmt.c          |  6 +++---
 drivers/hv/vmbus_drv.c             |  2 +-
 5 files changed, 15 insertions(+), 37 deletions(-)

diff --git a/arch/x86/include/uapi/asm/hyperv.h b/arch/x86/include/uapi/asm/hyperv.h
index 5d6e525..89ef9c2 100644
--- a/arch/x86/include/uapi/asm/hyperv.h
+++ b/arch/x86/include/uapi/asm/hyperv.h
@@ -276,7 +276,8 @@ struct hv_ref_tsc_page {
 /* Define synthetic interrupt controller message constants. */
 #define HV_MESSAGE_SIZE			(256)
 #define HV_MESSAGE_PAYLOAD_BYTE_COUNT	(240)
-#define HV_MESSAGE_PAYLOAD_QWORD_COUNT	(30)
+
+#define HV_MESSAGE_FLAG_PENDING		(1)
 
 /* Define hypervisor message types. */
 enum hv_message_type {
@@ -308,42 +309,19 @@ enum hv_message_type {
 	HVMSG_X64_LEGACY_FP_ERROR		= 0x80010005
 };
 
-/* Define synthetic interrupt controller message flags. */
-union hv_message_flags {
-	__u8 asu8;
-	struct {
-		__u8 msg_pending:1;
-		__u8 reserved:7;
-	};
-};
-
-/* Define port identifier type. */
-union hv_port_id {
-	__u32 asu32;
-	struct {
-		__u32 id:24;
-		__u32 reserved:8;
-	} u;
-};
-
 /* Define synthetic interrupt controller message header. */
 struct hv_message_header {
 	__u32 message_type;
 	__u8 payload_size;
-	union hv_message_flags message_flags;
+	__u8 message_flags;
 	__u8 reserved[2];
-	union {
-		__u64 sender;
-		union hv_port_id port;
-	};
+	__u64 origination_id;
 };
 
 /* Define synthetic interrupt controller message format. */
 struct hv_message {
 	struct hv_message_header header;
-	union {
-		__u64 payload[HV_MESSAGE_PAYLOAD_QWORD_COUNT];
-	} u;
+	__u64 payload[HV_MESSAGE_PAYLOAD_BYTE_COUNT / 8];
 };
 
 /* Define the synthetic interrupt message page layout. */
diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
index 8ce6d64..87713cc 100644
--- a/drivers/hv/hyperv_vmbus.h
+++ b/drivers/hv/hyperv_vmbus.h
@@ -260,7 +260,7 @@ static inline void vmbus_signal_eom(struct hv_message *msg, u32 old_msg_type)
 	 */
 	mb();
 
-	if (msg->header.message_flags.msg_pending) {
+	if (msg->header.message_flags & HV_MESSAGE_FLAG_PENDING) {
 		/*
 		 * This will cause message queue rescan to
 		 * possibly deliver another msg from the
diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
index c7db112..546dddc 100644
--- a/arch/x86/kvm/hyperv.c
+++ b/arch/x86/kvm/hyperv.c
@@ -137,7 +137,7 @@ static void synic_clear_sint_msg_pending(struct kvm_vcpu_hv_synic *synic,
 	msg_page = kmap_atomic(page);
 
 	msg = &msg_page->sint_message[sint];
-	msg->header.message_flags.msg_pending = 0;
+	msg->header.message_flags &= ~HV_MESSAGE_FLAG_PENDING;
 
 	kunmap_atomic(msg_page);
 	kvm_release_page_dirty(page);
@@ -564,10 +564,10 @@ static int synic_deliver_msg(struct kvm_vcpu_hv_synic *synic, u32 sint,
 	dst_msg = &msg_page->sint_message[sint];
 	if (sync_cmpxchg(&dst_msg->header.message_type, HVMSG_NONE,
 			 src_msg->header.message_type) != HVMSG_NONE) {
-		dst_msg->header.message_flags.msg_pending = 1;
+		dst_msg->header.message_flags |= HV_MESSAGE_FLAG_PENDING;
 		r = -EAGAIN;
 	} else {
-		memcpy(&dst_msg->u.payload, &src_msg->u.payload,
+		memcpy(&dst_msg->payload, &src_msg->payload,
 		       src_msg->header.payload_size);
 		dst_msg->header.message_type = src_msg->header.message_type;
 		dst_msg->header.payload_size = src_msg->header.payload_size;
@@ -588,7 +588,7 @@ static int stimer_send_msg(struct kvm_vcpu_hv_stimer *stimer)
 	struct kvm_vcpu *vcpu = stimer_to_vcpu(stimer);
 	struct hv_message *msg = &stimer->msg;
 	struct hv_timer_message_payload *payload =
-			(struct hv_timer_message_payload *)&msg->u.payload;
+			(struct hv_timer_message_payload *)&msg->payload;
 
 	payload->expiration_time = stimer->exp_time;
 	payload->delivery_time = get_time_ref_counter(vcpu->kvm);
@@ -653,7 +653,7 @@ static void stimer_prepare_msg(struct kvm_vcpu_hv_stimer *stimer)
 {
 	struct hv_message *msg = &stimer->msg;
 	struct hv_timer_message_payload *payload =
-			(struct hv_timer_message_payload *)&msg->u.payload;
+			(struct hv_timer_message_payload *)&msg->payload;
 
 	memset(&msg->header, 0, sizeof(msg->header));
 	msg->header.message_type = HVMSG_TIMER_EXPIRED;
diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index 4a5cc11..3576a0b 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -680,7 +680,7 @@ static void vmbus_wait_for_unload(void)
 				continue;
 
 			hdr = (struct vmbus_channel_message_header *)
-				msg->u.payload;
+				msg->payload;
 
 			if (hdr->msgtype == CHANNELMSG_UNLOAD_RESPONSE)
 				complete(&vmbus_connection.unload_event);
@@ -1071,14 +1071,14 @@ void vmbus_onmessage(void *context)
 	struct vmbus_channel_message_header *hdr;
 	int size;
 
-	hdr = (struct vmbus_channel_message_header *)msg->u.payload;
+	hdr = (struct vmbus_channel_message_header *)msg->payload;
 	size = msg->header.payload_size;
 
 	if (hdr->msgtype >= CHANNELMSG_COUNT) {
 		pr_err("Received invalid channel message type %d size %d\n",
 			   hdr->msgtype, size);
 		print_hex_dump_bytes("", DUMP_PREFIX_NONE,
-				     (unsigned char *)msg->u.payload, size);
+				     (unsigned char *)msg->payload, size);
 		return;
 	}
 
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index f6b626b..60571a8 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -883,7 +883,7 @@ void vmbus_on_msg_dpc(unsigned long data)
 		/* no msg */
 		return;
 
-	hdr = (struct vmbus_channel_message_header *)msg->u.payload;
+	hdr = (struct vmbus_channel_message_header *)msg->payload;
 
 	if (hdr->msgtype >= CHANNELMSG_COUNT) {
 		WARN_ONCE(1, "unknown msgtype=%d\n", hdr->msgtype);
-- 
2.9.3

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH 02/15] hyperv: uapi-fy synic event flags definitions
  2016-12-20 15:55 ` [PATCH 02/15] hyperv: uapi-fy synic event flags definitions Roman Kagan
@ 2016-12-20 17:24   ` Stephen Hemminger
  2016-12-21  6:33     ` Roman Kagan
  2016-12-21 10:58   ` kbuild test robot
  2016-12-21 18:58   ` KY Srinivasan
  2 siblings, 1 reply; 54+ messages in thread
From: Stephen Hemminger @ 2016-12-20 17:24 UTC (permalink / raw)
  To: Roman Kagan
  Cc: Paolo Bonzini, Radim Krčmář,
	K. Y. Srinivasan, Vitaly Kuznetsov, kvm, Denis V . Lunev,
	Haiyang Zhang, x86, linux-kernel, Ingo Molnar, H. Peter Anvin,
	devel, Thomas Gleixner

On Tue, 20 Dec 2016 18:55:49 +0300
Roman Kagan <rkagan@virtuozzo.com> wrote:

> Move definitions related to the Hyper-V SynIC event flags to a header
> where they can be consumed by userspace.
> 
> While doing so, also clean up their use by switching to standard bitops
> and struct-based dereferencing.  The latter is also done for message
> pages.
> 
> Signed-off-by: Roman Kagan <rkagan@virtuozzo.com>
> ---
>  arch/x86/include/uapi/asm/hyperv.h | 13 +++++++++
>  drivers/hv/hyperv_vmbus.h          | 24 ++--------------
>  drivers/hv/channel_mgmt.c          | 10 +++----
>  drivers/hv/connection.c            | 47 ++++++++++---------------------
>  drivers/hv/vmbus_drv.c             | 57 ++++++++++++++------------------------
>  5 files changed, 54 insertions(+), 97 deletions(-)
> 
> diff --git a/arch/x86/include/uapi/asm/hyperv.h b/arch/x86/include/uapi/asm/hyperv.h
> index 6098ab5..af542a3 100644
> --- a/arch/x86/include/uapi/asm/hyperv.h
> +++ b/arch/x86/include/uapi/asm/hyperv.h
> @@ -363,4 +363,17 @@ struct hv_timer_message_payload {
>  #define HV_STIMER_AUTOENABLE		(1ULL << 3)
>  #define HV_STIMER_SINT(config)		(__u8)(((config) >> 16) & 0x0F)
>  
> +/* Define synthetic interrupt controller flag constants. */
> +#define HV_EVENT_FLAGS_COUNT		(256 * 8)
> +
> +/* Define the synthetic interrupt controller event flags format. */
> +struct hv_synic_event_flags {
> +	__u64 flags[HV_EVENT_FLAGS_COUNT / 64];
> +};
> +
> +/* Define the synthetic interrupt flags page layout. */
> +struct hv_synic_event_flags_page {
> +	struct hv_synic_event_flags sintevent_flags[HV_SYNIC_SINT_COUNT];
> +};
> +
>  #endif

How are these going to be exposed to user space?

They should really be unsigned long since there is no guarantee of atomic operation
on 64 bit values on 32 bit architectures.

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH 12/15] hyperv: move VMBus connection ids to uapi
  2016-12-20 15:55 ` [PATCH 12/15] hyperv: move VMBus connection ids to uapi Roman Kagan
@ 2016-12-20 17:25   ` Stephen Hemminger
  2016-12-21  6:29     ` Roman Kagan
  0 siblings, 1 reply; 54+ messages in thread
From: Stephen Hemminger @ 2016-12-20 17:25 UTC (permalink / raw)
  To: Roman Kagan
  Cc: Paolo Bonzini, Radim Krčmář,
	K. Y. Srinivasan, Vitaly Kuznetsov, kvm, Denis V . Lunev,
	Haiyang Zhang, x86, linux-kernel, Ingo Molnar, H. Peter Anvin,
	devel, Thomas Gleixner

On Tue, 20 Dec 2016 18:55:59 +0300
Roman Kagan <rkagan@virtuozzo.com> wrote:

> Userspace will need them too.
> 
> Signed-off-by: Roman Kagan <rkagan@virtuozzo.com>

What userspace? I am worried about creating more stable API's that can't change.

^ permalink raw reply	[flat|nested] 54+ messages in thread

* RE: [PATCH 01/15] hyperv: consolidate TSC ref page definitions
  2016-12-20 15:55 ` [PATCH 01/15] hyperv: consolidate TSC ref page definitions Roman Kagan
@ 2016-12-20 20:57   ` KY Srinivasan
  2016-12-21  6:25     ` Roman Kagan
  0 siblings, 1 reply; 54+ messages in thread
From: KY Srinivasan @ 2016-12-20 20:57 UTC (permalink / raw)
  To: Roman Kagan, Paolo Bonzini, Radim Krčmář,
	Vitaly Kuznetsov
  Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, Haiyang Zhang,
	kvm, linux-kernel, devel, Denis V . Lunev



> -----Original Message-----
> From: Roman Kagan [mailto:rkagan@virtuozzo.com]
> Sent: Tuesday, December 20, 2016 7:56 AM
> To: Paolo Bonzini <pbonzini@redhat.com>; Radim Krčmář
> <rkrcmar@redhat.com>; KY Srinivasan <kys@microsoft.com>; Vitaly
> Kuznetsov <vkuznets@redhat.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>; Ingo Molnar
> <mingo@redhat.com>; H. Peter Anvin <hpa@zytor.com>; x86@kernel.org;
> Haiyang Zhang <haiyangz@microsoft.com>; kvm@vger.kernel.org; linux-
> kernel@vger.kernel.org; devel@linuxdriverproject.org; Denis V . Lunev
> <den@openvz.org>; Roman Kagan <rkagan@virtuozzo.com>
> Subject: [PATCH 01/15] hyperv: consolidate TSC ref page definitions
> 
> Consolidate the guest-side and kvm-side definitions for Hyper-V TSC
> reference page.
> 
> While at this, rewrite read_hv_clock_tsc using the existing helpers.

Why not beak this into separate patches.
> 
> Signed-off-by: Roman Kagan <rkagan@virtuozzo.com>
> ---
>  arch/x86/include/asm/kvm_host.h    |  2 +-
>  arch/x86/include/uapi/asm/hyperv.h |  4 +--
>  drivers/hv/hyperv_vmbus.h          |  8 ------
>  arch/x86/kvm/hyperv.c              |  4 +--
>  drivers/hv/hv.c                    | 54 +++++++++++++++-----------------------
>  5 files changed, 26 insertions(+), 46 deletions(-)
> 
> diff --git a/arch/x86/include/asm/kvm_host.h
> b/arch/x86/include/asm/kvm_host.h
> index 2e25038..2b85f49 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -713,7 +713,7 @@ struct kvm_hv {
>  	u64 hv_crash_param[HV_X64_MSR_CRASH_PARAMS];
>  	u64 hv_crash_ctl;
> 
> -	HV_REFERENCE_TSC_PAGE tsc_ref;
> +	struct hv_ref_tsc_page tsc_ref;
>  };
> 
>  struct kvm_arch {
> diff --git a/arch/x86/include/uapi/asm/hyperv.h
> b/arch/x86/include/uapi/asm/hyperv.h
> index 9b1a918..6098ab5 100644
> --- a/arch/x86/include/uapi/asm/hyperv.h
> +++ b/arch/x86/include/uapi/asm/hyperv.h
> @@ -252,12 +252,12 @@
>  #define HV_STATUS_INVALID_CONNECTION_ID		18
>  #define HV_STATUS_INSUFFICIENT_BUFFERS		19
> 
> -typedef struct _HV_REFERENCE_TSC_PAGE {
> +struct hv_ref_tsc_page {
>  	__u32 tsc_sequence;
>  	__u32 res1;
>  	__u64 tsc_scale;
>  	__s64 tsc_offset;
> -} HV_REFERENCE_TSC_PAGE, *PHV_REFERENCE_TSC_PAGE;
> +};
> 
>  /* Define the number of synthetic interrupt sources. */
>  #define HV_SYNIC_SINT_COUNT		(16)
> diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
> index 0675b39..4516498 100644
> --- a/drivers/hv/hyperv_vmbus.h
> +++ b/drivers/hv/hyperv_vmbus.h
> @@ -475,14 +475,6 @@ struct hv_context {
> 
>  extern struct hv_context hv_context;
> 
> -struct ms_hyperv_tsc_page {
> -	volatile u32 tsc_sequence;
> -	u32 reserved1;
> -	volatile u64 tsc_scale;
> -	volatile s64 tsc_offset;
> -	u64 reserved2[509];
> -};
> -
>  struct hv_ring_buffer_debug_info {
>  	u32 current_interrupt_mask;
>  	u32 current_read_index;
> diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
> index 1572c35..c7db112 100644
> --- a/arch/x86/kvm/hyperv.c
> +++ b/arch/x86/kvm/hyperv.c
> @@ -806,7 +806,7 @@ static int kvm_hv_msr_set_crash_data(struct
> kvm_vcpu *vcpu,
>   * These two equivalencies are implemented in this function.
>   */
>  static bool compute_tsc_page_parameters(struct pvclock_vcpu_time_info
> *hv_clock,
> -					HV_REFERENCE_TSC_PAGE *tsc_ref)
> +					struct hv_ref_tsc_page *tsc_ref)
>  {
>  	u64 max_mul;
> 
> @@ -847,7 +847,7 @@ void kvm_hv_setup_tsc_page(struct kvm *kvm,
>  	u64 gfn;
> 
>  	BUILD_BUG_ON(sizeof(tsc_seq) != sizeof(hv-
> >tsc_ref.tsc_sequence));
> -	BUILD_BUG_ON(offsetof(HV_REFERENCE_TSC_PAGE,
> tsc_sequence) != 0);
> +	BUILD_BUG_ON(offsetof(struct hv_ref_tsc_page, tsc_sequence) !=
> 0);
> 
>  	if (!(hv->hv_tsc_page & HV_X64_MSR_TSC_REFERENCE_ENABLE))
>  		return;
> diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
> index 446802a..a7256ec 100644
> --- a/drivers/hv/hv.c
> +++ b/drivers/hv/hv.c
> @@ -137,41 +137,29 @@ EXPORT_SYMBOL_GPL(hv_do_hypercall);
>  #ifdef CONFIG_X86_64
>  static cycle_t read_hv_clock_tsc(struct clocksource *arg)
>  {
> -	cycle_t current_tick;
> -	struct ms_hyperv_tsc_page *tsc_pg = hv_context.tsc_page;
> +	struct hv_ref_tsc_page *tsc_pg = hv_context.tsc_page;
> +	u32 sequence;
> +	u64 scale;
> +	s64 offset;
> +
> +	do {
> +		sequence = tsc_pg->tsc_sequence;
> +		virt_rmb();
> +
> +		if (!sequence) {
> +			/* fallback to MSR */
> +			cycle_t current_tick;
> +			rdmsrl(HV_X64_MSR_TIME_REF_COUNT,
> current_tick);
> +			return current_tick;
> +		}
> 
> -	if (tsc_pg->tsc_sequence != 0) {
> -		/*
> -		 * Use the tsc page to compute the value.
> -		 */
> +		scale = tsc_pg->tsc_scale;
> +		offset = tsc_pg->tsc_offset;
> 
> -		while (1) {
> -			cycle_t tmp;
> -			u32 sequence = tsc_pg->tsc_sequence;
> -			u64 cur_tsc;
> -			u64 scale = tsc_pg->tsc_scale;
> -			s64 offset = tsc_pg->tsc_offset;
> -
> -			rdtscll(cur_tsc);
> -			/* current_tick = ((cur_tsc *scale) >> 64) + offset */
> -			asm("mulq %3"
> -				: "=d" (current_tick), "=a" (tmp)
> -				: "a" (cur_tsc), "r" (scale));
> -
> -			current_tick += offset;
> -			if (tsc_pg->tsc_sequence == sequence)
> -				return current_tick;
> -
> -			if (tsc_pg->tsc_sequence != 0)
> -				continue;
> -			/*
> -			 * Fallback using MSR method.
> -			 */
> -			break;
> -		}
> -	}
> -	rdmsrl(HV_X64_MSR_TIME_REF_COUNT, current_tick);
> -	return current_tick;
> +		virt_rmb();
> +	} while (tsc_pg->tsc_sequence != sequence);
> +
> +	return mul_u64_u64_shr(rdtsc_ordered(), scale, 64) + offset;
>  }
> 
>  static struct clocksource hyperv_cs_tsc = {
> --
> 2.9.3

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH 01/15] hyperv: consolidate TSC ref page definitions
  2016-12-20 20:57   ` KY Srinivasan
@ 2016-12-21  6:25     ` Roman Kagan
  0 siblings, 0 replies; 54+ messages in thread
From: Roman Kagan @ 2016-12-21  6:25 UTC (permalink / raw)
  To: KY Srinivasan
  Cc: Paolo Bonzini, Radim Krčmář,
	Vitaly Kuznetsov, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	x86, Haiyang Zhang, kvm, linux-kernel, devel, Denis V . Lunev

On Tue, Dec 20, 2016 at 08:57:28PM +0000, KY Srinivasan wrote:
> 
> 
> > -----Original Message-----
> > From: Roman Kagan [mailto:rkagan@virtuozzo.com]
> > Sent: Tuesday, December 20, 2016 7:56 AM
> > To: Paolo Bonzini <pbonzini@redhat.com>; Radim Krčmář
> > <rkrcmar@redhat.com>; KY Srinivasan <kys@microsoft.com>; Vitaly
> > Kuznetsov <vkuznets@redhat.com>
> > Cc: Thomas Gleixner <tglx@linutronix.de>; Ingo Molnar
> > <mingo@redhat.com>; H. Peter Anvin <hpa@zytor.com>; x86@kernel.org;
> > Haiyang Zhang <haiyangz@microsoft.com>; kvm@vger.kernel.org; linux-
> > kernel@vger.kernel.org; devel@linuxdriverproject.org; Denis V . Lunev
> > <den@openvz.org>; Roman Kagan <rkagan@virtuozzo.com>
> > Subject: [PATCH 01/15] hyperv: consolidate TSC ref page definitions
> > 
> > Consolidate the guest-side and kvm-side definitions for Hyper-V TSC
> > reference page.
> > 
> > While at this, rewrite read_hv_clock_tsc using the existing helpers.
> 
> Why not beak this into separate patches.

Agreed, I'll do it in the next iteration.

Thanks,
Roman.

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH 12/15] hyperv: move VMBus connection ids to uapi
  2016-12-20 17:25   ` Stephen Hemminger
@ 2016-12-21  6:29     ` Roman Kagan
  2016-12-21 12:18       ` Christoph Hellwig
  0 siblings, 1 reply; 54+ messages in thread
From: Roman Kagan @ 2016-12-21  6:29 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: Paolo Bonzini, Radim Krčmář,
	K. Y. Srinivasan, Vitaly Kuznetsov, kvm, Denis V . Lunev,
	Haiyang Zhang, x86, linux-kernel, Ingo Molnar, H. Peter Anvin,
	devel, Thomas Gleixner

On Tue, Dec 20, 2016 at 09:25:43AM -0800, Stephen Hemminger wrote:
> On Tue, 20 Dec 2016 18:55:59 +0300
> Roman Kagan <rkagan@virtuozzo.com> wrote:
> 
> > Userspace will need them too.
> > 
> > Signed-off-by: Roman Kagan <rkagan@virtuozzo.com>
> 
> What userspace? I am worried about creating more stable API's that can't change.

QEMU in particular.  We're planning to implement VMBus devices in QEMU
and would like to have the definitions shared with the Linux guest
drivers for Hyper-V.

Roman.

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH 02/15] hyperv: uapi-fy synic event flags definitions
  2016-12-20 17:24   ` Stephen Hemminger
@ 2016-12-21  6:33     ` Roman Kagan
  0 siblings, 0 replies; 54+ messages in thread
From: Roman Kagan @ 2016-12-21  6:33 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: Paolo Bonzini, Radim Krčmář,
	K. Y. Srinivasan, Vitaly Kuznetsov, kvm, Denis V . Lunev,
	Haiyang Zhang, x86, linux-kernel, Ingo Molnar, H. Peter Anvin,
	devel, Thomas Gleixner

On Tue, Dec 20, 2016 at 09:24:53AM -0800, Stephen Hemminger wrote:
> On Tue, 20 Dec 2016 18:55:49 +0300
> Roman Kagan <rkagan@virtuozzo.com> wrote:
> 
> > Move definitions related to the Hyper-V SynIC event flags to a header
> > where they can be consumed by userspace.
> > 
> > While doing so, also clean up their use by switching to standard bitops
> > and struct-based dereferencing.  The latter is also done for message
> > pages.
> > 
> > Signed-off-by: Roman Kagan <rkagan@virtuozzo.com>
> > ---
> >  arch/x86/include/uapi/asm/hyperv.h | 13 +++++++++
> >  drivers/hv/hyperv_vmbus.h          | 24 ++--------------
> >  drivers/hv/channel_mgmt.c          | 10 +++----
> >  drivers/hv/connection.c            | 47 ++++++++++---------------------
> >  drivers/hv/vmbus_drv.c             | 57 ++++++++++++++------------------------
> >  5 files changed, 54 insertions(+), 97 deletions(-)
> > 
> > diff --git a/arch/x86/include/uapi/asm/hyperv.h b/arch/x86/include/uapi/asm/hyperv.h
> > index 6098ab5..af542a3 100644
> > --- a/arch/x86/include/uapi/asm/hyperv.h
> > +++ b/arch/x86/include/uapi/asm/hyperv.h
> > @@ -363,4 +363,17 @@ struct hv_timer_message_payload {
> >  #define HV_STIMER_AUTOENABLE		(1ULL << 3)
> >  #define HV_STIMER_SINT(config)		(__u8)(((config) >> 16) & 0x0F)
> >  
> > +/* Define synthetic interrupt controller flag constants. */
> > +#define HV_EVENT_FLAGS_COUNT		(256 * 8)
> > +
> > +/* Define the synthetic interrupt controller event flags format. */
> > +struct hv_synic_event_flags {
> > +	__u64 flags[HV_EVENT_FLAGS_COUNT / 64];
> > +};
> > +
> > +/* Define the synthetic interrupt flags page layout. */
> > +struct hv_synic_event_flags_page {
> > +	struct hv_synic_event_flags sintevent_flags[HV_SYNIC_SINT_COUNT];
> > +};
> > +
> >  #endif
> 
> How are these going to be exposed to user space?
> 
> They should really be unsigned long since there is no guarantee of atomic operation
> on 64 bit values on 32 bit architectures.

Indeed, absolutely.  These are bitmaps and should be unsigned long[], of
course.  I'll fix it in the next iteration.

Thanks,
Roman.

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH 02/15] hyperv: uapi-fy synic event flags definitions
  2016-12-20 15:55 ` [PATCH 02/15] hyperv: uapi-fy synic event flags definitions Roman Kagan
  2016-12-20 17:24   ` Stephen Hemminger
@ 2016-12-21 10:58   ` kbuild test robot
  2016-12-21 18:58   ` KY Srinivasan
  2 siblings, 0 replies; 54+ messages in thread
From: kbuild test robot @ 2016-12-21 10:58 UTC (permalink / raw)
  To: Roman Kagan
  Cc: kbuild-all, Paolo Bonzini, Radim Krčmář,
	K. Y. Srinivasan, Vitaly Kuznetsov, kvm, Denis V . Lunev,
	Haiyang Zhang, x86, linux-kernel, Ingo Molnar, Roman Kagan,
	H. Peter Anvin, devel, Thomas Gleixner

[-- Attachment #1: Type: text/plain, Size: 2836 bytes --]

Hi Roman,

[auto build test ERROR on linus/master]
[also build test ERROR on v4.9 next-20161221]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Roman-Kagan/hyperv-more-stuff-to-uapi-cleanup/20161221-130415
config: x86_64-rhel (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

Note: the linux-review/Roman-Kagan/hyperv-more-stuff-to-uapi-cleanup/20161221-130415 HEAD 35a54cd6795c4aa6f101c58ab076c6bdd63b3779 builds fine.
      It only hurts bisectibility.

All errors (new ones prefixed by >>):

   drivers/hv/connection.c: In function 'vmbus_set_event':
>> drivers/hv/connection.c:473:3: error: implicit declaration of function 'sync_set_bit' [-Werror=implicit-function-declaration]
      sync_set_bit(child_relid & 31,
      ^~~~~~~~~~~~
   cc1: some warnings being treated as errors
--
   drivers/hv/channel.c: In function 'vmbus_setevent':
>> drivers/hv/channel.c:53:3: error: implicit declaration of function 'sync_set_bit' [-Werror=implicit-function-declaration]
      sync_set_bit(channel->offermsg.child_relid & 31,
      ^~~~~~~~~~~~
   cc1: some warnings being treated as errors

vim +/sync_set_bit +473 drivers/hv/connection.c

1b807e101 drivers/hv/connection.c         K. Y. Srinivasan 2015-12-21  467  void vmbus_set_event(struct vmbus_channel *channel)
3e7ee4902 drivers/staging/hv/Connection.c Hank Janssen     2009-07-13  468  {
21c3bef5d drivers/hv/connection.c         K. Y. Srinivasan 2012-12-01  469  	u32 child_relid = channel->offermsg.child_relid;
3be777740 drivers/hv/connection.c         K. Y. Srinivasan 2012-12-01  470  
3be777740 drivers/hv/connection.c         K. Y. Srinivasan 2012-12-01  471  	if (!channel->is_dedicated_interrupt) {
454f18a96 drivers/staging/hv/Connection.c Bill Pemberton   2009-07-27  472  		/* Each u32 represents 32 channels */
223565857 drivers/staging/hv/connection.c Olaf Hering      2011-03-21 @473  		sync_set_bit(child_relid & 31,
da9fcb726 drivers/staging/hv/connection.c Haiyang Zhang    2011-01-26  474  			(unsigned long *)vmbus_connection.send_int_page +
15b2f6479 drivers/staging/hv/connection.c Haiyang Zhang    2011-01-26  475  			(child_relid >> 5));
3be777740 drivers/hv/connection.c         K. Y. Srinivasan 2012-12-01  476  	}

:::::: The code at line 473 was first introduced by commit
:::::: 22356585712d1ff08fbfed152edd8b386873b238 staging: hv: use sync_bitops when interacting with the hypervisor

:::::: TO: Olaf Hering <olaf@aepfle.de>
:::::: CC: Greg Kroah-Hartman <gregkh@suse.de>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 38270 bytes --]

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH 03/15] hyperv: use standard bitops
  2016-12-20 15:55 ` [PATCH 03/15] hyperv: use standard bitops Roman Kagan
@ 2016-12-21 12:00   ` Olaf Hering
  2016-12-21 13:23     ` Roman Kagan
  2016-12-21 19:08   ` KY Srinivasan
  1 sibling, 1 reply; 54+ messages in thread
From: Olaf Hering @ 2016-12-21 12:00 UTC (permalink / raw)
  To: Roman Kagan
  Cc: Paolo Bonzini, Radim Krčmář,
	K. Y. Srinivasan, Vitaly Kuznetsov, Thomas Gleixner, Ingo Molnar,
	H. Peter Anvin, x86, Haiyang Zhang, kvm, linux-kernel, devel,
	Denis V . Lunev

[-- Attachment #1: Type: text/plain, Size: 243 bytes --]

On Tue, Dec 20, Roman Kagan wrote:

Reverting commit 22356585712d ("staging: hv: use sync_bitops when
interacting with the hypervisor") is save because .......

> -		sync_set_bit(channel->monitor_bit,
> +		set_bit(channel->monitor_bit,


Olaf

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH 12/15] hyperv: move VMBus connection ids to uapi
  2016-12-21  6:29     ` Roman Kagan
@ 2016-12-21 12:18       ` Christoph Hellwig
  2016-12-21 12:59         ` Roman Kagan
  0 siblings, 1 reply; 54+ messages in thread
From: Christoph Hellwig @ 2016-12-21 12:18 UTC (permalink / raw)
  To: Roman Kagan, Stephen Hemminger, Paolo Bonzini,
	Radim Krčmář,
	K. Y. Srinivasan, Vitaly Kuznetsov, kvm, Denis V . Lunev,
	Haiyang Zhang, x86, linux-kernel, Ingo Molnar, H. Peter Anvin,
	devel, Thomas Gleixner

On Wed, Dec 21, 2016 at 09:29:39AM +0300, Roman Kagan wrote:
> QEMU in particular.  We're planning to implement VMBus devices in QEMU
> and would like to have the definitions shared with the Linux guest
> drivers for Hyper-V.

And that's everything but a userspace API.  The way to go for protocol
constants is to have a normal kernel header that is not exported, and
a copy of it wherever else you need it.

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH 12/15] hyperv: move VMBus connection ids to uapi
  2016-12-21 12:18       ` Christoph Hellwig
@ 2016-12-21 12:59         ` Roman Kagan
  2016-12-21 14:26           ` Christoph Hellwig
  0 siblings, 1 reply; 54+ messages in thread
From: Roman Kagan @ 2016-12-21 12:59 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Stephen Hemminger, Paolo Bonzini, Radim Krčmář,
	K. Y. Srinivasan, Vitaly Kuznetsov, kvm, Denis V . Lunev,
	Haiyang Zhang, x86, linux-kernel, Ingo Molnar, H. Peter Anvin,
	devel, Thomas Gleixner

On Wed, Dec 21, 2016 at 04:18:58AM -0800, Christoph Hellwig wrote:
> On Wed, Dec 21, 2016 at 09:29:39AM +0300, Roman Kagan wrote:
> > QEMU in particular.  We're planning to implement VMBus devices in QEMU
> > and would like to have the definitions shared with the Linux guest
> > drivers for Hyper-V.
> 
> And that's everything but a userspace API.  The way to go for protocol
> constants is to have a normal kernel header that is not exported, and
> a copy of it wherever else you need it.

That's fine by me.

I guess the series should then start with a complete move
arch/x86/include/uapi/asm/hyperv.h ->
arch/x86/include/asm/hyperv_proto.h, and the remaining patches have to
change the latter instead of the former?

Thanks,
Roman.

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH 03/15] hyperv: use standard bitops
  2016-12-21 12:00   ` Olaf Hering
@ 2016-12-21 13:23     ` Roman Kagan
  2016-12-22 12:33       ` Paolo Bonzini
  0 siblings, 1 reply; 54+ messages in thread
From: Roman Kagan @ 2016-12-21 13:23 UTC (permalink / raw)
  To: Olaf Hering
  Cc: Paolo Bonzini, Radim Krčmář,
	K. Y. Srinivasan, Vitaly Kuznetsov, Thomas Gleixner, Ingo Molnar,
	H. Peter Anvin, x86, Haiyang Zhang, kvm, linux-kernel, devel,
	Denis V . Lunev

On Wed, Dec 21, 2016 at 01:00:44PM +0100, Olaf Hering wrote:
> On Tue, Dec 20, Roman Kagan wrote:
> 
> Reverting commit 22356585712d ("staging: hv: use sync_bitops when
> interacting with the hypervisor") is save because .......
> 
> > -		sync_set_bit(channel->monitor_bit,
> > +		set_bit(channel->monitor_bit,

It isn't indeed.  I didn't realize there was a UP case where it made a
difference, and failed to locate the commit where it changed.

I'll drop this part, thanks.

Roman.

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH 12/15] hyperv: move VMBus connection ids to uapi
  2016-12-21 12:59         ` Roman Kagan
@ 2016-12-21 14:26           ` Christoph Hellwig
  2016-12-21 14:39             ` Roman Kagan
  2016-12-21 15:39             ` Paolo Bonzini
  0 siblings, 2 replies; 54+ messages in thread
From: Christoph Hellwig @ 2016-12-21 14:26 UTC (permalink / raw)
  To: Roman Kagan, Christoph Hellwig, Stephen Hemminger, Paolo Bonzini,
	Radim Krčmář,
	K. Y. Srinivasan, Vitaly Kuznetsov, kvm, Denis V . Lunev,
	Haiyang Zhang, x86, linux-kernel, Ingo Molnar, H. Peter Anvin,
	devel, Thomas Gleixner

On Wed, Dec 21, 2016 at 03:59:20PM +0300, Roman Kagan wrote:
> That's fine by me.
> 
> I guess the series should then start with a complete move
> arch/x86/include/uapi/asm/hyperv.h ->
> arch/x86/include/asm/hyperv_proto.h, and the remaining patches have to
> change the latter instead of the former?

That would be my preference, but we'll need to figure out why
hyperv has ever been a UABI header, and if anyone is using it.

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH 12/15] hyperv: move VMBus connection ids to uapi
  2016-12-21 14:26           ` Christoph Hellwig
@ 2016-12-21 14:39             ` Roman Kagan
  2016-12-21 15:39             ` Paolo Bonzini
  1 sibling, 0 replies; 54+ messages in thread
From: Roman Kagan @ 2016-12-21 14:39 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Stephen Hemminger, Paolo Bonzini, Radim Krčmář,
	K. Y. Srinivasan, Vitaly Kuznetsov, kvm, Denis V . Lunev,
	Haiyang Zhang, x86, linux-kernel, Ingo Molnar, H. Peter Anvin,
	devel, Thomas Gleixner

On Wed, Dec 21, 2016 at 06:26:54AM -0800, Christoph Hellwig wrote:
> On Wed, Dec 21, 2016 at 03:59:20PM +0300, Roman Kagan wrote:
> > That's fine by me.
> > 
> > I guess the series should then start with a complete move
> > arch/x86/include/uapi/asm/hyperv.h ->
> > arch/x86/include/asm/hyperv_proto.h, and the remaining patches have to
> > change the latter instead of the former?
> 
> That would be my preference, but we'll need to figure out why
> hyperv has ever been a UABI header, and if anyone is using it.

It was initially done by my former teammate, and I guess only QEMU
(outside kernel) is using some of it.

Roman.

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH 12/15] hyperv: move VMBus connection ids to uapi
  2016-12-21 14:26           ` Christoph Hellwig
  2016-12-21 14:39             ` Roman Kagan
@ 2016-12-21 15:39             ` Paolo Bonzini
  2016-12-21 15:43               ` Christoph Hellwig
  1 sibling, 1 reply; 54+ messages in thread
From: Paolo Bonzini @ 2016-12-21 15:39 UTC (permalink / raw)
  To: Christoph Hellwig, Roman Kagan, Stephen Hemminger,
	Radim Krčmář,
	K. Y. Srinivasan, Vitaly Kuznetsov, kvm, Denis V . Lunev,
	Haiyang Zhang, x86, linux-kernel, Ingo Molnar, H. Peter Anvin,
	devel, Thomas Gleixner



On 21/12/2016 15:26, Christoph Hellwig wrote:
> On Wed, Dec 21, 2016 at 03:59:20PM +0300, Roman Kagan wrote:
>> That's fine by me.
>>
>> I guess the series should then start with a complete move
>> arch/x86/include/uapi/asm/hyperv.h ->
>> arch/x86/include/asm/hyperv_proto.h, and the remaining patches have to
>> change the latter instead of the former?
> 
> That would be my preference, but we'll need to figure out why
> hyperv has ever been a UABI header, and if anyone is using it.

QEMU uses it, but we already bundle the header files and update them
periodically from the files that Linux installs.  So any change in Linux
would not break the QEMU build; having the header in UAPI is convenient
but I guess our update scripts could do whatever Linux's
scripts/headers_install.sh does.

That said, there are precedents in using UAPI this way for PV
interfaces.  See for example include/uapi/linux/virtio*.h and
arch/x86/include/uapi/asm/kvm_para.h.

Paolo

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH 12/15] hyperv: move VMBus connection ids to uapi
  2016-12-21 15:39             ` Paolo Bonzini
@ 2016-12-21 15:43               ` Christoph Hellwig
  2016-12-21 17:25                 ` Paolo Bonzini
  2016-12-21 17:50                 ` Stephen Hemminger
  0 siblings, 2 replies; 54+ messages in thread
From: Christoph Hellwig @ 2016-12-21 15:43 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Christoph Hellwig, Roman Kagan, Stephen Hemminger,
	Radim Krčmář,
	K. Y. Srinivasan, Vitaly Kuznetsov, kvm, Denis V . Lunev,
	Haiyang Zhang, x86, linux-kernel, Ingo Molnar, H. Peter Anvin,
	devel, Thomas Gleixner

On Wed, Dec 21, 2016 at 04:39:18PM +0100, Paolo Bonzini wrote:
> That said, there are precedents in using UAPI this way for PV
> interfaces.  See for example include/uapi/linux/virtio*.h and
> arch/x86/include/uapi/asm/kvm_para.h.

We have all kinds of historical examples, but most of them turned
into a major pain sooner or later - my favourite example are the
SCSI protocol headers.

Protocols needs to stay compatible on the (virtual) wire, but not
on the language level.  Locking us into the strict UABI policies
for them just make someone life horrible further down the road.

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH 12/15] hyperv: move VMBus connection ids to uapi
  2016-12-21 15:43               ` Christoph Hellwig
@ 2016-12-21 17:25                 ` Paolo Bonzini
  2016-12-21 17:50                 ` Stephen Hemminger
  1 sibling, 0 replies; 54+ messages in thread
From: Paolo Bonzini @ 2016-12-21 17:25 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: kvm, Radim Krčmář,
	Haiyang Zhang, x86, linux-kernel, devel, Ingo Molnar,
	Roman Kagan, H. Peter Anvin, Denis V . Lunev, Thomas Gleixner



On 21/12/2016 16:43, Christoph Hellwig wrote:
> On Wed, Dec 21, 2016 at 04:39:18PM +0100, Paolo Bonzini wrote:
>> That said, there are precedents in using UAPI this way for PV
>> interfaces.  See for example include/uapi/linux/virtio*.h and
>> arch/x86/include/uapi/asm/kvm_para.h.
> 
> We have all kinds of historical examples, but most of them turned
> into a major pain sooner or later - my favourite example are the
> SCSI protocol headers.

Mine too, and because of how uapi/ was created there are quite a few
such historical headers (my favourite is cuda.h, just because of the name).

> Protocols needs to stay compatible on the (virtual) wire, but not
> on the language level.  Locking us into the strict UABI policies
> for them just make someone life horrible further down the road.

The ABI for this kind of thing is not changing much anyway, because it's
the hardware or processor or (as in this case) hypervisor ABI.

The more interesting question is about the API, and here in the end it
seems to be up to the maintainer.

Some have explicitly asked to move stuff *out* of UAPI, for example the
x86 guys have removed msr-index.h from UAPI recently.  Others are okay
with it and they simply aren't strict on cleanups that might break the
*programming* interface, as in patch 15 of this series.  See for example
pci_regs.h commit 846fc70986a6, "PCI/AER: Rename PCI_ERR_UNC_TRAIN to
PCI_ERR_UNC_UND", everybody just moved on and QEMU adjusted its use of
PCI_ERR_UNC_TRAIN.

Having this in UAPI has been convenient for QEMU, but of course the
kernel couldn't care less.  So if KY prefers to have the header outside
UAPI, we will just follow suit...

Paolo

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH 12/15] hyperv: move VMBus connection ids to uapi
  2016-12-21 15:43               ` Christoph Hellwig
  2016-12-21 17:25                 ` Paolo Bonzini
@ 2016-12-21 17:50                 ` Stephen Hemminger
  2016-12-21 17:53                   ` Paolo Bonzini
  2016-12-21 17:58                   ` Christoph Hellwig
  1 sibling, 2 replies; 54+ messages in thread
From: Stephen Hemminger @ 2016-12-21 17:50 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Paolo Bonzini, Roman Kagan, Radim Krčmář,
	K. Y. Srinivasan, Vitaly Kuznetsov, kvm, Denis V . Lunev,
	Haiyang Zhang, x86, linux-kernel, Ingo Molnar, H. Peter Anvin,
	devel, Thomas Gleixner

On Wed, 21 Dec 2016 07:43:48 -0800
Christoph Hellwig <hch@infradead.org> wrote:

> On Wed, Dec 21, 2016 at 04:39:18PM +0100, Paolo Bonzini wrote:
> > That said, there are precedents in using UAPI this way for PV
> > interfaces.  See for example include/uapi/linux/virtio*.h and
> > arch/x86/include/uapi/asm/kvm_para.h.  
> 
> We have all kinds of historical examples, but most of them turned
> into a major pain sooner or later - my favourite example are the
> SCSI protocol headers.
> 
> Protocols needs to stay compatible on the (virtual) wire, but not
> on the language level.  Locking us into the strict UABI policies
> for them just make someone life horrible further down the road.

If the the protocols come from external sources (like the current NDIS definitions),
then it is not a big deal. There is some overlap already where NDIS is used multiple
places and there are multiple header files with same definition. That could be
fixed.

The bigger problem is that some of the API's between guest and host could be
implemented multiple ways and don't want userspace ABI files constraining how
something like atomic bit operation for wakeup is done.

The other problem with the hyperv headers is they were initially done with
only the Linux driver usage in mind. This made perfect sense at the time,
the problem is that they mix internal state with protocol definitions.

Lastly, there is licensing issues on headers. It would be good to have any
userspace ABI headers licensed with a more liberal license so that BSD and DPDK drivers
could use them directly. Right now each one reinvents.

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH 12/15] hyperv: move VMBus connection ids to uapi
  2016-12-21 17:50                 ` Stephen Hemminger
@ 2016-12-21 17:53                   ` Paolo Bonzini
  2016-12-21 17:58                   ` Christoph Hellwig
  1 sibling, 0 replies; 54+ messages in thread
From: Paolo Bonzini @ 2016-12-21 17:53 UTC (permalink / raw)
  To: Stephen Hemminger, Christoph Hellwig
  Cc: Roman Kagan, Radim Krčmář,
	K. Y. Srinivasan, Vitaly Kuznetsov, kvm, Denis V . Lunev,
	Haiyang Zhang, x86, linux-kernel, Ingo Molnar, H. Peter Anvin,
	devel, Thomas Gleixner



On 21/12/2016 18:50, Stephen Hemminger wrote:
> The other problem with the hyperv headers is they were initially done with
> only the Linux driver usage in mind. This made perfect sense at the time,
> the problem is that they mix internal state with protocol definitions.

Yes, and this was partly fixed when KVM started to use some of those
definitions too (the implementation of Hyper-V protocols is split
between kernel and userspace).

> Lastly, there is licensing issues on headers. It would be good to have any
> userspace ABI headers licensed with a more liberal license so that BSD and DPDK drivers
> could use them directly. Right now each one reinvents.

Other projects are using sanitized userspace ABI headers just fine, so
this is something that lawyers should sort out before kernel hackers do.

Paolo

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH 12/15] hyperv: move VMBus connection ids to uapi
  2016-12-21 17:50                 ` Stephen Hemminger
  2016-12-21 17:53                   ` Paolo Bonzini
@ 2016-12-21 17:58                   ` Christoph Hellwig
  2016-12-21 18:02                     ` Stephen Hemminger
  1 sibling, 1 reply; 54+ messages in thread
From: Christoph Hellwig @ 2016-12-21 17:58 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: Christoph Hellwig, Paolo Bonzini, Roman Kagan,
	Radim Krčmář,
	K. Y. Srinivasan, Vitaly Kuznetsov, kvm, Denis V . Lunev,
	Haiyang Zhang, x86, linux-kernel, Ingo Molnar, H. Peter Anvin,
	devel, Thomas Gleixner

On Wed, Dec 21, 2016 at 09:50:49AM -0800, Stephen Hemminger wrote:
> Lastly, there is licensing issues on headers. It would be good to have any
> userspace ABI headers licensed with a more liberal license so that BSD and DPDK drivers
> could use them directly. Right now each one reinvents.

Microsoft could easily solves this problem by offering a suitably
liberally licensed header documenting the full HyperV guest protocol
that Linux and other projects could use.

^ permalink raw reply	[flat|nested] 54+ messages in thread

* RE: [PATCH 00/15] hyperv: more stuff to uapi + cleanup
  2016-12-20 15:55 [PATCH 00/15] hyperv: more stuff to uapi + cleanup Roman Kagan
                   ` (14 preceding siblings ...)
  2016-12-20 15:56 ` [PATCH 15/15] hyperv: redefine hv_message without bitfields Roman Kagan
@ 2016-12-21 18:00 ` KY Srinivasan
  2016-12-28 16:57   ` Roman Kagan
  15 siblings, 1 reply; 54+ messages in thread
From: KY Srinivasan @ 2016-12-21 18:00 UTC (permalink / raw)
  To: Roman Kagan, Paolo Bonzini, Radim Krčmář,
	Vitaly Kuznetsov
  Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, Haiyang Zhang,
	kvm, linux-kernel, devel, Denis V . Lunev



> -----Original Message-----
> From: Roman Kagan [mailto:rkagan@virtuozzo.com]
> Sent: Tuesday, December 20, 2016 7:56 AM
> To: Paolo Bonzini <pbonzini@redhat.com>; Radim Krčmář
> <rkrcmar@redhat.com>; KY Srinivasan <kys@microsoft.com>; Vitaly
> Kuznetsov <vkuznets@redhat.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>; Ingo Molnar
> <mingo@redhat.com>; H. Peter Anvin <hpa@zytor.com>; x86@kernel.org;
> Haiyang Zhang <haiyangz@microsoft.com>; kvm@vger.kernel.org; linux-
> kernel@vger.kernel.org; devel@linuxdriverproject.org; Denis V . Lunev
> <den@openvz.org>; Roman Kagan <rkagan@virtuozzo.com>
> Subject: [PATCH 00/15] hyperv: more stuff to uapi + cleanup
> 
> Expose more Hyper-V-related definitions in the uapi header for
> consumption by userspace.
> 
> While doing so, get rid of a number of duplications between the KVM and
> the guest driver code.  Also a few other cleanups are made which are not
> strictly necessary for the main purpose of the series but appear
> reasonable to do at the same time.
> 
> The most controversial is the last patch which modifies the stuff
> already published in the uapi header, in the hope that no userspace
> applications have started relying on it; I'm ok dropping it if this is
> unacceptable.

Roman,

First, let me thank you. Broadly, this patch-set can be broken into
1. Moving existing definitions around - (to make it possible to share these
between Hyper-V guest drivers and KVM)
2. Cleanup of the existing code in the VMBUS driver (under drivers/hv).

To the extent possible, I want to take all non-KVM code through Greg's tree.
We can then modify the KVM code to use these common definitions. Currently, I too
am working on restructuring VMBUS driver code to fully isolate all x86 dependencies.
I can work with you on integration as I too am moving things around.

Regards,

K. Y 

> 
> Roman Kagan (15):
>   hyperv: consolidate TSC ref page definitions
>   hyperv: uapi-fy synic event flags definitions
>   hyperv: use standard bitops
>   hyperv: define VMBus message type
>   hyperv: GFP_ATOMIC -> GFP_KERNEL
>   hyperv: avoid unnecessary vmalloc
>   hyperv: dedup cpuid definitions
>   hyperv: dedup crash msr related definitions
>   hyperv: unify Hyper-V msr definitions
>   hyperv: uapi-fy PostMessage and SignalEvent hypercall structures
>   hyperv: uapi-fy monitored notification structures
>   hyperv: move VMBus connection ids to uapi
>   hyperv: move function close to its only callsite
>   hyperv_vmbus: drop unused definitions
>   hyperv: redefine hv_message without bitfields
> 
>  arch/x86/include/asm/kvm_host.h    |   2 +-
>  arch/x86/include/uapi/asm/hyperv.h | 101 +++++++---
>  drivers/hv/hyperv_vmbus.h          | 399 +------------------------------------
>  include/linux/hyperv.h             |  24 +--
>  arch/x86/kvm/hyperv.c              |  14 +-
>  drivers/hv/channel.c               |   8 +-
>  drivers/hv/channel_mgmt.c          |  30 +--
>  drivers/hv/connection.c            |  65 ++----
>  drivers/hv/hv.c                    | 300 +++++++++++++---------------
>  drivers/hv/vmbus_drv.c             |  67 +++----
>  10 files changed, 288 insertions(+), 722 deletions(-)
> 
> --
> 2.9.3

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH 12/15] hyperv: move VMBus connection ids to uapi
  2016-12-21 17:58                   ` Christoph Hellwig
@ 2016-12-21 18:02                     ` Stephen Hemminger
  2016-12-21 19:54                       ` KY Srinivasan
  0 siblings, 1 reply; 54+ messages in thread
From: Stephen Hemminger @ 2016-12-21 18:02 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Paolo Bonzini, Roman Kagan, Radim Krčmář,
	K. Y. Srinivasan, Vitaly Kuznetsov, kvm, Denis V . Lunev,
	Haiyang Zhang, x86, linux-kernel, Ingo Molnar, H. Peter Anvin,
	devel, Thomas Gleixner

On Wed, 21 Dec 2016 09:58:36 -0800
Christoph Hellwig <hch@infradead.org> wrote:

> On Wed, Dec 21, 2016 at 09:50:49AM -0800, Stephen Hemminger wrote:
> > Lastly, there is licensing issues on headers. It would be good to have any
> > userspace ABI headers licensed with a more liberal license so that BSD and DPDK drivers
> > could use them directly. Right now each one reinvents.  
> 
> Microsoft could easily solves this problem by offering a suitably
> liberally licensed header documenting the full HyperV guest protocol
> that Linux and other projects could use.

The issue is if same header file mixes kernel and userspace API stuff.

Once the files are arranged right, I will submit trivial change to comments
to indicate the liberal licensing of userspace API headers.

^ permalink raw reply	[flat|nested] 54+ messages in thread

* RE: [PATCH 02/15] hyperv: uapi-fy synic event flags definitions
  2016-12-20 15:55 ` [PATCH 02/15] hyperv: uapi-fy synic event flags definitions Roman Kagan
  2016-12-20 17:24   ` Stephen Hemminger
  2016-12-21 10:58   ` kbuild test robot
@ 2016-12-21 18:58   ` KY Srinivasan
  2 siblings, 0 replies; 54+ messages in thread
From: KY Srinivasan @ 2016-12-21 18:58 UTC (permalink / raw)
  To: Roman Kagan, Paolo Bonzini, Radim Krčmář,
	Vitaly Kuznetsov
  Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, Haiyang Zhang,
	kvm, linux-kernel, devel, Denis V . Lunev



> -----Original Message-----
> From: Roman Kagan [mailto:rkagan@virtuozzo.com]
> Sent: Tuesday, December 20, 2016 7:56 AM
> To: Paolo Bonzini <pbonzini@redhat.com>; Radim Krčmář
> <rkrcmar@redhat.com>; KY Srinivasan <kys@microsoft.com>; Vitaly
> Kuznetsov <vkuznets@redhat.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>; Ingo Molnar
> <mingo@redhat.com>; H. Peter Anvin <hpa@zytor.com>; x86@kernel.org;
> Haiyang Zhang <haiyangz@microsoft.com>; kvm@vger.kernel.org; linux-
> kernel@vger.kernel.org; devel@linuxdriverproject.org; Denis V . Lunev
> <den@openvz.org>; Roman Kagan <rkagan@virtuozzo.com>
> Subject: [PATCH 02/15] hyperv: uapi-fy synic event flags definitions
> 
> Move definitions related to the Hyper-V SynIC event flags to a header
> where they can be consumed by userspace.
> 
> While doing so, also clean up their use by switching to standard bitops
> and struct-based dereferencing.  The latter is also done for message
> pages.

Please breakup this patch.
> 
> Signed-off-by: Roman Kagan <rkagan@virtuozzo.com>
> ---
>  arch/x86/include/uapi/asm/hyperv.h | 13 +++++++++
>  drivers/hv/hyperv_vmbus.h          | 24 ++--------------
>  drivers/hv/channel_mgmt.c          | 10 +++----
>  drivers/hv/connection.c            | 47 ++++++++++---------------------
>  drivers/hv/vmbus_drv.c             | 57 ++++++++++++++------------------------
>  5 files changed, 54 insertions(+), 97 deletions(-)
> 
> diff --git a/arch/x86/include/uapi/asm/hyperv.h
> b/arch/x86/include/uapi/asm/hyperv.h
> index 6098ab5..af542a3 100644
> --- a/arch/x86/include/uapi/asm/hyperv.h
> +++ b/arch/x86/include/uapi/asm/hyperv.h
> @@ -363,4 +363,17 @@ struct hv_timer_message_payload {
>  #define HV_STIMER_AUTOENABLE		(1ULL << 3)
>  #define HV_STIMER_SINT(config)		(__u8)(((config) >> 16) &
> 0x0F)
> 
> +/* Define synthetic interrupt controller flag constants. */
> +#define HV_EVENT_FLAGS_COUNT		(256 * 8)
> +
> +/* Define the synthetic interrupt controller event flags format. */
> +struct hv_synic_event_flags {
> +	__u64 flags[HV_EVENT_FLAGS_COUNT / 64];
> +};
> +
> +/* Define the synthetic interrupt flags page layout. */
> +struct hv_synic_event_flags_page {
> +	struct hv_synic_event_flags
> sintevent_flags[HV_SYNIC_SINT_COUNT];
> +};
> +
>  #endif
> diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
> index 4516498..4fab154 100644
> --- a/drivers/hv/hyperv_vmbus.h
> +++ b/drivers/hv/hyperv_vmbus.h
> @@ -26,7 +26,6 @@
>  #define _HYPERV_VMBUS_H
> 
>  #include <linux/list.h>
> -#include <asm/sync_bitops.h>
>  #include <linux/atomic.h>
>  #include <linux/hyperv.h>
> 
> @@ -75,11 +74,6 @@ enum hv_cpuid_function {
> 
>  #define HV_ANY_VP			(0xFFFFFFFF)
> 
> -/* Define synthetic interrupt controller flag constants. */
> -#define HV_EVENT_FLAGS_COUNT		(256 * 8)
> -#define HV_EVENT_FLAGS_BYTE_COUNT	(256)
> -#define HV_EVENT_FLAGS_DWORD_COUNT	(256 / sizeof(u32))
> -
>  /* Define invalid partition identifier. */
>  #define HV_PARTITION_ID_INVALID		((u64)0x0)
> 
> @@ -146,20 +140,6 @@ union hv_timer_config {
>  	};
>  };
> 
> -/* Define the number of message buffers associated with each port. */
> -#define HV_PORT_MESSAGE_BUFFER_COUNT	(16)
> -
> -/* Define the synthetic interrupt controller event flags format. */
> -union hv_synic_event_flags {
> -	u8 flags8[HV_EVENT_FLAGS_BYTE_COUNT];
> -	u32 flags32[HV_EVENT_FLAGS_DWORD_COUNT];
> -};
> -
> -/* Define the synthetic interrupt flags page layout. */
> -struct hv_synic_event_flags_page {
> -	union hv_synic_event_flags
> sintevent_flags[HV_SYNIC_SINT_COUNT];
> -};
> -
>  /* Define SynIC control register. */
>  union hv_synic_scontrol {
>  	u64 as_uint64;
> @@ -434,8 +414,8 @@ struct hv_context {
> 
>  	bool synic_initialized;
> 
> -	void *synic_message_page[NR_CPUS];
> -	void *synic_event_page[NR_CPUS];
> +	struct hv_message_page *synic_message_page[NR_CPUS];
> +	struct hv_synic_event_flags_page *synic_event_page[NR_CPUS];
>  	/*
>  	 * Hypervisor's notion of virtual processor ID is different from
>  	 * Linux' notion of CPU ID. This information can only be retrieved
> diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
> index 26b4192..49eaae2 100644
> --- a/drivers/hv/channel_mgmt.c
> +++ b/drivers/hv/channel_mgmt.c
> @@ -654,7 +654,6 @@ static void init_vp_index(struct vmbus_channel
> *channel, u16 dev_type)
>  static void vmbus_wait_for_unload(void)
>  {
>  	int cpu;
> -	void *page_addr;
>  	struct hv_message *msg;
>  	struct vmbus_channel_message_header *hdr;
>  	u32 message_type;
> @@ -673,9 +672,8 @@ static void vmbus_wait_for_unload(void)
>  			break;
> 
>  		for_each_online_cpu(cpu) {
> -			page_addr = hv_context.synic_message_page[cpu];
> -			msg = (struct hv_message *)page_addr +
> -				VMBUS_MESSAGE_SINT;
> +			msg = &hv_context.synic_message_page[cpu]->
> +
> 	sint_message[VMBUS_MESSAGE_SINT];
> 
>  			message_type = READ_ONCE(msg-
> >header.message_type);
>  			if (message_type == HVMSG_NONE)
> @@ -699,8 +697,8 @@ static void vmbus_wait_for_unload(void)
>  	 * messages after we reconnect.
>  	 */
>  	for_each_online_cpu(cpu) {
> -		page_addr = hv_context.synic_message_page[cpu];
> -		msg = (struct hv_message *)page_addr +
> VMBUS_MESSAGE_SINT;
> +		msg = &hv_context.synic_message_page[cpu]->
> +				sint_message[VMBUS_MESSAGE_SINT];
>  		msg->header.message_type = HVMSG_NONE;
>  	}
>  }
> diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c
> index 6ce8b87..aaa2103 100644
> --- a/drivers/hv/connection.c
> +++ b/drivers/hv/connection.c
> @@ -381,17 +381,12 @@ static void process_chn_event(u32 relid)
>   */
>  void vmbus_on_event(unsigned long data)
>  {
> -	u32 dword;
> -	u32 maxdword;
> -	int bit;
> -	u32 relid;
> -	u32 *recv_int_page = NULL;
> -	void *page_addr;
> +	u32 relid, max_relid;
> +	unsigned long *recv_int_page;
>  	int cpu = smp_processor_id();
> -	union hv_synic_event_flags *event;
> 
>  	if (vmbus_proto_version < VERSION_WIN8) {
> -		maxdword = MAX_NUM_CHANNELS_SUPPORTED >> 5;
> +		max_relid = MAX_NUM_CHANNELS_SUPPORTED;
>  		recv_int_page = vmbus_connection.recv_int_page;
>  	} else {
>  		/*
> @@ -399,36 +394,22 @@ void vmbus_on_event(unsigned long data)
>  		 * can be directly checked to get the id of the channel
>  		 * that has the interrupt pending.
>  		 */
> -		maxdword = HV_EVENT_FLAGS_DWORD_COUNT;
> -		page_addr = hv_context.synic_event_page[cpu];
> -		event = (union hv_synic_event_flags *)page_addr +
> -						 VMBUS_MESSAGE_SINT;
> -		recv_int_page = event->flags32;
> +		struct hv_synic_event_flags *event =
> +			&hv_context.synic_event_page[cpu]->
> +				sintevent_flags[VMBUS_MESSAGE_SINT];
> +		max_relid = HV_EVENT_FLAGS_COUNT;
> +		recv_int_page = (unsigned long *)event->flags;
>  	}
> 
> -
> -
>  	/* Check events */
>  	if (!recv_int_page)
>  		return;
> -	for (dword = 0; dword < maxdword; dword++) {
> -		if (!recv_int_page[dword])
> -			continue;
> -		for (bit = 0; bit < 32; bit++) {
> -			if (sync_test_and_clear_bit(bit,
> -				(unsigned long *)&recv_int_page[dword])) {
> -				relid = (dword << 5) + bit;
> -
> -				if (relid == 0)
> -					/*
> -					 * Special case - vmbus
> -					 * channel protocol msg
> -					 */
> -					continue;
> -
> -				process_chn_event(relid);
> -			}
> -		}
> +
> +	/* relid == 0 is vmbus channel protocol msg */
> +	relid = 1;
> +	for_each_set_bit_from(relid, recv_int_page, max_relid) {
> +		clear_bit(relid, recv_int_page);

We are using this test_and_clear_bit paradigm for locking. The current code
used the sync variant to ensure the host saw the changes we were making
here (clearing the bit). You may want to add a barrier here or use the sync
variant.
 
> +		process_chn_event(relid);
>  	}
>  }
> 
> diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
> index 230c62e..13dd210 100644
> --- a/drivers/hv/vmbus_drv.c
> +++ b/drivers/hv/vmbus_drv.c
> @@ -872,9 +872,8 @@ static void hv_process_timer_expiration(struct
> hv_message *msg, int cpu)
>  void vmbus_on_msg_dpc(unsigned long data)
>  {
>  	int cpu = smp_processor_id();
> -	void *page_addr = hv_context.synic_message_page[cpu];
> -	struct hv_message *msg = (struct hv_message *)page_addr +
> -				  VMBUS_MESSAGE_SINT;
> +	struct hv_message *msg = &hv_context.synic_message_page[cpu]-
> >
> +
> 	sint_message[VMBUS_MESSAGE_SINT];
>  	struct vmbus_channel_message_header *hdr;
>  	struct vmbus_channel_message_table_entry *entry;
>  	struct onmessage_work_context *ctx;
> @@ -911,54 +910,40 @@ void vmbus_on_msg_dpc(unsigned long data)
>  static void vmbus_isr(void)
>  {
>  	int cpu = smp_processor_id();
> -	void *page_addr;
>  	struct hv_message *msg;
> -	union hv_synic_event_flags *event;
> -	bool handled = false;
> +	struct hv_synic_event_flags *event;
> 
> -	page_addr = hv_context.synic_event_page[cpu];
> -	if (page_addr == NULL)
> +	if (!hv_context.synic_event_page[cpu])
>  		return;
> 
> -	event = (union hv_synic_event_flags *)page_addr +
> -					 VMBUS_MESSAGE_SINT;
> +	event = &hv_context.synic_event_page[cpu]->
> +			sintevent_flags[VMBUS_MESSAGE_SINT];
>  	/*
>  	 * Check for events before checking for messages. This is the order
>  	 * in which events and messages are checked in Windows guests on
>  	 * Hyper-V, and the Windows team suggested we do the same.
>  	 */
> 
> -	if ((vmbus_proto_version == VERSION_WS2008) ||
> -		(vmbus_proto_version == VERSION_WIN7)) {
> -
> +	/* On win8 and above the channel interrupts are signaled directly in
> +	 * the event page and will be checked in the .event_dpc
> +	 */
> +	if (vmbus_proto_version >= VERSION_WIN8 ||
>  		/* Since we are a child, we only need to check bit 0 */
> -		if (sync_test_and_clear_bit(0,
> -			(unsigned long *) &event->flags32[0])) {
> -			handled = true;
> -		}
> -	} else {
> -		/*
> -		 * Our host is win8 or above. The signaling mechanism
> -		 * has changed and we can directly look at the event page.
> -		 * If bit n is set then we have an interrup on the channel
> -		 * whose id is n.
> -		 */
> -		handled = true;
> -	}
> -
> -	if (handled)
> +	    test_and_clear_bit(0, (unsigned long *)event->flags))

Don't we need the sync variant of test_and_clear_bit here.

>  		tasklet_schedule(hv_context.event_dpc[cpu]);
> 
> -
> -	page_addr = hv_context.synic_message_page[cpu];
> -	msg = (struct hv_message *)page_addr + VMBUS_MESSAGE_SINT;
> +	msg = &hv_context.synic_message_page[cpu]->
> +			sint_message[VMBUS_MESSAGE_SINT];
> 
>  	/* Check if there are actual msgs to be processed */
> -	if (msg->header.message_type != HVMSG_NONE) {
> -		if (msg->header.message_type == HVMSG_TIMER_EXPIRED)
> -			hv_process_timer_expiration(msg, cpu);
> -		else
> -			tasklet_schedule(hv_context.msg_dpc[cpu]);
> +	switch (READ_ONCE(msg->header.message_type)) {
> +	case HVMSG_NONE:
> +		break;
> +	case HVMSG_TIMER_EXPIRED:
> +		hv_process_timer_expiration(msg, cpu);
> +		break;
> +	default:
> +		tasklet_schedule(hv_context.msg_dpc[cpu]);
>  	}
> 
>  	add_interrupt_randomness(HYPERVISOR_CALLBACK_VECTOR, 0);
> --
> 2.9.3

^ permalink raw reply	[flat|nested] 54+ messages in thread

* RE: [PATCH 03/15] hyperv: use standard bitops
  2016-12-20 15:55 ` [PATCH 03/15] hyperv: use standard bitops Roman Kagan
  2016-12-21 12:00   ` Olaf Hering
@ 2016-12-21 19:08   ` KY Srinivasan
  1 sibling, 0 replies; 54+ messages in thread
From: KY Srinivasan @ 2016-12-21 19:08 UTC (permalink / raw)
  To: Roman Kagan, Paolo Bonzini, Radim Krčmář,
	Vitaly Kuznetsov
  Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, Haiyang Zhang,
	kvm, linux-kernel, devel, Denis V . Lunev



> -----Original Message-----
> From: Roman Kagan [mailto:rkagan@virtuozzo.com]
> Sent: Tuesday, December 20, 2016 7:56 AM
> To: Paolo Bonzini <pbonzini@redhat.com>; Radim Krčmář
> <rkrcmar@redhat.com>; KY Srinivasan <kys@microsoft.com>; Vitaly
> Kuznetsov <vkuznets@redhat.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>; Ingo Molnar
> <mingo@redhat.com>; H. Peter Anvin <hpa@zytor.com>; x86@kernel.org;
> Haiyang Zhang <haiyangz@microsoft.com>; kvm@vger.kernel.org; linux-
> kernel@vger.kernel.org; devel@linuxdriverproject.org; Denis V . Lunev
> <den@openvz.org>; Roman Kagan <rkagan@virtuozzo.com>
> Subject: [PATCH 03/15] hyperv: use standard bitops
>
No commit log?
 
> Signed-off-by: Roman Kagan <rkagan@virtuozzo.com>
> ---
>  drivers/hv/channel.c    | 8 +++-----
>  drivers/hv/connection.c | 9 +++------
>  2 files changed, 6 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
> index 5fb4c6d..f9df275 100644
> --- a/drivers/hv/channel.c
> +++ b/drivers/hv/channel.c
> @@ -49,15 +49,13 @@ void vmbus_setevent(struct vmbus_channel
> *channel)
>  	 */
>  	if ((channel->offermsg.monitor_allocated) &&
>  	    (!channel->low_latency)) {
> -		/* Each u32 represents 32 channels */
> -		sync_set_bit(channel->offermsg.child_relid & 31,
> -			(unsigned long *) vmbus_connection.send_int_page
> +
> -			(channel->offermsg.child_relid >> 5));
> +		set_bit(channel->offermsg.child_relid,
> +			(unsigned long
> *)vmbus_connection.send_int_page);
>
What is the rationale for dropping the sync variant?
 
>  		/* Get the child to parent monitor page */
>  		monitorpage = vmbus_connection.monitor_pages[1];
> 
> -		sync_set_bit(channel->monitor_bit,
> +		set_bit(channel->monitor_bit,
>  			(unsigned long *)&monitorpage->trigger_group
>  					[channel->monitor_grp].pending);
> 
Same comment as before.
> diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c
> index aaa2103..139b33e 100644
> --- a/drivers/hv/connection.c
> +++ b/drivers/hv/connection.c
> @@ -468,12 +468,9 @@ void vmbus_set_event(struct vmbus_channel
> *channel)
>  {
>  	u32 child_relid = channel->offermsg.child_relid;
> 
> -	if (!channel->is_dedicated_interrupt) {
> -		/* Each u32 represents 32 channels */
> -		sync_set_bit(child_relid & 31,
> -			(unsigned long *)vmbus_connection.send_int_page
> +
> -			(child_relid >> 5));
> -	}
> +	if (!channel->is_dedicated_interrupt)
> +		set_bit(child_relid,
> +			(unsigned long
> *)vmbus_connection.send_int_page);
> 
>  	hv_do_hypercall(HVCALL_SIGNAL_EVENT, channel->sig_event,
> NULL);
>  }
> --
> 2.9.3

^ permalink raw reply	[flat|nested] 54+ messages in thread

* RE: [PATCH 06/15] hyperv: avoid unnecessary vmalloc
  2016-12-20 15:55 ` [PATCH 06/15] hyperv: avoid unnecessary vmalloc Roman Kagan
@ 2016-12-21 19:19   ` KY Srinivasan
  0 siblings, 0 replies; 54+ messages in thread
From: KY Srinivasan @ 2016-12-21 19:19 UTC (permalink / raw)
  To: Roman Kagan, Paolo Bonzini, Radim Krčmář,
	Vitaly Kuznetsov
  Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, Haiyang Zhang,
	kvm, linux-kernel, devel, Denis V . Lunev



> -----Original Message-----
> From: Roman Kagan [mailto:rkagan@virtuozzo.com]
> Sent: Tuesday, December 20, 2016 7:56 AM
> To: Paolo Bonzini <pbonzini@redhat.com>; Radim Krčmář
> <rkrcmar@redhat.com>; KY Srinivasan <kys@microsoft.com>; Vitaly
> Kuznetsov <vkuznets@redhat.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>; Ingo Molnar
> <mingo@redhat.com>; H. Peter Anvin <hpa@zytor.com>; x86@kernel.org;
> Haiyang Zhang <haiyangz@microsoft.com>; kvm@vger.kernel.org; linux-
> kernel@vger.kernel.org; devel@linuxdriverproject.org; Denis V . Lunev
> <den@openvz.org>; Roman Kagan <rkagan@virtuozzo.com>
> Subject: [PATCH 06/15] hyperv: avoid unnecessary vmalloc
> 
> Make hypercall and tsc page allocation similar to the rest of the
> Hyper-V shared memory stuff instead of vmalloc-ing them.
> 
> Also perform cleanup unconditionally which is safe.
> 
> TODO: the skipping of free in case of a crash is probably no longer
> necessary, too.

Please breakup these patches.
> 
> Signed-off-by: Roman Kagan <rkagan@virtuozzo.com>
> ---
>  drivers/hv/hv.c | 42 ++++++++++++++++++------------------------
>  1 file changed, 18 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
> index 6bbc0b09..b40c7d9 100644
> --- a/drivers/hv/hv.c
> +++ b/drivers/hv/hv.c
> @@ -29,6 +29,7 @@
>  #include <linux/version.h>
>  #include <linux/interrupt.h>
>  #include <linux/clockchips.h>
> +#include <asm/cacheflush.h>
>  #include <asm/hyperv.h>
>  #include <asm/mshyperv.h>
>  #include "hyperv_vmbus.h"
> @@ -208,14 +209,15 @@ int hv_init(void)
>  	/* See if the hypercall page is already set */
>  	rdmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64);
> 
> -	virtaddr = __vmalloc(PAGE_SIZE, GFP_KERNEL,
> PAGE_KERNEL_EXEC);
> -
> -	if (!virtaddr)
> +	virtaddr = (void *)get_zeroed_page(GFP_KERNEL);
> +	if (!virtaddr || set_memory_x((unsigned long)virtaddr, 1))
>  		goto cleanup;

Have you tested this patch.
> +	hv_context.hypercall_page = virtaddr;
> 
>  	hypercall_msr.enable = 1;
> 
> -	hypercall_msr.guest_physical_address = vmalloc_to_pfn(virtaddr);
> +	hypercall_msr.guest_physical_address =
> +				virt_to_phys(virtaddr) >> PAGE_SHIFT;
>  	wrmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64);
> 
>  	/* Confirm that hypercall page did get setup. */
> @@ -225,14 +227,12 @@ int hv_init(void)
>  	if (!hypercall_msr.enable)
>  		goto cleanup;
> 
> -	hv_context.hypercall_page = virtaddr;
> -
>  #ifdef CONFIG_X86_64
>  	if (ms_hyperv.features &
> HV_X64_MSR_REFERENCE_TSC_AVAILABLE) {
>  		union hv_x64_msr_hypercall_contents tsc_msr;
>  		void *va_tsc;
> 
> -		va_tsc = __vmalloc(PAGE_SIZE, GFP_KERNEL,
> PAGE_KERNEL);
> +		va_tsc = (void *)get_zeroed_page(GFP_KERNEL);
>  		if (!va_tsc)
>  			goto cleanup;
>  		hv_context.tsc_page = va_tsc;
> @@ -240,7 +240,8 @@ int hv_init(void)
>  		rdmsrl(HV_X64_MSR_REFERENCE_TSC, tsc_msr.as_uint64);
> 
>  		tsc_msr.enable = 1;
> -		tsc_msr.guest_physical_address = vmalloc_to_pfn(va_tsc);
> +		tsc_msr.guest_physical_address =
> +				virt_to_phys(va_tsc) >> PAGE_SHIFT;
> 
>  		wrmsrl(HV_X64_MSR_REFERENCE_TSC, tsc_msr.as_uint64);
>  		clocksource_register_hz(&hyperv_cs_tsc,
> NSEC_PER_SEC/100);
> @@ -249,14 +250,9 @@ int hv_init(void)
>  	return 0;
> 
>  cleanup:
> -	if (virtaddr) {
> -		if (hypercall_msr.enable) {
> -			hypercall_msr.as_uint64 = 0;
> -			wrmsrl(HV_X64_MSR_HYPERCALL,
> hypercall_msr.as_uint64);
> -		}
> -
> -		vfree(virtaddr);
> -	}
> +	hypercall_msr.as_uint64 = 0;
> +	wrmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64);
> +	free_page((unsigned long)virtaddr);
> 
>  	return -ENOTSUPP;
>  }
> @@ -273,13 +269,11 @@ void hv_cleanup(bool crash)
>  	/* Reset our OS id */
>  	wrmsrl(HV_X64_MSR_GUEST_OS_ID, 0);
> 
> -	if (hv_context.hypercall_page) {
> -		hypercall_msr.as_uint64 = 0;
> -		wrmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64);
> -		if (!crash)
> -			vfree(hv_context.hypercall_page);
> -		hv_context.hypercall_page = NULL;
> -	}
> +	hypercall_msr.as_uint64 = 0;
> +	wrmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64);
> +	if (!crash)
> +		free_page((unsigned long)hv_context.hypercall_page);
> +	hv_context.hypercall_page = NULL;
> 
>  #ifdef CONFIG_X86_64
>  	/*
> @@ -298,7 +292,7 @@ void hv_cleanup(bool crash)
>  		hypercall_msr.as_uint64 = 0;
>  		wrmsrl(HV_X64_MSR_REFERENCE_TSC,
> hypercall_msr.as_uint64);
>  		if (!crash)
> -			vfree(hv_context.tsc_page);
> +			free_page((unsigned long)hv_context.tsc_page);
>  		hv_context.tsc_page = NULL;
>  	}
>  #endif
> --
> 2.9.3

^ permalink raw reply	[flat|nested] 54+ messages in thread

* RE: [PATCH 10/15] hyperv: uapi-fy PostMessage and SignalEvent hypercall structures
  2016-12-20 15:55 ` [PATCH 10/15] hyperv: uapi-fy PostMessage and SignalEvent hypercall structures Roman Kagan
@ 2016-12-21 19:27   ` KY Srinivasan
  0 siblings, 0 replies; 54+ messages in thread
From: KY Srinivasan @ 2016-12-21 19:27 UTC (permalink / raw)
  To: Roman Kagan, Paolo Bonzini, Radim Krčmář,
	Vitaly Kuznetsov
  Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, Haiyang Zhang,
	kvm, linux-kernel, devel, Denis V . Lunev



> -----Original Message-----
> From: Roman Kagan [mailto:rkagan@virtuozzo.com]
> Sent: Tuesday, December 20, 2016 7:56 AM
> To: Paolo Bonzini <pbonzini@redhat.com>; Radim Krčmář
> <rkrcmar@redhat.com>; KY Srinivasan <kys@microsoft.com>; Vitaly
> Kuznetsov <vkuznets@redhat.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>; Ingo Molnar
> <mingo@redhat.com>; H. Peter Anvin <hpa@zytor.com>; x86@kernel.org;
> Haiyang Zhang <haiyangz@microsoft.com>; kvm@vger.kernel.org; linux-
> kernel@vger.kernel.org; devel@linuxdriverproject.org; Denis V . Lunev
> <den@openvz.org>; Roman Kagan <rkagan@virtuozzo.com>
> Subject: [PATCH 10/15] hyperv: uapi-fy PostMessage and SignalEvent
> hypercall structures
> 
> Expose structures used for PostMessage and SignalEvent hypercalls in a
> uapi header.  While doing so, simplify alignment handling and drop
> unnecessary complications in the connectionid field.

Split up the patch.
> 
> Signed-off-by: Roman Kagan <rkagan@virtuozzo.com>
> ---
>  arch/x86/include/uapi/asm/hyperv.h | 18 ++++++++++++++++++
>  drivers/hv/hyperv_vmbus.h          | 16 ++--------------
>  include/linux/hyperv.h             | 24 +-----------------------
>  drivers/hv/channel_mgmt.c          | 14 ++++----------
>  drivers/hv/connection.c            |  9 +++------
>  drivers/hv/hv.c                    |  2 +-
>  drivers/hv/vmbus_drv.c             |  2 +-
>  7 files changed, 30 insertions(+), 55 deletions(-)
> 
> diff --git a/arch/x86/include/uapi/asm/hyperv.h
> b/arch/x86/include/uapi/asm/hyperv.h
> index 749fbb25..eb8d42a 100644
> --- a/arch/x86/include/uapi/asm/hyperv.h
> +++ b/arch/x86/include/uapi/asm/hyperv.h
> @@ -378,4 +378,22 @@ struct hv_synic_event_flags_page {
>  	struct hv_synic_event_flags
> sintevent_flags[HV_SYNIC_SINT_COUNT];
>  };
> 
> +#define HV_HYPERCALL_PARAM_ALIGN	8
> +
> +/* Definition of the hv_post_message hypercall input structure. */
> +struct hv_input_post_message {
> +	__u32 connectionid;
> +	__u32 reserved;
> +	__u32 message_type;
> +	__u32 payload_size;
> +	__u64 payload[HV_MESSAGE_PAYLOAD_BYTE_COUNT];
> +} __attribute__((aligned(HV_HYPERCALL_PARAM_ALIGN)));
> +
> +/* Definition of the hv_signal_event hypercall input structure. */
> +struct hv_input_signal_event {
> +	__u32 connectionid;
> +	__u16 flag_number;
> +	__u16 rsvdz;
> +} __attribute__((aligned(HV_HYPERCALL_PARAM_ALIGN)));
> +
>  #endif
> diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
> index ac73832..a96f021 100644
> --- a/drivers/hv/hyperv_vmbus.h
> +++ b/drivers/hv/hyperv_vmbus.h
> @@ -104,7 +104,7 @@ union hv_monitor_trigger_group {
>  };
> 
>  struct hv_monitor_parameter {
> -	union hv_connection_id connectionid;
> +	u32 connectionid;
>  	u16 flagnumber;
>  	u16 rsvdz;
>  };
> @@ -154,15 +154,6 @@ struct hv_monitor_page {
>  	u8 rsvdz4[1984];
>  };
> 
> -/* Definition of the hv_post_message hypercall input structure. */
> -struct hv_input_post_message {
> -	union hv_connection_id connectionid;
> -	u32 reserved;
> -	u32 message_type;
> -	u32 payload_size;
> -	u64 payload[HV_MESSAGE_PAYLOAD_QWORD_COUNT];
> -};
> -
>  /*
>   * Versioning definitions used for guests reporting themselves to the
>   * hypervisor, and visa versa.
> @@ -248,9 +239,6 @@ static inline  __u64 generate_guest_id(__u8 d_info1,
> __u32 kernel_version,
>  #define HV_CAPS_MAX			8
> 
> 
> -#define HV_HYPERCALL_PARAM_ALIGN	sizeof(u64)
> -
> -
>  /* Service definitions */
> 
>  #define HV_SERVICE_PARENT_PORT				(0)
> @@ -351,7 +339,7 @@ extern int hv_init(void);
> 
>  extern void hv_cleanup(bool crash);
> 
> -extern int hv_post_message(union hv_connection_id connection_id,
> +extern int hv_post_message(u32 connection_id,
>  			 enum hv_message_type message_type,
>  			 void *payload, size_t payload_size);
> 
> diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
> index 42fe43f..e92446e 100644
> --- a/include/linux/hyperv.h
> +++ b/include/linux/hyperv.h
> @@ -648,27 +648,6 @@ struct vmbus_close_msg {
>  	struct vmbus_channel_close_channel msg;
>  };
> 
> -/* Define connection identifier type. */
> -union hv_connection_id {
> -	u32 asu32;
> -	struct {
> -		u32 id:24;
> -		u32 reserved:8;
> -	} u;
> -};
> -
> -/* Definition of the hv_signal_event hypercall input structure. */
> -struct hv_input_signal_event {
> -	union hv_connection_id connectionid;
> -	u16 flag_number;
> -	u16 rsvdz;
> -};
> -
> -struct hv_input_signal_event_buffer {
> -	u64 align8;
> -	struct hv_input_signal_event event;
> -};
> -
>  enum hv_signal_policy {
>  	HV_SIGNAL_POLICY_DEFAULT = 0,
>  	HV_SIGNAL_POLICY_EXPLICIT,
> @@ -755,8 +734,7 @@ struct vmbus_channel {
>  	bool batched_reading;
> 
>  	bool is_dedicated_interrupt;
> -	struct hv_input_signal_event_buffer sig_buf;
> -	struct hv_input_signal_event *sig_event;
> +	struct hv_input_signal_event sig_event;
> 
>  	/*
>  	 * Starting with win8, this field will be used to specify
> diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
> index 49eaae2..4a5cc11 100644
> --- a/drivers/hv/channel_mgmt.c
> +++ b/drivers/hv/channel_mgmt.c
> @@ -766,20 +766,14 @@ static void vmbus_onoffer(struct
> vmbus_channel_message_header *hdr)
>  	/*
>  	 * Setup state for signalling the host.
>  	 */
> -	newchannel->sig_event = (struct hv_input_signal_event *)
> -				(ALIGN((unsigned long)
> -				&newchannel->sig_buf,
> -				HV_HYPERCALL_PARAM_ALIGN));
> -
> -	newchannel->sig_event->connectionid.asu32 = 0;
> -	newchannel->sig_event->connectionid.u.id =
> VMBUS_EVENT_CONNECTION_ID;
> -	newchannel->sig_event->flag_number = 0;
> -	newchannel->sig_event->rsvdz = 0;
> +	newchannel->sig_event.connectionid =
> VMBUS_EVENT_CONNECTION_ID;
> +	newchannel->sig_event.flag_number = 0;
> +	newchannel->sig_event.rsvdz = 0;
> 
>  	if (vmbus_proto_version != VERSION_WS2008) {
>  		newchannel->is_dedicated_interrupt =
>  				(offer->is_dedicated_interrupt != 0);
> -		newchannel->sig_event->connectionid.u.id =
> +		newchannel->sig_event.connectionid =
>  				offer->connection_id;
>  	}
> 
> diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c
> index d38b27f..16ae977 100644
> --- a/drivers/hv/connection.c
> +++ b/drivers/hv/connection.c
> @@ -418,21 +418,18 @@ void vmbus_on_event(unsigned long data)
>   */
>  int vmbus_post_msg(void *buffer, size_t buflen)
>  {
> -	union hv_connection_id conn_id;
>  	int ret = 0;
>  	int retries = 0;
>  	u32 usec = 1;
> 
> -	conn_id.asu32 = 0;
> -	conn_id.u.id = VMBUS_MESSAGE_CONNECTION_ID;
> -
>  	/*
>  	 * hv_post_message() can have transient failures because of
>  	 * insufficient resources. Retry the operation a couple of
>  	 * times before giving up.
>  	 */
>  	while (retries < 20) {
> -		ret = hv_post_message(conn_id, HVMSG_VMBUS, buffer,
> buflen);
> +		ret =
> hv_post_message(VMBUS_MESSAGE_CONNECTION_ID, HVMSG_VMBUS,
> +				      buffer, buflen);
> 
>  		switch (ret) {
>  		case HV_STATUS_INVALID_CONNECTION_ID:
> @@ -472,6 +469,6 @@ void vmbus_set_event(struct vmbus_channel
> *channel)
>  		set_bit(child_relid,
>  			(unsigned long
> *)vmbus_connection.send_int_page);
> 
> -	hv_do_hypercall(HVCALL_SIGNAL_EVENT, channel->sig_event,
> NULL);
> +	hv_do_hypercall(HVCALL_SIGNAL_EVENT, &channel->sig_event,
> NULL);
>  }
>  EXPORT_SYMBOL_GPL(vmbus_set_event);
> diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
> index 7d2a3d1..b9f50de 100644
> --- a/drivers/hv/hv.c
> +++ b/drivers/hv/hv.c
> @@ -297,7 +297,7 @@ void hv_cleanup(bool crash)
>   *
>   * This involves a hypercall.
>   */
> -int hv_post_message(union hv_connection_id connection_id,
> +int hv_post_message(u32 connection_id,
>  		  enum hv_message_type message_type,
>  		  void *payload, size_t payload_size)
>  {
> diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
> index 7564a7b..f6b626b 100644
> --- a/drivers/hv/vmbus_drv.c
> +++ b/drivers/hv/vmbus_drv.c
> @@ -159,7 +159,7 @@ static u32 channel_conn_id(struct vmbus_channel
> *channel,
>  {
>  	u8 monitor_group = channel_monitor_group(channel);
>  	u8 monitor_offset = channel_monitor_offset(channel);
> -	return monitor_page-
> >parameter[monitor_group][monitor_offset].connectionid.u.id;
> +	return monitor_page-
> >parameter[monitor_group][monitor_offset].connectionid;
>  }
> 
>  static ssize_t id_show(struct device *dev, struct device_attribute *dev_attr,
> --
> 2.9.3

^ permalink raw reply	[flat|nested] 54+ messages in thread

* RE: [PATCH 12/15] hyperv: move VMBus connection ids to uapi
  2016-12-21 18:02                     ` Stephen Hemminger
@ 2016-12-21 19:54                       ` KY Srinivasan
  2016-12-28 17:09                         ` Roman Kagan
  0 siblings, 1 reply; 54+ messages in thread
From: KY Srinivasan @ 2016-12-21 19:54 UTC (permalink / raw)
  To: Stephen Hemminger, Christoph Hellwig
  Cc: Paolo Bonzini, Roman Kagan, Radim Krčmář,
	Vitaly Kuznetsov, kvm, Denis V . Lunev, Haiyang Zhang, x86,
	linux-kernel, Ingo Molnar, H. Peter Anvin, devel,
	Thomas Gleixner



> -----Original Message-----
> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> Sent: Wednesday, December 21, 2016 10:03 AM
> To: Christoph Hellwig <hch@infradead.org>
> Cc: Paolo Bonzini <pbonzini@redhat.com>; Roman Kagan
> <rkagan@virtuozzo.com>; Radim Krčmář <rkrcmar@redhat.com>; KY
> Srinivasan <kys@microsoft.com>; Vitaly Kuznetsov
> <vkuznets@redhat.com>; kvm@vger.kernel.org; Denis V . Lunev
> <den@openvz.org>; Haiyang Zhang <haiyangz@microsoft.com>;
> x86@kernel.org; linux-kernel@vger.kernel.org; Ingo Molnar
> <mingo@redhat.com>; H. Peter Anvin <hpa@zytor.com>;
> devel@linuxdriverproject.org; Thomas Gleixner <tglx@linutronix.de>
> Subject: Re: [PATCH 12/15] hyperv: move VMBus connection ids to uapi
> 
> On Wed, 21 Dec 2016 09:58:36 -0800
> Christoph Hellwig <hch@infradead.org> wrote:
> 
> > On Wed, Dec 21, 2016 at 09:50:49AM -0800, Stephen Hemminger wrote:
> > > Lastly, there is licensing issues on headers. It would be good to have any
> > > userspace ABI headers licensed with a more liberal license so that BSD
> and DPDK drivers
> > > could use them directly. Right now each one reinvents.
> >
> > Microsoft could easily solves this problem by offering a suitably
> > liberally licensed header documenting the full HyperV guest protocol
> > that Linux and other projects could use.
> 
> The issue is if same header file mixes kernel and userspace API stuff.
> 
> Once the files are arranged right, I will submit trivial change to comments
> to indicate the liberal licensing of userspace API headers.

Let us take this one step at a time. I know for a fact that not all the guest host
protocols on Hyper-V are guaranteed to be stable. Some of the protocols are part of
the published MSFT standards such RNDIS and these obviously are guaranteed to be
stable. For the rest it is less clear. The fact that we need to ensure compatibility of existing
Windows guests tells me that any host side changes will be versioned and the hosts will always
support older guests.

I would like to minimize what we include in the uapi header; especially when MSFT has made no guarantees
with regards how  they may be evolved. I will also work on getting some clarity on both stability and
under what license we would expose the uapi header.

Regards,

K. Y

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH 03/15] hyperv: use standard bitops
  2016-12-21 13:23     ` Roman Kagan
@ 2016-12-22 12:33       ` Paolo Bonzini
  0 siblings, 0 replies; 54+ messages in thread
From: Paolo Bonzini @ 2016-12-22 12:33 UTC (permalink / raw)
  To: Roman Kagan, Olaf Hering, Radim Krčmář,
	K. Y. Srinivasan, Vitaly Kuznetsov, Thomas Gleixner, Ingo Molnar,
	H. Peter Anvin, x86, Haiyang Zhang, kvm, linux-kernel, devel,
	Denis V . Lunev



On 21/12/2016 14:23, Roman Kagan wrote:
> On Wed, Dec 21, 2016 at 01:00:44PM +0100, Olaf Hering wrote:
>> On Tue, Dec 20, Roman Kagan wrote:
>>
>> Reverting commit 22356585712d ("staging: hv: use sync_bitops when
>> interacting with the hypervisor") is save because .......
>>
>>> -		sync_set_bit(channel->monitor_bit,
>>> +		set_bit(channel->monitor_bit,
> 
> It isn't indeed.  I didn't realize there was a UP case where it made a
> difference, and failed to locate the commit where it changed.
> 
> I'll drop this part, thanks.

Perhaps the sync_bitops should be renamed to virt_bitops.  This would
match virt_* memory barriers and would make their usage much more obvious.

Paolo

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH 00/15] hyperv: more stuff to uapi + cleanup
  2016-12-21 18:00 ` [PATCH 00/15] hyperv: more stuff to uapi + cleanup KY Srinivasan
@ 2016-12-28 16:57   ` Roman Kagan
  2016-12-30 19:45     ` KY Srinivasan
  0 siblings, 1 reply; 54+ messages in thread
From: Roman Kagan @ 2016-12-28 16:57 UTC (permalink / raw)
  To: KY Srinivasan
  Cc: Paolo Bonzini, Radim Krčmář,
	Vitaly Kuznetsov, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	x86, Haiyang Zhang, kvm, linux-kernel, devel, Denis V . Lunev

[ Sorry for such a slow reply; flu and office relocation knocked me out
for a while ]

On Wed, Dec 21, 2016 at 06:00:17PM +0000, KY Srinivasan wrote:
> > -----Original Message-----
> > From: Roman Kagan [mailto:rkagan@virtuozzo.com]
> > Sent: Tuesday, December 20, 2016 7:56 AM
> > Subject: [PATCH 00/15] hyperv: more stuff to uapi + cleanup
> > 
> > Expose more Hyper-V-related definitions in the uapi header for
> > consumption by userspace.
> > 
> > While doing so, get rid of a number of duplications between the KVM and
> > the guest driver code.  Also a few other cleanups are made which are not
> > strictly necessary for the main purpose of the series but appear
> > reasonable to do at the same time.
> > 
> > The most controversial is the last patch which modifies the stuff
> > already published in the uapi header, in the hope that no userspace
> > applications have started relying on it; I'm ok dropping it if this is
> > unacceptable.
> 
> First, let me thank you. Broadly, this patch-set can be broken into
> 1. Moving existing definitions around - (to make it possible to share these
> between Hyper-V guest drivers and KVM)
> 2. Cleanup of the existing code in the VMBUS driver (under drivers/hv).

Right.  Another significant part of the series is, where two sets of
definitions exist for the same entity, consolidate on the one that looks
more Linux-style, usually the one in the (currently) uapi header.

> To the extent possible, I want to take all non-KVM code through Greg's tree.
> We can then modify the KVM code to use these common definitions.

Well, this patchset touches almost no KVM code, so we're fine here I
think.

> Currently, I too am working on restructuring VMBUS driver code to
> fully isolate all x86 dependencies.  I can work with you on
> integration as I too am moving things around.

Great!  Do you want me to rebase on some your public tree?  (Once I
split the patches as you requested, of course)?

Thanks,
Roman.

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH 12/15] hyperv: move VMBus connection ids to uapi
  2016-12-21 19:54                       ` KY Srinivasan
@ 2016-12-28 17:09                         ` Roman Kagan
  2016-12-29 18:29                           ` Stephen Hemminger
                                             ` (2 more replies)
  0 siblings, 3 replies; 54+ messages in thread
From: Roman Kagan @ 2016-12-28 17:09 UTC (permalink / raw)
  To: KY Srinivasan
  Cc: Stephen Hemminger, Christoph Hellwig, Paolo Bonzini,
	Radim Krčmář,
	Vitaly Kuznetsov, kvm, Denis V . Lunev, Haiyang Zhang, x86,
	linux-kernel, Ingo Molnar, H. Peter Anvin, devel,
	Thomas Gleixner

On Wed, Dec 21, 2016 at 07:54:11PM +0000, KY Srinivasan wrote:
> 
> 
> > -----Original Message-----
> > From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> > Sent: Wednesday, December 21, 2016 10:03 AM
> > To: Christoph Hellwig <hch@infradead.org>
> > Cc: Paolo Bonzini <pbonzini@redhat.com>; Roman Kagan
> > <rkagan@virtuozzo.com>; Radim Krčmář <rkrcmar@redhat.com>; KY
> > Srinivasan <kys@microsoft.com>; Vitaly Kuznetsov
> > <vkuznets@redhat.com>; kvm@vger.kernel.org; Denis V . Lunev
> > <den@openvz.org>; Haiyang Zhang <haiyangz@microsoft.com>;
> > x86@kernel.org; linux-kernel@vger.kernel.org; Ingo Molnar
> > <mingo@redhat.com>; H. Peter Anvin <hpa@zytor.com>;
> > devel@linuxdriverproject.org; Thomas Gleixner <tglx@linutronix.de>
> > Subject: Re: [PATCH 12/15] hyperv: move VMBus connection ids to uapi
> > 
> > On Wed, 21 Dec 2016 09:58:36 -0800
> > Christoph Hellwig <hch@infradead.org> wrote:
> > 
> > > On Wed, Dec 21, 2016 at 09:50:49AM -0800, Stephen Hemminger wrote:
> > > > Lastly, there is licensing issues on headers. It would be good to have any
> > > > userspace ABI headers licensed with a more liberal license so that BSD
> > and DPDK drivers
> > > > could use them directly. Right now each one reinvents.
> > >
> > > Microsoft could easily solves this problem by offering a suitably
> > > liberally licensed header documenting the full HyperV guest protocol
> > > that Linux and other projects could use.
> > 
> > The issue is if same header file mixes kernel and userspace API stuff.
> > 
> > Once the files are arranged right, I will submit trivial change to comments
> > to indicate the liberal licensing of userspace API headers.
> 
> Let us take this one step at a time. I know for a fact that not all the guest host
> protocols on Hyper-V are guaranteed to be stable. Some of the protocols are part of
> the published MSFT standards such RNDIS and these obviously are guaranteed to be
> stable. For the rest it is less clear. The fact that we need to ensure compatibility of existing
> Windows guests tells me that any host side changes will be versioned and the hosts will always
> support older guests.
> 
> I would like to minimize what we include in the uapi header; especially when MSFT has made no guarantees
> with regards how  they may be evolved. I will also work on getting some clarity on both stability and
> under what license we would expose the uapi header.

Am I correct assuming that QEMU is currently the only user of
arch/x86/include/uapi/asm/hyperv.h?

Then I think we're fine withdrawing it from uapi as a whole and letting
QEMU pull it in through its header-harvesting scripts (as does now
anyway).  This would lift all licensing and longterm API stability
expectations.

Roman.

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH 12/15] hyperv: move VMBus connection ids to uapi
  2016-12-28 17:09                         ` Roman Kagan
@ 2016-12-29 18:29                           ` Stephen Hemminger
  2017-01-02  8:19                           ` Paolo Bonzini
  2017-01-02 19:39                           ` Stephen Hemminger
  2 siblings, 0 replies; 54+ messages in thread
From: Stephen Hemminger @ 2016-12-29 18:29 UTC (permalink / raw)
  To: Roman Kagan
  Cc: KY Srinivasan, Christoph Hellwig, Paolo Bonzini,
	Radim Krčmář,
	Vitaly Kuznetsov, kvm, Denis V . Lunev, Haiyang Zhang, x86,
	linux-kernel, Ingo Molnar, H. Peter Anvin, devel,
	Thomas Gleixner

On Wed, 28 Dec 2016 20:09:44 +0300
Roman Kagan <rkagan@virtuozzo.com> wrote:

> On Wed, Dec 21, 2016 at 07:54:11PM +0000, KY Srinivasan wrote:
> > 
> >   
> > > -----Original Message-----
> > > From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> > > Sent: Wednesday, December 21, 2016 10:03 AM
> > > To: Christoph Hellwig <hch@infradead.org>
> > > Cc: Paolo Bonzini <pbonzini@redhat.com>; Roman Kagan
> > > <rkagan@virtuozzo.com>; Radim Krčmář <rkrcmar@redhat.com>; KY
> > > Srinivasan <kys@microsoft.com>; Vitaly Kuznetsov
> > > <vkuznets@redhat.com>; kvm@vger.kernel.org; Denis V . Lunev
> > > <den@openvz.org>; Haiyang Zhang <haiyangz@microsoft.com>;
> > > x86@kernel.org; linux-kernel@vger.kernel.org; Ingo Molnar
> > > <mingo@redhat.com>; H. Peter Anvin <hpa@zytor.com>;
> > > devel@linuxdriverproject.org; Thomas Gleixner <tglx@linutronix.de>
> > > Subject: Re: [PATCH 12/15] hyperv: move VMBus connection ids to uapi
> > > 
> > > On Wed, 21 Dec 2016 09:58:36 -0800
> > > Christoph Hellwig <hch@infradead.org> wrote:
> > >   
> > > > On Wed, Dec 21, 2016 at 09:50:49AM -0800, Stephen Hemminger wrote:  
> > > > > Lastly, there is licensing issues on headers. It would be good to have any
> > > > > userspace ABI headers licensed with a more liberal license so that BSD  
> > > and DPDK drivers  
> > > > > could use them directly. Right now each one reinvents.  
> > > >
> > > > Microsoft could easily solves this problem by offering a suitably
> > > > liberally licensed header documenting the full HyperV guest protocol
> > > > that Linux and other projects could use.  
> > > 
> > > The issue is if same header file mixes kernel and userspace API stuff.
> > > 
> > > Once the files are arranged right, I will submit trivial change to comments
> > > to indicate the liberal licensing of userspace API headers.  
> > 
> > Let us take this one step at a time. I know for a fact that not all the guest host
> > protocols on Hyper-V are guaranteed to be stable. Some of the protocols are part of
> > the published MSFT standards such RNDIS and these obviously are guaranteed to be
> > stable. For the rest it is less clear. The fact that we need to ensure compatibility of existing
> > Windows guests tells me that any host side changes will be versioned and the hosts will always
> > support older guests.
> > 
> > I would like to minimize what we include in the uapi header; especially when MSFT has made no guarantees
> > with regards how  they may be evolved. I will also work on getting some clarity on both stability and
> > under what license we would expose the uapi header.  
> 
> Am I correct assuming that QEMU is currently the only user of
> arch/x86/include/uapi/asm/hyperv.h?
> 
> Then I think we're fine withdrawing it from uapi as a whole and letting
> QEMU pull it in through its header-harvesting scripts (as does now
> anyway).  This would lift all licensing and longterm API stability
> expectations.

That makes sense, but if it is not in uapi then any changes may break QEMU
in future (without regret).

If QEMU is diving in and extracting non UAPI headers then that is a bad idea.
It is outside the scope of this.

^ permalink raw reply	[flat|nested] 54+ messages in thread

* RE: [PATCH 00/15] hyperv: more stuff to uapi + cleanup
  2016-12-28 16:57   ` Roman Kagan
@ 2016-12-30 19:45     ` KY Srinivasan
  0 siblings, 0 replies; 54+ messages in thread
From: KY Srinivasan @ 2016-12-30 19:45 UTC (permalink / raw)
  To: Roman Kagan
  Cc: Paolo Bonzini, Radim Krčmář,
	Vitaly Kuznetsov, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	x86, Haiyang Zhang, kvm, linux-kernel, devel, Denis V . Lunev



> -----Original Message-----
> From: Roman Kagan [mailto:rkagan@virtuozzo.com]
> Sent: Wednesday, December 28, 2016 8:57 AM
> To: KY Srinivasan <kys@microsoft.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>; Radim Krčmář
> <rkrcmar@redhat.com>; Vitaly Kuznetsov <vkuznets@redhat.com>; Thomas
> Gleixner <tglx@linutronix.de>; Ingo Molnar <mingo@redhat.com>; H. Peter
> Anvin <hpa@zytor.com>; x86@kernel.org; Haiyang Zhang
> <haiyangz@microsoft.com>; kvm@vger.kernel.org; linux-
> kernel@vger.kernel.org; devel@linuxdriverproject.org; Denis V . Lunev
> <den@openvz.org>
> Subject: Re: [PATCH 00/15] hyperv: more stuff to uapi + cleanup
> 
> [ Sorry for such a slow reply; flu and office relocation knocked me out
> for a while ]
> 
> On Wed, Dec 21, 2016 at 06:00:17PM +0000, KY Srinivasan wrote:
> > > -----Original Message-----
> > > From: Roman Kagan [mailto:rkagan@virtuozzo.com]
> > > Sent: Tuesday, December 20, 2016 7:56 AM
> > > Subject: [PATCH 00/15] hyperv: more stuff to uapi + cleanup
> > >
> > > Expose more Hyper-V-related definitions in the uapi header for
> > > consumption by userspace.
> > >
> > > While doing so, get rid of a number of duplications between the KVM and
> > > the guest driver code.  Also a few other cleanups are made which are not
> > > strictly necessary for the main purpose of the series but appear
> > > reasonable to do at the same time.
> > >
> > > The most controversial is the last patch which modifies the stuff
> > > already published in the uapi header, in the hope that no userspace
> > > applications have started relying on it; I'm ok dropping it if this is
> > > unacceptable.
> >
> > First, let me thank you. Broadly, this patch-set can be broken into
> > 1. Moving existing definitions around - (to make it possible to share these
> > between Hyper-V guest drivers and KVM)
> > 2. Cleanup of the existing code in the VMBUS driver (under drivers/hv).
> 
> Right.  Another significant part of the series is, where two sets of
> definitions exist for the same entity, consolidate on the one that looks
> more Linux-style, usually the one in the (currently) uapi header.
> 
> > To the extent possible, I want to take all non-KVM code through Greg's
> tree.
> > We can then modify the KVM code to use these common definitions.
> 
> Well, this patchset touches almost no KVM code, so we're fine here I
> think.
> 
> > Currently, I too am working on restructuring VMBUS driver code to
> > fully isolate all x86 dependencies.  I can work with you on
> > integration as I too am moving things around.
> 
> Great!  Do you want me to rebase on some your public tree?  (Once I
> split the patches as you requested, of course)?

I just sent out the patches that I have been working on. These are based on
Greg's char-misc tree. I do have a bunch of patches in flight that have not been
checked in yet. Sorry, I don't have a public tree I can point you to.
I will work with you and coordinate our work here.

Happy New Year!

K. Y 

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH 12/15] hyperv: move VMBus connection ids to uapi
  2016-12-28 17:09                         ` Roman Kagan
  2016-12-29 18:29                           ` Stephen Hemminger
@ 2017-01-02  8:19                           ` Paolo Bonzini
  2017-01-09  8:32                             ` Roman Kagan
  2017-01-02 19:39                           ` Stephen Hemminger
  2 siblings, 1 reply; 54+ messages in thread
From: Paolo Bonzini @ 2017-01-02  8:19 UTC (permalink / raw)
  To: Roman Kagan, KY Srinivasan, Stephen Hemminger, Christoph Hellwig,
	Radim Krčmář,
	Vitaly Kuznetsov, kvm, Denis V . Lunev, Haiyang Zhang, x86,
	linux-kernel, Ingo Molnar, H. Peter Anvin, devel,
	Thomas Gleixner



On 28/12/2016 18:09, Roman Kagan wrote:
> Am I correct assuming that QEMU is currently the only user of
> arch/x86/include/uapi/asm/hyperv.h?
> 
> Then I think we're fine withdrawing it from uapi as a whole and letting
> QEMU pull it in through its header-harvesting scripts (as does now
> anyway).  This would lift all licensing and longterm API stability
> expectations.

Actually, QEMU's header-harvesting scripts use uapi/ headers
exclusively, since they are built on "make headers_install".

The extra cleanups that QEMU does on top are to allow compilation of the
headers on non-Linux machines.  They don't really do much more than
changing Linux (linux/types.h) integer types to the C99 (stdint.h)
equivalents.

Paolo

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH 12/15] hyperv: move VMBus connection ids to uapi
  2016-12-28 17:09                         ` Roman Kagan
  2016-12-29 18:29                           ` Stephen Hemminger
  2017-01-02  8:19                           ` Paolo Bonzini
@ 2017-01-02 19:39                           ` Stephen Hemminger
  2017-01-03  9:32                             ` Paolo Bonzini
  2 siblings, 1 reply; 54+ messages in thread
From: Stephen Hemminger @ 2017-01-02 19:39 UTC (permalink / raw)
  To: Roman Kagan
  Cc: KY Srinivasan, Christoph Hellwig, Paolo Bonzini,
	Radim Krčmář,
	Vitaly Kuznetsov, kvm, Denis V . Lunev, Haiyang Zhang, x86,
	linux-kernel, Ingo Molnar, H. Peter Anvin, devel,
	Thomas Gleixner

On Wed, 28 Dec 2016 20:09:44 +0300
Roman Kagan <rkagan@virtuozzo.com> wrote:

> On Wed, Dec 21, 2016 at 07:54:11PM +0000, KY Srinivasan wrote:
> > 
> >   
> > > -----Original Message-----
> > > From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> > > Sent: Wednesday, December 21, 2016 10:03 AM
> > > To: Christoph Hellwig <hch@infradead.org>
> > > Cc: Paolo Bonzini <pbonzini@redhat.com>; Roman Kagan
> > > <rkagan@virtuozzo.com>; Radim Krčmář <rkrcmar@redhat.com>; KY
> > > Srinivasan <kys@microsoft.com>; Vitaly Kuznetsov
> > > <vkuznets@redhat.com>; kvm@vger.kernel.org; Denis V . Lunev
> > > <den@openvz.org>; Haiyang Zhang <haiyangz@microsoft.com>;
> > > x86@kernel.org; linux-kernel@vger.kernel.org; Ingo Molnar
> > > <mingo@redhat.com>; H. Peter Anvin <hpa@zytor.com>;
> > > devel@linuxdriverproject.org; Thomas Gleixner <tglx@linutronix.de>
> > > Subject: Re: [PATCH 12/15] hyperv: move VMBus connection ids to uapi
> > > 
> > > On Wed, 21 Dec 2016 09:58:36 -0800
> > > Christoph Hellwig <hch@infradead.org> wrote:
> > >   
> > > > On Wed, Dec 21, 2016 at 09:50:49AM -0800, Stephen Hemminger wrote:  
> > > > > Lastly, there is licensing issues on headers. It would be good to have any
> > > > > userspace ABI headers licensed with a more liberal license so that BSD  
> > > and DPDK drivers  
> > > > > could use them directly. Right now each one reinvents.  
> > > >
> > > > Microsoft could easily solves this problem by offering a suitably
> > > > liberally licensed header documenting the full HyperV guest protocol
> > > > that Linux and other projects could use.  
> > > 
> > > The issue is if same header file mixes kernel and userspace API stuff.
> > > 
> > > Once the files are arranged right, I will submit trivial change to comments
> > > to indicate the liberal licensing of userspace API headers.  
> > 
> > Let us take this one step at a time. I know for a fact that not all the guest host
> > protocols on Hyper-V are guaranteed to be stable. Some of the protocols are part of
> > the published MSFT standards such RNDIS and these obviously are guaranteed to be
> > stable. For the rest it is less clear. The fact that we need to ensure compatibility of existing
> > Windows guests tells me that any host side changes will be versioned and the hosts will always
> > support older guests.
> > 
> > I would like to minimize what we include in the uapi header; especially when MSFT has made no guarantees
> > with regards how  they may be evolved. I will also work on getting some clarity on both stability and
> > under what license we would expose the uapi header.  
> 
> Am I correct assuming that QEMU is currently the only user of
> arch/x86/include/uapi/asm/hyperv.h?
> 
> Then I think we're fine withdrawing it from uapi as a whole and letting
> QEMU pull it in through its header-harvesting scripts (as does now
> anyway).  This would lift all licensing and longterm API stability
> expectations.
> 
> Roman.

Thanks, that prevents lots of problems.
That is how I handle iproute2 as well.

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH 12/15] hyperv: move VMBus connection ids to uapi
  2017-01-02 19:39                           ` Stephen Hemminger
@ 2017-01-03  9:32                             ` Paolo Bonzini
  0 siblings, 0 replies; 54+ messages in thread
From: Paolo Bonzini @ 2017-01-03  9:32 UTC (permalink / raw)
  To: Stephen Hemminger, Roman Kagan
  Cc: KY Srinivasan, Christoph Hellwig, Radim Krčmář,
	Vitaly Kuznetsov, kvm, Denis V . Lunev, Haiyang Zhang, x86,
	linux-kernel, Ingo Molnar, H. Peter Anvin, devel,
	Thomas Gleixner



On 02/01/2017 20:39, Stephen Hemminger wrote:
>>>
>>> I would like to minimize what we include in the uapi header; especially when MSFT has made no guarantees
>>> with regards how  they may be evolved. I will also work on getting some clarity on both stability and
>>> under what license we would expose the uapi header.  
>> Am I correct assuming that QEMU is currently the only user of
>> arch/x86/include/uapi/asm/hyperv.h?
>>
>> Then I think we're fine withdrawing it from uapi as a whole and letting
>> QEMU pull it in through its header-harvesting scripts (as does now
>> anyway).  This would lift all licensing and longterm API stability
>> expectations.
>
> Thanks, that prevents lots of problems.
> That is how I handle iproute2 as well.

Except it wouldn't work.  But no big deal, I guess we'll just
synchronize hyperv.h manually. :((

Paolo

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH 12/15] hyperv: move VMBus connection ids to uapi
  2017-01-02  8:19                           ` Paolo Bonzini
@ 2017-01-09  8:32                             ` Roman Kagan
  2017-01-09  8:40                               ` hpa
  0 siblings, 1 reply; 54+ messages in thread
From: Roman Kagan @ 2017-01-09  8:32 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: KY Srinivasan, Stephen Hemminger, Christoph Hellwig,
	Radim Krčmář,
	Vitaly Kuznetsov, kvm, Denis V . Lunev, Haiyang Zhang, x86,
	linux-kernel, Ingo Molnar, H. Peter Anvin, devel,
	Thomas Gleixner

On Mon, Jan 02, 2017 at 09:19:57AM +0100, Paolo Bonzini wrote:
> On 28/12/2016 18:09, Roman Kagan wrote:
> > Am I correct assuming that QEMU is currently the only user of
> > arch/x86/include/uapi/asm/hyperv.h?
> > 
> > Then I think we're fine withdrawing it from uapi as a whole and letting
> > QEMU pull it in through its header-harvesting scripts (as does now
> > anyway).  This would lift all licensing and longterm API stability
> > expectations.
> 
> Actually, QEMU's header-harvesting scripts use uapi/ headers
> exclusively, since they are built on "make headers_install".
> 
> The extra cleanups that QEMU does on top are to allow compilation of the
> headers on non-Linux machines.  They don't really do much more than
> changing Linux (linux/types.h) integer types to the C99 (stdint.h)
> equivalents.

Ouch, I stand corrected.

So what should we do with it then?  I'm sorta lost...

We certainly can give it up and live with a private copy of the
definitions in the QEMU tree but that doesn't sound optimal in any
sense.

Thanks,
Roman.

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH 12/15] hyperv: move VMBus connection ids to uapi
  2017-01-09  8:32                             ` Roman Kagan
@ 2017-01-09  8:40                               ` hpa
  2017-01-09  8:58                                 ` Roman Kagan
  2017-01-09  9:02                                 ` Paolo Bonzini
  0 siblings, 2 replies; 54+ messages in thread
From: hpa @ 2017-01-09  8:40 UTC (permalink / raw)
  To: Roman Kagan, Paolo Bonzini
  Cc: KY Srinivasan, Stephen Hemminger, Christoph Hellwig,
	Radim Krčmář,
	Vitaly Kuznetsov, kvm, Denis V . Lunev, Haiyang Zhang, x86,
	linux-kernel, Ingo Molnar, devel, Thomas Gleixner

On January 9, 2017 12:32:23 AM PST, Roman Kagan <rkagan@virtuozzo.com> wrote:
>On Mon, Jan 02, 2017 at 09:19:57AM +0100, Paolo Bonzini wrote:
>> On 28/12/2016 18:09, Roman Kagan wrote:
>> > Am I correct assuming that QEMU is currently the only user of
>> > arch/x86/include/uapi/asm/hyperv.h?
>> > 
>> > Then I think we're fine withdrawing it from uapi as a whole and
>letting
>> > QEMU pull it in through its header-harvesting scripts (as does now
>> > anyway).  This would lift all licensing and longterm API stability
>> > expectations.
>> 
>> Actually, QEMU's header-harvesting scripts use uapi/ headers
>> exclusively, since they are built on "make headers_install".
>> 
>> The extra cleanups that QEMU does on top are to allow compilation of
>the
>> headers on non-Linux machines.  They don't really do much more than
>> changing Linux (linux/types.h) integer types to the C99 (stdint.h)
>> equivalents.
>
>Ouch, I stand corrected.
>
>So what should we do with it then?  I'm sorta lost...
>
>We certainly can give it up and live with a private copy of the
>definitions in the QEMU tree but that doesn't sound optimal in any
>sense.
>
>Thanks,
>Roman.

Why do that through header mangling rather than typedef?
-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH 12/15] hyperv: move VMBus connection ids to uapi
  2017-01-09  8:40                               ` hpa
@ 2017-01-09  8:58                                 ` Roman Kagan
  2017-01-09  9:02                                 ` Paolo Bonzini
  1 sibling, 0 replies; 54+ messages in thread
From: Roman Kagan @ 2017-01-09  8:58 UTC (permalink / raw)
  To: hpa
  Cc: Paolo Bonzini, KY Srinivasan, Stephen Hemminger,
	Christoph Hellwig, Radim Krčmář,
	Vitaly Kuznetsov, kvm, Denis V . Lunev, Haiyang Zhang, x86,
	linux-kernel, Ingo Molnar, devel, Thomas Gleixner

On Mon, Jan 09, 2017 at 12:40:48AM -0800, hpa@zytor.com wrote:
> On January 9, 2017 12:32:23 AM PST, Roman Kagan <rkagan@virtuozzo.com> wrote:
> >On Mon, Jan 02, 2017 at 09:19:57AM +0100, Paolo Bonzini wrote:
> >> On 28/12/2016 18:09, Roman Kagan wrote:
> >> > Am I correct assuming that QEMU is currently the only user of
> >> > arch/x86/include/uapi/asm/hyperv.h?
> >> > 
> >> > Then I think we're fine withdrawing it from uapi as a whole and
> >letting
> >> > QEMU pull it in through its header-harvesting scripts (as does now
> >> > anyway).  This would lift all licensing and longterm API stability
> >> > expectations.
> >> 
> >> Actually, QEMU's header-harvesting scripts use uapi/ headers
> >> exclusively, since they are built on "make headers_install".
> >> 
> >> The extra cleanups that QEMU does on top are to allow compilation of
> >the
> >> headers on non-Linux machines.  They don't really do much more than
> >> changing Linux (linux/types.h) integer types to the C99 (stdint.h)
> >> equivalents.
> >
> >Ouch, I stand corrected.
> >
> >So what should we do with it then?  I'm sorta lost...
> >
> >We certainly can give it up and live with a private copy of the
> >definitions in the QEMU tree but that doesn't sound optimal in any
> >sense.
> >
> >Thanks,
> >Roman.
> 
> Why do that through header mangling rather than typedef?

Sorry for not being clear, I actually asked what to do with the Hyper-V
and VMBus protocol definitions.

The typedef vs mangling is a different matter; I guess mangling was
chosen to avoid conflicts with system-provided definitions on non-Linux
systems, but I think Paolo can tell more.

Roman.

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH 12/15] hyperv: move VMBus connection ids to uapi
  2017-01-09  8:40                               ` hpa
  2017-01-09  8:58                                 ` Roman Kagan
@ 2017-01-09  9:02                                 ` Paolo Bonzini
  1 sibling, 0 replies; 54+ messages in thread
From: Paolo Bonzini @ 2017-01-09  9:02 UTC (permalink / raw)
  To: hpa, Roman Kagan
  Cc: KY Srinivasan, Stephen Hemminger, Christoph Hellwig,
	Radim Krčmář,
	Vitaly Kuznetsov, kvm, Denis V . Lunev, Haiyang Zhang, x86,
	linux-kernel, Ingo Molnar, devel, Thomas Gleixner



On 09/01/2017 09:40, hpa@zytor.com wrote:
> On January 9, 2017 12:32:23 AM PST, Roman Kagan <rkagan@virtuozzo.com> wrote:
>> On Mon, Jan 02, 2017 at 09:19:57AM +0100, Paolo Bonzini wrote:
>>> On 28/12/2016 18:09, Roman Kagan wrote:
>>>> Am I correct assuming that QEMU is currently the only user of
>>>> arch/x86/include/uapi/asm/hyperv.h?
>>>>
>>>> Then I think we're fine withdrawing it from uapi as a whole and
>> letting
>>>> QEMU pull it in through its header-harvesting scripts (as does now
>>>> anyway).  This would lift all licensing and longterm API stability
>>>> expectations.
>>>
>>> Actually, QEMU's header-harvesting scripts use uapi/ headers
>>> exclusively, since they are built on "make headers_install".
>>>
>>> The extra cleanups that QEMU does on top are to allow compilation of
>> the
>>> headers on non-Linux machines.  They don't really do much more than
>>> changing Linux (linux/types.h) integer types to the C99 (stdint.h)
>>> equivalents.
>>
>> Ouch, I stand corrected.
>>
>> So what should we do with it then?  I'm sorta lost...
>>
>> We certainly can give it up and live with a private copy of the
>> definitions in the QEMU tree but that doesn't sound optimal in any
>> sense.
> 
> Why do that through header mangling rather than typedef?

Because you are not suppose to typedef identifiers that start with "__",
and because it does do a few other ad-hoc changes:

        -e 's/<linux\/\([^>]*\)>/"standard-headers\/linux\/\1"/' \
        -e 's/__bitwise__//' \
        -e 's/__attribute__((packed))/QEMU_PACKED/' \
        -e 's/__inline__/inline/' \
        -e '/sys\/ioctl.h/d' \
        -e 's/SW_MAX/SW_MAX_/' \

Paolo

^ permalink raw reply	[flat|nested] 54+ messages in thread

end of thread, other threads:[~2017-01-09  9:02 UTC | newest]

Thread overview: 54+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-12-20 15:55 [PATCH 00/15] hyperv: more stuff to uapi + cleanup Roman Kagan
2016-12-20 15:55 ` [PATCH 01/15] hyperv: consolidate TSC ref page definitions Roman Kagan
2016-12-20 20:57   ` KY Srinivasan
2016-12-21  6:25     ` Roman Kagan
2016-12-20 15:55 ` [PATCH 02/15] hyperv: uapi-fy synic event flags definitions Roman Kagan
2016-12-20 17:24   ` Stephen Hemminger
2016-12-21  6:33     ` Roman Kagan
2016-12-21 10:58   ` kbuild test robot
2016-12-21 18:58   ` KY Srinivasan
2016-12-20 15:55 ` [PATCH 03/15] hyperv: use standard bitops Roman Kagan
2016-12-21 12:00   ` Olaf Hering
2016-12-21 13:23     ` Roman Kagan
2016-12-22 12:33       ` Paolo Bonzini
2016-12-21 19:08   ` KY Srinivasan
2016-12-20 15:55 ` [PATCH 04/15] hyperv: define VMBus message type Roman Kagan
2016-12-20 15:55 ` [PATCH 05/15] hyperv: GFP_ATOMIC -> GFP_KERNEL Roman Kagan
2016-12-20 15:55 ` [PATCH 06/15] hyperv: avoid unnecessary vmalloc Roman Kagan
2016-12-21 19:19   ` KY Srinivasan
2016-12-20 15:55 ` [PATCH 07/15] hyperv: dedup cpuid definitions Roman Kagan
2016-12-20 15:55 ` [PATCH 08/15] hyperv: dedup crash msr related definitions Roman Kagan
2016-12-20 15:55 ` [PATCH 09/15] hyperv: unify Hyper-V msr definitions Roman Kagan
2016-12-20 15:55 ` [PATCH 10/15] hyperv: uapi-fy PostMessage and SignalEvent hypercall structures Roman Kagan
2016-12-21 19:27   ` KY Srinivasan
2016-12-20 15:55 ` [PATCH 11/15] hyperv: uapi-fy monitored notification structures Roman Kagan
2016-12-20 15:55 ` [PATCH 12/15] hyperv: move VMBus connection ids to uapi Roman Kagan
2016-12-20 17:25   ` Stephen Hemminger
2016-12-21  6:29     ` Roman Kagan
2016-12-21 12:18       ` Christoph Hellwig
2016-12-21 12:59         ` Roman Kagan
2016-12-21 14:26           ` Christoph Hellwig
2016-12-21 14:39             ` Roman Kagan
2016-12-21 15:39             ` Paolo Bonzini
2016-12-21 15:43               ` Christoph Hellwig
2016-12-21 17:25                 ` Paolo Bonzini
2016-12-21 17:50                 ` Stephen Hemminger
2016-12-21 17:53                   ` Paolo Bonzini
2016-12-21 17:58                   ` Christoph Hellwig
2016-12-21 18:02                     ` Stephen Hemminger
2016-12-21 19:54                       ` KY Srinivasan
2016-12-28 17:09                         ` Roman Kagan
2016-12-29 18:29                           ` Stephen Hemminger
2017-01-02  8:19                           ` Paolo Bonzini
2017-01-09  8:32                             ` Roman Kagan
2017-01-09  8:40                               ` hpa
2017-01-09  8:58                                 ` Roman Kagan
2017-01-09  9:02                                 ` Paolo Bonzini
2017-01-02 19:39                           ` Stephen Hemminger
2017-01-03  9:32                             ` Paolo Bonzini
2016-12-20 15:56 ` [PATCH 13/15] hyperv: move function close to its only callsite Roman Kagan
2016-12-20 15:56 ` [PATCH 14/15] hyperv_vmbus: drop unused definitions Roman Kagan
2016-12-20 15:56 ` [PATCH 15/15] hyperv: redefine hv_message without bitfields Roman Kagan
2016-12-21 18:00 ` [PATCH 00/15] hyperv: more stuff to uapi + cleanup KY Srinivasan
2016-12-28 16:57   ` Roman Kagan
2016-12-30 19:45     ` KY Srinivasan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).