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
Userspace will need them too.
Signed-off-by: Roman Kagan <[email protected]>
---
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
None of these is used in the kernel.
Signed-off-by: Roman Kagan <[email protected]>
---
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
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 <[email protected]>
---
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
Signed-off-by: Roman Kagan <[email protected]>
---
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
On Tue, 20 Dec 2016 18:55:49 +0300
Roman Kagan <[email protected]> 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 <[email protected]>
> ---
> 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.
On Tue, 20 Dec 2016 18:55:59 +0300
Roman Kagan <[email protected]> wrote:
> Userspace will need them too.
>
> Signed-off-by: Roman Kagan <[email protected]>
What userspace? I am worried about creating more stable API's that can't change.
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 <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
Signed-off-by: Roman Kagan <[email protected]>
---
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
Use the definitions already present in the uapi header throughout the
guest driver, too.
Signed-off-by: Roman Kagan <[email protected]>
---
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
Give a name to the constant (1) already used in the code.
Signed-off-by: Roman Kagan <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
Use the definitions already present in the uapi header throughout the
guest driver, too.
Signed-off-by: Roman Kagan <[email protected]>
---
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
> -----Original Message-----
> From: Roman Kagan [mailto:[email protected]]
> Sent: Tuesday, December 20, 2016 7:56 AM
> To: Paolo Bonzini <[email protected]>; Radim Kr?m??
> <[email protected]>; KY Srinivasan <[email protected]>; Vitaly
> Kuznetsov <[email protected]>
> Cc: Thomas Gleixner <[email protected]>; Ingo Molnar
> <[email protected]>; H. Peter Anvin <[email protected]>; [email protected];
> Haiyang Zhang <[email protected]>; [email protected]; linux-
> [email protected]; [email protected]; Denis V . Lunev
> <[email protected]>; Roman Kagan <[email protected]>
> 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 <[email protected]>
> ---
> 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
On Tue, Dec 20, 2016 at 08:57:28PM +0000, KY Srinivasan wrote:
>
>
> > -----Original Message-----
> > From: Roman Kagan [mailto:[email protected]]
> > Sent: Tuesday, December 20, 2016 7:56 AM
> > To: Paolo Bonzini <[email protected]>; Radim Krčmář
> > <[email protected]>; KY Srinivasan <[email protected]>; Vitaly
> > Kuznetsov <[email protected]>
> > Cc: Thomas Gleixner <[email protected]>; Ingo Molnar
> > <[email protected]>; H. Peter Anvin <[email protected]>; [email protected];
> > Haiyang Zhang <[email protected]>; [email protected]; linux-
> > [email protected]; [email protected]; Denis V . Lunev
> > <[email protected]>; Roman Kagan <[email protected]>
> > 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.
On Tue, Dec 20, 2016 at 09:25:43AM -0800, Stephen Hemminger wrote:
> On Tue, 20 Dec 2016 18:55:59 +0300
> Roman Kagan <[email protected]> wrote:
>
> > Userspace will need them too.
> >
> > Signed-off-by: Roman Kagan <[email protected]>
>
> 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.
On Tue, Dec 20, 2016 at 09:24:53AM -0800, Stephen Hemminger wrote:
> On Tue, 20 Dec 2016 18:55:49 +0300
> Roman Kagan <[email protected]> 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 <[email protected]>
> > ---
> > 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.
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 <[email protected]>
:::::: CC: Greg Kroah-Hartman <[email protected]>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
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
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.
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.
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.
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.
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.
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
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.
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
On Wed, 21 Dec 2016 07:43:48 -0800
Christoph Hellwig <[email protected]> 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.
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
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.
> -----Original Message-----
> From: Roman Kagan [mailto:[email protected]]
> Sent: Tuesday, December 20, 2016 7:56 AM
> To: Paolo Bonzini <[email protected]>; Radim Kr?m??
> <[email protected]>; KY Srinivasan <[email protected]>; Vitaly
> Kuznetsov <[email protected]>
> Cc: Thomas Gleixner <[email protected]>; Ingo Molnar
> <[email protected]>; H. Peter Anvin <[email protected]>; [email protected];
> Haiyang Zhang <[email protected]>; [email protected]; linux-
> [email protected]; [email protected]; Denis V . Lunev
> <[email protected]>; Roman Kagan <[email protected]>
> 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
On Wed, 21 Dec 2016 09:58:36 -0800
Christoph Hellwig <[email protected]> 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.
> -----Original Message-----
> From: Roman Kagan [mailto:[email protected]]
> Sent: Tuesday, December 20, 2016 7:56 AM
> To: Paolo Bonzini <[email protected]>; Radim Kr?m??
> <[email protected]>; KY Srinivasan <[email protected]>; Vitaly
> Kuznetsov <[email protected]>
> Cc: Thomas Gleixner <[email protected]>; Ingo Molnar
> <[email protected]>; H. Peter Anvin <[email protected]>; [email protected];
> Haiyang Zhang <[email protected]>; [email protected]; linux-
> [email protected]; [email protected]; Denis V . Lunev
> <[email protected]>; Roman Kagan <[email protected]>
> 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 <[email protected]>
> ---
> 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
> -----Original Message-----
> From: Roman Kagan [mailto:[email protected]]
> Sent: Tuesday, December 20, 2016 7:56 AM
> To: Paolo Bonzini <[email protected]>; Radim Kr?m??
> <[email protected]>; KY Srinivasan <[email protected]>; Vitaly
> Kuznetsov <[email protected]>
> Cc: Thomas Gleixner <[email protected]>; Ingo Molnar
> <[email protected]>; H. Peter Anvin <[email protected]>; [email protected];
> Haiyang Zhang <[email protected]>; [email protected]; linux-
> [email protected]; [email protected]; Denis V . Lunev
> <[email protected]>; Roman Kagan <[email protected]>
> 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 <[email protected]>
> ---
> 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
> -----Original Message-----
> From: Roman Kagan [mailto:[email protected]]
> Sent: Tuesday, December 20, 2016 7:56 AM
> To: Paolo Bonzini <[email protected]>; Radim Kr?m??
> <[email protected]>; KY Srinivasan <[email protected]>; Vitaly
> Kuznetsov <[email protected]>
> Cc: Thomas Gleixner <[email protected]>; Ingo Molnar
> <[email protected]>; H. Peter Anvin <[email protected]>; [email protected];
> Haiyang Zhang <[email protected]>; [email protected]; linux-
> [email protected]; [email protected]; Denis V . Lunev
> <[email protected]>; Roman Kagan <[email protected]>
> Subject: [PATCH 03/15] hyperv: use standard bitops
>
No commit log?
> Signed-off-by: Roman Kagan <[email protected]>
> ---
> 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
> -----Original Message-----
> From: Roman Kagan [mailto:[email protected]]
> Sent: Tuesday, December 20, 2016 7:56 AM
> To: Paolo Bonzini <[email protected]>; Radim Kr?m??
> <[email protected]>; KY Srinivasan <[email protected]>; Vitaly
> Kuznetsov <[email protected]>
> Cc: Thomas Gleixner <[email protected]>; Ingo Molnar
> <[email protected]>; H. Peter Anvin <[email protected]>; [email protected];
> Haiyang Zhang <[email protected]>; [email protected]; linux-
> [email protected]; [email protected]; Denis V . Lunev
> <[email protected]>; Roman Kagan <[email protected]>
> 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 <[email protected]>
> ---
> 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
> -----Original Message-----
> From: Stephen Hemminger [mailto:[email protected]]
> Sent: Wednesday, December 21, 2016 10:03 AM
> To: Christoph Hellwig <[email protected]>
> Cc: Paolo Bonzini <[email protected]>; Roman Kagan
> <[email protected]>; Radim Kr?m?? <[email protected]>; KY
> Srinivasan <[email protected]>; Vitaly Kuznetsov
> <[email protected]>; [email protected]; Denis V . Lunev
> <[email protected]>; Haiyang Zhang <[email protected]>;
> [email protected]; [email protected]; Ingo Molnar
> <[email protected]>; H. Peter Anvin <[email protected]>;
> [email protected]; Thomas Gleixner <[email protected]>
> Subject: Re: [PATCH 12/15] hyperv: move VMBus connection ids to uapi
>
> On Wed, 21 Dec 2016 09:58:36 -0800
> Christoph Hellwig <[email protected]> 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
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
On Wed, Dec 21, 2016 at 07:54:11PM +0000, KY Srinivasan wrote:
>
>
> > -----Original Message-----
> > From: Stephen Hemminger [mailto:[email protected]]
> > Sent: Wednesday, December 21, 2016 10:03 AM
> > To: Christoph Hellwig <[email protected]>
> > Cc: Paolo Bonzini <[email protected]>; Roman Kagan
> > <[email protected]>; Radim Krčmář <[email protected]>; KY
> > Srinivasan <[email protected]>; Vitaly Kuznetsov
> > <[email protected]>; [email protected]; Denis V . Lunev
> > <[email protected]>; Haiyang Zhang <[email protected]>;
> > [email protected]; [email protected]; Ingo Molnar
> > <[email protected]>; H. Peter Anvin <[email protected]>;
> > [email protected]; Thomas Gleixner <[email protected]>
> > Subject: Re: [PATCH 12/15] hyperv: move VMBus connection ids to uapi
> >
> > On Wed, 21 Dec 2016 09:58:36 -0800
> > Christoph Hellwig <[email protected]> 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.
[ 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:[email protected]]
> > 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.
On Wed, 28 Dec 2016 20:09:44 +0300
Roman Kagan <[email protected]> wrote:
> On Wed, Dec 21, 2016 at 07:54:11PM +0000, KY Srinivasan wrote:
> >
> >
> > > -----Original Message-----
> > > From: Stephen Hemminger [mailto:[email protected]]
> > > Sent: Wednesday, December 21, 2016 10:03 AM
> > > To: Christoph Hellwig <[email protected]>
> > > Cc: Paolo Bonzini <[email protected]>; Roman Kagan
> > > <[email protected]>; Radim Krčmář <[email protected]>; KY
> > > Srinivasan <[email protected]>; Vitaly Kuznetsov
> > > <[email protected]>; [email protected]; Denis V . Lunev
> > > <[email protected]>; Haiyang Zhang <[email protected]>;
> > > [email protected]; [email protected]; Ingo Molnar
> > > <[email protected]>; H. Peter Anvin <[email protected]>;
> > > [email protected]; Thomas Gleixner <[email protected]>
> > > Subject: Re: [PATCH 12/15] hyperv: move VMBus connection ids to uapi
> > >
> > > On Wed, 21 Dec 2016 09:58:36 -0800
> > > Christoph Hellwig <[email protected]> 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.
> -----Original Message-----
> From: Roman Kagan [mailto:[email protected]]
> Sent: Wednesday, December 28, 2016 8:57 AM
> To: KY Srinivasan <[email protected]>
> Cc: Paolo Bonzini <[email protected]>; Radim Kr?m??
> <[email protected]>; Vitaly Kuznetsov <[email protected]>; Thomas
> Gleixner <[email protected]>; Ingo Molnar <[email protected]>; H. Peter
> Anvin <[email protected]>; [email protected]; Haiyang Zhang
> <[email protected]>; [email protected]; linux-
> [email protected]; [email protected]; Denis V . Lunev
> <[email protected]>
> 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:[email protected]]
> > > 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