On Thu, 12 Dec 2019 at 23:35, Arvind Sankar <nivedita@xxxxxxxxxxxx> wrote: > > 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? Yes > 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? > PAE can be used with anything, but UEFI won't map anything in places where it cannot access it itself. (UEFI mandates a 1:1 mapping) > 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? Sure. If it's unnecessary, let's get rid of it.
