Re: [PATCH v2 1/2] iommu/arm-smmu-v3: Print better events records

[Nicolin Chen <nicolinc@nvidia.com>]

On Mon, Sep 09, 2024 at 05:30:22PM +0000, Pranjal Shrivastava wrote:

> > > > > My only real concern is the fragility of using bitfields. I don't _think_
> > > > > the compiler is obliged to lay them out in the obvious way and I can't
> > > > > think of anything worse than being given a bad pretty-print when debugging
> > > > > a real driver issue!
> > >
> > > Well, if compiler is going to be an issue, I can't disagree with
> > > your point.
> > >
> > > Any reference to some existing issue with compiler failing to lay
> > > out properly? I wonder how other headers could define their unions
> > > using bitfields and stay safe..
> >
> > It's not so much about buggy compilers, but more that I don't think the
> > C standard defines the order and so relying on it can be fragile. If you
> > fancy going to the effort of ensuring that LLVM and GCC will agree on
> > the "obvious" layout forever more, then don't let me stop you ;)

I see!
 
> Hmmm.. +1
> I had some time, so.. I dived into the C specification[1] (free draft)
> to look for bitfield layouts, under section 6.7.2.1, point 11
> (page 101 of the pdf), and I see the following paragraph:
> 
> "An implementation may allocate any addressable storage unit large
> enough to hold a bit-field. If enough space remains, a bit-field that
> immediately follows another bit-field in a structure shall be packed
> into adjacent bits of the same unit. If insufficient space remains,
> whether a bit-field that does not fit is put into the next unit or
> overlaps adjacent units is implementation-defined. The order of
> allocation of bit-fields within a unit (high-order to low-order or
> low-order to high-order) is implementation-defined. The alignment
> of the addressable storage unit is unspecified."

Thanks for digging into the standard doc!

> However, I'm not sure how these "implementations" comprehend the spec.
> 
> > Will
> 
> > > I am trying to learn here: apart from what Will mentioned above,
> > > is there any other reason to stay away from bitfield?
> 
> One more thing I can think of is Alignment faults on certain archs.
> Accessing individual bitfields within the struct could lead to unaligned
> memory accesses. I have run into alignment faults on $ARCH=arm64 couple
> of times while using packed structs in the kernel code.

OK. It seems that we should stay safe with verbose FIELD_GETs then.

Thank you
Nicolin