Re: [PATCH v2 1/4] docs: add xen_ulong_t to the documented integers sizes/alignments

[Bertrand Marquis <Bertrand.Marquis@xxxxxxx>]

Hi Julien,

> On 15 Apr 2024, at 12:08, Julien Grall <julien@xxxxxxx> wrote:
> 
> Hi Bertrand,
> 
> On 15/04/2024 08:48, Bertrand Marquis wrote:
>> Hi Julien,
>>> On 12 Apr 2024, at 19:01, Julien Grall <julien.grall.oss@xxxxxxxxx> wrote:
>>> 
>>> 
>>> 
>>> On Fri, 12 Apr 2024 at 11:30, Bertrand Marquis <Bertrand.Marquis@xxxxxxx> 
>>> wrote:
>>> Hi Julien,
>>> 
>>>> On 12 Apr 2024, at 15:53, Julien Grall <julien.grall.oss@xxxxxxxxx> wrote:
>>>> 
>>>> 
>>>> 
>>>> On Thu, 11 Apr 2024 at 18:08, Stefano Stabellini <sstabellini@xxxxxxxxxx> 
>>>> wrote:
>>>> On Wed, 10 Apr 2024, Julien Grall wrote:
>>>>> On Wed, 10 Apr 2024 at 19:47, Stefano Stabellini 
>>>>> <stefano.stabellini@xxxxxxx> wrote:
>>>>>       xen_ulong_t is widely used in public headers.
>>>>> 
>>>>>       Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxx>
>>>>>       ---
>>>>> 
>>>>>       Given that xen_ulong_t is used in public headers there could be a 
>>>>> better
>>>>>       place for documenting it but this was the most straightforward to 
>>>>> add.
>>>>>       ---
>>>>>        docs/misra/C-language-toolchain.rst | 11 +++++++++++
>>>>>        1 file changed, 11 insertions(+)
>>>>> 
>>>>>       diff --git a/docs/misra/C-language-toolchain.rst 
>>>>> b/docs/misra/C-language-toolchain.rst
>>>>>       index 5ddfe7bdbe..7a334260e6 100644
>>>>>       --- a/docs/misra/C-language-toolchain.rst
>>>>>       +++ b/docs/misra/C-language-toolchain.rst
>>>>>       @@ -531,6 +531,17 @@ A summary table of data types, sizes and 
>>>>> alignment is below:
>>>>>             - 64 bits
>>>>>             - x86_64, ARMv8-A AArch64, RV64, PPC64
>>>>> 
>>>>>       +   * - xen_ulong_t
>>>>>       +     - 32 bits
>>>>>       +     - 32 bits
>>>>>       +     - x86_32
>>>>>       +
>>>>>       +   * - xen_ulong_t
>>>>>       +     - 64 bits
>>>>>       +     - 64 bits
>>>>>       +     - x86_64, ARMv8-A AArch64, RV64, PPC64, ARMv8-A AArch32, 
>>>>> ARMv8-R
>>>>>       +       AArch32, ARMv7-A
>>>>> 
>>>>> 
>>>>> We support neither ARMv8-R nor ARMv8-A Aarch32.
>>>>> 
>>>>> I could possibly accept the latter because it works to. But the former is 
>>>>> so far misleading.
>>>> 
>>>> Yes I think you are right. Moreover this document
>>>> (C-language-toolchain.rst) is meant for the Xen build. While this patch
>>>> is trying to document the types used in the public headers for the
>>>> external-facing ABI.
>>>> 
>>>> I'll move the information this patch is adding to a separate document,
>>>> specific to the public headers. I will only add the architectures
>>>> currently working: I'll add ARMv8-A Aarch32 because although it is
>>>> unsupported it is interesting to know the size of xen_ulong_t for
>>>> aarch32 in the public headers. I will remove ARMv8-R as it is not
>>>> available upstream.
>>>> 
>>>> Thinking a bit more. What about Armv9? Rather than listing each version, 
>>>> should we instead use ARMv7-A aarch32 and later, ARMv8-A aarch64 and later?
>>> 
>>> Definitely you are right here but as for Armv8-R, Armv9 is not something 
>>> that we explicitely support right now (even though it should work).
>>> 
>>> I am confused with the comparison. I thought you can’t boot Xen at all on 
>>> Armv8-R. But you can on Armv9-A as this just Armv8-A + features the 
>>> software don’t need to use.
>>> 
>>> Did you intend to draw the comparison with Armv8-A Aarch32?
>> Yes in my mind armv9 even if currently working it is not something 
>> officially supported so it is in the same state as armv8 aarch32.
> 
> AFAICT, Stefano said he will add ARMv8-A AArch32, so we should be consistent 
> and add Armv9-A in the list.

Yes that makes sense, I agree.

Cheers
Bertrand

> 
> Cheers,
> 
> -- 
> Julien Grall