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: <f35cc5d6607f143bb2f66da0ce629776c0b45802.camel@syntacore.com>
Date: Thu, 1 Aug 2024 08:09:42 +0000
From: Dmitry Shifrin <dmitry.shifrin@...tacore.com>
To: "linux-riscv@...ts.infradead.org" <linux-riscv@...ts.infradead.org>,
	"samuel.holland@...ive.com" <samuel.holland@...ive.com>
CC: "alexghiti@...osinc.com" <alexghiti@...osinc.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"mark.rutland@....com" <mark.rutland@....com>, "paul.walmsley@...ive.com"
	<paul.walmsley@...ive.com>, "atishp@...shpatra.org" <atishp@...shpatra.org>,
	"palmer@...belt.com" <palmer@...belt.com>,
	"linux-arm-kernel@...ts.infradead.org"
	<linux-arm-kernel@...ts.infradead.org>, "anup@...infault.org"
	<anup@...infault.org>, "will@...nel.org" <will@...nel.org>,
	"aou@...s.berkeley.edu" <aou@...s.berkeley.edu>
Subject: Re: [PATCH] perf: riscv: Fix selecting counters in legacy mode

Hi Samuel,

On Wed, 2024-07-31 at 02:37 -0500, Samuel Holland wrote:
> Hi Dmitry,
> 
> On 2024-07-29 7:58 AM, Shifrin Dmitry wrote:
> > It is required to check event type before checking event config.
> > Events with the different types can have the same config.
> > This check is missed for legacy mode code
> > 
> > For such perf usage:
> >     sysctl -w kernel.perf_user_access=2
> >     perf stat -e cycles,L1-dcache-loads --
> > driver will try to force both events to CYCLE counter.
> > 
> > This commit implements event type check before forcing
> > events on the special counters.
> 
> It looks like pmu_sbi_event_mapped() and pmu_sbi_event_mapped() have a similar
> bug where they do not check event->attr.type.

I am not sure that pmu_sbi_event_unmapped() and pmu_sbi_event_mapped() should be changed in the same way.
These functions in legacy mode just set/clear PERF_EVENT_FLAG_USER_READ_CNT for legacy events without any
mapping/unmapping. However in pmu_sbi_starting_cpu() CYCLE, TIME and INSTRET counters are always mapped to
user space in legacy mode. So it looks like in legacy mode we can just use

: if (event->hw.flags & PERF_EVENT_FLAG_LEGACY)
:	return;

Thanks, Dmitry

> 
> Regards,
> Samuel
> 
> > 
> > Signed-off-by: Shifrin Dmitry <dmitry.shifrin@...tacore.com>
> > ---
> >  drivers/perf/riscv_pmu_sbi.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/perf/riscv_pmu_sbi.c b/drivers/perf/riscv_pmu_sbi.c
> > index 36d128ff166f..bf14ab282e11 100644
> > --- a/drivers/perf/riscv_pmu_sbi.c
> > +++ b/drivers/perf/riscv_pmu_sbi.c
> > @@ -416,7 +416,7 @@ static int pmu_sbi_ctr_get_idx(struct perf_event *event)
> >          * but not in the user access mode as we want to use the other counters
> >          * that support sampling/filtering.
> >          */
> > -       if (hwc->flags & PERF_EVENT_FLAG_LEGACY) {
> > +       if ((hwc->flags & PERF_EVENT_FLAG_LEGACY) && (event->attr.type == PERF_TYPE_HARDWARE)) {
> >                 if (event->attr.config == PERF_COUNT_HW_CPU_CYCLES) {
> >                         cflags |= SBI_PMU_CFG_FLAG_SKIP_MATCH;
> >                         cmask = 1;
> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