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:   Tue, 1 Nov 2022 16:11:32 +0000
From:   Sean Christopherson <seanjc@google.com>
To:     Vitaly Kuznetsov <vkuznets@redhat.com>
Cc:     kvm@vger.kernel.org, Paolo Bonzini <pbonzini@redhat.com>,
        Wanpeng Li <wanpengli@tencent.com>,
        Jim Mattson <jmattson@google.com>,
        Michael Kelley <mikelley@microsoft.com>,
        Siddharth Chandrasekaran <sidcha@amazon.de>,
        Yuan Yao <yuan.yao@linux.intel.com>,
        Maxim Levitsky <mlevitsk@redhat.com>,
        linux-hyperv@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v13 45/48] KVM: selftests: Introduce rdmsr_from_l2() and
 use it for MSR-Bitmap tests
Message-ID: <Y2FFNO3Bu9Z3LtCW@google.com>
References: <20221101145426.251680-1-vkuznets@redhat.com>
 <20221101145426.251680-46-vkuznets@redhat.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20221101145426.251680-46-vkuznets@redhat.com>
Precedence: bulk

On Tue, Nov 01, 2022, Vitaly Kuznetsov wrote:
> Hyper-V MSR-Bitmap tests do RDMSR from L2 to exit to L1. While 'evmcs_test'
> correctly clobbers all GPRs (which are not preserved), 'hyperv_svm_test'
> does not. Introduce and use common rdmsr_from_l2() to avoid code
> duplication and remove hardcoding of MSRs.
> 
> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> ---
>  .../selftests/kvm/include/x86_64/processor.h  |  9 +++++++
>  .../testing/selftests/kvm/x86_64/evmcs_test.c | 24 ++++---------------
>  .../selftests/kvm/x86_64/hyperv_svm_test.c    |  8 +++----
>  3 files changed, 17 insertions(+), 24 deletions(-)
> 
> diff --git a/tools/testing/selftests/kvm/include/x86_64/processor.h b/tools/testing/selftests/kvm/include/x86_64/processor.h
> index fbaf0b6cec4b..a14b7e4ea7c4 100644
> --- a/tools/testing/selftests/kvm/include/x86_64/processor.h
> +++ b/tools/testing/selftests/kvm/include/x86_64/processor.h
> @@ -520,6 +520,15 @@ static inline void cpu_relax(void)
>  		"hlt\n"	\
>  		)
>  
> +/* Exit to L1 from L2 with RDMSR instruction */
> +static inline void rdmsr_from_l2(uint32_t msr)

I would prefer keeping this helper out of common x86-64 code, even if it means
duplicating code across multiple Hyper-V tests until the L1 VM-Enter/VM-Exit
sequences get cleaned up.  The name is misleading, e.g. it doesn't really read
the MSR since there are no outputs, and while we could obviously fix that with a
rename or a generic DO_VMEXIT_FROM_L2() macro, I would rather fix the underlying
problem of the world switches clobbering L2 state.  That way all the helpers that
exist for L1 can be used verbatim for L2 instead of needing dedicated helpers for
every instruction that is used to trigger a VM-Exit.