From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id C2F33C7EE2F for ; Tue, 6 Jun 2023 23:40:49 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S239658AbjFFXks (ORCPT ); Tue, 6 Jun 2023 19:40:48 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:33406 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S239503AbjFFXko (ORCPT ); Tue, 6 Jun 2023 19:40:44 -0400 Received: from mail-yw1-x1149.google.com (mail-yw1-x1149.google.com [IPv6:2607:f8b0:4864:20::1149]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id C577D10F1 for ; Tue, 6 Jun 2023 16:40:42 -0700 (PDT) Received: by mail-yw1-x1149.google.com with SMTP id 00721157ae682-565a1788f3fso111025457b3.0 for ; Tue, 06 Jun 2023 16:40:42 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20221208; t=1686094842; x=1688686842; h=content-transfer-encoding:cc:to:from:subject:message-id:references :mime-version:in-reply-to:date:from:to:cc:subject:date:message-id :reply-to; bh=Ub7lhejOg/lDdTY+htbdM3tFIdBZH/I/WvN3Xdx1uV0=; b=x3+7d7dOxbuMsgi4YuWRxiMypQc2TrTU2D9YB3UiXYC+EUc7Y9pseC434rQBLcpXAL GdebpzGU+Ppi+3pg/sPm2opKG0AIj3TpKgdafc0t7y19pnJwnX6lnXEW0+OoUhdwqDwE Z4ysfR37q5DV4NBHLSUb535QOnfJ37VpXUT0s/YKbyEmYt6p3WurjeOIxM5TXMaanlts Dbq1Z9rC6GiKn/Ofl3HttKVh+kDIVJTeUF0OknTzUCQHCr+pmySor7NykqACcPQcQ0xy DY4ddSj00fmaOy6VUmaFhckuswO05xoX5292LnCzhz9+pCxJfgV+HS3Cr0N3cRaGlaIo JIXQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1686094842; x=1688686842; h=content-transfer-encoding:cc:to:from:subject:message-id:references :mime-version:in-reply-to:date:x-gm-message-state:from:to:cc:subject :date:message-id:reply-to; bh=Ub7lhejOg/lDdTY+htbdM3tFIdBZH/I/WvN3Xdx1uV0=; b=Zdi0V8RoaPpS6Uk3i+Ur+HwrVdBS1981k8aL71PofV/8zdtUJRwRq+/pw6cddoE2bZ 9yI+FW9mTTdRcCesFZVAsHY8dJ1U9q50RAaNo410rEmR6BI2brj3jfOGKM93ZW+P4aku fsshR8iNW8GS2g4QFnTDLb7GEq8bLuDoIsYMq0hIDXXsWmLkebHVwnduFwcqNoqlMogy O/fQmb0cFBD4Bwcqia8zmsao27Wf7t4owi4pTBOjwgzYj6gtvwpdKnn7YXCKCq9llOrM QT0OT1qwN/iRl/0TgZhGlyHE2C8CMZNL+9Uivwa32XXgY+QnjlTJNmNLo/E1/AB8JNtg T6rA== X-Gm-Message-State: AC+VfDwTsoZih/QIDwu2rVPg1tdhIjpYXsdZOT+trWsx4+Us34X9sp/m ePpFlRr8B3JBvd24PKgCt33QbFsqQLI= X-Google-Smtp-Source: ACHHUZ4t4b8ZaKk9b7bDtFeK18uGx3qsOG0WbmPPzW7N60k5P+UvNaqfyxvCrb9uJeLbM2s4DI7dL+WqIoo= X-Received: from zagreus.c.googlers.com ([fda3:e722:ac3:cc00:7f:e700:c0a8:5c37]) (user=seanjc job=sendgmr) by 2002:a81:d007:0:b0:568:a244:d8e5 with SMTP id v7-20020a81d007000000b00568a244d8e5mr1898740ywi.9.1686094841928; Tue, 06 Jun 2023 16:40:41 -0700 (PDT) Date: Tue, 6 Jun 2023 16:40:40 -0700 In-Reply-To: <8AE71A62-1BE3-4D6F-B57C-B5FCEA93169F@nutanix.com> Mime-Version: 1.0 References: <20230530200152.18961-1-jon@nutanix.com> <2a6502e3-ba87-0355-af09-825e8467b81f@intel.com> <8AE71A62-1BE3-4D6F-B57C-B5FCEA93169F@nutanix.com> Message-ID: Subject: Re: [PATCH] x86/fpu/xstate: clear XSAVE features if DISABLED_MASK set From: Sean Christopherson To: Jon Kohler Cc: Dave Hansen , Thomas Gleixner , Ingo Molnar , Borislav Petkov , Dave Hansen , "x86@kernel.org" , "H. Peter Anvin" , "Chang S. Bae" , Kyle Huey , "neelnatu@google.com" , Andrew Cooper , "linux-kernel@vger.kernel.org" , "kvm@vger.kernel.org" , Paolo Bonzini Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Jun 05, 2023, Jon Kohler wrote: > > On May 31, 2023, at 5:09 PM, Jon Kohler wrote: > >> The CPUID bits that enumerate support for a feature are independent fr= om the CPUID > >> bits that enumerate what XCR0 bits are supported, i.e. what features c= an be saved > >> and restored via XSAVE/XRSTOR. > >>=20 > >> KVM does mostly account for host XCR0, but in a very ad hoc way. E.g.= MPX is > >> handled by manually checking host XCR0. > >>=20 > >> if (kvm_mpx_supported()) > >> kvm_cpu_cap_check_and_set(X86_FEATURE_MPX); > >>=20 > >> PKU manually checks too, but indirectly by looking at whether or not t= he kernel > >> has enabled CR4.OSPKE. > >>=20 > >> if (!tdp_enabled || !boot_cpu_has(X86_FEATURE_OSPKE)) > >> kvm_cpu_cap_clear(X86_FEATURE_PKU); > >>=20 > >> But unless I'm missing something, the various AVX and AMX bits rely so= lely on > >> boot_cpu_data, i.e. would break if someone added CONFIG_X86_AVX or CON= FIG_X86_AMX. > >=20 > > What if we simply moved static unsigned short xsave_cpuid_features[] = =E2=80=A6 into > > xstate.h, which is already included in arch/x86/kvm/cpuid.c, and do > > something similar to what I=E2=80=99m proposing in this patch already > >=20 > > This would future proof such breakages I=E2=80=99d imagine? > >=20 > > void kvm_set_cpu_caps(void) > > { > > ... > > /* > > * Clear CPUID for XSAVE features that are disabled. > > */ > > for (i =3D 0; i < ARRAY_SIZE(xsave_cpuid_features); i++) { > > unsigned short cid =3D xsave_cpuid_features[i]; > >=20 > > /* Careful: X86_FEATURE_FPU is 0! */ > > if ((i !=3D XFEATURE_FP && !cid) || !boot_cpu_has(cid) || > > !cpu_feature_enabled(cid)) > > kvm_cpu_cap_clear(cid); > > } > > =E2=80=A6 > > } > >=20 >=20 > Sean - following up on this rough idea code above, wanted to validate tha= t > this was the direction you were thinking of having kvm_set_cpu_caps() cle= ar > caps when a particular xsave feature was disabled? Ya, more or or less. But for KVM, that should be kvm_cpu_cap_has(), not bo= ot_cpu_has(). And then I think KVM could actually WARN on a feature being disabled, i.e. = put up a tripwire to detect if things change in the future and the kernel lets the= user disable a feature that KVM wants to expose to a guest. Side topic, I find the "cid" nomenclature super confusing, and the establis= hed name in KVM is x86_feature. Something like this? for (i =3D 0; i < ARRAY_SIZE(xsave_cpuid_features); i++) { unsigned int x86_feature =3D xsave_cpuid_features[i]; if (i !=3D XFEATURE_FP && !x86_feature) continue; if (!kvm_cpu_cap_has(x86_feature)) continue; if (WARN_ON_ONCE(!cpu_feature_enabled(x86_feature))) kvm_cpu_cap_clear(x86_feature); }