* Re: [PATCH] soc: fsl: Remove bogus packed attributes from qman.h [not found] ` <VE1PR04MB6687FB075B9A6A0923F576978F2E0@VE1PR04MB6687.eurprd04.prod.outlook.com> @ 2020-09-01 1:56 ` Herbert Xu 2020-09-01 21:40 ` Li Yang 0 siblings, 1 reply; 3+ messages in thread From: Herbert Xu @ 2020-09-01 1:56 UTC (permalink / raw) To: Leo Li; +Cc: linuxppc-dev, linux-arm-kernel, Linux Kernel Mailing List On Tue, Sep 01, 2020 at 01:50:38AM +0000, Leo Li wrote: > > Sorry for the late response. I missed this email previously. > > These structures are descriptors used by hardware, we cannot have _ANY_ padding from the compiler. The compiled result might be the same with or without the __packed attribute for now, but I think keep it there probably is safer for dealing with unexpected alignment requirements from the compiler in the future. > > Having conflicting alignment requirements warning might means something is wrong with the structure in certain scenario. I just tried a ARM64 build but didn't see the warnings. Could you share the warning you got and the build setup? Thanks. Just do a COMPILE_TEST build on x86-64: In file included from ../drivers/crypto/caam/qi.c:12: ../include/soc/fsl/qman.h:259:1: warning: alignment 1 of ‘struct qm_dqrr_entry’ is less than 8 [-Wpacked-not-aligned] } __packed; ^ ../include/soc/fsl/qman.h:292:2: warning: alignment 1 of ‘struct <anonymous>’ is less than 8 [-Wpacked-not-aligned] } __packed ern; ^ In any case, those packed markers are completely unnecessary because those structs contain no holes. Cheers, -- Email: Herbert Xu <herbert@gondor.apana.org.au> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] soc: fsl: Remove bogus packed attributes from qman.h 2020-09-01 1:56 ` [PATCH] soc: fsl: Remove bogus packed attributes from qman.h Herbert Xu @ 2020-09-01 21:40 ` Li Yang 2020-09-02 1:34 ` Herbert Xu 0 siblings, 1 reply; 3+ messages in thread From: Li Yang @ 2020-09-01 21:40 UTC (permalink / raw) To: Herbert Xu; +Cc: linuxppc-dev, linux-arm-kernel, Linux Kernel Mailing List On Mon, Aug 31, 2020 at 8:57 PM Herbert Xu <herbert@gondor.apana.org.au> wrote: > > On Tue, Sep 01, 2020 at 01:50:38AM +0000, Leo Li wrote: > > > > Sorry for the late response. I missed this email previously. > > > > These structures are descriptors used by hardware, we cannot have _ANY_ padding from the compiler. The compiled result might be the same with or without the __packed attribute for now, but I think keep it there probably is safer for dealing with unexpected alignment requirements from the compiler in the future. > > > > Having conflicting alignment requirements warning might means something is wrong with the structure in certain scenario. I just tried a ARM64 build but didn't see the warnings. Could you share the warning you got and the build setup? Thanks. > > Just do a COMPILE_TEST build on x86-64: > > In file included from ../drivers/crypto/caam/qi.c:12: Looks like the CAAM driver and dependent QBMAN driver doesn't support COMPILE_TEST yet. Are you trying to add the support for it? I changed the Kconfig to enable the COMPILE_TEST anyway and updated my toolchain to gcc-10 trying to duplicate the issue. The issues can only be reproduced with "W=1". > ../include/soc/fsl/qman.h:259:1: warning: alignment 1 of ‘struct qm_dqrr_entry’ is less than 8 [-Wpacked-not-aligned] > } __packed; > ^ > ../include/soc/fsl/qman.h:292:2: warning: alignment 1 of ‘struct <anonymous>’ is less than 8 [-Wpacked-not-aligned] > } __packed ern; > ^ I think this is a valid concern that if the parent structure doesn't meet certain alignment requirements, the alignment for the sub-structure cannot be guaranteed. If we just remove the __packed attribute from the parent structure, the compiler could try to add padding in the parent structure to fulfill the alignment requirements of the sub structure which is not good. I think the following changes are a better fix for the warnings: diff --git a/include/soc/fsl/qman.h b/include/soc/fsl/qman.h index cfe00e08e85b..9f484113cfda 100644 --- a/include/soc/fsl/qman.h +++ b/include/soc/fsl/qman.h @@ -256,7 +256,7 @@ struct qm_dqrr_entry { __be32 context_b; struct qm_fd fd; u8 __reserved4[32]; -} __packed; +} __packed __aligned(64); #define QM_DQRR_VERB_VBIT 0x80 #define QM_DQRR_VERB_MASK 0x7f /* where the verb contains; */ #define QM_DQRR_VERB_FRAME_DEQUEUE 0x60 /* "this format" */ @@ -289,7 +289,7 @@ union qm_mr_entry { __be32 tag; struct qm_fd fd; u8 __reserved1[32]; - } __packed ern; + } __packed __aligned(64) ern; struct { u8 verb; u8 fqs; /* Frame Queue Status */ Regards, Leo ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] soc: fsl: Remove bogus packed attributes from qman.h 2020-09-01 21:40 ` Li Yang @ 2020-09-02 1:34 ` Herbert Xu 0 siblings, 0 replies; 3+ messages in thread From: Herbert Xu @ 2020-09-02 1:34 UTC (permalink / raw) To: Li Yang; +Cc: linuxppc-dev, linux-arm-kernel, Linux Kernel Mailing List On Tue, Sep 01, 2020 at 04:40:16PM -0500, Li Yang wrote: > > Looks like the CAAM driver and dependent QBMAN driver doesn't support > COMPILE_TEST yet. Are you trying to add the support for it? Yes. > I think this is a valid concern that if the parent structure doesn't > meet certain alignment requirements, the alignment for the > sub-structure cannot be guaranteed. If we just remove the __packed > attribute from the parent structure, the compiler could try to add > padding in the parent structure to fulfill the alignment requirements > of the sub structure which is not good. I think the following changes > are a better fix for the warnings: This works for me. Thanks! -- Email: Herbert Xu <herbert@gondor.apana.org.au> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-09-02 1:34 UTC | newest] Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <20200730125259.GA8948@gondor.apana.org.au> [not found] ` <VE1PR04MB6687FB075B9A6A0923F576978F2E0@VE1PR04MB6687.eurprd04.prod.outlook.com> 2020-09-01 1:56 ` [PATCH] soc: fsl: Remove bogus packed attributes from qman.h Herbert Xu 2020-09-01 21:40 ` Li Yang 2020-09-02 1:34 ` Herbert Xu
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).