From bf5576c93b00aa9f63fef7236934f0a23a919f4a Mon Sep 17 00:00:00 2001 From: Abtin Keshavarzian Date: Mon, 9 Jan 2023 09:47:38 -0800 Subject: [PATCH] [rcp] RCP to inform host of its supported RCP API version range (#8617) This commit adds a new mechanism for RCP to inform host of the range of RCP API versions that it can work with. This helps address situations where RCP may be running a newer firmware (with higher RCP API version) and host side is older, ensuring that version checks can be performed correctly. This commit introduces a new RCP-specific spinel property `SPINEL_PROP_RCP_MIN_HOST_API_VERSION`. This new property is available when RCP indicates that it has the newly added capability `SPINEL_CAP_RCP_MIN_HOST_API_VERSION`. This is used on host side to determine if we can get this property from RCP. With addition of this new property, the `RadioSpinel` version check code is changed to get the min version from RCP itself and then check on the host side that its version is within the supported range. --- src/lib/spinel/radio_spinel.hpp | 10 ++-- src/lib/spinel/radio_spinel_impl.hpp | 87 +++++++++++++++++++++------- src/lib/spinel/spinel.c | 1 + src/lib/spinel/spinel.h | 28 +++++++-- src/ncp/ncp_base.cpp | 1 + src/ncp/ncp_base_dispatcher.cpp | 1 + src/ncp/ncp_base_radio.cpp | 5 ++ 7 files changed, 103 insertions(+), 30 deletions(-) diff --git a/src/lib/spinel/radio_spinel.hpp b/src/lib/spinel/radio_spinel.hpp index dc3118c61..65468c28f 100644 --- a/src/lib/spinel/radio_spinel.hpp +++ b/src/lib/spinel/radio_spinel.hpp @@ -727,14 +727,16 @@ public: /** * This method checks whether the spinel interface is radio-only. * - * @param[out] aSupportsRcpApiVersion A reference to a boolean variable to update whether the list of spinel - * capabilities include `SPINEL_CAP_RCP_API_VERSION`. + * @param[out] aSupportsRcpApiVersion A reference to a boolean variable to update whether the list of + * spinel capabilities includes `SPINEL_CAP_RCP_API_VERSION`. + * @param[out] aSupportsRcpMinHostApiVersion A reference to a boolean variable to update whether the list of + * spinel capabilities includes `SPINEL_CAP_RCP_MIN_HOST_API_VERSION`. * * @retval TRUE The radio chip is in radio-only mode. * @retval FALSE Otherwise. * */ - bool IsRcp(bool &aSupportsRcpApiVersion); + bool IsRcp(bool &aSupportsRcpApiVersion, bool &aSupportsRcpMinHostApiVersion); /** * This method checks whether there is pending frame in the buffer. @@ -960,7 +962,7 @@ private: otError CheckSpinelVersion(void); otError CheckRadioCapabilities(void); - otError CheckRcpApiVersion(bool aSupportsRcpApiVersion); + otError CheckRcpApiVersion(bool aSupportsRcpApiVersion, bool aSupportsMinHostRcpApiVersion); /** * This method triggers a state transfer of the state machine. diff --git a/src/lib/spinel/radio_spinel_impl.hpp b/src/lib/spinel/radio_spinel_impl.hpp index 567809afb..179bdbd60 100644 --- a/src/lib/spinel/radio_spinel_impl.hpp +++ b/src/lib/spinel/radio_spinel_impl.hpp @@ -231,6 +231,7 @@ void RadioSpinel::Init(bool aResetRadio, { otError error = OT_ERROR_NONE; bool supportsRcpApiVersion; + bool supportsRcpMinHostApiVersion; #if OPENTHREAD_SPINEL_CONFIG_RCP_RESTORATION_MAX_COUNT > 0 mResetRadioOnStartup = aResetRadio; @@ -257,7 +258,7 @@ void RadioSpinel::Init(bool aResetRadio, SuccessOrExit(error = Get(SPINEL_PROP_NCP_VERSION, SPINEL_DATATYPE_UTF8_S, mVersion, sizeof(mVersion))); SuccessOrExit(error = Get(SPINEL_PROP_HWADDR, SPINEL_DATATYPE_EUI64_S, mIeeeEui64.m8)); - if (!IsRcp(supportsRcpApiVersion)) + if (!IsRcp(supportsRcpApiVersion, supportsRcpMinHostApiVersion)) { uint8_t exitCode = OT_EXIT_RADIO_SPINEL_INCOMPATIBLE; @@ -273,7 +274,7 @@ void RadioSpinel::Init(bool aResetRadio, if (!aSkipRcpCompatibilityCheck) { - SuccessOrDie(CheckRcpApiVersion(supportsRcpApiVersion)); + SuccessOrDie(CheckRcpApiVersion(supportsRcpApiVersion, supportsRcpMinHostApiVersion)); SuccessOrDie(CheckRadioCapabilities()); } @@ -309,7 +310,8 @@ exit: } template -bool RadioSpinel::IsRcp(bool &aSupportsRcpApiVersion) +bool RadioSpinel::IsRcp(bool &aSupportsRcpApiVersion, + bool &aSupportsRcpMinHostApiVersion) { uint8_t capsBuffer[kCapsBufferSize]; const uint8_t *capsData = capsBuffer; @@ -317,7 +319,8 @@ bool RadioSpinel::IsRcp(bool &aSupportsRcpApi bool supportsRawRadio = false; bool isRcp = false; - aSupportsRcpApiVersion = false; + aSupportsRcpApiVersion = false; + aSupportsRcpMinHostApiVersion = false; SuccessOrDie(Get(SPINEL_PROP_CAPS, SPINEL_DATATYPE_DATA_S, capsBuffer, &capsLength)); @@ -349,6 +352,11 @@ bool RadioSpinel::IsRcp(bool &aSupportsRcpApi aSupportsRcpApiVersion = true; } + if (capability == SPINEL_PROP_RCP_MIN_HOST_API_VERSION) + { + aSupportsRcpMinHostApiVersion = true; + } + capsData += unpacked; capsLength -= static_cast(unpacked); } @@ -400,27 +408,50 @@ exit: } template -otError RadioSpinel::CheckRcpApiVersion(bool aSupportsRcpApiVersion) +otError RadioSpinel::CheckRcpApiVersion(bool aSupportsRcpApiVersion, + bool aSupportsRcpMinHostApiVersion) { - otError error = OT_ERROR_NONE; - unsigned int rcpApiVersion = 1; - - // Use RCP API Version value 1, when the RCP capability - // list does not contain `SPINEL_CAP_RCP_API_VERSION`. - - if (aSupportsRcpApiVersion) - { - SuccessOrExit(error = Get(SPINEL_PROP_RCP_API_VERSION, SPINEL_DATATYPE_UINT_PACKED_S, &rcpApiVersion)); - } + otError error = OT_ERROR_NONE; static_assert(SPINEL_MIN_HOST_SUPPORTED_RCP_API_VERSION <= SPINEL_RCP_API_VERSION, "MIN_HOST_SUPPORTED_RCP_API_VERSION must be smaller than or equal to RCP_API_VERSION"); - if ((rcpApiVersion < SPINEL_MIN_HOST_SUPPORTED_RCP_API_VERSION) || (rcpApiVersion > SPINEL_RCP_API_VERSION)) + if (aSupportsRcpApiVersion) { - otLogCritPlat("RCP API Version %u is not in the supported range [%u-%u]", rcpApiVersion, - SPINEL_MIN_HOST_SUPPORTED_RCP_API_VERSION, SPINEL_RCP_API_VERSION); - DieNow(OT_EXIT_RADIO_SPINEL_INCOMPATIBLE); + // Make sure RCP is not too old and its version is within the + // range host supports. + + unsigned int rcpApiVersion; + + SuccessOrExit(error = Get(SPINEL_PROP_RCP_API_VERSION, SPINEL_DATATYPE_UINT_PACKED_S, &rcpApiVersion)); + + if (rcpApiVersion < SPINEL_MIN_HOST_SUPPORTED_RCP_API_VERSION) + { + otLogCritPlat("RCP and host are using incompatible API versions"); + otLogCritPlat("RCP API Version %u is older than min required by host %u", rcpApiVersion, + SPINEL_MIN_HOST_SUPPORTED_RCP_API_VERSION); + DieNow(OT_EXIT_RADIO_SPINEL_INCOMPATIBLE); + } + } + + if (aSupportsRcpMinHostApiVersion) + { + // Check with RCP about min host API version it can work with, + // and make sure on host side our version is within the supported + // range. + + unsigned int minHostRcpApiVersion; + + SuccessOrExit( + error = Get(SPINEL_PROP_RCP_MIN_HOST_API_VERSION, SPINEL_DATATYPE_UINT_PACKED_S, &minHostRcpApiVersion)); + + if (SPINEL_RCP_API_VERSION < minHostRcpApiVersion) + { + otLogCritPlat("RCP and host are using incompatible API versions"); + otLogCritPlat("RCP requires min host API version %u but host is older and at version %u", + minHostRcpApiVersion, SPINEL_RCP_API_VERSION); + DieNow(OT_EXIT_RADIO_SPINEL_INCOMPATIBLE); + } } exit: @@ -2763,6 +2794,7 @@ void RadioSpinel::LogSpinelFrame(const uint8_ case SPINEL_PROP_RADIO_CAPS: case SPINEL_PROP_RCP_API_VERSION: + case SPINEL_PROP_RCP_MIN_HOST_API_VERSION: { const char *name; unsigned int value; @@ -2770,7 +2802,22 @@ void RadioSpinel::LogSpinelFrame(const uint8_ unpacked = spinel_datatype_unpack(data, len, SPINEL_DATATYPE_UINT_PACKED_S, &value); VerifyOrExit(unpacked > 0, error = OT_ERROR_PARSE); - name = (key == SPINEL_PROP_RADIO_CAPS) ? "caps" : "version"; + switch (key) + { + case SPINEL_PROP_RADIO_CAPS: + name = "caps"; + break; + case SPINEL_PROP_RCP_API_VERSION: + name = "version"; + break; + case SPINEL_PROP_RCP_MIN_HOST_API_VERSION: + name = "min-host-version"; + break; + default: + name = ""; + break; + } + start += Snprintf(start, static_cast(end - start), ", %s:%u", name, value); } break; diff --git a/src/lib/spinel/spinel.c b/src/lib/spinel/spinel.c index 72f440d76..8c0eba952 100644 --- a/src/lib/spinel/spinel.c +++ b/src/lib/spinel/spinel.c @@ -1426,6 +1426,7 @@ const char *spinel_prop_key_to_cstr(spinel_prop_key_t prop_key) {SPINEL_PROP_SERVER_SERVICES, "SERVER_SERVICES"}, {SPINEL_PROP_SERVER_LEADER_SERVICES, "SERVER_LEADER_SERVICES"}, {SPINEL_PROP_RCP_API_VERSION, "RCP_API_VERSION"}, + {SPINEL_PROP_RCP_MIN_HOST_API_VERSION, "RCP_MIN_HOST_API_VERSION"}, {SPINEL_PROP_UART_BITRATE, "UART_BITRATE"}, {SPINEL_PROP_UART_XON_XOFF, "UART_XON_XOFF"}, {SPINEL_PROP_15_4_PIB_PHY_CHANNELS_SUPPORTED, "15_4_PIB_PHY_CHANNELS_SUPPORTED"}, diff --git a/src/lib/spinel/spinel.h b/src/lib/spinel/spinel.h index a096a7f70..bf6a5ad1d 100644 --- a/src/lib/spinel/spinel.h +++ b/src/lib/spinel/spinel.h @@ -325,8 +325,11 @@ * as constant as possible. * * - On start, host implementation queries the RCP API version and accepts - * any version number from SPINEL_MIN_HOST_SUPPORTED_RCP_API_VERSION up to - * and including SPINEL_RCP_API_VERSION. + * any version starting from SPINEL_MIN_HOST_SUPPORTED_RCP_API_VERSION. + * + * - Host implementation also queries the RCP about the minimum host RCP + * API version it can work with, and then checks that its own version is + * within the range. * * Host and RCP compatibility guideline: * @@ -417,7 +420,7 @@ * Please see section "Spinel definition compatibility guideline" for more details. * */ -#define SPINEL_RCP_API_VERSION 7 +#define SPINEL_RCP_API_VERSION 8 /** * @def SPINEL_MIN_HOST_SUPPORTED_RCP_API_VERSION @@ -1278,9 +1281,10 @@ enum SPINEL_CAP_NET_THREAD_1_2 = (SPINEL_CAP_NET__BEGIN + 2), SPINEL_CAP_NET__END = 64, - SPINEL_CAP_RCP__BEGIN = 64, - SPINEL_CAP_RCP_API_VERSION = (SPINEL_CAP_RCP__BEGIN + 0), - SPINEL_CAP_RCP__END = 80, + SPINEL_CAP_RCP__BEGIN = 64, + SPINEL_CAP_RCP_API_VERSION = (SPINEL_CAP_RCP__BEGIN + 0), + SPINEL_CAP_RCP_MIN_HOST_API_VERSION = (SPINEL_CAP_RCP__BEGIN + 1), + SPINEL_CAP_RCP__END = 80, SPINEL_CAP_OPENTHREAD__BEGIN = 512, SPINEL_CAP_MAC_ALLOWLIST = (SPINEL_CAP_OPENTHREAD__BEGIN + 0), @@ -4349,6 +4353,18 @@ enum */ SPINEL_PROP_RCP_API_VERSION = SPINEL_PROP_RCP__BEGIN + 0, + /// Min host RCP API Version number + /** Format: `i` (read-only) + * + * Required capability: SPINEL_CAP_RADIO and SPINEL_CAP_RCP_MIN_HOST_API_VERSION. + * + * This property gives the minimum host RCP API Version number. + * + * Please see "Spinel definition compatibility guideline" section. + * + */ + SPINEL_PROP_RCP_MIN_HOST_API_VERSION = SPINEL_PROP_RCP__BEGIN + 1, + SPINEL_PROP_RCP__END = 0xFF, SPINEL_PROP_INTERFACE__BEGIN = 0x100, diff --git a/src/ncp/ncp_base.cpp b/src/ncp/ncp_base.cpp index 576740d40..522dd2308 100644 --- a/src/ncp/ncp_base.cpp +++ b/src/ncp/ncp_base.cpp @@ -1844,6 +1844,7 @@ template <> otError NcpBase::HandlePropertyGet(void) #if OPENTHREAD_RADIO SuccessOrExit(error = mEncoder.WriteUintPacked(SPINEL_CAP_RCP_API_VERSION)); + SuccessOrExit(error = mEncoder.WriteUintPacked(SPINEL_CAP_RCP_MIN_HOST_API_VERSION)); #endif #if OPENTHREAD_PLATFORM_POSIX diff --git a/src/ncp/ncp_base_dispatcher.cpp b/src/ncp/ncp_base_dispatcher.cpp index 973e0595e..3d7ebe61c 100644 --- a/src/ncp/ncp_base_dispatcher.cpp +++ b/src/ncp/ncp_base_dispatcher.cpp @@ -153,6 +153,7 @@ NcpBase::PropertyHandler NcpBase::FindGetPropertyHandler(spinel_prop_key_t aKey) #endif // OPENTHREAD_MTD || OPENTHREAD_FTD #if OPENTHREAD_RADIO OT_NCP_GET_HANDLER_ENTRY(SPINEL_PROP_RCP_API_VERSION), + OT_NCP_GET_HANDLER_ENTRY(SPINEL_PROP_RCP_MIN_HOST_API_VERSION), #endif #if OPENTHREAD_MTD || OPENTHREAD_FTD OT_NCP_GET_HANDLER_ENTRY(SPINEL_PROP_CNTR_TX_PKT_TOTAL), diff --git a/src/ncp/ncp_base_radio.cpp b/src/ncp/ncp_base_radio.cpp index 58e6d0316..443762730 100644 --- a/src/ncp/ncp_base_radio.cpp +++ b/src/ncp/ncp_base_radio.cpp @@ -53,6 +53,11 @@ template <> otError NcpBase::HandlePropertyGet(void { return mEncoder.WriteUintPacked(SPINEL_RCP_API_VERSION); } + +template <> otError NcpBase::HandlePropertyGet(void) +{ + return mEncoder.WriteUintPacked(SPINEL_MIN_HOST_SUPPORTED_RCP_API_VERSION); +} #endif // ----------------------------------------------------------------------------