lkml.org 
[lkml]   [2020]   [May]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    SubjectRe: [PATCH v11 05/11] perf/x86: Keep LBR stack unchanged in host context for guest LBR event
    From
    Date
    Hi Peter,

    Thanks for the clear attitude and code refinement.

    On 2020/5/18 20:02, Peter Zijlstra wrote:
    > On Thu, May 14, 2020 at 04:30:48PM +0800, Like Xu wrote:
    >> @@ -544,7 +562,12 @@ void intel_pmu_lbr_enable_all(bool pmi)
    >> {
    >> struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
    >>
    >> - if (cpuc->lbr_users)
    >> + /*
    >> + * When the LBR hardware is scheduled for a guest LBR event,
    >> + * the guest will dis/enables LBR itself at the appropriate time,
    >> + * including configuring MSR_LBR_SELECT.
    >> + */
    >> + if (cpuc->lbr_users && !cpuc->guest_lbr_enabled)
    >> __intel_pmu_lbr_enable(pmi);
    >> }
    >
    > No!, that should be done through perf_event_attr::exclude_host, as I
    > believe all the other KVM event do it.
    >

    Sure, I could reuse cpuc->intel_ctrl_guest_mask to rewrite this part:

    diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
    index d788edb7c1f9..f1243e8211ca 100644
    --- a/arch/x86/events/intel/core.c
    +++ b/arch/x86/events/intel/core.c
    @@ -2189,7 +2189,8 @@ static void intel_pmu_disable_event(struct perf_event
    *event)
    } else if (idx == INTEL_PMC_IDX_FIXED_BTS) {
    intel_pmu_disable_bts();
    intel_pmu_drain_bts_buffer();
    - }
    + } else if (idx == INTEL_PMC_IDX_FIXED_VLBR)
    + intel_clear_masks(event, idx);

    /*
    * Needs to be called after x86_pmu_disable_event,
    @@ -2271,7 +2272,8 @@ static void intel_pmu_enable_event(struct perf_event
    *event)
    if (!__this_cpu_read(cpu_hw_events.enabled))
    return;
    intel_pmu_enable_bts(hwc->config);
    - }
    + } else if (idx == INTEL_PMC_IDX_FIXED_VLBR)
    + intel_set_masks(event, idx);
    }

    static void intel_pmu_add_event(struct perf_event *event)
    diff --git a/arch/x86/events/intel/lbr.c b/arch/x86/events/intel/lbr.c
    index b8dabf1698d6..1b30c76815dd 100644
    --- a/arch/x86/events/intel/lbr.c
    +++ b/arch/x86/events/intel/lbr.c
    @@ -552,11 +552,19 @@ void intel_pmu_lbr_del(struct perf_event *event)
    perf_sched_cb_dec(event->ctx->pmu);
    }

    +static inline bool vlbr_is_enabled(void)
    +{
    + struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
    +
    + return test_bit(INTEL_PMC_IDX_FIXED_VLBR,
    + (unsigned long *)&cpuc->intel_ctrl_guest_mask);
    +}
    +
    void intel_pmu_lbr_enable_all(bool pmi)
    {
    struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);

    - if (cpuc->lbr_users)
    + if (cpuc->lbr_users && !vlbr_is_enabled())
    __intel_pmu_lbr_enable(pmi);
    }

    @@ -564,7 +572,7 @@ void intel_pmu_lbr_disable_all(void)
    {
    struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);

    - if (cpuc->lbr_users)
    + if (cpuc->lbr_users && !vlbr_is_enabled())
    __intel_pmu_lbr_disable();
    }

    @@ -706,7 +714,8 @@ void intel_pmu_lbr_read(void)
    * This could be smarter and actually check the event,
    * but this simple approach seems to work for now.
    */
    - if (!cpuc->lbr_users || cpuc->lbr_users == cpuc->lbr_pebs_users)
    + if (!cpuc->lbr_users || vlbr_is_enabled() ||
    + cpuc->lbr_users == cpuc->lbr_pebs_users)
    return;

    if (x86_pmu.intel_cap.lbr_format == LBR_FORMAT_32)
    Is this acceptable to you ?

    If you have more comments on the patchset, please let me know.

    Thanks,
    Like Xu

    \
     
     \ /
      Last update: 2020-05-19 05:09    [W:3.971 / U:0.004 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site