2023-10-06 12:04:13

by Shinas Rasheed

[permalink] [raw]
Subject: [net-next PATCH] octeon_ep: pack hardware structures

Add packed attribute to hardware structures.

Signed-off-by: Shinas Rasheed <[email protected]>
---
drivers/net/ethernet/marvell/octeon_ep/octep_rx.h | 6 +++---
drivers/net/ethernet/marvell/octeon_ep/octep_tx.h | 8 ++++----
2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/marvell/octeon_ep/octep_rx.h b/drivers/net/ethernet/marvell/octeon_ep/octep_rx.h
index 782a24f27f3e..ca42ddb77491 100644
--- a/drivers/net/ethernet/marvell/octeon_ep/octep_rx.h
+++ b/drivers/net/ethernet/marvell/octeon_ep/octep_rx.h
@@ -19,7 +19,7 @@
struct octep_oq_desc_hw {
dma_addr_t buffer_ptr;
u64 info_ptr;
-};
+} __packed;

#define OCTEP_OQ_DESC_SIZE (sizeof(struct octep_oq_desc_hw))

@@ -38,7 +38,7 @@ struct octep_oq_resp_hw_ext {

/* checksum verified. */
u64 csum_verified:2;
-};
+} __packed;

#define OCTEP_OQ_RESP_HW_EXT_SIZE (sizeof(struct octep_oq_resp_hw_ext))

@@ -49,7 +49,7 @@ struct octep_oq_resp_hw_ext {
struct octep_oq_resp_hw {
/* The Length of the packet. */
__be64 length;
-};
+} __packed;

#define OCTEP_OQ_RESP_HW_SIZE (sizeof(struct octep_oq_resp_hw))

diff --git a/drivers/net/ethernet/marvell/octeon_ep/octep_tx.h b/drivers/net/ethernet/marvell/octeon_ep/octep_tx.h
index 21e75ff9f5e7..74189e5a7d33 100644
--- a/drivers/net/ethernet/marvell/octeon_ep/octep_tx.h
+++ b/drivers/net/ethernet/marvell/octeon_ep/octep_tx.h
@@ -35,7 +35,7 @@
struct octep_tx_sglist_desc {
u16 len[4];
dma_addr_t dma_ptr[4];
-};
+} __packed;

/* Each Scatter/Gather entry sent to hardwar hold four pointers.
* So, number of entries required is (MAX_SKB_FRAGS + 1)/4, where '+1'
@@ -238,7 +238,7 @@ struct octep_instr_hdr {

/* Reserved3 */
u64 reserved3:1;
-};
+} __packed;

/* Hardware Tx completion response header */
struct octep_instr_resp_hdr {
@@ -262,7 +262,7 @@ struct octep_instr_resp_hdr {

/* Opcode for the return packet */
u64 opcode:16;
-};
+} __packed;

/* 64-byte Tx instruction format.
* Format of instruction for a 64-byte mode input queue.
@@ -292,7 +292,7 @@ struct octep_tx_desc_hw {

/* Additional headers available in a 64-byte instruction. */
u64 exhdr[4];
-};
+} __packed;

#define OCTEP_IQ_DESC_SIZE (sizeof(struct octep_tx_desc_hw))
#endif /* _OCTEP_TX_H_ */
--
2.25.1


2023-10-10 09:30:01

by Paolo Abeni

[permalink] [raw]
Subject: Re: [net-next PATCH] octeon_ep: pack hardware structures

On Fri, 2023-10-06 at 05:02 -0700, Shinas Rasheed wrote:
> Add packed attribute to hardware structures.

Could you please elaborate a bit more why this is needed? Is this a
bugfix? Or something needed by later changes? Please update the
changelog accordingly

Thanks!

Paolo

2023-10-10 19:20:45

by Shinas Rasheed

[permalink] [raw]
Subject: Re: [EXT] Re: [net-next PATCH] octeon_ep: pack hardware structures

Hi Paolo,

Could you please elaborate a bit more why this is needed?
>>> This is a necessary commit as these structures should be packed since they represent hardware data structures and padding is not allowed by hardware.

Is this a bugfix?
>>> There are no issues observed due to this, but this should be updated anyway.

Please update the changelog accordingly
>>> I shall include more information in the changelog as well.

Thanks!

Paolo

2023-10-10 19:41:17

by Shinas Rasheed

[permalink] [raw]
Subject: [net-next PATCH v2] octeon_ep: pack hardware structures

Add packed attribute to structures correlating to hardware
data, as padding is not allowed by hardware.

Signed-off-by: Shinas Rasheed <[email protected]>
---
V1 -> V2: Updated changelog

drivers/net/ethernet/marvell/octeon_ep/octep_rx.h | 6 +++---
drivers/net/ethernet/marvell/octeon_ep/octep_tx.h | 8 ++++----
2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/marvell/octeon_ep/octep_rx.h b/drivers/net/ethernet/marvell/octeon_ep/octep_rx.h
index 782a24f27f3e..ca42ddb77491 100644
--- a/drivers/net/ethernet/marvell/octeon_ep/octep_rx.h
+++ b/drivers/net/ethernet/marvell/octeon_ep/octep_rx.h
@@ -19,7 +19,7 @@
struct octep_oq_desc_hw {
dma_addr_t buffer_ptr;
u64 info_ptr;
-};
+} __packed;

#define OCTEP_OQ_DESC_SIZE (sizeof(struct octep_oq_desc_hw))

@@ -38,7 +38,7 @@ struct octep_oq_resp_hw_ext {

/* checksum verified. */
u64 csum_verified:2;
-};
+} __packed;

#define OCTEP_OQ_RESP_HW_EXT_SIZE (sizeof(struct octep_oq_resp_hw_ext))

@@ -49,7 +49,7 @@ struct octep_oq_resp_hw_ext {
struct octep_oq_resp_hw {
/* The Length of the packet. */
__be64 length;
-};
+} __packed;

#define OCTEP_OQ_RESP_HW_SIZE (sizeof(struct octep_oq_resp_hw))

diff --git a/drivers/net/ethernet/marvell/octeon_ep/octep_tx.h b/drivers/net/ethernet/marvell/octeon_ep/octep_tx.h
index 21e75ff9f5e7..74189e5a7d33 100644
--- a/drivers/net/ethernet/marvell/octeon_ep/octep_tx.h
+++ b/drivers/net/ethernet/marvell/octeon_ep/octep_tx.h
@@ -35,7 +35,7 @@
struct octep_tx_sglist_desc {
u16 len[4];
dma_addr_t dma_ptr[4];
-};
+} __packed;

/* Each Scatter/Gather entry sent to hardwar hold four pointers.
* So, number of entries required is (MAX_SKB_FRAGS + 1)/4, where '+1'
@@ -238,7 +238,7 @@ struct octep_instr_hdr {

/* Reserved3 */
u64 reserved3:1;
-};
+} __packed;

/* Hardware Tx completion response header */
struct octep_instr_resp_hdr {
@@ -262,7 +262,7 @@ struct octep_instr_resp_hdr {

/* Opcode for the return packet */
u64 opcode:16;
-};
+} __packed;

/* 64-byte Tx instruction format.
* Format of instruction for a 64-byte mode input queue.
@@ -292,7 +292,7 @@ struct octep_tx_desc_hw {

/* Additional headers available in a 64-byte instruction. */
u64 exhdr[4];
-};
+} __packed;

#define OCTEP_IQ_DESC_SIZE (sizeof(struct octep_tx_desc_hw))
#endif /* _OCTEP_TX_H_ */
--
2.25.1

2023-10-13 11:17:01

by Simon Horman

[permalink] [raw]
Subject: Re: [net-next PATCH v2] octeon_ep: pack hardware structures

On Tue, Oct 10, 2023 at 12:40:26PM -0700, Shinas Rasheed wrote:
> Add packed attribute to structures correlating to hardware
> data, as padding is not allowed by hardware.

Hi Shinas,

in the v1 you stated that this is bot a bugfix.
So I am assuming, that by some happy coincidence,
the layout of the structures is the same before and after
this patch. And this patch is thus a cleanup.

If so I think it would be worth noting in the patch description.