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.
Tags
Referenced Files
Unknown Object (File)
Sun, Mar 24, 8:06 AM
Unknown Object (File)
Feb 2 2024, 8:22 AM
Unknown Object (File)
Dec 27 2023, 5:37 PM
Unknown Object (File)
Dec 27 2023, 5:10 PM
Unknown Object (File)
Dec 26 2023, 4:11 AM
Unknown Object (File)
Dec 25 2023, 4:14 AM
Unknown Object (File)
Dec 23 2023, 3:39 AM
Unknown Object (File)
Dec 4 2023, 5:39 PM
Subscribers

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 - subversion
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

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

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

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