From 01a0b822d9cedf14c3f98a8dbf84b6040839bf8e Mon Sep 17 00:00:00 2001 From: Jon Olson Date: Sun, 18 Oct 2026 17:42:40 -0700 Subject: [PATCH] Settle pending SELECT before banked DP access. An acknowledged bank-changing SELECT remained provisional, but immediate banked access rejected it before the RDBUFF barrier could settle it. Wide-address release needed SELECT1 and could therefore remain stuck on every retry, retaining power and response-mode ownership. Settle that selection before banked access. If a WAIT makes the underlying SWD connection require repair, invalidate DAP framing too. Honor the explicit release context during re-entry and preserve independent recovery budgets inside failed operations. An out-of-range AP base at ASIZE 32 also needs a regression because truncating it could silently select another AP. --- dap/banked_test.go | 2 +- dap/cleanup_test.go | 58 +++++++-- dap/context_test.go | 44 +++++++ dap/doc.go | 13 +- dap/jtag_cleanup_test.go | 16 ++- dap/jtag_model_test.go | 3 + dap/lifecycle.go | 21 ++-- dap/select_completion_test.go | 220 ++++++++++++++++++++++++++++++++++ dap/state.go | 4 - dap/swddp.go | 12 +- dap/wait.go | 6 +- dap/wait_test.go | 30 ++--- docs/ports/dap.md | 11 ++ 13 files changed, 388 insertions(+), 52 deletions(-) create mode 100644 dap/select_completion_test.go diff --git a/dap/banked_test.go b/dap/banked_test.go index 759b772..0874f1a 100644 --- a/dap/banked_test.go +++ b/dap/banked_test.go @@ -29,7 +29,7 @@ func TestBankedDPAccessPreservesSELECTFields(t *testing.T) { if value != dlcr { t.Fatalf("DLCR = %#08x, want %#08x", value, dlcr) } - wantRequests := []swdsim.Request{dpWrite(0x08), dpRead(0x0c), dpRead(0x04)} + wantRequests := []swdsim.Request{dpRead(0x0c), dpWrite(0x08), dpRead(0x0c), dpRead(0x04)} if got := target.requests[beforeBankedRead : beforeBankedRead+len(wantRequests)]; !equalRequests(got, wantRequests) { t.Fatalf("banked DP requests = %#v, want %#v", got, wantRequests) } diff --git a/dap/cleanup_test.go b/dap/cleanup_test.go index f4c097c..e1b7adb 100644 --- a/dap/cleanup_test.go +++ b/dap/cleanup_test.go @@ -13,10 +13,10 @@ import ( ) type cleanupBudgetWire struct { - inner swd.Wire - fail bool - calls int - entryBudget time.Duration + inner swd.Wire + fail bool + calls int + entryDeadline time.Time } func (w *cleanupBudgetWire) SWDIO(ctx context.Context, direction, output []byte, bits int) ([]byte, error) { @@ -28,7 +28,7 @@ func (w *cleanupBudgetWire) SWDIO(ctx context.Context, direction, output []byte, if bits == 136 { deadline, ok := ctx.Deadline() if ok { - w.entryBudget = time.Until(deadline) + w.entryDeadline = deadline } } return w.inner.SWDIO(ctx, direction, output, bits) @@ -53,7 +53,7 @@ func TestConnectRejectsNilContextBeforeTraffic(t *testing.T) { } } -func TestRecoveryUsesConfiguredIndependentBudget(t *testing.T) { +func TestReleaseReentryUsesCallerBudget(t *testing.T) { wire := &cleanupBudgetWire{inner: swdsim.New(sim.New(0x2ba01477))} dp := dap.NewDebugPort(dap.SWDP(swd.New(wire)), dap.WithCleanupTimeout(3*time.Second)) if _, err := dp.Connect(t.Context()); err != nil { @@ -65,10 +65,52 @@ func TestRecoveryUsesConfiguredIndependentBudget(t *testing.T) { } ctx, cancel := context.WithCancel(t.Context()) cancel() + before := wire.calls + if err := dp.Release(ctx); !errors.Is(err, context.Canceled) || wire.calls != before { + t.Fatalf("canceled release = %v, wire calls = %d; want %d", err, wire.calls, before) + } + ctx, cancel = context.WithTimeout(t.Context(), 5*time.Second) + defer cancel() + deadline, _ := ctx.Deadline() if err := dp.Release(ctx); err != nil { t.Fatal(err) } - if wire.entryBudget < 2*time.Second || wire.entryBudget > 3*time.Second { - t.Fatalf("recovery entry budget = %v", wire.entryBudget) + if wire.entryDeadline != deadline { + t.Fatalf("release entry deadline = %v, want %v", wire.entryDeadline, deadline) + } +} + +type recoveryBudgetTarget struct { + *waitTarget + abortBudget time.Duration +} + +func (target *recoveryBudgetTarget) Write(ctx context.Context, req swdsim.Request, value uint32) error { + if req == dpWrite(0) && value == 1 { + deadline, _ := ctx.Deadline() + target.abortBudget = time.Until(deadline) + } + return target.waitTarget.Write(ctx, req, value) +} + +func TestRecoveryUsesConfiguredIndependentBudget(t *testing.T) { + target := &recoveryBudgetTarget{waitTarget: newWaitTarget()} + addAP(t, target.waitTarget, 0, 0x24770011) + dp := dap.NewDebugPort(dap.SWDP(swd.New(swdsim.New(target))), + dap.WithMaxWaits(1), dap.WithCleanupTimeout(3*time.Second)) + if _, err := dp.Connect(t.Context()); err != nil { + t.Fatal(err) + } + ctx, cancel := context.WithTimeout(t.Context(), time.Second) + defer cancel() + target.arm(apRead(12), 1) + if _, err := dp.ReadAPIDR(ctx, dap.NewAPSel(0)); !errors.Is(err, dap.ErrWait) { + t.Fatalf("AP WAIT = %v", err) + } + if target.abortBudget < 2*time.Second || target.abortBudget > 3*time.Second { + t.Fatalf("independent recovery budget = %v", target.abortBudget) + } + if err := dp.Release(t.Context()); err != nil { + t.Fatal(err) } } diff --git a/dap/context_test.go b/dap/context_test.go index e80b2c8..8cba8eb 100644 --- a/dap/context_test.go +++ b/dap/context_test.go @@ -2,9 +2,11 @@ package dap_test import ( "context" + "errors" "maps" "slices" "testing" + "time" "github.com/jon/ostiole/dap" "github.com/jon/ostiole/dap/sim" @@ -97,3 +99,45 @@ func nilContextOperations(dp *dap.DebugPort, mem *dap.MemAP, sel dap.APSel, link }, } } + +func TestReleasePreflightContextPreservesConnectedPort(t *testing.T) { + for _, link := range []string{"SWD", "JTAG"} { + for _, failure := range []string{"canceled", "expired"} { + t.Run(link+"/"+failure, func(t *testing.T) { + checkReleasePreflightContext(t, link, failure) + }) + } + } +} + +func checkReleasePreflightContext(t *testing.T, link, failure string) { + t.Helper() + wire := &cleanupBudgetWire{inner: swdsim.New(sim.New(0x2ba01477))} + port := dap.SWDP(swd.New(wire)) + calls := func() int { return wire.calls } + if link == "JTAG" { + _, jwire, chain := jtagModelChain(t, 4, 0) + port = dap.JTAGDP(chain, 0) + calls = func() int { return jwire.calls } + } + dp := dap.NewDebugPort(port) + if _, err := dp.Connect(t.Context()); err != nil { + t.Fatal(err) + } + ctx, cancel := context.WithCancel(t.Context()) + if failure == "expired" { + cancel() + ctx, cancel = context.WithDeadline(t.Context(), time.Now().Add(-time.Second)) + } + cancel() + before := calls() + if err := dp.Release(ctx); !errors.Is(err, ctx.Err()) || calls() != before { + t.Fatalf("Release = %v, wire calls = %d; want %d", err, calls(), before) + } + if _, err := dp.ReadDP(t.Context(), dap.CTRLSTAT); err != nil { + t.Fatalf("preflight context invalidated connected port: %v", err) + } + if err := dp.Release(t.Context()); err != nil { + t.Fatal(err) + } +} diff --git a/dap/doc.go b/dap/doc.go index 9a0afb5..6967700 100644 --- a/dap/doc.go +++ b/dap/doc.go @@ -63,11 +63,14 @@ // // A failed Connect attempts bounded cleanup before returning. If that cleanup // also fails, Release remains available but other debug-port and access-port -// operations fail until cleanup succeeds. A failed Release has the same -// cleanup-only behavior and may be retried. WithCleanupTimeout configures each -// independent recovery attempt: one second by default for SWD, thirty for JTAG. -// JTAG recovery revalidates the exact chain and reacquires the TAP before -// restoration. The debug port never closes or automatically reopens the probe. +// operations fail until cleanup succeeds. A failure after Release starts +// restoration has the same cleanup-only behavior and may be retried. Calling +// Release with an expired or canceled context sends no traffic and leaves a +// connected port usable. +// WithCleanupTimeout configures each independent recovery attempt: one second +// by default for SWD, thirty for JTAG. JTAG recovery revalidates the exact chain +// and reacquires the TAP before restoration. The debug port never closes or +// automatically reopens the probe. // // A DebugPort does not replay a request which returns FAULT. It reads // bank-zero CTRL/STAT when the register selection is known, clears the sticky diff --git a/dap/jtag_cleanup_test.go b/dap/jtag_cleanup_test.go index 4c33d6d..0a6c7d7 100644 --- a/dap/jtag_cleanup_test.go +++ b/dap/jtag_cleanup_test.go @@ -289,7 +289,7 @@ func TestJTAGReservedACKInvalidatesProtocol(t *testing.T) { } } -func TestJTAGRecoveryDeadlineCanBeRetriedIndependently(t *testing.T) { +func TestJTAGReleaseReentryHonorsCallerContext(t *testing.T) { _, wire, chain := jtagModelChain(t, 4, 0) dp := dap.NewDebugPort(dap.JTAGDP(chain, 0), dap.WithCleanupTimeout(time.Second)) if _, err := dp.Connect(t.Context()); err != nil { @@ -302,11 +302,21 @@ func TestJTAGRecoveryDeadlineCanBeRetriedIndependently(t *testing.T) { wire.expire = true ctx, cancel := context.WithCancel(t.Context()) cancel() + before := wire.calls + if err := dp.Release(ctx); !errors.Is(err, context.Canceled) || wire.calls != before { + t.Fatalf("canceled release = %v, wire calls = %d; want %d", err, wire.calls, before) + } + ctx, cancel = context.WithTimeout(t.Context(), 2*time.Second) + defer cancel() + deadline, _ := ctx.Deadline() if err := dp.Release(ctx); !errors.Is(err, context.DeadlineExceeded) { t.Fatalf("bounded revalidation: %v", err) } - if err := dp.Release(ctx); err != nil { - t.Fatalf("fresh recovery with canceled caller: %v", err) + if wire.deadline != deadline { + t.Fatalf("release scan deadline = %v, want %v", wire.deadline, deadline) + } + if err := dp.Release(t.Context()); err != nil { + t.Fatalf("fresh release attempt: %v", err) } } diff --git a/dap/jtag_model_test.go b/dap/jtag_model_test.go index 0c978b4..d20da53 100644 --- a/dap/jtag_model_test.go +++ b/dap/jtag_model_test.go @@ -4,6 +4,7 @@ import ( "context" "errors" "testing" + "time" "github.com/jon/ostiole/jtag" ) @@ -34,6 +35,7 @@ type jtagDPWire struct { fail bool failAfter bool expire bool + deadline time.Time } type jtagRequest struct { @@ -148,6 +150,7 @@ func (m *jtagDPModel) execute(r jtagRequest) uint32 { func (w *jtagDPWire) MaxTransferBits() int { return w.limit } func (w *jtagDPWire) JTAGIO(ctx context.Context, tms, tdi []byte, bits int) ([]byte, error) { + w.deadline, _ = ctx.Deadline() if err := ctx.Err(); err != nil { return nil, err } diff --git a/dap/lifecycle.go b/dap/lifecycle.go index e699afa..cbdfaff 100644 --- a/dap/lifecycle.go +++ b/dap/lifecycle.go @@ -161,8 +161,10 @@ func (dp *DebugPort) failConnect(cause error) error { // Release restores volatile debug-port state owned by this connection. // -// Release may be retried. Once an attempt starts, other DP and AP operations -// remain blocked until Release succeeds. +// Release may be retried. Each attempt, including protocol re-entry, uses ctx. +// Once an attempt starts, other DP and AP operations remain blocked until +// Release succeeds. Supply a fresh bounded context for cleanup retries. +// Recovery inside a failed transport operation retains its independent budget. func (dp *DebugPort) Release(ctx context.Context) error { if !dp.bound() { return nil @@ -173,23 +175,22 @@ func (dp *DebugPort) Release(ctx context.Context) error { if ctx == nil { return errors.New("dap: nil context") } + if err := ctx.Err(); err != nil { + return err + } dp.state.beginRepair() - releaseCtx := ctx if !dp.state.responseKnown() { - var cancel context.CancelFunc - releaseCtx, cancel = dp.cleanupContext() - defer cancel() - if err := dp.reenter(releaseCtx); err != nil { + if err := dp.reenter(ctx); err != nil { return fmt.Errorf("dap: restore protocol state for release: %w", err) } } - if err := dp.selectAPAddress(releaseCtx, 0); err != nil { + if err := dp.selectAPAddress(ctx, 0); err != nil { return fmt.Errorf("dap: clear selection while releasing debug port: %w", err) } - if err := dp.releasePower(releaseCtx); err != nil { + if err := dp.releasePower(ctx); err != nil { return err } - if err := dp.releaseLink(releaseCtx); err != nil { + if err := dp.releaseLink(ctx); err != nil { return err } dp.completeRelease() diff --git a/dap/select_completion_test.go b/dap/select_completion_test.go new file mode 100644 index 0000000..06a49ef --- /dev/null +++ b/dap/select_completion_test.go @@ -0,0 +1,220 @@ +package dap_test + +import ( + "context" + "errors" + "fmt" + "testing" + "time" + + "github.com/jon/ostiole/dap" + dapsim "github.com/jon/ostiole/dap/sim" + "github.com/jon/ostiole/swd" + swdsim "github.com/jon/ostiole/swd/sim" +) + +type cancelAfterSelectWire struct { + swd.Wire + target *waitTarget + cancel context.CancelFunc + calls int +} + +func (w *cancelAfterSelectWire) SWDIO(ctx context.Context, direction, output []byte, bits int) ([]byte, error) { + w.calls++ + input, err := w.Wire.SWDIO(ctx, direction, output, bits) + values := w.target.selectValues + if err == nil && w.cancel != nil && len(values) != 0 && values[len(values)-1]&15 == 5 { + w.cancel() + w.cancel = nil + } + return input, err +} + +func newWideSelectPort(t *testing.T, width uint32, options ...dap.Option) (*dap.DebugPort, *waitTarget, *cancelAfterSelectWire) { + t.Helper() + target := newWaitTarget() + target.Target = dapsim.New(0x4c013477) + if err := target.SetDPRegister(dap.DPIDR1, width); err != nil { + t.Fatal(err) + } + low, _ := dap.APAt(0) + high, _ := dap.APAt(uint64(1) << (width - 1)) + for sel, id := range map[dap.APSel]uint32{low: 0x34770008, high: 0x24770011} { + if err := target.AddAP(sel, id); err != nil { + t.Fatal(err) + } + } + if err := target.SetDebugWord(0, 0x13579bdf); err != nil { + t.Fatal(err) + } + wire := &cancelAfterSelectWire{Wire: swdsim.New(target), target: target} + dp := dap.NewDebugPort(dap.SWDP(swd.New(wire)), options...) + if _, err := dp.Connect(t.Context()); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { + ctx, cancel := context.WithTimeout(context.Background(), time.Second) + defer cancel() + if err := dp.Release(ctx); err != nil { + t.Error(err) + } + }) + return dp, target, wire +} + +func TestPendingSELECTBeforeBankedAccess(t *testing.T) { + for _, width := range []uint32{40, 48, 52} { + for _, access := range []struct { + reg dap.DPRegister + write bool + value uint32 + }{ + {reg: dap.CTRLSTAT, value: 0xf0000001}, + {reg: dap.CTRLSTAT, write: true, value: 0x50000001}, + {reg: dap.DLCR}, {reg: dap.DPIDR1, value: width}, + {reg: dap.SELECT1, write: true, value: 1}, + } { + t.Run(fmt.Sprintf("%d/%s/write=%t", width, access.reg, access.write), func(t *testing.T) { + dp, target, _ := newWideSelectPort(t, width) + if err := dp.WriteDP(t.Context(), dap.SELECT, 1); err != nil { + t.Fatal(err) + } + before := len(target.requests) + if access.write { + if err := dp.WriteDP(t.Context(), access.reg, access.value); err != nil { + t.Fatal(err) + } + } else if value, err := dp.ReadDP(t.Context(), access.reg); err != nil || value != access.value { + t.Fatalf("%s = %#x, %v; want %#x", access.reg, value, err, access.value) + } + if got := target.requests[before]; got != dpRead(12) { + t.Fatalf("first request = %+v, want RDBUFF", got) + } + }) + } + } +} + +func TestPendingSELECTBeforeWideAPAccess(t *testing.T) { + for _, width := range []uint32{40, 48, 52} { + t.Run(fmt.Sprint(width), func(t *testing.T) { + dp, target, _ := newWideSelectPort(t, width) + if err := dp.WriteDP(t.Context(), dap.SELECT, 1); err != nil { + t.Fatal(err) + } + before := len(target.requests) + high, _ := dap.APAt(uint64(1) << (width - 1)) + id, err := dp.ReadAPIDR(t.Context(), high) + if err != nil || id.Raw != 0x24770011 { + t.Fatalf("high AP IDR = %#x, %v", id.Raw, err) + } + if got := target.requests[before]; got != dpRead(12) { + t.Fatalf("first request = %+v, want RDBUFF", got) + } + }) + } +} + +func TestWideSELECTReleaseRetry(t *testing.T) { + for _, width := range []uint32{40, 48, 52} { + for _, failure := range []string{"none", "cancel", "WAIT"} { + t.Run(fmt.Sprintf("%d/%s", width, failure), func(t *testing.T) { + checkWideSELECTReleaseRetry(t, width, failure) + }) + } + } +} + +func checkWideSELECTReleaseRetry(t *testing.T, width uint32, failure string) { + t.Helper() + dp, target, wire := newWideSelectPort(t, width, dap.WithMaxWaits(1)) + high, _ := dap.APAt(uint64(1) << (width - 1)) + if _, err := dp.ReadAPIDR(t.Context(), high); err != nil { + t.Fatal(err) + } + if err := dp.WriteDP(t.Context(), dap.SELECT, 1); err != nil { + t.Fatal(err) + } + if failure != "none" { + ctx, cancel := context.WithCancel(t.Context()) + defer cancel() + want := dap.ErrWait + if failure == "cancel" { + wire.cancel = cancel + want = context.Canceled + } else { + target.arm(dpRead(12), 1) + } + if err := dp.Release(ctx); !errors.Is(err, want) { + t.Fatalf("first Release = %v, want %v", err, want) + } + } + ctx, cancel := context.WithCancel(t.Context()) + cancel() + calls := wire.calls + if err := dp.Release(ctx); !errors.Is(err, context.Canceled) || wire.calls != calls { + t.Fatalf("canceled Release = %v, wire calls = %d; want %d", err, wire.calls, calls) + } + before := len(target.requests) + selectionCount := len(target.selectValues) + if err := dp.Release(t.Context()); err != nil { + t.Fatal(err) + } + if failure == "cancel" { + for _, value := range target.selectValues[selectionCount:] { + if value&15 == 5 { + t.Fatal("replayed the acknowledged bank-five selection after cancellation") + } + } + } + if len(target.requests) == before { + t.Fatal("release retry sent no traffic") + } + assertWideSelectionReleased(t, target) + before = len(target.requests) + if err := dp.Release(t.Context()); err != nil || len(target.requests) != before { + t.Fatalf("completed Release = %v, requests = %d; want %d", err, len(target.requests), before) + } +} + +func assertWideSelectionReleased(t *testing.T, target *waitTarget) { + t.Helper() + id, err := target.Target.Read(t.Context(), dpRead(0)) + if err != nil || id != 0x4c013477 { + t.Fatalf("released bank-zero DPIDR = %#x, %v", id, err) + } + state, err := target.Target.Read(t.Context(), dpRead(4)) + if err != nil || state&0xf0000001 != 0 { + t.Fatalf("released CTRL/STAT = %#x, %v", state, err) + } + if _, err := target.Target.Read(t.Context(), apRead(0)); err != nil { + t.Fatal(err) + } + word, err := target.Target.Read(t.Context(), dpRead(12)) + if err != nil || word != 0x13579bdf { + t.Fatalf("debug word without replacing SELECT or SELECT1 = %#x, %v", word, err) + } +} + +func TestAPv2ASIZE32RejectsHighBaseBeforeTraffic(t *testing.T) { + for _, operation := range []string{"IDR", "raw", "MEM-AP"} { + t.Run(operation, func(t *testing.T) { + dp, target, _ := newWideSelectPort(t, 32) + outside, _ := dap.APAt(1 << 32) + before := len(target.requests) + var err error + switch operation { + case "IDR": + _, err = dp.ReadAPIDR(t.Context(), outside) + case "raw": + _, err = dp.ReadRawAP(t.Context(), outside.Address(0xdfc)) + case "MEM-AP": + _, err = dap.OpenMemAP(t.Context(), dp, outside) + } + if err == nil || len(target.requests) != before { + t.Fatalf("out-of-range access = %v, requests = %d; want rejection with %d requests", err, len(target.requests), before) + } + }) + } +} diff --git a/dap/state.go b/dap/state.go index 2b9532f..1944db6 100644 --- a/dap/state.go +++ b/dap/state.go @@ -82,10 +82,6 @@ func (s *debugPortState) resolveSELECTFromCTRLSTAT(ctrlStat uint32) { s.confirmSELECT() } -func (s *debugPortState) dpBankAmbiguous() bool { - return s.selectPending && (!s.priorSELECT.valid || s.priorSELECT.value&0x0f != s.selectDP.value&0x0f) -} - func (s *debugPortState) confirmResponse(state uint32) { if state&overrunDetect == 0 { s.response = responseSimple diff --git a/dap/swddp.go b/dap/swddp.go index 42d1dfe..982119a 100644 --- a/dap/swddp.go +++ b/dap/swddp.go @@ -124,10 +124,10 @@ func (dp *DebugPort) readDP(ctx context.Context, reg DPRegister) (uint32, error) if dp.jtag != nil { return dp.jtag.readDP(ctx, reg) } - if !info.bankIndependent && dp.state.dpBankAmbiguous() { - return 0, errors.New("dap: DP register bank is ambiguous after an unconfirmed SELECT write") - } if !info.bankIndependent { + if err := dp.confirmPendingSELECT(ctx); err != nil { + return 0, err + } if err := dp.selectDPBank(ctx, info.bank); err != nil { return 0, err } @@ -280,10 +280,10 @@ func (dp *DebugPort) writeDP(ctx context.Context, reg DPRegister, value uint32) } func (dp *DebugPort) prepareDPWrite(ctx context.Context, reg DPRegister, info dpRegisterInfo) error { - if !info.bankIndependent && dp.state.dpBankAmbiguous() { - return errors.New("dap: DP register bank is ambiguous after an unconfirmed SELECT write") - } if !info.bankIndependent { + if err := dp.confirmPendingSELECT(ctx); err != nil { + return err + } if err := dp.selectDPBank(ctx, info.bank); err != nil { return err } diff --git a/dap/wait.go b/dap/wait.go index db3337f..cf602b3 100644 --- a/dap/wait.go +++ b/dap/wait.go @@ -110,6 +110,10 @@ func (e *swdExecutor) resolveSELECT(req transferRequest, value uint32, err error return } switch { + case errors.Is(err, swd.ErrWait): + if e.dp.state.response == responseOverrun { + e.dp.state.loseFraming() + } case isABORTWrite(req): e.resolveSELECTAfterABORT(err) case isDPIDRRead(req): @@ -118,7 +122,7 @@ func (e *swdExecutor) resolveSELECT(req transferRequest, value uint32, err error if err == nil { e.dp.state.resolveSELECTFromCTRLSTAT(value) } - case err == nil || err == swd.ErrWait || err == swd.ErrParity: + case err == nil || err == swd.ErrParity: e.dp.state.confirmSELECT() } } diff --git a/dap/wait_test.go b/dap/wait_test.go index f0dfb7c..ef90c0e 100644 --- a/dap/wait_test.go +++ b/dap/wait_test.go @@ -1200,17 +1200,16 @@ func TestDebugPortKeepsSELECTProvisionalAfterDPIDRRead(t *testing.T) { t.Fatal(err) } before := len(target.requests) - if _, err := dp.ReadDP(t.Context(), dap.CTRLSTAT); err == nil || !strings.Contains(err.Error(), "bank is ambiguous") { - t.Fatalf("ReadDP(CTRLSTAT) error = %v, want ambiguous bank", err) + if _, err := dp.ReadDP(t.Context(), dap.CTRLSTAT); !errors.Is(err, dap.ErrFault) { + t.Fatalf("ReadDP(CTRLSTAT) error = %v, want SELECT completion FAULT", err) } - if got := len(target.requests); got != before { - t.Fatalf("requests after ambiguous CTRL/STAT read = %d, want %d", got, before) - } - if err := dp.WriteDP(t.Context(), dap.CTRLSTAT, overrunDetect); err == nil || !strings.Contains(err.Error(), "bank is ambiguous") { - t.Fatalf("WriteDP(CTRLSTAT) error = %v, want ambiguous bank", err) + if len(target.requests) != before+1 || target.requests[before] != dpRead(12) { + t.Fatalf("requests after SELECT failure = %+v, want only RDBUFF", target.requests[before:]) } - if got := len(target.requests); got != before { - t.Fatalf("requests after ambiguous CTRL/STAT write = %d, want %d", got, before) + before = len(target.requests) + assertRepairBlocksTraffic(t, dp, target, before) + if err := dp.Release(t.Context()); err != nil { + t.Fatal(err) } } @@ -1267,7 +1266,7 @@ func TestConnectRepairsFailedRDBUFFConfirmation(t *testing.T) { } } -func TestDebugPortRejectsAmbiguousCTRLSTATReadBeforeTraffic(t *testing.T) { +func TestDebugPortRejectsFailedSELECTBeforeCTRLSTAT(t *testing.T) { target := newWaitTarget() dp := newDebugPort(t, target) if _, err := dp.Connect(t.Context()); err != nil { @@ -1281,11 +1280,14 @@ func TestDebugPortRejectsAmbiguousCTRLSTATReadBeforeTraffic(t *testing.T) { t.Fatal(err) } before := len(target.requests) - if _, err := dp.ReadDP(t.Context(), dap.CTRLSTAT); err == nil || !strings.Contains(err.Error(), "bank is ambiguous") { - t.Fatalf("ReadDP(CTRLSTAT) error = %v, want ambiguous bank", err) + if _, err := dp.ReadDP(t.Context(), dap.CTRLSTAT); !errors.Is(err, dap.ErrFault) { + t.Fatalf("ReadDP(CTRLSTAT) error = %v, want SELECT completion FAULT", err) } - if got := len(target.requests); got != before { - t.Fatalf("requests after ambiguous CTRL/STAT read = %d, want %d", got, before) + if len(target.requests) != before+1 || target.requests[before] != dpRead(12) { + t.Fatalf("requests after SELECT failure = %+v, want only RDBUFF", target.requests[before:]) + } + if err := dp.Release(t.Context()); err != nil { + t.Fatal(err) } } diff --git a/docs/ports/dap.md b/docs/ports/dap.md index 4d01f89..f09e88c 100644 --- a/docs/ports/dap.md +++ b/docs/ports/dap.md @@ -570,6 +570,17 @@ Reading the discovery registers alone does not discover or acquire an AP. Immediate and queued `SELECT` and `SELECT1` writes reject address bits beyond DPIDR1.ASIZE before traffic; the low DPBANKSEL bits remain independent. +After an immediate `SELECT` write, the next banked DP access settles that write +through RDBUFF before using or changing the bank. This also applies to SELECT1 +when addressing an AP above 32 bits, and while clearing selection during +`Release`. Cancellation or WAIT exhaustion during confirmation returns the +operation error; a fresh release attempt resumes cleanup instead of rejecting +the pending selection without traffic. A failed confirmation never sends the +banked access whose selection it could not establish. Each explicit release +attempt, including protocol re-entry after a failed confirmation, uses the +caller's context; provide a fresh bounded context for the retry. Recovery inside +a failed transport operation can still finish under its own independent budget. + ```go width, err := dp.ReadDP(ctx, dap.DPIDR1) if err != nil {