Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20210422001736.3255735-1-seanjc@google.com> <CAAeT=FxaRV+za7yk8_9p45k4ui3QJx90gN4b8k4egrxux=QWFA@mail.gmail.com>
 <YIHYsa1+psfnszcv@google.com>
In-Reply-To: <YIHYsa1+psfnszcv@google.com>
From:   Reiji Watanabe <reijiw@google.com>
Date:   Thu, 22 Apr 2021 15:39:07 -0700
Message-ID: <CAAeT=FwXuaHM+_ZpoCwHgXyBtSn_gA3r8j+gZ9rfgUBxEwkpWA@mail.gmail.com>
Subject: Re: [PATCH v2] KVM: SVM: Delay restoration of host MSR_TSC_AUX until
 return to userspace
To:     Sean Christopherson <seanjc@google.com>
Cc:     Paolo Bonzini <pbonzini@redhat.com>,
        Vitaly Kuznetsov <vkuznets@redhat.com>,
        Wanpeng Li <wanpengli@tencent.com>,
        Jim Mattson <jmattson@google.com>,
        Joerg Roedel <joro@8bytes.org>, kvm@vger.kernel.org,
        linux-kernel@vger.kernel.org,
        Tom Lendacky <thomas.lendacky@amd.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

> All in all, I think we want this:
>
>         case MSR_TSC_AUX:
>                 if (!boot_cpu_has(X86_FEATURE_RDTSCP))
>                         return 1;
>
>                 if (!msr_info->host_initiated &&
>                     !guest_cpuid_has(vcpu, X86_FEATURE_RDTSCP))
>                         return 1;
>
>                 /*
>                  * TSC_AUX is usually changed only during boot and never read
>                  * directly.  Intercept TSC_AUX instead of exposing it to the
>                  * guest via direct_access_msrs, and switch it via user return.
>                  */
>                 preempt_disable();
>                 r = kvm_set_user_return_msr(TSC_AUX_URET_SLOT, data, -1ull);
>                 preempt_enable();
>                 if (r)
>                         return 1;
>
>                 /*
>                  * Bits 63:32 are dropped by AMD CPUs, but are reserved on
>                  * Intel CPUs.  AMD's APM has incomplete and conflicting info
>                  * on the architectural behavior; emulate current hardware as
>                  * doing so ensures migrating from AMD to Intel won't explode.
>                  */
>                 svm->tsc_aux = (u32)data;
>                 break;


Thank you for the explanation.
I understand and the code above looks good to me.
(I would assume we want to check the msr_info->host_initiated and
guest_cpuid_has in svm_get_msr as well)

Thanks,
Reiji