Re: [Linaro-acpi] [PATCHv2] Added armacpi command to loader to load ACPI blob for arm and add this to the configuration tables using API

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."));
  

  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) { -- 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