Re: [PATCH v2 6/7] KVM: arm64: Upgrade PMU support to ARMv8.4

From: Marc Zyngier <maz@kernel.org>
To: Alexandru Elisei <alexandru.elisei@arm.com>
Cc: linux-arm-kernel@lists.infradead.org,
	kvmarm@lists.cs.columbia.edu, kvm@vger.kernel.org,
	James Morse <james.morse@arm.com>,
	Julien Thierry <julien.thierry.kdev@gmail.com>,
	Suzuki K Poulose <suzuki.poulose@arm.com>,
	Eric Auger <eric.auger@redhat.com>,
	kernel-team@android.com
Subject: Re: [PATCH v2 6/7] KVM: arm64: Upgrade PMU support to ARMv8.4
Date: Wed, 27 Jan 2021 17:23:50 +0000	[thread overview]
Message-ID: <54926337c1779a492f94f57ee63e5c69@kernel.org> (raw)
In-Reply-To: <cd3d33ff-6217-6a7f-3110-fe728d6c11be@arm.com>

On 2021-01-27 17:00, Alexandru Elisei wrote:
> Hi Marc,
> 
> On 1/27/21 2:35 PM, Marc Zyngier wrote:
>> Hi Alex,
>> 
>> On 2021-01-27 14:09, Alexandru Elisei wrote:
>>> Hi Marc,
>>> 
>>> On 1/25/21 12:26 PM, Marc Zyngier wrote:
>>>> Upgrading the PMU code from ARMv8.1 to ARMv8.4 turns out to be
>>>> pretty easy. All that is required is support for PMMIR_EL1, which
>>>> is read-only, and for which returning 0 is a valid option as long
>>>> as we don't advertise STALL_SLOT as an implemented event.
>>> 
>>> According to ARM DDI 0487F.b, page D7-2743:
>>> 
>>> "If ARMv8.4-PMU is implemented:
>>> - If STALL_SLOT is not implemented, it is IMPLEMENTATION DEFINED
>>> whether the PMMIR
>>> System registers are implemented.
>>> - If STALL_SLOT is implemented, then the PMMIR System registers are
>>> implemented."
>>> 
>>> I tried to come up with a reason why PMMIR is emulated instead of 
>>> being left
>>> undefined, but I couldn't figure it out. Would you mind adding a 
>>> comment or
>>> changing the commit message to explain that?
>> 
>> The main reason is that PMMIR gets new fields down the line,
>> and doing the bare minimum in term of implementation allows
>> us to gently ease into it.
> I think I understand what you are saying - add a bare minimum emulation 
> of the
> PMMIR register now so it's less work when we do decide to support the 
> STALL_SLOT
> event for a guest.
>> 
>> We could also go for the full PMMIR reporting on homogeneous
>> systems too, as a further improvement.
>> 
>> What do you think?
> 
> I don't have an opinion either way. But if you do decide to add full
> emulation for
> STALL_SLOT, I would like to help with reviewing the patches (I'm 
> curious to see
> how KVM will detect that it's running on a homogeneous system).

I'm not sure we can *detect* it. We'd need some more information
from the core arch code and firmware. To be honest, PMU emulation
is a joke on BL, so maybe we shouldn't really care and expose
what we have.

         M.
-- 
Jazz is not dead. It just smells funny...