Re: [PATCH v2 11/16] KVM: x86/pmu: Emulate the GLOBAL_STATUS_SET and GLOBAL_INUSE MSRs

[Yosry Ahmed]

> >>> 1. GLOBAL_STATUS_TRACE_TOPAPMI_BIT can leak from the host into the
> >>> guest. If a host PT PMI arrives while the guest context is loaded, bit
> >>> 55 will be set for the guest, and the guest won't be able to clear it if
> >>> OVF_CTRL is intercepted (it will #GP as it is reserved for the guest).
> >>> Similarly, the value can't be restored through KVM_SET_MSRS.
> >>
> >> I think mediated vPMU assumes that no host PMIs can be generated while
> >> guest PMU state is loaded.
> >>
> >> If PT state can leak into the guest via bit 55, similar leakage can also
> >> occur for other status bits, e.g. when the guest exposes fewer
> >> general-purpose or fixed counters than the host.
> >>
> >> The scenario where both host and guest can be active simultaneously is
> >> handled by the perfmon_mask case, where intel_mediated_pmu_{load,put}()
> >> filter guest-owned status bits.
> >
> > Looking at the code:
> >
> > kvm_mediated_pmu_load() -> perf_load_guest_context() ->
> > perf_ctx_disable() will iterate PMUs with for_each_epc() and disable
> > them, which skips PMUs without PERF_PMU_CAP_MEDIATED_VPMU.
> >
> > Same for kvm_mediated_pmu_load() -> perf_load_guest_context()
> > ->ctx_sched_out(), it also uses for_each_epc() to iterate PMUs and
> > schedule them out.
> >
> > pt_init() does not set PERF_PMU_CAP_MEDIATED_VPMU on pt_pmu.pmu.capabilities
> >
> > Please correct me if I am wrong.
>
> My apologies, this bug is indeed real because pt_pmu lacks the
> PERF_PMU_CAP_MEDIATED_VPMU capability.
>
> Adding this capability to pt_pmu seems intuitive, but unlike
>
>                        the core PMU (which merely toggles enable bits),
> Intel PT is an AUX-based PMU. Stopping and restarting it on every VM
> transition is heavier, and I'm not sure whether it could introduce a
> noticeable performance penalty.
>
> However, whether the solution is adding PERF_PMU_CAP_MEDIATED_VPMU or
> some other mechanism to enforce mutual exclusion between pt_pmu and
> mediated vPMU, we can address that in a separate patch.
>
> To immediately protect guest state and live migration in this series,
> I'm thinking of adding the following defensive filter in
> intel_mediated_pmu_put():
>
> + /*
> +  * Scrub bits that are not virtualized for the guest to prevent
> +  * possible host state from leaking into guest state and breaking
> +  * live migration.
> +  */
> +  pmu->global_status &= ~pmu->global_status_rsvd;

This doesn't prevent the leakage, it just clears it on the guest->host
switch. So it helps live migration but I am not sure if it's better or
worse if the guest sees the bit unexpectedly appear AND then
unexpectedly disappear.

We can clear it when the MSR is read by userspace, which should fix
live migration, but the guest will still see the bit, and it will
still disappear unexpectedly but only during live migration.

The guest still won't be able to clear it when OVF_CTRL is intercepted.