Re: MISRA violations in hypercall-defs

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Wed, 9 Aug 2023, Jan Beulich wrote:
> On 09.08.2023 01:22, Stefano Stabellini wrote:
> > On Tue, 8 Aug 2023, Jan Beulich wrote:
> >> On 08.08.2023 10:47, Federico Serafini wrote:
> >>> Hello everyone.
> >>>
> >>> I would like to to ask your opinion about the auto-generated file
> >>> xen/include/xen/hypercall-defs.h which contains some violations of
> >>> MISRA C:2012 Rule 8.3:
> >>> "All declarations of an object or function shall use the same names and
> >>> type qualifiers".
> >>>
> >>> Such violations can be seen at the following links
> >>> (copy and paste the link on you browser, including also the characters
> >>> after the '#'):
> >>>
> >>> - arm
> >>> https://saas.eclairit.com:3787/fs/var/local/eclair/XEN.ecdf/ECLAIR_normal/origin/staging/ARM64-Set1/218/PROJECT.ecd;/by_service/MC3R1.R8.3.html#{"select":true,"selection":{"hiddenAreaKinds":[],"hiddenSubareaKinds":[],"show":true,"selector":{"enabled":true,"negated":false,"kind":2,"children":[{"enabled":true,"negated":false,"kind":0,"domain":"file","inputs":[{"enabled":true,"text":"xen/include/xen/hypercall-defs.h"}]}]}}}
> >>>
> >>> - x86
> >>> https://saas.eclairit.com:3787/fs/var/local/eclair/XEN.ecdf/ECLAIR_normal/origin/staging/X86_64-Set1/218/PROJECT.ecd;/by_service/MC3R1.R8.3.html#{"select":true,"selection":{"hiddenAreaKinds":[],"hiddenSubareaKinds":[],"show":true,"selector":{"enabled":true,"negated":false,"kind":2,"children":[{"enabled":true,"negated":false,"kind":0,"domain":"file","inputs":[{"enabled":true,"text":"xen/include/xen/hypercall-defs.h"}]}]}}}
> >>>
> >>> Some of the violations are due to mismatches on the return types
> >>> and the use of `ret_t`.
> >>
> >> We already said that ret_t will need deviating. For parameter names
> >> it ought to be possible to suitably rename, like done elsewhere. Whether
> >> that means renaming in the generator script or in the definitions likely
> >> again needs judging on a case-by-case basis.
> > 
> > Is it the case that ret_t is purposedly defined as 'long' for 64-bit x86
> > guests and 'int' for 32-bit x86 guests?
> 
> Yes.
> 
> > I am asking because it looks like we don't use ret_t at all on ARM and
> > on the public interfaces.
> 
> And I wonder how you get away with this. Aiui hypercall return values are
> 32-bit on Arm32, so I'd expect you to be at risk of truncation issues in
> a limited set of cases (see in particular the bottom of compat_memory_op(),
> where out-of-range values are saturated rather than truncated). But maybe
> in practice this can't happen?

Yes I think that must be it. Also consider that mixing 32/64
environments on ARM is rare: the arm32 version of Xen is mostly to
run on arm32 targets with only 32-bit guests. The arm64 version of Xen
is to run on arm64 targets with mostly (only?) 64-bit guests. Which is a
different situation compared to x86.

So the ret_t deviation would only affect the x86 version of the
hypervisor.


P.S.
Julien, Bertrand, do you think we should unsupport (in SUPPORT.md, today
it is not clarified) 32-bit guests on a 64-bit ARM hypervisor?