Skip to contentSkip to conversation

ARM: assembler: Add uniform assembler framework

RFC PATCH New Dave Martin 3 participants 11 messages Feb 28, 2012 – Mar 2, 2012 Arm (32-bit)

lore
Share & tools
lore
#0 lore raw

cover ARM: assembler: Add uniform assembler framework

Currently, we have two substantially different dialects of assembly language in arch/arm/ -- assmbler for .S files (with macros and definitions from <asm/assembler.h>) and inline asm in .c files (with macros and definitions from a variety of other places).

There is often a need to define macros to work around missing or broken assembler features, abstract architectural features and enable various magical build-time transformations. Unfortunately, such macros often have to be implemented twice, since macros that work for inline asm often don't work (or are sometimes not easily implementable) for .S files, and vice-versa.

The first patch in this series makes <asm/assembler.h> a viable place for common declarations to be passed to the assembler, and also paves the way for splitting out extra declarations which are not relevant everywhere into separate headers.

The second patch demonstrated how all this might be used in practice (and is also likely to be needed by the kvm and Xen guys, based on previous discussion).

To achieve all this, a script arch/arm/tools/asm-header.pl is now run by the archprepare target, to preprocess arch/arm/include/asm/*.h.asm. In these special headers, blocks of native assembler declarations are transformed as shown below.

Macros defined in such blocks get transformed into something that is includable in .c or .S files, with the desired effect:

arch/arm/include/asm/some_header.h.asm:

ASM(
stuff
for
the
assembler
)

--- becomes --->

include/generated/asm-some_header.h:

#ifdef __ASSEMBLY__
stuff
for
the
assembler
#else
asm("stuff");
asm("for");
asm("the");
asm("assembler");
#endif

Of course, there may be other, nicer ways of achieving all this...

Comments appreciated.

Cheers
---Dave

Dave Martin (2):
ARM: assembler: Add uniform assembler framework
ARM: virt: Add assembler helpers for the Virtualization Extensions

 arch/arm/Makefile                    |    8 +-
 arch/arm/include/asm/arch-virt.h     |    1 +
 arch/arm/include/asm/arch-virt.h.asm |  104 +++++++++++
 arch/arm/include/asm/assembler.h     |  322 +---------------------------------
 arch/arm/include/asm/assembler.h.asm |  323 ++++++++++++++++++++++++++++++++++
 arch/arm/include/asm/opcodes.h       |   80 +--------
 arch/arm/include/asm/opcodes.h.asm   |  191 ++++++++++++++++++++
 arch/arm/tools/Makefile              |    4 +
 arch/arm/tools/asm-header.pl         |   76 ++++++++
 9 files changed, 708 insertions(+), 401 deletions(-)
 create mode 100644 arch/arm/include/asm/arch-virt.h
 create mode 100644 arch/arm/include/asm/arch-virt.h.asm
 create mode 100644 arch/arm/include/asm/assembler.h.asm
 create mode 100644 arch/arm/include/asm/opcodes.h.asm
 create mode 100644 arch/arm/tools/asm-header.pl

--
1.7.4.1

#1 lore raw

RFC PATCH 1/2 ARM: assembler: Add uniform assembler framework

This patch creates a framework for adding common assembler macros and declarations usable identically by .S files and inline asm.

A preprocessing rule converts special uniform assembler headers arch/arm/include/*.h.asm into generated headers include/generates/asm-*.h. These headers can be included directly by .c or .S files to get the common macros.

A simple script arch/arm/tools/asm-header.pl converts marked blocks of assembler declarations appropriately for inclusion in both types of file. These blocks can be intermingled with ordinary C preprocessing directives (and even C content, if that content is protected with #ifndef __ASSEMBLY__ as is usual for any header in <asm/>. This makes it straightforward to write config-dependent code in the normal way.

<asm/assembler.h> is converted to the new framework by this patch, so it is now safe to include that header in .c files. There are no common declarations yet, but over time many features from this header could be replaced with common implementations if this looks worthwhile. The content is now in arch/arm/include/asm/assembler.h.asm (which is where maintenance should be done).

---
 arch/arm/Makefile                    |    8 +-
 arch/arm/include/asm/assembler.h     |  322 +---------------------------------
 arch/arm/include/asm/assembler.h.asm |  323 ++++++++++++++++++++++++++++++++++
 arch/arm/tools/Makefile              |    4 +
 arch/arm/tools/asm-header.pl         |   76 ++++++++
 5 files changed, 411 insertions(+), 322 deletions(-)
 create mode 100644 arch/arm/include/asm/assembler.h.asm
 create mode 100644 arch/arm/tools/asm-header.pl
arch/arm/Makefile +7 -1
index 1683bfb..9cfef30 100644
@@ -269,8 +269,14 @@ all:	$(KBUILD_IMAGE)
 
 boot := arch/arm/boot
 
+asm_headers := $(wildcard arch/arm/include/asm/*.h.asm)
+generated_asm_headers := \
+	$(patsubst arch/arm/include/asm/%.h.asm,include/generated/asm-%.h,$(asm_headers))
+
 archprepare:
-	$(Q)$(MAKE) $(build)=arch/arm/tools include/generated/mach-types.h
+	$(Q)$(MAKE) $(build)=arch/arm/tools \
+		include/generated/mach-types.h \
+		$(generated_asm_headers)
 
 # Convert bzImage to zImage
 bzImage: zImage
arch/arm/include/asm/assembler.h +1 -321

322 lines not on this page · raw message

arch/arm/include/asm/assembler.h.asm new file +323 -0

323 lines not on this page · raw message

arch/arm/tools/Makefile +4 -0
index 635cb18..f0140b6 100644
@@ -8,3 +8,7 @@ include/generated/mach-types.h: $(src)/gen-mach-types $(src)/mach-types
 	@echo '  Generating $@'
 	@mkdir -p $(dir $@)
 	$(Q)$(AWK) -f $^ > $@ || { rm -f $@; /bin/false; }
+
+include/generated/asm-%.h: arch/arm/include/asm/%.h.asm
+	@echo '  GEN     $@'
+	$(Q)$(PERL) $(src)/asm-header.pl $< >$@ || { $(RM) $@; /bin/false; }
arch/arm/tools/asm-header.pl new file +76 -0

76 lines not on this page · raw message

Signature · 1 line
1.7.4.1
#2 lore raw

RFC PATCH 2/2 ARM: virt: Add assembler helpers for the Virtualization Extensions

In reply to Dave Martin

For the benefit of hypervisor implementations such as kvm, Xen etc., this patch adds support for generating code to use the CPU virtualization extensions. This allows hypervisor calls to be inlined without changing the CFLAGS for the whole kernel, as well as supporting older binutils.

---
 arch/arm/include/asm/arch-virt.h     |    1 +
 arch/arm/include/asm/arch-virt.h.asm |  104 ++++++++++++++++++
 arch/arm/include/asm/opcodes.h       |   80 +--------------
 arch/arm/include/asm/opcodes.h.asm   |  191 ++++++++++++++++++++++++++++++++++
 4 files changed, 297 insertions(+), 79 deletions(-)
 create mode 100644 arch/arm/include/asm/arch-virt.h
 create mode 100644 arch/arm/include/asm/arch-virt.h.asm
 create mode 100644 arch/arm/include/asm/opcodes.h.asm
arch/arm/include/asm/arch-virt.h new file +1 -0
new file mode 100644
index 0000000..de077f8
@@ -0,0 +1 @@
+#include <generated/asm-arch-virt.h>
arch/arm/include/asm/arch-virt.h.asm new file +104 -0

104 lines not on this page · raw message

arch/arm/include/asm/opcodes.h +1 -79

80 lines not on this page · raw message

arch/arm/include/asm/opcodes.h.asm new file +191 -0

191 lines not on this page · raw message

Signature · 1 line
1.7.4.1
#3 lore raw

cover ARM: assembler: Add uniform assembler framework

In reply to Dave Martin
Quoted 10 lines · Dave Martin

#ifdef __ASSEMBLY__
stuff
for
the
assembler
#else
asm("stuff");
asm("for");
asm("the");
asm("assembler");

That's invalid - nothing guarantees that the compiler won't place anything in between these asm statements in the output assembly file.

#4 lore raw

cover ARM: assembler: Add uniform assembler framework

Quoted 12 lines · Russell King - ARM Linux
Quoted 10 lines · Dave Martin

#ifdef __ASSEMBLY__
stuff
for
the
assembler
#else
asm("stuff");
asm("for");
asm("the");
asm("assembler");

Quoted 3 lines

That's invalid - nothing guarantees that the compiler won't place anything in between these asm statements in the output assembly file.

Within a function that would be certainly true -- at the top-level, I'm less sure. Since there is no sequential code at the top level, only declarations, it's hard to see why the compiler would ever consider pasting something in the middle of that block or reordering it. Without constraints, the compiler simply has no idea what's there (such asms are implicitly "volatile", though that could be added for clarity). You're right that all this may amount to less than a guarantee, though...

One obvious improvement is to emit a single big asm statement for each block. There's still an assumption that the compiler will not reorder stuff around that block -- unified.h actually already relies on that prepoerty in order to work (though if we can find a better way, we could migrate some of the unified.h stuff to use it).

Another option is to .include a separate file.

This is possible, but I hadn't figured out how to autogenerate the correct dependencies if things are one that way. Alternatively, the .included file can be the header file itself, but this relies on the fact that C preprocessor look like line comments to the assembler, so it's rather painful.

If there is no other way to be certain that the included content really does appear at the start of the compilation unit, it would be necessary to wrap the assembler in a script which explicitly pastes that extra stuff at the start -- but this doesn't feel like a desirable option, since it's hard to avoid extra complexity and overheads when doing this.

Any thoughts?

Cheers
---Dave

#5 lore raw

cover ARM: assembler: Add uniform assembler framework

Quoted 17 lines · Dave Martin
Quoted 12 lines · Russell King - ARM Linux
Quoted 10 lines · Dave Martin

#ifdef __ASSEMBLY__
stuff
for
the
assembler
#else
asm("stuff");
asm("for");
asm("the");
asm("assembler");

Quoted 3 lines

That's invalid - nothing guarantees that the compiler won't place anything in between these asm statements in the output assembly file.

Quoted 8 lines

Within a function that would be certainly true -- at the top-level, I'm less sure. Since there is no sequential code at the top level, only declarations, it's hard to see why the compiler would ever consider pasting something in the middle of that block or reordering it. Without constraints, the compiler simply has no idea what's there (such asms are implicitly "volatile", though that could be added for clarity). You're right that all this may amount to less than a guarantee, though...

A quick follow up on this -- having chatted to tools guys, it sounds like you're quite right on this: it is indeed unsafe, without -fno-toplevel-reorder (which would impair some optimisations)

Of course, this also means that unified.h is unsafe for the same reasons -- there is no guarantee that the big asm block containing all the IT macro definitions will precede compiler-generated code in the input to the assembler (and, presumably, inline asms embedded in it) in the compiler output...

Cheers
---Dave

#6 lore raw

cover ARM: assembler: Add uniform assembler framework

In reply to Russell King - ARM Linux
Quoted 12 lines · Russell King - ARM Linux
Quoted 10 lines · Dave Martin

#ifdef __ASSEMBLY__
stuff
for
the
assembler
#else
asm("stuff");
asm("for");
asm("the");
asm("assembler");

Quoted 3 lines

That's invalid - nothing guarantees that the compiler won't place anything in between these asm statements in the output assembly file.

Are you still interested in this series?

I have a workaround for the above now, but it's not pretty -- I seem to have opened a can of worms here.

We would need something like

#define EXTENDED_ASM(stuff) asm ( ".include \"macros.inc\"\n\t" stuff ) #define EXTENDED_VASM(stuff) asm volatile ( ".include \"macros.inc\"\n\t" stuff )

...where macros.inc includes some logic to protect against multiple inclusion.

This would avoid problems caused by reordering done by the compiler.

I think this can work, but it may be a step too far...

The above could help fix the incorrect way that asm/unified.h defines the it* macros (though admittedly that does not seem to have caused problems for anyone so far...)

Cheers
---Dave

#7 lore raw

cover ARM: assembler: Add uniform assembler framework

Quoted 11 lines · Dave Martin

Are you still interested in this series?

I have a workaround for the above now, but it's not pretty -- I seem to have opened a can of worms here.

We would need something like

#define EXTENDED_ASM(stuff) asm ( ".include \"macros.inc\"\n\t" stuff ) #define EXTENDED_VASM(stuff) asm volatile ( ".include \"macros.inc\"\n\t" stuff )

Quoted 6 lines

...where macros.inc includes some logic to protect against multiple inclusion.

This would avoid problems caused by reordering done by the compiler.

I think this can work, but it may be a step too far...

I'm fairly tempted to say: "if it ain't broke, don't fix". AFAIK no one has reported problems here, so lets not hastily try to fix something that will error out if it does go wrong.

#8 lore raw

cover ARM: assembler: Add uniform assembler framework

Quoted 15 lines · Russell King - ARM Linux
Quoted 14 lines · Dave Martin

Are you still interested in this series?

I have a workaround for the above now, but it's not pretty -- I seem to have opened a can of worms here.

We would need something like

#define EXTENDED_ASM(stuff) asm ( ".include \"macros.inc\"\n\t" stuff ) #define EXTENDED_VASM(stuff) asm volatile ( ".include \"macros.inc\"\n\t" stuff )

...where macros.inc includes some logic to protect against multiple inclusion.

Quoted 7 lines
Quoted 3 lines

This would avoid problems caused by reordering done by the compiler.

I think this can work, but it may be a step too far...

I'm fairly tempted to say: "if it ain't broke, don't fix". AFAIK no one has reported problems here, so lets not hastily try to fix something that will error out if it does go wrong.

OK -- I'll shelve this series for now and we'll see if we can come up with something more localised for the kvm/Xen case.

unified.h may work for now, but I don't want to add anything more along those lines for unless it's definitely correct to do so...

Cheers
---Dave

#9 lore raw

cover ARM: assembler: Add uniform assembler framework

In reply to Russell King - ARM Linux
Quoted 15 lines · Russell King - ARM Linux
Quoted 14 lines · Dave Martin

Are you still interested in this series?

I have a workaround for the above now, but it's not pretty -- I seem to have opened a can of worms here.

We would need something like

#define EXTENDED_ASM(stuff) asm ( ".include \"macros.inc\"\n\t" stuff ) #define EXTENDED_VASM(stuff) asm volatile ( ".include \"macros.inc\"\n\t" stuff )

...where macros.inc includes some logic to protect against multiple inclusion.

Quoted 7 lines
Quoted 3 lines

This would avoid problems caused by reordering done by the compiler.

I think this can work, but it may be a step too far...

I'm fairly tempted to say: "if it ain't broke, don't fix". AFAIK no one has reported problems here, so lets not hastily try to fix something that will error out if it does go wrong.

OK, exactly what I said.

(I should have read further down the thread before replying)

Nicolas

#10 lore raw

cover ARM: assembler: Add uniform assembler framework

In reply to Dave Martin
Quoted 12 lines · Dave Martin

I have a workaround for the above now, but it's not pretty -- I seem to have opened a can of worms here.

We would need something like

#define EXTENDED_ASM(stuff) asm ( ".include \"macros.inc\"\n\t" stuff ) #define EXTENDED_VASM(stuff) asm volatile ( ".include \"macros.inc\"\n\t" stuff )

...where macros.inc includes some logic to protect against multiple inclusion.

Quoted 8 lines

This would avoid problems caused by reordering done by the compiler.

I think this can work, but it may be a step too far...

The above could help fix the incorrect way that asm/unified.h defines the it* macros (though admittedly that does not seem to have caused problems for anyone so far...)

"If it aint broken dont fix it."

Yes, we might be making assumptions that are not guaranteed by the tools here, but as long as you make sure that some assumption going wrong won't go unnoticed and produce a broken binary then it is probably fine.

I did like your initial approach better, even if there is a risk that future tools might break it. At least there was very little chances for the build to succeed if the global asm macros are not defined early enough.

Nicolas