diff --git a/sonic_platform_base/sonic_xcvr/api/public/cdb_fw.py b/sonic_platform_base/sonic_xcvr/api/public/cdb_fw.py index 915f57a02..ddb1ce016 100644 --- a/sonic_platform_base/sonic_xcvr/api/public/cdb_fw.py +++ b/sonic_platform_base/sonic_xcvr/api/public/cdb_fw.py @@ -44,6 +44,35 @@ def _create_cdb_fw_handler(self): self._init_cdb_fw_handler = False return None + def _enter_password(self, password=cdb_consts.CDB_DEFAULT_PASSWORD): + """ + Enter the CMIS host password to unlock protected CDB/EEPROM access. + + The password is entered via CDB command 0001h first, since that path is + synchronous (send_cmd waits for and checks the CDB status). If it fails + -- or CDB is not available at all -- fall back to the non-CDB Password + Entry Area write (the register is in the CMIS memory map, not the CDB + one). + + Returns True if the password was accepted (or delivered best-effort), + False otherwise. + """ + 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 + + if self.cdb_fw_hdlr is None: + log.log_notice('CDB not available; entering password via Password Entry Area write') + return self.enter_password_via_memory(password) is True + + if self.cdb_fw_hdlr.enter_password(password) is True: + return True + + log.log_notice('CDB command 0001h password entry failed; ' + 'falling back to Password Entry Area write') + return self.enter_password_via_memory(password) is True + def get_module_fw_mgmt_feature(self, verbose = False): """ This function obtains CDB features supported by the module from CDB command 0041h, @@ -105,7 +134,7 @@ def get_module_fw_info(self): if fw_info is False or fw_info is None: if self.get_status_code() == cdb_consts.CDB_PASSWORD_ERROR_STATUS: log.log_notice('Get module FW info: Need to enter password') - self.cdb_fw_hdlr.enter_password() + self._enter_password() fw_info = self.cdb_fw_hdlr.get_firmware_info() if fw_info is False or fw_info is None: @@ -238,7 +267,7 @@ def module_fw_run(self, mode = 0x01): if fw_run_status == cdb_consts.CDB_PASSWORD_ERROR_STATUS: string = 'Module FW run: Need to enter password\n' log.log_notice(string) - self.cdb_fw_hdlr.enter_password() + self._enter_password() result = self.cdb_fw_hdlr.run_fw_image(mode) if result is not True: txt += 'Module FW run: Fail after password retry\n' @@ -276,7 +305,7 @@ def module_fw_commit(self): if fw_commit_status == cdb_consts.CDB_PASSWORD_ERROR_STATUS: string = 'Module FW commit: Need to enter password\n' log.log_notice(string) - self.cdb_fw_hdlr.enter_password() + self._enter_password() result = self.cdb_fw_hdlr.commit_fw_image() if result is not True: txt += 'Module FW commit: Fail after password retry\n' @@ -339,7 +368,7 @@ def cdb_epl_block_write(self, address, data): def cdb_enter_host_password(self, password): if self.cdb_fw_hdlr is None: return 0 - if self.cdb_fw_hdlr.enter_password(password) is True: + if self._enter_password(password) is True: log.log_notice('CDB host auth status: Success') return 1 status = self.get_status_code() @@ -373,7 +402,7 @@ def module_fw_start_download(self, imagepath): # password error - retry with default password if fw_start_status == cdb_consts.CDB_PASSWORD_ERROR_STATUS: log.log_notice('Start module FW download: Need to enter password\n') - self.cdb_fw_hdlr.enter_password() + self._enter_password() if self.cdb_fw_hdlr.start_fw_download(imagepath) is True: return True, '' txt = 'Start module FW download: Fail after password retry\n' diff --git a/sonic_platform_base/sonic_xcvr/api/public/cmis.py b/sonic_platform_base/sonic_xcvr/api/public/cmis.py index 51c8cb652..2d53a376a 100644 --- a/sonic_platform_base/sonic_xcvr/api/public/cmis.py +++ b/sonic_platform_base/sonic_xcvr/api/public/cmis.py @@ -323,6 +323,90 @@ def get_cmis_rev(self): cmis_rev = [str(num) for num in [cmis_major, cmis_minor]] return '.'.join(cmis_rev) + 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. + """ + try: + major, minor = (int(part) for part in self.get_cmis_rev().split(".")) + except (AttributeError, ValueError): + return False + return (major, minor) >= consts.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 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 < consts.PASSWORD_RESULT_POLL_TIMEOUT: + result = self.xcvr_eeprom.read(consts.PASSWORD_CMD_RESULT) + if result is not None and \ + result != consts.PASSWORD_RESULT_IN_PROGRESS: + return result + time.sleep(consts.PASSWORD_RESULT_POLL_INTERVAL / 1000) + elapsed += consts.PASSWORD_RESULT_POLL_INTERVAL + return None + + def enter_password_via_memory(self, password): + """ + Enter the host password by writing the 4-byte value (MSB first) to the + Password Entry Area (page 00h bytes 122-125). This is the non-CDB + password entry method, used as a fallback for modules that unlock via + that register rather than CDB command 0001h. + + Per CMIS 8.2.14 the write only delivers the password; its acceptance is + reported asynchronously in PasswordCmdResult (00h:42.3-0), which is only + defined on CMIS 5.3+ modules. So: + - On CMIS 5.3+, poll PasswordCmdResult and honor its verdict; a module + that explicitly rejects the password returns False. + - On earlier modules the register is reserved, so a successful write + is taken at face value (best-effort). The same best-effort result is + returned on 5.3+ when the register reports "not supported" or its + outcome cannot be determined. + + Returns True if the password was accepted (or, best-effort, delivered), + False otherwise. + """ + # NumberRegField(format=">I") packs the password MSB-first for us. + if not self.xcvr_eeprom.write(consts.PASSWORD_ENTRY, password): + logger.warning("Password Entry Area write failed") + return False + + # PasswordCmdResult (00h:42.3-0) only exists on CMIS 5.3+. On earlier + # modules there is no register to confirm acceptance, so treat the + # successful write as success. + if not self._supports_password_cmd_result(): + return True + + result = self._read_password_cmd_result() + if result in (consts.PASSWORD_RESULT_HOST_ACCEPTED, + consts.PASSWORD_RESULT_MODULE_ACCEPTED): + return True + + if result == consts.PASSWORD_RESULT_NOT_ACCEPTED: + # Module honored the Password Entry Area and rejected the password. + logger.warning("Password not accepted by the module (PasswordCmdResult=0x3)") + return False + + # 5.3+ module but the result is NOT_SUPPORTED or could not be determined: + # no register confirmation available, so treat the write as best-effort. + logger.warning("PasswordCmdResult unavailable (result={}); " + "treating Password Entry Area write as best-effort".format(result)) + return True + # Transceiver status def get_module_state(self): ''' diff --git a/sonic_platform_base/sonic_xcvr/cdb/cdb.py b/sonic_platform_base/sonic_xcvr/cdb/cdb.py index 1a234e39d..1951972cf 100644 --- a/sonic_platform_base/sonic_xcvr/cdb/cdb.py +++ b/sonic_platform_base/sonic_xcvr/cdb/cdb.py @@ -135,15 +135,22 @@ 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 host password via CDB command 0001h. This path is synchronous: + send_cmd waits for and checks the CDB status. + + Modules that do not unlock this way are handled by the caller, which + falls back to the non-CDB Password Entry Area write on the CmisApi. + + Returns True if the password was accepted, 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) - + def write_lpl_block(self, blkaddr, blkdata, timeout=None): """ Write LPL block diff --git a/sonic_platform_base/sonic_xcvr/fields/consts.py b/sonic_platform_base/sonic_xcvr/fields/consts.py index c4881da34..e09bf0f43 100644 --- a/sonic_platform_base/sonic_xcvr/fields/consts.py +++ b/sonic_platform_base/sonic_xcvr/fields/consts.py @@ -173,6 +173,29 @@ HW_MINOR_REV = "ModuleHardwareMinorRevision" CMIS_MAJOR_REVISION = "CmisMajorRevision" CMIS_MINOR_REVISION = "CmisMinorRevision" + +# CMIS Password Entry Area: page 00h bytes 122-125, 32-bit host password written +# MSB-first (big-endian). Standard, universally-supported way to unlock +# password-protected CDB/EEPROM access (per CMIS 8.2.14). +PASSWORD_ENTRY = "PasswordEntry" +PASSWORD_ENTRY_OFFSET = 122 +PASSWORD_ENTRY_SIZE = 4 +# PasswordCmdResult (00h:42.3-0): result of the most recent password entry/change +# written to the Password Entry Area. Writing the password only delivers it; its +# acceptance is reported asynchronously here. Only defined from CMIS 5.3 onward; +# on earlier modules those bits are reserved and must not be interpreted. +PASSWORD_CMD_RESULT = "PasswordCmdResult" +PASSWORD_RESULT_MIN_CMIS_REV = (5, 3) +PASSWORD_RESULT_NOT_SUPPORTED = 0x0 # not supported (legacy before CMIS 5.3) +PASSWORD_RESULT_MODULE_ACCEPTED = 0x1 # module password entry/change accepted +PASSWORD_RESULT_HOST_ACCEPTED = 0x2 # host password entry/change accepted +PASSWORD_RESULT_NOT_ACCEPTED = 0x3 # password entry not accepted +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. +PASSWORD_RESULT_POLL_INTERVAL = 20 # msec +PASSWORD_RESULT_POLL_TIMEOUT = 1000 # msec ACTIVE_FW_MAJOR_REV = "ModuleActiveFirmwareMajorRevision" ACTIVE_FW_MINOR_REV = "ModuleActiveFirmwareMinorRevision" INACTIVE_FW_MAJOR_REV = "ModuleInactiveFirmwareMajorRevision" diff --git a/sonic_platform_base/sonic_xcvr/mem_maps/public/cmis/pages/page00_lower.py b/sonic_platform_base/sonic_xcvr/mem_maps/public/cmis/pages/page00_lower.py index 62cd37491..f9cb36e92 100644 --- a/sonic_platform_base/sonic_xcvr/mem_maps/public/cmis/pages/page00_lower.py +++ b/sonic_platform_base/sonic_xcvr/mem_maps/public/cmis/pages/page00_lower.py @@ -117,3 +117,18 @@ def __init__(self, codes, page=ADMINISTRATIVE_PAGE): CodeRegField(consts.MODULE_FUNCTION_TYPE, self.getaddr(57), codes.MODULE_FUNCTION_TYPE), ] + # Password Entry Area (00h:122-125): 4-byte host password written + # MSB-first (big-endian) to unlock protected CDB/EEPROM access. + self.fields[consts.PASSWORD_ENTRY] = [ + NumberRegField(consts.PASSWORD_ENTRY, self.getaddr(consts.PASSWORD_ENTRY_OFFSET), + format=">I", size=consts.PASSWORD_ENTRY_SIZE, ro=False), + ] + + # PasswordCmdResult (00h:42.3-0): result of the most recent password + # entry/change written to the Password Entry Area. + self.fields[consts.PASSWORD_CMD_RESULT] = [ + NumberRegField(consts.PASSWORD_CMD_RESULT, self.getaddr(42), + *(RegBitField("Bit%d" % (bit), bit) for bit in range (0, 4)) + ), + ] + diff --git a/tests/sonic_xcvr/test_cdb.py b/tests/sonic_xcvr/test_cdb.py index d832b98cb..7088d3e9a 100644 --- a/tests/sonic_xcvr/test_cdb.py +++ b/tests/sonic_xcvr/test_cdb.py @@ -888,7 +888,7 @@ 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 sends CDB command 0001h""" self.handler.send_cmd = MagicMock(return_value=True) result = self.handler.enter_password(0x00001011) assert result is True @@ -898,7 +898,7 @@ def test_enter_password_valid(self): ) def test_enter_password_default(self): - """Test enter_password with default password""" + """Test enter_password with the default password""" self.handler.send_cmd = MagicMock(return_value=True) result = self.handler.enter_password() assert result is True @@ -907,6 +907,18 @@ def test_enter_password_default(self): {"password": cdb_consts.CDB_DEFAULT_PASSWORD} ) + def test_enter_password_cdb_failure(self): + """Test enter_password propagates a CDB command failure to the caller""" + self.handler.send_cmd = MagicMock(return_value=False) + assert self.handler.enter_password(0x00001011) is False + + def test_enter_password_invalid_no_cmd(self): + """Test an invalid password is rejected without sending a CDB command""" + self.handler.send_cmd = MagicMock(return_value=True) + result = self.handler.enter_password("not_an_int") + assert result is False + 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) diff --git a/tests/sonic_xcvr/test_cdb_fw.py b/tests/sonic_xcvr/test_cdb_fw.py index b25380d73..9fd134075 100644 --- a/tests/sonic_xcvr/test_cdb_fw.py +++ b/tests/sonic_xcvr/test_cdb_fw.py @@ -13,7 +13,7 @@ def setup_method(self): self.reader = MagicMock() self.writer = MagicMock() self.mem_map = MagicMock() - + # Mock the parent class initialization with patch.object(CdbFwHandler, 'initFwHandler', return_value=True): self.handler = CdbFwHandler(self.reader, self.writer, self.mem_map) @@ -441,7 +441,7 @@ def test_full_firmware_update_flow_lpl(self, mock_file): reader = MagicMock() writer = MagicMock() mem_map = MagicMock() - + # Mock successful initialization with patch.object(CdbFwHandler, 'send_cmd', return_value=True): with patch.object(CdbFwHandler, 'read_reply', return_value={ diff --git a/tests/sonic_xcvr/test_cmis.py b/tests/sonic_xcvr/test_cmis.py index 508102423..daad45b53 100755 --- a/tests/sonic_xcvr/test_cmis.py +++ b/tests/sonic_xcvr/test_cmis.py @@ -229,6 +229,76 @@ def test_get_cmis_rev(self, mock_response, expected): result = self.api.get_cmis_rev() assert result == expected + @pytest.mark.parametrize("rev_str, expected", [ + ("5.3", True), # CMIS 5.3 -> PasswordCmdResult defined + ("5.4", True), # CMIS 5.4 + ("6.0", True), # CMIS 6.0 + ("5.2", False), # CMIS 5.2 -> register reserved + ("4.0", False), # CMIS 4.0 + ("None.None", False), # unreadable revision + ("", False), # unparseable + ]) + def test_supports_password_cmd_result(self, rev_str, expected): + """_supports_password_cmd_result gates on CMIS >= 5.3""" + self.api.get_cmis_rev = MagicMock(return_value=rev_str) + assert self.api._supports_password_cmd_result() is expected + + @patch("sonic_platform_base.sonic_xcvr.api.public.cmis.time.sleep", MagicMock()) + def test_read_password_cmd_result_polls_past_in_progress(self): + """_read_password_cmd_result polls past 'in progress'/unreadable states""" + self.api.xcvr_eeprom.read = MagicMock(side_effect=[ + consts.PASSWORD_RESULT_IN_PROGRESS, + None, # module may reject reads until the result is determined + consts.PASSWORD_RESULT_HOST_ACCEPTED, + ]) + assert self.api._read_password_cmd_result() == consts.PASSWORD_RESULT_HOST_ACCEPTED + assert self.api.xcvr_eeprom.read.call_count == 3 + + @patch("sonic_platform_base.sonic_xcvr.api.public.cmis.time.sleep", MagicMock()) + def test_read_password_cmd_result_timeout(self): + """_read_password_cmd_result returns None when the result never resolves""" + self.api.xcvr_eeprom.read = MagicMock(return_value=consts.PASSWORD_RESULT_IN_PROGRESS) + assert self.api._read_password_cmd_result() is None + + def test_enter_password_via_memory_write_fails(self): + """enter_password_via_memory returns False when the Password Entry Area write fails""" + self.api.xcvr_eeprom.write = MagicMock(return_value=False) + self.api._supports_password_cmd_result = MagicMock(return_value=True) + self.api.xcvr_eeprom.read = MagicMock() + assert self.api.enter_password_via_memory(0x00001011) is False + self.api.xcvr_eeprom.write.assert_called_once_with(consts.PASSWORD_ENTRY, 0x00001011) + # Write failed -> PasswordCmdResult is never read + self.api.xcvr_eeprom.read.assert_not_called() + + def test_enter_password_via_memory_pre_5_3_best_effort(self): + """enter_password_via_memory is best-effort on pre-5.3 without reading PasswordCmdResult""" + self.api.xcvr_eeprom.write = MagicMock(return_value=True) + self.api._supports_password_cmd_result = MagicMock(return_value=False) + self.api.xcvr_eeprom.read = MagicMock() + assert self.api.enter_password_via_memory(0x00001011) is True + self.api.xcvr_eeprom.write.assert_called_once_with(consts.PASSWORD_ENTRY, 0x00001011) + self.api.xcvr_eeprom.read.assert_not_called() + + @pytest.mark.parametrize("result_code, expected", [ + (consts.PASSWORD_RESULT_HOST_ACCEPTED, True), + (consts.PASSWORD_RESULT_MODULE_ACCEPTED, True), + (consts.PASSWORD_RESULT_NOT_ACCEPTED, False), + (consts.PASSWORD_RESULT_NOT_SUPPORTED, True), # no confirmation -> best-effort + ]) + def test_enter_password_via_memory_result_codes(self, result_code, expected): + """enter_password_via_memory honors PasswordCmdResult on CMIS 5.3+""" + self.api.xcvr_eeprom.write = MagicMock(return_value=True) + self.api._supports_password_cmd_result = MagicMock(return_value=True) + self.api._read_password_cmd_result = MagicMock(return_value=result_code) + assert self.api.enter_password_via_memory(0x00001011) is expected + + def test_enter_password_via_memory_result_undetermined_best_effort(self): + """enter_password_via_memory is best-effort when PasswordCmdResult can't be determined""" + self.api.xcvr_eeprom.write = MagicMock(return_value=True) + self.api._supports_password_cmd_result = MagicMock(return_value=True) + self.api._read_password_cmd_result = MagicMock(return_value=None) + assert self.api.enter_password_via_memory(0x00001011) is True + @pytest.mark.parametrize("mock_response, expected", [ ("ModuleReady", "ModuleReady") ]) @@ -1826,6 +1896,9 @@ def test_get_status_code(self, status_dict, expected): def _setup_cdb_fw_hdlr(self): mock_fw_hdlr = MagicMock() + # CDB command 0001h succeeds by default, so _enter_password does not + # fall back to the Password Entry Area write unless a test asks for it. + mock_fw_hdlr.enter_password.return_value = True self.api._cdb_fw_hdlr = mock_fw_hdlr self.api._init_cdb_fw_handler = True return mock_fw_hdlr @@ -1850,7 +1923,6 @@ def test_cdb_commands_success(self, method, handler_method, args): ('cdb_firmware_download_complete', 'complete_fw_download', []), ('cdb_start_firmware_download', 'start_fw_download', ['/tmp/fw.bin']), ('cdb_lpl_block_write', 'write_lpl_block', [0x1000, b'\x01\x02']), - ('cdb_enter_host_password', 'enter_password', [0x00001011]), ]) def test_cdb_commands_failure(self, method, handler_method, args): mock_fw_hdlr = self._setup_cdb_fw_hdlr() @@ -1863,6 +1935,73 @@ def test_cdb_commands_failure(self, method, handler_method, args): result = getattr(self.api, method)(*args) assert result == 0x44 + def test_cdb_enter_host_password_failure(self): + """Both CDB command 0001h and the Password Entry Area fallback fail""" + mock_fw_hdlr = self._setup_cdb_fw_hdlr() + mock_fw_hdlr.enter_password.return_value = False + mock_fw_hdlr.get_cmd_status_code.return_value = { + cdb_consts.CDB1_IS_BUSY: False, + cdb_consts.CDB1_HAS_FAILED: True, + cdb_consts.CDB1_STATUS: 0x04, + } + self.api.enter_password_via_memory = MagicMock(return_value=False) + assert self.api.cdb_enter_host_password(0x00001011) == 0x44 + self.api.enter_password_via_memory.assert_called_once_with(0x00001011) + del self.api.enter_password_via_memory + + def test_enter_password_cdb_success(self): + """_enter_password stops at CDB command 0001h when it succeeds""" + mock_fw_hdlr = self._setup_cdb_fw_hdlr() + mock_fw_hdlr.enter_password.return_value = True + self.api.enter_password_via_memory = MagicMock(return_value=True) + assert self.api._enter_password() is True + mock_fw_hdlr.enter_password.assert_called_once_with(cdb_consts.CDB_DEFAULT_PASSWORD) + self.api.enter_password_via_memory.assert_not_called() + del self.api.enter_password_via_memory + + @pytest.mark.parametrize("memory_result, expected", [ + (True, True), + (False, False), + ]) + def test_enter_password_memory_fallback(self, memory_result, expected): + """_enter_password falls back to the Password Entry Area write and propagates its result""" + mock_fw_hdlr = self._setup_cdb_fw_hdlr() + mock_fw_hdlr.enter_password.return_value = False + self.api.enter_password_via_memory = MagicMock(return_value=memory_result) + assert self.api._enter_password(0x00001011) is expected + mock_fw_hdlr.enter_password.assert_called_once_with(0x00001011) + self.api.enter_password_via_memory.assert_called_once_with(0x00001011) + del self.api.enter_password_via_memory + + @pytest.mark.parametrize("memory_result, expected", [ + (True, True), + (False, False), + ]) + def test_enter_password_no_cdb_handler(self, memory_result, expected): + """_enter_password goes straight to the Password Entry Area write without CDB""" + self.api._cdb_fw_hdlr = None + self.api._init_cdb_fw_handler = False + self.api.enter_password_via_memory = MagicMock(return_value=memory_result) + assert self.api._enter_password(0x00001011) is expected + self.api.enter_password_via_memory.assert_called_once_with(0x00001011) + del self.api.enter_password_via_memory + + @pytest.mark.parametrize("password", [ + "not_an_int", + -1, + 0x100000000, + 12.5, + None, + ]) + def test_enter_password_invalid(self, password): + """_enter_password rejects out-of-range passwords without touching either path""" + mock_fw_hdlr = self._setup_cdb_fw_hdlr() + self.api.enter_password_via_memory = MagicMock(return_value=True) + assert self.api._enter_password(password) is False + mock_fw_hdlr.enter_password.assert_not_called() + self.api.enter_password_via_memory.assert_not_called() + del self.api.enter_password_via_memory + @pytest.mark.parametrize("method, args", [ ('cdb_run_firmware', [0x01]), ('cdb_commit_firmware', []),