[PATCH 3/5] loader/efi/fdt.c: fixup grub_file_open call

[Leif Lindholm]

The verifiers framework changed the api of grub_file_open, but did not
fix up all users. Add the file type GRUB_FILE_TYPE_DEVICE_TREE_IMAGE
to the "devicetree" command handler call.

Signed-off-by: Leif Lindholm <***@linaro.org>
---
grub-core/loader/efi/fdt.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/grub-core/loader/efi/fdt.c b/grub-core/loader/efi/fdt.c
index a4c6e8036..a18ca8ccb 100644
--- a/grub-core/loader/efi/fdt.c
+++ b/grub-core/loader/efi/fdt.c
@@ -123,7 +123,7 @@ grub_cmd_devicetree (grub_command_t cmd __attribute__ ((unused)),
return GRUB_ERR_NONE;
}

- dtb = grub_file_open (argv[0]);
+ dtb = grub_file_open (argv[0], GRUB_FILE_TYPE_DEVICE_TREE_IMAGE);
if (!dtb)
goto out;

[Leif Lindholm]

The api change of grub_file_open for adding verifiers did not include
a type for device tree blobs. Add GRUB_FILE_TYPE_DEVICE_TREE_IMAGE to
the grub_file_type enum.

Signed-off-by: Leif Lindholm <***@linaro.org>
---
include/grub/file.h | 2 ++
1 file changed, 2 insertions(+)

diff --git a/include/grub/file.h b/include/grub/file.h
index 19dda67f6..9aae46355 100644
--- a/include/grub/file.h
+++ b/include/grub/file.h
@@ -69,6 +69,8 @@ enum grub_file_type

GRUB_FILE_TYPE_EFI_CHAINLOADED_IMAGE,

+ GRUB_FILE_TYPE_DEVICE_TREE_IMAGE,
+
/* File holding signature. */
GRUB_FILE_TYPE_SIGNATURE,
/* File holding public key to verify signature once. */

[Leif Lindholm]

