Repository navigation
VAL: add opt-in per-run base-rule result caching - #671
Conversation
Reuse completed base-rule results by rule ID when -rule-cache or ACS_RULE_CACHE is enabled. Keep alias traversal, prechecks, reporting, path guards and existing SYS-MPAM entry caching unchanged. Reset results for each run and retain repeat execution by default. Add portable actual-source regressions for status reuse, reporting counters, separate rule identities, lifecycle, skips, aliases, cycle/depth checks, MPAM behavior and baremetal enablement. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 64fb407d-eb53-484a-bc49-606a91214662
|
Hi @cdwilde, |
|
@SapthagiriP, do we have a list of which tests fall into this category? |
|
@cdwilde , These are those tests
|
Exclude the ten parent-dependent PCIe rules identified in PR 671 review from per-run rule-result reuse. Their entries must execute for every parent context, including repeated aliases and direct top-level execution. Exercise all ten exclusions with parent-dependent PASS/FAIL results, both sorted execution orders, reporting counters and repeated parent visits. Retain the existing cacheable-rule and SYS-MPAM regression coverage. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 859192f3-6b41-407a-a3f0-d453e3e93c7a
|
Alright, those tests are excluded now. We can take the gains for the tests that we can. I would love to be able to break this dependency also when we have a chance and be able to use caching for these 10 also. |
| case RI_BAR_1: | ||
| case RI_BAR_3: | ||
| case RI_INT_1: | ||
| case RI_ORD_1: |
There was a problem hiding this comment.
Could we put the cache policy in rule_test_map_t instead of maintaining a separate switch on rule IDs?. keeping that property alongside each rule’s metadata would make it easier to update when rules change
There was a problem hiding this comment.
Done in b8ae978. Added RULE_CACHE_POLICY_e and a cache_policy field to rule_test_map_t, and removed the switch. RULE_CACHE_REUSE is 0 and is the default, so only the ten parent-dependent rules set .cache_policy = RULE_CACHE_RERUN in val/src/rule_metadata.c. The orchestrator now checks rule_test_map[rule_id].cache_policy.
| "-I" + str(ROOT), | ||
| "-I" + str(ROOT / "apps/baremetal"), | ||
| "-I" + str(ROOT / "val/include"), | ||
| "-I" + str(ROOT / "pal/baremetal/base/include"), |
There was a problem hiding this comment.
Lets not have unit tests as part of this PR
There was a problem hiding this comment.
Done in b8ae978. I removed tools/tests/ from this PR and updated the PR description.
Address PR 671 review: record each rule's result-reuse policy in its rule_test_map_t metadata instead of a separate switch on rule IDs in the orchestrator. Add RULE_CACHE_POLICY_e; RULE_CACHE_REUSE (0) is the default, so only the ten parent-dependent PCIe rules set RULE_CACHE_RERUN. Remove the tools/tests harness from this PR, as requested in review. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: aa96196c-fd99-476b-a579-8de0fe7f65e7
Summary
Fixes #527 with opt-in, per-run base-rule result caching. The existing SYS-MPAM test-entry cache remains unchanged; this does not enable cross-rule entry reuse for BSA/SBSA.
rule_status_map[]only for the same base-rule ID. Unknown results execute again. Existing run-start reset prevents reuse across suite invocations.(cached)marker alongside the normal report.rule_test_map_tentry (.cache_policy = RULE_CACHE_RERUNinval/src/rule_metadata.c; the defaultRULE_CACHE_REUSEis 0):PCI_IN_04,PCI_IN_17,PCI_LI_01,PCI_LI_03,RI_BAR_1,RI_BAR_3,RI_INT_1,RI_ORD_1,RI_PWR_1, andRI_SMU_1. Their results must not be reused across parent contexts.-rule-cacheto the seven rule-based UEFI apps, and-DACS_RULE_CACHE=ONfor baremetal CMake builds, including nested suite wrappers. Default behavior is unchanged; omit the UEFI flag or leave the CMake option OFF.rule_cache_enabledbeforerun_tests().Safety / intended interpretation
Opting in intentionally omits repeat execution and its side effects. This is not a claim that every rule is idempotent or independent of parent/order/platform state; use the original mode when those dependencies matter. Results still reflect the first execution in this invocation, not a second hardware observation. Maintainer review and integrated platform validation are needed before relying on the optimization for a compliance submission. No platform pass-rate or measured runtime improvement is claimed here.
Validation
RULE_CACHE_RERUNin both execution orders. With the marking removed, the parent-dependent scenario fails as expected.-Wall -Werror, both ON and default OFF. Verified the generated commands for all four wrapper targets forward ON.-Wall -Werroragainst genuine EDK II headers at07b0a29928373eed213ba5965eae0be2a09b0a74. For these isolated translation-unit checks, the unsupported+smeflag was omitted for local GCC13; the orchestrator preincludedUefi.hto supply EDK II AutoGen-like header ordering. This was not a full EDK II application link.val/src/acs_pe.cbecause the local GCC13 assembler does not recognizeid_aa64mmfr3_el1. Changed units compile successfully; no unrelated toolchain workaround is included.