[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <aCXv_cp1ITJeobQv@ysun46-mobl.ccr.corp.intel.com>
Date: Thu, 15 May 2025 21:45:33 +0800
From: Yi Sun <yi.sun@...el.com>
To: <vkoul@...nel.org>
CC: <gordon.jin@...el.com>, <yi.sun@...ux.intel.com>, Andy Shevchenko
<andriy.shevchenko@...ux.intel.com>, Dave Jiang <dave.jiang@...el.com>,
Fenghua Yu <fenghuay@...dia.com>, <linux-kernel@...r.kernel.org>,
<dmaengine@...r.kernel.org>
Subject: Re: [PATCH v2] dmaengine: idxd: Remove __packed from structures
On 04.04.2025 13:36, 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@...el.com>
>Reviewed-by: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
>Reviewed-by: Dave Jiang <dave.jiang@...el.com>
>Reviewed-by: Fenghua Yu <fenghuay@...dia.com>
>
Hi Vinod,
Gentle ping.
Thanks
--Sun, Yi
>---
>Changelog:
>- v2: Change the subject to match idxd driver patch convention (Fenghua)
>
>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
>--
>2.43.0
>
Powered by blists - more mailing lists