Re: [PATCH] dma/idxd: Remove __packed from structures

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 3/20/25 1:18 AM, Yi Sun wrote:
> The __packed attribute introduces potential unaligned memory accesses
> and endianness portability issues. Instead of relying on compiler-specific
> packing, it's much better to explicitly fill structure gaps using padding
> fields, ensuring natural alignment.
> 
> Since all previously __packed structures already enforce proper alignment
> through manual padding, the __packed qualifiers are unnecessary and can be
> safely removed.
> 
> Signed-off-by: Yi Sun <yi.sun@xxxxxxxxx>

Although endian portability is probably not a concern given this driver is only for an Intel platform device.

Reviewed-by: Dave Jiang <dave.jiang@xxxxxxxxx>

> 
> diff --git a/drivers/dma/idxd/registers.h b/drivers/dma/idxd/registers.h
> index 006ba206ab1b..9c1c546fe443 100644
> --- a/drivers/dma/idxd/registers.h
> +++ b/drivers/dma/idxd/registers.h
> @@ -45,7 +45,7 @@ union gen_cap_reg {
>  		u64 rsvd3:32;
>  	};
>  	u64 bits;
> -} __packed;
> +};
>  #define IDXD_GENCAP_OFFSET		0x10
>  
>  union wq_cap_reg {
> @@ -65,7 +65,7 @@ union wq_cap_reg {
>  		u64 rsvd4:8;
>  	};
>  	u64 bits;
> -} __packed;
> +};
>  #define IDXD_WQCAP_OFFSET		0x20
>  #define IDXD_WQCFG_MIN			5
>  
> @@ -79,7 +79,7 @@ union group_cap_reg {
>  		u64 rsvd:45;
>  	};
>  	u64 bits;
> -} __packed;
> +};
>  #define IDXD_GRPCAP_OFFSET		0x30
>  
>  union engine_cap_reg {
> @@ -88,7 +88,7 @@ union engine_cap_reg {
>  		u64 rsvd:56;
>  	};
>  	u64 bits;
> -} __packed;
> +};
>  
>  #define IDXD_ENGCAP_OFFSET		0x38
>  
> @@ -114,7 +114,7 @@ union offsets_reg {
>  		u64 rsvd:48;
>  	};
>  	u64 bits[2];
> -} __packed;
> +};
>  
>  #define IDXD_TABLE_MULT			0x100
>  
> @@ -128,7 +128,7 @@ union gencfg_reg {
>  		u32 rsvd2:18;
>  	};
>  	u32 bits;
> -} __packed;
> +};
>  
>  #define IDXD_GENCTRL_OFFSET		0x88
>  union genctrl_reg {
> @@ -139,7 +139,7 @@ union genctrl_reg {
>  		u32 rsvd:29;
>  	};
>  	u32 bits;
> -} __packed;
> +};
>  
>  #define IDXD_GENSTATS_OFFSET		0x90
>  union gensts_reg {
> @@ -149,7 +149,7 @@ union gensts_reg {
>  		u32 rsvd:28;
>  	};
>  	u32 bits;
> -} __packed;
> +};
>  
>  enum idxd_device_status_state {
>  	IDXD_DEVICE_STATE_DISABLED = 0,
> @@ -183,7 +183,7 @@ union idxd_command_reg {
>  		u32 int_req:1;
>  	};
>  	u32 bits;
> -} __packed;
> +};
>  
>  enum idxd_cmd {
>  	IDXD_CMD_ENABLE_DEVICE = 1,
> @@ -213,7 +213,7 @@ union cmdsts_reg {
>  		u8 active:1;
>  	};
>  	u32 bits;
> -} __packed;
> +};
>  #define IDXD_CMDSTS_ACTIVE		0x80000000
>  #define IDXD_CMDSTS_ERR_MASK		0xff
>  #define IDXD_CMDSTS_RES_SHIFT		8
> @@ -284,7 +284,7 @@ union sw_err_reg {
>  		u64 rsvd5;
>  	};
>  	u64 bits[4];
> -} __packed;
> +};
>  
>  union iaa_cap_reg {
>  	struct {
> @@ -303,7 +303,7 @@ union iaa_cap_reg {
>  		u64 rsvd:52;
>  	};
>  	u64 bits;
> -} __packed;
> +};
>  
>  #define IDXD_IAACAP_OFFSET	0x180
>  
> @@ -320,7 +320,7 @@ union evlcfg_reg {
>  		u64 rsvd2:28;
>  	};
>  	u64 bits[2];
> -} __packed;
> +};
>  
>  #define IDXD_EVL_SIZE_MIN	0x0040
>  #define IDXD_EVL_SIZE_MAX	0xffff
> @@ -334,7 +334,7 @@ union msix_perm {
>  		u32 pasid:20;
>  	};
>  	u32 bits;
> -} __packed;
> +};
>  
>  union group_flags {
>  	struct {
> @@ -352,13 +352,13 @@ union group_flags {
>  		u64 rsvd5:26;
>  	};
>  	u64 bits;
> -} __packed;
> +};
>  
>  struct grpcfg {
>  	u64 wqs[4];
>  	u64 engines;
>  	union group_flags flags;
> -} __packed;
> +};
>  
>  union wqcfg {
>  	struct {
> @@ -410,7 +410,7 @@ union wqcfg {
>  		u64 op_config[4];
>  	};
>  	u32 bits[16];
> -} __packed;
> +};
>  
>  #define WQCFG_PASID_IDX                2
>  #define WQCFG_PRIVL_IDX		2
> @@ -474,7 +474,7 @@ union idxd_perfcap {
>  		u64 rsvd3:8;
>  	};
>  	u64 bits;
> -} __packed;
> +};
>  
>  #define IDXD_EVNTCAP_OFFSET		0x80
>  union idxd_evntcap {
> @@ -483,7 +483,7 @@ union idxd_evntcap {
>  		u64 rsvd:36;
>  	};
>  	u64 bits;
> -} __packed;
> +};
>  
>  struct idxd_event {
>  	union {
> @@ -493,7 +493,7 @@ struct idxd_event {
>  		};
>  		u32 val;
>  	};
> -} __packed;
> +};
>  
>  #define IDXD_CNTRCAP_OFFSET		0x800
>  struct idxd_cntrcap {
> @@ -506,7 +506,7 @@ struct idxd_cntrcap {
>  		u32 val;
>  	};
>  	struct idxd_event events[];
> -} __packed;
> +};
>  
>  #define IDXD_PERFRST_OFFSET		0x10
>  union idxd_perfrst {
> @@ -516,7 +516,7 @@ union idxd_perfrst {
>  		u32 rsvd:30;
>  	};
>  	u32 val;
> -} __packed;
> +};
>  
>  #define IDXD_OVFSTATUS_OFFSET		0x30
>  #define IDXD_PERFFRZ_OFFSET		0x20
> @@ -533,7 +533,7 @@ union idxd_cntrcfg {
>  		u64 rsvd3:4;
>  	};
>  	u64 val;
> -} __packed;
> +};
>  
>  #define IDXD_FLTCFG_OFFSET		0x300
>  
> @@ -543,7 +543,7 @@ union idxd_cntrdata {
>  		u64 event_count_value;
>  	};
>  	u64 val;
> -} __packed;
> +};
>  
>  union event_cfg {
>  	struct {
> @@ -551,7 +551,7 @@ union event_cfg {
>  		u64 event_enc:28;
>  	};
>  	u64 val;
> -} __packed;
> +};
>  
>  union filter_cfg {
>  	struct {
> @@ -562,7 +562,7 @@ union filter_cfg {
>  		u64 eng:8;
>  	};
>  	u64 val;
> -} __packed;
> +};
>  
>  #define IDXD_EVLSTATUS_OFFSET		0xf0
>  
> @@ -580,7 +580,7 @@ union evl_status_reg {
>  		u32 bits_upper32;
>  	};
>  	u64 bits;
> -} __packed;
> +};
>  
>  #define IDXD_MAX_BATCH_IDENT	256
>  
> @@ -620,17 +620,17 @@ struct __evl_entry {
>  	};
>  	u64 fault_addr;
>  	u64 rsvd5;
> -} __packed;
> +};
>  
>  struct dsa_evl_entry {
>  	struct __evl_entry e;
>  	struct dsa_completion_record cr;
> -} __packed;
> +};
>  
>  struct iax_evl_entry {
>  	struct __evl_entry e;
>  	u64 rsvd[4];
>  	struct iax_completion_record cr;
> -} __packed;
> +};
>  
>  #endif





[Index of Archives]     [Linux Kernel]     [Linux ARM (vger)]     [Linux ARM MSM]     [Linux Omap]     [Linux Arm]     [Linux Tegra]     [Fedora ARM]     [Linux for Samsung SOC]     [eCos]     [Linux PCI]     [Linux Fastboot]     [Gcc Help]     [Git]     [DCCP]     [IETF Announce]     [Security]     [Linux MIPS]     [Yosemite Campsites]

  Powered by Linux