Re: [PATCH RFC v2 01/29] mm: asi: Make some utility functions noinstr compatible

[Borislav Petkov <bp@alien8.de>]

On Fri, Jan 10, 2025 at 06:40:27PM +0000, Brendan Jackman wrote:
> Subject: Re: [PATCH RFC v2 01/29] mm: asi: Make some utility functions noinstr compatible

The tip tree preferred format for patch subject prefixes is
'subsys/component:', e.g. 'x86/apic:', 'x86/mm/fault:', 'sched/fair:',
'genirq/core:'. Please do not use file names or complete file paths as
prefix. 'git log path/to/file' should give you a reasonable hint in most
cases.

So I guess "x86/asm:" or so.

> Some existing utility functions would need to be called from a noinstr
> context in the later patches. So mark these as either noinstr or
> __always_inline.
> 
> An earlier version of this by Junaid had a macro that was intended to
> tell the compiler "either inline this function, or call it in the
> noinstr section", which basically boiled down to:
> 
>  #define inline_or_noinstr noinline __section(".noinstr.text")
> 
> Unfortunately Thomas pointed out this will prevent the function from
> being inlined at call sites in .text.
> 
> So far I haven't been able[1] to find a formulation that lets us :
> 1. avoid calls from .noinstr.text -> .text,
> 2. while also letting the compiler freely decide what to inline.
> 
> 1 is a functional requirement so here I'm just giving up on 2. Existing
> callsites of this code are just forced inline. For the incoming code
> that needs to call it from noinstr, they will be out-of-line calls.

I'm not sure some of that belongs in the commit message - if you want to have
it in the submission, you should put it under the --- line below, right above
the diffstat.

> [1] https://lore.kernel.org/lkml/CA+i-1C1z35M8wA_4AwMq7--c1OgjNoLGTkn4+Td5gKg7QQAzWw@mail.gmail.com/
> 
> Checkpatch-args: --ignore=COMMIT_LOG_LONG_LINE

Yeah, you can drop those. People should not turn off brain, use checkpatch and
point at all the silly errors it spits anyway.

> Signed-off-by: Brendan Jackman <jackmanb@google.com>
> ---
>  arch/x86/include/asm/processor.h     |  2 +-
>  arch/x86/include/asm/special_insns.h |  8 ++++----
>  arch/x86/include/asm/tlbflush.h      |  3 +++
>  arch/x86/mm/tlb.c                    | 13 +++++++++----
>  4 files changed, 17 insertions(+), 9 deletions(-)

So I was just about to look at the below diff but then booting the patch in my
guest causes it to stop at:

[    1.110988] sr 2:0:0:0: Attached scsi generic sg1 type 5
[    1.114210] PM: Image not found (code -22)
[    1.114903] clk: Disabling unused clocks
[    1.119397] EXT4-fs (sda2): mounted filesystem 90868bc4-a017-4fa2-ac81-931ba260346f ro with ordered data mode. Quota mode: disabled.
[    1.121069] VFS: Mounted root (ext4 filesystem) readonly on device 8:2.
<--- EOF

with the below call stack.

Booting it on Linus' master branch is ok but this is tip/master with all that
we've accumulated for the next merge window along with other stuff I'm poking
at...

Long story short, lemme try to poke around tomorrow to try to figure out what
actually happens. It could be caused by the part of Rik's patches and this one
inlining things. We'll see...

native_flush_tlb_one_user (addr=2507219558400) at arch/x86/mm/tlb.c:1177
1177            if (!static_cpu_has(X86_FEATURE_PTI))
(gdb) bt
#0  native_flush_tlb_one_user (addr=2507219558400) at arch/x86/mm/tlb.c:1177
#1  0xffffffff8128206e in flush_tlb_one_user (addr=addr@entry=2507219558400) at arch/x86/mm/tlb.c:1196
#2  flush_tlb_one_kernel (addr=addr@entry=2507219558400) at arch/x86/mm/tlb.c:1151
#3  0xffffffff812820b7 in do_kernel_range_flush (info=0xffff88807dc311c0) at arch/x86/mm/tlb.c:1092
#4  0xffffffff8137beb6 in csd_do_func (csd=0x0 <fixed_percpu_data>, info=0xffff88807dc311c0, 
    func=0xffffffff81282090 <do_kernel_range_flush>) at kernel/smp.c:134
#5  smp_call_function_many_cond (mask=<optimized out>, func=func@entry=0xffffffff81282090 <do_kernel_range_flush>, 
    info=0xffff88807dc311c0, scf_flags=scf_flags@entry=3, cond_func=cond_func@entry=0x0 <fixed_percpu_data>)
    at kernel/smp.c:876
#6  0xffffffff8137c254 in on_each_cpu_cond_mask (cond_func=cond_func@entry=0x0 <fixed_percpu_data>, 
    func=func@entry=0xffffffff81282090 <do_kernel_range_flush>, info=<optimized out>, wait=wait@entry=true, 
    mask=<optimized out>) at kernel/smp.c:1052
#7  0xffffffff81282020 in on_each_cpu (wait=1, info=<optimized out>, func=0xffffffff81282090 <do_kernel_range_flush>)
    at ./include/linux/smp.h:71
#8  flush_tlb_kernel_range (start=start@entry=18446683600570097664, end=<optimized out>, end@entry=18446683600579907584)
    at arch/x86/mm/tlb.c:1106
#9  0xffffffff81481c3f in __purge_vmap_area_lazy (start=18446683600570097664, start@entry=18446744073709551615, 
    end=18446683600579907584, end@entry=0, full_pool_decay=full_pool_decay@entry=false) at mm/vmalloc.c:2284
#10 0xffffffff81481fde in _vm_unmap_aliases (start=start@entry=18446744073709551615, end=end@entry=0, 
    flush=<optimized out>, flush@entry=0) at mm/vmalloc.c:2899
#11 0xffffffff81482049 in vm_unmap_aliases () at mm/vmalloc.c:2922
#12 0xffffffff81284d9f in change_page_attr_set_clr (addr=0xffffc9000001fef0, numpages=<optimized out>, mask_set=..., 
    mask_clr=..., force_split=<optimized out>, in_flag=0, pages=0x0 <fixed_percpu_data>)
    at arch/x86/mm/pat/set_memory.c:1881
#13 0xffffffff81285c52 in change_page_attr_set (array=0, mask=..., numpages=511, addr=0xffffc9000001fef0)
    at arch/x86/mm/pat/set_memory.c:1922
#14 set_memory_nx (addr=<optimized out>, addr@entry=18446744071759204352, numpages=numpages@entry=511)
    at arch/x86/mm/pat/set_memory.c:2110
#15 0xffffffff8127b729 in free_init_pages (what=what@entry=0xffffffff8226bac0 "unused decrypted", 
    begin=18446744071759204352, end=18446744071761297408) at arch/x86/mm/init.c:929
#16 0xffffffff898f25aa in mem_encrypt_free_decrypted_mem () at arch/x86/mm/mem_encrypt_amd.c:574
#17 0xffffffff81ca06c3 in free_initmem () at arch/x86/mm/init.c:973
#18 0xffffffff81c9fa48 in kernel_init (unused=<optimized out>) at init/main.c:1475
#19 0xffffffff812398a0 in ret_from_fork (prev=<optimized out>, regs=0xffffc9000001ff58, 
    fn=0xffffffff81c9fa10 <kernel_init>, fn_arg=0x0 <fixed_percpu_data>) at arch/x86/kernel/process.c:148
#20 0xffffffff81206f7a in ret_from_fork_asm () at arch/x86/entry/entry_64.S:244
#21 0x1f0f2e6600000000 in ?? ()
#22 0x2e66000000000084 in ?? ()
#23 0x0000000000841f0f in ?? ()
#24 0x000000841f0f2e66 in ?? ()
#25 0x00841f0f2e660000 in ?? ()
#26 0x1f0f2e6600000000 in ?? ()
#27 0x2e66000000000084 in ?? ()
#28 0x0000000000841f0f in ?? ()
#29 0x000000841f0f2e66 in ?? ()
#30 0x00841f0f2e660000 in ?? ()
#31 0x0000000000000000 in ?? ()
(gdb)

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette