[PATCH 3/4] x86: Adjust arch LBR CPU policy

ngoc-tu.dinh@vates.tech posted 4 patches 1 year, 2 months ago
There is a newer version of this series
[PATCH 3/4] x86: Adjust arch LBR CPU policy
Posted by ngoc-tu.dinh@vates.tech 1 year, 2 months ago
From: Tu Dinh <ngoc-tu.dinh@vates.tech>

Allow virtual arch LBR with a single depth that's equal to that of the
host. If this is not possible, disable arch LBR altogether.

Signed-off-by: Tu Dinh <ngoc-tu.dinh@vates.tech>
---
 xen/arch/x86/cpu-policy.c | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/xen/arch/x86/cpu-policy.c b/xen/arch/x86/cpu-policy.c
index cf6b212fb6..2ac76eb058 100644
--- a/xen/arch/x86/cpu-policy.c
+++ b/xen/arch/x86/cpu-policy.c
@@ -638,6 +638,36 @@ static void __init calculate_pv_max_policy(void)
     p->extd.raw[0xa] = EMPTY_LEAF; /* No SVM for PV guests. */
 }
 
+/*
+ * Allow virtual arch LBR with a single depth that's equal to that of the
+ * host. If this is not possible, disable arch LBR altogether.
+ */
+static void adjust_arch_lbr_depth(uint32_t fs[FEATURESET_NR_ENTRIES])
+{
+    uint64_t host_lbr_depth;
+    bool lbr_supported = true;
+
+    rdmsrl(MSR_IA32_LASTBRANCH_DEPTH, host_lbr_depth);
+    if ((host_lbr_depth == 0) ||
+        (host_lbr_depth % 8) ||
+        (host_lbr_depth > 64))
+        lbr_supported = false;
+
+    host_lbr_depth = 1ul << ((host_lbr_depth / 8) - 1);
+    if ((host_lbr_depth & fs[FEATURESET_1Ca] & 0xff) == 0)
+        lbr_supported = false;
+
+    if (lbr_supported)
+    {
+        fs[FEATURESET_1Ca] = (fs[FEATURESET_1Ca] & ~0xffu) | host_lbr_depth;
+    }
+    else
+    {
+        __clear_bit(X86_FEATURE_ARCH_LBR, fs);
+        fs[FEATURESET_1Ca] = fs[FEATURESET_1Cb] = fs[FEATURESET_1Cc] = 0;
+    }
+}
+
 static void __init calculate_pv_def_policy(void)
 {
     struct cpu_policy *p = &pv_def_cpu_policy;
@@ -760,6 +790,9 @@ static void __init calculate_hvm_max_policy(void)
             __clear_bit(X86_FEATURE_XSAVES, fs);
     }
 
+    if ( test_bit(X86_FEATURE_ARCH_LBR, fs) )
+        adjust_arch_lbr_depth(fs);
+
     /*
      * Xen doesn't use PKS, so the guest support for it has opted to not use
      * the VMCS load/save controls for efficiency reasons.  This depends on
-- 
2.43.0



Ngoc Tu Dinh | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech
Re: [PATCH 3/4] x86: Adjust arch LBR CPU policy
Posted by Jan Beulich 1 year, 2 months ago
On 18.11.2024 09:49, ngoc-tu.dinh@vates.tech wrote:
> From: Tu Dinh <ngoc-tu.dinh@vates.tech>
> 
> Allow virtual arch LBR with a single depth that's equal to that of the
> host. If this is not possible, disable arch LBR altogether.

How will this work across migration?

What does "single depth that's equal to that of the host" mean, when
multiple depths can be advertised as supported? Perhaps I'm irritated
by ...

> --- a/xen/arch/x86/cpu-policy.c
> +++ b/xen/arch/x86/cpu-policy.c
> @@ -638,6 +638,36 @@ static void __init calculate_pv_max_policy(void)
>      p->extd.raw[0xa] = EMPTY_LEAF; /* No SVM for PV guests. */
>  }
>  
> +/*
> + * Allow virtual arch LBR with a single depth that's equal to that of the
> + * host. If this is not possible, disable arch LBR altogether.
> + */
> +static void adjust_arch_lbr_depth(uint32_t fs[FEATURESET_NR_ENTRIES])
> +{
> +    uint64_t host_lbr_depth;
> +    bool lbr_supported = true;
> +
> +    rdmsrl(MSR_IA32_LASTBRANCH_DEPTH, host_lbr_depth);

... you reading an MSR here which was never set by Xen. Whatever the firmware
left there should not be relevant to us. 

> +    if ((host_lbr_depth == 0) ||
> +        (host_lbr_depth % 8) ||
> +        (host_lbr_depth > 64))
> +        lbr_supported = false;

Here and below: Please familiarize yourself with Xen coding style. if() and
alike want blanks immediately inside the parentheses.

> +    host_lbr_depth = 1ul << ((host_lbr_depth / 8) - 1);
> +    if ((host_lbr_depth & fs[FEATURESET_1Ca] & 0xff) == 0)
> +        lbr_supported = false;
> +
> +    if (lbr_supported)
> +    {
> +        fs[FEATURESET_1Ca] = (fs[FEATURESET_1Ca] & ~0xffu) | host_lbr_depth;
> +    }
> +    else
> +    {
> +        __clear_bit(X86_FEATURE_ARCH_LBR, fs);
> +        fs[FEATURESET_1Ca] = fs[FEATURESET_1Cb] = fs[FEATURESET_1Cc] = 0;
> +    }
> +}

Hmm, is it really a good idea to fiddle with the featureset, rather than (after
conversion back) with the policy? Cc-ing Andrew. This would then also avoid use
of several plain integer literals.

Jan