| Message ID | 20241029181632.69600-1-alejandro.vallejo@cloud.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | xen/abi: On wide bitfields inside primitive types | expand |
On Tue Oct 29, 2024 at 6:16 PM GMT, Alejandro Vallejo wrote: > Non-boolean bitfields in the hypercall ABI make it fairly inconvenient to > create bindings for any language because (a) they are always ad-hoc and are > subject to restrictions regular fields are not (b) require boilerplate that > regular fields do not and (c) might not even be part of the core language, > forcing avoidable external libraries into any sort of generic library. > > This patch (it's a series merely to split roughly by maintainer) is one such > case that I happened to spot while playing around. It's the grant_version > field, buried under an otherwise empty grant_opts. > > The invariant I'd like to (slowly) introduce and discuss is that fields may > have bitflags (e.g: a packed array of booleans indexed by some enumerated > type), but not be mixed with wider fields in the same primitive type. This > ensures any field containing an integer of any kind can be referred by pointer > and treated the same way as any other with regards to sizeof() and the like. > > I'd like to have a certain consensus about this general point before going > establishing this restriction in the IDL system I'm working on. > > My preference would be to fold everything into a single patch if we decide to > follow through with this particular case. As I said before, the split is > artificial for review. > > Alejandro Vallejo (6): > xen/domctl: Refine grant_opts into grant_version > tools: Rename grant_opts to grant_version > tools/ocaml: Rename grant_opts to grant_version > xen/arm: Rename grant_opts to grant_version > xen/x86: Rename grant_opts to grant_version > xen/common: Rename grant_opts to grant_version > > tools/helpers/init-xenstore-domain.c | 2 +- > tools/libs/light/libxl_create.c | 2 +- > tools/ocaml/libs/xc/xenctrl_stubs.c | 3 +-- > tools/python/xen/lowlevel/xc/xc.c | 2 +- > tools/tests/paging-mempool/test-paging-mempool.c | 2 +- > tools/tests/resource/test-resource.c | 6 +++--- > tools/tests/tsx/test-tsx.c | 4 ++-- > xen/arch/arm/dom0less-build.c | 4 ++-- > xen/arch/arm/domain_build.c | 2 +- > xen/arch/x86/setup.c | 2 +- > xen/common/domain.c | 6 +++--- > xen/common/grant_table.c | 3 +-- > xen/include/public/domctl.h | 15 +++++++++++---- > xen/include/xen/grant_table.h | 4 ++-- > 14 files changed, 31 insertions(+), 26 deletions(-) Bah. I sent it too early. The new field in patches 2-6 ought to be max_grant_version. Regardless, the general point still holds, I hope. Cheers, Alejandro
> On 29 Oct 2024, at 18:16, Alejandro Vallejo <alejandro.vallejo@cloud.com> wrote: > > > The invariant I'd like to (slowly) introduce and discuss is that fields may > have bitflags (e.g: a packed array of booleans indexed by some enumerated > type), but not be mixed with wider fields in the same primitive type. This > ensures any field containing an integer of any kind can be referred by pointer > and treated the same way as any other with regards to sizeof() and the like. Acked-by: Christian Lindig <christian.lindig@cloud.com> Fine with me but the OCaml part is not very exposed to this. — C
On 29.10.2024 19:16, Alejandro Vallejo wrote: > Non-boolean bitfields in the hypercall ABI make it fairly inconvenient to > create bindings for any language because (a) they are always ad-hoc and are > subject to restrictions regular fields are not (b) require boilerplate that > regular fields do not and (c) might not even be part of the core language, > forcing avoidable external libraries into any sort of generic library. > > This patch (it's a series merely to split roughly by maintainer) is one such > case that I happened to spot while playing around. It's the grant_version > field, buried under an otherwise empty grant_opts. > > The invariant I'd like to (slowly) introduce and discuss is that fields may > have bitflags (e.g: a packed array of booleans indexed by some enumerated > type), but not be mixed with wider fields in the same primitive type. This > ensures any field containing an integer of any kind can be referred by pointer > and treated the same way as any other with regards to sizeof() and the like. While I don't strictly mind, I'm also not really seeing why taking addresses or applying sizeof() would be commonly necessary. Can you perhaps provide a concrete example of where the present way of dealing with grant max version is getting in the way? After all your use of the term "bitfield" doesn't really mean C's understanding of it, so especially (c) above escapes me to a fair degree. > I'd like to have a certain consensus about this general point before going > establishing this restriction in the IDL system I'm working on. > > My preference would be to fold everything into a single patch if we decide to > follow through with this particular case. As I said before, the split is > artificial for review. That's not just a preference, but a requirement, or else the build will break in the middle of the series (harming bisection at the very least). Jan