Re: [PATCH v3] docs/misra: document the expected sizes of integer types

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Fri, 5 Apr 2024, Jan Beulich wrote:
> 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 ...

Yes, I am fine with that



> >>> +   * - 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).
 
I don't mind having two sections, one for my goal and one for yours.
However, I am a bit unsure how to word the generic part in a way that is
clear and unambiguous, so I'll keep only the explicitly listed
architectures in the next version (following your suggestion further
up.)

 
> >>    * - 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.
>