[PATCH 0/2] Honey, I shrunk the EFI stub

[PATCH 0/2] Honey, I shrunk the EFI stub

[Lukas Wunner]

Demonstrate the code reduction attainable by efi_call_proto()
which was proffered in a patch I've posted a few minutes ago.

For this to work, all three protocol variants (_32_t and _64_t for x86
and _t for ARM) need to be declared as typedefs.  The declaration and
naming of protocols in include/linux/efi.h currently isn't consistent,
some are declared as typedefs and some aren't, some use a "_t" suffix
and some don't.  These inconsistencies need to be straightened out
when converting to efi_call_proto().  It should be noted that checkpatch
complains about newly introduced typedefs.  It would be possible to
retool efi_call_proto() to work without typedef declarations as long
as it's done consistently.

In __file_size32() all protocol calls are currently cast to unsigned long,
which is 64 bit when compiled on x86_64.  Matt has said that the register
needs to be loaded with a 32 bit address, so it looks to me like this is
currently broken for mixed-mode.  Patch [1/2] should fix this.  E.g.:

	efi_file_handle_32_t *h, *fh = __fh;
[...]
	status = efi_early->call((unsigned long)h->get_info, h, &info_guid,
				 &info_sz, NULL);

Another oddity is that info_sz is declared u32 in __file_size32(),
yet the spec says that the third argument to EFI_FILE_PROTOCOL.GetInfo()
is of type UINTN, which I assume is 64 bit regardless of mixed-mode,
or am I missing something?  Patch [1/2] uses an unsigned long instead.

Thanks,

Lukas


Lukas Wunner (2):
  efi: Deduplicate efi_file_size() / _read() / _close()
  x86/efi: Deduplicate efi_char16_printk()

 arch/x86/boot/compressed/eboot.c               | 174 +------------------------
 drivers/firmware/efi/libstub/arm-stub.c        |  69 ----------
 drivers/firmware/efi/libstub/efi-stub-helper.c |  63 +++++++++
 drivers/firmware/efi/libstub/efistub.h         |   8 --
 include/linux/efi.h                            |   8 +-
 5 files changed, 69 insertions(+), 253 deletions(-)

-- 
2.10.1

[PATCH 1/2] efi: Deduplicate efi_file_size() / _read() / _close()

[Lukas Wunner]

There's one ARM, one x86_32 and one x86_64 version which can be folded
into a single shared version by masking their differences with the shiny
new efi_call_proto() macro.

No functional change intended.

Cc: Matt Fleming <matt-mF/unelCI9GS6iBeEJttW/XRex20P6io@public.gmane.org>
Cc: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Signed-off-by: Lukas Wunner <lukas-JFq808J9C/izQB+pC5nmwQ@public.gmane.org>
---
 arch/x86/boot/compressed/eboot.c               | 148 -------------------------
 drivers/firmware/efi/libstub/arm-stub.c        |  69 ------------
 drivers/firmware/efi/libstub/efi-stub-helper.c |  63 +++++++++++
 drivers/firmware/efi/libstub/efistub.h         |   8 --
 4 files changed, 63 insertions(+), 225 deletions(-)

diff --git a/arch/x86/boot/compressed/eboot.c b/arch/x86/boot/compressed/eboot.c
index ff01c8f..f1cf284 100644
--- a/arch/x86/boot/compressed/eboot.c
+++ b/arch/x86/boot/compressed/eboot.c
@@ -38,154 +38,6 @@ static void setup_boot_services##bits(struct efi_config *c)		\
 BOOT_SERVICES(32);
 BOOT_SERVICES(64);
 
