Re: [PATCH] Drivers: hv: vmbus: Add /sys/bus/vmbus/supported_features
On Wed, Jan 06, 2021 at 08:49:32PM +, Dexuan Cui wrote: > > From: Michael Kelley > > Sent: Wednesday, January 6, 2021 9:38 AM > > From: Dexuan Cui > > Sent: Tuesday, December 22, 2020 4:12 PM > > > > > > When a Linux VM runs on Hyper-V, if the host toolstack doesn't support > > > hibernation for the VM (this happens on old Hyper-V hosts like Windows > > > Server 2016, or new Hyper-V hosts if the admin or user doesn't declare > > > the hibernation intent for the VM), the VM is discouraged from trying > > > hibernation (because the host doesn't guarantee that the VM's virtual > > > hardware configuration will remain exactly the same across hibernation), > > > i.e. the VM should not try to set up the swap partition/file for > > > hibernation, etc. > > > > > > x86 Hyper-V uses the presence of the virtual ACPI S4 state as the > > > indication of the host toolstack support for a VM. Currently there is > > > no easy and reliable way for the userspace to detect the presence of > > > the state (see ...). Add > > > /sys/bus/vmbus/supported_features for this purpose. > > > > I'm OK with surfacing the hibernation capability via an entry in > > /sys/bus/vmbus. Correct me if I'm wrong, but I think the concept > > being surfaced is not "ACPI S4 state" precisely, but slightly more > > generally whether hibernation is supported for the VM. While > > those two concepts may be 1:1 for the moment, there might be > > future configurations where "hibernation is supported" depends > > on other factors as well. > > For x86, I believe the virtual ACPI S4 state exists only when the > admin/user declares the intent of "enable hibernation for the VM" via > some PowwerShell/WMI command. On Azure, if a VM size is not suitable > for hibernation (e.g. an existing VM has an ephemeral local disk), > the toolstack on the host should not enable the ACPI S4 state for the > VM. That's why we implemented hv_is_hibernation_supported() for x86 by > checking the ACPI S4 state, and we have used the function > hv_is_hibernation_supported() in hv_utils and hv_balloon for quite a > while. > > For ARM, IIRC there is no concept of ACPI S4 state, so currently > hv_is_hibernation_supported() is actually not implemented. Not sure Because the core support for ARM64 Hyper-V is not merged yet. In Michael's core patchset, hv_is_hibernation_supported() is implemented as always returning false, and there is more work (other than Michael's core pachset) to make hiberation work on ARM64 Hyper-V guest. Regards, Boqun > why hv_utils and hv_balloon can build successfully... :-) Probably > Boqun can help to take a look. > > > > > The guidance for things in /sys is that they generally should > > be single valued (see Documentation/filesystems/sysfs.rst). So my > > recommendation is to create a "hibernation" entry that has a value > > of 0 or 1. > > > > Michael > > Got it. Then let's use /sys/bus/vmbus/hibernation. > > Will post v3. > > Thanks, > -- Dexuan >
RE: [PATCH] Drivers: hv: vmbus: Add /sys/bus/vmbus/supported_features
> From: Michael Kelley > Sent: Wednesday, January 6, 2021 9:38 AM > From: Dexuan Cui > Sent: Tuesday, December 22, 2020 4:12 PM > > > > When a Linux VM runs on Hyper-V, if the host toolstack doesn't support > > hibernation for the VM (this happens on old Hyper-V hosts like Windows > > Server 2016, or new Hyper-V hosts if the admin or user doesn't declare > > the hibernation intent for the VM), the VM is discouraged from trying > > hibernation (because the host doesn't guarantee that the VM's virtual > > hardware configuration will remain exactly the same across hibernation), > > i.e. the VM should not try to set up the swap partition/file for > > hibernation, etc. > > > > x86 Hyper-V uses the presence of the virtual ACPI S4 state as the > > indication of the host toolstack support for a VM. Currently there is > > no easy and reliable way for the userspace to detect the presence of > > the state (see ...). Add > > /sys/bus/vmbus/supported_features for this purpose. > > I'm OK with surfacing the hibernation capability via an entry in > /sys/bus/vmbus. Correct me if I'm wrong, but I think the concept > being surfaced is not "ACPI S4 state" precisely, but slightly more > generally whether hibernation is supported for the VM. While > those two concepts may be 1:1 for the moment, there might be > future configurations where "hibernation is supported" depends > on other factors as well. For x86, I believe the virtual ACPI S4 state exists only when the admin/user declares the intent of "enable hibernation for the VM" via some PowwerShell/WMI command. On Azure, if a VM size is not suitable for hibernation (e.g. an existing VM has an ephemeral local disk), the toolstack on the host should not enable the ACPI S4 state for the VM. That's why we implemented hv_is_hibernation_supported() for x86 by checking the ACPI S4 state, and we have used the function hv_is_hibernation_supported() in hv_utils and hv_balloon for quite a while. For ARM, IIRC there is no concept of ACPI S4 state, so currently hv_is_hibernation_supported() is actually not implemented. Not sure why hv_utils and hv_balloon can build successfully... :-) Probably Boqun can help to take a look. > > The guidance for things in /sys is that they generally should > be single valued (see Documentation/filesystems/sysfs.rst). So my > recommendation is to create a "hibernation" entry that has a value > of 0 or 1. > > Michael Got it. Then let's use /sys/bus/vmbus/hibernation. Will post v3. Thanks, -- Dexuan
RE: [PATCH] Drivers: hv: vmbus: Add /sys/bus/vmbus/supported_features
> From: Wei Liu > Sent: Wednesday, January 6, 2021 8:24 AM > To: Dexuan Cui > > > > That said, I don't know if there is any hard rule in regard of > > the timing here. If there is, then v5.12 is OK to me. :-) > > > > By the time you posted this (Dec 22), 5.11 was already more or less > "frozen". Linus wanted -next patches to be merged before Christmas. Got it. Thanks for the explanation! > The way I see it this is a new sysfs interface so I think this is > something new, which is for 5.12. Ok. > Do you think this should be considered a bug fix? > > Wei. Then let's not consider it for v5.11. For now I think the userspace tool/daemon can check 'dmesg' for the existence of ACPI S4 state as a workaround. This is not ideal, but it should work reasonably well, assuming the tool/daemon runs early enough, so the kernel msg buffer is not yet filled up and overwritten. :-) Thanks, -- Dexuan
RE: [PATCH] Drivers: hv: vmbus: Add /sys/bus/vmbus/supported_features
From: Dexuan Cui Sent: Tuesday, December 22, 2020 4:12 PM > > When a Linux VM runs on Hyper-V, if the host toolstack doesn't support > hibernation for the VM (this happens on old Hyper-V hosts like Windows > Server 2016, or new Hyper-V hosts if the admin or user doesn't declare > the hibernation intent for the VM), the VM is discouraged from trying > hibernation (because the host doesn't guarantee that the VM's virtual > hardware configuration will remain exactly the same across hibernation), > i.e. the VM should not try to set up the swap partition/file for > hibernation, etc. > > x86 Hyper-V uses the presence of the virtual ACPI S4 state as the > indication of the host toolstack support for a VM. Currently there is > no easy and reliable way for the userspace to detect the presence of > the state (see https://lkml.org/lkml/2020/12/11/1097). Add > /sys/bus/vmbus/supported_features for this purpose. I'm OK with surfacing the hibernation capability via an entry in /sys/bus/vmbus. Correct me if I'm wrong, but I think the concept being surfaced is not "ACPI S4 state" precisely, but slightly more generally whether hibernation is supported for the VM. While those two concepts may be 1:1 for the moment, there might be future configurations where "hibernation is supported" depends on other factors as well. The guidance for things in /sys is that they generally should be single valued (see Documentation/filesystems/sysfs.rst). So my recommendation is to create a "hibernation" entry that has a value of 0 or 1. That's the pattern I see in lots of other places in /sys. If other Hyper-V or VMbus-related features need to be surfaced in the future, they would have their own single-valued entry. Michael > > Signed-off-by: Dexuan Cui > --- > Documentation/ABI/stable/sysfs-bus-vmbus | 7 +++ > drivers/hv/vmbus_drv.c | 20 > 2 files changed, 27 insertions(+) > > diff --git a/Documentation/ABI/stable/sysfs-bus-vmbus > b/Documentation/ABI/stable/sysfs-bus-vmbus > index c27b7b89477c..3ba765ae6695 100644 > --- a/Documentation/ABI/stable/sysfs-bus-vmbus > +++ b/Documentation/ABI/stable/sysfs-bus-vmbus > @@ -1,3 +1,10 @@ > +What:/sys/bus/vmbus/supported_features > +Date:Dec 2020 > +KernelVersion: 5.11 > +Contact: Dexuan Cui > +Description: Features specific to VMs running on Hyper-V > +Users: Daemon that sets up swap partition/file for hibernation > + > What:/sys/bus/vmbus/devices//id > Date:Jul 2009 > KernelVersion: 2.6.31 > diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c > index d491fdcee61f..958487a40a18 100644 > --- a/drivers/hv/vmbus_drv.c > +++ b/drivers/hv/vmbus_drv.c > @@ -678,6 +678,25 @@ static const struct attribute_group vmbus_dev_group = { > }; > __ATTRIBUTE_GROUPS(vmbus_dev); > > +/* Set up bus attribute(s) for /sys/bus/vmbus/supported_features */ > +static ssize_t supported_features_show(struct bus_type *bus, char *buf) > +{ > + bool hb = hv_is_hibernation_supported(); > + > + return sprintf(buf, "%s\n", hb ? "hibernation" : ""); > +} > + > +static BUS_ATTR_RO(supported_features); > + > +static struct attribute *vmbus_bus_attrs[] = { > + &bus_attr_supported_features.attr, > + NULL, > +}; > +static const struct attribute_group vmbus_bus_group = { > + .attrs = vmbus_bus_attrs, > +}; > +__ATTRIBUTE_GROUPS(vmbus_bus); > + > /* > * vmbus_uevent - add uevent for our device > * > @@ -1024,6 +1043,7 @@ static struct bus_type hv_bus = { > .uevent = vmbus_uevent, > .dev_groups = vmbus_dev_groups, > .drv_groups = vmbus_drv_groups, > + .bus_groups = vmbus_bus_groups, > .pm = &vmbus_pm, > }; > > -- > 2.19.1
Re: [PATCH] Drivers: hv: vmbus: Add /sys/bus/vmbus/supported_features
On Tue, Jan 05, 2021 at 11:04:15PM +, Dexuan Cui wrote: > > From: Wei Liu > > Sent: Tuesday, January 5, 2021 4:58 AM > > > ... > > > Documentation/ABI/stable/sysfs-bus-vmbus | 7 +++ > > > drivers/hv/vmbus_drv.c | 20 > > > > > 2 files changed, 27 insertions(+) > > > > > > diff --git a/Documentation/ABI/stable/sysfs-bus-vmbus > > b/Documentation/ABI/stable/sysfs-bus-vmbus > > > index c27b7b89477c..3ba765ae6695 100644 > > > --- a/Documentation/ABI/stable/sysfs-bus-vmbus > > > +++ b/Documentation/ABI/stable/sysfs-bus-vmbus > > > @@ -1,3 +1,10 @@ > > > +What:/sys/bus/vmbus/supported_features > > > +Date:Dec 2020 > > > +KernelVersion: 5.11 > > > > Too late for 5.11 now. > > The patch was posted 2 weeks ago, before 5.11-rc1. :-) > > If possible, we still want this patch to be in 5.11, because: > 1. The patch is small and IMO pretty safe. > 2. The patch adds new code and doesn't really change any old code. > 3. The patch adds a new sys file, which is needed by some > user space tool to auto-setup the configuration for hibernation. > We'd like to integrate the patch into the Linux distros ASAP and > as we know some distros don't accept a patch if it's not in the > mainline. > > So, if the patch itself looks good, IMO it would be better to be > in v5.11 rather than in v5.12, which will need extra time of > 2~3 months. > > That said, I don't know if there is any hard rule in regard of > the timing here. If there is, then v5.12 is OK to me. :-) > By the time you posted this (Dec 22), 5.11 was already more or less "frozen". Linus wanted -next patches to be merged before Christmas. The way I see it this is a new sysfs interface so I think this is something new, which is for 5.12. Do you think this should be considered a bug fix? Wei. > > Given this is a list of strings, do you want to enumerate them in a > > Values section or the Description section? > > > > Wei. > > Currently there is only one possible string "hibernation". > Not sure if we would need to add more strings in future, but yes > it's a good idea to list the string in a "Value" section. > > Will post v2 shortly. > > Thanks, > -- Dexuan
RE: [PATCH] Drivers: hv: vmbus: Add /sys/bus/vmbus/supported_features
> From: Wei Liu > Sent: Tuesday, January 5, 2021 4:58 AM > > ... > > Documentation/ABI/stable/sysfs-bus-vmbus | 7 +++ > > drivers/hv/vmbus_drv.c | 20 > > > 2 files changed, 27 insertions(+) > > > > diff --git a/Documentation/ABI/stable/sysfs-bus-vmbus > b/Documentation/ABI/stable/sysfs-bus-vmbus > > index c27b7b89477c..3ba765ae6695 100644 > > --- a/Documentation/ABI/stable/sysfs-bus-vmbus > > +++ b/Documentation/ABI/stable/sysfs-bus-vmbus > > @@ -1,3 +1,10 @@ > > +What: /sys/bus/vmbus/supported_features > > +Date: Dec 2020 > > +KernelVersion: 5.11 > > Too late for 5.11 now. The patch was posted 2 weeks ago, before 5.11-rc1. :-) If possible, we still want this patch to be in 5.11, because: 1. The patch is small and IMO pretty safe. 2. The patch adds new code and doesn't really change any old code. 3. The patch adds a new sys file, which is needed by some user space tool to auto-setup the configuration for hibernation. We'd like to integrate the patch into the Linux distros ASAP and as we know some distros don't accept a patch if it's not in the mainline. So, if the patch itself looks good, IMO it would be better to be in v5.11 rather than in v5.12, which will need extra time of 2~3 months. That said, I don't know if there is any hard rule in regard of the timing here. If there is, then v5.12 is OK to me. :-) > Given this is a list of strings, do you want to enumerate them in a > Values section or the Description section? > > Wei. Currently there is only one possible string "hibernation". Not sure if we would need to add more strings in future, but yes it's a good idea to list the string in a "Value" section. Will post v2 shortly. Thanks, -- Dexuan
Re: [PATCH] Drivers: hv: vmbus: Add /sys/bus/vmbus/supported_features
On Tue, Dec 22, 2020 at 04:12:22PM -0800, Dexuan Cui wrote: > When a Linux VM runs on Hyper-V, if the host toolstack doesn't support > hibernation for the VM (this happens on old Hyper-V hosts like Windows > Server 2016, or new Hyper-V hosts if the admin or user doesn't declare > the hibernation intent for the VM), the VM is discouraged from trying > hibernation (because the host doesn't guarantee that the VM's virtual > hardware configuration will remain exactly the same across hibernation), > i.e. the VM should not try to set up the swap partition/file for > hibernation, etc. > > x86 Hyper-V uses the presence of the virtual ACPI S4 state as the > indication of the host toolstack support for a VM. Currently there is > no easy and reliable way for the userspace to detect the presence of > the state (see https://lkml.org/lkml/2020/12/11/1097). Add > /sys/bus/vmbus/supported_features for this purpose. > > Signed-off-by: Dexuan Cui > --- > Documentation/ABI/stable/sysfs-bus-vmbus | 7 +++ > drivers/hv/vmbus_drv.c | 20 > 2 files changed, 27 insertions(+) > > diff --git a/Documentation/ABI/stable/sysfs-bus-vmbus > b/Documentation/ABI/stable/sysfs-bus-vmbus > index c27b7b89477c..3ba765ae6695 100644 > --- a/Documentation/ABI/stable/sysfs-bus-vmbus > +++ b/Documentation/ABI/stable/sysfs-bus-vmbus > @@ -1,3 +1,10 @@ > +What:/sys/bus/vmbus/supported_features > +Date:Dec 2020 > +KernelVersion: 5.11 Too late for 5.11 now. Given this is a list of strings, do you want to enumerate them in a Values section or the Description section? Wei.
[PATCH] Drivers: hv: vmbus: Add /sys/bus/vmbus/supported_features
When a Linux VM runs on Hyper-V, if the host toolstack doesn't support hibernation for the VM (this happens on old Hyper-V hosts like Windows Server 2016, or new Hyper-V hosts if the admin or user doesn't declare the hibernation intent for the VM), the VM is discouraged from trying hibernation (because the host doesn't guarantee that the VM's virtual hardware configuration will remain exactly the same across hibernation), i.e. the VM should not try to set up the swap partition/file for hibernation, etc. x86 Hyper-V uses the presence of the virtual ACPI S4 state as the indication of the host toolstack support for a VM. Currently there is no easy and reliable way for the userspace to detect the presence of the state (see https://lkml.org/lkml/2020/12/11/1097). Add /sys/bus/vmbus/supported_features for this purpose. Signed-off-by: Dexuan Cui --- Documentation/ABI/stable/sysfs-bus-vmbus | 7 +++ drivers/hv/vmbus_drv.c | 20 2 files changed, 27 insertions(+) diff --git a/Documentation/ABI/stable/sysfs-bus-vmbus b/Documentation/ABI/stable/sysfs-bus-vmbus index c27b7b89477c..3ba765ae6695 100644 --- a/Documentation/ABI/stable/sysfs-bus-vmbus +++ b/Documentation/ABI/stable/sysfs-bus-vmbus @@ -1,3 +1,10 @@ +What: /sys/bus/vmbus/supported_features +Date: Dec 2020 +KernelVersion: 5.11 +Contact: Dexuan Cui +Description: Features specific to VMs running on Hyper-V +Users: Daemon that sets up swap partition/file for hibernation + What: /sys/bus/vmbus/devices//id Date: Jul 2009 KernelVersion: 2.6.31 diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c index d491fdcee61f..958487a40a18 100644 --- a/drivers/hv/vmbus_drv.c +++ b/drivers/hv/vmbus_drv.c @@ -678,6 +678,25 @@ static const struct attribute_group vmbus_dev_group = { }; __ATTRIBUTE_GROUPS(vmbus_dev); +/* Set up bus attribute(s) for /sys/bus/vmbus/supported_features */ +static ssize_t supported_features_show(struct bus_type *bus, char *buf) +{ + bool hb = hv_is_hibernation_supported(); + + return sprintf(buf, "%s\n", hb ? "hibernation" : ""); +} + +static BUS_ATTR_RO(supported_features); + +static struct attribute *vmbus_bus_attrs[] = { + &bus_attr_supported_features.attr, + NULL, +}; +static const struct attribute_group vmbus_bus_group = { + .attrs = vmbus_bus_attrs, +}; +__ATTRIBUTE_GROUPS(vmbus_bus); + /* * vmbus_uevent - add uevent for our device * @@ -1024,6 +1043,7 @@ static struct bus_type hv_bus = { .uevent = vmbus_uevent, .dev_groups = vmbus_dev_groups, .drv_groups = vmbus_drv_groups, + .bus_groups = vmbus_bus_groups, .pm = &vmbus_pm, }; -- 2.19.1