On 18/07/13 01:22, Leif Lindholm wrote:

On Wed, Jul 17, 2013 at 04:03:59PM +0100, Graeme Gregory wrote:

...

Please find attached patch to add new armacpi command to grub. This will load the blob and do the correct thing to add it to the system configuration table.

Cool - it even does it properly :) Question: are you asking me to carry this patch in my tree?

That would be good, I shall address your comments and send a v2.

Graeme

A couple of comments inline:

...

=== modified file 'grub-core/loader/arm/linux.c' --- grub-core/loader/arm/linux.c 2013-04-26 11:32:58 +0000 +++ grub-core/loader/arm/linux.c 2013-07-17 14:12:24 +0000 @@ -45,6 +45,8 @@ 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 @@ 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;

@@ -402,7 +412,7 @@ fdt_move (blob, (void *) boot_data, fdt_totalsize (blob)); grub_free (blob);

• /*

• /*

Only whitespace changes - strip?

...

* We've successfully loaded an FDT, so any machine type passed
* from firmware is now obsolete.
*/

@@ -416,7 +426,73 @@ 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;

+}

+grub_cmd_armacpi (grub_command_t cmd __attribute__ ((unused)),

•      int argc, char *argv[])
  

Missing return type?

...

+{

• 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)
• return GRUB_ERR_FILE_NOT_FOUND;

Not closing file?

...

+#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 +502,8 @@ 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 +516,5 @@ grub_unregister_command (cmd_linux); grub_unregister_command (cmd_initrd); grub_unregister_command (cmd_devicetree);

• grub_unregister_command (cmd_acpi);

}

=== modified file 'include/grub/arm/linux.h' --- include/grub/arm/linux.h 2013-04-28 03:33:46 +0000 +++ include/grub/arm/linux.h 2013-07-15 09:04:54 +0000 @@ -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) {