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;
Subject: Re: [PATCH 11/13] dma-direct: handle the memory encryption bit in
 common code
To:     Christoph Hellwig <hch@lst.de>
Cc:     x86@kernel.org, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>,
        David Woodhouse <dwmw2@infradead.org>,
        Muli Ben-Yehuda <mulix@mulix.org>,
        Jon Mason <jdmason@kudzu.us>, Joerg Roedel <joro@8bytes.org>,
        iommu@lists.linux-foundation.org, linux-kernel@vger.kernel.org
References: <20180305174655.9878-1-hch@lst.de>
 <20180305174655.9878-12-hch@lst.de>
 <114e27be-5814-4258-4985-af08763c8a74@amd.com>
 <0c9d3e4a-8407-282a-73eb-21d28047e0e3@amd.com> <20180313131005.GA6260@lst.de>
From:   Tom Lendacky <thomas.lendacky@amd.com>
Message-ID: <69e98163-beca-fb8a-a26b-66dfeef2f689@amd.com>
Date:   Wed, 14 Mar 2018 10:44:29 -0500
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101
 Thunderbird/52.1.1
MIME-Version: 1.0
In-Reply-To: <20180313131005.GA6260@lst.de>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Received-SPF: None (protection.outlook.com: amd.com does not designate
 permitted sender hosts)
SpamDiagnosticOutput: 1:99
SpamDiagnosticMetadata: NSPM
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 14 Mar 2018 15:44:38.2748 (UTC)
X-MS-Exchange-CrossTenant-Network-Message-Id: a89d119c-582f-469b-30e4-08d589c27f19
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM5PR12MB1145
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 03/13/2018 08:10 AM, Christoph Hellwig wrote:
> On Mon, Mar 12, 2018 at 02:48:51PM -0500, Tom Lendacky wrote:
>> Ok, I found one issue that allows this to work when the IOMMU isn't
>> enabled (see below).
> 
> Thanks, folded!
> 
>> But the bigger issue is when the IOMMU is enabled.  The IOMMU code uses
>> a common mapping routine to create the I/O page tables.  This routine
>> assumes that all memory being mapped is encrypted and therefore sets the
>> encryption bit in the I/O page tables.  With this patch, the call to
>> dma_alloc_direct() now returns un-encrypted memory which results in an
>> encryption mis-match.  I think keeping dma_alloc_direct() as it was prior
>> to this patch is the way to go.  It allows SME DMA allocations to remain
>> encrypted and avoids added complexity in the amd_iommu.c file.  This
>> would mean that SEV would still have special DMA operations (so that the
>> alloc/free can change the memory to un-encrypted).
>>
>> What do you think?
> 
> In terms of logic you are right.  I still don't like keeping a just
> slightly tweaked version of dma_alloc_direct around just for this, it
> will be perpetually out of sync in terms of features and bug fixes.
> 
> What do you think about this version that does the decision at runtime:
> 
> 	http://git.infradead.org/users/hch/misc.git/commitdiff/b89f24dc856595dc7610d672bf077195ab0dabf4
> 
> The full tree is available here for testing:
> 
> 	git://git.infradead.org/users/hch/misc.git dma-direct-x86
> 

Thanks for the pointer to the tree.  I did find one bug in the
allocation routine, that once fixed (see below), worked with SME
for IOMMU on and off and worked with an SEV guest.

I understand the comment about using sev_active() in the dma-direct
code, maybe we can up with something later to address that.

Thanks,
Tom

diff --git a/lib/dma-direct.c b/lib/dma-direct.c
index 856e140..988a3d8 100644
--- a/lib/dma-direct.c
+++ b/lib/dma-direct.c
@@ -82,10 +82,12 @@ void *dma_direct_alloc(struct device *dev, size_t
size, dma_addr_t *dma_handle,

 	if (!page)
 		return NULL;
-	*dma_handle = __phys_to_dma(dev, page_to_phys(page));
+	*dma_handle = phys_to_dma(dev, page_to_phys(page));
 	ret = page_address(page);
-	if (sev_active())
+	if (sev_active()) {
+		*dma_handle = __phys_to_dma(dev, page_to_phys(page));
 		set_memory_decrypted((unsigned long)ret, 1 << page_order);
+	}
 	memset(ret, 0, size);
 	return ret;
 }