Re: [PATCH] efi-bgrt: Add error handling; inform the user when ignoring the BGRT

[Josh Triplett <josh-iaAMLnmF4UmaiuxdJuQwMA@public.gmane.org>]

On Fri, Aug 01, 2014 at 10:19:49AM +0100, Matt Fleming wrote:
> (Including akpm, the __GFP_NOWARN police)

Andrew suggested __GFP_NOWARN here in the first place.

> On Thu, 31 Jul, at 09:11:33AM, Josh Triplett wrote:
> > 
> > I started to add an explicit limit, but any reasonable limit (large
> > enough for modern screens) would be large enough that there's still a
> > non-trivial possibility of allocation failure.  And I think it makes
> > sense for BGRT image allocation to be non-fatal and minimally noisy
> > (just a single-line error, not a scary-looking allocation warning),
> > considering the highly optional and cosmetic nature of BGRT.  So, I
> > believe __GFP_NOWARN makes sense.
> 
> Yes, I agree that we don't want to trigger the page allocator warning,
> but I don't agree that passing __GFP_NOWARN is OK, which is why I'm
> advocating some size limit checks.
> 
> We need to distinguish between "Your BGRT image size is huge, and
> assumed buggy" and "Your BGRT looks valid, but we ran out of memory".
> 
> We've already got enough problems with the EFI code because we silently
> paper over bugs, and using the page allocator's failure path as a way to
> check for buggy BGRT images just doesn't make any sense to me at all.
> 
> If we get the limit wrong, it's not the end of the world, we can change
> it later, but it's a safe bet that if the firmware engineers start
> seeing "BGRT is buggy" in the kernel log they're going to start a
> dialogue with us.

The original bug report was about an allocation failure for a fairly
reasonable BGRT size.  We can certainly prohibit absurdly huge ones (for
instance, bigger than the maximum likely screen resolution times 4 bytes
per pixel), but allocation failures may well occur for smaller sizes,
and I don't think we want to spew a massive warning for that either.

- Josh Triplett