| Message ID | 20210203135714.318356-5-like.xu@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: x86/pmu: Guest Architectural LBR Enabling | expand |
On 03/02/21 14:57, Like Xu wrote: > If CPUID.(EAX=07H, ECX=0):EDX[19] is exposed to 1, the KVM supports Arch > LBRs and CPUID leaf 01CH indicates details of the Arch LBRs capabilities. > As the first step, KVM only exposes the current LBR depth on the host for > guest, which is likely to be the maximum supported value on the host. > > If KVM supports XSAVES, the CPUID.(EAX=0DH, ECX=1):EDX:ECX[bit 15] > is also exposed to 1, which means the availability of support for Arch > LBR configuration state save and restore. When available, guest software > operating at CPL=0 can use XSAVES/XRSTORS manage supervisor state > component Arch LBR for own purposes once IA32_XSS [bit 15] is set. > XSAVE support for Arch LBRs is enumerated in CPUID.(EAX=0DH, ECX=0FH). > > Signed-off-by: Like Xu <like.xu@linux.intel.com> > --- > arch/x86/kvm/cpuid.c | 23 +++++++++++++++++++++++ > arch/x86/kvm/vmx/vmx.c | 2 ++ > arch/x86/kvm/x86.c | 10 +++++++++- > 3 files changed, 34 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c > index 944f518ca91b..900149eec42d 100644 > --- a/arch/x86/kvm/cpuid.c > +++ b/arch/x86/kvm/cpuid.c > @@ -778,6 +778,29 @@ static inline int __do_cpuid_func(struct kvm_cpuid_array *array, u32 function) > entry->edx = 0; > } > break; > + /* Architectural LBR */ > + case 0x1c: > + { > + u64 lbr_depth_mask = 0; > + > + if (!kvm_cpu_cap_has(X86_FEATURE_ARCH_LBR)) { > + entry->eax = entry->ebx = entry->ecx = entry->edx = 0; > + break; > + } > + > + /* > + * KVM only exposes the maximum supported depth, > + * which is also the fixed value used on the host. > + * > + * KVM doesn't allow VMM user sapce to adjust depth > + * per guest, because the guest LBR emulation depends > + * on the implementation of the host LBR driver. > + */ > + lbr_depth_mask = 1UL << fls(entry->eax & 0xff); > + entry->eax &= ~0xff; > + entry->eax |= lbr_depth_mask; > + break; > + } > /* Intel PT */ > case 0x14: > if (!kvm_cpu_cap_has(X86_FEATURE_INTEL_PT)) { > diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c > index 9ddf0a14d75c..c22175d9564e 100644 > --- a/arch/x86/kvm/vmx/vmx.c > +++ b/arch/x86/kvm/vmx/vmx.c > @@ -7498,6 +7498,8 @@ static __init void vmx_set_cpu_caps(void) > kvm_cpu_cap_check_and_set(X86_FEATURE_INVPCID); > if (vmx_pt_mode_is_host_guest()) > kvm_cpu_cap_check_and_set(X86_FEATURE_INTEL_PT); > + if (cpu_has_vmx_arch_lbr()) > + kvm_cpu_cap_check_and_set(X86_FEATURE_ARCH_LBR); > > if (vmx_umip_emulated()) > kvm_cpu_cap_set(X86_FEATURE_UMIP); > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index 667d0042d0b7..107f2e72f526 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -10385,8 +10385,16 @@ int kvm_arch_hardware_setup(void *opaque) > > if (!kvm_cpu_cap_has(X86_FEATURE_XSAVES)) > supported_xss = 0; > - else > + else { > supported_xss &= host_xss; > + /* > + * The host doesn't always set ARCH_LBR bit to hoss_xss since this > + * Arch_LBR component is used on demand in the Arch LBR driver. > + * Check e649b3f0188f "Support dynamic supervisor feature for LBR". > + */ > + if (kvm_cpu_cap_has(X86_FEATURE_ARCH_LBR)) > + supported_xss |= XFEATURE_MASK_LBR; > + } > > /* Update CET features now that supported_xss is finalized. */ > if (!kvm_cet_supported()) { > This requires some of the XSS patches that Weijang posted for CET, right? Also, who takes care of saving/restoring the MSRs, if the host has not added XFEATURE_MASK_LBR to MSR_IA32_XSS? Thanks, Paolo
On 2021/2/3 22:37, Paolo Bonzini wrote: > On 03/02/21 14:57, Like Xu wrote: >> If CPUID.(EAX=07H, ECX=0):EDX[19] is exposed to 1, the KVM supports Arch >> LBRs and CPUID leaf 01CH indicates details of the Arch LBRs capabilities. >> As the first step, KVM only exposes the current LBR depth on the host for >> guest, which is likely to be the maximum supported value on the host. >> >> If KVM supports XSAVES, the CPUID.(EAX=0DH, ECX=1):EDX:ECX[bit 15] >> is also exposed to 1, which means the availability of support for Arch >> LBR configuration state save and restore. When available, guest software >> operating at CPL=0 can use XSAVES/XRSTORS manage supervisor state >> component Arch LBR for own purposes once IA32_XSS [bit 15] is set. >> XSAVE support for Arch LBRs is enumerated in CPUID.(EAX=0DH, ECX=0FH). >> >> Signed-off-by: Like Xu <like.xu@linux.intel.com> >> --- >> arch/x86/kvm/cpuid.c | 23 +++++++++++++++++++++++ >> arch/x86/kvm/vmx/vmx.c | 2 ++ >> arch/x86/kvm/x86.c | 10 +++++++++- >> 3 files changed, 34 insertions(+), 1 deletion(-) >> >> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c >> index 944f518ca91b..900149eec42d 100644 >> --- a/arch/x86/kvm/cpuid.c >> +++ b/arch/x86/kvm/cpuid.c >> @@ -778,6 +778,29 @@ static inline int __do_cpuid_func(struct >> kvm_cpuid_array *array, u32 function) >> entry->edx = 0; >> } >> break; >> + /* Architectural LBR */ >> + case 0x1c: >> + { >> + u64 lbr_depth_mask = 0; >> + >> + if (!kvm_cpu_cap_has(X86_FEATURE_ARCH_LBR)) { >> + entry->eax = entry->ebx = entry->ecx = entry->edx = 0; >> + break; >> + } >> + >> + /* >> + * KVM only exposes the maximum supported depth, >> + * which is also the fixed value used on the host. >> + * >> + * KVM doesn't allow VMM user sapce to adjust depth >> + * per guest, because the guest LBR emulation depends >> + * on the implementation of the host LBR driver. >> + */ >> + lbr_depth_mask = 1UL << fls(entry->eax & 0xff); >> + entry->eax &= ~0xff; >> + entry->eax |= lbr_depth_mask; >> + break; >> + } >> /* Intel PT */ >> case 0x14: >> if (!kvm_cpu_cap_has(X86_FEATURE_INTEL_PT)) { >> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c >> index 9ddf0a14d75c..c22175d9564e 100644 >> --- a/arch/x86/kvm/vmx/vmx.c >> +++ b/arch/x86/kvm/vmx/vmx.c >> @@ -7498,6 +7498,8 @@ static __init void vmx_set_cpu_caps(void) >> kvm_cpu_cap_check_and_set(X86_FEATURE_INVPCID); >> if (vmx_pt_mode_is_host_guest()) >> kvm_cpu_cap_check_and_set(X86_FEATURE_INTEL_PT); >> + if (cpu_has_vmx_arch_lbr()) >> + kvm_cpu_cap_check_and_set(X86_FEATURE_ARCH_LBR); >> if (vmx_umip_emulated()) >> kvm_cpu_cap_set(X86_FEATURE_UMIP); >> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c >> index 667d0042d0b7..107f2e72f526 100644 >> --- a/arch/x86/kvm/x86.c >> +++ b/arch/x86/kvm/x86.c >> @@ -10385,8 +10385,16 @@ int kvm_arch_hardware_setup(void *opaque) >> if (!kvm_cpu_cap_has(X86_FEATURE_XSAVES)) >> supported_xss = 0; >> - else >> + else { >> supported_xss &= host_xss; >> + /* >> + * The host doesn't always set ARCH_LBR bit to hoss_xss since this >> + * Arch_LBR component is used on demand in the Arch LBR driver. >> + * Check e649b3f0188f "Support dynamic supervisor feature for >> LBR". >> + */ >> + if (kvm_cpu_cap_has(X86_FEATURE_ARCH_LBR)) >> + supported_xss |= XFEATURE_MASK_LBR; >> + } >> /* Update CET features now that supported_xss is finalized. */ >> if (!kvm_cet_supported()) { >> > > This requires some of the XSS patches that Weijang posted for CET, right? Yes, at least we need three of them for Arch LBR: 3009dfd6d61f KVM: x86: Load guest fpu state when accessing MSRs managed by XSAVES d39b0a16ad1f KVM: x86: Refresh CPUID on writes to MSR_IA32_XSS e98bf65e51c9 KVM: x86: Report XSS as an MSR to be saved if there are supported features > > Also, who takes care of saving/restoring the MSRs, if the host has not > added XFEATURE_MASK_LBR to MSR_IA32_XSS? I may not understand your concern on this. Let me try to explain: The guest Arch LBR driver will save the origin host_xss and mark the LBR bit only in the XSS and then save/restore MSRs in the extra specified guest memory, and restore the origin host_xss. On the host side, the same thing happens to vcpu thread due to the help of guest LBR event created by the vPMU and the hardware LBR MSRs are saved/restored in a exclusive way. > > Thanks, > > Paolo >
Hi Paolo, I am wondering if it is acceptable for you to review the minor Architecture LBR patch set without XSAVES for v5.12 ? As far as I know, the guest Arch LBR can still work without XSAVES support. --- thx,likexu On 2021/2/4 8:59, Xu, Like wrote: > On 2021/2/3 22:37, Paolo Bonzini wrote: >> On 03/02/21 14:57, Like Xu wrote: >>> If CPUID.(EAX=07H, ECX=0):EDX[19] is exposed to 1, the KVM supports Arch >>> LBRs and CPUID leaf 01CH indicates details of the Arch LBRs capabilities. >>> As the first step, KVM only exposes the current LBR depth on the host for >>> guest, which is likely to be the maximum supported value on the host. >>> >>> If KVM supports XSAVES, the CPUID.(EAX=0DH, ECX=1):EDX:ECX[bit 15] >>> is also exposed to 1, which means the availability of support for Arch >>> LBR configuration state save and restore. When available, guest software >>> operating at CPL=0 can use XSAVES/XRSTORS manage supervisor state >>> component Arch LBR for own purposes once IA32_XSS [bit 15] is set. >>> XSAVE support for Arch LBRs is enumerated in CPUID.(EAX=0DH, ECX=0FH). >>> >>> Signed-off-by: Like Xu <like.xu@linux.intel.com> >>> --- >>> arch/x86/kvm/cpuid.c | 23 +++++++++++++++++++++++ >>> arch/x86/kvm/vmx/vmx.c | 2 ++ >>> arch/x86/kvm/x86.c | 10 +++++++++- >>> 3 files changed, 34 insertions(+), 1 deletion(-) >>> >>> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c >>> index 944f518ca91b..900149eec42d 100644 >>> --- a/arch/x86/kvm/cpuid.c >>> +++ b/arch/x86/kvm/cpuid.c >>> @@ -778,6 +778,29 @@ static inline int __do_cpuid_func(struct >>> kvm_cpuid_array *array, u32 function) >>> entry->edx = 0; >>> } >>> break; >>> + /* Architectural LBR */ >>> + case 0x1c: >>> + { >>> + u64 lbr_depth_mask = 0; >>> + >>> + if (!kvm_cpu_cap_has(X86_FEATURE_ARCH_LBR)) { >>> + entry->eax = entry->ebx = entry->ecx = entry->edx = 0; >>> + break; >>> + } >>> + >>> + /* >>> + * KVM only exposes the maximum supported depth, >>> + * which is also the fixed value used on the host. >>> + * >>> + * KVM doesn't allow VMM user sapce to adjust depth >>> + * per guest, because the guest LBR emulation depends >>> + * on the implementation of the host LBR driver. >>> + */ >>> + lbr_depth_mask = 1UL << fls(entry->eax & 0xff); >>> + entry->eax &= ~0xff; >>> + entry->eax |= lbr_depth_mask; >>> + break; >>> + } >>> /* Intel PT */ >>> case 0x14: >>> if (!kvm_cpu_cap_has(X86_FEATURE_INTEL_PT)) { >>> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c >>> index 9ddf0a14d75c..c22175d9564e 100644 >>> --- a/arch/x86/kvm/vmx/vmx.c >>> +++ b/arch/x86/kvm/vmx/vmx.c >>> @@ -7498,6 +7498,8 @@ static __init void vmx_set_cpu_caps(void) >>> kvm_cpu_cap_check_and_set(X86_FEATURE_INVPCID); >>> if (vmx_pt_mode_is_host_guest()) >>> kvm_cpu_cap_check_and_set(X86_FEATURE_INTEL_PT); >>> + if (cpu_has_vmx_arch_lbr()) >>> + kvm_cpu_cap_check_and_set(X86_FEATURE_ARCH_LBR); >>> if (vmx_umip_emulated()) >>> kvm_cpu_cap_set(X86_FEATURE_UMIP); >>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c >>> index 667d0042d0b7..107f2e72f526 100644 >>> --- a/arch/x86/kvm/x86.c >>> +++ b/arch/x86/kvm/x86.c >>> @@ -10385,8 +10385,16 @@ int kvm_arch_hardware_setup(void *opaque) >>> if (!kvm_cpu_cap_has(X86_FEATURE_XSAVES)) >>> supported_xss = 0; >>> - else >>> + else { >>> supported_xss &= host_xss; >>> + /* >>> + * The host doesn't always set ARCH_LBR bit to hoss_xss since >>> this >>> + * Arch_LBR component is used on demand in the Arch LBR driver. >>> + * Check e649b3f0188f "Support dynamic supervisor feature for >>> LBR". >>> + */ >>> + if (kvm_cpu_cap_has(X86_FEATURE_ARCH_LBR)) >>> + supported_xss |= XFEATURE_MASK_LBR; >>> + } >>> /* Update CET features now that supported_xss is finalized. */ >>> if (!kvm_cet_supported()) { >>> >> >> This requires some of the XSS patches that Weijang posted for CET, right? > > Yes, at least we need three of them for Arch LBR: > > 3009dfd6d61f KVM: x86: Load guest fpu state when accessing MSRs managed > by XSAVES > d39b0a16ad1f KVM: x86: Refresh CPUID on writes to MSR_IA32_XSS > e98bf65e51c9 KVM: x86: Report XSS as an MSR to be saved if there are > supported features > >> >> Also, who takes care of saving/restoring the MSRs, if the host has not >> added XFEATURE_MASK_LBR to MSR_IA32_XSS? > > I may not understand your concern on this. Let me try to explain: > > The guest Arch LBR driver will save the origin host_xss and > mark the LBR bit only in the XSS and then save/restore MSRs > in the extra specified guest memory, and restore the origin host_xss. > > On the host side, the same thing happens to vcpu thread > due to the help of guest LBR event created by the vPMU > and the hardware LBR MSRs are saved/restored in a exclusive way. > >> >> Thanks, >> >> Paolo >> >
On 05/02/21 09:16, Xu, Like wrote: > Hi Paolo, > > I am wondering if it is acceptable for you to > review the minor Architecture LBR patch set without XSAVES for v5.12 ? > > As far as I know, the guest Arch LBR can still work without XSAVES > support. I dopn't think it can work. You could have two guests on the same physical CPU and the MSRs would be corrupted if the guests write to the MSR but they do not enable the LBRs. Paolo
On 2021/2/5 19:00, Paolo Bonzini wrote: > On 05/02/21 09:16, Xu, Like wrote: >> Hi Paolo, >> >> I am wondering if it is acceptable for you to >> review the minor Architecture LBR patch set without XSAVES for v5.12 ? >> >> As far as I know, the guest Arch LBR can still work without XSAVES >> support. > > I dopn't think it can work. You could have two guests on the same > physical CPU and the MSRs would be corrupted if the guests write to the > MSR but they do not enable the LBRs. > > Paolo > Neither Arch LBR nor the old version of LBR have this corruption issue, and we will not use XSAVES for at least LBR MSRs in the VMX transaction. This is because we have reused the LBR save/restore swicth support from the host perf mechanism in the legacy LBR support, which will save/restore the LBR MSRs of the vcpu (thread) when the vcpu is sched in/out. Therefore, if we have two guests on the same physical CPU, the usage of LBR MSRs is isolated, and it's also true when we use LBR to trace the hypervisor on the host. The same thing happens on the platforms which supports Arch LBR. I propose that we don't support using XSAVES to save/restore Arch LRB *in the guest* (just like the guest Intel PT), but use the traditional RD/WRMSR, which still works like the legacy LBR. Since we already have legacy LBR support, we can add a small amount of effort (just two more MSRs emulation and related CPUID exposure) to support Arch LBR w/o XSAVES. I estimate that there are many issues we need to address when we supporting guests to use xsaves instructions. As a rational choice, we could enable the basic Arch LBR. Paolo and Sean, what do you think ? --- thx, likexu
On 07/02/21 02:02, Xu, Like wrote: > On 2021/2/5 19:00, Paolo Bonzini wrote: >> On 05/02/21 09:16, Xu, Like wrote: >>> Hi Paolo, >>> >>> I am wondering if it is acceptable for you to >>> review the minor Architecture LBR patch set without XSAVES for v5.12 ? >>> >>> As far as I know, the guest Arch LBR can still work without XSAVES >>> support. >> >> I dopn't think it can work. You could have two guests on the same >> physical CPU and the MSRs would be corrupted if the guests write to >> the MSR but they do not enable the LBRs. >> >> Paolo >> > Neither Arch LBR nor the old version of LBR have this corruption issue, > and we will not use XSAVES for at least LBR MSRs in the VMX transaction. > > This is because we have reused the LBR save/restore swicth support from the > host perf mechanism in the legacy LBR support, which will save/restore > the LBR > MSRs of the vcpu (thread) when the vcpu is sched in/out. > > Therefore, if we have two guests on the same physical CPU, the usage of > LBR MSRs > is isolated, and it's also true when we use LBR to trace the hypervisor > on the host. > The same thing happens on the platforms which supports Arch LBR. > > I propose that we don't support using XSAVES to save/restore Arch LRB > *in the guest* > (just like the guest Intel PT), but use the traditional RD/WRMSR, which > still works > like the legacy LBR. Ok, this makes sense. I'll review the patches more carefully, looking at 5.13 for the target. Paolo > Since we already have legacy LBR support, we can add a small amount of > effort (just > two more MSRs emulation and related CPUID exposure) to support Arch LBR > w/o XSAVES. > > I estimate that there are many issues we need to address when we > supporting guests > to use xsaves instructions. As a rational choice, we could enable the > basic Arch LBR. > > Paolo and Sean, what do you think ? > > --- > thx, likexu >
Hi Paolo, Do we have a chance to make Arch LBR into the mainline in the upcoming merger window? https://lore.kernel.org/kvm/20210314155225.206661-1-like.xu@linux.intel.com/ Thanks, Like Xu On 2021/2/8 18:31, Paolo Bonzini wrote: > Ok, this makes sense. I'll review the patches more carefully, looking at > 5.13 for the target. > > Paolo
diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c index 944f518ca91b..900149eec42d 100644 --- a/arch/x86/kvm/cpuid.c +++ b/arch/x86/kvm/cpuid.c @@ -778,6 +778,29 @@ static inline int __do_cpuid_func(struct kvm_cpuid_array *array, u32 function) entry->edx = 0; } break; + /* Architectural LBR */ + case 0x1c: + { + u64 lbr_depth_mask = 0; + + if (!kvm_cpu_cap_has(X86_FEATURE_ARCH_LBR)) { + entry->eax = entry->ebx = entry->ecx = entry->edx = 0; + break; + } + + /* + * KVM only exposes the maximum supported depth, + * which is also the fixed value used on the host. + * + * KVM doesn't allow VMM user sapce to adjust depth + * per guest, because the guest LBR emulation depends + * on the implementation of the host LBR driver. + */ + lbr_depth_mask = 1UL << fls(entry->eax & 0xff); + entry->eax &= ~0xff; + entry->eax |= lbr_depth_mask; + break; + } /* Intel PT */ case 0x14: if (!kvm_cpu_cap_has(X86_FEATURE_INTEL_PT)) { diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c index 9ddf0a14d75c..c22175d9564e 100644 --- a/arch/x86/kvm/vmx/vmx.c +++ b/arch/x86/kvm/vmx/vmx.c @@ -7498,6 +7498,8 @@ static __init void vmx_set_cpu_caps(void) kvm_cpu_cap_check_and_set(X86_FEATURE_INVPCID); if (vmx_pt_mode_is_host_guest()) kvm_cpu_cap_check_and_set(X86_FEATURE_INTEL_PT); + if (cpu_has_vmx_arch_lbr()) + kvm_cpu_cap_check_and_set(X86_FEATURE_ARCH_LBR); if (vmx_umip_emulated()) kvm_cpu_cap_set(X86_FEATURE_UMIP); diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 667d0042d0b7..107f2e72f526 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -10385,8 +10385,16 @@ int kvm_arch_hardware_setup(void *opaque) if (!kvm_cpu_cap_has(X86_FEATURE_XSAVES)) supported_xss = 0; - else + else { supported_xss &= host_xss; + /* + * The host doesn't always set ARCH_LBR bit to hoss_xss since this + * Arch_LBR component is used on demand in the Arch LBR driver. + * Check e649b3f0188f "Support dynamic supervisor feature for LBR". + */ + if (kvm_cpu_cap_has(X86_FEATURE_ARCH_LBR)) + supported_xss |= XFEATURE_MASK_LBR; + } /* Update CET features now that supported_xss is finalized. */ if (!kvm_cet_supported()) {
If CPUID.(EAX=07H, ECX=0):EDX[19] is exposed to 1, the KVM supports Arch LBRs and CPUID leaf 01CH indicates details of the Arch LBRs capabilities. As the first step, KVM only exposes the current LBR depth on the host for guest, which is likely to be the maximum supported value on the host. If KVM supports XSAVES, the CPUID.(EAX=0DH, ECX=1):EDX:ECX[bit 15] is also exposed to 1, which means the availability of support for Arch LBR configuration state save and restore. When available, guest software operating at CPL=0 can use XSAVES/XRSTORS manage supervisor state component Arch LBR for own purposes once IA32_XSS [bit 15] is set. XSAVE support for Arch LBRs is enumerated in CPUID.(EAX=0DH, ECX=0FH). Signed-off-by: Like Xu <like.xu@linux.intel.com> --- arch/x86/kvm/cpuid.c | 23 +++++++++++++++++++++++ arch/x86/kvm/vmx/vmx.c | 2 ++ arch/x86/kvm/x86.c | 10 +++++++++- 3 files changed, 34 insertions(+), 1 deletion(-)