verify.h was added without include guards. This means compiling anything
including both grub/verify.h and grub/lib/cmdline.h fails (at least
loader/arm64/linux.c.

Add the necessary include guard.

Signed-off-by: Leif Lindholm <***@linaro.org>
---
include/grub/verify.h | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/include/grub/verify.h b/include/grub/verify.h
index 79022b422..dedb14a1d 100644
--- a/include/grub/verify.h
+++ b/include/grub/verify.h
@@ -16,6 +16,9 @@
* along with GRUB. If not, see <http://www.gnu.org/licenses/>.
*/

+#ifndef GRUB_VERIFY_HEADER
+#define GRUB_VERIFY_HEADER
+
#include <grub/file.h>
#include <grub/list.h>

@@ -76,3 +79,5 @@ grub_verifier_unregister (struct grub_file_verifier *ver)

grub_err_t
grub_verify_string (char *str, enum grub_verify_string_type type);
+
+#endif /* ! GRUB_VERIFY_HEADER */

[Leif Lindholm]

- add variable "err" (used but not defined)
- add GRUB_FILE_TYPE_LINUX_KERNEL to grub_file_open call

Signed-off-by: Leif Lindholm <***@linaro.org>
---
grub-core/loader/arm64/linux.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/grub-core/loader/arm64/linux.c b/grub-core/loader/arm64/linux.c
index b8315b53f..c37295c0b 100644
--- a/grub-core/loader/arm64/linux.c
+++ b/grub-core/loader/arm64/linux.c
@@ -288,6 +288,7 @@ grub_cmd_linux (grub_command_t cmd __attribute__ ((unused)),
{
grub_file_t file = 0;
struct linux_armxx_kernel_header lh;
+ grub_err_t err;

grub_dl_ref (my_mod);

@@ -297,7 +298,7 @@ grub_cmd_linux (grub_command_t cmd __attribute__ ((unused)),
goto fail;
}

- file = grub_file_open (argv[0]);
+ file = grub_file_open (argv[0], GRUB_FILE_TYPE_LINUX_KERNEL);
if (!file)
goto fail;

[Leif Lindholm]

The verifiers framework changed the grub_file_open interface,
breaking all non-x86 linux loaders.
Add file types to the grub_file_open calls to make them build
again.

Signed-off-by: Leif Lindholm <***@linaro.org>
---

Bundling these changes together in a single patch, since I
haven't actually tested these.

grub-core/loader/arm/linux.c | 6 +++---
grub-core/loader/ia64/efi/linux.c | 2 +-
grub-core/loader/sparc64/ieee1275/linux.c | 2 +-
3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/grub-core/loader/arm/linux.c b/grub-core/loader/arm/linux.c
index 80293fb1f..67ed79359 100644
--- a/grub-core/loader/arm/linux.c
+++ b/grub-core/loader/arm/linux.c
@@ -363,7 +363,7 @@ grub_cmd_linux (grub_command_t cmd __attribute__ ((unused)),
if (argc == 0)
return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("filename expected"));

- file = grub_file_open (argv[0]);
+ file = grub_file_open (argv[0], GRUB_FILE_TYPE_LINUX_KERNEL);
if (!file)
goto fail;

@@ -408,7 +408,7 @@ grub_cmd_initrd (grub_command_t cmd __attribute__ ((unused)),
if (argc == 0)
return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("filename expected"));

- file = grub_file_open (argv[0]);
+ file = grub_file_open (argv[0], GRUB_FILE_TYPE_LINUX_INITRD);
if (!file)
return grub_errno;

@@ -471,7 +471,7 @@ grub_cmd_devicetree (grub_command_t cmd __attribute__ ((unused)),
if (argc != 1)
return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("filename expected"));

- dtb = grub_file_open (argv[0]);
+ dtb = grub_file_open (argv[0], GRUB_FILE_TYPE_DEVICE_TREE_IMAGE);
if (!dtb)
return grub_errno;

diff --git a/grub-core/loader/ia64/efi/linux.c b/grub-core/loader/ia64/efi/linux.c
index 6477d70f0..639a1f379 100644
--- a/grub-core/loader/ia64/efi/linux.c
+++ b/grub-core/loader/ia64/efi/linux.c
@@ -460,7 +460,7 @@ grub_cmd_linux (grub_command_t cmd __attribute__ ((unused)),
goto fail;
}

- file = grub_file_open (argv[0]);
+ file = grub_file_open (argv[0], GRUB_FILE_TYPE_LINUX_KERNEL);
if (! file)
goto fail;

diff --git a/grub-core/loader/sparc64/ieee1275/linux.c b/grub-core/loader/sparc64/ieee1275/linux.c
index abe46faa0..bb47ee0cc 100644
--- a/grub-core/loader/sparc64/ieee1275/linux.c
+++ b/grub-core/loader/sparc64/ieee1275/linux.c
@@ -306,7 +306,7 @@ grub_cmd_linux (grub_command_t cmd __attribute__ ((unused)),
goto out;
}

- file = grub_file_open (argv[0]);
+ file = grub_file_open (argv[0], GRUB_FILE_TYPE_LINUX_KERNEL);
if (!file)
goto out;

[Fu Wei Fu]

Hi Leif,

Great thanks for your information, will do ASAP when I am back home.

The verifiers framework caused some breakage for non-x86
platforms that need fixing.
- Add an include guard for verify.h.
- Add a new file type for device tree blobs.
- Fix an error added to arm64/linux.c.
- Add file types to grub_file_open calls.
Note: only the common and arm64 fixes have actually been tested.
Fu Wei, Julien - can you guys have a look at fixing arm64/xen_boot.c?
I'm not 100% certain how multiboot should be handled.
grub/verify.h: add include guard
file.h: add device tree file type
loader/efi/fdt.c: fixup grub_file_open call
arm64/efi: fix breakage caused by verifiers
arm-uboot, ia64, sparc64: fix up grub_file_open calls
grub-core/loader/arm/linux.c | 6 +++---
grub-core/loader/arm64/linux.c | 3 ++-
grub-core/loader/efi/fdt.c | 2 +-
grub-core/loader/ia64/efi/linux.c | 2 +-
grub-core/loader/sparc64/ieee1275/linux.c | 2 +-
include/grub/file.h | 2 ++
include/grub/verify.h | 5 +++++
7 files changed, 15 insertions(+), 7 deletions(-)
--
2.11.0

[Daniel Kiper]

Post by Fu Wei Fu
Hi Leif,
Great thanks for your information, will do ASAP when I am back home.

I have pushed fixes into the master, so, please take a look at it.

Daniel

[Julien Grall]

Hi,

arm64/xen: Fix implicit declaration of function ‘grub_file_filter_disable_compression

This patch seems to drop support for --nounzip. Can you explain why?

Note that the option added on Arm64 to keep the compatibility with x86 multiboot
loading.

Cheers,

loader/arm64/xen_boot.c:433:5: error: implicit declaration of function ‘grub_file_filter_disable_compression’; did you mean ‘grub_file_filter_unregister’? [-Werror=implicit-function-declaration]
grub_file_filter_disable_compression ();
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
grub_file_filter_unregister
loader/arm64/xen_boot.c:433:5: error: nested extern declaration of ‘grub_file_filter_disable_compression’ [-Werror=nested-externs]
diff --git a/grub-core/loader/arm64/xen_boot.c b/grub-core/loader/arm64/xen_boot.c
index 33a855df4..5820412e8 100644
--- a/grub-core/loader/arm64/xen_boot.c
+++ b/grub-core/loader/arm64/xen_boot.c
@@ -391,7 +391,6 @@ grub_cmd_xen_module (grub_command_t cmd __attribute__((unused)),
struct xen_boot_binary *module = NULL;
grub_file_t file = 0;
- int nounzip = 0;
if (!argc)
{
@@ -399,13 +398,6 @@ grub_cmd_xen_module (grub_command_t cmd __attribute__((unused)),
goto fail;
}
- if (grub_strcmp (argv[0], "--nounzip") == 0)
- {
- argv++;
- argc--;
- nounzip = 1;
- }
-
if (!argc)
{
grub_error (GRUB_ERR_BAD_ARGUMENT, N_("filename expected"));
@@ -429,8 +421,6 @@ grub_cmd_xen_module (grub_command_t cmd __attribute__((unused)),
grub_dprintf ("xen_loader", "Init module and node info\n");
- if (nounzip)
- grub_file_filter_disable_compression ();
file = grub_file_open (argv[0]);
if (!file)
goto fail;
diff --git a/grub-core/osdep/generic/blocklist.c b/grub-core/osdep/generic/blocklist.c
index 74024fd06..63e0aed35 100644
--- a/grub-core/osdep/generic/blocklist.c
+++ b/grub-core/osdep/generic/blocklist.c
@@ -59,7 +59,6 @@ grub_install_get_blocklist (grub_device_t root_dev,
grub_disk_cache_invalidate_all ();
- grub_file_filter_disable_compression ();
file = grub_file_open (core_path_dev);
if (file)
{
@@ -117,7 +116,6 @@ grub_install_get_blocklist (grub_device_t root_dev,
grub_file_t file;
/* Now read the core image to determine where the sectors are. */
- grub_file_filter_disable_compression ();
file = grub_file_open (core_path_dev);
if (! file)
grub_util_error ("%s", grub_errmsg);

[Julien Grall]

Hi,

Post by Julien Grall

arm64/xen: Fix implicit declaration of function ‘grub_file_filter_disable_compression

This patch seems to drop support for --nounzip. Can you explain why?

It only seemed to serve the invocation of grub_file_filter_disable_compression()
which was removed in this patch.

But --nounzip will not work as expected after this patch. Looking at GRUB, it
seems the call to that function was replaced by passing
GRUB_FILE_TYPE_NO_DECOMPRESSION to grub_file_open.

Is there any reason to not use it for Arm?

Cheers,

On closer inspection, I do see that
argv/argc still needs to be shifted though. I will fix that.

Post by Julien Grall
Note that the option added on Arm64 to keep the compatibility with x86
multiboot loading.

[Lee Jones]

Post by Julien Grall
Hi,

Post by Julien Grall

arm64/xen: Fix implicit declaration of function ‘grub_file_filter_disable_compression

This patch seems to drop support for --nounzip. Can you explain why?

It only seemed to serve the invocation of grub_file_filter_disable_compression()
which was removed in this patch.

But --nounzip will not work as expected after this patch. Looking at GRUB,
it seems the call to that function was replaced by passing
GRUB_FILE_TYPE_NO_DECOMPRESSION to grub_file_open.

Ah yes, I see. Please bear with me.

I need to split this patch too.

Post by Julien Grall
Is there any reason to not use it for Arm?

[Julien Grall]

Hi Lee,

arm64/xen: Fix too few arguments to function ‘grub_file_open’
loader/arm64/xen_boot.c:424:10: error: too few arguments to function ‘grub_file_open’
file = grub_file_open (argv[0]);
^~~~~~~~~~~~~~
In file included from ../include/grub/cache.h:23:0,
../include/grub/file.h:204:25: note: declared here
grub_file_t EXPORT_FUNC(grub_file_open) (const char *name, enum grub_file_type type);
^
../include/grub/symbol.h:68:25: note: in definition of macro ‘EXPORT_FUNC’
# define EXPORT_FUNC(x) x
^
loader/arm64/xen_boot.c:456:10: error: too few arguments to function ‘grub_file_open’
file = grub_file_open (argv[0]);
^~~~~~~~~~~~~~
diff --git a/grub-core/loader/arm64/xen_boot.c b/grub-core/loader/arm64/xen_boot.c
index 5820412e8..1f49e3278 100644
--- a/grub-core/loader/arm64/xen_boot.c
+++ b/grub-core/loader/arm64/xen_boot.c
@@ -421,7 +421,7 @@ grub_cmd_xen_module (grub_command_t cmd __attribute__((unused)),
grub_dprintf ("xen_loader", "Init module and node info\n");
- file = grub_file_open (argv[0]);
+ file = grub_file_open (argv[0], GRUB_FILE_TYPE_NONE);
if (!file)
goto fail;
@@ -453,7 +453,7 @@ grub_cmd_xen_hypervisor (grub_command_t cmd __attribute__ ((unused)),
goto fail;
}
- file = grub_file_open (argv[0]);
+ file = grub_file_open (argv[0], GRUB_FILE_TYPE_NONE);

I would prefer if we try to keep the type similar to x86 Xen. In that case it
would be GRUB_FILE_TYPE_LINUX_KERNEL.

Cheers,

[Lee Jones]

Post by Julien Grall
Hi Lee,

arm64/xen: Fix too few arguments to function ‘grub_file_open’
loader/arm64/xen_boot.c:424:10: error: too few arguments to function ‘grub_file_open’
file = grub_file_open (argv[0]);
^~~~~~~~~~~~~~
In file included from ../include/grub/cache.h:23:0,
../include/grub/file.h:204:25: note: declared here
grub_file_t EXPORT_FUNC(grub_file_open) (const char *name, enum grub_file_type type);
^
../include/grub/symbol.h:68:25: note: in definition of macro ‘EXPORT_FUNC’
# define EXPORT_FUNC(x) x
^
loader/arm64/xen_boot.c:456:10: error: too few arguments to function ‘grub_file_open’
file = grub_file_open (argv[0]);
^~~~~~~~~~~~~~
diff --git a/grub-core/loader/arm64/xen_boot.c b/grub-core/loader/arm64/xen_boot.c
index 5820412e8..1f49e3278 100644
--- a/grub-core/loader/arm64/xen_boot.c
+++ b/grub-core/loader/arm64/xen_boot.c
@@ -421,7 +421,7 @@ grub_cmd_xen_module (grub_command_t cmd __attribute__((unused)),
grub_dprintf ("xen_loader", "Init module and node info\n");
- file = grub_file_open (argv[0]);
+ file = grub_file_open (argv[0], GRUB_FILE_TYPE_NONE);
if (!file)
goto fail;
@@ -453,7 +453,7 @@ grub_cmd_xen_hypervisor (grub_command_t cmd __attribute__ ((unused)),
goto fail;
}
- file = grub_file_open (argv[0]);
+ file = grub_file_open (argv[0], GRUB_FILE_TYPE_NONE);

I would prefer if we try to keep the type similar to x86 Xen. In that case
it would be GRUB_FILE_TYPE_LINUX_KERNEL.

Thanks Julien. Will fix.

[Julien Grall]

arm64/xen: Fix too few arguments to function ‘grub_create_loader_cmdline’
loader/arm64/xen_boot.c:370:7: error: too few arguments to function ‘grub_create_loader_cmdline’
grub_create_loader_cmdline (argc - 1, argv + 1, binary->cmdline,
^~~~~~~~~~~~~~~~~~~~~~~~~~
../include/grub/lib/cmdline.h:29:12: note: declared here
grub_err_t grub_create_loader_cmdline (int argc, char *argv[], char *buf,

Reviewed-by: Julien Grall <***@arm.com>

Cheers,

diff --git a/grub-core/loader/arm64/xen_boot.c b/grub-core/loader/arm64/xen_boot.c
index 1003a0b99..33a855df4 100644
--- a/grub-core/loader/arm64/xen_boot.c
+++ b/grub-core/loader/arm64/xen_boot.c
@@ -368,7 +368,8 @@ xen_boot_binary_load (struct xen_boot_binary *binary, grub_file_t file,
return;
}
grub_create_loader_cmdline (argc - 1, argv + 1, binary->cmdline,
- binary->cmdline_size);
+ binary->cmdline_size,
+ GRUB_VERIFY_KERNEL_CMDLINE);
grub_dprintf ("xen_loader",
binary->cmdline, binary->cmdline, binary->cmdline_size);

[Lee Jones]

From: Lee Jones <***@linaro.org>

arm64/xen: Fix implicit declaration of function ‘grub_file_filter_disable_compression'

Without this fix, building xen_boot.c emits:

loader/arm64/xen_boot.c:433:5: error: implicit declaration of function ‘grub_file_filter_disable_compression’; did you mean ‘grub_file_filter_unregister’? [-Werror=implicit-function-declaration]
grub_file_filter_disable_compression ();
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
grub_file_filter_unregister
loader/arm64/xen_boot.c:433:5: error: nested extern declaration of ‘grub_file_filter_disable_compression’ [-Werror=nested-externs]

Signed-off-by: Lee Jones <***@linaro.org>

diff --git a/grub-core/loader/arm64/xen_boot.c b/grub-core/loader/arm64/xen_boot.c
index 7d1adce1b..a01792a72 100644
--- a/grub-core/loader/arm64/xen_boot.c
+++ b/grub-core/loader/arm64/xen_boot.c
@@ -429,9 +429,9 @@ grub_cmd_xen_module (grub_command_t cmd __attribute__((unused)),

grub_dprintf ("xen_loader", "Init module and node info\n");

- if (nounzip)
- grub_file_filter_disable_compression ();
- file = grub_file_open (argv[0], GRUB_FILE_TYPE_LINUX_KERNEL);
+ file = grub_file_open (argv[0], GRUB_FILE_TYPE_LINUX_KERNEL
+ | (nounzip ? GRUB_FILE_TYPE_NO_DECOMPRESS
+ : GRUB_FILE_TYPE_NONE));
if (!file)
goto fail;

[Daniel Kiper]

Post by Lee Jones
arm64/xen: Fix implicit declaration of function ‘grub_file_filter_disable_compression'
loader/arm64/xen_boot.c:433:5: error: implicit declaration of function ‘grub_file_filter_disable_compression’; did you mean ‘grub_file_filter_unregister’? [-Werror=implicit-function-declaration]
grub_file_filter_disable_compression ();
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
grub_file_filter_unregister
loader/arm64/xen_boot.c:433:5: error: nested extern declaration of ‘grub_file_filter_disable_compression’ [-Werror=nested-externs]

I have not pushed this patch because it depends on "arm64/xen: Fix too
few arguments to function ‘grub_file_open’" patch.

Daniel

[Lee Jones]

From: Lee Jones <***@linaro.org>

generic/blocklist: Fix implicit declaration of function ‘grub_file_filter_disable_compression'

grub_file_filter_disable_compression() no longer exists.

Signed-off-by: Lee Jones <***@linaro.org>

diff --git a/grub-core/osdep/generic/blocklist.c b/grub-core/osdep/generic/blocklist.c
index 74024fd06..43186949d 100644
--- a/grub-core/osdep/generic/blocklist.c
+++ b/grub-core/osdep/generic/blocklist.c
@@ -59,8 +59,7 @@ grub_install_get_blocklist (grub_device_t root_dev,

grub_disk_cache_invalidate_all ();

- grub_file_filter_disable_compression ();
- file = grub_file_open (core_path_dev);
+ file = grub_file_open (core_path_dev, FILE_TYPE_NO_DECOMPRESS);
if (file)
{
if (grub_file_size (file) != core_size)
@@ -117,8 +116,7 @@ grub_install_get_blocklist (grub_device_t root_dev,

grub_file_t file;
/* Now read the core image to determine where the sectors are. */
- grub_file_filter_disable_compression ();
- file = grub_file_open (core_path_dev);
+ file = grub_file_open (core_path_dev, FILE_TYPE_NO_DECOMPRESS);
if (! file)
grub_util_error ("%s", grub_errmsg);

[Vladimir 'phcoder' Serbinenko]

LGTM

Post by Lee Jones
generic/blocklist: Fix implicit declaration of function
‘grub_file_filter_disable_compression'
grub_file_filter_disable_compression() no longer exists.
diff --git a/grub-core/osdep/generic/blocklist.c
b/grub-core/osdep/generic/blocklist.c
index 74024fd06..43186949d 100644
--- a/grub-core/osdep/generic/blocklist.c
+++ b/grub-core/osdep/generic/blocklist.c
@@ -59,8 +59,7 @@ grub_install_get_blocklist (grub_device_t root_dev,
grub_disk_cache_invalidate_all ();
- grub_file_filter_disable_compression ();
- file = grub_file_open (core_path_dev);
+ file = grub_file_open (core_path_dev, FILE_TYPE_NO_DECOMPRESS);
if (file)
{
if (grub_file_size (file) != core_size)
@@ -117,8 +116,7 @@ grub_install_get_blocklist (grub_device_t root_dev,
grub_file_t file;
/* Now read the core image to determine where the sectors are. */
- grub_file_filter_disable_compression ();
- file = grub_file_open (core_path_dev);
+ file = grub_file_open (core_path_dev, FILE_TYPE_NO_DECOMPRESS);
if (! file)
grub_util_error ("%s", grub_errmsg);
_______________________________________________
Grub-devel mailing list
https://lists.gnu.org/mailman/listinfo/grub-devel

[Daniel Kiper]

Post by Lee Jones
generic/blocklist: Fix implicit declaration of function ‘grub_file_filter_disable_compression'
grub_file_filter_disable_compression() no longer exists.

Pushed with two minor changes. Thanks a lot!

Daniel

[Lee Jones]

From: Lee Jones <***@linaro.org>

arm64/xen: Remove duplicate check for arguments

Signed-off-by: Lee Jones <***@linaro.org>

diff --git a/grub-core/loader/arm64/xen_boot.c b/grub-core/loader/arm64/xen_boot.c
index 1b1dd8c65..5af15c70d 100644
--- a/grub-core/loader/arm64/xen_boot.c
+++ b/grub-core/loader/arm64/xen_boot.c
@@ -404,12 +404,6 @@ grub_cmd_xen_module (grub_command_t cmd __attribute__((unused)),
argc--;
}

- if (!argc)
- {
- grub_error (GRUB_ERR_BAD_ARGUMENT, N_("filename expected"));
- goto fail;
- }
-
if (!loaded)
{
grub_error (GRUB_ERR_BAD_ARGUMENT,

[Vladimir 'phcoder' Serbinenko]

Post by Lee Jones
arm64/xen: Remove duplicate check for arguments

It's not duplicate because of argc-- right before it

Post by Lee Jones
diff --git a/grub-core/loader/arm64/xen_boot.c
b/grub-core/loader/arm64/xen_boot.c
index 1b1dd8c65..5af15c70d 100644
--- a/grub-core/loader/arm64/xen_boot.c
+++ b/grub-core/loader/arm64/xen_boot.c
@@ -404,12 +404,6 @@ grub_cmd_xen_module (grub_command_t cmd
__attribute__((unused)),
argc--;
}
- if (!argc)
- {
- grub_error (GRUB_ERR_BAD_ARGUMENT, N_("filename expected"));
- goto fail;
- }
-
if (!loaded)
{
grub_error (GRUB_ERR_BAD_ARGUMENT,
_______________________________________________
Grub-devel mailing list
https://lists.gnu.org/mailman/listinfo/grub-devel

[Lee Jones]

From: Lee Jones <***@linaro.org>

arm64/xen: Fix too few arguments to function ‘grub_file_open’

Without this fix xen_boot.c omits:

loader/arm64/xen_boot.c: In function ‘grub_cmd_xen_module’:
loader/arm64/xen_boot.c:424:10: error: too few arguments to function ‘grub_file_open’
file = grub_file_open (argv[0]);
^~~~~~~~~~~~~~
In file included from ../include/grub/cache.h:23:0,
from loader/arm64/xen_boot.c:19:
../include/grub/file.h:204:25: note: declared here
grub_file_t EXPORT_FUNC(grub_file_open) (const char *name, enum grub_file_type type);
^
../include/grub/symbol.h:68:25: note: in definition of macro ‘EXPORT_FUNC’
# define EXPORT_FUNC(x) x
^
loader/arm64/xen_boot.c: In function ‘grub_cmd_xen_hypervisor’:
loader/arm64/xen_boot.c:456:10: error: too few arguments to function ‘grub_file_open’
file = grub_file_open (argv[0]);
^~~~~~~~~~~~~~

Signed-off-by: Lee Jones <***@linaro.org>

diff --git a/grub-core/loader/arm64/xen_boot.c b/grub-core/loader/arm64/xen_boot.c
index 117293fb9..1b1dd8c65 100644
--- a/grub-core/loader/arm64/xen_boot.c
+++ b/grub-core/loader/arm64/xen_boot.c
@@ -427,7 +427,7 @@ grub_cmd_xen_module (grub_command_t cmd __attribute__((unused)),

grub_dprintf ("xen_loader", "Init module and node info\n");

- file = grub_file_open (argv[0]);
+ file = grub_file_open (argv[0], GRUB_FILE_TYPE_LINUX_KERNEL);
if (!file)
goto fail;

@@ -459,7 +459,7 @@ grub_cmd_xen_hypervisor (grub_command_t cmd __attribute__ ((unused)),
goto fail;
}

- file = grub_file_open (argv[0]);
+ file = grub_file_open (argv[0], GRUB_FILE_TYPE_LINUX_KERNEL);
if (!file)
goto fail;

[Daniel Kiper]

arm64/xen: Fix too few arguments to function ‘grub_file_open’
loader/arm64/xen_boot.c:424:10: error: too few arguments to function ‘grub_file_open’
file = grub_file_open (argv[0]);
^~~~~~~~~~~~~~
In file included from ../include/grub/cache.h:23:0,
../include/grub/file.h:204:25: note: declared here
grub_file_t EXPORT_FUNC(grub_file_open) (const char *name, enum grub_file_type type);
^
../include/grub/symbol.h:68:25: note: in definition of macro ‘EXPORT_FUNC’
# define EXPORT_FUNC(x) x
^
loader/arm64/xen_boot.c:456:10: error: too few arguments to function ‘grub_file_open’
file = grub_file_open (argv[0]);
^~~~~~~~~~~~~~
diff --git a/grub-core/loader/arm64/xen_boot.c b/grub-core/loader/arm64/xen_boot.c
index 117293fb9..1b1dd8c65 100644
--- a/grub-core/loader/arm64/xen_boot.c
+++ b/grub-core/loader/arm64/xen_boot.c
@@ -427,7 +427,7 @@ grub_cmd_xen_module (grub_command_t cmd __attribute__((unused)),
grub_dprintf ("xen_loader", "Init module and node info\n");
- file = grub_file_open (argv[0]);
+ file = grub_file_open (argv[0], GRUB_FILE_TYPE_LINUX_KERNEL);
if (!file)
goto fail;
@@ -459,7 +459,7 @@ grub_cmd_xen_hypervisor (grub_command_t cmd __attribute__ ((unused)),
goto fail;
}
- file = grub_file_open (argv[0]);
+ file = grub_file_open (argv[0], GRUB_FILE_TYPE_LINUX_KERNEL);

Please introduce GRUB_FILE_TYPE_XEN_HYPERVISOR and GRUB_FILE_TYPE_XEN_MODULE.
You can put both of them behind GRUB_FILE_TYPE_MULTIBOOT_MODULE.

Daniel

[Daniel Kiper]

arm64/xen: Fix too few arguments to function ‘grub_create_loader_cmdline’
loader/arm64/xen_boot.c:370:7: error: too few arguments to function ‘grub_create_loader_cmdline’
grub_create_loader_cmdline (argc - 1, argv + 1, binary->cmdline,
^~~~~~~~~~~~~~~~~~~~~~~~~~
../include/grub/lib/cmdline.h:29:12: note: declared here
grub_err_t grub_create_loader_cmdline (int argc, char *argv[], char *buf,

Pushed despite it had malformed tabs. Next time please use "git format-patch"
and "git send-email" to build and send the patches.

Daniel

[Lee Jones]

Post by Daniel Kiper

arm64/xen: Fix too few arguments to function ‘grub_create_loader_cmdline’
loader/arm64/xen_boot.c:370:7: error: too few arguments to function ‘grub_create_loader_cmdline’
grub_create_loader_cmdline (argc - 1, argv + 1, binary->cmdline,
^~~~~~~~~~~~~~~~~~~~~~~~~~
../include/grub/lib/cmdline.h:29:12: note: declared here
grub_err_t grub_create_loader_cmdline (int argc, char *argv[], char *buf,

Pushed despite it had malformed tabs. Next time please use "git format-patch"
and "git send-email" to build and send the patches.

I normally do this (for my kernel work), but Leif asked me to reply
directly to this mail (since I am not set-up for Grub).

If I continue to contribute, I'll set something up more enduring.

Thank you for your help.

[Daniel Kiper]

The verifiers framework caused some breakage for non-x86
platforms that need fixing.
- Add an include guard for verify.h.
- Add a new file type for device tree blobs.
- Fix an error added to arm64/linux.c.
- Add file types to grub_file_open calls.
Note: only the common and arm64 fixes have actually been tested.
Fu Wei, Julien - can you guys have a look at fixing arm64/xen_boot.c?
I'm not 100% certain how multiboot should be handled.

Thanks a lot! Pushed!

By the way, have you cross compiled GRUB2 ARM stuff on x86? I would
like to add ARM builds to my test suite and I do not want to reinvent
the wheel.

Daniel

[Leif Lindholm]

Post by Daniel Kiper

The verifiers framework caused some breakage for non-x86
platforms that need fixing.
- Add an include guard for verify.h.
- Add a new file type for device tree blobs.
- Fix an error added to arm64/linux.c.
- Add file types to grub_file_open calls.
Note: only the common and arm64 fixes have actually been tested.
Fu Wei, Julien - can you guys have a look at fixing arm64/xen_boot.c?
I'm not 100% certain how multiboot should be handled.

Thanks a lot! Pushed!
By the way, have you cross compiled GRUB2 ARM stuff on x86? I would
like to add ARM builds to my test suite and I do not want to reinvent
the wheel.

Yeah, when I started in 2012, we didn't exactly have arm64 hardware :)

(And sometimes these days I'm still on x86.)

--target=aarch64-linux-gnu or --target=arm-linux-gnueabihf "just
works" with the caveat that 32-bit defaults to the U-Boot API version,
so needs --with-platform=efi for building the UEFI (including U-Boot)
flavour.

Several distros package cross compilation toolchains for both above
targets. Otherwise, latest (gcc 8) can be found at
https://developer.arm.com/open-source/gnu-toolchain/gnu-a/downloads
and less bleeding edge ones can be found under
https://releases.linaro.org/components/toolchain/binaries/

My smoke test is building an image with grub-mkstandalone and loading
a distro kernel/initrd in qemu with EDK2 images from
https://releases.linaro.org/reference-platform/enterprise/firmware/18.02/release/

qemu-system-arm -M virt -cpu cortex-a15 -m 1024M -bios QEMU_EFI.fd -hda fat:rw:fatdir/ -nographic -net none
qemu-system-aarch64 -M virt -cpu cortex-a57 -m 1024M -bios QEMU_EFI.fd -hda fat:rw:fatdir/ -nographic -net none

(I do that even when building/testing natively.)

/
Leif