[v2,08/15] KVM: x86: Add Arch LBR MSRs to msrs_to_save_all list

Message ID 20221125040604.5051-9-weijiang.yang@intel.com
State New
Headers
Series Introduce Architectural LBR for vPMU |

Commit Message

Yang, Weijiang Nov. 25, 2022, 4:05 a.m. UTC
  Arch LBR MSR_ARCH_LBR_DEPTH and MSR_ARCH_LBR_CTL are queried by
userspace application before it wants to {save|restore} the Arch LBR
data. Other LBR related data MSRs are omitted here intentionally due
to lengthy list(32*3). Userspace can still use KVM_{GET|SET}_MSRS to
access them if necessary.

Suggested-by: Jim Mattson <jmattson@google.com>
Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
---
 arch/x86/kvm/x86.c | 10 ++++++++++
 1 file changed, 10 insertions(+)
  

Comments

Sean Christopherson Jan. 27, 2023, 9:43 p.m. UTC | #1
On Thu, Nov 24, 2022, Yang Weijiang wrote:
> Arch LBR MSR_ARCH_LBR_DEPTH and MSR_ARCH_LBR_CTL are queried by
> userspace application before it wants to {save|restore} the Arch LBR
> data. Other LBR related data MSRs are omitted here intentionally due
> to lengthy list(32*3). Userspace can still use KVM_{GET|SET}_MSRS to
> access them if necessary.

Absolutely not.  "there are a lot of them" isn't sufficient justification.  If
the MSRs need to be migrated, then KVM needs to report them.  If the expectation
is that XSAVES will handle them, then KVM needs to take a dependency on XSAVES
when enabling arch LBRs.
  
Yang, Weijiang Jan. 30, 2023, 12:27 p.m. UTC | #2
On 1/28/2023 5:43 AM, Sean Christopherson wrote:
> On Thu, Nov 24, 2022, Yang Weijiang wrote:
>> Arch LBR MSR_ARCH_LBR_DEPTH and MSR_ARCH_LBR_CTL are queried by
>> userspace application before it wants to {save|restore} the Arch LBR
>> data. Other LBR related data MSRs are omitted here intentionally due
>> to lengthy list(32*3). Userspace can still use KVM_{GET|SET}_MSRS to
>> access them if necessary.
> Absolutely not.  "there are a lot of them" isn't sufficient justification.  If
> the MSRs need to be migrated, then KVM needs to report them.  If the expectation
> is that XSAVES will handle them, then KVM needs to take a dependency on XSAVES
> when enabling arch LBRs.

OK, will add the full MSR list.
  

Patch

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 888a153e32bc..74c858eaa1ea 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -1466,6 +1466,7 @@  static const u32 msrs_to_save_all[] = {
 
 	MSR_IA32_XFD, MSR_IA32_XFD_ERR,
 	MSR_IA32_XSS,
+	MSR_ARCH_LBR_CTL, MSR_ARCH_LBR_DEPTH,
 };
 
 static u32 msrs_to_save[ARRAY_SIZE(msrs_to_save_all)];
@@ -3877,6 +3878,8 @@  int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
 	case MSR_IA32_PEBS_ENABLE:
 	case MSR_IA32_DS_AREA:
 	case MSR_PEBS_DATA_CFG:
+	case MSR_ARCH_LBR_CTL:
+	case MSR_ARCH_LBR_DEPTH:
 	case MSR_F15H_PERF_CTL0 ... MSR_F15H_PERF_CTR5:
 		if (kvm_pmu_is_valid_msr(vcpu, msr))
 			return kvm_pmu_set_msr(vcpu, msr_info);
@@ -3980,6 +3983,8 @@  int kvm_get_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
 	case MSR_IA32_PEBS_ENABLE:
 	case MSR_IA32_DS_AREA:
 	case MSR_PEBS_DATA_CFG:
+	case MSR_ARCH_LBR_CTL:
+	case MSR_ARCH_LBR_DEPTH:
 	case MSR_F15H_PERF_CTL0 ... MSR_F15H_PERF_CTR5:
 		if (kvm_pmu_is_valid_msr(vcpu, msr_info->index))
 			return kvm_pmu_get_msr(vcpu, msr_info);
@@ -7068,6 +7073,11 @@  static void kvm_init_msr_list(void)
 			if (!kvm_caps.supported_xss)
 				continue;
 			break;
+		case MSR_ARCH_LBR_DEPTH:
+		case MSR_ARCH_LBR_CTL:
+			if (!kvm_cpu_cap_has(X86_FEATURE_ARCH_LBR))
+				continue;
+			break;
 		default:
 			break;
 		}