x86/mce: Initialize "bank" when we find a fatal error in mce_no_way_out()

Internal injection testing crashed with a console log that said:

mce: [Hardware Error]: CPU 7: Machine Check Exception: f Bank 0: bd80000000100134

This caused a lot of head scratching because the MCACOD (bits 15:0) of that status is a signature from an L1 data cache error. But Linux says that it found it in "Bank 0", which on this model CPU only reports L1 instruction cache errors.

The answer was that Linux doesn't initialize "m->bank" in the case that it finds a fatal error in the mce_no_way_out() pre-scan of banks. If this was a local machine check, then we pass this partially initialized "struct mce" to mce_panic().

Fix is simple. Just initialize m->bank in the case that we found a fatal error.

---
 arch/x86/kernel/cpu/mce/core.c | 1 +
 1 file changed, 1 insertion(+)

index 672c7225cb1b..6ce290c506d9 100644
@@ -784,6 +784,7 @@ static int mce_no_way_out(struct mce *m, char **msg, unsigned long *validp,
 			quirk_no_way_out(i, m, regs);
 
 		if (mce_severity(m, mca_cfg.tolerant, &tmp, true) >= MCE_PANIC_SEVERITY) {
+			m->bank = i;
 			mce_read_aux(m, i);
 			*msg = tmp;
 			return 1;

2.19.1

Really? That function starts with the comment

	/*
	 * Read ADDR and MISC registers.
	 */

and the name is pretty descriptive that it is "reading auxiliary" information.

Only two exising uses:

1) in machine_check_poll()

	mce_gather_info(&m, NULL);
	for (each bank) {
		m.bank = i;
		mce_read_aux(&m, i);
		...
	}

2) in __mc_scan_banks

	// mce_gather_info() already done in do_machine_check
	for (each bank) {
		m.bank = i;
		mce_read_aux(m, i);
		...
	}

It would be redundant to move it there for both
existing uses.

Agreed that "bank" is clearer than "i". But also agreed that should be a separate patch for another day.

-Tony

Maybe. But if the bank write happens there, it won't be "forgotten" again.

I don't care what the functions are called. If they need to do something more, like *fully* populating struct mce so that a proper record gets logged further down the line, then we need to make them do so. That's the point I'm trying to make.

Sure, the stable fix should be simple for easier backporting but in order to avoid this thing happening again in the future, we should probably look at unifying and making those paths easier to use.

Right now we have

1. mce_setup() - initial population of struct mce, called from mce_gather_info(), a.o.
2. mce_gather_info() collects global regs
3. mce_read_aux() reading aux regs

and at the very end, we hand it into mce_log().

And in-between we poke (or don't poke) in some fields.

IOW, if the struct mce finishing population is concentrated in a single function, it will be much harder to forget setting such fields in the future.

In this case, the devil is in the detail so I'll have a look at this when more time.

Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.