From: Arnd Bergmann <[email protected]>
clang points out that the lpfc_name structure has an 8-byte alignement requirement
on most architectures, but is embedded in a number of other structures that are
forced to be only 1-byte aligned:
drivers/scsi/lpfc/lpfc_hw.h:1516:30: error: field pe within 'struct lpfc_fdmi_reg_port_list' is less aligned than 'struct lpfc_fdmi_port_entry' and is usually due to 'struct lpfc_fdmi_reg_port_list' being packed, which can lead to unaligned accesses [-Werror,-Wunaligned-access]
struct lpfc_fdmi_port_entry pe;
drivers/scsi/lpfc/lpfc_hw.h:850:19: error: field portName within 'struct _ADISC' is less aligned than 'struct lpfc_name' and is usually due to 'struct _ADISC' being packed, which can lead to unaligned accesses [-Werror,-Wunaligned-access]
drivers/scsi/lpfc/lpfc_hw.h:851:19: error: field nodeName within 'struct _ADISC' is less aligned than 'struct lpfc_name' and is usually due to 'struct _ADISC' being packed, which can lead to unaligned accesses [-Werror,-Wunaligned-access]
drivers/scsi/lpfc/lpfc_hw.h:922:19: error: field portName within 'struct _RNID' is less aligned than 'struct lpfc_name' and is usually due to 'struct _RNID' being packed, which can lead to unaligned accesses [-Werror,-Wunaligned-access]
drivers/scsi/lpfc/lpfc_hw.h:923:19: error: field nodeName within 'struct _RNID' is less aligned than 'struct lpfc_name' and is usually due to 'struct _RNID' being packed, which can lead to unaligned accesses [-Werror,-Wunaligned-access]
From the git history, I can see that all the __packed annotations were done
specifically to avoid introducing implicit padding around the lpfc_name
instances, though this was probably the wrong approach.
To improve this, only annotate the one uint64_t field inside of lpfc_name
as packed, with an explicit 4-byte alignment, as is the default already on
the 32-bit x86 ABI but not on most others. With this, the other __packed
annotations can be removed again, as this avoids the incorrect padding.
Two other structures change their layout as a result of this change:
- struct _LOGO never gained a __packed annotation even though it has the
same alignment problem as the others but is not used anywhere in the
driver today.
- struct serv_param similarly has this issue, and it is used, my guess
is that this is only an internal structure rather than part of a binary
interface, so the padding has no negative effect here.
Signed-off-by: Arnd Bergmann <[email protected]>
---
drivers/scsi/lpfc/lpfc_hw.h | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/scsi/lpfc/lpfc_hw.h b/drivers/scsi/lpfc/lpfc_hw.h
index 663755842e4a4..aaea3e31944d0 100644
--- a/drivers/scsi/lpfc/lpfc_hw.h
+++ b/drivers/scsi/lpfc/lpfc_hw.h
@@ -365,7 +365,7 @@ struct lpfc_name {
uint8_t IEEE[6]; /* FC IEEE address */
} s;
uint8_t wwn[8];
- uint64_t name;
+ uint64_t name __packed __aligned(4);
} u;
};
@@ -850,7 +850,7 @@ typedef struct _ADISC { /* Structure is in Big Endian format */
struct lpfc_name portName;
struct lpfc_name nodeName;
uint32_t DID;
-} __packed ADISC;
+} ADISC;
typedef struct _FARP { /* Structure is in Big Endian format */
uint32_t Mflags:8;
@@ -880,7 +880,7 @@ typedef struct _FAN { /* Structure is in Big Endian format */
uint32_t Fdid;
struct lpfc_name FportName;
struct lpfc_name FnodeName;
-} __packed FAN;
+} FAN;
typedef struct _SCR { /* Structure is in Big Endian format */
uint8_t resvd1;
@@ -924,7 +924,7 @@ typedef struct _RNID { /* Structure is in Big Endian format */
union {
RNID_TOP_DISC topologyDisc; /* topology disc (0xdf) */
} un;
-} __packed RNID;
+} RNID;
struct RLS { /* Structure is in Big Endian format */
uint32_t rls;
@@ -1514,7 +1514,7 @@ struct lpfc_fdmi_hba_ident {
struct lpfc_fdmi_reg_port_list {
__be32 EntryCnt;
struct lpfc_fdmi_port_entry pe;
-} __packed;
+};
/*
* Register HBA(RHBA)
--
2.39.2
Hi Arnd,
Thanks looks good.
Reviewed-by: Justin Tee <[email protected]>
On Fri, Jun 16, 2023 at 2:07 AM Arnd Bergmann <[email protected]> wrote:
>
> From: Arnd Bergmann <[email protected]>
>
> clang points out that the lpfc_name structure has an 8-byte alignement requirement
> on most architectures, but is embedded in a number of other structures that are
> forced to be only 1-byte aligned:
>
> drivers/scsi/lpfc/lpfc_hw.h:1516:30: error: field pe within 'struct lpfc_fdmi_reg_port_list' is less aligned than 'struct lpfc_fdmi_port_entry' and is usually due to 'struct lpfc_fdmi_reg_port_list' being packed, which can lead to unaligned accesses [-Werror,-Wunaligned-access]
> struct lpfc_fdmi_port_entry pe;
> drivers/scsi/lpfc/lpfc_hw.h:850:19: error: field portName within 'struct _ADISC' is less aligned than 'struct lpfc_name' and is usually due to 'struct _ADISC' being packed, which can lead to unaligned accesses [-Werror,-Wunaligned-access]
> drivers/scsi/lpfc/lpfc_hw.h:851:19: error: field nodeName within 'struct _ADISC' is less aligned than 'struct lpfc_name' and is usually due to 'struct _ADISC' being packed, which can lead to unaligned accesses [-Werror,-Wunaligned-access]
> drivers/scsi/lpfc/lpfc_hw.h:922:19: error: field portName within 'struct _RNID' is less aligned than 'struct lpfc_name' and is usually due to 'struct _RNID' being packed, which can lead to unaligned accesses [-Werror,-Wunaligned-access]
> drivers/scsi/lpfc/lpfc_hw.h:923:19: error: field nodeName within 'struct _RNID' is less aligned than 'struct lpfc_name' and is usually due to 'struct _RNID' being packed, which can lead to unaligned accesses [-Werror,-Wunaligned-access]
>
> From the git history, I can see that all the __packed annotations were done
> specifically to avoid introducing implicit padding around the lpfc_name
> instances, though this was probably the wrong approach.
>
> To improve this, only annotate the one uint64_t field inside of lpfc_name
> as packed, with an explicit 4-byte alignment, as is the default already on
> the 32-bit x86 ABI but not on most others. With this, the other __packed
> annotations can be removed again, as this avoids the incorrect padding.
>
> Two other structures change their layout as a result of this change:
>
> - struct _LOGO never gained a __packed annotation even though it has the
> same alignment problem as the others but is not used anywhere in the
> driver today.
>
> - struct serv_param similarly has this issue, and it is used, my guess
> is that this is only an internal structure rather than part of a binary
> interface, so the padding has no negative effect here.
>
> Signed-off-by: Arnd Bergmann <[email protected]>
> ---
> drivers/scsi/lpfc/lpfc_hw.h | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/scsi/lpfc/lpfc_hw.h b/drivers/scsi/lpfc/lpfc_hw.h
> index 663755842e4a4..aaea3e31944d0 100644
> --- a/drivers/scsi/lpfc/lpfc_hw.h
> +++ b/drivers/scsi/lpfc/lpfc_hw.h
> @@ -365,7 +365,7 @@ struct lpfc_name {
> uint8_t IEEE[6]; /* FC IEEE address */
> } s;
> uint8_t wwn[8];
> - uint64_t name;
> + uint64_t name __packed __aligned(4);
> } u;
> };
>
> @@ -850,7 +850,7 @@ typedef struct _ADISC { /* Structure is in Big Endian format */
> struct lpfc_name portName;
> struct lpfc_name nodeName;
> uint32_t DID;
> -} __packed ADISC;
> +} ADISC;
>
> typedef struct _FARP { /* Structure is in Big Endian format */
> uint32_t Mflags:8;
> @@ -880,7 +880,7 @@ typedef struct _FAN { /* Structure is in Big Endian format */
> uint32_t Fdid;
> struct lpfc_name FportName;
> struct lpfc_name FnodeName;
> -} __packed FAN;
> +} FAN;
>
> typedef struct _SCR { /* Structure is in Big Endian format */
> uint8_t resvd1;
> @@ -924,7 +924,7 @@ typedef struct _RNID { /* Structure is in Big Endian format */
> union {
> RNID_TOP_DISC topologyDisc; /* topology disc (0xdf) */
> } un;
> -} __packed RNID;
> +} RNID;
>
> struct RLS { /* Structure is in Big Endian format */
> uint32_t rls;
> @@ -1514,7 +1514,7 @@ struct lpfc_fdmi_hba_ident {
> struct lpfc_fdmi_reg_port_list {
> __be32 EntryCnt;
> struct lpfc_fdmi_port_entry pe;
> -} __packed;
> +};
>
> /*
> * Register HBA(RHBA)
> --
> 2.39.2
>
--
This electronic communication and the information and any files transmitted
with it, or attached to it, are confidential and are intended solely for
the use of the individual or entity to whom it is addressed and may contain
information that is confidential, legally privileged, protected by privacy
laws, or otherwise restricted from disclosure to anyone else. If you are
not the intended recipient or the person responsible for delivering the
e-mail to the intended recipient, you are hereby notified that any use,
copying, distributing, dissemination, forwarding, printing, or copying of
this e-mail is strictly prohibited. If you received this e-mail in error,
please return the e-mail to the sender, delete it from your computer, and
destroy any printed copy of it.
Arnd,
> clang points out that the lpfc_name structure has an 8-byte alignement
> requirement on most architectures, but is embedded in a number of
> other structures that are forced to be only 1-byte aligned:
Applied to 6.5/scsi-staging, thanks!
--
Martin K. Petersen Oracle Linux Engineering
On Fri, 16 Jun 2023 11:06:56 +0200, Arnd Bergmann wrote:
> clang points out that the lpfc_name structure has an 8-byte alignement requirement
> on most architectures, but is embedded in a number of other structures that are
> forced to be only 1-byte aligned:
>
> drivers/scsi/lpfc/lpfc_hw.h:1516:30: error: field pe within 'struct lpfc_fdmi_reg_port_list' is less aligned than 'struct lpfc_fdmi_port_entry' and is usually due to 'struct lpfc_fdmi_reg_port_list' being packed, which can lead to unaligned accesses [-Werror,-Wunaligned-access]
> struct lpfc_fdmi_port_entry pe;
> drivers/scsi/lpfc/lpfc_hw.h:850:19: error: field portName within 'struct _ADISC' is less aligned than 'struct lpfc_name' and is usually due to 'struct _ADISC' being packed, which can lead to unaligned accesses [-Werror,-Wunaligned-access]
> drivers/scsi/lpfc/lpfc_hw.h:851:19: error: field nodeName within 'struct _ADISC' is less aligned than 'struct lpfc_name' and is usually due to 'struct _ADISC' being packed, which can lead to unaligned accesses [-Werror,-Wunaligned-access]
> drivers/scsi/lpfc/lpfc_hw.h:922:19: error: field portName within 'struct _RNID' is less aligned than 'struct lpfc_name' and is usually due to 'struct _RNID' being packed, which can lead to unaligned accesses [-Werror,-Wunaligned-access]
> drivers/scsi/lpfc/lpfc_hw.h:923:19: error: field nodeName within 'struct _RNID' is less aligned than 'struct lpfc_name' and is usually due to 'struct _RNID' being packed, which can lead to unaligned accesses [-Werror,-Wunaligned-access]
>
> [...]
Applied to 6.5/scsi-queue, thanks!
[1/1] scsi: lpfc: fix lpfc_name struct packing
https://git.kernel.org/mkp/scsi/c/00c2cae6b66a
--
Martin K. Petersen Oracle Linux Engineering