From 4f4eb9542edfb2ce198cfd2070c9c08165fe0749 Mon Sep 17 00:00:00 2001 From: Junchao Chen Date: Tue, 21 Jul 2026 08:38:43 +0300 Subject: [PATCH 1/3] [cmis] Fix enter_password to include standard password entry method MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #### Why I did it `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. #### How I did it - `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. #### How to verify it 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 --- sonic_platform_base/sonic_xcvr/cdb/cdb.py | 25 ++++++++++++-- .../sonic_xcvr/fields/cdb_consts.py | 5 +++ tests/sonic_xcvr/test_cdb.py | 33 ++++++++++++++++--- 3 files changed, 55 insertions(+), 8 deletions(-) diff --git a/sonic_platform_base/sonic_xcvr/cdb/cdb.py b/sonic_platform_base/sonic_xcvr/cdb/cdb.py index 1a234e39d..a719d1ad8 100644 --- a/sonic_platform_base/sonic_xcvr/cdb/cdb.py +++ b/sonic_platform_base/sonic_xcvr/cdb/cdb.py @@ -4,6 +4,7 @@ CDB Command handler """ +import struct import time from sonic_py_common.syslogger import SysLogger from ..fields import cdb_consts @@ -135,12 +136,30 @@ def get_cmd_status_code(self): def enter_password(self, password=cdb_consts.CDB_DEFAULT_PASSWORD): """ - Enter host password via CDB command 0001h. - Returns True if password accepted, False/None otherwise. + Enter the CMIS host password to unlock protected CDB/EEPROM access. + + Per CMIS, the password is entered by writing the 4-byte value (MSB + first) to the Password Entry Area at page 00h bytes 122-125. This + register-based mechanism is the standard and is honored by all CMIS + modules, so it is attempted first. Some modules also accept the + password via CDB command 0001h; that is kept as a fallback for the + modules that do not unlock via the Password Entry Area. + + Returns True if the password was delivered, False/None otherwise. """ - if not isinstance(password, int) or password < 0 or password > 0xFFFFFFFF: + if not isinstance(password, int) or \ + password < 0 or password > 0xFFFFFFFF: log.log_notice("Invalid password: must be an integer in range 0..0xFFFFFFFF") return False + + # Preferred: write the password to the Password Entry Area (MSB first). + pwd_bytes = bytearray(struct.pack(">I", password)) + if self.write_raw(cdb_consts.CDB_HOST_PASSWORD_ENTRY_OFFSET, + cdb_consts.CDB_HOST_PASSWORD_ENTRY_SIZE, pwd_bytes): + return True + + # Fallback: enter the password via CDB command 0001h. + log.log_notice("Password Entry Area write failed; falling back to CDB command 0001h") payload = {"password": password} return self.send_cmd(cdb_consts.CDB_ENTER_PASSWORD_CMD, payload) diff --git a/sonic_platform_base/sonic_xcvr/fields/cdb_consts.py b/sonic_platform_base/sonic_xcvr/fields/cdb_consts.py index 9d9737a4b..12d12df24 100644 --- a/sonic_platform_base/sonic_xcvr/fields/cdb_consts.py +++ b/sonic_platform_base/sonic_xcvr/fields/cdb_consts.py @@ -73,6 +73,11 @@ CDB_PASSWORD_ERROR_CODE = 0x06 CDB_PASSWORD_ERROR_STATUS = 0x46 CDB_DEFAULT_PASSWORD = 0x00001011 +# CMIS Password Entry Area: page 00h bytes 122-125 (linear offset 122 in lower +# memory), 32-bit host password written MSB-first. This is the standard, +# universally-supported way to unlock password-protected CDB/EEPROM access. +CDB_HOST_PASSWORD_ENTRY_OFFSET = 122 +CDB_HOST_PASSWORD_ENTRY_SIZE = 4 CDB_CHANGE_PASSWORD_CMD = 0x0002 CDB_ABORT_CMD = 0x0003 CDB_MODULE_FEATURE_CMD= 0x0004 diff --git a/tests/sonic_xcvr/test_cdb.py b/tests/sonic_xcvr/test_cdb.py index d832b98cb..ee37a4cdc 100644 --- a/tests/sonic_xcvr/test_cdb.py +++ b/tests/sonic_xcvr/test_cdb.py @@ -888,23 +888,46 @@ def test_enter_password_invalid(self, password, expected): assert result == expected def test_enter_password_valid(self): - """Test enter_password with valid password""" + """Test enter_password writes to the Password Entry Area (page 00h 122-125)""" + self.handler.write_raw = MagicMock(return_value=True) self.handler.send_cmd = MagicMock(return_value=True) result = self.handler.enter_password(0x00001011) assert result is True - self.handler.send_cmd.assert_called_once_with( - cdb_consts.CDB_ENTER_PASSWORD_CMD, - {"password": 0x00001011} + # Password written MSB-first to the Password Entry Area, no CDB command needed + self.handler.write_raw.assert_called_once_with( + cdb_consts.CDB_HOST_PASSWORD_ENTRY_OFFSET, + cdb_consts.CDB_HOST_PASSWORD_ENTRY_SIZE, + bytearray(struct.pack(">I", 0x00001011)) ) + self.handler.send_cmd.assert_not_called() def test_enter_password_default(self): """Test enter_password with default password""" + self.handler.write_raw = MagicMock(return_value=True) self.handler.send_cmd = MagicMock(return_value=True) result = self.handler.enter_password() assert result is True + self.handler.write_raw.assert_called_once_with( + cdb_consts.CDB_HOST_PASSWORD_ENTRY_OFFSET, + cdb_consts.CDB_HOST_PASSWORD_ENTRY_SIZE, + bytearray(struct.pack(">I", cdb_consts.CDB_DEFAULT_PASSWORD)) + ) + self.handler.send_cmd.assert_not_called() + + def test_enter_password_fallback_to_cdb_command(self): + """Test enter_password falls back to CDB command 0001h when the register write fails""" + self.handler.write_raw = MagicMock(return_value=False) + self.handler.send_cmd = MagicMock(return_value=True) + result = self.handler.enter_password(0x00001011) + assert result is True + self.handler.write_raw.assert_called_once_with( + cdb_consts.CDB_HOST_PASSWORD_ENTRY_OFFSET, + cdb_consts.CDB_HOST_PASSWORD_ENTRY_SIZE, + bytearray(struct.pack(">I", 0x00001011)) + ) self.handler.send_cmd.assert_called_once_with( cdb_consts.CDB_ENTER_PASSWORD_CMD, - {"password": cdb_consts.CDB_DEFAULT_PASSWORD} + {"password": 0x00001011} ) def test_write_lpl_block(self): From b6282d6fce3008a2055098a7ef9912dbe87f0628 Mon Sep 17 00:00:00 2001 From: Junchao Chen Date: Wed, 22 Jul 2026 12:18:57 +0300 Subject: [PATCH 2/3] Fix PR comments Signed-off-by: Junchao Chen --- sonic_platform_base/sonic_xcvr/cdb/cdb.py | 120 +++++++++++++++-- .../sonic_xcvr/fields/cdb_consts.py | 23 ++++ .../mem_maps/public/cmis/pages/page00_cdb.py | 18 +++ tests/sonic_xcvr/test_cdb.py | 127 ++++++++++++++++++ 4 files changed, 274 insertions(+), 14 deletions(-) diff --git a/sonic_platform_base/sonic_xcvr/cdb/cdb.py b/sonic_platform_base/sonic_xcvr/cdb/cdb.py index a719d1ad8..ad6d42d7c 100644 --- a/sonic_platform_base/sonic_xcvr/cdb/cdb.py +++ b/sonic_platform_base/sonic_xcvr/cdb/cdb.py @@ -134,18 +134,86 @@ def get_cmd_status_code(self): """ return self.last_cmd_status + def _get_cmis_rev(self): + """ + Read the CMIS revision the module complies to (00h:1). + + Returns a (major, minor) tuple, or None if it could not be read. + """ + rev = self.read(cdb_consts.CDB_CMIS_REVISION) + if rev is None: + return None + return (rev >> 4, rev & 0x0F) + + def _supports_password_cmd_result(self): + """ + Whether the PasswordCmdResult register (00h:42.3-0) is defined for this + module. It was introduced in CMIS 5.3; on earlier modules those bits are + reserved, so their value must not be used to judge password acceptance. + + Returns False if the CMIS revision cannot be determined, so the code + does not interpret a reserved register on a legacy module. + """ + rev = self._get_cmis_rev() + return rev is not None and rev >= cdb_consts.CDB_PASSWORD_RESULT_MIN_CMIS_REV + + def _read_password_cmd_result(self): + """ + Poll PasswordCmdResult (00h:42.3-0) until validation completes. + + Per CMIS 8.2.14, after a password entry/change WRITE the module updates + PasswordCmdResult within tWRITE, and until then may report "validation + in progress" or reject reads of the register. Poll past those transient + states, bounded by CDB_PASSWORD_RESULT_POLL_TIMEOUT. + + Returns the 4-bit result code, or None if it could not be determined + within the timeout. + """ + elapsed = 0 + while elapsed < cdb_consts.CDB_PASSWORD_RESULT_POLL_TIMEOUT: + result = self.read(cdb_consts.CDB_PASSWORD_CMD_RESULT) + if result is not None and \ + result != cdb_consts.CDB_PASSWORD_RESULT_IN_PROGRESS: + return result + time.sleep(cdb_consts.CDB_PASSWORD_RESULT_POLL_INTERVAL / 1000) + elapsed += cdb_consts.CDB_PASSWORD_RESULT_POLL_INTERVAL + return None + + def _enter_password_via_cdb(self, password): + """ + Enter the host password via CDB command 0001h. Fallback for modules that + do not unlock via the Password Entry Area. + """ + payload = {"password": password} + return self.send_cmd(cdb_consts.CDB_ENTER_PASSWORD_CMD, payload) + def enter_password(self, password=cdb_consts.CDB_DEFAULT_PASSWORD): """ Enter the CMIS host password to unlock protected CDB/EEPROM access. - Per CMIS, the password is entered by writing the 4-byte value (MSB - first) to the Password Entry Area at page 00h bytes 122-125. This - register-based mechanism is the standard and is honored by all CMIS - modules, so it is attempted first. Some modules also accept the - password via CDB command 0001h; that is kept as a fallback for the - modules that do not unlock via the Password Entry Area. - - Returns True if the password was delivered, False/None otherwise. + Per CMIS 8.2.14 the password is entered by writing the 4-byte value + (MSB first) to the Password Entry Area at page 00h bytes 122-125. The + write succeeding at the transport level does NOT mean the password was + accepted: validation is asynchronous and its outcome is reported in the + PasswordCmdResult register (00h:42.3-0). So: + 1. Write the password to the Password Entry Area (standard method). + 2. Read PasswordCmdResult to learn whether it was accepted -- but only + on CMIS 5.3+ modules, where that register is defined. On earlier + modules the register is reserved, so a successful write is taken + at face value (best-effort); a module that only unlocks via CDB + command 0001h is handled reactively by the caller, which re-enters + the password when a protected CDB command returns + CDB_PASSWORD_ERROR_STATUS. + 3. Fall back to CDB command 0001h only when the register method is not + usable (transport write failed, or -- on a 5.3+ module -- the + PasswordCmdResult could not be determined), for modules that unlock + via the CDB command. + A password a 5.3+ module explicitly rejects (PasswordCmdResult = "not + accepted") is NOT retried via CDB, since the same wrong password would + be rejected again. + + Returns True if the password was accepted (or, pre-5.3, delivered), + False/None otherwise. """ if not isinstance(password, int) or \ password < 0 or password > 0xFFFFFFFF: @@ -154,14 +222,38 @@ def enter_password(self, password=cdb_consts.CDB_DEFAULT_PASSWORD): # Preferred: write the password to the Password Entry Area (MSB first). pwd_bytes = bytearray(struct.pack(">I", password)) - if self.write_raw(cdb_consts.CDB_HOST_PASSWORD_ENTRY_OFFSET, - cdb_consts.CDB_HOST_PASSWORD_ENTRY_SIZE, pwd_bytes): + if not self.write_raw(cdb_consts.CDB_HOST_PASSWORD_ENTRY_OFFSET, + cdb_consts.CDB_HOST_PASSWORD_ENTRY_SIZE, pwd_bytes): + log.log_notice("Password Entry Area write failed; falling back to CDB command 0001h") + return self._enter_password_via_cdb(password) + + # PasswordCmdResult (00h:42.3-0) only exists on CMIS 5.3+. On earlier + # modules the Password Entry Area write is the standard mechanism and + # there is no register to confirm acceptance, so treat the successful + # write as success and let the caller's CDB_PASSWORD_ERROR_STATUS path + # cover modules that unlock only via CDB command 0001h. + if not self._supports_password_cmd_result(): return True - # Fallback: enter the password via CDB command 0001h. - log.log_notice("Password Entry Area write failed; falling back to CDB command 0001h") - payload = {"password": password} - return self.send_cmd(cdb_consts.CDB_ENTER_PASSWORD_CMD, payload) + # The write only delivered the password; its acceptance is reported + # asynchronously in PasswordCmdResult (00h:42.3-0). + result = self._read_password_cmd_result() + if result in (cdb_consts.CDB_PASSWORD_RESULT_HOST_ACCEPTED, + cdb_consts.CDB_PASSWORD_RESULT_MODULE_ACCEPTED): + return True + + if result == cdb_consts.CDB_PASSWORD_RESULT_NOT_ACCEPTED: + # Module honored the Password Entry Area and rejected the password; + # the CDB command would reject the same password too. + log.log_notice("Password not accepted by the module (PasswordCmdResult=0x3)") + return False + + # 5.3+ module but the result is NOT_SUPPORTED or could not be + # determined: the register method is not usable here, fall back to the + # CDB command. + log.log_notice("PasswordCmdResult unavailable (result={}); " + "falling back to CDB command 0001h".format(result)) + return self._enter_password_via_cdb(password) def write_lpl_block(self, blkaddr, blkdata, timeout=None): """ diff --git a/sonic_platform_base/sonic_xcvr/fields/cdb_consts.py b/sonic_platform_base/sonic_xcvr/fields/cdb_consts.py index 12d12df24..e001b4266 100644 --- a/sonic_platform_base/sonic_xcvr/fields/cdb_consts.py +++ b/sonic_platform_base/sonic_xcvr/fields/cdb_consts.py @@ -78,6 +78,29 @@ # universally-supported way to unlock password-protected CDB/EEPROM access. CDB_HOST_PASSWORD_ENTRY_OFFSET = 122 CDB_HOST_PASSWORD_ENTRY_SIZE = 4 +# CMIS revision: page 00h byte 1, major = bits 7-4, minor = bits 3-0 +# (e.g. 0x53 -> CMIS 5.3). Used to decide whether the PasswordCmdResult +# register below is defined for this module. +CDB_CMIS_REVISION = "CdbCmisRevision" +# PasswordCmdResult (00h:42.3-0) is only defined from CMIS 5.3 onward; on +# earlier modules those bits are reserved and must not be interpreted. +CDB_PASSWORD_RESULT_MIN_CMIS_REV = (5, 3) +# CMIS PasswordCmdResult register: page 00h byte 42, bits 3-0 (00h:42.3-0). +# Reports the result of the most recent password entry/change written to the +# Password Entry Area (00h:118-125). Writing the password only delivers it; its +# acceptance is reported asynchronously here (per CMIS 8.2.14). +CDB_PASSWORD_CMD_RESULT = "CdbPasswordCmdResult" +CDB_PASSWORD_CMD_RESULT_CODE = "CdbPasswordCmdResultCode" +CDB_PASSWORD_RESULT_NOT_SUPPORTED = 0x0 # not supported (legacy before CMIS 5.3) +CDB_PASSWORD_RESULT_MODULE_ACCEPTED = 0x1 # module password entry/change accepted +CDB_PASSWORD_RESULT_HOST_ACCEPTED = 0x2 # host password entry/change accepted +CDB_PASSWORD_RESULT_NOT_ACCEPTED = 0x3 # password entry not accepted +CDB_PASSWORD_RESULT_IN_PROGRESS = 0x8 # password validation in progress +# Poll bound for PasswordCmdResult after writing the Password Entry Area. The +# module updates the result within tWRITE and may reject reads (or report +# "in progress") until then; give it a small margin. +CDB_PASSWORD_RESULT_POLL_INTERVAL = 100 # msec +CDB_PASSWORD_RESULT_POLL_TIMEOUT = 1000 # msec CDB_CHANGE_PASSWORD_CMD = 0x0002 CDB_ABORT_CMD = 0x0003 CDB_MODULE_FEATURE_CMD= 0x0004 diff --git a/sonic_platform_base/sonic_xcvr/mem_maps/public/cmis/pages/page00_cdb.py b/sonic_platform_base/sonic_xcvr/mem_maps/public/cmis/pages/page00_cdb.py index f55fb0901..49675f21a 100644 --- a/sonic_platform_base/sonic_xcvr/mem_maps/public/cmis/pages/page00_cdb.py +++ b/sonic_platform_base/sonic_xcvr/mem_maps/public/cmis/pages/page00_cdb.py @@ -29,3 +29,21 @@ def __init__(self, codes): deps=[(cdb_consts.CDB1_IS_BUSY, cdb_consts.CDB1_HAS_FAILED, cdb_consts.CDB1_STATUS)], ), ] + + # CMIS revision (00h:1): major = bits 7-4, minor = bits 3-0. Read as a + # single byte so the CDB handler can decide whether PasswordCmdResult + # (00h:42.3-0) is defined for this module (CMIS 5.3+). + self.fields[cdb_consts.CDB_CMIS_REVISION] = [ + NumberRegField( + cdb_consts.CDB_CMIS_REVISION, self.getaddr(1), format="B", size=1, + ), + ] + + # PasswordCmdResult (00h:42.3-0): result of the most recent password + # entry/change written to the Password Entry Area (00h:118-125). + self.fields[cdb_consts.CDB_PASSWORD_CMD_RESULT] = [ + NumberRegField( + cdb_consts.CDB_PASSWORD_CMD_RESULT, self.getaddr(42), + RegBitsField(cdb_consts.CDB_PASSWORD_CMD_RESULT_CODE, bitpos=0, size=4), + ), + ] diff --git a/tests/sonic_xcvr/test_cdb.py b/tests/sonic_xcvr/test_cdb.py index ee37a4cdc..6cddda04d 100644 --- a/tests/sonic_xcvr/test_cdb.py +++ b/tests/sonic_xcvr/test_cdb.py @@ -891,6 +891,10 @@ def test_enter_password_valid(self): """Test enter_password writes to the Password Entry Area (page 00h 122-125)""" self.handler.write_raw = MagicMock(return_value=True) self.handler.send_cmd = MagicMock(return_value=True) + # CMIS 5.3+: PasswordCmdResult is defined and reports acceptance + self.handler._supports_password_cmd_result = MagicMock(return_value=True) + # PasswordCmdResult reports the host password was accepted + self.handler.read = MagicMock(return_value=cdb_consts.CDB_PASSWORD_RESULT_HOST_ACCEPTED) result = self.handler.enter_password(0x00001011) assert result is True # Password written MSB-first to the Password Entry Area, no CDB command needed @@ -899,12 +903,15 @@ def test_enter_password_valid(self): cdb_consts.CDB_HOST_PASSWORD_ENTRY_SIZE, bytearray(struct.pack(">I", 0x00001011)) ) + self.handler.read.assert_called_once_with(cdb_consts.CDB_PASSWORD_CMD_RESULT) self.handler.send_cmd.assert_not_called() def test_enter_password_default(self): """Test enter_password with default password""" self.handler.write_raw = MagicMock(return_value=True) self.handler.send_cmd = MagicMock(return_value=True) + self.handler._supports_password_cmd_result = MagicMock(return_value=True) + self.handler.read = MagicMock(return_value=cdb_consts.CDB_PASSWORD_RESULT_HOST_ACCEPTED) result = self.handler.enter_password() assert result is True self.handler.write_raw.assert_called_once_with( @@ -914,10 +921,76 @@ def test_enter_password_default(self): ) self.handler.send_cmd.assert_not_called() + def test_enter_password_module_accepted(self): + """Test enter_password accepts a module-password result code as success""" + self.handler.write_raw = MagicMock(return_value=True) + self.handler.send_cmd = MagicMock(return_value=True) + self.handler._supports_password_cmd_result = MagicMock(return_value=True) + self.handler.read = MagicMock(return_value=cdb_consts.CDB_PASSWORD_RESULT_MODULE_ACCEPTED) + result = self.handler.enter_password(0x00001011) + assert result is True + self.handler.send_cmd.assert_not_called() + + def test_enter_password_rejected_no_fallback(self): + """Test enter_password returns False (no CDB fallback) when the module rejects the password""" + self.handler.write_raw = MagicMock(return_value=True) + self.handler.send_cmd = MagicMock(return_value=True) + self.handler._supports_password_cmd_result = MagicMock(return_value=True) + # Module honored the Password Entry Area and rejected the password + self.handler.read = MagicMock(return_value=cdb_consts.CDB_PASSWORD_RESULT_NOT_ACCEPTED) + result = self.handler.enter_password(0x00001011) + assert result is False + self.handler.send_cmd.assert_not_called() + + def test_enter_password_result_not_supported_falls_back(self): + """Test enter_password falls back to CDB when a 5.3+ module reports PasswordCmdResult not supported""" + self.handler.write_raw = MagicMock(return_value=True) + self.handler.send_cmd = MagicMock(return_value=True) + self.handler._supports_password_cmd_result = MagicMock(return_value=True) + self.handler.read = MagicMock(return_value=cdb_consts.CDB_PASSWORD_RESULT_NOT_SUPPORTED) + result = self.handler.enter_password(0x00001011) + assert result is True + self.handler.send_cmd.assert_called_once_with( + cdb_consts.CDB_ENTER_PASSWORD_CMD, + {"password": 0x00001011} + ) + + @patch("sonic_platform_base.sonic_xcvr.cdb.cdb.time.sleep", MagicMock()) + def test_enter_password_result_in_progress_then_accepted(self): + """Test enter_password polls past 'validation in progress' until accepted""" + self.handler.write_raw = MagicMock(return_value=True) + self.handler.send_cmd = MagicMock(return_value=True) + self.handler._supports_password_cmd_result = MagicMock(return_value=True) + self.handler.read = MagicMock(side_effect=[ + cdb_consts.CDB_PASSWORD_RESULT_IN_PROGRESS, + None, # module may reject reads until the result is determined + cdb_consts.CDB_PASSWORD_RESULT_HOST_ACCEPTED, + ]) + result = self.handler.enter_password(0x00001011) + assert result is True + assert self.handler.read.call_count == 3 + self.handler.send_cmd.assert_not_called() + + @patch("sonic_platform_base.sonic_xcvr.cdb.cdb.time.sleep", MagicMock()) + def test_enter_password_result_timeout_falls_back(self): + """Test enter_password falls back to CDB when PasswordCmdResult can't be determined""" + self.handler.write_raw = MagicMock(return_value=True) + self.handler.send_cmd = MagicMock(return_value=True) + self.handler._supports_password_cmd_result = MagicMock(return_value=True) + # PasswordCmdResult never resolves within the poll timeout + self.handler.read = MagicMock(return_value=cdb_consts.CDB_PASSWORD_RESULT_IN_PROGRESS) + result = self.handler.enter_password(0x00001011) + assert result is True + self.handler.send_cmd.assert_called_once_with( + cdb_consts.CDB_ENTER_PASSWORD_CMD, + {"password": 0x00001011} + ) + def test_enter_password_fallback_to_cdb_command(self): """Test enter_password falls back to CDB command 0001h when the register write fails""" self.handler.write_raw = MagicMock(return_value=False) self.handler.send_cmd = MagicMock(return_value=True) + self.handler.read = MagicMock() result = self.handler.enter_password(0x00001011) assert result is True self.handler.write_raw.assert_called_once_with( @@ -925,11 +998,65 @@ def test_enter_password_fallback_to_cdb_command(self): cdb_consts.CDB_HOST_PASSWORD_ENTRY_SIZE, bytearray(struct.pack(">I", 0x00001011)) ) + # Transport write failed -> no PasswordCmdResult read, straight to CDB + self.handler.read.assert_not_called() self.handler.send_cmd.assert_called_once_with( cdb_consts.CDB_ENTER_PASSWORD_CMD, {"password": 0x00001011} ) + @pytest.mark.parametrize("rev_byte, expected", [ + (0x53, (5, 3)), # CMIS 5.3 + (0x50, (5, 0)), # CMIS 5.0 + (0x40, (4, 0)), # CMIS 4.0 + (0x00, (0, 0)), # unreadable/zero + ]) + def test_get_cmis_rev(self, rev_byte, expected): + """Test _get_cmis_rev decodes the major/minor nibbles of 00h:1""" + self.handler.read = MagicMock(return_value=rev_byte) + assert self.handler._get_cmis_rev() == expected + self.handler.read.assert_called_once_with(cdb_consts.CDB_CMIS_REVISION) + + def test_get_cmis_rev_unreadable(self): + """Test _get_cmis_rev returns None when the revision cannot be read""" + self.handler.read = MagicMock(return_value=None) + assert self.handler._get_cmis_rev() is None + + @pytest.mark.parametrize("rev_byte, expected", [ + (0x53, True), # CMIS 5.3 -> PasswordCmdResult defined + (0x54, True), # CMIS 5.4 + (0x60, True), # CMIS 6.0 + (0x52, False), # CMIS 5.2 -> register reserved + (0x40, False), # CMIS 4.0 + ]) + def test_supports_password_cmd_result(self, rev_byte, expected): + """Test _supports_password_cmd_result gates on CMIS >= 5.3""" + self.handler.read = MagicMock(return_value=rev_byte) + assert self.handler._supports_password_cmd_result() is expected + + def test_supports_password_cmd_result_unreadable(self): + """Test _supports_password_cmd_result is False when the revision is unknown""" + self.handler.read = MagicMock(return_value=None) + assert self.handler._supports_password_cmd_result() is False + + def test_enter_password_pre_5_3_best_effort(self): + """Test enter_password returns True on a pre-5.3 module without reading PasswordCmdResult""" + self.handler.write_raw = MagicMock(return_value=True) + self.handler.send_cmd = MagicMock(return_value=True) + # CMIS 5.2 -> PasswordCmdResult is reserved and must not be interpreted + self.handler.read = MagicMock(return_value=0x52) + result = self.handler.enter_password(0x00001011) + assert result is True + # Password delivered via the Password Entry Area only; no CDB command, + # and PasswordCmdResult is never read (only the CMIS revision is). + self.handler.write_raw.assert_called_once_with( + cdb_consts.CDB_HOST_PASSWORD_ENTRY_OFFSET, + cdb_consts.CDB_HOST_PASSWORD_ENTRY_SIZE, + bytearray(struct.pack(">I", 0x00001011)) + ) + self.handler.read.assert_called_once_with(cdb_consts.CDB_CMIS_REVISION) + self.handler.send_cmd.assert_not_called() + def test_write_lpl_block(self): """Test write_lpl_block sends correct command""" self.handler.send_cmd = MagicMock(return_value=True) From 2abdcf02bb3d2fc7a9df01d555f55469fecd9de5 Mon Sep 17 00:00:00 2001 From: Junchao-Mellanox <57339448+Junchao-Mellanox@users.noreply.github.com> Date: Thu, 23 Jul 2026 10:49:12 +0800 Subject: [PATCH 3/3] Change CDB_PASSWORD_RESULT_POLL_INTERVAL to 20 msec Reduced the polling interval for password result checks. Signed-off-by: Junchao Chen --- sonic_platform_base/sonic_xcvr/fields/cdb_consts.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sonic_platform_base/sonic_xcvr/fields/cdb_consts.py b/sonic_platform_base/sonic_xcvr/fields/cdb_consts.py index e001b4266..91fbd033a 100644 --- a/sonic_platform_base/sonic_xcvr/fields/cdb_consts.py +++ b/sonic_platform_base/sonic_xcvr/fields/cdb_consts.py @@ -99,7 +99,7 @@ # Poll bound for PasswordCmdResult after writing the Password Entry Area. The # module updates the result within tWRITE and may reject reads (or report # "in progress") until then; give it a small margin. -CDB_PASSWORD_RESULT_POLL_INTERVAL = 100 # msec +CDB_PASSWORD_RESULT_POLL_INTERVAL = 20 # msec CDB_PASSWORD_RESULT_POLL_TIMEOUT = 1000 # msec CDB_CHANGE_PASSWORD_CMD = 0x0002 CDB_ABORT_CMD = 0x0003