-void efi_char16_printk(efi_system_table_t *, efi_char16_t *);
-
-static efi_status_t
-__file_size32(void *__fh, efi_char16_t *filename_16,
-	      void **handle, u64 *file_sz)
-{
-	efi_file_handle_32_t *h, *fh = __fh;
-	efi_file_info_t *info;
-	efi_status_t status;
-	efi_guid_t info_guid = EFI_FILE_INFO_ID;
-	u32 info_sz;
-
-	status = efi_early->call((unsigned long)fh->open, fh, &h, filename_16,
-				 EFI_FILE_MODE_READ, (u64)0);
-	if (status != EFI_SUCCESS) {
-		efi_printk(sys_table, "Failed to open file: ");
-		efi_char16_printk(sys_table, filename_16);
-		efi_printk(sys_table, "\n");
-		return status;
-	}
-
-	*handle = h;
-
-	info_sz = 0;
-	status = efi_early->call((unsigned long)h->get_info, h, &info_guid,
-				 &info_sz, NULL);
-	if (status != EFI_BUFFER_TOO_SMALL) {
-		efi_printk(sys_table, "Failed to get file info size\n");
-		return status;
-	}
-
-grow:
-	status = efi_call_early(allocate_pool, EFI_LOADER_DATA,
-				info_sz, (void **)&info);
-	if (status != EFI_SUCCESS) {
-		efi_printk(sys_table, "Failed to alloc mem for file info\n");
-		return status;
-	}
-
-	status = efi_early->call((unsigned long)h->get_info, h, &info_guid,
-				 &info_sz, info);
-	if (status == EFI_BUFFER_TOO_SMALL) {
-		efi_call_early(free_pool, info);
-		goto grow;
-	}
-
-	*file_sz = info->file_size;
-	efi_call_early(free_pool, info);
-
-	if (status != EFI_SUCCESS)
-		efi_printk(sys_table, "Failed to get initrd info\n");
-
-	return status;
-}
-
-static efi_status_t
-__file_size64(void *__fh, efi_char16_t *filename_16,
-	      void **handle, u64 *file_sz)
-{
-	efi_file_handle_64_t *h, *fh = __fh;
-	efi_file_info_t *info;
-	efi_status_t status;
-	efi_guid_t info_guid = EFI_FILE_INFO_ID;
-	u64 info_sz;
-
-	status = efi_early->call((unsigned long)fh->open, fh, &h, filename_16,
-				 EFI_FILE_MODE_READ, (u64)0);
-	if (status != EFI_SUCCESS) {
-		efi_printk(sys_table, "Failed to open file: ");
-		efi_char16_printk(sys_table, filename_16);
-		efi_printk(sys_table, "\n");
-		return status;
-	}
-
-	*handle = h;
-
-	info_sz = 0;
-	status = efi_early->call((unsigned long)h->get_info, h, &info_guid,
-				 &info_sz, NULL);
-	if (status != EFI_BUFFER_TOO_SMALL) {
-		efi_printk(sys_table, "Failed to get file info size\n");
-		return status;
-	}
-
-grow:
-	status = efi_call_early(allocate_pool, EFI_LOADER_DATA,
-				info_sz, (void **)&info);
-	if (status != EFI_SUCCESS) {
-		efi_printk(sys_table, "Failed to alloc mem for file info\n");
-		return status;
-	}
-
-	status = efi_early->call((unsigned long)h->get_info, h, &info_guid,
-				 &info_sz, info);
-	if (status == EFI_BUFFER_TOO_SMALL) {
-		efi_call_early(free_pool, info);
-		goto grow;
-	}
-
-	*file_sz = info->file_size;
-	efi_call_early(free_pool, info);
-
-	if (status != EFI_SUCCESS)
-		efi_printk(sys_table, "Failed to get initrd info\n");
-
-	return status;
-}
-efi_status_t
-efi_file_size(efi_system_table_t *sys_table, void *__fh,
-	      efi_char16_t *filename_16, void **handle, u64 *file_sz)
-{
-	if (efi_early->is64)
-		return __file_size64(__fh, filename_16, handle, file_sz);
-
-	return __file_size32(__fh, filename_16, handle, file_sz);
-}
-
-efi_status_t
-efi_file_read(void *handle, unsigned long *size, void *addr)
-{
-	unsigned long func;
-
-	if (efi_early->is64) {
-		efi_file_handle_64_t *fh = handle;
-
-		func = (unsigned long)fh->read;
-		return efi_early->call(func, handle, size, addr);
-	} else {
-		efi_file_handle_32_t *fh = handle;
-
-		func = (unsigned long)fh->read;
-		return efi_early->call(func, handle, size, addr);
-	}
-}
-
-efi_status_t efi_file_close(void *handle)
-{
-	if (efi_early->is64) {
-		efi_file_handle_64_t *fh = handle;
-
-		return efi_early->call((unsigned long)fh->close, handle);
-	} else {
-		efi_file_handle_32_t *fh = handle;
-
-		return efi_early->call((unsigned long)fh->close, handle);
-	}
-}
-
 static inline efi_status_t __open_volume32(void *__image, void **__fh)
 {
 	efi_file_io_interface_t *io;
diff --git a/drivers/firmware/efi/libstub/arm-stub.c b/drivers/firmware/efi/libstub/arm-stub.c
index b4f7d78..6fca48c 100644
--- a/drivers/firmware/efi/libstub/arm-stub.c
+++ b/drivers/firmware/efi/libstub/arm-stub.c
@@ -91,75 +91,6 @@ efi_status_t efi_open_volume(efi_system_table_t *sys_table_arg,
 	return status;
 }
 
-efi_status_t efi_file_close(void *handle)
-{
-	efi_file_handle_t *fh = handle;
-
-	return fh->close(handle);
-}
-
-efi_status_t
-efi_file_read(void *handle, unsigned long *size, void *addr)
-{
-	efi_file_handle_t *fh = handle;
-
-	return fh->read(handle, size, addr);
-}
-
-
-efi_status_t
-efi_file_size(efi_system_table_t *sys_table_arg, void *__fh,
-	      efi_char16_t *filename_16, void **handle, u64 *file_sz)
-{
-	efi_file_handle_t *h, *fh = __fh;
-	efi_file_info_t *info;
-	efi_status_t status;
-	efi_guid_t info_guid = EFI_FILE_INFO_ID;
-	unsigned long info_sz;
-
-	status = fh->open(fh, &h, filename_16, EFI_FILE_MODE_READ, (u64)0);
-	if (status != EFI_SUCCESS) {
-		efi_printk(sys_table_arg, "Failed to open file: ");
-		efi_char16_printk(sys_table_arg, filename_16);
-		efi_printk(sys_table_arg, "\n");
-		return status;
-	}
-
-	*handle = h;
-
-	info_sz = 0;
-	status = h->get_info(h, &info_guid, &info_sz, NULL);
-	if (status != EFI_BUFFER_TOO_SMALL) {
-		efi_printk(sys_table_arg, "Failed to get file info size\n");
-		return status;
-	}
-
-grow:
-	status = sys_table_arg->boottime->allocate_pool(EFI_LOADER_DATA,
-				 info_sz, (void **)&info);
-	if (status != EFI_SUCCESS) {
-		efi_printk(sys_table_arg, "Failed to alloc mem for file info\n");
-		return status;
-	}
-
-	status = h->get_info(h, &info_guid, &info_sz,
-						   info);
-	if (status == EFI_BUFFER_TOO_SMALL) {
-		sys_table_arg->boottime->free_pool(info);
-		goto grow;
-	}
-
-	*file_sz = info->file_size;
-	sys_table_arg->boottime->free_pool(info);
-
-	if (status != EFI_SUCCESS)
-		efi_printk(sys_table_arg, "Failed to get initrd info\n");
-
-	return status;
-}
-
-
-
 void efi_char16_printk(efi_system_table_t *sys_table_arg,
 			      efi_char16_t *str)
 {
diff --git a/drivers/firmware/efi/libstub/efi-stub-helper.c b/drivers/firmware/efi/libstub/efi-stub-helper.c
index 757badc..6ee9164 100644
--- a/drivers/firmware/efi/libstub/efi-stub-helper.c
+++ b/drivers/firmware/efi/libstub/efi-stub-helper.c
@@ -338,6 +338,69 @@ void efi_free(efi_system_table_t *sys_table_arg, unsigned long size,
 	efi_call_early(free_pages, addr, nr_pages);
 }
 
+static efi_status_t efi_file_size(efi_system_table_t *sys_table_arg, void *__fh,
+				  efi_char16_t *filename_16, void **handle,
+				  u64 *file_sz)
+{
+	efi_file_handle_t *h, *fh = __fh;
+	efi_file_info_t *info;
+	efi_status_t status;
+	efi_guid_t info_guid = EFI_FILE_INFO_ID;
+	unsigned long info_sz;
+
+	status = efi_call_proto(efi_file_handle, open, fh, &h, filename_16,
+				EFI_FILE_MODE_READ, (u64)0);
+	if (status != EFI_SUCCESS) {
+		efi_printk(sys_table_arg, "Failed to open file: ");
+		efi_char16_printk(sys_table_arg, filename_16);
+		efi_printk(sys_table_arg, "\n");
+		return status;
+	}
+
+	*handle = h;
+
+	info_sz = 0;
+	status = efi_call_proto(efi_file_handle, get_info, h, &info_guid,
+				&info_sz, NULL);
+	if (status != EFI_BUFFER_TOO_SMALL) {
+		efi_printk(sys_table_arg, "Failed to get file info size\n");
+		return status;
+	}
+
+grow:
+	status = efi_call_early(allocate_pool, EFI_LOADER_DATA,
+				info_sz, (void **)&info);
+	if (status != EFI_SUCCESS) {
+		efi_printk(sys_table_arg, "Failed to alloc mem for file info\n");
+		return status;
+	}
+
+	status = efi_call_proto(efi_file_handle, get_info, h, &info_guid,
+				&info_sz, info);
+	if (status == EFI_BUFFER_TOO_SMALL) {
+		efi_call_early(free_pool, info);
+		goto grow;
+	}
+
+	*file_sz = info->file_size;
+	efi_call_early(free_pool, info);
+
+	if (status != EFI_SUCCESS)
+		efi_printk(sys_table_arg, "Failed to get initrd info\n");
+
+	return status;
+}
+
+static efi_status_t efi_file_read(void *handle, unsigned long *size, void *addr)
+{
+	return efi_call_proto(efi_file_handle, read, handle, size, addr);
+}
+
+static efi_status_t efi_file_close(void *handle)
+{
+	return efi_call_proto(efi_file_handle, close, handle);
+}
+
 /*
  * Parse the ASCII string 'cmdline' for EFI options, denoted by the efi=
  * option, e.g. efi=nochunk.
diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h
index b98824e..2fe7746 100644
--- a/drivers/firmware/efi/libstub/efistub.h
+++ b/drivers/firmware/efi/libstub/efistub.h
@@ -29,14 +29,6 @@ void efi_char16_printk(efi_system_table_t *, efi_char16_t *);
 efi_status_t efi_open_volume(efi_system_table_t *sys_table_arg, void *__image,
 			     void **__fh);
 
-efi_status_t efi_file_size(efi_system_table_t *sys_table_arg, void *__fh,
-			   efi_char16_t *filename_16, void **handle,
-			   u64 *file_sz);
-
-efi_status_t efi_file_read(void *handle, unsigned long *size, void *addr);
-
-efi_status_t efi_file_close(void *handle);
-
 unsigned long get_dram_base(efi_system_table_t *sys_table_arg);
 
 efi_status_t update_fdt(efi_system_table_t *sys_table, void *orig_fdt,
-- 
2.10.1

[PATCH 2/2] x86/efi: Deduplicate efi_char16_printk()

[Lukas Wunner]

Eliminate the separate 32 bit and 64 bit code paths by way of the shiny
new efi_call_proto() macro.

No functional change intended.

Cc: Matt Fleming <matt-mF/unelCI9GS6iBeEJttW/XRex20P6io@public.gmane.org>
Cc: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Signed-off-by: Lukas Wunner <lukas-JFq808J9C/izQB+pC5nmwQ@public.gmane.org>
---
 arch/x86/boot/compressed/eboot.c | 26 ++------------------------
 include/linux/efi.h              |  8 ++++----
 2 files changed, 6 insertions(+), 28 deletions(-)

diff --git a/arch/x86/boot/compressed/eboot.c b/arch/x86/boot/compressed/eboot.c
index f1cf284..6d3aeab 100644
--- a/arch/x86/boot/compressed/eboot.c
+++ b/arch/x86/boot/compressed/eboot.c
@@ -101,30 +101,8 @@ efi_open_volume(efi_system_table_t *sys_table, void *__image, void **__fh)
 
 void efi_char16_printk(efi_system_table_t *table, efi_char16_t *str)
 {
-	unsigned long output_string;
-	size_t offset;
-
-	if (efi_early->is64) {
-		struct efi_simple_text_output_protocol_64 *out;
-		u64 *func;
-
-		offset = offsetof(typeof(*out), output_string);
-		output_string = efi_early->text_output + offset;
-		out = (typeof(out))(unsigned long)efi_early->text_output;
-		func = (u64 *)output_string;
-
-		efi_early->call(*func, out, str);
-	} else {
-		struct efi_simple_text_output_protocol_32 *out;
-		u32 *func;
-
-		offset = offsetof(typeof(*out), output_string);
-		output_string = efi_early->text_output + offset;
-		out = (typeof(out))(unsigned long)efi_early->text_output;
-		func = (u32 *)output_string;
-
-		efi_early->call(*func, out, str);
-	}
+	efi_call_proto(efi_simple_text_output_protocol, output_string,
+		       efi_early->text_output, str);
 }
 
 static efi_status_t
diff --git a/include/linux/efi.h b/include/linux/efi.h
index a07a476..4995869 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -1238,17 +1238,17 @@ struct efivar_entry {
 	bool deleting;
 };
 
-struct efi_simple_text_output_protocol_32 {
+typedef struct {
 	u32 reset;
 	u32 output_string;
 	u32 test_string;
-};
+} efi_simple_text_output_protocol_32_t;
 
-struct efi_simple_text_output_protocol_64 {
+typedef struct {
 	u64 reset;
 	u64 output_string;
 	u64 test_string;
-};
+} efi_simple_text_output_protocol_64_t;
 
 struct efi_simple_text_output_protocol {
 	void *reset;
-- 
2.10.1

Re: [PATCH 0/2] Honey, I shrunk the EFI stub

[Matt Fleming]

On Mon, 07 Nov, at 12:17:00PM, Lukas Wunner wrote:
> Demonstrate the code reduction attainable by efi_call_proto()
> which was proffered in a patch I've posted a few minutes ago.
> 
> For this to work, all three protocol variants (_32_t and _64_t for x86
> and _t for ARM) need to be declared as typedefs.  The declaration and
> naming of protocols in include/linux/efi.h currently isn't consistent,
> some are declared as typedefs and some aren't, some use a "_t" suffix
> and some don't.  These inconsistencies need to be straightened out
> when converting to efi_call_proto().  It should be noted that checkpatch
> complains about newly introduced typedefs.  It would be possible to
> retool efi_call_proto() to work without typedef declarations as long
> as it's done consistently.
 
This is probably v4.11 material. We *may* be able to get this into
v4.10 if I review and merge this soon, but it definitely isn't going
to be included in the imminent pull request.

I do like the general idea though.

> In __file_size32() all protocol calls are currently cast to unsigned long,
> which is 64 bit when compiled on x86_64.  Matt has said that the register
> needs to be loaded with a 32 bit address, so it looks to me like this is
> currently broken for mixed-mode.  Patch [1/2] should fix this.  E.g.:
> 
> 	efi_file_handle_32_t *h, *fh = __fh;
> [...]
> 	status = efi_early->call((unsigned long)h->get_info, h, &info_guid,
> 				 &info_sz, NULL);

There's a subtle distinction here between 32-bit address and 32-bit
value. A 64-bit value can be a valid 32-bit address, provided that the
upper 32-bits are zero, e.g. 0x00000000ffffffff.

So when I say "32-bit address" I really just mean some value where
only the lower 32-bits are important.

That is why using unsigned long in mixed-mode is OK for the early call
code.
 
> Another oddity is that info_sz is declared u32 in __file_size32(),
> yet the spec says that the third argument to EFI_FILE_PROTOCOL.GetInfo()
> is of type UINTN, which I assume is 64 bit regardless of mixed-mode,
> or am I missing something?  Patch [1/2] uses an unsigned long instead.

UINTN is an unsigned value of native width as seen by the firmware. On
32-bit firmware that's 32-bits and 64-bit firmware 64-bits.

Using 'u32' in __file_size32() is correct, unsigned long is not.

Re: [PATCH 0/2] Honey, I shrunk the EFI stub

[Lukas Wunner]

On Sat, Nov 12, 2016 at 08:55:14PM +0000, Matt Fleming wrote:
> On Mon, 07 Nov, at 12:17:00PM, Lukas Wunner wrote:
> > Demonstrate the code reduction attainable by efi_call_proto()
> > which was proffered in a patch I've posted a few minutes ago.
> > 
> > For this to work, all three protocol variants (_32_t and _64_t for x86
> > and _t for ARM) need to be declared as typedefs.  The declaration and
> > naming of protocols in include/linux/efi.h currently isn't consistent,
> > some are declared as typedefs and some aren't, some use a "_t" suffix
> > and some don't.  These inconsistencies need to be straightened out
> > when converting to efi_call_proto().  It should be noted that checkpatch
> > complains about newly introduced typedefs.  It would be possible to
> > retool efi_call_proto() to work without typedef declarations as long
> > as it's done consistently.
>  
> This is probably v4.11 material. We *may* be able to get this into
> v4.10 if I review and merge this soon, but it definitely isn't going
> to be included in the imminent pull request.
> 
> I do like the general idea though.

Yes, this was posted quite late in the cycle so I didn't expect it
to make it into 4.10 really.  It was meant as a demonstration,
I can respin this into a series that deduplicates these redundancies
more thoroughly, but I wanted to gauge the reaction of the community
first.  Ard should probably also weigh in since it touches ARM code.

By the way, you mentioned that you have a MacBook2,1, is this the
Late 2006 version and would you be able to test changes to the
efistub on that machine?  I was thinking about obtaining such a
machine myself on ebay since right now I can only test x86_64,
not mixed mode.  If noone else is able to perform tests I might
just do that.  (Only the Late 2006 version uses mixed mode, the
Mid 2007 has a native 64-bit EFI.)


> > In __file_size32() all protocol calls are currently cast to unsigned long,
> > which is 64 bit when compiled on x86_64.  Matt has said that the register
> > needs to be loaded with a 32 bit address, so it looks to me like this is
> > currently broken for mixed-mode.  Patch [1/2] should fix this.  E.g.:
> > 
> > 	efi_file_handle_32_t *h, *fh = __fh;
> > [...]
> > 	status = efi_early->call((unsigned long)h->get_info, h, &info_guid,
> > 				 &info_sz, NULL);
> 
> There's a subtle distinction here between 32-bit address and 32-bit
> value. A 64-bit value can be a valid 32-bit address, provided that the
> upper 32-bits are zero, e.g. 0x00000000ffffffff.
> 
> So when I say "32-bit address" I really just mean some value where
> only the lower 32-bits are important.
> 
> That is why using unsigned long in mixed-mode is OK for the early call
> code.
>  
> > Another oddity is that info_sz is declared u32 in __file_size32(),
> > yet the spec says that the third argument to EFI_FILE_PROTOCOL.GetInfo()
> > is of type UINTN, which I assume is 64 bit regardless of mixed-mode,
> > or am I missing something?  Patch [1/2] uses an unsigned long instead.
> 
> UINTN is an unsigned value of native width as seen by the firmware. On
> 32-bit firmware that's 32-bits and 64-bit firmware 64-bits.
> 
> Using 'u32' in __file_size32() is correct, unsigned long is not.

Okay since this is all little endian, it should be okay to have a
64 bit wide variable on the stack whose address is passed to GetInfo()
as BufferSize argument.  But I guess I need to initialize it to 0
upon declaration so that the upper 32 bit are zeroed out in mixed mode,
right?  That would be a bug in patch [1/2] then.

Thanks,

Lukas

Re: [PATCH 0/2] Honey, I shrunk the EFI stub

[Lukas Wunner]

On Mon, Nov 14, 2016 at 12:19:06PM +0100, Lukas Wunner wrote:
> On Sat, Nov 12, 2016 at 08:55:14PM +0000, Matt Fleming wrote:
> > On Mon, 07 Nov, at 12:17:00PM, Lukas Wunner wrote:
> > > Another oddity is that info_sz is declared u32 in __file_size32(),
> > > yet the spec says that the third argument to EFI_FILE_PROTOCOL.GetInfo()
> > > is of type UINTN, which I assume is 64 bit regardless of mixed-mode,
> > > or am I missing something?  Patch [1/2] uses an unsigned long instead.
> > 
> > UINTN is an unsigned value of native width as seen by the firmware. On
> > 32-bit firmware that's 32-bits and 64-bit firmware 64-bits.
> > 
> > Using 'u32' in __file_size32() is correct, unsigned long is not.
> 
> Okay since this is all little endian, it should be okay to have a
> 64 bit wide variable on the stack whose address is passed to GetInfo()
> as BufferSize argument.  But I guess I need to initialize it to 0
> upon declaration so that the upper 32 bit are zeroed out in mixed mode,
> right?  That would be a bug in patch [1/2] then.

Oh the info_sz variable *is* already initialized to 0 before the first
invocation of GetInfo().  So the patch should be fine, reviewed it
once more with the explanations in mind that you provided and couldn't
see anything else that would cause issues in mixed-mode.

Thanks,

Lukas

Re: [PATCH 0/2] Honey, I shrunk the EFI stub

[Matt Fleming]

On Mon, 14 Nov, at 04:32:31PM, Lukas Wunner wrote:
> 
> Oh the info_sz variable *is* already initialized to 0 before the first
> invocation of GetInfo().  So the patch should be fine, reviewed it
> once more with the explanations in mind that you provided and couldn't
> see anything else that would cause issues in mixed-mode.

Thanks for double-checking that. These patches look good to me and
pass all my testing. Queued for v4.11, thanks Lukas!