From mboxrd@z Thu Jan 1 00:00:00 1970 From: Joerg Roedel Subject: Re: WARNING: CPU: 4 PID: 863 at include/drm/drm_crtc.h:1577 drm_helper_choose_encoder_dpms+0x88/0x90() - evildoer found and neutralized Date: Wed, 30 Sep 2015 20:06:21 +0200 Message-ID: <20150930180621.GV3036@8bytes.org> References: <20150923085950.GA3440@pd.tnic> <20150923144450.GD3383@phenom.ffwll.local> <20150923160621.GA3446@pd.tnic> <20150923161839.GB3446@pd.tnic> <20150926164651.GA3640@pd.tnic> <560A50DC.1040505@linux.intel.com> <20150929105138.GA12037@nazgul.tnic> <560B9323.6000309@linux.intel.com> <20150930124432.GS3036@8bytes.org> <560C153C.10600@linux.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from theia.8bytes.org (8bytes.org [81.169.241.247]) by gabe.freedesktop.org (Postfix) with ESMTPS id ABA976E3E4 for ; Wed, 30 Sep 2015 11:06:23 -0700 (PDT) Content-Disposition: inline In-Reply-To: <560C153C.10600@linux.intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Jiang Liu Cc: lkml , Maling list - DRI developers , Borislav Petkov , Bjorn Helgaas , Alex Deucher , Thomas Gleixner , Christian =?iso-8859-1?Q?K=F6nig?= List-Id: dri-devel@lists.freedesktop.org T24gVGh1LCBPY3QgMDEsIDIwMTUgYXQgMDE6MDA6NDRBTSArMDgwMCwgSmlhbmcgTGl1IHdyb3Rl Ogo+IFRoYW5rcyBKb2VyZywgdGhhdCBtYWtlcyBzZW5zZS4gSWYgc29tZSBkcml2ZXIgdHJpZXMg dG8gYmluZGluZyB0byB0aGUKPiBJT01NVSBkZXZpY2UsIGl0IHdpbGwgdHJpZ2dlciB0aGUgc2Nl bmFyaW8gYXMgeW91IGRlc2NyaWJlZC4gRm9yCj4gZXhhbXBsZSwgIFhlbiBiYWNrZW5kIGRyaXZl ciB3aWxsIHRyeSB0byBwcm9iZSBhbGwgUENJIGRldmljZXMKPiBpZiBlbmFibGVkLiBJIHdpbGwg ZG8gbW9yZSBpbnZlc3RpZ2F0aW9uIHRvbW9ycm93LgoKTm90IG9ubHkgdGhhdCwgdGhlIHByb2Jl IGNvZGUgbG9va3MgbGlrZSB0aGlzIGluIF9fcGNpX2RldmljZV9wcm9iZToKCiAgICAgICAgICAg ICAgICBlcnJvciA9IC1FTk9ERVY7CgogICAgICAgICAgICAgICAgaWQgPSBwY2lfbWF0Y2hfZGV2 aWNlKGRydiwgcGNpX2Rldik7CiAgICAgICAgICAgICAgICBpZiAoaWQpCiAgICAgICAgICAgICAg ICAgICAgICAgIGVycm9yID0gcGNpX2NhbGxfcHJvYmUoZHJ2LCBwY2lfZGV2LCBpZCk7CiAgICAg ICAgICAgICAgICBpZiAoZXJyb3IgPj0gMCkKICAgICAgICAgICAgICAgICAgICAgICAgZXJyb3Ig PSAwOwoKVGhlIHBjaV9tYXRjaF9kZXZpY2UoKSBmdW5jdGlvbiB3aWxsIGFsd2F5cyByZXR1cm4g TlVMTCBmb3IgdGhlIGlvbW11CnBjaV9kZXYsIGJlY2F1c2Ugbm8gZHJpdmVyIG1hdGNoZXMgdGhl IGlkcyBvZiBpdC4gU28gdGhlIGZ1bmN0aW9uCnJldHVybnMgLUVOT0RFViwgd2hpY2ggd2lsbCBi ZSBoYW5kbGVkIGluIHRoZSBjYWxsZXIgKHBjaV9kZXZpY2VfcHJvYmUpOgoKCiAgICAgICAgZXJy b3IgPSBwY2liaW9zX2FsbG9jX2lycShwY2lfZGV2KTsKICAgICAgICBpZiAoZXJyb3IgPCAwKQog ICAgICAgICAgICAgICAgcmV0dXJuIGVycm9yOwoKICAgICAgICBwY2lfZGV2X2dldChwY2lfZGV2 KTsKICAgICAgICBlcnJvciA9IF9fcGNpX2RldmljZV9wcm9iZShkcnYsIHBjaV9kZXYpOwogICAg ICAgIGlmIChlcnJvcikgewogICAgICAgICAgICAgICAgcGNpYmlvc19mcmVlX2lycShwY2lfZGV2 KTsKICAgICAgICAgICAgICAgIHBjaV9kZXZfcHV0KHBjaV9kZXYpOwogICAgICAgIH0KCkZvciB0 aGUgSU9NTVUgcGNpX2RldiBhIHBjaWJpb3MtaXJxIHdpbGwgYmUgYWxsb2NhdGVkIChpZiB0aGVy ZSBpcyBvbmUsCmxpa2Ugb24gQm9yaXMnIHN5c3RlbSkgYW5kIGJlY2F1c2UgX19wY2lfZGV2aWNl X3Byb2JlIHJldHVybnMgLUVOT0RFViBpdAp3aWxsIGJlIGZyZWVkIGFnYWluIHdpdGggcGNpYmlv c19mcmVlX2lycSgpLgoKVGhlIHBjaWJpb3NfZnJlZV9pcnEoKSBmdW5jdGlvbiB3aWxsIHNldCBk ZXYtPmlycSA9IDAsIHdoaWNoIG92ZXJ3cml0ZXMKdGhlIHZhbHVlIHRoYXQgcGNpX2VuYWJsZV9t c2koKSB3cm90ZSB0aGVyZS4gU28gbGF0ZXIgaW4gc3VzcGVuZC9yZXN1bWUKY29kZSB0aGUgbXNp LWhhbmRsaW5nIHBhcnQgdHJpZXMgdG8gZmV0Y2ggdGhlIGlycS1kZXNjcmlwdG9yIGZvciB0aGUK d3JvbmcgaXJxICh3aGljaCBpcyBOVUxMKSBhbmQgY2F1c2VzIHRoZSBjcmFzaC4KClRoZSBpc3N1 ZSBnb3QgaW50cm9kdWNlZCBiZWNhdXNlIHdpdGggeW91ciBjaGFuZ2VzIHBjaV9lbmFibGVfbXNp KCkgaXMKb25seSBhbGxvd2VkIGFmdGVyIGEgcGNpLWRldmljZSB3YXMgc3VjY2Vzc2Z1bGx5IHBy b2JlZCBieSB0aGUgZHJpdmVyLgpCdXQgdGhpcyBhc3N1bXB0aW9uIGlzIG5vdCB0cnVlLCBhcyB0 aGUgQU1EIElPTU1VIGRyaXZlciBkb2VzIG5vdApyZWdpc3RlciBhcyBhIHBjaS1kcml2ZXIuCgpS ZWdpc3RlcmluZyBhIHBjaS1kcml2ZXIgd291bGQgYWN0dWFsbHkgYmUgaGFybWZ1bCwgYmVjYXVz ZSBhIGRldmljZSBjYW4KYmUgZm9yY2libHkgdW5ib3VuZCBmcm9tIGl0cyBkcml2ZXIsIHdoaWNo IHdvdWxkIGJlIHByZXR0eSBiYWQgZm9yIGFuCklPTU1VIGluIHRoZSBydW5uaW5nIHN5c3RlbS4K ClNvIHRoZSByaWdodCBmaXggaXMgdG8gYWxsb3cgcGNpX2VuYWJsZV9tc2koKSBmb3IgcGNpLWRl dmljZXMgbm90CnJlZ2lzdGVyZWQgYWdhaW5zdCBhIGRyaXZlci4gVGhlIGZpeCBJIHNlbnQgQm9y aXMgaGFzIGlzc3VlcyAoSSB0aGluayBpdApsZWFrcyBwY2liaW9zIGlycXMgd2hlbiBNU0kgaXMg aW4gdXNlKSwgYnV0IHdhcyB0aGlua2luZyBhYm91dCBmaXhpbmcgaXQKaW4gcGNpX2RldmljZV9w cm9iZSBieSBub3QgYWxsb2NhdGluZyBhIHBjaWJpb3MtaXJxIHdoZW4gTVNJIGlzIGFscmVhZHkK YWN0aXZlLiBXaGF0IGRvIHlvdSB0aGluaz8KClJlZ2FyZHMsCgoJSm9lcmcKX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVsIG1haWxpbmcgbGlz dApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHA6Ly9saXN0cy5mcmVlZGVza3Rv cC5vcmcvbWFpbG1hbi9saXN0aW5mby9kcmktZGV2ZWwK From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932934AbbI3SGY (ORCPT ); Wed, 30 Sep 2015 14:06:24 -0400 Received: from 8bytes.org ([81.169.241.247]:42887 "EHLO theia.8bytes.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932190AbbI3SGX (ORCPT ); Wed, 30 Sep 2015 14:06:23 -0400 Date: Wed, 30 Sep 2015 20:06:21 +0200 From: Joerg Roedel To: Jiang Liu Cc: Borislav Petkov , Daniel Vetter , Thomas Gleixner , Bjorn Helgaas , Alex Deucher , Alex Deucher , Christian =?iso-8859-1?Q?K=F6nig?= , Maling list - DRI developers , lkml Subject: Re: WARNING: CPU: 4 PID: 863 at include/drm/drm_crtc.h:1577 drm_helper_choose_encoder_dpms+0x88/0x90() - evildoer found and neutralized Message-ID: <20150930180621.GV3036@8bytes.org> References: <20150923085950.GA3440@pd.tnic> <20150923144450.GD3383@phenom.ffwll.local> <20150923160621.GA3446@pd.tnic> <20150923161839.GB3446@pd.tnic> <20150926164651.GA3640@pd.tnic> <560A50DC.1040505@linux.intel.com> <20150929105138.GA12037@nazgul.tnic> <560B9323.6000309@linux.intel.com> <20150930124432.GS3036@8bytes.org> <560C153C.10600@linux.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <560C153C.10600@linux.intel.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Oct 01, 2015 at 01:00:44AM +0800, Jiang Liu wrote: > Thanks Joerg, that makes sense. If some driver tries to binding to the > IOMMU device, it will trigger the scenario as you described. For > example, Xen backend driver will try to probe all PCI devices > if enabled. I will do more investigation tomorrow. Not only that, the probe code looks like this in __pci_device_probe: error = -ENODEV; id = pci_match_device(drv, pci_dev); if (id) error = pci_call_probe(drv, pci_dev, id); if (error >= 0) error = 0; The pci_match_device() function will always return NULL for the iommu pci_dev, because no driver matches the ids of it. So the function returns -ENODEV, which will be handled in the caller (pci_device_probe): error = pcibios_alloc_irq(pci_dev); if (error < 0) return error; pci_dev_get(pci_dev); error = __pci_device_probe(drv, pci_dev); if (error) { pcibios_free_irq(pci_dev); pci_dev_put(pci_dev); } For the IOMMU pci_dev a pcibios-irq will be allocated (if there is one, like on Boris' system) and because __pci_device_probe returns -ENODEV it will be freed again with pcibios_free_irq(). The pcibios_free_irq() function will set dev->irq = 0, which overwrites the value that pci_enable_msi() wrote there. So later in suspend/resume code the msi-handling part tries to fetch the irq-descriptor for the wrong irq (which is NULL) and causes the crash. The issue got introduced because with your changes pci_enable_msi() is only allowed after a pci-device was successfully probed by the driver. But this assumption is not true, as the AMD IOMMU driver does not register as a pci-driver. Registering a pci-driver would actually be harmful, because a device can be forcibly unbound from its driver, which would be pretty bad for an IOMMU in the running system. So the right fix is to allow pci_enable_msi() for pci-devices not registered against a driver. The fix I sent Boris has issues (I think it leaks pcibios irqs when MSI is in use), but was thinking about fixing it in pci_device_probe by not allocating a pcibios-irq when MSI is already active. What do you think? Regards, Joerg