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/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/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) + } +} 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 }