diff mbox series

[RFC,1/12] x86/Hyper-V: Add visibility parameter for vmbus_establish_gpadl()

Message ID 20210228150315.2552437-2-ltykernel@gmail.com
State New
Headers show
Series x86/Hyper-V: Add Hyper-V Isolation VM support | expand

Commit Message

Tianyu Lan Feb. 28, 2021, 3:03 p.m. UTC
From: Tianyu Lan <Tianyu.Lan@microsoft.com>

Add visibility parameter for vmbus_establish_gpadl() and prepare
to change host visibility when create gpadl for buffer.

Signed-off-by: Sunil Muthuswamy <sunilmut@microsoft.com>
Co-Developed-by: Sunil Muthuswamy <sunilmut@microsoft.com>
Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com>
---
 arch/x86/include/asm/hyperv-tlfs.h |  9 +++++++++
 drivers/hv/channel.c               | 20 +++++++++++---------
 drivers/net/hyperv/netvsc.c        |  8 ++++++--
 drivers/uio/uio_hv_generic.c       |  7 +++++--
 include/linux/hyperv.h             |  3 ++-
 5 files changed, 33 insertions(+), 14 deletions(-)

Comments

Vitaly Kuznetsov March 3, 2021, 4:27 p.m. UTC | #1
Tianyu Lan <ltykernel@gmail.com> writes:

> From: Tianyu Lan <Tianyu.Lan@microsoft.com>
>
> Add visibility parameter for vmbus_establish_gpadl() and prepare
> to change host visibility when create gpadl for buffer.
>

"No functional change" as you don't actually use the parameter.

> Signed-off-by: Sunil Muthuswamy <sunilmut@microsoft.com>
> Co-Developed-by: Sunil Muthuswamy <sunilmut@microsoft.com>
> Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com>

Nit: Sunil's SoB looks misleading because the patch is from you,
Co-Developed-by should be sufficient.

> ---
>  arch/x86/include/asm/hyperv-tlfs.h |  9 +++++++++
>  drivers/hv/channel.c               | 20 +++++++++++---------
>  drivers/net/hyperv/netvsc.c        |  8 ++++++--
>  drivers/uio/uio_hv_generic.c       |  7 +++++--
>  include/linux/hyperv.h             |  3 ++-
>  5 files changed, 33 insertions(+), 14 deletions(-)
>
> diff --git a/arch/x86/include/asm/hyperv-tlfs.h b/arch/x86/include/asm/hyperv-tlfs.h
> index e6cd3fee562b..fb1893a4c32b 100644
> --- a/arch/x86/include/asm/hyperv-tlfs.h
> +++ b/arch/x86/include/asm/hyperv-tlfs.h
> @@ -236,6 +236,15 @@ enum hv_isolation_type {
>  /* TSC invariant control */
>  #define HV_X64_MSR_TSC_INVARIANT_CONTROL	0x40000118
>  
> +/* Hyper-V GPA map flags */
> +#define HV_MAP_GPA_PERMISSIONS_NONE		0x0
> +#define HV_MAP_GPA_READABLE			0x1
> +#define HV_MAP_GPA_WRITABLE			0x2
> +
> +#define VMBUS_PAGE_VISIBLE_READ_ONLY HV_MAP_GPA_READABLE
> +#define VMBUS_PAGE_VISIBLE_READ_WRITE (HV_MAP_GPA_READABLE|HV_MAP_GPA_WRITABLE)
> +#define VMBUS_PAGE_NOT_VISIBLE HV_MAP_GPA_PERMISSIONS_NONE
> +

Are these x86-only? If not, then we should probably move these defines
to include/asm-generic/hyperv-tlfs.h. In case they are, we should do
something as we're using them from arch neutral places.

Also, could you please add a comment stating that these flags define
host's visibility of a page and not guest's (this seems to be not
obvious at least to me).

>  /*
>   * Declare the MSR used to setup pages used to communicate with the hypervisor.
>   */
> diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
> index 0bd202de7960..daa21cc72beb 100644
> --- a/drivers/hv/channel.c
> +++ b/drivers/hv/channel.c
> @@ -242,7 +242,7 @@ EXPORT_SYMBOL_GPL(vmbus_send_modifychannel);
>   */
>  static int create_gpadl_header(enum hv_gpadl_type type, void *kbuffer,
>  			       u32 size, u32 send_offset,
> -			       struct vmbus_channel_msginfo **msginfo)
> +			       struct vmbus_channel_msginfo **msginfo, u32 visibility)
>  {
>  	int i;
>  	int pagecount;
> @@ -391,7 +391,7 @@ static int create_gpadl_header(enum hv_gpadl_type type, void *kbuffer,
>  static int __vmbus_establish_gpadl(struct vmbus_channel *channel,
>  				   enum hv_gpadl_type type, void *kbuffer,
>  				   u32 size, u32 send_offset,
> -				   u32 *gpadl_handle)
> +				   u32 *gpadl_handle, u32 visibility)
>  {
>  	struct vmbus_channel_gpadl_header *gpadlmsg;
>  	struct vmbus_channel_gpadl_body *gpadl_body;
> @@ -405,7 +405,8 @@ static int __vmbus_establish_gpadl(struct vmbus_channel *channel,
>  	next_gpadl_handle =
>  		(atomic_inc_return(&vmbus_connection.next_gpadl_handle) - 1);
>  
> -	ret = create_gpadl_header(type, kbuffer, size, send_offset, &msginfo);
> +	ret = create_gpadl_header(type, kbuffer, size, send_offset,
> +				  &msginfo, visibility);
>  	if (ret)
>  		return ret;
>  
> @@ -496,10 +497,10 @@ static int __vmbus_establish_gpadl(struct vmbus_channel *channel,
>   * @gpadl_handle: some funky thing
>   */
>  int vmbus_establish_gpadl(struct vmbus_channel *channel, void *kbuffer,
> -			  u32 size, u32 *gpadl_handle)
> +			  u32 size, u32 *gpadl_handle, u32 visibility)
>  {
>  	return __vmbus_establish_gpadl(channel, HV_GPADL_BUFFER, kbuffer, size,
> -				       0U, gpadl_handle);
> +				       0U, gpadl_handle, visibility);
>  }
>  EXPORT_SYMBOL_GPL(vmbus_establish_gpadl);
>  
> @@ -610,10 +611,11 @@ static int __vmbus_open(struct vmbus_channel *newchannel,
>  	newchannel->ringbuffer_gpadlhandle = 0;
>  
>  	err = __vmbus_establish_gpadl(newchannel, HV_GPADL_RING,
> -				      page_address(newchannel->ringbuffer_page),
> -				      (send_pages + recv_pages) << PAGE_SHIFT,
> -				      newchannel->ringbuffer_send_offset << PAGE_SHIFT,
> -				      &newchannel->ringbuffer_gpadlhandle);
> +			page_address(newchannel->ringbuffer_page),
> +			(send_pages + recv_pages) << PAGE_SHIFT,
> +			newchannel->ringbuffer_send_offset << PAGE_SHIFT,
> +			&newchannel->ringbuffer_gpadlhandle,
> +			VMBUS_PAGE_VISIBLE_READ_WRITE);

Nit: I liked the original alignment more and we can avoid the unneeded
code churn.

>  	if (err)
>  		goto error_clean_ring;
>  
> diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
> index 2353623259f3..bb72c7578330 100644
> --- a/drivers/net/hyperv/netvsc.c
> +++ b/drivers/net/hyperv/netvsc.c
> @@ -333,7 +333,8 @@ static int netvsc_init_buf(struct hv_device *device,
>  	 */
>  	ret = vmbus_establish_gpadl(device->channel, net_device->recv_buf,
>  				    buf_size,
> -				    &net_device->recv_buf_gpadl_handle);
> +				    &net_device->recv_buf_gpadl_handle,
> +				    VMBUS_PAGE_VISIBLE_READ_WRITE);
>  	if (ret != 0) {
>  		netdev_err(ndev,
>  			"unable to establish receive buffer's gpadl\n");
> @@ -422,10 +423,13 @@ static int netvsc_init_buf(struct hv_device *device,
>  	/* Establish the gpadl handle for this buffer on this
>  	 * channel.  Note: This call uses the vmbus connection rather
>  	 * than the channel to establish the gpadl handle.
> +	 * Send buffer should theoretically be only marked as "read-only", but
> +	 * the netvsp for some reason needs write capabilities on it.
>  	 */
>  	ret = vmbus_establish_gpadl(device->channel, net_device->send_buf,
>  				    buf_size,
> -				    &net_device->send_buf_gpadl_handle);
> +				    &net_device->send_buf_gpadl_handle,
> +				    VMBUS_PAGE_VISIBLE_READ_WRITE);
>  	if (ret != 0) {
>  		netdev_err(ndev,
>  			   "unable to establish send buffer's gpadl\n");
> diff --git a/drivers/uio/uio_hv_generic.c b/drivers/uio/uio_hv_generic.c
> index 0330ba99730e..813a7bee5139 100644
> --- a/drivers/uio/uio_hv_generic.c
> +++ b/drivers/uio/uio_hv_generic.c
> @@ -29,6 +29,7 @@
>  #include <linux/hyperv.h>
>  #include <linux/vmalloc.h>
>  #include <linux/slab.h>
> +#include <asm/mshyperv.h>
>  
>  #include "../hv/hyperv_vmbus.h"
>  
> @@ -295,7 +296,8 @@ hv_uio_probe(struct hv_device *dev,
>  	}
>  
>  	ret = vmbus_establish_gpadl(channel, pdata->recv_buf,
> -				    RECV_BUFFER_SIZE, &pdata->recv_gpadl);
> +				    RECV_BUFFER_SIZE, &pdata->recv_gpadl,
> +				    VMBUS_PAGE_VISIBLE_READ_WRITE);
>  	if (ret)
>  		goto fail_close;
>  
> @@ -315,7 +317,8 @@ hv_uio_probe(struct hv_device *dev,
>  	}
>  
>  	ret = vmbus_establish_gpadl(channel, pdata->send_buf,
> -				    SEND_BUFFER_SIZE, &pdata->send_gpadl);
> +				    SEND_BUFFER_SIZE, &pdata->send_gpadl,
> +				    VMBUS_PAGE_VISIBLE_READ_ONLY);

Actually, this is the only place where you use 'READ_ONLY' mapping --
which makes me wonder if it's actually worth it or we can hard-code
VMBUS_PAGE_VISIBLE_READ_WRITE for now and avoid this additional
parameter.

>  	if (ret)
>  		goto fail_close;
>  
> diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
> index f1d74dcf0353..016fdca20d6e 100644
> --- a/include/linux/hyperv.h
> +++ b/include/linux/hyperv.h
> @@ -1179,7 +1179,8 @@ extern int vmbus_sendpacket_mpb_desc(struct vmbus_channel *channel,
>  extern int vmbus_establish_gpadl(struct vmbus_channel *channel,
>  				      void *kbuffer,
>  				      u32 size,
> -				      u32 *gpadl_handle);
> +				      u32 *gpadl_handle,
> +				      u32 visibility);
>  
>  extern int vmbus_teardown_gpadl(struct vmbus_channel *channel,
>  				     u32 gpadl_handle);
Tianyu Lan March 5, 2021, 6:06 a.m. UTC | #2
Hi Vitaly:
      Thanks for your review.

On 3/4/2021 12:27 AM, Vitaly Kuznetsov wrote:
> Tianyu Lan <ltykernel@gmail.com> writes:

> 

>> From: Tianyu Lan <Tianyu.Lan@microsoft.com>

>>

>> Add visibility parameter for vmbus_establish_gpadl() and prepare

>> to change host visibility when create gpadl for buffer.

>>

> 

> "No functional change" as you don't actually use the parameter.


Yes, will add it into commit log.

> 

>> Signed-off-by: Sunil Muthuswamy <sunilmut@microsoft.com>

>> Co-Developed-by: Sunil Muthuswamy <sunilmut@microsoft.com>

>> Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com>

> 

> Nit: Sunil's SoB looks misleading because the patch is from you,

> Co-Developed-by should be sufficient.

> 


Will update.

>> ---

>>   arch/x86/include/asm/hyperv-tlfs.h |  9 +++++++++

>>   drivers/hv/channel.c               | 20 +++++++++++---------

>>   drivers/net/hyperv/netvsc.c        |  8 ++++++--

>>   drivers/uio/uio_hv_generic.c       |  7 +++++--

>>   include/linux/hyperv.h             |  3 ++-

>>   5 files changed, 33 insertions(+), 14 deletions(-)

>>

>> diff --git a/arch/x86/include/asm/hyperv-tlfs.h b/arch/x86/include/asm/hyperv-tlfs.h

>> index e6cd3fee562b..fb1893a4c32b 100644

>> --- a/arch/x86/include/asm/hyperv-tlfs.h

>> +++ b/arch/x86/include/asm/hyperv-tlfs.h

>> @@ -236,6 +236,15 @@ enum hv_isolation_type {

>>   /* TSC invariant control */

>>   #define HV_X64_MSR_TSC_INVARIANT_CONTROL	0x40000118

>>   

>> +/* Hyper-V GPA map flags */

>> +#define HV_MAP_GPA_PERMISSIONS_NONE		0x0

>> +#define HV_MAP_GPA_READABLE			0x1

>> +#define HV_MAP_GPA_WRITABLE			0x2

>> +

>> +#define VMBUS_PAGE_VISIBLE_READ_ONLY HV_MAP_GPA_READABLE

>> +#define VMBUS_PAGE_VISIBLE_READ_WRITE (HV_MAP_GPA_READABLE|HV_MAP_GPA_WRITABLE)

>> +#define VMBUS_PAGE_NOT_VISIBLE HV_MAP_GPA_PERMISSIONS_NONE

>> +

> 

> Are these x86-only? If not, then we should probably move these defines

> to include/asm-generic/hyperv-tlfs.h. In case they are, we should do

> something as we're using them from arch neutral places.

> 

> Also, could you please add a comment stating that these flags define

> host's visibility of a page and not guest's (this seems to be not

> obvious at least to me).

>





>>   /*

>>    * Declare the MSR used to setup pages used to communicate with the hypervisor.

>>    */

>> diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c

>> index 0bd202de7960..daa21cc72beb 100644

>> --- a/drivers/hv/channel.c

>> +++ b/drivers/hv/channel.c

>> @@ -242,7 +242,7 @@ EXPORT_SYMBOL_GPL(vmbus_send_modifychannel);

>>    */

>>   static int create_gpadl_header(enum hv_gpadl_type type, void *kbuffer,

>>   			       u32 size, u32 send_offset,

>> -			       struct vmbus_channel_msginfo **msginfo)

>> +			       struct vmbus_channel_msginfo **msginfo, u32 visibility)

>>   {

>>   	int i;

>>   	int pagecount;

>> @@ -391,7 +391,7 @@ static int create_gpadl_header(enum hv_gpadl_type type, void *kbuffer,

>>   static int __vmbus_establish_gpadl(struct vmbus_channel *channel,

>>   				   enum hv_gpadl_type type, void *kbuffer,

>>   				   u32 size, u32 send_offset,

>> -				   u32 *gpadl_handle)

>> +				   u32 *gpadl_handle, u32 visibility)

>>   {

>>   	struct vmbus_channel_gpadl_header *gpadlmsg;

>>   	struct vmbus_channel_gpadl_body *gpadl_body;

>> @@ -405,7 +405,8 @@ static int __vmbus_establish_gpadl(struct vmbus_channel *channel,

>>   	next_gpadl_handle =

>>   		(atomic_inc_return(&vmbus_connection.next_gpadl_handle) - 1);

>>   

>> -	ret = create_gpadl_header(type, kbuffer, size, send_offset, &msginfo);

>> +	ret = create_gpadl_header(type, kbuffer, size, send_offset,

>> +				  &msginfo, visibility);

>>   	if (ret)

>>   		return ret;

>>   

>> @@ -496,10 +497,10 @@ static int __vmbus_establish_gpadl(struct vmbus_channel *channel,

>>    * @gpadl_handle: some funky thing

>>    */

>>   int vmbus_establish_gpadl(struct vmbus_channel *channel, void *kbuffer,

>> -			  u32 size, u32 *gpadl_handle)

>> +			  u32 size, u32 *gpadl_handle, u32 visibility)

>>   {

>>   	return __vmbus_establish_gpadl(channel, HV_GPADL_BUFFER, kbuffer, size,

>> -				       0U, gpadl_handle);

>> +				       0U, gpadl_handle, visibility);

>>   }

>>   EXPORT_SYMBOL_GPL(vmbus_establish_gpadl);

>>   

>> @@ -610,10 +611,11 @@ static int __vmbus_open(struct vmbus_channel *newchannel,

>>   	newchannel->ringbuffer_gpadlhandle = 0;

>>   

>>   	err = __vmbus_establish_gpadl(newchannel, HV_GPADL_RING,

>> -				      page_address(newchannel->ringbuffer_page),

>> -				      (send_pages + recv_pages) << PAGE_SHIFT,

>> -				      newchannel->ringbuffer_send_offset << PAGE_SHIFT,

>> -				      &newchannel->ringbuffer_gpadlhandle);

>> +			page_address(newchannel->ringbuffer_page),

>> +			(send_pages + recv_pages) << PAGE_SHIFT,

>> +			newchannel->ringbuffer_send_offset << PAGE_SHIFT,

>> +			&newchannel->ringbuffer_gpadlhandle,

>> +			VMBUS_PAGE_VISIBLE_READ_WRITE);

> 

> Nit: I liked the original alignment more and we can avoid the unneeded

> code churn.

> 

>>   	if (err)

>>   		goto error_clean_ring;

>>   

>> diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c

>> index 2353623259f3..bb72c7578330 100644

>> --- a/drivers/net/hyperv/netvsc.c

>> +++ b/drivers/net/hyperv/netvsc.c

>> @@ -333,7 +333,8 @@ static int netvsc_init_buf(struct hv_device *device,

>>   	 */

>>   	ret = vmbus_establish_gpadl(device->channel, net_device->recv_buf,

>>   				    buf_size,

>> -				    &net_device->recv_buf_gpadl_handle);

>> +				    &net_device->recv_buf_gpadl_handle,

>> +				    VMBUS_PAGE_VISIBLE_READ_WRITE);

>>   	if (ret != 0) {

>>   		netdev_err(ndev,

>>   			"unable to establish receive buffer's gpadl\n");

>> @@ -422,10 +423,13 @@ static int netvsc_init_buf(struct hv_device *device,

>>   	/* Establish the gpadl handle for this buffer on this

>>   	 * channel.  Note: This call uses the vmbus connection rather

>>   	 * than the channel to establish the gpadl handle.

>> +	 * Send buffer should theoretically be only marked as "read-only", but

>> +	 * the netvsp for some reason needs write capabilities on it.

>>   	 */

>>   	ret = vmbus_establish_gpadl(device->channel, net_device->send_buf,

>>   				    buf_size,

>> -				    &net_device->send_buf_gpadl_handle);

>> +				    &net_device->send_buf_gpadl_handle,

>> +				    VMBUS_PAGE_VISIBLE_READ_WRITE);

>>   	if (ret != 0) {

>>   		netdev_err(ndev,

>>   			   "unable to establish send buffer's gpadl\n");

>> diff --git a/drivers/uio/uio_hv_generic.c b/drivers/uio/uio_hv_generic.c

>> index 0330ba99730e..813a7bee5139 100644

>> --- a/drivers/uio/uio_hv_generic.c

>> +++ b/drivers/uio/uio_hv_generic.c

>> @@ -29,6 +29,7 @@

>>   #include <linux/hyperv.h>

>>   #include <linux/vmalloc.h>

>>   #include <linux/slab.h>

>> +#include <asm/mshyperv.h>

>>   

>>   #include "../hv/hyperv_vmbus.h"

>>   

>> @@ -295,7 +296,8 @@ hv_uio_probe(struct hv_device *dev,

>>   	}

>>   

>>   	ret = vmbus_establish_gpadl(channel, pdata->recv_buf,

>> -				    RECV_BUFFER_SIZE, &pdata->recv_gpadl);

>> +				    RECV_BUFFER_SIZE, &pdata->recv_gpadl,

>> +				    VMBUS_PAGE_VISIBLE_READ_WRITE);

>>   	if (ret)

>>   		goto fail_close;

>>   

>> @@ -315,7 +317,8 @@ hv_uio_probe(struct hv_device *dev,

>>   	}

>>   

>>   	ret = vmbus_establish_gpadl(channel, pdata->send_buf,

>> -				    SEND_BUFFER_SIZE, &pdata->send_gpadl);

>> +				    SEND_BUFFER_SIZE, &pdata->send_gpadl,

>> +				    VMBUS_PAGE_VISIBLE_READ_ONLY);

> 

> Actually, this is the only place where you use 'READ_ONLY' mapping --

> which makes me wonder if it's actually worth it or we can hard-code

> VMBUS_PAGE_VISIBLE_READ_WRITE for now and avoid this additional

> parameter.

> 


Another option is to set host visibility out of vmbus_establish_gpadl(). 
There are three places calling vmbus_establish_gpadl(). Vmbus, netvsc 
and uio drivers.

>>   	if (ret)

>>   		goto fail_close;

>>   

>> diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h

>> index f1d74dcf0353..016fdca20d6e 100644

>> --- a/include/linux/hyperv.h

>> +++ b/include/linux/hyperv.h

>> @@ -1179,7 +1179,8 @@ extern int vmbus_sendpacket_mpb_desc(struct vmbus_channel *channel,

>>   extern int vmbus_establish_gpadl(struct vmbus_channel *channel,

>>   				      void *kbuffer,

>>   				      u32 size,

>> -				      u32 *gpadl_handle);

>> +				      u32 *gpadl_handle,

>> +				      u32 visibility);

>>   

>>   extern int vmbus_teardown_gpadl(struct vmbus_channel *channel,

>>   				     u32 gpadl_handle);

>
diff mbox series

Patch

diff --git a/arch/x86/include/asm/hyperv-tlfs.h b/arch/x86/include/asm/hyperv-tlfs.h
index e6cd3fee562b..fb1893a4c32b 100644
--- a/arch/x86/include/asm/hyperv-tlfs.h
+++ b/arch/x86/include/asm/hyperv-tlfs.h
@@ -236,6 +236,15 @@  enum hv_isolation_type {
 /* TSC invariant control */
 #define HV_X64_MSR_TSC_INVARIANT_CONTROL	0x40000118
 
+/* Hyper-V GPA map flags */
+#define HV_MAP_GPA_PERMISSIONS_NONE		0x0
+#define HV_MAP_GPA_READABLE			0x1
+#define HV_MAP_GPA_WRITABLE			0x2
+
+#define VMBUS_PAGE_VISIBLE_READ_ONLY HV_MAP_GPA_READABLE
+#define VMBUS_PAGE_VISIBLE_READ_WRITE (HV_MAP_GPA_READABLE|HV_MAP_GPA_WRITABLE)
+#define VMBUS_PAGE_NOT_VISIBLE HV_MAP_GPA_PERMISSIONS_NONE
+
 /*
  * Declare the MSR used to setup pages used to communicate with the hypervisor.
  */
diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
index 0bd202de7960..daa21cc72beb 100644
--- a/drivers/hv/channel.c
+++ b/drivers/hv/channel.c
@@ -242,7 +242,7 @@  EXPORT_SYMBOL_GPL(vmbus_send_modifychannel);
  */
 static int create_gpadl_header(enum hv_gpadl_type type, void *kbuffer,
 			       u32 size, u32 send_offset,
-			       struct vmbus_channel_msginfo **msginfo)
+			       struct vmbus_channel_msginfo **msginfo, u32 visibility)
 {
 	int i;
 	int pagecount;
@@ -391,7 +391,7 @@  static int create_gpadl_header(enum hv_gpadl_type type, void *kbuffer,
 static int __vmbus_establish_gpadl(struct vmbus_channel *channel,
 				   enum hv_gpadl_type type, void *kbuffer,
 				   u32 size, u32 send_offset,
-				   u32 *gpadl_handle)
+				   u32 *gpadl_handle, u32 visibility)
 {
 	struct vmbus_channel_gpadl_header *gpadlmsg;
 	struct vmbus_channel_gpadl_body *gpadl_body;
@@ -405,7 +405,8 @@  static int __vmbus_establish_gpadl(struct vmbus_channel *channel,
 	next_gpadl_handle =
 		(atomic_inc_return(&vmbus_connection.next_gpadl_handle) - 1);
 
-	ret = create_gpadl_header(type, kbuffer, size, send_offset, &msginfo);
+	ret = create_gpadl_header(type, kbuffer, size, send_offset,
+				  &msginfo, visibility);
 	if (ret)
 		return ret;
 
@@ -496,10 +497,10 @@  static int __vmbus_establish_gpadl(struct vmbus_channel *channel,
  * @gpadl_handle: some funky thing
  */
 int vmbus_establish_gpadl(struct vmbus_channel *channel, void *kbuffer,
-			  u32 size, u32 *gpadl_handle)
+			  u32 size, u32 *gpadl_handle, u32 visibility)
 {
 	return __vmbus_establish_gpadl(channel, HV_GPADL_BUFFER, kbuffer, size,
-				       0U, gpadl_handle);
+				       0U, gpadl_handle, visibility);
 }
 EXPORT_SYMBOL_GPL(vmbus_establish_gpadl);
 
@@ -610,10 +611,11 @@  static int __vmbus_open(struct vmbus_channel *newchannel,
 	newchannel->ringbuffer_gpadlhandle = 0;
 
 	err = __vmbus_establish_gpadl(newchannel, HV_GPADL_RING,
-				      page_address(newchannel->ringbuffer_page),
-				      (send_pages + recv_pages) << PAGE_SHIFT,
-				      newchannel->ringbuffer_send_offset << PAGE_SHIFT,
-				      &newchannel->ringbuffer_gpadlhandle);
+			page_address(newchannel->ringbuffer_page),
+			(send_pages + recv_pages) << PAGE_SHIFT,
+			newchannel->ringbuffer_send_offset << PAGE_SHIFT,
+			&newchannel->ringbuffer_gpadlhandle,
+			VMBUS_PAGE_VISIBLE_READ_WRITE);
 	if (err)
 		goto error_clean_ring;
 
diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
index 2353623259f3..bb72c7578330 100644
--- a/drivers/net/hyperv/netvsc.c
+++ b/drivers/net/hyperv/netvsc.c
@@ -333,7 +333,8 @@  static int netvsc_init_buf(struct hv_device *device,
 	 */
 	ret = vmbus_establish_gpadl(device->channel, net_device->recv_buf,
 				    buf_size,
-				    &net_device->recv_buf_gpadl_handle);
+				    &net_device->recv_buf_gpadl_handle,
+				    VMBUS_PAGE_VISIBLE_READ_WRITE);
 	if (ret != 0) {
 		netdev_err(ndev,
 			"unable to establish receive buffer's gpadl\n");
@@ -422,10 +423,13 @@  static int netvsc_init_buf(struct hv_device *device,
 	/* Establish the gpadl handle for this buffer on this
 	 * channel.  Note: This call uses the vmbus connection rather
 	 * than the channel to establish the gpadl handle.
+	 * Send buffer should theoretically be only marked as "read-only", but
+	 * the netvsp for some reason needs write capabilities on it.
 	 */
 	ret = vmbus_establish_gpadl(device->channel, net_device->send_buf,
 				    buf_size,
-				    &net_device->send_buf_gpadl_handle);
+				    &net_device->send_buf_gpadl_handle,
+				    VMBUS_PAGE_VISIBLE_READ_WRITE);
 	if (ret != 0) {
 		netdev_err(ndev,
 			   "unable to establish send buffer's gpadl\n");
diff --git a/drivers/uio/uio_hv_generic.c b/drivers/uio/uio_hv_generic.c
index 0330ba99730e..813a7bee5139 100644
--- a/drivers/uio/uio_hv_generic.c
+++ b/drivers/uio/uio_hv_generic.c
@@ -29,6 +29,7 @@ 
 #include <linux/hyperv.h>
 #include <linux/vmalloc.h>
 #include <linux/slab.h>
+#include <asm/mshyperv.h>
 
 #include "../hv/hyperv_vmbus.h"
 
@@ -295,7 +296,8 @@  hv_uio_probe(struct hv_device *dev,
 	}
 
 	ret = vmbus_establish_gpadl(channel, pdata->recv_buf,
-				    RECV_BUFFER_SIZE, &pdata->recv_gpadl);
+				    RECV_BUFFER_SIZE, &pdata->recv_gpadl,
+				    VMBUS_PAGE_VISIBLE_READ_WRITE);
 	if (ret)
 		goto fail_close;
 
@@ -315,7 +317,8 @@  hv_uio_probe(struct hv_device *dev,
 	}
 
 	ret = vmbus_establish_gpadl(channel, pdata->send_buf,
-				    SEND_BUFFER_SIZE, &pdata->send_gpadl);
+				    SEND_BUFFER_SIZE, &pdata->send_gpadl,
+				    VMBUS_PAGE_VISIBLE_READ_ONLY);
 	if (ret)
 		goto fail_close;
 
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index f1d74dcf0353..016fdca20d6e 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -1179,7 +1179,8 @@  extern int vmbus_sendpacket_mpb_desc(struct vmbus_channel *channel,
 extern int vmbus_establish_gpadl(struct vmbus_channel *channel,
 				      void *kbuffer,
 				      u32 size,
-				      u32 *gpadl_handle);
+				      u32 *gpadl_handle,
+				      u32 visibility);
 
 extern int vmbus_teardown_gpadl(struct vmbus_channel *channel,
 				     u32 gpadl_handle);