Re: [PATCH 2/4] arm64/efi: move EFI_PAGE definitions to efi/memory.h

[Daniel Kiper]

On Tue, Sep 05, 2017 at 09:41:12PM +0100, Leif Lindholm wrote:
> The EFI page definitions and macros are generic and should not be confined
> to arm64 headers - so move to efi/memory.h.

I am OK with this.

> Also add EFI_PAGE_SIZE macro.

However, this should be added in 3rd patch.

> Update loader sources to reflect new header location.
>
> Signed-off-by: Leif Lindholm <address@hidden>
> ---
>  grub-core/loader/arm64/linux.c    | 1 +
>  grub-core/loader/arm64/xen_boot.c | 1 +
>  grub-core/loader/efi/fdt.c        | 1 +
>  include/grub/efi/fdtload.h        | 3 ---
>  include/grub/efi/memory.h         | 7 +++++++
>  5 files changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/grub-core/loader/arm64/linux.c b/grub-core/loader/arm64/linux.c
> index 746edd104..ca01a2349 100644
> --- a/grub-core/loader/arm64/linux.c
> +++ b/grub-core/loader/arm64/linux.c
> @@ -28,6 +28,7 @@
>  #include <grub/cpu/linux.h>
>  #include <grub/efi/efi.h>
>  #include <grub/efi/fdtload.h>
> +#include <grub/efi/memory.h>
>  #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 c95d6c5a8..67ada136b 100644
> --- a/grub-core/loader/arm64/xen_boot.c
> +++ b/grub-core/loader/arm64/xen_boot.c
> @@ -30,6 +30,7 @@
>  #include <grub/cpu/linux.h>
>  #include <grub/efi/efi.h>
>  #include <grub/efi/fdtload.h>
> +#include <grub/efi/memory.h>
>  #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/efi/fdt.c b/grub-core/loader/efi/fdt.c
> index 17212c38d..c0c6800f7 100644
> --- a/grub-core/loader/efi/fdt.c
> +++ b/grub-core/loader/efi/fdt.c
> @@ -24,6 +24,7 @@
>  #include <grub/file.h>
>  #include <grub/efi/efi.h>
>  #include <grub/efi/fdtload.h>
> +#include <grub/efi/memory.h>
>
>  static void *loaded_fdt;
>  static void *fdt;
> diff --git a/include/grub/efi/fdtload.h b/include/grub/efi/fdtload.h
> index 7b9ddba91..713c9424d 100644
> --- a/include/grub/efi/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..08fe62277 100644
> --- a/include/grub/efi/memory.h
> +++ b/include/grub/efi/memory.h
> @@ -22,6 +22,13 @@
>  #include <grub/err.h>
>  #include <grub/types.h>
>
> +/* The term "page" in UEFI refers only to a 4 KiB-aligned 4 KiB size region 
> of
> +   memory. It is not concerned with underlying translation management 
> concepts,
> +   but only used as the granule for memory allocations. */
> +#define GRUB_EFI_PAGE_SHIFT             12
> +#define GRUB_EFI_PAGE_SIZE              (1 << GRUB_EFI_PAGE_SHIFT)
> +#define GRUB_EFI_BYTES_TO_PAGES(bytes)  (((bytes) + 0xfff) >> 
> GRUB_EFI_PAGE_SHIFT)

If you add GRUB_EFI_PAGE_SIZE why not change GRUB_EFI_BYTES_TO_PAGES() macro to

#define GRUB_EFI_BYTES_TO_PAGES(bytes) ALIGN_UP (bytes, GRUB_EFI_PAGE_SIZE)?

Of course in separate patch.

Daniel