Re: [tip: x86/boot] x86/boot: Provide __pti_set_user_pgtbl() to startup code

Ard Biesheuvel
2025-05-05T16:20:02Z

On Mon, 5 May 2025 at 18:04, Borislav Petkov <bp@alien8.de> wrote:
>
> On Sun, May 04, 2025 at 02:20:04PM -0000, tip-bot2 for Ard Biesheuvel wrote:
> > The following commit has been merged into the x86/boot branch of tip:
> >
> > Commit-ID: 5297886f0cc45db5f4a804caf359e6e7874ee864
> > Gitweb: https://git.kernel.org/tip/5297886f0cc45db5f4a804caf359e6e7874ee864
> > Author: Ard Biesheuvel <ardb@kernel.org>
> > AuthorDate: Sun, 04 May 2025 11:52:45 +02:00
> > Committer: Ingo Molnar <mingo@kernel.org>
> > CommitterDate: Sun, 04 May 2025 15:59:43 +02:00
> >
> > x86/boot: Provide __pti_set_user_pgtbl() to startup code
> >
> > The SME encryption startup code populates page tables using the ordinary
> > set_pXX() helpers, and in a PTI build, these will call out to
> > __pti_set_user_pgtbl() to manipulate the shadow copy of the page tables
> > for user space.
> >
> > This is unneeded for the startup code, which only manipulates the
> > swapper page tables, and so this call could be avoided in this
> > particular case. So instead of exposing the ordinary
> > __pti_set_user_pgtblt() to the startup code after its gets confined into
> > its own symbol space, provide an alternative which just returns pgd,
> > which is always correct in the startup context.
> >
> > Annotate it as __weak for now, this will be dropped in a subsequent
> > patch.
> >
> > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> > Signed-off-by: Ingo Molnar <mingo@kernel.org>
> > Cc: Arnd Bergmann <arnd@arndb.de>
> > Cc: David Woodhouse <dwmw@amazon.co.uk>
> > Cc: Dionna Amalie Glaze <dionnaglaze@google.com>
> > Cc: H. Peter Anvin <hpa@zytor.com>
> > Cc: Kees Cook <keescook@chromium.org>
> > Cc: Kevin Loughlin <kevinloughlin@google.com>
> > Cc: Len Brown <len.brown@intel.com>
> > Cc: Linus Torvalds <torvalds@linux-foundation.org>
> > Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > Cc: Tom Lendacky <thomas.lendacky@amd.com>
> > Cc: linux-efi@vger.kernel.org
> > Link: https://lore.kernel.org/r/20250504095230.2932860-40-ardb+git@google.com
> > ---
> > arch/x86/boot/startup/sme.c | 9 +++++++++
> > 1 file changed, 9 insertions(+)
> >
> > diff --git a/arch/x86/boot/startup/sme.c b/arch/x86/boot/startup/sme.c
> > index 5738b31..753cd20 100644
> > --- a/arch/x86/boot/startup/sme.c
> > +++ b/arch/x86/boot/startup/sme.c
> > @@ -564,3 +564,12 @@ void __head sme_enable(struct boot_params *bp)
> > cc_vendor = CC_VENDOR_AMD;
> > cc_set_mask(me_mask);
> > }
> > +
> > +#ifdef CONFIG_MITIGATION_PAGE_TABLE_ISOLATION
> > +/* Local version for startup code, which never operates on user page tables */
> > +__weak
> > +pgd_t __pti_set_user_pgtbl(pgd_t *pgdp, pgd_t pgd)
> > +{
> > + return pgd;
> > +}
> > +#endif
>
> [ 1.227968] smp: Brought up 1 node, 32 CPUs
> [ 1.231576] smpboot: Total of 32 processors activated (191999.61 BogoMIPS)
> [ 1.247644] ------------[ cut here ]------------
> [ 1.248697] WARNING: CPU: 17 PID: 104 at kernel/jump_label.c:276 __static_key_slow_dec_cpuslocked+0x2a/0x80
> [ 1.251592] Modules linked in:
> [ 1.252370] CPU: 17 UID: 0 PID: 104 Comm: kworker/17:0 Not tainted 6.15.0-rc4+ #2 PREEMPT(voluntary)
> [ 1.253173] node 0 deferred pages initialised in 12ms
> [ 1.254539] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS unknown 02/02/2022
> [ 1.257490] Workqueue: events unaccepted_cleanup_work
> [ 1.258698] RIP: 0010:__static_key_slow_dec_cpuslocked+0x2a/0x80
> [ 1.259574] Code: 0f 1f 44 00 00 53 48 89 fb e8 82 66 e5 ff 8b 03 85 c0 78 16 74 54 83 f8 01 74 11 8d 50 ff f0 0f b1 13 75 ec 5b e9 66 bf 8c 00 <0f> 0b 48 c7 c7 00 44 54 82 e8 a8 5f 8c 00 8b 03 83 f8 ff 74 33 85
> [ 1.266446] Memory: 7574396K/8381588K available (13737K kernel code, 2487K rwdata, 6056K rodata, 3916K init, 3592K bss, 791248K reserved, 0K cma-reserved)
> [ 1.263574] RSP: 0018:ffffc9000048fe40 EFLAGS: 00010286
> [ 1.267573] RAX: 00000000ffffffff RBX: ffffffff82f10d00 RCX: 0000000000000000
> [ 1.269388] RDX: 0000000000000001 RSI: 0000000000000000 RDI: ffffffff82f10d00
> [ 1.275578] RBP: ffff88827b7d4d98 R08: 8080808080808080 R09: ffff888100b59100
> [ 1.277441] R10: ffff888100050cc0 R11: fefefefefefefeff R12: ffff88827326e300
> [ 1.279574] R13: ffff888100bc1940 R14: ffff8881000e2405 R15: ffff8881000e2400
> [ 1.281460] FS: 0000000000000000(0000) GS:ffff8882f0400000(0000) knlGS:0000000000000000
> [ 1.281460] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 1.281460] CR2: 0000000000000000 CR3: 0008000002c22000 CR4: 00000000003506f0
> [ 1.287576] Call Trace:
> [ 1.288381] <TASK>
> [ 1.289015] static_key_slow_dec+0x1f/0x40
> [ 1.289980] process_one_work+0x171/0x330
> [ 1.290988] worker_thread+0x247/0x390
> [ 1.291576] ? __pfx_worker_thread+0x10/0x10
> [ 1.292773] kthread+0x107/0x240
> [ 1.293536] ? __pfx_kthread+0x10/0x10
> [ 1.295573] ret_from_fork+0x30/0x50
> [ 1.295575] ? __pfx_kthread+0x10/0x10
> [ 1.296580] ret_from_fork_asm+0x1a/0x30
> [ 1.297507] </TASK>
> [ 1.298085] ---[ end trace 0000000000000000 ]---
>
> mingo simply doesn't want to listen and stop queueing untested patches.
>
> So lemme whack this one.
>
> My SNP guest had CONFIG_MITIGATION_PAGE_TABLE_ISOLATION=y leading to the
> above.
>

This patch by itself does nothing. The symbol is __weak for the time
being, and given that this code does not have its __pi_ prefixes yet,
this function will be superseded by the existing one. (If you remove
the __weak you will get a linker error)

Are you sure this patch is causing the issue?