[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [BUG]i2c_hid_acpi broken with 4.17.2 on Framework Laptop 13 AMD
Le mar. 19 déc. 2023 à 16:15, Sébastien Chaumat <euidzero@xxxxxxxxx> a écrit :
>
> I did add an extra printk in PHYSDEVOP_setup_gsi
> so the "first one" is my printk (available in xl dmesg)
> the second message is from xen_register_gsi (from linux kernel)
>
> Le mar. 19 déc. 2023 à 14:15, 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:
> > >>>>>>> [ 2.464598] amd_gpio AMDI0030:00: failed to enable wake-up interrupt
> > >>>>>>
> > >>>>>> Is it expected that IRQ7 goes from fasteoi (kernel 6.6.4 ) to
> > >>>>>> ioapic-edge and IRQ9 to ioapic-level ?
> > >>>>>>
> > >>>>>> IR-IO-APIC 7-fasteoi pinctrl_amd
> > >>>>>> IR-IO-APIC 9-fasteoi acpi
> > >>>>>>
> > >>>>>> to (xen 4.18.0)
> > >>>>>>
> > >>>>>> xen-pirq -ioapic-edge pinctrl_amd
> > >>>>>> xen-pirq -ioapic-level acpi
> > >>>>>>
> > >>>>>> ?
> > >>>
> > >
> > >>> This look similar to
> > >>> https://yhbt.net/lore/all/20201006044941.fdjsp346kc5thyzy@Rk/t/
> > >>>
> > >>> 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
> >
> > Linux has exactly one place where this message is logged from, and that's
> > ahead of it calling PHYSDEVOP_setup_gsi. Since you said "later", can you
> > confirm that actually you see two instances of the Xen message and two
> > instances of the Linux one (each of them with respectively matching
> > trigger and polarity values)? Or are we indeed observing what would look
> > to be corruption of a hypercall argument?
> >
> > If there were two calls, it would be important to realize that Xen will
> > respect only the first one.
> >
> > Jan
Adding a printk to catch the gsi immediately before the hypercall in
linux/arch/x86/pci/xen.c
#ifdef CONFIG_XEN_PV_DOM0
static int xen_register_gsi(u32 gsi, int triggering, int polarity)
{
int rc, irq;
struct physdev_setup_gsi setup_gsi;
if (!xen_pv_domain())
return -1;
printk(KERN_DEBUG "xen: registering gsi %u triggering %d polarity %d\n",
gsi, triggering, polarity);
there we have :
[ 1.848051] xen: registering gsi 7 triggering 0 polarity 1
then in the next call :
irq = xen_register_pirq(gsi, triggering, true);
I added a printk at the very beginning :
static int xen_register_pirq(u32 gsi, int triggering, bool set_pirq)
{
int rc, pirq = -1, irq;
struct physdev_map_pirq map_irq;
int shareable = 0;
char *name;
printk(KERN_DEBUG "xen_register_pirq start gsi %u triggering %d set_pirq %d\n", gsi, triggering, set_pirq)
And I get in this printk result for IRQ7 : triggering=1 while it was passed with value 0 in the call !?
Sorry bad format %d instead of %i for triggering ...
So I replaced the format with %i and suprise :
There are 2 calls to xen_register_pirq as I can see 2 messages per IRQ
the first call is right after the message "NR_IRQS: ..."from early_irq_init() ( kernel/irq/irqdec.c )
I see :
xen_register_pirq start gsi 7 triggering 1 set_pirq 1 ... so in xen_register_pirq() Then I get the message "Before PHYSDEVOPS_setup_gsi) proving we are called by xen_register_gsi()
Then right after the message "ACPI: 26 ACPI AML tables successfully acquired and loaded"
I get again, but with reversed triggering : xen: registering gsi 7 triggering 0 polarity 1 xen_register_pirq start gsi 7 triggering 0 set_pirq 1 Before PHYSDEVOPS_setup_gsi ...
So once again a call to xen_register_gsi()
|
