Re: [PATCH] x86/efi-bgrt: Switch pr_err() to pr_debug() for invalid BGRT

[josh-iaAMLnmF4UmaiuxdJuQwMA@public.gmane.org]

On Mon, Jun 29, 2015 at 03:49:40PM +0100, Matt Fleming wrote:
> On Mon, 29 Jun, at 04:17:24PM, Borislav Petkov wrote:
> > On Mon, Jun 29, 2015 at 07:00:22AM -0700, Josh Triplett wrote:
> > > Definitely not FW_BUG.  The field is reserved *now*; it would be
> > > legitimate for a new version of the BGRT spec to define one of those
> > > bits for something else.
> > 
> > Which would mean that booting old kernels on new FW which defines those
> > reserved bits would cause that warning to fire erroneously.
> > 
> > So then we probably don't need it at all or we need to check implemented
> > BGRT version of the FW running to know which bits are defined by the
> > spec and which are reserved...
>  
> It still makes sense to have the error message because the kernel
> literally does not know what the firmware is trying to achieve by
> setting those bits.
> 
> But I agree with Josh that for the specific case of "reserved bits",
> FW_BUG is wrong, because if in some future version of the spec those
> bits get used, seeing,
> 
>   "[Firmware Bug]: Ignoring BGRT: reserved bits are non-zero 0x3"
> 
> is going to confuse the hell outta any firmware engineers, since it's
> not the firmware that's buggy, it's just that the kernel lacks support.

Right.  Same reason we shouldn't use FW_BUG for bgrt_tab->version != 1
or bgrt_tab->image_type != 0 .

> This discussion should definitely be summarised in printk.h.
> 
> However, other error messages in efi-bgrt.c probably do want to be
> sprinkled with FW_* if they represent things that should never happen
> ever.

I think the length check and null pointer check would fall in that
category.

- Josh Triplett