ia64: build fix in cache.h

Message ID 20190604185142.5316-1-leif.lindholm@linaro.org
State Superseded
Headers show
Series
  • ia64: build fix in cache.h
Related show

Commit Message

Leif Lindholm June 4, 2019, 6:51 p.m.
Add ia64 to the architectures excluding a declaration for
grub_arch_sync_dma_caches.

IA64 does not include any of the source files that require the function,
but was overlooked for d8901e3ba115 ("cache: Fix compilation for ppc,
sparc and arm64").

Add it to the list of excluding architectures in order to not get
missing symbol errors when running grub-mkimage.

Reported-by: Alexander Graf <agraf@csgraf.de>
Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>

---
 include/grub/cache.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

-- 
2.11.0


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

Comments

John Paul Adrian Glaubitz June 4, 2019, 10:11 p.m. | #1
On 6/4/19 8:51 PM, Leif Lindholm wrote:
> Add ia64 to the architectures excluding a declaration for

> grub_arch_sync_dma_caches.

> 

> IA64 does not include any of the source files that require the function,

> but was overlooked for d8901e3ba115 ("cache: Fix compilation for ppc,

> sparc and arm64").

> 

> Add it to the list of excluding architectures in order to not get

> missing symbol errors when running grub-mkimage.


Ah, now I get what the actual problem is. I'll test that tomorrow on real hardware. 

>  #ifndef GRUB_MACHINE_EMU

> -#if defined (__aarch64__) || defined (__powerpc__) || defined (__sparc__)

> +#if defined (__aarch64__) || defined (__powerpc__) || defined (__sparc__) || \

> +    defined (__ia64__)


Nitpick, but could you actually put the arch names in alphabetical order?

It looks weird adding ia64 at the end when everything is already sorted.

Adrian

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - glaubitz@debian.org
`. `'   Freie Universitaet Berlin - glaubitz@physik.fu-berlin.de
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Leif Lindholm June 4, 2019, 10:33 p.m. | #2
On Wed, Jun 05, 2019 at 12:11:00AM +0200, John Paul Adrian Glaubitz wrote:
> On 6/4/19 8:51 PM, Leif Lindholm wrote:

> > Add ia64 to the architectures excluding a declaration for

> > grub_arch_sync_dma_caches.

> > 

> > IA64 does not include any of the source files that require the function,

> > but was overlooked for d8901e3ba115 ("cache: Fix compilation for ppc,

> > sparc and arm64").

> > 

> > Add it to the list of excluding architectures in order to not get

> > missing symbol errors when running grub-mkimage.

> 

> Ah, now I get what the actual problem is. I'll test that tomorrow on real hardware. 

> 

> >  #ifndef GRUB_MACHINE_EMU

> > -#if defined (__aarch64__) || defined (__powerpc__) || defined (__sparc__)

> > +#if defined (__aarch64__) || defined (__powerpc__) || defined (__sparc__) || \

> > +    defined (__ia64__)

> 

> Nitpick, but could you actually put the arch names in alphabetical order?

> 

> It looks weird adding ia64 at the end when everything is already sorted.


Haha, you're worse than me - I love it :)

If you can confirm the end result actually works on hardware, I'm
happy to resubmit with macros sorted. (If not, I'm with Alex on
dropping the CI until it's actually testable.)

/
    Leif

> Adrian

> 

> -- 

>  .''`.  John Paul Adrian Glaubitz

> : :' :  Debian Developer - glaubitz@debian.org

> `. `'   Freie Universitaet Berlin - glaubitz@physik.fu-berlin.de

>   `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913


_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
John Paul Adrian Glaubitz June 5, 2019, 10:26 a.m. | #3
Hi!

On 6/5/19 12:33 AM, Leif Lindholm wrote:
> On Wed, Jun 05, 2019 at 12:11:00AM +0200, John Paul Adrian Glaubitz wrote:

>> On 6/4/19 8:51 PM, Leif Lindholm wrote:

>>>  #ifndef GRUB_MACHINE_EMU

>>> -#if defined (__aarch64__) || defined (__powerpc__) || defined (__sparc__)

>>> +#if defined (__aarch64__) || defined (__powerpc__) || defined (__sparc__) || \

>>> +    defined (__ia64__)

>>

>> Nitpick, but could you actually put the arch names in alphabetical order?

>>

>> It looks weird adding ia64 at the end when everything is already sorted.

> 

> Haha, you're worse than me - I love it :)


Yes, I'm a bit of an OCD person when it comes to sending patches.

> If you can confirm the end result actually works on hardware, I'm

> happy to resubmit with macros sorted. (If not, I'm with Alex on

> dropping the CI until it's actually testable.)

So, I have build-tested the patch on ia64 now. GRUB still builds fine with the
patch applied. So no regression this regard.

And here are the tests with grub-mkimage.

First, without the patch:

glaubitz@titanium:/srv/tmp/grub$ ./grub-mkimage --config=/boot/grub/grub.cfg --prefix="" -d grub-core -O ia64-efi -o ia64.efi
./grub-mkimage: error: undefined symbol grub_arch_sync_dma_caches.
glaubitz@titanium:/srv/tmp/grub$

And now with the patch:

glaubitz@titanium:/srv/tmp/grub$ ./grub-mkimage --config=/boot/grub/grub.cfg --prefix="" -d grub-core -O ia64-efi -o ia64.efi
glaubitz@titanium:/srv/tmp/grub$

So, I can confirm the patch does what was expected.

Tested-by: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>


Thanks,
Adrian

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - glaubitz@debian.org
`. `'   Freie Universitaet Berlin - glaubitz@physik.fu-berlin.de
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Daniel Kiper June 6, 2019, 8:20 a.m. | #4
On Wed, Jun 05, 2019 at 12:26:23PM +0200, John Paul Adrian Glaubitz wrote:
> Hi!

>

> On 6/5/19 12:33 AM, Leif Lindholm wrote:

> > On Wed, Jun 05, 2019 at 12:11:00AM +0200, John Paul Adrian Glaubitz wrote:

> >> On 6/4/19 8:51 PM, Leif Lindholm wrote:

> >>>  #ifndef GRUB_MACHINE_EMU

> >>> -#if defined (__aarch64__) || defined (__powerpc__) || defined (__sparc__)

> >>> +#if defined (__aarch64__) || defined (__powerpc__) || defined (__sparc__) || \

> >>> +    defined (__ia64__)

> >>

> >> Nitpick, but could you actually put the arch names in alphabetical order?

> >>

> >> It looks weird adding ia64 at the end when everything is already sorted.

> >

> > Haha, you're worse than me - I love it :)

>

> Yes, I'm a bit of an OCD person when it comes to sending patches.

>

> > If you can confirm the end result actually works on hardware, I'm

> > happy to resubmit with macros sorted. (If not, I'm with Alex on

> > dropping the CI until it's actually testable.)

> So, I have build-tested the patch on ia64 now. GRUB still builds fine with the

> patch applied. So no regression this regard.

>

> And here are the tests with grub-mkimage.

>

> First, without the patch:

>

> glaubitz@titanium:/srv/tmp/grub$ ./grub-mkimage --config=/boot/grub/grub.cfg --prefix="" -d grub-core -O ia64-efi -o ia64.efi

> ./grub-mkimage: error: undefined symbol grub_arch_sync_dma_caches.

> glaubitz@titanium:/srv/tmp/grub$

>

> And now with the patch:

>

> glaubitz@titanium:/srv/tmp/grub$ ./grub-mkimage --config=/boot/grub/grub.cfg --prefix="" -d grub-core -O ia64-efi -o ia64.efi

> glaubitz@titanium:/srv/tmp/grub$

>

> So, I can confirm the patch does what was expected.

>

> Tested-by: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>


Leif, could you repost the patch as you promised? I would like to get
it and release soon.

Daniel

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

Patch

diff --git a/include/grub/cache.h b/include/grub/cache.h
index ccfa717e6..85819fe2d 100644
--- a/include/grub/cache.h
+++ b/include/grub/cache.h
@@ -34,7 +34,8 @@  void EXPORT_FUNC(grub_arch_sync_caches) (void *address, grub_size_t len);
 #endif
 
 #ifndef GRUB_MACHINE_EMU
-#if defined (__aarch64__) || defined (__powerpc__) || defined (__sparc__)
+#if defined (__aarch64__) || defined (__powerpc__) || defined (__sparc__) || \
+    defined (__ia64__)
 
 #elif defined (__i386__) || defined (__x86_64__)
 static inline void