From c7be823fcff42b43d2750479f7ca75eec1e25f30 Mon Sep 17 00:00:00 2001 From: Ellis Sarza-Nguyen Date: Fri, 29 May 2026 08:47:08 -0700 Subject: [PATCH] [protocol] Convert hello command and libhoth to use the new error As a way to centeralize the way we report error through different domains, we are using hello command as a poc. This should make a temporary libhoth exec to be non instrusive and allow for changes. Signed-off-by: Ellis Sarza-Nguyen --- BUILD | 8 --- examples/htool.c | 29 +++++++- meson.build | 4 +- protocol/BUILD | 8 +++ protocol/hello.c | 14 ++-- protocol/hello.h | 3 +- protocol/hello_test.cc | 2 +- protocol/host_cmd.c | 59 +++++++++++++--- protocol/host_cmd.h | 8 +++ protocol/meson.build | 3 +- protocol/status.c | 96 ++++++++++++++++++++++++++ {include/libhoth => protocol}/status.h | 61 +++++++++------- 12 files changed, 238 insertions(+), 57 deletions(-) create mode 100644 protocol/status.c rename {include/libhoth => protocol}/status.h (62%) diff --git a/BUILD b/BUILD index 1ce7d8d..d0048cb 100644 --- a/BUILD +++ b/BUILD @@ -7,14 +7,6 @@ cc_library( hdrs = [":gen_version_header"], ) -cc_library( - name = "libhoth_status", - hdrs = ["include/libhoth/status.h"], - strip_include_prefix = "include", - visibility = ["//visibility:public"], -) - - genrule( name = "gen_version_header", outs = ["git_version.h"], diff --git a/examples/htool.c b/examples/htool.c index 65a8c36..7c5efc2 100644 --- a/examples/htool.c +++ b/examples/htool.c @@ -71,6 +71,28 @@ #include "transports/libhoth_device.h" #include "transports/libhoth_spi.h" +static void htool_report_error(const char* cmd_name, libhoth_error err) { + if (err == HOTH_SUCCESS) { + return; + } + + uint32_t ctx = LIBHOTH_ERR_GET_CTX(err); + uint16_t space = LIBHOTH_ERR_GET_SPACE(err); + uint16_t code = LIBHOTH_ERR_GET_CODE(err); + + fprintf(stderr, "Error: '%s' failed (0x%016lx)\n", cmd_name, err); + fprintf(stderr, " Context: %u (%s)\n", ctx, libhoth_error_ctx_str(ctx)); + fprintf(stderr, " Space: %u (%s)\n", space, + libhoth_error_space_str(space)); + + if (space == HOTH_HOST_SPACE_FW) { + fprintf(stderr, " Code: %u (%s)\n", code, + libhoth_error_code_fw_str(code)); + } else { + fprintf(stderr, " Code: %u\n", code); + } +} + static int command_usb_list(const struct htool_invocation* inv) { return htool_usb_print_devices(); } @@ -816,9 +838,10 @@ static int command_hello(const struct htool_invocation* inv) { } uint32_t output = 0; - const int rv = libhoth_hello(dev, input, &output); - if (rv) { - return rv; + const libhoth_error err = libhoth_hello(dev, input, &output); + if (err != HOTH_SUCCESS) { + htool_report_error("hello", err); + return -1; } printf("output: 0x%08x\n", output); diff --git a/meson.build b/meson.build index 424b7a5..b6b2921 100644 --- a/meson.build +++ b/meson.build @@ -1,8 +1,8 @@ project('libhoth', 'c', 'cpp', license: 'Apache-2.0', version: '0.0.0') -install_headers('include/libhoth/status.h', subdir: 'libhoth') +# No public headers to install yet during refactor. -libhoth_include_dirs = include_directories('include', '.') +libhoth_include_dirs = include_directories('.') header_subdirs = ['libhoth'] libhoth_objs = [] diff --git a/protocol/BUILD b/protocol/BUILD index e2b9747..fb2ef11 100644 --- a/protocol/BUILD +++ b/protocol/BUILD @@ -3,11 +3,19 @@ load("@rules_cc//cc:cc_test.bzl", "cc_test") package(default_visibility = ["//visibility:public"]) +cc_library( + name = "libhoth_status", + srcs = ["status.c"], + hdrs = ["status.h"], + visibility = ["//visibility:public"], +) + cc_library( name = "host_cmd", srcs = ["host_cmd.c"], hdrs = ["host_cmd.h"], deps = [ + ":libhoth_status", "//transports:libhoth_device", ], ) diff --git a/protocol/hello.c b/protocol/hello.c index 9a05779..1e7ca3f 100644 --- a/protocol/hello.c +++ b/protocol/hello.c @@ -16,15 +16,17 @@ #include -int libhoth_hello(struct libhoth_device* const dev, const uint32_t input, - uint32_t* const output) { +libhoth_error libhoth_hello(struct libhoth_device* const dev, + const uint32_t input, uint32_t* const output) { const struct hoth_request_hello request = { .input = input, }; struct hoth_response_hello response; - const int rv = - libhoth_hostcmd_exec(dev, HOTH_CMD_HELLO, /*version=*/0, &request, - sizeof(request), &response, sizeof(response), NULL); - *output = response.output; + const libhoth_error rv = libhoth_hostcmd_exec_v2( + dev, HOTH_CMD_HELLO, /*version=*/0, &request, sizeof(request), &response, + sizeof(response), NULL); + if (rv == HOTH_SUCCESS) { + *output = response.output; + } return rv; } diff --git a/protocol/hello.h b/protocol/hello.h index 88804d2..b8cb1b0 100644 --- a/protocol/hello.h +++ b/protocol/hello.h @@ -35,7 +35,8 @@ struct hoth_response_hello { uint32_t output; } __hoth_align4; -int libhoth_hello(struct libhoth_device* dev, uint32_t input, uint32_t* output); +libhoth_error libhoth_hello(struct libhoth_device* dev, uint32_t input, + uint32_t* output); #ifdef __cplusplus } diff --git a/protocol/hello_test.cc b/protocol/hello_test.cc index ea77bb6..232ae2d 100644 --- a/protocol/hello_test.cc +++ b/protocol/hello_test.cc @@ -39,6 +39,6 @@ TEST_F(LibHothTest, hello_test) { const uint32_t input = 0xa0b0c0d0; uint32_t output = 0; - EXPECT_EQ(libhoth_hello(&hoth_dev_, input, &output), LIBHOTH_OK); + EXPECT_EQ(libhoth_hello(&hoth_dev_, input, &output), HOTH_SUCCESS); EXPECT_EQ(output, response.output); } diff --git a/protocol/host_cmd.c b/protocol/host_cmd.c index 95cea32..20d6cd2 100644 --- a/protocol/host_cmd.c +++ b/protocol/host_cmd.c @@ -162,10 +162,43 @@ static int validate_ec_response_header( return 0; } +// libhoth_hostcmd_exec is a compatibility wrapper that converts the new +// 64-bit libhoth_error into the legacy 32-bit int return type. This allows +// existing callers to remain unchanged while we incrementally migrate the +// library to the more descriptive libhoth_error system. int libhoth_hostcmd_exec(struct libhoth_device* dev, uint16_t command, uint8_t version, const void* req_payload, size_t req_payload_size, void* resp_buf, size_t resp_buf_size, size_t* out_resp_size) { + // We use this temporary variable to capture the rich 64-bit error from the + // new implementation before downgrading it to the legacy format. + libhoth_error err = libhoth_hostcmd_exec_v2( + dev, command, version, req_payload, req_payload_size, resp_buf, + resp_buf_size, out_resp_size); + + if (err == HOTH_SUCCESS) { + return 0; + } + + uint32_t space = (uint32_t)((err >> 16) & 0xFFFFULL); + uint32_t code = (uint32_t)(err & 0xFFFFULL); + + if (space == HOTH_HOST_SPACE_FW) { + return HTOOL_ERROR_HOST_COMMAND_START + code; + } + + return -1; +} + +// libhoth_hostcmd_exec_v2 is the new core implementation that returns a +// rich 64-bit libhoth_error. It should be used for all new commands and +// when migrating existing commands to the centralized status reporting system. +libhoth_error libhoth_hostcmd_exec_v2(struct libhoth_device* dev, + uint16_t command, uint8_t version, + const void* req_payload, + size_t req_payload_size, void* resp_buf, + size_t resp_buf_size, + size_t* out_resp_size) { struct { struct hoth_host_request hdr; uint8_t @@ -174,7 +207,8 @@ int libhoth_hostcmd_exec(struct libhoth_device* dev, uint16_t command, if (req_payload_size > sizeof(req.payload_buf)) { fprintf(stderr, "req_payload_size too large: %d > %d\n", (int)req_payload_size, (int)sizeof(req.payload_buf)); - return -1; + return LIBHOTH_ERR_CONSTRUCT(HOTH_CTX_CMD_EXEC, HOTH_HOST_SPACE_LIBHOTH, + LIBHOTH_ERR_OUT_UNDERFLOW); } if (req_payload) { memcpy(req.payload_buf, req_payload, req_payload_size); @@ -183,12 +217,14 @@ int libhoth_hostcmd_exec(struct libhoth_device* dev, uint16_t command, req_payload_size, &req.hdr); if (status != 0) { fprintf(stderr, "populate_ec_request_header() failed: %d\n", status); - return -1; + return LIBHOTH_ERR_CONSTRUCT(HOTH_CTX_CMD_EXEC, HOTH_HOST_SPACE_POSIX, + -status); } status = libhoth_send_request(dev, &req, sizeof(req.hdr) + req_payload_size); if (status != LIBHOTH_OK) { fprintf(stderr, "libhoth_send_request() failed: %d\n", status); - return -1; + return LIBHOTH_ERR_CONSTRUCT(HOTH_CTX_CMD_EXEC, HOTH_HOST_SPACE_LIBHOTH, + status); } struct { struct hoth_host_response hdr; @@ -200,12 +236,14 @@ int libhoth_hostcmd_exec(struct libhoth_device* dev, uint16_t command, HOTH_CMD_TIMEOUT_MS_DEFAULT); if (status != LIBHOTH_OK) { fprintf(stderr, "libhoth_receive_response() failed: %d\n", status); - return -1; + return LIBHOTH_ERR_CONSTRUCT(HOTH_CTX_CMD_EXEC, HOTH_HOST_SPACE_LIBHOTH, + status); } status = validate_ec_response_header(&resp.hdr, resp.payload_buf, resp_size); if (status != 0) { fprintf(stderr, "EC response header invalid: %d\n", status); - return -1; + return LIBHOTH_ERR_CONSTRUCT(HOTH_CTX_CMD_EXEC, HOTH_HOST_SPACE_POSIX, + -status); } if (resp.hdr.result != HOTH_RES_SUCCESS) { fprintf(stderr, "EC response contained error: %d", resp.hdr.result); @@ -216,7 +254,8 @@ int libhoth_hostcmd_exec(struct libhoth_device* dev, uint16_t command, } else { fprintf(stderr, "\n"); } - return HTOOL_ERROR_HOST_COMMAND_START + resp.hdr.result; + return LIBHOTH_ERR_CONSTRUCT(HOTH_CTX_CMD_EXEC, HOTH_HOST_SPACE_FW, + resp.hdr.result); } size_t resp_payload_size = resp_size - sizeof(struct hoth_host_response); @@ -226,14 +265,16 @@ int libhoth_hostcmd_exec(struct libhoth_device* dev, uint16_t command, stderr, "Response payload too large to fit in supplied buffer: %zu > %zu\n", resp_payload_size, resp_buf_size); - return -1; + return LIBHOTH_ERR_CONSTRUCT(HOTH_CTX_CMD_EXEC, HOTH_HOST_SPACE_LIBHOTH, + LIBHOTH_ERR_RESPONSE_BUFFER_OVERFLOW); } } else { if (resp_payload_size != resp_buf_size) { fprintf(stderr, "Unexpected response payload size: got %zu expected %zu\n", resp_payload_size, resp_buf_size); - return -1; + return LIBHOTH_ERR_CONSTRUCT(HOTH_CTX_CMD_EXEC, HOTH_HOST_SPACE_LIBHOTH, + LIBHOTH_ERR_FAIL); } } if (resp_buf) { @@ -242,5 +283,5 @@ int libhoth_hostcmd_exec(struct libhoth_device* dev, uint16_t command, if (out_resp_size) { *out_resp_size = resp_payload_size; } - return 0; + return HOTH_SUCCESS; } diff --git a/protocol/host_cmd.h b/protocol/host_cmd.h index 17e8100..882d4b0 100644 --- a/protocol/host_cmd.h +++ b/protocol/host_cmd.h @@ -18,6 +18,7 @@ #include #include +#include "status.h" #include "transports/libhoth_device.h" #ifdef __cplusplus @@ -106,6 +107,13 @@ int libhoth_hostcmd_exec(struct libhoth_device* dev, uint16_t command, size_t req_payload_size, void* resp_buf, size_t resp_buf_size, size_t* out_resp_size); +libhoth_error libhoth_hostcmd_exec_v2(struct libhoth_device* dev, + uint16_t command, uint8_t version, + const void* req_payload, + size_t req_payload_size, void* resp_buf, + size_t resp_buf_size, + size_t* out_resp_size); + uint8_t libhoth_calculate_checksum(const void* header, size_t header_size, const void* data, size_t data_size); diff --git a/protocol/meson.build b/protocol/meson.build index 64b1b33..4fea5d6 100644 --- a/protocol/meson.build +++ b/protocol/meson.build @@ -25,7 +25,8 @@ protocol_srcs = [ 'firmware_update.c', 'util.c', 'console.c', - 'gpio_drive_strength.c' + 'gpio_drive_strength.c', + 'status.c' ] incdir = libhoth_include_dirs diff --git a/protocol/status.c b/protocol/status.c new file mode 100644 index 0000000..a742af6 --- /dev/null +++ b/protocol/status.c @@ -0,0 +1,96 @@ +// Copyright 2026 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "status.h" + +const char* libhoth_error_ctx_str(uint32_t ctx) { + switch (ctx) { + case HOTH_CTX_NONE: + return "NONE"; + case HOTH_CTX_INIT: + return "INIT"; + case HOTH_CTX_USB: + return "USB"; + case HOTH_CTX_SPI: + return "SPI"; + case HOTH_CTX_CMD_EXEC: + return "CMD_EXEC"; + default: + return "UNKNOWN"; + } +} + +const char* libhoth_error_space_str(uint16_t space) { + switch (space) { + case HOTH_HOST_SPACE_FW: + return "FW"; + case HOTH_HOST_SPACE_POSIX: + return "POSIX"; + case HOTH_HOST_SPACE_LIBUSB: + return "LIBUSB"; + case HOTH_HOST_SPACE_LIBHOTH: + return "LIBHOTH"; + default: + return "UNKNOWN"; + } +} + +const char* libhoth_error_code_fw_str(uint16_t code) { + switch (code) { + case HOTH_FW_SUCCESS: + return "SUCCESS"; + case HOTH_FW_INVALID_COMMAND: + return "INVALID_COMMAND"; + case HOTH_FW_ERROR: + return "ERROR"; + case HOTH_FW_INVALID_PARAM: + return "INVALID_PARAM"; + case HOTH_FW_ACCESS_DENIED: + return "ACCESS_DENIED"; + case HOTH_FW_INVALID_RESPONSE: + return "INVALID_RESPONSE"; + case HOTH_FW_INVALID_VERSION: + return "INVALID_VERSION"; + case HOTH_FW_INVALID_CHECKSUM: + return "INVALID_CHECKSUM"; + case HOTH_FW_IN_PROGRESS: + return "IN_PROGRESS"; + case HOTH_FW_UNAVAILABLE: + return "UNAVAILABLE"; + case HOTH_FW_TIMEOUT: + return "TIMEOUT"; + case HOTH_FW_OVERFLOW: + return "OVERFLOW"; + case HOTH_FW_INVALID_HEADER: + return "INVALID_HEADER"; + case HOTH_FW_REQUEST_TRUNCATED: + return "REQUEST_TRUNCATED"; + case HOTH_FW_RESPONSE_TOO_BIG: + return "RESPONSE_TOO_BIG"; + case HOTH_FW_BUS_ERROR: + return "BUS_ERROR"; + case HOTH_FW_BUSY: + return "BUSY"; + case HOTH_FW_INVALID_HEADER_VERSION: + return "INVALID_HEADER_VERSION"; + case HOTH_FW_INVALID_HEADER_CRC: + return "INVALID_HEADER_CRC"; + case HOTH_FW_INVALID_DATA_CRC: + return "INVALID_DATA_CRC"; + case HOTH_FW_DUP_UNAVAILABLE: + return "DUP_UNAVAILABLE"; + default: + return "UNKNOWN"; + } +} diff --git a/include/libhoth/status.h b/protocol/status.h similarity index 62% rename from include/libhoth/status.h rename to protocol/status.h index dfd889a..411a7eb 100644 --- a/include/libhoth/status.h +++ b/protocol/status.h @@ -1,4 +1,3 @@ - // Copyright 2026 Google LLC // // Licensed under the Apache License, Version 2.0 (the "License"); @@ -13,8 +12,8 @@ // See the License for the specific language governing permissions and // limitations under the License. -#ifndef LIBHOTH_INCLUDE_LIBHOTH_STATUS_H_ -#define LIBHOTH_INCLUDE_LIBHOTH_STATUS_H_ +#ifndef LIBHOTH_PROTOCOL_STATUS_H_ +#define LIBHOTH_PROTOCOL_STATUS_H_ #include @@ -22,6 +21,10 @@ typedef uint64_t libhoth_error; +const char* libhoth_error_ctx_str(uint32_t ctx); +const char* libhoth_error_space_str(uint16_t space); +const char* libhoth_error_code_fw_str(uint16_t code); + /** Constructs a libhoth error code from its components. * Cxt uniquely identifies the libhoth operation or subsystem. * Space indicates the domain of errors. @@ -31,6 +34,12 @@ typedef uint64_t libhoth_error; ((((uint64_t)(ctx) & 0xFFFFFFFFULL) << 32) | \ (((uint64_t)(space) & 0xFFFFULL) << 16) | ((uint64_t)(code) & 0xFFFFULL)) +#define LIBHOTH_ERR_GET_CTX(err) \ + ((uint32_t)(((uint64_t)(err) >> 32) & 0xFFFFFFFFULL)) +#define LIBHOTH_ERR_GET_SPACE(err) \ + ((uint16_t)(((uint64_t)(err) >> 16) & 0xFFFFULL)) +#define LIBHOTH_ERR_GET_CODE(err) ((uint16_t)((uint64_t)(err) & 0xFFFFULL)) + // hoth_context_id: High 32 bits of the error code. // Uniquely identifies the libhoth operation or subsystem. enum hoth_context_id { @@ -57,28 +66,28 @@ enum hoth_host_space { // Hoth error code: Low 16 bits of the 32-bit Base Error Code // Firmware Error enum hoth_fw_error_status { - HOTH_RES_SUCCESS = 0, - HOTH_RES_INVALID_COMMAND = 1, - HOTH_RES_ERROR = 2, - HOTH_RES_INVALID_PARAM = 3, - HOTH_RES_ACCESS_DENIED = 4, - HOTH_RES_INVALID_RESPONSE = 5, - HOTH_RES_INVALID_VERSION = 6, - HOTH_RES_INVALID_CHECKSUM = 7, - HOTH_RES_IN_PROGRESS = 8, - HOTH_RES_UNAVAILABLE = 9, - HOTH_RES_TIMEOUT = 10, - HOTH_RES_OVERFLOW = 11, - HOTH_RES_INVALID_HEADER = 12, - HOTH_RES_REQUEST_TRUNCATED = 13, - HOTH_RES_RESPONSE_TOO_BIG = 14, - HOTH_RES_BUS_ERROR = 15, - HOTH_RES_BUSY = 16, - HOTH_RES_INVALID_HEADER_VERSION = 17, - HOTH_RES_INVALID_HEADER_CRC = 18, - HOTH_RES_INVALID_DATA_CRC = 19, - HOTH_RES_DUP_UNAVAILABLE = 20, - HOTH_RES_MAX = UINT16_MAX + HOTH_FW_SUCCESS = 0, + HOTH_FW_INVALID_COMMAND = 1, + HOTH_FW_ERROR = 2, + HOTH_FW_INVALID_PARAM = 3, + HOTH_FW_ACCESS_DENIED = 4, + HOTH_FW_INVALID_RESPONSE = 5, + HOTH_FW_INVALID_VERSION = 6, + HOTH_FW_INVALID_CHECKSUM = 7, + HOTH_FW_IN_PROGRESS = 8, + HOTH_FW_UNAVAILABLE = 9, + HOTH_FW_TIMEOUT = 10, + HOTH_FW_OVERFLOW = 11, + HOTH_FW_INVALID_HEADER = 12, + HOTH_FW_REQUEST_TRUNCATED = 13, + HOTH_FW_RESPONSE_TOO_BIG = 14, + HOTH_FW_BUS_ERROR = 15, + HOTH_FW_BUSY = 16, + HOTH_FW_INVALID_HEADER_VERSION = 17, + HOTH_FW_INVALID_HEADER_CRC = 18, + HOTH_FW_INVALID_DATA_CRC = 19, + HOTH_FW_DUP_UNAVAILABLE = 20, + HOTH_FW_MAX = UINT16_MAX } __packed; -#endif // LIBHOTH_INCLUDE_LIBHOTH_STATUS_H_ +#endif // LIBHOTH_PROTOCOL_STATUS_H_