On Sun, Sep 06, 2026 at 09:48:44AM +0530, Shaikh Kamaluddin wrote:
> ELC alias retirement depends on the error severity and on whether
> vendor-specific MCE rules consider the reported address usable. Add
> regression coverage for these decisions to cxl-elc.sh.
>
> Inject MCE records through the mce-inject debugfs interface and verify
> the resulting bank status and alias-offlining messages in dmesg. Test
> corrected and uncorrected errors on Intel, and exercise the poison,
> deferred, and legacy bank-4 cases required by AMD address validation.
Hi Shaikh,
Thanks for taking this on. I ran it both ways: on a pre-fix kernel it
fails on the corrected case, and with 6e7ab3c8107c ("cxl/mce: Avoid
alias page retirement for corrected errors") applied the Intel cases
pass. So it does detect the regression it was written for.
I do have a scope question about the vendor-specific coverage.
The cxl tests in this repo exercise behavior controlled by the cxl_test
mock topology. CPU vendor, MCA bank layout, and SMCA capability aren't
properties cxl_test controls. Should the test matrix depend on them?
Looking at cxl_handle_mce(), I see these decision points:
if (mce_is_correctable(mce)) /* 1 */
return NOTIFY_DONE;
if (!mce_usable_address(mce)) /* 2 */
return NOTIFY_DONE;
spa = mce->addr & MCI_ADDR_PHYSADDR;
if (!cxl_resource_contains_addr(p->res, spa)) /* 3 */
return NOTIFY_DONE;
if (spa >= p->res->start + p->cache_size) /* 4 */
spa_alias = spa - p->cache_size;
else
spa_alias = spa + p->cache_size;
pfn = spa_alias >> PAGE_SHIFT;
if (!pfn_valid(pfn)) /* 5 */
return NOTIFY_DONE;
...
if (!memory_failure(pfn, 0)) /* 6 */
set_mce_nospec(pfn);
This patch mostly exercises 1 and 2. Point 2 is where the arch-specific
MCE policy lives. Points 3 through 6 are driven by the cxl_test topology.
Could we keep this test focused on the behavior cxl_test controls and go
deeper on those cases instead? Can we test both alias directions, the
boundary between them, and an SPA outside the ELC region.
That would mean dropping the vendor dispatch, bank discovery, has_bank(),
non_b4_bank, and the AMD matrix from this patch.
I don't think that analysis should be thrown away. It looks like useful
coverage of the arch-specific MCE handling. Could we treat that as separate
follow-on work and discuss the appropriate test mechanism and home with the
x86 MCE maintainers rather than making it part of v2?
More feedback on the cxl test appropriate pieces inline -
>
> Derive the ELC SPA and its alias from cxl-list output, select offline
> test addresses from lsmem, and discover available MCA banks dynamically.
> Use a distinct SPA for each case and clean up modules loaded by the test.
>
> Suggested-by: Ben Cheatham <[email protected]>
> Suggested-by: Alison Schofield <[email protected]>
> Signed-off-by: Shaikh Kamaluddin <[email protected]>
> ---
> Testing:
> The test was run under virtme-ng with cxl_test configured for Extended
> Linear Cache operation:
>
> Intel, GenuineIntel:
> --------------------
>
> 1. vng -v -r ./arch/x86/boot/bzImage --disable-kvm --qemu-opts='-cpu
> Skylake-Server-v4,+mce,+mca -m 4G -machine q35,cxl=on -object
> memory-backend-ram,id=cxl-mem0,size=512M -device
> pxb-cxl,bus_nr=12,bus=pcie.0,id=cxl.0 -device
> cxl-rp,port=0,bus=cxl.0,id=root_port0,chassis=0,slot=0 -device
> cxl-type3,bus=root_port0,volatile-memdev=cxl-mem0,id=cxl-mem-device0 -M
> cxl-fmw.0.targets.0=cxl.0,cxl-fmw.0.size=512M'
>
> 2 . Go to ndctl-main directoy with this patches
>
> env NDCTL="$PWD/build/ndctl/ndctl"
> DAXCTL="$PWD/build/daxctl/daxctl"
> CXL="$PWD/build/cxl/cxl"
> ./test/cxl-elc.sh
>
> RESULT:
> --------
>
> PASS Intel corrected: alias no
> PASS Intel uncorrected: alias yes
> CXL ELC MCE validation passed for GenuineIntel: 2 cases
> -----------------------------------------------------------------
>
> AMD, AuthenticAMD legacy MCA:
> -----------------------------
>
> 1. vng -v -r ./arch/x86/boot/bzImage --disable-kvm --qemu-opts='-cpu
> EPYC-Milan -m 4G -machine q35,cxl=on -object
> memory-backend-ram,id=cxl-mem0,size=512M -device
> pxb-cxl,bus_nr=12,bus=pcie.0,id=cxl.0 -device
> cxl-rp,port=0,bus=cxl.0,id=root_port0,chassis=0,slot=0 -device
> cxl-type3,bus=root_port0,volatile-memdev=cxl-mem0,id=cxl-mem-device0 -M
> cxl-fmw.0.targets.0=cxl.0,cxl-fmw.0.size=512M'
>
> 2. Go to ndctl-main directoy with this patches
> env NDCTL="$PWD/build/ndctl/ndctl"
> DAXCTL="$PWD/build/daxctl/daxctl"
> CXL="$PWD/build/cxl/cxl"
> ./test/cxl-elc.sh
>
> RESULT:
> ---------
> PASS AMD plain corrected: alias no
> PASS AMD plain uncorrected: alias no
> PASS AMD poison-only (invalid encoding): alias no
> PASS AMD deferred without poison: alias no
> PASS AMD deferred with poison: alias yes
> PASS AMD uncorrected with poison: alias yes
> PASS AMD bank-4 non-memory poison (invalid encoding): alias no
> PASS AMD legacy bank-4 corrected DRAM ECC: alias no
> CXL ELC MCE validation passed for AuthenticAMD: 8 cases
>
> test/cxl-elc.sh | 313 +++++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 312 insertions(+), 1 deletion(-)
>
> diff --git a/test/cxl-elc.sh b/test/cxl-elc.sh
> index cfa09cb..bd600b4 100755
> --- a/test/cxl-elc.sh
> +++ b/test/cxl-elc.sh
> @@ -8,6 +8,30 @@ rc=77
>
> set -ex
>
> +mce_inject_loaded=0
> +
> +cleanup()
> +{
> + local status=$?
> + local cleanup_status=0
> +
> + trap - EXIT ERR
> + set +e
> +
> + if ((mce_inject_loaded)); then
> + modprobe -r mce-inject || cleanup_status=1
> + fi
> +
> + modprobe -r cxl_test || cleanup_status=1
> +
> + if ((status == 0 && cleanup_status != 0)); then
> + status=1
> + fi
> +
> + exit "$status"
> +}
> +
> +trap cleanup EXIT
> trap 'err $LINENO' ERR
Don't need EXIT trap and status rewrite here.
The cxl tests generally use the ERR trap and unload explicitly. For a
multi-module example, cxl-dax-hmem.sh has an unload() helper using:
modprobe -r ... 2>/dev/null || true
Could we follow that pattern here and leave the test result coming from
err()/do_skip()?
>
> check_prereq "jq"
> @@ -85,10 +109,297 @@ compare_bases()
> ((cxlrd_hpa == ep1_hpa - ep1_size)) || err "$LINENO"
> }
>
> +mce_skip()
> +{
> + echo "SKIP CXL ELC MCE subtest: $*"
> + return 0
> +}
> +
Please use do_skip() here.
As written, a prerequisite failure returns 0, so meson reports PASS even
if none of the MCE coverage ran. I think this test needs to be
all-or-nothing: if its prerequisites aren't available, the test should
SKIP rather than PASS with cases omitted.
> +dmesg_count()
> +{
> + dmesg | grep -F -c -- "$1" || true
> +}
> +
> +wait_for_dmesg()
> +{
> + local pattern="$1"
> + local previous="$2"
> + local count
> +
> + for _ in {1..50}; do
> + count="$(dmesg_count "$pattern")"
> + ((count > previous)) && return 0
> + sleep 0.1
> + done
> +
> + return 1
> +}
Do we want to count matches across the whole ring buffer?
What happens if the ring buffer wraps between the before and after
samples? Also, are messages left by an earlier run something the
negative checks need to account for?
cxl-translate.sh has the pattern of recording a start time and looking
at messages since that point. Could a scoped log window work here?
> +
> +address_is_offline()
> +{
> + local address="$1"
> + local range start end
> +
> + while read -r range; do
> + [[ "$range" == *-* ]] || continue
> + start=$(( ${range%%-*} ))
> + end=$(( ${range##*-} ))
> + ((address >= start && address <= end)) && return 0
> + done < <(echo "$lsmem_json" | jq -r \
> + '.memory[] | select((.state | ascii_downcase) == "offline") |
> + .range')
With set -x, doesn't this expand the JSON into the trace each time this
is evaluated?
Could the offline ranges be parsed once and then iterated?
> +
> + return 1
> +}
> +
> +find_offline_spas()
> +{
> + local nr_spas="$1"
> + local range start end low high candidate alias spa_string alias_string
> + local page_size region_end cache_end i
> + local stride=0x10000
> +
> + lsmem_json="$(lsmem --json --bytes --output RANGE,STATE)"
> + page_size="$(getconf PAGESIZE)"
> + region_end=$((region_hpa + region_size))
> + cache_end=$((region_hpa + elc_size))
> + inject_spas=()
> + alias_spas=()
> +
> + while read -r range; do
> + [[ "$range" == *-* ]] || continue
> + start=$(( ${range%%-*} ))
> + end=$(( ${range##*-} ))
> +
> + low=$start
> + ((low < region_hpa)) && low=$region_hpa
> + high=$((end + 1))
> + ((high > cache_end)) && high=$cache_end
Doesn't limiting high to cache_end mean every injected SPA comes from
the first half of the ELC region?
If so, aren't we only exercising:
spa_alias = spa + p->cache_size;
and never:
spa_alias = spa - p->cache_size;
Could we select addresses from both halves? Are there other useful
boundary cases?
> + ((low < high)) || continue
> +
> + # Stay away from the first pages of the hotplugged range.
> + candidate=$((low + stride))
> + candidate=$((
> + (candidate + page_size - 1) / page_size * page_size
> + ))
> + inject_spas=()
> + alias_spas=()
> + for ((i = 0; i < nr_spas; i++)); do
> + candidate=$((candidate + (i > 0 ? stride : 0)))
> + alias=$((candidate + elc_size))
> + ((candidate < high && alias < region_end)) || break
> + address_is_offline "$alias" || break
> + printf -v spa_string '0x%x' "$candidate"
> + printf -v alias_string '0x%x' "$alias"
> + inject_spas+=("$spa_string")
> + alias_spas+=("$alias_string")
> + done
> + ((${#inject_spas[@]} == nr_spas)) || continue
> + return 0
> + done < <(echo "$lsmem_json" | jq -r \
> + '.memory[] | select((.state | ascii_downcase) == "offline") |
> + .range')
> +
> + return 1
> +}
> +
> +discover_mca_banks()
> +{
> + local path name
> +
> + for path in \
> + /sys/devices/system/machinecheck/machinecheck0/bank[0-9]*; do
> + [[ -e "$path" ]] || continue
> + name="${path##*/}"
> + printf '%s\n' "${name#bank}"
> + done | sort -n -u
> +}
> +
> +has_bank()
> +{
> + local wanted="$1"
> + local bank
> +
> + for bank in "${mca_banks[@]}"; do
> + [[ "$bank" == "$wanted" ]] && return 0
> + done
> +
> + return 1
> +}
> +
> +select_mce_parameters()
> +{
> + local bank
> +
> + vendor="$(awk '/^vendor_id/{ print $3; exit }' /proc/cpuinfo)"
> + readarray -t mca_banks < <(discover_mca_banks)
> + ((${#mca_banks[@]})) || return 1
> +
> + case "$vendor" in
> + GenuineIntel)
> + test_bank="${mca_banks[-1]}"
> + test_case_count=2
> + ;;
> + AuthenticAMD)
> + # The usable-address corrected-error case is the legacy bank-4
> DRAM
> + # ECC encoding (XEC 8). It is not valid for an SMCA bank.
> + grep -qw smca /proc/cpuinfo && return 1
> + has_bank 4 || return 1
> + non_b4_bank=
> + for bank in "${mca_banks[@]}"; do
> + [[ "$bank" == 4 ]] || non_b4_bank="$bank"
> + done
> + [[ -n "$non_b4_bank" ]] || return 1
> + test_case_count=8
> + ;;
> + *)
> + return 1
> + ;;
> + esac
> +}
The above host specfic funcs should move out of this test.
> +
> +inject_mce()
> +{
> + local status="$1"
> + local bank="$2"
> + local mce_inject=/sys/kernel/debug/mce-inject
> +
> + echo sw > "$mce_inject/flags"
> + echo "$status" > "$mce_inject/status"
> + echo 0x80 > "$mce_inject/misc"
That 0x80 needs a comment.
> + echo "$inject_spa" > "$mce_inject/addr"
> + echo "$bank" > "$mce_inject/bank"
Also isn't the write to "bank" what triggers the injection? If so,
could that ordering get a comment too, since it needs to remain last?
> +}
> +
> +run_mce_case()
> +{
> + local name="$1"
> + local status="$2"
> + local bank="$3"
> + local expect_alias="$4"
> + local index="$5"
> + local alias_msg bank_msg status_tag
> + local alias_before bank_before
> +
> + inject_spa="${inject_spas[$index]}"
> + alias_spa="${alias_spas[$index]}"
> + alias_msg="Offlining aliased SPA address0: $alias_spa"
> + status_tag="${status#0x}"
> + bank_msg="Bank $bank: $status_tag"
Does this depend on status already being written as exactly 16 hex
digits to match __print_mce()?
Would formatting it explicitly be safer? like -
printf -v status_tag '%016x' "$status"
> + alias_before="$(dmesg_count "$alias_msg")"
> + bank_before="$(dmesg_count "$bank_msg")"
> +
> + inject_mce "$status" "$bank"
> + wait_for_dmesg "$bank_msg" "$bank_before" || err "$LINENO"
> +
> + if [[ "$expect_alias" == "yes" ]]; then
> + wait_for_dmesg "$alias_msg" "$alias_before" || err "$LINENO"
> + else
> + (($(dmesg_count "$alias_msg") == alias_before)) || \
> + err "$LINENO"
> + fi
Wondering about checking dmesg at all. Would an mce_record tracepoint give
us a more reliable indication that the injection occurred? cxl-poison.sh
and cxl-events.sh already have examples of using tracefs. (Which would
make irrelevant my comments about dmesg log checking earlier.)
Stopping here for now. Will pick up again in a v2 that's been trimmed.
-- Alison
> +
> + echo "PASS $name: SPA $inject_spa, alias $alias_spa, bank $bank, alias
> $expect_alias"
> +}
> +
> +run_intel_mce_cases()
> +{
> + run_mce_case "Intel corrected" 0x9c00000000000080 \
> + "$test_bank" no 0
> + run_mce_case "Intel uncorrected" 0xbc00000000000080 \
> + "$test_bank" yes 1
> +}
> +
> +run_amd_mce_cases()
> +{
> + # Mirror the full AMD matrix documented with the kernel fix. The two
> + # poison-only records have UC=0 and Deferred=0, so they are not valid
> + # architectural records. Keep them as software-injection robustness
> + # coverage and expect the corrected-error filter to reject them.
> + run_mce_case "AMD plain corrected" 0x9c00000000000080 \
> + "$non_b4_bank" no 0
> + run_mce_case "AMD plain uncorrected" 0xbc00000000000080 \
> + "$non_b4_bank" no 1
> + run_mce_case "AMD poison-only (invalid encoding)" \
> + 0x9c00080000000080 "$non_b4_bank" no 2
> + run_mce_case "AMD deferred without poison" 0x9c00100000000080 \
> + "$non_b4_bank" no 3
> + run_mce_case "AMD deferred with poison" 0x9c00180000000080 \
> + "$non_b4_bank" yes 4
> + run_mce_case "AMD uncorrected with poison" 0xbc00080000000080 \
> + "$non_b4_bank" yes 5
> + run_mce_case "AMD bank-4 non-memory poison (invalid encoding)" \
> + 0x9c00080000000080 4 no 6
> + run_mce_case "AMD legacy bank-4 corrected DRAM ECC" \
> + 0x9c00000000080000 4 no 7
> +}
> +
> +test_mce_alias()
> +{
> + local region_json
> +
> + [[ "$(uname -m)" == "x86_64" ]] || {
> + mce_skip "x86_64 is required"
> + return 0
> + }
> + command -v lsmem >/dev/null 2>&1 || {
> + mce_skip "lsmem is unavailable"
> + return 0
> + }
> +
> + region_json="$(echo "$json" | jq -c --arg region "$region" \
> + '.[] | select(.region == $region)')"
> + region_hpa="$(echo "$region_json" | jq -r '.resource // empty')"
> + region_size="$(echo "$region_json" | jq -r '.size // empty')"
> + elc_size="$(echo "$region_json" | jq -r \
> + '.extended_linear_cache_size // empty')"
> +
> + [[ "$region_hpa" =~ ^[0-9]+$ ]] || err "$LINENO"
> + [[ "$region_size" =~ ^[0-9]+$ ]] || err "$LINENO"
> + [[ "$elc_size" =~ ^[0-9]+$ ]] || err "$LINENO"
> + ((elc_size > 0x10000 && region_size >= elc_size * 2)) || \
> + err "$LINENO"
> +
> + select_mce_parameters || {
> + mce_skip "no supported MCA bank configuration for $vendor"
> + return 0
> + }
> +
> + if [[ ! -d /sys/module/mce_inject ]]; then
> + modprobe mce-inject || {
> + mce_skip "mce-inject module is unavailable"
> + return 0
> + }
> + mce_inject_loaded=1
> + fi
> + [[ -d /sys/kernel/debug/mce-inject ]] || {
> + mce_skip "mce-inject debugfs interface is unavailable"
> + return 0
> + }
> +
> + find_offline_spas "$test_case_count" || {
> + mce_skip "offline lsmem range lacks enough ELC test addresses"
> + return 0
> + }
> +
> + case "$vendor" in
> + GenuineIntel)
> + run_intel_mce_cases
> + ;;
> + AuthenticAMD)
> + run_amd_mce_cases
> + ;;
> + esac
> +
> + echo "CXL ELC MCE validation passed for $vendor: $test_case_count cases"
> +}
> +
> find_region
> retrieve_info
> compare_sizes
> compare_bases
>
> +# Check before injecting the expected machine-check messages.
> +check_dmesg "$LINENO"
> +test_mce_alias
> check_dmesg "$LINENO"
> -modprobe -r cxl_test
>
> base-commit: 15e932c4e1318a9608ad9b799ad83a32a8b5970d
> --
> 2.43.0
>
