Re: [PATCH v4 1/3] iommu/arm-smmu-v3: Introduce struct arm_smmu_event

[Jason Gunthorpe <jgg@nvidia.com>]

On Thu, Oct 24, 2024 at 05:37:11PM +0000, Pranjal Shrivastava wrote:
> On Thu, Oct 24, 2024 at 02:03:29PM -0300, Jason Gunthorpe wrote:
> > On Thu, Oct 24, 2024 at 05:02:08PM +0000, Pranjal Shrivastava wrote:
> > > 	/* --- cacheline 1 boundary (64 bytes) was 24 bytes ago --- */
> > > 	bool                       stall;                /*    88     1 */
> > > 	bool                       ssid_valid;           /*    89     1 */
> > > 	bool                       privileged;           /*    90     1 */
> > > 	bool                       instruction;          /*    91     1 */
> > > 	bool                       s2;                   /*    92     1 */
> > > 	bool                       read;                 /*    93     1 */
> > > 	bool                       ttrnw;                /*    94     1 */
> > > 	bool                       ttrnw_valid;          /*    95     1 */
> > 
> > Linus has had negative things to say about lists of bools in
> > structs. Use a bitfield?
> 
> Hmm, I agree, each bool is a waste of 7-bits. I tried the following:
> 
> struct arm_smmu_event {
> 	u8				id;
> 	u8				class;
> 	u16				stag;
> 
> 	/* Group the boolean values together */
> 	unsigned int stall:1;
> 	unsigned int ssid_valid:1;
> 	unsigned int privileged:1;
> 	unsigned int instruction:1;
> 	unsigned int s2:1;
> 	unsigned int read:1;
> 	unsigned int ttrnw:1;
> 	unsigned int ttrnw_valid:1;

You can make this a u8

> struct arm_smmu_event {
> 	u8				id;
> 	u8				class;
> 	u16				stag;
> 	
> 	/* Group the boolean values together */
> 	union {
> 		struct {
> 			unsigned int stall:1;
> 			unsigned int ssid_valid:1;
> 			unsigned int privileged:1;
> 			unsigned int instruction:1;
> 			unsigned int s2:1;
> 			unsigned int read:1;
> 			unsigned int ttrnw:1;
> 			unsigned int ttrnw_valid:1;
> 		};
> 		u8 flags;

I wouldn't do this..

Jason