lkml.org 
[lkml]   [2018]   [Feb]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRE: [PATCH v2 2/8] efi: Decode IA32/X64 Processor Error Section
    Date
    > -----Original Message-----
    > From: Borislav Petkov [mailto:bp@suse.de]
    > Sent: Tuesday, February 27, 2018 12:45 PM
    > To: Ghannam, Yazen <Yazen.Ghannam@amd.com>
    > Cc: linux-efi@vger.kernel.org; linux-kernel@vger.kernel.org;
    > ard.biesheuvel@linaro.org; x86@kernel.org
    > 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
    \
     
     \ /
      Last update: 2018-02-27 19:07    [W:3.504 / U:0.328 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site