From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mellanox.co.il (mail-il-dmz.mellanox.com [193.47.165.129]) by dpdk.org (Postfix) with ESMTP id 2872F1B113 for ; Thu, 3 Jan 2019 09:14:42 +0100 (CET) Received: from Internal Mail-Server by MTLPINE1 (envelope-from yskoh@mellanox.com) with ESMTPS (AES256-SHA encrypted); 3 Jan 2019 10:14:38 +0200 Received: from scfae-sc-2.mti.labs.mlnx (scfae-sc-2.mti.labs.mlnx [10.101.0.96]) by labmailer.mlnx (8.13.8/8.13.8) with ESMTP id x038EJfd008603; Thu, 3 Jan 2019 10:14:37 +0200 From: Yongseok Koh To: Andy Green Cc: Hemant Agrawal , dpdk stable Date: Thu, 3 Jan 2019 00:13:33 -0800 Message-Id: <20190103081400.14191-10-yskoh@mellanox.com> X-Mailer: git-send-email 2.11.0 In-Reply-To: <20190103081400.14191-1-yskoh@mellanox.com> References: <20190103081400.14191-1-yskoh@mellanox.com> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Subject: [dpdk-stable] patch 'bus/dpaa: fix inconsistent struct alignment' has been queued to LTS release 17.11.5 X-BeenThere: stable@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches for DPDK stable branches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 03 Jan 2019 08:14:42 -0000 Hi, FYI, your patch has been queued to LTS release 17.11.5 Note it hasn't been pushed to http://dpdk.org/browse/dpdk-stable yet. It will be pushed if I get no objections before 01/04/19. So please shout if anyone has objections. Also note that after the patch there's a diff of the upstream commit vs the patch applied to the branch. If the code is different (ie: not only metadata diffs), due for example to a change in context or macro names, please double check it. Thanks. Yongseok --- >>From 84f4ff2891bab8ddca99b73983f7c3108b532528 Mon Sep 17 00:00:00 2001 From: Andy Green Date: Mon, 14 May 2018 13:00:12 +0800 Subject: [PATCH] bus/dpaa: fix inconsistent struct alignment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit [ upstream commit dd6f8d712e927397da27f2a3d90a759eeac4f694 ] The actual descriptor for qm_mr_entry is 64-byte aligned. But the original code plays a trick, and puts a u8 common to the three descriptor subtypes in the union afterwards outside their structure definitions. Unfortunately since they compose a struct qm_fd with alignment 8, this trick destroys the ability of the compiler to understand what has happened, resulting in this kind of problem: drivers/bus/dpaa/include/fsl_qman.h:354:3: error: alignment 1 of ‘struct ’ is less than 8 [-Werror=packed-not-aligned] } __packed dcern; on gcc 8 / Fedora 28 out of the box. This patch moves the u8 verb into the structure definitions composed into the union, so the alignment of the parent struct containing the alignment 8 object can also be seen to be alignment 8 by the compiler. Uses of .verb are fixed up to use .ern.verb (the same offset of +0 inside all the structs in the union). The final struct layout should be unchanged. Fixes: c47ff048b99a ("bus/dpaa: add QMAN driver core routines") Fixes: f6fadc3e6310 ("bus/dpaa: add QMAN interface driver") Signed-off-by: Andy Green Acked-by: Hemant Agrawal Tested-by: Hemant Agrawal --- drivers/bus/dpaa/base/qbman/qman.c | 14 +++++++------- drivers/bus/dpaa/include/fsl_qman.h | 24 +++++++++++++----------- 2 files changed, 20 insertions(+), 18 deletions(-) diff --git a/drivers/bus/dpaa/base/qbman/qman.c b/drivers/bus/dpaa/base/qbman/qman.c index b8511103c..84f0cb0be 100644 --- a/drivers/bus/dpaa/base/qbman/qman.c +++ b/drivers/bus/dpaa/base/qbman/qman.c @@ -344,9 +344,9 @@ loop: if (!msg) return 0; } - if ((msg->verb & QM_MR_VERB_TYPE_MASK) != QM_MR_VERB_FQRNI) { + if ((msg->ern.verb & QM_MR_VERB_TYPE_MASK) != QM_MR_VERB_FQRNI) { /* We aren't draining anything but FQRNIs */ - pr_err("Found verb 0x%x in MR\n", msg->verb); + pr_err("Found verb 0x%x in MR\n", msg->ern.verb); return -1; } qm_mr_next(p); @@ -513,7 +513,7 @@ static inline void qm_mr_pvb_update(struct qm_portal *portal) /* when accessing 'verb', use __raw_readb() to ensure that compiler * inlining doesn't try to optimise out "excess reads". */ - if ((__raw_readb(&res->verb) & QM_MR_VERB_VBIT) == mr->vbit) { + if ((__raw_readb(&res->ern.verb) & QM_MR_VERB_VBIT) == mr->vbit) { mr->pi = (mr->pi + 1) & (QM_MR_SIZE - 1); if (!mr->pi) mr->vbit ^= QM_MR_VERB_VBIT; @@ -810,7 +810,7 @@ mr_loop: goto mr_done; swapped_msg = *msg; hw_fd_to_cpu(&swapped_msg.ern.fd); - verb = msg->verb & QM_MR_VERB_TYPE_MASK; + verb = msg->ern.verb & QM_MR_VERB_TYPE_MASK; /* The message is a software ERN iff the 0x20 bit is set */ if (verb & 0x20) { switch (verb) { @@ -1504,7 +1504,7 @@ int qman_retire_fq(struct qman_fq *fq, u32 *flags) */ struct qm_mr_entry msg; - msg.verb = QM_MR_VERB_FQRNI; + msg.ern.verb = QM_MR_VERB_FQRNI; msg.fq.fqs = mcr->alterfq.fqs; msg.fq.fqid = fq->fqid; #ifdef CONFIG_FSL_QMAN_FQ_LOOKUP @@ -2389,7 +2389,7 @@ int qman_shutdown_fq(u32 fqid) qm_mr_pvb_update(low_p); msg = qm_mr_current(low_p); while (msg) { - if ((msg->verb & + if ((msg->ern.verb & QM_MR_VERB_TYPE_MASK) == QM_MR_VERB_FQRN) found_fqrn = 1; @@ -2457,7 +2457,7 @@ int qman_shutdown_fq(u32 fqid) qm_mr_pvb_update(low_p); msg = qm_mr_current(low_p); while (msg) { - if ((msg->verb & QM_MR_VERB_TYPE_MASK) == + if ((msg->ern.verb & QM_MR_VERB_TYPE_MASK) == QM_MR_VERB_FQRL) orl_empty = 1; qm_mr_next(low_p); diff --git a/drivers/bus/dpaa/include/fsl_qman.h b/drivers/bus/dpaa/include/fsl_qman.h index 72556dc12..5d00bc73a 100644 --- a/drivers/bus/dpaa/include/fsl_qman.h +++ b/drivers/bus/dpaa/include/fsl_qman.h @@ -316,20 +316,20 @@ static inline dma_addr_t qm_sg_addr(const struct qm_sg_entry *sg) } while (0) /* See 1.5.8.1: "Enqueue Command" */ -struct qm_eqcr_entry { +struct __rte_aligned(8) qm_eqcr_entry { u8 __dont_write_directly__verb; u8 dca; u16 seqnum; u32 orp; /* 24-bit */ u32 fqid; /* 24-bit */ u32 tag; - struct qm_fd fd; + struct qm_fd fd; /* this has alignment 8 */ u8 __reserved3[32]; } __packed; /* "Frame Dequeue Response" */ -struct qm_dqrr_entry { +struct __rte_aligned(8) qm_dqrr_entry { u8 verb; u8 stat; u16 seqnum; /* 15-bit */ @@ -337,7 +337,7 @@ struct qm_dqrr_entry { u8 __reserved2[3]; u32 fqid; /* 24-bit */ u32 contextB; - struct qm_fd fd; + struct qm_fd fd; /* this has alignment 8 */ u8 __reserved4[32]; }; @@ -355,18 +355,19 @@ struct qm_dqrr_entry { /* "ERN Message Response" */ /* "FQ State Change Notification" */ struct qm_mr_entry { - u8 verb; union { struct { + u8 verb; u8 dca; u16 seqnum; u8 rc; /* Rejection Code */ u32 orp:24; u32 fqid; /* 24-bit */ u32 tag; - struct qm_fd fd; - } __packed ern; + struct qm_fd fd; /* this has alignment 8 */ + } __packed __rte_aligned(8) ern; struct { + u8 verb; #if __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__ u8 colour:2; /* See QM_MR_DCERN_COLOUR_* */ u8 __reserved1:4; @@ -381,18 +382,19 @@ struct qm_mr_entry { u32 __reserved3:24; u32 fqid; /* 24-bit */ u32 tag; - struct qm_fd fd; - } __packed dcern; + struct qm_fd fd; /* this has alignment 8 */ + } __packed __rte_aligned(8) dcern; struct { + u8 verb; u8 fqs; /* Frame Queue Status */ u8 __reserved1[6]; u32 fqid; /* 24-bit */ u32 contextB; u8 __reserved2[16]; - } __packed fq; /* FQRN/FQRNI/FQRL/FQPN */ + } __packed __rte_aligned(8) fq; /* FQRN/FQRNI/FQRL/FQPN */ }; u8 __reserved2[32]; -} __packed; +} __packed __rte_aligned(8); #define QM_MR_VERB_VBIT 0x80 /* * ERNs originating from direct-connect portals ("dcern") use 0x20 as a verb -- 2.11.0 --- Diff of the applied patch vs upstream commit (please double-check if non-empty: --- --- - 2019-01-02 23:59:12.646228426 -0800 +++ 0010-bus-dpaa-fix-inconsistent-struct-alignment.patch 2019-01-02 23:59:12.036815000 -0800 @@ -1,4 +1,4 @@ -From dd6f8d712e927397da27f2a3d90a759eeac4f694 Mon Sep 17 00:00:00 2001 +From 84f4ff2891bab8ddca99b73983f7c3108b532528 Mon Sep 17 00:00:00 2001 From: Andy Green Date: Mon, 14 May 2018 13:00:12 +0800 Subject: [PATCH] bus/dpaa: fix inconsistent struct alignment @@ -6,6 +6,8 @@ Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit +[ upstream commit dd6f8d712e927397da27f2a3d90a759eeac4f694 ] + The actual descriptor for qm_mr_entry is 64-byte aligned. But the original code plays a trick, and puts a u8 common @@ -34,7 +36,6 @@ Fixes: c47ff048b99a ("bus/dpaa: add QMAN driver core routines") Fixes: f6fadc3e6310 ("bus/dpaa: add QMAN interface driver") -Cc: stable@dpdk.org Signed-off-by: Andy Green Acked-by: Hemant Agrawal @@ -45,10 +46,10 @@ 2 files changed, 20 insertions(+), 18 deletions(-) diff --git a/drivers/bus/dpaa/base/qbman/qman.c b/drivers/bus/dpaa/base/qbman/qman.c -index 2810fdd26..27d98cc10 100644 +index b8511103c..84f0cb0be 100644 --- a/drivers/bus/dpaa/base/qbman/qman.c +++ b/drivers/bus/dpaa/base/qbman/qman.c -@@ -314,9 +314,9 @@ loop: +@@ -344,9 +344,9 @@ loop: if (!msg) return 0; } @@ -60,7 +61,7 @@ return -1; } qm_mr_next(p); -@@ -483,7 +483,7 @@ static inline void qm_mr_pvb_update(struct qm_portal *portal) +@@ -513,7 +513,7 @@ static inline void qm_mr_pvb_update(struct qm_portal *portal) /* when accessing 'verb', use __raw_readb() to ensure that compiler * inlining doesn't try to optimise out "excess reads". */ @@ -69,7 +70,7 @@ mr->pi = (mr->pi + 1) & (QM_MR_SIZE - 1); if (!mr->pi) mr->vbit ^= QM_MR_VERB_VBIT; -@@ -832,7 +832,7 @@ mr_loop: +@@ -810,7 +810,7 @@ mr_loop: goto mr_done; swapped_msg = *msg; hw_fd_to_cpu(&swapped_msg.ern.fd); @@ -78,7 +79,7 @@ /* The message is a software ERN iff the 0x20 bit is set */ if (verb & 0x20) { switch (verb) { -@@ -1666,7 +1666,7 @@ int qman_retire_fq(struct qman_fq *fq, u32 *flags) +@@ -1504,7 +1504,7 @@ int qman_retire_fq(struct qman_fq *fq, u32 *flags) */ struct qm_mr_entry msg; @@ -87,7 +88,7 @@ msg.fq.fqs = mcr->alterfq.fqs; msg.fq.fqid = fq->fqid; #ifdef CONFIG_FSL_QMAN_FQ_LOOKUP -@@ -2643,7 +2643,7 @@ int qman_shutdown_fq(u32 fqid) +@@ -2389,7 +2389,7 @@ int qman_shutdown_fq(u32 fqid) qm_mr_pvb_update(low_p); msg = qm_mr_current(low_p); while (msg) { @@ -96,7 +97,7 @@ QM_MR_VERB_TYPE_MASK) == QM_MR_VERB_FQRN) found_fqrn = 1; -@@ -2711,7 +2711,7 @@ int qman_shutdown_fq(u32 fqid) +@@ -2457,7 +2457,7 @@ int qman_shutdown_fq(u32 fqid) qm_mr_pvb_update(low_p); msg = qm_mr_current(low_p); while (msg) { @@ -106,10 +107,10 @@ orl_empty = 1; qm_mr_next(low_p); diff --git a/drivers/bus/dpaa/include/fsl_qman.h b/drivers/bus/dpaa/include/fsl_qman.h -index e9793f30d..e4ad7ae48 100644 +index 72556dc12..5d00bc73a 100644 --- a/drivers/bus/dpaa/include/fsl_qman.h +++ b/drivers/bus/dpaa/include/fsl_qman.h -@@ -284,20 +284,20 @@ static inline dma_addr_t qm_sg_addr(const struct qm_sg_entry *sg) +@@ -316,20 +316,20 @@ static inline dma_addr_t qm_sg_addr(const struct qm_sg_entry *sg) } while (0) /* See 1.5.8.1: "Enqueue Command" */ @@ -133,7 +134,7 @@ u8 verb; u8 stat; u16 seqnum; /* 15-bit */ -@@ -305,7 +305,7 @@ struct qm_dqrr_entry { +@@ -337,7 +337,7 @@ struct qm_dqrr_entry { u8 __reserved2[3]; u32 fqid; /* 24-bit */ u32 contextB; @@ -142,7 +143,7 @@ u8 __reserved4[32]; }; -@@ -323,18 +323,19 @@ struct qm_dqrr_entry { +@@ -355,18 +355,19 @@ struct qm_dqrr_entry { /* "ERN Message Response" */ /* "FQ State Change Notification" */ struct qm_mr_entry { @@ -165,7 +166,7 @@ #if __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__ u8 colour:2; /* See QM_MR_DCERN_COLOUR_* */ u8 __reserved1:4; -@@ -349,18 +350,19 @@ struct qm_mr_entry { +@@ -381,18 +382,19 @@ struct qm_mr_entry { u32 __reserved3:24; u32 fqid; /* 24-bit */ u32 tag;