Re: [kvm-unit-tests PATCH v5 1/6] lib: s390x: introduce bitfield for PSW mask

Nico Boehr
2023-07-13T09:36:01Z

Quoting Claudio Imbrenda (2023-07-13 10:20:13)
[...]
> > diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h
> > index bb26e008cc68..53279572a9ee 100644
> > --- a/lib/s390x/asm/arch_def.h
> > +++ b/lib/s390x/asm/arch_def.h
> > @@ -37,12 +37,36 @@ struct stack_frame_int {
> > };
> >
> > struct psw {
> > - uint64_t mask;
> > + union {
> > + uint64_t mask;
> > + struct {
> > + uint8_t reserved00:1;
> > + uint8_t per:1;
> > + uint8_t reserved02:3;
> > + uint8_t dat:1;
> > + uint8_t io:1;
> > + uint8_t ext:1;
> > + uint8_t key:4;
> > + uint8_t reserved12:1;
> > + uint8_t mchk:1;
> > + uint8_t wait:1;
> > + uint8_t pstate:1;
> > + uint8_t as:2;
> > + uint8_t cc:2;
> > + uint8_t prg_mask:4;
> > + uint8_t reserved24:7;
> > + uint8_t ea:1;
> > + uint8_t ba:1;
> > + uint32_t reserved33:31;
>
> since you will need to respin this anyway:
>
> can you use uint64_t everywhere in the bitfield? it would look more
> consistent and it would avoid some potential pitfalls of using
> bitfields. In our case we're lucky because all fields are properly
> aligned, but using uint64_t makes it easier to understand.

I will do that for consistency, I'd be interested to understand what pitfalls
you're talking about and how uint64_t avoids them, since we have a few bitfields
in the codebase.