linux-kernel - Re: [PATCH] perf/x86/intel: KVM: Mask PEBS_ENABLE loaded for guest with vCPU's value.

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] [day] [month] [year] [list]

Message-ID: <a08c7053-26b8-43ae-a036-05ae2c132e4f@linux.intel.com>
Date: Fri, 9 May 2025 10:19:25 +0800
From: "Mi, Dapeng" <dapeng1.mi@...ux.intel.com>
To: Sean Christopherson <seanjc@...gle.com>
Cc: Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...hat.com>,
 Arnaldo Carvalho de Melo <acme@...nel.org>,
 Namhyung Kim <namhyung@...nel.org>, Thomas Gleixner <tglx@...utronix.de>,
 Borislav Petkov <bp@...en8.de>, Dave Hansen <dave.hansen@...ux.intel.com>,
 x86@...nel.org, linux-perf-users@...r.kernel.org,
 linux-kernel@...r.kernel.org, Seth Forshee <sforshee@...nel.org>
Subject: Re: [PATCH] perf/x86/intel: KVM: Mask PEBS_ENABLE loaded for guest
 with vCPU's value.


On 5/8/2025 9:47 PM, Sean Christopherson wrote:
> On Tue, May 06, 2025, Dapeng Mi wrote:
>> On 5/2/2025 11:04 PM, Sean Christopherson wrote:
>>> On Sun, Apr 27, 2025, Dapeng Mi wrote:
>>>> On 4/26/2025 8:13 AM, Sean Christopherson wrote:
>>>> Currently we have this Sean's fix, only the guest PEBS event bits of
>>>> IA32_PEBS_ENABLE MSR are enabled in non-root mode, suppose we can simply
>>>> change global_ctrl guest value calculation to this.
>>>>
>>>> diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
>>>> index 09d2d66c9f21..5bc56bb616ec 100644
>>>> --- a/arch/x86/events/intel/core.c
>>>> +++ b/arch/x86/events/intel/core.c
>>>> @@ -4342,9 +4342,12 @@ static struct perf_guest_switch_msr
>>>> *intel_guest_get_msrs(int *nr, void *data)
>>>>         arr[global_ctrl] = (struct perf_guest_switch_msr){
>>>>                 .msr = MSR_CORE_PERF_GLOBAL_CTRL,
>>>>                 .host = intel_ctrl & ~cpuc->intel_ctrl_guest_mask,
>>>> -               .guest = intel_ctrl & ~cpuc->intel_ctrl_host_mask & ~pebs_mask,
>>>> +               .guest = intel_ctrl & ~cpuc->intel_ctrl_host_mask,
>>>>         };
>>> Hmm, that's not as clear cut.  PEBS needs to be disabled because leaving it enabled
>>> will crash the guest.  For the counter itself, unless leaving it enabled breaks
>>> perf and/or degrades the sampling, I don't think there's an obvious right/wrong
>>> approach.
>>>
>>> E.g. if the host wants to profile host and guest, then keeping the count running
>>> while the guest is active might be a good thing.  It's still far, far from
>>> perfect, as a counter that overflows when the guest is active won't generate a
>>> PEBS record, but without digging further, it's not obvious that even that flaw
>>> is overall worse than always disabling the counter.
>> Hmm, if the host PEBS event only samples host side, whether the HW counter
>> or the PEBS engine would be disabled once VM enters non-root mode, the KVM
>> PEBS implementation is correct. But for the host PEBS events which sampling
>> both host and guest, the implementation seems incorrect.
> Well, yeah, because there is no correct implementation.
>
>> As the below code shows, as long as there are host PEBS events regardless
>> of the host PEBS events only sample guest or both host and guest, the host
>> PEBS events would be disabled on both HW counters and PEBS engine once VM
>> enters non-root mode.
>>
>>     arr[global_ctrl] = (struct perf_guest_switch_msr){
>>         .msr = MSR_CORE_PERF_GLOBAL_CTRL,
>>         .host = intel_ctrl & ~cpuc->intel_ctrl_guest_mask,
>>         .guest = intel_ctrl & ~cpuc->intel_ctrl_host_mask & ~pebs_mask,
>>     };
>>
>>     if (arr[pebs_enable].host) {
>>         /* Disable guest PEBS if host PEBS is enabled. */
>>         arr[pebs_enable].guest = 0;
>>
>>     }
>>
>> So the host PEBS events which hopes to sample both host and guest only
>> samples host side in fact. This is unexpected.
> It's only unexpected in the sense that it's probably not well documented.  Because
> the DS buffer is virtually address, there simply isn't a sane way to enable PEBS
> (or any feature that utizies the DS buffer) while running a KVM guest that isn't
> enlightened to explicitly allow profiling via host PEBS (and AFAIK, no such guest
> exists).
>
> Even when KVM is using shadowing paging, i.e. fully controls the page tables used
> while the guest is running, enabling PEBS isn't feasible as KVM has no way to
> prevent the guest from using the virtual address.  E.g. KVM could shove in mappings
> for the DS buffer, but that DoS the guest if the guest wants to use the same
> virtual address range for its own purposes, and would be a massive data leak to
> the guest since the guest could read host data from the buffer.

Yeah, that's true. Thanks for the explanation.


>