Messages in this thread | | | Date | Wed, 25 May 2022 15:56:42 +0800 | Subject | Re: [PATCH RESEND v12 00/17] KVM: x86/pmu: Add basic support to enable guest PEBS via DS | From | Like Xu <> |
| |
On 19/5/2022 10:46 pm, Vitaly Kuznetsov wrote: > Like Xu <like.xu.linux@gmail.com> writes: > >> On 19/5/2022 9:31 pm, Like Xu wrote: >>> ==== Test Assertion Failure ==== >>> lib/x86_64/processor.c:1207: r == nmsrs >>> pid=6702 tid=6702 errno=7 - Argument list too long >>> 1 0x000000000040da11: vcpu_save_state at processor.c:1207 >>> (discriminator 4) >>> 2 0x00000000004024e5: main at state_test.c:209 (discriminator 6) >>> 3 0x00007f9f48c2d55f: ?? ??:0 >>> 4 0x00007f9f48c2d60b: ?? ??:0 >>> 5 0x00000000004026d4: _start at ??:? >>> Unexpected result from KVM_GET_MSRS, r: 29 (failed MSR was 0x3f1) >>> >>> I don't think any of these failing tests care about MSR_IA32_PEBS_ENABLE >>> in particular, they're just trying to do KVM_GET_MSRS/KVM_SET_MSRS. >> >> One of the lessons I learned here is that the members of msrs_to_save_all[] >> are part of the KVM ABI. We don't add feature-related MSRs until the last >> step of the KVM exposure feature (in this case, adding MSR_IA32_PEBS_ENABLE, >> MSR_IA32_DS_AREA, MSR_PEBS_DATA_CFG to msrs_to_save_all[] should take >> effect along with exposing the CPUID bits). > > AFAIR the basic rule here is that whatever gets returned with > KVM_GET_MSR_INDEX_LIST can be passed to KVM_GET_MSRS and read > successfully by the host (not necessarily by the guest) so my guess is > that MSR_IA32_PEBS_ENABLE is now returned in KVM_GET_MSR_INDEX_LIST but > can't be read with KVM_GET_MSRS. Later, the expectation is that what was > returned by KVM_GET_MSRS can be set successfully with KVM_SET_MSRS. >
Thanks for the clarification.
Some kvm x86 selftests have been failing due to this issue even after the last commit.
I blame myself for not passing the msr_info->host_initiated to the intel_is_valid_msr(), meanwhile I pondered further whether we should check only the MSR addrs range in the kvm_pmu_is_valid_msr() and apply this kind of sanity check in the pmu_set/get_msr().
Vitaly && Paolo, any preference to move forward ?
Thanks, Like Xu
| |