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

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

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.

 
> > +   * - 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:
> 
>    * - char 
>      - at least 8 bits

this

>      - equaling size
>      - all architectures
> 
>    * - char
>      - 8 bits
>      - 8 bits
>      - x86, ARM, RISC-V, PPC
> 
>    * - 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.