Le mer. 20 déc. 2023 à 00:50, Sébastien Chaumat <euidz...@gmail.com> a écrit :
> > > Le mer. 20 déc. 2023 à 00:11, Sébastien Chaumat <euidz...@gmail.com> a > écrit : > >> >> >> Le mer. 20 déc. 2023 à 00:06, Sébastien Chaumat <euidz...@gmail.com> a >> écrit : >> >>> >>> >>> Le mar. 19 déc. 2023 à 20:03, Sébastien Chaumat <euidz...@gmail.com> a >>> écrit : >>> >>>> Le mar. 19 déc. 2023 à 16:15, Sébastien Chaumat <euidz...@gmail.com> 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 <jbeul...@suse.com> 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() > I had to triple check: The first call is from xen_register_pirq() and seem to originate from early_irq_init() : triggering is 1 in this first call the HYPERVISOR_physdev_ops is called with triggering 1 shareable 0 The second call is from xen_register_pirq() called from xen_register_gsi() : trigger=0 polarity 1 at the start of xen_register_pirq() but then trigger=1 polarity=1 just before the call to PHYSDEVOPS_setup_gsi
