From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Alex G." Subject: Re: [RFC PATCH v2 3/4] acpi: apei: Do not panic() when correctable errors are marked as fatal. Date: Thu, 19 Apr 2018 09:57:07 -0500 Message-ID: References: <20180416215903.7318-1-mr.nuke.me@gmail.com> <20180416215903.7318-4-mr.nuke.me@gmail.com> <20180418175415.GJ4795@pd.tnic> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20180418175415.GJ4795@pd.tnic> Content-Language: en-US Sender: linux-kernel-owner@vger.kernel.org To: Borislav Petkov Cc: linux-acpi@vger.kernel.org, linux-edac@vger.kernel.org, rjw@rjwysocki.net, lenb@kernel.org, tony.luck@intel.com, tbaicar@codeaurora.org, will.deacon@arm.com, james.morse@arm.com, shiju.jose@huawei.com, zjzhang@codeaurora.org, gengdongjiu@huawei.com, linux-kernel@vger.kernel.org, alex_gagniuc@dellteam.com, austin_bolen@dell.com, shyam_iyer@dell.com, devel@acpica.org, mchehab@kernel.org, robert.moore@intel.com, erik.schmauss@intel.com List-Id: linux-acpi@vger.kernel.org On 04/18/2018 12:54 PM, Borislav Petkov wrote: > On Mon, Apr 16, 2018 at 04:59:02PM -0500, Alexandru Gagniuc wrote: >> Firmware is evil: >> - ACPI was created to "try and make the 'ACPI' extensions somehow >> Windows specific" in order to "work well with NT and not the others >> even if they are open" >> - EFI was created to hide "secret" registers from the OS. >> - UEFI was created to allow compromising an otherwise secure OS. >> >> Never has firmware been created to solve a problem or simplify an >> otherwise cumbersome process. It is of no surprise then, that >> firmware nowadays intentionally crashes an OS. > > I don't believe I'm saying this but, get rid of that rant. Even though I > agree, it doesn't belong in a commit message. Of course. (snip)> Well, Tyler touched that AER error severity handling recently and we had > it all nicely documented in the comment above ghes_handle_aer(). > > Your ghes_handle_aer_irqsafe() graft basically bypasses > ghes_handle_aer() instead of incorporating in it. > > If all you wanna say is, the severity computation should go through all > the sections and look at each error's severity before making a decision, > then add that to ghes_severity() instead of doing that "deferrable" > severity dance. ghes_severity() is a one-to-one mapping from a set of unsorted severities to monotonically increasing numbers. The "one-to-one" mapping part of the sentence is obvious from the function name. To change it to parse the entire GHES would completely destroy this, and I think it would apply policy in the wrong place. Should I do that, I might have to call it something like ghes_parse_and_apply_policy_to_severity(). But that misses the whole point if these changes. I would like to get to the handlers first, and then decide if things are okay or not, but the ARM guys didn't exactly like this approach. It seems there are quite some per-error-type considerations. The logical step is to associate these considerations with the specific error type they apply to, rather than hide them as a decision under an innocent ghes_severity(). > And add the changes to the policy to the comment above > ghes_handle_aer(). I don't want any changes from people coming and going > and leaving us scratching heads why we did it this way. > > And no need for those handlers and so on - make it simple first - then we > can talk more complex handling. I don't want to leave people scratching their heads, but I also don't want to make AER a special case without having a generic way to handle these cases. People are just as susceptible to scratch their heads wondering why AER is a special case and everything else crashes. Maybe it's better move the AER handling to NMI/IRQ context, since ghes_handle_aer() is only scheduling the real AER andler, and is irq safe. I'm scratching my head about why we're messing with IRQ work from NMI context, instead of just scheduling a regular handler to take care of things. Alex From mboxrd@z Thu Jan 1 00:00:00 1970 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Subject: [RFC,v2,3/4] acpi: apei: Do not panic() when correctable errors are marked as fatal. From: Alexandru Gagniuc Message-Id: Date: Thu, 19 Apr 2018 09:57:07 -0500 To: Borislav Petkov Cc: linux-acpi@vger.kernel.org, linux-edac@vger.kernel.org, rjw@rjwysocki.net, lenb@kernel.org, tony.luck@intel.com, tbaicar@codeaurora.org, will.deacon@arm.com, james.morse@arm.com, shiju.jose@huawei.com, zjzhang@codeaurora.org, gengdongjiu@huawei.com, linux-kernel@vger.kernel.org, alex_gagniuc@dellteam.com, austin_bolen@dell.com, shyam_iyer@dell.com, devel@acpica.org, mchehab@kernel.org, robert.moore@intel.com, erik.schmauss@intel.com List-ID: T24gMDQvMTgvMjAxOCAxMjo1NCBQTSwgQm9yaXNsYXYgUGV0a292IHdyb3RlOgo+IE9uIE1vbiwg QXByIDE2LCAyMDE4IGF0IDA0OjU5OjAyUE0gLTA1MDAsIEFsZXhhbmRydSBHYWduaXVjIHdyb3Rl Ogo+PiBGaXJtd2FyZSBpcyBldmlsOgo+PiAgLSBBQ1BJIHdhcyBjcmVhdGVkIHRvICJ0cnkgYW5k IG1ha2UgdGhlICdBQ1BJJyBleHRlbnNpb25zIHNvbWVob3cKPj4gIFdpbmRvd3Mgc3BlY2lmaWMi IGluIG9yZGVyIHRvICJ3b3JrIHdlbGwgd2l0aCBOVCBhbmQgbm90IHRoZSBvdGhlcnMKPj4gIGV2 ZW4gaWYgdGhleSBhcmUgb3BlbiIKPj4gIC0gRUZJIHdhcyBjcmVhdGVkIHRvIGhpZGUgInNlY3Jl dCIgcmVnaXN0ZXJzIGZyb20gdGhlIE9TLgo+PiAgLSBVRUZJIHdhcyBjcmVhdGVkIHRvIGFsbG93 IGNvbXByb21pc2luZyBhbiBvdGhlcndpc2Ugc2VjdXJlIE9TLgo+Pgo+PiBOZXZlciBoYXMgZmly bXdhcmUgYmVlbiBjcmVhdGVkIHRvIHNvbHZlIGEgcHJvYmxlbSBvciBzaW1wbGlmeSBhbgo+PiBv dGhlcndpc2UgY3VtYmVyc29tZSBwcm9jZXNzLiBJdCBpcyBvZiBubyBzdXJwcmlzZSB0aGVuLCB0 aGF0Cj4+IGZpcm13YXJlIG5vd2FkYXlzIGludGVudGlvbmFsbHkgY3Jhc2hlcyBhbiBPUy4KPiAK PiBJIGRvbid0IGJlbGlldmUgSSdtIHNheWluZyB0aGlzIGJ1dCwgZ2V0IHJpZCBvZiB0aGF0IHJh bnQuIEV2ZW4gdGhvdWdoIEkKPiBhZ3JlZSwgaXQgZG9lc24ndCBiZWxvbmcgaW4gYSBjb21taXQg bWVzc2FnZS4KCk9mIGNvdXJzZS4KCihzbmlwKT4gV2VsbCwgVHlsZXIgdG91Y2hlZCB0aGF0IEFF UiBlcnJvciBzZXZlcml0eSBoYW5kbGluZyByZWNlbnRseQphbmQgd2UgaGFkCj4gaXQgYWxsIG5p Y2VseSBkb2N1bWVudGVkIGluIHRoZSBjb21tZW50IGFib3ZlIGdoZXNfaGFuZGxlX2FlcigpLgo+ IAo+IFlvdXIgZ2hlc19oYW5kbGVfYWVyX2lycXNhZmUoKSBncmFmdCBiYXNpY2FsbHkgYnlwYXNz ZXMKPiBnaGVzX2hhbmRsZV9hZXIoKSBpbnN0ZWFkIG9mIGluY29ycG9yYXRpbmcgaW4gaXQuCj4g Cj4gSWYgYWxsIHlvdSB3YW5uYSBzYXkgaXMsIHRoZSBzZXZlcml0eSBjb21wdXRhdGlvbiBzaG91 bGQgZ28gdGhyb3VnaCBhbGwKPiB0aGUgc2VjdGlvbnMgYW5kIGxvb2sgYXQgZWFjaCBlcnJvcidz IHNldmVyaXR5IGJlZm9yZSBtYWtpbmcgYSBkZWNpc2lvbiwKPiB0aGVuIGFkZCB0aGF0IHRvIGdo ZXNfc2V2ZXJpdHkoKSBpbnN0ZWFkIG9mIGRvaW5nIHRoYXQgImRlZmVycmFibGUiCj4gc2V2ZXJp dHkgZGFuY2UuCgpnaGVzX3NldmVyaXR5KCkgaXMgYSBvbmUtdG8tb25lIG1hcHBpbmcgZnJvbSBh IHNldCBvZiB1bnNvcnRlZApzZXZlcml0aWVzIHRvIG1vbm90b25pY2FsbHkgaW5jcmVhc2luZyBu dW1iZXJzLiBUaGUgIm9uZS10by1vbmUiIG1hcHBpbmcKcGFydCBvZiB0aGUgc2VudGVuY2UgaXMg b2J2aW91cyBmcm9tIHRoZSBmdW5jdGlvbiBuYW1lLiBUbyBjaGFuZ2UgaXQgdG8KcGFyc2UgdGhl IGVudGlyZSBHSEVTIHdvdWxkIGNvbXBsZXRlbHkgZGVzdHJveSB0aGlzLCBhbmQgSSB0aGluayBp dAp3b3VsZCBhcHBseSBwb2xpY3kgaW4gdGhlIHdyb25nIHBsYWNlLgoKU2hvdWxkIEkgZG8gdGhh dCwgSSBtaWdodCBoYXZlIHRvIGNhbGwgaXQgc29tZXRoaW5nIGxpa2UKZ2hlc19wYXJzZV9hbmRf YXBwbHlfcG9saWN5X3RvX3NldmVyaXR5KCkuIEJ1dCB0aGF0IG1pc3NlcyB0aGUgd2hvbGUKcG9p bnQgaWYgdGhlc2UgY2hhbmdlcy4KCkkgd291bGQgbGlrZSB0byBnZXQgdG8gdGhlIGhhbmRsZXJz IGZpcnN0LCBhbmQgdGhlbiBkZWNpZGUgaWYgdGhpbmdzIGFyZQpva2F5IG9yIG5vdCwgYnV0IHRo ZSBBUk0gZ3V5cyBkaWRuJ3QgZXhhY3RseSBsaWtlIHRoaXMgYXBwcm9hY2guIEl0CnNlZW1zIHRo ZXJlIGFyZSBxdWl0ZSBzb21lIHBlci1lcnJvci10eXBlIGNvbnNpZGVyYXRpb25zLgpUaGUgbG9n aWNhbCBzdGVwIGlzIHRvIGFzc29jaWF0ZSB0aGVzZSBjb25zaWRlcmF0aW9ucyB3aXRoIHRoZSBz cGVjaWZpYwplcnJvciB0eXBlIHRoZXkgYXBwbHkgdG8sIHJhdGhlciB0aGFuIGhpZGUgdGhlbSBh cyBhIGRlY2lzaW9uIHVuZGVyIGFuCmlubm9jZW50IGdoZXNfc2V2ZXJpdHkoKS4KCj4gQW5kIGFk ZCB0aGUgY2hhbmdlcyB0byB0aGUgcG9saWN5IHRvIHRoZSBjb21tZW50IGFib3ZlCj4gZ2hlc19o YW5kbGVfYWVyKCkuIEkgZG9uJ3Qgd2FudCBhbnkgY2hhbmdlcyBmcm9tIHBlb3BsZSBjb21pbmcg YW5kIGdvaW5nCj4gYW5kIGxlYXZpbmcgdXMgc2NyYXRjaGluZyBoZWFkcyB3aHkgd2UgZGlkIGl0 IHRoaXMgd2F5Lgo+Cj4gQW5kIG5vIG5lZWQgZm9yIHRob3NlIGhhbmRsZXJzIGFuZCBzbyBvbiAt IG1ha2UgaXQgc2ltcGxlIGZpcnN0IC0gdGhlbiB3ZQo+IGNhbiB0YWxrIG1vcmUgY29tcGxleCBo YW5kbGluZy4KCkkgZG9uJ3Qgd2FudCB0byBsZWF2ZSBwZW9wbGUgc2NyYXRjaGluZyB0aGVpciBo ZWFkcywgYnV0IEkgYWxzbyBkb24ndAp3YW50IHRvIG1ha2UgQUVSIGEgc3BlY2lhbCBjYXNlIHdp dGhvdXQgaGF2aW5nIGEgZ2VuZXJpYyB3YXkgdG8gaGFuZGxlCnRoZXNlIGNhc2VzLiBQZW9wbGUg YXJlIGp1c3QgYXMgc3VzY2VwdGlibGUgdG8gc2NyYXRjaCB0aGVpciBoZWFkcwp3b25kZXJpbmcg d2h5IEFFUiBpcyBhIHNwZWNpYWwgY2FzZSBhbmQgZXZlcnl0aGluZyBlbHNlIGNyYXNoZXMuCgpN YXliZSBpdCdzIGJldHRlciBtb3ZlIHRoZSBBRVIgaGFuZGxpbmcgdG8gTk1JL0lSUSBjb250ZXh0 LCBzaW5jZQpnaGVzX2hhbmRsZV9hZXIoKSBpcyBvbmx5IHNjaGVkdWxpbmcgdGhlIHJlYWwgQUVS IGFuZGxlciwgYW5kIGlzIGlycQpzYWZlLiBJJ20gc2NyYXRjaGluZyBteSBoZWFkIGFib3V0IHdo eSB3ZSdyZSBtZXNzaW5nIHdpdGggSVJRIHdvcmsgZnJvbQpOTUkgY29udGV4dCwgaW5zdGVhZCBv ZiBqdXN0IHNjaGVkdWxpbmcgYSByZWd1bGFyIGhhbmRsZXIgdG8gdGFrZSBjYXJlCm9mIHRoaW5n cy4KCkFsZXgKLS0tClRvIHVuc3Vic2NyaWJlIGZyb20gdGhpcyBsaXN0OiBzZW5kIHRoZSBsaW5l ICJ1bnN1YnNjcmliZSBsaW51eC1lZGFjIiBpbgp0aGUgYm9keSBvZiBhIG1lc3NhZ2UgdG8gbWFq b3Jkb21vQHZnZXIua2VybmVsLm9yZwpNb3JlIG1ham9yZG9tbyBpbmZvIGF0ICBodHRwOi8vdmdl ci5rZXJuZWwub3JnL21ham9yZG9tby1pbmZvLmh0bWwK