* [PATCH 0/3] Disable Suspend-to-Idle in Hyper-V and Fix Hibernation Interruptions
@ 2024-09-12 21:27 Erni Sri Satya Vennela
2024-09-12 21:27 ` [PATCH 1/3] Drivers: hv: vmbus: Disable Suspend-to-Idle for VMBus Erni Sri Satya Vennela
` (3 more replies)
0 siblings, 4 replies; 19+ messages in thread
From: Erni Sri Satya Vennela @ 2024-09-12 21:27 UTC (permalink / raw)
To: kys, haiyangz, wei.liu, decui, jikos, bentiss, dmitry.torokhov,
linux-hyperv, linux-input, linux-kernel
Cc: ernis, Erni Sri Satya Vennela
It has been reported that Hyper-V VM users can unintentionally abort
hibernation by mouse or keyboard movements. To address this issue,
we have decided to remove the wakeup events for the Hyper-V keyboard
and mouse driver. However, this change introduces another problem:
Suspend-to-Idle brings the system down with no method to wake it back up.
Given that there are no real users of Suspend-to-Idle in Hyper-V,
we have decided to disable this feature for VMBus. This results in:
$echo freeze > /sys/power/state
> bash: echo: write error: Operation not supported
The keyboard and mouse were previously registered as wakeup sources to
interrupt the freeze operation in a VM. Since the freeze operation itself
is no longer supported, we are disabling them as wakeup events.
This patchset ensures that the system remains stable and prevents
unintended interruptions during hibernation.
Erni Sri Satya Vennela (3):
Drivers: hv: vmbus: Disable Suspend-to-Idle for VMBus
Revert "Input: hyperv-keyboard - register as a wakeup source"
Revert "HID: hyperv: register as a wakeup source"
drivers/hid/hid-hyperv.c | 6 ------
drivers/hv/vmbus_drv.c | 15 ++++++++++++++-
drivers/input/serio/hyperv-keyboard.c | 12 ------------
3 files changed, 14 insertions(+), 19 deletions(-)
--
2.34.1
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 1/3] Drivers: hv: vmbus: Disable Suspend-to-Idle for VMBus 2024-09-12 21:27 [PATCH 0/3] Disable Suspend-to-Idle in Hyper-V and Fix Hibernation Interruptions Erni Sri Satya Vennela @ 2024-09-12 21:27 ` Erni Sri Satya Vennela 2024-09-13 6:38 ` Saurabh Singh Sengar ` (4 more replies) 2024-09-12 21:27 ` [PATCH 2/3] Revert "Input: hyperv-keyboard - register as a wakeup source" Erni Sri Satya Vennela ` (2 subsequent siblings) 3 siblings, 5 replies; 19+ messages in thread From: Erni Sri Satya Vennela @ 2024-09-12 21:27 UTC (permalink / raw) To: kys, haiyangz, wei.liu, decui, jikos, bentiss, dmitry.torokhov, linux-hyperv, linux-input, linux-kernel Cc: ernis, Erni Sri Satya Vennela, Saurabh Sengar If the Virtual Machine Connection window is focused, a Hyper-V VM user can unintentionally touch the keyboard/mouse when the VM is hibernating or resuming, and consequently the hibernation or resume operation can be aborted unexpectedly. Fix the issue by no longer registering the keyboard/mouse as wakeup devices (see the other two patches for the changes to drivers/input/serio/hyperv-keyboard.c and drivers/hid/hid-hyperv.c). The keyboard/mouse were registered as wakeup devices because the VM needs to be woken up from the Suspend-to-Idle state after a user runs "echo freeze > /sys/power/state". It seems like the Suspend-to-Idle feature has no real users in practice, so let's no longer support that by returning -EOPNOTSUPP if a user tries to use that. Signed-off-by: Saurabh Sengar <ssengar@linux.microsoft.com> Signed-off-by: Erni Sri Satya Vennela <ernis@linux.microsoft.com> --- drivers/hv/vmbus_drv.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c index 965d2a4efb7e..4efd8856392f 100644 --- a/drivers/hv/vmbus_drv.c +++ b/drivers/hv/vmbus_drv.c @@ -900,6 +900,19 @@ static void vmbus_shutdown(struct device *child_device) } #ifdef CONFIG_PM_SLEEP +/* + * vmbus_freeze - Suspend-to-Idle + */ +static int vmbus_freeze(struct device *child_device) +{ +/* + * Do not support Suspend-to-Idle ("echo freeze > /sys/power/state") as + * that would require registering the Hyper-V synthetic mouse/keyboard + * devices as wakeup devices, which can abort hibernation/resume unexpectedly. + */ + return -EOPNOTSUPP; +} + /* * vmbus_suspend - Suspend a vmbus device */ @@ -969,7 +982,7 @@ static void vmbus_device_release(struct device *device) */ static const struct dev_pm_ops vmbus_pm = { - .suspend_noirq = NULL, + .suspend_noirq = vmbus_freeze, .resume_noirq = NULL, .freeze_noirq = vmbus_suspend, .thaw_noirq = vmbus_resume, -- 2.34.1 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH 1/3] Drivers: hv: vmbus: Disable Suspend-to-Idle for VMBus 2024-09-12 21:27 ` [PATCH 1/3] Drivers: hv: vmbus: Disable Suspend-to-Idle for VMBus Erni Sri Satya Vennela @ 2024-09-13 6:38 ` Saurabh Singh Sengar 2024-09-13 7:19 ` Naman Jain ` (3 subsequent siblings) 4 siblings, 0 replies; 19+ messages in thread From: Saurabh Singh Sengar @ 2024-09-13 6:38 UTC (permalink / raw) To: Erni Sri Satya Vennela Cc: kys, haiyangz, wei.liu, decui, jikos, bentiss, dmitry.torokhov, linux-hyperv, linux-input, linux-kernel, ernis On Thu, Sep 12, 2024 at 02:27:48PM -0700, Erni Sri Satya Vennela wrote: > If the Virtual Machine Connection window is focused, > a Hyper-V VM user can unintentionally touch the keyboard/mouse > when the VM is hibernating or resuming, and consequently the > hibernation or resume operation can be aborted unexpectedly. > Fix the issue by no longer registering the keyboard/mouse as > wakeup devices (see the other two patches for the > changes to drivers/input/serio/hyperv-keyboard.c and > drivers/hid/hid-hyperv.c). > > The keyboard/mouse were registered as wakeup devices because the > VM needs to be woken up from the Suspend-to-Idle state after > a user runs "echo freeze > /sys/power/state". It seems like > the Suspend-to-Idle feature has no real users in practice, so > let's no longer support that by returning -EOPNOTSUPP if a > user tries to use that. > > Signed-off-by: Saurabh Sengar <ssengar@linux.microsoft.com> > Signed-off-by: Erni Sri Satya Vennela <ernis@linux.microsoft.com> Can we have a fixes tag ? My vote is to backport this upto this commit atleast: https://lore.kernel.org/all/1586663435-36243-1-git-send-email-decui@microsoft.com/ - Saurabh > --- > drivers/hv/vmbus_drv.c | 15 ++++++++++++++- > 1 file changed, 14 insertions(+), 1 deletion(-) > > diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c > index 965d2a4efb7e..4efd8856392f 100644 > --- a/drivers/hv/vmbus_drv.c > +++ b/drivers/hv/vmbus_drv.c > @@ -900,6 +900,19 @@ static void vmbus_shutdown(struct device *child_device) > } > > #ifdef CONFIG_PM_SLEEP > +/* > + * vmbus_freeze - Suspend-to-Idle > + */ > +static int vmbus_freeze(struct device *child_device) > +{ > +/* > + * Do not support Suspend-to-Idle ("echo freeze > /sys/power/state") as > + * that would require registering the Hyper-V synthetic mouse/keyboard > + * devices as wakeup devices, which can abort hibernation/resume unexpectedly. > + */ > + return -EOPNOTSUPP; > +} > + > /* > * vmbus_suspend - Suspend a vmbus device > */ > @@ -969,7 +982,7 @@ static void vmbus_device_release(struct device *device) > */ > > static const struct dev_pm_ops vmbus_pm = { > - .suspend_noirq = NULL, > + .suspend_noirq = vmbus_freeze, > .resume_noirq = NULL, > .freeze_noirq = vmbus_suspend, > .thaw_noirq = vmbus_resume, > -- > 2.34.1 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/3] Drivers: hv: vmbus: Disable Suspend-to-Idle for VMBus 2024-09-12 21:27 ` [PATCH 1/3] Drivers: hv: vmbus: Disable Suspend-to-Idle for VMBus Erni Sri Satya Vennela 2024-09-13 6:38 ` Saurabh Singh Sengar @ 2024-09-13 7:19 ` Naman Jain 2024-09-21 19:09 ` Erni Sri Satya Vennela 2024-09-13 17:01 ` kernel test robot ` (2 subsequent siblings) 4 siblings, 1 reply; 19+ messages in thread From: Naman Jain @ 2024-09-13 7:19 UTC (permalink / raw) To: Erni Sri Satya Vennela, kys, haiyangz, wei.liu, decui, jikos, bentiss, dmitry.torokhov, linux-hyperv, linux-input, linux-kernel Cc: ernis, Saurabh Sengar On 9/13/2024 2:57 AM, Erni Sri Satya Vennela wrote: > If the Virtual Machine Connection window is focused, > a Hyper-V VM user can unintentionally touch the keyboard/mouse > when the VM is hibernating or resuming, and consequently the > hibernation or resume operation can be aborted unexpectedly. > Fix the issue by no longer registering the keyboard/mouse as > wakeup devices (see the other two patches for the > changes to drivers/input/serio/hyperv-keyboard.c and > drivers/hid/hid-hyperv.c). > > The keyboard/mouse were registered as wakeup devices because the > VM needs to be woken up from the Suspend-to-Idle state after > a user runs "echo freeze > /sys/power/state". It seems like > the Suspend-to-Idle feature has no real users in practice, so > let's no longer support that by returning -EOPNOTSUPP if a > user tries to use that. > Maybe it would be good to capture here the kind of VMs that this is going to be not supported - HyperV based VMs. You mentioned it in cover letter, but it would be good to add it here as well, as cover letter does not go to git log. Also, the subject suggests that we are disabling suspend-to-idle for vmbus specifically, but from commit description, it suggests that "suspend to idle" feature itself is no longer supported on these particular VMs. Please rephrase it based on what exactly we are trying to do here. IIUC, we are now returning an error (EOPNOTSUPP) in vmbus driver callback, which insures that we don't support Suspend-to-Idle in these VMs. > Signed-off-by: Saurabh Sengar <ssengar@linux.microsoft.com> > Signed-off-by: Erni Sri Satya Vennela <ernis@linux.microsoft.com> > --- > drivers/hv/vmbus_drv.c | 15 ++++++++++++++- > 1 file changed, 14 insertions(+), 1 deletion(-) > > diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c > index 965d2a4efb7e..4efd8856392f 100644 > --- a/drivers/hv/vmbus_drv.c > +++ b/drivers/hv/vmbus_drv.c > @@ -900,6 +900,19 @@ static void vmbus_shutdown(struct device *child_device) > } > > #ifdef CONFIG_PM_SLEEP > +/* > + * vmbus_freeze - Suspend-to-Idle > + */ > +static int vmbus_freeze(struct device *child_device) > +{ > +/* > + * Do not support Suspend-to-Idle ("echo freeze > /sys/power/state") as > + * that would require registering the Hyper-V synthetic mouse/keyboard > + * devices as wakeup devices, which can abort hibernation/resume unexpectedly. > + */ > + return -EOPNOTSUPP; > +} > + > /* > * vmbus_suspend - Suspend a vmbus device > */ > @@ -969,7 +982,7 @@ static void vmbus_device_release(struct device *device) > */ > > static const struct dev_pm_ops vmbus_pm = { > - .suspend_noirq = NULL, > + .suspend_noirq = vmbus_freeze, > .resume_noirq = NULL, > .freeze_noirq = vmbus_suspend, I am not sure if this is OK or how it works, but this naming scheme seems a bit confusing to me - *suspend* -> vmbus_*freeze* *freeze* -> vmbus_*suspend* and we are removing support for "freeze" by returning EOPNOTSUPP in suspend callback. I'll try to understand more on this, but just see if its OK. > .thaw_noirq = vmbus_resume, Regards, Naman ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/3] Drivers: hv: vmbus: Disable Suspend-to-Idle for VMBus 2024-09-13 7:19 ` Naman Jain @ 2024-09-21 19:09 ` Erni Sri Satya Vennela 0 siblings, 0 replies; 19+ messages in thread From: Erni Sri Satya Vennela @ 2024-09-21 19:09 UTC (permalink / raw) To: Naman Jain Cc: kys, haiyangz, wei.liu, decui, jikos, bentiss, dmitry.torokhov, linux-hyperv, linux-input, linux-kernel, ernis, Saurabh Sengar On Fri, Sep 13, 2024 at 12:49:27PM +0530, Naman Jain wrote: > > > On 9/13/2024 2:57 AM, Erni Sri Satya Vennela wrote: > >If the Virtual Machine Connection window is focused, > >a Hyper-V VM user can unintentionally touch the keyboard/mouse > >when the VM is hibernating or resuming, and consequently the > >hibernation or resume operation can be aborted unexpectedly. > >Fix the issue by no longer registering the keyboard/mouse as > >wakeup devices (see the other two patches for the > >changes to drivers/input/serio/hyperv-keyboard.c and > >drivers/hid/hid-hyperv.c). > > > >The keyboard/mouse were registered as wakeup devices because the > >VM needs to be woken up from the Suspend-to-Idle state after > >a user runs "echo freeze > /sys/power/state". It seems like > >the Suspend-to-Idle feature has no real users in practice, so > >let's no longer support that by returning -EOPNOTSUPP if a > >user tries to use that. > > > > Maybe it would be good to capture here the kind of VMs that this is > going to be not supported - HyperV based VMs. You mentioned it in cover > letter, but it would be good to add it here as well, as cover letter > does not go to git log. > Sure, I'll specify this in the next version of the patch. > Also, the subject suggests that we are disabling suspend-to-idle for > vmbus specifically, but from commit description, it suggests that > "suspend to idle" feature itself is no longer supported on these > particular VMs. Please rephrase it based on what exactly we are trying > to do here. IIUC, we are now returning an error (EOPNOTSUPP) in vmbus > driver callback, which insures that we don't support Suspend-to-Idle in > these VMs. > Yes, that's correct. > >Signed-off-by: Saurabh Sengar <ssengar@linux.microsoft.com> > >Signed-off-by: Erni Sri Satya Vennela <ernis@linux.microsoft.com> > >--- > > drivers/hv/vmbus_drv.c | 15 ++++++++++++++- > > 1 file changed, 14 insertions(+), 1 deletion(-) > > > >diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c > >index 965d2a4efb7e..4efd8856392f 100644 > >--- a/drivers/hv/vmbus_drv.c > >+++ b/drivers/hv/vmbus_drv.c > >@@ -900,6 +900,19 @@ static void vmbus_shutdown(struct device *child_device) > > } > > #ifdef CONFIG_PM_SLEEP > >+/* > >+ * vmbus_freeze - Suspend-to-Idle > >+ */ > >+static int vmbus_freeze(struct device *child_device) > >+{ > >+/* > >+ * Do not support Suspend-to-Idle ("echo freeze > /sys/power/state") as > >+ * that would require registering the Hyper-V synthetic mouse/keyboard > >+ * devices as wakeup devices, which can abort hibernation/resume unexpectedly. > >+ */ > >+ return -EOPNOTSUPP; > >+} > >+ > > /* > > * vmbus_suspend - Suspend a vmbus device > > */ > >@@ -969,7 +982,7 @@ static void vmbus_device_release(struct device *device) > > */ > > static const struct dev_pm_ops vmbus_pm = { > >- .suspend_noirq = NULL, > >+ .suspend_noirq = vmbus_freeze, > > .resume_noirq = NULL, > > .freeze_noirq = vmbus_suspend, > > I am not sure if this is OK or how it works, but this naming scheme > seems a bit confusing to me - > *suspend* -> vmbus_*freeze* > *freeze* -> vmbus_*suspend* > and we are removing support for "freeze" by returning EOPNOTSUPP in > suspend callback. AFAIU, suspend_noirq is used for Suspend-to-Idle operation and we use "freeze > /sys/power/state" for the same. Maybe the naming convention comes that way. The key point to understand here is suspend_noirq prepares the machine to low power state and freeze_noirq prepares the machine for hibernation (saving state to disk). > > I'll try to understand more on this, but just see if its OK. > > > .thaw_noirq = vmbus_resume, > > Regards, > Naman Yes, thaw_noirq is to restore devices from hibernation state. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/3] Drivers: hv: vmbus: Disable Suspend-to-Idle for VMBus 2024-09-12 21:27 ` [PATCH 1/3] Drivers: hv: vmbus: Disable Suspend-to-Idle for VMBus Erni Sri Satya Vennela 2024-09-13 6:38 ` Saurabh Singh Sengar 2024-09-13 7:19 ` Naman Jain @ 2024-09-13 17:01 ` kernel test robot 2024-09-13 17:43 ` kernel test robot 2024-09-24 3:27 ` Srivatsa S. Bhat 4 siblings, 0 replies; 19+ messages in thread From: kernel test robot @ 2024-09-13 17:01 UTC (permalink / raw) To: Erni Sri Satya Vennela, kys, haiyangz, wei.liu, decui, jikos, bentiss, dmitry.torokhov, linux-hyperv, linux-input, linux-kernel Cc: oe-kbuild-all, ernis, Erni Sri Satya Vennela, Saurabh Sengar Hi Erni, kernel test robot noticed the following build errors: [auto build test ERROR on hid/for-next] [also build test ERROR on dtor-input/next dtor-input/for-linus linus/master v6.11-rc7 next-20240913] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Erni-Sri-Satya-Vennela/Drivers-hv-vmbus-Disable-Suspend-to-Idle-for-VMBus/20240913-053127 base: https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git for-next patch link: https://lore.kernel.org/r/1726176470-13133-2-git-send-email-ernis%40linux.microsoft.com patch subject: [PATCH 1/3] Drivers: hv: vmbus: Disable Suspend-to-Idle for VMBus config: x86_64-randconfig-161-20240913 (https://download.01.org/0day-ci/archive/20240914/202409140042.imFE8dSL-lkp@intel.com/config) compiler: clang version 18.1.8 (https://github.com/llvm/llvm-project 3b5b5c1ec4a3095ab096dd780e84d7ab81f3d7ff) reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240914/202409140042.imFE8dSL-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202409140042.imFE8dSL-lkp@intel.com/ All errors (new ones prefixed by >>): >> drivers/hv/vmbus_drv.c:985:20: error: use of undeclared identifier 'vmbus_freeze' 985 | .suspend_noirq = vmbus_freeze, | ^ drivers/hv/vmbus_drv.c:1913:42: warning: shift count >= width of type [-Wshift-count-overflow] 1913 | dma_set_mask(&child_device_obj->device, DMA_BIT_MASK(64)); | ^~~~~~~~~~~~~~~~ include/linux/dma-mapping.h:77:54: note: expanded from macro 'DMA_BIT_MASK' 77 | #define DMA_BIT_MASK(n) (((n) == 64) ? ~0ULL : ((1ULL<<(n))-1)) | ^ ~~~ 1 warning and 1 error generated. vim +/vmbus_freeze +985 drivers/hv/vmbus_drv.c 973 974 /* 975 * Note: we must use the "noirq" ops: see the comment before vmbus_bus_pm. 976 * 977 * suspend_noirq/resume_noirq are set to NULL to support Suspend-to-Idle: we 978 * shouldn't suspend the vmbus devices upon Suspend-to-Idle, otherwise there 979 * is no way to wake up a Generation-2 VM. 980 * 981 * The other 4 ops are for hibernation. 982 */ 983 984 static const struct dev_pm_ops vmbus_pm = { > 985 .suspend_noirq = vmbus_freeze, 986 .resume_noirq = NULL, 987 .freeze_noirq = vmbus_suspend, 988 .thaw_noirq = vmbus_resume, 989 .poweroff_noirq = vmbus_suspend, 990 .restore_noirq = vmbus_resume, 991 }; 992 -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/3] Drivers: hv: vmbus: Disable Suspend-to-Idle for VMBus 2024-09-12 21:27 ` [PATCH 1/3] Drivers: hv: vmbus: Disable Suspend-to-Idle for VMBus Erni Sri Satya Vennela ` (2 preceding siblings ...) 2024-09-13 17:01 ` kernel test robot @ 2024-09-13 17:43 ` kernel test robot 2024-09-24 3:27 ` Srivatsa S. Bhat 4 siblings, 0 replies; 19+ messages in thread From: kernel test robot @ 2024-09-13 17:43 UTC (permalink / raw) To: Erni Sri Satya Vennela, kys, haiyangz, wei.liu, decui, jikos, bentiss, dmitry.torokhov, linux-hyperv, linux-input, linux-kernel Cc: oe-kbuild-all, ernis, Erni Sri Satya Vennela, Saurabh Sengar Hi Erni, kernel test robot noticed the following build errors: [auto build test ERROR on hid/for-next] [also build test ERROR on dtor-input/next dtor-input/for-linus linus/master v6.11-rc7 next-20240913] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Erni-Sri-Satya-Vennela/Drivers-hv-vmbus-Disable-Suspend-to-Idle-for-VMBus/20240913-053127 base: https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git for-next patch link: https://lore.kernel.org/r/1726176470-13133-2-git-send-email-ernis%40linux.microsoft.com patch subject: [PATCH 1/3] Drivers: hv: vmbus: Disable Suspend-to-Idle for VMBus config: i386-randconfig-003-20240913 (https://download.01.org/0day-ci/archive/20240914/202409140137.5jZxplAp-lkp@intel.com/config) compiler: gcc-12 (Debian 12.2.0-14) 12.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240914/202409140137.5jZxplAp-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202409140137.5jZxplAp-lkp@intel.com/ All errors (new ones prefixed by >>): >> drivers/hv/vmbus_drv.c:985:27: error: 'vmbus_freeze' undeclared here (not in a function); did you mean 'vmbus_remove'? 985 | .suspend_noirq = vmbus_freeze, | ^~~~~~~~~~~~ | vmbus_remove vim +985 drivers/hv/vmbus_drv.c 973 974 /* 975 * Note: we must use the "noirq" ops: see the comment before vmbus_bus_pm. 976 * 977 * suspend_noirq/resume_noirq are set to NULL to support Suspend-to-Idle: we 978 * shouldn't suspend the vmbus devices upon Suspend-to-Idle, otherwise there 979 * is no way to wake up a Generation-2 VM. 980 * 981 * The other 4 ops are for hibernation. 982 */ 983 984 static const struct dev_pm_ops vmbus_pm = { > 985 .suspend_noirq = vmbus_freeze, 986 .resume_noirq = NULL, 987 .freeze_noirq = vmbus_suspend, 988 .thaw_noirq = vmbus_resume, 989 .poweroff_noirq = vmbus_suspend, 990 .restore_noirq = vmbus_resume, 991 }; 992 -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/3] Drivers: hv: vmbus: Disable Suspend-to-Idle for VMBus 2024-09-12 21:27 ` [PATCH 1/3] Drivers: hv: vmbus: Disable Suspend-to-Idle for VMBus Erni Sri Satya Vennela ` (3 preceding siblings ...) 2024-09-13 17:43 ` kernel test robot @ 2024-09-24 3:27 ` Srivatsa S. Bhat 4 siblings, 0 replies; 19+ messages in thread From: Srivatsa S. Bhat @ 2024-09-24 3:27 UTC (permalink / raw) To: Erni Sri Satya Vennela Cc: kys, haiyangz, wei.liu, decui, jikos, bentiss, dmitry.torokhov, linux-hyperv, linux-input, linux-kernel, ernis, Saurabh Sengar, rafael, pavel, lenb, linux-pm [+linux-pm, Rafael, Len, Pavel] On Thu, Sep 12, 2024 at 02:27:48PM -0700, Erni Sri Satya Vennela wrote: > If the Virtual Machine Connection window is focused, > a Hyper-V VM user can unintentionally touch the keyboard/mouse > when the VM is hibernating or resuming, and consequently the > hibernation or resume operation can be aborted unexpectedly. > Fix the issue by no longer registering the keyboard/mouse as > wakeup devices (see the other two patches for the > changes to drivers/input/serio/hyperv-keyboard.c and > drivers/hid/hid-hyperv.c). > > The keyboard/mouse were registered as wakeup devices because the > VM needs to be woken up from the Suspend-to-Idle state after > a user runs "echo freeze > /sys/power/state". It seems like > the Suspend-to-Idle feature has no real users in practice, so > let's no longer support that by returning -EOPNOTSUPP if a > user tries to use that. > > Signed-off-by: Saurabh Sengar <ssengar@linux.microsoft.com> > Signed-off-by: Erni Sri Satya Vennela <ernis@linux.microsoft.com> > --- > drivers/hv/vmbus_drv.c | 15 ++++++++++++++- > 1 file changed, 14 insertions(+), 1 deletion(-) > > diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c > index 965d2a4efb7e..4efd8856392f 100644 > --- a/drivers/hv/vmbus_drv.c > +++ b/drivers/hv/vmbus_drv.c > @@ -900,6 +900,19 @@ static void vmbus_shutdown(struct device *child_device) > } > > #ifdef CONFIG_PM_SLEEP > +/* > + * vmbus_freeze - Suspend-to-Idle > + */ > +static int vmbus_freeze(struct device *child_device) > +{ > +/* > + * Do not support Suspend-to-Idle ("echo freeze > /sys/power/state") as > + * that would require registering the Hyper-V synthetic mouse/keyboard > + * devices as wakeup devices, which can abort hibernation/resume unexpectedly. > + */ > + return -EOPNOTSUPP; > +} > + > /* > * vmbus_suspend - Suspend a vmbus device > */ > @@ -969,7 +982,7 @@ static void vmbus_device_release(struct device *device) > */ > > static const struct dev_pm_ops vmbus_pm = { > - .suspend_noirq = NULL, > + .suspend_noirq = vmbus_freeze, > .resume_noirq = NULL, > .freeze_noirq = vmbus_suspend, > .thaw_noirq = vmbus_resume, > -- > 2.34.1 > > ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 2/3] Revert "Input: hyperv-keyboard - register as a wakeup source" 2024-09-12 21:27 [PATCH 0/3] Disable Suspend-to-Idle in Hyper-V and Fix Hibernation Interruptions Erni Sri Satya Vennela 2024-09-12 21:27 ` [PATCH 1/3] Drivers: hv: vmbus: Disable Suspend-to-Idle for VMBus Erni Sri Satya Vennela @ 2024-09-12 21:27 ` Erni Sri Satya Vennela 2024-09-24 3:28 ` Srivatsa S. Bhat 2024-09-12 21:27 ` [PATCH 3/3] Revert "HID: hyperv: " Erni Sri Satya Vennela 2024-09-24 3:26 ` [PATCH 0/3] Disable Suspend-to-Idle in Hyper-V and Fix Hibernation Interruptions Srivatsa S. Bhat 3 siblings, 1 reply; 19+ messages in thread From: Erni Sri Satya Vennela @ 2024-09-12 21:27 UTC (permalink / raw) To: kys, haiyangz, wei.liu, decui, jikos, bentiss, dmitry.torokhov, linux-hyperv, linux-input, linux-kernel Cc: ernis, Erni Sri Satya Vennela This reverts commit 62238f3aadc9bc56da70100e19ec61b9f8d72a5f. Remove keyboard as wakeup source since Suspend-to-Idle feature is disabled. Signed-off-by: Erni Sri Satya Vennela <ernis@linux.microsoft.com> --- drivers/input/serio/hyperv-keyboard.c | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/drivers/input/serio/hyperv-keyboard.c b/drivers/input/serio/hyperv-keyboard.c index 31d9dacd2fd1..b42c546636bf 100644 --- a/drivers/input/serio/hyperv-keyboard.c +++ b/drivers/input/serio/hyperv-keyboard.c @@ -162,15 +162,6 @@ static void hv_kbd_on_receive(struct hv_device *hv_dev, serio_interrupt(kbd_dev->hv_serio, scan_code, 0); } spin_unlock_irqrestore(&kbd_dev->lock, flags); - - /* - * Only trigger a wakeup on key down, otherwise - * "echo freeze > /sys/power/state" can't really enter the - * state because the Enter-UP can trigger a wakeup at once. - */ - if (!(info & IS_BREAK)) - pm_wakeup_hard_event(&hv_dev->device); - break; default: @@ -356,9 +347,6 @@ static int hv_kbd_probe(struct hv_device *hv_dev, goto err_close_vmbus; serio_register_port(kbd_dev->hv_serio); - - device_init_wakeup(&hv_dev->device, true); - return 0; err_close_vmbus: -- 2.34.1 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH 2/3] Revert "Input: hyperv-keyboard - register as a wakeup source" 2024-09-12 21:27 ` [PATCH 2/3] Revert "Input: hyperv-keyboard - register as a wakeup source" Erni Sri Satya Vennela @ 2024-09-24 3:28 ` Srivatsa S. Bhat 2024-10-04 8:14 ` Dmitry Torokhov 0 siblings, 1 reply; 19+ messages in thread From: Srivatsa S. Bhat @ 2024-09-24 3:28 UTC (permalink / raw) To: Erni Sri Satya Vennela Cc: kys, haiyangz, wei.liu, decui, jikos, bentiss, dmitry.torokhov, linux-hyperv, linux-input, linux-kernel, ernis, rafael, pavel, lenb, linux-pm [+linux-pm, Rafael, Len, Pavel] On Thu, Sep 12, 2024 at 02:27:49PM -0700, Erni Sri Satya Vennela wrote: > This reverts commit 62238f3aadc9bc56da70100e19ec61b9f8d72a5f. > > Remove keyboard as wakeup source since Suspend-to-Idle feature > is disabled. > > Signed-off-by: Erni Sri Satya Vennela <ernis@linux.microsoft.com> > --- > drivers/input/serio/hyperv-keyboard.c | 12 ------------ > 1 file changed, 12 deletions(-) > > diff --git a/drivers/input/serio/hyperv-keyboard.c b/drivers/input/serio/hyperv-keyboard.c > index 31d9dacd2fd1..b42c546636bf 100644 > --- a/drivers/input/serio/hyperv-keyboard.c > +++ b/drivers/input/serio/hyperv-keyboard.c > @@ -162,15 +162,6 @@ static void hv_kbd_on_receive(struct hv_device *hv_dev, > serio_interrupt(kbd_dev->hv_serio, scan_code, 0); > } > spin_unlock_irqrestore(&kbd_dev->lock, flags); > - > - /* > - * Only trigger a wakeup on key down, otherwise > - * "echo freeze > /sys/power/state" can't really enter the > - * state because the Enter-UP can trigger a wakeup at once. > - */ > - if (!(info & IS_BREAK)) > - pm_wakeup_hard_event(&hv_dev->device); > - > break; > > default: > @@ -356,9 +347,6 @@ static int hv_kbd_probe(struct hv_device *hv_dev, > goto err_close_vmbus; > > serio_register_port(kbd_dev->hv_serio); > - > - device_init_wakeup(&hv_dev->device, true); > - > return 0; > > err_close_vmbus: > -- > 2.34.1 > > ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/3] Revert "Input: hyperv-keyboard - register as a wakeup source" 2024-09-24 3:28 ` Srivatsa S. Bhat @ 2024-10-04 8:14 ` Dmitry Torokhov 2024-10-17 13:44 ` Erni Sri Satya Vennela 0 siblings, 1 reply; 19+ messages in thread From: Dmitry Torokhov @ 2024-10-04 8:14 UTC (permalink / raw) To: Srivatsa S. Bhat Cc: Erni Sri Satya Vennela, kys, haiyangz, wei.liu, decui, jikos, bentiss, linux-hyperv, linux-input, linux-kernel, ernis, rafael, pavel, lenb, linux-pm On Tue, Sep 24, 2024 at 03:28:51AM +0000, Srivatsa S. Bhat wrote: > [+linux-pm, Rafael, Len, Pavel] > > On Thu, Sep 12, 2024 at 02:27:49PM -0700, Erni Sri Satya Vennela wrote: > > This reverts commit 62238f3aadc9bc56da70100e19ec61b9f8d72a5f. > > > > Remove keyboard as wakeup source since Suspend-to-Idle feature > > is disabled. > > > > Signed-off-by: Erni Sri Satya Vennela <ernis@linux.microsoft.com> > > --- > > drivers/input/serio/hyperv-keyboard.c | 12 ------------ > > 1 file changed, 12 deletions(-) > > > > diff --git a/drivers/input/serio/hyperv-keyboard.c b/drivers/input/serio/hyperv-keyboard.c > > index 31d9dacd2fd1..b42c546636bf 100644 > > --- a/drivers/input/serio/hyperv-keyboard.c > > +++ b/drivers/input/serio/hyperv-keyboard.c > > @@ -162,15 +162,6 @@ static void hv_kbd_on_receive(struct hv_device *hv_dev, > > serio_interrupt(kbd_dev->hv_serio, scan_code, 0); > > } > > spin_unlock_irqrestore(&kbd_dev->lock, flags); > > - > > - /* > > - * Only trigger a wakeup on key down, otherwise > > - * "echo freeze > /sys/power/state" can't really enter the > > - * state because the Enter-UP can trigger a wakeup at once. > > - */ > > - if (!(info & IS_BREAK)) > > - pm_wakeup_hard_event(&hv_dev->device); > > - > > break; > > > > default: > > @@ -356,9 +347,6 @@ static int hv_kbd_probe(struct hv_device *hv_dev, > > goto err_close_vmbus; > > > > serio_register_port(kbd_dev->hv_serio); > > - > > - device_init_wakeup(&hv_dev->device, true); If you do not want the keyboard to be a wakeup source by default maybe change this to: device_set_wakeup_capable(&hv_dev->device, true); and leave the rest of the driver alone? Same for the HID change. Thanks. -- Dmitry ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/3] Revert "Input: hyperv-keyboard - register as a wakeup source" 2024-10-04 8:14 ` Dmitry Torokhov @ 2024-10-17 13:44 ` Erni Sri Satya Vennela 2024-11-08 10:47 ` Erni Sri Satya Vennela 0 siblings, 1 reply; 19+ messages in thread From: Erni Sri Satya Vennela @ 2024-10-17 13:44 UTC (permalink / raw) To: Dmitry Torokhov Cc: Srivatsa S. Bhat, kys, haiyangz, wei.liu, decui, jikos, bentiss, linux-hyperv, linux-input, linux-kernel, ernis, rafael, pavel, lenb, linux-pm On Fri, Oct 04, 2024 at 01:14:10AM -0700, Dmitry Torokhov wrote: > On Tue, Sep 24, 2024 at 03:28:51AM +0000, Srivatsa S. Bhat wrote: > > [+linux-pm, Rafael, Len, Pavel] > > > > On Thu, Sep 12, 2024 at 02:27:49PM -0700, Erni Sri Satya Vennela wrote: > > > This reverts commit 62238f3aadc9bc56da70100e19ec61b9f8d72a5f. > > > > > > Remove keyboard as wakeup source since Suspend-to-Idle feature > > > is disabled. > > > > > > Signed-off-by: Erni Sri Satya Vennela <ernis@linux.microsoft.com> > > > --- > > > drivers/input/serio/hyperv-keyboard.c | 12 ------------ > > > 1 file changed, 12 deletions(-) > > > > > > diff --git a/drivers/input/serio/hyperv-keyboard.c b/drivers/input/serio/hyperv-keyboard.c > > > index 31d9dacd2fd1..b42c546636bf 100644 > > > --- a/drivers/input/serio/hyperv-keyboard.c > > > +++ b/drivers/input/serio/hyperv-keyboard.c > > > @@ -162,15 +162,6 @@ static void hv_kbd_on_receive(struct hv_device *hv_dev, > > > serio_interrupt(kbd_dev->hv_serio, scan_code, 0); > > > } > > > spin_unlock_irqrestore(&kbd_dev->lock, flags); > > > - > > > - /* > > > - * Only trigger a wakeup on key down, otherwise > > > - * "echo freeze > /sys/power/state" can't really enter the > > > - * state because the Enter-UP can trigger a wakeup at once. > > > - */ > > > - if (!(info & IS_BREAK)) > > > - pm_wakeup_hard_event(&hv_dev->device); > > > - > > > break; > > > > > > default: > > > @@ -356,9 +347,6 @@ static int hv_kbd_probe(struct hv_device *hv_dev, > > > goto err_close_vmbus; > > > > > > serio_register_port(kbd_dev->hv_serio); > > > - > > > - device_init_wakeup(&hv_dev->device, true); > > If you do not want the keyboard to be a wakeup source by default maybe > change this to: > > device_set_wakeup_capable(&hv_dev->device, true); > > and leave the rest of the driver alone? > > Same for the HID change. > > Thanks. > device_set_wakeup_capable() sets the @dev's power.can_wakeup flag and adds wakeup-related attributes in sysfs. Could you please help me understand why explicitly calling this function can be helpful in our use case? > -- > Dmitry ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/3] Revert "Input: hyperv-keyboard - register as a wakeup source" 2024-10-17 13:44 ` Erni Sri Satya Vennela @ 2024-11-08 10:47 ` Erni Sri Satya Vennela 2024-12-09 17:16 ` Saurabh Singh Sengar 0 siblings, 1 reply; 19+ messages in thread From: Erni Sri Satya Vennela @ 2024-11-08 10:47 UTC (permalink / raw) To: Dmitry Torokhov Cc: Srivatsa S. Bhat, kys, haiyangz, wei.liu, decui, jikos, bentiss, linux-hyperv, linux-input, linux-kernel, ernis, rafael, pavel, lenb, linux-pm On Thu, Oct 17, 2024 at 06:44:38AM -0700, Erni Sri Satya Vennela wrote: > On Fri, Oct 04, 2024 at 01:14:10AM -0700, Dmitry Torokhov wrote: > > On Tue, Sep 24, 2024 at 03:28:51AM +0000, Srivatsa S. Bhat wrote: > > > [+linux-pm, Rafael, Len, Pavel] > > > > > > On Thu, Sep 12, 2024 at 02:27:49PM -0700, Erni Sri Satya Vennela wrote: > > > > This reverts commit 62238f3aadc9bc56da70100e19ec61b9f8d72a5f. > > > > > > > > Remove keyboard as wakeup source since Suspend-to-Idle feature > > > > is disabled. > > > > > > > > Signed-off-by: Erni Sri Satya Vennela <ernis@linux.microsoft.com> > > > > --- > > > > drivers/input/serio/hyperv-keyboard.c | 12 ------------ > > > > 1 file changed, 12 deletions(-) > > > > > > > > diff --git a/drivers/input/serio/hyperv-keyboard.c b/drivers/input/serio/hyperv-keyboard.c > > > > index 31d9dacd2fd1..b42c546636bf 100644 > > > > --- a/drivers/input/serio/hyperv-keyboard.c > > > > +++ b/drivers/input/serio/hyperv-keyboard.c > > > > @@ -162,15 +162,6 @@ static void hv_kbd_on_receive(struct hv_device *hv_dev, > > > > serio_interrupt(kbd_dev->hv_serio, scan_code, 0); > > > > } > > > > spin_unlock_irqrestore(&kbd_dev->lock, flags); > > > > - > > > > - /* > > > > - * Only trigger a wakeup on key down, otherwise > > > > - * "echo freeze > /sys/power/state" can't really enter the > > > > - * state because the Enter-UP can trigger a wakeup at once. > > > > - */ > > > > - if (!(info & IS_BREAK)) > > > > - pm_wakeup_hard_event(&hv_dev->device); > > > > - > > > > break; > > > > > > > > default: > > > > @@ -356,9 +347,6 @@ static int hv_kbd_probe(struct hv_device *hv_dev, > > > > goto err_close_vmbus; > > > > > > > > serio_register_port(kbd_dev->hv_serio); > > > > - > > > > - device_init_wakeup(&hv_dev->device, true); > > > > If you do not want the keyboard to be a wakeup source by default maybe > > change this to: > > > > device_set_wakeup_capable(&hv_dev->device, true); > > > > and leave the rest of the driver alone? > > > > Same for the HID change. > > > > Thanks. > > > device_set_wakeup_capable() sets the @dev's power.can_wakeup flag and > adds wakeup-related attributes in sysfs. > > Could you please help me understand why explicitly calling this function > can be helpful in our use case? > > > -- > > Dmitry Just following up on this patch. Could you please help me understand the reason for the change? ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/3] Revert "Input: hyperv-keyboard - register as a wakeup source" 2024-11-08 10:47 ` Erni Sri Satya Vennela @ 2024-12-09 17:16 ` Saurabh Singh Sengar 2024-12-12 14:33 ` Erni Sri Satya Vennela 0 siblings, 1 reply; 19+ messages in thread From: Saurabh Singh Sengar @ 2024-12-09 17:16 UTC (permalink / raw) To: Erni Sri Satya Vennela Cc: Dmitry Torokhov, Srivatsa S. Bhat, kys, haiyangz, wei.liu, decui, jikos, bentiss, linux-hyperv, linux-input, linux-kernel, ernis, rafael, pavel, lenb, linux-pm On Fri, Nov 08, 2024 at 02:47:41AM -0800, Erni Sri Satya Vennela wrote: > On Thu, Oct 17, 2024 at 06:44:38AM -0700, Erni Sri Satya Vennela wrote: > > On Fri, Oct 04, 2024 at 01:14:10AM -0700, Dmitry Torokhov wrote: > > > On Tue, Sep 24, 2024 at 03:28:51AM +0000, Srivatsa S. Bhat wrote: > > > > [+linux-pm, Rafael, Len, Pavel] > > > > > > > > On Thu, Sep 12, 2024 at 02:27:49PM -0700, Erni Sri Satya Vennela wrote: > > > > > This reverts commit 62238f3aadc9bc56da70100e19ec61b9f8d72a5f. > > > > > > > > > > Remove keyboard as wakeup source since Suspend-to-Idle feature > > > > > is disabled. > > > > > > > > > > Signed-off-by: Erni Sri Satya Vennela <ernis@linux.microsoft.com> > > > > > --- > > > > > drivers/input/serio/hyperv-keyboard.c | 12 ------------ > > > > > 1 file changed, 12 deletions(-) > > > > > > > > > > diff --git a/drivers/input/serio/hyperv-keyboard.c b/drivers/input/serio/hyperv-keyboard.c > > > > > index 31d9dacd2fd1..b42c546636bf 100644 > > > > > --- a/drivers/input/serio/hyperv-keyboard.c > > > > > +++ b/drivers/input/serio/hyperv-keyboard.c > > > > > @@ -162,15 +162,6 @@ static void hv_kbd_on_receive(struct hv_device *hv_dev, > > > > > serio_interrupt(kbd_dev->hv_serio, scan_code, 0); > > > > > } > > > > > spin_unlock_irqrestore(&kbd_dev->lock, flags); > > > > > - > > > > > - /* > > > > > - * Only trigger a wakeup on key down, otherwise > > > > > - * "echo freeze > /sys/power/state" can't really enter the > > > > > - * state because the Enter-UP can trigger a wakeup at once. > > > > > - */ > > > > > - if (!(info & IS_BREAK)) > > > > > - pm_wakeup_hard_event(&hv_dev->device); > > > > > - > > > > > break; > > > > > > > > > > default: > > > > > @@ -356,9 +347,6 @@ static int hv_kbd_probe(struct hv_device *hv_dev, > > > > > goto err_close_vmbus; > > > > > > > > > > serio_register_port(kbd_dev->hv_serio); > > > > > - > > > > > - device_init_wakeup(&hv_dev->device, true); > > > > > > If you do not want the keyboard to be a wakeup source by default maybe > > > change this to: > > > > > > device_set_wakeup_capable(&hv_dev->device, true); > > > > > > and leave the rest of the driver alone? > > > > > > Same for the HID change. > > > > > > Thanks. > > > > > device_set_wakeup_capable() sets the @dev's power.can_wakeup flag and > > adds wakeup-related attributes in sysfs. > > > > Could you please help me understand why explicitly calling this function > > can be helpful in our use case? > > > > > -- > > > Dmitry > Just following up on this patch. Could you please help me understand the > reason for the change? Vennela, There is a difference between "wakeup source registration" and "wakeup capable". For this there are two flags defined in power management framework: 1. power.wakeup 2. power.can_wakeup More details on these flags can be read here: https://www.kernel.org/doc/html/v6.12/driver-api/pm/devices.html 'device_init_wakeup(dev, true)' sets both; ie it registers the device as a wakeup source and marks it as wakeup capable too. In our case, the device is "wakeup capable" but we do not want to "register it as a wakeup source". 'device_set_wakeup_capable(dev, true)' is more appropriate because this marks the device as wakeup capable but doesn't register it as a wakeup source knowingly. I understand that Dimitry suggests not to revert the entire patch but to replace 'device_init_wakeup' with 'device_set_wakeup_capable', to mark the device as capable of wakeup but knowingly skipping the registering part. Requesting Dimitry to correct me if there is any misinterpretation. While fixing this in next version, please fix the kernel bot warings as well reported for 1/3 patch of this series. - Saurabh ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/3] Revert "Input: hyperv-keyboard - register as a wakeup source" 2024-12-09 17:16 ` Saurabh Singh Sengar @ 2024-12-12 14:33 ` Erni Sri Satya Vennela 0 siblings, 0 replies; 19+ messages in thread From: Erni Sri Satya Vennela @ 2024-12-12 14:33 UTC (permalink / raw) To: Saurabh Singh Sengar Cc: Dmitry Torokhov, Srivatsa S. Bhat, kys, haiyangz, wei.liu, decui, jikos, bentiss, linux-hyperv, linux-input, linux-kernel, ernis, rafael, pavel, lenb, linux-pm On Mon, Dec 09, 2024 at 09:16:23AM -0800, Saurabh Singh Sengar wrote: > On Fri, Nov 08, 2024 at 02:47:41AM -0800, Erni Sri Satya Vennela wrote: > > On Thu, Oct 17, 2024 at 06:44:38AM -0700, Erni Sri Satya Vennela wrote: > > > On Fri, Oct 04, 2024 at 01:14:10AM -0700, Dmitry Torokhov wrote: > > > > On Tue, Sep 24, 2024 at 03:28:51AM +0000, Srivatsa S. Bhat wrote: > > > > > [+linux-pm, Rafael, Len, Pavel] > > > > > > > > > > On Thu, Sep 12, 2024 at 02:27:49PM -0700, Erni Sri Satya Vennela wrote: > > > > > > This reverts commit 62238f3aadc9bc56da70100e19ec61b9f8d72a5f. > > > > > > > > > > > > Remove keyboard as wakeup source since Suspend-to-Idle feature > > > > > > is disabled. > > > > > > > > > > > > Signed-off-by: Erni Sri Satya Vennela <ernis@linux.microsoft.com> > > > > > > --- > > > > > > drivers/input/serio/hyperv-keyboard.c | 12 ------------ > > > > > > 1 file changed, 12 deletions(-) > > > > > > > > > > > > diff --git a/drivers/input/serio/hyperv-keyboard.c b/drivers/input/serio/hyperv-keyboard.c > > > > > > index 31d9dacd2fd1..b42c546636bf 100644 > > > > > > --- a/drivers/input/serio/hyperv-keyboard.c > > > > > > +++ b/drivers/input/serio/hyperv-keyboard.c > > > > > > @@ -162,15 +162,6 @@ static void hv_kbd_on_receive(struct hv_device *hv_dev, > > > > > > serio_interrupt(kbd_dev->hv_serio, scan_code, 0); > > > > > > } > > > > > > spin_unlock_irqrestore(&kbd_dev->lock, flags); > > > > > > - > > > > > > - /* > > > > > > - * Only trigger a wakeup on key down, otherwise > > > > > > - * "echo freeze > /sys/power/state" can't really enter the > > > > > > - * state because the Enter-UP can trigger a wakeup at once. > > > > > > - */ > > > > > > - if (!(info & IS_BREAK)) > > > > > > - pm_wakeup_hard_event(&hv_dev->device); > > > > > > - > > > > > > break; > > > > > > > > > > > > default: > > > > > > @@ -356,9 +347,6 @@ static int hv_kbd_probe(struct hv_device *hv_dev, > > > > > > goto err_close_vmbus; > > > > > > > > > > > > serio_register_port(kbd_dev->hv_serio); > > > > > > - > > > > > > - device_init_wakeup(&hv_dev->device, true); > > > > > > > > If you do not want the keyboard to be a wakeup source by default maybe > > > > change this to: > > > > > > > > device_set_wakeup_capable(&hv_dev->device, true); > > > > > > > > and leave the rest of the driver alone? > > > > > > > > Same for the HID change. > > > > > > > > Thanks. > > > > > > > device_set_wakeup_capable() sets the @dev's power.can_wakeup flag and > > > adds wakeup-related attributes in sysfs. > > > > > > Could you please help me understand why explicitly calling this function > > > can be helpful in our use case? > > > > > > > -- > > > > Dmitry > > Just following up on this patch. Could you please help me understand the > > reason for the change? > > > Vennela, > > There is a difference between "wakeup source registration" and "wakeup capable". > For this there are two flags defined in power management framework: > 1. power.wakeup > 2. power.can_wakeup > > More details on these flags can be read here: > https://www.kernel.org/doc/html/v6.12/driver-api/pm/devices.html > > 'device_init_wakeup(dev, true)' sets both; ie it registers the device as a wakeup > source and marks it as wakeup capable too. > > In our case, the device is "wakeup capable" but we do not want to > "register it as a wakeup source". 'device_set_wakeup_capable(dev, true)' is more > appropriate because this marks the device as wakeup capable but doesn't register > it as a wakeup source knowingly. > > I understand that Dimitry suggests not to revert the entire patch but to replace > 'device_init_wakeup' with 'device_set_wakeup_capable', to mark the device as > capable of wakeup but knowingly skipping the registering part. > > Requesting Dimitry to correct me if there is any misinterpretation. > > While fixing this in next version, please fix the kernel bot warings as well > reported for 1/3 patch of this series. > > - Saurabh Thanks for the clarification Saurabh. I'll be incorporating these changes in the next verison of the patch. - Vennela ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 3/3] Revert "HID: hyperv: register as a wakeup source" 2024-09-12 21:27 [PATCH 0/3] Disable Suspend-to-Idle in Hyper-V and Fix Hibernation Interruptions Erni Sri Satya Vennela 2024-09-12 21:27 ` [PATCH 1/3] Drivers: hv: vmbus: Disable Suspend-to-Idle for VMBus Erni Sri Satya Vennela 2024-09-12 21:27 ` [PATCH 2/3] Revert "Input: hyperv-keyboard - register as a wakeup source" Erni Sri Satya Vennela @ 2024-09-12 21:27 ` Erni Sri Satya Vennela 2024-09-24 3:29 ` Srivatsa S. Bhat 2024-09-24 3:26 ` [PATCH 0/3] Disable Suspend-to-Idle in Hyper-V and Fix Hibernation Interruptions Srivatsa S. Bhat 3 siblings, 1 reply; 19+ messages in thread From: Erni Sri Satya Vennela @ 2024-09-12 21:27 UTC (permalink / raw) To: kys, haiyangz, wei.liu, decui, jikos, bentiss, dmitry.torokhov, linux-hyperv, linux-input, linux-kernel Cc: ernis, Erni Sri Satya Vennela This reverts commit f1210455e78a610c7b316389b31c162c371d888c. Remove mouse as wakeup source since Suspend-to-Idle feature is disabled. Signed-off-by: Erni Sri Satya Vennela <ernis@linux.microsoft.com> --- drivers/hid/hid-hyperv.c | 6 ------ 1 file changed, 6 deletions(-) diff --git a/drivers/hid/hid-hyperv.c b/drivers/hid/hid-hyperv.c index f33485d83d24..b6d0f7db4c93 100644 --- a/drivers/hid/hid-hyperv.c +++ b/drivers/hid/hid-hyperv.c @@ -293,9 +293,6 @@ static void mousevsc_on_receive(struct hv_device *device, memcpy(input_dev->input_buf, input_report->buffer, len); hid_input_report(input_dev->hid_device, HID_INPUT_REPORT, input_dev->input_buf, len, 1); - - pm_wakeup_hard_event(&input_dev->device->device); - break; default: pr_err("unsupported hid msg type - type %d len %d\n", @@ -502,8 +499,6 @@ static int mousevsc_probe(struct hv_device *device, goto probe_err2; } - device_init_wakeup(&device->device, true); - input_dev->connected = true; input_dev->init_complete = true; @@ -526,7 +521,6 @@ static void mousevsc_remove(struct hv_device *dev) { struct mousevsc_dev *input_dev = hv_get_drvdata(dev); - device_init_wakeup(&dev->device, false); vmbus_close(dev->channel); hid_hw_stop(input_dev->hid_device); hid_destroy_device(input_dev->hid_device); -- 2.34.1 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH 3/3] Revert "HID: hyperv: register as a wakeup source" 2024-09-12 21:27 ` [PATCH 3/3] Revert "HID: hyperv: " Erni Sri Satya Vennela @ 2024-09-24 3:29 ` Srivatsa S. Bhat 0 siblings, 0 replies; 19+ messages in thread From: Srivatsa S. Bhat @ 2024-09-24 3:29 UTC (permalink / raw) To: Erni Sri Satya Vennela Cc: kys, haiyangz, wei.liu, decui, jikos, bentiss, dmitry.torokhov, linux-hyperv, linux-input, linux-kernel, ernis, rafael, pavel, lenb, linux-pm [+linux-pm, Rafael, Len, Pavel] On Thu, Sep 12, 2024 at 02:27:50PM -0700, Erni Sri Satya Vennela wrote: > This reverts commit f1210455e78a610c7b316389b31c162c371d888c. > > Remove mouse as wakeup source since Suspend-to-Idle feature > is disabled. > > Signed-off-by: Erni Sri Satya Vennela <ernis@linux.microsoft.com> > --- > drivers/hid/hid-hyperv.c | 6 ------ > 1 file changed, 6 deletions(-) > > diff --git a/drivers/hid/hid-hyperv.c b/drivers/hid/hid-hyperv.c > index f33485d83d24..b6d0f7db4c93 100644 > --- a/drivers/hid/hid-hyperv.c > +++ b/drivers/hid/hid-hyperv.c > @@ -293,9 +293,6 @@ static void mousevsc_on_receive(struct hv_device *device, > memcpy(input_dev->input_buf, input_report->buffer, len); > hid_input_report(input_dev->hid_device, HID_INPUT_REPORT, > input_dev->input_buf, len, 1); > - > - pm_wakeup_hard_event(&input_dev->device->device); > - > break; > default: > pr_err("unsupported hid msg type - type %d len %d\n", > @@ -502,8 +499,6 @@ static int mousevsc_probe(struct hv_device *device, > goto probe_err2; > } > > - device_init_wakeup(&device->device, true); > - > input_dev->connected = true; > input_dev->init_complete = true; > > @@ -526,7 +521,6 @@ static void mousevsc_remove(struct hv_device *dev) > { > struct mousevsc_dev *input_dev = hv_get_drvdata(dev); > > - device_init_wakeup(&dev->device, false); > vmbus_close(dev->channel); > hid_hw_stop(input_dev->hid_device); > hid_destroy_device(input_dev->hid_device); > -- > 2.34.1 > > ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 0/3] Disable Suspend-to-Idle in Hyper-V and Fix Hibernation Interruptions 2024-09-12 21:27 [PATCH 0/3] Disable Suspend-to-Idle in Hyper-V and Fix Hibernation Interruptions Erni Sri Satya Vennela ` (2 preceding siblings ...) 2024-09-12 21:27 ` [PATCH 3/3] Revert "HID: hyperv: " Erni Sri Satya Vennela @ 2024-09-24 3:26 ` Srivatsa S. Bhat 2024-09-26 3:45 ` Erni Sri Satya Vennela 3 siblings, 1 reply; 19+ messages in thread From: Srivatsa S. Bhat @ 2024-09-24 3:26 UTC (permalink / raw) To: Erni Sri Satya Vennela Cc: kys, haiyangz, wei.liu, decui, jikos, bentiss, dmitry.torokhov, linux-hyperv, linux-input, linux-kernel, ernis, rafael, pavel, lenb, linux-pm Hi Vennela, [+linux-pm, Rafael, Pavel, Len] Let's CC the linux-pm mailing list for discussions related to power management features (such as suspend/resume and hibernation). On Thu, Sep 12, 2024 at 02:27:47PM -0700, Erni Sri Satya Vennela wrote: > It has been reported that Hyper-V VM users can unintentionally abort > hibernation by mouse or keyboard movements. To address this issue, > we have decided to remove the wakeup events for the Hyper-V keyboard > and mouse driver. From the description of the problem, it doesn't occur to me that this is specific to Hyper-V. I was wondering if VMs on other hypervisor platforms wouldn't face the same issue? I'd like to recommend exploring how this problem has been solved for other platforms, so that we can reuse the same approach here. (If it turns out that removing keyboard and mouse wakeup events is the way to go, then great; otherwise, we can learn and apply the recommended solution). > However, this change introduces another problem: > Suspend-to-Idle brings the system down with no method to wake it back up. > > Given that there are no real users of Suspend-to-Idle in Hyper-V, > we have decided to disable this feature for VMBus. This results in: > > $echo freeze > /sys/power/state > > bash: echo: write error: Operation not supported > > The keyboard and mouse were previously registered as wakeup sources to > interrupt the freeze operation in a VM. Since the freeze operation itself > is no longer supported, we are disabling them as wakeup events. > > This patchset ensures that the system remains stable and prevents > unintended interruptions during hibernation. > > Erni Sri Satya Vennela (3): > Drivers: hv: vmbus: Disable Suspend-to-Idle for VMBus > Revert "Input: hyperv-keyboard - register as a wakeup source" > Revert "HID: hyperv: register as a wakeup source" > > drivers/hid/hid-hyperv.c | 6 ------ > drivers/hv/vmbus_drv.c | 15 ++++++++++++++- > drivers/input/serio/hyperv-keyboard.c | 12 ------------ > 3 files changed, 14 insertions(+), 19 deletions(-) > > -- > 2.34.1 > > Regards, Srivatsa Microsoft Linux Systems Group ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 0/3] Disable Suspend-to-Idle in Hyper-V and Fix Hibernation Interruptions 2024-09-24 3:26 ` [PATCH 0/3] Disable Suspend-to-Idle in Hyper-V and Fix Hibernation Interruptions Srivatsa S. Bhat @ 2024-09-26 3:45 ` Erni Sri Satya Vennela 0 siblings, 0 replies; 19+ messages in thread From: Erni Sri Satya Vennela @ 2024-09-26 3:45 UTC (permalink / raw) To: Srivatsa S. Bhat Cc: kys, haiyangz, wei.liu, decui, jikos, bentiss, dmitry.torokhov, linux-hyperv, linux-input, linux-kernel, ernis, rafael, pavel, lenb, linux-pm On Tue, Sep 24, 2024 at 03:26:14AM +0000, Srivatsa S. Bhat wrote: > Hi Vennela, > > [+linux-pm, Rafael, Pavel, Len] > > Let's CC the linux-pm mailing list for discussions related to power > management features (such as suspend/resume and hibernation). > > On Thu, Sep 12, 2024 at 02:27:47PM -0700, Erni Sri Satya Vennela wrote: > > It has been reported that Hyper-V VM users can unintentionally abort > > hibernation by mouse or keyboard movements. To address this issue, > > we have decided to remove the wakeup events for the Hyper-V keyboard > > and mouse driver. > > >From the description of the problem, it doesn't occur to me that this > is specific to Hyper-V. I was wondering if VMs on other hypervisor > platforms wouldn't face the same issue? I'd like to recommend > exploring how this problem has been solved for other platforms, so > that we can reuse the same approach here. (If it turns out that > removing keyboard and mouse wakeup events is the way to go, then > great; otherwise, we can learn and apply the recommended solution). > This is how the keyboard and mouse devices can be disabled manually and is the proper way to address this issue. >echo disabled > /sys/bus/vmbus/drivers/hid_hyperv/$HV_MOUSE/power/wakeup >echo disabled > >/sys/bus/vmbus/drivers/hyperv_keyboard/$HV_KEYBOARD/power/wakeup >systemctl hibernate But based on customer feedback we are totally eliminating them as wakeup events. Initially, they were registered as wakeup events to wakeup from Suspend-to-Idle state. Since there is no real user of this operation, we are disabling them to ensure they do not interfere with hibernation process. > > However, this change introduces another problem: > > Suspend-to-Idle brings the system down with no method to wake it back up. > > > > Given that there are no real users of Suspend-to-Idle in Hyper-V, > > we have decided to disable this feature for VMBus. This results in: > > > > $echo freeze > /sys/power/state > > > bash: echo: write error: Operation not supported > > > > The keyboard and mouse were previously registered as wakeup sources to > > interrupt the freeze operation in a VM. Since the freeze operation itself > > is no longer supported, we are disabling them as wakeup events. > > > > This patchset ensures that the system remains stable and prevents > > unintended interruptions during hibernation. > > > > Erni Sri Satya Vennela (3): > > Drivers: hv: vmbus: Disable Suspend-to-Idle for VMBus > > Revert "Input: hyperv-keyboard - register as a wakeup source" > > Revert "HID: hyperv: register as a wakeup source" > > > > drivers/hid/hid-hyperv.c | 6 ------ > > drivers/hv/vmbus_drv.c | 15 ++++++++++++++- > > drivers/input/serio/hyperv-keyboard.c | 12 ------------ > > 3 files changed, 14 insertions(+), 19 deletions(-) > > > > -- > > 2.34.1 > > > > > > Regards, > Srivatsa > Microsoft Linux Systems Group ^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2024-12-12 14:33 UTC | newest] Thread overview: 19+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-09-12 21:27 [PATCH 0/3] Disable Suspend-to-Idle in Hyper-V and Fix Hibernation Interruptions Erni Sri Satya Vennela 2024-09-12 21:27 ` [PATCH 1/3] Drivers: hv: vmbus: Disable Suspend-to-Idle for VMBus Erni Sri Satya Vennela 2024-09-13 6:38 ` Saurabh Singh Sengar 2024-09-13 7:19 ` Naman Jain 2024-09-21 19:09 ` Erni Sri Satya Vennela 2024-09-13 17:01 ` kernel test robot 2024-09-13 17:43 ` kernel test robot 2024-09-24 3:27 ` Srivatsa S. Bhat 2024-09-12 21:27 ` [PATCH 2/3] Revert "Input: hyperv-keyboard - register as a wakeup source" Erni Sri Satya Vennela 2024-09-24 3:28 ` Srivatsa S. Bhat 2024-10-04 8:14 ` Dmitry Torokhov 2024-10-17 13:44 ` Erni Sri Satya Vennela 2024-11-08 10:47 ` Erni Sri Satya Vennela 2024-12-09 17:16 ` Saurabh Singh Sengar 2024-12-12 14:33 ` Erni Sri Satya Vennela 2024-09-12 21:27 ` [PATCH 3/3] Revert "HID: hyperv: " Erni Sri Satya Vennela 2024-09-24 3:29 ` Srivatsa S. Bhat 2024-09-24 3:26 ` [PATCH 0/3] Disable Suspend-to-Idle in Hyper-V and Fix Hibernation Interruptions Srivatsa S. Bhat 2024-09-26 3:45 ` Erni Sri Satya Vennela
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).