| 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
| ||
|
Message-ID: <18c9faf4-82ca-4ab3-9a63-4ac553d7b23c@linux.intel.com>
Date: Thu, 15 May 2025 09:37:51 +0800
From: "Mi, Dapeng" <dapeng1.mi@...ux.intel.com>
To: Sean Christopherson <seanjc@...gle.com>,
Peter Zijlstra <peterz@...radead.org>
Cc: Mingwei Zhang <mizhang@...gle.com>, Ingo Molnar <mingo@...hat.com>,
Arnaldo Carvalho de Melo <acme@...nel.org>,
Namhyung Kim <namhyung@...nel.org>, Paolo Bonzini <pbonzini@...hat.com>,
Mark Rutland <mark.rutland@....com>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
Jiri Olsa <jolsa@...nel.org>, Ian Rogers <irogers@...gle.com>,
Adrian Hunter <adrian.hunter@...el.com>, Liang@...gle.com,
Kan <kan.liang@...ux.intel.com>, "H. Peter Anvin" <hpa@...or.com>,
linux-perf-users@...r.kernel.org, linux-kernel@...r.kernel.org,
kvm@...r.kernel.org, linux-kselftest@...r.kernel.org,
Yongwei Ma <yongwei.ma@...el.com>,
Xiong Zhang <xiong.y.zhang@...ux.intel.com>,
Jim Mattson <jmattson@...gle.com>, Sandipan Das <sandipan.das@....com>,
Zide Chen <zide.chen@...el.com>, Eranian Stephane <eranian@...gle.com>,
Shukla Manali <Manali.Shukla@....com>,
Nikunj Dadhania <nikunj.dadhania@....com>
Subject: Re: [PATCH v4 05/38] perf: Add generic exclude_guest support
On 5/15/2025 7:19 AM, Sean Christopherson wrote:
> On Fri, Apr 25, 2025, Peter Zijlstra wrote:
>> On Mon, Mar 24, 2025 at 05:30:45PM +0000, Mingwei Zhang wrote:
>>
>>> @@ -6040,6 +6041,71 @@ void perf_put_mediated_pmu(void)
>>> }
>>> EXPORT_SYMBOL_GPL(perf_put_mediated_pmu);
>>>
>>> +static inline void perf_host_exit(struct perf_cpu_context *cpuctx)
>>> +{
>>> + perf_ctx_disable(&cpuctx->ctx, EVENT_GUEST);
>>> + ctx_sched_out(&cpuctx->ctx, NULL, EVENT_GUEST);
>>> + perf_ctx_enable(&cpuctx->ctx, EVENT_GUEST);
>>> + if (cpuctx->task_ctx) {
>>> + perf_ctx_disable(cpuctx->task_ctx, EVENT_GUEST);
>>> + task_ctx_sched_out(cpuctx->task_ctx, NULL, EVENT_GUEST);
>>> + perf_ctx_enable(cpuctx->task_ctx, EVENT_GUEST);
>>> + }
>>> +}
>>> +
>>> +/* When entering a guest, schedule out all exclude_guest events. */
>>> +void perf_guest_enter(void)
>>> +{
>>> + struct perf_cpu_context *cpuctx = this_cpu_ptr(&perf_cpu_context);
>>> +
>>> + lockdep_assert_irqs_disabled();
>>> +
>>> + perf_ctx_lock(cpuctx, cpuctx->task_ctx);
>>> +
>>> + if (WARN_ON_ONCE(__this_cpu_read(perf_in_guest)))
>>> + goto unlock;
>>> +
>>> + perf_host_exit(cpuctx);
>>> +
>>> + __this_cpu_write(perf_in_guest, true);
>>> +
>>> +unlock:
>>> + perf_ctx_unlock(cpuctx, cpuctx->task_ctx);
>>> +}
>>> +EXPORT_SYMBOL_GPL(perf_guest_enter);
>>> +
>>> +static inline void perf_host_enter(struct perf_cpu_context *cpuctx)
>>> +{
>>> + perf_ctx_disable(&cpuctx->ctx, EVENT_GUEST);
>>> + if (cpuctx->task_ctx)
>>> + perf_ctx_disable(cpuctx->task_ctx, EVENT_GUEST);
>>> +
>>> + perf_event_sched_in(cpuctx, cpuctx->task_ctx, NULL, EVENT_GUEST);
>>> +
>>> + if (cpuctx->task_ctx)
>>> + perf_ctx_enable(cpuctx->task_ctx, EVENT_GUEST);
>>> + perf_ctx_enable(&cpuctx->ctx, EVENT_GUEST);
>>> +}
>>> +
>>> +void perf_guest_exit(void)
>>> +{
>>> + struct perf_cpu_context *cpuctx = this_cpu_ptr(&perf_cpu_context);
>>> +
>>> + lockdep_assert_irqs_disabled();
>>> +
>>> + perf_ctx_lock(cpuctx, cpuctx->task_ctx);
>>> +
>>> + if (WARN_ON_ONCE(!__this_cpu_read(perf_in_guest)))
>>> + goto unlock;
>>> +
>>> + perf_host_enter(cpuctx);
>>> +
>>> + __this_cpu_write(perf_in_guest, false);
>>> +unlock:
>>> + perf_ctx_unlock(cpuctx, cpuctx->task_ctx);
>>> +}
>>> +EXPORT_SYMBOL_GPL(perf_guest_exit);
>> This naming is confusing on purpose? Pick either guest/host and stick
>> with it.
> +1. I also think the inner perf_host_{enter,exit}() helpers are superflous.
> These flows
>
> After a bit of hacking, and with a few spoilers, this is what I ended up with
> (not anywhere near fully tested). I like following KVM's kvm_xxx_{load,put}()
> nomenclature to tie everything together, so I went with "guest" instead of "host"
> even though the majority of work being down is to shedule out/in host context.
>
> /* When loading a guest's mediated PMU, schedule out all exclude_guest events. */
> void perf_load_guest_context(unsigned long data)
> {
> struct perf_cpu_context *cpuctx = this_cpu_ptr(&perf_cpu_context);
>
> lockdep_assert_irqs_disabled();
>
> perf_ctx_lock(cpuctx, cpuctx->task_ctx);
>
> if (WARN_ON_ONCE(__this_cpu_read(guest_ctx_loaded)))
> goto unlock;
>
> perf_ctx_disable(&cpuctx->ctx, EVENT_GUEST);
> ctx_sched_out(&cpuctx->ctx, NULL, EVENT_GUEST);
> perf_ctx_enable(&cpuctx->ctx, EVENT_GUEST);
> if (cpuctx->task_ctx) {
> perf_ctx_disable(cpuctx->task_ctx, EVENT_GUEST);
> task_ctx_sched_out(cpuctx->task_ctx, NULL, EVENT_GUEST);
> perf_ctx_enable(cpuctx->task_ctx, EVENT_GUEST);
> }
>
> arch_perf_load_guest_context(data);
>
> __this_cpu_write(guest_ctx_loaded, true);
>
> unlock:
> perf_ctx_unlock(cpuctx, cpuctx->task_ctx);
> }
> EXPORT_SYMBOL_GPL(perf_load_guest_context);
>
> void perf_put_guest_context(void)
> {
> struct perf_cpu_context *cpuctx = this_cpu_ptr(&perf_cpu_context);
>
> lockdep_assert_irqs_disabled();
>
> perf_ctx_lock(cpuctx, cpuctx->task_ctx);
>
> if (WARN_ON_ONCE(!__this_cpu_read(guest_ctx_loaded)))
> goto unlock;
>
> arch_perf_put_guest_context();
>
> perf_ctx_disable(&cpuctx->ctx, EVENT_GUEST);
> if (cpuctx->task_ctx)
> perf_ctx_disable(cpuctx->task_ctx, EVENT_GUEST);
>
> perf_event_sched_in(cpuctx, cpuctx->task_ctx, NULL, EVENT_GUEST);
>
> if (cpuctx->task_ctx)
> perf_ctx_enable(cpuctx->task_ctx, EVENT_GUEST);
> perf_ctx_enable(&cpuctx->ctx, EVENT_GUEST);
>
> __this_cpu_write(guest_ctx_loaded, false);
> unlock:
> perf_ctx_unlock(cpuctx, cpuctx->task_ctx);
> }
> EXPORT_SYMBOL_GPL(perf_put_guest_context);
I'm fine with the name. Thanks.
>
Powered by blists - more mailing lists