Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752141AbdFOJlX (ORCPT ); Thu, 15 Jun 2017 05:41:23 -0400 Received: from mail.skyhub.de ([5.9.137.197]:33452 "EHLO mail.skyhub.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750861AbdFOJlV (ORCPT ); Thu, 15 Jun 2017 05:41:21 -0400 Date: Thu, 15 Jun 2017 11:41:12 +0200 From: Borislav Petkov To: Tom Lendacky Cc: linux-arch@vger.kernel.org, linux-efi@vger.kernel.org, kvm@vger.kernel.org, linux-doc@vger.kernel.org, x86@kernel.org, kexec@lists.infradead.org, linux-kernel@vger.kernel.org, kasan-dev@googlegroups.com, linux-mm@kvack.org, iommu@lists.linux-foundation.org, Rik van Riel , Radim =?utf-8?B?S3LEjW3DocWZ?= , Toshimitsu Kani , Arnd Bergmann , Jonathan Corbet , Matt Fleming , "Michael S. Tsirkin" , Joerg Roedel , Konrad Rzeszutek Wilk , Paolo Bonzini , Larry Woodman , Brijesh Singh , Ingo Molnar , Andy Lutomirski , "H. Peter Anvin" , Andrey Ryabinin , Alexander Potapenko , Dave Young , Thomas Gleixner , Dmitry Vyukov Subject: Re: [PATCH v6 26/34] iommu/amd: Allow the AMD IOMMU to work with memory encryption Message-ID: <20170615094111.wga334kg2bhxqib3@pd.tnic> References: <20170607191309.28645.15241.stgit@tlendack-t1.amdoffice.net> <20170607191745.28645.81756.stgit@tlendack-t1.amdoffice.net> <20170614174208.p2yr5exs4b6pjxhf@pd.tnic> <0611d01a-19f8-d6ae-2682-932789855518@amd.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <0611d01a-19f8-d6ae-2682-932789855518@amd.com> User-Agent: NeoMutt/20170113 (1.7.2) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1910 Lines: 58 On Wed, Jun 14, 2017 at 03:40:28PM -0500, Tom Lendacky wrote: > I was trying to keep all the logic for it here in the SME related files > rather than put it in the iommu code itself. But it is easy enough to > move if you think it's worth it. Yes please - the less needlessly global symbols, the better. > > Also, you said in another mail on this subthread that c->microcode > > is not yet set. Are you saying, that the iommu init gunk runs before > > init_amd(), where we do set c->microcode? > > > > If so, we can move the setting to early_init_amd() or so. > > I'll look into that. And I don't think c->microcode is not set by the time we init the iommu because, AFAICT, we do the latter in pci_iommu_init() and that's a rootfs_initcall() which happens later then the CPU init stuff. > I'll look into simplifying the checks. Something like this maybe? if (rev >= 0x1205) return true; if (rev <= 0x11ff && rev >= 0x1126) return true; return false; > > WARNING: Use of volatile is usually wrong: see Documentation/process/volatile-considered-harmful.rst > > #134: FILE: drivers/iommu/amd_iommu.c:866: > > +static void build_completion_wait(struct iommu_cmd *cmd, volatile u64 *sem) > > > > The semaphore area is written to by the device so the use of volatile is > appropriate in this case. Do you mean this is like the last exception case in that document above: " - Pointers to data structures in coherent memory which might be modified by I/O devices can, sometimes, legitimately be volatile. A ring buffer used by a network adapter, where that adapter changes pointers to indicate which descriptors have been processed, is an example of this type of situation." ? If so, it did work fine until now, without the volatile. Why is it needed now, all of a sudden? -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply.