Re: [PATCH] x86/efi-bgrt: Don't ignore the BGRT if the 'valid' bit is 0

[Matt Fleming <matt-mF/unelCI9GS6iBeEJttW/XRex20P6io@public.gmane.org>]

On Tue, 05 Jan, at 05:38:54PM, Môshe van der Sterre wrote:
> On 01/05/2016 04:46 PM, Matt Fleming wrote:
> >Shouldn't this be pr_debug() instead of pr_err()?
> 
> It is an actual error for the image to be anything else. I did look at your
> commit "x86/efi-bgrt: Switch pr_err() to pr_debug() for invalid BGRT", and
> actually intentionally chose pr_err() instead of pr_debug() because of it.
> 
> From that commit message:
>     However, Josh points that out it still makes sense to test the
>     validity of the upper 7 bits of the 'status' field, since
>     they're marked as "reserved" in the spec and must be zero. If
>     firmware violates this it really *is* an error.
> 
> From the ACPI spec:
>     If the value is 0, the Image Type is Bitmap. The format for a Bitmap is
> defined at the reference located in the ACPI Link Document under the heading
> "Types of Bitmaps". All other values not defined in the table are reserved
> for future use.
> and also:
>     Implementations must present the image in a 24 bit bitmap with pixel
> format 0xRRGGBB, or a 32-bit bitmap with the pixel format 0xrrRRGGBB, where
> ‘rr’ is reserved.
> Following the link to MSDN, the header id is definitely also required to be
> "BM" for those types,
> That said, I'm not 100% sure if pr_err() is right for any of the errors in

OK, I'll queue this up. Josh, does this look OK to you?

> efi-bgrt.c. I don't really feel knowledgeable enough to suggest anything
> about that, so I tried to follow the previous discussion outcome as closely
> as possible.
> 
> By the way... Now I'm thinking about the ID, what about endianness? This
> code is under "arch/x86", does this always imply a little-endian
> architecture? The new check will probably fail on big-endian but that should
> never happen, right?

Yes, we can ignore big-endian.