From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.0 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id D3842C433ED for ; Thu, 8 Apr 2021 18:38:35 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id B04F461108 for ; Thu, 8 Apr 2021 18:38:35 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232814AbhDHSiq (ORCPT ); Thu, 8 Apr 2021 14:38:46 -0400 Received: from mail.kernel.org ([198.145.29.99]:44230 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231676AbhDHSin (ORCPT ); Thu, 8 Apr 2021 14:38:43 -0400 Received: by mail.kernel.org (Postfix) with ESMTPSA id 050CA6113A for ; Thu, 8 Apr 2021 18:38:32 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1617907112; bh=eFzHf51U5luHJHHIT+EzCxofnc4C9Q/qr5YD8mH+tzY=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=fUsuCezIVkzXoG1q+ZJJu4tDezH+CxpEFLjbyNib4Ui5VwMBLpxWeAfoE/v4VlW5e FGd+Sz6TtKJphj7XD8tPK8wnZhS2NeTdkPcq+NW++5pc01XzrjmiQm9jh9rjf9AnU4 lDR+mzSIgzx6Wlwb6VrWY54IGene5wPHFlop6W08uopaFCJ9bE+vuGWD3caXkoDaps 8mAJNuXXk/9DVbHMem7g9NE/h3KbokSZD4ZHSR4Ffz221Ccijv7tiBEi8elcvK09Wt yeBQXOu2h/Yd33iltrDITLJLG2lfpI4VVf7+iE5pQOzN4dSBhIoq97CTRo8FL48VXs YkQBgIU7Ccflg== Received: by mail-ed1-f51.google.com with SMTP id m3so3624399edv.5 for ; Thu, 08 Apr 2021 11:38:31 -0700 (PDT) X-Gm-Message-State: AOAM531r5bfxCUQ6xGj9rPFUzX7YJ7z4We9KYmWUtIgLyKw64v+AvH2I ToqDUqz5h35M2j9S007REG+42S+DPIS5JHqfKw== X-Google-Smtp-Source: ABdhPJwQVvZUqQi3NeevrsgJYQi6WLV6Puz9Ne7f37tq3Sbjx6Eg1zWhV0JxTnYmzfs+ZNNx5mMqYWFmY3YymbBRjKU= X-Received: by 2002:a50:fd12:: with SMTP id i18mr7719021eds.137.1617907110474; Thu, 08 Apr 2021 11:38:30 -0700 (PDT) MIME-Version: 1.0 References: <20210311000837.3630499-1-robh@kernel.org> <20210311000837.3630499-3-robh@kernel.org> <20210330153125.GC6567@willie-the-truck> <20210331160059.GD7815@willie-the-truck> <20210407124437.GA15622@willie-the-truck> <20210408110800.GA32792@C02TD0UTHF1T.local> In-Reply-To: <20210408110800.GA32792@C02TD0UTHF1T.local> From: Rob Herring Date: Thu, 8 Apr 2021 13:38:17 -0500 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH v6 02/10] arm64: perf: Enable PMU counter direct access for perf event To: Mark Rutland , Will Deacon Cc: Catalin Marinas , Peter Zijlstra , Ingo Molnar , Arnaldo Carvalho de Melo , Jiri Olsa , Ian Rogers , Alexander Shishkin , Honnappa Nagarahalli , Zachary.Leaf@arm.com, Raphael Gault , Jonathan Cameron , Namhyung Kim , Itaru Kitayama , linux-arm-kernel , "linux-kernel@vger.kernel.org" Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Apr 8, 2021 at 6:08 AM Mark Rutland wrote: > > On Wed, Apr 07, 2021 at 01:44:37PM +0100, Will Deacon wrote: > > [Moving Mark to To: since I'd like his view on this] > > > > On Thu, Apr 01, 2021 at 02:45:21PM -0500, Rob Herring wrote: > > > On Wed, Mar 31, 2021 at 11:01 AM Will Deacon wrote: > > > > > > > > On Tue, Mar 30, 2021 at 12:09:38PM -0500, Rob Herring wrote: > > > > > On Tue, Mar 30, 2021 at 10:31 AM Will Deacon wrote: > > > > > > > > > > > > On Wed, Mar 10, 2021 at 05:08:29PM -0700, Rob Herring wrote: > > > > > > > From: Raphael Gault > > > > > > > > +static void armv8pmu_event_unmapped(struct perf_event *event, struct mm_struct *mm) > > > > > > > +{ > > > > > > > + struct arm_pmu *armpmu = to_arm_pmu(event->pmu); > > > > > > > + > > > > > > > + if (!(event->hw.flags & ARMPMU_EL0_RD_CNTR)) > > > > > > > + return; > > > > > > > + > > > > > > > + if (atomic_dec_and_test(&mm->context.pmu_direct_access)) > > > > > > > + on_each_cpu_mask(&armpmu->supported_cpus, refresh_pmuserenr, mm, 1); > > > > > > > > > > > > Given that the pmu_direct_access field is global per-mm, won't this go > > > > > > wrong if multiple PMUs are opened by the same process but only a subset > > > > > > are exposed to EL0? Perhaps pmu_direct_access should be treated as a mask > > > > > > rather than a counter, so that we can 'and' it with the supported_cpus for > > > > > > the PMU we're dealing with. > > > > > > > > > > It needs to be a count to support multiple events on the same PMU. If > > > > > the event is not enabled for EL0, then we'd exit out on the > > > > > ARMPMU_EL0_RD_CNTR check. So I think we're fine. > > > > > > > > I'm still not convinced; pmu_direct_access is shared between PMUs, so > > > > testing the result of atomic_dec_and_test() just doesn't make sense to > > > > me, as another PMU could be playing with the count. > > > > > > How is that a problem? Let's make a concrete example: > > > > > > map PMU1:event2 -> pmu_direct_access = 1 -> enable access > > > map PMU2:event3 -> pmu_direct_access = 2 > > > map PMU1:event4 -> pmu_direct_access = 3 > > > unmap PMU2:event3 -> pmu_direct_access = 2 > > > unmap PMU1:event2 -> pmu_direct_access = 1 > > > unmap PMU1:event4 -> pmu_direct_access = 0 -> disable access > > > > > > The only issue I can see is PMU2 remains enabled for user access until > > > the last unmap. But we're sharing the mm, so who cares? Also, in this > > > scenario it is the user's problem to pin themselves to cores sharing a > > > PMU. If the user doesn't do that, they get to keep the pieces. > > > > I guess I'm just worried about exposing the counters to userspace after > > the PMU driver (or perf core?) thinks that they're no longer exposed in > > case we leak other events. > > IMO that's not practically different from the single-PMU case (i.e. > multi-PMU isn't material, either we have a concern with leaking or we > don't); more on that below. > > While it looks odd to place this on the mm, I don't think it's the end > of the world. > > > However, I'm not sure how this is supposed to work normally: what > > happens if e.g. a privileged user has a per-cpu counter for a kernel > > event while a task has a counter with direct access -- can that task > > read the kernel event out of the PMU registers from userspace? > > Yes -- userspace could go read any counters even though it isn't > supposed to, and could potentially infer information from those. It > won't have access to the config registers or kernel data structures, so > it isn't guaranteed to know what the even is or when it is > context-switched/reprogrammed/etc. > > If we believe that's a problem, then it's difficult to do anything > robust other than denying userspace access entirely, since disabling > userspace access while in use would surprise applications, and denying > privileged events would need some global state that we consult at event > creation time (in addition to being an inversion of privilege). > > IIRC there was some fuss about this a while back on x86; I'll go dig and > see what I can find, unless Peter has a memory... Maybe this one[1]. Rob [1] https://lore.kernel.org/lkml/20200730123815.18518-1-kan.liang@linux.intel.com/ From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.0 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 0DB15C433ED for ; Thu, 8 Apr 2021 18:40:48 +0000 (UTC) Received: from desiato.infradead.org (desiato.infradead.org [90.155.92.199]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id A9D3C610CB for ; Thu, 8 Apr 2021 18:40:47 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org A9D3C610CB Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=desiato.20200630; h=Sender:Content-Transfer-Encoding :Content-Type:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:Cc:To:Subject:Message-ID:Date:From:In-Reply-To: References:MIME-Version:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=SMhwIFmw2X2GLlkgle4MDiD1xWfT3+lhPEcmGjPRkys=; b=AyXJ9ngUY+eBsjUDbjXj/DP/3 CFAsZQNdGDbinBq37/7SS9oguIgdnzDYO3IIIf1JfNWHBNkCgunJdABBxzraNXD5agHC0dnIO7weP Yk7L05DLwruo7jAn+6ESq9wpHmCeLRnOb499+bwOmii3iZateXElvAgSItm8GvmlId4XC7mkz/X62 QcqmfuDzcbh16oneni0w6gdlLESN3p0Au5Sm0zUWNeEshyuRmGC8c+EHNnzqmlEMOcBTNMpNcpCoc TCtGZb29XRUDYOES24m8+pcBXOq5RWUTQqzafq2unW7cBehXf6PwSMnulcH125bGnUhy5Nj1WYWiI tiPuJovqw==; Received: from localhost ([::1] helo=desiato.infradead.org) by desiato.infradead.org with esmtp (Exim 4.94 #2 (Red Hat Linux)) id 1lUZYL-008yaU-E7; Thu, 08 Apr 2021 18:38:54 +0000 Received: from mail.kernel.org ([198.145.29.99]) by desiato.infradead.org with esmtps (Exim 4.94 #2 (Red Hat Linux)) id 1lUZY3-008yTu-L0 for linux-arm-kernel@lists.infradead.org; Thu, 08 Apr 2021 18:38:46 +0000 Received: by mail.kernel.org (Postfix) with ESMTPSA id F09CE61131 for ; Thu, 8 Apr 2021 18:38:31 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1617907112; bh=eFzHf51U5luHJHHIT+EzCxofnc4C9Q/qr5YD8mH+tzY=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=fUsuCezIVkzXoG1q+ZJJu4tDezH+CxpEFLjbyNib4Ui5VwMBLpxWeAfoE/v4VlW5e FGd+Sz6TtKJphj7XD8tPK8wnZhS2NeTdkPcq+NW++5pc01XzrjmiQm9jh9rjf9AnU4 lDR+mzSIgzx6Wlwb6VrWY54IGene5wPHFlop6W08uopaFCJ9bE+vuGWD3caXkoDaps 8mAJNuXXk/9DVbHMem7g9NE/h3KbokSZD4ZHSR4Ffz221Ccijv7tiBEi8elcvK09Wt yeBQXOu2h/Yd33iltrDITLJLG2lfpI4VVf7+iE5pQOzN4dSBhIoq97CTRo8FL48VXs YkQBgIU7Ccflg== Received: by mail-ed1-f51.google.com with SMTP id w18so3672636edc.0 for ; Thu, 08 Apr 2021 11:38:31 -0700 (PDT) X-Gm-Message-State: AOAM533BIJkm7cUCbmUUDilvySxn6BdYFjKsXHK3QU9Ak5A8s3qoQsCW mrDeNHMAz2kZHPs33FpDpXB0GpeBZkuDN48ybg== X-Google-Smtp-Source: ABdhPJwQVvZUqQi3NeevrsgJYQi6WLV6Puz9Ne7f37tq3Sbjx6Eg1zWhV0JxTnYmzfs+ZNNx5mMqYWFmY3YymbBRjKU= X-Received: by 2002:a50:fd12:: with SMTP id i18mr7719021eds.137.1617907110474; Thu, 08 Apr 2021 11:38:30 -0700 (PDT) MIME-Version: 1.0 References: <20210311000837.3630499-1-robh@kernel.org> <20210311000837.3630499-3-robh@kernel.org> <20210330153125.GC6567@willie-the-truck> <20210331160059.GD7815@willie-the-truck> <20210407124437.GA15622@willie-the-truck> <20210408110800.GA32792@C02TD0UTHF1T.local> In-Reply-To: <20210408110800.GA32792@C02TD0UTHF1T.local> From: Rob Herring Date: Thu, 8 Apr 2021 13:38:17 -0500 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH v6 02/10] arm64: perf: Enable PMU counter direct access for perf event To: Mark Rutland , Will Deacon Cc: Catalin Marinas , Peter Zijlstra , Ingo Molnar , Arnaldo Carvalho de Melo , Jiri Olsa , Ian Rogers , Alexander Shishkin , Honnappa Nagarahalli , Zachary.Leaf@arm.com, Raphael Gault , Jonathan Cameron , Namhyung Kim , Itaru Kitayama , linux-arm-kernel , "linux-kernel@vger.kernel.org" X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210408_193836_289547_6EB8F717 X-CRM114-Status: GOOD ( 45.81 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Thu, Apr 8, 2021 at 6:08 AM Mark Rutland wrote: > > On Wed, Apr 07, 2021 at 01:44:37PM +0100, Will Deacon wrote: > > [Moving Mark to To: since I'd like his view on this] > > > > On Thu, Apr 01, 2021 at 02:45:21PM -0500, Rob Herring wrote: > > > On Wed, Mar 31, 2021 at 11:01 AM Will Deacon wrote: > > > > > > > > On Tue, Mar 30, 2021 at 12:09:38PM -0500, Rob Herring wrote: > > > > > On Tue, Mar 30, 2021 at 10:31 AM Will Deacon wrote: > > > > > > > > > > > > On Wed, Mar 10, 2021 at 05:08:29PM -0700, Rob Herring wrote: > > > > > > > From: Raphael Gault > > > > > > > > +static void armv8pmu_event_unmapped(struct perf_event *event, struct mm_struct *mm) > > > > > > > +{ > > > > > > > + struct arm_pmu *armpmu = to_arm_pmu(event->pmu); > > > > > > > + > > > > > > > + if (!(event->hw.flags & ARMPMU_EL0_RD_CNTR)) > > > > > > > + return; > > > > > > > + > > > > > > > + if (atomic_dec_and_test(&mm->context.pmu_direct_access)) > > > > > > > + on_each_cpu_mask(&armpmu->supported_cpus, refresh_pmuserenr, mm, 1); > > > > > > > > > > > > Given that the pmu_direct_access field is global per-mm, won't this go > > > > > > wrong if multiple PMUs are opened by the same process but only a subset > > > > > > are exposed to EL0? Perhaps pmu_direct_access should be treated as a mask > > > > > > rather than a counter, so that we can 'and' it with the supported_cpus for > > > > > > the PMU we're dealing with. > > > > > > > > > > It needs to be a count to support multiple events on the same PMU. If > > > > > the event is not enabled for EL0, then we'd exit out on the > > > > > ARMPMU_EL0_RD_CNTR check. So I think we're fine. > > > > > > > > I'm still not convinced; pmu_direct_access is shared between PMUs, so > > > > testing the result of atomic_dec_and_test() just doesn't make sense to > > > > me, as another PMU could be playing with the count. > > > > > > How is that a problem? Let's make a concrete example: > > > > > > map PMU1:event2 -> pmu_direct_access = 1 -> enable access > > > map PMU2:event3 -> pmu_direct_access = 2 > > > map PMU1:event4 -> pmu_direct_access = 3 > > > unmap PMU2:event3 -> pmu_direct_access = 2 > > > unmap PMU1:event2 -> pmu_direct_access = 1 > > > unmap PMU1:event4 -> pmu_direct_access = 0 -> disable access > > > > > > The only issue I can see is PMU2 remains enabled for user access until > > > the last unmap. But we're sharing the mm, so who cares? Also, in this > > > scenario it is the user's problem to pin themselves to cores sharing a > > > PMU. If the user doesn't do that, they get to keep the pieces. > > > > I guess I'm just worried about exposing the counters to userspace after > > the PMU driver (or perf core?) thinks that they're no longer exposed in > > case we leak other events. > > IMO that's not practically different from the single-PMU case (i.e. > multi-PMU isn't material, either we have a concern with leaking or we > don't); more on that below. > > While it looks odd to place this on the mm, I don't think it's the end > of the world. > > > However, I'm not sure how this is supposed to work normally: what > > happens if e.g. a privileged user has a per-cpu counter for a kernel > > event while a task has a counter with direct access -- can that task > > read the kernel event out of the PMU registers from userspace? > > Yes -- userspace could go read any counters even though it isn't > supposed to, and could potentially infer information from those. It > won't have access to the config registers or kernel data structures, so > it isn't guaranteed to know what the even is or when it is > context-switched/reprogrammed/etc. > > If we believe that's a problem, then it's difficult to do anything > robust other than denying userspace access entirely, since disabling > userspace access while in use would surprise applications, and denying > privileged events would need some global state that we consult at event > creation time (in addition to being an inversion of privilege). > > IIRC there was some fuss about this a while back on x86; I'll go dig and > see what I can find, unless Peter has a memory... Maybe this one[1]. Rob [1] https://lore.kernel.org/lkml/20200730123815.18518-1-kan.liang@linux.intel.com/ _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel