Re: [BUG]i2c_hid_acpi broken with 4.17.2 on Framework Laptop 13 AMD

[Jan Beulich <jbeulich@xxxxxxxx>]

On 18.12.2023 18:04, Sébastien Chaumat wrote:
> Le lun. 18 déc. 2023, 17:44, Jan Beulich <jbeulich@xxxxxxxx> a écrit :
> 
>> On 18.12.2023 17:21, Sébastien Chaumat wrote:
>>>>>>> On 05.12.2023 21:31, Sébastien Chaumat wrote:
>>>>> This issue seems that IRQ 7 (the GPIO controller) is natively fasteoi
>>>>> (so level type) while in xen it  is mapped to oapic-edge  instead of
>>>>> oapic-level
>>>>> as the SSDT indicates :
>>>>>
>>>>>  Device (GPIO)
>>>>>
>>>>>      {
>>>>>          Name (_HID, "AMDI0030")  // _HID: Hardware ID
>>>>>          Name (_CID, "AMDI0030")  // _CID: Compatible ID
>>>>>          Name (_UID, Zero)  // _UID: Unique ID
>>>>>          Method (_CRS, 0, NotSerialized)  // _CRS: Current Resource
>> Settings
>>>>>          {
>>>>>              Name (RBUF, ResourceTemplate ()
>>>>>              {
>>>>>                  Interrupt (ResourceConsumer, Level, ActiveLow,
>> Shared, ,, )
>>>>>                  {
>>>>>                      0x00000007,
>>>>>            }
>>>>> Any idea why ?
>>>>
>>>> Information coming from AML is required to be handed down by Dom0 to
>> Xen.
>>>> May want checking that (a) Dom0 properly does so and (b) Xen doesn't
>> screw
>>>> up in consuming that data. See PHYSDEVOP_setup_gsi. I wonder if this is
>>>> specific to it being IRQ7 which GPIO uses, as at the (master) PIC IRQ7
>> is
>>>> also the spurious vector. You may want to retry with the tip of the 4.17
>>>> branch (soon to become 4.17.3) - while it doesn't look very likely to me
>>>> that recent backports there were related, it may still be that they make
>>>> a difference.
>>>>
>>>
>>> testing with 4.17.3:
>>>
>>> Adding some printk in PHYSDEVOP_setup_gsi, I  see (in xl dmesg)  that
>>> (XEN) PHYSDEVOP_setup_gsi setup_gsi : gsi: 7 triggering: 1 polarity: 1
>>>
>>> but later on in dmesg I see :
>>> [    1.747958] xen: registering gsi 7 triggering 0 polarity 1
>>>
>>> So triggering is flipped from 1 to 0 (cannot find the definition for
>>> those values).
>>> Could this be the culprit ?
>>
>> Possibly. Since it would be the kernel to invoke PHYSDEVOP_setup_gsi, it
>> looks as if the kernel was confused about which trigger mode to use. Have
>> you figured from where the kernel takes the two different values?
>>
> 
>> Would you mind pointing me to the definition for those values first ? I
> did not find what 0/1 means in this context.

See e.g. the IO-APIC spec, or Xen's io_apic.h:

struct IO_APIC_route_entry {
    ...
            unsigned int polarity:1;      /* 0: low, 1: high */
    ...
            unsigned int trigger:1;       /* 0: edge, 1: level */

(Sadly the comment may be the wrong way round for polarity, but then I
may also be missing something, as msi.h and apicdef.h suggest things
are as the comment above says.)

In any event the PHYSDEVOP_setup_gsi invocation looks fishy, at least
if this was a PCI IRQ. Just to mention it - according to the hypervisor
log you sent earlier there's also no source override for IRQ 7 (in the
log lines starting "ACPI: INT_SRC_OVR ").

Jan