Skip to content

Commit 043fad9

Browse files
committed
[multi-arch-dev] doc: Add initial multi-arch-coding-guideline
Tracked-On: #8782 Signed-off-by: Yifan Liu <yifan1.liu@intel.com>
1 parent ffd8ee2 commit 043fad9

1 file changed

Lines changed: 334 additions & 0 deletions

File tree

multi-arch-coding-guideline.rst

Lines changed: 334 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,334 @@
1+
Multi-architecture development coding guideline
2+
###############################################
3+
4+
This document is intended for guiding initial development of
5+
ACRN multi-architecture.
6+
7+
This document is NOT a coding style guideline (where one normally expects
8+
that the document explains things such as indentation, naming convention,
9+
typedef rules, etc.)
10+
11+
During early development of multi-architecture project, the APIs
12+
and data structures should be organized in a consistent approach for
13+
better readability and maintainability. There are multiple ways to organize
14+
these factors and this documentation lays out a guideline to assist future
15+
development.
16+
17+
General Assumptions and Practices
18+
*********************************
19+
20+
Due to FuSa constraints, we have several compiler features or coding styles that
21+
are NOT encouraged in ``hypervisor/`` code:
22+
23+
* Weak link (``__weak``): There are several MISRA rules that discourage uses of weak
24+
link (Rule 1.2, 5.3, 5.8, 5.9). See `coding-guidelines`_ for more information.
25+
26+
* Function-like macro: Use inline function whenever possible. Function-like macros
27+
should be avoided unless absolutely necessary. This also implies that section-based
28+
variable placement is also discouraged (utilizes linker to group symbols in certain
29+
binary sections. See ``devicemodel/include/types.h``, macro ``SET_*``).
30+
31+
Practices and general rule of thumb:
32+
33+
* Enable compiler ``-O2`` as default.
34+
35+
* APIs that are public (globally visible) but implemented in arch domains should start
36+
their name with ``arch_``. If this API is optional, a default/empty ``arch_`` API
37+
can be implemented in common C/header file.
38+
39+
* Architecture headers should be exposure-controlled: If a structure or an API is used/shared
40+
only within the module, then do not expose it to public headers, put it to private headers.
41+
There is also a reverse statement: all arch symbols exposed to common scope should be public.
42+
This is nearly impossible in practice. So we follow only the former part, and leave the latter
43+
as future optimization.
44+
45+
.. _coding-guidelines: https://projectacrn.github.io/latest/developer-guides/coding_guidelines.html
46+
47+
Data Structures
48+
***************
49+
50+
In multi-architecture project we might have a common data structure that needs
51+
to carry some architecture specific information (for example, a vCPU object).
52+
53+
For this type of data structures we prefer the following style:
54+
55+
In common header:
56+
57+
.. code-block:: c
58+
59+
/* File: common.h */
60+
#include <arch-public.h>
61+
62+
struct foo {
63+
<common member1>;
64+
<common member2>;
65+
<common member3>;
66+
struct arch_foo arch;
67+
};
68+
69+
struct bar {
70+
<common member1>;
71+
<common member2>;
72+
<common member3>;
73+
}
74+
75+
And in architecture specific header:
76+
77+
.. code-block:: c
78+
79+
/* File: arch-public.h */
80+
81+
/* do NOT include common.h here */
82+
83+
struct arch_foo {
84+
<arch member1>;
85+
<arch member2>;
86+
<arch member3>;
87+
};
88+
89+
/* Forward declaration when the reference of struct is opaque */
90+
struct foo;
91+
struct bar;
92+
93+
struct baz {
94+
struct foo *f;
95+
struct bar *b;
96+
}
97+
98+
/* Forward declaration is also needed for function argument pointer */
99+
void some_function(struct foo *);
100+
101+
The architecture public header should NOT in turn include common header
102+
that already included the public header.
103+
104+
Consider an inclusion graph with nodes being headers, and edges being
105+
inclusions. If A includes B, there's an edge from A to B. Ideally, the
106+
header inclusion graph should be **acyclic**.
107+
108+
If a downstream header needs something from upstream headers, (upstream and
109+
downstream refers to positions relative to inclusion chain) a forward declaration
110+
SHOULD be sufficient. If not, then there are problems with your organization of headers.
111+
112+
However in practice, people will not spend time following inclusion chain to
113+
check if it is acyclic, and this issue is often noticable when there's a circular
114+
dependency compilation error. In most projects (especially large ones), as long
115+
as it compiles, it is OK.
116+
117+
So in our project, the **acyclic inclusion** is not an enforcement, but a
118+
practice that is strongly advised.
119+
120+
Interfaces
121+
**********
122+
123+
Here we consider only common-arch interfaces. Interfaces within each architecture
124+
are out of scope of this section.
125+
126+
There are several form of interfaces used in different use cases.
127+
128+
Function prototypes
129+
===================
130+
131+
This is the most common form of interface where a function is referenced in common scope
132+
and implemented in arch-specific scope.
133+
134+
We prefer the following style:
135+
136+
In common header:
137+
138+
.. code-block:: c
139+
140+
/* File: common.h */
141+
142+
#include <arch-public.h>
143+
144+
/*
145+
* Each architecture MUST implement this API in arch C file.
146+
* The implementation SHOULD NOT be marked as static inline
147+
* even if the implementation is small.
148+
*/
149+
void arch_api_mandatory(void);
150+
151+
/*
152+
* Short helpers that each architecture MAY provide.
153+
* Due to FuSa constraints described above,
154+
* Use a static inline function instead of a macro
155+
* to select arch implementation.
156+
*/
157+
static inline void quick_helper(void) { arch_helper(); }
158+
159+
/*
160+
* Architecture MAY implement this API.
161+
* If it does not, an empty stub is provided.
162+
* Define ARCH_HAS_SOME_CAPABILITY in arch headers to
163+
* indicate implementation.
164+
*/
165+
#ifdef ARCH_HAS_SOME_CAPABILITY
166+
int arch_api_optional(void);
167+
#else
168+
static inline int arch_api_optional(void) { return 0; }
169+
#endif
170+
171+
/*
172+
* Architecture MAY implement this API.
173+
* If it does not, a common default is provided in common C file.
174+
* Due to FuSa constraint described above, __weak is discouraged.
175+
* Define ARCH_HAS_OPTIONAL_WITH_DEFAULT in arch header to
176+
* indicate implementation instead.
177+
*/
178+
void arch_api_optional_with_default(void);
179+
180+
/*
181+
* Configuration-based APIs
182+
* The CONFIG macro is provided by header generated by build-system.
183+
*/
184+
#ifdef CONFIG_SWITCH_1
185+
void arch_api_conditional(void);
186+
#endif
187+
188+
In common C file:
189+
190+
.. code-block:: c
191+
192+
/* File: common.c */
193+
194+
#include <common.h>
195+
196+
#ifndef ARCH_HAS_OPTIONAL_WITH_DEFAULT
197+
void arch_api_optional_with_default(void) {
198+
/* default implementation */
199+
}
200+
#endif
201+
202+
void common_logic(void) {
203+
arch_api_mandatory();
204+
205+
arch_api_optional();
206+
207+
arch_api_optional_with_default();
208+
209+
#ifdef CONFIG_SWITCH_1
210+
arch_api_conditional();
211+
#endif
212+
}
213+
214+
In architecture-specific public header file:
215+
216+
.. code-block:: c
217+
218+
/* File: arch-public.h */
219+
220+
/* Indicates implementation to optional API */
221+
#define ARCH_HAS_SOME_CAPABILITY
222+
223+
/* Indicates implementation to optional_with_default API */
224+
#define ARCH_HAS_OPTIONAL_WITH_DEFAULT
225+
226+
static inline void arch_helper(void) {
227+
/* short implementation */
228+
}
229+
230+
In architecture-specific C file:
231+
232+
.. code-block:: c
233+
234+
/* File: arch.c */
235+
#include <common.h>
236+
#include <arch-priate.h>
237+
238+
void arch_api_mandatory(void) {
239+
/* implementation */
240+
}
241+
242+
/* ARCH_HAS_SOME_CAPABILITY is defined in arch-public header */
243+
void arch_api_optional(void) {
244+
/* implementation */
245+
}
246+
247+
/* ARCH_HAS_OPTIONAL_WITH_DEFAULT is defined in arch-public header */
248+
void arch_api_optional_with_default(void) {
249+
/* implementation */
250+
}
251+
252+
#ifdef CONFIG_SWITCH_1
253+
void arch_api_conditional(void) {
254+
/* implementation */
255+
}
256+
#endif
257+
258+
Register Callbacks
259+
==================
260+
261+
Callbacks are used when we do NOT know which implementation to be used
262+
until runtime. To determine implementation we need to do either detection,
263+
or accepting user input.
264+
265+
With this approach, all implementations will be compiled-in, and each implementation
266+
exposes a callback data structure typically called ``*_ops`` to common scope.
267+
268+
In common header file:
269+
270+
.. code-block:: c
271+
272+
/* File: common.h */
273+
struct foo_ops {
274+
void (*op1)(void);
275+
int (*op2)(int);
276+
int (*op3)(struct bar *);
277+
};
278+
279+
extern struct foo_ops a_ops;
280+
extern struct foo_ops b_ops;
281+
282+
283+
In common C file:
284+
285+
.. code-block:: c
286+
287+
/* File: common.c */
288+
289+
#include <common.h>
290+
291+
void do_detection() {
292+
/* detects environment */
293+
if (condition_a) {
294+
register_callback(a_ops);
295+
} else {
296+
register_callback(b_ops);
297+
}
298+
}
299+
300+
void actual_call() {
301+
op->op1();
302+
op->op2(1);
303+
}
304+
305+
In arch header file:
306+
307+
.. code-block:: c
308+
309+
/* File: arch-public.h */
310+
311+
/* Do nothing */
312+
313+
.. code-block:: c
314+
315+
/* File: arch.c */
316+
#include <common.h>
317+
318+
static void a_op1(void) {
319+
/* implementation */
320+
}
321+
322+
static int a_op2(void) {
323+
/* implementation */
324+
}
325+
326+
static int a_op2(struct bar *b) {
327+
/* implementation */
328+
}
329+
330+
const struct foo_ops a_ops = {
331+
.op1 = a_op1,
332+
.op2 = a_op2,
333+
.op3 = a_op3,
334+
}

0 commit comments

Comments
 (0)