Re: [edk2-devel] [PATCH v3 08/13] OvmfPkg/PlatformInitLib: Create gEfiSmmSmramMemoryGuid

["Wu, Jiaxin" <jiaxin.wu@intel.com>]

More info:
I quick dump the SMRAM info with original SmmAccess implementation, it's same as I produced in the gEfiSmmSmramMemoryGuid HOB.

SmmAccess:
SmmAccessPeiEntryPoint: SMRAM map follows, 2 entries
SmmAccessPeiEntryPoint:             7F000000                 1000             7F000000                   1A    ---> for the S3 Resume in gEfiAcpiVariableGuid
SmmAccessPeiEntryPoint:             7F001000               FFF000             7F001000                    A

Smram map in the gEfiSmmSmramMemoryGuid:
PlatformQemuInitializeRam:             7F000000                 1000             7F000000                   1A    --> ---> for the S3 Resume in gEfiAcpiVariableGuid
PlatformQemuInitializeRam:             7F001000               FFF000             7F001000                    A


Thanks,
Jiaxin

> -----Original Message-----
> From: Wu, Jiaxin
> Sent: Tuesday, April 23, 2024 8:19 PM
> To: Gerd Hoffmann <kraxel@redhat.com>
> Cc: devel@edk2.groups.io; Ard Biesheuvel <ardb+tianocore@kernel.org>; Yao,
> Jiewen <jiewen.yao@intel.com>; Ni, Ray <ray.ni@intel.com>
> Subject: RE: [PATCH v3 08/13] OvmfPkg/PlatformInitLib: Create
> gEfiSmmSmramMemoryGuid
> 
> >
> > > +    SmramHobDescriptorBlock                             =
> > (EFI_SMRAM_HOB_DESCRIPTOR_BLOCK *)(Hob.Raw);
> >
> > > +    SmramHobDescriptorBlock->Descriptor[0].PhysicalStart =
> > PlatformInfoHob->LowMemory - TsegSize;
> > > +    SmramHobDescriptorBlock->Descriptor[0].CpuStart      =
> > PlatformInfoHob->LowMemory - TsegSize;
> > > +    SmramHobDescriptorBlock->Descriptor[0].PhysicalSize  =
> EFI_PAGE_SIZE;
> > > +    SmramHobDescriptorBlock->Descriptor[0].RegionState   =
> > EFI_SMRAM_CLOSED | EFI_CACHEABLE | EFI_ALLOCATED;
> >
> > > +    SmramHobDescriptorBlock->Descriptor[1].PhysicalStart =
> > SmramHobDescriptorBlock->Descriptor[0].PhysicalStart + EFI_PAGE_SIZE;
> > > +    SmramHobDescriptorBlock->Descriptor[1].CpuStart      =
> > SmramHobDescriptorBlock->Descriptor[0].CpuStart + EFI_PAGE_SIZE;
> > > +    SmramHobDescriptorBlock->Descriptor[1].PhysicalSize  = TsegSize -
> > EFI_PAGE_SIZE;
> > > +    SmramHobDescriptorBlock->Descriptor[1].RegionState   =
> > EFI_SMRAM_CLOSED | EFI_CACHEABLE;
> >
> > This is not going to fly.
> >
> > First, smram allocation doesn't work that way.  Have a look at
> > OvmfPkg/SmmAccess.  I guess that easily explains why this series
> > breaks S3 suspend.
> >
> 
> Oh? Could you explain a bit more for 1) how smram allocation works? 2)
> what's the possible reason break the S3? I haven't check yet.
> 
> > Second, storing these descriptors in a HOB (which is PEI memory)
> > is questionable from a security point of view.
> >
> 
> HOB is only to expose the SMRAM address and size, not the contents in
> smram, what's the security concern?
> 
> 
> Thanks,
> Jiaxin


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#118141): https://edk2.groups.io/g/devel/message/118141
Mute This Topic: https://groups.io/mt/105593577/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-