* 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).