On Tue, Sep 25, 2018 at 02:33:48PM +0000, Lendacky, Thomas wrote:
> On 09/25/2018 06:10 AM, Kairui Song wrote:
> > Commit 1958b5fc4010 ("x86/boot: Add early boot support when running
> > with SEV active") is causing kexec becomes sometimes unstable, kexec
> > reboot won't start a second kernel bypassing BIOS boot process, instead,
> > the system got reset.
> >
> > That's because, in get_sev_encryption_bit function, we are using
> > 32-bit RIP-relative addressing to read the value of enc_bit, but
> > kexec may alloc the early boot up code to a higher location, which
> > is beyond 32-bit addressing limit. Some garbage will be read and
> > get_sev_encryption_bit will return the wrong value, which lead to
> > wrong memory page flag.
> >
> > This patch adds a get_sev_encryption_bit_64 function to avoid this
> > problem. 64-bit early boot code will use this function instead, it
> > uses native RIP addressing to read the enc_bit which have no problem
> > with any location.
> >
> > Fixes: 1958b5fc4010 ("x86/boot: Add early boot support when running with SEV active")
> > Signed-off-by: Kairui Song <kasong@xxxxxxxxxx>
>
> Reviewed-by: Tom Lendacky <thomas.lendacky@xxxxxxx>
>
> > ---
> > arch/x86/boot/compressed/mem_encrypt.S | 64 ++++++++++++++++++--------
> > 1 file changed, 45 insertions(+), 19 deletions(-)
> >
> > diff --git a/arch/x86/boot/compressed/mem_encrypt.S b/arch/x86/boot/compressed/mem_encrypt.S
> > index eaa843a52907..41933550449a 100644
> > --- a/arch/x86/boot/compressed/mem_encrypt.S
> > +++ b/arch/x86/boot/compressed/mem_encrypt.S
> > @@ -18,27 +18,13 @@
> >
> > .text
> > .code32
> > -ENTRY(get_sev_encryption_bit)
> > +do_get_sev_encryption_bit:
> > xor %eax, %eax
> >
> > #ifdef CONFIG_AMD_MEM_ENCRYPT
> > push %ebx
> > push %ecx
> > push %edx
> > - push %edi
> > -
> > - /*
> > - * RIP-relative addressing is needed to access the encryption bit
> > - * variable. Since we are running in 32-bit mode we need this call/pop
> > - * sequence to get the proper relative addressing.
> > - */
> > - call 1f
> > -1: popl %edi
> > - subl $1b, %edi
> > -
> > - movl enc_bit(%edi), %eax
> > - cmpl $0, %eax
> > - jge .Lsev_exit
> >
> > /* Check if running under a hypervisor */
> > movl $1, %eax
> > @@ -69,25 +55,65 @@ ENTRY(get_sev_encryption_bit)
> >
> > movl %ebx, %eax
> > andl $0x3f, %eax /* Return the encryption bit location */
> > - movl %eax, enc_bit(%edi)
IINM, the problem can be addressed in a simpler way by getting rid of
enc_bit and thus getting rid of the need to do relative addressing of
anything and simply doing the whole dance of figuring out the C-bit each
time. It probably wouldn't be even measurable...
--
Regards/Gruss,
Boris.
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
--
_______________________________________________
kexec mailing list
kexec@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/kexec
