linux-kernel - RE: [PATCH v4 2/2] ACPI / button: Add document for ACPI control method lid device restrictions

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1AE640813FDE7649BE1B193DEA596E883BC064D9@SHSMSX101.ccr.corp.intel.com>
Date:	Mon, 25 Jul 2016 00:38:45 +0000
From:	"Zheng, Lv" <lv.zheng@...el.com>
To:	Bastien Nocera <hadess@...ess.net>,
	Benjamin Tissoires <benjamin.tissoires@...il.com>
CC:	Dmitry Torokhov <dmitry.torokhov@...il.com>,
	"Zhang, Rui" <rui.zhang@...el.com>,
	"Wysocki, Rafael J" <rafael.j.wysocki@...el.com>,
	"Rafael J. Wysocki" <rjw@...ysocki.net>,
	"Brown, Len" <len.brown@...el.com>, Lv Zheng <zetalog@...il.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-acpi@...r.kernel.org" <linux-acpi@...r.kernel.org>,
	"linux-input@...r.kernel.org" <linux-input@...r.kernel.org>
Subject: RE: [PATCH v4 2/2] ACPI / button: Add document for ACPI control
 method lid device restrictions

Hi, Bastien

> From: Bastien Nocera [mailto:hadess@...ess.net]
> Subject: Re: [PATCH v4 2/2] ACPI / button: Add document for ACPI control
> method lid device restrictions
> 
> On Fri, 2016-07-22 at 11:08 +0200, Benjamin Tissoires wrote:
> >
> <snip>
> > Then you just need to amend the documentation to say that the
> > fallback
> > of the KEY events is not the "future" but a way to get events on some
> > reduced platforms and it will not be the default.
> > Please make sure userspace knows that the default is the good SW_LID,
> > and some particular cases will need to be handled through the KEY
> > events, not the other way around.
> >
> > [few thoughts later]
> >
> > How about:
> > - you send only one patch with the SW_LID ON/OFF or OFF/ON when we
> > receive the notification on buggy platform
> > - in the same patch, you add the documentation saying that on most
> > platforms, LID is reliable but some don't provide a reliable LID
> > state, but you guarantee to send an event when the state changes
> > - in userspace, we add the hwdb which says "on this particular
> > platform, don't rely on the actual state, but wait for events" ->
> > this
> > basically removes the polling on these platforms.
> >
> > Bastien, Dmitry?
> >
> > I still don't like relying on userspace to actually set the SW_LID
> > back to open on resume, as we should not rely on some userspace
> > program to set the value (but if logind really wants it, it's up to
> > them).
> 
> From my point of view, I would only send the events that can actually
> be generated by the system, not any synthetic ones, because user-space
> would have no way to know that this was synthetic, and how accurate it
> would be.
> 
> So we'd have a separate API, or a separate event for the "close to
> Windows behaviour" devices. We'd then use hwdb in udev to tag the
> machines that don't have a reliable LID status, in user-space, so we
> can have a quick turn around for those machines.
> 
> That should hopefully give us a way to tag test systems, so we can test
> the new behaviour, though we'll certainly need to have some changes
> made in the stack.
 [Lv Zheng] 
That's the original motivation of PATCH 02.

However, the PATCH 01 is valid fix.
Without it, running SW_LID on such buggy platforms could cause no event.
For example, if a platform always reports close, and never reports open.
Then after the first SW_LID(close), userspace could never see the follow-up SW_LID(close).
Thus that fix is required.

Then after upstreaming PATCH 01, we can see something redundant to KEY_LID_XXX approach.
Since with PATCH 01, we managed to ensure that platform triggered event will always be delivered to the userspace.
Since:
1. Open event is not reliable
2. Close event is reliable
We finally can see that:
1. All platform triggered close event can be seen by the userspace as SW_LID(close).
2. On the buggy platforms, SW_LID(open) is meaningless.

It then looks like the KEY_LID_XXX is redundant to the improved SW_LID now.
As with the key event approach, we still cannot guarantee to send "open" when the state is changed to "opened".
__Unless we start to fix the buggy firmware tables__.
And what we want to do - delivering reliable "close" to userspace can also be achieved with the SW_LID improvement.

Thus, finally, there's no difference between the new userspace behaviors:
1. SW_LID with reliable close: userspace matches hwdb and stops acting upon open
2. KEY_LID_xxx with reliable close: userspace matches hwdb and starts acting only upon KEY_LID_CLOSE

So we just need you and Dmitry to reach an agreement here.
And this doesn't look like a big conflict.

IMO, since SW_LID(CLOSE) is reliable now, we needn't introduce the new KEY_LID_xxx events.
That means we can leave the kernel input layer unchanged.
And limits this issue to the ACPI subsystem and the userspace programs.
What do you think?

> 
> As Benjamin mentioned, it would be nice to have a list of devices that
> don't work today, because of this problem.

[Lv Zheng] 
We'll try to find that.
Before working out the full list, you can use the above mentioned 3 platforms to test.

Cheers