Re: [PATCH 3/7] efi: move fdt helper library

[Daniel Kiper]

On Mon, Jun 12, 2017 at 03:53:37PM +0100, Leif Lindholm wrote:
> There is nothing ARM64 (or even ARM) specific about the efi fdt helper
> library, which is used for locating or overriding a firmware-provided
> devicetree in a UEFI system - so move it to loader/efi for reuse.

OK.

> Move the fdtload.h include file to grub/efi

This begs one patch...

> and move the EFI page size
> definitions to grub/efi/memory.h. (These definitions refer strictly to
> allocation operations, as opposed to translation granules.)

Great! This is what I requested earlier. However, please put this in separate 
patch.

> ---
>  grub-core/Makefile.core.def           | 2 +-
>  grub-core/loader/arm64/linux.c        | 3 ++-
>  grub-core/loader/arm64/xen_boot.c     | 3 ++-
>  grub-core/loader/{arm64 => efi}/fdt.c | 3 ++-
>  include/grub/{arm64 => efi}/fdtload.h | 3 ---
>  include/grub/efi/memory.h             | 3 +++
>  6 files changed, 10 insertions(+), 7 deletions(-)
>  rename grub-core/loader/{arm64 => efi}/fdt.c (98%)
>  rename include/grub/{arm64 => efi}/fdtload.h (89%)
>
> diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
> index 1d86bd22e..a65c27f7f 100644
> --- a/grub-core/Makefile.core.def
> +++ b/grub-core/Makefile.core.def
> @@ -1707,7 +1707,7 @@ module = {
>
>  module = {
>    name = fdt;
> -  arm64 = loader/arm64/fdt.c;
> +  arm64 = loader/efi/fdt.c;
>    common = lib/fdt.c;
>    enable = fdt;
>  };
> diff --git a/grub-core/loader/arm64/linux.c b/grub-core/loader/arm64/linux.c
> index 9519d2e4d..cac94d53d 100644
> --- a/grub-core/loader/arm64/linux.c
> +++ b/grub-core/loader/arm64/linux.c
> @@ -26,8 +26,9 @@
>  #include <grub/mm.h>
>  #include <grub/types.h>
>  #include <grub/cpu/linux.h>
> -#include <grub/cpu/fdtload.h>
>  #include <grub/efi/efi.h>
> +#include <grub/efi/fdtload.h>
> +#include <grub/efi/memory.h>

Why do you shuffle and add headers?

>  #include <grub/efi/pe32.h>
>  #include <grub/i18n.h>
>  #include <grub/lib/cmdline.h>
> diff --git a/grub-core/loader/arm64/xen_boot.c 
> b/grub-core/loader/arm64/xen_boot.c
> index 27ede46ca..d092a53ed 100644
> --- a/grub-core/loader/arm64/xen_boot.c
> +++ b/grub-core/loader/arm64/xen_boot.c
> @@ -27,9 +27,10 @@
>  #include <grub/misc.h>
>  #include <grub/mm.h>
>  #include <grub/types.h>
> -#include <grub/cpu/fdtload.h>
>  #include <grub/cpu/linux.h>
>  #include <grub/efi/efi.h>
> +#include <grub/efi/fdtload.h>
> +#include <grub/efi/memory.h>

Ditto?

>  #include <grub/efi/pe32.h>   /* required by struct xen_hypervisor_header */
>  #include <grub/i18n.h>
>  #include <grub/lib/cmdline.h>
> diff --git a/grub-core/loader/arm64/fdt.c b/grub-core/loader/efi/fdt.c
> similarity index 98%
> rename from grub-core/loader/arm64/fdt.c
> rename to grub-core/loader/efi/fdt.c
> index db49cf649..be369fd9d 100644
> --- a/grub-core/loader/arm64/fdt.c
> +++ b/grub-core/loader/efi/fdt.c
> @@ -18,12 +18,13 @@
>
>  #include <grub/fdt.h>
>  #include <grub/mm.h>
> -#include <grub/cpu/fdtload.h>
>  #include <grub/err.h>
>  #include <grub/dl.h>
>  #include <grub/command.h>
>  #include <grub/file.h>
>  #include <grub/efi/efi.h>
> +#include <grub/efi/fdtload.h>
> +#include <grub/machine/memory.h>

Ditto?

>  static void *loaded_fdt;
>  static void *fdt;
> diff --git a/include/grub/arm64/fdtload.h b/include/grub/efi/fdtload.h
> similarity index 89%
> rename from include/grub/arm64/fdtload.h
> rename to include/grub/efi/fdtload.h
> index 7b9ddba91..713c9424d 100644
> --- a/include/grub/arm64/fdtload.h
> +++ b/include/grub/efi/fdtload.h
> @@ -29,7 +29,4 @@ grub_fdt_unload (void);
>  grub_err_t
>  grub_fdt_install (void);
>
> -#define GRUB_EFI_PAGE_SHIFT  12
> -#define GRUB_EFI_BYTES_TO_PAGES(bytes)   (((bytes) + 0xfff) >> 
> GRUB_EFI_PAGE_SHIFT)
> -
>  #endif
> diff --git a/include/grub/efi/memory.h b/include/grub/efi/memory.h
> index 20526b146..0eb0b70b6 100644
> --- a/include/grub/efi/memory.h
> +++ b/include/grub/efi/memory.h
> @@ -22,6 +22,9 @@
>  #include <grub/err.h>
>  #include <grub/types.h>
>
> +#define GRUB_EFI_PAGE_SHIFT  12
> +#define GRUB_EFI_BYTES_TO_PAGES(bytes)   (((bytes) + 0xfff) >> 
> GRUB_EFI_PAGE_SHIFT)

If you move this then fix the aligment. I mean more or less this:

#define GRUB_EFI_PAGE_SHIFT              12
#define GRUB_EFI_BYTES_TO_PAGES(bytes)   (((bytes) + 0xfff) >> 
GRUB_EFI_PAGE_SHIFT)

#define GRUB_MMAP_REGISTER_BY_FIRMWARE   1

Daniel