From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from out-174.mta1.migadu.com (out-174.mta1.migadu.com [95.215.58.174]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 5867220A5D8 for ; Tue, 24 Jun 2025 07:05:22 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=95.215.58.174 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1750748724; cv=none; b=EBnOvvEa1tbd+GmV8jofRp9gpP5uI2T9fNFcQx4jbuurnj4YHAirZuX+L3uRMTiYOZgy0RmMWfjoeCH53nWpPaM63cxUmNnEJY4tlHpa6xxekYHAttmocwRPxhj4cRv9K7lf6hCWBOKtNtvrwvAGAlpKv9mBk6781+LsM8FL6aY= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1750748724; c=relaxed/simple; bh=uCj01wXJRSXVsrZB3o811ZhTorLLjK4lSt9J6v1K598=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=mr7pfE708T00+ULE5V0kpamYuBD5lqFGEzC8Ve1NwAjtzSuRtX38zNWp+w7CiPXOQrUgJxkHXnTc9WlpWLWAlTJbS/vpeXWLf+0RQgfhtX49JWKO17Iibfm3D2H7E3JsoZAzoNaR+NC66e/qHSYKBUPf+Y+zPd4l7finCFXc1AQ= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev; spf=pass smtp.mailfrom=linux.dev; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b=YdCNEjXQ; arc=none smtp.client-ip=95.215.58.174 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linux.dev Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b="YdCNEjXQ" Date: Tue, 24 Jun 2025 00:05:07 -0700 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1750748720; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=Rvl+LfkUxz42F+u2AhuU0DC7MpN+KcVkghfyXMKrAJU=; b=YdCNEjXQf9Nhf5KzU5i8ggHBOyIXgK50+X3MqptwbGqKbrP84GlAPmVg79d0ScGVzHNWqI 9KAlGO56nfnuIB3W0BcRK3X5GneOcZB9c6cWMPJsMsmV7O7M5uCWupxxC81piynZV/OZ4M jVC45rQOrDmJbnTY2tgEni5IU6Cvt4M= X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: Oliver Upton To: Colton Lewis Cc: kvm@vger.kernel.org, pbonzini@redhat.com, corbet@lwn.net, linux@armlinux.org.uk, catalin.marinas@arm.com, will@kernel.org, maz@kernel.org, joey.gouly@arm.com, suzuki.poulose@arm.com, yuzenghui@huawei.com, mark.rutland@arm.com, shuah@kernel.org, linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, kvmarm@lists.linux.dev, linux-perf-users@vger.kernel.org, linux-kselftest@vger.kernel.org Subject: Re: [PATCH v2 07/23] perf: arm_pmuv3: Introduce method to partition the PMU Message-ID: References: Precedence: bulk X-Mailing-List: kvm@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Migadu-Flow: FLOW_OUT On Mon, Jun 23, 2025 at 06:26:42PM +0000, Colton Lewis wrote: > Oliver Upton writes: > > > On Fri, Jun 20, 2025 at 10:13:07PM +0000, Colton Lewis wrote: > > > For PMUv3, the register field MDCR_EL2.HPMN partitiones the PMU > > > counters into two ranges where counters 0..HPMN-1 are accessible by > > > EL1 and, if allowed, EL0 while counters HPMN..N are only accessible by > > > EL2. > > > > Create module parameters partition_pmu and reserved_guest_counters to > > > reserve a number of counters for the guest. These numbers are set at > > > boot because the perf subsystem assumes the number of counters will > > > not change after the PMU is probed. > > > > Introduce the function armv8pmu_partition() to modify the PMU driver's > > > cntr_mask of available counters to exclude the counters being reserved > > > for the guest and record reserved_guest_counters as the maximum > > > allowable value for HPMN. > > > > Due to the difficulty this feature would create for the driver running > > > at EL1 on the host, partitioning is only allowed in VHE mode. Working > > > on nVHE mode would require a hypercall for every counter access in the > > > driver because the counters reserved for the host by HPMN are only > > > accessible to EL2. > > > > Signed-off-by: Colton Lewis > > > --- > > > arch/arm/include/asm/arm_pmuv3.h | 10 ++++ > > > arch/arm64/include/asm/arm_pmuv3.h | 5 ++ > > > drivers/perf/arm_pmuv3.c | 95 +++++++++++++++++++++++++++++- > > > include/linux/perf/arm_pmu.h | 1 + > > > 4 files changed, 109 insertions(+), 2 deletions(-) > > > > diff --git a/arch/arm/include/asm/arm_pmuv3.h > > > b/arch/arm/include/asm/arm_pmuv3.h > > > index 2ec0e5e83fc9..9dc43242538c 100644 > > > --- a/arch/arm/include/asm/arm_pmuv3.h > > > +++ b/arch/arm/include/asm/arm_pmuv3.h > > > @@ -228,6 +228,11 @@ static inline bool kvm_set_pmuserenr(u64 val) > > > > static inline void kvm_vcpu_pmu_resync_el0(void) {} > > > > +static inline bool has_vhe(void) > > > +{ > > > + return false; > > > +} > > > + > > > This has nothing to do with PMUv3, I'm a bit surprised to see you're > > touching 32-bit ARM. Can you just gate the whole partitioning thing on > > arm64? > > The PMUv3 driver also has to compile on 32-bit ARM. Quite aware. > My first series had the partitioning code in arch/arm64 but you asked me > to move it to the PMUv3 driver. > > How are you suggesting I square those two requirements? You should try to structure your predicates in such a way that the partitioning stuff all resolves to false for 32 bit arm, generally. That way we can avoid stubbing out silly things like has_vhe() which doesn't make sense in the context of 32 bit. > > > +static bool partition_pmu __read_mostly; > > > +static u8 reserved_guest_counters __read_mostly; > > > + > > > +module_param(partition_pmu, bool, 0); > > > +MODULE_PARM_DESC(partition_pmu, > > > + "Partition the PMU into host and guest VM counters [y/n]"); > > > + > > > +module_param(reserved_guest_counters, byte, 0); > > > +MODULE_PARM_DESC(reserved_guest_counters, > > > + "How many counters to reserve for guest VMs [0-$NR_COUNTERS]"); > > > + > > > This is confusing and not what we discussed offline. > > > Please use a single parameter that describes the number of counters used > > by the *host*. This affects the *host* PMU driver, KVM can discover (and > > use) the leftovers. > > > If the single module parameter goes unspecified the user did not ask for > > PMU partitioning. > > I understand what we discussed offline, but I had a dilemma. > > If we do a single module parameter for number of counters used by the > host, then it defaults to 0 if unset and there is no way to distinguish > between no partitioning and a request for partitioning reserving 0 > counters to the host which I also thought you requested. Would you be > happy leaving no way to specify that? You can make the command line use a signed integer for storage and a reset value of -1. -1 would imply default behavior (no partitioning) and a non-negative value would imply partitioning. > In any case, I think the usage is more self explainatory if > partitition=[y/n] is a separate bit. What would be the user's intent of "partition_pmu=n reserved_guest_counters=$X"? Thanks, Oliver