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: x86/MCE, EDAC/mce_amd: Save all aux registers on SMCA systems From: Borislav Petkov Message-Id: <20180417172102.GA3633@pd.tnic> Date: Tue, 17 Apr 2018 19:21:02 +0200 To: Yazen Ghannam Cc: linux-edac@vger.kernel.org, linux-kernel@vger.kernel.org, tony.luck@intel.com, x86@kernel.org List-ID: T24gTW9uLCBBcHIgMDIsIDIwMTggYXQgMDI6NTc6MDdQTSAtMDUwMCwgWWF6ZW4gR2hhbm5hbSB3 cm90ZToKPiBGcm9tOiBZYXplbiBHaGFubmFtIDx5YXplbi5naGFubmFtQGFtZC5jb20+Cj4gCj4g VGhlIEludGVsIFNETSBhbmQgQU1EIEFQTSBib3RoIHN0YXRlIHRoYXQgdGhlIGF1eGlsaWFyeSBN Q0EgcmVnaXN0ZXJzCj4gc2hvdWxkIGJlIHJlYWQgaWYgdGhlaXIgcmVzcGVjdGl2ZSB2YWxpZCBi aXRzIGFyZSBzZXQgaW4gTUNBX1NUQVRVUy4KPiAKPiBUaGUgUHJvY2Vzc29yIFByb2dyYW1taW5n IFJlZmVyZW5jZSBmb3IgQU1EIEZhbTE3aCBzeXN0ZW1zIGhhcyBhIG5ldwo+IHJlY29tbWVuZGF0 aW9uIHRoYXQgdGhlIGF1eGlsaWFyeSByZWdpc3RlcnMgc2hvdWxkIGJlIHNhdmVkCj4gdW5jb25k aXRpb25hbGx5LiBUaGlzIHJlY29tbWVuZGF0aW9uIGNhbiBiZSByZXRyb2FjdGl2ZWx5IGFwcGxp ZWQgdG8KPiBvbGRlciBBTUQgc3lzdGVtcy4gSG93ZXZlciwgd2Ugb25seSBuZWVkIHRvIGFwcGx5 IHRoaXMgdG8gU01DQSBzeXN0ZW1zCj4gdG8gYXZvaWQgbW9kaWZ5aW5nIGJlaGF2aW9yIG9uIG9s ZGVyIHN5c3RlbXMuCgpBcHBseWluZyB0aGUgbG9naWMgb2YgdGhhdCByZWNvbW1lbmRhdGlvbiBv biBvbGRlciBzeXN0ZW1zOiB3b3VsZG4ndCBpdApiZSBwcnVkZW50IHRvIHNhdmUgdGhlbSB0aGVy ZSB0b28sIGlmIGl0IGhlbHBzIGRlYnVnZ2luZyBhbiBNQ0U/Cgo+IERlZmluZSBhIHNlcGFyYXRl IGZ1bmN0aW9uIHRvIHNhdmUgYWxsIGF1eGlsaWFyeSByZWdpc3RlcnMgb24gU01DQQo+IHN5c3Rl bXMuIENhbGwgdGhpcyBmdW5jdGlvbiBmcm9tIGJvdGggdGhlIE1DRSBoYW5kbGVycyBhbmQgdGhl IEFNRCBMVlQKPiBpbnRlcnJ1cHQgaGFuZGxlcnMgc28gdGhhdCB3ZSBkb24ndCBkdXBsaWNhdGUg Y29kZS4KPiAKPiBQcmludCBhbGwgYXV4aWxpYXJ5IHJlZ2lzdGVycyBpbiBFREFDL21jZV9hbWQu IERvbid0IHJlc3RyaWN0IHRoaXMgdG8KPiBTTUNBIHN5c3RlbXMgaW4gb3JkZXIgdG8gc2F2ZSBh IGNvbmRpdGlvbmFsIGFuZCBrZWVwIHRoZSBmb3JtYXQgc2ltaWxhcgo+IGJldHdlZW4gU01DQSBh bmQgbm9uLVNNQ0Egc3lzdGVtcy4KPiAKPiBTaWduZWQtb2ZmLWJ5OiBZYXplbiBHaGFubmFtIDx5 YXplbi5naGFubmFtQGFtZC5jb20+CgouLi4KCj4gZGlmZiAtLWdpdCBhL2FyY2gveDg2L2tlcm5l bC9jcHUvbWNoZWNrL21jZV9hbWQuYyBiL2FyY2gveDg2L2tlcm5lbC9jcHUvbWNoZWNrL21jZV9h bWQuYwo+IGluZGV4IGY3NjY2ZWVmNGE4Ny4uYjAwZDVmZmYxODQ4IDEwMDY0NAo+IC0tLSBhL2Fy Y2gveDg2L2tlcm5lbC9jcHUvbWNoZWNrL21jZV9hbWQuYwo+ICsrKyBiL2FyY2gveDg2L2tlcm5l bC9jcHUvbWNoZWNrL21jZV9hbWQuYwo+IEBAIC0yNDQsNiArMjQ0LDQ3IEBAIHN0YXRpYyB2b2lk IHNtY2FfY29uZmlndXJlKHVuc2lnbmVkIGludCBiYW5rLCB1bnNpZ25lZCBpbnQgY3B1KQo+ICAJ fQo+ICB9Cj4gIAo+ICsKPiArc3RhdGljIGJvb2wgX3NtY2FfcmVhZF9hdXgoc3RydWN0IG1jZSAq bSwgaW50IGJhbmssIGJvb2wgcmVhZF9hZGRyKQo+ICt7Cj4gKwlpZiAoIW1jZV9mbGFncy5zbWNh KQo+ICsJCXJldHVybiBmYWxzZTsKPiArCj4gKwlyZG1zcmwoTVNSX0FNRDY0X1NNQ0FfTUN4X0lQ SUQoYmFuayksIG0tPmlwaWQpOwo+ICsJcmRtc3JsKE1TUl9BTUQ2NF9TTUNBX01DeF9TWU5EKGJh bmspLCBtLT5zeW5kKTsKPiArCj4gKwkvKgo+ICsJICogV2Ugc2hvdWxkIGFscmVhZHkgaGF2ZSBh IHZhbHVlIGlmIHdlJ3JlIGNvbWluZyBmcm9tIHRoZSBUaHJlc2hvbGQgTFZUCj4gKwkgKiBpbnRl cnJ1cHQgaGFuZGxlci4gT3RoZXJ3aXNlLCByZWFkIGl0IG5vdy4KPiArCSAqLwo+ICsJaWYgKCFt LT5taXNjKQo+ICsJCXJkbXNybChtc3Jfb3BzLm1pc2MoYmFuayksIG0tPm1pc2MpOwo+ICsKPiAr CS8qCj4gKwkgKiBSZWFkIE1DQV9BRERSIGlmIHdlIGRvbid0IGhhdmUgaXQgYWxyZWFkeS4gV2Ug c2hvdWxkIGFscmVhZHkgaGF2ZSBpdAo+ICsJICogaWYgd2UncmUgY29taW5nIGZyb20gdGhlIGlu dGVycnVwdCBoYW5kbGVycy4KPiArCSAqLwo+ICsJaWYgKHJlYWRfYWRkcikKCldoeSBub3QKCglp ZiAoIW0tPmFkZHIpCgo/CgpBbmQgeWVhaCwgaWYgaXQgaGFzIGJlZW4gcmVhZCB0byAwIGFscmVh ZHksIHJlYWRpbmcgaXQgYWdhaW4gd29uJ3QKY2hhbmdlIGFueXRoaW5nLgoKQW5kIHRoaW5raW5n IGFib3V0IGl0IG1vcmUsIHlvdSBkb24ndCByZWFsbHkgbmVlZCB0aG9zZSBpZi10ZXN0cywgSSdk CnNheS4gU28gd2hhdCwgeW91J2xsIHJlYWQgb25lIG9yIHR3byBNU1JzIG9uY2UgbW9yZS4gSXQg aXMgbm90IHN1Y2ggYQpob3QgcGF0aCB0aGF0IHdlIGNhbid0IHN0b21hY2ggdGhlIHBlcmYgcGVu YWx0eSBvZiByZWFkaW5nIHRoZSBNU1JzLgoKPiArCQlyZG1zcmwobXNyX29wcy5hZGRyKGJhbmsp LCBtLT5hZGRyKTsKPiArCj4gKwkvKgo+ICsJICogRXh0cmFjdCBbNTU6PGxzYj5dIHdoZXJlIGxz YiBpcyB0aGUgbGVhc3Qgc2lnbmlmaWNhbnQKPiArCSAqICp2YWxpZCogYml0IG9mIHRoZSBhZGRy ZXNzIGJpdHMuCj4gKwkgKi8KPiArCWlmIChtLT5hZGRyKSB7CgpBbmQgdGhhdCB0ZXN0IGlzIHBy b2JhYmx5IG5vdCBuZWVkZWQgZWl0aGVyOiBpZiBtLT5hZGRyIGlzIDAsIHRoZQpiZWxvdyB3b3Vs ZCBiZSAwIGFueXdheS4KCj4gKwkJdTggbHNiID0gKG0tPmFkZHIgPj4gNTYpICYgMHgzZjsKPiAr Cj4gKwkJbS0+YWRkciAmPSBHRU5NQVNLX1VMTCg1NSwgbHNiKTsKPiArCX0KPiArCj4gKwlyZXR1 cm4gdHJ1ZTsKPiArfQoKSU9XLCB0aG9zZSB0ZXN0cyBhcmUgcHJvYmFibHkgb2sgYnV0IGdldHRp bmcgcmlkIG9mIHRoZW0gd291bGQgbWFrZSB0aGUKY29kZSBtb3JlIHJlYWRhYmxlIGFuZCBJIHRo aW5rIHdlIGNhbiBhZmZvcmQgdGhhdCBoZXJlLgoKVGh4Lgo= From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752651AbeDQRVY (ORCPT ); Tue, 17 Apr 2018 13:21:24 -0400 Received: from mail.skyhub.de ([5.9.137.197]:32840 "EHLO mail.skyhub.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752503AbeDQRVW (ORCPT ); Tue, 17 Apr 2018 13:21:22 -0400 Date: Tue, 17 Apr 2018 19:21:02 +0200 From: Borislav Petkov To: Yazen Ghannam Cc: linux-edac@vger.kernel.org, linux-kernel@vger.kernel.org, tony.luck@intel.com, x86@kernel.org Subject: Re: [PATCH] x86/MCE, EDAC/mce_amd: Save all aux registers on SMCA systems Message-ID: <20180417172102.GA3633@pd.tnic> References: <20180402195707.42875-1-Yazen.Ghannam@amd.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20180402195707.42875-1-Yazen.Ghannam@amd.com> User-Agent: Mutt/1.9.3 (2018-01-21) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Apr 02, 2018 at 02:57:07PM -0500, Yazen Ghannam wrote: > From: Yazen Ghannam > > The Intel SDM and AMD APM both state that the auxiliary MCA registers > should be read if their respective valid bits are set in MCA_STATUS. > > The Processor Programming Reference for AMD Fam17h systems has a new > recommendation that the auxiliary registers should be saved > unconditionally. This recommendation can be retroactively applied to > older AMD systems. However, we only need to apply this to SMCA systems > to avoid modifying behavior on older systems. Applying the logic of that recommendation on older systems: wouldn't it be prudent to save them there too, if it helps debugging an MCE? > Define a separate function to save all auxiliary registers on SMCA > systems. Call this function from both the MCE handlers and the AMD LVT > interrupt handlers so that we don't duplicate code. > > Print all auxiliary registers in EDAC/mce_amd. Don't restrict this to > SMCA systems in order to save a conditional and keep the format similar > between SMCA and non-SMCA systems. > > Signed-off-by: Yazen Ghannam ... > diff --git a/arch/x86/kernel/cpu/mcheck/mce_amd.c b/arch/x86/kernel/cpu/mcheck/mce_amd.c > index f7666eef4a87..b00d5fff1848 100644 > --- a/arch/x86/kernel/cpu/mcheck/mce_amd.c > +++ b/arch/x86/kernel/cpu/mcheck/mce_amd.c > @@ -244,6 +244,47 @@ static void smca_configure(unsigned int bank, unsigned int cpu) > } > } > > + > +static bool _smca_read_aux(struct mce *m, int bank, bool read_addr) > +{ > + if (!mce_flags.smca) > + return false; > + > + rdmsrl(MSR_AMD64_SMCA_MCx_IPID(bank), m->ipid); > + rdmsrl(MSR_AMD64_SMCA_MCx_SYND(bank), m->synd); > + > + /* > + * We should already have a value if we're coming from the Threshold LVT > + * interrupt handler. Otherwise, read it now. > + */ > + if (!m->misc) > + rdmsrl(msr_ops.misc(bank), m->misc); > + > + /* > + * Read MCA_ADDR if we don't have it already. We should already have it > + * if we're coming from the interrupt handlers. > + */ > + if (read_addr) Why not if (!m->addr) ? And yeah, if it has been read to 0 already, reading it again won't change anything. And thinking about it more, you don't really need those if-tests, I'd say. So what, you'll read one or two MSRs once more. It is not such a hot path that we can't stomach the perf penalty of reading the MSRs. > + rdmsrl(msr_ops.addr(bank), m->addr); > + > + /* > + * Extract [55:] where lsb is the least significant > + * *valid* bit of the address bits. > + */ > + if (m->addr) { And that test is probably not needed either: if m->addr is 0, the below would be 0 anyway. > + u8 lsb = (m->addr >> 56) & 0x3f; > + > + m->addr &= GENMASK_ULL(55, lsb); > + } > + > + return true; > +} IOW, those tests are probably ok but getting rid of them would make the code more readable and I think we can afford that here. Thx. -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply.