From 7a76118a8572564c265881da08891199f2cee114 Mon Sep 17 00:00:00 2001 From: Jon Olson Date: Sat, 26 Sep 2026 17:44:35 -0700 Subject: [PATCH 1/2] Honor caller deadlines in Cortex-M control. Cortex-M operations imposed a five-second limit even when their caller allowed more time. Pass the supplied context through acquisition, run control, register access, and release so the caller chooses the operation deadline. Keep the independent five-second restoration attempt after failed acquisition. Cancellation must not prevent that cleanup, and a retained target still permits the caller to retry release with its own context. --- docs/cortexm.md | 9 ++-- target/cortexm/context_test.go | 71 ++++++++++++++++++++++++++++++++ target/cortexm/control.go | 39 ++++++++---------- target/cortexm/register.go | 12 +++--- target/cortexm/register_write.go | 4 +- target/cortexm/run.go | 6 --- 6 files changed, 100 insertions(+), 41 deletions(-) create mode 100644 target/cortexm/context_test.go diff --git a/docs/cortexm.md b/docs/cortexm.md index 02c920b..a36512a 100644 --- a/docs/cortexm.md +++ b/docs/cortexm.md @@ -23,7 +23,8 @@ resume it. `Halted` reads the current status without acquiring halt ownership. Release the target before its MEM-AP or Arm debug owner. `Release` restores the debug control changed by the target and leaves an inherited halt alone. -Each operation is bounded to five seconds or the caller's earlier deadline. +The caller controls operation cancellation and deadlines, including release. +Without either, an operation may wait indefinitely for the processor. Failed acquisition attempts cleanup with a fresh five-second context; a non-nil target returned with an error must be retained for release retries. Once release starts, or a control write fails, ordinary target calls stop. @@ -80,9 +81,9 @@ pc, err := core.ReadRegister(ctx, cortexm.PC) ``` Reads write DCRSR and replace DCRDR; these transfer registers are not restored. -The target waits for S_REGRDY before and after selecting a register, with the -same five-second bound as control operations. It does not require observing -S_REGRDY clear, since a transfer may finish before the first status read. +The target waits for S_REGRDY before and after selecting a register, using the +caller's context. It does not require observing S_REGRDY clear, since a transfer +may finish before the first status read. A failed transfer leaves only `Release` available. Release waits for any pending transfer, including one found busy before selection, before resuming diff --git a/target/cortexm/context_test.go b/target/cortexm/context_test.go new file mode 100644 index 0000000..3d5136e --- /dev/null +++ b/target/cortexm/context_test.go @@ -0,0 +1,71 @@ +package cortexm_test + +import ( + "context" + "testing" + "time" + + "github.com/jon/ostiole/target/cortexm" +) + +type deadlineMemory struct { + cortexm.Memory + t *testing.T + ctx context.Context +} + +func (m *deadlineMemory) check(ctx context.Context) { + m.t.Helper() + got, gotOK := ctx.Deadline() + want, wantOK := m.ctx.Deadline() + if gotOK != wantOK || !got.Equal(want) { + m.t.Errorf("memory deadline = %v, %v; want %v, %v", got, gotOK, want, wantOK) + } +} + +func (m *deadlineMemory) ReadWord(ctx context.Context, addr uint32) (uint32, error) { + m.check(ctx) + return m.Memory.ReadWord(ctx, addr) +} + +func (m *deadlineMemory) WriteWord(ctx context.Context, addr, value uint32) error { + m.check(ctx) + return m.Memory.WriteWord(ctx, addr, value) +} + +func TestOperationsPreserveCallerDeadline(t *testing.T) { + for _, timeout := range []time.Duration{0, time.Second, time.Minute} { + t.Run(timeout.String(), func(t *testing.T) { + ctx := t.Context() + if timeout != 0 { + var cancel context.CancelFunc + ctx, cancel = context.WithTimeout(ctx, timeout) + defer cancel() + } + m := &deadlineMemory{Memory: newRegisterMemory(), t: t, ctx: ctx} + core, err := cortexm.Acquire(ctx, m) + if err != nil { + t.Fatal(err) + } + operations := []struct { + name string + call func(context.Context) error + }{ + {"Halt", core.Halt}, + {"Halted", func(ctx context.Context) error { _, err := core.Halted(ctx); return err }}, + {"ReadRegister", func(ctx context.Context) error { _, err := core.ReadRegister(ctx, cortexm.R0); return err }}, + {"WriteRegister", func(ctx context.Context) error { return core.WriteRegister(ctx, cortexm.R0, 42) }}, + {"Resume", core.Resume}, + {"Release", core.Release}, + } + for _, op := range operations { + t.Run(op.name, func(t *testing.T) { + m.t = t + if err := op.call(ctx); err != nil { + t.Fatal(err) + } + }) + } + }) + } +} diff --git a/target/cortexm/control.go b/target/cortexm/control.go index 91fa1d6..644c807 100644 --- a/target/cortexm/control.go +++ b/target/cortexm/control.go @@ -8,14 +8,14 @@ import ( ) const ( - dhcsrAddress = uint32(0xe000edf0) - debugKey = uint32(0xa05f0000) - cDebugEnable = uint32(1) - cHalt = uint32(2) - cStep = uint32(4) - cMaskInts = uint32(8) - sHalt = uint32(1 << 17) - controlTimeout = 5 * time.Second + dhcsrAddress = uint32(0xe000edf0) + debugKey = uint32(0xa05f0000) + cDebugEnable = uint32(1) + cHalt = uint32(2) + cStep = uint32(4) + cMaskInts = uint32(8) + sHalt = uint32(1 << 17) + acquireCleanupTimeout = 5 * time.Second ) // Memory reads and writes aligned 32-bit target words. A successful write must @@ -29,7 +29,8 @@ type Memory interface { // Target owns Cortex-M0 halting debug state through borrowed memory. Do not copy // it. Calls and all access to the underlying memory must be serialized. Keep // exclusive control of the processor's debug registers until Release succeeds, -// then release the memory owner. The zero value is inactive. +// then release the memory owner. The caller controls operation cancellation +// and deadlines. The zero value is inactive. type Target struct { memory Memory identity Identity @@ -48,10 +49,10 @@ type Target struct { // and an unfinished halt transition before writing. DHCSR reads consume its // sticky reset and instruction-retirement indicators. // -// Calls are bounded to five seconds or the caller's earlier deadline. Failed -// setup attempts restoration with an independent five-second context. A non-nil -// target returned with an error retains cleanup obligations; only Release is -// then available. Memory remains borrowed on every return. +// The caller controls cancellation and deadlines. Failed setup attempts +// restoration with an independent five-second context. A non-nil target +// returned with an error retains cleanup obligations; only Release is then +// available. Memory remains borrowed on every return. func Acquire(ctx context.Context, memory Memory) (*Target, error) { if memory == nil { return nil, errors.New("cortexm: nil memory") @@ -59,8 +60,6 @@ func Acquire(ctx context.Context, memory Memory) (*Target, error) { if err := liveContext(ctx); err != nil { return nil, err } - ctx, cancel := context.WithTimeout(ctx, controlTimeout) - defer cancel() identity, err := Identify(ctx, memory) if err != nil { return nil, err @@ -81,7 +80,7 @@ func Acquire(ctx context.Context, memory Memory) (*Target, error) { } t.saved = 0 if err := t.writeControl(ctx, cDebugEnable); err != nil { - cleanup, cancel := context.WithTimeout(context.Background(), controlTimeout) + cleanup, cancel := context.WithTimeout(context.Background(), acquireCleanupTimeout) defer cancel() if releaseErr := t.Release(cleanup); releaseErr != nil { return t, errors.Join(err, releaseErr) @@ -115,9 +114,9 @@ func (t *Target) Identity() Identity { // Release restores the inherited debug control. A target initially halted // remains halted. Failed restoration is retryable and blocks ordinary calls. // Nil and released targets need no cleanup. Use a fresh context after operation -// cancellation; each attempt is capped at five seconds. Release requires usable -// memory and cannot repair a disconnected or invalidated memory client. It -// never repeats a completed resume. An unconfirmed control change, or a new halt while +// cancellation and choose its deadline. Release requires usable memory and +// cannot repair a disconnected or invalidated memory client. It never repeats +// a completed resume. An unconfirmed control change, or a new halt while // restoring disabled debug, can prevent cleanup until execution resumes. // Pending register transfers must settle first. Reset or loss of Debug state // during a transfer prevents automatic cleanup. @@ -129,8 +128,6 @@ func (t *Target) Release(ctx context.Context) error { if err := liveContext(ctx); err != nil { return err } - ctx, cancel := context.WithTimeout(ctx, controlTimeout) - defer cancel() if t.registerPending { if err := t.waitRegister(ctx); err != nil { return err diff --git a/target/cortexm/register.go b/target/cortexm/register.go index b099fa4..3ae20be 100644 --- a/target/cortexm/register.go +++ b/target/cortexm/register.go @@ -43,11 +43,11 @@ const ( ) // ReadRegister reads a register while halted, without acquiring halt ownership. -// It writes debug transfer registers and consumes DHCSR's sticky status. Calls -// are bounded to five seconds or the caller's earlier deadline. An uncertain -// transfer blocks ordinary calls; Release must settle it before changing debug -// control. Loss of Debug state or reset during a pending transfer prevents -// automatic cleanup. An error returns no valid register value. +// It writes debug transfer registers and consumes DHCSR's sticky status. The +// caller controls cancellation and deadlines. An uncertain transfer blocks +// ordinary calls; Release must settle it before changing debug control. Loss +// of Debug state or reset during a pending transfer prevents automatic cleanup. +// An error returns no valid register value. func (t *Target) ReadRegister(ctx context.Context, reg Register) (uint32, error) { if reg < R0 || reg > PSP { return 0, errors.New("cortexm: invalid register") @@ -55,8 +55,6 @@ func (t *Target) ReadRegister(ctx context.Context, reg Register) (uint32, error) if err := t.active(ctx); err != nil { return 0, err } - ctx, cancel := context.WithTimeout(ctx, controlTimeout) - defer cancel() if err := t.waitRegister(ctx); err != nil { return 0, err } diff --git a/target/cortexm/register_write.go b/target/cortexm/register_write.go index f6ca826..583bd53 100644 --- a/target/cortexm/register_write.go +++ b/target/cortexm/register_write.go @@ -10,7 +10,7 @@ import ( // requires bit zero clear and does not change Thumb state. Invalid identifiers // and values are rejected before traffic. Release does not undo register writes. // -// The transfer and cleanup bounds are those of ReadRegister. An error after +// The transfer and cleanup rules are those of ReadRegister. An error after // staging data leaves only Release available; the register may have changed // if selection was attempted. Release settles that transfer without replaying // it. Callers own the consequences when execution resumes. @@ -21,8 +21,6 @@ func (t *Target) WriteRegister(ctx context.Context, reg Register, value uint32) if err := t.active(ctx); err != nil { return err } - ctx, cancel := context.WithTimeout(ctx, controlTimeout) - defer cancel() if err := t.waitRegister(ctx); err != nil { return err } diff --git a/target/cortexm/run.go b/target/cortexm/run.go index 4ccb495..2bb62fb 100644 --- a/target/cortexm/run.go +++ b/target/cortexm/run.go @@ -15,8 +15,6 @@ func (t *Target) Halt(ctx context.Context) error { if err := t.active(ctx); err != nil { return err } - ctx, cancel := context.WithTimeout(ctx, controlTimeout) - defer cancel() value, err := t.readControl(ctx) if err != nil || value&sHalt != 0 { return err @@ -36,8 +34,6 @@ func (t *Target) Resume(ctx context.Context) error { if !t.haltOwned { return errors.New("cortexm: no owned halt to resume") } - ctx, cancel := context.WithTimeout(ctx, controlTimeout) - defer cancel() if err := t.resume(ctx); err != nil { return err } @@ -50,8 +46,6 @@ func (t *Target) Halted(ctx context.Context) (bool, error) { if err := t.active(ctx); err != nil { return false, err } - ctx, cancel := context.WithTimeout(ctx, controlTimeout) - defer cancel() value, err := t.readControl(ctx) return value&sHalt != 0, err } From 84c5fe38bd276f1751632b90998eac658560b8f0 Mon Sep 17 00:00:00 2001 From: Jon Olson Date: Sat, 26 Sep 2026 17:44:35 -0700 Subject: [PATCH 2/2] Let caller cancellation bound J-Link configuration retries. J-Link inspection stopped after one second or 101 attempts even when the open context allowed more time. Retry an unconfigured USB device until configuration appears or the caller's context ends, preserving both cancellation and the unconfigured-state error when waiting fails. Other inspection errors still return immediately, and retries retain their ten-millisecond spacing. --- docs/architecture.md | 7 +++-- docs/capabilities.md | 2 +- jlink/session.go | 20 ++++--------- jlink/session_test.go | 69 +++++++++++++++++++++++++++++++++++++++---- 4 files changed, 75 insertions(+), 23 deletions(-) diff --git a/docs/architecture.md b/docs/architecture.md index bd85e48..da98675 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -112,9 +112,10 @@ than a vendor wildcard. `jlink.Open` inspects the active descriptors, rejects missing or ambiguous application interfaces, selects the descriptor-chosen alternate, resolves its active bulk endpoints, and reads metadata. With no options it does not select a target interface. An immediate reopen may briefly -find the probe unconfigured; `jlink.Open` retries only that typed USB state for -at most one second. `jlink.WithSWD` selects SWD during open and requests a -whole-kHz clock no greater than the requested ceiling. +find the probe unconfigured; `jlink.Open` retries only that typed USB state +until configuration appears or the caller's context ends. Without cancellation +or a deadline, it may wait indefinitely. `jlink.WithSWD` selects SWD during open +and requests a whole-kHz clock no greater than the requested ceiling. `WithJTAG` does the same for the advertised JTAG interface. An open session can be explicitly reconfigured after releasing its existing protocol owner; `SWDIO` and `JTAGIO` reject calls for the wrong selected interface. J-Link diff --git a/docs/capabilities.md b/docs/capabilities.md index 816a971..8f437a4 100644 --- a/docs/capabilities.md +++ b/docs/capabilities.md @@ -132,7 +132,7 @@ wires its MPSSE port for debugging. | Capability | Implemented | Validation and boundary | | --- | --- | --- | | Exact discovery catalog | Yes | Reviewed SEGGER application PIDs only; CDC-only `0x0106`, CMSIS-DAP `0x1008`, vendor wildcards, and inferred neighboring products are excluded. | -| Application interface selection | Yes | Requires one unambiguous `ff/ff/ff` alternate setting with exactly one bulk IN and one bulk OUT endpoint. The descriptors select the interface, alternate, and endpoint addresses; after selection, the session resolves the active endpoint properties and uses the active bulk IN maximum packet size. An immediate reopen retries only `usb.ErrNotConfigured`, at 10 ms intervals for at most one second; other inspection errors return immediately. | +| Application interface selection | Yes | Requires one unambiguous `ff/ff/ff` alternate setting with exactly one bulk IN and one bulk OUT endpoint. The descriptors select the interface, alternate, and endpoint addresses; after selection, the session resolves the active endpoint properties and uses the active bulk IN maximum packet size. An immediate reopen retries only `usb.ErrNotConfigured`, at 10 ms intervals until configuration appears or the caller's context ends; other inspection errors return immediately. | | Firmware record | Yes | Retains the complete length-delimited record and exposes its first NUL-delimited field for display. | | Capabilities | Yes | Preserves the opaque short or long bitset. The long query is gated by short bit 31, and the common prefix must agree. | | Optional metadata | Yes | Capability-gated hardware version, workspace hint, available target interfaces, and current target interface. A selected interface outside the 0–31 range represented by the availability mask is rejected. | diff --git a/jlink/session.go b/jlink/session.go index 2fae302..80d4641 100644 --- a/jlink/session.go +++ b/jlink/session.go @@ -44,11 +44,7 @@ type usbBulkTransfer interface { type ownedUSBDevice struct{ *usb.Device } -const ( - configurationInspectionAttempts = 101 - configurationInspectionInterval = 10 * time.Millisecond - configurationInspectionTimeout = time.Second -) +const configurationInspectionInterval = 10 * time.Millisecond func (d ownedUSBDevice) claimInterface(number uint8) (usbClaim, error) { claim, err := d.ClaimInterface(number) @@ -86,7 +82,9 @@ type Session struct { // Open claims the J-Link application interface, reads probe metadata, applies // its options, and takes ownership of the device on success. With no options it // does not select or configure a target interface. After an error, the caller -// must still call device.Close; Open has already attempted cleanup. +// must still call device.Close; Open has already attempted cleanup. Inspection +// retries an unconfigured USB device until the caller cancels or its deadline +// expires. Without either, an unconfigured device can keep Open waiting. func Open(ctx context.Context, device *usb.Device, options ...Option) (*Session, error) { if device == nil { return nil, errors.New("jlink: nil USB device") @@ -187,14 +185,12 @@ func applyOptions(options []Option) (openConfig, error) { } func inspectApplication(ctx context.Context, device usbDevice) (applicationInterface, error) { - inspectionCtx, cancel := context.WithTimeout(ctx, configurationInspectionTimeout) - defer cancel() - return inspectApplicationWithWait(inspectionCtx, device, waitForConfigurationInspection) + return inspectApplicationWithWait(ctx, device, waitForConfigurationInspection) } func inspectApplicationWithWait(ctx context.Context, device usbDevice, wait func(context.Context) error) (applicationInterface, error) { var unavailable error - for attempt := range configurationInspectionAttempts { + for { configuration, err := device.ActiveConfiguration(ctx) if err == nil { return findApplicationInterface(configuration) @@ -206,14 +202,10 @@ func inspectApplicationWithWait(ctx context.Context, device usbDevice, wait func return applicationInterface{}, fmt.Errorf("jlink: inspect active USB configuration: %w", err) } unavailable = err - if attempt == configurationInspectionAttempts-1 { - return applicationInterface{}, fmt.Errorf("jlink: inspect active USB configuration: %w", err) - } if waitErr := wait(ctx); waitErr != nil { return applicationInterface{}, fmt.Errorf("jlink: inspect active USB configuration: %w", errors.Join(err, waitErr)) } } - panic("unreachable") } func waitForConfigurationInspection(ctx context.Context) error { diff --git a/jlink/session_test.go b/jlink/session_test.go index 86f8507..ba61b37 100644 --- a/jlink/session_test.go +++ b/jlink/session_test.go @@ -9,6 +9,7 @@ import ( "reflect" "strings" "testing" + "time" "github.com/jon/ostiole/usb" ) @@ -434,9 +435,9 @@ func TestInspectApplicationRetriesUnconfiguredState(t *testing.T) { } } -func TestInspectApplicationBoundsUnconfiguredRetries(t *testing.T) { +func TestInspectApplicationRetriesUntilConfigured(t *testing.T) { device := metadataPeer(t, nil) - device.configurationErrs = make([]error, configurationInspectionAttempts) + device.configurationErrs = make([]error, 150) for i := range device.configurationErrs { device.configurationErrs[i] = usb.ErrNotConfigured } @@ -445,10 +446,10 @@ func TestInspectApplicationBoundsUnconfiguredRetries(t *testing.T) { waits++ return nil }) - if !errors.Is(err, usb.ErrNotConfigured) { - t.Fatalf("inspectApplicationWithWait error = %v, want ErrNotConfigured", err) + if err != nil { + t.Fatal(err) } - if device.configurationN != configurationInspectionAttempts || waits != configurationInspectionAttempts-1 { + if device.configurationN != 151 || waits != 150 { t.Fatalf("inspections = %d waits %d", device.configurationN, waits) } } @@ -822,3 +823,61 @@ func metadataPeer(t *testing.T, operations []peerOperation) *peerUSBDevice { } return device } + +type deadlineUSBDevice struct { + *peerUSBDevice + ctx context.Context + t *testing.T +} + +func (d deadlineUSBDevice) ActiveConfiguration(ctx context.Context) (usb.Configuration, error) { + d.t.Helper() + got, gotOK := ctx.Deadline() + want, wantOK := d.ctx.Deadline() + if gotOK != wantOK || !got.Equal(want) { + d.t.Errorf("inspection deadline = %v, %v; want %v, %v", got, gotOK, want, wantOK) + } + return d.peerUSBDevice.ActiveConfiguration(ctx) +} + +func TestInspectApplicationPreservesCallerDeadline(t *testing.T) { + for _, timeout := range []time.Duration{0, 100 * time.Millisecond, time.Minute} { + t.Run(timeout.String(), func(t *testing.T) { + ctx := t.Context() + if timeout != 0 { + var cancel context.CancelFunc + ctx, cancel = context.WithTimeout(ctx, timeout) + defer cancel() + } + device := deadlineUSBDevice{peerUSBDevice: metadataPeer(t, nil), ctx: ctx, t: t} + if _, err := inspectApplication(ctx, device); err != nil { + t.Fatal(err) + } + }) + } +} + +func TestInspectApplicationCallerCancelsRetries(t *testing.T) { + ctx, cancel := context.WithCancel(t.Context()) + defer cancel() + device := metadataPeer(t, nil) + device.configurationErrs = make([]error, 150) + for i := range device.configurationErrs { + device.configurationErrs[i] = usb.ErrNotConfigured + } + waits := 0 + _, err := inspectApplicationWithWait(ctx, device, func(ctx context.Context) error { + waits++ + if waits == 150 { + cancel() + return waitForConfigurationInspection(ctx) + } + return nil + }) + if !errors.Is(err, context.Canceled) || !errors.Is(err, usb.ErrNotConfigured) { + t.Fatalf("inspection error = %v, want cancellation and unconfigured state", err) + } + if device.configurationN != 150 { + t.Fatalf("inspections = %d, want 150", device.configurationN) + } +}