Received-SPF: pass (google.com: domain of linux-kernel+bounces-52937-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Message-ID: <c773393c-221e-edd1-00b7-0ce6a2481c15@arm.com>
Date: Mon, 5 Feb 2024 15:37:34 +0000
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.15.1
Subject: Re: [PATCH v4 2/7] arm64: KVM: Use shared area to pass PMU event
 state to hypervisor
Content-Language: en-US
To: Marc Zyngier <maz@kernel.org>
Cc: Oliver Upton <oliver.upton@linux.dev>, coresight@lists.linaro.org,
 linux-arm-kernel@lists.infradead.org, kvmarm@lists.linux.dev,
 broonie@kernel.org, suzuki.poulose@arm.com, acme@kernel.org,
 James Morse <james.morse@arm.com>, Zenghui Yu <yuzenghui@huawei.com>,
 Catalin Marinas <catalin.marinas@arm.com>, Will Deacon <will@kernel.org>,
 Mike Leach <mike.leach@linaro.org>, Leo Yan <leo.yan@linaro.org>,
 Alexander Shishkin <alexander.shishkin@linux.intel.com>,
 Anshuman Khandual <anshuman.khandual@arm.com>, Rob Herring
 <robh@kernel.org>, Miguel Luis <miguel.luis@oracle.com>,
 Jintack Lim <jintack.lim@linaro.org>, Ard Biesheuvel <ardb@kernel.org>,
 Mark Rutland <mark.rutland@arm.com>, Arnd Bergmann <arnd@arndb.de>,
 Vincent Donnefort <vdonnefort@google.com>,
 Kristina Martsenko <kristina.martsenko@arm.com>,
 Fuad Tabba <tabba@google.com>, Joey Gouly <joey.gouly@arm.com>,
 Akihiko Odaki <akihiko.odaki@daynix.com>, Jing Zhang
 <jingzhangos@google.com>, linux-kernel@vger.kernel.org
References: <20240104162714.1062610-1-james.clark@arm.com>
 <20240104162714.1062610-3-james.clark@arm.com> <Zb1mCCi13AJ_YjFZ@linux.dev>
 <8a908ee8-620a-d9c2-734b-5a6402950072@arm.com> <ZcDc8-FQo8wKavA4@linux.dev>
 <867cjj6ohz.wl-maz@kernel.org> <ZcDg1sP3EYZG-i_3@linux.dev>
 <a7cc7791-c476-fd81-b79d-a151293b8302@arm.com> <864jen6k0i.wl-maz@kernel.org>
From: James Clark <james.clark@arm.com>
In-Reply-To: <864jen6k0i.wl-maz@kernel.org>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit


On 05/02/2024 14:52, Marc Zyngier wrote:
> On Mon, 05 Feb 2024 14:17:10 +0000,
> James Clark <james.clark@arm.com> wrote:
>>
>> On 05/02/2024 13:21, Oliver Upton wrote:
>>> On Mon, Feb 05, 2024 at 01:15:36PM +0000, Marc Zyngier wrote:
>>>> On Mon, 05 Feb 2024 13:04:51 +0000,
>>>> Oliver Upton <oliver.upton@linux.dev> wrote:
>>>>>
>>>>> Unless someone has strong opinions about making this work in protected
>>>>> mode, I am happy to see tracing support limited to the 'normal' nVHE
>>>>> configuration. The protected feature as a whole is just baggage until
>>>>> upstream support is completed.
>>>>
>>>> Limiting tracing to non-protected mode is a must IMO. Allowing tracing
>>>> when pKVM is enabled is a sure way to expose secrets that should
>>>> stay... secret. The only exception I can think of is when
>>>> CONFIG_NVHE_EL2_DEBUG is enabled, at which point all bets are off.
>>>
>>> Zero argument there :) I left off the "and PMU" part of what I was
>>> saying, because that was a feature that semi-worked in protected mode
>>> before VM/VCPU shadowing support landed.
>>>
>>
>> In that case I can hide all this behind CONFIG_NVHE_EL2_DEBUG for pKVM.
>> This will also have the effect of disabling PMU again for pKVM because I
>> moved that into this new shared area.
> 
> I'm not sure what you have in mind, but dropping PMU support for
> non-protected guests when protected-mode is enabled is not an
> acceptable outcome.
> 
> Hiding the trace behind a debug option is fine as this is a global
> setting that has no userspace impact, but impacting guests isn't.
> 
> 	M.
> 

Hmmm in that case if there's currently no way to distinguish between
normal VMs and pVMs in protected-mode then what I was thinking of
probably won't work.

I'll actually just leave PMU as it is and only have tracing disabled in
protected-mode.

My only question now is whether to:

 * Keep this new shared area and use it for both PMU and trace status
   (well, for PMU only in protected mode as trace would always be
   disabled and doesn't actually need any state)

 * Delete patch 2, add a new normal per-cpu struct just for
   trace status that's only used in non-protected mode and revert to
   copying the PMU status into the vCPU on guest switch as it was
   previously.