Re: [kvm-unit-tests PATCH v4 2/8] x86: nSVM: Move all nNPT test cases from svm_tests.c to a separate file.

From: "Shukla, Manali" <mashukla@amd.com>
To: Sean Christopherson <seanjc@google.com>
Cc: pbonzini@redhat.com, kvm@vger.kernel.org
Subject: Re: [kvm-unit-tests PATCH v4 2/8] x86: nSVM: Move all nNPT test cases from svm_tests.c to a separate file.
Date: Mon, 20 Jun 2022 07:33:35 +0530	[thread overview]
Message-ID: <d9cac7fe-c38a-1470-2048-3319eb9d3470@amd.com> (raw)
In-Reply-To: <Yqpur6a7oYqUEfGd@google.com>

On 6/16/2022 5:13 AM, Sean Christopherson wrote:
> On Thu, Apr 28, 2022, Manali Shukla wrote:
>> nNPT test cases are moved to a separate file svm_npt.c
>> so that they can be run independently with PTE_USER_MASK disabled.
> 
> Nit, phrase changelogs as command.  Referring to the patch in the past tense is
> confusing because it's not always obvious if the author is referring to this
> patch or something else that happened from long ago.
> 
> And kind of a nit, but not really because I suggested this and still forgot why
> moving to a new file helps.  Moving to a new file isn't what's important, it's
> having a separate setup that we really care about.  So something like:
> 
>   Move the nNPT testcases to their own test and file, svm_npt.c, so that the
>   nNPT tests can run without the USER bit set in the host PTEs (in order to
>   toggle CR4.SMEP) without preventing other nSVM testacases from running code
>   in usersmode.
> 
>> Rest of the test cases can be run with PTE_USER_MASK enabled.
>>
>> No functional change intended.
> 
> Heh, this isn't technically true, as running the "svm" testcase will no do
> different things.

Hi Sean,

Thank you for the review comments.

I will work on comments.

Manali