[cmis] Fix enter_password to include standard password entry method

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
@@ -133,16 +134,126 @@ 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 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 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:
+        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
-        payload = {"password": password}
-        return self.send_cmd(cdb_consts.CDB_ENTER_PASSWORD_CMD, payload)
+
+        # Preferred: write the password to the Password Entry Area (MSB first).
+        pwd_bytes = bytearray(struct.pack(">I", password))
+        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
+
+        # 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):
         """

sonic_platform_base/sonic_xcvr/fields/cdb_consts.py

@@ -73,6 +73,34 @@
 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
+# 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 = 20     # msec
+CDB_PASSWORD_RESULT_POLL_TIMEOUT = 1000    # msec
 CDB_CHANGE_PASSWORD_CMD = 0x0002
 CDB_ABORT_CMD = 0x0003
 CDB_MODULE_FEATURE_CMD= 0x0004

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),
+            ),
+        ]

tests/sonic_xcvr/test_cdb.py

@@ -888,24 +888,174 @@ 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)
+        # 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
-        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.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(
+            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_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": cdb_consts.CDB_DEFAULT_PASSWORD}
+            {"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(
+            cdb_consts.CDB_HOST_PASSWORD_ENTRY_OFFSET,
+            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"""