Merge pull request atomvm#578 from pguyot/w20/fix-flappiness-of-esp32-qemu-tests

Fix flappiness of esp32 qemu tests

ESP32 qemu tests were flappy because of a race condition in esp-idf that
probably only occurs on qemu under heavy load (or within GitHub CI which
doesn't really allocate two cores to actions)

The fix consists in applying a patch against v4.4.4
The patch was also provided in espressif/esp-idf#11447
See discussion in espressif/esp-idf#11433

Also bump versions of pytest-embedded plugins
Also optimize esp32 test by using espressif precompiled qemu binary

These changes are made under both the "Apache 2.0" and the "GNU Lesser General
Public License 2.1 or later" license terms (dual license).

SPDX-License-Identifier: Apache-2.0 OR LGPL-2.1-or-later

Author: bettio

.github/workflows/esp32-test-dockerfile

@@ -27,38 +27,65 @@ on:
 jobs:
   esp32-test:
     runs-on: ubuntu-latest
+    container: espressif/idf:v4.4.4
 
     strategy:
       fail-fast: false
 
+    defaults:
+      run:
+        shell: bash
+        working-directory: ./src/platforms/esp32/test/
+
     steps:
     - name: Checkout repo
       uses: actions/checkout@v2
 
-    - uses: actions/cache@v2
-      id: cache
-      with:
-        path: esp32-test.tar
-        key: ${{ hashFiles('.github/workflows/esp32-test-dockerfile') }}
+    - name: Install dependencies to build host AtomVM and run qemu
+      run: |
+        set -eu
+        apt update
+        DEBIAN_FRONTEND=noninteractive apt install -y -q \
+            doxygen erlang-base \
+            libglib2.0-0 libpixman-1-0
+
+    - name: Install qemu binary from espressif/qemu
+      run: |
+        set -eu
+        QEMU_VER=esp-develop-20220919
+        QEMU_DIST=qemu-${QEMU_VER}.tar.bz2
+        QEMU_SHA256=f6565d3f0d1e463a63a7f81aec94cce62df662bd42fc7606de4b4418ed55f870
+        wget --no-verbose https://github.com/espressif/qemu/releases/download/${QEMU_VER}/${QEMU_DIST}
+        echo "${QEMU_SHA256} *${QEMU_DIST}" | sha256sum --check --strict -
+        tar -xf ${QEMU_DIST} -C /opt
 
-    - name: "Load cached docker image"
-      if: steps.cache.outputs.cache-hit == 'true'
+    - name: Install pytest and pytest-embedded plugins
       run: |
-        sudo docker load -i esp32-test.tar
+        set -e
+        . $IDF_PATH/export.sh
+        pip install pytest==7.0.1 \
+            pytest-embedded==1.2.5 \
+            pytest-embedded-serial-esp==1.2.5 \
+            pytest-embedded-idf==1.2.5 \
+            pytest-embedded-qemu==1.2.5
 
-    - name: "Build: customized esp-idf-v4.4 container"
-      if: steps.cache.outputs.cache-hit != 'true'
-      run: docker build -t atomvm/esp32-test -f esp32-test-dockerfile .
-      working-directory: .github/workflows
+    - name: Patch esp-idf to fix race condition that affects qemu under heavy load
+      run: |
+        set -e
+        PATCH=$PWD/esp_ipc_isr.patch
+        cd $IDF_PATH
+        patch -p1 < $PATCH
 
-    - name: "Save custom docker image"
-      if: steps.cache.outputs.cache-hit != 'true'
-      run: docker save -o esp32-test.tar atomvm/esp32-test
+    - name: Build ESP32 tests using idf.py
+      run: |
+        set -e
+        . $IDF_PATH/export.sh
+        idf.py build
 
-    - name: "Run ESP32 tests using qemu-system-xtensa on Docker"
+    - name: Run ESP32 tests using qemu
       timeout-minutes: 10
       run: |
-        docker run --rm -v $PWD:/atomvm -w /atomvm/src/platforms/esp32/test atomvm/esp32-test /bin/bash -c '
-        idf.py build
+        set -e
+        . $IDF_PATH/export.sh
+        export PATH=/opt/qemu/bin:${PATH}
         pytest --embedded-services=idf,qemu -s
-        '

.github/workflows/esp32-test.yaml

@@ -0,0 +1,98 @@
+#
+# This file is part of AtomVM.
+#
+# Copyright 2023 Paul Guyot <[email protected]>
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+# SPDX-License-Identifier: Apache-2.0 OR LGPL-2.1-or-later
+#
+
+diff --git a/components/esp_ipc/src/esp_ipc_isr/esp_ipc_isr.c b/components/esp_ipc/src/esp_ipc_isr/esp_ipc_isr.c
+index dbdbe9027b..97025d5c65 100644
+--- a/components/esp_ipc/src/esp_ipc_isr/esp_ipc_isr.c
++++ b/components/esp_ipc/src/esp_ipc_isr/esp_ipc_isr.c
+@@ -57,7 +57,7 @@ typedef enum {
+ #define IPC_ISR_EXIT_CRITICAL()  portEXIT_CRITICAL_SAFE(&s_ipc_isr_mux)
+ 
+ static void esp_ipc_isr_call_and_wait(esp_ipc_isr_func_t func, void* arg, esp_ipc_isr_wait_t wait_for);
+-
++static void IRAM_ATTR esp_ipc_isr_wait_for_end();
+ 
+ /* Initializing IPC_ISR */
+ 
+@@ -130,6 +130,8 @@ void IRAM_ATTR esp_ipc_isr_release_other_cpu(void)
+         const uint32_t cpu_id = xPortGetCoreID();
+         if (--s_count_of_nested_calls[cpu_id] == 0) {
+             esp_ipc_isr_finish_cmd = 1;
++            // Make sure end flag is cleared and esp_ipc_isr_waiting_for_finish_cmd is done.
++            esp_ipc_isr_wait_for_end();
+             IPC_ISR_EXIT_CRITICAL();
+             portCLEAR_INTERRUPT_MASK_FROM_ISR(s_stored_interrupt_level);
+         } else if (s_count_of_nested_calls[cpu_id] < 0) {
+@@ -174,7 +176,7 @@ static void IRAM_ATTR esp_ipc_isr_call_and_wait(esp_ipc_isr_func_t func, void* a
+     const uint32_t cpu_id = xPortGetCoreID();
+ 
+     // waiting for the end of the previous call
+-    while (!esp_ipc_isr_end_fl) {};
++    esp_ipc_isr_wait_for_end();
+ 
+     esp_ipc_func = func;
+     esp_ipc_func_arg = arg;
+@@ -196,8 +198,13 @@ static void IRAM_ATTR esp_ipc_isr_call_and_wait(esp_ipc_isr_func_t func, void* a
+         while (!esp_ipc_isr_start_fl) {};
+     } else {
+         // IPC_ISR_WAIT_FOR_END
+-        while (!esp_ipc_isr_end_fl) {};
++        esp_ipc_isr_wait_for_end();
+     }
+ }
+ 
++static void IRAM_ATTR esp_ipc_isr_wait_for_end()
++{
++    while (!esp_ipc_isr_end_fl) {};
++}
++
+ /* End private functions*/
+diff --git a/components/esp_ipc/src/esp_ipc_isr/esp_ipc_isr_handler.S b/components/esp_ipc/src/esp_ipc_isr/esp_ipc_isr_handler.S
+index 0fb4ae676a..20638a6895 100644
+--- a/components/esp_ipc/src/esp_ipc_isr/esp_ipc_isr_handler.S
++++ b/components/esp_ipc/src/esp_ipc_isr/esp_ipc_isr_handler.S
+@@ -96,6 +96,7 @@ esp_ipc_isr_handler:
+ 
+     /* set the start flag */
+     movi    a0, esp_ipc_isr_start_fl
++    memw
+     s32i    a0, a0, 0
+ 
+     /* Call the esp_ipc_function(void* arg) */
+@@ -113,6 +114,7 @@ esp_ipc_isr_handler:
+ 
+     /* set the end flag */
+     movi    a0, esp_ipc_isr_end_fl
++    memw
+     s32i    a0, a0, 0
+ 
+     /* restore a0 */
+diff --git a/components/esp_ipc/src/esp_ipc_isr/esp_ipc_isr_routines.S b/components/esp_ipc/src/esp_ipc_isr/esp_ipc_isr_routines.S
+index 01ddf8b528..e947c3ed2f 100644
+--- a/components/esp_ipc/src/esp_ipc_isr/esp_ipc_isr_routines.S
++++ b/components/esp_ipc/src/esp_ipc_isr/esp_ipc_isr_routines.S
+@@ -23,6 +23,7 @@
+ esp_ipc_isr_waiting_for_finish_cmd:
+     /* waiting for the finish command */
+ .check_finish_cmd:
++    memw
+     l32i    a3, a2, 0
+     beqz    a3, .check_finish_cmd
+     ret

src/platforms/esp32/test/esp_ipc_isr.patch

@@ -24,7 +24,7 @@
 @pytest.mark.parametrize(
     'qemu_extra_args',
     [
-        '"-nic user,model=open_eth"',
+        '-nic user,model=open_eth',
     ],
     indirect=True,
 )