From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2607:f8b0:4001:c06::232; helo=mail-io0-x232.google.com; envelope-from=vladimir.olovyannikov@broadcom.com; receiver=edk2-devel@lists.01.org Received: from mail-io0-x232.google.com (mail-io0-x232.google.com [IPv6:2607:f8b0:4001:c06::232]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id A5B672205BE9B for ; Tue, 2 Jan 2018 12:37:46 -0800 (PST) Received: by mail-io0-x232.google.com with SMTP id i143so2676ioa.3 for ; Tue, 02 Jan 2018 12:42:48 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=broadcom.com; s=google; h=from:references:in-reply-to:mime-version:thread-index:date :message-id:subject:to:cc; bh=NSdACZYTabz5hDRoYSkIMAWb1DDc3Ib6KR2P3nzz/Tg=; b=fzR28kw8DfIaDdK8IDkLTQnKcxSw8JYxHS4I0rJZ+r7k8jDzDQhnxTIR13+KXo2FXQ 8OiM+c3lL68lvgnd2sFX5yU4oeQ4BlcNvuhxVzf6oflHo4c8AANqY3MoIyJA3N8yUPQz KtzgzvIfa+uCoK/XJpRxyBny/NJy8aPHDv5OQ= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:references:in-reply-to:mime-version :thread-index:date:message-id:subject:to:cc; bh=NSdACZYTabz5hDRoYSkIMAWb1DDc3Ib6KR2P3nzz/Tg=; b=JQhmhPM9osu7zxw0+4kJ1Szip2/bo9aIFGtZ76NrtQWVycpp+tGfap3aHxTdYvB9aj Ti1PW+PuFZOaJBhyJ4oHWPNIxfqhXpWY1/fcqMq7BMI6n18zj507fEeZIvBrPGRl8Tvu nk/QVBIeUllMEJGWzS8UMM3jBaLwYta1rthYm/dw6anaKaTGimM/h3lnz/XbA1dH7xkU Sksc04zqwuK48UVAtUM9bkyySxRyUqErwwi9OPOEtoO7tLolcbMM1knzqazOXypnY0Cs d0df3X3/4iZuViETt8Y3mYNjKHHqY+s+Q8xda1P8/kQdkWRshyBHpjcOsqLef0n/j1VL JhHw== X-Gm-Message-State: AKGB3mI9O8Oaugy00bYDGe/oKoZ5LqiuTWp0Dtpg6v3nQfk8SmEbi3mb eVieZigyPgl6jhDM7Qw0de0a+7PZ0QZcJjfpHPdGnQ== X-Google-Smtp-Source: ACJfBouyeZoEc+ghwJjwpV+jWibyFMbpaH447c/PkIQJ+8m2sAX70LfTd/2+bZv/UiDpl37CmcQfeeH3WBnq9Sx0Dz8= X-Received: by 10.107.158.200 with SMTP id h191mr11941277ioe.159.1514925767735; Tue, 02 Jan 2018 12:42:47 -0800 (PST) From: Vladimir Olovyannikov References: <20180102155034.13688-1-ard.biesheuvel@linaro.org> In-Reply-To: <20180102155034.13688-1-ard.biesheuvel@linaro.org> MIME-Version: 1.0 X-Mailer: Microsoft Outlook 14.0 Thread-Index: AQFcWbeRhtcVjPscIkfpeLQXAC6Gm6RPWGLw Date: Tue, 2 Jan 2018 12:42:46 -0800 Message-ID: <8f91754c63e434ab3d46a20cc5cb3f0a@mail.gmail.com> To: Ard Biesheuvel , edk2-devel@lists.01.org Cc: leif.lindholm@linaro.org, udit.kumar@nxp.com, meenakshi.aggarwal@nxp.com Subject: Re: [PATCH] ArmPlatformPkg/MemoryInitPeiLib: mark primary FV region as boot services data X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 02 Jan 2018 20:37:47 -0000 Content-Type: text/plain; charset="UTF-8" > -----Original Message----- > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] > Sent: Tuesday, January 2, 2018 7:51 AM > To: edk2-devel@lists.01.org > Cc: leif.lindholm@linaro.org; vladimir.olovyannikov@broadcom.com; > udit.kumar@nxp.com; meenakshi.aggarwal@nxp.com; Ard Biesheuvel > Subject: [PATCH] ArmPlatformPkg/MemoryInitPeiLib: mark primary FV > region as boot services data > > Commit 8ae5fc182941 ("ArmPlatformPkg/MemoryInitPeiLib: don't reserve > primary FV in memory") deleted the code that removes the memory > covering the primary firmware volume from the memory map. The > assumption was that this is no longer necessary now that we no longer > expose compression and PE/COFF loader library code from the PrePi module > to DXE core. > > However, the FV is still declared, and so code may attempt to access it > anyway, which may cause unexpected results depending on whether the > memory has been reused for other purposes in the mean time. > > So reinstate the code that splits off the resource descriptor HOB that > describes the firmware device, but this time, don't mark the memory as > unusable, but create a memory allocation HOB that marks the region as boot > services data. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Ard Biesheuvel > --- > Vladimir, Udit, Meenakshi: please confirm whether this code works for you. Thanks Ard, Works for me. Vladimir > > ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c | 74 > ++++++++++++++++++++ > 1 file changed, 74 insertions(+) > > diff --git a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c > b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c > index d03214b5df66..d1b5c0be9497 100644 > --- a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c > +++ b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c > @@ -70,7 +70,11 @@ MemoryPeim ( > { > ARM_MEMORY_REGION_DESCRIPTOR *MemoryTable; > EFI_RESOURCE_ATTRIBUTE_TYPE ResourceAttributes; > + UINT64 ResourceLength; > EFI_PEI_HOB_POINTERS NextHob; > + EFI_PHYSICAL_ADDRESS FdTop; > + EFI_PHYSICAL_ADDRESS SystemMemoryTop; > + EFI_PHYSICAL_ADDRESS ResourceTop; > BOOLEAN Found; > > // Get Virtual Memory Map from the Platform Library @@ -117,6 +121,76 > @@ MemoryPeim ( > ); > } > > + // > + // Reserve the memory space occupied by the firmware volume // > + > + SystemMemoryTop = (EFI_PHYSICAL_ADDRESS)PcdGet64 > + (PcdSystemMemoryBase) + (EFI_PHYSICAL_ADDRESS)PcdGet64 > + (PcdSystemMemorySize); FdTop = (EFI_PHYSICAL_ADDRESS)PcdGet64 > + (PcdFdBaseAddress) + (EFI_PHYSICAL_ADDRESS)PcdGet32 (PcdFdSize); > + > + // EDK2 does not have the concept of boot firmware copied into DRAM. > + To avoid the DXE // core to overwrite this area we must create a > + memory allocation HOB for the region, // but this only works if we split off > the underlying resource descriptor as well. > + if ((PcdGet64 (PcdFdBaseAddress) >= PcdGet64 > (PcdSystemMemoryBase)) && (FdTop <= SystemMemoryTop)) { > + Found = FALSE; > + > + // Search for System Memory Hob that contains the firmware > + NextHob.Raw = GetHobList (); > + while ((NextHob.Raw = GetNextHob > (EFI_HOB_TYPE_RESOURCE_DESCRIPTOR, NextHob.Raw)) != NULL) { > + if ((NextHob.ResourceDescriptor->ResourceType == > EFI_RESOURCE_SYSTEM_MEMORY) && > + (PcdGet64 (PcdFdBaseAddress) >= NextHob.ResourceDescriptor- > >PhysicalStart) && > + (FdTop <= NextHob.ResourceDescriptor->PhysicalStart + > NextHob.ResourceDescriptor->ResourceLength)) > + { > + ResourceAttributes = NextHob.ResourceDescriptor- > >ResourceAttribute; > + ResourceLength = NextHob.ResourceDescriptor->ResourceLength; > + ResourceTop = NextHob.ResourceDescriptor->PhysicalStart + > + ResourceLength; > + > + if (PcdGet64 (PcdFdBaseAddress) == NextHob.ResourceDescriptor- > >PhysicalStart) { > + if (SystemMemoryTop != FdTop) { > + // Create the System Memory HOB for the firmware with the non- > present attribute > + BuildResourceDescriptorHob (EFI_RESOURCE_SYSTEM_MEMORY, > + ResourceAttributes, > + PcdGet64 (PcdFdBaseAddress), > + PcdGet32 (PcdFdSize)); > + > + // Top of the FD is system memory available for UEFI > + NextHob.ResourceDescriptor->PhysicalStart += PcdGet32(PcdFdSize); > + NextHob.ResourceDescriptor->ResourceLength -= > PcdGet32(PcdFdSize); > + } > + } else { > + // Create the System Memory HOB for the firmware > + BuildResourceDescriptorHob (EFI_RESOURCE_SYSTEM_MEMORY, > + ResourceAttributes, > + PcdGet64 (PcdFdBaseAddress), > + PcdGet32 (PcdFdSize)); > + > + // Update the HOB > + NextHob.ResourceDescriptor->ResourceLength = PcdGet64 > + (PcdFdBaseAddress) - NextHob.ResourceDescriptor->PhysicalStart; > + > + // If there is some memory available on the top of the FD then create a > HOB > + if (FdTop < NextHob.ResourceDescriptor->PhysicalStart + > ResourceLength) { > + // Create the System Memory HOB for the remaining region (top of > the FD) > + BuildResourceDescriptorHob (EFI_RESOURCE_SYSTEM_MEMORY, > + ResourceAttributes, > + FdTop, > + ResourceTop - FdTop); > + } > + } > + > + // Mark the memory covering the Firmware Device as boot services > data > + BuildMemoryAllocationHob (PcdGet64 (PcdFdBaseAddress), > + PcdGet32 (PcdFdSize), > + EfiBootServicesData); > + > + Found = TRUE; > + break; > + } > + NextHob.Raw = GET_NEXT_HOB (NextHob); > + } > + > + ASSERT(Found); > + } > + > // Build Memory Allocation Hob > InitMmu (MemoryTable); > > -- > 2.11.0