Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S980387AbdDYDny (ORCPT ); Mon, 24 Apr 2017 23:43:54 -0400 Received: from mail-oi0-f65.google.com ([209.85.218.65]:34280 "EHLO mail-oi0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S980357AbdDYDnm (ORCPT ); Mon, 24 Apr 2017 23:43:42 -0400 MIME-Version: 1.0 In-Reply-To: <20170424154530.GO12323@arm.com> References: <1492623846-29335-1-git-send-email-ganapatrao.kulkarni@cavium.com> <20170420084928.GC31436@leverpostej> <20170424154530.GO12323@arm.com> From: Ganapatrao Kulkarni Date: Tue, 25 Apr 2017 09:13:40 +0530 Message-ID: Subject: Re: [PATCH v2] arm64: perf: Use only exclude_kernel attribute when kernel is running in HYP To: Will Deacon Cc: Mark Rutland , "Andrew.Pinski@caviumnetworks.com" , Ganapatrao Kulkarni , "linux-kernel@vger.kernel.org" , "linux-arm-kernel@lists.infradead.org" , Catalin Marinas , acme@kernel.org, alexander.shishkin@linux.intel.com, peterz@infradead.org, Ingo Molnar , jnair@caviumnetworks.com Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2847 Lines: 68 On Mon, Apr 24, 2017 at 9:15 PM, Will Deacon wrote: > On Thu, Apr 20, 2017 at 02:56:50PM +0530, Ganapatrao Kulkarni wrote: >> On Thu, Apr 20, 2017 at 2:19 PM, Mark Rutland wrote: >> > On Wed, Apr 19, 2017 at 11:14:06PM +0530, Ganapatrao Kulkarni wrote: >> >> commit d98ecda (arm64: perf: Count EL2 events if the kernel is running in HYP) >> >> is returning error for perf syscall with mixed attribute set for exclude_kernel >> >> and exclude_hv. This change is breaking some applications (observed with hhvm) >> >> when ran on VHE enabled platforms. >> >> >> >> Adding fix to consider only exclude_kernel attribute when kernel is >> >> running in HYP. Also adding sysfs file to notify the bhehaviour >> >> of attribute exclude_hv. >> >> >> >> Signed-off-by: Ganapatrao Kulkarni >> >> --- >> >> >> >> Changelog: >> >> >> >> V2: >> >> - Changes as per Will Deacon's suggestion. >> >> >> >> V1: Initial patch >> >> >> >> arch/arm64/kernel/perf_event.c | 28 ++++++++++++++++++++++++---- >> >> include/linux/perf/arm_pmu.h | 1 + >> >> 2 files changed, 25 insertions(+), 4 deletions(-) >> >> >> >> @@ -871,14 +890,13 @@ static int armv8pmu_set_event_filter(struct hw_perf_event *event, >> >> >> >> if (attr->exclude_idle) >> >> return -EPERM; >> >> - if (is_kernel_in_hyp_mode() && >> >> - attr->exclude_kernel != attr->exclude_hv) >> >> - return -EINVAL; >> >> + if (is_kernel_in_hyp_mode() && !attr->exclude_kernel) >> >> + config_base |= ARMV8_PMU_INCLUDE_EL2; >> >> if (attr->exclude_user) >> >> config_base |= ARMV8_PMU_EXCLUDE_EL0; >> >> if (!is_kernel_in_hyp_mode() && attr->exclude_kernel) >> >> config_base |= ARMV8_PMU_EXCLUDE_EL1; >> >> - if (!attr->exclude_hv) >> >> + if (!is_kernel_in_hyp_mode() && !attr->exclude_hv) >> >> config_base |= ARMV8_PMU_INCLUDE_EL2; >> > >> > This isn't quite what Will suggested. >> > >> > The idea was that userspace would read sysfs, then use that to determine >> > the correct exclusion parameters [1,2]. This logic was not expected to >> > change; it correctly validates whether we can provide what the user >> > requests. >> >> OK, if you are ok with sysfs part, i can send next version with that >> change only?. > > I think the sysfs part is still a little dodgy, since you still expose the > "exclude_hv" file with a value of 0 when not running at EL2, which would > imply that exclude_hv is forced to zero. I don't think that's correct. okay, i can make exclude_hv visible only when kernel booted in EL2. is it ok to have empty directory "attr" when kernel booted to EL1? attr can be place holder for any other miscellaneous attributes, that can be added in future. > > Will thanks Ganapat