Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752026AbdFNJTs convert rfc822-to-8bit (ORCPT ); Wed, 14 Jun 2017 05:19:48 -0400 Received: from us-smtp-delivery-107.mimecast.com ([216.205.24.107]:32776 "EHLO us-smtp-delivery-107.mimecast.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751960AbdFNJTR (ORCPT ); Wed, 14 Jun 2017 05:19:17 -0400 Subject: Re: [PATCH v7 3/3] PCI: Add tango MSI controller support From: Marc Gonzalez To: Marc Zyngier , Thomas Gleixner CC: Bjorn Helgaas , Robin Murphy , Lorenzo Pieralisi , Liviu Dudau , David Laight , linux-pci , Linux ARM , LKML , Mason References: <741766e5-cff2-db5f-d40b-6866e08fd966@sigmadesigns.com> <9975fa6c-f426-519b-6b3e-a2d102d36fd5@arm.com> <82d6bf37-96b9-cd9d-134b-f01638fa2b1b@sigmadesigns.com> <49b4138d-0030-1e8c-6a14-0e9151b63d24@sigmadesigns.com> <1588fa00-92c6-046d-ccfc-9a2deae61486@arm.com> <08d84ce3-5345-dc69-bbc3-37372a3df22b@sigmadesigns.com> Message-ID: <870f833b-6bff-b3b1-6591-7d46e374de66@sigmadesigns.com> Date: Wed, 14 Jun 2017 11:19:04 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Firefox/52.0 SeaMonkey/2.49 MIME-Version: 1.0 In-Reply-To: <08d84ce3-5345-dc69-bbc3-37372a3df22b@sigmadesigns.com> X-Originating-IP: [172.27.0.114] X-MC-Unique: y46ad8M9P3S_M-GYTduaKA-1 Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 8BIT Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 9745 Lines: 171 On 14/06/2017 11:00, Marc Gonzalez wrote: > The MSI controller in Tango supports 256 message-signaled interrupts, > and a single doorbell address. > > Signed-off-by: Marc Gonzalez > --- > Changes from v6 to v7 > o Call spin_lock() not spin_lock_irqsave() in the ISR > --- > drivers/pci/host/pcie-tango.c | 225 ++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 225 insertions(+) Someone on IRC suggested testing the driver with LOCKDEP. If I understand the warning below correctly, I am not supposed to call irq_domain_set_info() while holding used_msi_lock? NB: in probe, my driver calls add_taint(TAINT_FIRMWARE_WORKAROUND, LOCKDEP_STILL_OK); This should not break LOCKDEP analysis, right? Regards. [ 0.685615] ====================================================== [ 0.685621] [ INFO: possible circular locking dependency detected ] [ 0.685632] 4.9.20-1-rc3 #2 Tainted: G I [ 0.685638] ------------------------------------------------------- [ 0.685644] swapper/0/1 is trying to acquire lock: [ 0.685650] (&(&pcie->used_msi_lock)->rlock){......}, at: [] update_msi_enable+0x2c/0x58 [ 0.685692] but task is already holding lock: [ 0.685698] (&irq_desc_lock_class){......}, at: [] __setup_irq+0xa4/0x5f0 [ 0.685718] which lock already depends on the new lock. [ 0.685718] [ 0.685725] [ 0.685725] the existing dependency chain (in reverse order) is: [ 0.685731] [ 0.685731] -> #1 (&irq_desc_lock_class){......}: [ 0.685758] __irq_get_desc_lock+0x54/0x94 [ 0.685768] __irq_set_handler+0x24/0x54 [ 0.685778] irq_domain_set_info+0x38/0x4c [ 0.685786] tango_irq_domain_alloc+0x98/0xbc [ 0.685795] msi_domain_alloc+0x68/0x130 [ 0.685802] __irq_domain_alloc_irqs+0x11c/0x2ac [ 0.685809] msi_domain_alloc_irqs+0x78/0x188 [ 0.685816] __pci_enable_msi_range+0x200/0x37c [ 0.685824] pcie_port_device_register+0x3cc/0x494 [ 0.685831] pcie_portdrv_probe+0x2c/0xa4 [ 0.685844] pci_device_probe+0x8c/0xe8 [ 0.685852] driver_probe_device+0x1c8/0x2ac [ 0.685863] bus_for_each_drv+0x60/0x94 [ 0.685870] __device_attach+0xb4/0x118 [ 0.685883] pci_bus_add_device+0x44/0x90 [ 0.685891] pci_bus_add_devices+0x3c/0x80 [ 0.685898] pci_host_common_probe+0x100/0x314 [ 0.685906] platform_drv_probe+0x50/0xb0 [ 0.685912] driver_probe_device+0x21c/0x2ac [ 0.685918] __driver_attach+0xc0/0xc4 [ 0.685926] bus_for_each_dev+0x68/0x9c [ 0.685933] bus_add_driver+0x108/0x214 [ 0.685939] driver_register+0x78/0xf4 [ 0.685947] do_one_initcall+0x44/0x174 [ 0.685961] kernel_init_freeable+0x158/0x1e8 [ 0.685971] kernel_init+0x8/0x10c [ 0.685980] ret_from_fork+0x14/0x24 [ 0.685985] [ 0.685985] -> #0 (&(&pcie->used_msi_lock)->rlock){......}: [ 0.686003] _raw_spin_lock_irqsave+0x44/0x58 [ 0.686012] update_msi_enable+0x2c/0x58 [ 0.686019] irq_enable+0x30/0x44 [ 0.686025] irq_startup+0x80/0x84 [ 0.686031] __setup_irq+0x558/0x5f0 [ 0.686038] request_threaded_irq+0xe4/0x184 [ 0.686044] pcie_pme_probe+0xc4/0x1f0 [ 0.686051] pcie_port_probe_service+0x34/0x70 [ 0.686057] driver_probe_device+0x21c/0x2ac [ 0.686065] bus_for_each_drv+0x60/0x94 [ 0.686071] __device_attach+0xb4/0x118 [ 0.686079] bus_probe_device+0x88/0x90 [ 0.686085] device_add+0x3f4/0x584 [ 0.686092] pcie_port_device_register+0x228/0x494 [ 0.686099] pcie_portdrv_probe+0x2c/0xa4 [ 0.686106] pci_device_probe+0x8c/0xe8 [ 0.686113] driver_probe_device+0x1c8/0x2ac [ 0.686120] bus_for_each_drv+0x60/0x94 [ 0.686126] __device_attach+0xb4/0x118 [ 0.686134] pci_bus_add_device+0x44/0x90 [ 0.686141] pci_bus_add_devices+0x3c/0x80 [ 0.686149] pci_host_common_probe+0x100/0x314 [ 0.686155] platform_drv_probe+0x50/0xb0 [ 0.686161] driver_probe_device+0x21c/0x2ac [ 0.686167] __driver_attach+0xc0/0xc4 [ 0.686175] bus_for_each_dev+0x68/0x9c [ 0.686182] bus_add_driver+0x108/0x214 [ 0.686188] driver_register+0x78/0xf4 [ 0.686194] do_one_initcall+0x44/0x174 [ 0.686203] kernel_init_freeable+0x158/0x1e8 [ 0.686210] kernel_init+0x8/0x10c [ 0.686218] ret_from_fork+0x14/0x24 [ 0.686222] [ 0.686222] other info that might help us debug this: [ 0.686222] [ 0.686229] Possible unsafe locking scenario: [ 0.686229] [ 0.686235] CPU0 CPU1 [ 0.686239] ---- ---- [ 0.686243] lock(&irq_desc_lock_class); [ 0.686251] lock(&(&pcie->used_msi_lock)->rlock); [ 0.686260] lock(&irq_desc_lock_class); [ 0.686267] lock(&(&pcie->used_msi_lock)->rlock); [ 0.686275] [ 0.686275] *** DEADLOCK *** [ 0.686275] [ 0.686283] 5 locks held by swapper/0/1: [ 0.686288] #0: (&dev->mutex){......}, at: [] __driver_attach+0x50/0xc4 [ 0.686303] #1: (&dev->mutex){......}, at: [] __driver_attach+0x60/0xc4 [ 0.686318] #2: (&dev->mutex){......}, at: [] __device_attach+0x20/0x118 [ 0.686333] #3: (&dev->mutex){......}, at: [] __device_attach+0x20/0x118 [ 0.686348] #4: (&irq_desc_lock_class){......}, at: [] __setup_irq+0xa4/0x5f0 [ 0.686363] [ 0.686363] stack backtrace: [ 0.686373] CPU: 0 PID: 1 Comm: swapper/0 Tainted: G I 4.9.20-1-rc3 #2 [ 0.686379] Hardware name: Sigma Tango DT [ 0.686398] [] (unwind_backtrace) from [] (show_stack+0x10/0x14) [ 0.686411] [] (show_stack) from [] (dump_stack+0x98/0xc4) [ 0.686423] [] (dump_stack) from [] (print_circular_bug+0x214/0x334) [ 0.686432] [] (print_circular_bug) from [] (__lock_acquire+0x171c/0x1a28) [ 0.686441] [] (__lock_acquire) from [] (lock_acquire+0x6c/0x88) [ 0.686452] [] (lock_acquire) from [] (_raw_spin_lock_irqsave+0x44/0x58) [ 0.686464] [] (_raw_spin_lock_irqsave) from [] (update_msi_enable+0x2c/0x58) [ 0.686475] [] (update_msi_enable) from [] (irq_enable+0x30/0x44) [ 0.686484] [] (irq_enable) from [] (irq_startup+0x80/0x84) [ 0.686493] [] (irq_startup) from [] (__setup_irq+0x558/0x5f0) [ 0.686502] [] (__setup_irq) from [] (request_threaded_irq+0xe4/0x184) [ 0.686511] [] (request_threaded_irq) from [] (pcie_pme_probe+0xc4/0x1f0) [ 0.686520] [] (pcie_pme_probe) from [] (pcie_port_probe_service+0x34/0x70) [ 0.686530] [] (pcie_port_probe_service) from [] (driver_probe_device+0x21c/0x2ac) [ 0.686540] [] (driver_probe_device) from [] (bus_for_each_drv+0x60/0x94) [ 0.686550] [] (bus_for_each_drv) from [] (__device_attach+0xb4/0x118) [ 0.686560] [] (__device_attach) from [] (bus_probe_device+0x88/0x90) [ 0.686570] [] (bus_probe_device) from [] (device_add+0x3f4/0x584) [ 0.686580] [] (device_add) from [] (pcie_port_device_register+0x228/0x494) [ 0.686589] [] (pcie_port_device_register) from [] (pcie_portdrv_probe+0x2c/0xa4) [ 0.686600] [] (pcie_portdrv_probe) from [] (pci_device_probe+0x8c/0xe8) [ 0.686611] [] (pci_device_probe) from [] (driver_probe_device+0x1c8/0x2ac) [ 0.686620] [] (driver_probe_device) from [] (bus_for_each_drv+0x60/0x94) [ 0.686630] [] (bus_for_each_drv) from [] (__device_attach+0xb4/0x118) [ 0.686641] [] (__device_attach) from [] (pci_bus_add_device+0x44/0x90) [ 0.686651] [] (pci_bus_add_device) from [] (pci_bus_add_devices+0x3c/0x80) [ 0.686662] [] (pci_bus_add_devices) from [] (pci_host_common_probe+0x100/0x314) [ 0.686673] [] (pci_host_common_probe) from [] (platform_drv_probe+0x50/0xb0) [ 0.686682] [] (platform_drv_probe) from [] (driver_probe_device+0x21c/0x2ac) [ 0.686690] [] (driver_probe_device) from [] (__driver_attach+0xc0/0xc4) [ 0.686700] [] (__driver_attach) from [] (bus_for_each_dev+0x68/0x9c) [ 0.686711] [] (bus_for_each_dev) from [] (bus_add_driver+0x108/0x214) [ 0.686721] [] (bus_add_driver) from [] (driver_register+0x78/0xf4) [ 0.686730] [] (driver_register) from [] (do_one_initcall+0x44/0x174) [ 0.686742] [] (do_one_initcall) from [] (kernel_init_freeable+0x158/0x1e8) [ 0.686754] [] (kernel_init_freeable) from [] (kernel_init+0x8/0x10c) [ 0.686765] [] (kernel_init) from [] (ret_from_fork+0x14/0x24) [ 0.686824] pcieport 0000:00:00.0: Signaling PME through PCIe PME interrupt [ 0.686835] pci 0000:01:00.0: Signaling PME through PCIe PME interrupt [ 0.686849] pcie_pme 0000:00:00.0:pcie001: service driver pcie_pme loaded [ 0.687049] aer 0000:00:00.0:pcie002: service driver aer loaded [ 0.687276] pci 0000:01:00.0: enabling device (0140 -> 0142)