Messages in this thread | | | From | "Liu, Jing2" <> | Subject | Re: [PATCH RFC 5/7] kvm: x86: Revise CPUID.D.1.EBX for alignment rule | Date | Thu, 3 Jun 2021 12:45:48 +0800 |
| |
On 5/25/2021 5:28 AM, Sean Christopherson wrote: > On Sun, Feb 07, 2021, Jing Liu wrote: >> CPUID.0xD.1.EBX[1] is set if, when the compacted format of an XSAVE >> area is used, this extended state component located on the next >> 64-byte boundary following the preceding state component (otherwise, >> it is located immediately following the preceding state component). >> >> AMX tileconfig and tiledata are the first to use 64B alignment. >> Revise the runtime cpuid modification for this rule. >> >> Signed-off-by: Jing Liu<jing2.liu@linux.intel.com> >> --- >> arch/x86/kvm/cpuid.c | 5 +++++ >> 1 file changed, 5 insertions(+) >> >> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c >> index 04a73c395c71..ee1fac0a865e 100644 >> --- a/arch/x86/kvm/cpuid.c >> +++ b/arch/x86/kvm/cpuid.c >> @@ -35,12 +35,17 @@ static u32 xstate_required_size(u64 xstate_bv, bool compacted) >> { >> int feature_bit = 0; >> u32 ret = XSAVE_HDR_SIZE + XSAVE_HDR_OFFSET; >> + bool is_aligned = false; >> >> xstate_bv &= XFEATURE_MASK_EXTEND; >> while (xstate_bv) { >> if (xstate_bv & 0x1) { >> u32 eax, ebx, ecx, edx, offset; >> cpuid_count(0xD, feature_bit, &eax, &ebx, &ecx, &edx); >> + /* ECX[2]: 64B alignment in compacted form */ >> + is_aligned = !!(ecx & 2); >> + if (is_aligned && compacted) > I'd forego the local is_aligned, and also check "compacted" first so that the > uncompacted variant isn't required to evaluated ecx. Non-compacted only works for XCR0 (user states), do we need add a check or simply do 'state_bv &= XFEATURE_MASK_USER_ENABLED'?
xstate_bv &= XFEATURE_MASK_EXTEND; + /* Only user states use non-compacted format. */ + if (!compacted) + xstate_bv &= XFEATURE_MASK_USER_SUPPORTED; + while (xstate_bv) { if (xstate_bv & 0x1) { u32 eax, ebx, ecx, edx, offset; cpuid_count(0xD, feature_bit, &eax, &ebx, &ecx, &edx); - offset = compacted ? ret : ebx; + offset = compacted ? ((ecx & 0x2) ? ALIGN(ret, 64) : ret) : ebx; ret = max(ret, offset + eax); }
> > And the real reason I am responding... can you post this as a standalone patch? Sure. Let me separate it. > I stumbled across the "aligned" flag when reading through the SDM for a completely > unrelated reason, and also discovered that the flag has been documented since > 2016. While AMX may be the first to "officially" utilize the alignment flag, > the flag itself is architectural and not strictly limited to AMX. Yes, this is not a new feature, but seems no one use it before.
Thanks, Jing >> + ret = ALIGN(ret, 64); >> offset = compacted ? ret : ebx; >> ret = max(ret, offset + eax); >> } >> -- >> 2.18.4 >>
| |