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=-8.5 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=unavailable 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 CC148C433E0 for ; Wed, 29 Jul 2020 07:10:38 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (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 7EC36206D7 for ; Wed, 29 Jul 2020 07:10:38 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="d47mvxVT"; dkim=fail reason="signature verification failed" (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b="wvW/rLi3" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 7EC36206D7 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linaro.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=merlin.20170209; h=Sender:Content-Transfer-Encoding: Content-Type:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References:Message-ID: Subject:To:From:Date:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=HOwGmAVbL0yup0RlXQjdmhgIP8w+5XxP/oHzxnfJhCg=; b=d47mvxVT6GaVY7BbiHBMWtmVN PcSBa7Vx11qHtMKqTv3FLziMssjzKBv7abweD6CR5HDPDIR1MR0RTs+kBGX7H/z8l9ii3jGHIwh5+ QLtse36IXrfTfuBVNd/mag0eA+c98CsKvQGV+gyXD+Tch1PBAu06xroSSmrjU/py1FVnEXQSm+OM8 jKIVkYMnwCOH9rGhY5ttseMCD2Y+909xFDnumcxcDUL81LRUYirft0SNX5CyRRD2ydq4fa9n6OI+5 ETFrZXth1CfWri4XSrDd3O4WC2b4zuvCzE8YD/BU2M+xe+RRwiBqvJnN2SzfJobz/qAYTfIsIez6Y k/9zOwSXA==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1k0gDA-0006zL-Ko; Wed, 29 Jul 2020 07:09:12 +0000 Received: from mail-pg1-x542.google.com ([2607:f8b0:4864:20::542]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1k0gD8-0006yX-0f for linux-arm-kernel@lists.infradead.org; Wed, 29 Jul 2020 07:09:11 +0000 Received: by mail-pg1-x542.google.com with SMTP id d4so13773844pgk.4 for ; Wed, 29 Jul 2020 00:09:09 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=2obqLjys8AeYSVhAJYGRsNrAjNS+DMczZ9/H7mL1HFo=; b=wvW/rLi3izA2Xhmlvs6bxFF2t+hckjft3bkzJuxO8hAs5k1q5smDywONrcHFowXXxi BkRXBVZGbAKp9KzowuitT+2FZdD7DKUHa+TYO/1MO5hCT5hKgJ+tWcdpn6QwfBWiGOll 1+sVz8iGFQ4TYrhUCaiAv9SUWUk1Lo1HBDh1XHHCuegSu44GcD46lOyBSbbjQvkiylbY l6ksv4NfWTWR4j6hUX5JGViou692sRcvZup/gxwUF5ShFpzSmm2asK+7Zq5awvj2WrLL wSvIURVcKP2mzrNf1OI7w/GXQs5zw4GHu3cef+K8Bg42Mp7pDKSqUnzsSF76o6rwFj4H Osig== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=2obqLjys8AeYSVhAJYGRsNrAjNS+DMczZ9/H7mL1HFo=; b=sbe2yjGUflf4Of45z7F/SmSIf5H3gzyhMogcbWkI+daMQ9X1r7PemxLcnm6qgFLA4A jqu7cjVp6QlwdSSWjNjKqxH3gxYNn46HL3OvXeZy2zEH8RLJEDQEaMk0gGy7UIWNkQQh NpnOBtIgzAVMgqEOjPR1gpl7Y4aqwkmOH2/wLwyjqR+2FDfkl2FrJi+LNnzwHgT6hPcE doxdXonQmo8gLM6yuOfpCVXj3scq696KBHKcO0p4Qy00JRJwttXa8Fk3S8Mtc1f4Uy/J tV/cbgaiG6pTTO8mo757kgGbpZY1Qu7LXGEPRhV3g5tICTb2svqpoCztkCBvD65UyaK4 5cuA== X-Gm-Message-State: AOAM533iIz1MDBFH76kZ+VBwam0Ye/mgKlLNS1rGsM1nIVfZInTR9EgS MSbNNg4UZAjr3EPxLmRRmO8mkQ== X-Google-Smtp-Source: ABdhPJzBA6ghqqfikYLCUBQXKEqaYTg6w2z2pyFkR+M88VMWpZUPAcv/psv5HZVaJYNg9uk9UoKXvw== X-Received: by 2002:aa7:971d:: with SMTP id a29mr18617453pfg.308.1596006547727; Wed, 29 Jul 2020 00:09:07 -0700 (PDT) Received: from leoy-ThinkPad-X240s ([2600:3c01::f03c:91ff:fe8a:bb03]) by smtp.gmail.com with ESMTPSA id e124sm1075048pfe.176.2020.07.29.00.08.58 (version=TLS1_2 cipher=ECDHE-ECDSA-CHACHA20-POLY1305 bits=256/256); Wed, 29 Jul 2020 00:09:07 -0700 (PDT) Date: Wed, 29 Jul 2020 15:08:55 +0800 From: Leo Yan To: "liwei (GF)" , Al Grant Subject: Re: [PATCH 1/4] drivers/perf: Add support for ARMv8.3-SPE Message-ID: <20200729070855.GG4343@leoy-ThinkPad-X240s> References: <20200724091607.41903-1-liwei391@huawei.com> <20200724091607.41903-2-liwei391@huawei.com> <20200728122742.GB4343@leoy-ThinkPad-X240s> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.9.4 (2018-02-28) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200729_030910_212530_23EC671A X-CRM114-Status: GOOD ( 29.83 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Mark Rutland , Suzuki K Poulose , Alexander Shishkin , Catalin Marinas , Jiri Olsa , Adrian Hunter , Arnaldo Carvalho de Melo , linux-kernel@vger.kernel.org, zhangshaokun@hisilicon.com, Peter Zijlstra , Ingo Molnar , James Clark , guohanjun@huawei.com, Namhyung Kim , Will Deacon , linux-arm-kernel@lists.infradead.org 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 Tue, Jul 28, 2020 at 09:24:42PM +0800, liwei (GF) wrote: [...] > >> diff --git a/drivers/perf/arm_spe_pmu.c b/drivers/perf/arm_spe_pmu.c > >> index e51ddb6d63ed..5ec7ee0c8fa1 100644 > >> --- a/drivers/perf/arm_spe_pmu.c > >> +++ b/drivers/perf/arm_spe_pmu.c > >> @@ -54,7 +54,7 @@ struct arm_spe_pmu { > >> struct hlist_node hotplug_node; > >> > >> int irq; /* PPI */ > >> - > >> + int pmuver; > > > > Since the version number is only 4 bits width, 'u16' would be enough > > to record SPE version number. > > Sounds reasonable, i can change it to 'u16' if you insist. > > >> u16 min_period; > >> u16 counter_sz; > >> > >> @@ -80,6 +80,15 @@ struct arm_spe_pmu { > >> /* Keep track of our dynamic hotplug state */ > >> static enum cpuhp_state arm_spe_pmu_online; > >> > >> +static u64 sys_pmsevfr_el1_mask[] = { > >> + [ID_AA64DFR0_PMSVER_8_2] = GENMASK_ULL(63, 48) | GENMASK_ULL(31, 24) | > >> + GENMASK_ULL(15, 12) | BIT_ULL(7) | BIT_ULL(5) | BIT_ULL(3) | > >> + BIT_ULL(1), > >> + [ID_AA64DFR0_PMSVER_8_3] = GENMASK_ULL(63, 48) | GENMASK_ULL(31, 24) | > >> + GENMASK_ULL(18, 17) | GENMASK_ULL(15, 11) | BIT_ULL(7) | > >> + BIT_ULL(5) | BIT_ULL(3) | BIT_ULL(1), > >> +}; > > > > Seems to me, the definitions for Aarch64 system registers should be > > placed into the file 'arch/arm64/include/asm/sysreg.h'. Like below > > two macros: > > > > #define SYS_PMSEVFR_EL1_RES0_8_2 0x0000ffff00ff0f55UL > > #define SYS_PMSEVFR_EL1_RES0_8_3 ... > > I really think using GENMASK_ULL() to generate the mask is better than a definition > with magic number. It is beneficial to be reviewed and extended later. Understand. Here I just want to remind, you could see the ARMv8's system registers definition usually are placed into the global header sysreg.h rather than define them in separate source files. You could define the bit mask with GENMASK_ULL() for the two macros in sysreg.h. > > Let's wait for Will or Mark Rutland's comments for this, in case I > > mislead for this. > >> + > >> enum arm_spe_pmu_buf_fault_action { > >> SPE_PMU_BUF_FAULT_ACT_SPURIOUS, > >> SPE_PMU_BUF_FAULT_ACT_FATAL, > >> @@ -670,7 +679,7 @@ static int arm_spe_pmu_event_init(struct perf_event *event) > >> !cpumask_test_cpu(event->cpu, &spe_pmu->supported_cpus)) > >> return -ENOENT; > >> > >> - if (arm_spe_event_to_pmsevfr(event) & SYS_PMSEVFR_EL1_RES0) > >> + if (arm_spe_event_to_pmsevfr(event) & ~sys_pmsevfr_el1_mask[spe_pmu->pmuver]) > >> return -EOPNOTSUPP; > >> > >> if (attr->exclude_idle) > >> @@ -937,6 +946,7 @@ static void __arm_spe_pmu_dev_probe(void *info) > >> fld, smp_processor_id()); > >> return; > >> } > >> + spe_pmu->pmuver = fld; > >> > >> /* Read PMBIDR first to determine whether or not we have access */ > >> reg = read_sysreg_s(SYS_PMBIDR_EL1); > >> @@ -1027,8 +1037,8 @@ static void __arm_spe_pmu_dev_probe(void *info) > >> } > >> > >> dev_info(dev, > >> - "probed for CPUs %*pbl [max_record_sz %u, align %u, features 0x%llx]\n", > >> - cpumask_pr_args(&spe_pmu->supported_cpus), > >> + "v%d probed for CPUs %*pbl [max_record_sz %u, align %u, features 0x%llx]\n", > > > > Let's output explict info, like: > > > > "probed for CPUs %*pbl [pmuver %d, max_record_sz %u, align %u, features 0x%llx]\n", > > > > Agree, and i have a little question here: > Currently, the of_compatible of SPE PMU is "arm,statistical-profiling-extension-v1", and > the platform_device name is "arm,spe-v1". So this message looks weird when supporting > ARMv8.3-SPE because the pmuver is 2. I think here we need to distinguish two things: SPE (as an IP) and ARMv8.2/ARMv8.3 (as CPU architectures). From my understanding, now we are working on SPE-v1, but it needs to support ARMv8 variants, e.g. ARMv8.2 and ARMv8.3 with SVE extension. I am not the best person to clarify the version number for SPE, if Arm colleagues disagree with this, very welcome to correct me. Also loop in Al for this. > As the version of SPE can be probed by reading 'ID_AA64DFR0_EL1.PMSVer', can we remove > the version hint in of_compatible and platform_device name? No, for device tree, usually we need to keep back compability for the DT binding, so we cannot remove compatible string. Thanks, Leo _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel