[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <69763ade82cd7718e709547c977f354dd324e6fe.camel@intel.com>
Date: Thu, 16 Feb 2023 17:33:21 +0000
From: "Edgecombe, Rick P" <rick.p.edgecombe@...el.com>
To: "kirill@...temov.name" <kirill@...temov.name>
CC: "kexec@...ts.infradead.org" <kexec@...ts.infradead.org>,
"kirill.shutemov@...ux.intel.com" <kirill.shutemov@...ux.intel.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"tglx@...utronix.de" <tglx@...utronix.de>,
"sathyanarayanan.kuppuswamy@...ux.intel.com"
<sathyanarayanan.kuppuswamy@...ux.intel.com>,
"x86@...nel.org" <x86@...nel.org>,
"linux-coco@...ts.linux.dev" <linux-coco@...ts.linux.dev>,
"bp@...en8.de" <bp@...en8.de>,
"Hansen, Dave" <dave.hansen@...el.com>,
"Yamahata, Isaku" <isaku.yamahata@...el.com>
Subject: Re: [PATCH 1/2] x86/kexec: Preserve CR4.MCE during kexec
On Thu, 2023-02-16 at 12:43 +0300, Kirill A. Shutemov wrote:
> On Thu, Feb 16, 2023 at 01:49:39AM +0000, Edgecombe, Rick P wrote:
> > On Tue, 2023-02-14 at 02:48 +0300, Kirill A. Shutemov wrote:
> > > TDX guests are not allowed to clear CR4.MCE. Attempt to clear it
> > > leads
> > > to #VE.
> > >
> > > Preserve the flag during kexec.
> > >
> > > Signed-off-by: Kirill A. Shutemov <
> > > kirill.shutemov@...ux.intel.com>
> >
> > I wonder whats going on with the pre-existing switching between eax
> > and
> > rax in this code for the cr0 and cr4 manipulations. Do you know
> > what
> > the reason is?
>
> 32-bit ORs and ANDs save one byte per instruction. And there's no 32-
> bit
> MOV to/from control registers in 64-bit mode.
Oh right, I think I recall now. There is a 64 bit AND in the CR0 piece
here too, which of course is outside of these changes.
But otherwise, it's not clear from the patch what the implications are
of leaving CR4.MCE set for the non-TDX environment. I see in head_64.S
it will clear it during boot if the kernel doesn't support machine
check. So it leaves a little window where CR4.MCE is set where it
wasn't before.
The piece in head_64.S talks about how an #MC will crash the system if
it happens before the machine check stuff is fully setup anyway, so it
doesn't hurt to leave it on. Is that the reasoning for this change as
well? If so it might help to add a little more about the reasoning in
the commit log.
Powered by blists - more mailing lists