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

["Gerd Hoffmann" <kraxel@redhat.com>]

  Hi,

> +    Hob.Raw = BuildGuidHob (
> +                &gEfiSmmSmramMemoryGuid,
> +                BufferSize
> +                );

> +    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.

Second, storing these descriptors in a HOB (which is PEI memory)
is questionable from a security point of view.

take care,
  Gerd



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#118131): https://edk2.groups.io/g/devel/message/118131
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]
-=-=-=-=-=-=-=-=-=-=-=-