Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 25 May 2023 14:10:10 +0800
From:   Chao Gao <chao.gao@intel.com>
To:     Yang Weijiang <weijiang.yang@intel.com>
CC:     <seanjc@google.com>, <pbonzini@redhat.com>, <kvm@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>, <peterz@infradead.org>,
        <rppt@kernel.org>, <binbin.wu@linux.intel.com>,
        <rick.p.edgecombe@intel.com>, <john.allen@amd.com>,
        Zhang Yi Z <yi.z.zhang@linux.intel.com>
Subject: Re: [PATCH v3 07/21] KVM:x86: Refresh CPUID on write to guest
 MSR_IA32_XSS
Message-ID: <ZG77wu4PyBn1z587@chao-email>
References: <20230511040857.6094-1-weijiang.yang@intel.com>
 <20230511040857.6094-8-weijiang.yang@intel.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20230511040857.6094-8-weijiang.yang@intel.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?JrZRjt5KeFGIpyrrk9cWi061g8CvthrNghd4TQRNOGuiMnIgKvhUGnJzKqav?=
 =?us-ascii?Q?BqeVr/YobIEjjg1GRgcTXIv3narwGhgcz+YEFTdjEo0Gzysb63Sdz8EJD6Zz?=
 =?us-ascii?Q?kOeuSD39q6GuqQvwbvES1O8Rfs7rBSVvBFEkvQuplKK49kpD8wxOPupt+sm6?=
 =?us-ascii?Q?aCGCzBcWwtlMxwqXvOV1gHiaQqZk/o1bmpsjGjbfKp2W20laTy0e0AC1/VQw?=
 =?us-ascii?Q?2FaIF+vzUMSVB6UetPspNFad2XPnAqch9tiEskwNeQrhqdznSzW4x1L0pYPB?=
 =?us-ascii?Q?+YGlm+50HcBOLzy9tG9hguFV8vfuUsJbBEXi/5g6E7LT0txRlx+EHPji15d7?=
 =?us-ascii?Q?HM76XQdb0cTdVf8VL7+y5TYvQhy9f1OVGUehTGrP5oJkz7ryfvXqxkDNIpOh?=
 =?us-ascii?Q?UpGNTxwQHYypDOq241AhsQ7zGe14kmwIXxDJkbIyF3ML18ieVKs2J5ZNjRVm?=
 =?us-ascii?Q?KmnxgKJDIILKhPaMQE9JFfCoMziIDmoYqmYXq0VF00DM3Jd1j0YFSUljl8F+?=
 =?us-ascii?Q?5d30B2kDkNiSUrlB6SwDi/I3hM3Wye3RfVBDy81NpxLkHJJTd+O/w2P80Oqr?=
 =?us-ascii?Q?MPouBUqZTb5PAikGSQOpNw7w4oekRJen4DoMHpOW24O3qtgRcF7L3F/rsWqA?=
 =?us-ascii?Q?Ro1jEI7/UheI7eTV55ifO4dmxHHa2nD3NUI2/iGry4oGCxpMSKwSMPzu5vzd?=
 =?us-ascii?Q?J377d/r1HMFbtcJafAQ+SODWND+UthxHzqURB3HCIUf/z9EKS2n4a0elrBCl?=
 =?us-ascii?Q?fDxTpR2r0Hu2xZB+PecRV8B1z0NJWXCF0qd+HK+8x/2Yi8gsEK2DWtS8qPca?=
 =?us-ascii?Q?x5mhg0jQMj29EynTo9YySrWXmzOgitU1O8ZKEyb0PUN2o1Cilv8eVgV1AgTj?=
 =?us-ascii?Q?06O6ZMVyPKRGgoLcdiPCOGEKmeXYRXfSdg7JfDortK3Mls6yChskRk5bnskH?=
 =?us-ascii?Q?sk7J8yQgjkKz/RHJh4tXK5nhEPCqfvoR+xyLpy58pcO9nsEe881hXlyG5ClG?=
 =?us-ascii?Q?s63fZW16YbIh8Y2RHKS6crzM/3eLd+yKWR6Dq5QiI0FfmZ14C1hqolnsZ86c?=
 =?us-ascii?Q?D2qNKiAMIP1xk0KYwJG0gBYXzYpTvr/Bw0K5IPLCN77koGb1QMvhnYwo9WOh?=
 =?us-ascii?Q?9Ey76OytuasZDosi2aUPRljphbM7lIQ4i8XXgjp8i4tZdUg9XNamg3YlNAht?=
 =?us-ascii?Q?MsMiU4bUjMliS2MPyU+0ALnWfwvcpvi6gj5756axsf7cMo1p5rqlMlcGmcCI?=
 =?us-ascii?Q?be5IH6ELZLfAVQXjHqI4sJDXMtPDV3ZnFFp4zV0/KNeodubDGwe8A48tArgL?=
 =?us-ascii?Q?lX17amhIvQfPGY2UFwSdBCBc8SUCdYaM8Frd46AaDGWJQFOMLPWCuWWSel9k?=
 =?us-ascii?Q?yM9tx1U9FTtNoZE2MyDH2NPebSQf6y9DkpJoxNMrCmIIIi6Fn/C0fxVkbzjW?=
 =?us-ascii?Q?jZrX53HvYh/6ifRIww3+ITyi1ojftS1pQ9CMAojSMl6z2PViN87I3M3rpYqG?=
 =?us-ascii?Q?EvjYmPJcS0MnRO9O/M/vbs8M+aDdGszRsVDefsz1ZqRJonLp6/Xhbs9vRSHu?=
 =?us-ascii?Q?bxz9AeSfjEK0YjPBe9s8m9Gp7z+nWL6x6KWxhwAM?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 78f344ee-80b6-4376-d8c4-08db5ce6b87a
