feat: updated check for chip revision and respective testcases

nileshkale123 · nileshkale123 · commit 54eb749fd2d0 · 2025-04-07T18:18:16.000+08:00
This commit have updated check for max chip revision along with min chip revision.
Also added qemu based pytest to verify chip revision while performing OTA.
diff --git a/components/bootloader_support/include/bootloader_common.h b/components/bootloader_support/include/bootloader_common.h
@@ -1,5 +1,5 @@
 /*
- * SPDX-FileCopyrightText: 2018-2024 Espressif Systems (Shanghai) CO LTD
+ * SPDX-FileCopyrightText: 2018-2025 Espressif Systems (Shanghai) CO LTD
  *
  * SPDX-License-Identifier: Apache-2.0
  */
@@ -24,6 +24,19 @@ typedef enum {
     ESP_IMAGE_APPLICATION
 } esp_image_type;
 
+/**
+ * @brief Check if the chip revision meets the image requirements.
+ *
+ * This function verifies whether the actual chip revision satisfies the minimum
+ * and optionally the maximum chip revision requirements specified in the image.
+ *
+ * @param image_header Pointer to the image header containing revision details.
+ * @param check_max_revision If true, also checks the maximum chip revision requirements.
+ *
+ * @return true if the chip revision meets the requirements, false otherwise.
+ */
+bool bootloader_common_check_chip_revision_validity(const esp_image_header_t *image_header, bool check_max_revision);
+
 /**
  * @brief Read ota_info partition and fill array from two otadata structures.
  *
diff --git a/components/bootloader_support/src/bootloader_common_loader.c b/components/bootloader_support/src/bootloader_common_loader.c
@@ -1,5 +1,5 @@
 /*
- * SPDX-FileCopyrightText: 2020-2024 Espressif Systems (Shanghai) CO LTD
+ * SPDX-FileCopyrightText: 2020-2025 Espressif Systems (Shanghai) CO LTD
  *
  * SPDX-License-Identifier: Apache-2.0
  */
@@ -31,6 +31,37 @@
 
 static const char* TAG = "boot_comm";
 
