linux-kernel - Re: [PATCH v1] KVM/x86/vPMU: Guest PMI Optimization

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sun, 14 Oct 2018 20:41:28 +0800
From:   Wei Wang <wei.w.wang@...el.com>
To:     Paolo Bonzini <pbonzini@...hat.com>,
        Andi Kleen <ak@...ux.intel.com>
CC:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "kvm@...r.kernel.org" <kvm@...r.kernel.org>,
        "peterz@...radead.org" <peterz@...radead.org>,
        "mingo@...hat.com" <mingo@...hat.com>,
        "rkrcmar@...hat.com" <rkrcmar@...hat.com>,
        "Xu, Like" <like.xu@...el.com>
Subject: Re: [PATCH v1] KVM/x86/vPMU: Guest PMI Optimization

On 10/13/2018 04:09 PM, Paolo Bonzini wrote:
>
>> It's not clear to me why you're special casing PMIs here. The optimization
>> should work generically, right?
> Yeah, you can even just check if the counter is in the struct
> cpu_hw_events guest mask, and if so always write the counter MSR directly.

Not sure if we could do that. I think the guest mask on the host 
reflects which counters are used by the host.

Here is the plan I have in mind:
#1 Creates a host perf event on the guest's first bit-setting to 
MSR_CORE_PERF_GLOBAL_CTRL; Meanwhile, disable the intercept of guest 
access to this perf counter related MSRs (i.e. config_base and event_base).
#2 When the vCPU is sched in,
     #2.1 make the MSRs of the perf counters (assigned to the guest in 
#1) interceptible, so that guest accesses to such a counter is captured, 
and marked it "used", and disable the intercept again;
     #2.2 also check if there is any counter that wasn't "used" in the 
last vCPU time slice, if there is, release that counter and the perf event.


>
>>> @@ -237,9 +267,23 @@ static int intel_pmu_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
>>>   	default:
>>>   		if ((pmc = get_gp_pmc(pmu, msr, MSR_IA32_PERFCTR0)) ||
>>>   		    (pmc = get_fixed_pmc(pmu, msr))) {
>>> -			if (!msr_info->host_initiated)
>>> -				data = (s64)(s32)data;
>>> -			pmc->counter += data - pmc_read_counter(pmc);
>>> +			if (pmu->in_pmi) {
>>> +				/*
>>> +				 * Since we are not re-allocating a perf event
>>> +				 * to reconfigure the sampling time when the
>>> +				 * guest pmu is in PMI, just set the value to
>>> +				 * the hardware perf counter. Counting will
>>> +				 * continue after the guest enables the
>>> +				 * counter bit in MSR_CORE_PERF_GLOBAL_CTRL.
>>> +				 */
>>> +				struct hw_perf_event *hwc =
>>> +						&pmc->perf_event->hw;
>>> +				wrmsrl(hwc->event_base, data);
>> Is that guaranteed to be always called on the right CPU that will run the vcpu?
>>
>> AFAIK there's an ioctl to set MSRs in the guest from qemu, I'm pretty sure
>> it won't handle that.
> How much of the performance improvement comes from here?  In theory
> pmc_read_counter() should always hit a relatively fast path, because the
> smp_call_function_single in perf_event_read doesn't need an IPI.
>
> In any case, this should be a separate patch.

Actually this change wasn't intended for performance improvement. It was 
adapted for the "fast path" we added to the MSR_CORE_PERF_GLOBAL_CTRL 
write handling.

The old implementation captures the guest updating of the period in 
pmc->counter, and then uses the pmc->counter for the perf event 
creation, which gets the guest requested period written to the 
underlying counter via the host perf core. The fast path avoids the perf 
event creation, and accordingly, we need to update the period value 
directly to the hardware counter.

Best,
Wei