Re: [PATCH] HID: i2c-hid: Block a rogue device on ASUS TUF A16

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 6/27/2023 09:38, Friedrich Vock wrote:
Hi,

On 26.06.23 17:10, Limonciello, Mario wrote:

On 6/18/2023 10:05 PM, Mario Limonciello wrote:
On 6/15/23 07:41, Friedrich Vock wrote:
Hi,

sorry for taking so long to reply.

On 02.06.23 20:43, Limonciello, Mario wrote:
+ some AMD guys

On 5/30/2023 10:40 AM, Friedrich Vock wrote:
On these laptops, there seems to be a device that, when probed by
i2c-hid, constantly sends bogus interrupts and interferes with the
keyboard controller. When the device is enabled, it takes the
keyboard
around 8 seconds to register that keys are being pressed or released.

Do you know what interrupt is firing constantly?
Presumably it is the GPIO controller master interrupt, right?
And it's for GPIO 7 (guessed from acpidump on one of the bug
reports).

To confirm check /proc/interrupts.
Seems likely that you guessed correctly. The corresponsing line in
/proc/interrupts (with the interrupts counts omitted):
71:   amd_gpio    7  ITE5570:00

OK.
I had asked in the past for R/W everything output to compare to
/sys/kernel/debug/gpio.

I've added more explicit instructions how to get this to
the kernel bugzilla 217336 – keyboard not working Asus TUF FA617NS
(kernel.org) <https://bugzilla.kernel.org/show_bug.cgi?id=217336#c126>
Thanks for this. R/W everything didn't work for the other people with
the same models, so I spent this day getting Windows and R/W everything
running myself. I managed to make it run and left a comment with the
results in that bugzilla report.


If it's not obvious which GPIO is firing there is also a dynamic
debug statement in pinctrl-amd.c that may be helpful to figure
this out.

I would also suspect in Windows this doesn't happen.  If possible
can you confirm that? Check in Device Manager what driver is bound
to this device. Is it "inbox" from Microsoft or is it an ASUS
specific driver?

I wonder if the GPIO controller got programmed differently in
Windows for some reason. We may want to confirm the values for
GPIO registers from /sys/kernel/debug/gpio in Linux against those
that are programmed in Windows.

This can be accomplished using R/W everything in Windows.

Unfortunately I don't have Windows installed on this system. I know
some
people with the Ryzen 9 7940HS model which might, I'll ask them if they
can give me the configuration on Windows and Linux.

OK, I suggest directing everyone over to the bugs I linked and we
should gather the relevant GPIO controller register dumps from
Windows and Linux on the same hardware there.




Nothing I tried seemed to make the device work, and preventing the
device from being probed doesn't seem to break any functionality of
the laptop.

Signed-off-by: Friedrich Vock <friedrich.vock@xxxxxx>

There are a few bug reports that popped up around this issue that
should
probably also be tagged.

Link: https://bugzilla.kernel.org/show_bug.cgi?id=217336
Link: https://bugzilla.kernel.org/show_bug.cgi?id=217493

---
  drivers/hid/i2c-hid/i2c-hid-core.c       |  5 +++
  drivers/hid/i2c-hid/i2c-hid-dmi-quirks.c | 48
++++++++++++++++++++++++
  drivers/hid/i2c-hid/i2c-hid.h            |  3 ++
  3 files changed, 56 insertions(+)

diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c
b/drivers/hid/i2c-hid/i2c-hid-core.c
index efbba0465eef..5f0686d058df 100644
--- a/drivers/hid/i2c-hid/i2c-hid-core.c
+++ b/drivers/hid/i2c-hid/i2c-hid-core.c
@@ -1035,6 +1035,11 @@ int i2c_hid_core_probe(struct i2c_client
*client, struct i2chid_ops *ops,

      ihid->quirks = i2c_hid_lookup_quirk(hid->vendor, hid->product);

+    if (i2c_hid_device_blocked(hid->vendor, hid->product)) {
+        ret = -ENODEV;
+        goto err_irq;
+    }
+
The thing I worry about here is that an unserviced interrupt can
prevent the
hardware from going into proper low power states; particularly at
runtime.

I think we should better understand what's going on before going down
this
path of just ignoring it.
Yeah, I guess I should've searched more for a proper explanation/fix
before submitting hacks like this. Let's see if this can be fixed in a
cleaner manner than preemptively disabling parts of the system.

Can you please have a try with linux-next or apply this series:
https://lore.kernel.org/linux-gpio/20230421120625.3366-1-mario.limonciello@xxxxxxx/


That does change some of the configuration for the GPIO controller
registers to align how windows is handling debouncing.

I don't think it will change the outcome of your bug, but I'd love
to be surprised.
Any updates for this?
Tried this out today. You won't be surprised, it didn't change anything.

      ret = hid_add_device(hid);
      if (ret) {
          if (ret != -ENODEV)
diff --git a/drivers/hid/i2c-hid/i2c-hid-dmi-quirks.c
b/drivers/hid/i2c-hid/i2c-hid-dmi-quirks.c
index 210f17c3a0be..d2c2806b64b4 100644
--- a/drivers/hid/i2c-hid/i2c-hid-dmi-quirks.c
+++ b/drivers/hid/i2c-hid/i2c-hid-dmi-quirks.c
@@ -440,6 +440,38 @@ static const struct dmi_system_id
i2c_hid_dmi_quirk_table[] = {
      { }    /* Terminate list */
  };

+static const struct hid_device_id i2c_hid_blocked_ite_device = {
+    HID_DEVICE(BUS_I2C, HID_GROUP_GENERIC, USB_VENDOR_ID_ITE,
0x8051)
+};
+
+/*
+ * This list contains devices that can't be activated at all, for
example
+ * because activating them breaks other important parts of the
system.
+ */
+static const struct dmi_system_id i2c_hid_dmi_block_table[] = {
+    /*
+     * On ASUS TUF Gaming A16 laptops, there is a device that will
make the
+     * keyboard controller stop working correctly and flood the CPU
with bogus
+     * interrupts.
+     */
+    {
+        .ident = "ASUS TUF Gaming A16 (Ryzen 7 7735HS)",
+        .matches = {
+            DMI_EXACT_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER
INC."),
+            DMI_MATCH(DMI_PRODUCT_NAME, "FA617NS"),
+        },
+        .driver_data = (void *)&i2c_hid_blocked_ite_device,
+    },
+    {
+        .ident = "ASUS TUF Gaming A16 (Ryzen 9 7940HS)",
+        .matches = {
+            DMI_EXACT_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER
INC."),
+            DMI_MATCH(DMI_PRODUCT_NAME, "FA617XS"),
+        },
+        .driver_data = (void *)&i2c_hid_blocked_ite_device,
+    },
+    { }    /* Terminate list */
If this *does* end up being the best solution, I think it's better
to patch in the DMI to gpiolib-acpi.c similar to other quirks for
floating
GPIOs.  Example:

https://github.com/torvalds/linux/blob/master/drivers/gpio/gpiolib-acpi.c#L1654



+};

  struct i2c_hid_desc *i2c_hid_get_dmi_i2c_hid_desc_override(uint8_t
*i2c_name)
  {
@@ -492,3 +524,19 @@ u32 i2c_hid_get_dmi_quirks(const u16 vendor,
const u16 product)

      return quirks;
  }
+
+bool i2c_hid_device_blocked(const u16 vendor, const u16 product)
+{
+    const struct dmi_system_id *system_id =
+            dmi_first_match(i2c_hid_dmi_block_table);
+
+    if (system_id) {
+        const struct hid_device_id *device_id =
+                (struct hid_device_id *)(system_id->driver_data);
+
+        if (device_id && device_id->vendor == vendor &&
+            device_id->product == product)
+            return true;
+    }
+    return false;
+}
diff --git a/drivers/hid/i2c-hid/i2c-hid.h
b/drivers/hid/i2c-hid/i2c-hid.h
index 2c7b66d5caa0..e17bdd758f39 100644
--- a/drivers/hid/i2c-hid/i2c-hid.h
+++ b/drivers/hid/i2c-hid/i2c-hid.h
@@ -10,6 +10,7 @@ struct i2c_hid_desc
*i2c_hid_get_dmi_i2c_hid_desc_override(uint8_t *i2c_name);
  char *i2c_hid_get_dmi_hid_report_desc_override(uint8_t *i2c_name,
                             unsigned int *size);
  u32 i2c_hid_get_dmi_quirks(const u16 vendor, const u16 product);
+bool i2c_hid_device_blocked(const u16 vendor, const u16 product);
  #else
  static inline struct i2c_hid_desc
*i2c_hid_get_dmi_i2c_hid_desc_override(uint8_t *i2c_name)
@@ -19,6 +20,8 @@ static inline char
*i2c_hid_get_dmi_hid_report_desc_override(uint8_t *i2c_name,
  { return NULL; }
  static inline u32 i2c_hid_get_dmi_quirks(const u16 vendor, const
u16 product)
  { return 0; }
+static inline bool i2c_hid_device_blocked(const u16 vendor, const
u16 product)
+{ return false; }
  #endif

  /**
--
2.40.1



Thanks,
Friedrich

Friderich and some people on CC are already aware of this, but mostly for Benjamin and Jiri I wanted to let you know that the additional register fetching comparing Windows and Linux allowed me to come up with a proper root cause.

This series has been sent out to fix the issue properly.

https://lore.kernel.org/linux-gpio/20230630194716.6497-1-mario.limonciello@xxxxxxx/



[Index of Archives]     [Linux Media Devel]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Linux Wireless Networking]     [Linux Omap]

  Powered by Linux