Repository navigation
perf(exerciser): poll DPC link recovery in a VAL helper - #675
Conversation
| if (status == PCIE_DLL_LINK_STATUS_ACTIVE) | ||
| return 0; | ||
|
|
||
| if (status != PCIE_DLL_LINK_STATUS_NOT_ACTIVE || elapsed == timeout_ticks) |
There was a problem hiding this comment.
At this point the status can be only PCIE_DLL_LINK_STATUS_ACTIVE or PCIE_DLL_LINK_STATUS_NOT_ACTIVE, So this condition becomes void.
There was a problem hiding this comment.
Agreed. The helper no longer has that condition. It loops until the status is PCIE_DLL_LINK_STATUS_ACTIVE, and the unsupported case is handled once before the loop.
| return 1; | ||
| } | ||
|
|
||
| start = ArmArchTimerReadReg(CntPct); |
There was a problem hiding this comment.
Can we avoid direct ArmArchTimerReadReg(CntPct) usage here and use a bounded val_time_delay_ms() based poll loop instead? That still fixes the busy-loop issue while keeping the exerciser test independent of architectural timer counter reads.
There was a problem hiding this comment.
Done. val_pcie_wait_for_link_up() now polls with val_time_delay_ms(ONE_MILLISECOND) for at most timeout_ms polls, with no CntPct reads.
| static uint32_t cfg_space_count; | ||
|
|
||
| static uint32_t | ||
| wait_for_link_up(uint32_t rp_bdf) |
There was a problem hiding this comment.
Same logic needed in e027 as well. This can be in val so that both can use it
There was a problem hiding this comment.
Done. Added val_pcie_wait_for_link_up(bdf, timeout_ms) in val/src/acs_pcie.c, and e024 and e027 both use it. If Link Active reporting is not supported, it waits the full timeout and returns success, as e027 did before.
| } | ||
| } | ||
|
|
||
| return 1; |
There was a problem hiding this comment.
Msg here that Link is not yet active
There was a problem hiding this comment.
Added. Each retry logs "Link not yet active for BDF 0x%x, retrying" at DEBUG, and a timeout logs the error.
| if (wait_for_link_up(erp_bdf)) | ||
| { | ||
| val_print(ERROR, | ||
| "\n The link not active after reset for BDF 0x%x: ", erp_bdf); |
There was a problem hiding this comment.
This can be removed as the return from the wait_for_link_up
There was a problem hiding this comment.
Removed. The helper prints the link-down error, so e024 and e027 no longer print it.
|
Hi @cdwilde , Thank you for raising the PR. We have provided few comments, Request you to consider them. Thanks, |
Replace the uncalibrated post-SBR spin and one-shot link-up sleep with one-millisecond polling requests and a 100-millisecond architectural-counter deadline. Stop when DLL Link Active is observed within the recovery window; retain the SBR hold, post-link initialization delay, error-injection coverage and configuration restoration. Fixes ARM-software#674. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 64fb407d-eb53-484a-bc49-606a91214662
Per review, add val_pcie_wait_for_link_up() and use it in e024 (PCI_ER_06) and e027 (PCI_ER_10). It polls DLL Link Active once per millisecond with val_time_delay_ms() for at most the given number of milliseconds, with no direct architectural counter reads. When Link Active reporting is not supported it waits the full timeout instead, as e027 previously did not fail in that case. The helper reports the link-down error, so the callers' duplicate message is removed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: aa96196c-fd99-476b-a579-8de0fe7f65e7
670e140 to
6dfabd0
Compare
cdwilde
changed the title
|
Thanks @Sujana-M. I've addressed all five comments in 6dfabd0, rebased on main. The link-up polling is now a VAL helper used by both e024 and e027. Validation is in the updated description: host tests of the helper, plus a full SBSA suite run on our emulation platform where PCI_ER_06 still passes and all rule results are unchanged. |
Fixes #674.
Summary
Replace the uncalibrated post-SBR spin and one-shot 100 ms link-recovery delay in
e024(PCI_ER_06) ande027(PCI_ER_10) with bounded DLL Link Active polling, using a common VAL helper.val_pcie_wait_for_link_up(bdf, timeout_ms)toval/src/acs_pcie.c. It checks link status immediately, then polls once per millisecond withval_time_delay_ms(ONE_MILLISECOND)for at mosttimeout_mspolls, returning as soon as the link is active. It does not read the architectural counter directly.DEBUG. On timeout it logs "The link not active after reset" atERROR, so the callers no longer print their own message. A failed delay is reported as an error.e024already fails such Root Ports before injecting errors, so this case only applies toe027.e024, both error-injection modes, configuration restoration and cleanup are unchanged.Validation
-O0and-O2with-Wall -Wextra -Werror. The 8 cases cover:-Werror) and complete suite run on a pre-silicon emulation platform.PCI_ER_06passes across eight exerciser endpoints, and its elapsed simulated time drops from 331.1 ms to 172.3 ms. All 222 rule results match the baseline run.PCI_ER_10is not exercised on this platform, soe027is covered by compilation and the helper tests only.The emulation PAL scales delays, so the timing improvement should not be assumed to apply to other platforms.