Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Tue, 7 Jan 2020 11:57:24 -0800 (PST)
From:   David Rientjes <rientjes@google.com>
To:     Christoph Hellwig <hch@lst.de>
cc:     Robin Murphy <robin.murphy@arm.com>,
        "Lendacky, Thomas" <Thomas.Lendacky@amd.com>,
        "Singh, Brijesh" <brijesh.singh@amd.com>,
        "Grimm, Jon" <jon.grimm@amd.com>, baekhw@google.com,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "iommu@lists.linux-foundation.org" <iommu@lists.linux-foundation.org>
Subject: Re: [rfc] dma-mapping: preallocate unencrypted DMA atomic pool
In-Reply-To: <20200107105458.GA3139@lst.de>
Message-ID: <alpine.DEB.2.21.2001071152020.183706@chino.kir.corp.google.com>
References: <alpine.DEB.2.21.1912311738130.68206@chino.kir.corp.google.com> <3213a6ac-5aad-62bc-bf95-fae8ba088b9e@arm.com> <20200107105458.GA3139@lst.de>
User-Agent: Alpine 2.21 (DEB 202 2017-01-01)
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Tue, 7 Jan 2020, Christoph Hellwig wrote:

> > On 01/01/2020 1:54 am, David Rientjes via iommu wrote:
> >> Christoph, Thomas, is something like this (without the diagnosic
> >> information included in this patch) acceptable for these allocations?
> >> Adding expansion support when the pool is half depleted wouldn't be *that*
> >> hard.
> >>
> >> Or are there alternatives we should consider?  Thanks!
> >
> > Are there any platforms which require both non-cacheable remapping *and* 
> > unencrypted remapping for distinct subsets of devices?
> >
> > If not (and I'm assuming there aren't, because otherwise this patch is 
> > incomplete in covering only 2 of the 3 possible combinations), then 
> > couldn't we keep things simpler by just attributing both properties to the 
> > single "atomic pool" on the basis that one or the other will always be a 
> > no-op? In other words, basically just tweaking the existing "!coherent" 
> > tests to "!coherent || force_dma_unencrypted()" and doing 
> > set_dma_unencrypted() unconditionally in atomic_pool_init().
> 
> I think that would make most sense.
> 

I'll rely on Thomas to chime in if this doesn't make sense for the SEV 
usecase.

I think the sizing of the single atomic pool needs to be determined.  Our 
peak usage that we have measured from NVMe is ~1.4MB and atomic_pool is 
currently sized to 256KB by default.  I'm unsure at this time if we need 
to be able to dynamically expand this pool with a kworker.

Maybe when CONFIG_AMD_MEM_ENCRYPT is enabled this atomic pool should be 
sized to 2MB or so and then when it reaches half capacity we schedule some 
background work to dynamically increase it?  That wouldn't be hard unless 
the pool can be rapidly depleted.

Do we want to increase the atomic pool size by default and then do 
background dynamic expansion?