Hi, this is v2 of the patch after review from Leif. Leif it would be good if you could carry this in your branch to save us having to maintain another grub tree.
Changes since v1: Address Leifs comments:- return type for grub_cmd_armcpi does not return with bogus error leaving file open, not jumps to out: properly bogus whitespace change removed.
Thanks
Graeme
Added location of ACPI blob loading to linux.h --- grub-core/loader/arm/linux.c | 82 +++++++++++++++++++++++++++++++++++++++++- include/grub/arm/linux.h | 3 ++ 2 files changed, 84 insertions(+), 1 deletion(-)
diff --git a/grub-core/loader/arm/linux.c b/grub-core/loader/arm/linux.c index f5faa7b..dee49d9 100644 --- a/grub-core/loader/arm/linux.c +++ b/grub-core/loader/arm/linux.c @@ -45,6 +45,8 @@ static grub_addr_t firmware_boot_data; static grub_addr_t boot_data; static grub_uint32_t machine_type;
+static grub_addr_t acpi_data; + /* * linux_prepare_fdt(): * Prepares a loaded FDT for being passed to Linux. @@ -202,6 +204,14 @@ linux_boot (void) linuxmain = (kernel_entry_t) linux_addr;
#ifdef GRUB_MACHINE_EFI + if (acpi_data) { + struct grub_efi_guid acpi = GRUB_EFI_ACPI_TABLE_GUID; + struct grub_efi_guid acpi20 = GRUB_EFI_ACPI_20_TABLE_GUID; + + grub_efi_system_table->boot_services->install_configuration_table + (&acpi20, acpi_data); + } + err = grub_efi_prepare_platform(); if (err != GRUB_ERR_NONE) return err; @@ -416,7 +426,74 @@ grub_cmd_devicetree (grub_command_t cmd __attribute__ ((unused)), return grub_errno; }
-static grub_command_t cmd_linux, cmd_initrd, cmd_devicetree; +#define ACPI_BLOB_HEADER_SIZE 8 + +static void * +load_acpi (grub_file_t acpi, int size) +{ + void *blob; + + blob = grub_malloc (size); + if (!blob) + return NULL; + + if (grub_file_read (acpi, blob, size) != size) + { + grub_free (blob); + return NULL; + } + + if (grub_strncmp("ACPI", blob, 4)) + { + grub_free (blob); + return NULL; + } + + return blob; +} + +static grub_err_t +grub_cmd_armacpi (grub_command_t cmd __attribute__ ((unused)), + int argc, char *argv[]) +{ + grub_file_t acpi; + void *blob; + int size; + + if (argc != 1) + return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("filename expected")); + + acpi = grub_file_open (argv[0]); + if (!acpi) + return grub_error (GRUB_ERR_FILE_NOT_FOUND, N_("failed to open file")); + + size = grub_file_size (acpi); + if (size == 0) + goto out; + + blob = load_acpi (acpi, size); + if (!blob) + goto out; + +#ifdef GRUB_MACHINE_EFI + acpi_data = (grub_addr_t) grub_efi_allocate_loader_memory (LINUX_ACPI_PHYS_OFFSET, size - ACPI_BLOB_HEADER_SIZE); +#else + acpi_data = LINUX_ACPI_ADDRESS; +#endif + grub_dprintf ("loader", "Loading ACPI to 0x%08x\n", + (grub_addr_t) acpi_data); + grub_memmove ((void *)acpi_data, blob + ACPI_BLOB_HEADER_SIZE, size - ACPI_BLOB_HEADER_SIZE); + grub_free (blob); + + grub_errno = GRUB_ERR_NONE; + +out: + grub_file_close (acpi); + + return grub_errno; +} + +static grub_command_t cmd_linux, cmd_initrd, cmd_devicetree, cmd_acpi;
GRUB_MOD_INIT (linux) { @@ -426,6 +503,8 @@ GRUB_MOD_INIT (linux) 0, N_("Load initrd.")); cmd_devicetree = grub_register_command ("devicetree", grub_cmd_devicetree, 0, N_("Load DTB file.")); + cmd_acpi = grub_register_command ("armacpi", grub_cmd_armacpi, + 0, N_("Load ARM ACPI blob.")); my_mod = mod; firmware_boot_data = firmware_get_boot_data ();
@@ -438,4 +517,5 @@ GRUB_MOD_FINI (linux) grub_unregister_command (cmd_linux); grub_unregister_command (cmd_initrd); grub_unregister_command (cmd_devicetree); + grub_unregister_command (cmd_acpi); } diff --git a/include/grub/arm/linux.h b/include/grub/arm/linux.h index bb04636..5269b8e 100644 --- a/include/grub/arm/linux.h +++ b/include/grub/arm/linux.h @@ -30,6 +30,7 @@ # define LINUX_ADDRESS (start_of_ram + 0x8000) # define LINUX_INITRD_ADDRESS (start_of_ram + 0x02000000) # define LINUX_FDT_ADDRESS (LINUX_INITRD_ADDRESS - 0x10000) +# define LINUX_ACPI_ADDRESS (LINUX_FDT_ADDRESS - 0x10000) # define firmware_get_boot_data grub_uboot_get_boot_data # define firmware_get_machine_type grub_uboot_get_machine_type #elif defined GRUB_MACHINE_EFI @@ -40,6 +41,8 @@ # define LINUX_PHYS_OFFSET (0x00008000) # define LINUX_INITRD_PHYS_OFFSET (LINUX_PHYS_OFFSET + 0x02000000) # define LINUX_FDT_PHYS_OFFSET (LINUX_INITRD_PHYS_OFFSET - 0x10000) +# define LINUX_ACPI_PHYS_OFFSET (LINUX_FDT_PHYS_OFFSET - 0x10000) + static inline grub_addr_t firmware_get_boot_data (void) {
I'm getting build failures on the default build with this patch (due to -Werror).
On Thu, Jul 18, 2013 at 04:17:48PM +0100, Graeme Gregory wrote:
Added location of ACPI blob loading to linux.h
grub-core/loader/arm/linux.c | 82 +++++++++++++++++++++++++++++++++++++++++- include/grub/arm/linux.h | 3 ++ 2 files changed, 84 insertions(+), 1 deletion(-)
diff --git a/grub-core/loader/arm/linux.c b/grub-core/loader/arm/linux.c index f5faa7b..dee49d9 100644 --- a/grub-core/loader/arm/linux.c +++ b/grub-core/loader/arm/linux.c @@ -45,6 +45,8 @@ static grub_addr_t firmware_boot_data; static grub_addr_t boot_data; static grub_uint32_t machine_type; +static grub_addr_t acpi_data;
/*
- linux_prepare_fdt():
- Prepares a loaded FDT for being passed to Linux.
@@ -202,6 +204,14 @@ linux_boot (void) linuxmain = (kernel_entry_t) linux_addr; #ifdef GRUB_MACHINE_EFI
- if (acpi_data) {
- struct grub_efi_guid acpi = GRUB_EFI_ACPI_TABLE_GUID;
loader/arm/linux.c:208:26: error: unused variable 'acpi' [-Werror=unused-variable] struct grub_efi_guid acpi = GRUB_EFI_ACPI_TABLE_GUID; ^
- struct grub_efi_guid acpi20 = GRUB_EFI_ACPI_20_TABLE_GUID;
- grub_efi_system_table->boot_services->install_configuration_table
(&acpi20, acpi_data);
loader/arm/linux.c:212:7: error: passing argument 2 of 'grub_efi_system_table->boot_services->install_configuration_table' makes pointer from integer without a cast [-Werror] (&acpi20, acpi_data); ^ loader/arm/linux.c:212:7: note: expected 'void *' but argument is of type 'grub_addr_t'
- }
- err = grub_efi_prepare_platform(); if (err != GRUB_ERR_NONE) return err;
@@ -416,7 +426,74 @@ grub_cmd_devicetree (grub_command_t cmd __attribute__ ((unused)), return grub_errno; } -static grub_command_t cmd_linux, cmd_initrd, cmd_devicetree; +#define ACPI_BLOB_HEADER_SIZE 8
+static void * +load_acpi (grub_file_t acpi, int size) +{
- void *blob;
- blob = grub_malloc (size);
- if (!blob)
- return NULL;
- if (grub_file_read (acpi, blob, size) != size)
- {
grub_free (blob);return NULL;- }
- if (grub_strncmp("ACPI", blob, 4))
- {
grub_free (blob);return NULL;- }
- return blob;
+}
+static grub_err_t +grub_cmd_armacpi (grub_command_t cmd __attribute__ ((unused)),
int argc, char *argv[])+{
- grub_file_t acpi;
- void *blob;
- int size;
- if (argc != 1)
- return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("filename expected"));
- acpi = grub_file_open (argv[0]);
- if (!acpi)
- return grub_error (GRUB_ERR_FILE_NOT_FOUND, N_("failed to open file"));
- size = grub_file_size (acpi);
- if (size == 0)
- goto out;
- blob = load_acpi (acpi, size);
- if (!blob)
- goto out;
+#ifdef GRUB_MACHINE_EFI
- acpi_data = (grub_addr_t) grub_efi_allocate_loader_memory (LINUX_ACPI_PHYS_OFFSET, size - ACPI_BLOB_HEADER_SIZE);
+#else
- acpi_data = LINUX_ACPI_ADDRESS;
+#endif
- grub_dprintf ("loader", "Loading ACPI to 0x%08x\n",
(grub_addr_t) acpi_data);- grub_memmove ((void *)acpi_data, blob + ACPI_BLOB_HEADER_SIZE, size - ACPI_BLOB_HEADER_SIZE);
loader/arm/linux.c:485:41: error: pointer of type 'void *' used in arithmetic [-Werror=pointer-arith] grub_memmove ((void *)acpi_data, blob + ACPI_BLOB_HEADER_SIZE, size - ACPI_BLOB_HEADER_SIZE);
- grub_free (blob);
- grub_errno = GRUB_ERR_NONE;
+out:
- grub_file_close (acpi);
- return grub_errno;
+}
+static grub_command_t cmd_linux, cmd_initrd, cmd_devicetree, cmd_acpi; GRUB_MOD_INIT (linux) { @@ -426,6 +503,8 @@ GRUB_MOD_INIT (linux) 0, N_("Load initrd.")); cmd_devicetree = grub_register_command ("devicetree", grub_cmd_devicetree, 0, N_("Load DTB file."));
- cmd_acpi = grub_register_command ("armacpi", grub_cmd_armacpi,
0, N_("Load ARM ACPI blob."));@@ -438,4 +517,5 @@ GRUB_MOD_FINI (linux) grub_unregister_command (cmd_linux); grub_unregister_command (cmd_initrd); grub_unregister_command (cmd_devicetree);
- grub_unregister_command (cmd_acpi);
} diff --git a/include/grub/arm/linux.h b/include/grub/arm/linux.h index bb04636..5269b8e 100644 --- a/include/grub/arm/linux.h +++ b/include/grub/arm/linux.h @@ -30,6 +30,7 @@ # define LINUX_ADDRESS (start_of_ram + 0x8000) # define LINUX_INITRD_ADDRESS (start_of_ram + 0x02000000) # define LINUX_FDT_ADDRESS (LINUX_INITRD_ADDRESS - 0x10000) +# define LINUX_ACPI_ADDRESS (LINUX_FDT_ADDRESS - 0x10000) # define firmware_get_boot_data grub_uboot_get_boot_data # define firmware_get_machine_type grub_uboot_get_machine_type #elif defined GRUB_MACHINE_EFI @@ -40,6 +41,8 @@ # define LINUX_PHYS_OFFSET (0x00008000) # define LINUX_INITRD_PHYS_OFFSET (LINUX_PHYS_OFFSET + 0x02000000) # define LINUX_FDT_PHYS_OFFSET (LINUX_INITRD_PHYS_OFFSET - 0x10000) +# define LINUX_ACPI_PHYS_OFFSET (LINUX_FDT_PHYS_OFFSET - 0x10000)
static inline grub_addr_t firmware_get_boot_data (void) { -- 1.7.10.4
In short, the following makes it build successfully:
=== modified file 'grub-core/loader/arm/linux.c' --- grub-core/loader/arm/linux.c 2013-07-18 17:32:34 +0000 +++ grub-core/loader/arm/linux.c 2013-07-18 17:41:59 +0000 @@ -205,11 +205,10 @@
#ifdef GRUB_MACHINE_EFI if (acpi_data) { - struct grub_efi_guid acpi = GRUB_EFI_ACPI_TABLE_GUID; struct grub_efi_guid acpi20 = GRUB_EFI_ACPI_20_TABLE_GUID;
grub_efi_system_table->boot_services->install_configuration_table - (&acpi20, acpi_data); + (&acpi20, (void *) acpi_data); }
err = grub_efi_prepare_platform(); @@ -482,7 +481,7 @@ #endif grub_dprintf ("loader", "Loading ACPI to 0x%08x\n", (grub_addr_t) acpi_data); - grub_memmove ((void *)acpi_data, blob + ACPI_BLOB_HEADER_SIZE, size - ACPI_BLOB_HEADER_SIZE); + grub_memmove ((void *)acpi_data, (void *)((grub_addr_t)blob + ACPI_BLOB_HEADER_SIZE), size - ACPI_BLOB_HEADER_SIZE); grub_free (blob);
grub_errno = GRUB_ERR_NONE;
---
So if you merge that into your patch, I'll push it into my arm-uefi branch of linaro-grub.
/ Leif
On 18/07/13 18:59, Leif Lindholm wrote:
I'm getting build failures on the default build with this patch (due to -Werror).
Curses I knew having to turn that off would bite my ass!
grub_addr_t is not a byte sized pointer so arithmetic went all wrong so I did it slightly different way, updated patch will follow!
Thanks
Graeme
On Thu, Jul 18, 2013 at 04:17:48PM +0100, Graeme Gregory wrote:
Added location of ACPI blob loading to linux.h
grub-core/loader/arm/linux.c | 82 +++++++++++++++++++++++++++++++++++++++++- include/grub/arm/linux.h | 3 ++ 2 files changed, 84 insertions(+), 1 deletion(-)
diff --git a/grub-core/loader/arm/linux.c b/grub-core/loader/arm/linux.c index f5faa7b..dee49d9 100644 --- a/grub-core/loader/arm/linux.c +++ b/grub-core/loader/arm/linux.c @@ -45,6 +45,8 @@ static grub_addr_t firmware_boot_data; static grub_addr_t boot_data; static grub_uint32_t machine_type; +static grub_addr_t acpi_data;
/*
- linux_prepare_fdt():
- Prepares a loaded FDT for being passed to Linux.
@@ -202,6 +204,14 @@ linux_boot (void) linuxmain = (kernel_entry_t) linux_addr; #ifdef GRUB_MACHINE_EFI
- if (acpi_data) {
- struct grub_efi_guid acpi = GRUB_EFI_ACPI_TABLE_GUID;
loader/arm/linux.c:208:26: error: unused variable 'acpi' [-Werror=unused-variable] struct grub_efi_guid acpi = GRUB_EFI_ACPI_TABLE_GUID; ^
- struct grub_efi_guid acpi20 = GRUB_EFI_ACPI_20_TABLE_GUID;
- grub_efi_system_table->boot_services->install_configuration_table
(&acpi20, acpi_data);loader/arm/linux.c:212:7: error: passing argument 2 of 'grub_efi_system_table->boot_services->install_configuration_table' makes pointer from integer without a cast [-Werror] (&acpi20, acpi_data); ^ loader/arm/linux.c:212:7: note: expected 'void *' but argument is of type 'grub_addr_t'
- }
- err = grub_efi_prepare_platform(); if (err != GRUB_ERR_NONE) return err;
@@ -416,7 +426,74 @@ grub_cmd_devicetree (grub_command_t cmd __attribute__ ((unused)), return grub_errno; } -static grub_command_t cmd_linux, cmd_initrd, cmd_devicetree; +#define ACPI_BLOB_HEADER_SIZE 8
+static void * +load_acpi (grub_file_t acpi, int size) +{
- void *blob;
- blob = grub_malloc (size);
- if (!blob)
- return NULL;
- if (grub_file_read (acpi, blob, size) != size)
- {
grub_free (blob);return NULL;- }
- if (grub_strncmp("ACPI", blob, 4))
- {
grub_free (blob);return NULL;- }
- return blob;
+}
+static grub_err_t +grub_cmd_armacpi (grub_command_t cmd __attribute__ ((unused)),
int argc, char *argv[])+{
- grub_file_t acpi;
- void *blob;
- int size;
- if (argc != 1)
- return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("filename expected"));
- acpi = grub_file_open (argv[0]);
- if (!acpi)
- return grub_error (GRUB_ERR_FILE_NOT_FOUND, N_("failed to open file"));
- size = grub_file_size (acpi);
- if (size == 0)
- goto out;
- blob = load_acpi (acpi, size);
- if (!blob)
- goto out;
+#ifdef GRUB_MACHINE_EFI
- acpi_data = (grub_addr_t) grub_efi_allocate_loader_memory (LINUX_ACPI_PHYS_OFFSET, size - ACPI_BLOB_HEADER_SIZE);
+#else
- acpi_data = LINUX_ACPI_ADDRESS;
+#endif
- grub_dprintf ("loader", "Loading ACPI to 0x%08x\n",
(grub_addr_t) acpi_data);- grub_memmove ((void *)acpi_data, blob + ACPI_BLOB_HEADER_SIZE, size - ACPI_BLOB_HEADER_SIZE);
loader/arm/linux.c:485:41: error: pointer of type 'void *' used in arithmetic [-Werror=pointer-arith] grub_memmove ((void *)acpi_data, blob + ACPI_BLOB_HEADER_SIZE, size - ACPI_BLOB_HEADER_SIZE);
- grub_free (blob);
- grub_errno = GRUB_ERR_NONE;
+out:
- grub_file_close (acpi);
- return grub_errno;
+}
+static grub_command_t cmd_linux, cmd_initrd, cmd_devicetree, cmd_acpi; GRUB_MOD_INIT (linux) { @@ -426,6 +503,8 @@ GRUB_MOD_INIT (linux) 0, N_("Load initrd.")); cmd_devicetree = grub_register_command ("devicetree", grub_cmd_devicetree, 0, N_("Load DTB file."));
- cmd_acpi = grub_register_command ("armacpi", grub_cmd_armacpi,
0, N_("Load ARM ACPI blob."));@@ -438,4 +517,5 @@ GRUB_MOD_FINI (linux) grub_unregister_command (cmd_linux); grub_unregister_command (cmd_initrd); grub_unregister_command (cmd_devicetree);
- grub_unregister_command (cmd_acpi);
} diff --git a/include/grub/arm/linux.h b/include/grub/arm/linux.h index bb04636..5269b8e 100644 --- a/include/grub/arm/linux.h +++ b/include/grub/arm/linux.h @@ -30,6 +30,7 @@ # define LINUX_ADDRESS (start_of_ram + 0x8000) # define LINUX_INITRD_ADDRESS (start_of_ram + 0x02000000) # define LINUX_FDT_ADDRESS (LINUX_INITRD_ADDRESS - 0x10000) +# define LINUX_ACPI_ADDRESS (LINUX_FDT_ADDRESS - 0x10000) # define firmware_get_boot_data grub_uboot_get_boot_data # define firmware_get_machine_type grub_uboot_get_machine_type #elif defined GRUB_MACHINE_EFI @@ -40,6 +41,8 @@ # define LINUX_PHYS_OFFSET (0x00008000) # define LINUX_INITRD_PHYS_OFFSET (LINUX_PHYS_OFFSET + 0x02000000) # define LINUX_FDT_PHYS_OFFSET (LINUX_INITRD_PHYS_OFFSET - 0x10000) +# define LINUX_ACPI_PHYS_OFFSET (LINUX_FDT_PHYS_OFFSET - 0x10000)
static inline grub_addr_t firmware_get_boot_data (void) { -- 1.7.10.4
In short, the following makes it build successfully:
=== modified file 'grub-core/loader/arm/linux.c' --- grub-core/loader/arm/linux.c 2013-07-18 17:32:34 +0000 +++ grub-core/loader/arm/linux.c 2013-07-18 17:41:59 +0000 @@ -205,11 +205,10 @@ #ifdef GRUB_MACHINE_EFI if (acpi_data) {
- struct grub_efi_guid acpi = GRUB_EFI_ACPI_TABLE_GUID; struct grub_efi_guid acpi20 = GRUB_EFI_ACPI_20_TABLE_GUID;
grub_efi_system_table->boot_services->install_configuration_table
(&acpi20, acpi_data);
(&acpi20, (void *) acpi_data);err = grub_efi_prepare_platform(); @@ -482,7 +481,7 @@ #endif grub_dprintf ("loader", "Loading ACPI to 0x%08x\n", (grub_addr_t) acpi_data);
- grub_memmove ((void *)acpi_data, blob + ACPI_BLOB_HEADER_SIZE, size - ACPI_BLOB_HEADER_SIZE);
- grub_memmove ((void *)acpi_data, (void *)((grub_addr_t)blob + ACPI_BLOB_HEADER_SIZE), size - ACPI_BLOB_HEADER_SIZE); grub_free (blob);
grub_errno = GRUB_ERR_NONE;
So if you merge that into your patch, I'll push it into my arm-uefi branch of linaro-grub.
/ Leif
-
Graeme Gregory -
Leif Lindholm