All of lore.kernel.org
 help / color / mirror / Atom feed
From: Oliver Upton <oliver.upton@linux.dev>
To: Colton Lewis <coltonlewis@google.com>
Cc: kvm@vger.kernel.org, Russell King <linux@armlinux.org.uk>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will@kernel.org>, Marc Zyngier <maz@kernel.org>,
	Joey Gouly <joey.gouly@arm.com>,
	Suzuki K Poulose <suzuki.poulose@arm.com>,
	Zenghui Yu <yuzenghui@huawei.com>,
	Mark Rutland <mark.rutland@arm.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Shuah Khan <shuah@kernel.org>,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, kvmarm@lists.linux.dev,
	linux-perf-users@vger.kernel.org,
	linux-kselftest@vger.kernel.org
Subject: Re: [RFC PATCH v2 2/4] perf: arm_pmuv3: Introduce module param to partition the PMU
Date: Sun, 9 Feb 2025 23:25:20 -0800	[thread overview]
Message-ID: <Z6mp4NeklzORJg5y@linux.dev> (raw)
In-Reply-To: <20250208020111.2068239-3-coltonlewis@google.com>

Hi Colton,

On Sat, Feb 08, 2025 at 02:01:09AM +0000, Colton Lewis wrote:
> For PMUv3, the register 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.
> 
> Introduce a module parameter in the PMUv3 driver to set this
> register. The name reserved_host_counters reflects the intent to
> reserve some counters for the host so the guest may eventually be
> allowed direct access to a subset of PMU functionality for increased
> performance.
> 
> Track HPMN and whether the pmu is partitioned in struct arm_pmu
> because KVM will need to know that to handle guests correctly.
> 
> While FEAT_HPMN0 does allow HPMN to be set to 0, this patch
> specifically disallows that case because it's not useful given the
> intention to allow guests access to their own counters.

Quite the contrary.

FEAT_HPMN0 is useful if userspace wants to provide a vPMU that has a
fixed cycle counter w/o event counters. Certain OSes refuse to boot
without it...

> static inline u32 read_mdcr(void)
> {
> 	return read_sysreg(HDCR);
> }
> 
> static inline void write_mdcr(u32 val)
> {
> 	write_sysreg(val, HDCR);
> }
> 

Hmm... While this fixes the 32bit compilation issues, it opens a new can
of worms.

VHE is a 64bit only feature, so you're *guaranteed* that these accessors
will undef (running at EL1).

> +static void armv8pmu_partition(u8 hpmn)
> +{
> +	u64 mdcr = armv8pmu_mdcr_read();
> +
> +	mdcr &= ~ARMV8_PMU_MDCR_HPMN;
> +	mdcr |= FIELD_PREP(ARMV8_PMU_MDCR_HPMN, hpmn);
> +	/* Prevent guest counters counting at EL2 */
> +	mdcr |= ARMV8_PMU_MDCR_HPMD;
> +
> +	armv8pmu_mdcr_write(mdcr);
> +}
> +

After giving this a read, I don't think the host PMU driver should care
about MDCR_EL2 at all. The only time that 'guest' events are loaded into
the PMU is between vcpu_load() / vcpu_put(), at which point *KVM* has
reconfigured MDCR_EL2.

I'm not sure if there's much involvement with the host PMU driver beyond
it pinky-swearing to only use the specified counters. KVM is what
actually will program HPMN.

That'd alleviate the PMU driver from having VHE awareness or caring
about MDCR_EL2.

-- 
Thanks,
Oliver

  reply	other threads:[~2025-02-10  7:25 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-02-08  2:01 [RFC PATCH v2 0/4] PMU partitioning driver support Colton Lewis
2025-02-08  2:01 ` [RFC PATCH v2 1/4] perf: arm_pmuv3: Generalize counter bitmasks Colton Lewis
2025-02-08  2:01 ` [RFC PATCH v2 2/4] perf: arm_pmuv3: Introduce module param to partition the PMU Colton Lewis
2025-02-10  7:25   ` Oliver Upton [this message]
2025-02-10 23:07     ` Colton Lewis
2025-02-08  2:01 ` [RFC PATCH v2 3/4] perf: arm_pmuv3: Keep out of guest counter partition Colton Lewis
2025-02-08  2:01 ` [RFC PATCH v2 4/4] KVM: arm64: Make guests see only counters they can access Colton Lewis
2025-02-10  7:37   ` Oliver Upton
2025-02-10 23:08     ` Colton Lewis

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Z6mp4NeklzORJg5y@linux.dev \
    --to=oliver.upton@linux.dev \
    --cc=catalin.marinas@arm.com \
    --cc=coltonlewis@google.com \
    --cc=joey.gouly@arm.com \
    --cc=kvm@vger.kernel.org \
    --cc=kvmarm@lists.linux.dev \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=linux@armlinux.org.uk \
    --cc=mark.rutland@arm.com \
    --cc=maz@kernel.org \
    --cc=pbonzini@redhat.com \
    --cc=shuah@kernel.org \
    --cc=suzuki.poulose@arm.com \
    --cc=will@kernel.org \
    --cc=yuzenghui@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.