Skip to content

Make hiffy_decode return a non-nested error#661

Merged
mkeeter merged 1 commit into
masterfrom
mkeeter/hiffy-decode-flat
May 21, 2026
Merged

Make hiffy_decode return a non-nested error#661
mkeeter merged 1 commit into
masterfrom
mkeeter/hiffy-decode-flat

Conversation

@mkeeter
Copy link
Copy Markdown
Contributor

@mkeeter mkeeter commented May 12, 2026

(staged on #660)

hiffy_decode returns a Result<std::result::Result<T, String>>, which is an awkward type.

We can instead use the HiffyCallError (renamed HiffyError), which distinguishes between internal errors (returned by the HIF program) and external errors (which occur when calling hiffy).

This simplifies most cases (where we want to return both inner and outer error), and adds a tiny bit of boilerplate to cases where we care about the difference – but that boilerplate is more clear than the nested type.

@mkeeter mkeeter requested a review from labbott May 12, 2026 21:46
@mkeeter mkeeter force-pushed the mkeeter/loading-idol-result branch from e0a515b to 2dea381 Compare May 13, 2026 21:41
@mkeeter mkeeter force-pushed the mkeeter/hiffy-decode-flat branch from 70f547e to 02dc755 Compare May 13, 2026 21:41
@mkeeter mkeeter force-pushed the mkeeter/loading-idol-result branch from 2dea381 to 861ffd1 Compare May 14, 2026 16:20
@mkeeter mkeeter force-pushed the mkeeter/hiffy-decode-flat branch from 02dc755 to 27e0c3a Compare May 14, 2026 16:20
@mkeeter mkeeter force-pushed the mkeeter/loading-idol-result branch from 861ffd1 to c3de5fa Compare May 14, 2026 16:48
@mkeeter mkeeter force-pushed the mkeeter/hiffy-decode-flat branch from 27e0c3a to 1e0ed19 Compare May 14, 2026 16:48
@mkeeter mkeeter force-pushed the mkeeter/loading-idol-result branch from c3de5fa to e66628c Compare May 14, 2026 19:08
@mkeeter mkeeter force-pushed the mkeeter/hiffy-decode-flat branch from 1e0ed19 to a35dcb6 Compare May 14, 2026 19:08
@mkeeter mkeeter force-pushed the mkeeter/loading-idol-result branch from e66628c to a9801cb Compare May 18, 2026 19:30
@mkeeter mkeeter force-pushed the mkeeter/hiffy-decode-flat branch 2 times, most recently from 99df9fd to effa762 Compare May 18, 2026 19:53
@mkeeter mkeeter force-pushed the mkeeter/loading-idol-result branch 2 times, most recently from 61239ac to 1fe703a Compare May 18, 2026 20:21
@mkeeter mkeeter force-pushed the mkeeter/hiffy-decode-flat branch from effa762 to 0b5c70a Compare May 18, 2026 20:21
@mkeeter mkeeter force-pushed the mkeeter/loading-idol-result branch 3 times, most recently from 4a5d1b2 to a25af4c Compare May 18, 2026 20:53
@mkeeter mkeeter force-pushed the mkeeter/hiffy-decode-flat branch from 0b5c70a to 607b95e Compare May 18, 2026 20:54
Copy link
Copy Markdown
Contributor

@labbott labbott left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor question about performance but it looks correct

Comment thread cmd/monorail/src/lib.rs
Comment on lines +749 to +759
let cfg = s.field::<Struct>("cfg")?;
assert_eq!(cfg.name(), "PortConfig");
let dev = decode_dev(&cfg.field("dev")?)?;
let serdes = decode_dev(&cfg.field("serdes")?)?;
let (mode, speed) = decode_mode(&cfg["mode"]);
(
dev.replace("DEV", ""),
serdes.replace("SERDES", ""),
mode,
speed,
)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How expensive is each .field call?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a lookup in a hashmap and then a Load call (basically the same as before, the hashmap lookup was previously hidden in the indexing operation).

@mkeeter mkeeter force-pushed the mkeeter/loading-idol-result branch from a25af4c to f59ccb1 Compare May 20, 2026 19:40
@mkeeter mkeeter force-pushed the mkeeter/hiffy-decode-flat branch 2 times, most recently from b28c3a5 to f98a018 Compare May 20, 2026 20:09
@mkeeter mkeeter force-pushed the mkeeter/loading-idol-result branch 2 times, most recently from 84759c9 to 46713c4 Compare May 20, 2026 20:45
@mkeeter mkeeter force-pushed the mkeeter/hiffy-decode-flat branch from f98a018 to 82dd66e Compare May 20, 2026 20:45
@mkeeter mkeeter force-pushed the mkeeter/loading-idol-result branch from 46713c4 to 7e4fe3f Compare May 20, 2026 23:03
@mkeeter mkeeter force-pushed the mkeeter/hiffy-decode-flat branch from 82dd66e to 1d7bdca Compare May 20, 2026 23:03
@mkeeter mkeeter force-pushed the mkeeter/loading-idol-result branch from 7e4fe3f to d72d23e Compare May 21, 2026 13:45
@mkeeter mkeeter force-pushed the mkeeter/hiffy-decode-flat branch from 1d7bdca to ced5065 Compare May 21, 2026 13:45
@mkeeter mkeeter force-pushed the mkeeter/loading-idol-result branch from d72d23e to fdfe094 Compare May 21, 2026 14:09
@mkeeter mkeeter force-pushed the mkeeter/hiffy-decode-flat branch from ced5065 to ff30bea Compare May 21, 2026 14:09
@mkeeter mkeeter force-pushed the mkeeter/loading-idol-result branch from fdfe094 to 88959bc Compare May 21, 2026 14:24
@mkeeter mkeeter force-pushed the mkeeter/hiffy-decode-flat branch from ff30bea to 38483a1 Compare May 21, 2026 14:24
@mkeeter mkeeter force-pushed the mkeeter/loading-idol-result branch from 88959bc to ee01fe0 Compare May 21, 2026 14:37
@mkeeter mkeeter force-pushed the mkeeter/hiffy-decode-flat branch 2 times, most recently from e6a5158 to 70d7c6e Compare May 21, 2026 15:49
@mkeeter mkeeter force-pushed the mkeeter/loading-idol-result branch from ee01fe0 to 22fae46 Compare May 21, 2026 15:49
@mkeeter mkeeter force-pushed the mkeeter/hiffy-decode-flat branch from 70d7c6e to 649d9d4 Compare May 21, 2026 16:45
@mkeeter mkeeter force-pushed the mkeeter/loading-idol-result branch from 22fae46 to 4718de5 Compare May 21, 2026 16:45
@mkeeter mkeeter force-pushed the mkeeter/hiffy-decode-flat branch from 649d9d4 to c120380 Compare May 21, 2026 17:31
@mkeeter mkeeter force-pushed the mkeeter/loading-idol-result branch 2 times, most recently from 36ecdb0 to 5171b49 Compare May 21, 2026 17:52
@mkeeter mkeeter force-pushed the mkeeter/hiffy-decode-flat branch from c120380 to f1b8bcc Compare May 21, 2026 17:52
Base automatically changed from mkeeter/loading-idol-result to master May 21, 2026 18:13
@mkeeter mkeeter force-pushed the mkeeter/hiffy-decode-flat branch 2 times, most recently from e722703 to bfd295e Compare May 21, 2026 20:03
@mkeeter mkeeter force-pushed the mkeeter/hiffy-decode-flat branch from bfd295e to 24d7716 Compare May 21, 2026 20:20
@mkeeter
Copy link
Copy Markdown
Contributor Author

mkeeter commented May 21, 2026

Quick test sweep over affected commands:

matt@niles ~ () $ pfexec ./humility -t cosmo-b host cosmo last-post-code
humility: attached to 0483:374f:002A001C4D46500F20373033 via ST-Link V3
Bootloader Code: 0xed80000f
  Source:  ASP TEE
  Status:  BL_ERR_BOUNDARY_CHECK (0x0f)
  Detail:  Out of Boundary Condition Reached
matt@niles ~ () $ pfexec ./humility -t cosmo-b host cosmo post-codes | head -n10
humility: attached to 0483:374f:002A001C4D46500F20373033 via ST-Link V3
Bootloader Code: 0xee1000b3
  Source:  ASP BL2
  Status:  BL_SUCCESS_BYPASS_IDEVID_CHECK (0xb3)
  Detail:  IDEVID validation failed but bypassed (unsecure)
Bootloader Code: 0xee1000a0
  Source:  ASP BL2
  Status:  BL_SUCCESS_C_MAIN (0xa0)
  Detail:  Successfully entered C Main
Bootloader Code: 0xee1000a3
  Source:  ASP BL2

matt@niles ~ () $ pfexec ./humility -t sidecar-b monorail dump DEV1G[0]
humility: attached to 0483:3754:002600184D4B500E20373831 via ST-Link V3
Dumping target DEV1G[0] (0x71040000 -> 0x710400a0)
DEV1G[0]:DEV_CFG_STATUS:DEV_RST_CTRL    0x00100000
DEV1G[0]:DEV_CFG_STATUS:DEV_STICKY    0x00004000
DEV1G[0]:DEV_CFG_STATUS:DEV_DBG_CFG    0x00000800
DEV1G[0]:DEV_CFG_STATUS:DEV_PORT_PROTECT    0x00000000
DEV1G[0]:DEV_CFG_STATUS:EEE_CFG    0x0011940a
DEV1G[0]:DEV_CFG_STATUS:PTP_CFG    0x00400000
DEV1G[0]:DEV_CFG_STATUS:PTP_EVENTS    0x00000000
DEV1G[0]:MAC_CFG_STATUS:MAC_ENA_CFG    0x00000011
DEV1G[0]:MAC_CFG_STATUS:MAC_MODE_CFG    0x00000001
DEV1G[0]:MAC_CFG_STATUS:MAC_MAXLEN_CFG    0x000005ee
DEV1G[0]:MAC_CFG_STATUS:MAC_TAGS_CFG    0x88a80008
DEV1G[0]:MAC_CFG_STATUS:MAC_TAGS_CFG2    0x88a888a8
DEV1G[0]:MAC_CFG_STATUS:MAC_ADV_CHK_CFG    0x00000000
DEV1G[0]:MAC_CFG_STATUS:MAC_IFG_CFG    0x00000641
DEV1G[0]:MAC_CFG_STATUS:MAC_HDX_CFG    0x00000043
DEV1G[0]:MAC_CFG_STATUS:MAC_STICKY    0x00000000

matt@niles ~ () $ pfexec ./humility -t sidecar-b monorail status
humility: attached to 0483:3754:002600184D4B500E20373831 via ST-Link V3
PORT | MODE    SPEED  DEV     SERDES  LINK |   PHY    MAC LINK  MEDIA LINK
-----|-------------------------------------|-------------------------------
 0   | SGMII   100M   1G_0    1G_1    down | --       --         --
 1   | SGMII   100M   1G_1    1G_2    down | --       --         --
 2   | SGMII   100M   1G_2    1G_3    down | --       --         --
 3   | SGMII   100M   1G_3    1G_4    down | --       --         --
 4   | SGMII   100M   1G_4    1G_5    down | --       --         --
 5   | SGMII   100M   1G_5    1G_6    down | --       --         --
 6   | SGMII   100M   1G_6    1G_7    down | --       --         --
 7   | SGMII   100M   1G_7    1G_8    down | --       --         --
 8   | SGMII   100M   2G5_0   6G_0    down | --       --         --
 9   | SGMII   100M   2G5_1   6G_1    down | --       --         --
 10  | SGMII   100M   2G5_2   6G_2    down | --       --         --
 11  | SGMII   100M   2G5_3   6G_3    down | --       --         --
 12  | SGMII   100M   2G5_4   6G_4    down | --       --         --
 13  | SGMII   100M   2G5_5   6G_5    up   | --       --         --
 14  | SGMII   100M   2G5_6   6G_6    up   | --       --         --
 15  | SGMII   100M   2G5_7   6G_7    up   | --       --         --
 16  | SGMII   100M   2G5_8   6G_8    down | --       --         --
 17  | SGMII   100M   2G5_9   6G_9    down | --       --         --
 18  | SGMII   100M   2G5_10  6G_10   down | --       --         --
 19  | SGMII   100M   2G5_11  6G_11   down | --       --         --
 20  | SGMII   100M   2G5_12  6G_12   down | --       --         --
 21  | SGMII   100M   2G5_13  6G_13   down | --       --         --
 22  | --      --     --      --      --   | --       --         --
 23  | --      --     --      --      --   | --       --         --
 24  | SGMII   100M   2G5_16  6G_16   down | --       --         --
 25  | SGMII   100M   2G5_17  6G_17   down | --       --         --
 26  | SGMII   100M   2G5_18  6G_18   down | --       --         --
 27  | SGMII   100M   2G5_19  6G_19   down | --       --         --
 28  | SGMII   100M   2G5_20  6G_20   down | --       --         --
 29  | SGMII   100M   2G5_21  6G_21   down | --       --         --
 30  | SGMII   100M   2G5_22  6G_22   down | --       --         --
 31  | SGMII   100M   2G5_23  6G_23   down | --       --         --
 32  | --      --     --      --      --   | --       --         --
 33  | --      --     --      --      --   | --       --         --
 34  | --      --     --      --      --   | --       --         --
 35  | --      --     --      --      --   | --       --         --
 36  | --      --     --      --      --   | --       --         --
 37  | --      --     --      --      --   | --       --         --
 38  | --      --     --      --      --   | --       --         --
 39  | --      --     --      --      --   | --       --         --
 40  | QSGMII  100M   1G_16   6G_14   up   | VSC8504  up        up
 41  | QSGMII  100M   1G_17   6G_14   up   | VSC8504  up        down
 42  | QSGMII  100M   1G_18   6G_14   up   | VSC8504  up        down
 43  | QSGMII  100M   1G_19   6G_14   up   | VSC8504  up        down
 44  | QSGMII  1G     1G_20   6G_15   up   | VSC8562  err       up
 45  | QSGMII  1G     1G_21   6G_15   up   | VSC8562  err       down
 46  | --      --     --      --      --   | --       --         --
 47  | --      --     --      --      --   | --       --         --
 48  | SGMII   100M   2G5_24  1G_0    up   | --       --         --
 49  | BASEKR  --     10G_0   10G_0   down | --       --         --
 50  | --      --     --      --      --   | --       --         --
 51  | SGMII   100M   2G5_27  10G_2   down | --       --         --
 52  | SGMII   100M   2G5_28  10G_3   down | --       --         --

matt@niles ~ () $ pfexec ./humility -t sidecar-b net mac
humility: attached to 0483:3754:002600184D4B500E20373831 via ST-Link V3
humility: Reading 7 MAC addresses...
 PORT |        MAC
------|-------------------
    1 | 00:0e:c6:2c:bb:12
      | 1c:61:b4:19:39:40
      | e8:ea:6a:09:ca:3d
    3 | a8:40:25:05:ff:00
      | a8:40:25:05:ff:00
      | a8:40:25:05:ff:00
      | a8:40:25:05:ff:01

@mkeeter mkeeter merged commit f679141 into master May 21, 2026
12 checks passed
@mkeeter mkeeter deleted the mkeeter/hiffy-decode-flat branch May 21, 2026 20:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants