sys/intr.h: Make it safe to include from assembler

[imp]

Summary

Sometimes we need defines from this file in assembler code. Today we do
the heavyweight approach of using genassym for that. However, they are
just #defines, so in the future we want to include sys/intr.h to pick up
the needed constants in exception.S.

Sponsored by: Netflix

[imp]

Another way to do this would be to move INTR_IRQ_INVALID and INTR_ROOT_IRQ into machine/intr.h and then include that later in the patch series.

[ehem_freebsd_m5p.com]

sys/arm/include/intr.h

50–52

Why expose NIRQ to assembly-language? Perhaps if a future commit needed it, but right now why not keep it out. If nothing else, this serves to document the value isn't touched by assembly-language and hinting going back isn't too difficult.

sys/arm64/include/intr.h

39–49

Similar to above, the only values touched by assembly-language are the INTR_ROOT_* values.

sys/riscv/include/intr.h

57

Unneeded. RISC-V doesn't use any of this in assembly, so why make the header assembly-safe?

sys/sys/intr.h

42

I'm against having sys/intr.h be assembly-safe. This is an architecture detail and shouldn't dictate what this common header looks like.

[ehem_freebsd_m5p.com]

Unless there is something I'm unaware of, INTR_IRQ_INVALID isn't ever used by assembly-language.

[jhb]

sys/arm/include/intr.h

42

It would be nice if we could be somewhat consistent. Currently LOCORE is the most prominent thing we test for (and in particular this is what we use in x86 arches) and the one documented as such in sys/conf/kern.pre.mk:

# XXX LOCORE means "don't declare C stuff" not "for locore.s".
ASM_CFLAGS= -x assembler-with-cpp -DLOCORE ${CFLAGS} ${ASM_CFLAGS.${.IMPSRC:T}}

Next is ASSEMBLER and last is __ASSEMBLER__.

50–52

In general if we are going to make a header assembly-safe, we should default to exposing all the assembly-safe things.

sys/riscv/include/intr.h

57

Presumably the goal is to make <machine/intr.h> universally safe across all architectures that define one?

[imp]

yea, the only question I have is if we move all the assembly values into machine/intr.h, duplicating one or two across architectures and include machine/intr.h for the assembler, or if I leave it as I have it inthe review. @jhb

sys/arm/include/intr.h

42

Somehow I had it in my head that ASSEMBLER was the preferred way, since toolchains define that now and LOCORE was legacy... I'm not entirely sure how I got there, but I'm happy to change it to LOCORE... There's a few places we test for both though (well, 2, both in elf_common.h)

sys/sys/elf_common.h:#if !defined(LOCORE) && !defined(ASSEMBLER)

should se change that?

50–52

yea, NIRQ is assembly safe

sys/riscv/include/intr.h

57

Yes.

[jhb]

In D47846#1090926, @imp wrote:

yea, the only question I have is if we move all the assembly values into machine/intr.h, duplicating one or two across architectures and include machine/intr.h for the assembler, or if I leave it as I have it inthe review. @jhb

I don't see a compelling reason to duplicate values across architectures. That would seem to defeat the point of the shared header file.

[jhb]

sys/arm/include/intr.h

42

It may be that we should adopt __ASSEMBLER__ if that is what modern toolchains always define, but we should do a principled adoption of that if so. For now I would stick with LOCORE and maybe we can do a global sweep to switch to __ASSEMBLER__ (and note it in style.9) as a separate change.

[ehem_freebsd_m5p.com]

In D47846#1090953, @jhb wrote:

I don't see a compelling reason to duplicate values across architectures. That would seem to defeat the point of the shared header file.

The values in question actually belong to the architecture. An architecture can arbitrarily replace them without problems. The issue is having all these associated values in 1 place makes keeping them synchronized (non-conflicting) easier. For another the issue is having a default which is overridden by 2 of 3 architectures doesn't make sense either.

sys/arm/include/intr.h

50–52

Certainly assembly-safe, but doesn't seem assembly-necessary. If there was a desire to convert back in the future, all of these would need checking and not exposing extra values would make this easier. x86/include/intr_machdep.h appears to no longer be used in assembly-language yet no action has been taken yet (I've got a patch/commit, but it is extremely low priority).

sys/riscv/include/intr.h

57

Shouldn't the goal be D47848? This is one way to achieve this, but there are other ways.

[andrew]

sys/arm/include/intr.h

42

I added __ASSEMBLER__ to elf_common.h as I wanted to include it from userspace asm where LOCORE may not be defined. I probably kept LOCORE in that file as I'm not sure if all compilers that we care about define it when building asm files.

[imp]

Use LOCORE