Hi all,
The patchset:
- simplifies/refactors the VMBus negotiation code by introducing
the table of VMBus protocol versions (patch 1/2),
- enables VMBus protocol versions 5.1 and 5.2 (patch 2/2).
Thanks,
Andrea
Andrea Parri (2):
Drivers: hv: vmbus: Introduce table of VMBus protocol versions
Drivers: hv: vmbus: Enable VMBus protocol versions 5.1 and 5.2
drivers/hv/connection.c | 63 +++++++++++++++++------------------------
include/linux/hyperv.h | 6 ++--
2 files changed, 30 insertions(+), 39 deletions(-)
--
2.23.0
The technique used to get the next VMBus version seems increasisly
clumsy as the number of VMBus versions increases. Performance is
not a concern since this is only done once during system boot; it's
just that we'll end up with more lines of code than is really needed.
As an alternative, introduce a table with the version numbers listed
in order (from the most recent to the oldest). vmbus_connect() loops
through the versions listed in the table until it gets an accepted
connection or gets to the end of the table (invalid version).
Suggested-by: Michael Kelley <[email protected]>
Signed-off-by: Andrea Parri <[email protected]>
---
drivers/hv/connection.c | 51 +++++++++++++++--------------------------
include/linux/hyperv.h | 2 --
2 files changed, 19 insertions(+), 34 deletions(-)
diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c
index 6e4c015783ffc..90a32c9d79403 100644
--- a/drivers/hv/connection.c
+++ b/drivers/hv/connection.c
@@ -40,29 +40,19 @@ EXPORT_SYMBOL_GPL(vmbus_connection);
__u32 vmbus_proto_version;
EXPORT_SYMBOL_GPL(vmbus_proto_version);
-static __u32 vmbus_get_next_version(__u32 current_version)
-{
- switch (current_version) {
- case (VERSION_WIN7):
- return VERSION_WS2008;
-
- case (VERSION_WIN8):
- return VERSION_WIN7;
-
- case (VERSION_WIN8_1):
- return VERSION_WIN8;
-
- case (VERSION_WIN10):
- return VERSION_WIN8_1;
-
- case (VERSION_WIN10_V5):
- return VERSION_WIN10;
-
- case (VERSION_WS2008):
- default:
- return VERSION_INVAL;
- }
-}
+/*
+ * Table of VMBus versions listed from newest to oldest; the table
+ * must terminate with VERSION_INVAL.
+ */
+__u32 vmbus_versions[] = {
+ VERSION_WIN10_V5,
+ VERSION_WIN10,
+ VERSION_WIN8_1,
+ VERSION_WIN8,
+ VERSION_WIN7,
+ VERSION_WS2008,
+ VERSION_INVAL
+};
int vmbus_negotiate_version(struct vmbus_channel_msginfo *msginfo, u32 version)
{
@@ -169,8 +159,8 @@ int vmbus_negotiate_version(struct vmbus_channel_msginfo *msginfo, u32 version)
*/
int vmbus_connect(void)
{
- int ret = 0;
struct vmbus_channel_msginfo *msginfo = NULL;
+ int i, ret = 0;
__u32 version;
/* Initialize the vmbus connection */
@@ -244,21 +234,18 @@ int vmbus_connect(void)
* version.
*/
- version = VERSION_CURRENT;
+ for (i = 0; ; i++) {
+ version = vmbus_versions[i];
+ if (version == VERSION_INVAL)
+ goto cleanup;
- do {
ret = vmbus_negotiate_version(msginfo, version);
if (ret == -ETIMEDOUT)
goto cleanup;
if (vmbus_connection.conn_state == CONNECTED)
break;
-
- version = vmbus_get_next_version(version);
- } while (version != VERSION_INVAL);
-
- if (version == VERSION_INVAL)
- goto cleanup;
+ }
vmbus_proto_version = version;
pr_info("Vmbus version:%d.%d\n",
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index b4a017093b697..7073f1eb3618c 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -194,8 +194,6 @@ static inline u32 hv_get_avail_to_write_percent(
#define VERSION_INVAL -1
-#define VERSION_CURRENT VERSION_WIN10_V5
-
/* Make maximum size of pipe payload of 16K */
#define MAX_PIPE_DATA_PAYLOAD (sizeof(u8) * 16384)
--
2.23.0
Hyper-V has added VMBus protocol versions 5.1 and 5.2 in recent release
versions. Allow Linux guests to negotiate these new protocol versions
on versions of Hyper-V that support them.
Signed-off-by: Andrea Parri <[email protected]>
---
drivers/hv/connection.c | 12 +++++++-----
include/linux/hyperv.h | 4 ++++
2 files changed, 11 insertions(+), 5 deletions(-)
diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c
index 90a32c9d79403..d05fef3e09080 100644
--- a/drivers/hv/connection.c
+++ b/drivers/hv/connection.c
@@ -45,6 +45,8 @@ EXPORT_SYMBOL_GPL(vmbus_proto_version);
* must terminate with VERSION_INVAL.
*/
__u32 vmbus_versions[] = {
+ VERSION_WIN10_V5_2,
+ VERSION_WIN10_V5_1,
VERSION_WIN10_V5,
VERSION_WIN10,
VERSION_WIN8_1,
@@ -70,12 +72,12 @@ int vmbus_negotiate_version(struct vmbus_channel_msginfo *msginfo, u32 version)
msg->vmbus_version_requested = version;
/*
- * VMBus protocol 5.0 (VERSION_WIN10_V5) requires that we must use
- * VMBUS_MESSAGE_CONNECTION_ID_4 for the Initiate Contact Message,
+ * VMBus protocol 5.0 (VERSION_WIN10_V5) and higher require that we must
+ * use VMBUS_MESSAGE_CONNECTION_ID_4 for the Initiate Contact Message,
* and for subsequent messages, we must use the Message Connection ID
* field in the host-returned Version Response Message. And, with
- * VERSION_WIN10_V5, we don't use msg->interrupt_page, but we tell
- * the host explicitly that we still use VMBUS_MESSAGE_SINT(2) for
+ * VERSION_WIN10_V5 and higher, we don't use msg->interrupt_page, but we
+ * tell the host explicitly that we still use VMBUS_MESSAGE_SINT(2) for
* compatibility.
*
* On old hosts, we should always use VMBUS_MESSAGE_CONNECTION_ID (1).
@@ -400,7 +402,7 @@ int vmbus_post_msg(void *buffer, size_t buflen, bool can_sleep)
case HV_STATUS_INVALID_CONNECTION_ID:
/*
* See vmbus_negotiate_version(): VMBus protocol 5.0
- * requires that we must use
+ * and higher require that we must use
* VMBUS_MESSAGE_CONNECTION_ID_4 for the Initiate
* Contact message, but on old hosts that only
* support VMBus protocol 4.0 or lower, here we get
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index 7073f1eb3618c..5ecb2ff7cc25d 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -183,6 +183,8 @@ static inline u32 hv_get_avail_to_write_percent(
* 3 . 0 (Windows 8 R2)
* 4 . 0 (Windows 10)
* 5 . 0 (Newer Windows 10)
+ * 5 . 1 (Windows 10 RS4)
+ * 5 . 2 (Windows Server 2019, RS5)
*/
#define VERSION_WS2008 ((0 << 16) | (13))
@@ -191,6 +193,8 @@ static inline u32 hv_get_avail_to_write_percent(
#define VERSION_WIN8_1 ((3 << 16) | (0))
#define VERSION_WIN10 ((4 << 16) | (0))
#define VERSION_WIN10_V5 ((5 << 16) | (0))
+#define VERSION_WIN10_V5_1 ((5 << 16) | (1))
+#define VERSION_WIN10_V5_2 ((5 << 16) | (2))
#define VERSION_INVAL -1
--
2.23.0
Andrea Parri <[email protected]> writes:
> The technique used to get the next VMBus version seems increasisly
> clumsy as the number of VMBus versions increases. Performance is
> not a concern since this is only done once during system boot; it's
> just that we'll end up with more lines of code than is really needed.
>
> As an alternative, introduce a table with the version numbers listed
> in order (from the most recent to the oldest). vmbus_connect() loops
> through the versions listed in the table until it gets an accepted
> connection or gets to the end of the table (invalid version).
>
> Suggested-by: Michael Kelley <[email protected]>
> Signed-off-by: Andrea Parri <[email protected]>
> ---
> drivers/hv/connection.c | 51 +++++++++++++++--------------------------
> include/linux/hyperv.h | 2 --
> 2 files changed, 19 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c
> index 6e4c015783ffc..90a32c9d79403 100644
> --- a/drivers/hv/connection.c
> +++ b/drivers/hv/connection.c
> @@ -40,29 +40,19 @@ EXPORT_SYMBOL_GPL(vmbus_connection);
> __u32 vmbus_proto_version;
> EXPORT_SYMBOL_GPL(vmbus_proto_version);
>
> -static __u32 vmbus_get_next_version(__u32 current_version)
> -{
> - switch (current_version) {
> - case (VERSION_WIN7):
> - return VERSION_WS2008;
> -
> - case (VERSION_WIN8):
> - return VERSION_WIN7;
> -
> - case (VERSION_WIN8_1):
> - return VERSION_WIN8;
> -
> - case (VERSION_WIN10):
> - return VERSION_WIN8_1;
> -
> - case (VERSION_WIN10_V5):
> - return VERSION_WIN10;
> -
> - case (VERSION_WS2008):
> - default:
> - return VERSION_INVAL;
> - }
> -}
> +/*
> + * Table of VMBus versions listed from newest to oldest; the table
> + * must terminate with VERSION_INVAL.
> + */
> +__u32 vmbus_versions[] = {
> + VERSION_WIN10_V5,
> + VERSION_WIN10,
> + VERSION_WIN8_1,
> + VERSION_WIN8,
> + VERSION_WIN7,
> + VERSION_WS2008,
> + VERSION_INVAL
> +};
>
> int vmbus_negotiate_version(struct vmbus_channel_msginfo *msginfo, u32 version)
> {
> @@ -169,8 +159,8 @@ int vmbus_negotiate_version(struct vmbus_channel_msginfo *msginfo, u32 version)
> */
> int vmbus_connect(void)
> {
> - int ret = 0;
> struct vmbus_channel_msginfo *msginfo = NULL;
> + int i, ret = 0;
> __u32 version;
>
> /* Initialize the vmbus connection */
> @@ -244,21 +234,18 @@ int vmbus_connect(void)
> * version.
> */
>
> - version = VERSION_CURRENT;
> + for (i = 0; ; i++) {
> + version = vmbus_versions[i];
> + if (version == VERSION_INVAL)
> + goto cleanup;
If you use e.g. ARRAY_SIZE() you can get rid of VERSION_INVAL - and make
this code look more natural.
>
> - do {
> ret = vmbus_negotiate_version(msginfo, version);
> if (ret == -ETIMEDOUT)
> goto cleanup;
>
> if (vmbus_connection.conn_state == CONNECTED)
> break;
> -
> - version = vmbus_get_next_version(version);
> - } while (version != VERSION_INVAL);
> -
> - if (version == VERSION_INVAL)
> - goto cleanup;
> + }
>
> vmbus_proto_version = version;
> pr_info("Vmbus version:%d.%d\n",
> diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
> index b4a017093b697..7073f1eb3618c 100644
> --- a/include/linux/hyperv.h
> +++ b/include/linux/hyperv.h
> @@ -194,8 +194,6 @@ static inline u32 hv_get_avail_to_write_percent(
>
> #define VERSION_INVAL -1
>
> -#define VERSION_CURRENT VERSION_WIN10_V5
> -
> /* Make maximum size of pipe payload of 16K */
> #define MAX_PIPE_DATA_PAYLOAD (sizeof(u8) * 16384)
--
Vitaly
> From: [email protected]
> <[email protected]> On Behalf Of Andrea Parri
> Sent: Monday, October 7, 2019 9:31 AM
> ....
> +/*
> + * Table of VMBus versions listed from newest to oldest; the table
> + * must terminate with VERSION_INVAL.
> + */
> +__u32 vmbus_versions[] = {
> + VERSION_WIN10_V5,
This should be "static"?
Thanks,
Dexuan
> From: [email protected]
> <[email protected]> On Behalf Of Andrea Parri
> Sent: Monday, October 7, 2019 9:31 AM
>
> Hi all,
>
> The patchset:
>
> - simplifies/refactors the VMBus negotiation code by introducing
> the table of VMBus protocol versions (patch 1/2),
>
> - enables VMBus protocol versions 5.1 and 5.2 (patch 2/2).
>
> Thanks,
> Andrea
>
> Andrea Parri (2):
> Drivers: hv: vmbus: Introduce table of VMBus protocol versions
> Drivers: hv: vmbus: Enable VMBus protocol versions 5.1 and 5.2
Should we add a module parameter to allow the user to specify a lower
protocol version, when the VM runs on the latest host?
This can be useful for testing and debugging purpose: the variable
"vmbus_proto_version" is referenced by the vmbus driver itself and
some VSC drivers: if we always use the latest available proto version,
some code paths can not be tested on the latest hosts.
Thanks,
-- Dexuan
> > @@ -244,21 +234,18 @@ int vmbus_connect(void)
> > * version.
> > */
> >
> > - version = VERSION_CURRENT;
> > + for (i = 0; ; i++) {
> > + version = vmbus_versions[i];
> > + if (version == VERSION_INVAL)
> > + goto cleanup;
>
> If you use e.g. ARRAY_SIZE() you can get rid of VERSION_INVAL - and make
> this code look more natural.
Thank you for pointing this out, Vitaly.
IIUC, you're suggesting that I do:
for (i = 0; i < ARRAY_SIZE(vmbus_versions); i++) {
version = vmbus_versions[i];
ret = vmbus_negotiate_version(msginfo, version);
if (ret == -ETIMEDOUT)
goto cleanup;
if (vmbus_connection.conn_state == CONNECTED)
break;
}
if (vmbus_connection.conn_state != CONNECTED)
break;
and that I remove VERSION_INVAL from vmbus_versions, yes?
Looking at the uses of VERSION_INVAL, I find one remaining occurrence
of this macro in vmbus_bus_resume(), which does:
if (vmbus_proto_version == VERSION_INVAL ||
vmbus_proto_version == 0) {
...
}
TBH I'm looking at vmbus_bus_resume() and vmbus_bus_suspend() for the
first time and I'm not sure about how to change such check yet... any
suggestions?
Mmh, I see that vmbus_bus_resume() and vmbus_bus_suspend() can access
vmbus_connection.conn_state: can such accesses race with a concurrent
vmbus_connect()?
Thanks,
Andrea
> >
> > - do {
> > ret = vmbus_negotiate_version(msginfo, version);
> > if (ret == -ETIMEDOUT)
> > goto cleanup;
> >
> > if (vmbus_connection.conn_state == CONNECTED)
> > break;
> > -
> > - version = vmbus_get_next_version(version);
> > - } while (version != VERSION_INVAL);
> > -
> > - if (version == VERSION_INVAL)
> > - goto cleanup;
> > + }
> >
> > vmbus_proto_version = version;
> > pr_info("Vmbus version:%d.%d\n",
On Mon, Oct 07, 2019 at 05:25:18PM +0000, Dexuan Cui wrote:
> > From: [email protected]
> > <[email protected]> On Behalf Of Andrea Parri
> > Sent: Monday, October 7, 2019 9:31 AM
> > ....
> > +/*
> > + * Table of VMBus versions listed from newest to oldest; the table
> > + * must terminate with VERSION_INVAL.
> > + */
> > +__u32 vmbus_versions[] = {
> > + VERSION_WIN10_V5,
>
> This should be "static"?
I think so, will add in v2. Thank you for pointing this out, Dexuan.
Andrea
> IIUC, you're suggesting that I do:
>
> for (i = 0; i < ARRAY_SIZE(vmbus_versions); i++) {
> version = vmbus_versions[i];
>
> ret = vmbus_negotiate_version(msginfo, version);
> if (ret == -ETIMEDOUT)
> goto cleanup;
>
> if (vmbus_connection.conn_state == CONNECTED)
> break;
> }
>
> if (vmbus_connection.conn_state != CONNECTED)
> break;
This "break" should have been "goto cleanup". Sorry.
Andrea
Andrea Parri <[email protected]> writes:
>> > @@ -244,21 +234,18 @@ int vmbus_connect(void)
>> > * version.
>> > */
>> >
>> > - version = VERSION_CURRENT;
>> > + for (i = 0; ; i++) {
>> > + version = vmbus_versions[i];
>> > + if (version == VERSION_INVAL)
>> > + goto cleanup;
>>
>> If you use e.g. ARRAY_SIZE() you can get rid of VERSION_INVAL - and make
>> this code look more natural.
>
> Thank you for pointing this out, Vitaly.
>
> IIUC, you're suggesting that I do:
>
> for (i = 0; i < ARRAY_SIZE(vmbus_versions); i++) {
> version = vmbus_versions[i];
>
> ret = vmbus_negotiate_version(msginfo, version);
> if (ret == -ETIMEDOUT)
> goto cleanup;
>
> if (vmbus_connection.conn_state == CONNECTED)
> break;
> }
>
> if (vmbus_connection.conn_state != CONNECTED)
> break;
>
> and that I remove VERSION_INVAL from vmbus_versions, yes?
>
Yes, something like that.
> Looking at the uses of VERSION_INVAL, I find one remaining occurrence
> of this macro in vmbus_bus_resume(), which does:
>
> if (vmbus_proto_version == VERSION_INVAL ||
> vmbus_proto_version == 0) {
> ...
> }
>
> TBH I'm looking at vmbus_bus_resume() and vmbus_bus_suspend() for the
> first time and I'm not sure about how to change such check yet... any
> suggestions?
Hm, I don't think vmbus_proto_version can ever become == VERSION_INVAL
if we rewrite the code the way you suggest, right? So you'll reduce this
to 'if (!vmbus_proto_version)' meaning we haven't negotiated any version
(yet).
>
> Mmh, I see that vmbus_bus_resume() and vmbus_bus_suspend() can access
> vmbus_connection.conn_state: can such accesses race with a concurrent
> vmbus_connect()?
Let's summon Dexuan who's the author! :-)
>
> Thanks,
> Andrea
>
>
>> >
>> > - do {
>> > ret = vmbus_negotiate_version(msginfo, version);
>> > if (ret == -ETIMEDOUT)
>> > goto cleanup;
>> >
>> > if (vmbus_connection.conn_state == CONNECTED)
>> > break;
>> > -
>> > - version = vmbus_get_next_version(version);
>> > - } while (version != VERSION_INVAL);
>> > -
>> > - if (version == VERSION_INVAL)
>> > - goto cleanup;
>> > + }
>> >
>> > vmbus_proto_version = version;
>> > pr_info("Vmbus version:%d.%d\n",
--
Vitaly
On Mon, Oct 07, 2019 at 05:41:10PM +0000, Dexuan Cui wrote:
> > From: [email protected]
> > <[email protected]> On Behalf Of Andrea Parri
> > Sent: Monday, October 7, 2019 9:31 AM
> >
> > Hi all,
> >
> > The patchset:
> >
> > - simplifies/refactors the VMBus negotiation code by introducing
> > the table of VMBus protocol versions (patch 1/2),
> >
> > - enables VMBus protocol versions 5.1 and 5.2 (patch 2/2).
> >
> > Thanks,
> > Andrea
> >
> > Andrea Parri (2):
> > Drivers: hv: vmbus: Introduce table of VMBus protocol versions
> > Drivers: hv: vmbus: Enable VMBus protocol versions 5.1 and 5.2
>
> Should we add a module parameter to allow the user to specify a lower
> protocol version, when the VM runs on the latest host?
>
> This can be useful for testing and debugging purpose: the variable
> "vmbus_proto_version" is referenced by the vmbus driver itself and
> some VSC drivers: if we always use the latest available proto version,
> some code paths can not be tested on the latest hosts.
The idea is appealing to me (altough I made no attempt to implement/test
it yet). What do others think about this? Maybe can be considered as a
follow-up patch/work to this series?
Thanks,
Andrea
Andrea Parri <[email protected]> writes:
> On Mon, Oct 07, 2019 at 05:41:10PM +0000, Dexuan Cui wrote:
>> > From: [email protected]
>> > <[email protected]> On Behalf Of Andrea Parri
>> > Sent: Monday, October 7, 2019 9:31 AM
>> >
>> > Hi all,
>> >
>> > The patchset:
>> >
>> > - simplifies/refactors the VMBus negotiation code by introducing
>> > the table of VMBus protocol versions (patch 1/2),
>> >
>> > - enables VMBus protocol versions 5.1 and 5.2 (patch 2/2).
>> >
>> > Thanks,
>> > Andrea
>> >
>> > Andrea Parri (2):
>> > Drivers: hv: vmbus: Introduce table of VMBus protocol versions
>> > Drivers: hv: vmbus: Enable VMBus protocol versions 5.1 and 5.2
>>
>> Should we add a module parameter to allow the user to specify a lower
>> protocol version, when the VM runs on the latest host?
>>
>> This can be useful for testing and debugging purpose: the variable
>> "vmbus_proto_version" is referenced by the vmbus driver itself and
>> some VSC drivers: if we always use the latest available proto version,
>> some code paths can not be tested on the latest hosts.
>
> The idea is appealing to me (altough I made no attempt to implement/test
> it yet). What do others think about this? Maybe can be considered as a
> follow-up patch/work to this series?
IMO such debug option makes sense, it shouldn be a simple patch so you
may want to squeeze it in this series as it will definitely have code
dependencies.
--
Vitaly
> From: Vitaly Kuznetsov <[email protected]>
> Sent: Tuesday, October 8, 2019 6:00 AM
> ...
> > Looking at the uses of VERSION_INVAL, I find one remaining occurrence
> > of this macro in vmbus_bus_resume(), which does:
> >
> > if (vmbus_proto_version == VERSION_INVAL ||
> > vmbus_proto_version == 0) {
> > ...
> > }
> >
> > TBH I'm looking at vmbus_bus_resume() and vmbus_bus_suspend() for the
> > first time and I'm not sure about how to change such check yet... any
> > suggestions?
>
> Hm, I don't think vmbus_proto_version can ever become == VERSION_INVAL
> if we rewrite the code the way you suggest, right? So you'll reduce this
> to 'if (!vmbus_proto_version)' meaning we haven't negotiated any version
> (yet).
Yeah, Vitaly is correct. The check may be a little paranoid as I believe
"vmbus_proto_version" must be a negotiated value in vmbus_bus_resume()
and vmbus_bus_suspend(). I added the check just in case.
> > Mmh, I see that vmbus_bus_resume() and vmbus_bus_suspend() can access
> > vmbus_connection.conn_state: can such accesses race with a concurrent
> > vmbus_connect()?
>
> Let's summon Dexuan who's the author! :-)
There should not be an issue:
vmbus_connect() is called in the early subsys_initcall(hv_acpi_init).
vmbus_bus_suspend() is called late in the PM code after the kernel boots up, e.g.
in the hibernation function hibernation_snapshot() -> dpm_suspend().
vmbus_bus_resume() is also called later in late_initcall_sync(software_resume).
In the hibernatin process, vmbus_bus_suspend()/resume() can also be called a
few times, and vmbus_bus_resume() calls vmbus_negotiate_version(). As I
checked, there is no issue, either.
Thanks,
Dexuan
On Tue, Oct 08, 2019 at 10:41:42PM +0000, Dexuan Cui wrote:
> > From: Vitaly Kuznetsov <[email protected]>
> > Sent: Tuesday, October 8, 2019 6:00 AM
> > ...
> > > Looking at the uses of VERSION_INVAL, I find one remaining occurrence
> > > of this macro in vmbus_bus_resume(), which does:
> > >
> > > if (vmbus_proto_version == VERSION_INVAL ||
> > > vmbus_proto_version == 0) {
> > > ...
> > > }
> > >
> > > TBH I'm looking at vmbus_bus_resume() and vmbus_bus_suspend() for the
> > > first time and I'm not sure about how to change such check yet... any
> > > suggestions?
> >
> > Hm, I don't think vmbus_proto_version can ever become == VERSION_INVAL
> > if we rewrite the code the way you suggest, right? So you'll reduce this
> > to 'if (!vmbus_proto_version)' meaning we haven't negotiated any version
> > (yet).
>
> Yeah, Vitaly is correct. The check may be a little paranoid as I believe
> "vmbus_proto_version" must be a negotiated value in vmbus_bus_resume()
> and vmbus_bus_suspend(). I added the check just in case.
>
> > > Mmh, I see that vmbus_bus_resume() and vmbus_bus_suspend() can access
> > > vmbus_connection.conn_state: can such accesses race with a concurrent
> > > vmbus_connect()?
> >
> > Let's summon Dexuan who's the author! :-)
>
> There should not be an issue:
>
> vmbus_connect() is called in the early subsys_initcall(hv_acpi_init).
>
> vmbus_bus_suspend() is called late in the PM code after the kernel boots up, e.g.
> in the hibernation function hibernation_snapshot() -> dpm_suspend().
>
> vmbus_bus_resume() is also called later in late_initcall_sync(software_resume).
>
> In the hibernatin process, vmbus_bus_suspend()/resume() can also be called a
> few times, and vmbus_bus_resume() calls vmbus_negotiate_version(). As I
> checked, there is no issue, either.
Thank you both for these remarks.
So, I'll proceed with the removal of VERSION_INVAL in v2 of this series.
Thanks,
Andrea