Page MenuHomeFreeBSD

ARM: include machine/acle-compat.h in armreg.h if __ARM_ARCH is not defined
ClosedPublic

Authored by sgalabov on Nov 7 2016, 9:50 AM.

Details

Summary

In cases where ARM_ARCH is not defined, include machine/acle-compat.h in armreg.h so that compilation doesn't fail due to an undefined ARM_ARCH macro.

One such case at the moment is the SHEEVAPLUG kernel (possibly other Marvell kirkwood/orion based systems would be the same).

This change, together with D8459, allow us to build a SHEEVAPLUG kernel that seems to be working fine from the limited testing so far.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

sgalabov updated this revision to Diff 22046.Nov 7 2016, 9:50 AM
sgalabov retitled this revision from to ARM: include machine/acle-compat.h in armreg.h if __ARM_ARCH is not defined.
sgalabov updated this object.
sgalabov edited the test plan for this revision. (Show Details)
sgalabov added a reviewer: ARM.
sgalabov set the repository for this revision to rS FreeBSD src repository.
sgalabov added a project: ARM.

When is it the case that __ARM_ARCH is undefined? machine/acle-compat.h is included from sys/cdefs.h so this should have already been included.

mmel added a subscriber: mmel.Nov 7 2016, 10:08 AM

Are you sure? Also for D8459? I'm able to compile actual SHEEVAPLUG kernel without any issue.

When is it the case that __ARM_ARCH is undefined? machine/acle-compat.h is included from sys/cdefs.h so this should have already been included.

When compiling a SHEEVAPLUG kernel with
make TARGET_ARCH=arm KERNCONF=SHEEVAPLUG buildkernel

compilation fails due to __ARM_ARCH not being defined within machine/armreg.h, which is included from several files, such as sys/arm/arm/cpufunc_asm_sheeva.S...
Seeing mmel@'s comment in the meantime - am I compiling things incorrectly? I am using current from git as of today.

In D8460#175937, @meloun-miracle-cz wrote:

Are you sure? Also for D8459? I'm able to compile actual SHEEVAPLUG kernel without any issue.

How do you compile and how recent is your snapshot of current? I am using today's current from github and compilation is failing... maybe I am not using the "right" way to build?

All files, which report an issue of __ARM_ARCH not being defined (and include machine/armreg.h):
sys/arm/arm/cpufunc_asm_sheeva.S
sys/arm/arm/fiq_subr.S
sys/arm/arm/setstack.s

mmel added a comment.Nov 7 2016, 10:21 AM

I just compile it again, with

make -j8 buildkernel TARGET=arm TARGETARCH=arm KERNCONF=SHEEVAPLUG

...

>>> Kernel build for SHEEVAPLUG completed on Mon Nov 7 11:17:27 CET 2016

I'm not sure if TARGETARCH=arm is necessary, but I have 'setenv TARGET_ARCH armv6' in my login script.

Hmm, this is weird, it still fails the same way for me, although I am using the default gcc:
gcc version 4.2.1 20070831 patched [FreeBSD]

mmel accepted this revision.Nov 7 2016, 10:51 AM
mmel added a reviewer: mmel.

It's clear now. clang (and and any newer gcc) defines symbols from acle-compat.h by itself, implicitly. Unfortunately, outdated gcc from base not.

This revision is now accepted and ready to land.Nov 7 2016, 10:51 AM

Any other comments on this? If it's ok with everyone I'd like to commit it...

andrew requested changes to this revision.Nov 7 2016, 10:59 AM
andrew added a reviewer: andrew.

I'm about to commit the correct fix of including armreg.h after asm.h.

This revision now requires changes to proceed.Nov 7 2016, 10:59 AM

Can you try with rS308408. It might pay to add an error if this file is included but _MACHINE_ASM_H_ is undefined.

Can you try with rS308408. It might pay to add an error if this file is included but _MACHINE_ASM_H_ is undefined.

Even better would be _SYS_CDEFS_H_.

sgalabov updated this revision to Diff 22049.Nov 7 2016, 11:14 AM
sgalabov edited edge metadata.

Can you try with rS308408. It might pay to add an error if this file is included but _MACHINE_ASM_H_ is undefined.

Even better would be _SYS_CDEFS_H_.

This works now. I've updated the diff to test whether _SYS_CDEFS_H_ is defined within armreg.h and, if not, throw an #error.

andrew accepted this revision.Nov 7 2016, 11:30 AM
andrew edited edge metadata.
This revision is now accepted and ready to land.Nov 7 2016, 11:30 AM
This revision was automatically updated to reflect the committed changes.