From 18e0095743e801191b7c075833e62e1f5e66dd26 Mon Sep 17 00:00:00 2001 From: Emmanuel Blot Date: Thu, 31 Oct 2024 15:32:45 +0100 Subject: [PATCH 01/19] [ot] python/qemu/ot: do not repeat information for multiple proxy instances Signed-off-by: Emmanuel Blot --- python/qemu/ot/devproxy.py | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/python/qemu/ot/devproxy.py b/python/qemu/ot/devproxy.py index 5934657d7dfaf..532d6daebf20d 100644 --- a/python/qemu/ot/devproxy.py +++ b/python/qemu/ot/devproxy.py @@ -9,6 +9,7 @@ from binascii import hexlify, unhexlify from collections import deque from enum import IntEnum +from inspect import currentframe from logging import getLogger from socket import create_connection, socket, SHUT_RDWR from struct import calcsize as scalc, pack as spack, unpack as sunpack @@ -1255,6 +1256,9 @@ class LogOp(IntEnum): REMOVE = 2 SET = 3 + LogOnce: set[str] = set() + # whether initial log of remote device capabilities has been done + def __init__(self): self._log = getLogger('proxy.proxy') self._socket: Optional[socket] = None @@ -1345,7 +1349,10 @@ def discover_devices(self) -> None: if len(devices) % devlen: raise ValueError('Unexpected response length') devcount = len(devices) // devlen - self._log.info('Found %d remote devices', devcount) + fname = currentframe().f_code.co_name + if fname not in self.LogOnce: + self.LogOnce.add(fname) + self._log.info('Found %d remote devices', devcount) self._devices.clear() while devices: devhdr = devices[0:devlen] @@ -1380,7 +1387,10 @@ def discover_memory_spaces(self) -> None: if len(mregions) % mrlen: raise ValueError('Unexpected response length') mrcount = len(mregions) // mrlen - self._log.info('Found %d remote memory root regions', mrcount) + fname = currentframe().f_code.co_name + if fname not in self.LogOnce: + self.LogOnce.add(fname) + self._log.info('Found %d remote memory root regions', mrcount) self._mroots.clear() while mregions: mrhdr = mregions[0:mrlen] @@ -1778,8 +1788,11 @@ def _handshake(self): if len(payload) < hshdrlen: raise ValueError('Unexpected response length') vmin, vmaj, _ = sunpack(hshdrfmt, payload[:hshdrlen]) - self._log.info('Local version %d.%d, remote version %d.%d', - self.VERSION[0], self.VERSION[1], vmaj, vmin) + fname = currentframe().f_code.co_name + if fname not in self.LogOnce: + self.LogOnce.add(fname) + self._log.info('Local version %d.%d, remote version %d.%d', + self.VERSION[0], self.VERSION[1], vmaj, vmin) if vmaj != self.VERSION[0]: raise ValueError('Unsuppported version: {vmaj}.{vmin}') if vmin < self.VERSION[1]: From f4f65e7f229ff7cc2fb4e185f7f5c280b9ccc67c Mon Sep 17 00:00:00 2001 From: Emmanuel Blot Date: Thu, 7 Nov 2024 11:18:19 +0000 Subject: [PATCH 02/19] [ot] hw/opentitan: ot_dma: add a property to define the DMA transfer block size The property expects a value between 8 (256 bytes) and 20 (1MiB). The higher the value, the faster the DMA transfer, the higher the latency to handle other device requests using the I/O thread. Signed-off-by: Emmanuel Blot --- hw/opentitan/ot_dma.c | 25 ++++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/hw/opentitan/ot_dma.c b/hw/opentitan/ot_dma.c index ab140f8872a1d..1758ed93f683a 100644 --- a/hw/opentitan/ot_dma.c +++ b/hw/opentitan/ot_dma.c @@ -256,6 +256,7 @@ struct OtDMAState { OtDMAOp op; OtDMASHA sha; uint32_t *regs; + unsigned tb_size; /* DMA transfer block size */ OtDMAControl control; /* captured regs on initial IDLE-GO transition */ bool abort; @@ -263,6 +264,7 @@ struct OtDMAState { char *ot_as_name; /* private AS unique name */ char *ctn_as_name; /* externel port AS unique name */ char *sys_as_name; /* external system AS unique name */ + uint8_t block_size_lg2; /* log2 of DMA transfer block size */ #ifdef OT_DMA_HAS_ROLE uint8_t role; #endif @@ -288,8 +290,8 @@ struct OtDMAState { #define DMA_ERROR(_err_) (1u << (_err_)) /* the following values are arbitrary end may be changed if needed */ -#define DMA_PACE_NS 10000u /* 10us: slow down DMA, handle aborts */ -#define DMA_TRANSFER_BLOCK_SIZE 4096u /* size of a single DMA block */ +#define DMA_PACE_NS 10000u /* 10us: slow down DMA, handle aborts */ +#define DMA_TRANSFER_BLOCK_LG2 12u /* log2(size) of a single DMA block */ #define REG_NAME_ENTRY(_reg_) [R_##_reg_] = stringify(_reg_) static const char *REG_NAMES[REGS_COUNT] = { @@ -1037,7 +1039,7 @@ static void ot_dma_transfer(void *opaque) smp_mb(); - hwaddr size = MIN(op->size, DMA_TRANSFER_BLOCK_SIZE); + hwaddr size = MIN(op->size, s->tb_size); CHANGE_STATE(s, SEND_WRITE); @@ -1313,6 +1315,8 @@ static Property ot_dma_properties[] = { DEFINE_PROP_STRING("ot_as_name", OtDMAState, ot_as_name), DEFINE_PROP_STRING("ctn_as_name", OtDMAState, ctn_as_name), DEFINE_PROP_STRING("sys_as_name", OtDMAState, sys_as_name), + DEFINE_PROP_UINT8("block_size_lg2", OtDMAState, block_size_lg2, + DMA_TRANSFER_BLOCK_LG2), #ifdef OT_DMA_HAS_ROLE DEFINE_PROP_UINT8("role", OtDMAState, role, UINT8_MAX), #endif @@ -1376,6 +1380,20 @@ static void ot_dma_reset(DeviceState *dev) } } +static void ot_dma_realize(DeviceState *dev, Error **errp) +{ + OtDMAState *s = OT_DMA(dev); + (void)errp; + + if (s->block_size_lg2 < 8u || s->block_size_lg2 > 20u) { + error_setg(&error_fatal, "%s: invalid DMA transfer block size: %u", + __func__, 1u << s->block_size_lg2); + return; + } + + s->tb_size = 1u << s->block_size_lg2; +} + static void ot_dma_init(Object *obj) { OtDMAState *s = OT_DMA(obj); @@ -1401,6 +1419,7 @@ static void ot_dma_class_init(ObjectClass *klass, void *data) DeviceClass *dc = DEVICE_CLASS(klass); (void)data; + dc->realize = &ot_dma_realize; dc->reset = &ot_dma_reset; device_class_set_props(dc, ot_dma_properties); set_bit(DEVICE_CATEGORY_MISC, dc->categories); From 2747216c3cb2c38f711365092ba9ec918a7d0d14 Mon Sep 17 00:00:00 2001 From: Emmanuel Blot Date: Thu, 7 Nov 2024 13:48:31 +0000 Subject: [PATCH 03/19] [ot] hw/opentitan: ot_dma: add a property to define the DMA pace delay. The smaller the delay, the faster the DMA transfer. Signed-off-by: Emmanuel Blot --- hw/opentitan/ot_dma.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/hw/opentitan/ot_dma.c b/hw/opentitan/ot_dma.c index 1758ed93f683a..53b4036302d63 100644 --- a/hw/opentitan/ot_dma.c +++ b/hw/opentitan/ot_dma.c @@ -264,6 +264,7 @@ struct OtDMAState { char *ot_as_name; /* private AS unique name */ char *ctn_as_name; /* externel port AS unique name */ char *sys_as_name; /* external system AS unique name */ + uint32_t pace_delay; /* pace DMA scheduling (ns) */ uint8_t block_size_lg2; /* log2 of DMA transfer block size */ #ifdef OT_DMA_HAS_ROLE uint8_t role; @@ -289,7 +290,7 @@ struct OtDMAState { #define DMA_ERROR(_err_) (1u << (_err_)) -/* the following values are arbitrary end may be changed if needed */ +/* default values, can be overridden with properties */ #define DMA_PACE_NS 10000u /* 10us: slow down DMA, handle aborts */ #define DMA_TRANSFER_BLOCK_LG2 12u /* log2(size) of a single DMA block */ @@ -924,7 +925,7 @@ static bool ot_dma_go(OtDMAState *s) timer_del(s->timer); uint64_t now = qemu_clock_get_ns(OT_VIRTUAL_CLOCK); - timer_mod(s->timer, (int64_t)(now + DMA_PACE_NS)); + timer_mod_anticipate(s->timer, (int64_t)(now + s->pace_delay)); return true; } @@ -943,7 +944,7 @@ static void ot_dma_abort(OtDMAState *s) /* simulate a delayed response */ timer_del(s->timer); uint64_t now = qemu_clock_get_ns(OT_VIRTUAL_CLOCK); - timer_mod(s->timer, (int64_t)(now + DMA_PACE_NS)); + timer_mod(s->timer, (int64_t)(now + s->pace_delay)); } static void ot_dma_complete(OtDMAState *s) @@ -1064,7 +1065,7 @@ static void ot_dma_transfer(void *opaque) /* schedule next block if any */ uint64_t now = qemu_clock_get_ns(OT_VIRTUAL_CLOCK); - timer_mod(s->timer, (int64_t)(now + DMA_PACE_NS)); + timer_mod(s->timer, (int64_t)(now + s->pace_delay)); return; } } @@ -1315,6 +1316,7 @@ static Property ot_dma_properties[] = { DEFINE_PROP_STRING("ot_as_name", OtDMAState, ot_as_name), DEFINE_PROP_STRING("ctn_as_name", OtDMAState, ctn_as_name), DEFINE_PROP_STRING("sys_as_name", OtDMAState, sys_as_name), + DEFINE_PROP_UINT32("pace_delay", OtDMAState, pace_delay, DMA_PACE_NS), DEFINE_PROP_UINT8("block_size_lg2", OtDMAState, block_size_lg2, DMA_TRANSFER_BLOCK_LG2), #ifdef OT_DMA_HAS_ROLE From bef1425ce52d33e3c6a3c3d840965e13785cd3d2 Mon Sep 17 00:00:00 2001 From: Emmanuel Blot Date: Thu, 7 Nov 2024 11:21:13 +0000 Subject: [PATCH 04/19] [ot] hw/opentitan: ot_spi_host: do not refill RX FIFO on each RXDATA read out Refill the RX FIFO only when it is about to be empty or when its level reaches the specified RX watermark level. This reduces the overhead required for filling up the RX FIFO. Signed-off-by: Emmanuel Blot --- hw/opentitan/ot_spi_host.c | 26 ++++++++++++++++++++------ hw/opentitan/trace-events | 1 + 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/hw/opentitan/ot_spi_host.c b/hw/opentitan/ot_spi_host.c index 0d20bd33e3da0..b9e589de74df2 100644 --- a/hw/opentitan/ot_spi_host.c +++ b/hw/opentitan/ot_spi_host.c @@ -816,11 +816,11 @@ static void ot_spi_host_step_fsm(OtSPIHostState *s, const char *cause) if (length) { /* if the transfer early ended, a stall condition has been detected */ if (write && txfifo_is_empty(s->tx_fifo)) { - trace_ot_spi_host_debug(s->ot_id, "Tx stall"); + trace_ot_spi_host_stall(s->ot_id, "TX", length); s->fsm.tx_stall = true; } if (read && fifo8_is_full(s->rx_fifo)) { - trace_ot_spi_host_debug(s->ot_id, "Rx stall"); + trace_ot_spi_host_stall(s->ot_id, "RX", length); s->fsm.rx_stall = true; } @@ -834,8 +834,8 @@ static void ot_spi_host_step_fsm(OtSPIHostState *s, const char *cause) headcmd->command = (uint16_t)command; headcmd->ongoing = ongoing; - timer_mod(s->fsm_delay, - qemu_clock_get_ns(OT_VIRTUAL_CLOCK) + FSM_TRIGGER_DELAY_NS); + timer_mod_anticipate(s->fsm_delay, qemu_clock_get_ns(OT_VIRTUAL_CLOCK) + + FSM_TRIGGER_DELAY_NS); ot_spi_host_trace_status(s->ot_id, "S<", ot_spi_host_get_status(s)); } @@ -973,8 +973,19 @@ static uint64_t ot_spi_host_io_read(void *opaque, hwaddr addr, val32 |= (uint32_t)fifo8_pop(s->rx_fifo); } val32 = bswap32(val32); - bool resume = !cmdfifo_is_empty(s->cmd_fifo) && s->fsm.rx_stall && - !s->fsm.tx_stall; + bool resume = false; + if (!cmdfifo_is_empty(s->cmd_fifo)) { + if (!s->fsm.tx_stall) { + uint32_t rxwm = REG_GET(s, CONTROL, RX_WATERMARK); + if (rxwm < sizeof(uint32_t)) { + rxwm = sizeof(uint32_t); + } + uint32_t inlen = fifo8_num_used(s->rx_fifo) / sizeof(uint32_t); + if (inlen <= rxwm) { + resume = true; + } + } + } s->fsm.rx_stall = false; if (resume) { ot_spi_host_step_fsm(s, "rx"); @@ -1073,6 +1084,9 @@ static void ot_spi_host_io_write(void *opaque, hwaddr addr, uint64_t val64, ot_spi_host_reset(s); } s->fsm.output_en = FIELD_EX32(val32, CONTROL, OUTPUT_EN); + if (!cmdfifo_is_empty(s->cmd_fifo)) { + ot_spi_host_step_fsm(s, "ctrl"); + } break; case R_CONFIGOPTS: /* Update the respective config-opts register based on CSIDth index */ diff --git a/hw/opentitan/trace-events b/hw/opentitan/trace-events index bae9ffbe98201..9b7ec78cd9f6a 100644 --- a/hw/opentitan/trace-events +++ b/hw/opentitan/trace-events @@ -473,6 +473,7 @@ ot_spi_host_io_read_repeat(const char *id, size_t count) "%s: last read repeated ot_spi_host_io_write(const char *id, uint32_t addr, const char * regname, uint32_t val, uint32_t pc) "%s: addr:0x%02x (%s), val:0x%x, pc:0x%x" ot_spi_host_reject(const char *id, const char *msg) "%s: %s" ot_spi_host_reset(const char *id, const char *msg) "%s: %s" +ot_spi_host_stall(const char *id, const char *msg, uint32_t val) "%s: %s %u" ot_spi_host_status(const char *id, const char *msg, uint32_t status, const char *str, unsigned cmd, unsigned rxd, unsigned txd) "%s: %s 0x%08x s:%s cq:%u rq:%u tq:%u" ot_spi_host_transfer(const char *id, uint32_t tx_data, uint32_t rx_data) "%s: tx_data: 0x%02x rx_data: 0x%02x" ot_spi_host_update_irq(const char *id, const char *channel, int level) "%s: %s: %d" From 550332c99272302fa535fe1744265bcb61ca43d5 Mon Sep 17 00:00:00 2001 From: Emmanuel Blot Date: Wed, 27 Nov 2024 10:58:37 +0100 Subject: [PATCH 05/19] [ot] hw/opentitan: ot_spi_host: rework SPI event management Should be closer to real HW. Signed-off-by: Emmanuel Blot --- hw/opentitan/ot_spi_host.c | 70 +++++++++++++++++++++----------------- hw/opentitan/trace-events | 13 +++---- 2 files changed, 45 insertions(+), 38 deletions(-) diff --git a/hw/opentitan/ot_spi_host.c b/hw/opentitan/ot_spi_host.c index b9e589de74df2..df9a995c5795d 100644 --- a/hw/opentitan/ot_spi_host.c +++ b/hw/opentitan/ot_spi_host.c @@ -58,7 +58,7 @@ /* ------------------------------------------------------------------------ */ /* undef to get all the repeated, identical status query traces */ -#define DISCARD_REPEATED_STATUS_TRACES +#undef DISCARD_REPEATED_STATUS_TRACES /* fake delayed completion of HW commands */ #define FSM_TRIGGER_DELAY_NS 100U /* nanoseconds */ @@ -249,17 +249,21 @@ static void ot_spi_host_trace_status(const char *ot_id, const char *msg, unsigned rxd = FIELD_EX32(status, STATUS, RXQD); unsigned txd = FIELD_EX32(status, STATUS, TXQD); char str[64u]; - (void)snprintf(str, sizeof(str), "%s%s%s%s%s%s%s%s%s%s", - FIELD_EX32(status, STATUS, RXWM) ? "RXM|" : "", - FIELD_EX32(status, STATUS, RXSTALL) ? "RXS|" : "", - FIELD_EX32(status, STATUS, RXEMPTY) ? "RXE|" : "", - FIELD_EX32(status, STATUS, RXFULL) ? "RXF|" : "", - FIELD_EX32(status, STATUS, TXWM) ? "TXM|" : "", - FIELD_EX32(status, STATUS, TXSTALL) ? "TXS|" : "", - FIELD_EX32(status, STATUS, TXEMPTY) ? "TXE|" : "", - FIELD_EX32(status, STATUS, TXFULL) ? "TXF|" : "", - FIELD_EX32(status, STATUS, ACTIVE) ? "ACT|" : "", - FIELD_EX32(status, STATUS, READY) ? "RDY|" : ""); + int last = + snprintf(str, sizeof(str), "%s%s%s%s%s%s%s%s%s%s", + FIELD_EX32(status, STATUS, RXWM) ? "RXM|" : "", + FIELD_EX32(status, STATUS, RXSTALL) ? "RXS|" : "", + FIELD_EX32(status, STATUS, RXEMPTY) ? "RXE|" : "", + FIELD_EX32(status, STATUS, RXFULL) ? "RXF|" : "", + FIELD_EX32(status, STATUS, TXWM) ? "TXM|" : "", + FIELD_EX32(status, STATUS, TXSTALL) ? "TXS|" : "", + FIELD_EX32(status, STATUS, TXEMPTY) ? "TXE|" : "", + FIELD_EX32(status, STATUS, TXFULL) ? "TXF|" : "", + FIELD_EX32(status, STATUS, ACTIVE) ? "ACT|" : "", + FIELD_EX32(status, STATUS, READY) ? "RDY|" : ""); + if (str[last - 1] == '|') { + str[last - 1] = '\0'; + } trace_ot_spi_host_status(ot_id, msg, status, str, cmd, rxd, txd); } @@ -325,6 +329,7 @@ struct OtSPIHostState { IbexIRQ alert; /**< OpenTitan alert */ uint32_t events; /**< Active events */ uint32_t last_events; /**< Last detected events */ + uint64_t total_transfer; /**< Transfered bytes since reset */ OtSPIHostFsm fsm; bool on_reset; @@ -617,26 +622,19 @@ static bool ot_spi_host_update_event(OtSPIHostState *s) /* new events' state */ uint32_t events = ot_spi_host_build_event_bits(s); - /* events that have changed since last call (detect rising/falling edges) */ - uint32_t changes = s->last_events ^ events; - /* RXWM/TXWM are not edge events, but level ones */ - changes |= R_EVENT_ENABLE_RXWM_MASK | R_EVENT_ENABLE_TXWM_MASK; + /* events that have been raised since last call */ + uint32_t raised_events = events & (s->last_events ^ events); s->last_events = events; - /* pick up changes */ - events &= changes; - /* accumulate events */ - s->events |= events; + s->events |= raised_events; /* mask disabled events to get the spi event state */ - bool event = (bool)(s->events & s->regs[R_EVENT_ENABLE]); - trace_ot_spi_host_debug1(s->ot_id, "event", event); + uint32_t eff_events = s->events & s->regs[R_EVENT_ENABLE]; + trace_ot_spi_host_events(s->ot_id, events, raised_events, s->events, + s->regs[R_EVENT_ENABLE], eff_events); - /* - * if the spi event test has been enabled, force event and clear its bit - * right away - */ + bool event = (bool)eff_events; event |= (bool)(s->regs[R_INTR_TEST] & INTR_SPI_EVENT_MASK); s->regs[R_INTR_TEST] &= ~INTR_SPI_EVENT_MASK; if (event) { @@ -645,7 +643,6 @@ static bool ot_spi_host_update_event(OtSPIHostState *s) s->regs[R_INTR_STATE] &= ~INTR_SPI_EVENT_MASK; } - /* now update the IRQ signal (event could have been already signalled) */ bool event_level = (bool)(s->regs[R_INTR_STATE] & s->regs[R_INTR_ENABLE] & INTR_SPI_EVENT_MASK); if (event_level != (bool)ibex_irq_get_level(&s->irqs[IRQ_SPI_EVENT])) { @@ -737,6 +734,8 @@ static void ot_spi_host_reset(OtSPIHostState *s) ot_spi_host_update_regs(s); ot_spi_host_update_alert(s); + + s->total_transfer = 0; } /** @@ -807,7 +806,8 @@ static void ot_spi_host_step_fsm(OtSPIHostState *s, const char *cause) fifo8_push(s->rx_fifo, rx); } - trace_ot_spi_host_transfer(s->ot_id, tx, rx); + trace_ot_spi_host_transfer(s->ot_id, s->total_transfer, tx, rx); + s->total_transfer += 1u; length--; } @@ -834,6 +834,8 @@ static void ot_spi_host_step_fsm(OtSPIHostState *s, const char *cause) headcmd->command = (uint16_t)command; headcmd->ongoing = ongoing; + ot_spi_host_update_regs(s); + timer_mod_anticipate(s->fsm_delay, qemu_clock_get_ns(OT_VIRTUAL_CLOCK) + FSM_TRIGGER_DELAY_NS); @@ -901,7 +903,7 @@ static void ot_spi_host_post_fsm(void *opaque) /* more commands have been scheduled */ trace_ot_spi_host_debug(s->ot_id, "Next cmd"); if (!ot_spi_host_is_on_error(s)) { - qemu_bh_schedule(s->fsm_bh); + ot_spi_host_step_fsm(s, "post"); } else { trace_ot_spi_host_debug(s->ot_id, "no resched: on err"); } @@ -962,6 +964,8 @@ static uint64_t ot_spi_host_io_read(void *opaque, hwaddr addr, case R_RXDATA: { /* here, size != 4 is illegal, what to do in this case? */ if (fifo8_num_used(s->rx_fifo) < sizeof(uint32_t)) { + qemu_log_mask(LOG_GUEST_ERROR, "%s: %s: Read underflow: %u\n", + __func__, s->ot_id, fifo8_num_used(s->rx_fifo)); REG_UPDATE(s, ERROR_STATUS, UNDERFLOW, 1); ot_spi_host_update_regs(s); val32 = 0u; @@ -972,6 +976,7 @@ static uint64_t ot_spi_host_io_read(void *opaque, hwaddr addr, val32 <<= 8u; val32 |= (uint32_t)fifo8_pop(s->rx_fifo); } + ot_spi_host_trace_status(s->ot_id, "rxd", ot_spi_host_get_status(s)); val32 = bswap32(val32); bool resume = false; if (!cmdfifo_is_empty(s->cmd_fifo)) { @@ -1009,7 +1014,9 @@ static uint64_t ot_spi_host_io_read(void *opaque, hwaddr addr, if (trace_cache.pc != pc || trace_cache.addr != addr || trace_cache.value != val32) { if (trace_cache.count > 1u) { - trace_ot_spi_host_io_read_repeat(s->ot_id, trace_cache.count); + hwaddr rreg = R32_OFF(trace_cache.addr); + trace_ot_spi_host_io_read_repeat(s->ot_id, REG_NAME(rreg), + trace_cache.count); } #endif /* DISCARD_REPEATED_STATUS_TRACES */ trace_ot_spi_host_io_read(s->ot_id, (uint32_t)addr, REG_NAME(reg), @@ -1055,11 +1062,10 @@ static void ot_spi_host_io_write(void *opaque, hwaddr addr, uint64_t val64, val32 &= INTR_MASK; s->regs[R_INTR_STATE] &= ~val32; if (val32 & INTR_SPI_EVENT_MASK) { - /* store current state */ - s->last_events = ot_spi_host_build_event_bits(s); /* clear up all signalled events */ s->events = 0u; } + /* this call also regenerates all raised events */ ot_spi_host_update_regs(s); break; case R_INTR_ENABLE: diff --git a/hw/opentitan/trace-events b/hw/opentitan/trace-events index 9b7ec78cd9f6a..bb4604615f1c1 100644 --- a/hw/opentitan/trace-events +++ b/hw/opentitan/trace-events @@ -467,16 +467,17 @@ ot_spi_host_command(const char *id, const char *dir, const char *spd, uint32_t c ot_spi_host_cs(const char *id, uint32_t csid, const char *level) "%s: cs#:%u %sselected" ot_spi_host_debug(const char *id, const char *msg) "%s: %s" ot_spi_host_debug1(const char *id, const char *msg, uint32_t val) "%s: %s 0x%x" +ot_spi_host_events(const char *id, uint32_t cur, uint32_t raised, uint32_t active, uint32_t mask, uint32_t eff) "%s: cur:0x%02x rsd:0x%02x act:0x%02x msk:0x%02x eff:0x%02x" ot_spi_host_fsm(const char *id, const char *cause) "%s: step %s" -ot_spi_host_io_read(const char *id, uint32_t addr, const char * regname, uint32_t val, uint32_t pc) "%s: addr:0x%02x (%s), val:0x%x, pc:0x%x" -ot_spi_host_io_read_repeat(const char *id, size_t count) "%s: last read repeated %zu times" -ot_spi_host_io_write(const char *id, uint32_t addr, const char * regname, uint32_t val, uint32_t pc) "%s: addr:0x%02x (%s), val:0x%x, pc:0x%x" +ot_spi_host_io_read(const char *id, uint32_t addr, const char * regname, uint32_t val, uint32_t pc) "%s: addr:0x%02x (%s), val:0x%08x, pc:0x%x" +ot_spi_host_io_read_repeat(const char *id, const char * regname, size_t count) "%s: last %s read repeated %zu times" +ot_spi_host_io_write(const char *id, uint32_t addr, const char * regname, uint32_t val, uint32_t pc) "%s: addr:0x%02x (%s), val:0x%08x, pc:0x%x" ot_spi_host_reject(const char *id, const char *msg) "%s: %s" ot_spi_host_reset(const char *id, const char *msg) "%s: %s" -ot_spi_host_stall(const char *id, const char *msg, uint32_t val) "%s: %s %u" +ot_spi_host_stall(const char *id, const char *msg, uint32_t val) "%s: %s rem %u" ot_spi_host_status(const char *id, const char *msg, uint32_t status, const char *str, unsigned cmd, unsigned rxd, unsigned txd) "%s: %s 0x%08x s:%s cq:%u rq:%u tq:%u" -ot_spi_host_transfer(const char *id, uint32_t tx_data, uint32_t rx_data) "%s: tx_data: 0x%02x rx_data: 0x%02x" -ot_spi_host_update_irq(const char *id, const char *channel, int level) "%s: %s: %d" +ot_spi_host_transfer(const char *id, uint64_t transfer, uint32_t tx_data, uint32_t rx_data) "%s: {%" PRIu64 "} tx_data: 0x%02x rx_data: 0x%02x" +ot_spi_host_update_irq(const char *id, const char *channel, int level) "%s: irq %s: %d" # ot_sram_ctrl.c From a4ebcfa7fd7493b343541b7d51621dbfa6e3a21d Mon Sep 17 00:00:00 2001 From: Emmanuel Blot Date: Mon, 2 Dec 2024 18:25:21 +0100 Subject: [PATCH 06/19] [ot] hw/opentitan: ot_spi_host: add a SPI command state. Replace previous boolean-based state with an enumeration, as non-executing command can be either about to execute or in finalization stage. This enables differentiation of zero length command which can either be a 1-byte command to be executed, or a command whose all bytes have been processed. Signed-off-by: Emmanuel Blot --- hw/opentitan/ot_spi_host.c | 34 +++++++++++++++++++++++----------- 1 file changed, 23 insertions(+), 11 deletions(-) diff --git a/hw/opentitan/ot_spi_host.c b/hw/opentitan/ot_spi_host.c index df9a995c5795d..d1f6cdae7d80d 100644 --- a/hw/opentitan/ot_spi_host.c +++ b/hw/opentitan/ot_spi_host.c @@ -369,15 +369,22 @@ struct TxFifo { uint32_t num; }; +enum CmdState { + CMD_SCHEDULED, /* command scheduled for execution, not yet handled */ + CMD_ONGOING, /* command is being executed, not yet completed */ + CMD_EXECUTED, /* commmand has been executed, need to be popped out */ +}; + /** * Command FIFO stores commands alongs with SPI device configuration. - * To fit into 64-bit word, limit supported CS lines down to 64K rather than 4G. + * To fit into 64-bit word, limit supported CS lines down to 256 rather than 4G, + * and use "int8_t" for command state. */ typedef struct { uint32_t opts; /* configopts */ uint16_t command; /* command[15:0] */ uint8_t csid; /* csid[7:0] */ - bool ongoing; /* command is being processed */ + int8_t state; /* enum CmdState */ } CmdFifoSlot; static_assert(sizeof(TxFifoSlot) == sizeof(uint64_t), @@ -751,6 +758,10 @@ static void ot_spi_host_step_fsm(OtSPIHostState *s, const char *cause) s->fsm.active = true; ot_spi_host_update_event(s); + if (headcmd->state == CMD_EXECUTED) { + goto post; + } + uint32_t command = (uint32_t)headcmd->command; bool read = ot_spi_host_is_rx(command); bool write = ot_spi_host_is_tx(command); @@ -812,7 +823,7 @@ static void ot_spi_host_step_fsm(OtSPIHostState *s, const char *cause) length--; } - bool ongoing; + int8_t cmd_state; if (length) { /* if the transfer early ended, a stall condition has been detected */ if (write && txfifo_is_empty(s->tx_fifo)) { @@ -825,15 +836,16 @@ static void ot_spi_host_step_fsm(OtSPIHostState *s, const char *cause) } command = FIELD_DP32(command, COMMAND, LEN, length - 1); - ongoing = true; + cmd_state = CMD_ONGOING; } else { command = FIELD_DP32(command, COMMAND, LEN, 0); - ongoing = false; + cmd_state = CMD_EXECUTED; } headcmd->command = (uint16_t)command; - headcmd->ongoing = ongoing; + headcmd->state = cmd_state; +post: ot_spi_host_update_regs(s); timer_mod_anticipate(s->fsm_delay, qemu_clock_get_ns(OT_VIRTUAL_CLOCK) + @@ -863,11 +875,11 @@ static void ot_spi_host_post_fsm(void *opaque) CmdFifoSlot *headcmd = cmdfifo_peek(s->cmd_fifo); uint32_t command = (uint32_t)headcmd->command; - bool ongoing = headcmd->ongoing; + bool retire = headcmd->state == CMD_EXECUTED; ot_spi_host_trace_status(s->ot_id, "P>", ot_spi_host_get_status(s)); - if (!ongoing) { + if (retire) { if (ot_spi_host_is_rx(command)) { /* * transfer has been completed, RX FIFO may need padding up to a @@ -897,11 +909,11 @@ static void ot_spi_host_post_fsm(void *opaque) ot_spi_host_trace_status(s->ot_id, "P<", ot_spi_host_get_status(s)); - if (!ongoing) { + if (retire) { /* last command has completed */ if (!cmdfifo_is_empty(s->cmd_fifo)) { /* more commands have been scheduled */ - trace_ot_spi_host_debug(s->ot_id, "Next cmd"); + trace_ot_spi_host_debug(s->ot_id, "next cmd"); if (!ot_spi_host_is_on_error(s)) { ot_spi_host_step_fsm(s, "post"); } else { @@ -1152,7 +1164,7 @@ static void ot_spi_host_io_write(void *opaque, hwaddr addr, uint64_t val64, .opts = s->config_opts[csid], .command = (uint16_t)val32, /* only b15..b0 are meaningful */ .csid = csid, - .ongoing = false, + .state = CMD_SCHEDULED, }; bool activate = cmdfifo_is_empty(s->cmd_fifo) && !s->fsm.rx_stall && From 46687fbe711e0363ad201a324d7ebccecdb0e3b2 Mon Sep 17 00:00:00 2001 From: Emmanuel Blot Date: Wed, 13 Nov 2024 19:04:19 +0100 Subject: [PATCH 07/19] [ot] scripts/opentitan: spiflashpat.py: add a tiny pattern generator for SPI flash images Signed-off-by: Emmanuel Blot --- python/qemu/ot/util/misc.py | 36 ++++++- scripts/opentitan/spiflashpat.py | 167 +++++++++++++++++++++++++++++++ 2 files changed, 201 insertions(+), 2 deletions(-) create mode 100755 scripts/opentitan/spiflashpat.py diff --git a/python/qemu/ot/util/misc.py b/python/qemu/ot/util/misc.py index 43a0464eee667..c832301602231 100644 --- a/python/qemu/ot/util/misc.py +++ b/python/qemu/ot/util/misc.py @@ -8,7 +8,7 @@ from io import BytesIO from sys import stdout -from typing import Any, Iterable, Optional, TextIO +from typing import Any, Iterable, Optional, TextIO, Union import re try: @@ -32,7 +32,8 @@ def __repr__(self) -> str: return f'0x{self:x}' @staticmethod - def parse(val: Optional[str], base: Optional[int] = None) -> Optional[int]: + def parse(val: Optional[str], base: Optional[int] = None) \ + -> Optional['HexInt']: """Simple helper to support hexadecimal integer in argument parser.""" if val is None: return None @@ -40,6 +41,37 @@ def parse(val: Optional[str], base: Optional[int] = None) -> Optional[int]: return HexInt(int(val, base)) return HexInt(int(val, val.startswith('0x') and 16 or 10)) + @staticmethod + def xparse(value: Union[None, int, str]) -> Optional['HexInt']: + """Parse a value and convert it into an integer value if possible. + + Input value may be: + - None + - a string with an integer coded as a decimal value + - a string with an integer coded as a hexadecimal value + - a integral value + - a integral value with a unit specifier (kilo or mega) + + :param value: input value to convert to an integer + :return: the value as an integer + :raise ValueError: if the input value cannot be converted into an int + """ + if value is None: + return None + if isinstance(value, int): + return HexInt(value) + imo = re.match(r'^\s*(\d+)\s*(?:([KMkm]i?)?B?)?\s*$', value) + if imo: + mult = {'K': (1000), + 'KI': (1 << 10), + 'M': (1000 * 1000), + 'MI': (1 << 20)} + value = int(imo.group(1)) + if imo.group(2): + value *= mult[imo.group(2).upper()] + return value + return HexInt(int(value.strip(), value.startswith('0x') and 16 or 10)) + class EasyDict(dict): """Dictionary whose members can be accessed as instance members diff --git a/scripts/opentitan/spiflashpat.py b/scripts/opentitan/spiflashpat.py new file mode 100755 index 0000000000000..c2151924e4791 --- /dev/null +++ b/scripts/opentitan/spiflashpat.py @@ -0,0 +1,167 @@ +#!/usr/bin/env python3 + +"""SPI flash pattern generator tiny tool. + + :author: Emmanuel Blot +""" + +# Copyright (c) 2024 Rivos, Inc. +# SPDX-License-Identifier: Apache2 + +from argparse import ArgumentParser +from logging import getLogger +from os import SEEK_SET, linesep, stat, truncate, unlink +from os.path import dirname, exists, join as joinpath, normpath +from traceback import format_exc +import sys + +QEMU_PYPATH = joinpath(dirname(dirname(dirname(normpath(__file__)))), + 'python', 'qemu') +sys.path.append(QEMU_PYPATH) + +from ot.util.log import configure_loggers +from ot.util.misc import HexInt + + +class SpiFlashPatternGenerator: + """Tiny pattern generator for SPI flash device image.""" + + def __init__(self, size: int, path: str, big_endian: bool): + self._log = getLogger('spiflash.gen') + self._size = size + self._path = path + self._be = big_endian + + def resize(self, reset: bool) -> None: + """Resize the flash image to the expected flash size. + Either truncate or zero-extend the image to match the target size. + + :param reset: whether to clear out any existing flash image content. + """ + if reset and exists(self._path): + unlink(self._path) + try: + stat_res = stat(self._path) + except FileNotFoundError: + self._log.info('Creating file of %d bytes', self._size) + with open(self._path, 'wb') as ffp: + ffp.write(bytes(self._size)) + return + size = stat_res.st_size + if size > self._size: + self._log.info('Truncating file from %d bytes down to %d bytes', + size, self._size) + truncate(self._path, self._size) + elif size < self._size: + self._log.info('Extending file from %d bytes up to %d bytes', + size, self._size) + with open(self._path, 'ab') as ffp: + ffp.write(bytes(self._size - size)) + + def generate(self, address: int, byte_count: int, inc: int, length: int, + width: int) -> None: + """Generate binary patterns as flash content. + + :param address: start address of the first pattern + :param byte_count: how many bytes to generate + :param inc: increment value between each pattern + :param length: pattern length in byte + :param width: pattern width in bits, i.e. pattern repetition inside + a single length-long pattern + """ + count = byte_count // length + nlen = length * 2 + dbg_str = f'Old val: 0x%0{nlen}x, new val: 0x%0{nlen}x, length: %d' + with open(self._path, 'r+b') as ffp: + ffp.seek(address, SEEK_SET) + rdata = ffp.read(length) + value = int.from_bytes(rdata, 'big' if self._be else 'little') + while count: + count -= 1 + nvalue = self._gen_next_value(value, inc, length, width) + wdata = nvalue.to_bytes(length, 'big' if self._be else 'little') + self._log.debug(dbg_str, value, nvalue, len(wdata)) + ffp.write(wdata) + value = nvalue + + def _gen_next_value(self, value: int, inc: int, length: int, width: int) \ + -> int: + out = 0 + for pos in range(0, length * 8, width): + mask = (1 << width) - 1 + val = (value >> pos) & mask + val = (val + inc) & mask + out |= val << pos + return out + + +def main(): + """Main routine""" + debug = True + widths = { + 'nibble': 4, + 'byte': 8, + 'half': 16, + 'word': 32, + 'double': 64, + } + + try: + desc = sys.modules[__name__].__doc__.split('.', 1)[0].strip() + argparser = ArgumentParser(description=f'{desc}.') + argparser.add_argument('-f', '--file', required=True, + help='Flash image file') + argparser.add_argument('-a', '--address', type=HexInt.xparse, + default='0', + help='Start address') + argparser.add_argument('-b', '--big-endian', action='store_true', + help='Use big-endian encoding (default: little)') + argparser.add_argument('-c', '--count', default='0', type=HexInt.xparse, + help='How many bytes to generate') + argparser.add_argument('-i', '--inc', default=1, type=int, + help='Increment, may be negative (default: 1)') + argparser.add_argument('-l', '--length', choices=list(widths)[1:], + default='byte', + help='Length of each slot (default: byte)') + argparser.add_argument('-r', '--reset', action='store_true', + help='Reset the flash image content (to 0)') + argparser.add_argument('-s', '--size', type=HexInt.xparse, + default='64MiB', + help='Flash image size (default: 64MiB)') + argparser.add_argument('-w', '--width', choices=widths, default='byte', + help='Pattern width for each slot ' + '(default: byte)') + argparser.add_argument('-v', '--verbose', action='count', + help='increase verbosity') + argparser.add_argument('-d', '--debug', action='store_true', + help='enable debug mode') + args = argparser.parse_args() + debug = args.debug + + configure_loggers(args.verbose, 'spiflash') + + length = widths[args.length] // 8 + if args.address + length > args.size: + argparser.error('Start address cannot extend after end of file') + bitwidth = widths[args.width] + if bitwidth > length * 8: + argparser.error('Pattern width larger than slot size') + + flasher = SpiFlashPatternGenerator(args.size, args.file, args.big_endian) + flasher.resize(args.reset) + flasher.generate(args.address, args.count, args.inc, length, + bitwidth) + + sys.exit(0) + # pylint: disable=broad-except + except Exception as exc: + print(f'{linesep}Error: {exc}', file=sys.stderr) + if debug: + print(format_exc(chain=False), file=sys.stderr) + sys.exit(1) + except KeyboardInterrupt: + sys.exit(2) + + +if __name__ == '__main__': + main() From f8156e91da07d1adb481051acd8a979fcb8327f2 Mon Sep 17 00:00:00 2001 From: Emmanuel Blot Date: Wed, 20 Nov 2024 17:21:36 +0100 Subject: [PATCH 08/19] [ot] python/qemu: ot.util.log: create a service for remote UDP logging. This service enables multiple Python applications to use a unified logging system. Signed-off-by: Emmanuel Blot --- python/qemu/ot/util/log.py | 108 +++++++++++++++++++++++++++++++++++-- 1 file changed, 104 insertions(+), 4 deletions(-) diff --git a/python/qemu/ot/util/log.py b/python/qemu/ot/util/log.py index 781067005560b..0a5eb866dab46 100644 --- a/python/qemu/ot/util/log.py +++ b/python/qemu/ot/util/log.py @@ -7,11 +7,17 @@ """ from os import getenv, isatty +from pickle import loads as ploads +from socketserver import DatagramRequestHandler, ThreadingUDPServer +from struct import calcsize as scalc, unpack as sunpack from sys import stderr +from threading import Thread from typing import NamedTuple, Optional, Sequence, Union import logging -from logging.handlers import MemoryHandler +from logging.handlers import (DatagramHandler, MemoryHandler, + DEFAULT_UDP_LOGGING_PORT) + try: getLevelNamesMapping = logging.getLevelNamesMapping @@ -127,7 +133,7 @@ def __init__(self, *args, **kwargs): else: tfmt = '' sep = ' ' if not use_lineno else '' - fnc = f' %(funcName)s{sep}' if use_func else ' ' + fnc = f' %(funcName)s(){sep}' if use_func else ' ' sep = ' ' if not use_func else '' lno = f'{sep}[%(lineno)d] ' if use_lineno else '' fmt_trail = f' %(name)-{name_width}s{fnc}{lno}%(scr)s%(message)s%(ecr)s' @@ -190,7 +196,93 @@ def override_xcolors(cls, codes: Sequence[int]) -> None: cls.XCOLORS = xcolors -def configure_loggers(level: int, *lognames: list[Union[str | int | Color]], +class RemoteLogHandler(DatagramRequestHandler): + """Handle a remote log UDP request.""" + + HEADER_FMT = '!L' + """UDP prefix that specifies the payload length.""" + + HEADER_SIZE = scalc(HEADER_FMT) + """The length in bytes of the header.""" + + def handle(self): + buffer = bytearray() + while True: + data = self.rfile.read(4) + if not data: + break + buffer.extend(data) + if len(buffer) < self.HEADER_SIZE: + continue + header, buffer = (buffer[:self.HEADER_SIZE], + buffer[self.HEADER_SIZE:]) + record_len, = sunpack(self.HEADER_FMT, header) + while len(buffer) < record_len: + data = self.rfile.read(4096) + if not data: + break + buffer.extend(data) + obj_data = buffer[:record_len] + obj = ploads(obj_data) + record = logging.makeLogRecord(obj) + logname = f'pyot.{record.name}' + self.server.do_log(logname, record) + + +class RemoteLogServer(ThreadingUDPServer): + """ThreadingUDPServer extension with logger cache manager.""" + + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + self._cache: dict[str, logging.Logger] = {} + + def do_log(self, logname: str, record: logging.LogRecord): + """Do actual log. + + This function enables log level local filtering and caching. + + :param logname: name of the logger + :param record: the log record + """ + if logname not in self._cache: + logger = logging.getLogger(logname) + self._cache[logname] = logger + lgr = self._cache[logname] + lgr.handle(record) + + +class RemoteLogService: + """Simple UDP receiver for handling remote log record.""" + + allow_reuse_address = True + daemon_threads = True + + def __init__(self, host='127.0.0.1', port=0): + RemoteLogServer.allow_reuse_address = True + RemoteLogServer.daemon_threads = True + self._port = port or DEFAULT_UDP_LOGGING_PORT + self._server = RemoteLogServer((host, self._port), RemoteLogHandler) + self._thread = Thread(target=self._serve, daemon=True) + + @property + def port(self) -> int: + """Return the selected UDP port.""" + return self._port + + def start(self): + """Start the logging service thread.""" + self._thread.start() + + def stop(self): + """Stop the logging service thread.""" + self._server.shutdown() + + def _serve(self): + with self._server: + self._server.serve_forever() + + +def configure_loggers(level: int, *lognames: list[Union[str, int, Color]], **kwargs) -> list[logging.Logger]: """Configure loggers. @@ -209,8 +301,16 @@ def configure_loggers(level: int, *lognames: list[Union[str | int | Color]], lnames = [lnames] loglevels[lvl] = tuple(lnames) quiet = kwargs.pop('quiet', False) + udplog = kwargs.pop('udplog', None) formatter = ColorLogFormatter(**kwargs) - shandler = logging.StreamHandler(stderr) + if udplog is not None: + udpport = udplog or DEFAULT_UDP_LOGGING_PORT + assert isinstance(udpport, int) + # use IP value rather than localhost host to avoid repetitive name + # resolution + shandler = DatagramHandler('127.0.0.1', udpport) + else: + shandler = logging.StreamHandler(stderr) shandler.setFormatter(formatter) if quiet: logh = MemoryHandler(100000, target=shandler, flushOnClose=False) From ceca18e57fa577a9b480304d94b3985d6fb70ef6 Mon Sep 17 00:00:00 2001 From: Emmanuel Blot Date: Wed, 20 Nov 2024 17:26:07 +0100 Subject: [PATCH 09/19] [ot] scripts/opentitan: pyot.py: add an option to start an UDP log listener. This service enables applications spawn in execution context to log messages into the pyot log stream. Signed-off-by: Emmanuel Blot --- docs/opentitan/pyot.md | 65 +++++++++++++++++++-------------------- scripts/opentitan/pyot.py | 24 +++++++++++++-- 2 files changed, 52 insertions(+), 37 deletions(-) diff --git a/docs/opentitan/pyot.md b/docs/opentitan/pyot.md index 6386e3ec03885..f2b7c0a9a4bbb 100644 --- a/docs/opentitan/pyot.md +++ b/docs/opentitan/pyot.md @@ -11,7 +11,8 @@ usage: pyot.py [-h] [-D DELAY] [-i ICOUNT] [-L LOG_FILE] [-M VARIANT] [-N LOG] [-f RAW] [-g file] [-K] [-l file] [-O RAW] [-o VMEM] [-r ELF] [-w CSV] [-x file] [-X] [-F TEST] [-k SECONDS] [-z] [-R] [-T FACTOR] [-Z] [-v] [-V] [-d] [--quiet] [--log-time] - [--debug LOGGER] [--info LOGGER] [--warn LOGGER] + [--log-udp UDP_PORT] [--debug LOGGER] [--info LOGGER] + [--warn LOGGER] OpenTitan QEMU unit test sequencer. @@ -19,62 +20,54 @@ options: -h, --help show this help message and exit Virtual machine: - -D DELAY, --start-delay DELAY + -D, --start-delay DELAY QEMU start up delay before initial comm - -i ICOUNT, --icount ICOUNT - virtual instruction counter with 2^ICOUNT clock ticks + -i, --icount ICOUNT virtual instruction counter with 2^ICOUNT clock ticks per inst. or 'auto' - -L LOG_FILE, --log_file LOG_FILE + -L, --log_file LOG_FILE log file for trace and log messages - -M VARIANT, --variant VARIANT + -M, --variant VARIANT machine variant (machine specific) - -N LOG, --log LOG log message types - -m MACHINE, --machine MACHINE + -N, --log LOG log message types + -m, --machine MACHINE virtual machine (default to ot-earlgrey) - -Q OPTS, --opts OPTS QEMU verbatim option (can be repeated) - -q QEMU, --qemu QEMU path to qemu application (default: build/qemu-system- + -Q, --opts OPTS QEMU verbatim option (can be repeated) + -q, --qemu QEMU path to qemu application (default: build/qemu-system- riscv32) - -P VCP, --vcp VCP serial port devices (default: use serial0) - -p DEVICE, --device DEVICE - serial port device name / template name (default to + -P, --vcp VCP serial port devices (default: use serial0) + -p, --device DEVICE serial port device name / template name (default to localhost:8000) - -t TRACE, --trace TRACE - trace event definition file - -S FIRST_SOC, --first-soc FIRST_SOC + -t, --trace TRACE trace event definition file + -S, --first-soc FIRST_SOC Identifier of the first SoC, if any -s, --singlestep enable "single stepping" QEMU execution mode -U, --muxserial enable multiple virtual UARTs to be muxed into same host output channel Files: - -b file, --boot file bootloader 0 file - -c HJSON, --config HJSON - path to HJSON configuration file + -b, --boot file bootloader 0 file + -c, --config HJSON path to HJSON configuration file -e, --embedded-flash generate an embedded flash image file - -f RAW, --flash RAW SPI flash image file - -g file, --otcfg file - configuration options for OpenTitan devices + -f, --flash RAW SPI flash image file + -g, --otcfg file configuration options for OpenTitan devices -K, --keep-tmp Do not automatically remove temporary files and dirs on exit - -l file, --loader file - ROM trampoline to execute, if any - -O RAW, --otp-raw RAW - OTP image file - -o VMEM, --otp VMEM OTP VMEM file - -r ELF, --rom ELF ROM file (can be repeated, in load order) - -w CSV, --result CSV path to output result file - -x file, --exec file application to load + -l, --loader file ROM trampoline to execute, if any + -O, --otp-raw RAW OTP image file + -o, --otp VMEM OTP VMEM file + -r, --rom ELF ROM file (can be repeated, in load order) + -w, --result CSV path to output result file + -x, --exec file application to load -X, --rom-exec load application as ROM image (default: as kernel) Execution: - -F TEST, --filter TEST - run tests with matching filter, prefix with "!" to + -F, --filter TEST run tests with matching filter, prefix with "!" to exclude matching tests - -k SECONDS, --timeout SECONDS + -k, --timeout SECONDS exit after the specified seconds (default: 60 secs) -z, --list show a list of tests to execute and exit -R, --summary show a result summary - -T FACTOR, --timeout-factor FACTOR + -T, --timeout-factor FACTOR timeout factor -Z, --zero do not error if no test can be executed @@ -84,6 +77,7 @@ Extras: -d enable debug mode --quiet quiet logging: only be verbose on errors --log-time show local time in log messages + --log-udp UDP_PORT Change UDP port for log messages, use 0 to disable --debug LOGGER assign debug level to logger(s) --info LOGGER assign info level to logger(s) --warn LOGGER assign warning level to logger(s) @@ -185,6 +179,8 @@ This tool may be used in two ways, which can be combined: * `-d` only useful to debug the script, reports any Python traceback to the standard error stream. * `--quiet` only emit verbose log traces if an error is detected * `--log-time` show local time before each logged message +* `--log-udp` change the port of the UDP log service on specified UDP port. Use `0` to disable the + service. * `--debug` enable the debug level for the selected logger, may be repeated * `--info` enable the info level for the selected logger, may be repeated * `--warn` enable the warning level for the selected logger, may be repeated @@ -386,6 +382,7 @@ Sample config for running some non-OpenTitan tests: * `${UTPATH}` absolute path to the executed OT test * `${UTDIR}` absolute path to the directory containing the executed OT test * `${UTFILE}` file name of the executed OT test (without directory specifier) + * `${UDPLOG}` UDP log service, if UDP log service has been enabled, see `--log-udp` option. * `testdir` This section may be used to define the default path where to look for tests to run. diff --git a/scripts/opentitan/pyot.py b/scripts/opentitan/pyot.py index d88409b1cb2ae..a4984be513477 100755 --- a/scripts/opentitan/pyot.py +++ b/scripts/opentitan/pyot.py @@ -42,13 +42,12 @@ def hjload(*_, **__): # noqa: E301 QEMU_PYPATH = joinpath(dirname(dirname(dirname(normpath(__file__)))), 'python', 'qemu') sys.path.append(QEMU_PYPATH) -print(__file__, sys.path[-1]) # pylint: disable=wrong-import-position # pylint: disable=wrong-import-order # pylint: disable=import-error -from ot.util.log import ColorLogFormatter, configure_loggers +from ot.util.log import ColorLogFormatter, RemoteLogService, configure_loggers from ot.util.misc import EasyDict @@ -592,6 +591,13 @@ def set_config_dir(self, path: str) -> None: """ self._env['CONFIG'] = abspath(path) + def set_udp_log_port(self, port: int) -> None: + """Assign the UDP logger port. + + :param port: the UDP logger port + """ + self._env['UDPLOG'] = f'{port}' + def interpolate(self, value: Any) -> str: """Interpolate a ${...} marker with shell substitutions or local substitution. @@ -1758,6 +1764,7 @@ def main(): qemu_path = None tmp_result: Optional[str] = None result_file: Optional[str] = None + rlog: Optional[RemoteLogService] = None try: args: Optional[Namespace] = None desc = sys.modules[__name__].__doc__.split('.', 1)[0].strip() @@ -1855,6 +1862,9 @@ def main(): help='quiet logging: only be verbose on errors') extra.add_argument('--log-time', action='store_true', help='show local time in log messages') + extra.add_argument('--log-udp', type=int, metavar='UDP_PORT', + help='Change UDP port for log messages, ' + 'use 0 to disable') extra.add_argument('--debug', action='append', metavar='LOGGER', help='assign debug level to logger(s)') extra.add_argument('--info', action='append', metavar='LOGGER', @@ -1887,7 +1897,7 @@ def main(): log = configure_loggers(args.verbose, 'pyot', args.vcp_verbose or 0, - 'pyot.vcp', name_width=16, + 'pyot.vcp', name_width=30, ms=args.log_time, quiet=args.quiet, debug=args.debug, info=args.info, warning=args.warn)[0] @@ -1895,6 +1905,12 @@ def main(): qfm = QEMUFileManager(args.keep_tmp) qfm.set_qemu_src_dir(qemu_dir) + if args.log_udp != 0: + rlog = RemoteLogService(port=args.log_udp) + rlog.start() + qfm.set_udp_log_port(rlog.port) + + # this is a bit circomvulted, as we need to parse the config filename # if any, and load the default values out of the configuration file, # without overriding any command line argument that should take @@ -1994,6 +2010,8 @@ def main(): except KeyboardInterrupt: sys.exit(2) finally: + if rlog: + rlog.stop() if result_file: rfmt = ResultFormatter() try: From 30317dfbc936633374136eeb475da116c3c5f804 Mon Sep 17 00:00:00 2001 From: Emmanuel Blot Date: Mon, 25 Nov 2024 11:51:05 +0100 Subject: [PATCH 10/19] [ot]: python/qemu: ot.util.log: add an API to discard MemoryHandler records Signed-off-by: Emmanuel Blot --- python/qemu/ot/util/log.py | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/python/qemu/ot/util/log.py b/python/qemu/ot/util/log.py index 0a5eb866dab46..b51a4c857ce46 100644 --- a/python/qemu/ot/util/log.py +++ b/python/qemu/ot/util/log.py @@ -343,3 +343,31 @@ def configure_loggers(level: int, *lognames: list[Union[str, int, Color]], if not log.hasHandlers(): log.addHandler(logh) return loggers + + +def flush_memory_loggers(loggers: Sequence[Union[logging.Logger, str]], + level: Optional[int] = None) -> None: + """Discard all buffered records of any MemoryHandler logger handlers. + + :param loggers: the loggers to consider, or the root logger if none is + specified. + :param level: if not None, flush all records whose level is higher or + equal to the specified level. + """ + if not loggers: + loggers = [logging.getLogger()] + for log in loggers: + if isinstance(log, str): + log = logging.getLogger(log) + for hdlr in log.handlers: + if not isinstance(hdlr, MemoryHandler): + continue + if level is None: + super(MemoryHandler, hdlr).flush() + continue + with hdlr.lock: + if hdlr.target: + for record in hdlr.buffer: + if record.levelno >= level: + hdlr.target.handle(record) + hdlr.buffer.clear() From be864fdcfa1ff4c065a73ac83d3f63fc586b3124 Mon Sep 17 00:00:00 2001 From: Emmanuel Blot Date: Mon, 25 Nov 2024 11:53:06 +0100 Subject: [PATCH 11/19] [ot] scripts/opentitan: pyot.py: discard memoryhandler logger content after each test. This enables only reporting all log messages of a test that fails, not all the log messages from all previously succeeded tests. Signed-off-by: Emmanuel Blot --- scripts/opentitan/pyot.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/scripts/opentitan/pyot.py b/scripts/opentitan/pyot.py index a4984be513477..e87ccff6b3e86 100755 --- a/scripts/opentitan/pyot.py +++ b/scripts/opentitan/pyot.py @@ -47,7 +47,8 @@ def hjload(*_, **__): # noqa: E301 # pylint: disable=wrong-import-order # pylint: disable=import-error -from ot.util.log import ColorLogFormatter, RemoteLogService, configure_loggers +from ot.util.log import (ColorLogFormatter, RemoteLogService, configure_loggers, + flush_memory_loggers) from ot.util.misc import EasyDict @@ -1284,6 +1285,7 @@ def run(self, debug: bool, allow_no_test: bool) -> int: err = str(exc) finally: self._qfm.cleanup_transient() + flush_memory_loggers(['pyot', 'pyot.vcp'], logging.INFO) results[tret] += 1 sret = self.RESULT_MAP.get(tret, tret) if targs: From 5242000e1c02e93b95cfb71330fc22873e9be242 Mon Sep 17 00:00:00 2001 From: Emmanuel Blot Date: Mon, 25 Nov 2024 11:54:45 +0100 Subject: [PATCH 12/19] [ot] scripts/opentitan: pyot.py: limit the length of a reported test error message. Signed-off-by: Emmanuel Blot --- scripts/opentitan/pyot.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/scripts/opentitan/pyot.py b/scripts/opentitan/pyot.py index e87ccff6b3e86..695e9f57ab2dc 100755 --- a/scripts/opentitan/pyot.py +++ b/scripts/opentitan/pyot.py @@ -31,6 +31,7 @@ def hjload(*_, **__): # noqa: E301 from subprocess import Popen, PIPE, TimeoutExpired from threading import Event, Thread from tempfile import mkdtemp, mkstemp +from textwrap import shorten from time import time as now from traceback import format_exc from typing import Any, Iterator, NamedTuple, Optional @@ -101,11 +102,12 @@ def show(self, spacing: bool = False) -> None: """ if spacing: print('') - widths = [max(len(x) for x in col) for col in zip(*self._results)] + results = [r[:-1] + [shorten(r[-1], width=100)] for r in self._results] + widths = [max(len(x) for x in col) for col in zip(*results)] self._show_line(widths, '-') - self._show_row(widths, self._results[0]) + self._show_row(widths, results[0]) self._show_line(widths, '=') - for row in self._results[1:]: + for row in results[1:]: self._show_row(widths, row) self._show_line(widths, '-') if spacing: @@ -1758,7 +1760,7 @@ def _build_test_context(self, test_name: str) -> QEMUContext: def main(): - """Main routine""" + """Main routine.""" debug = True qemu_dir = normpath(joinpath(dirname(dirname(dirname(normpath(__file__)))))) qemu_path = normpath(joinpath(qemu_dir, 'build', 'qemu-system-riscv32')) From 7e15b87fe2c65f877d3df9e61c256070249047a4 Mon Sep 17 00:00:00 2001 From: Emmanuel Blot Date: Wed, 27 Nov 2024 10:50:30 +0100 Subject: [PATCH 13/19] [ot] scripts/opentitan: pyot.py: add a small delay on exit to handle UDP log messages Signed-off-by: Emmanuel Blot --- scripts/opentitan/pyot.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/scripts/opentitan/pyot.py b/scripts/opentitan/pyot.py index 695e9f57ab2dc..14b591059fb3a 100755 --- a/scripts/opentitan/pyot.py +++ b/scripts/opentitan/pyot.py @@ -32,7 +32,7 @@ def hjload(*_, **__): # noqa: E301 from threading import Event, Thread from tempfile import mkdtemp, mkstemp from textwrap import shorten -from time import time as now +from time import sleep, time as now from traceback import format_exc from typing import Any, Iterator, NamedTuple, Optional @@ -2004,6 +2004,9 @@ def main(): argparser.error(str(exc)) ret = qexc.run(debug, args.zero) log.debug('End of execution with code %d', ret or 0) + if rlog: + # leave extra time for last logging packets to be received + sleep(0.1) sys.exit(ret) # pylint: disable=broad-except except Exception as exc: From 7c98542fb7cbed040e58ffc62466457de3d93877 Mon Sep 17 00:00:00 2001 From: Emmanuel Blot Date: Thu, 21 Nov 2024 16:40:53 +0100 Subject: [PATCH 14/19] [ot] python/qemu: reorganize and clean up jtag module. * bits & jtag: import latest version from pyjtagtools --- .github/workflows/build_test.yaml | 2 +- python/qemu/jtag/__init__.py | 4 - python/qemu/{jtag => jtagtools}/.flake8 | 0 python/qemu/{jtag => jtagtools}/.pylintrc | 0 python/qemu/jtagtools/__init__.py | 8 + .../bits.py => jtagtools/bits/__init__.py} | 372 ++++++++++++++---- python/qemu/jtagtools/jtag/__init__.py | 11 + python/qemu/jtagtools/jtag/controller.py | 73 ++++ python/qemu/jtagtools/jtag/engine.py | 116 ++++++ python/qemu/jtagtools/jtag/error.py | 14 + .../jtag.py => jtagtools/jtag/machine.py} | 149 +------ python/qemu/jtagtools/rbb/__init__.py | 8 + .../qemu/{jtag => jtagtools/rbb}/bitbang.py | 8 +- python/qemu/ot/dm/dm.py | 3 +- python/qemu/ot/dtm/dtm.py | 14 +- scripts/opentitan/dtm.py | 9 +- scripts/opentitan/otpdm.py | 4 +- 17 files changed, 559 insertions(+), 236 deletions(-) delete mode 100644 python/qemu/jtag/__init__.py rename python/qemu/{jtag => jtagtools}/.flake8 (100%) rename python/qemu/{jtag => jtagtools}/.pylintrc (100%) create mode 100644 python/qemu/jtagtools/__init__.py rename python/qemu/{jtag/bits.py => jtagtools/bits/__init__.py} (71%) create mode 100644 python/qemu/jtagtools/jtag/__init__.py create mode 100644 python/qemu/jtagtools/jtag/controller.py create mode 100644 python/qemu/jtagtools/jtag/engine.py create mode 100644 python/qemu/jtagtools/jtag/error.py rename python/qemu/{jtag/jtag.py => jtagtools/jtag/machine.py} (59%) create mode 100644 python/qemu/jtagtools/rbb/__init__.py rename python/qemu/{jtag => jtagtools/rbb}/bitbang.py (97%) diff --git a/.github/workflows/build_test.yaml b/.github/workflows/build_test.yaml index 0c23be068b17f..f510eff838eca 100644 --- a/.github/workflows/build_test.yaml +++ b/.github/workflows/build_test.yaml @@ -94,7 +94,7 @@ jobs: - name: Lint Python code run: | pylint --rcfile scripts/opentitan/.pylintrc -d 'duplicate-code' -d 'fixme' \ - scripts/opentitan/*.py python/qemu/jtag python/qemu/ot + scripts/opentitan/*.py python/qemu/jtagtools python/qemu/ot lint-clang: runs-on: ubuntu-latest diff --git a/python/qemu/jtag/__init__.py b/python/qemu/jtag/__init__.py deleted file mode 100644 index fe3d00da1f204..0000000000000 --- a/python/qemu/jtag/__init__.py +++ /dev/null @@ -1,4 +0,0 @@ -# Copyright (c) 2024 Rivos, Inc. -# SPDX-License-Identifier: Apache2 - -"""JTAG tools.""" diff --git a/python/qemu/jtag/.flake8 b/python/qemu/jtagtools/.flake8 similarity index 100% rename from python/qemu/jtag/.flake8 rename to python/qemu/jtagtools/.flake8 diff --git a/python/qemu/jtag/.pylintrc b/python/qemu/jtagtools/.pylintrc similarity index 100% rename from python/qemu/jtag/.pylintrc rename to python/qemu/jtagtools/.pylintrc diff --git a/python/qemu/jtagtools/__init__.py b/python/qemu/jtagtools/__init__.py new file mode 100644 index 0000000000000..ad9e1081ea683 --- /dev/null +++ b/python/qemu/jtagtools/__init__.py @@ -0,0 +1,8 @@ +# Copyright (c) 2024 Rivos Inc. Emmanuel Blot +# Copyright (c) 2024, Emmanuel Blot +# All rights reserved. +# +# SPDX-License-Identifier: Apache2 + +"""JTAG Tools.""" +__version__ = '0.11.0' diff --git a/python/qemu/jtag/bits.py b/python/qemu/jtagtools/bits/__init__.py similarity index 71% rename from python/qemu/jtag/bits.py rename to python/qemu/jtagtools/bits/__init__.py index 23153134f8eb2..148f716e25b68 100644 --- a/python/qemu/jtag/bits.py +++ b/python/qemu/jtagtools/bits/__init__.py @@ -1,12 +1,12 @@ -# Copyright (c) 2010-2024 Emmanuel Blot +# Copyright (c) 2024 Rivos Inc. Emmanuel Blot +# Copyright (c) 2024 Emmanuel Blot # All rights reserved. # -# SPDX-License-Identifier: BSD-3-Clause +# SPDX-License-Identifier: Apache2 -"""Bit sequence helpers for JTAG/DTM. +"""Bit sequence helpers for JTAG. - BitSequence is a wrapper to help manipulating bits with the JTAG tools. - Imported from pyftdi project. + BitSequence handle bit manipulation for the JTAG tools. """ from typing import Any, Iterable, Union @@ -22,7 +22,7 @@ class BitSequenceError(Exception): """ -BitSequenceInitializer = Union['BitSequence', str, int, bytes, bytearray, +BitSequenceInitializer = Union['BitSequence', str, int, memoryview, Iterable[int], Iterable[bool], None] """Supported types to initialize a BitSequence.""" @@ -36,7 +36,7 @@ class BitSequence: Bit sequence objects are iterable. :param value: initial value - :param length: count of signficant bits in the bit sequence + :param width: count of significant bits in the bit sequence >>> BitSequence() [] @@ -44,6 +44,8 @@ class BitSequence: [0, 1] >>> BitSequence([0, 1, 1]) [0, 1, 1] + >>> BitSequence(BitSequence([0, 1, 1]), 5) + [0, 0, 0, 1, 1] >>> BitSequence('100') [1, 0, 0] >>> BitSequence(b'101') @@ -54,8 +56,15 @@ class BitSequence: Traceback (most recent call last): ... ValueError: Value cannot be stored in specified width + >>> BitSequence(BitSequence([1, 1, 0, 0]), 3) + Traceback (most recent call last): + ... + ValueError: Specified width too short """ + NIBBLE_SEP = '' + # Optional string separator between each nibble for string representation + # pylint: disable=protected-access def __init__(self, value: BitSequenceInitializer = None, @@ -71,9 +80,17 @@ def __init__(self, value: BitSequenceInitializer = None, self._width = width return if isinstance(value, BitSequence): - self._int, self._width = value._int, value._width + if width: + if value._width > width: + raise ValueError('Specified width too short') + width = max(width, value._width) + else: + width = value._width + self._int, self._width = value._int, width return if isinstance(value, int): + if width == 0: + raise ValueError('Integer value should have a non-zero width') if value >= 1 << width: raise ValueError('Value cannot be stored in specified width') bseq = self.from_int(value, width) @@ -90,11 +107,17 @@ def __init__(self, value: BitSequenceInitializer = None, @classmethod def from_iterable(cls, iterable: Iterable) -> 'BitSequence': """Instanciate a BitSequence from an iterable. + Each element of the iterable should represent a single bit. + + See BitSequence.from_bytes to create a BitSequence for a byte + stream. >>> BitSequence('11000100') [1, 1, 0, 0, 0, 1, 0, 0] >>> BitSequence(b'11000100') [1, 1, 0, 0, 0, 1, 0, 0] + >>> BitSequence(b'\\x01\\x01\\x00\\x00\\x00\\x01\\x000') + [1, 1, 0, 0, 0, 1, 0, 0] >>> BitSequence([1, 1, 0, 0, 0, 1, 0, 0]) [1, 1, 0, 0, 0, 1, 0, 0] """ @@ -113,8 +136,8 @@ def from_iterable(cls, iterable: Iterable) -> 'BitSequence': value |= smap[bit] width += 1 except KeyError as exc: - raise ValueError(f"Invalid item '{bit}' in iterable at pos " - f"{width}") from exc + raise ValueError(f"Invalid item value '{bit}' in iterable at " + f"pos {width}") from exc return BitSequence.from_int(value, width) @classmethod @@ -123,12 +146,46 @@ def from_int(cls, value: int, width: int) -> 'BitSequence': >>> BitSequence(0xC4, 8) [1, 1, 0, 0, 0, 1, 0, 0] + >>> BitSequence(-1, 9) + [1, 1, 1, 1, 1, 1, 1, 1, 1] """ bseq = BitSequence() - bseq._int = value + bseq._int = value & ((1 << width) - 1) bseq._width = width return bseq + @classmethod + def from_bytes(cls, value: memoryview) -> 'BitSequence': + """Instanciate a BitSequence from a sequence of bytes, one bit for each + input byte. + + >>> BitSequence(b'\\x01\\x01\\x00\\x00\\x00\\x01\\x000') + [1, 1, 0, 0, 0, 1, 0, 0] + """ + return cls.from_iterable(value) + + @classmethod + def from_bytestream(cls, value: memoryview, lsbyte: bool = False) \ + -> 'BitSequence': + """Instanciate a BitSequence from a sequence of bytes, 8 bits for each + input byte. + + >>> BitSequence.from_bytestream(b'\\xca\\xfe') + [1, 1, 0, 0, 1, 0, 1, 0, 1, 1, 1, 1, 1, 1, 1, 0] + >>> BitSequence.from_bytestream(b'\\xca\\xfe', True) + [1, 1, 1, 1, 1, 1, 1, 0, 1, 1, 0, 0, 1, 0, 1, 0] + """ + bseq = BitSequence(0, 8 * len(value)) + if lsbyte: + for byte in reversed(value): + bseq._int <<= 8 + bseq._int |= byte + else: + for byte in value: + bseq._int <<= 8 + bseq._int |= byte + return bseq + def __len__(self): """ >>> len(BitSequence()) @@ -178,33 +235,36 @@ def __str__(self) -> str: """ >>> str(BitSequence(0xC4, 8)) '11000100' - """ - return f'{self._int:0{self._width}b}' - - def copy(self) -> 'BitSequence': - """Duplicate bitsequence. + >>> BitSequence.NIBBLE_SEP = '.' - >>> bs1 = BitSequence(0xC4, 8) - - >>> bs2 = bs1.copy() - - >>> bs1 == bs2 - True - >>> id(bs1) == id(bs2) - False - >>> bs1._int = 0 - - >>> bs1 == bs2 - False - >>> int(bs1) - 0 - >>> int(bs2) - 196 - """ - bseq = self.__class__() - bseq._int = self._int - bseq._width = self._width - return bseq + >>> str(BitSequence(0xC4, 8)) + '1100.0100' + >>> str(BitSequence(0x2C4, 10)) + '10.1100.0100' + >>> BitSequence.NIBBLE_SEP = '' + + """ + bss = f'{self._int:0{self._width}b}' + if not self.NIBBLE_SEP: + return bss + xlen = len(bss) % 4 + padlen = 4 - xlen + if xlen: + pad = ' ' * padlen + bss = f'{pad}{bss}' + nbss = self.NIBBLE_SEP.join((''.join(nb) + for nb in BitSequence.group(bss, 4))) + return nbss[padlen:] if xlen else nbss + + @staticmethod + def group(lst, count): + """Group a list into consecutive count-tuples. Incomplete tuples are + discarded. + + >>> BitSequence.group([0,3,4,10,2,3], 2) + [(0, 3), (4, 10), (2, 3)] + """ + return list(zip(*[lst[i::count] for i in range(count)])) @property def mask(self) -> int: @@ -233,17 +293,17 @@ def to_bit(self) -> bool: raise BitSequenceError("BitSequence too large") return bool(self._int & 1) - def to_byte(self, msb: bool = False) -> int: + def to_byte(self, lsb: bool = False) -> int: """Convert the sequence into a single byte value, if possible. >>> hex(BitSequence(0xC4, 8).to_byte()) - '0x23' - >>> hex(BitSequence(0xC4, 8).to_byte(True)) '0xc4' + >>> hex(BitSequence(0xC4, 8).to_byte(True)) + '0x23' """ if self._width > 8: raise BitSequenceError("Cannot fit into a single byte") - if not msb: + if lsb: bseq = BitSequence(self) bseq.reverse() else: @@ -266,32 +326,82 @@ def to_bool_list(self) -> list[bool]: """ return list(self) - def to_bytestream(self, msb: bool = False, msby: bool = False) -> bytes: + def to_bytestream(self, lsbyte: bool = False, lsbit: bool = False)\ + -> bytes: """Convert the sequence into a sequence of byte values. >>> from binascii import hexlify - >>> hexlify(BitSequence(0xC4A5D0, 24).to_bytestream(True, True)) + >>> hexlify(BitSequence(0xC4A5D0, 24).to_bytestream(False, False)) b'c4a5d0' >>> hexlify(BitSequence(0xC4A5D0, 24).to_bytestream(True, False)) b'd0a5c4' >>> hexlify(BitSequence(0xC4A5D0, 24).to_bytestream(False, True)) b'23a50b' - >>> hexlify(BitSequence(0xC4A5D0, 24).to_bytestream(False, False)) + >>> hexlify(BitSequence(0xC4A5D0, 24).to_bytestream(True, True)) b'0ba523' + >>> hexlify(BitSequence(0xC4A5D01234, 40).to_bytestream(False, False)) + b'c4a5d01234' """ out: list[int] = [] bseq = BitSequence(self) - if not msb: + if lsbit: bseq.reverse() while bseq._width: out.append(bseq._int & 0xff) bseq._int >>= 8 bseq._width -= 8 - if not msby ^ msb: + if not lsbit ^ lsbyte: out.reverse() return bytes(out) + def resize(self, width: int) -> 'BitSequence': + """Change the width of the BitSequence. + Truncate the stored bits of new width is shorter. + + :return: self + + >>> bs1 = BitSequence(0xC4, 8) + + >>> bs1.resize(10) + [0, 0, 1, 1, 0, 0, 0, 1, 0, 0] + >>> bs1.resize(5) + [0, 0, 1, 0, 0] + """ + if width > self._width: + self._int &= (1 << width) - 1 + self._width = width + return self + + def copy(self, reverse=False) -> 'BitSequence': + """Duplicate bitsequence. + + >>> bs1 = BitSequence(0xC4, 8) + + >>> bs2 = bs1.copy() + + >>> bs1 == bs2 + True + >>> id(bs1) == id(bs2) + False + >>> bs1._int = 0 + + >>> bs1 == bs2 + False + >>> int(bs1) + 0 + >>> int(bs2) + 196 + >>> BitSequence(0b11000100, 8).copy(True) + [0, 0, 1, 0, 0, 0, 1, 1] + """ + if not reverse: + bseq = self.__class__() + bseq._int = self._int + bseq._width = self._width + return bseq + return self.__class__.from_iterable(reversed(self)) + def reverse(self) -> 'BitSequence': """In-place reverse. @@ -425,6 +535,12 @@ def pop_left_bit(self) -> bool: >>> bs [1, 0, 0, 0, 1, 0, 0] + >>> bs = BitSequence(1, 2048) + + >>> for _ in range(2047): _ = bs.pop_left_bit() + + >>> bs + [1] """ if self._width == 0: raise RuntimeError('Empty bit sequence') @@ -704,6 +820,54 @@ def __xor__(self, other: 'BitSequence') -> 'BitSequence': value = self._int ^ other._int return self.__class__.from_int(value, self._width) + def __iand__(self, other: 'BitSequence') -> 'BitSequence': + """ + >>> bs = BitSequence(0b10011, 5) + + >>> bs &= BitSequence(0b11001, 5) + + >>> bs + [1, 0, 0, 0, 1] + """ + if not isinstance(other, self.__class__): + raise ValueError('Need a BitSequence to combine') + if self._width != other._width: + raise ValueError('Sequences must be the same size') + self._int &= other._int + return self + + def __ior__(self, other: 'BitSequence') -> 'BitSequence': + """ + >>> bs = BitSequence(0b10011, 5) + + >>> bs |= BitSequence(0b11001, 5) + + >>> bs + [1, 1, 0, 1, 1] + """ + if not isinstance(other, self.__class__): + raise ValueError('Need a BitSequence to combine') + if self._width != other._width: + raise ValueError('Sequences must be the same size') + self._int |= other._int + return self + + def __ixor__(self, other: 'BitSequence') -> 'BitSequence': + """ + >>> bs = BitSequence(0b10011, 5) + + >>> bs ^= BitSequence(0b11001, 5) + + >>> bs + [0, 1, 0, 1, 0] + """ + if not isinstance(other, self.__class__): + raise ValueError('Need a BitSequence to combine') + if self._width != other._width: + raise ValueError('Sequences must be the same size') + self._int ^= other._int + return self + def __invert__(self) -> 'BitSequence': """ >>> ~BitSequence(0b10011, 5) @@ -736,6 +900,27 @@ def __irshift__(self, count) -> 'BitSequence': value = (self._int >> count) & self.mask return self.__class__.from_int(value, self._width) + def __iadd__(self, other) -> 'BitSequence': + """ + >>> bs = BitSequence(0b10011, 5) + + >>> bs += BitSequence([0, 1, 0]) + + >>> len(bs) + 8 + + >>> bs + [1, 0, 0, 1, 1, 0, 1, 0] + """ + if not isinstance(other, self.__class__): + raise TypeError(f"unsupported operand type(s) for +=: " + f"'{self.__class__.__name__}' and " + f"'{other.__class__.__name__}'") + self._width += other._width + self._int <<= other._width + self._int |= other._int + return self + def __getitem__(self, index) -> 'BitSequence': """ >>> bs = BitSequence(0b11000100, 8) @@ -756,17 +941,30 @@ def __getitem__(self, index) -> 'BitSequence': [1, 1, 0, 0, 0] >>> bs[-4:-1] [0, 1, 0] + >>> bs[:-2] + [1, 1, 0, 0, 0, 1] + >>> bs[::-1] + [0, 0, 1, 0, 0, 0, 1, 1] + >>> bs[-5::-2] + [0, 1] """ if isinstance(index, slice): bits: list[int] = [] - for bpos in range(index.start or 0, index.stop or 0, - index.step or 1): - if bpos >= self._width: - break - if bpos >= 0: - bits.append((self._int >> (self._width - bpos - 1)) & 1) - else: - bits.append((self._int >> (- bpos - 1)) & 1) + length = self._width + start = index.start + stop = index.stop + step = index.step or 1 + if start is None: + start = 0 if step > 0 else length + elif start < 0: + start = max(0, length+start) + if stop is None: + stop = length if step > 0 else 0 + elif stop < 0: + stop = min(length+stop, length) + off = -1 if step > 0 else 0 + for bpos in range(start, stop, step): + bits.append((self._int >> (self._width - bpos + off)) & 1) return self.__class__.from_iterable(bits) if not isinstance(index, int): raise TypeError(f'{self.__class__.__name__} indices must be ' @@ -799,6 +997,27 @@ def __setitem__(self, index, value) -> None: >>> bs [1, 0, 1, 1, 0, 1, 0, 1] + >>> bs[7] = False + + >>> bs + [1, 0, 1, 1, 0, 1, 0, 0] + + >>> bs[-3:] = [0, 1, 1] + + >>> bs + [1, 0, 1, 1, 0, 0, 1, 1] + + >>> bs = BitSequence(0, 8) + + >>> bs[-5::-2] = [1, 1] + + >>> bs + [0, 1, 0, 1, 0, 0, 0, 0] + + >>> bs[-5::-2] = [1, 1, 1] + Traceback (most recent call last): + ... + ValueError: Cannot assign sequence of size 3 to ext. slice of size 2 """ if isinstance(index, slice): if not isinstance(value, BitSequence): @@ -807,18 +1026,34 @@ def __setitem__(self, index, value) -> None: value = self.from_iterable(value) else: value = value.copy() - for bpos in range(index.start or 0, index.stop or 0, - index.step or 1): - if bpos >= self._width: - break - if not value: - break - shift = self._width - bpos - 1 if bpos >= 0 else - bpos - 1 - bit = value.pop_left() - if bit: - self._int |= 1 << shift + length = self._width + start = index.start + stop = index.stop + step = index.step or 1 + if start is None: + start = 0 if step > 0 else length + elif start < 0: + start = max(0, length+start) + if stop is None: + stop = length if step > 0 else 0 + elif stop < 0: + stop = min(length+stop, length) + slen = len(range(start, stop, step)) + vlen = len(value) + if slen != len(value): + raise ValueError(f'Cannot assign sequence of size {vlen} ' + f'to ext. slice of size {slen}') + nint = self._int + for bpos in range(start, stop, step): + pos = length - 1 - bpos + if value.pop_left_bit(): + nint |= 1 << pos else: - self._int &= ~(1 << shift) + nint &= ~(1 << pos) + if value: + raise ValueError('Slice assignment does not support width ' + 'extension') + self._int = nint return if not isinstance(index, int): raise TypeError(f'{self.__class__.__name__} indices must be ' @@ -828,15 +1063,13 @@ def __setitem__(self, index, value) -> None: elif isinstance(value, int): if value not in (0, 1): raise ValueError('Invalid value') - value = int(value) if index >= 0: bpos = self._width - index - 1 else: bpos = - index - 1 if not 0 <= bpos < self._width: - raise IndexError(f'{self.__class__.__name__} index out of ' - f'range') - if value: + raise IndexError(f'{self.__class__.__name__} index out of range') + if bool(value): self._int |= 1 << bpos else: self._int &= ~(1 << bpos) @@ -875,5 +1108,6 @@ def is_iterable(obj: Any) -> bool: if __name__ == "__main__": + # note: tests expect an empty BitSequence.NIBBLE_SEP string import doctest doctest.testmod() diff --git a/python/qemu/jtagtools/jtag/__init__.py b/python/qemu/jtagtools/jtag/__init__.py new file mode 100644 index 0000000000000..9114df3dd7e18 --- /dev/null +++ b/python/qemu/jtagtools/jtag/__init__.py @@ -0,0 +1,11 @@ +# Copyright (c) 2024 Rivos Inc. Emmanuel Blot +# Copyright (c) 2010-2024, Emmanuel Blot +# All rights reserved. +# +# SPDX-License-Identifier: Apache2 + +"""JTAG Tools.""" + +from .controller import JtagController +from .engine import JtagEngine +from .error import JtagError diff --git a/python/qemu/jtagtools/jtag/controller.py b/python/qemu/jtagtools/jtag/controller.py new file mode 100644 index 0000000000000..799c3695ccba3 --- /dev/null +++ b/python/qemu/jtagtools/jtag/controller.py @@ -0,0 +1,73 @@ +# Copyright (c) 2024 Rivos Inc. Emmanuel Blot +# Copyright (c) 2010-2024, Emmanuel Blot +# Copyright (c) 2016, Emmanuel Bouaziz +# All rights reserved. +# +# SPDX-License-Identifier: Apache2 + +"""JTAG controller API.""" + +from ..bits import BitSequence + + +class JtagController: + """JTAG master API.""" + + INSTRUCTIONS = {'bypass': 0x0, 'idcode': 0x1} + """Common instruction register codes.""" + + def tap_reset(self, use_trst: bool = False) -> None: + """Reset the TAP controller. + + :param use_trst: use TRST HW wire if available + """ + raise NotImplementedError('ABC') + + def system_reset(self) -> None: + """Reset the device.""" + + def quit(self) -> None: + """Terminate session.""" + + def scan(self) -> BitSequence: + """Flush output buffer and read back the requested BitSequence. + """ + raise NotImplementedError('ABC') + + def write_tms(self, modesel: BitSequence, read_tdo: bool = False) -> None: + """Change the TAP controller state. + + :note: modesel content may be consumed, i.e. emptied + :note: last TMS bit should be stored and clocked on next write + request + + :param modesel: the bit sequence of TMS bits to clock in + :param read_tdo: whether to read back the TDO bit on the first CLK + cycle of the TMS sequence + """ + raise NotImplementedError('ABC') + + def write(self, out: BitSequence) -> None: + """Write a sequence of bits to TDI. + + :note: out content may be consumed, i.e. emptied + :param out: the bit sequence of TDI bits to clock in + """ + raise NotImplementedError('ABC') + + def read(self, length: int) -> None: + """Read out a sequence of bits from TDO. + + :param length: the number of bits to clock out from the remote + device + """ + raise NotImplementedError('ABC') + + def exchange(self, out: BitSequence) -> BitSequence: + """Write a sequence to TDI and read out a sequence of the same length + from TDO + + :param out: the bit sequence of TDI bits to clock in + :return: the bit sequence received from TDO + """ + raise NotImplementedError('ABC') diff --git a/python/qemu/jtagtools/jtag/engine.py b/python/qemu/jtagtools/jtag/engine.py new file mode 100644 index 0000000000000..afe43fa5ed4cf --- /dev/null +++ b/python/qemu/jtagtools/jtag/engine.py @@ -0,0 +1,116 @@ +# Copyright (c) 2024 Rivos Inc. Emmanuel Blot +# Copyright (c) 2010-2024, Emmanuel Blot +# Copyright (c) 2016, Emmanuel Bouaziz +# All rights reserved. +# +# SPDX-License-Identifier: Apache2 + +"""JTAG engine.""" + +from logging import getLogger + +from ..bits import BitSequence +from .controller import JtagController +from .machine import JtagStateMachine + + +class JtagEngine: + """High-level JTAG engine.""" + + def __init__(self, ctrl: 'JtagController'): + self._ctrl = ctrl + self._log = getLogger('jtag.eng') + self._fsm = JtagStateMachine() + self._tr_cache: dict[tuple[str, # from state + str], # to state + BitSequence] = {} # TMS sequence + self._seq = bytearray() + + @property + def fsm(self) -> JtagStateMachine: + """Return the state machine.""" + return self._fsm + + @property + def controller(self) -> 'JtagController': + """Return the JTAG controller.""" + return self._ctrl + + def reset(self) -> None: + """Reset the attached TAP controller.""" + self._ctrl.tap_reset() + self._fsm.reset() + + def get_available_statenames(self): + """Return a list of supported state names.""" + return [str(s) for s in self._fsm.states] + + def scan(self) -> BitSequence: + """Perform a JTAG scan, where all buffered TDI bits are sent and + incoming TDO bits are returned. + + :return: the received TDO bits. + """ + return self._ctrl.scan() + + def change_state(self, statename, read_tdo: bool = False) -> None: + """Advance the TAP controller to the defined state""" + transition = (self._fsm.state.name, statename) + if transition not in self._tr_cache: + # find the state machine path to move to the new instruction + path = self._fsm.find_path(statename) + self._log.debug('new path: %s', + ', '.join((str(s).upper() for s in path[1:]))) + # convert the path into an event sequence + events = self._fsm.get_events(path) + self._tr_cache[transition] = events + else: + # transition already in cache + events = self._tr_cache[transition] + # update the remote device tap controller (write TMS consumes the seq) + self._ctrl.write_tms(events.copy(), read_tdo) + self._log.debug('change state to %s', statename) + # update the current state machine's state + self._fsm.handle_events(events.copy()) + + def go_idle(self) -> None: + """Schedule the TAP controller to go to the IDLE state.""" + self.change_state('run_test_idle') + + def run(self) -> None: + """Schedule the TAP controller to go to the IDLE state.""" + self.change_state('run_test_idle') + + def capture_ir(self) -> None: + """Schedule the capture of the current instruction from the TAP + controller.""" + self.change_state('capture_ir') + + def write_ir(self, instruction: BitSequence) -> None: + """Schedule an instruction to be sent to the TAP controller.""" + self.change_state('shift_ir') + self._ctrl.write(instruction) + self.change_state('update_ir') + + def capture_dr(self) -> None: + """Schedule the current data register of the TAP controller to be + read.""" + self.change_state('capture_dr') + + def write_dr(self, data: BitSequence) -> None: + """Schedule data to be written to the TAP controller.""" + self.change_state('shift_dr') + self._ctrl.write(data) + self.change_state('update_dr') + + def read_dr(self, length: int) -> None: + """Schedule data register to be retrieved from the TAP controller.""" + self.change_state('shift_dr') + self._ctrl.read(length-1) + self.change_state('update_dr', True) + + def exchange_dr(self, data: BitSequence) -> None: + """Schedule data register content exchange.""" + self.change_state('shift_dr') + self._ctrl.exchange(data) + self.change_state('update_dr', True) diff --git a/python/qemu/jtagtools/jtag/error.py b/python/qemu/jtagtools/jtag/error.py new file mode 100644 index 0000000000000..dcf1732430974 --- /dev/null +++ b/python/qemu/jtagtools/jtag/error.py @@ -0,0 +1,14 @@ +# Copyright (c) 2024 Rivos Inc. Emmanuel Blot +# Copyright (c) 2010-2024, Emmanuel Blot +# All rights reserved. +# +# SPDX-License-Identifier: Apache2 + +"""JTAG Errors""" + +class JtagError(Exception): + """Generic JTAG error.""" + + +class JtagStateError(JtagError): + """JTAG state machine error.""" diff --git a/python/qemu/jtag/jtag.py b/python/qemu/jtagtools/jtag/machine.py similarity index 59% rename from python/qemu/jtag/jtag.py rename to python/qemu/jtagtools/jtag/machine.py index 769c00475425f..ac22eebd49e33 100644 --- a/python/qemu/jtag/jtag.py +++ b/python/qemu/jtagtools/jtag/machine.py @@ -2,21 +2,15 @@ # Copyright (c) 2016, Emmanuel Bouaziz # All rights reserved. # -# SPDX-License-Identifier: BSD-3-Clause +# SPDX-License-Identifier: Apache2 -"""JTAG tools. - - Based on JTAG support for FTDI from PyFtdi module -""" +"""JTAG state machine.""" from logging import getLogger from typing import Union -from .bits import BitSequence - - -class JtagError(Exception): - """Generic JTAG error.""" +from .error import JtagStateError +from ..bits import BitSequence class JtagState: @@ -170,7 +164,7 @@ def get_events(cls, path): if xstate == dstate: events.append(epos) if len(events) != len(path) - 1: - raise JtagError("Invalid path") + raise JtagStateError("Invalid path") return BitSequence(events) def handle_events(self, events: BitSequence) -> None: @@ -185,136 +179,3 @@ def handle_events(self, events: BitSequence) -> None: for event in events: self._current = self._current.getx(event) self._tr_cache[transit] = self._current - - -class JtagController: - """JTAG master API.""" - - INSTRUCTIONS = {'bypass': 0x0, 'idcode': 0x1} - """Common instruction register codes.""" - - def tap_reset(self, use_trst: bool = False) -> None: - """Reset the TAP controller. - - :param use_trst: use TRST HW wire if available - """ - raise NotImplementedError('ABC') - - def system_reset(self) -> None: - """Reset the device.""" - - def quit(self) -> None: - """Terminate session.""" - - def write_tms(self, modesel: BitSequence) -> None: - """Change the TAP controller state. - - :note: modesel content may be consumed, i.e. emptied - :note: last TMS bit should be stored and clocked on next write - request - - :param modesel: the bit sequence of TMS bits to clock in - """ - raise NotImplementedError('ABC') - - def write(self, out: BitSequence): - """Write a sequence of bits to TDI. - - :note: out content may be consumed, i.e. emptied - :param out: the bot sequence of TDI bits to clock in - """ - raise NotImplementedError('ABC') - - def read(self, length: int) -> BitSequence: - """Read out a sequence of bits from TDO. - - :param length: the number of bits to clock out from the remote device - :return: the received TDO bits (length-long) - """ - raise NotImplementedError('ABC') - - -class JtagEngine: - """High-level JTAG engine controller""" - - def __init__(self, ctrl: 'JtagController'): - self._ctrl = ctrl - self._log = getLogger('jtag.eng') - self._fsm = JtagStateMachine() - self._tr_cache: dict[tuple[str, # from state - str], # to state - BitSequence] = {} # TMS sequence - self._seq = bytearray() - - @property - def fsm(self) -> JtagStateMachine: - """Return the state machine.""" - return self._fsm - - @property - def controller(self) -> 'JtagController': - """Return the JTAG controller.""" - return self._ctrl - - def reset(self) -> None: - """Reset the attached TAP controller""" - self._ctrl.reset() - self._fsm.reset() - - def get_available_statenames(self): - """Return a list of supported state name""" - return [str(s) for s in self._fsm.states] - - def change_state(self, statename) -> None: - """Advance the TAP controller to the defined state""" - transition = (self._fsm.state, statename) - if transition not in self._tr_cache: - # find the state machine path to move to the new instruction - path = self._fsm.find_path(statename) - self._log.debug('new path: %s', - ', '.join((str(s).upper() for s in path[1:]))) - # convert the path into an event sequence - events = self._fsm.get_events(path) - self._tr_cache[transition] = events - else: - # transition already in cache - events = self._tr_cache[transition] - # update the remote device tap controller (write TMS consumes the seq) - self._ctrl.write_tms(events.copy()) - # update the current state machine's state - self._fsm.handle_events(events.copy()) - - def go_idle(self) -> None: - """Change the current TAP controller to the IDLE state""" - self.change_state('run_test_idle') - - def run(self) -> None: - """Change the current TAP controller to the IDLE state""" - self.change_state('run_test_idle') - - def capture_ir(self) -> None: - """Capture the current instruction from the TAP controller""" - self.change_state('capture_ir') - - def write_ir(self, instruction) -> None: - """Change the current instruction of the TAP controller""" - self.change_state('shift_ir') - self._ctrl.write(instruction) - self.change_state('update_ir') - - def capture_dr(self) -> None: - """Capture the current data register from the TAP controller""" - self.change_state('capture_dr') - - def write_dr(self, data) -> None: - """Change the data register of the TAP controller""" - self.change_state('shift_dr') - self._ctrl.write(data) - self.change_state('update_dr') - - def read_dr(self, length: int) -> BitSequence: - """Read the data register from the TAP controller""" - self.change_state('shift_dr') - data = self._ctrl.read(length) - self.change_state('update_dr') - return data diff --git a/python/qemu/jtagtools/rbb/__init__.py b/python/qemu/jtagtools/rbb/__init__.py new file mode 100644 index 0000000000000..e85ca18fb4113 --- /dev/null +++ b/python/qemu/jtagtools/rbb/__init__.py @@ -0,0 +1,8 @@ +# Copyright (c) 2024 Rivos Inc. Emmanuel Blot +# All rights reserved. +# +# SPDX-License-Identifier: BSD-3-Clause + +"""JTAG Remote BitBang Controller""" + +from .bitbang import JtagBitbangController diff --git a/python/qemu/jtag/bitbang.py b/python/qemu/jtagtools/rbb/bitbang.py similarity index 97% rename from python/qemu/jtag/bitbang.py rename to python/qemu/jtagtools/rbb/bitbang.py index 5526b4a369ed3..e1134d6fa87c5 100644 --- a/python/qemu/jtag/bitbang.py +++ b/python/qemu/jtagtools/rbb/bitbang.py @@ -1,10 +1,8 @@ -# Copyright (c) 2024 Rivos, Inc. +# Copyright (c) 2024 Rivos Inc. Emmanuel Blot # SPDX-License-Identifier: Apache2 """JTAG controller for OpenOCD/QEMU bitbang protocol. - :author: Emmanuel Blot - Protocol abstract: TCLK TMS TDI @@ -32,8 +30,8 @@ from time import time as now from typing import Optional -from .bits import BitSequence -from .jtag import JtagController +from ..bits import BitSequence +from ..jtag.controller import JtagController class JtagBitbangController(JtagController): diff --git a/python/qemu/ot/dm/dm.py b/python/qemu/ot/dm/dm.py index 6483d906fa7c8..64de099b16526 100644 --- a/python/qemu/ot/dm/dm.py +++ b/python/qemu/ot/dm/dm.py @@ -190,6 +190,7 @@ def decode(cls, name: str, value: int) -> dict[str, Any]: def initialize(self) -> None: """Initialize the debug module.""" + self._log.info('Initialize') btf = self.BITFIELDS['DMCONTROL'] self.dmcontrol = 0 enable = btf.encode(dmactive=True) @@ -201,7 +202,7 @@ def initialize(self) -> None: select = btf.encode(dmactive=True, hasel=False, hartsello=self._hart) self.dmcontrol = select dmcontrol = btf.decode(self.dmcontrol) - assert dmcontrol['dmactive'] + assert dmcontrol['dmactive'], "Debug Module not active" btf = self.BITFIELDS['DMSTATUS'] version = btf.decode(self.dmstatus)['version'] if version == self.VERSION['v0.11']: diff --git a/python/qemu/ot/dtm/dtm.py b/python/qemu/ot/dtm/dtm.py index 23b48642452b1..d10961e282409 100644 --- a/python/qemu/ot/dtm/dtm.py +++ b/python/qemu/ot/dtm/dtm.py @@ -11,8 +11,8 @@ from sys import modules from typing import Optional -from jtag.bits import BitSequence -from jtag.jtag import JtagEngine +from jtagtools.bits import BitSequence +from jtagtools.jtag import JtagEngine class DMIError(RuntimeError): @@ -40,7 +40,8 @@ def __init__(self, dtm: 'DebugTransportModule'): name = self.__class__.__name__ if (not hasattr(self.__class__, 'ADDRESS') or not isinstance(getattr(self, 'ADDRESS'), int)): - raise NotImplementedError(f'{name} address not defined') + raise NotImplementedError(f'Unknown/invalid register address for ' + f'{self.__class__.__name__}') self._dtm = dtm self._log = getLogger(f'dtm.{name.lower()}') @@ -168,12 +169,12 @@ def length(self) -> int: def write(self, address: int, value: int) -> None: """Write a 32-bit value to the specified address """ + self._log.debug('dmi 0x%x 0x%08x', address, value) dmi = self._build_dmi(address) value &= 0xffff_ffff dmi |= value << 2 dmi |= self.OPS['write'] wbseq = BitSequence(dmi, self.length) - self._log.debug('write: 0x%08x', value) self._write(wbseq) rbseq = self._read(self.length) res = int(rbseq) & 0b11 @@ -260,14 +261,15 @@ def read(self, address: int, length: int) -> BitSequence: """Read a bit sequence value.""" self._log.debug("read addr: 0x%x len: %d", address, length) self._engine.write_ir(BitSequence(address, self._ir_length)) - return self._engine.read_dr(length) + self._engine.read_dr(length) + return self._engine.scan() def write(self, address: int, bseq: BitSequence) -> None: """Write a bit sequence value.""" self._log.debug("write addr: 0x%x len: %d", address, len(bseq)) self._engine.write_ir(BitSequence(address, self._ir_length)) self._engine.write_dr(bseq) - self._engine.run() + self._engine.scan() def read_word(self, address: int) -> int: """Read a 32-bit value.""" diff --git a/scripts/opentitan/dtm.py b/scripts/opentitan/dtm.py index 8cc5aa16216c7..bedd9e09f3d1d 100755 --- a/scripts/opentitan/dtm.py +++ b/scripts/opentitan/dtm.py @@ -21,9 +21,9 @@ 'python', 'qemu') sys.path.append(QEMU_PYPATH) -from jtag.bitbang import JtagBitbangController -from jtag.bits import BitSequence -from jtag.jtag import JtagEngine +from jtagtools.rbb import JtagBitbangController +from jtagtools.bits import BitSequence +from jtagtools.jtag import JtagEngine from ot.dm import DebugModule from ot.dtm import DebugTransportModule @@ -42,8 +42,9 @@ def idcode(engine: JtagEngine, ir_length: int) -> None: """Retrieve ID code.""" code = JtagBitbangController.INSTRUCTIONS['idcode'] engine.write_ir(BitSequence(code, ir_length)) - value = engine.read_dr(32) + engine.read_dr(32) engine.go_idle() + value = engine.scan() return int(value) diff --git a/scripts/opentitan/otpdm.py b/scripts/opentitan/otpdm.py index 1703f63ad7809..ac9fc25d7d611 100755 --- a/scripts/opentitan/otpdm.py +++ b/scripts/opentitan/otpdm.py @@ -22,8 +22,8 @@ 'python', 'qemu') sys.path.append(QEMU_PYPATH) -from jtag.bitbang import JtagBitbangController -from jtag.jtag import JtagEngine +from jtagtools.rbb import JtagBitbangController +from jtagtools.jtag.engine import JtagEngine from ot.dm import DebugModule from ot.dm.otp import OTPController From 5636562eeb34db29cfca405f07a018381d826448 Mon Sep 17 00:00:00 2001 From: Emmanuel Blot Date: Fri, 22 Nov 2024 19:44:53 +0100 Subject: [PATCH 15/19] [ot] python/qemu: bitbang.py: update the new write_tms API from jtagtools. Signed-off-by: Emmanuel Blot --- python/qemu/jtagtools/rbb/bitbang.py | 103 ++++++++++++++++----------- 1 file changed, 60 insertions(+), 43 deletions(-) diff --git a/python/qemu/jtagtools/rbb/bitbang.py b/python/qemu/jtagtools/rbb/bitbang.py index e1134d6fa87c5..d918355f26c37 100644 --- a/python/qemu/jtagtools/rbb/bitbang.py +++ b/python/qemu/jtagtools/rbb/bitbang.py @@ -44,15 +44,15 @@ class JtagBitbangController(JtagController): DEFAULT_PORT = 3335 """Default TCP port.""" - RECV_TIMEOUT = 0.25 + RECV_TIMEOUT = 2.0 """Maximum allowed time in seconds to receive a response from the JTAG controller. """ - READ = b'R' + READ = ord('R') """JTAG bitbang code to receive data from TDO.""" - QUIT = b'Q' + QUIT = ord('Q') """JTAG bitbang code to quit.""" def __init__(self, sock: socket, link_log: bool = False): @@ -61,6 +61,7 @@ def __init__(self, sock: socket, link_log: bool = False): self._link_log = link_log self._last: Optional[bool] = None # Last deferred TDO bit self._outbuf = bytearray() + self._inlen = 0 self._tck = False self._tms = False self._tdi = False @@ -76,6 +77,7 @@ def tap_reset(self, use_trst: bool = False) -> None: self._write(self._reset_code(self._trst, self._srst)) else: self.write_tms(BitSequence('11111')) + self.scan() def system_reset(self) -> None: self._log.info('System reset') @@ -84,12 +86,40 @@ def system_reset(self) -> None: self._write(self._reset_code(self._trst, self._srst)) self._srst = not self._srst self._write(self._reset_code(self._trst, self._srst)) + self.scan() def quit(self) -> None: self._log.info('Quit') - self._sock.send(self.QUIT) + self._outbuf.append(self.QUIT) + self.scan() + + def scan(self) -> BitSequence: + if self._outbuf: + if self._link_log: + self._log.debug('write %d bytes to TDI', len(self._outbuf)) + self._sock.send(self._outbuf) + self._outbuf.clear() + bseq = BitSequence() + length = self._inlen + self._inlen = 0 + if length: + timeout = now() + self.RECV_TIMEOUT + while length: + try: + data = self._sock.recv(length) + except TimeoutError as exc: + if now() < timeout: + continue + raise TimeoutError('No response from RBB server') from exc + length -= len(data) + bseq.push_right(data) + timeout = now() + self.RECV_TIMEOUT / 2 + bseq.reverse() + if self._link_log: + self._log.debug('read TDO [%d] %s', len(bseq), bseq) + return bseq - def write_tms(self, modesel: BitSequence) -> None: + def write_tms(self, modesel: BitSequence, read_tdo: bool = False) -> None: if not isinstance(modesel, BitSequence): raise ValueError('Expect a BitSequence') # apply the last TDO bit @@ -102,77 +132,64 @@ def write_tms(self, modesel: BitSequence) -> None: self._log.debug('write TMS [%d] %s', len(modesel), modesel) tck = self._tck tdi = self._tdi + tms = self._tms + read = self.READ code = self._bus_code - stream = bytearray() while modesel: tms = modesel.pop_left_bit() - stream.append(code(tck, tms, tdi)) + self._outbuf.append(code(tck, tms, tdi)) tck = not tck - stream.append(code(tck, tms, tdi)) + self._outbuf.append(code(tck, tms, tdi)) tck = not tck - self._sock.send(stream) + if read_tdo: + self._outbuf.append(read) + self._inlen += 1 + read_tdo = False self._tms = tms self._tck = tck - def write(self, out: BitSequence): + def write(self, out: BitSequence) -> None: if not isinstance(out, BitSequence): raise ValueError('out is not a BitSequence') if self._last is not None: # TODO: check if this case needs to be handled raise NotImplementedError('Last is lost') self._last = out.pop_left_bit() - if self._link_log: - self._log.debug('write TDI [%d] %s', len(out), out) tms = self._tms tck = self._tck + tdi = self._tdi code = self._bus_code - stream = bytearray() while out: tdi = out.pop_right_bit() - stream.append(code(tck, tms, tdi)) + self._outbuf.append(code(tck, tms, tdi)) tck = not tck - stream.append(code(tck, tms, tdi)) + self._outbuf.append(code(tck, tms, tdi)) tck = not tck - self._sock.send(stream) self._tdi = tdi self._tms = tms self._tck = tck - def read(self, length: int) -> BitSequence: + def read(self, length: int) -> None: if length == 0: raise ValueError() - bseq = BitSequence() - rem = length - timeout = now() + self.RECV_TIMEOUT if self._link_log: - self._log.debug('read %d bits, TMS: %d', length, self._tms) + self._log.debug('read %d TDO bits, TMS: %d', length, self._tms) tms = self._tms tck = self._tck tdi = self._tdi - read = ord(self.READ) + read = self.READ code = self._bus_code - stream = bytearray() - for _ in range(length): - stream.append(code(tck, tms, tdi)) + self._inlen += length + while length: + length -= 1 + self._outbuf.append(code(tck, tms, tdi)) tck = not tck - stream.append(code(tck, tms, tdi)) + self._outbuf.append(code(tck, tms, tdi)) tck = not tck - stream.append(read) - self._sock.send(stream) - while rem: - try: - data = self._sock.recv(length) - except TimeoutError: - if now() < timeout: - continue - raise - rem -= len(data) - bseq.push_right(data) - timeout = now() + self.RECV_TIMEOUT - bseq.reverse() - if self._link_log: - self._log.debug('read TDI [%d] %s', len(bseq), bseq) - return bseq + self._outbuf.append(read) + + def exchange(self, out: BitSequence) -> BitSequence: + raise NotImplementedError('RBB exchange support not implemented yet') @classmethod def _bus_code(cls, tclk: bool, tms: bool, tdi: bool) -> int: @@ -186,4 +203,4 @@ def _write(self, code: int): if self._link_log: self._log.debug('_write 0x%02x %s (%s)', code, f'{code-0x30:03b}', chr(code)) - self._sock.send(bytes([code])) + self._outbuf.append(code) From 2087a4117d5fa38fdcf318538e2ea29faba999df Mon Sep 17 00:00:00 2001 From: Emmanuel Blot Date: Fri, 22 Nov 2024 20:31:02 +0100 Subject: [PATCH 16/19] [ot] scripts/opentitan: dtm: add support for PyFtdi. It is now possible to use a PyFtdi URL as the socket specifier, e.g. ftdi://ftdi:2232/1 for a Digilent Arty board Signed-off-by: Emmanuel Blot --- docs/opentitan/dtm.md | 14 ++++++--- scripts/opentitan/dtm.py | 68 +++++++++++++++++++++++++++------------- 2 files changed, 55 insertions(+), 27 deletions(-) diff --git a/docs/opentitan/dtm.md b/docs/opentitan/dtm.md index 0499d5789d477..fd4e54f7974ff 100644 --- a/docs/opentitan/dtm.md +++ b/docs/opentitan/dtm.md @@ -80,10 +80,15 @@ Extras: operation, `--file` argument is mandatory. The content of the binary file is copied into the memory, starting at the `--address`. See also the `--elf` option for uploading applications. -* `-S` specify the socket info of the JTAG server in the QEMU VM. For TCP, this should follow the - setting of the `-chardev socket,id=taprbb,host=,port=,...` option for invoking QEMU. - For unix sockets, host/port are replaced with the unix socket path specified instead: - `-chardev socket,id=taprbb,path=`. +* `-S` specify the socket info to connect to the remote device. + * for QEMU VM JTAG server, TCP and Unix sockets are supported, _i.e._ `tcp:host:port` or + `unix:path/to/socket`. + * For TCP, this should follow the setting of the + `-chardev socket,id=taprbb,host=,port=,...` option for invoking QEMU. + * for Unix sockets, host/port are replaced with the unix socket path specified instead: + `-chardev socket,id=taprbb,path=`. + * for FTDI *232H USB-JTAG interfaces, PyFtdi v0.59+ Python module should be installed, and + pyftdi URL should be used to specify the FTDI device: _e.g._ `ftdi://ftdi:2232/1` * `-s` specify the number of bytes to read from or write to memory. Useful with the `--mem` option. See also the `--address` option. This option may be omitted for the `write` memory operation, in @@ -134,4 +139,3 @@ option: ./scripts/opentitan/dtm.py -m write -a 0x1000_0000 -f file.dat -X ```` - diff --git a/scripts/opentitan/dtm.py b/scripts/opentitan/dtm.py index bedd9e09f3d1d..5f247807eb55c 100755 --- a/scripts/opentitan/dtm.py +++ b/scripts/opentitan/dtm.py @@ -31,6 +31,16 @@ from ot.util.log import configure_loggers from ot.util.misc import HexInt, dump_buffer +try: + _FTDI_ERR = None + PYFTDI_MIN_VER = (0, 59) + from pyftdi import __version__ as PyFtdiVersion + from pyftdi.jtag import JtagFtdiController +except ImportError as _exc: + _FTDI_ERR = str(_exc).split(':')[-1] + JtagFtdiController = None + + DEFAULT_IR_LENGTH = 5 """Default TAP Instruction Register length.""" @@ -51,7 +61,7 @@ def idcode(engine: JtagEngine, ir_length: int) -> None: def main(): """Entry point.""" debug = True - default_host = 'localhost' + default_socket = f'tcp:localhost:{JtagBitbangController.DEFAULT_PORT}' default_port = JtagBitbangController.DEFAULT_PORT try: args: Optional[Namespace] = None @@ -59,9 +69,8 @@ def main(): description=sys.modules[__name__].__doc__.split('.')[0]) qvm = argparser.add_argument_group(title='Virtual machine') - qvm.add_argument('-S', '--socket', - help=f'unix:path/to/socket or tcp:host:port ' - f'(default tcp:{default_host}:{default_port})') + qvm.add_argument('-S', '--socket', default=default_socket, + help=f'connection (default {default_socket})') qvm.add_argument('-t', '--terminate', action='store_true', help='terminate QEMU when done') dmi = argparser.add_argument_group(title='DMI') @@ -109,29 +118,44 @@ def main(): args = argparser.parse_args() debug = args.debug - configure_loggers(args.verbose, 'dtm.rvdm', -1, 'dtm', 'jtag') - + main_loggers = ['dtm'] try: - if args.socket: - socket_type, socket_args = args.socket.split(":", 1) - if socket_type == "tcp": - host, port = socket_args.split(":") - sock = create_connection((host, int(port)), timeout=0.5) - elif socket_type == "unix": - sock = socket(AF_UNIX, SOCK_STREAM) - sock.connect(socket_args) - else: - raise ValueError(f"Invalid socket type {socket_type}") + socket_type, socket_args = args.socket.split(':', 1) + if socket_type == 'tcp': + host, port = socket_args.split(':') + sock = create_connection((host, int(port)), timeout=0.5) + elif socket_type == 'unix': + sock = socket(AF_UNIX, SOCK_STREAM) + sock.connect(socket_args) + elif socket_type == 'ftdi': + if not JtagFtdiController: + argparser.error(f'Cannot use PyFdti; {_FTDI_ERR}') + pyver = tuple(map(int, PyFtdiVersion.split('.')[:2])) + if pyver < PYFTDI_MIN_VER: + argparser.error(f'PyFtdi version {PYFTDI_MIN_VER[0]}.' + f'{PYFTDI_MIN_VER[1]}+ required') + sock = None + main_loggers.append('pyftdi.jtag') else: - sock = create_connection((default_host, default_port), - timeout=0.5) + raise ValueError(f'Invalid socket type {socket_type}') except Exception as exc: raise RuntimeError(f'Cannot connect to {args.socket}: ' f'{exc}') from exc - sock.settimeout(0.1) - ctrl = JtagBitbangController(sock) + + configure_loggers(args.verbose, *main_loggers, -1, 'jtag', + funcname=True, name_width=20) + + if sock: + sock.settimeout(0.1) + ctrl = JtagBitbangController(sock) + trst = True + else: + ctrl = JtagFtdiController() + ctrl.configure(args.socket, frequency=4000000, direction=0x60eb, + initial=0x00e8) + trst = False eng = JtagEngine(ctrl) - ctrl.tap_reset(True) + ctrl.tap_reset(trst) ir_length = args.ir_length dtm = DebugTransportModule(eng, ir_length) rvdm = None @@ -174,7 +198,7 @@ def main(): f'0x{args.csr_check:08x}') else: pad = ' ' * (10 - len(args.csr)) - print(f'{args.csr}:{pad}0x{csr_val:08x}') + print(f'{args.csr}:{pad}0x{csr_val:08x} (0b{csr_val:032b})') if args.mem: if args.address is None: argparser.error('no address specified for memory operation') From 470fcbaad77347c7a64b51e40fd429780e393e87 Mon Sep 17 00:00:00 2001 From: Emmanuel Blot Date: Thu, 21 Nov 2024 19:08:01 +0100 Subject: [PATCH 17/19] [ot] scripts/opentitan: dtm.py add support for custom DTMCS and DMI register. This is required for the IbexDemo project Signed-off-by: Emmanuel Blot --- scripts/opentitan/dtm.py | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/scripts/opentitan/dtm.py b/scripts/opentitan/dtm.py index 5f247807eb55c..294cf5f747a3e 100755 --- a/scripts/opentitan/dtm.py +++ b/scripts/opentitan/dtm.py @@ -27,6 +27,7 @@ from ot.dm import DebugModule from ot.dtm import DebugTransportModule +from ot.dtm.dtm import DMI, DTMCS from ot.util.elf import ElfBlob from ot.util.log import configure_loggers from ot.util.misc import HexInt, dump_buffer @@ -37,7 +38,7 @@ from pyftdi import __version__ as PyFtdiVersion from pyftdi.jtag import JtagFtdiController except ImportError as _exc: - _FTDI_ERR = str(_exc).split(':')[-1] + _FTDI_ERR = str(_exc).split(':', 1)[-1] JtagFtdiController = None @@ -48,9 +49,8 @@ """Default DMI address of the DM.""" -def idcode(engine: JtagEngine, ir_length: int) -> None: +def idcode(engine: JtagEngine, code: int, ir_length: int) -> int: """Retrieve ID code.""" - code = JtagBitbangController.INSTRUCTIONS['idcode'] engine.write_ir(BitSequence(code, ir_length)) engine.read_dr(32) engine.go_idle() @@ -62,7 +62,7 @@ def main(): """Entry point.""" debug = True default_socket = f'tcp:localhost:{JtagBitbangController.DEFAULT_PORT}' - default_port = JtagBitbangController.DEFAULT_PORT + default_idcode = JtagBitbangController.INSTRUCTIONS['idcode'] try: args: Optional[Namespace] = None argparser = ArgumentParser( @@ -78,6 +78,14 @@ def main(): default=DEFAULT_IR_LENGTH, help=f'bit length of the IR register ' f'(default: {DEFAULT_IR_LENGTH})') + dmi.add_argument('--idcode', type=int, default=default_idcode, + help=f'define the ID code (default: {default_idcode})') + dmi.add_argument('--dtmcs', type=HexInt.parse, + help=f'define an alternative DTMCS register ' + f'(default: 0x{DTMCS.ADDRESS:x})') + dmi.add_argument('--dmi', type=HexInt.parse, + help=f'define an alternative DMI register ' + f'(default: 0x{DMI.ADDRESS:x})') dmi.add_argument('-b', '--base', type=HexInt.parse, default=DEFAULT_DMI_ADDRESS, help=f'define DMI base address ' @@ -157,11 +165,15 @@ def main(): eng = JtagEngine(ctrl) ctrl.tap_reset(trst) ir_length = args.ir_length + if args.dtmcs: + DTMCS.ADDRESS = args.dtmcs + if args.dmi: + DMI.ADDRESS = args.dmi dtm = DebugTransportModule(eng, ir_length) rvdm = None try: if args.info: - code = idcode(eng, ir_length) + code = idcode(eng, args.idcode, ir_length) print(f'IDCODE: 0x{code:x}') version = dtm['dtmcs'].dmi_version abits = dtm['dtmcs'].abits From b23df94500df6e761751f12d50836964b26be84f Mon Sep 17 00:00:00 2001 From: Emmanuel Blot Date: Mon, 25 Nov 2024 18:19:59 +0100 Subject: [PATCH 18/19] [ot] hw/jtag: tap_ctrl_rbb: ensure socket is configured with nodelay Signed-off-by: Emmanuel Blot --- hw/jtag/tap_ctrl_rbb.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/hw/jtag/tap_ctrl_rbb.c b/hw/jtag/tap_ctrl_rbb.c index 2cd1d72c1e8ce..342dce5cd5405 100644 --- a/hw/jtag/tap_ctrl_rbb.c +++ b/hw/jtag/tap_ctrl_rbb.c @@ -35,6 +35,7 @@ #include "qapi/error.h" #include "qom/object.h" #include "chardev/char-fe.h" +#include "chardev/char-socket.h" #include "chardev/char.h" #include "hw/jtag/tap_ctrl.h" #include "hw/jtag/tap_ctrl_rbb.h" @@ -511,6 +512,12 @@ static void tap_ctrl_rbb_chr_event_hander(void *opaque, QEMUChrEvent event) return; } + Object *sock; + sock = object_dynamic_cast(OBJECT(tap->chr.chr), TYPE_CHARDEV_SOCKET); + if (sock) { + qio_channel_set_delay(SOCKET_CHARDEV(sock)->ioc, false); + } + tap_ctrl_rbb_tap_reset(tap); } } From 131c32026813920ec8857435a02e268938d8b850 Mon Sep 17 00:00:00 2001 From: Emmanuel Blot Date: Mon, 25 Nov 2024 18:20:46 +0100 Subject: [PATCH 19/19] [ot] python/qemu: jtagtools.rbb: ensure TCP socket is configured with nodelay Signed-off-by: Emmanuel Blot --- python/qemu/jtagtools/rbb/bitbang.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/python/qemu/jtagtools/rbb/bitbang.py b/python/qemu/jtagtools/rbb/bitbang.py index d918355f26c37..48d74af014e4c 100644 --- a/python/qemu/jtagtools/rbb/bitbang.py +++ b/python/qemu/jtagtools/rbb/bitbang.py @@ -26,7 +26,7 @@ """ from logging import getLogger -from socket import socket +from socket import socket, SOCK_STREAM, IPPROTO_TCP, TCP_NODELAY from time import time as now from typing import Optional @@ -67,6 +67,9 @@ def __init__(self, sock: socket, link_log: bool = False): self._tdi = False self._trst = False self._srst = False + if self._sock.proto == IPPROTO_TCP and self._sock.type == SOCK_STREAM: + self._sock.setsockopt(IPPROTO_TCP, TCP_NODELAY, 1) + self._sock.settimeout(self.RECV_TIMEOUT) def tap_reset(self, use_trst: bool = False) -> None: self._log.info('TAP reset (%s)', 'TRST' if use_trst else 'SW')