Message-ID: <9b5104631f827fe44ad1030c971c72fba9a8ac90.camel@ndufresne.ca>
Date: Wed, 05 Feb 2025 13:07:25 -0500
From: Nicolas Dufresne <nicolas@...fresne.ca>
To: Maxime Ripard <mripard@...nel.org>
Cc: Florent Tomasin <florent.tomasin@....com>, Vinod Koul
 <vkoul@...nel.org>,  Rob Herring <robh@...nel.org>, Krzysztof Kozlowski
 <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>,  Boris Brezillon
 <boris.brezillon@...labora.com>, Steven Price <steven.price@....com>, Liviu
 Dudau	 <liviu.dudau@....com>, Maarten Lankhorst
 <maarten.lankhorst@...ux.intel.com>,  Thomas Zimmermann
 <tzimmermann@...e.de>, David Airlie <airlied@...il.com>, Simona Vetter
 <simona@...ll.ch>, Sumit Semwal <sumit.semwal@...aro.org>, Benjamin
 Gaignard <benjamin.gaignard@...labora.com>, Brian Starkey
 <Brian.Starkey@....com>, John Stultz <jstultz@...gle.com>, "T . J .
 Mercier"	 <tjmercier@...gle.com>, Christian König	
 <christian.koenig@....com>, Matthias Brugger <matthias.bgg@...il.com>, 
 AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>, Yong
 Wu <yong.wu@...iatek.com>, dmaengine@...r.kernel.org, 
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org, 
	dri-devel@...ts.freedesktop.org, linux-media@...r.kernel.org, 
	linaro-mm-sig@...ts.linaro.org, linux-arm-kernel@...ts.infradead.org, 
	linux-mediatek@...ts.infradead.org, nd@....com, Akash Goel
 <akash.goel@....com>
Subject: Re: [RFC PATCH 0/5] drm/panthor: Protected mode support for Mali
 CSF GPUs

Le mercredi 05 février 2025 à 15:53 +0100, Maxime Ripard a écrit :
> On Tue, Feb 04, 2025 at 01:22:58PM -0500, Nicolas Dufresne wrote:
> > Le lundi 03 février 2025 à 16:43 +0000, Florent Tomasin a écrit :
> > > Hi Maxime, Nicolas
> > > 
> > > On 30/01/2025 17:47, Nicolas Dufresne wrote:
> > > > Le jeudi 30 janvier 2025 à 17:38 +0100, Maxime Ripard a écrit :
> > > > > Hi Nicolas,
> > > > > 
> > > > > On Thu, Jan 30, 2025 at 10:59:56AM -0500, Nicolas Dufresne wrote:
> > > > > > Le jeudi 30 janvier 2025 à 14:46 +0100, Maxime Ripard a écrit :
> > > > > > > Hi,
> > > > > > > 
> > > > > > > I started to review it, but it's probably best to discuss it here.
> > > > > > > 
> > > > > > > On Thu, Jan 30, 2025 at 01:08:56PM +0000, Florent Tomasin wrote:
> > > > > > > > Hi,
> > > > > > > > 
> > > > > > > > This is a patch series covering the support for protected mode execution in
> > > > > > > > Mali Panthor CSF kernel driver.
> > > > > > > > 
> > > > > > > > The Mali CSF GPUs come with the support for protected mode execution at the
> > > > > > > > HW level. This feature requires two main changes in the kernel driver:
> > > > > > > > 
> > > > > > > > 1) Configure the GPU with a protected buffer. The system must provide a DMA
> > > > > > > >    heap from which the driver can allocate a protected buffer.
> > > > > > > >    It can be a carved-out memory or dynamically allocated protected memory region.
> > > > > > > >    Some system includes a trusted FW which is in charge of the protected memory.
> > > > > > > >    Since this problem is integration specific, the Mali Panthor CSF kernel
> > > > > > > >    driver must import the protected memory from a device specific exporter.
> > > > > > > 
> > > > > > > Why do you need a heap for it in the first place? My understanding of
> > > > > > > your series is that you have a carved out memory region somewhere, and
> > > > > > > you want to allocate from that carved out memory region your buffers.
> > > > > > > 
> > > > > > > How is that any different from using a reserved-memory region, adding
> > > > > > > the reserved-memory property to the GPU device and doing all your
> > > > > > > allocation through the usual dma_alloc_* API?
> > > > > > 
> > > > > > How do you then multiplex this region so it can be shared between
> > > > > > GPU/Camera/Display/Codec drivers and also userspace ?
> > > > > 
> > > > > You could point all the devices to the same reserved memory region, and
> > > > > they would all allocate from there, including for their userspace-facing
> > > > > allocations.
> > > > 
> > > > I get that using memory region is somewhat more of an HW description, and
> > > > aligned with what a DT is supposed to describe. One of the challenge is that
> > > > Mediatek heap proposal endup calling into their TEE, meaning knowing the region
> > > > is not that useful. You actually need the TEE APP guid and its IPC protocol. If
> > > > we can dell drivers to use a head instead, we can abstract that SoC specific
> > > > complexity. I believe each allocated addressed has to be mapped to a zone, and
> > > > that can only be done in the secure application. I can imagine similar needs
> > > > when the protection is done using some sort of a VM / hypervisor.
> > > > 
> > > > Nicolas
> > > > 
> > > 
> > > The idea in this design is to abstract the heap management from the
> > > Panthor kernel driver (which consumes a DMA buffer from it).
> > > 
> > > In a system, an integrator would have implemented a secure heap driver,
> > > and could be based on TEE or a carved-out memory with restricted access,
> > > or else. This heap driver would be responsible of implementing the
> > > logic to: allocate, free, refcount, etc.
> > > 
> > > The heap would be retrieved by the Panthor kernel driver in order to
> > > allocate protected memory to load the FW and allow the GPU to enter/exit
> > > protected mode. This memory would not belong to a user space process.
> > > The driver allocates it at the time of loading the FW and initialization
> > > of the GPU HW. This is a device globally owned protected memory.
> > 
> > This use case also applies well for codec. The Mediatek SCP firmware needs to be
> > loaded with a restricted memory too, its a very similar scenario, plus Mediatek
> > chips often include a Mali. On top of that, V4L2 codecs (in general) do need to
> > allocate internal scratch buffer for the IP to write to for things like motion
> > vectors, reconstruction frames, entropy statistics, etc. The IP will only be
> > able to write if the memory is restricted.
> 
> BTW, in such a case, do the scratch buffers need to be
> protected/secure/whatever too, or would codecs be able to use any buffer
> as a scratch buffer?

They need to be protected yes. Its not very fine grained on the platform I work
on. When that protection is enabled, the decoder can only read and write from
protected memory. I know there is platform were it can read from both, but
generally all IOs regardless what they are used for endup with the same
restriction.

Nicolas

p.s. since Khronos seems to have adopted "protected", perhaps it will be advised
to go for that in the end.

> 
> Maxime

Powered by blists - more mailing lists