linux-kernel - RE: [PATCH 1/4] perf/x86/intel: Get shared reg constraints first for vLBR

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]

Message-ID: <MW4PR11MB5824E7C159A4B5CCA39101CFBB25A@MW4PR11MB5824.namprd11.prod.outlook.com>
Date:   Thu, 29 Jun 2023 02:11:04 +0000
From:   "Zhang, Xiong Y" <xiong.y.zhang@...el.com>
To:     Like Xu <like.xu.linux@...il.com>,
        "Christopherson,, Sean" <seanjc@...gle.com>
CC:     "pbonzini@...hat.com" <pbonzini@...hat.com>,
        "peterz@...radead.org" <peterz@...radead.org>,
        "kan.liang@...ux.intel.com" <kan.liang@...ux.intel.com>,
        "zhenyuw@...ux.intel.com" <zhenyuw@...ux.intel.com>,
        "Lv, Zhiyuan" <zhiyuan.lv@...el.com>,
        kvm list <kvm@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH 1/4] perf/x86/intel: Get shared reg constraints first for
 vLBR

> On 16/6/2023 7:33 pm, Xiong Zhang wrote:
> > When host has per cpu pinned LBR event and guest use LBR also, host
> > couldn't get correct LBR data, as the physical LBR is preempted by
> > guest.
> >
> > The rule for multi events sharing LBR is defined in
> > __intel_shared_reg_get_constraints(), but guest vLBR event skips this
> > function, so even if host has per cpu pinned LBR event, guest vLBR
> > event could get constraints successfully and make vlbr_exclude_host
> > returns true, finally host couldn't enable LBR in intel_pmu_lbr_enable_all().
> 
> Although it goes against the "per cpu pinned LBR event" priority expectation, the
> order is intentionally specified. For two reasons:
> 
> - vlbr uses the fake event mechanism in its implementation, a presence similar to
>    BTS event, thus the question here is whether we can get the per cpu pinned
> BTS
>    event to work as expected;
> 
> - this change should not be applied first before KVM has done a good job
>    of making guest lbr and other lbr events coexist correctly;
> 
> In treating vlbr event as an ordinary perf_event behind a guest counter that is
> expected to comply equally with the scheduling rules of host perf, the first thing
> we need to address is how a guest counter should continue to function during
> the time when the backend event is preempted by a higher priority one.
Both arch vPMU and vLBR have such issue, this is vPMU arch gap, you discussed this in the past: https://lore.kernel.org/lkml/810c3148-1791-de57-27c0-d1ac5ed35fb8@gmail.com/
this needs perf and kvm coordinate.
1) In order to let guest get correct data, guest perf event could have higher priority and could not be preempted by host, or passthrough/reserved some counter into guest. This is similar as "guest-only lbr use" in SDM. But this make host perf compromised. 
2) guest perf event is an ordinary perf_event. Like event preemption on host, when an event is preempted, this event is stopped and perf stop this event's total_time_running, finally perf calculates the event counter like multiplex an event, then the event count is an estimated value. But perf couldn't notify the backend counter stopped into guest, and let guest perf notice its event has stopped. Here two new interface are needed:  perf notify kvm that guest perf event is stopped, kvm notify guest that counter is stopped. 
Any suggestion are welcome.

thanks
> 
> >
> > This commit move intel_vlbr_constraints() behind
> > intel_shared_regs_constraints(), guest vLBR event will use LBR also
> > and it should get LBR resource through intel_shared_regs_constraints().
> >
> > Fixes: 097e4311cda9 ("perf/x86: Add constraint to create guest LBR
> > event without hw counter")
> > Signed-off-by: Xiong Zhang <xiong.y.zhang@...el.com>
> > ---
> >   arch/x86/events/intel/core.c | 6 +++---
> >   1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/x86/events/intel/core.c
> > b/arch/x86/events/intel/core.c index 6fd3cd97a6ac..2e27a69e9725 100644
> > --- a/arch/x86/events/intel/core.c
> > +++ b/arch/x86/events/intel/core.c
> > @@ -3347,15 +3347,15 @@ __intel_get_event_constraints(struct
> cpu_hw_events *cpuc, int idx,
> >   {
> >   	struct event_constraint *c;
> >
> > -	c = intel_vlbr_constraints(event);
> > +	c = intel_bts_constraints(event);
> >   	if (c)
> >   		return c;
> >
> > -	c = intel_bts_constraints(event);
> > +	c = intel_shared_regs_constraints(cpuc, event);
> >   	if (c)
> >   		return c;
> >
> > -	c = intel_shared_regs_constraints(cpuc, event);
> > +	c = intel_vlbr_constraints(event);
> >   	if (c)
> >   		return c;
> >