linux-kernel - Re: [RFC PATCH 1/7] x86/kexec: Clean up and document register use in relocate_kernel

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <d2138bfeff7caf5d65b2cb2417c92ea6eedbcb9a.camel@infradead.org>
Date: Tue, 05 Nov 2024 12:17:05 -0800
From: David Woodhouse <dwmw2@...radead.org>
To: "Huang, Kai" <kai.huang@...el.com>, "kexec@...ts.infradead.org"
	 <kexec@...ts.infradead.org>
Cc: "x86@...nel.org" <x86@...nel.org>, "bp@...en8.de" <bp@...en8.de>, 
 "hpa@...or.com" <hpa@...or.com>, "mingo@...hat.com" <mingo@...hat.com>,
 "tglx@...utronix.de" <tglx@...utronix.de>,
 "kirill.shutemov@...ux.intel.com" <kirill.shutemov@...ux.intel.com>,
 "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
 "horms@...nel.org" <horms@...nel.org>,  "dave.hansen@...ux.intel.com"
 <dave.hansen@...ux.intel.com>, "nik.borisov@...e.com" <nik.borisov@...e.com>
Subject: Re: [RFC PATCH 1/7] x86/kexec: Clean up and document register use
 in relocate_kernel_64.S

On Tue, 2024-11-05 at 10:00 +0000, Huang, Kai wrote:
> On Sun, 2024-11-03 at 05:35 +0000, David Woodhouse wrote:
> > From: David Woodhouse <dwmw@...zon.co.uk>
> > 
> > Add more comments explaining what each register contains, and save the
> > preserve_context flag to a non-clobbered register sooner, to keep things
> > simpler.
> > 
> > Signed-off-by: David Woodhouse <dwmw@...zon.co.uk>
> > ---
> >  arch/x86/kernel/relocate_kernel_64.S | 17 +++++++++++++----
> >  1 file changed, 13 insertions(+), 4 deletions(-)
> > 
> > diff --git a/arch/x86/kernel/relocate_kernel_64.S b/arch/x86/kernel/relocate_kernel_64.S
> > index e9e88c342f75..c065806884f8 100644
> > --- a/arch/x86/kernel/relocate_kernel_64.S
> > +++ b/arch/x86/kernel/relocate_kernel_64.S
> > @@ -100,6 +100,9 @@ SYM_CODE_START_NOALIGN(relocate_kernel)
> >         movq    %r10, CP_PA_SWAP_PAGE(%r11)
> >         movq    %rdi, CP_PA_BACKUP_PAGES_MAP(%r11)
> >  
> > +       /* Save the preserve_context to %r11 as swap_pages clobbers %rcx. */
> > +       movq    %rcx, %r11
> > +
> 
> Seems moving this here won't break anything.  From functionality's perspective
> there's no need move this here, but fine to me since the move is needed for the
> sake of adding the comment (below) to identity_mapped.

I believe I did actually use %rcx in the debug code I added later,
didn't I?

> >         /* Switch to the identity mapped page tables */
> >         movq    %r9, %cr3
> >  
> > @@ -116,6 +119,13 @@ SYM_CODE_END(relocate_kernel)
> >  
> >  SYM_CODE_START_LOCAL_NOALIGN(identity_mapped)
> >         UNWIND_HINT_END_OF_STACK
> > +       /*
> > +        * %rdi indirection page
> > +        * %rdx start address
> > +        * %r11 preserve_context
> > +        * %r12 host_mem_enc_active
> > +        */
> > +
> 
> I think adding this comment is the main purpose of this patch.  Since we have
> listed 4 regs in the comment, how about also add an entry for %r13?
> 
> Something like:
> 
>         %r13 original CR4 when relocate_kernel() is invoked
> 
> Yeah this is kinda mentioned in later code but it seems it's more complete if we
> also mention %r13 here.

Ack, I'll add that too. Thanks.

> Anyway, I suppose adding this comment is kinda helpful, so:
> 
> Acked-by: Kai Huang <kai.huang@...el.com>
> 


Download attachment "smime.p7s" of type "application/pkcs7-signature" (5965 bytes)