[2/2] PCI: hv: handle all pending messages in hv_pci_onchannelcallback()
diff mbox

Message ID 1464617879-19581-3-git-send-email-vkuznets@redhat.com
State Accepted
Headers show

Commit Message

Vitaly Kuznetsov May 30, 2016, 2:17 p.m. UTC
When we have an interrupt from host we have a bit set in event page
indicating there are messages for the particular channel. We need to read
them all as we won't get signaled for what was on the queue before we
cleared the bit in vmbus_on_event(). This applies to all Hyper-V drivers
and the pass-through driver should do the same.
I did non meet any bugs, the issue was found by code inspection. We don't
have many events going through hv_pci_onchannelcallback(), this explains
why nobody reported the issue before.

While on it, fix handling non-zero vmbus_recvpacket_raw() return values by
dropping out. If the return value is not zero it is wrong to inspect
buffer or bytes_recvd as these may contain invalid data.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
 drivers/pci/host/pci-hyperv.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

Comments

Jake Oshins May 31, 2016, 5:36 p.m. UTC | #1
> -----Original Message-----
> From: Vitaly Kuznetsov [mailto:vkuznets@redhat.com]
> Sent: Monday, May 30, 2016 7:18 AM
> To: linux-pci@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org; devel@linuxdriverproject.org; Bjorn
> Helgaas <bhelgaas@google.com>; Haiyang Zhang
> <haiyangz@microsoft.com>; KY Srinivasan <kys@microsoft.com>; Jake
> Oshins <jakeo@microsoft.com>
> Subject: [PATCH 2/2] PCI: hv: handle all pending messages in
> hv_pci_onchannelcallback()
> 
> When we have an interrupt from host we have a bit set in event page
> indicating there are messages for the particular channel. We need to read
> them all as we won't get signaled for what was on the queue before we
> cleared the bit in vmbus_on_event(). This applies to all Hyper-V drivers
> and the pass-through driver should do the same.
> I did non meet any bugs, the issue was found by code inspection. We don't
> have many events going through hv_pci_onchannelcallback(), this explains
> why nobody reported the issue before.
> 
> While on it, fix handling non-zero vmbus_recvpacket_raw() return values by
> dropping out. If the return value is not zero it is wrong to inspect
> buffer or bytes_recvd as these may contain invalid data.
> 
> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Acked-by: Jake Oshins <jakeo@microsoft.com>

> ---
>  drivers/pci/host/pci-hyperv.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/host/pci-hyperv.c b/drivers/pci/host/pci-hyperv.c
> index a68ec49..7de341d 100644
> --- a/drivers/pci/host/pci-hyperv.c
> +++ b/drivers/pci/host/pci-hyperv.c
> @@ -1657,12 +1657,16 @@ static void hv_pci_onchannelcallback(void
> *context)
>  			continue;
>  		}
> 
> +		/* Zero length indicates there are no more packets. */
> +		if (ret || !bytes_recvd)
> +			break;
> +
>  		/*
>  		 * All incoming packets must be at least as large as a
>  		 * response.
>  		 */
>  		if (bytes_recvd <= sizeof(struct pci_response))
> -			break;
> +			continue;
>  		desc = (struct vmpacket_descriptor *)buffer;
> 
>  		switch (desc->type) {
> @@ -1724,7 +1728,6 @@ static void hv_pci_onchannelcallback(void
> *context)
>  				desc->type, req_id, bytes_recvd);
>  			break;
>  		}
> -		break;
>  	}
> 
>  	kfree(buffer);
> --
> 2.5.5

This is good, too.

Thanks,
Jake Oshins

--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch
diff mbox

diff --git a/drivers/pci/host/pci-hyperv.c b/drivers/pci/host/pci-hyperv.c
index a68ec49..7de341d 100644
--- a/drivers/pci/host/pci-hyperv.c
+++ b/drivers/pci/host/pci-hyperv.c
@@ -1657,12 +1657,16 @@  static void hv_pci_onchannelcallback(void *context)
 			continue;
 		}
 
+		/* Zero length indicates there are no more packets. */
+		if (ret || !bytes_recvd)
+			break;
+
 		/*
 		 * All incoming packets must be at least as large as a
 		 * response.
 		 */
 		if (bytes_recvd <= sizeof(struct pci_response))
-			break;
+			continue;
 		desc = (struct vmpacket_descriptor *)buffer;
 
 		switch (desc->type) {
@@ -1724,7 +1728,6 @@  static void hv_pci_onchannelcallback(void *context)
 				desc->type, req_id, bytes_recvd);
 			break;
 		}
-		break;
 	}
 
 	kfree(buffer);