Re: [PATCH 1/4] efi/libstub: Allocate headspace in efi_get_memory_map()

[Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>]

On Mon, Jul 18, 2016 at 09:03:31AM -0600, Jeffrey Hugo wrote:
> On 7/18/2016 5:00 AM, Mark Rutland wrote:
> >Hi,
> >
> >On Sun, Jul 17, 2016 at 02:45:59PM -0600, Jeffrey Hugo wrote:
> >>+	if (status == EFI_BUFFER_TOO_SMALL ||
> >>+				(*buff_size - *map_size) / sizeof(*m) < 8) {
> >> 		efi_call_early(free_pool, m);
> >>+		/*
> >>+		 * Make sure there is 8 entries worth of headroom so that the
> >>+		 * buffer can be reused for a new map after allocations are
> >>+		 * no longer permitted.  Its unlikely that the map will grow to
> >>+		 * exceed this headroom once we are ready to trigger
> >>+		 * ExitBootServices()
> >>+		 */
> >>+		*map_size += sizeof(*m) * 8;

> >As a general note, it's a shame that we don't know desc_size first time
> >through. If descs grow, 8 * sizeof(*m) might be too small. That's an
> >existing problem though, so I guess it makes sense to be consistent and
> >use sizeof(*m) here.
> 
> Yep.  Unless I've missed something, it seems like the two options
> are either make a decent attempt to guess at desc_size the first
> time (current approach), or intentionally fail the first time just
> to get the size.  The current approach doesn't seem perfect, but it
> does have a chance at passing with just one attempt.  I couldn't see
> a better solution, but if someone else has an idea, I'd like to hear
> it.

Sure, that was more of an observation than a critique. In the absence of
any suggestions better, sticking with sizeof(*m) makes sense.

If it does become a problem, we're likely to have to fix up other sites
regardless.

Thanks,
Mark.