CM3DS: fix non aligned access in Ethernet driver

This patch changes the way data is put in the TX_DATA_PORT register when
sending packet over Ethernet.
When this driver is compiled with release compilation profile
(space optimization compiler options) with Arm compiler version 5,
the line:
SMSC9220->TX_DATA_PORT = *pktptr;
generates the assembly instruction to get the pktptr pointed value:
LDM r2!, {r3}
with pktptr = r2
However, the code does not prevent the pktptr value from being unaligned
(to a 32 bits boundary) in that zone and the LDM instruction causes a
HardFault if this is the case. When the compiler option is not activated
(debug and develop compilation profiles), the compiler generates LDR
instruction instead which does not cause a HardFault.
The ARM v7-M states page B3-601: "Unaligned load-store multiples and
word or halfword exclusive accesses always fault."

To face that problem, we check if the data pointer is aligned or not. If
it is, we apply the same algorithm than before. If not, a local variable
is created and we copy in it, byte per byte, the contents at the
unaligned pointer. However, it will impact performances adding 8
instructions (one LD and one ST for each copied byte).

Change-Id: I11f6e82ce5521960d2ecf499f718f76fec29c0b0
Signed-off-by: Hugues de Valon <[email protected]>

Author: hug-dev

features/FEATURE_LWIP/lwip-interface/lwip-eth/arch/TARGET_CM3DS_MPS2/lwipopts_conf.h

@@ -1,4 +1,4 @@
-/* Copyright (c) 2017 ARM Limited
+/* Copyright (c) 2017-2018 ARM Limited
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -22,12 +22,22 @@
 /* size is 1522 bytes to accommodate the four-byte VLAN tag. */
 #define ETH_MAX_FLEN                  1522u /* recommended size for a VLAN frame */
 
-  /* Maximum Transfer Unit
-   * The IEEE 802.3 specification limits the data portion of the 802.3 frame
-   * to a minimum of 46 and a maximum of 1500 bytes, this is on L3 level.
-   */
+/*
+ * Maximum Transfer Unit
+ * The IEEE 802.3 specification limits the data portion of the 802.3 frame
+ * to a minimum of 46 and a maximum of 1522 bytes, this is on L2 level.
+ */
 #define ETH_L2_HEADER_LEN             22u
 
 #define ETH_MAX_PAYLOAD_LEN           (ETH_MAX_FLEN - ETH_L2_HEADER_LEN)
 
+/*
+ * Set this value to 2 to ensure that payload address of packet buffers is
+ * aligned on a 32 bits boundary.
+ * The padding is removed before passing the packet to the ethernet driver,
+ * hence defining this value to 2 will not prevent alignment issues inside the
+ * ethernet driver.
+ */
+#define ETH_PAD_SIZE                  0
+
 #endif /* LWIPOPTS_CONF_H */

targets/TARGET_ARM_SSG/TARGET_CM3DS_MPS2/SDK/smsc9220_eth.c

@@ -1,5 +1,5 @@
 /* mbed Microcontroller Library
- * Copyright (c) 2017 ARM Limited
+ * Copyright (c) 2017-2018 ARM Limited
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -426,6 +426,53 @@ static int smsc9220_check_id(void)
     return 0;
 }
 
+/**
+ * \brief Fill the SMSC9220 TX FIFO with a number of words at an aligned
+ *        address.
+ *
+ * \param[in] data Pointer to the aligned data that should be sent.
+ * \param[in] dwords_to_write Number of data words to write.
+ */
+static void fill_tx_fifo_aligned(unsigned int *data,
+                                 unsigned int dwords_to_write)
+{
+    while (dwords_to_write > 0) {
+        SMSC9220->TX_DATA_PORT = *data;
+        data++;
+        dwords_to_write--;
+    }
+}
+
+/**
+ * \brief Fill the SMSC9220 TX FIFO with a number of words at an unaligned
+ *        address. This function ensures that loading words at that address will
+ *        not generate unaligned access which can trigger an exception to the
+ *        processor.
+ *
+ * \param[in] data Pointer to the unaligned data that should be sent.
+ * \param[in] dwords_to_write Number of data words to write.
+ */
+static void fill_tx_fifo_unaligned(uint8_t *data, unsigned int dwords_to_write)
+{
+    /*
+     * Prevent unaligned word access from data pointer, 4 bytes are copied to
+     * this variable for each word that need to be sent.
+     */
+    unsigned int tx_data_port_tmp = 0;
+    uint8_t *tx_data_port_tmp_ptr = (uint8_t *)&tx_data_port_tmp;
+
+    while (dwords_to_write > 0) {
+        /* Keep the same endianness in data than in the temp variable */
+        tx_data_port_tmp_ptr[0] = data[0];
+        tx_data_port_tmp_ptr[1] = data[1];
+        tx_data_port_tmp_ptr[2] = data[2];
+        tx_data_port_tmp_ptr[3] = data[3];
+        SMSC9220->TX_DATA_PORT = tx_data_port_tmp;
+        data += 4;
+        dwords_to_write--;
+    }
+}
+
 /*----------------------------------------------------------------------------
   Public API
  *----------------------------------------------------------------------------*/
@@ -574,7 +621,6 @@ int smsc9220_send_by_chunks(unsigned int total_packet_length, int is_new_packet,
     int is_last_segment = 0; /* signing this is the last segment of the packet to be sent */
     unsigned int txcmd_a, txcmd_b = 0;
     unsigned int dwords_to_write = 0;
-    unsigned int *pktptr = 0;
     unsigned int xmit_inf = 0;
     unsigned int tx_buffer_free_space = 0;
     volatile unsigned int xmit_stat = 0;
@@ -602,7 +648,6 @@ int smsc9220_send_by_chunks(unsigned int total_packet_length, int is_new_packet,
         is_last_segment = 1;
     }
 
-    pktptr = (unsigned int *) data;
     txcmd_a = 0;
     txcmd_b = 0;
 
@@ -616,11 +661,16 @@ int smsc9220_send_by_chunks(unsigned int total_packet_length, int is_new_packet,
     SMSC9220->TX_DATA_PORT = txcmd_b;
     dwords_to_write = (current_size + 3) >> 2;
 
-    /* PIO Copy to FIFO. Could replace this with DMA. */
-    while(dwords_to_write > 0) {
-        SMSC9220->TX_DATA_PORT = *pktptr;
-        pktptr++;
-        dwords_to_write--;
+    /*
+     * Copy to TX FIFO
+     * The function to use depends on the alignment of the data pointer on a 32
+     * bits boundary.
+     */
+    if (((unsigned int)data % sizeof(uint32_t)) == 0) {
+        /* Cast is safe because we know data is aligned */
+        fill_tx_fifo_aligned((unsigned int *)data, dwords_to_write);
+    } else {
+        fill_tx_fifo_unaligned((uint8_t *)data, dwords_to_write);
     }
 
     if (is_last_segment) {