On 8/13/24 03:56, Octavian Purdila wrote:
   typedef struct {
     ...
     union {
       uint32_t PSELID;              /* 0x00000FF8 Peripheral Select and
                                      * Flexcomm module ID */
       struct {
         uint32_t PERSEL : 3;        /* [2..0] Peripheral Select */
         uint32_t LOCK : 1;          /* [3..3] Lock the peripheral select */
         uint32_t USARTPRESENT : 1;  /* [4..4] USART present indicator */
         uint32_t SPIPRESENT : 1;    /* [5..5] SPI present indicator */
         uint32_t I2CPRESENT : 1;    /* [6..6] I2C present indicator */
         uint32_t I2SPRESENT : 1;    /* [7..7] I2S Present */
         uint32_t : 4;
         uint32_t ID : 20;           /* [31..12] Flexcomm ID */
       } PSELID_b;
     };

Bitfield layout in C isn't portable, so don't generate this kind
of union-of-a-integer-and-some-bitfields, please. You can
generate FIELD() macro invocations (see include/hw/registerfields.h)
which define shift/mask/length macros that can be used with
FIELD_EX*/FIELD_DP* to do extract/deposit operations.


I see that C bitfields are already used in a few places in qemu. Could
you please elaborate on the portability issue?

Bitfields are fine, so long as you're only using them for storage compression and do not care about the underlying layout.

The moment you put them into a union with an integer, clearly you are expecting the bitfields to be in some particular order with respect to the integer, and that is the portability issue.

In particular, big-endian hosts will generally flip the order, layout starting at the most signifiacnt bit and work down. Other compilers will pad bits for alignment in ways that you do not expect.

Just Don't Do It.


r~

Reply via email to