From: Dexuan Cui <decui@microsoft.com>
To: "dmitry.torokhov@gmail.com" <dmitry.torokhov@gmail.com>
Cc: KY Srinivasan <kys@microsoft.com>,
Haiyang Zhang <haiyangz@microsoft.com>,
Stephen Hemminger <sthemmin@microsoft.com>,
"sashal@kernel.org" <sashal@kernel.org>,
"linux-hyperv@vger.kernel.org" <linux-hyperv@vger.kernel.org>,
"linux-input@vger.kernel.org" <linux-input@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
Michael Kelley <mikelley@microsoft.com>
Subject: RE: [PATCH] Input: hyperv-keyboard: Add the support of hibernation
Date: Mon, 30 Sep 2019 22:09:27 +0000 [thread overview]
Message-ID: <PU1P153MB0169C315F7F9EBEBED4C7A7DBF820@PU1P153MB0169.APCP153.PROD.OUTLOOK.COM> (raw)
In-Reply-To: <20190928003156.GU237523@dtor-ws>
> From: dmitry.torokhov@gmail.com <dmitry.torokhov@gmail.com>
> Sent: Friday, September 27, 2019 5:32 PM
> > ...
> > pm_wakeup_pending() is tested in a lot of places in the suspend
> > process and eventually an unintentional keystroke (or mouse movement,
> > when it comes to the Hyper-V mouse driver drivers/hid/hid-hyperv.c)
> > causes the whole hibernation process to be aborted. Usually this
> > behavior is not expected by the user, I think.
>
> Why not? If a device is configured as wakeup source, then it activity
> should wake up the system, unless you disable it.
Generally speaking, I agree, but compared to a physical machine, IMO
the scenario is a little differnet when it comes to a VM running on Hyper-V:
on the host there is a window that represents the VM, and the user can
unintentionally switch the keyboard input focus to the window (or move
the mouse/cursor over the window) and then the host automatically
sends some special keystrokes (and mouse events) , and this aborts the
hibernation process.
And, when it comes to the Hyper-V mouse device, IMO it's easy for the
user to unintentionally move the mouse after the "hibernation" button
is clicked. I suppose a physical machine would have the same issue, though.
> > So, I use the notifier to set the flag variable and with it the driver can
> > know when it should not call pm_wakeup_hard_event().
>
> No, please implement hibernation support properly, as notifier + flag is
> a hack.
The keyboard/mouse driver can avoid the flag by disabling the
keyboard/mouse event handling, but the problem is that they don't know
when exactly they should disable the event handling. I think the PM
notifier is the only way to tell the drivers a hibernation process is ongoing.
Do you think this idea (notifer + disabling event handling) is acceptable?
If not, then I'll have to remove the notifer completely, and document this as
a known issue to the user: when a hibernation process is started, be careful
to not switch input focus and not touch the keyboard/mouse until the
hibernation process is finished. :-)
> In this particular case you do not want to have your
> hv_kbd_resume() to be called in place of pm_ops->thaw() as that is what
> reenables the keyboard vmbus channel and causes the undesired wakeup
> events.
This is only part of the issues. Another example: before the
pm_ops()->freeze()'s of all the devices are called, pm_wakeup_pending()
is already tested in a lot of places (e.g. in try_to_freeze_tasks ()) in the
suspend process, and can abort the whole suspend process upon the user's
unintentional input focus switch, keystroke and mouse movement.
> Your vmbus implementation should allow individual drivers to
> control the set of PM operations that they wish to use, instead of
> forcing everything through suspend/resume.
>
> Dmitry
Since the devices are pure software-emulated devices, no PM operation was
supported in the past, and now suspend/resume are the only two PM operations
we're going to support. If the idea (notifer + disabling event handling) is not
good enough, we'll have to document the issue to the user, as I described above.
Thanks,
-- Dexuan
next prev parent reply other threads:[~2019-09-30 22:09 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-09-11 23:36 [PATCH] Input: hyperv-keyboard: Add the support of hibernation Dexuan Cui
2019-09-19 16:17 ` dmitry.torokhov
2019-09-21 6:56 ` Dexuan Cui
2019-09-25 19:49 ` Dexuan Cui
2019-09-28 0:31 ` dmitry.torokhov
2019-09-30 22:09 ` Dexuan Cui [this message]
2019-09-30 23:06 ` dmitry.torokhov
2019-10-03 5:35 ` Dexuan Cui
2019-10-03 6:44 ` Dexuan Cui
2019-10-03 17:45 ` dmitry.torokhov
2019-10-03 18:18 ` Dexuan Cui
2019-11-05 5:18 ` Dexuan Cui
2019-11-05 5:43 ` Dexuan Cui
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=PU1P153MB0169C315F7F9EBEBED4C7A7DBF820@PU1P153MB0169.APCP153.PROD.OUTLOOK.COM \
--to=decui@microsoft.com \
--cc=dmitry.torokhov@gmail.com \
--cc=haiyangz@microsoft.com \
--cc=kys@microsoft.com \
--cc=linux-hyperv@vger.kernel.org \
--cc=linux-input@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mikelley@microsoft.com \
--cc=sashal@kernel.org \
--cc=sthemmin@microsoft.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).