Skip to content

Conversation

HonestQiao
Copy link

Description

This PR adds a guarded Kconfig option ESP_EFUSE_VDD_SPI_AS_GPIO_ALLOWED to control access to the dangerous VDD_SPI_AS_GPIO eFuse operation.

Motivation:
Writing to the VDD_SPI_AS_GPIO eFuse is an irreversible operation that permanently reconfigures the VDD_SPI pin as a GPIO. This immediately disables power to embedded flash, rendering it permanently unusable. Without proper safeguards, users might accidentally execute this destructive operation.

Changes:

  • Added Kconfig option with clear danger warnings
  • Set default value to n (disabled) to prevent accidental enablement
  • Limited availability to relevant ESP32-C series and H2 targets only
  • Included detailed help text explaining the irreversible consequences

Related

This is a preventive safety measure and not directly related to any existing issue.

Testing

  • Verified Kconfig option appears correctly in menuconfig for supported targets
  • Confirmed option is hidden for unsupported targets
  • Tested that the default value is properly set to n
  • Ensured help text displays correctly with proper formatting

@CLAassistant
Copy link

CLAassistant commented Aug 25, 2025

CLA assistant check
All committers have signed the CLA.

@github-actions github-actions bot changed the title efuse: add Kconfig option to allow VDD_SPI_AS_GPIO write efuse: add Kconfig option to allow VDD_SPI_AS_GPIO write (IDFGH-16349) Aug 25, 2025
@espressif-bot espressif-bot added the Status: Opened Issue is new label Aug 25, 2025
Copy link

github-actions bot commented Aug 25, 2025

Warnings
⚠️

Some issues found for the commit messages in this PR:

  • the commit message "efuse: add Kconfig option to allow VDD_SPI_AS_GPIO write":
    • type/action should be one of [change, ci, docs, feat, fix, refactor, remove, revert, test]
  • the commit message "efuse: add choice for VDD_SPI_AS_GPIO with explicit allow/not_allow options":
    • type/action should be one of [change, ci, docs, feat, fix, refactor, remove, revert, test]

Please fix these commit messages - here are some basic tips:

  • follow Conventional Commits style
  • correct format of commit message should be: <type/action>(<scope/component>): <summary>, for example fix(esp32): Fixed startup timeout issue
  • allowed types are: change,ci,docs,feat,fix,refactor,remove,revert,test
  • sufficiently descriptive message summary should be between 20 to 72 characters and start with upper case letter
  • avoid Jira references in commit messages (unavailable/irrelevant for our customers)

TIP: Install pre-commit hooks and run this check when committing (uses the Conventional Precommit Linter).

Messages
📖 You might consider squashing your 3 commits (simplifying branch history).

👋 Hello HonestQiao, we appreciate your contribution to this project!


📘 Please review the project's Contributions Guide for key guidelines on code, documentation, testing, and more.

🖊️ Please also make sure you have read and signed the Contributor License Agreement for this project.

Click to see more instructions ...


This automated output is generated by the PR linter DangerJS, which checks if your Pull Request meets the project's requirements and helps you fix potential issues.

DangerJS is triggered with each push event to a Pull Request and modify the contents of this comment.

Please consider the following:
- Danger mainly focuses on the PR structure and formatting and can't understand the meaning behind your code or changes.
- Danger is not a substitute for human code reviews; it's still important to request a code review from your colleagues.
- Resolve all warnings (⚠️ ) before requesting a review from human reviewers - they will appreciate it.
- Addressing info messages (📖) is strongly recommended; they're less critical but valuable.
- To manually retry these Danger checks, please navigate to the Actions tab and re-run last Danger workflow.

Review and merge process you can expect ...


We do welcome contributions in the form of bug reports, feature requests and pull requests via this public GitHub repository.

This GitHub project is public mirror of our internal git repository

1. An internal issue has been created for the PR, we assign it to the relevant engineer.
2. They review the PR and either approve it or ask you for changes or clarifications.
3. Once the GitHub PR is approved, we synchronize it into our internal git repository.
4. In the internal git repository we do the final review, collect approvals from core owners and make sure all the automated tests are passing.
- At this point we may do some adjustments to the proposed change, or extend it by adding tests or documentation.
5. If the change is approved and passes the tests it is merged into the default branch.
5. On next sync from the internal git repository merged change will appear in this public GitHub repository.

Generated by 🚫 dangerJS against 2334467

@HonestQiao HonestQiao changed the title efuse: add Kconfig option to allow VDD_SPI_AS_GPIO write (IDFGH-16349) feat: add Kconfig option to allow VDD_SPI_AS_GPIO write (IDFGH-16349) Aug 25, 2025
@HonestQiao
Copy link
Author

If merged, I'll backport this to active branches.

extern const esp_efuse_desc_t* ESP_EFUSE_DIS_DOWNLOAD_MANUAL_ENCRYPT[];
extern const esp_efuse_desc_t* ESP_EFUSE_USB_EXCHG_PINS[];
extern const esp_efuse_desc_t* ESP_EFUSE_VDD_SPI_AS_GPIO[];
#define ESP_EFUSE_VDD_SPI_AS_GPIO_DESC ESP_EFUSE_VDD_SPI_AS_GPIO
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

// This file was generated from the file esp_efuse_table.csv. DO NOT CHANGE THIS FILE MANUALLY.

See the comment at the top of this file

@KonstantinKondrashov
Copy link
Collaborator

Hi @HonestQiao!
Yes, burning the VDD_SPI_AS_GPIO efuse bit literally disconnects its power from the embedded flash, so the flash goes dead. This bit should never be used in normal workflows with embedded flash. For sure, we have some other efuses that can brick the chip.
I think we can keep the Kconfig option that you are adding and add a dedicated function (esp_efuse_burn_vdd_spi_as_gpio() in components/efuse/esp32xx/esp_efuse_fields.c), default off. I am not sure if we need to add this log to the common API.

@HonestQiao
Copy link
Author

Hi @HonestQiao! Yes, burning the VDD_SPI_AS_GPIO efuse bit literally disconnects its power from the embedded flash, so the flash goes dead. This bit should never be used in normal workflows with embedded flash. For sure, we have some other efuses that can brick the chip. I think we can keep the Kconfig option that you are adding and add a dedicated function (esp_efuse_burn_vdd_spi_as_gpio() in components/efuse/esp32xx/esp_efuse_fields.c), default off. I am not sure if we need to add this log to the common API.

Thank you for the feedback. Yes, my intention with the Kconfig warning was precisely to highlight this irreversible danger. Your suggestion is excellent – a dedicated function makes perfect sense for this special case, and having the generic API reject it with ESP_ERR_NOT_SUPPORTED is the right way to guide users to the safe path.

@HonestQiao
Copy link
Author

Hi @HonestQiao! Yes, burning the VDD_SPI_AS_GPIO efuse bit literally disconnects its power from the embedded flash, so the flash goes dead. This bit should never be used in normal workflows with embedded flash. For sure, we have some other efuses that can brick the chip. I think we can keep the Kconfig option that you are adding and add a dedicated function (esp_efuse_burn_vdd_spi_as_gpio() in components/efuse/esp32xx/esp_efuse_fields.c), default off. I am not sure if we need to add this log to the common API.

Absolutely. Since this operation is extremely dangerous for the affected chips, I added the prominent warning in Kconfig as a safeguard.

At first, I wanted to automatically detect whether there is built-in flash and prohibit the operation accordingly, but I abandoned that idea due to the high coupling it would introduce.

Also, could you please let us know what other operations can similarly "brick" the chip?
Perhaps we can specifically list these dangerous operations in the documentation to help users avoid these pitfalls.

@HonestQiao
Copy link
Author

Per your advice, I'm thinking of structuring the code like this. Is this the right direction?

diff --git a/components/efuse/esp32c3/esp_efuse_fields.c b/components/efuse/esp32c3/esp_efuse_fields.c
index b2765c7cd5..d9195a7130 100644
--- a/components/efuse/esp32c3/esp_efuse_fields.c
+++ b/components/efuse/esp32c3/esp_efuse_fields.c
@@ -51,3 +51,13 @@ esp_err_t esp_efuse_enable_rom_secure_download_mode(void)
     }
     return esp_efuse_write_field_bit(ESP_EFUSE_ENABLE_SECURITY_DOWNLOAD);
 }
+
+esp_err_t esp_efuse_burn_vdd_spi_as_gpio(void)
+{
+    #if defined(ESP_EFUSE_VDD_SPI_AS_GPIO_DANGEROUS_WRITE_NOT_ALLOWED)
+    ESP_LOGE(TAG, "Writing to VDD_SPI_AS_GPIO is not allowed.");
+    return ESP_ERR_NOT_SUPPORTED;
+    #else
+    return esp_efuse_write_field_bit(ESP_EFUSE_VDD_SPI_AS_GPIO);
+    #endif
+}
\ No newline at end of file

diff --git a/components/efuse/src/esp_efuse_api.c b/components/efuse/src/esp_efuse_api.c
index 922dbad71c..5aa4bc9be8 100644
--- a/components/efuse/src/esp_efuse_api.c
+++ b/components/efuse/src/esp_efuse_api.c
esp_err_t esp_efuse_write_field_bit(const esp_efuse_desc_t* field[])
{
    esp_err_t err;
    uint8_t existing = 0;
    const uint8_t one = 1;
@@ -148,11 +148,6 @@ esp_err_t esp_efuse_write_field_bit(const esp_efuse_desc_t* field[])
     #if defined(ESP_EFUSE_VDD_SPI_AS_GPIO_DANGEROUS_WRITE_NOT_ALLOWED)
     if (field == ESP_EFUSE_VDD_SPI_AS_GPIO) {
         ESP_LOGE(TAG, "Writing to VDD_SPI_AS_GPIO is not allowed.");
-        ESP_LOGW(TAG, "[DANGER] Enable CONFIG_ESP_EFUSE_VDD_SPI_AS_GPIO_ALLOWED in menuconfig.");
-        ESP_LOGW(TAG, "WARNING: This is an irreversible operation! Once written, it cannot be undone.");
-        ESP_LOGW(TAG, "CRITICAL: If you use Embedded Flash, it will immediately stop working and can never be recovered.");
-        ESP_LOGW(TAG, "This flag changes the VDD_SPI pin to function as a GPIO, which will permanently disable power to the Embedded Flash.");
-        ESP_LOGW(TAG, "Ensure that you fully understand the implications of setting this flag.");
         return ESP_ERR_NOT_SUPPORTED;
     }
     #endif

We can use a dedicated esp_efuse_burn_vdd_spi_as_gpio() function.
But do we also need to add a safeguard inside esp_efuse_write_field_bit() to block it if someone passes in ESP_EFUSE_VDD_SPI_AS_GPIO?

@KonstantinKondrashov
Copy link
Collaborator

Yes, you got me right.

I think that developers typically burn eFuses via espefuse.py (esptool) rather
than at runtime. The primary safeguard should be:
espefuse.py refuses writes to VDD_SPI_AS_GPIO by default, detects embedded-flash variants, and requires an explicit
--force. https://github.com/espressif/esptool/blob/master/espefuse/efuse/base_operations.py#L883

The esp-idf provides a small explicit helper (esp_efuse_burn_vdd_spi_as_gpio()) gated by CONFIG_EFUSE_ALLOW_VDD_SPI_AS_GPIO_BURN (default n). Do not special-case the generic esp_efuse_write_field_* APIs.

A short list of the “instant brick” eFuses if burned incorrectly:
VDD_SPI_AS_GPIO - kills flash I/O supply on embedded-flash packages.
SECURE_BOOT_EN – enables Secure Boot. If no valid secure boot key/digest is provisioned first → ROM refuses to boot.
SPI_BOOT_CRYPT_CNT – controls flash encryption. Wrong value without valid XTS key → ROM can’t read flash, no boot.
DIS_DOWNLOAD_MODE – disable download interfaces. If no valid image in flash → no way to recover.
WR_DIS / RD_DIS on BLOCK_KEYx or KEY_PURPOSE_x – locking or mis-assigning keys before provisioning → required keys (flash-enc/secure boot) unavailable
FORCE_USE_KEY_MANAGER_KEY / KM_DISABLE_DEPLOY_MODE / KM_DEPLOY_ONLY_ONCE – force key manager usage or block deployments before keys are loaded → chip cannot access needed keys → boot fails.
All other fuses (e.g. JTAG disable, TWAI disable, USB swaps) are “dangerous” but not permanent bricks.

We can not check all of them in esp-idf and the chip condition. I think espefuse is the proper way to do it.

@HonestQiao
Copy link
Author

Thank you for the detailed guidance and the list of critical eFuses. This is extremely valuable.

I understand the approach now: the primary safeguard should be in espefuse.py with a forced --force option for dangerous writes like this, while ESP-IDF provides an explicit helper function gated by a Kconfig.

I will study these points and work on implementing the corresponding handling in espefuse.py.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Status: Opened Issue is new

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants