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. -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
