On Thu, Dec 12, 2019 at 10:08:49PM +0000, Ard Biesheuvel wrote: > 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 > Sorry, it's based on 5.5-rc1 + 3 earlier efi/gop patches in tip/efi/urgent: efi/gop: Return EFI_NOT_FOUND if there are no usable GOPs efi/gop: Return EFI_SUCCESS if a usable GOP was found efi/gop: Fix memory leak in __gop_query32/64() > > 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. > Ok I will create a separate patch for that > > > 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. > Is that still true with PAE? i.e. is it conceivable that some system uses the 36-bit address space to map PCI memory to above 4G physical addresses, or can PAE only be used with actual RAM? I can mostly drop patch 5 if this can't happen. I'd still like to get rid of the unnecessary cast of fb_base if that's fine?