Hello Arvind, On Thu, 12 Dec 2019 at 22:34, Arvind Sankar <nivedita@xxxxxxxxxxxx> wrote: > > This series unifies the 32-bit and 64-bit firmware functions in gop.c. > > Patches 1, 2 and 5 are bugfix/cleanup. The merge is in patches 3/4. > Which tree are these patches based on? I get a conflict applying #3 > Patch 1 removes __packed from the GOP structures, which is wrong but > didn't impact their layout. > > The UEFI spec differs from the standard gcc layout for 32-bit systems, > in that it specifies 64-bit alignment for 64-bit members. We have a bit > of a mix in the type definitions currently, with different types doing > one of the below: > (a) nothing, if a 64-bit member happens to fall naturally on a > 64-bit boundary > (b) explicit padding fields > (c) use of __aligned_u64 > The last method is the only one that gets the alignment requirement of > the structure itself correct as well as the internal layout -- is it > worth fixing everything to use it? > Absolutely. > Patch 5 is to make sure that we don't wind up with a >4G framebuffer on > a 32-bit kernel that can only address 4G. I'm not sure if this can > practically happen on anything that we can run a 32-bit kernel on, but > UEFI specs the framebuffer physical address to be 64-bit even on 32-bit > systems, so I figured we might as well cover this edge case. > This cannot happen. The 32-bit kernel can only run on 32-bit firmware, which cannot create mappings above 4 GB, so in that case, fb_base is guaranteed to be 32-bit addressable. > For the code merge, formatting is a bit annoying as the types are named > "graphics_output_protocol", which is overly long to let code fit nicely > into 80 columns. I've used an intermediate macro to address that > somewhat. Alternatively, would renaming the types to use just "gop" be > acceptable? > > Arvind Sankar (5): > efi/gop: Remove bogus packed attribute from GOP structures > efi/gop: Remove unused typedef > efi/gop: Convert GOP structures to typedef and cleanup some types > efi/gop: Unify 32/64-bit functions > efi/gop: Check that the framebuffer will be accessible > > drivers/firmware/efi/libstub/gop.c | 150 ++++++----------------------- > include/linux/efi.h | 50 +++++----- > 2 files changed, 50 insertions(+), 150 deletions(-) > > -- > 2.23.0 >