Date: Tue, 27 Feb 2024 10:49:32 +0100
From: David Hildenbrand <david@...hat.com>
To: Christoph Hellwig <hch@...radead.org>, Will Deacon <will@...nel.org>,
 Quentin Perret <qperret@...gle.com>,
 Chris Goldsworthy <quic_cgoldswo@...cinc.com>,
 Android KVM <android-kvm@...gle.com>, Patrick Daly <quic_pdaly@...cinc.com>,
 Alex Elder <elder@...aro.org>,
 Srinivas Kandagatla <srinivas.kandagatla@...aro.org>,
 Murali Nalajal <quic_mnalajal@...cinc.com>,
 Trilok Soni <quic_tsoni@...cinc.com>,
 Srivatsa Vaddagiri <quic_svaddagi@...cinc.com>,
 Carl van Schaik <quic_cvanscha@...cinc.com>,
 Philip Derrin <quic_pderrin@...cinc.com>,
 Prakruthi Deepak Heragu <quic_pheragu@...cinc.com>,
 Jonathan Corbet <corbet@....net>, Rob Herring <robh+dt@...nel.org>,
 Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
 Conor Dooley <conor+dt@...nel.org>, Catalin Marinas
 <catalin.marinas@....com>, Konrad Dybcio <konrad.dybcio@...aro.org>,
 Bjorn Andersson <andersson@...nel.org>,
 Dmitry Baryshkov <dmitry.baryshkov@...aro.org>, Fuad Tabba
 <tabba@...gle.com>, Sean Christopherson <seanjc@...gle.com>,
 Andrew Morton <akpm@...ux-foundation.org>, linux-arm-msm@...r.kernel.org,
 linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org,
 devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
 linux-mm@...ck.org
Subject: Re: [PATCH v17 19/35] arch/mm: Export direct {un,}map functions

On 26.02.24 18:27, Elliot Berman wrote:
> On Mon, Feb 26, 2024 at 12:53:48PM +0100, David Hildenbrand wrote:
>> On 26.02.24 12:06, Christoph Hellwig wrote:
>>> The point is that we can't we just allow modules to unmap data from
>>> the kernel mapping, no matter how noble your intentions are.
>>
>> I absolutely agree.
>>
> 
> Hi David and Chirstoph,
> 
> Are your preferences that we should make Gunyah builtin only or should add
> fixing up S2 PTW errors (or something else)?

Having that built into the kernel certainly does sound better than 
exposing that functionality to arbitrary OOT modules. But still, this 
feels like it is using a "too-low-level" interface.

> 
> Also, do you extend that preference to modifying S2 mappings? This would
> require any hypervisor driver that supports confidential compute
> usecases to only ever be builtin.
> 
> Is your concern about unmapping data from kernel mapping, then module
> being unloaded, and then having no way to recover the mapping? Would a
> permanent module be better? The primary reason we were wanting to have
> it as module was to avoid having driver in memory if you're not a Gunyah
> guest.

What I didn't grasp from this patch description: is the area where a 
driver would unmap/remap that memory somehow known ahead of time and 
limited?

How would the driver obtain that memory it would try to unmap/remap the 
direct map of? Simply allocate some pages and then unmap the direct map?

For example, we do have mm/secretmem.c, where we unmap the directmap on 
allocation and remap when freeing a page. A nice abstraction on 
alloc/free, so one cannot really do a lot of harm.

Further, we enlightened the remainder of the system about secretmem, 
such that we can detect that the directmap is no longer there. As one 
example, see the secretmem_active() check in kernel/power/hibernate.c.

A similar abstraction would make sense (I remember a discussion about 
having secretmem functionality in guest_memfd, would that help?), but 
the question is "which" memory you want to unmap the direct map of, and 
how the driver became "owner" of that memory such that it would really 
be allowed to mess with the directmap.

-- 
Cheers,

David / dhildenb

Powered by blists - more mailing lists