|
| 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 | +* Consistent function storage class: If a common function prototype indicates a non-static |
| 36 | + function, do NOT implement it as ``static inline`` in header. Implement as non-static |
| 37 | + in C file. |
| 38 | + |
| 39 | +* APIs that are public (globally visible) but implemented in arch domains should start |
| 40 | + their name with ``arch_``. |
| 41 | + |
| 42 | +* Architecture headers should be exposure-controlled: If a structure or an API is used/shared |
| 43 | + only within the module, then do not expose it to public headers, put it to private headers if needed. |
| 44 | + There is also a reverse statement: all arch symbols exposed to common scope should be public. |
| 45 | + This is nearly impossible in practice. So we follow only the former part, and leave the latter |
| 46 | + as future optimization. |
| 47 | + |
| 48 | +.. _coding-guidelines: https://projectacrn.github.io/latest/developer-guides/coding_guidelines.html |
| 49 | + |
| 50 | +Data Structures |
| 51 | +*************** |
| 52 | + |
| 53 | +In multi-architecture project we might have a common data structure that needs |
| 54 | +to carry some architecture specific information (for example, a vCPU object). |
| 55 | + |
| 56 | +For this type of data structures we prefer the following style: |
| 57 | + |
| 58 | +In common header: |
| 59 | + |
| 60 | +.. code-block:: c |
| 61 | +
|
| 62 | + /* File: common.h */ |
| 63 | + #include <arch-public.h> |
| 64 | +
|
| 65 | + struct foo { |
| 66 | + <common member1>; |
| 67 | + <common member2>; |
| 68 | + <common member3>; |
| 69 | + struct foo_arch arch; |
| 70 | + }; |
| 71 | +
|
| 72 | + struct bar { |
| 73 | + <common member1>; |
| 74 | + <common member2>; |
| 75 | + <common member3>; |
| 76 | + } |
| 77 | +
|
| 78 | +And in architecture specific header: |
| 79 | + |
| 80 | +.. code-block:: c |
| 81 | +
|
| 82 | + /* File: arch-public.h */ |
| 83 | +
|
| 84 | + /* Forward declaration when the reference of struct is opaque */ |
| 85 | + struct foo; |
| 86 | + struct bar; |
| 87 | +
|
| 88 | + struct foo_arch { |
| 89 | + <arch member1>; |
| 90 | + <arch member2>; |
| 91 | + <arch member3>; |
| 92 | + struct foo *opaque_foo; |
| 93 | + struct bar *opaque_bar; |
| 94 | + }; |
| 95 | +
|
| 96 | +If you encounter a case where you have non-opaque circular dependency, |
| 97 | +reconsider and/or rearrange your data structure scope. |
| 98 | +You're likely mixing things up. |
| 99 | + |
| 100 | +Interfaces |
| 101 | +********** |
| 102 | + |
| 103 | +Here we consider only common-arch interfaces. Interfaces within each architecture |
| 104 | +are out of scope of this section. |
| 105 | + |
| 106 | +There are several form of interfaces used in different use cases. |
| 107 | + |
| 108 | +Function prototypes |
| 109 | +=================== |
| 110 | + |
| 111 | +This is the most common form of interface where a function is referenced in common scope |
| 112 | +and implemented in arch-specific scope. |
| 113 | + |
| 114 | +We prefer the following style: |
| 115 | + |
| 116 | +In common header: |
| 117 | + |
| 118 | +.. code-block:: c |
| 119 | +
|
| 120 | + /* File: common.h */ |
| 121 | +
|
| 122 | + #include <arch-public.h> |
| 123 | +
|
| 124 | + /* |
| 125 | + * Each architecture MUST implement this API in arch C file. |
| 126 | + * The implementation SHOULD NOT be marked as static inline |
| 127 | + * even if the implementation is small. |
| 128 | + */ |
| 129 | + void arch_api_mandatory(void); |
| 130 | +
|
| 131 | + /* |
| 132 | + * Short helpers that each architecture MAY provide. |
| 133 | + * Due to FuSa constraints described above, |
| 134 | + * Use a static inline function instead of a macro |
| 135 | + * to select arch implementation. |
| 136 | + */ |
| 137 | + static inline void quick_helper(void) { arch_helper(); } |
| 138 | +
|
| 139 | + /* |
| 140 | + * Architecture MAY implement this API. |
| 141 | + * If it does not, an empty stub is provided. |
| 142 | + * Define __ARCH_HAS_SOME_CAPABILITY in arch headers to |
| 143 | + * indicate implementation. |
| 144 | + */ |
| 145 | + #ifdef __ARCH_HAS_SOME_CAPABILITY |
| 146 | + int arch_api_optional(void); |
| 147 | + #else |
| 148 | + static inline int arch_api_optional(void) { return 0; } |
| 149 | + #endif |
| 150 | +
|
| 151 | + /* |
| 152 | + * Architecture MAY implement this API. |
| 153 | + * If it does not, a common default is provided in common C file. |
| 154 | + * Due to FuSa constraint described above, __weak is discouraged. |
| 155 | + * Define __ARCH_HAS_OPTIONAL_WITH_DEFAULT in arch header to |
| 156 | + * indicate implementation instead. |
| 157 | + */ |
| 158 | + void arch_api_optional_with_default(void); |
| 159 | +
|
| 160 | + /* |
| 161 | + * Configuration-based APIs |
| 162 | + * The CONFIG macro is provided by header generated by build-system. |
| 163 | + */ |
| 164 | + #ifdef CONFIG_SWITCH_1 |
| 165 | + void arch_api_conditional(void); |
| 166 | + #endif |
| 167 | +
|
| 168 | +In common C file: |
| 169 | + |
| 170 | +.. code-block:: c |
| 171 | +
|
| 172 | + /* File: common.c */ |
| 173 | +
|
| 174 | + #include <common.h> |
| 175 | +
|
| 176 | + #ifndef __ARCH_HAS_OPTIONAL_WITH_DEFAULT |
| 177 | + void arch_api_optional_with_default(void) { |
| 178 | + /* default implementation */ |
| 179 | + } |
| 180 | + #endif |
| 181 | +
|
| 182 | + void common_logic(void) { |
| 183 | + arch_api_mandatory(); |
| 184 | +
|
| 185 | + arch_api_optional(); |
| 186 | +
|
| 187 | + arch_api_optional_with_default(); |
| 188 | +
|
| 189 | + #ifdef CONFIG_SWITCH_1 |
| 190 | + arch_api_conditional(); |
| 191 | + #endif |
| 192 | + } |
| 193 | +
|
| 194 | +In architecture-specific public header file: |
| 195 | + |
| 196 | +.. code-block:: c |
| 197 | +
|
| 198 | + /* File: arch-public.h */ |
| 199 | +
|
| 200 | + /* Indicates implementation to optional API */ |
| 201 | + #define __ARCH_HAS_SOME_CAPABILITY |
| 202 | +
|
| 203 | + /* Indicates implementation to optional_with_default API */ |
| 204 | + #define __ARCH_HAS_OPTIONAL_WITH_DEFAULT |
| 205 | +
|
| 206 | + static inline void arch_helper(void) { |
| 207 | + /* short implementation */ |
| 208 | + } |
| 209 | +
|
| 210 | +In architecture-specific C file: |
| 211 | + |
| 212 | +.. code-block:: c |
| 213 | +
|
| 214 | + /* File: arch.c */ |
| 215 | + #include <common.h> |
| 216 | + #include <arch-priate.h> |
| 217 | +
|
| 218 | + void arch_api_mandatory(void) { |
| 219 | + /* implementation */ |
| 220 | + } |
| 221 | +
|
| 222 | + /* __ARCH_HAS_SOME_CAPABILITY is defined in arch-public header */ |
| 223 | + void arch_api_optional(void) { |
| 224 | + /* implementation */ |
| 225 | + } |
| 226 | +
|
| 227 | + /* __ARCH_HAS_OPTIONAL_WITH_DEFAULT is defined in arch-public header */ |
| 228 | + void arch_api_optional_with_default(void) { |
| 229 | + /* implementation */ |
| 230 | + } |
| 231 | +
|
| 232 | + #ifdef CONFIG_SWITCH_1 |
| 233 | + void arch_api_conditional(void) { |
| 234 | + /* implementation */ |
| 235 | + } |
| 236 | + #endif |
| 237 | +
|
| 238 | +Register Callbacks |
| 239 | +================== |
| 240 | + |
| 241 | +Callbacks are used when we do NOT know which implementation to be used |
| 242 | +until runtime. To determine implementation we need to do either detection, |
| 243 | +or accepting user input. |
| 244 | + |
| 245 | +With this approach, all implementations will be compiled-in, and each implementation |
| 246 | +exposes a callback data structure typically called ``*_ops`` to common scope. |
| 247 | + |
| 248 | +In common header file: |
| 249 | + |
| 250 | +.. code-block:: c |
| 251 | +
|
| 252 | + /* File: common.h */ |
| 253 | + struct foo_ops { |
| 254 | + void (*op1)(void); |
| 255 | + int (*op2)(int); |
| 256 | + int (*op3)(struct bar *); |
| 257 | + }; |
| 258 | +
|
| 259 | + extern struct foo_ops a_ops; |
| 260 | + extern struct foo_ops b_ops; |
| 261 | +
|
| 262 | +
|
| 263 | +In common C file: |
| 264 | + |
| 265 | +.. code-block:: c |
| 266 | +
|
| 267 | + /* File: common.c */ |
| 268 | +
|
| 269 | + #include <common.h> |
| 270 | +
|
| 271 | + void do_detection() { |
| 272 | + /* detects environment */ |
| 273 | + if (condition_a) { |
| 274 | + register_callback(a_ops); |
| 275 | + } else { |
| 276 | + register_callback(b_ops); |
| 277 | + } |
| 278 | + } |
| 279 | +
|
| 280 | + void actual_call() { |
| 281 | + op->op1(); |
| 282 | + op->op2(1); |
| 283 | + } |
| 284 | +
|
| 285 | +In arch header file: |
| 286 | + |
| 287 | +.. code-block:: c |
| 288 | +
|
| 289 | + /* File: arch-public.h */ |
| 290 | +
|
| 291 | + /* Do nothing */ |
| 292 | +
|
| 293 | +.. code-block:: c |
| 294 | +
|
| 295 | + /* File: arch.c */ |
| 296 | + #include <common.h> |
| 297 | +
|
| 298 | + static void a_op1(void) { |
| 299 | + /* implementation */ |
| 300 | + } |
| 301 | +
|
| 302 | + static int a_op2(void) { |
| 303 | + /* implementation */ |
| 304 | + } |
| 305 | +
|
| 306 | + static int a_op2(struct bar *b) { |
| 307 | + /* implementation */ |
| 308 | + } |
| 309 | +
|
| 310 | + const struct foo_ops a_ops = { |
| 311 | + .op1 = a_op1, |
| 312 | + .op2 = a_op2, |
| 313 | + .op3 = a_op3, |
| 314 | + } |
0 commit comments