Re: [PATCH 08/28] KVM: nSVM: Use dedicated array of MSRPM offsets to merge L0 and L1 bitmaps

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

>Me either.  One option would be to use wrapper macros for the interception helpers
>to fill an array at compile time (similar to how kernel exception fixup works),
>but (a) it requires modifications to the linker scripts to generate the arrays,
>(b) is quite confusing/complex, and (c) it doesn't actually solve the problem,
>it just inverts the problem.  Because as above, there are MSRs we *don't* want
>to expose to L2, and so we'd need to add yet more code to filter those out.
>
>And the failure mode for the inverted case would be worse, because if we missed
>an MSR, KVM would incorrectly give L2 access to an MSR.  Whereas with the current
>approach, a missed MSR simply means L2 gets a slower path; but functionally, it's
>fine (and it has to be fine, because KVM can't force L1 to disable interception).

I agree. The risk of breaking functionality should be very low.

>
>> Removing this array and iterating over direct_access_msrs[] directly is an
>> option but it contradicts this series as one of its purposes is to remove
>> direct_access_msrs[].
>
>Using direct_access_msrs[] wouldn't solve the problem either, because nothing
>ensures _that_ array is up-to-date either.
>
>The best idea I have is to add a test that verifies the MSRs that are supposed
>to be passed through actually are passed through.  It's still effectively manual
>checking, but it would require us to screw up twice, i.e. forget to update both
>the array and the test.  The problem is that there's no easy and foolproof way to
>verify that an MSR is passed through in a selftest.
>
>E.g. it would be possible to precisely detect L2 => L0 MSR exits via a BPF program,
>but incorporating a BPF program into a KVM selftest just to detect exits isn't
>something I'm keen on doing (or maintaining).
>
>Using the "exits" stat isn't foolproof due to NMIs (IRQs can be accounted for via
>"irq_exits", and to a lesser extent page faults (especially if shadow paging is
>in use).
>
>If KVM provided an "msr_exits" stats, it would be trivial to verify interception
>via a selftest, but I can't quite convince myself that MSR exits are interesting
>enough to warrant their own stat.

Yes, there doesn't seem to be an easy solution to this problem. Given its low
risk, I think we can live with it for now and revisit the issue if it truely
becomes a recurring problem (i.e., people keep forgetting to update the array).