[cmis] Fix enter_password to include standard password entry method by Junchao-Mellanox · Pull Request #724 · sonic-net/sonic-platform-common

Junchao-Mellanox · 2026-07-21T05:39:45Z

Why I did it

CMIS defines two ways to deliver the host password that unlocks protected CDB/EEPROM access:

CDB command 0001h (CdbCmdHandler.enter_password()), and
writing the 4-byte password MSB-first to the Password Entry Area at page 00h bytes 122-125.

CDB command 0001h is not honored by all CMIS modules. On modules that unlock only via the Password Entry Area, password entry silently fails, which blocks every password-protected CDB/EEPROM operation — firmware download/run/commit and SED-protected access among them. This change makes password entry work across both module families without changing behavior for modules that already unlock via CDB.

How I did it

cdb.py — Reworked enter_password() to first write the password to the Password Entry Area (page 00h bytes - api/public/cmis.py — New CmisApi.enter_password_via_memory(password), which writes the password to the Password Entry Area and then interprets the result:
- _supports_password_cmd_result() gates on CMIS revision — PasswordCmdResult (00h:42.3-0) was introduced in CMIS 5.3, so on earlier modules (or when the revision can't be parsed) those bits are reserved and are never interpreted.
- _read_password_cmd_result() polls PasswordCmdResult past the transient "validation in progress" / unreadable states, bounded by a 1000 ms timeout at 20 ms intervals (per CMIS 8.2.14 the module updates the result within tWRITE).
- Verdict handling: HOST_ACCEPTED/MODULE_ACCEPTED → True; NOT_ACCEPTED → False; pre-5.3, NOT_SUPPORTED, or undetermined → best-effort True on a successful write.
api/public/cdb_fw.py — New CmisCdbFw._enter_password() becomes the single entry point for all password entry. It validates the password (int in 0..0xFFFFFFFF), tries CDB 0001h first because that path is synchronous (send_cmd waits for and checks CDB status), and falls back to the Password Entry Area write on failure — or goes straight to it when no CDB handler is available. All existing call sites (get_module_fw_info, FW run/commit/download-start password retries, and cdb_enter_host_password) now go through it.
cdb/cdb.py — CdbCmdHandler.enter_password() keeps its original CDB-only behavior; only the docstring is updated to state that the fallback is the caller's responsibility.
fields/consts.py — Added PASSWORD_ENTRY (offset 122, size 4), PASSWORD_CMD_RESULT, the PASSWORD_RESULT_* code constants, PASSWORD_RESULT_MIN_CMIS_REV = (5, 3), and the poll interval/timeout constants, with comments citing CMIS 8.2.14.
mem_maps/.../page00_lower.py — Registered the PasswordEntry field (NumberRegField, format=">I", writable — the format packs MSB-first) and the 4-bit PasswordCmdResult field at 00h:42.

How to verify it

Unit tests in tests/sonic_xcvr/test_cmis.py and tests/sonic_xcvr/test_cdb.py:

_supports_password_cmd_result gating across CMIS 4.0 / 5.2 / 5.3 / 5.4 / 6.0 and unparseable revisions.
_read_password_cmd_result polling past IN_PROGRESS and None reads, plus the timeout case.
enter_password_via_memory: write failure, pre-5.3 best-effort (no register read), all four PasswordCmdResult codes, and the undetermined-result case.
_enter_password: CDB success short-circuits the fallback; fallback is invoked and its result propagated on CDB failure; no-CDB-handler path; invalid passwords ("not_an_int", -1, 0x100000000, 12.5, None) rejected without touching either path.
CdbCmdHandler.enter_password retains coverage for CDB failure propagation and invalid-input rejection without sending a command.

mssonicbld · 2026-07-21T05:39:53Z

/azp run

azure-pipelines · 2026-07-21T05:39:53Z

Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run.

azure-pipelines · 2026-07-21T05:40:02Z

Azure Pipelines: Successfully started running 1 pipeline(s).

Junchao-Mellanox · 2026-07-21T05:54:12Z

Hi @prgeor , @mihirpat1 , could you please help review this? Thanks!

mssonicbld · 2026-07-22T09:24:15Z

/azp run

azure-pipelines · 2026-07-22T09:24:24Z

Azure Pipelines: Successfully started running 1 pipeline(s).

bgallagher-nexthop

LGTM 👍

mssonicbld · 2026-07-23T02:49:20Z

/azp run

azure-pipelines · 2026-07-23T02:49:29Z

Azure Pipelines: Successfully started running 1 pipeline(s).

mssonicbld · 2026-07-23T02:53:19Z

/azp run

azure-pipelines · 2026-07-23T02:53:28Z

Azure Pipelines: Successfully started running 1 pipeline(s).

mssonicbld · 2026-07-29T05:52:44Z

/azp run

azure-pipelines · 2026-07-29T05:52:53Z

Azure Pipelines: Successfully started running 1 pipeline(s).

mssonicbld · 2026-07-29T05:55:02Z

/azp run

azure-pipelines · 2026-07-29T05:55:11Z

Azure Pipelines: Successfully started running 1 pipeline(s).

Copilot

Pull request overview

This PR extends the SONiC CMIS transceiver support to enter the host password using the CMIS-standard Password Entry Area (page 00h bytes 122–125, MSB-first) and to optionally confirm acceptance via the PasswordCmdResult register (CMIS 5.3+). It also wires this capability into the CDB handler path and updates unit tests accordingly.

Changes:

Added CMIS-side password entry via Password Entry Area with optional polling of PasswordCmdResult (CMIS 5.3+).
Extended the CMIS page00 lower memory map and field constants to expose Password Entry Area and PasswordCmdResult.
Updated CDB/CDB-FW handler construction and tests to support delegating password entry to CMIS as a fallback path.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
tests/sonic_xcvr/test_cmis.py	Adds unit tests for CMIS password-entry support checks and polling behavior.
tests/sonic_xcvr/test_cdb.py	Updates CDB handler tests to include CMIS fallback behavior and weakref semantics.
tests/sonic_xcvr/test_cdb_fw.py	Updates firmware handler tests for the new handler constructor signature.
sonic_platform_base/sonic_xcvr/mem_maps/public/cmis/pages/page00_lower.py	Adds CMIS page00 mappings for Password Entry Area and PasswordCmdResult.
sonic_platform_base/sonic_xcvr/fields/consts.py	Introduces constants for password entry/result registers and polling parameters.
sonic_platform_base/sonic_xcvr/cdb/cdb.py	Adds CMIS API weakref and password-entry fallback wiring in CdbCmdHandler.
sonic_platform_base/sonic_xcvr/cdb/cdb_fw.py	Updates firmware handler to pass through CMIS API reference.
sonic_platform_base/sonic_xcvr/api/public/cmis.py	Implements Password Entry Area write + PasswordCmdResult polling and CMIS revision gating.
sonic_platform_base/sonic_xcvr/api/public/cdb_fw.py	Passes `cmis_api=self` into the CDB firmware handler factory.

mssonicbld · 2026-08-07T04:03:43Z

/azp run

azure-pipelines · 2026-08-07T04:03:53Z

Azure Pipelines: Successfully started running 1 pipeline(s).

mssonicbld · 2026-08-07T04:05:04Z

/azp run

azure-pipelines · 2026-08-07T04:05:13Z

Azure Pipelines: Successfully started running 1 pipeline(s).

`CdbCmdHandler.enter_password()` previously delivered the CMIS host password only via CDB command `0001h`. This is not the standard mechanism defined by the CMIS spec and is not honored by all modules — the standard, universally-supported way to unlock password-protected CDB/EEPROM access is to write the 4-byte host password (MSB first) to the Password Entry Area at page 00h bytes 122-125. As a result, password entry would fail on modules that only unlock via the Password Entry Area, blocking protected CDB/EEPROM operations (e.g. firmware/SED-protected access) on those modules. - `cdb.py` — Reworked `enter_password()` to first write the password to the Password Entry Area (page 00h bytes 122-125, MSB first) using `write_raw`, which is the standard method honored by all CMIS modules. If that write fails, it falls back to the original CDB command `0001h` path for modules that rely on it. Input validation (integer in range `0..0xFFFFFFFF`) is preserved, and `struct` is used to pack the 32-bit password big-endian. - `cdb_consts.py` — Added `CDB_HOST_PASSWORD_ENTRY_OFFSET = 122` and `CDB_HOST_PASSWORD_ENTRY_SIZE = 4` with documentation describing the Password Entry Area register layout. - `test_cdb.py` — Updated existing tests to assert the password is now written MSB-first to the Password Entry Area (and that no CDB command is sent when the register write succeeds), and added `test_enter_password_fallback_to_cdb_command` to cover the fallback path when the register write fails. Ran the updated unit tests in `tests/sonic_xcvr/test_cdb.py`, covering: - `test_enter_password_valid` — password written MSB-first to the Password Entry Area, no CDB command issued. - `test_enter_password_default` — default password (`0x00001011`) written to the Password Entry Area. - `test_enter_password_fallback_to_cdb_command` — CDB command `0001h` used as fallback when the register write returns `False`. Signed-off-by: Junchao Chen <junchao@mellanox.com>

mssonicbld · 2026-08-07T06:26:31Z

/azp run

azure-pipelines · 2026-08-07T06:26:40Z

Azure Pipelines: Successfully started running 1 pipeline(s).

prgeor · 2026-08-10T05:28:48Z

        self._init_cdb_fw_handler = False
        return None

+    def _enter_password(self, password=cdb_consts.CDB_DEFAULT_PASSWORD):


@Junchao-Mellanox how does password access via EEPROM relates to the CDB firmware handler. this file is for cdb_fw handling.

As we discussed, it needs to fallback to standard memory way if CDB password way does not work. This file is the place to call enter_password, so it should handle the logic like this:

Try CDB password way first; otherwise

Fall back to EEPROM write password

@Junchao-Mellanox see my comments below. A new options in the cli can avoid this.

@prgeor , there is a auto flow in xcvrd, which cannot rely on the CLI option.

prgeor · 2026-08-10T05:29:46Z

        """
-        Enter host password via CDB command 0001h.
-        Returns True if password accepted, False/None otherwise.
+        Enter the host password via CDB command 0001h. This path is synchronous:


@Junchao-Mellanox keep this file changes limited to CDB protocol

There is no code logic change in this file. I will revert this.

Hi @prgeor , I am fine to add a new CLI option. However, the auto fallback logic is for xcvrd who might periodically call CmisApi.get_transceiver_info_firmware_versions which in turn calls api.get_module_fw_info(). In this flow, it might require entering password. As this is an auto flow, CLI option could not help here.

prgeor · 2026-08-10T05:31:14Z

        cmis_rev = [str(num) for num in [cmis_major, cmis_minor]]
        return '.'.join(cmis_rev)

+    def _supports_password_cmd_result(self):


@Junchao-Mellanox to maintain backward compatibility, I would suggest to add a new option in the CLI where user can exercise the password via eeprom write. keep the default password to CDB.

Currently, there is automatically fallback. Do you think we still need to add an extra option in CLI?

@Junchao-Mellanox yes, lets add new option. As mentioned earlier, the EEPROM approach is not reliable compared to the CDB approach. Let the user decide which option to choose.

Junchao-Mellanox requested review from mihirpat1, prgeor and stephenxs July 21, 2026 05:40

stephenxs previously approved these changes Jul 21, 2026

View reviewed changes

bgallagher-nexthop reviewed Jul 21, 2026

View reviewed changes

Comment thread sonic_platform_base/sonic_xcvr/cdb/cdb.py Outdated

Comment thread sonic_platform_base/sonic_xcvr/cdb/cdb.py Outdated

Junchao-Mellanox dismissed stephenxs’s stale review via 600270d July 22, 2026 09:22

Junchao-Mellanox force-pushed the public/pr-19 branch from 600270d to b6282d6 Compare July 22, 2026 09:23

bgallagher-nexthop approved these changes Jul 22, 2026

View reviewed changes

Comment thread sonic_platform_base/sonic_xcvr/fields/cdb_consts.py Outdated

Junchao-Mellanox force-pushed the public/pr-19 branch from 8676fb0 to 2abdcf0 Compare July 23, 2026 02:53

keboliu previously approved these changes Jul 28, 2026

View reviewed changes

prgeor reviewed Jul 28, 2026

View reviewed changes

Comment thread sonic_platform_base/sonic_xcvr/cdb/cdb.py Outdated

prgeor reviewed Jul 28, 2026

View reviewed changes

Comment thread sonic_platform_base/sonic_xcvr/cdb/cdb.py Outdated

prgeor reviewed Jul 28, 2026

View reviewed changes

Comment thread sonic_platform_base/sonic_xcvr/cdb/cdb.py Outdated

prgeor reviewed Jul 28, 2026

View reviewed changes

Comment thread sonic_platform_base/sonic_xcvr/cdb/cdb.py Outdated

prgeor reviewed Jul 28, 2026

View reviewed changes

Comment thread sonic_platform_base/sonic_xcvr/cdb/cdb.py Outdated

Junchao-Mellanox dismissed keboliu’s stale review via 59eeec7 July 29, 2026 05:51

Junchao-Mellanox force-pushed the public/pr-19 branch from 59eeec7 to 2d17bd7 Compare July 29, 2026 05:54

Junchao-Mellanox requested a review from prgeor July 30, 2026 03:13

prgeor reviewed Jul 31, 2026

View reviewed changes

Comment thread sonic_platform_base/sonic_xcvr/cdb/cdb_fw.py Outdated

Junchao-Mellanox requested a review from prgeor August 2, 2026 02:18

prgeor requested a lite review from Copilot August 5, 2026 04:56

Copilot started reviewing on behalf of prgeor August 5, 2026 04:56 View session

Copilot AI reviewed Aug 5, 2026

View reviewed changes

Comment thread sonic_platform_base/sonic_xcvr/cdb/cdb.py Outdated

Comment thread sonic_platform_base/sonic_xcvr/cdb/cdb_fw.py Outdated

Comment thread sonic_platform_base/sonic_xcvr/api/public/cmis.py

Comment thread sonic_platform_base/sonic_xcvr/cdb/cdb.py Outdated

Junchao-Mellanox force-pushed the public/pr-19 branch from e1574c1 to 0b965c7 Compare August 7, 2026 04:04

Junchao-Mellanox force-pushed the public/pr-19 branch from 0b965c7 to 8557319 Compare August 7, 2026 06:26

prgeor reviewed Aug 10, 2026

View reviewed changes

Uh oh!

Conversation

Junchao-Mellanox commented Jul 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why I did it

How I did it

How to verify it

Uh oh!

mssonicbld commented Jul 21, 2026

Uh oh!

azure-pipelines Bot commented Jul 21, 2026

Uh oh!

azure-pipelines Bot commented Jul 21, 2026

Uh oh!

Junchao-Mellanox commented Jul 21, 2026

Uh oh!

Uh oh!

Uh oh!

mssonicbld commented Jul 22, 2026

Uh oh!

azure-pipelines Bot commented Jul 22, 2026

Uh oh!

bgallagher-nexthop left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mssonicbld commented Jul 23, 2026

Uh oh!

azure-pipelines Bot commented Jul 23, 2026

Uh oh!

mssonicbld commented Jul 23, 2026

Uh oh!

azure-pipelines Bot commented Jul 23, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mssonicbld commented Jul 29, 2026

Uh oh!

azure-pipelines Bot commented Jul 29, 2026

Uh oh!

mssonicbld commented Jul 29, 2026

Uh oh!

azure-pipelines Bot commented Jul 29, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mssonicbld commented Aug 7, 2026

Uh oh!

azure-pipelines Bot commented Aug 7, 2026

Uh oh!

mssonicbld commented Aug 7, 2026

Uh oh!

azure-pipelines Bot commented Aug 7, 2026

Uh oh!

mssonicbld commented Aug 7, 2026

Uh oh!

azure-pipelines Bot commented Aug 7, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Junchao-Mellanox commented Jul 21, 2026 •

edited

Loading

Junchao-Mellanox Aug 11, 2026 •

edited

Loading