From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751641AbdFOPdf (ORCPT ); Thu, 15 Jun 2017 11:33:35 -0400 Received: from mail.skyhub.de ([5.9.137.197]:45896 "EHLO mail.skyhub.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750728AbdFOPdd (ORCPT ); Thu, 15 Jun 2017 11:33:33 -0400 Date: Thu, 15 Jun 2017 17:33:22 +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: <20170615153322.nwylo3dzn4fdx6n6@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> <20170615094111.wga334kg2bhxqib3@pd.tnic> <921153f5-1528-31d8-b815-f0419e819aeb@amd.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <921153f5-1528-31d8-b815-f0419e819aeb@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 On Thu, Jun 15, 2017 at 09:59:45AM -0500, Tom Lendacky wrote: > Actually the detection routine, amd_iommu_detect(), is part of the > IOMMU_INIT_FINISH macro support which is called early through mm_init() > from start_kernel() and that routine is called before init_amd(). Ah, we do that there too: for (p = __iommu_table; p < __iommu_table_end; p++) { Can't say that that code with the special section and whatnot is obvious. :-\ Oh, well, early_init_amd() then. That is called in start_kernel->setup_arch->early_cpu_init and thus before mm_init(). > > If so, it did work fine until now, without the volatile. Why is it > > needed now, all of a sudden? > > If you run checkpatch against the whole amd_iommu.c file you'll see that I'm, of course, not talking about the signature change: I'm *actually* questioning the need to make this argument volatile, all of a sudden. If there's a need, please explain why. It worked fine until now. If it didn't, we would've seen it. If it is a bug, then it needs a proper explanation, a *separate* patch and so on. But not like now, a drive-by change in an IOMMU enablement patch. If it is wrong, then wait_on_sem() needs to be fixed too. AFAICT, wait_on_sem() gets called in both cases with interrupts disabled, while holding a lock so I'd like to pls know why, even in that case, does this variable need to be volatile. -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply. From mboxrd@z Thu Jan 1 00:00:00 1970 From: Borislav Petkov Subject: Re: [PATCH v6 26/34] iommu/amd: Allow the AMD IOMMU to work with memory encryption Date: Thu, 15 Jun 2017 17:33:22 +0200 Message-ID: <20170615153322.nwylo3dzn4fdx6n6@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> <20170615094111.wga334kg2bhxqib3@pd.tnic> <921153f5-1528-31d8-b815-f0419e819aeb@amd.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Return-path: Content-Disposition: inline In-Reply-To: <921153f5-1528-31d8-b815-f0419e819aeb@amd.com> Sender: owner-linux-mm@kvack.org 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 List-Id: linux-efi@vger.kernel.org On Thu, Jun 15, 2017 at 09:59:45AM -0500, Tom Lendacky wrote: > Actually the detection routine, amd_iommu_detect(), is part of the > IOMMU_INIT_FINISH macro support which is called early through mm_init() > from start_kernel() and that routine is called before init_amd(). Ah, we do that there too: for (p = __iommu_table; p < __iommu_table_end; p++) { Can't say that that code with the special section and whatnot is obvious. :-\ Oh, well, early_init_amd() then. That is called in start_kernel->setup_arch->early_cpu_init and thus before mm_init(). > > If so, it did work fine until now, without the volatile. Why is it > > needed now, all of a sudden? > > If you run checkpatch against the whole amd_iommu.c file you'll see that I'm, of course, not talking about the signature change: I'm *actually* questioning the need to make this argument volatile, all of a sudden. If there's a need, please explain why. It worked fine until now. If it didn't, we would've seen it. If it is a bug, then it needs a proper explanation, a *separate* patch and so on. But not like now, a drive-by change in an IOMMU enablement patch. If it is wrong, then wait_on_sem() needs to be fixed too. AFAICT, wait_on_sem() gets called in both cases with interrupts disabled, while holding a lock so I'd like to pls know why, even in that case, does this variable need to be volatile. -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply. -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr0-f199.google.com (mail-wr0-f199.google.com [209.85.128.199]) by kanga.kvack.org (Postfix) with ESMTP id B960A6B0279 for ; Thu, 15 Jun 2017 11:33:32 -0400 (EDT) Received: by mail-wr0-f199.google.com with SMTP id n18so4053048wra.11 for ; Thu, 15 Jun 2017 08:33:32 -0700 (PDT) Received: from mail.skyhub.de (mail.skyhub.de. [5.9.137.197]) by mx.google.com with ESMTP id i62si430018wmd.155.2017.06.15.08.33.31 for ; Thu, 15 Jun 2017 08:33:31 -0700 (PDT) Date: Thu, 15 Jun 2017 17:33:22 +0200 From: Borislav Petkov Subject: Re: [PATCH v6 26/34] iommu/amd: Allow the AMD IOMMU to work with memory encryption Message-ID: <20170615153322.nwylo3dzn4fdx6n6@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> <20170615094111.wga334kg2bhxqib3@pd.tnic> <921153f5-1528-31d8-b815-f0419e819aeb@amd.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <921153f5-1528-31d8-b815-f0419e819aeb@amd.com> Sender: owner-linux-mm@kvack.org List-ID: 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 On Thu, Jun 15, 2017 at 09:59:45AM -0500, Tom Lendacky wrote: > Actually the detection routine, amd_iommu_detect(), is part of the > IOMMU_INIT_FINISH macro support which is called early through mm_init() > from start_kernel() and that routine is called before init_amd(). Ah, we do that there too: for (p = __iommu_table; p < __iommu_table_end; p++) { Can't say that that code with the special section and whatnot is obvious. :-\ Oh, well, early_init_amd() then. That is called in start_kernel->setup_arch->early_cpu_init and thus before mm_init(). > > If so, it did work fine until now, without the volatile. Why is it > > needed now, all of a sudden? > > If you run checkpatch against the whole amd_iommu.c file you'll see that I'm, of course, not talking about the signature change: I'm *actually* questioning the need to make this argument volatile, all of a sudden. If there's a need, please explain why. It worked fine until now. If it didn't, we would've seen it. If it is a bug, then it needs a proper explanation, a *separate* patch and so on. But not like now, a drive-by change in an IOMMU enablement patch. If it is wrong, then wait_on_sem() needs to be fixed too. AFAICT, wait_on_sem() gets called in both cases with interrupts disabled, while holding a lock so I'd like to pls know why, even in that case, does this variable need to be volatile. -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply. -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from mail.skyhub.de ([5.9.137.197]) by bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux)) id 1dLWmQ-0003bQ-L3 for kexec@lists.infradead.org; Thu, 15 Jun 2017 15:33:56 +0000 Date: Thu, 15 Jun 2017 17:33:22 +0200 From: Borislav Petkov Subject: Re: [PATCH v6 26/34] iommu/amd: Allow the AMD IOMMU to work with memory encryption Message-ID: <20170615153322.nwylo3dzn4fdx6n6@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> <20170615094111.wga334kg2bhxqib3@pd.tnic> <921153f5-1528-31d8-b815-f0419e819aeb@amd.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <921153f5-1528-31d8-b815-f0419e819aeb@amd.com> List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "kexec" Errors-To: kexec-bounces+dwmw2=infradead.org@lists.infradead.org To: Tom Lendacky Cc: linux-efi@vger.kernel.org, Brijesh Singh , Toshimitsu Kani , Radim =?utf-8?B?S3LEjW3DocWZ?= , Matt Fleming , x86@kernel.org, linux-mm@kvack.org, Alexander Potapenko , "H. Peter Anvin" , Larry Woodman , linux-arch@vger.kernel.org, kvm@vger.kernel.org, Jonathan Corbet , Joerg Roedel , linux-doc@vger.kernel.org, kasan-dev@googlegroups.com, Ingo Molnar , Andrey Ryabinin , Dave Young , Rik van Riel , Arnd Bergmann , Konrad Rzeszutek Wilk , Andy Lutomirski , Thomas Gleixner , Dmitry Vyukov , kexec@lists.infradead.org, linux-kernel@vger.kernel.org, iommu@lists.linux-foundation.org, "Michael S. Tsirkin" , Paolo Bonzini On Thu, Jun 15, 2017 at 09:59:45AM -0500, Tom Lendacky wrote: > Actually the detection routine, amd_iommu_detect(), is part of the > IOMMU_INIT_FINISH macro support which is called early through mm_init() > from start_kernel() and that routine is called before init_amd(). Ah, we do that there too: for (p = __iommu_table; p < __iommu_table_end; p++) { Can't say that that code with the special section and whatnot is obvious. :-\ Oh, well, early_init_amd() then. That is called in start_kernel->setup_arch->early_cpu_init and thus before mm_init(). > > If so, it did work fine until now, without the volatile. Why is it > > needed now, all of a sudden? > > If you run checkpatch against the whole amd_iommu.c file you'll see that I'm, of course, not talking about the signature change: I'm *actually* questioning the need to make this argument volatile, all of a sudden. If there's a need, please explain why. It worked fine until now. If it didn't, we would've seen it. If it is a bug, then it needs a proper explanation, a *separate* patch and so on. But not like now, a drive-by change in an IOMMU enablement patch. If it is wrong, then wait_on_sem() needs to be fixed too. AFAICT, wait_on_sem() gets called in both cases with interrupts disabled, while holding a lock so I'd like to pls know why, even in that case, does this variable need to be volatile. -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply. _______________________________________________ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec