> -----Original Message----- > From: Borislav Petkov [mailto:bp@xxxxxxx] > Sent: Tuesday, February 27, 2018 12:45 PM > To: Ghannam, Yazen <Yazen.Ghannam@xxxxxxx> > Cc: linux-efi@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; > ard.biesheuvel@xxxxxxxxxx; x86@xxxxxxxxxx > Subject: Re: [PATCH v2 2/8] efi: Decode IA32/X64 Processor Error Section > > On Tue, Feb 27, 2018 at 05:27:39PM +0000, Ghannam, Yazen wrote: > > Sure, we can print the fields unconditionally if Ard is okay with that. > > > > The issue is that the x86 behavior will be different from all the others, so > that > > might be confusing. > > Confusing for whom? > > Are we sharing tools with other arches or what am I missing? > It's not just about arches but record types. A single platform can report using arch-specific records, memory records, PCIe records, etc. So all the other record types only show fields with a valid bit set and this has always been the case. There may be users or tools who expect that same behavior. > > This set does decode everything fully so that people can read the error. > > No, it doesn't. It dumps > > printk("%sValidation Bits: 0x%016llx\n", pfx, proc->validation_bits); > > printk("%sError Structure Type: %pUl\n", newpfx, > &err_info->err_type); > > printk("%sCheck Information: 0x%016llx\n", newpfx, > err_info->check_info); > > and so on which are half-baked. > > Think of it this way: if you look at the error record and you still need > to look at the spec to decode it, then it is still not good enough. > > Users don't care about validation bits, or unreadable GUIDs or WTF is > "Check Information" even? > > "This section details the layout of the Processor Error Information > Structure and the detailed check information which is contained within." > > And that "Check Information" thing is mentioned only twice in the whole > spec. > > StructureErrorType only there in the table. > > Very informative that. > > So no, users don't care about any of that internal crap - they wanna > know what is wrong with their box and that should be written in plain > english. > Please see the other patches where these are decoded further. As I mentioned the cover letter, the decoding is applied incrementally rather than having one large patch. Also, like I said in another thread, we should always print the raw value followed by the decoding. That way the raw info is there in case a user wants to send the data to the vendor or other expert party. > > I already mentioned in the Context info patch that there could be > > further decoding which we can do in the future. > > Then decode only those pieces fully now which you can do now and when > you add something in the future, add it then with the full decoding > functionality. No fields which need additional decoding with the spec > opened on one side. > Okay. Thanks, Yazen ��.n��������+%������w��{.n�����{����* jg��� �����ݢj����G���� ���j:+v���w�m������w�������h�����٥
