| Message ID | CAF7UmSw0GfWDiLEwf2f6JnWrJ-Qpx3Rp1OZJ-DNUJx1VrerX3w@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
On Thu, May 11, 2017, 17:35 Leif Lindholm <leif.lindholm@linaro.org> wrote: > Commit 265292f ("arm_coreboot: Support DMA") breaks arm64 grub-mkimage > with: > /work/local/bin/grub-mkimage: error: undefined symbol > grub_arch_sync_dma_caches. > > This appears to be caused purely by the false symbol dependency > created by the non-x86 version being an EXPORT_FUNC, in order to be > usable by modules. > > A not very pretty but functional workaround is: > > diff --git a/include/grub/cache.h b/include/grub/cache.h > index 1c98ce2..cc4c833 100644 > --- a/include/grub/cache.h > +++ b/include/grub/cache.h > @@ -40,7 +40,7 @@ grub_arch_sync_dma_caches (volatile void *address > __attribute__ ((unused)), > grub_size_t len __attribute__ ((unused))) > { > } > -#else > +#elif !defined (__aarch64__) > void EXPORT_FUNC(grub_arch_sync_dma_caches) (volatile void *address, > grub_size_t len); > #endif > #endif > > Thoughts? > I don't think that functions in cache.h are used outside of kernel on arm64 currently. We can just remove cache.h from the list of exported headers > > / > Leif > > _______________________________________________ > Grub-devel mailing list > Grub-devel@gnu.org > https://lists.gnu.org/mailman/listinfo/grub-devel > _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Apologies for delay, got knocked out by a bad cold. On Thu, May 11, 2017 at 06:33:34PM +0000, Vladimir 'phcoder' Serbinenko wrote: > > Thoughts? > > > I don't think that functions in cache.h are used outside of kernel on arm64 > currently. We can just remove cache.h from the list of exported headers That's true. So, would something like the below be acceptable? / Leif _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-develdiff --git a/grub-core/Makefile.am b/grub-core/Makefile.am index 1045138..622f946 100644 --- a/grub-core/Makefile.am +++ b/grub-core/Makefile.am @@ -66,7 +66,9 @@ CLEANFILES += grub_script.yy.c grub_script.yy.h include $(srcdir)/Makefile.core.am +if !COND_arm64 KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/cache.h +endif KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/command.h KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/device.h KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/disk.h
On Wed, May 24, 2017 at 12:35:51PM +0100, Leif Lindholm wrote: > Apologies for delay, got knocked out by a bad cold. > > On Thu, May 11, 2017 at 06:33:34PM +0000, Vladimir 'phcoder' Serbinenko wrote: > > > Thoughts? > > > > > I don't think that functions in cache.h are used outside of kernel on arm64 > > currently. We can just remove cache.h from the list of exported headers > > That's true. So, would something like the below be acceptable? Or would a new AM_CONDITIONAL be preferable? Once the 32/64-bit ARM UEFI linux loaders are unified, the same change should probably be made for the 32-bit port, so there is a case for doing it that way. Maybe COND_efi? Although that could require a workaround for ia64. / Leif > diff --git a/grub-core/Makefile.am b/grub-core/Makefile.am > index 1045138..622f946 100644 > --- a/grub-core/Makefile.am > +++ b/grub-core/Makefile.am > @@ -66,7 +66,9 @@ CLEANFILES += grub_script.yy.c grub_script.yy.h > > include $(srcdir)/Makefile.core.am > > +if !COND_arm64 > KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/cache.h > +endif > KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/command.h > KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/device.h > KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/disk.h _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
LGTM On Wed, May 24, 2017, 13:36 Leif Lindholm <leif.lindholm@linaro.org> wrote: > Apologies for delay, got knocked out by a bad cold. > > On Thu, May 11, 2017 at 06:33:34PM +0000, Vladimir 'phcoder' Serbinenko > wrote: > > > Thoughts? > > > > > I don't think that functions in cache.h are used outside of kernel on > arm64 > > currently. We can just remove cache.h from the list of exported headers > > That's true. So, would something like the below be acceptable? > > / > Leif > > diff --git a/grub-core/Makefile.am b/grub-core/Makefile.am > index 1045138..622f946 100644 > --- a/grub-core/Makefile.am > +++ b/grub-core/Makefile.am > @@ -66,7 +66,9 @@ CLEANFILES += grub_script.yy.c grub_script.yy.h > > include $(srcdir)/Makefile.core.am > > +if !COND_arm64 > KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/cache.h > +endif > KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/command.h > KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/device.h > KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/disk.h > > _______________________________________________ > Grub-devel mailing list > Grub-devel@gnu.org > https://lists.gnu.org/mailman/listinfo/grub-devel > _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
On Tue, May 30, 2017 at 04:06:52PM +0000, Vladimir 'phcoder' Serbinenko wrote: > LGTM Thanks - so am I OK to push the following?: From bb9aa73e172ee284d9b41a0a4657154c355d6f63 Mon Sep 17 00:00:00 2001 From: Leif Lindholm <leif.lindholm@linaro.org> Date: Mon, 5 Jun 2017 10:47:35 +0100 Subject: [PATCH] arm64: don't export cache.h arm64 does not implement grub_arch_sync_dma_caches(), which causes an error when running grub-mkimage due to the missing exported symbol. Make the exporting of cache.h conditional on !COND_arm64. --- grub-core/Makefile.am | 2 ++ 1 file changed, 2 insertions(+) -- 2.1.4 _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-develdiff --git a/grub-core/Makefile.am b/grub-core/Makefile.am index 1045138..622f946 100644 --- a/grub-core/Makefile.am +++ b/grub-core/Makefile.am @@ -66,7 +66,9 @@ CLEANFILES += grub_script.yy.c grub_script.yy.h include $(srcdir)/Makefile.core.am +if !COND_arm64 KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/cache.h +endif KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/command.h KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/device.h KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/disk.h
diff --git a/include/grub/cache.h b/include/grub/cache.h index 1c98ce2..cc4c833 100644 --- a/include/grub/cache.h +++ b/include/grub/cache.h @@ -40,7 +40,7 @@ grub_arch_sync_dma_caches (volatile void *address __attribute__ ((unused)), grub_size_t len __attribute__ ((unused))) { } -#else +#elif !defined (__aarch64__) void EXPORT_FUNC(grub_arch_sync_dma_caches) (volatile void *address, grub_size_t len);
Commit 265292f ("arm_coreboot: Support DMA") breaks arm64 grub-mkimage with: /work/local/bin/grub-mkimage: error: undefined symbol grub_arch_sync_dma_caches. This appears to be caused purely by the false symbol dependency created by the non-x86 version being an EXPORT_FUNC, in order to be usable by modules. A not very pretty but functional workaround is: #endif #endif Thoughts? / Leif _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel