]> git.ipfire.org Git - thirdparty/kernel/linux.git/commitdiff
net: rnpgbe: fix mailbox endianness and remove pointer casts
authorDong Yibo <dong100@mucse.com>
Wed, 1 Jul 2026 03:22:08 +0000 (11:22 +0800)
committerPaolo Abeni <pabeni@redhat.com>
Tue, 7 Jul 2026 08:45:42 +0000 (10:45 +0200)
The rnpgbe mailbox exchanges data through 32-bit MMIO registers in
little-endian wire format. The original code had two problems:

  1. FW structs (with __le16/__le32 fields) were cast to (u32 *)
     before reaching the mailbox transport, hiding the endian
     annotations from sparse.

  2. No cpu_to_le32()/le32_to_cpu() conversion was done between
     CPU-endian MMIO values and the little-endian payload, causing
     data corruption on big-endian systems.

Fix by adding the missing byte-order conversions in the transport
layer and introducing union wrappers (mbx_fw_cmd_req_u,
mbx_fw_cmd_reply_u) that overlay each FW struct with a __le32
dwords[] array. Callers fill named fields using cpu_to_le16/32(),
then pass dwords[] to the transport, which now takes explicit
__le32 * instead of u32 *. This eliminates all pointer casts on
the mailbox data path and lets sparse verify the conversions.

Fixes: 4543534c3ef5 ("net: rnpgbe: Add basic mbx ops support")
Signed-off-by: Dong Yibo <dong100@mucse.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Link: https://patch.msgid.link/20260701032208.1843156-2-dong100@mucse.com
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx.c
drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx.h
drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx_fw.c
drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx_fw.h

index de5e29230b3c8deb12f1da98ae9bc1d2bc92f8e0..c46408698263251b8f148c19ff4b1cef93255be8 100644 (file)
@@ -166,18 +166,23 @@ static void mucse_mbx_inc_pf_ack(struct mucse_hw *hw)
  *
  * Return: 0 on success, negative errno on failure
  **/
