Re: [PATCH v3 1/2] efi: Fix allocation problems if ExitBootServices() fails

[Ian Campbell <ian.campbell@citrix.com>]

On Mon, 2015-06-01 at 14:20 -0700, Roy Franz wrote:
> On Mon, Jun 1, 2015 at 4:24 AM, Ian Campbell <ian.campbell@citrix.com> wrote:
> > On Mon, 2015-06-01 at 12:10 +0100, Jan Beulich wrote:
> >> >>> On 01.06.15 at 12:17, <ross.lagerwall@citrix.com> wrote:
> >> > If calling ExitBootServices() fails, the required memory map size may
> >> > have increased. When initially allocating the memory map, allocate a
> >> > slightly larger buffer (by an arbitrary 8 entries) to fix this.
> >> >
> >> > The ARM code path was already allocating a larger buffer than required,
> >> > so this moves the code to be common for all architectures.
> >> >
> >> > This was seen on the following machine when using the iscsidxe UEFI
> >> > driver. The machine would consistently fail the first call to
> >> > ExitBootServices().
> >> > System Information
> >> >         Manufacturer: Supermicro
> >> >         Product Name: X10SLE-F/HF
> >> > BIOS Information
> >> >         Vendor: American Megatrends Inc.
> >> >         Version: 2.00
> >> >         Release Date: 04/24/2014
> >> >
> >> > Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
> >>
> >> Provided ARM folks are happy with the reduced increase,
> >
> > Hi Roy,
> >
> > This patch[0] turns a +PAGE_SIZE in efi_arch_allocate_mmap_buffer into a
> > "8 * efi_mdesc_size" in the common code.
> >
> > The +PAGE_SIZE came from [1] so I think it is as arbitrary as the
> > +8*sizeof here.
> >
> > IOW this change looks ok to me, what do you think?
> 
> Yeah, this should be fine.  Most EFI allocations have page-size
> granularity within the firmware,
> so there wasn't much point doing something smaller.  I haven't
> actually used firmware that
> changed the memmap size on ExitBootServices, so that size was not
> based on any actual
> firmware's behavior.  The x86 allocations are done differently and are
> more size constrained,
> so a smaller value should be fine for common code.
> 
> Roy
> 
> Reviewed-by: Roy Franz <roy.franz@linaro.org>

Thanks. On that basis:

Acked-by: Ian Campbell <ian.campbell@citrix.com>