This is a note to let you know that I've just added the patch titled Drivers: hv: vmbus: On write cleanup the logic to interrupt the host to the 4.9-stable tree which can be found at: http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary The filename of the patch is: drivers-hv-vmbus-on-write-cleanup-the-logic-to-interrupt-the-host.patch and it can be found in the queue-4.9 subdirectory. If you, or anyone else, feels it should not be added to the stable tree, please let <stable@xxxxxxxxxxxxxxx> know about it. >From 1f6ee4e7d83586c8b10bd4f2f4346353d04ce884 Mon Sep 17 00:00:00 2001 From: "K. Y. Srinivasan" <kys@xxxxxxxxxxxxx> Date: Sun, 6 Nov 2016 13:14:17 -0800 Subject: Drivers: hv: vmbus: On write cleanup the logic to interrupt the host From: K. Y. Srinivasan <kys@xxxxxxxxxxxxx> commit 1f6ee4e7d83586c8b10bd4f2f4346353d04ce884 upstream. Signal the host when we determine the host is to be signaled. The currrent code determines the need to signal in the ringbuffer code and actually issues the signal elsewhere. This can result in the host viewing this interrupt as spurious since the host may also poll the channel. Make the necessary adjustments. Signed-off-by: K. Y. Srinivasan <kys@xxxxxxxxxxxxx> Cc: Rolf Neugebauer <rolf.neugebauer@xxxxxxxxxx> Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> --- drivers/hv/channel.c | 99 ++++------------------------------------------ drivers/hv/hyperv_vmbus.h | 6 +- drivers/hv/ring_buffer.c | 30 +++++++++---- include/linux/hyperv.h | 1 4 files changed, 35 insertions(+), 101 deletions(-) --- a/drivers/hv/channel.c +++ b/drivers/hv/channel.c @@ -39,7 +39,7 @@ * vmbus_setevent- Trigger an event notification on the specified * channel. */ -static void vmbus_setevent(struct vmbus_channel *channel) +void vmbus_setevent(struct vmbus_channel *channel) { struct hv_monitor_page *monitorpage; @@ -65,6 +65,7 @@ static void vmbus_setevent(struct vmbus_ vmbus_set_event(channel); } } +EXPORT_SYMBOL_GPL(vmbus_setevent); /* * vmbus_open - Open the specified channel. @@ -635,8 +636,6 @@ int vmbus_sendpacket_ctl(struct vmbus_ch u32 packetlen_aligned = ALIGN(packetlen, sizeof(u64)); struct kvec bufferlist[3]; u64 aligned_data = 0; - int ret; - bool signal = false; bool lock = channel->acquire_ring_lock; int num_vecs = ((bufferlen != 0) ? 3 : 1); @@ -656,41 +655,9 @@ int vmbus_sendpacket_ctl(struct vmbus_ch bufferlist[2].iov_base = &aligned_data; bufferlist[2].iov_len = (packetlen_aligned - packetlen); - ret = hv_ringbuffer_write(&channel->outbound, bufferlist, num_vecs, - &signal, lock, channel->signal_policy); - - /* - * Signalling the host is conditional on many factors: - * 1. The ring state changed from being empty to non-empty. - * This is tracked by the variable "signal". - * 2. The variable kick_q tracks if more data will be placed - * on the ring. We will not signal if more data is - * to be placed. - * - * Based on the channel signal state, we will decide - * which signaling policy will be applied. - * - * If we cannot write to the ring-buffer; signal the host - * even if we may not have written anything. This is a rare - * enough condition that it should not matter. - * NOTE: in this case, the hvsock channel is an exception, because - * it looks the host side's hvsock implementation has a throttling - * mechanism which can hurt the performance otherwise. - * - * KYS: Oct. 30, 2016: - * It looks like Windows hosts have logic to deal with DOS attacks that - * can be triggered if it receives interrupts when it is not expecting - * the interrupt. The host expects interrupts only when the ring - * transitions from empty to non-empty (or full to non full on the guest - * to host ring). - * So, base the signaling decision solely on the ring state until the - * host logic is fixed. - */ - - if (((ret == 0) && signal)) - vmbus_setevent(channel); + return hv_ringbuffer_write(channel, bufferlist, num_vecs, + lock, kick_q); - return ret; } EXPORT_SYMBOL(vmbus_sendpacket_ctl); @@ -731,7 +698,6 @@ int vmbus_sendpacket_pagebuffer_ctl(stru u32 flags, bool kick_q) { - int ret; int i; struct vmbus_channel_packet_page_buffer desc; u32 descsize; @@ -739,7 +705,6 @@ int vmbus_sendpacket_pagebuffer_ctl(stru u32 packetlen_aligned; struct kvec bufferlist[3]; u64 aligned_data = 0; - bool signal = false; bool lock = channel->acquire_ring_lock; if (pagecount > MAX_PAGE_BUFFER_COUNT) @@ -777,38 +742,8 @@ int vmbus_sendpacket_pagebuffer_ctl(stru bufferlist[2].iov_base = &aligned_data; bufferlist[2].iov_len = (packetlen_aligned - packetlen); - ret = hv_ringbuffer_write(&channel->outbound, bufferlist, 3, - &signal, lock, channel->signal_policy); - - /* - * Signalling the host is conditional on many factors: - * 1. The ring state changed from being empty to non-empty. - * This is tracked by the variable "signal". - * 2. The variable kick_q tracks if more data will be placed - * on the ring. We will not signal if more data is - * to be placed. - * - * Based on the channel signal state, we will decide - * which signaling policy will be applied. - * - * If we cannot write to the ring-buffer; signal the host - * even if we may not have written anything. This is a rare - * enough condition that it should not matter. - * - * KYS: Oct. 30, 2016: - * It looks like Windows hosts have logic to deal with DOS attacks that - * can be triggered if it receives interrupts when it is not expecting - * the interrupt. The host expects interrupts only when the ring - * transitions from empty to non-empty (or full to non full on the guest - * to host ring). - * So, base the signaling decision solely on the ring state until the - * host logic is fixed. - */ - - if (((ret == 0) && signal)) - vmbus_setevent(channel); - - return ret; + return hv_ringbuffer_write(channel, bufferlist, 3, + lock, kick_q); } EXPORT_SYMBOL_GPL(vmbus_sendpacket_pagebuffer_ctl); @@ -839,12 +774,10 @@ int vmbus_sendpacket_mpb_desc(struct vmb u32 desc_size, void *buffer, u32 bufferlen, u64 requestid) { - int ret; u32 packetlen; u32 packetlen_aligned; struct kvec bufferlist[3]; u64 aligned_data = 0; - bool signal = false; bool lock = channel->acquire_ring_lock; packetlen = desc_size + bufferlen; @@ -865,13 +798,8 @@ int vmbus_sendpacket_mpb_desc(struct vmb bufferlist[2].iov_base = &aligned_data; bufferlist[2].iov_len = (packetlen_aligned - packetlen); - ret = hv_ringbuffer_write(&channel->outbound, bufferlist, 3, - &signal, lock, channel->signal_policy); - - if (ret == 0 && signal) - vmbus_setevent(channel); - - return ret; + return hv_ringbuffer_write(channel, bufferlist, 3, + lock, true); } EXPORT_SYMBOL_GPL(vmbus_sendpacket_mpb_desc); @@ -883,14 +811,12 @@ int vmbus_sendpacket_multipagebuffer(str struct hv_multipage_buffer *multi_pagebuffer, void *buffer, u32 bufferlen, u64 requestid) { - int ret; struct vmbus_channel_packet_multipage_buffer desc; u32 descsize; u32 packetlen; u32 packetlen_aligned; struct kvec bufferlist[3]; u64 aligned_data = 0; - bool signal = false; bool lock = channel->acquire_ring_lock; u32 pfncount = NUM_PAGES_SPANNED(multi_pagebuffer->offset, multi_pagebuffer->len); @@ -930,13 +856,8 @@ int vmbus_sendpacket_multipagebuffer(str bufferlist[2].iov_base = &aligned_data; bufferlist[2].iov_len = (packetlen_aligned - packetlen); - ret = hv_ringbuffer_write(&channel->outbound, bufferlist, 3, - &signal, lock, channel->signal_policy); - - if (ret == 0 && signal) - vmbus_setevent(channel); - - return ret; + return hv_ringbuffer_write(channel, bufferlist, 3, + lock, true); } EXPORT_SYMBOL_GPL(vmbus_sendpacket_multipagebuffer); --- a/drivers/hv/hyperv_vmbus.h +++ b/drivers/hv/hyperv_vmbus.h @@ -527,10 +527,10 @@ int hv_ringbuffer_init(struct hv_ring_bu void hv_ringbuffer_cleanup(struct hv_ring_buffer_info *ring_info); -int hv_ringbuffer_write(struct hv_ring_buffer_info *ring_info, +int hv_ringbuffer_write(struct vmbus_channel *channel, struct kvec *kv_list, - u32 kv_count, bool *signal, bool lock, - enum hv_signal_policy policy); + u32 kv_count, bool lock, + bool kick_q); int hv_ringbuffer_read(struct hv_ring_buffer_info *inring_info, void *buffer, u32 buflen, u32 *buffer_actual_len, --- a/drivers/hv/ring_buffer.c +++ b/drivers/hv/ring_buffer.c @@ -66,14 +66,25 @@ u32 hv_end_read(struct hv_ring_buffer_in * once the ring buffer is empty, it will clear the * interrupt_mask and re-check to see if new data has * arrived. + * + * KYS: Oct. 30, 2016: + * It looks like Windows hosts have logic to deal with DOS attacks that + * can be triggered if it receives interrupts when it is not expecting + * the interrupt. The host expects interrupts only when the ring + * transitions from empty to non-empty (or full to non full on the guest + * to host ring). + * So, base the signaling decision solely on the ring state until the + * host logic is fixed. */ -static bool hv_need_to_signal(u32 old_write, struct hv_ring_buffer_info *rbi, - enum hv_signal_policy policy) +static void hv_signal_on_write(u32 old_write, struct vmbus_channel *channel, + bool kick_q) { + struct hv_ring_buffer_info *rbi = &channel->outbound; + virt_mb(); if (READ_ONCE(rbi->ring_buffer->interrupt_mask)) - return false; + return; /* check interrupt_mask before read_index */ virt_rmb(); @@ -82,9 +93,9 @@ static bool hv_need_to_signal(u32 old_wr * ring transitions from being empty to non-empty. */ if (old_write == READ_ONCE(rbi->ring_buffer->read_index)) - return true; + vmbus_setevent(channel); - return false; + return; } /* Get the next write location for the specified ring buffer. */ @@ -273,9 +284,9 @@ void hv_ringbuffer_cleanup(struct hv_rin } /* Write to the ring buffer. */ -int hv_ringbuffer_write(struct hv_ring_buffer_info *outring_info, - struct kvec *kv_list, u32 kv_count, bool *signal, bool lock, - enum hv_signal_policy policy) +int hv_ringbuffer_write(struct vmbus_channel *channel, + struct kvec *kv_list, u32 kv_count, bool lock, + bool kick_q) { int i = 0; u32 bytes_avail_towrite; @@ -285,6 +296,7 @@ int hv_ringbuffer_write(struct hv_ring_b u32 old_write; u64 prev_indices = 0; unsigned long flags = 0; + struct hv_ring_buffer_info *outring_info = &channel->outbound; for (i = 0; i < kv_count; i++) totalbytes_towrite += kv_list[i].iov_len; @@ -337,7 +349,7 @@ int hv_ringbuffer_write(struct hv_ring_b if (lock) spin_unlock_irqrestore(&outring_info->ring_lock, flags); - *signal = hv_need_to_signal(old_write, outring_info, policy); + hv_signal_on_write(old_write, channel, kick_q); return 0; } --- a/include/linux/hyperv.h +++ b/include/linux/hyperv.h @@ -1447,6 +1447,7 @@ void hv_event_tasklet_enable(struct vmbu void hv_process_channel_removal(struct vmbus_channel *channel, u32 relid); +void vmbus_setevent(struct vmbus_channel *channel); /* * Negotiated version with the Host. */ Patches currently in stable-queue which might be from kys@xxxxxxxxxxxxx are queue-4.9/drivers-hv-vmbus-on-the-read-path-cleanup-the-logic-to-interrupt-the-host.patch queue-4.9/drivers-hv-vmbus-finally-fix-hv_need_to_signal_on_read.patch queue-4.9/drivers-hv-vmbus-base-host-signaling-strictly-on-the-ring-state.patch queue-4.9/drivers-hv-vmbus-on-write-cleanup-the-logic-to-interrupt-the-host.patch -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html