Skip to content

[litmus] Enable check_dic_idc for PreSi and Kvm modes#1727

Open
diaolo01 wants to merge 1 commit intoherd:masterfrom
diaolo01:fix-dic-idc
Open

[litmus] Enable check_dic_idc for PreSi and Kvm modes#1727
diaolo01 wants to merge 1 commit intoherd:masterfrom
diaolo01:fix-dic-idc

Conversation

@diaolo01
Copy link
Copy Markdown
Contributor

@diaolo01 diaolo01 commented Feb 23, 2026

This PR enables PreSi and Kvm modes to check whether a core has DIC/IDC implemented. It also runs tests with DIC=0 & IDC=0 as part of make-aarch64-litmus, while only compiling tests that require either DIC or IDC.

=====

This has been identified as part of #1716. This fix has been tested against that patch and the desired behaviour was observed on an M2:

Fatal error: hardware does not support the CacheType feature IDC=1, DIC=1

EXIT: STATUS=1

The missing call was not flagged by the compiler because GCC/Clang do not warn about unused static inline functions by default.

@diaolo01 diaolo01 marked this pull request as draft February 23, 2026 17:42
litmus/preSi.ml Outdated
| Some cache_type -> cache_type.dic, cache_type.idc in
begin match forall_procs test needs_dic, forall_procs test needs_idc with
| Some dic, Some idc ->
O.fi "check_dic_idc(%d, %d);" (if dic then 1 else 0)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like we are checking DIC/IDC on one core and we assume that it's the same for all cores? Is that a good assumption?

If yes can you please add a comment here?

Copy link
Copy Markdown
Contributor Author

@diaolo01 diaolo01 Mar 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, this code was checking per core rather than a single core. The Arm ARM states: "All PEs in the same Inner Shareable shareability domain must have a common value of this field.".
I moved the check at the test level to match what is done for hardware updates. I have included a comment to capture the requirement above.

@diaolo01 diaolo01 marked this pull request as ready for review March 3, 2026 14:37
Copy link
Copy Markdown
Member

@relokin relokin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two comments, otherwise this PR looks good to me.


litmus-cata-aarch64-ifetch-test-kvm: litmus-aarch64-dep
mkdir $(KUT_DIR_AARCH64)/kvm-unit-tests/t
if $(RUN_TESTS); then NORUN=UDF+2FH; else NORUN=NO; fi ; \
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couldn't you just do something along the lines of:

Suggested change
if $(RUN_TESTS); then NORUN=UDF+2FH; else NORUN=NO; fi ; \
if $(RUN_TESTS); then \
NORUN=UDF+2FH \
TESTS=catalogue/aarch64-ifetch/tests/@dic0-idc0 \
else \
NORUN=NP \
TESTS=catalogue/aarch64-ifetch/tests/@all \
fi \

and then leave the recipe common for both?

MP.FF+dc.cvau-dmb.ish+dsb.ish-ic.ivau-dsb.ish-rfiINSTNOP.litmus
MP.FF+dc.cvau-dsb.ish-ic.ivau-dsb.ish+dmb.ish+rfiINST.litmus
MP.FF+dc.cvau-dsb.ish-ic.ivau-dsb.ish+po.litmus
MP.FF+dc.cvau-dsb.ish-ic.vau-dsb.ish+dmb.ish+rfiINST.litmus
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This tests doesn't exist

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants