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@xxxxxxxxxxxxxxx
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
