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: <32854318.MItBFFhxJX@natalenko.name>
Date:   Mon, 20 Nov 2017 22:31:18 +0100
From:   Oleksandr Natalenko <oleksandr@...alenko.name>
To:     Pali Rohár <pali.rohar@...il.com>
Cc:     platform-driver-x86@...r.kernel.org, linux-kernel@...r.kernel.org,
        Matthew Garrett <mjg59@...f.ucam.org>,
        Darren Hart <dvhart@...radead.org>,
        Andy Shevchenko <andy@...radead.org>,
        Mario Limonciello <mario_limonciello@...l.com>
Subject: Re: Dell Vostro 3360 multimedia keys

Hi, Pali et al.

Thanks for the detailed explanation. I've managed to craft the following ugly 
hack:

(I explain my thoughts on it under the snippet)

===
commit a2baeaba1657a47359efea61865aa012da509e03
Author: Oleksandr Natalenko <oleksandr@...alenko.name>
Date:   Mon Nov 20 13:25:43 2017 +0100

    dell-wmi: support Dell Vostro 3360
    
    Signed-off-by: Oleksandr Natalenko <oleksandr@...alenko.name>

diff --git a/drivers/platform/x86/dell-wmi.c b/drivers/platform/x86/dell-wmi.c
index 28d9f8696081..23dd55a6463c 100644
--- a/drivers/platform/x86/dell-wmi.c
+++ b/drivers/platform/x86/dell-wmi.c
@@ -49,6 +49,7 @@ MODULE_LICENSE("GPL");
 #define DELL_DESCRIPTOR_GUID "8D9DDCBC-A997-11DA-B012-B622A1EF5492"
 
 static bool wmi_requires_smbios_request;
+static bool wmi_keycode_combined;
 
 MODULE_ALIAS("wmi:"DELL_EVENT_GUID);
 MODULE_ALIAS("wmi:"DELL_DESCRIPTOR_GUID);
@@ -64,6 +65,12 @@ static int __init dmi_matched(const struct dmi_system_id 
*dmi)
 	return 1;
 }
 
+static int __init dmi_keycode_combined_matched(const struct dmi_system_id 
*dmi)
+{
+	wmi_keycode_combined = 1;
+	return 1;
+}
+
 static const struct dmi_system_id dell_wmi_smbios_list[] __initconst = {
 	{
 		.callback = dmi_matched,
@@ -81,6 +88,14 @@ static const struct dmi_system_id dell_wmi_smbios_list[] 
__initconst = {
 			DMI_MATCH(DMI_PRODUCT_NAME, "Vostro V131"),
 		},
 	},
+	{
+		.callback = dmi_keycode_combined_matched,
+		.ident = "Dell Vostro 3360",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
+			DMI_MATCH(DMI_PRODUCT_NAME, "Vostro 3360"),
+		},
+	},
 	{ }
 };
 
@@ -189,6 +204,10 @@ static const struct key_entry dell_wmi_keymap_type_0000[] 
= {
 	/* Dell Support Center key */
 	{ KE_IGNORE, 0xe06e, { KEY_RESERVED } },
 
+	/* Dell Vostro 3360 combined keycode */
+	{ KE_KEY,    0xe0f1, { KEY_PROG1 } },
+	{ KE_KEY,    0xe0f2, { KEY_PROG2 } },
+
 	{ KE_IGNORE, 0xe0f7, { KEY_MUTE } },
 	{ KE_IGNORE, 0xe0f8, { KEY_VOLUMEDOWN } },
 	{ KE_IGNORE, 0xe0f9, { KEY_VOLUMEUP } },
@@ -398,7 +417,16 @@ static void dell_wmi_notify(struct wmi_device *wdev,
 
 		switch (buffer_entry[1]) {
 		case 0x0000: /* One key pressed or event occurred */
-			if (len > 2)
+			pr_info("len: %d, quirk: %d\n",
+				len, wmi_keycode_combined);
+			if (len > 3) {
+				pr_info("%x, %x, %x\n",
+					buffer_entry[2], buffer_entry[3],
+					buffer_entry[2] | buffer_entry[3]);
+				if (wmi_keycode_combined)
+					dell_wmi_process_key(wdev, 0x0000,
+							     buffer_entry[2] | buffer_entry[3]);
+			} else if (len > 2)
 				dell_wmi_process_key(wdev, 0x0000,
 						     buffer_entry[2]);
 			/* Other entries could contain additional information */

===

First, I've added a new quirk handler, specifically for my laptop. It hides 
all the further ugly hackery under distinct code branch.

Then, if buffer length is > 3 and if quirk is set, bitwise OR on 
buffer_entry[2] and buffer_entry[3] is performed (IOW, on "key code" and INF3 
field in DSDT) to get new "fake" keycode, which will be different for key "1" 
and "2":

code   | INF3 | fake code
-------+------+----------
0xe0f0 | 0x1  | 0xe0f1
0xe0f0 | 0x2  | 0xe0f2

Finally, I add two new fake keycodes to the table, and kernel recognizes 
multimedia keys like this. First key:

===
Nov 20 22:24:45 spock kernel: dell_wmi: len: 5, quirk: 1
Nov 20 22:24:45 spock kernel: dell_wmi: e0f0, 1, e0f1
===

Second key:

===
Nov 20 22:25:07 spock kernel: dell_wmi: len: 5, quirk: 1
Nov 20 22:25:07 spock kernel: dell_wmi: e0f0, 2, e0f2
===

In evtest I get this:

===
Event: time 1511213126.916920, type 4 (EV_MSC), code 4 (MSC_SCAN), value e0f1
Event: time 1511213126.916920, type 1 (EV_KEY), code 148 (KEY_PROG1), value 1
Event: time 1511213126.916920, -------------- SYN_REPORT ------------
Event: time 1511213126.916944, type 1 (EV_KEY), code 148 (KEY_PROG1), value 0
Event: time 1511213126.916944, -------------- SYN_REPORT ------------

Event: time 1511213127.266903, type 4 (EV_MSC), code 4 (MSC_SCAN), value e0f2
Event: time 1511213127.266903, type 1 (EV_KEY), code 149 (KEY_PROG2), value 1
Event: time 1511213127.266903, -------------- SYN_REPORT ------------
Event: time 1511213127.266919, type 1 (EV_KEY), code 149 (KEY_PROG2), value 0
Event: time 1511213127.266919, -------------- SYN_REPORT ------------
===

Third key (the one that is handled via PS/2 keyboard) produces this in the 
kernel buffer:

===
Nov 20 22:27:08 spock kernel: atkbd serio0: Unknown key pressed (translated 
set 2, code 0x60 on isa0060/serio0).
Nov 20 22:27:08 spock kernel: atkbd serio0: Use 'setkeycodes 60 <keycode>' to 
make it known.
Nov 20 22:27:08 spock kernel: dell_wmi: len: 3, quirk: 1
===

But evtest remains silent.

So, kinda working?

The only question that remains is how to avoid this ugly hack with OR'ing 
multiple values. Apparently, calling dell_wmi_process_key() with only INF2 
DSDT field is insufficient to distinguish two different keys with the same 
code. Also, I've noticed this remark:

===
/* Other entries could contain additional information */
===

As you may see, they indeed do. Any unimplemented idea behind this statement?

Any other ideas on how to avoid ugly hackery? Maybe, key table should be 
extended with extra column for INF3 value, and new quirk for this should be 
implemented?

Thanks.

On pondělí 20. listopadu 2017 18:05:50 CET Pali Rohár wrote:
> Hi!
> 
> On Monday 20 November 2017 16:08:41 Oleksandr Natalenko wrote:
> > Hi.
> > 
> > I've got Dell Vostro 3360 with extra multimedia keys as shown here [1],
> > but
> > have no luck to get them working.
> > 
> > I've modified dell_wmi_smbios_list structure in drivers/platform/x86/dell-
> > wmi.c adding new entry:
> > 
> > ===
> > 
> >  84     {
> >  85         .callback = dmi_matched,
> >  86         .ident = "Dell Vostro 3360",
> >  87         .matches = {
> >  88             DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
> >  89             DMI_MATCH(DMI_PRODUCT_NAME, "Vostro 3360"),
> >  90         },
> >  91     },
> > 
> > ===
> 
> What would happen if you do *not* add this new entry? Please do this
> test after your notebook was fully turned off to prevent some cached
> configuration in BIOS.
> 
> > While pressing keys "1" and/or "2" I get the following notice in dmesg:
> > 
> > ===
> > Nov 20 15:53:35 spock kernel: dell_wmi: Unknown key with type 0x0000 and
> > code 0xe0f0 pressed
> > ===
> 
> This means that in dell-wmi.c is missing mapping for code 0xe0f0 in key
> type 0x0000.
> 
> Find array dell_wmi_keymap_type_0000[] and add there line:
> 
>   { KE_KEY, 0xe0f0, { KEY_something } },
> 
> (where something is correct name of key)
> 
> > (it is the same for both keys)
> 
> So for both first and second key you got same type+code? That is bad :-(
> In this case with above definition we cannot distinguish which was was
> pressed. But at least something.
> 
> > While pressing key "3" I get the following:
> > 
> > ===
> > Nov 20 15:36:51 spock kernel: atkbd serio0: Unknown key pressed
> > (translated
> > set 2, code 0x60 on isa0060/serio0).
> > Nov 20 15:36:51 spock kernel: atkbd serio0: Use 'setkeycodes 60 <keycode>'
> > to make it known.
> > ===
> 
> This key is not received by dell-wmi.c, but rather via PS/2 keyboard.
> Use appropriate setkeycodes command line program to map that code 60 to
> some key.
> 
> Today in most cases this mapping for PS/2 keyboards is done in udev or
> systemd at system startup. They have database for it.
> 
> > Here is what I've found in DSDT:
> > 
> > ===
> > 
> >                     Method (_Q70, 0, NotSerialized)  // _Qxx: EC Query
> >                     {
> >                     
> >                         P8XH (Zero, 0x70)
> >                         Notify (MBT1, 0x80) // Status Change
> >                         ^^^^AMW0.INF0 = 0x04
> >                         ^^^^AMW0.INF1 = Zero
> >                         ^^^^AMW0.INF2 = 0xE0F0
> >                         ^^^^AMW0.INF3 = One
> >                         Notify (AMW0, 0xD0) // Hardware-Specific
> >                     
> >                     }
> >                     
> >                     Method (_QAF, 0, NotSerialized)  // _Qxx: EC Query
> >                     {
> >                     
> >                         P8XH (Zero, 0xAF)
> >                         Notify (MBT, 0x80) // Status Change
> >                         ^^^^AMW0.INF0 = 0x04
> >                         ^^^^AMW0.INF1 = Zero
> >                         ^^^^AMW0.INF2 = 0xE0F0
> >                         ^^^^AMW0.INF3 = 0x02
> >                         Notify (AMW0, 0xD0) // Hardware-Specific
> >                     
> >                     }
> > 
> > ===
> > 
> > These are the only 2 records that contain 0xe0f0 sequence, and if they
> > correspond to first two multimedia keys, as you can see they differ only
> > in
> > .INF3 field. Unfortunately, I do not know what it might mean.
> 
> Just guessing... if I concatenate INF0, INF1, INF2, INF3 and treat every
> INF* as 16bit number I got this sequence:
> 
>   0x0004 0x0000 0xE0F0 0x0001
> 
>   0x0004 0x0000 0xE0F0 0x0002
> 
> And it looks like a valid buffer for dell_wmi_notify() function.
> 
> Look at this part of that function:
> 
> 		switch (buffer_entry[1]) {
> 		case 0x0000: /* One key pressed or event occurred */
> 			if (len > 2)
> 				dell_wmi_process_key(wdev, 0x0000,
> 						     buffer_entry[2]);
> 			/* Other entries could contain additional information */
> 
> If I'm right that above INF* sequence is put into dell_wmi_notify()
> function, then in buffer[3] you should be able to see either 0x01 or
> 0x02. And distinguish which key was pressed.
> 
> Can you test it? Beware of "len" of buffer_entry when you would do
> tests.
> 
> > I was monitoring /proc/interrupts file and noticed that values here:
> > 
> > ===
> > 
> >   9:        430        293         65         24   IO-APIC   9-fasteoi  
> >   acpi
> > 
> > ===
> > 
> > are incremented by one on each key press. Also, if i press key "3" (the
> > one
> > that generates different message in kernel log), the following interrupt
> > is
> > fired too:
> > 
> > ===
> > 
> >   1:        646       6391        358        487   IO-APIC   1-edge     
> >   i8042> 
> > ===
> > 
> > Running evtest, I'm able to catch some output while pressing key "3":
> > 
> > ===
> > Event: time 1511189973.016907, type 4 (EV_MSC), code 4 (MSC_SCAN), value
> > e025 Event: time 1511189973.016907, type 1 (EV_KEY), code 203
> > (KEY_PROG4), value 1 Event: time 1511189973.016907, --------------
> > SYN_REPORT ------------ Event: time 1511189973.016942, type 1 (EV_KEY),
> > code 203 (KEY_PROG4), value 0 Event: time 1511189973.016942,
> > -------------- SYN_REPORT ------------ ===
> 
> So it means that third key is also received by dell-wmi? Anyway see
> function dell_wmi_process_key() and line:
> 
> 	if (type == 0x0000 && code == 0xe025 && !wmi_requires_smbios_request)
> 		return;
> 
> It is there just to ensure that key is not send via both PS/2 keyboard
> and dell-wmi.
> 
> So I guess you should disable wmi_requires_smbios_request.
> 
> > I think this corresponds to what I see in drivers/platform/x86/dell-wmi.c
> > file here:
> > 
> > ===
> > 143     /* Dell Instant Launch key */
> > 144     { KE_KEY,    0xe025, { KEY_PROG4 } },
> > ===
> > 
> > Other two keys do not produce any evtest output.
> > 
> > Here is acpidump output: [2]
> > Here is decompiled DSDT: [3]
> > 
> > Also, I've raised this question before a couple of times (for instance,
> > [4]), but unfortunately got no result :(.
> > 
> > Could you please help me in fixing multimedia keys?
> 
> I Hope that this email helps you.
> 
> > Thanks.
> > 
> > [1] http://beta.hstor.org/files/c3b/a26/628/
> > c3ba26628409486f8b9ae16d97be7d21.jpg
> > [2] https://gist.github.com/7c04035ba2a3f0e5501af83efdb1456d
> > [3] https://gist.github.com/83687126c46417b5bc0b48529de52460
> > [4] https://www.spinics.net/lists/platform-driver-x86/msg05251.html

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