Re: [RFC PATCH] x86/fpu/xstate: Add more diagnostic information on inconsistent xstate sizes

From: Fenghua Yu
Date: Mon Apr 10 2023 - 16:43:01 EST


Hi, Chang,

On 4/7/23 11:22, Chang S. Bae wrote:
On 4/5/2023 11:39 AM, Fenghua Yu wrote:
A warning is emitted when xstate sizes are found inconsistent.
But the warning message doesn't show enough information to diagnose
the issue.

Provide more detailed xstate size information to help debug the issue.
As a hypothetical example, on a platform that may report incorrect
xstate size in CPUID.0xd.1:EBX, the diagnostic information after
the warning will show:
[    0.000000] x86/fpu: max_features=0x407
[    0.000000] x86/fpu: xstate_offset[2]:  576, xstate_sizes[2]:  256
[    0.000000] x86/fpu: xstate_offset[10]: 832, xstate_sizes[10]:    8
[    0.000000] x86/fpu: total size: 840 bytes
[    0.000000] x86/fpu: XCR0=0x7, IA32_XSS=0x400
[    0.000000] x86/fpu: kernel_size from CPUID.0xd.0x1:EBX: 576 bytes

XCR0 | IA32_XSS is 0x407 which is consistent with max_features.
CPUID.0xd.0x1:EBX should report the size of the xsave area
containing xstate components corresponding to bits set in
XCR0 | IA32_XSS. But it only reports 576 bytes of xsave area which
doesn't count xstate sizes for AVX (offset 2 and 256 bytes) and
PASID (offset 10 and 8 bytes). This confirms that the platform
reports xstate size incorrectly through the CPUID bits.

Suggest-by: Dave Hansen <dave.hansen@xxxxxxxxxxxxxxx>
Tested-by: Chintan M Patel <chintan.m.patel@xxxxxxxxx>
Signed-off-by: Fenghua Yu <fenghua.yu@xxxxxxxxx>
---
  arch/x86/kernel/fpu/xstate.c | 33 +++++++++++++++++++++++++++++++--
  1 file changed, 31 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
index 0bab497c9436..5f27fcdc6c90 100644
--- a/arch/x86/kernel/fpu/xstate.c
+++ b/arch/x86/kernel/fpu/xstate.c
@@ -602,8 +602,37 @@ static bool __init paranoid_xstate_size_valid(unsigned int kernel_size)
          }
      }
      size = xstate_calculate_size(fpu_kernel_cfg.max_features, compacted);
-    XSTATE_WARN_ON(size != kernel_size,
-               "size %u != kernel_size %u\n", size, kernel_size);
+    if (size != kernel_size) {
+        u64 xcr0, ia32_xss;
+
+        XSTATE_WARN_ON(1, "size %u != kernel_size %u\n",
+                   size, kernel_size);
+
+        /* Show more information to help diagnose the size issue. */
+        pr_info("x86/fpu: max_features=0x%llx\n",
+            fpu_kernel_cfg.max_features);
+        print_xstate_offset_size();
+        pr_info("x86/fpu: total size: %u bytes\n", size);
+        xcr0 = xgetbv(XCR_XFEATURE_ENABLED_MASK);
+        if (compacted) {
+            rdmsrl(MSR_IA32_XSS, ia32_xss);

This shouldn't be directly read here because of the LBR state component.

See the function comment:

 * Independent XSAVE features allocate their own buffers and are not
 * covered by these checks. Only the size of the buffer for task->fpu
 * is checked here.

But, isn't that max_features bitmask pretty much about it?

How about getting IA32_XSS from xfeatures_mask_supervisor()? That's how to get kernel_size by setting IA32_XSS without independent features in get_xsave_compacted_size().

Thanks.

-Fenghua