RE: [PATCH v2 3/8] efi: Decode IA32/X64 Processor Error Info Structure

From: Ghannam, Yazen
Date: Tue Feb 27 2018 - 12:47:02 EST


> -----Original Message-----
> From: Borislav Petkov [mailto:bp@xxxxxxx]
> Sent: Tuesday, February 27, 2018 12:04 PM
> To: Ghannam, Yazen <Yazen.Ghannam@xxxxxxx>
> Cc: linux-efi@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx;
> ard.biesheuvel@xxxxxxxxxx; x86@xxxxxxxxxx
> Subject: Re: [PATCH v2 3/8] efi: Decode IA32/X64 Processor Error Info
> Structure
>
> On Tue, Feb 27, 2018 at 03:25:06PM +0000, Ghannam, Yazen wrote:
> > This is the same as the other CPER code.
>
> Dude, turn on brain!
>
> So if it is in the other CPER code, we should copy it, no matter how
> dumb it is?!?
>

I think there's value in following the conventions in a subsystem.

I can change this if you give a reason besides "it's dumb". This could apply
to the other CPER code also.

> > Also, the spec allows platform-defined GUIDs. So we should always print
> this
> > even if the GUID is not defined by the spec.
>
> We need to have a way to map the GUID to a hw part. Dumb numbers mean
> shit
> because the error record is worthless.
>

We do map the spec-defined GUIDs in patch 4 of this set. I don't know if there's
a central place where all vendor-defined GUIDs are listed. I can look into this.

> > The Check Information will be decoded further in another patch.
> >
> > I don't think there's much else to decode for the ID fields.
>
> Again, those numbers can't help decoding the error, no need to dump
> them. Or we find a way to make sense out of that info.
>

Which numbers? The Check Information? Like I said, that's decoded in another
patch, patches 5, 6, and 7.

And the raw value should still be printed because
1) It may represent a type that we can't decode. Maybe a type that's not part of
the spec.
2) It's good to have the raw value for reference. We do this with MCA_STATUS
where we print the raw value followed by the decoding.

> > Other tables may have the same fields but the offsets are usually different.
> > So it may be more trouble than it's worth trying to unify the different
> tables.
>
> If the structs are the same, you can use generic functions for dumping -
> the offsets are meaningless then.
>

The structs are all different even though some fields may be the same.

Thanks,
Yazen