| Message ID | CAJZ5v0idFBHG=OKRiBqvsYPgUrWQsqjTKYXfKxfdG023qsCbkw@mail.gmail.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On Sun, Jan 08, 2017 at 03:20:20AM +0100, Rafael J. Wysocki wrote: > drivers/iommu/amd_iommu_init.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > Index: linux-pm/drivers/iommu/amd_iommu_init.c > =================================================================== > --- linux-pm.orig/drivers/iommu/amd_iommu_init.c > +++ linux-pm/drivers/iommu/amd_iommu_init.c > @@ -2230,7 +2230,7 @@ static int __init early_amd_iommu_init(v > */ > ret = check_ivrs_checksum(ivrs_base); > if (ret) > - return ret; > + goto out; > > amd_iommu_target_ivhd_type = get_highest_supported_ivhd_type(ivrs_base); > DUMP_printk("Using IVHD type %#x\n", amd_iommu_target_ivhd_type); Good catch, this one needs to be applied regardless. However, it doesn't fix my issue though. But I think I have it - I went and applied the well-proven debugging technique of sprinkling printks around. Here's what I'm seeing: early_amd_iommu_init() |-> acpi_put_table(ivrs_base); |-> acpi_tb_put_table(table_desc); |-> acpi_tb_invalidate_table(table_desc); |-> acpi_tb_release_table(...) |-> acpi_os_unmap_memory |-> acpi_os_unmap_iomem |-> acpi_os_map_cleanup |-> synchronize_rcu_expedited <-- the kernel/rcu/tree_exp.h version with CONFIG_PREEMPT_RCU=y Now that function goes and sends IPIs, i.e., schedule_work() but this is too early - we haven't even done workqueue_init(). Actually, from looking at the callstack, we do kernel_init_freeable->native_smp_prepare_cpus() and workqueue_init() comes next. And this makes sense because the splat rIP points to __queue_work() but we haven't done that yet. So that acpi_put_table() is happening too early. Looks like AMD IOMMU should not put the table but WTH do I know?! In any case, commenting out: acpi_put_table(ivrs_base); ivrs_base = NULL; and the end of early_amd_iommu_init() makes the box boot again.
SGksDQoNCj4gRnJvbTogbGludXgtYWNwaS1vd25lckB2Z2VyLmtlcm5lbC5vcmcgW21haWx0bzps aW51eC1hY3BpLW93bmVyQHZnZXIua2VybmVsLm9yZ10gT24gQmVoYWxmIE9mIEJvcmlzbGF2DQo+ IFBldGtvdg0KPiBTdWJqZWN0OiBSZTogMTc0Y2M3MTg3ZTZmIEFDUElDQTogVGFibGVzOiBCYWNr IHBvcnQgYWNwaV9nZXRfdGFibGVfd2l0aF9zaXplKCkgYW5kDQo+IGVhcmx5X2FjcGlfb3NfdW5t YXBfbWVtb3J5KCkgZnJvbSBMaW51eCBrZXJuZWwNCj4gDQo+IE9uIFN1biwgSmFuIDA4LCAyMDE3 IGF0IDAzOjIwOjIwQU0gKzAxMDAsIFJhZmFlbCBKLiBXeXNvY2tpIHdyb3RlOg0KPiA+ICBkcml2 ZXJzL2lvbW11L2FtZF9pb21tdV9pbml0LmMgfCAgICAyICstDQo+ID4gIDEgZmlsZSBjaGFuZ2Vk LCAxIGluc2VydGlvbigrKSwgMSBkZWxldGlvbigtKQ0KPiA+DQo+ID4gSW5kZXg6IGxpbnV4LXBt L2RyaXZlcnMvaW9tbXUvYW1kX2lvbW11X2luaXQuYw0KPiA+ID09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0NCj4gPiAtLS0g bGludXgtcG0ub3JpZy9kcml2ZXJzL2lvbW11L2FtZF9pb21tdV9pbml0LmMNCj4gPiArKysgbGlu dXgtcG0vZHJpdmVycy9pb21tdS9hbWRfaW9tbXVfaW5pdC5jDQo+ID4gQEAgLTIyMzAsNyArMjIz MCw3IEBAIHN0YXRpYyBpbnQgX19pbml0IGVhcmx5X2FtZF9pb21tdV9pbml0KHYNCj4gPiAgCSAq Lw0KPiA+ICAJcmV0ID0gY2hlY2tfaXZyc19jaGVja3N1bShpdnJzX2Jhc2UpOw0KPiA+ICAJaWYg KHJldCkNCj4gPiAtCQlyZXR1cm4gcmV0Ow0KPiA+ICsJCWdvdG8gb3V0Ow0KPiA+DQo+ID4gIAlh bWRfaW9tbXVfdGFyZ2V0X2l2aGRfdHlwZSA9IGdldF9oaWdoZXN0X3N1cHBvcnRlZF9pdmhkX3R5 cGUoaXZyc19iYXNlKTsNCj4gPiAgCURVTVBfcHJpbnRrKCJVc2luZyBJVkhEIHR5cGUgJSN4XG4i LCBhbWRfaW9tbXVfdGFyZ2V0X2l2aGRfdHlwZSk7DQo+IA0KPiBHb29kIGNhdGNoLCB0aGlzIG9u ZSBuZWVkcyB0byBiZSBhcHBsaWVkIHJlZ2FyZGxlc3MuDQo+IA0KPiBIb3dldmVyLCBpdCBkb2Vz bid0IGZpeCBteSBpc3N1ZSB0aG91Z2guDQo+IA0KPiBCdXQgSSB0aGluayBJIGhhdmUgaXQgLSBJ IHdlbnQgYW5kIGFwcGxpZWQgdGhlIHdlbGwtcHJvdmVuIGRlYnVnZ2luZw0KPiB0ZWNobmlxdWUg b2Ygc3ByaW5rbGluZyBwcmludGtzIGFyb3VuZC4gSGVyZSdzIHdoYXQgSSdtIHNlZWluZzoNCj4g DQo+IGVhcmx5X2FtZF9pb21tdV9pbml0KCkNCj4gfC0+IGFjcGlfcHV0X3RhYmxlKGl2cnNfYmFz ZSk7DQo+IHwtPiBhY3BpX3RiX3B1dF90YWJsZSh0YWJsZV9kZXNjKTsNCj4gfC0+IGFjcGlfdGJf aW52YWxpZGF0ZV90YWJsZSh0YWJsZV9kZXNjKTsNCj4gfC0+IGFjcGlfdGJfcmVsZWFzZV90YWJs ZSguLi4pDQo+IHwtPiBhY3BpX29zX3VubWFwX21lbW9yeQ0KPiB8LT4gYWNwaV9vc191bm1hcF9p b21lbQ0KPiB8LT4gYWNwaV9vc19tYXBfY2xlYW51cA0KPiB8LT4gc3luY2hyb25pemVfcmN1X2V4 cGVkaXRlZAk8LS0gdGhlIGtlcm5lbC9yY3UvdHJlZV9leHAuaCB2ZXJzaW9uIHdpdGggQ09ORklH X1BSRUVNUFRfUkNVPXkNCj4gDQo+IE5vdyB0aGF0IGZ1bmN0aW9uIGdvZXMgYW5kIHNlbmRzIElQ SXMsIGkuZS4sIHNjaGVkdWxlX3dvcmsoKQ0KPiBidXQgdGhpcyBpcyB0b28gZWFybHkgLSB3ZSBo YXZlbid0IGV2ZW4gZG9uZSB3b3JrcXVldWVfaW5pdCgpLg0KPiBBY3R1YWxseSwgZnJvbSBsb29r aW5nIGF0IHRoZSBjYWxsc3RhY2ssIHdlIGRvDQo+IGtlcm5lbF9pbml0X2ZyZWVhYmxlLT5uYXRp dmVfc21wX3ByZXBhcmVfY3B1cygpIGFuZCB3b3JrcXVldWVfaW5pdCgpDQo+IGNvbWVzIG5leHQu DQo+IA0KPiBBbmQgdGhpcyBtYWtlcyBzZW5zZSBiZWNhdXNlIHRoZSBzcGxhdCBySVAgcG9pbnRz IHRvIF9fcXVldWVfd29yaygpIGJ1dA0KPiB3ZSBoYXZlbid0IGRvbmUgdGhhdCB5ZXQuDQo+IA0K PiBTbyB0aGF0IGFjcGlfcHV0X3RhYmxlKCkgaXMgaGFwcGVuaW5nIHRvbyBlYXJseS4gTG9va3Mg bGlrZSBBTUQgSU9NTVUNCj4gc2hvdWxkIG5vdCBwdXQgdGhlIHRhYmxlIGJ1dCBXVEggZG8gSSBr bm93PyENCj4gDQo+IEluIGFueSBjYXNlLCBjb21tZW50aW5nIG91dDoNCj4gDQo+ICAgICAgICAg YWNwaV9wdXRfdGFibGUoaXZyc19iYXNlKTsNCj4gICAgICAgICBpdnJzX2Jhc2UgPSBOVUxMOw0K PiANCj4gYW5kIHRoZSBlbmQgb2YgZWFybHlfYW1kX2lvbW11X2luaXQoKSBtYWtlcyB0aGUgYm94 IGJvb3QgYWdhaW4uDQoNClNvIHBsZWFzZSBoZWxwIHRvIGNvbW1lbnQgb3V0IHRoZXNlIDIgbGlu ZXMgKHdpdGggZGVzY3JpcHRpb25zIGFuZCBkbyBub3QgZGVsZXRlIHRoZW0pLg0KVW50aWwgYWNw aV9vc191bm1hcF9tZW1vcnkoKSBpcyBhYmxlIHRvIGhhbmRsZSBzdWNoIGFuIGVhcmx5IGNhc2Uu DQoNClRoYW5rcyBhbmQgYmVzdCByZWdhcmRzDQpMdg0KDQo+IA0KPiAtLQ0KPiBSZWdhcmRzL0dy dXNzLA0KPiAgICAgQm9yaXMuDQo+IA0KPiBHb29kIG1haWxpbmcgcHJhY3RpY2VzIGZvciA0MDA6 IGF2b2lkIHRvcC1wb3N0aW5nIGFuZCB0cmltIHRoZSByZXBseS4NCj4gLS0NCj4gVG8gdW5zdWJz Y3JpYmUgZnJvbSB0aGlzIGxpc3Q6IHNlbmQgdGhlIGxpbmUgInVuc3Vic2NyaWJlIGxpbnV4LWFj cGkiIGluDQo+IHRoZSBib2R5IG9mIGEgbWVzc2FnZSB0byBtYWpvcmRvbW9Admdlci5rZXJuZWwu b3JnDQo+IE1vcmUgbWFqb3Jkb21vIGluZm8gYXQgIGh0dHA6Ly92Z2VyLmtlcm5lbC5vcmcvbWFq b3Jkb21vLWluZm8uaHRtbA0K -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
SGksDQoNCj4gRnJvbTogbGludXgtYWNwaS1vd25lckB2Z2VyLmtlcm5lbC5vcmcgW21haWx0bzps aW51eC1hY3BpLW93bmVyQHZnZXIua2VybmVsLm9yZ10gT24gQmVoYWxmIE9mIFpoZW5nLA0KPiBM dg0KPiBTdWJqZWN0OiBSRTogMTc0Y2M3MTg3ZTZmIEFDUElDQTogVGFibGVzOiBCYWNrIHBvcnQg YWNwaV9nZXRfdGFibGVfd2l0aF9zaXplKCkgYW5kDQo+IGVhcmx5X2FjcGlfb3NfdW5tYXBfbWVt b3J5KCkgZnJvbSBMaW51eCBrZXJuZWwNCj4gDQo+IEhpLA0KPiANCj4gPiBGcm9tOiBsaW51eC1h Y3BpLW93bmVyQHZnZXIua2VybmVsLm9yZyBbbWFpbHRvOmxpbnV4LWFjcGktb3duZXJAdmdlci5r ZXJuZWwub3JnXSBPbiBCZWhhbGYgT2YNCj4gQm9yaXNsYXYNCj4gPiBQZXRrb3YNCj4gPiBTdWJq ZWN0OiBSZTogMTc0Y2M3MTg3ZTZmIEFDUElDQTogVGFibGVzOiBCYWNrIHBvcnQgYWNwaV9nZXRf dGFibGVfd2l0aF9zaXplKCkgYW5kDQo+ID4gZWFybHlfYWNwaV9vc191bm1hcF9tZW1vcnkoKSBm cm9tIExpbnV4IGtlcm5lbA0KPiA+DQo+ID4gT24gU3VuLCBKYW4gMDgsIDIwMTcgYXQgMDM6MjA6 MjBBTSArMDEwMCwgUmFmYWVsIEouIFd5c29ja2kgd3JvdGU6DQo+ID4gPiAgZHJpdmVycy9pb21t dS9hbWRfaW9tbXVfaW5pdC5jIHwgICAgMiArLQ0KPiA+ID4gIDEgZmlsZSBjaGFuZ2VkLCAxIGlu c2VydGlvbigrKSwgMSBkZWxldGlvbigtKQ0KPiA+ID4NCj4gPiA+IEluZGV4OiBsaW51eC1wbS9k cml2ZXJzL2lvbW11L2FtZF9pb21tdV9pbml0LmMNCj4gPiA+ID09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0NCj4gPiA+IC0t LSBsaW51eC1wbS5vcmlnL2RyaXZlcnMvaW9tbXUvYW1kX2lvbW11X2luaXQuYw0KPiA+ID4gKysr IGxpbnV4LXBtL2RyaXZlcnMvaW9tbXUvYW1kX2lvbW11X2luaXQuYw0KPiA+ID4gQEAgLTIyMzAs NyArMjIzMCw3IEBAIHN0YXRpYyBpbnQgX19pbml0IGVhcmx5X2FtZF9pb21tdV9pbml0KHYNCj4g PiA+ICAJICovDQo+ID4gPiAgCXJldCA9IGNoZWNrX2l2cnNfY2hlY2tzdW0oaXZyc19iYXNlKTsN Cj4gPiA+ICAJaWYgKHJldCkNCj4gPiA+IC0JCXJldHVybiByZXQ7DQo+ID4gPiArCQlnb3RvIG91 dDsNCj4gPiA+DQo+ID4gPiAgCWFtZF9pb21tdV90YXJnZXRfaXZoZF90eXBlID0gZ2V0X2hpZ2hl c3Rfc3VwcG9ydGVkX2l2aGRfdHlwZShpdnJzX2Jhc2UpOw0KPiA+ID4gIAlEVU1QX3ByaW50aygi VXNpbmcgSVZIRCB0eXBlICUjeFxuIiwgYW1kX2lvbW11X3RhcmdldF9pdmhkX3R5cGUpOw0KPiA+ DQo+ID4gR29vZCBjYXRjaCwgdGhpcyBvbmUgbmVlZHMgdG8gYmUgYXBwbGllZCByZWdhcmRsZXNz Lg0KPiA+DQo+ID4gSG93ZXZlciwgaXQgZG9lc24ndCBmaXggbXkgaXNzdWUgdGhvdWdoLg0KPiA+ DQo+ID4gQnV0IEkgdGhpbmsgSSBoYXZlIGl0IC0gSSB3ZW50IGFuZCBhcHBsaWVkIHRoZSB3ZWxs LXByb3ZlbiBkZWJ1Z2dpbmcNCj4gPiB0ZWNobmlxdWUgb2Ygc3ByaW5rbGluZyBwcmludGtzIGFy b3VuZC4gSGVyZSdzIHdoYXQgSSdtIHNlZWluZzoNCj4gPg0KPiA+IGVhcmx5X2FtZF9pb21tdV9p bml0KCkNCj4gPiB8LT4gYWNwaV9wdXRfdGFibGUoaXZyc19iYXNlKTsNCj4gPiB8LT4gYWNwaV90 Yl9wdXRfdGFibGUodGFibGVfZGVzYyk7DQo+ID4gfC0+IGFjcGlfdGJfaW52YWxpZGF0ZV90YWJs ZSh0YWJsZV9kZXNjKTsNCj4gPiB8LT4gYWNwaV90Yl9yZWxlYXNlX3RhYmxlKC4uLikNCj4gPiB8 LT4gYWNwaV9vc191bm1hcF9tZW1vcnkNCj4gPiB8LT4gYWNwaV9vc191bm1hcF9pb21lbQ0KPiA+ IHwtPiBhY3BpX29zX21hcF9jbGVhbnVwDQo+ID4gfC0+IHN5bmNocm9uaXplX3JjdV9leHBlZGl0 ZWQJPC0tIHRoZSBrZXJuZWwvcmN1L3RyZWVfZXhwLmggdmVyc2lvbiB3aXRoIENPTkZJR19QUkVF TVBUX1JDVT15DQo+ID4NCj4gPiBOb3cgdGhhdCBmdW5jdGlvbiBnb2VzIGFuZCBzZW5kcyBJUElz LCBpLmUuLCBzY2hlZHVsZV93b3JrKCkNCj4gPiBidXQgdGhpcyBpcyB0b28gZWFybHkgLSB3ZSBo YXZlbid0IGV2ZW4gZG9uZSB3b3JrcXVldWVfaW5pdCgpLg0KPiA+IEFjdHVhbGx5LCBmcm9tIGxv b2tpbmcgYXQgdGhlIGNhbGxzdGFjaywgd2UgZG8NCj4gPiBrZXJuZWxfaW5pdF9mcmVlYWJsZS0+ bmF0aXZlX3NtcF9wcmVwYXJlX2NwdXMoKSBhbmQgd29ya3F1ZXVlX2luaXQoKQ0KPiA+IGNvbWVz IG5leHQuDQo+ID4NCj4gPiBBbmQgdGhpcyBtYWtlcyBzZW5zZSBiZWNhdXNlIHRoZSBzcGxhdCBy SVAgcG9pbnRzIHRvIF9fcXVldWVfd29yaygpIGJ1dA0KPiA+IHdlIGhhdmVuJ3QgZG9uZSB0aGF0 IHlldC4NCj4gPg0KPiA+IFNvIHRoYXQgYWNwaV9wdXRfdGFibGUoKSBpcyBoYXBwZW5pbmcgdG9v IGVhcmx5LiBMb29rcyBsaWtlIEFNRCBJT01NVQ0KPiA+IHNob3VsZCBub3QgcHV0IHRoZSB0YWJs ZSBidXQgV1RIIGRvIEkga25vdz8hDQo+ID4NCj4gPiBJbiBhbnkgY2FzZSwgY29tbWVudGluZyBv dXQ6DQo+ID4NCj4gPiAgICAgICAgIGFjcGlfcHV0X3RhYmxlKGl2cnNfYmFzZSk7DQo+ID4gICAg ICAgICBpdnJzX2Jhc2UgPSBOVUxMOw0KPiA+DQo+ID4gYW5kIHRoZSBlbmQgb2YgZWFybHlfYW1k X2lvbW11X2luaXQoKSBtYWtlcyB0aGUgYm94IGJvb3QgYWdhaW4uDQo+IA0KPiBTbyBwbGVhc2Ug aGVscCB0byBjb21tZW50IG91dCB0aGVzZSAyIGxpbmVzICh3aXRoIGRlc2NyaXB0aW9ucyBhbmQg ZG8gbm90IGRlbGV0ZSB0aGVtKS4NCj4gVW50aWwgYWNwaV9vc191bm1hcF9tZW1vcnkoKSBpcyBh YmxlIHRvIGhhbmRsZSBzdWNoIGFuIGVhcmx5IGNhc2UuDQoNCklNTywgc3luY2hyb25pemVfcmN1 X2V4cGVkaXRlZCgpIHNob3VsZCBiZSBpbXByb3ZlZDoNCklmIHJjdV9pbml0KCkgaXNuJ3QgY2Fs bGVkIG9yIHRoZXJlIGlzIG5vdGhpbmcgdG8gc3luY2hyb25pemUsIHNjaGVkdWxlX3dvcmsoKSBz aG91bGRuJ3QgYmUgaW52b2tlZC4NCg0KVGhhbmtzIGFuZCBiZXN0IHJlZ2FyZHMNCkx2DQoNCj4g DQo+IFRoYW5rcyBhbmQgYmVzdCByZWdhcmRzDQo+IEx2DQo+IA0KPiA+DQo+ID4gLS0NCj4gPiBS ZWdhcmRzL0dydXNzLA0KPiA+ICAgICBCb3Jpcy4NCj4gPg0KPiA+IEdvb2QgbWFpbGluZyBwcmFj dGljZXMgZm9yIDQwMDogYXZvaWQgdG9wLXBvc3RpbmcgYW5kIHRyaW0gdGhlIHJlcGx5Lg0K -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, Borislav > From: Zheng, Lv > Subject: RE: 174cc7187e6f ACPICA: Tables: Back port acpi_get_table_with_size() and > early_acpi_os_unmap_memory() from Linux kernel > > Hi, > > > From: linux-acpi-owner@vger.kernel.org [mailto:linux-acpi-owner@vger.kernel.org] On Behalf Of Zheng, > > Lv > > Subject: RE: 174cc7187e6f ACPICA: Tables: Back port acpi_get_table_with_size() and > > early_acpi_os_unmap_memory() from Linux kernel > > > > Hi, > > > > > From: linux-acpi-owner@vger.kernel.org [mailto:linux-acpi-owner@vger.kernel.org] On Behalf Of > > Borislav > > > Petkov > > > Subject: Re: 174cc7187e6f ACPICA: Tables: Back port acpi_get_table_with_size() and > > > early_acpi_os_unmap_memory() from Linux kernel > > > > > > On Sun, Jan 08, 2017 at 03:20:20AM +0100, Rafael J. Wysocki wrote: > > > > drivers/iommu/amd_iommu_init.c | 2 +- > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > Index: linux-pm/drivers/iommu/amd_iommu_init.c > > > > =================================================================== > > > > --- linux-pm.orig/drivers/iommu/amd_iommu_init.c > > > > +++ linux-pm/drivers/iommu/amd_iommu_init.c > > > > @@ -2230,7 +2230,7 @@ static int __init early_amd_iommu_init(v > > > > */ > > > > ret = check_ivrs_checksum(ivrs_base); > > > > if (ret) > > > > - return ret; > > > > + goto out; > > > > > > > > amd_iommu_target_ivhd_type = get_highest_supported_ivhd_type(ivrs_base); > > > > DUMP_printk("Using IVHD type %#x\n", amd_iommu_target_ivhd_type); > > > > > > Good catch, this one needs to be applied regardless. > > > > > > However, it doesn't fix my issue though. > > > > > > But I think I have it - I went and applied the well-proven debugging > > > technique of sprinkling printks around. Here's what I'm seeing: > > > > > > early_amd_iommu_init() > > > |-> acpi_put_table(ivrs_base); > > > |-> acpi_tb_put_table(table_desc); > > > |-> acpi_tb_invalidate_table(table_desc); > > > |-> acpi_tb_release_table(...) > > > |-> acpi_os_unmap_memory > > > |-> acpi_os_unmap_iomem > > > |-> acpi_os_map_cleanup > > > |-> synchronize_rcu_expedited <-- the kernel/rcu/tree_exp.h version with CONFIG_PREEMPT_RCU=y > > > > > > Now that function goes and sends IPIs, i.e., schedule_work() > > > but this is too early - we haven't even done workqueue_init(). > > > Actually, from looking at the callstack, we do > > > kernel_init_freeable->native_smp_prepare_cpus() and workqueue_init() > > > comes next. > > > > > > And this makes sense because the splat rIP points to __queue_work() but > > > we haven't done that yet. > > > > > > So that acpi_put_table() is happening too early. Looks like AMD IOMMU > > > should not put the table but WTH do I know?! > > > > > > In any case, commenting out: > > > > > > acpi_put_table(ivrs_base); > > > ivrs_base = NULL; > > > > > > and the end of early_amd_iommu_init() makes the box boot again. > > > > So please help to comment out these 2 lines (with descriptions and do not delete them). > > Until acpi_os_unmap_memory() is able to handle such an early case. > > IMO, synchronize_rcu_expedited() should be improved: > If rcu_init() isn't called or there is nothing to synchronize, schedule_work() shouldn't be invoked. Hmm, I think this problem has its history. In APEI code, NMI handlers cannot utilize spinlock. So the developers there used RCU to synchronize NMI handlers before the register region is unmapped. At that time, there might not be pre-map/post-unmap code prepared in the APEI drivers. So the APEI developers relied on map/unmap logics implemented in the ACPICA register read/write APIs where the OSL map/unmap is invoked. That's why the RCU code is in acpi_os_xxx(). If: 1. there is map/unmap/read/write operations available for APEI developers to invoke; 2. RCU synchronization is invoked before invoking the last unmap operation; 3. map/unmap/read/write order is strictly ensured inside of the APEI drivers. Then we can remove RCU stuffs from acpi_os_xxx. And the root cause of this issue can be fixed. I'm not sure if this approach is possible, but can give it a try. Before that I think all such acpi_put_table() should just be commented out. Thanks and best regards Lv > > Thanks and best regards > Lv > > > > > Thanks and best regards > > Lv > > > > > > > > -- > > > Regards/Gruss, > > > Boris. > > > > > > Good mailing practices for 400: avoid top-posting and trim the reply.
+ Paul for comment. Leaving in the rest for him. On Mon, Jan 09, 2017 at 02:36:33AM +0000, Zheng, Lv wrote: > Hi, > > > From: linux-acpi-owner@vger.kernel.org [mailto:linux-acpi-owner@vger.kernel.org] On Behalf Of Zheng, > > Lv > > Subject: RE: 174cc7187e6f ACPICA: Tables: Back port acpi_get_table_with_size() and > > early_acpi_os_unmap_memory() from Linux kernel > > > > Hi, > > > > > From: linux-acpi-owner@vger.kernel.org [mailto:linux-acpi-owner@vger.kernel.org] On Behalf Of > > Borislav > > > Petkov > > > Subject: Re: 174cc7187e6f ACPICA: Tables: Back port acpi_get_table_with_size() and > > > early_acpi_os_unmap_memory() from Linux kernel > > > > > > On Sun, Jan 08, 2017 at 03:20:20AM +0100, Rafael J. Wysocki wrote: > > > > drivers/iommu/amd_iommu_init.c | 2 +- > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > Index: linux-pm/drivers/iommu/amd_iommu_init.c > > > > =================================================================== > > > > --- linux-pm.orig/drivers/iommu/amd_iommu_init.c > > > > +++ linux-pm/drivers/iommu/amd_iommu_init.c > > > > @@ -2230,7 +2230,7 @@ static int __init early_amd_iommu_init(v > > > > */ > > > > ret = check_ivrs_checksum(ivrs_base); > > > > if (ret) > > > > - return ret; > > > > + goto out; > > > > > > > > amd_iommu_target_ivhd_type = get_highest_supported_ivhd_type(ivrs_base); > > > > DUMP_printk("Using IVHD type %#x\n", amd_iommu_target_ivhd_type); > > > > > > Good catch, this one needs to be applied regardless. > > > > > > However, it doesn't fix my issue though. > > > > > > But I think I have it - I went and applied the well-proven debugging > > > technique of sprinkling printks around. Here's what I'm seeing: > > > > > > early_amd_iommu_init() > > > |-> acpi_put_table(ivrs_base); > > > |-> acpi_tb_put_table(table_desc); > > > |-> acpi_tb_invalidate_table(table_desc); > > > |-> acpi_tb_release_table(...) > > > |-> acpi_os_unmap_memory > > > |-> acpi_os_unmap_iomem > > > |-> acpi_os_map_cleanup > > > |-> synchronize_rcu_expedited <-- the kernel/rcu/tree_exp.h version with CONFIG_PREEMPT_RCU=y > > > > > > Now that function goes and sends IPIs, i.e., schedule_work() > > > but this is too early - we haven't even done workqueue_init(). > > > Actually, from looking at the callstack, we do > > > kernel_init_freeable->native_smp_prepare_cpus() and workqueue_init() > > > comes next. > > > > > > And this makes sense because the splat rIP points to __queue_work() but > > > we haven't done that yet. > > > > > > So that acpi_put_table() is happening too early. Looks like AMD IOMMU > > > should not put the table but WTH do I know?! > > > > > > In any case, commenting out: > > > > > > acpi_put_table(ivrs_base); > > > ivrs_base = NULL; > > > > > > and the end of early_amd_iommu_init() makes the box boot again. > > > > So please help to comment out these 2 lines (with descriptions and do not delete them). > > Until acpi_os_unmap_memory() is able to handle such an early case. > > IMO, synchronize_rcu_expedited() should be improved: > If rcu_init() isn't called or there is nothing to synchronize, schedule_work() shouldn't be invoked.
Hi Rafael, On Sun, Jan 08, 2017 at 03:20:20AM +0100, Rafael J. Wysocki wrote: > --- > drivers/iommu/amd_iommu_init.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > Index: linux-pm/drivers/iommu/amd_iommu_init.c > =================================================================== > --- linux-pm.orig/drivers/iommu/amd_iommu_init.c > +++ linux-pm/drivers/iommu/amd_iommu_init.c > @@ -2230,7 +2230,7 @@ static int __init early_amd_iommu_init(v > */ > ret = check_ivrs_checksum(ivrs_base); > if (ret) > - return ret; > + goto out; > > amd_iommu_target_ivhd_type = get_highest_supported_ivhd_type(ivrs_base); > DUMP_printk("Using IVHD type %#x\n", amd_iommu_target_ivhd_type); Yeah, good catch. Can you send me a patch for this and I am going to send the fix upstream asap. Thanks, Joerg -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
--- drivers/iommu/amd_iommu_init.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Index: linux-pm/drivers/iommu/amd_iommu_init.c =================================================================== --- linux-pm.orig/drivers/iommu/amd_iommu_init.c +++ linux-pm/drivers/iommu/amd_iommu_init.c @@ -2230,7 +2230,7 @@ static int __init early_amd_iommu_init(v */ ret = check_ivrs_checksum(ivrs_base); if (ret) - return ret; + goto out; amd_iommu_target_ivhd_type = get_highest_supported_ivhd_type(ivrs_base); DUMP_printk("Using IVHD type %#x\n", amd_iommu_target_ivhd_type);