[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v3] docs/misra: document the expected sizes of integer types
On 05.04.2024 01:56, Stefano Stabellini wrote: > On Thu, 4 Apr 2024, Jan Beulich wrote: >> On 04.04.2024 03:12, Stefano Stabellini wrote: >>> --- a/docs/misra/C-language-toolchain.rst >>> +++ b/docs/misra/C-language-toolchain.rst >>> @@ -480,4 +480,73 @@ The table columns are as follows: >>> - See Section "4.13 Preprocessing Directives" of GCC_MANUAL and >>> Section "11.1 Implementation-defined behavior" of CPP_MANUAL. >>> >>> >>> +Sizes of Integer types >>> +______________________ >>> + >>> +Xen expects System V ABI on x86_64: >>> + https://gitlab.com/x86-psABIs/x86-64-ABI >>> + >>> +Xen expects AAPCS32 on ARMv8-A AArch32: >>> + https://github.com/ARM-software/abi-aa/blob/main/aapcs32/aapcs32.rst >>> + >>> +Xen expects AAPCS64 LP64 on ARMv8-A AArch64: >>> + https://github.com/ARM-software/abi-aa/blob/main/aapcs64/aapcs64.rst >>> + >>> +A summary table of data types, sizes and alignment is below: >>> + >>> +.. list-table:: >>> + :widths: 10 10 10 45 >>> + :header-rows: 1 >>> + >>> + * - Type >>> + - Size >>> + - Alignment >>> + - Architectures >>> + >>> + * - char >>> + - 8 bits >>> + - 8 bits >>> + - all architectures >> >> This one _may_ be acceptable, but already feels like going too far. >> >>> + * - short >>> + - 16 bits >>> + - 16 bits >>> + - all architectures >>> + >>> + * - int >>> + - 32 bits >>> + - 32 bits >>> + - all architectures >> >> These two I continue to disagree with. The values are minimum required ones. > > The purpose of the document docs/misra/C-language-toolchain.rst is to > describe the reference safety-supported configuration. In a way, this > document is similar to SUPPORT.md but for safety instead of security. > > Here, we need to write down the stable configuration, the one everyone > is aligned and convinced should work correctly. > > Now, let's say that I believe you and agree with you that it should be > possible to support int as 64-bit. This configuration is not tested. If > I can draw a comparison, it would be similar to ask for XSM to be > security supported while in fact is marked as experimental in > SUPPORT.md. > > If you want, taking inspiration from SUPPORT.md, we can have a > "supported" column and a "experimental" column. In the experimental > column we can write down "at least 32-bit" or "32-bit or larger". > > >> Even if code changes may be needed (Misra already helps us here by stopping >> undue mixing of e.g. unsigned int and uint32_t in at least some situations), >> there's no inherent requirement in Xen for such restrictions. > > I hope that my comparison with XSM and SUPPORT.md helps explain why we > need to clarify the safety supported configuration with the values we > actually validate Xen with. > > Your goal is to write down what should work with Xen, which is also OK > but it is a different goal. It is OK to say that we aim for Xen to also > work with other configurations too, and list them. That was not my > intention, but I can expand the scope if you request. To achieve just your goal, would you then please replace all instances of "all architectures" and "all <N>-bit architectures" by an enumeration of the specific ones, as you have it elsewhere? This would then allow architectures I'm thinking about without impacting your goal. FTAOD ... >>> + * - long >>> + - 32 bits >>> + - 32 bits >>> + - 32-bit architectures (x86_32, ARMv8-A AArch32, ARMv8-R AArch32) >>> + >>> + * - long >>> + - 64 bits >>> + - 64 bits >>> + - 64-bit architectures (x86_64, ARMv8-A AArch64, RV64, PPC64) >>> + >>> + * - long long >>> + - 64-bit >>> + - 32-bit >>> + - x86_32 >>> + >>> + * - long long >>> + - 64-bit >>> + - 64-bit >>> + - 64-bit architectures, ARMv8-A AArch32, ARMv8-R AArch32 >> >> Along the lines of the above, simply saying "64-bit architectures" here >> is too generic. Whereas for long (above) and ... >> >>> + * - pointer >>> + - 32-bit >>> + - 32-bit >>> + - 32-bit architectures (x86_32, ARMv8-A AArch32, ARMv8-R AArch32) >>> + >>> + * - pointer >>> + - 64-bit >>> + - 64-bit >>> + - 64-bit architectures (x86_64, ARMv8-A AArch64, RV64, PPC64) >> >> ... pointers I agree (and the special mentioning of the architectures >> in parentheses could even be omitted imo). To summarize, my counter >> proposal: ... this counter proposal already specifically addressed that aspect, by e.g. ... >> * - char >> - at least 8 bits > > this > >> - equaling size >> - all architectures >> >> * - char >> - 8 bits >> - 8 bits >> - x86, ARM, RISC-V, PPC ... having two sections here: One to address your goal, and one to address mine. My further suggestion further up merely would mean dropping the generic parts (for imo no good reason). Jan >> * - short >> - at least 16 bits > > and this > >> - equaling size >> - all architectures >> >> * - short >> - 16 bits >> - 16 bits >> - x86, ARM, RISC-V, PPC >> >> * - int >> - at least 32 bits > > and this, more below > > >> - equaling size >> - all architectures >> >> * - int >> - 32 bits >> - 32 bits >> - x86, ARM, RISC-V, PPC >> >> * - long >> - 32 bits >> - 32 bits >> - 32-bit architectures >> >> * - long >> - 64 bits >> - 64 bits >> - 64-bit architectures >> >> * - long long >> - 64-bit >> - 32-bit >> - x86_32 >> >> * - long long >> - 64-bit >> - 64-bit >> - x86_64, ARMv8-A AArch64, RV64, PPC64, ARMv8-A AArch32, ARMv8-R AArch32 >> >> * - pointer >> - 32-bit >> - 32-bit >> - 32-bit architectures >> >> * - pointer >> - 64-bit >> - 64-bit >> - 64-bit architectures >> >> Eventually, by properly decoupling pointers from longs (via using >> {,u}intptr_t >> appropriately), the restrictions on "long" could also be lifted. >> >> Note that the generic requirements on char and short also are imposed by C99. >> It may therefore not be necessary to state them explicitly, but rather refer >> to that standard (just like you're now referencing the SysV psABI-s). > > I am OK with the above, except for the three instances of "at least". As > mentioned earlier, we need to specify the supported and validated > configuration. If you want we can also add another field to express what > we aim at getting Xen to work with, but it should be separate.
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |