From 5f878aa8d52c5aa000d7db8c9f6afa6c1ff0e6ff Mon Sep 17 00:00:00 2001 From: johnchia Date: Mon, 7 Sep 2026 18:51:59 -0700 Subject: [PATCH 1/3] hisi: read the V5 die ID from the OTP shadow `ipcinfo -i` has covered only V4, so every HISI_OT part -- 3516CV608, 3516CV610, 3516CV613, 3516DV500, 3519DV500 -- answers with nothing. On OpenIPC that is not cosmetic: rcS derives the camera's MAC address from this value, so a V5 board with no vendor-provisioned ethaddr comes up on the shared placeholder and a second one collides with it. V5 has no counterpart to the V4 die-ID block at 0x12020400. The vendor reads its die ID through a bootrom call, but the OTP rows are shadowed into a register window at the offset they occupy in OTP, which puts OTP_DIE_ID's 16 bytes at 0x101E00F0. Offsets are from the CV610 SDK and cited in the comment; the window is reachable from the non-secure side, as the OEM's own hwconf.ko ioremaps 0x101E010C from a kernel module. An unfused or unreadable row reads all-zeroes or all-ones. Reject both: a caller that turns the string into a MAC would otherwise give every board in a fleet the same address. `if (!serial)` in print_serial() tested the address of a stack array, which is never null, so a chip with no reader printed whatever the stack held rather than failing. Honour the reader's return value instead. Measured on a Hi3516CV608 (H4-52POX-S). The shadow window reads: 0x101E00F0 = 0x97090A14 0x101E00F4 = 0x20E41210 0x101E00F8 = 0x39182919 0x101E00FC = 0x04921EAC 0x101E010C = 0x00240180 That last one is the window identifying itself: 0x240180 is exactly OTP_608_VERSION_ID from the SDK's svb.h, so this is the OTP shadow and not some unrelated block that happens to hold entropy. `ipcinfo -i` went from printing nothing to 140a09971012e42019291839ac1e9204. One part cannot prove a value is per-die rather than per-model. This is the field the vendor documents as the die ID, which is a far stronger prior than a register found by sweeping, but a second V5 part is what would settle it. Claude-Session: https://claude.ai/code/session_011qHvUfNDKptXf41shaU1vE --- example/ipcinfo.c | 13 ++++++--- src/hal/hisi/hal_hisi.h | 1 + src/hal/hisi/ispreg.c | 65 ++++++++++++++++++++++++++++++++++++++++- 3 files changed, 74 insertions(+), 5 deletions(-) diff --git a/example/ipcinfo.c b/example/ipcinfo.c index a467226..0590807 100644 --- a/example/ipcinfo.c +++ b/example/ipcinfo.c @@ -97,17 +97,22 @@ static void print_chip_temperature() { } static void print_serial() { - char serial[512]; + char serial[512] = {0}; + bool found = false; const char *vendor = getchipvendor(); if (strstr(vendor, VENDOR_HISI) || strstr(vendor, VENDOR_GOKE)) - hisi_ev300_get_die_id(serial, sizeof serial); + found = hisi_get_die_id(serial, sizeof serial); #ifdef IPCHW_VENDOR_SSTAR if (strstr(vendor, VENDOR_SSTAR)) - sstar_get_die_id(serial, sizeof serial); + found = sstar_get_die_id(serial, sizeof serial); #endif - if (!serial) + // The test used to be `if (!serial)`, the address of a stack array, which + // is never null -- a chip with no reader printed whatever the stack held. + // Provisioning scripts derive a MAC from this, so a miss has to be silent + // on stdout and non-zero on exit. + if (!found || !*serial) exit(EXIT_FAILURE); puts(serial); } diff --git a/src/hal/hisi/hal_hisi.h b/src/hal/hisi/hal_hisi.h index dfb31be..58480fa 100644 --- a/src/hal/hisi/hal_hisi.h +++ b/src/hal/hisi/hal_hisi.h @@ -30,6 +30,7 @@ bool hisi_ev300_get_die_id(char *buf, ssize_t len); +bool hisi_get_die_id(char *buf, ssize_t len); void hisi_vi_information(sensor_ctx_t *ctx); unsigned long hisi_totalmem(unsigned long *media_mem); bool hisi_detect_cpu(char *chip_name, uint32_t SC_CTRL_base); diff --git a/src/hal/hisi/ispreg.c b/src/hal/hisi/ispreg.c index 3ad798c..b5e15e8 100644 --- a/src/hal/hisi/ispreg.c +++ b/src/hal/hisi/ispreg.c @@ -714,6 +714,69 @@ bool hisi_ev300_get_die_id(char *buf, ssize_t len) { return true; } +/* Per-die identity on V5 (HISI_OT). + * + * V5 has no counterpart to the V4 die-ID block at 0x12020400. The vendor keeps + * the die ID in OTP and reaches it through a bootrom call (otp_get_die_id() in + * gsl/drivers/share_drivers/share_drivers.c), but every OTP row is also + * shadowed into a register window at the same offset it occupies in OTP, so + * the 16 bytes of OTP_DIE_ID appear at OTP_SHADOW_BASE + 0xF0. Offsets from + * Hi3516CV610_SDK_V1.0.2.0: + * .../bsp/components/gsl/drivers/otp/otp.h OTP_DIE_ID 0xF0, 16 bytes + * .../bsp/components/gsl/include/platform.h OTP_SHADOW_BASE = 0x101E0000 + * + * The window is readable from the non-secure side: the OEM's own hwconf.ko + * ioremaps OTP_SHADOW_BASE + 0x10C (OTP_VERSION_ID_REG, the ATE chip version) + * from an ordinary kernel module. + * + * Bytes are emitted in OTP order. The shadow words are little-endian, so byte + * i of what otp_get_die_id() would hand back is word[i / 4] >> (8 * (i % 4)). + */ +#define V5_OTP_SHADOW_BASE 0x101E0000u +#define V5_OTP_DIE_ID 0xF0 +#define V5_DIE_ID_WORDS 4 + +static bool hisi_ot_get_die_id(char *buf, ssize_t len) { + if (len < V5_DIE_ID_WORDS * 8 + 1) + return false; + + uint32_t id[V5_DIE_ID_WORDS]; + uint32_t any_bit_set = 0; + uint32_t all_bits_set = 0xFFFFFFFF; + for (int i = 0; i < V5_DIE_ID_WORDS; i++) { + if (!mem_reg(V5_OTP_SHADOW_BASE + V5_OTP_DIE_ID + i * 4, &id[i], + OP_READ)) + return false; + any_bit_set |= id[i]; + all_bits_set &= id[i]; + } + + // An unfused or unreadable row reads all-zeroes or all-ones. Neither is an + // identity, and callers turn this string into a MAC address -- handing one + // out would give every board in a fleet the same address, so fail instead. + if (!any_bit_set || all_bits_set == 0xFFFFFFFF) + return false; + + char *ptr = buf; + for (int i = 0; i < V5_DIE_ID_WORDS; i++) + for (int b = 0; b < 4; b++) + ptr += snprintf(ptr, buf + len - ptr, "%02x", + (id[i] >> (8 * b)) & 0xFF); + + return true; +} + +bool hisi_get_die_id(char *buf, ssize_t len) { + switch (chip_generation) { + case HISI_V4: + return hisi_ev300_get_die_id(buf, len); + case HISI_OT: + return hisi_ot_get_die_id(buf, len); + default: + return false; + } +} + #define CV300_ISP_AF_CFG_ADDR 0x12200 struct CV300_ISP_AF_CFG { bool en : 1; @@ -927,7 +990,7 @@ struct PT_OFFSET { void hisi_chip_properties(cJSON *j_inner) { char buf[1024]; - if (hisi_ev300_get_die_id(buf, sizeof buf)) { + if (hisi_get_die_id(buf, sizeof buf)) { ADD_PARAM("id", buf); } } From c64e95396148d904989d21b99ab15018d897fdaf Mon Sep 17 00:00:00 2001 From: johnchia Date: Wed, 9 Sep 2026 00:30:36 -0700 Subject: [PATCH 2/3] hisi: apply the die-ID reject on V4 too, and tidy the V5 reader Review follow-up on the OTP shadow reader. The all-zero/all-ones reject only covered the V5 arm, so an unfused V4 die-ID block still returned true: the trailing-zero strip never finds a non-zero digit, never breaks, and `ipcinfo -i` prints 48 zeros with exit 0. The check also combined all four V5 words, so a partially fused row reading {0xFFFFFFFF, 0xFFFFFFFF, 0, 0} passed as an identity. Move it to the dispatcher, where it guards both arms, and test the emitted digits rather than whole words so the partial mixture is caught as well. A legitimate id containing a zero word still passes. hisi_ot_get_die_id() now assembles the sixteen bytes and hex-encodes them in one indexed loop, as bootrom.c does, instead of sixteen snprintf() calls whose running pointer arithmetic leaned on the length precheck. hisi_ev300_get_die_id() becomes static -- the dispatcher is its only caller now -- and loses its inner generation guard, so the routing is encoded in one place. Both readers take size_t, matching sstar_get_die_id(), which is called beside them in print_serial(); the V4 arm gains the length precheck that makes its `len -= outsz` safe under an unsigned type. print_serial() drops the zero-fill and the `!*serial` retest: `found` is the authoritative signal, and every reader writes before returning true. Verified by extracting both readers and the filter into a host harness with a stubbed mem_reg(): the CV608's measured words still encode to 140a09971012e42019291839ac1e9204, byte-identical to the pre-refactor output, and the all-zero, all-ones and partially fused cases are rejected on both arms. Claude-Session: https://claude.ai/code/session_0135iarzELmev2nXzBx4SFLU --- example/ipcinfo.c | 6 +-- src/hal/hisi/hal_hisi.h | 3 +- src/hal/hisi/ispreg.c | 98 +++++++++++++++++++++++++---------------- 3 files changed, 64 insertions(+), 43 deletions(-) diff --git a/example/ipcinfo.c b/example/ipcinfo.c index 0590807..2e13e57 100644 --- a/example/ipcinfo.c +++ b/example/ipcinfo.c @@ -97,7 +97,7 @@ static void print_chip_temperature() { } static void print_serial() { - char serial[512] = {0}; + char serial[512]; bool found = false; const char *vendor = getchipvendor(); @@ -108,11 +108,9 @@ static void print_serial() { found = sstar_get_die_id(serial, sizeof serial); #endif - // The test used to be `if (!serial)`, the address of a stack array, which - // is never null -- a chip with no reader printed whatever the stack held. // Provisioning scripts derive a MAC from this, so a miss has to be silent // on stdout and non-zero on exit. - if (!found || !*serial) + if (!found) exit(EXIT_FAILURE); puts(serial); } diff --git a/src/hal/hisi/hal_hisi.h b/src/hal/hisi/hal_hisi.h index 58480fa..dabfe44 100644 --- a/src/hal/hisi/hal_hisi.h +++ b/src/hal/hisi/hal_hisi.h @@ -29,8 +29,7 @@ #define IS_7205V500 IS_CHIP("7205V500") || IS_CHIP("7205V510") || IS_CHIP("7205V530") -bool hisi_ev300_get_die_id(char *buf, ssize_t len); -bool hisi_get_die_id(char *buf, ssize_t len); +bool hisi_get_die_id(char *buf, size_t len); void hisi_vi_information(sensor_ctx_t *ctx); unsigned long hisi_totalmem(unsigned long *media_mem); bool hisi_detect_cpu(char *chip_name, uint32_t SC_CTRL_base); diff --git a/src/hal/hisi/ispreg.c b/src/hal/hisi/ispreg.c index b5e15e8..e725b65 100644 --- a/src/hal/hisi/ispreg.c +++ b/src/hal/hisi/ispreg.c @@ -685,15 +685,16 @@ static void hisi_ev300_sensor_clock(cJSON *j_inner) { } } -bool hisi_ev300_get_die_id(char *buf, ssize_t len) { - if (chip_generation != HISI_V4) { +static bool hisi_ev300_get_die_id(char *buf, size_t len) { + const uint32_t base_id_addr = 0x12020400; + const int words = 6; + + if (len < (size_t)words * 8 + 1) return false; - } - uint32_t base_id_addr = 0x12020400; char *ptr = buf; - for (uint32_t id_addr = base_id_addr + 5 * 4; id_addr >= base_id_addr; - id_addr -= 4) { + for (uint32_t id_addr = base_id_addr + (words - 1) * 4; + id_addr >= base_id_addr; id_addr -= 4) { uint32_t val; if (!mem_reg(id_addr, &val, OP_READ)) return false; @@ -720,61 +721,84 @@ bool hisi_ev300_get_die_id(char *buf, ssize_t len) { * the die ID in OTP and reaches it through a bootrom call (otp_get_die_id() in * gsl/drivers/share_drivers/share_drivers.c), but every OTP row is also * shadowed into a register window at the same offset it occupies in OTP, so - * the 16 bytes of OTP_DIE_ID appear at OTP_SHADOW_BASE + 0xF0. Offsets from - * Hi3516CV610_SDK_V1.0.2.0: - * .../bsp/components/gsl/drivers/otp/otp.h OTP_DIE_ID 0xF0, 16 bytes - * .../bsp/components/gsl/include/platform.h OTP_SHADOW_BASE = 0x101E0000 + * the 16 bytes of OTP_DIE_ID appear at OTP_SHADOW_BASE + 0xF0. * - * The window is readable from the non-secure side: the OEM's own hwconf.ko - * ioremaps OTP_SHADOW_BASE + 0x10C (OTP_VERSION_ID_REG, the ATE chip version) - * from an ordinary kernel module. + * Both vendor SDKs that cover the five HISI_OT parts agree on the window: + * Hi3516CV610_SDK_V1.0.2.0, covering 3516CV608/CV610/CV613: + * gsl/include/platform.h OTP_SHADOW_BASE = SCPU_OTPC_BASE_ADDR + * = 0x101E0000 + * gsl/drivers/otp/otp.h OTP_DIE_ID 0xF0, 16 bytes + * Hi3519DV500 SDK R11, covering both DV500 parts in one bsp tree (see + * bsp/pub/hi3516dv500_image_glibc and svb.h's OTP_16D/OTP_19D ids): + * gsl/include/platform.h OTP_SHADOW_BASE = SCPU_OTPC_BASE_ADDR + * = 0x101E0000 + * gsl/drivers/otp/otp.h OTP_DIE_ID 0xF0, 16 bytes * - * Bytes are emitted in OTP order. The shadow words are little-endian, so byte - * i of what otp_get_die_id() would hand back is word[i / 4] >> (8 * (i % 4)). + * The window is readable from the non-secure side: the OEM's own hwconf.ko + * ioremaps the ATE chip version register in it from an ordinary kernel module + * (OTP_VERSION_ID_REG, +0x10C on the CV6xx parts and +0x120 on the DV500s). */ #define V5_OTP_SHADOW_BASE 0x101E0000u #define V5_OTP_DIE_ID 0xF0 #define V5_DIE_ID_WORDS 4 -static bool hisi_ot_get_die_id(char *buf, ssize_t len) { - if (len < V5_DIE_ID_WORDS * 8 + 1) +static bool hisi_ot_get_die_id(char *buf, size_t len) { + uint32_t id[V5_DIE_ID_WORDS]; + uint8_t bytes[sizeof id]; + + if (len < 2 * sizeof bytes + 1) return false; - uint32_t id[V5_DIE_ID_WORDS]; - uint32_t any_bit_set = 0; - uint32_t all_bits_set = 0xFFFFFFFF; - for (int i = 0; i < V5_DIE_ID_WORDS; i++) { + for (size_t i = 0; i < V5_DIE_ID_WORDS; i++) { if (!mem_reg(V5_OTP_SHADOW_BASE + V5_OTP_DIE_ID + i * 4, &id[i], OP_READ)) return false; - any_bit_set |= id[i]; - all_bits_set &= id[i]; + // Bytes are emitted in OTP order. The shadow words are little-endian, + // so byte i of what otp_get_die_id() would hand back is + // word[i / 4] >> (8 * (i % 4)). + for (size_t b = 0; b < 4; b++) + bytes[i * 4 + b] = (id[i] >> (8 * b)) & 0xFF; } - // An unfused or unreadable row reads all-zeroes or all-ones. Neither is an - // identity, and callers turn this string into a MAC address -- handing one - // out would give every board in a fleet the same address, so fail instead. - if (!any_bit_set || all_bits_set == 0xFFFFFFFF) - return false; - - char *ptr = buf; - for (int i = 0; i < V5_DIE_ID_WORDS; i++) - for (int b = 0; b < 4; b++) - ptr += snprintf(ptr, buf + len - ptr, "%02x", - (id[i] >> (8 * b)) & 0xFF); + for (size_t i = 0; i < sizeof bytes; i++) + snprintf(buf + 2 * i, len - 2 * i, "%02x", bytes[i]); return true; } -bool hisi_get_die_id(char *buf, ssize_t len) { +/* An OTP row that is unfused, locked or unbacked reads all-zeroes or all-ones, + * and a partially fused one reads a mixture of the two -- as does a V4 die-ID + * block on a part that never had one fused, which the trailing-zero strip above + * leaves as a string of zeroes rather than rejecting. + * + * None of those is an identity, and callers turn this string into a MAC + * address: handing one out would give every board in a fleet the same address. + * Rejecting on the digits rather than on whole words keeps the partially fused + * mixture out too, while still accepting a legitimate id that contains a zero + * word. + */ +static bool die_id_is_usable(const char *buf) { + for (const char *p = buf; *p; p++) + if (*p != '0' && *p != 'f') + return true; + return false; +} + +bool hisi_get_die_id(char *buf, size_t len) { + bool ok; + switch (chip_generation) { case HISI_V4: - return hisi_ev300_get_die_id(buf, len); + ok = hisi_ev300_get_die_id(buf, len); + break; case HISI_OT: - return hisi_ot_get_die_id(buf, len); + ok = hisi_ot_get_die_id(buf, len); + break; default: return false; } + + return ok && die_id_is_usable(buf); } #define CV300_ISP_AF_CFG_ADDR 0x12200 From 38ba095a0873fad8c6b26ecca2432167f354e3dc Mon Sep 17 00:00:00 2001 From: johnchia Date: Wed, 9 Sep 2026 00:31:10 -0700 Subject: [PATCH 3/3] ipcinfo: restore printk and the sensor clock on exit Every reporter in ipcinfo goes through getchipname(), which runs setup_hal_*(): printk is written to "0 0 0 0" and, on HISI_OT, the sensor clock is force-enabled via CRG8464/CRG8472. Only sensors.c and i2cspi.c ever call hal_cleanup(), and ipcinfo exits straight out of the reporters, so both side effects outlive the process -- the console stays silent and the CRG stays modified for the rest of boot. Pre-existing, but the die-ID reader puts `ipcinfo -i` on the rcS path of every V5 board through ethaddr_provision(), which is what makes it visible. Register an atexit() wrapper rather than calling hal_cleanup() before each of the exits: the pointer is only set once a HAL has been selected, so the wrapper reads it at exit time and guards against NULL. Both restore paths are idempotent -- restore_printk() no-ops without a saved state, and the CRG writes are gated on their changed flags -- so this stays correct alongside the existing explicit calls. Claude-Session: https://claude.ai/code/session_0135iarzELmev2nXzBx4SFLU --- example/ipcinfo.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/example/ipcinfo.c b/example/ipcinfo.c index 2e13e57..c47e3db 100644 --- a/example/ipcinfo.c +++ b/example/ipcinfo.c @@ -209,6 +209,18 @@ static void print_xm_mac() { exit(EXIT_FAILURE); } +/* Every reporter here goes through getchipname(), which runs setup_hal_*(): + * printk is silenced and, on HISI_OT, the sensor clock is force-enabled. + * ipcinfo exits straight out of the reporters, so without this the console + * stays quiet and the CRG stays modified for the rest of boot -- which + * ethaddr_provision() in rcS would do on every V5 boot. hal_cleanup is only + * set once a HAL has been selected, and both restore paths are idempotent. + */ +static void cleanup_hal(void) { + if (hal_cleanup) + hal_cleanup(); +} + int main(int argc, char **argv) { const char *short_options = "cfvhlstiFSxV"; const struct option long_options[] = { @@ -228,6 +240,7 @@ int main(int argc, char **argv) { int opt; int long_index = 0; + atexit(cleanup_hal); while ((opt = getopt_long_only(argc, argv, short_options, long_options, &long_index)) != -1) { switch (opt) {