Re: [PATCH v4 0/9] Linear Address Masking (LAM) KVM Enabling

[Chao Gao <chao.gao@intel.com>]

On Fri, Feb 10, 2023 at 04:39:49PM +0800, Robert Hoo wrote:
>On Fri, 2023-02-10 at 10:07 +0800, Robert Hoo wrote:
>> On Thu, 2023-02-09 at 17:27 +0000, Sean Christopherson wrote:
>> > On Thu, Feb 09, 2023, Robert Hoo wrote:
>> > > On Thu, 2023-02-09 at 14:15 +0800, Chao Gao wrote:
>> > > > On Thu, Feb 09, 2023 at 10:40:13AM +0800, Robert Hoo wrote:
>> > > > Please add a kvm-unit-test or kselftest for LAM, particularly
>> > > > for
>> > > > operations (e.g., canonical check for supervisor pointers,
>> > > > toggle
>> > > > CR4.LAM_SUP) which aren't covered by the test in Kirill's
>> > > > series.
>> > > 
>> > > OK, I can explore for kvm-unit-test in separate patch set.
>> > 
>> > Please make tests your top priority.  Without tests, I am not going
>> > to spend any
>> > time reviewing this series, or any other hardware enabling
>> > series[*].  I don't
>> > expect KVM specific tests for everything, i.e. it's ok to to rely
>> > things like
>> > running VMs that utilize LAM and/or running LAM selftests in the
>> > guest, but I do
>> > want a reasonably thorough explanation of how all the test pieces
>> > fit
>> > together to
>> > validate KVM's implementation.
>> 
>> Sure, and ack on unit test is part of development work.
>> 
>> This patch set had always been unit tested before sent out, i.e.
>> "running LAM selftests in guest" on both ept=Y/N.
>> 
>> CR4.LAM_SUP, as Chao pointed out, could not be covered by kselftest,
>> I
>> may explore it in kvm-unit-test.
>> 
>When I come to kvm-unit-test, just find that I had already developed
>some test case on CR4.LAM_SUP toggle and carried out on this patch set.
>Just forgot about it.
>
>Is it all right? if so, I will include it in next version.

You can add more steps to the test, e.g.,

1. check if CR4.LAM_SUP setting takes effort by storing metadata into
   a supervisor pointer and dereferencing the pointer.
2. turn 5-level paging on/off and check if LAM width complies with
   the spec.
3. add some negtive tests. e.g., if LAM isn't advertised to the guest,
   setting CR4.LAM_SUP isn't allowed.

...