[PATCH 0/5] Fix non-x86 builds after verifiers framework merged

[PATCH 0/5] Fix non-x86 builds after verifiers framework merged

[Leif Lindholm]

The verifiers framework caused some breakage for non-x86
platforms that need fixing.

Basically, this boils down to:
- 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.

Cc: Fu Wei <fu.wei@linaro.org>
Cc: Julien Grall <julien.grall@arm.com>
Reported-by: Alexander Graf <agraf@suse.de>

Leif Lindholm (5):
  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

[PATCH 1/5] grub/verify.h: add include guard

[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 <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 */
-- 
2.11.0

[PATCH 2/5] file.h: add device tree file type

[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 <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.  */
-- 
2.11.0

[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 <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;
 
-- 
2.11.0

[PATCH 4/5] arm64/efi: fix breakage caused by verifiers

[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 <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;
 
-- 
2.11.0

[PATCH 5/5] arm-uboot, ia64, sparc64: fix up grub_file_open calls

[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 <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;
 
-- 
2.11.0

Re: [PATCH 0/5] Fix non-x86 builds after verifiers framework merged

[Fu Wei Fu]

Hi Leif,

Great thanks for your information, will do ASAP when I am back home.
On Thu, 15 Nov 2018 at 03:29, Leif Lindholm <leif.lindholm@linaro.org> wrote:
>
> The verifiers framework caused some breakage for non-x86
> platforms that need fixing.
>
> Basically, this boils down to:
> - 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.
>
> Cc: Fu Wei <fu.wei@linaro.org>
> Cc: Julien Grall <julien.grall@arm.com>
> Reported-by: Alexander Graf <agraf@suse.de>
>
> Leif Lindholm (5):
>   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
>


-- 
Best regards,

Fu Wei
Software Engineer
Red Hat

Re: [PATCH 0/5] Fix non-x86 builds after verifiers framework merged

[Daniel Kiper]

On Wed, Nov 14, 2018 at 07:29:14PM +0000, Leif Lindholm wrote:
> The verifiers framework caused some breakage for non-x86
> platforms that need fixing.
>
> Basically, this boils down to:
> - 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.
>
> Cc: Fu Wei <fu.wei@linaro.org>
> Cc: Julien Grall <julien.grall@arm.com>
> Reported-by: Alexander Graf <agraf@suse.de>

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

Re: [PATCH 0/5] Fix non-x86 builds after verifiers framework merged

[Daniel Kiper]

On Fri, Nov 16, 2018 at 10:31:08AM +0800, Fu Wei Fu wrote:
> 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

Re: [PATCH 0/5] Fix non-x86 builds after verifiers framework merged

[Leif Lindholm]

On Fri, Nov 16, 2018 at 03:05:22PM +0100, Daniel Kiper wrote:
> On Wed, Nov 14, 2018 at 07:29:14PM +0000, Leif Lindholm wrote:
> > The verifiers framework caused some breakage for non-x86
> > platforms that need fixing.
> >
> > Basically, this boils down to:
> > - 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.
> >
> > Cc: Fu Wei <fu.wei@linaro.org>
> > Cc: Julien Grall <julien.grall@arm.com>
> > Reported-by: Alexander Graf <agraf@suse.de>
> 
> 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

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

[Lee Jones]

From: Lee Jones <lee.jones@linaro.org>

arm64/xen: Fix too few arguments to function ‘grub_create_loader_cmdline’
    
Without this fix, building xen_boot.c omits:
    
loader/arm64/xen_boot.c: In function ‘xen_boot_binary_load’:
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,
       ^~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from loader/arm64/xen_boot.c:36:0:
../include/grub/lib/cmdline.h:29:12: note: declared here
 grub_err_t grub_create_loader_cmdline (int argc, char *argv[], char *buf,

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

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",
                    "Xen_boot cmdline @ %p %s, size: %d\n",
                    binary->cmdline, binary->cmdline, binary->cmdline_size);

arm64/xen: Fix implicit declaration of function ‘grub_file_filter_disable_compression

[Lee Jones]

From: Lee Jones <lee.jones@linaro.org>

arm64/xen: Fix implicit declaration of function ‘grub_file_filter_disable_compression
    
Without this fix, building xen_boot.c omits:
    
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 <lee.jones@linaro.org>

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

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

[Lee Jones]

From: Lee Jones <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 <lee.jones@linaro.org>

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);
   if (!file)
     goto fail;

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

[Julien Grall]

Hi,

On 20/11/2018 10:46, Lee Jones wrote:
> From: Lee Jones <lee.jones@linaro.org>
> 
> 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,

>      
> Without this fix, building xen_boot.c omits:
>      
> 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 <lee.jones@linaro.org>
> 
> 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

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

[Julien Grall]

Hi Lee,

On 20/11/2018 10:48, Lee Jones wrote:
> From: Lee Jones <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 <lee.jones@linaro.org>
> 
> 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,

-- 
Julien Grall

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

[Julien Grall]

On 20/11/2018 10:45, Lee Jones wrote:
> From: Lee Jones <lee.jones@linaro.org>
> 
> arm64/xen: Fix too few arguments to function ‘grub_create_loader_cmdline’
>      
> Without this fix, building xen_boot.c omits:
>      
> loader/arm64/xen_boot.c: In function ‘xen_boot_binary_load’:
> 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,
>         ^~~~~~~~~~~~~~~~~~~~~~~~~~
> In file included from loader/arm64/xen_boot.c:36:0:
> ../include/grub/lib/cmdline.h:29:12: note: declared here
>   grub_err_t grub_create_loader_cmdline (int argc, char *argv[], char *buf,
> 
> Signed-off-by: Lee Jones <lee.jones@linaro.org>

Reviewed-by: Julien Grall <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",
>                      "Xen_boot cmdline @ %p %s, size: %d\n",
>                      binary->cmdline, binary->cmdline, binary->cmdline_size);
> 

-- 
Julien Grall

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

[Lee Jones]

On Tue, 20 Nov 2018, Julien Grall wrote:

> Hi Lee,
> 
> On 20/11/2018 10:48, Lee Jones wrote:
> > From: Lee Jones <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 <lee.jones@linaro.org>
> > 
> > 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.

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

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

[Lee Jones]

On Tue, 20 Nov 2018, Julien Grall wrote:
> On 20/11/2018 10:46, Lee Jones wrote:
> > From: Lee Jones <lee.jones@linaro.org>
> > 
> > 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.  On closer inspection, I do see that
argv/argc still needs to be shifted though.  I will fix that.

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

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

arm64/xen: Remove duplicate check for arguments

[Lee Jones]

From: Lee Jones <lee.jones@linaro.org>

arm64/xen: Remove duplicate check for arguments
    
Signed-off-by: Lee Jones <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,

[PATCH v2] arm64/xen: Fix implicit declaration of function ‘grub_file_filter_disable_compression

[Lee Jones]

From: Lee Jones <lee.jones@linaro.org>

arm64/xen: Fix implicit declaration of function ‘grub_file_filter_disable_compression
    
Without this fix, building xen_boot.c omits:
    
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 <lee.jones@linaro.org>

diff --git a/grub-core/loader/arm64/xen_boot.c b/grub-core/loader/arm64/xen_boot.c
index 33a855df4..117293fb9 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)
     {
@@ -403,7 +402,6 @@ grub_cmd_xen_module (grub_command_t cmd __attribute__((unused)),
     {
       argv++;
       argc--;
-      nounzip = 1;
     }
 
   if (!argc)
@@ -429,8 +427,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);

[PATCH v2] arm64/xen: Fix too few arguments to function ‘grub_file_open’

[Lee Jones]

From: Lee Jones <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 <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;

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

[Julien Grall]

Hi,

On 20/11/2018 11:22, Lee Jones wrote:
> On Tue, 20 Nov 2018, Julien Grall wrote:
>> On 20/11/2018 10:46, Lee Jones wrote:
>>> From: Lee Jones <lee.jones@linaro.org>
>>>
>>> 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.
> 
>> Note that the option added on Arm64 to keep the compatibility with x86
>> multiboot loading.
> 

-- 
Julien Grall

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

[Lee Jones]

On Tue, 20 Nov 2018, Julien Grall wrote:

> Hi,
> 
> On 20/11/2018 11:22, Lee Jones wrote:
> > On Tue, 20 Nov 2018, Julien Grall wrote:
> > > On 20/11/2018 10:46, Lee Jones wrote:
> > > > From: Lee Jones <lee.jones@linaro.org>
> > > > 
> > > > 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.

> Is there any reason to not use it for Arm?

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH v3 (part 1)] arm64/xen: Fix implicit declaration of function ‘grub_file_filter_disable_compression'

[Lee Jones]

From: Lee Jones <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 <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;

[PATCH v3 (part 2)] generic/blocklist: Fix implicit declaration of function ‘grub_file_filter_disable_compression'

[Lee Jones]

From: Lee Jones <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 <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);

Re: arm64/xen: Remove duplicate check for arguments

[Vladimir 'phcoder' Serbinenko]

[-- Attachment #1: Type: text/plain, Size: 949 bytes --]

On Tue, 20 Nov 2018, 14:40 Lee Jones, <lee.jones@linaro.org> wrote:

> From: Lee Jones <lee.jones@linaro.org>
>
> arm64/xen: Remove duplicate check for arguments
>
It's not duplicate because of argc-- right before it

>
> Signed-off-by: Lee Jones <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,
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>

[-- Attachment #2: Type: text/html, Size: 1734 bytes --]

Re: [PATCH v3 (part 2)] generic/blocklist: Fix implicit declaration of function ‘grub_file_filter_disable_compression'

[Vladimir 'phcoder' Serbinenko]

[-- Attachment #1: Type: text/plain, Size: 1510 bytes --]

LGTM

On Tue, 20 Nov 2018, 14:40 Lee Jones, <lee.jones@linaro.org> wrote:

> From: Lee Jones <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 <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);
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>

[-- Attachment #2: Type: text/html, Size: 2136 bytes --]

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

[Daniel Kiper]

On Tue, Nov 20, 2018 at 10:45:04AM +0000, Lee Jones wrote:
> From: Lee Jones <lee.jones@linaro.org>
>
> arm64/xen: Fix too few arguments to function ‘grub_create_loader_cmdline’
>
> Without this fix, building xen_boot.c omits:
>
> loader/arm64/xen_boot.c: In function ‘xen_boot_binary_load’:
> 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,
>        ^~~~~~~~~~~~~~~~~~~~~~~~~~
> In file included from loader/arm64/xen_boot.c:36:0:
> ../include/grub/lib/cmdline.h:29:12: note: declared here
>  grub_err_t grub_create_loader_cmdline (int argc, char *argv[], char *buf,
>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>

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

Daniel

Re: [PATCH v3 (part 2)] generic/blocklist: Fix implicit declaration of function ‘grub_file_filter_disable_compression'

[Daniel Kiper]

On Tue, Nov 20, 2018 at 12:37:01PM +0000, Lee Jones wrote:
> From: Lee Jones <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 <lee.jones@linaro.org>

Pushed with two minor changes. Thanks a lot!

Daniel

Re: [PATCH v2] arm64/xen: Fix too few arguments to function ‘grub_file_open’

[Daniel Kiper]

On Tue, Nov 20, 2018 at 11:39:42AM +0000, Lee Jones wrote:
> From: Lee Jones <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 <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);

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

Re: [PATCH v3 (part 1)] arm64/xen: Fix implicit declaration of function ‘grub_file_filter_disable_compression'

[Daniel Kiper]

On Tue, Nov 20, 2018 at 12:35:39PM +0000, Lee Jones wrote:
> From: Lee Jones <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 <lee.jones@linaro.org>

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

Daniel

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

[Lee Jones]

On Wed, 21 Nov 2018, Daniel Kiper wrote:

> On Tue, Nov 20, 2018 at 10:45:04AM +0000, Lee Jones wrote:
> > From: Lee Jones <lee.jones@linaro.org>
> >
> > arm64/xen: Fix too few arguments to function ‘grub_create_loader_cmdline’
> >
> > Without this fix, building xen_boot.c omits:
> >
> > loader/arm64/xen_boot.c: In function ‘xen_boot_binary_load’:
> > 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,
> >        ^~~~~~~~~~~~~~~~~~~~~~~~~~
> > In file included from loader/arm64/xen_boot.c:36:0:
> > ../include/grub/lib/cmdline.h:29:12: note: declared here
> >  grub_err_t grub_create_loader_cmdline (int argc, char *argv[], char *buf,
> >
> > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> 
> 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.

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog