From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail.skyhub.de (mail.skyhub.de [5.9.137.197]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 4E53A2F27 for ; Tue, 1 Feb 2022 21:28:51 +0000 (UTC) Received: from zn.tnic (dslb-088-067-221-104.088.067.pools.vodafone-ip.de [88.67.221.104]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.skyhub.de (SuperMail on ZX Spectrum 128k) with ESMTPSA id BB1171EC04AD; Tue, 1 Feb 2022 22:28:44 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=alien8.de; s=dkim; t=1643750924; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:in-reply-to:in-reply-to: references:references; bh=7l7vocFxdezL9X3EUKTSMo+nYjMem25yARKYTuyX8jo=; b=UqY6/pu2Ue28uGppx/YJY4gkORX0RykPUqAdSDRchnnYt4NjKqnkwxUmhNgCRs7FckxK7f 1pzKGK6KEJUNmRbkUwTZape/D5jU3FBobXFz22E3nxw6f5eTL9EmNgVU99rgmM7c7b816l I+PJ5mEE7Bl4LDHK9aNUF7u8RSHEypM= Date: Tue, 1 Feb 2022 22:28:39 +0100 From: Borislav Petkov To: Michael Roth Cc: Brijesh Singh , x86@kernel.org, linux-kernel@vger.kernel.org, kvm@vger.kernel.org, linux-efi@vger.kernel.org, platform-driver-x86@vger.kernel.org, linux-coco@lists.linux.dev, linux-mm@kvack.org, Thomas Gleixner , Ingo Molnar , Joerg Roedel , Tom Lendacky , "H. Peter Anvin" , Ard Biesheuvel , Paolo Bonzini , Sean Christopherson , Vitaly Kuznetsov , Jim Mattson , Andy Lutomirski , Dave Hansen , Sergio Lopez , Peter Gonda , Peter Zijlstra , Srinivas Pandruvada , David Rientjes , Dov Murik , Tobin Feldman-Fitzthum , Vlastimil Babka , "Kirill A . Shutemov" , Andi Kleen , "Dr . David Alan Gilbert" , brijesh.ksingh@gmail.com, tony.luck@intel.com, marcorr@google.com, sathyanarayanan.kuppuswamy@linux.intel.com Subject: Re: [PATCH v9 05/43] x86/compressed/64: Detect/setup SEV/SME features earlier in boot Message-ID: References: <20220128171804.569796-1-brijesh.singh@amd.com> <20220128171804.569796-6-brijesh.singh@amd.com> <20220201203507.goibbaln6dxyoogv@amd.com> Precedence: bulk X-Mailing-List: linux-coco@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20220201203507.goibbaln6dxyoogv@amd.com> On Tue, Feb 01, 2022 at 02:35:07PM -0600, Michael Roth wrote: > Unfortunately rdmsr()/wrmsr()/__rdmsr()/__wrmsr() etc. definitions are all > already getting pulled in via: > > misc.h: > #include linux/elf.h > #include linux/thread_info.h > #include linux/cpufeature.h > #include linux/processor.h > #include linux/msr.h > > Those definitions aren't usable in boot/compressed because of __ex_table > and possibly some other dependency hellishness. And they should not be. Mixing kernel proper and decompressor code needs to stop and untangling that is a multi-year effort, unfortunately. ;-\ > Would read_msr()/write_msr() be reasonable alternative names for these new > helpers, or something else that better distinguishes them from the > kernel proper definitions? Nah, just call them rdmsr/wrmsr(). There is already {read,write}_msr() tracepoint symbols in kernel proper and there's no point in keeping them apart using different names - that ship has long sailed. > It doesn't look like anything in boot/ pulls in boot/compressed/ > headers. It seems to be the other way around, with boot/compressed > pulling in headers and whole C files from boot/. > > So perhaps these new definitions should be added to a small boot/msr.h > header and pulled in from there? That sounds good too. > Should we introduce something like this as well for cpucheck.c? Or > re-write cpucheck.c to make use of the u64 versions? Or just set the > cpucheck.c rework aside for now? (but still introduce the above helpers > as boot/msr.h in preparation)? How about you model it after static int msr_read(u32 msr, struct msr *m) from arch/x86/lib/msr.c which takes struct msr from which you can return either u32s or a u64? The stuff you share between the decompressor and kernel proper you put in a arch/x86/include/asm/shared/ folder, for an example, see what we do there in the TDX patchset: https://lore.kernel.org/r/20220124150215.36893-11-kirill.shutemov@linux.intel.com I.e., you move struct msr in such a shared header and then you include it everywhere needed. The arch/x86/boot/ msr helpers are then plain and simple, without tracepoints and exception fixups and you define them in ...boot/msr.c or so. If the patch gets too big, make sure to split it in a couple so that it is clear what happens at each step. How does that sound? Thx. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette