On Wed, 23 Nov 2022 at 11:49, Borislav Petkov <bp@xxxxxxxxx> wrote: > > On Tue, Nov 22, 2022 at 03:49:29PM -0600, Tom Lendacky wrote: > > diff --git a/drivers/firmware/efi/libstub/x86-stub.c b/drivers/firmware/efi/libstub/x86-stub.c > > index cb5f0befee57..a0bfd31358ba 100644 > > --- a/drivers/firmware/efi/libstub/x86-stub.c > > +++ b/drivers/firmware/efi/libstub/x86-stub.c > > @@ -23,7 +23,7 @@ > > const efi_system_table_t *efi_system_table; > > const efi_dxe_services_table_t *efi_dxe_table; > > -u32 image_offset; > > +u32 image_offset __section(".data"); > > static efi_loaded_image_t *image = NULL; > > static efi_status_t > > > > I assume it has to do with being in .data vs .bss and not being explicitly > > cleared with the encryption bit set. With the change to put image_offset in > > the .data section, it is read as zero, where as when it was in the .bss > > section it was reading "ciphertext". > > Hmm, two points about this: > > 1. Can we do > > u32 image_offset __bss_decrypted; > > here instead? We have this special section just for that fun and it > self-documents this way. > The patch moves it from .data to .bss inadvertently, and I am not convinced Tom's analysis is entirely accurate: we may simply have garbage in image_offset if we access it before .bss gets cleared. > 2. Also, why does my SEV-ES guest boot just fine without that change? > Indeed, so it needs to be in .data > [ 0.000000] Linux version 6.1.0-rc6+ (root@ml) (gcc (Debian 11.3.0-1) 11.3.0, GNU ld (GNU Binutils for Debian) 2.38) #1 SMP PREEMPT_DYNAMIC Wed Nov 23 11:27:17 CET 2022 > ... > [ 0.336132] Memory Encryption Features active: AMD SEV SEV-ES > > Thx. > > -- > Regards/Gruss, > Boris. > > https://people.kernel.org/tglx/notes-about-netiquette