On Mon, Aug 31, 2020 at 8:57 PM Herbert Xu <herb...@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