[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] do_multicall and MISRA Rule 8.3



On Mon, 18 Mar 2024, Jan Beulich wrote:
> The named reasons simply aren't convincing to me, at all. There's no
> loss towards the "end goals" when "unsigned int" is used instead of
> "uint32_t". Plus I recall Andrew putting under question use of
> "unsigned int" for various hypercall parameter declarations, indicating
> his belief that "unsigned long" ought to be used. That's, to me, quite
> the opposite of using fixed-width types here, as there's no uniformly
> correct fixed-width type to use in that case.
> 
> So to me, besides there not having been technical arguments towards
> the need to use fixed width types here, there's actually added
> confusion about what's actually wanted. Imo if we started using fixed-
> width types for hypercall handler parameter declarations, (almost?) all
> non-handle ones would likely want to be converted. Consistency, after
> all, is part of the "easy to maintain code" goal. Plus without
> consistency how would one determine when to use what kind of types.

[...]

> The main use of fixed width types, to me, is in interface structure
> definitions - between Xen and hardware / firmware, or in hypercall
> structures. I'm afraid I have a hard time seeing good uses outside of
> that. Even in inline assembly, types of variables used for inputs or
> outputs don't strictly need to be fixed-width; I can somewhat accept
> their use there for documentation purposes.


Non-ABI interfaces are OK with native types.

Our ABI interfaces are, for better or for worse, described using C
header files. Looking at these header files, it should be clear the size
and alignment of all integer parameters.

To that end, I think we should use fixed-width types in all ABIs,
including hypercall entry points. In my opinion, C hypercall entry
points are part of the ABI and should match the integer types used in
the public header files. I don't consider the little assembly code on
hypercall entry as important. I think we should avoid having one
description of the hypercall types in sched.h and different types used
in do_sched_op.

Sometimes, we might have parameters that vary in size depending on the
architecture. For instance, a parameter could be 32-bit for 32-bit
architectures and 64-bit for 64-bit architectures.

For these cases, using "unsigned long", together with a document like
the one I submitted recently to xen-devel, is a good way forward: it
is semantically correct on all architectures, and it still comes with a
precise size and alignment as described in the document. In this
context, "unsigned long" means "register size" (on ARM we have
register_t). This is the one case where it makes sense not to use a
fixed-width type. Alternatively we would have to use #ifdefs.



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.