-static int mucse_read_mbx_pf(struct mucse_hw *hw, u32 *msg, u16 size)
+static int mucse_read_mbx_pf(struct mucse_hw *hw, __le32 *msg, u16 size)
 {
-       const int size_in_words = size / sizeof(u32);
+       const int size_in_words = size / sizeof(__le32);
        struct mucse_mbx_info *mbx = &hw->mbx;
+       int off = MUCSE_MBX_FWPF_SHM;
        int err;
 
        err = mucse_obtain_mbx_lock_pf(hw);
        if (err)
                return err;
 
+       /* memcpy_fromio() is unsuitable: the mailbox uses 32-bit MMIO
+        * registers, not byte-addressable RAM. readl() guarantees
+        * the required 32-bit access width.
+        */
        for (int i = 0; i < size_in_words; i++)
-               msg[i] = mbx_data_rd32(mbx, MUCSE_MBX_FWPF_SHM + 4 * i);
+               msg[i] = cpu_to_le32(mbx_data_rd32(mbx, off + 4 * i));
        /* Hw needs write data_reg at last */
        mbx_data_wr32(mbx, MUCSE_MBX_FWPF_SHM, 0);
        /* flush reqs as we have read this request data */
@@ -236,7 +241,7 @@ static int mucse_poll_for_msg(struct mucse_hw *hw)
  * Return: 0 if it successfully received a message notification and
  * copied it into the receive buffer, negative errno on failure
  **/
-int mucse_poll_and_read_mbx(struct mucse_hw *hw, u32 *msg, u16 size)
+int mucse_poll_and_read_mbx(struct mucse_hw *hw, __le32 *msg, u16 size)
 {
        int err;
 
@@ -290,9 +295,9 @@ static void mucse_mbx_inc_pf_req(struct mucse_hw *hw)
  * Return: 0 if it successfully copied message into the buffer,
  * negative errno on failure
  **/
-static int mucse_write_mbx_pf(struct mucse_hw *hw, u32 *msg, u16 size)
+static int mucse_write_mbx_pf(struct mucse_hw *hw, const __le32 *msg, u16 size)
 {
-       const int size_in_words = size / sizeof(u32);
+       const int size_in_words = size / sizeof(__le32);
        struct mucse_mbx_info *mbx = &hw->mbx;
        int err;
 
@@ -300,8 +305,12 @@ static int mucse_write_mbx_pf(struct mucse_hw *hw, u32 *msg, u16 size)
        if (err)
                return err;
 
+       /* memcpy_toio() would decompose into arbitrary-width accesses;
+        * the mailbox requires 32-bit MMIO writes via writel().
+        */
        for (int i = 0; i < size_in_words; i++)
-               mbx_data_wr32(mbx, MUCSE_MBX_FWPF_SHM + i * 4, msg[i]);
+               mbx_data_wr32(mbx, MUCSE_MBX_FWPF_SHM + i * 4,
+                             le32_to_cpu(msg[i]));
 
        /* flush acks as we are overwriting the message buffer */
        hw->mbx.fw_ack = mucse_mbx_get_fwack(mbx);
@@ -360,7 +369,8 @@ static int mucse_poll_for_ack(struct mucse_hw *hw)
  * Return: 0 if it successfully copied message into the buffer and
  * received an ack to that message within delay * timeout_cnt period
  **/
-int mucse_write_and_wait_ack_mbx(struct mucse_hw *hw, u32 *msg, u16 size)
+int mucse_write_and_wait_ack_mbx(struct mucse_hw *hw, const __le32 *msg,
+                                u16 size)
 {
        int err;
 
index e6fcc8d1d3ca72c548c4ac1286327126c183cada..75b88b18b04d34b67a35a6150bc82f4499c2c947 100644 (file)
@@ -14,7 +14,8 @@
 #define MUCSE_MBX_REQ             BIT(0) /* Request a req to mailbox */
 #define MUCSE_MBX_PFU             BIT(3) /* PF owns the mailbox buffer */
 
-int mucse_write_and_wait_ack_mbx(struct mucse_hw *hw, u32 *msg, u16 size);
+int mucse_write_and_wait_ack_mbx(struct mucse_hw *hw,
+                                const __le32 *msg, u16 size);
 void mucse_init_mbx_params_pf(struct mucse_hw *hw);
-int mucse_poll_and_read_mbx(struct mucse_hw *hw, u32 *msg, u16 size);
+int mucse_poll_and_read_mbx(struct mucse_hw *hw, __le32 *msg, u16 size);
 #endif /* _RNPGBE_MBX_H */
index 8c8bd5e8e1db10300106c9cfb0faa1a94dee4061..5ba74997beacf25ca58be367f300b7cd7a90c0bf 100644 (file)
  * Return: 0 on success, negative errno on failure
  **/
 static int mucse_fw_send_cmd_wait_resp(struct mucse_hw *hw,
-                                      struct mbx_fw_cmd_req *req,
-                                      struct mbx_fw_cmd_reply *reply)
+                                      union mbx_fw_cmd_req_u *req,
+                                      union mbx_fw_cmd_reply_u *reply)
 {
-       int len = le16_to_cpu(req->datalen);
+       int len = le16_to_cpu(req->r.datalen);
        int retry_cnt = 3;
        int err;
 
        mutex_lock(&hw->mbx.lock);
-       err = mucse_write_and_wait_ack_mbx(hw, (u32 *)req, len);
+       err = mucse_write_and_wait_ack_mbx(hw, req->dwords, len);
        if (err)
                goto out;
        do {
-               err = mucse_poll_and_read_mbx(hw, (u32 *)reply,
-                                             sizeof(*reply));
+               err = mucse_poll_and_read_mbx(hw, reply->dwords,
+                                             sizeof(reply->r));
                if (err)
                        goto out;
                /* mucse_write_and_wait_ack_mbx return 0 means fw has
                 * received request, wait for the expect opcode
                 * reply with 'retry_cnt' times.
                 */
-       } while (--retry_cnt >= 0 && reply->opcode != req->opcode);
+       } while (--retry_cnt >= 0 && reply->r.opcode != req->r.opcode);
 out:
        mutex_unlock(&hw->mbx.lock);
        if (!err && retry_cnt < 0)
                return -ETIMEDOUT;
-       if (!err && reply->error_code)
+       if (!err && reply->r.error_code)
                return -EIO;
 
        return err;
@@ -61,17 +61,19 @@ out:
  **/
 static int mucse_mbx_get_info(struct mucse_hw *hw)
 {
-       struct mbx_fw_cmd_req req = {
-               .datalen = cpu_to_le16(MUCSE_MBX_REQ_HDR_LEN),
-               .opcode  = cpu_to_le16(GET_HW_INFO),
+       union mbx_fw_cmd_req_u req = {
+               .r = {
+                       .datalen = cpu_to_le16(MUCSE_MBX_REQ_HDR_LEN),
+                       .opcode  = cpu_to_le16(GET_HW_INFO),
+               },
        };
-       struct mbx_fw_cmd_reply reply = {};
+       union mbx_fw_cmd_reply_u reply = {};
        int err;
 
        err = mucse_fw_send_cmd_wait_resp(hw, &req, &reply);
        if (!err)
                hw->pfvfnum = FIELD_GET(GENMASK_U16(7, 0),
-                                       le16_to_cpu(reply.hw_info.pfnum));
+                                       le16_to_cpu(reply.r.hw_info.pfnum));
 
        return err;
 }
@@ -111,21 +113,23 @@ int mucse_mbx_sync_fw(struct mucse_hw *hw)
  **/
 int mucse_mbx_powerup(struct mucse_hw *hw, bool is_powerup)
 {
-       struct mbx_fw_cmd_req req = {
-               .datalen = cpu_to_le16(sizeof(req.powerup) +
-                                      MUCSE_MBX_REQ_HDR_LEN),
-               .opcode  = cpu_to_le16(POWER_UP),
-               .powerup = {
-                       /* fw needs this to reply correct cmd */
-                       .version = cpu_to_le32(GENMASK_U32(31, 0)),
-                       .status  = cpu_to_le32(is_powerup ? 1 : 0),
+       union mbx_fw_cmd_req_u req = {
+               .r = {
+                       .datalen = cpu_to_le16(sizeof(req.r.powerup) +
+                                              MUCSE_MBX_REQ_HDR_LEN),
+                       .opcode  = cpu_to_le16(POWER_UP),
+                       .powerup = {
+                               /* fw needs this to reply correct cmd */
+                               .version = cpu_to_le32(GENMASK_U32(31, 0)),
+                               .status  = cpu_to_le32(is_powerup ? 1 : 0),
+                       },
                },
        };
        int len, err;
 
-       len = le16_to_cpu(req.datalen);
+       len = le16_to_cpu(req.r.datalen);
        mutex_lock(&hw->mbx.lock);
-       err = mucse_write_and_wait_ack_mbx(hw, (u32 *)&req, len);
+       err = mucse_write_and_wait_ack_mbx(hw, req.dwords, len);
        mutex_unlock(&hw->mbx.lock);
 
        return err;
@@ -142,11 +146,13 @@ int mucse_mbx_powerup(struct mucse_hw *hw, bool is_powerup)
  **/
 int mucse_mbx_reset_hw(struct mucse_hw *hw)
 {
-       struct mbx_fw_cmd_req req = {
-               .datalen = cpu_to_le16(MUCSE_MBX_REQ_HDR_LEN),
-               .opcode  = cpu_to_le16(RESET_HW),
+       union mbx_fw_cmd_req_u req = {
+               .r = {
+                       .datalen = cpu_to_le16(MUCSE_MBX_REQ_HDR_LEN),
+                       .opcode  = cpu_to_le16(RESET_HW),
+               },
        };
-       struct mbx_fw_cmd_reply reply = {};
+       union mbx_fw_cmd_reply_u reply = {};
 
        return mucse_fw_send_cmd_wait_resp(hw, &req, &reply);
 }
@@ -166,24 +172,26 @@ int mucse_mbx_get_macaddr(struct mucse_hw *hw, int pfvfnum,
                          u8 *mac_addr,
                          int port)
 {
-       struct mbx_fw_cmd_req req = {
-               .datalen      = cpu_to_le16(sizeof(req.get_mac_addr) +
-                                           MUCSE_MBX_REQ_HDR_LEN),
-               .opcode       = cpu_to_le16(GET_MAC_ADDRESS),
-               .get_mac_addr = {
-                       .port_mask = cpu_to_le32(BIT(port)),
-                       .pfvf_num  = cpu_to_le32(pfvfnum),
+       union mbx_fw_cmd_req_u req = {
+               .r = {
+                       .datalen      = cpu_to_le16(sizeof(req.r.get_mac_addr) +
+                                                   MUCSE_MBX_REQ_HDR_LEN),
+                       .opcode       = cpu_to_le16(GET_MAC_ADDRESS),
+                       .get_mac_addr = {
+                               .port_mask = cpu_to_le32(BIT(port)),
+                               .pfvf_num  = cpu_to_le32(pfvfnum),
+                       },
                },
        };
-       struct mbx_fw_cmd_reply reply = {};
+       union mbx_fw_cmd_reply_u reply = {};
        int err;
 
        err = mucse_fw_send_cmd_wait_resp(hw, &req, &reply);
        if (err)
                return err;
 
-       if (le32_to_cpu(reply.mac_addr.ports) & BIT(port))
-               memcpy(mac_addr, reply.mac_addr.addrs[port].mac, ETH_ALEN);
+       if (le32_to_cpu(reply.r.mac_addr.ports) & BIT(port))
+               memcpy(mac_addr, reply.r.mac_addr.addrs[port].mac, ETH_ALEN);
        else
                return -ENODATA;
 
index fb24fc12b613933d237deb12be9925fb31848ca3..fe996aeffc4de73cc0c611ac3e3763f6b50573bf 100644 (file)
@@ -80,6 +80,20 @@ struct mbx_fw_cmd_reply {
        };
 } __packed;
 
+/* Union wrappers to expose struct as __le32 dword array for mailbox
+ * transport, eliminating the need for pointer casts.  The __packed
+ * structs have no padding, so dwords[] overlays the fields exactly.
+ */
+union mbx_fw_cmd_req_u {
+       struct mbx_fw_cmd_req r;
+       __le32 dwords[sizeof(struct mbx_fw_cmd_req) / sizeof(__le32)];
+};
+
+union mbx_fw_cmd_reply_u {
+       struct mbx_fw_cmd_reply r;
+       __le32 dwords[sizeof(struct mbx_fw_cmd_reply) / sizeof(__le32)];
+};
+
 int mucse_mbx_sync_fw(struct mucse_hw *hw);
 int mucse_mbx_powerup(struct mucse_hw *hw, bool is_powerup);
 int mucse_mbx_reset_hw(struct mucse_hw *hw);