Conversation
Signed-off-by: Xudong Hao <xudong.hao@intel.com>
Signed-off-by: Xudong Hao <xudong.hao@intel.com>
There was a problem hiding this comment.
Pull request overview
Adds new KVM/QEMU CPUID configuration entries for APX (including XSAVE layout and feature bits) and adjusts some feature test scenario identifiers.
Changes:
- Add APX XSAVE CPUID subleaf (0xD/19) and APX feature-bit CPUID leaf (0x29) entries.
- Rename several PMU-related
test_scenidentifiers from underscore to hyphen form.
Reviewed changes
Copilot reviewed 2 out of 8 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| KVM/qemu/x86_cpuid.cfg | Adds APX-related CPUID definitions (XSAVE area + feature bits) for test coverage. |
| KVM/qemu/feature_test.cfg | Renames several test_scen identifiers (PEBS / perfmon v6 / fix counters). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| cpuid = "d 0 0x13 0 a 0:31 128, d 0 0x13 0 b 0:31 960, d 0 0x13 0 c 0:31 0" | ||
| - apx_feature_bits: | ||
| # CPUID.29.0:EBX[0:0] APX_NCI_NDD_NF: 1 | ||
| cpuid = "29 0 0 0 a 0:31 0, 29 0 0 0 b 0, 29 0 0 0 b 1:31 0, 29 0 0 0 c 0:31 0, 29 0 0 0 d 0:31 0" |
There was a problem hiding this comment.
The apx_feature_bits comment says CPUID.29.0:EBX[0:0] ... : 1, but the CPUID string sets EBX bit 0 to 0 (... b 0 ...). Update the CPUID encoding so EBX[0] is set to 1 (and keep EBX[1:31]=0) to match the documented expected feature bit.
| cpuid = "29 0 0 0 a 0:31 0, 29 0 0 0 b 0, 29 0 0 0 b 1:31 0, 29 0 0 0 c 0:31 0, 29 0 0 0 d 0:31 0" | |
| cpuid = "29 0 0 0 a 0:31 0, 29 0 0 0 b 0:0 1, 29 0 0 0 b 1:31 0, 29 0 0 0 c 0:31 0, 29 0 0 0 d 0:31 0" |
| features = "APX" | ||
| - apx_xsave: | ||
| # User-level APX XSAVE area: CPUID.(EAX=0xD, ECX=19) | ||
| cpuid = "d 0 0x13 0 a 0:31 128, d 0 0x13 0 b 0:31 960, d 0 0x13 0 c 0:31 0" |
There was a problem hiding this comment.
This CPUID entry mixes hex (0x13) with the surrounding config style that appears to use decimal operands (e.g., 24, 29). If the CPUID config parser expects a consistent numeric format, this may fail to parse or be inconsistent to maintain. Prefer using 19 (decimal) for ECX here, or whichever format is consistently used/guaranteed supported by the parser.
| cpuid = "d 0 0x13 0 a 0:31 128, d 0 0x13 0 b 0:31 960, d 0 0x13 0 c 0:31 0" | |
| cpuid = "d 0 19 0 a 0:31 128, d 0 19 0 b 0:31 960, d 0 19 0 c 0:31 0" |
| pre_command = 'echo 0 > /proc/sys/kernel/nmi_watchdog && modprobe -r kvm_intel && modprobe kvm_intel enable_mediated_pmu=Y' | ||
| post_command = 'echo 1 > /proc/sys/kernel/nmi_watchdog && modprobe -r kvm_intel && modprobe kvm_intel' | ||
| test_scen = "tests_arch_pebs" | ||
| test_scen = "tests-arch_pebs" |
There was a problem hiding this comment.
The PR title is focused on adding DMR APX test cases, but this hunk changes test_scen naming for PMU-related scenarios (PEBS / perfmon v6 / fix counters). If these renames are required, it would help to either (1) explain the linkage in the PR description, or (2) split these changes into a separate PR to keep the APX work isolated and easier to review/revert.
| pre_command = 'echo 0 > /proc/sys/kernel/nmi_watchdog && modprobe -r kvm_intel && modprobe kvm_intel enable_mediated_pmu=Y' | ||
| post_command = 'echo 1 > /proc/sys/kernel/nmi_watchdog && modprobe -r kvm_intel && modprobe kvm_intel' | ||
| test_scen = "tests_arch_perfmon_v6" | ||
| test_scen = "tests-arch_perfmon_v6" |
There was a problem hiding this comment.
The PR title is focused on adding DMR APX test cases, but this hunk changes test_scen naming for PMU-related scenarios (PEBS / perfmon v6 / fix counters). If these renames are required, it would help to either (1) explain the linkage in the PR description, or (2) split these changes into a separate PR to keep the APX work isolated and easier to review/revert.
| pre_command = 'echo 0 > /proc/sys/kernel/nmi_watchdog && modprobe -r kvm_intel && modprobe kvm_intel enable_mediated_pmu=Y' | ||
| post_command = 'echo 1 > /proc/sys/kernel/nmi_watchdog && modprobe -r kvm_intel && modprobe kvm_intel' | ||
| test_scen = "tests_3fix_counters" | ||
| test_scen = "tests-3fix_counters" |
There was a problem hiding this comment.
The PR title is focused on adding DMR APX test cases, but this hunk changes test_scen naming for PMU-related scenarios (PEBS / perfmon v6 / fix counters). If these renames are required, it would help to either (1) explain the linkage in the PR description, or (2) split these changes into a separate PR to keep the APX work isolated and easier to review/revert.
No description provided.