[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <8b9ddac38a612a7ecee7511c657ba0bf84660d3b.camel@intel.com>
Date: Tue, 10 Sep 2024 09:52:49 +0000
From: "Huang, Kai" <kai.huang@...el.com>
To: "Hansen, Dave" <dave.hansen@...el.com>, "bp@...en8.de" <bp@...en8.de>,
"peterz@...radead.org" <peterz@...radead.org>, "hpa@...or.com"
<hpa@...or.com>, "mingo@...hat.com" <mingo@...hat.com>, "tglx@...utronix.de"
<tglx@...utronix.de>, "David.Kaplan@....com" <David.Kaplan@....com>,
"kirill.shutemov@...ux.intel.com" <kirill.shutemov@...ux.intel.com>
CC: "Edgecombe, Rick P" <rick.p.edgecombe@...el.com>, "seanjc@...gle.com"
<seanjc@...gle.com>, "x86@...nel.org" <x86@...nel.org>, "dyoung@...hat.com"
<dyoung@...hat.com>, "sagis@...gle.com" <sagis@...gle.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, "Williams, Dan
J" <dan.j.williams@...el.com>, "Thomas.Lendacky@....com"
<Thomas.Lendacky@....com>, "pbonzini@...hat.com" <pbonzini@...hat.com>,
"Ashish.Kalra@....com" <Ashish.Kalra@....com>, "Yamahata, Isaku"
<isaku.yamahata@...el.com>, "bhe@...hat.com" <bhe@...hat.com>,
"nik.borisov@...e.com" <nik.borisov@...e.com>
Subject: Re: [PATCH v6 2/5] x86/kexec: do unconditional WBINVD for bare-metal
in relocate_kernel()
On Tue, 2024-09-10 at 02:46 +0000, Kaplan, David wrote:
> [AMD Official Use Only - AMD Internal Distribution Only]
>
> > -----Original Message-----
> > From: Huang, Kai <kai.huang@...el.com>
> > Sent: Monday, September 9, 2024 9:42 PM
> > To: Kaplan, David <David.Kaplan@....com>; Hansen, Dave
> > <dave.hansen@...el.com>; bp@...en8.de; tglx@...utronix.de;
> > peterz@...radead.org; mingo@...hat.com; hpa@...or.com;
> > kirill.shutemov@...ux.intel.com
> > Cc: x86@...nel.org; linux-kernel@...r.kernel.org; pbonzini@...hat.com;
> > seanjc@...gle.com; Williams, Dan J <dan.j.williams@...el.com>; Lendacky,
> > Thomas <Thomas.Lendacky@....com>; Edgecombe, Rick P
> > <rick.p.edgecombe@...el.com>; Yamahata, Isaku
> > <isaku.yamahata@...el.com>; Kalra, Ashish <Ashish.Kalra@....com>;
> > bhe@...hat.com; nik.borisov@...e.com; sagis@...gle.com; Dave Young
> > <dyoung@...hat.com>
> > Subject: Re: [PATCH v6 2/5] x86/kexec: do unconditional WBINVD for bare-
> > metal in relocate_kernel()
> >
> > Caution: This message originated from an External Source. Use proper
> > caution when opening attachments, clicking links, or responding.
> >
> >
> > > > --- a/arch/x86/kernel/machine_kexec_64.c
> > > > +++ b/arch/x86/kernel/machine_kexec_64.c
> > > > @@ -322,16 +322,9 @@ void machine_kexec_cleanup(struct kimage
> > *image)
> > > > void machine_kexec(struct kimage *image) {
> > > > unsigned long page_list[PAGES_NR];
> > > > - unsigned int host_mem_enc_active;
> > > > int save_ftrace_enabled;
> > > > void *control_page;
> > > >
> > > > - /*
> > > > - * This must be done before load_segments() since if call depth
> > tracking
> > > > - * is used then GS must be valid to make any function calls.
> > > > - */
> > > > - host_mem_enc_active =
> > > > cc_platform_has(CC_ATTR_HOST_MEM_ENCRYPT);
> > > > -
> > >
> > > Functionally the patch looks fine. I would suggest keeping some form of
> > this comment though, because the limitation about not being able to make
> > function calls after load_segments() is arguably non-obvious and this
> > comment served as a warning for future modifications in this area.
> >
> > Yeah this makes sense. Thanks.
> >
> > I think we can add some text to the existing comment of load_segments() to
> > call out this. Allow me to dig into more about call depth tracking to
> > understand it better -- relocate_kernel() after load_segments() seems to be a
> > real function call and I want to know how does it interact with call depth
> > tracking.
>
> That one is explicitly ignored, see skip_addr() in arch/x86/kernel/callthunks.c
>
That was I thought too. Thanks for pointing out.
How about below?
--- a/arch/x86/kernel/machine_kexec_64.c
+++ b/arch/x86/kernel/machine_kexec_64.c
@@ -351,6 +351,11 @@ void machine_kexec(struct kimage *image)
*
* I take advantage of this here by force loading the
* segments, before I zap the gdt with an invalid value.
+ *
+ * Note this resets GS to 0. Don't make any function call after
+ * here since call depth tracking uses per-cpu variables to
+ * operate (relocate_kernel is explicitly ignored by call depth
+ * tracking).
*/
Btw, it would be very helpful if you can help to verify this patch doesn't break
call depth tracking in your environment. Thanks!
Powered by blists - more mailing lists