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 AEBB3C7EE22 for ; Thu, 4 May 2023 17:24:04 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229806AbjEDRYD (ORCPT ); Thu, 4 May 2023 13:24:03 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:55046 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229553AbjEDRYA (ORCPT ); Thu, 4 May 2023 13:24:00 -0400 Received: from mail-yb1-xb4a.google.com (mail-yb1-xb4a.google.com [IPv6:2607:f8b0:4864:20::b4a]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id DF64E449C for ; Thu, 4 May 2023 10:23:59 -0700 (PDT) Received: by mail-yb1-xb4a.google.com with SMTP id 3f1490d57ef6-ba1b052e53dso1147703276.1 for ; Thu, 04 May 2023 10:23:59 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20221208; t=1683221039; x=1685813039; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:from:to:cc:subject:date:message-id:reply-to; bh=ONB00+6Q69z3/h03ZXeAoDFdXXdOjEaMLtfkrHED77E=; b=a19ly5QJ1Q1N22wDXhgi9csT1yq+d14FzfsBWgUtF4wbUqyCVwSYl75NVDF+a4VtDE Ii9TlZsiACL2LTTI08ua87HBuUovZFuBkxtAs65RU+NYy+LOeL81Yx25IxhPkStxvE4a puKgvwWz8lVMfccj9E41YPBeKftkVqVLBUUOm8MaEYjLyhmqBwWKsChhLCXouqxxIb0V ZxHq+oddfmEkNiR82cqG4X6vidPMoanq8BFAbBQA+v97hN4fxCRmNe8QoTf169a64ul8 +l0aISINsGVg9egilXnwsORqMbtmn6H6Vr9fiRayC/WofHyGLboRtxmtxwXGO5i2CIIe IROA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1683221039; x=1685813039; h=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=ONB00+6Q69z3/h03ZXeAoDFdXXdOjEaMLtfkrHED77E=; b=WJcBCOYGq7gEfSdliHuj5WcVEka2Obkx630FqE3Sej3tG0LHJ3gwjSnKb9CWUk46qu U/aIe68VsE2TQg+lTAZEVAoHzvQnSbwl3K3XEAWdSaE+OWKPVVi21gDmp+kwtwhlYJPj 7aWn6suUWNu4cnyhwP99Ws8pyrtOrfm+TMvhj+4ndiD45GtVvSwbxdbVZe7k7NpVemjy 5SXgPwmOXpP1cc9XM56yR6iIvi5gkMKNagLCwWMABCZoaozu5I2ZWF9baP9XQas4SnKX l1uIjrWe8hP/FIdYM+voxO2t97V1F5ziIrHoWK6Zm1bcgAu1RRvfHNC/Pb9hMwmjO2Jn fmYA== X-Gm-Message-State: AC+VfDyQyoYu28lprRGMnLyHozsrJflvo3mSI0XPwgoTAjfvjgAxXFN0 iVRtxOhE5/Uj7eSEloQ2kkuu2K4H6OA= X-Google-Smtp-Source: ACHHUZ6nT2T395WRubeB3aSWGrfqRKKIgCUhsMtSEWpYYWZrkznq+sx7rqITmcWPGdxyYiBgCtn8mm8/UAQ= X-Received: from zagreus.c.googlers.com ([fda3:e722:ac3:cc00:7f:e700:c0a8:5c37]) (user=seanjc job=sendgmr) by 2002:a25:8b03:0:b0:b9d:f5b9:3d58 with SMTP id i3-20020a258b03000000b00b9df5b93d58mr281680ybl.3.1683221039062; Thu, 04 May 2023 10:23:59 -0700 (PDT) Date: Thu, 4 May 2023 10:23:57 -0700 In-Reply-To: <34f44b2748ad1365907c7927a3cbee794b986243.camel@intel.com> Mime-Version: 1.0 References: <20230503182852.3431281-1-seanjc@google.com> <20230503182852.3431281-2-seanjc@google.com> <56fa0da83203b20c9945e7c82a0eed11f3d31ea5.camel@intel.com> <34f44b2748ad1365907c7927a3cbee794b986243.camel@intel.com> Message-ID: Subject: Re: [PATCH 1/5] KVM: VMX: Open code writing vCPU's PAT in VMX's MSR handler From: Sean Christopherson To: Kai Huang Cc: "kvm@vger.kernel.org" , "pbonzini@redhat.com" , "guoke@uniontech.com" , "linux-kernel@vger.kernel.org" , "haiwenyao@uniontech.com" Content-Type: text/plain; charset="us-ascii" Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, May 03, 2023, Kai Huang wrote: > On Wed, 2023-05-03 at 16:25 -0700, Sean Christopherson wrote: > > On Wed, May 03, 2023, Kai Huang wrote: > > > On Wed, 2023-05-03 at 11:28 -0700, Sean Christopherson wrote: > > > > From: Wenyao Hai > > > > > > > > Open code setting "vcpu->arch.pat" in vmx_set_msr() instead of bouncing > > > > through kvm_set_msr_common() to get to the same code in kvm_mtrr_set_msr(). > > > > > > What's the value of doing so, besides saving a function of kvm_set_msr_common()? > > > > To avoid complicating a very simple operation (writing vcpu->arch.pat), and to > > align with SVM. > > > > > PAT change shouldn't be something frequent so shouldn't in a performance > > > critical path. Given the PAT logic on Intel and AMD are basically the same , > > > isn't it better to do in kvm_set_msr_common()? > > > > I could go either way on calling into kvm_set_msr_common(). I agree that > > performance isn't a concern. Hmm, and kvm_set_msr_common() still has a case > > statement for MSR_IA32_CR_PAT, so handling the write fully in vendor code won't > > impact the code generation for other MSRs. > > > > Though I am leaning towards saying we should either handle loads and stores to > > vcpu->arch.pat in common code _or_ vendor code, i.e. either teach VMX and SVM to > > handle reads of PAT, or have their write paths call kvm_set_msr_common(). A mix > > of both is definitely odd. > > Agreed. Alternatively we can move SVM's setting vcpu->arch.pat to common code. > > > > > I don't have strong preference on which of those two we choose. I dislike duplicating > > logic across VMX and SVM, but on the other hands it's so little code. I think > > I'd vote for handling everything in vendor code, mostly because this gives the > > appearance that the write can fail, which is silly and misleading. > > > > ret = kvm_set_msr_common(vcpu, msr_info); > > No opinion either. First glance is having > > case MSR_IA32_CR_PAT: > vcpu->arch.pat = data; > > in kvm_set_msr_common() is clearer because it is symmetrical to the read path. > > Anyway your decision :) Duh, the obvious answer is to do ret = kvm_set_msr_common(vcpu, msr_info); if (ret) break; That's an established pattern for other MSRs, and addresses my main concern of not unwinding the VMCS updates in the should-be-impossible scenario of kvm_set_msr_common() failing after the kvm_pat_valid() check. Thanks Kai!