Re: [PATCH v7 08/12] KVM/x86/vPMU: Add APIs to support host save/restore the guest lbr stack

From: Peter Zijlstra
Date: Tue Jul 09 2019 - 07:46:17 EST


On Mon, Jul 08, 2019 at 09:23:15AM +0800, Wei Wang wrote:
> +int intel_pmu_enable_save_guest_lbr(struct kvm_vcpu *vcpu)
> +{
> + struct kvm_pmu *pmu = vcpu_to_pmu(vcpu);
> + struct perf_event *event;
> +
> + /*
> + * The main purpose of this perf event is to have the host perf core
> + * help save/restore the guest lbr stack on vcpu switching. There is
> + * no perf counters allocated for the event.
> + *
> + * About the attr:
> + * exclude_guest: set to true to indicate that the event runs on the
> + * host only.

That's a lie; it _never_ runs. You specifically don't want the host to
enable LBR so as not to corrupt the guest state.

> + * pinned: set to false, so that the FLEXIBLE events will not
> + * be rescheduled for this event which actually doesn't
> + * need a perf counter.

Unparsable gibberish. Specifically by making it flexible it is
susceptible to rotation and there's no guarantee it will actually get
scheduled.

> + * config: Actually this field won't be used by the perf core
> + * as this event doesn't have a perf counter.
> + * sample_period: Same as above.

If it's unused; why do we need to set it at all?

> + * sample_type: tells the perf core that it is an lbr event.
> + * branch_sample_type: tells the perf core that the lbr event works in
> + * the user callstack mode so that the lbr stack will be
> + * saved/restored on vCPU switching.

Again; doesn't make sense. What does the user part have to do with
save/restore? What happens when this vcpu thread drops to userspace for
an assist?

> + */
> + struct perf_event_attr attr = {
> + .type = PERF_TYPE_RAW,
> + .size = sizeof(attr),
> + .exclude_guest = true,
> + .pinned = false,
> + .config = 0,
> + .sample_period = 0,
> + .sample_type = PERF_SAMPLE_BRANCH_STACK,
> + .branch_sample_type = PERF_SAMPLE_BRANCH_CALL_STACK |
> + PERF_SAMPLE_BRANCH_USER,
> + };
> +
> + if (pmu->vcpu_lbr_event)
> + return 0;
> +
> + event = perf_event_create(&attr, -1, current, NULL, NULL, false);
> + if (IS_ERR(event)) {
> + pr_err("%s: failed %ld\n", __func__, PTR_ERR(event));
> + return -ENOENT;
> + }
> + pmu->vcpu_lbr_event = event;
> +
> + return 0;
> +}