Suspend with ps2 keyboard/mice

Suspend with ps2 keyboard/mice

[Zack Rusin]

Hi, Dmitry.

We've been scratching our heads about:
a1cec0617738 ("Input: psmouse - properly reset mouse on shutdown/suspend")
because we're having trouble seeing how it can work with suspend on ps2. We were
planning to wait another 4 years to get to a 20 year anniversary of that patch but
we figured 16 is probably good enough. 

So the kernels i8042 module specifically calls device_set_wakeup_enable only on the
keyboard port. Then because of the a1cec0617738 psbase leaves the mouse enabled on
suspend which, in case of a mouse move/click right before the suspend makes sure
Linux will never wake up. 

That's because the dataport will be effectively latched to the mouse (mouse irq
wasn't serviced in the guest) and it won't be possible to deliver a keyboard
interrupt anymore, and mouse interrupts won't wake Linux.

Our suggestion would be to simply remove the:
ps2_command(&psmouse->ps2dev, NULL, PSMOUSE_CMD_ENABLE);
added in a1cec0617738 . I don't have access to HP nx7400 from 2007 but it seems like
that code breaks every other well formed ps/2 controller with suspend/wakeup.

I think the ps2 mouse has to be disabled on suspend in order to be able to get the
keyboard interrupts required to wake up. Otherwise if the mouse is supposed to be
really enabled (but mouse irq's will be ignored), then how does the PS/2 controller
decide whether to return mouse or keyboard data when they do the data port read?

z

Re: Suspend with ps2 keyboard/mice

[dmitry.torokhov]

Hi Zack,

On Thu, Feb 02, 2023 at 03:23:34AM +0000, Zack Rusin wrote:
> Hi, Dmitry.
> 
> We've been scratching our heads about:
> a1cec0617738 ("Input: psmouse - properly reset mouse on shutdown/suspend")
> because we're having trouble seeing how it can work with suspend on ps2. We were
> planning to wait another 4 years to get to a 20 year anniversary of that patch but
> we figured 16 is probably good enough.

Heh, yeah, it's been a while.

> 
> So the kernels i8042 module specifically calls device_set_wakeup_enable only on the
> keyboard port. Then because of the a1cec0617738 psbase leaves the mouse enabled on
> suspend which, in case of a mouse move/click right before the suspend makes sure
> Linux will never wake up. 
> 
> That's because the dataport will be effectively latched to the mouse (mouse irq
> wasn't serviced in the guest) and it won't be possible to deliver a keyboard
> interrupt anymore, and mouse interrupts won't wake Linux.
> 
> Our suggestion would be to simply remove the:
> ps2_command(&psmouse->ps2dev, NULL, PSMOUSE_CMD_ENABLE);
> added in a1cec0617738 . I don't have access to HP nx7400 from 2007 but it seems like
> that code breaks every other well formed ps/2 controller with suspend/wakeup.
> 
> I think the ps2 mouse has to be disabled on suspend in order to be able to get the
> keyboard interrupts required to wake up. Otherwise if the mouse is supposed to be
> really enabled (but mouse irq's will be ignored), then how does the PS/2 controller
> decide whether to return mouse or keyboard data when they do the data port read?

IIRC the reason we leave mouse enabled is because firmware on multiple
devices was unhappy if the mouse was disabled on suspend. It was not
only nx7400, but others as well. I am not sure if newer generation
devices exhibit the same problem or not.

Regarding inability to deliver keyboard interrupt: I am not sure why you
are saying that Embedded Controller (EC) can not raise IRQ1 and IRQ12
together. Also, they do not have to be wakeup sources, I think on some
designs these signals were wired to other pins as well, and those were
actually configured to wake up the AP.

Both mouse and keyboard data are coming from the same data port, so it
really up to the Embedded Controller to figure out what data it wants to
send first, and Linux is using the same interrupt handler for both IRQ1
and IRQ12, and can dispatch data as needed. But you do not need to read
the data to wake up the system. You just need EC to actually signal
interrupt that is configured as a wakeup source.

I think you should look into how you implement EC and make sure it does
not get stuck.

Thanks.

-- 
Dmitry

Re: Suspend with ps2 keyboard/mice

[Zack Rusin]

On Wed, 2023-02-01 at 22:12 -0800, dmitry.torokhov@gmail.com wrote:
> On Thu, Feb 02, 2023 at 03:23:34AM +0000, Zack Rusin wrote:
> > So the kernels i8042 module specifically calls device_set_wakeup_enable only on
> > the
> > keyboard port. Then because of the a1cec0617738 psbase leaves the mouse enabled
> > on
> > suspend which, in case of a mouse move/click right before the suspend makes sure
> > Linux will never wake up.
> > 
> > That's because the dataport will be effectively latched to the mouse (mouse irq
> > wasn't serviced in the guest) and it won't be possible to deliver a keyboard
> > interrupt anymore, and mouse interrupts won't wake Linux.
> > 
> > Our suggestion would be to simply remove the:
> > ps2_command(&psmouse->ps2dev, NULL, PSMOUSE_CMD_ENABLE);
> > added in a1cec0617738 . I don't have access to HP nx7400 from 2007 but it seems
> > like
> > that code breaks every other well formed ps/2 controller with suspend/wakeup.
> > 
> > I think the ps2 mouse has to be disabled on suspend in order to be able to get
> > the
> > keyboard interrupts required to wake up. Otherwise if the mouse is supposed to
> > be
> > really enabled (but mouse irq's will be ignored), then how does the PS/2
> > controller
> > decide whether to return mouse or keyboard data when they do the data port read?
> 
> IIRC the reason we leave mouse enabled is because firmware on multiple
> devices was unhappy if the mouse was disabled on suspend. It was not
> only nx7400, but others as well. I am not sure if newer generation
> devices exhibit the same problem or not.
> 
> Regarding inability to deliver keyboard interrupt: I am not sure why you
> are saying that Embedded Controller (EC) can not raise IRQ1 and IRQ12
> together. Also, they do not have to be wakeup sources, I think on some
> designs these signals were wired to other pins as well, and those were
> actually configured to wake up the AP.
> 
> Both mouse and keyboard data are coming from the same data port, so it
> really up to the Embedded Controller to figure out what data it wants to
> send first, and Linux is using the same interrupt handler for both IRQ1
> and IRQ12, and can dispatch data as needed. But you do not need to read
> the data to wake up the system. You just need EC to actually signal
> interrupt that is configured as a wakeup source.
> 
> I think you should look into how you implement EC and make sure it does
> not get stuck.

That makes sense. Thanks a lot for going over this. We were being overly cautious
with the IRQ delivery and would not deliver IRQ's on PS/2 unless the dataport had
data from the respective device. We went back now and checked Win98 and WinXP to
make sure that they actually don't depend on the IRQ number that's delivered to
figure out the routing and it looks like they properly do check the status register.
So we went ahead and fixed it in our code. 
For the kernel I will write a small patch just for vmmouse that overwrites the serio
cleanup to avoid the enabling of the mouse on suspend just to fix it for folks who
won't be running our latest hypervisors and everything should be fine.

z