+bool bootloader_common_check_chip_revision_validity(const esp_image_header_t *img_hdr, bool check_max_revision)
+{
+    if (!img_hdr) {
+        return false;
+    }
+
+    unsigned revision = efuse_hal_chip_revision();
+    unsigned min_rev = img_hdr->min_chip_rev_full;
+
+    bool is_min_rev_invalid = !ESP_CHIP_REV_ABOVE(revision, min_rev);
+    if (is_min_rev_invalid) {
+        ESP_LOGE(TAG, "chip revision check failed. Required >= v%d.%d, found v%d.%d.",
+            min_rev / 100, min_rev % 100,
+            revision / 100, revision % 100);
+        return false;
+    }
+
+    if (check_max_revision) {
+        unsigned int max_rev = img_hdr->max_chip_rev_full;
+        bool is_max_rev_invalid = IS_FIELD_SET(max_rev) && revision > max_rev && !efuse_hal_get_disable_wafer_version_major();
+        if (is_max_rev_invalid) {
+            ESP_LOGE(TAG, "chip revision check failed. Required <= v%d.%d, found v%d.%d.",
+                max_rev / 100, max_rev % 100,
+                revision / 100, revision % 100);
+            return false;
+        }
+    }
+
+    return true;
+}
+
 uint32_t bootloader_common_ota_select_crc(const esp_ota_select_entry_t *s)
 {
     return esp_rom_crc32_le(UINT32_MAX, (uint8_t*)&s->ota_seq, 4);
@@ -91,24 +122,15 @@ esp_err_t bootloader_common_check_chip_validity(const esp_image_header_t* img_hd
         err = ESP_FAIL;
     } else {
 #ifndef CONFIG_IDF_ENV_FPGA
-        unsigned revision = efuse_hal_chip_revision();
-        unsigned int major_rev = revision / 100;
-        unsigned int minor_rev = revision % 100;
-        unsigned min_rev = img_hdr->min_chip_rev_full;
-        if (type == ESP_IMAGE_BOOTLOADER || type == ESP_IMAGE_APPLICATION) {
-            if (!ESP_CHIP_REV_ABOVE(revision, min_rev)) {
-                ESP_LOGE(TAG, "Image requires chip rev >= v%d.%d, but chip is v%d.%d",
-                         min_rev / 100, min_rev % 100,
-                         major_rev, minor_rev);
+        if (type == ESP_IMAGE_APPLICATION) {
+            if (!bootloader_common_check_chip_revision_validity(img_hdr, true)) {
                 err = ESP_FAIL;
             }
         }
-        if (type == ESP_IMAGE_APPLICATION) {
-            unsigned max_rev = img_hdr->max_chip_rev_full;
-            if ((IS_FIELD_SET(max_rev) && (revision > max_rev) && !efuse_hal_get_disable_wafer_version_major())) {
-                ESP_LOGE(TAG, "Image requires chip rev <= v%d.%d, but chip is v%d.%d",
-                         max_rev / 100, max_rev % 100,
-                         major_rev, minor_rev);
+
+        // Maximum revision check is skipped for bootloader images
+        if (type == ESP_IMAGE_BOOTLOADER) {
+            if (!bootloader_common_check_chip_revision_validity(img_hdr, false)) {
                 err = ESP_FAIL;
             }
         }
diff --git a/components/esp_https_ota/src/esp_https_ota.c b/components/esp_https_ota/src/esp_https_ota.c
@@ -631,10 +631,7 @@ static esp_err_t esp_ota_verify_chip_revision(const void *arg)
     esp_image_header_t *data = (esp_image_header_t *)(arg);
     esp_https_ota_dispatch_event(ESP_HTTPS_OTA_VERIFY_CHIP_REVISION, (void *)(&data->min_chip_rev_full), sizeof(uint16_t));
 
-    uint16_t ota_img_revision = data->min_chip_rev_full;
-    uint32_t chip_revision = efuse_hal_chip_revision();
-    if (ota_img_revision > chip_revision) {
-        ESP_LOGE(TAG, "Image requires chip rev >= v%d.%d, but chip is v%d.%d", ota_img_revision / 100, ota_img_revision % 100, chip_revision / 100, chip_revision % 100);
+    if (!bootloader_common_check_chip_revision_validity(data, true)) {
         return ESP_ERR_INVALID_VERSION;
     }
     return ESP_OK;
diff --git a/examples/system/ota/advanced_https_ota/efuse_esp32c3.bin b/examples/system/ota/advanced_https_ota/efuse_esp32c3.bin
diff --git a/examples/system/ota/advanced_https_ota/pytest_advanced_ota.py b/examples/system/ota/advanced_https_ota/pytest_advanced_ota.py
@@ -10,6 +10,7 @@
 import subprocess
 import time
 from typing import Callable
+from typing import Optional
 
 import pexpect
 import pytest
@@ -79,17 +80,19 @@ def start_https_server(ota_image_dir: str, server_ip: str, server_port: int) ->
 
 def start_chunked_server(ota_image_dir: str, server_port: int) -> subprocess.Popen:
     os.chdir(ota_image_dir)
-    chunked_server = subprocess.Popen([
-        'openssl',
-        's_server',
-        '-WWW',
-        '-key',
-        key_file,
-        '-cert',
-        server_file,
-        '-port',
-        str(server_port),
-    ])
+    chunked_server = subprocess.Popen(
+        [
+            'openssl',
+            's_server',
+            '-WWW',
+            '-key',
+            key_file,
+            '-cert',
+            server_file,
+            '-port',
+            str(server_port),
+        ]
+    )
     return chunked_server
 
 
@@ -129,6 +132,48 @@ def start_redirect_server(ota_image_dir: str, server_ip: str, server_port: int,
     httpd.serve_forever()
 
 
+# Function to modify chip revisions in the app header
+def modify_chip_revision(
+    app_path: str, min_rev: Optional[int] = None, max_rev: Optional[int] = None, increment_min: bool = False
+) -> None:
+    """
+    Modify min_chip_rev_full and max_chip_rev_full in the app header.
+
+    :param app_path: Path to the app binary.
+    :param min_rev: Value to set min_chip_rev_full (if provided).
+    :param max_rev: Value to set max_chip_rev_full (if provided).
+    :param increment_min: If True, increments min_chip_rev_full.
+    """
+
+    HEADER_SIZE = 512
+    TARGET_OFFSET_MIN_REV = 0x0F
+    TARGET_OFFSET_MAX_REV = 0x11
+
+    if not os.path.exists(app_path):
+        raise FileNotFoundError(f"App binary file '{app_path}' not found")
+
+    try:
+        with open(app_path, 'rb') as f:
+            header = bytearray(f.read(HEADER_SIZE))
+
+        # Increment or set min revision value
+        if increment_min:
+            header[TARGET_OFFSET_MIN_REV] = (header[TARGET_OFFSET_MIN_REV] + 1) & 0xFF
+        elif min_rev is not None:
+            header[TARGET_OFFSET_MIN_REV] = min_rev & 0xFF
+
+        # Set max revision value
+        if max_rev is not None:
+            header[TARGET_OFFSET_MAX_REV] = max_rev & 0xFF
+
+        # Write back the modified header to the binary file
+        with open(app_path, 'r+b') as f:
+            f.write(header)
+
+    except IOError as e:
+        raise RuntimeError(f'Failed to modify app header: {e}')
+
+
 @pytest.mark.ethernet_ota
 @idf_parametrize('target', ['esp32'], indirect=['target'])
 def test_examples_protocol_advanced_https_ota_example(dut: Dut) -> None:
@@ -253,7 +298,8 @@ def test_examples_protocol_advanced_https_ota_example_truncated_bin(dut: Dut) ->
     bin_name = 'advanced_https_ota.bin'
     # Truncated binary file to be generated from original binary file
     truncated_bin_name = 'truncated.bin'
-    # Size of truncated file to be grnerated. This value can range from 288 bytes (Image header size) to size of original binary file
+    # Size of truncated file to be grnerated.
+    # This value can range from 288 bytes (Image header size) to size of original binary file
     # truncated_bin_size is set to 64000 to reduce consumed by the test case
     truncated_bin_size = 64000
     binary_file = os.path.join(dut.app.binary_path, bin_name)
@@ -757,7 +803,8 @@ def test_examples_protocol_advanced_https_ota_example_ota_resumption_partial_dow
 @idf_parametrize('target', ['esp32', 'esp32c3', 'esp32s3'], indirect=['target'])
 def test_examples_protocol_advanced_https_ota_example_nimble_gatts(dut: Dut) -> None:
     """
-    Run an OTA image update while a BLE GATT Server is running in background. This GATT server will be using NimBLE Host stack.
+    Run an OTA image update while a BLE GATT Server is running in background.
+    This GATT server will be using NimBLE Host stack.
     steps: |
       1. join AP/Ethernet
       2. Run BLE advertise and then GATT server.
@@ -812,7 +859,8 @@ def test_examples_protocol_advanced_https_ota_example_nimble_gatts(dut: Dut) ->
 @idf_parametrize('target', ['esp32', 'esp32c3', 'esp32s3'], indirect=['target'])
 def test_examples_protocol_advanced_https_ota_example_bluedroid_gatts(dut: Dut) -> None:
     """
-    Run an OTA image update while a BLE GATT Server is running in background. This GATT server will be using Bluedroid Host stack.
+    Run an OTA image update while a BLE GATT Server is running in background.
+    This GATT server will be using Bluedroid Host stack.
     steps: |
       1. join AP/Ethernet
       2. Run BLE advertise and then GATT server.
@@ -907,3 +955,115 @@ def test_examples_protocol_advanced_https_ota_example_openssl_aligned_bin(dut: D
             pass
     finally:
         chunked_server.kill()
+
+
+@pytest.mark.qemu
+@pytest.mark.nightly_run
+@pytest.mark.host_test
+@pytest.mark.parametrize(
+    'qemu_extra_args',
+    [
+        f'-drive file={os.path.join(os.path.dirname(__file__), "efuse_esp32c3.bin")},if=none,format=raw,id=efuse '
+        '-global driver=nvram.esp32c3.efuse,property=drive,value=efuse '
+        '-global driver=timer.esp32c3.timg,property=wdt_disable,value=true',
+    ],
+    indirect=True,
+)
+@idf_parametrize('target', ['esp32c3'], indirect=['target'])
+@pytest.mark.parametrize('config', ['verify_revision'], indirect=True)
+def test_examples_protocol_advanced_https_ota_example_verify_min_chip_revision(dut: Dut) -> None:
+    """
+    This is a QEMU test case that verifies the chip revision value in the application header.
+    steps: |
+      1. join AP/Ethernet
+      2. Fetch OTA image over HTTPS
+      3. Reboot with the new OTA image
+    """
+
+    # Update the min full revision field in the app header
+    app_path = os.path.join(dut.app.binary_path, 'advanced_https_ota.bin')
+    # Increment min_chip_rev_full
+    modify_chip_revision(app_path, increment_min=True)
+
+    server_port = 8001
+    bin_name = 'advanced_https_ota.bin'
+    # Start server
+    thread1 = multiprocessing.Process(target=start_https_server, args=(dut.app.binary_path, '0.0.0.0', server_port))
+    thread1.daemon = True
+    thread1.start()
+    try:
+        # start test
+        dut.expect('Loaded app from partition at offset', timeout=30)
+
+        try:
+            ip_address = dut.expect(r'IPv4 address: (\d+\.\d+\.\d+\.\d+)[^\d]', timeout=30)[1].decode()
+            print('Connected to AP/Ethernet with IP: {}'.format(ip_address))
+        except pexpect.exceptions.TIMEOUT:
+            raise ValueError('ENV_TEST_FAILURE: Cannot connect to AP/Ethernet')
+
+        dut.expect('Starting Advanced OTA example', timeout=30)
+        host_ip = get_host_ip4_by_dest_ip(ip_address)
+
+        print('writing to device: {}'.format('https://' + host_ip + ':' + str(server_port) + '/' + bin_name))
+        dut.write('https://' + host_ip + ':' + str(server_port) + '/' + bin_name)
+        dut.expect('Starting OTA...', timeout=60)
+        dut.expect('chip revision check failed.', timeout=150)
+
+    finally:
+        thread1.terminate()
+
+
+@pytest.mark.qemu
+@pytest.mark.nightly_run
+@pytest.mark.host_test
+@pytest.mark.parametrize(
+    'qemu_extra_args',
+    [
+        f'-drive file={os.path.join(os.path.dirname(__file__), "efuse_esp32c3.bin")},if=none,format=raw,id=efuse '
+        '-global driver=nvram.esp32c3.efuse,property=drive,value=efuse '
+        '-global driver=timer.esp32c3.timg,property=wdt_disable,value=true',
+    ],
+    indirect=True,
+)
+@idf_parametrize('target', ['esp32c3'], indirect=['target'])
+@pytest.mark.parametrize('config', ['verify_revision'], indirect=True)
+def test_examples_protocol_advanced_https_ota_example_verify_max_chip_revision(dut: Dut) -> None:
+    """
+    This is a QEMU test case that verifies the chip revision value in the application header.
+    steps: |
+      1. join AP/Ethernet
+      2. Fetch OTA image over HTTPS
+      3. Reboot with the new OTA image
+    """
+
+    # Update the min full revision field in the app header
+    app_path = os.path.join(dut.app.binary_path, 'advanced_https_ota.bin')
+    # Set min_chip_rev_full to 0.0 and max_chip_rev_full to 0.2
+    modify_chip_revision(app_path, min_rev=0x00, max_rev=0x02)
+
+    server_port = 8001
+    bin_name = 'advanced_https_ota.bin'
+    # Start server
+    thread1 = multiprocessing.Process(target=start_https_server, args=(dut.app.binary_path, '0.0.0.0', server_port))
+    thread1.daemon = True
+    thread1.start()
+    try:
+        # start test
+        dut.expect('Loaded app from partition at offset', timeout=30)
+
+        try:
+            ip_address = dut.expect(r'IPv4 address: (\d+\.\d+\.\d+\.\d+)[^\d]', timeout=30)[1].decode()
+            print('Connected to AP/Ethernet with IP: {}'.format(ip_address))
+        except pexpect.exceptions.TIMEOUT:
+            raise ValueError('ENV_TEST_FAILURE: Cannot connect to AP/Ethernet')
+
+        dut.expect('Starting Advanced OTA example', timeout=30)
+        host_ip = get_host_ip4_by_dest_ip(ip_address)
+
+        print('writing to device: {}'.format('https://' + host_ip + ':' + str(server_port) + '/' + bin_name))
+        dut.write('https://' + host_ip + ':' + str(server_port) + '/' + bin_name)
+        dut.expect('Starting OTA...', timeout=60)
+        dut.expect('chip revision check failed.', timeout=150)
+
+    finally:
+        thread1.terminate()
diff --git a/examples/system/ota/advanced_https_ota/sdkconfig.ci.verify_revision_esp32c3 b/examples/system/ota/advanced_https_ota/sdkconfig.ci.verify_revision_esp32c3
@@ -0,0 +1,12 @@
+CONFIG_IDF_TARGET="esp32c3"
+
+CONFIG_EXAMPLE_FIRMWARE_UPGRADE_URL="FROM_STDIN"
+CONFIG_EXAMPLE_SKIP_COMMON_NAME_CHECK=y
+CONFIG_EXAMPLE_SKIP_VERSION_CHECK=y
+CONFIG_EXAMPLE_OTA_RECV_TIMEOUT=3000
+
+# QEMU-Related configurations
+CONFIG_EXAMPLE_CONNECT_ETHERNET=y
+CONFIG_EXAMPLE_USE_OPENETH=y
+CONFIG_EXAMPLE_CONNECT_WIFI=n
+CONFIG_ETH_USE_SPI_ETHERNET=n
