Messages in this thread | | | Date | Tue, 1 Feb 2022 22:28:39 +0100 | From | Borislav Petkov <> | Subject | Re: [PATCH v9 05/43] x86/compressed/64: Detect/setup SEV/SME features earlier in boot |
| |
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
| |