Received: by 2002:a05:6358:7058:b0:131:369:b2a3 with SMTP id 24csp6209780rwp; Mon, 17 Jul 2023 17:31:56 -0700 (PDT) X-Google-Smtp-Source: APBJJlH1zqy6LW74CR2G7OIQtSnri/o1D0ahmLLqVFnvdY1C2yr8uAi1OQya45koKPEn+0w69nWm X-Received: by 2002:a17:90a:e644:b0:263:76e8:b66f with SMTP id ep4-20020a17090ae64400b0026376e8b66fmr10911887pjb.30.1689640316125; Mon, 17 Jul 2023 17:31:56 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1689640316; cv=none; d=google.com; s=arc-20160816; b=d0AcUPIkTJCvD9wLf44YU9LfmnByHyLlEoQy7vdqEg+VibhiW55zK9nIRVwCCGJOg6 vcm//EnufSzrLNyI7wNG2gZYIXpqW8we4HHyf6zudR3q/LLIvabwgec+NokmoQOURLoP 25z8suPo+4/aAFOgltwgXkBHONuGH2PzYfcXUCUlr4X8A3+biWEA/sUY2UV7Wbg/9pbm H/5s1+gVKeQ2updMxkmeW+im3+LYJfwF8J9dkxsgLQuR6Hf/4+r+FpNrvkeChXG9S8pr 3DApPIfsBVr1MX9Ue+3wmLEoLD3J+iVBqrVFCEEq1VfwfFXX1ijmHeSyJMb2FPmgw5An IkrQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:cc:to:subject :message-id:date:from:in-reply-to:references:mime-version :dkim-signature; bh=T0vSTddCvpCRB5+yiUAWYvUbtvxTOv2Q4zjZH3DHeeg=; fh=DEnwMZjjYO8NsIXOsROvujxXpyQj1NovFgTEvovAOeU=; b=RO2YSijKw0b1ozTjJ4FsDeNOZifcTJ5CyWAJDfrZfOrBJ25qARp1ms3Nf4boRfk9Op bEOicAcuUR5lcxINs0tmpaxv0jeF8rXg3wfyS+1J60ay7I/7u9O50QCep67y4vkIxjNM 2rCOiOzh7Y8tXiq8c0haF1uRP0l+FVJHhTMA39o7N5ulziM842gXAu+Z/Kxd1FI9kmVT jPqwuD87FcqA7kklizo4bXky41G3ZSNiaaXqDNEmBKCPz1mArq7TYbYjgQwUvAn9qMh/ 02wBq0fbvDT42xvkTPRL5gMZ3zUDGg7561jvgWw5gaRMxgfBCNhzMuJA6gDOOj5KRFQG w1pg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@atishpatra.org header.s=google header.b="I/mUjLqy"; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id k2-20020a17090aaa0200b0025bf45ac365si6401750pjq.82.2023.07.17.17.31.44; Mon, 17 Jul 2023 17:31:56 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@atishpatra.org header.s=google header.b="I/mUjLqy"; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231622AbjGQXZx (ORCPT + 99 others); Mon, 17 Jul 2023 19:25:53 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:43724 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230062AbjGQXZw (ORCPT ); Mon, 17 Jul 2023 19:25:52 -0400 Received: from mail-lf1-x12e.google.com (mail-lf1-x12e.google.com [IPv6:2a00:1450:4864:20::12e]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 5BDE31AC for ; Mon, 17 Jul 2023 16:25:06 -0700 (PDT) Received: by mail-lf1-x12e.google.com with SMTP id 2adb3069b0e04-4fd32e611e0so4158810e87.0 for ; Mon, 17 Jul 2023 16:25:06 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=atishpatra.org; s=google; t=1689636186; x=1692228186; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=T0vSTddCvpCRB5+yiUAWYvUbtvxTOv2Q4zjZH3DHeeg=; b=I/mUjLqy4mLdHkR+BwTOC9b9TZ5wXxeSVspl1j2Zqu3jsQwKW3DoCSqiugdzgoEVWm oeY70oMe9WX7hoKzgQTt/msrf/dGJysdm0YUXnhlICEDQOgHG3ckzl1Lagm0FK6Wfqo7 c1Ueu026ITVD+35nvX4n2T/sCzjiNLb1gxqHY= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1689636186; x=1692228186; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=T0vSTddCvpCRB5+yiUAWYvUbtvxTOv2Q4zjZH3DHeeg=; b=V/WKIt47v8O647nz0AI88RAJIBFN02oOOV8ZEa9PofgLgkSwfaTKIVaDbqmZZ9FD27 VrSfjM+c9yZlQH37nQQ50UgQMn5P2T4GLqe7k/LGNkhVPqeFmwcM0FlAPUSyp5iEc0fU x6aRjuY2/1LtKqND0MKZX/LxkiKYfv5Sl6COdwZuURJuOu1GBzqunXNKZFcj22yShPQ7 kThXdXpwc+En2QYvGbBLV/551//OYuFbBQvKxHCQAUWPfrO8JCTxR0uRtIfXjRlw6Mdi 3pGYL5C18ZtTOaeZLmIjf0X+YyiUFEmeV0/LEZ+wxVKvkQmio2KzxlMi5f4CezTTsc/W 2tew== X-Gm-Message-State: ABy/qLYSNMqta3WiVytL4QJuHorJHlxYQIj5NZ79j2aUF/Yxy/H3voXY umIZzmui1wKrxsOGDkImzF41xRdQknjWSyR1p80y X-Received: by 2002:ac2:4c45:0:b0:4eb:46c2:e771 with SMTP id o5-20020ac24c45000000b004eb46c2e771mr4215007lfk.14.1689636186128; Mon, 17 Jul 2023 16:23:06 -0700 (PDT) MIME-Version: 1.0 References: <20230703124647.215952-1-alexghiti@rivosinc.com> In-Reply-To: From: Atish Patra Date: Mon, 17 Jul 2023 16:22:54 -0700 Message-ID: Subject: Re: [PATCH v4 00/10] riscv: Allow userspace to directly access perf counters To: =?UTF-8?Q?R=C3=A9mi_Denis=2DCourmont?= Cc: linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org, Aurelien Jarno , Mathieu Malaterre , Jan Newger Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_BLOCKED, SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Jul 17, 2023 at 6:38=E2=80=AFAM R=C3=A9mi Denis-Courmont wrote: > > Hi, > > Le 3 juillet 2023 15:46:37 GMT+03:00, Alexandre Ghiti a =C3=A9crit : > >riscv used to allow direct access to cycle/time/instret counters, > >bypassing the perf framework, this patchset intends to allow the user to > >mmap any counter when accessed through perf. But we can't break the > >existing behaviour so we introduce a sysctl perf_user_access like arm64 > >does, which defaults to the legacy mode described above. > > AFAIK, if the default settings breaks user space, the patchset is conside= red to break user space. That being the case, either this case is deemed sp= ecial enough that breaking user space is OK, or it is not. > This case is a special case as the usage was incorrect in the first place. Any application that genuinely requires rdcycle can always get it now via the perf interface. If the insecure and incorrect behavior is allowed, we suspect the user space behavior will never be fixed as it is hard to put a future flag date in these cases. Given that the RISC-V eco-system is still young, it is better to disallow this behavior to avoid further proliferation of the same incorrect usage. > If it is not OK, then the only way out that I can think of, consists of t= rapping and emulating the counters, returning the same sanitised values tha= t Linux perf would return. Then you can add a kernel config option to disab= le that trap-and-emulation code in the future. > What do you mean by "sanitised" value ? If you mean the correct value, it doesn't know when to stop providing that value (without reinventing the entire context switch which the perf framework already does for us). If we just provide a dummy value, that doesn't help the current users as well. They will probably get unexpected results instead of sigill. I have cc'd a few folks who were involved in the initial discussions when this issue was reported to collect the feedback. > Either way, I don't suppose that there should be an option to be insecure= ly backward compatible. > > > > >This version needs openSBI v1.3 *and* a kernel fix that went upstream la= tely > >(https://lore.kernel.org/lkml/20230616114831.3186980-1-maz@kernel.org/T/= ). > > > >**Important**: In this version, the default mode is now user access, not > >the legacy so some applications will break. > > > >base-commit-tag: v6.4-rc6 > > > >Changes in v4: > >- Fixed some nits in riscv_pmu_sbi.c thanks to Andrew > >- Fixed the documentation thanks to Andrew > >- Added RB from Andrew \o/ > > > >Changes in v3: > >- patch 1 now contains the ref to the faulty commit (no Fixes tag as it = is only a comment), as Andrew suggested > >- Removed RISCV_PMU_LEGACY_TIME from patch 3, as Andrew suggested > >- Rename RISCV_PMU_PDEV_NAME to "riscv-pmu-sbi", patch4 is just cosmetic= now, as Andrew suggested > >- Removed a few useless (and wrong) comments, as Andrew suggested > >- Simplify arch_perf_update_userpage code, as Andrew suggested > >- Documentation now mentions that time CSR is *always* accessible, whate= ver the mode, as suggested by Andrew > >- Removed CYCLEH reference and add TODO for rv32 support, as suggested b= y Atish > >- Do not rename the pmu instance as Atish suggested > >- Set pmc_width only if rdpmc is enabled and CONFIG_RISCV_PMU is set and= the event is a hw event > >- Move arch_perf_update_userpage https://lore.kernel.org/lkml/2023061611= 4831.3186980-1-maz@kernel.org/T/ > >- **Switch to user mode access by default** > > > >Changes in v2: > >- Split into smaller patches, way better! > >- Add RB from Conor > >- Simplify the way we checked riscv architecture > >- Fix race mmap and other thread running on other cpus > >- Use hwc when available > >- Set all userspace access flags in event_init, too cumbersome to handle= sysctl changes > >- Fix arch_perf_update_userpage for pmu other than riscv-pmu by renaming= pmu driver > >- Fixed kernel test robot build error > >- Fixed documentation (Andrew and Bagas) > >- perf testsuite passes mmap tests in all 3 modes > > > >Alexandre Ghiti (10): > > perf: Fix wrong comment about default event_idx > > include: riscv: Fix wrong include guard in riscv_pmu.h > > riscv: Make legacy counter enum match the HW numbering > > drivers: perf: Rename riscv pmu sbi driver > > riscv: Prepare for user-space perf event mmap support > > drivers: perf: Implement perf event mmap support in the legacy backend > > drivers: perf: Implement perf event mmap support in the SBI backend > > Documentation: admin-guide: Add riscv sysctl_perf_user_access > > tools: lib: perf: Implement riscv mmap support > > perf: tests: Adapt mmap-basic.c for riscv > > > > Documentation/admin-guide/sysctl/kernel.rst | 27 ++- > > drivers/perf/riscv_pmu.c | 113 +++++++++++ > > drivers/perf/riscv_pmu_legacy.c | 28 ++- > > drivers/perf/riscv_pmu_sbi.c | 196 +++++++++++++++++++- > > include/linux/perf/riscv_pmu.h | 12 +- > > include/linux/perf_event.h | 3 +- > > tools/lib/perf/mmap.c | 65 +++++++ > > tools/perf/tests/mmap-basic.c | 4 +- > > 8 files changed, 428 insertions(+), 20 deletions(-) > > > > _______________________________________________ > linux-riscv mailing list > linux-riscv@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-riscv --=20 Regards, Atish