Re: [PATCH 4/4] KVM: selftests: Add test case for guest and host LBR preemption

[Yang, Weijiang]

On 6/29/2023 10:52 AM, Zhang, Xiong Y wrote:
> > What kind of issues you expect this selftest to find?
> >
> > IMO, it verifies the generic perf schedule rules but cannot find specific issues.
> Current vLBR event break the generic perf schedule rule. So I write the fix commits and selftest to avoid future broken again.

OK, I think you need to refine the assert failure messages and give the 
users  straightforward messages showing something

wrong is happening and stop the following tests since something is 
broken, no need to trigger more errors. E.g.,

+	TEST_ASSERT(r == LBR_MSR_WRITE_ERROR,
+		    "1. Unexpected successfully read/write guest LBR_TO msr");


"1. Unexpected successfully read/write guest LBR_TO msr"
=> "Host LBR ON: Detected unexpected results when write guest vLBR MSRs. Stop testing."

Then at the end of tests, you can print a successful message showing the perf/LBR is working as
expected. This way, testers can got clear result indication of the app.

> > e.g., whether the LBR data is corrupted in some cases. If the selftest can verify
> > whether
> >
> > guest/host data is maintained correctly in a high contention env., that can be
> > better to
> >
> > sever the purpose of selftest.
> Once vLBR event is preempted, I see designing gap and guest should get wrong data, it is out of this commits scope to fix the gap and to verify the result.
> I should add this into commit message.

Yes, refine the change log of this patch to tell clear purpose of this 
app so that users know why this app is needed.

> thanks
> > On 6/16/2023 7:33 PM, Xiong Zhang wrote:
> > > When guest access LBR msr at the first time, kvm will create a vLBR
> > > event, vLBR event joins perf scheduler and occupy physical LBR for guest usage.
> > > Once vLBR event is active and own LBR, guest could access LBR msr.
> > >
> > > But vLBR event is per process pinned event, perf has higher priority event:
> > > per cpu pinned LBR event, perf has lower priority events also: per cpu
> > > LBR event and per process LBR event.
> > > So if host doesn't have higher priority per cpu pinned LBR event, vLBR
> > > event could occupy physical LBR always. But once per cpu pinned LBR
> > > event is active, vLBR event couldn't be active anymore, then guest
> > > couldn't access LBR msr.
> > >
> > > This commit adds test case to cover guest and host lbr contend.
> > >
> > > Signed-off-by: Xiong Zhang <xiong.y.zhang@xxxxxxxxx>
> > > ---
> > >    tools/testing/selftests/kvm/Makefile          |   1 +
> > >    .../selftests/kvm/include/ucall_common.h      |  17 ++
[...]