X-MS-Exchange-CrossTenant-AuthSource: PH8PR11MB6780.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 25 May 2023 06:10:21.2119
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: ms4+hpYTWGufqbRWtKMtJSxFryduUZkisgprpNZNDbkV0//JzQM7ul/IHDzAepTVoHDTqvwowjB2wnaDFtqUvA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DS0PR11MB7359
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-4.4 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_MED,
        SPF_HELO_NONE,SPF_NONE,T_SCC_BODY_TEXT_LINE,URIBL_BLOCKED
        autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Thu, May 11, 2023 at 12:08:43AM -0400, Yang Weijiang wrote:
>Update CPUID(EAX=0DH,ECX=1) when the guest's XSS is modified.
>CPUID(EAX=0DH,ECX=1).EBX reports current required storage size for all
>features enabled via XCR0 | XSS so that guest can allocate correct xsave
>buffer.
>
>Note, KVM does not yet support any XSS based features, i.e. supported_xss
>is guaranteed to be zero at this time.
>
>Co-developed-by: Zhang Yi Z <yi.z.zhang@linux.intel.com>
>Signed-off-by: Zhang Yi Z <yi.z.zhang@linux.intel.com>
>Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
>---
> arch/x86/kvm/cpuid.c | 7 +++++--
> arch/x86/kvm/x86.c   | 6 ++++--
> 2 files changed, 9 insertions(+), 4 deletions(-)
>
>diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
>index 123bf8b97a4b..cbb1b8a65502 100644
>--- a/arch/x86/kvm/cpuid.c
>+++ b/arch/x86/kvm/cpuid.c
>@@ -277,8 +277,11 @@ static void __kvm_update_cpuid_runtime(struct kvm_vcpu *vcpu, struct kvm_cpuid_e
> 
> 	best = cpuid_entry2_find(entries, nent, 0xD, 1);
> 	if (best && (cpuid_entry_has(best, X86_FEATURE_XSAVES) ||
>-		     cpuid_entry_has(best, X86_FEATURE_XSAVEC)))
>-		best->ebx = xstate_required_size(vcpu->arch.xcr0, true);
>+		cpuid_entry_has(best, X86_FEATURE_XSAVEC))) {

Align indentation.

 	if (best && (cpuid_entry_has(best, X86_FEATURE_XSAVES) ||
		     cpuid_entry_has(best, X86_FEATURE_XSAVEC))) {

>+		u64 xstate = vcpu->arch.xcr0 | vcpu->arch.ia32_xss;
>+
>+		best->ebx = xstate_required_size(xstate, true);
>+	}
> 
> 	best = __kvm_find_kvm_cpuid_features(vcpu, entries, nent);
> 	if (kvm_hlt_in_guest(vcpu->kvm) && best &&
>diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>index 33a780fe820b..ab3360a10933 100644
>--- a/arch/x86/kvm/x86.c
>+++ b/arch/x86/kvm/x86.c
>@@ -3776,8 +3776,10 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
> 		 */
> 		if (data & ~kvm_caps.supported_xss)

Shouldn't we check against the supported value of _this_ guest? similar to
guest_supported_xcr0.

> 			return 1;
>-		vcpu->arch.ia32_xss = data;
>-		kvm_update_cpuid_runtime(vcpu);
>+		if (vcpu->arch.ia32_xss != data) {
>+			vcpu->arch.ia32_xss = data;
>+			kvm_update_cpuid_runtime(vcpu);
>+		}
> 		break;
> 	case MSR_SMI_COUNT:
> 		if (!msr_info->host_initiated)
>-- 
>2.27.0
>