diff mbox series

[3/7] efi: move fdt helper library

Message ID 20170612145341.3351-4-leif.lindholm@linaro.org
State New
Headers show
Series efi: improved correctness, arm unification, and cleanup | expand

Commit Message

Leif Lindholm June 12, 2017, 2:53 p.m. UTC 
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.

Move the fdtload.h include file to grub/efi and move the EFI page size
definitions to grub/efi/memory.h. (These definitions refer strictly to
allocation operations, as opposed to translation granules.)
---
 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%)

-- 
2.11.0


_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Comments

Daniel Kiper July 27, 2017, 2:54 p.m. UTC | #1 
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

_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Leif Lindholm July 28, 2017, 6:08 p.m. UTC | #2 
On Thu, Jul 27, 2017 at 04:54:19PM +0200, Daniel Kiper wrote:
> 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...


Sure.

> > 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.


Sure.

> > ---

> >  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?


The shuffling is to fit alphabetically sorted when moving from
cpu/fdtload.h to efi/fdtload.h.

The addition is because the EFI_PAGE_SHIFT & co moves from
cpu/fdtload.h into 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 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?


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?


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


Will do.

/
    Leif


_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Daniel Kiper Aug. 1, 2017, 12:02 p.m. UTC | #3 
On Fri, Jul 28, 2017 at 07:08:42PM +0100, Leif Lindholm wrote:
> On Thu, Jul 27, 2017 at 04:54:19PM +0200, Daniel Kiper wrote:

> > 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...

>

> Sure.

>

> > > 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.

>

> Sure.

>

> > > ---

> > >  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?

>

> The shuffling is to fit alphabetically sorted when moving from

> cpu/fdtload.h to efi/fdtload.h.

>

> The addition is because the EFI_PAGE_SHIFT & co moves from

> cpu/fdtload.h into efi/memory.h.


OK, but say about this in commit message please.

Daniel

_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
diff mbox series

Patch

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>
 #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>
 #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>
 
 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)
+
 #define GRUB_MMAP_REGISTER_BY_FIRMWARE  1
 
 grub_err_t grub_machine_mmap_register (grub_uint64_t start, grub_uint64_t size,