Re: [RFC PATCH 1/12] x86/Hyper-V: Add visibility parameter for vmbus_establish_gpadl()
Hi Vitaly:
Thanks for your review.
On 3/4/2021 12:27 AM, Vitaly Kuznetsov wrote:
Tianyu Lan writes:
From: Tianyu Lan
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
Co-Developed-by: Sunil Muthuswamy
Signed-off-by: Tianyu Lan
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 0x4118
+/* Hyper-V GPA map flags */
+#define HV_MAP_GPA_PERMISSIONS_NONE0x0
+#define HV_MAP_GPA_READABLE0x1
+#define HV_MAP_GPA_WRITABLE0x2
+
+#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,
Re: [RFC PATCH 1/12] x86/Hyper-V: Add visibility parameter for vmbus_establish_gpadl()
Tianyu Lan writes:
> From: Tianyu Lan
>
> 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
> Co-Developed-by: Sunil Muthuswamy
> Signed-off-by: Tianyu Lan
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 0x4118
>
> +/* 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_RE
[RFC PATCH 1/12] x86/Hyper-V: Add visibility parameter for vmbus_establish_gpadl()
From: Tianyu Lan
Add visibility parameter for vmbus_establish_gpadl() and prepare
to change host visibility when create gpadl for buffer.
Signed-off-by: Sunil Muthuswamy
Co-Developed-by: Sunil Muthuswamy
Signed-off-by: Tianyu Lan
---
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 0x4118
+/* Hyper-V GPA map flags */
+#define HV_MAP_GPA_PERMISSIONS_NONE0x0
+#define HV_MAP_GPA_READABLE0x1
+#define HV_MAP_GPA_WRITABLE0x2
+
+#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 (re
