From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ian Campbell Subject: Re: [PATCH v3 1/2] efi: Fix allocation problems if ExitBootServices() fails Date: Tue, 2 Jun 2015 09:30:57 +0100 Message-ID: <1433233857.15036.237.camel@citrix.com> References: <1433153878-32199-1-git-send-email-ross.lagerwall@citrix.com> <556C59B8020000780007FABA@mail.emea.novell.com> <1433157851.15036.122.camel@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Roy Franz Cc: Keir Fraser , Andrew Cooper , Tim Deegan , xen-devel , Ross Lagerwall , Stefano Stabellini , Jan Beulich List-Id: xen-devel@lists.xenproject.org On Mon, 2015-06-01 at 14:20 -0700, Roy Franz wrote: > On Mon, Jun 1, 2015 at 4:24 AM, Ian Campbell wrote: > > On Mon, 2015-06-01 at 12:10 +0100, Jan Beulich wrote: > >> >>> On 01.06.15 at 12:17, 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 > >> > >> 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 Thanks. On that basis: Acked-by: Ian Campbell