Re: [edk2] [PATCH V2 10/15] ArmVirtXen: Use merged variable driver for emulated NV mode
Hi, On 2019/1/22 3:40, Ard Biesheuvel wrote: On Mon, 21 Jan 2019 at 14:36, Julien Grall wrote: Hi, On 21/01/2019 10:46, Zeng, Star wrote: On 2019/1/18 2:59, Julien Grall wrote: I saw the discussion at https://lists.01.org/pipermail/edk2-devel/2019-January/035405.html. Fortunately, it has been fixed. So I did rebase for the code. Repo: g...@github.com:lzeng14/edk2.git Branch: MergedVariableDriver_EmuNvMode_V3_rebased I was about to ask a branch as there were conflicts in the rebase. Thank you for providing the branch! If you can help have a quick test, that will be very helpful. :) With your series applied, EDK2 is crashing while the Linux EFI stub is running. See the log below. My knowledge of EDK2 is quite limited, so I am not entirely where to look at. I am happy to help debugging if you provide guidance. Thanks for the test. Fortunately, we could catch this before the patch is pushed. :) Hi Julien, Could you try the patch below please? diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c index a8bb9cf25ebd..adaf6ccb48b0 100644 --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c @@ -236,14 +236,16 @@ VariableClassAddressChangeEvent ( { UINTN Index; - EfiConvertPointer (0x0, (VOID **) >FvbInstance->GetBlockSize); - EfiConvertPointer (0x0, (VOID **) >FvbInstance->GetPhysicalAddress); - EfiConvertPointer (0x0, (VOID **) >FvbInstance->GetAttributes); - EfiConvertPointer (0x0, (VOID **) >FvbInstance->SetAttributes); - EfiConvertPointer (0x0, (VOID **) >FvbInstance->Read); - EfiConvertPointer (0x0, (VOID **) >FvbInstance->Write); - EfiConvertPointer (0x0, (VOID **) >FvbInstance->EraseBlocks); - EfiConvertPointer (0x0, (VOID **) >FvbInstance); + if (mVariableModuleGlobal->FvbInstance != NULL) { +EfiConvertPointer (0x0, (VOID **) >FvbInstance->GetBlockSize); +EfiConvertPointer (0x0, (VOID **) >FvbInstance->GetPhysicalAddress); +EfiConvertPointer (0x0, (VOID **) >FvbInstance->GetAttributes); +EfiConvertPointer (0x0, (VOID **) >FvbInstance->SetAttributes); +EfiConvertPointer (0x0, (VOID **) >FvbInstance->Read); +EfiConvertPointer (0x0, (VOID **) >FvbInstance->Write); +EfiConvertPointer (0x0, (VOID **) >FvbInstance->EraseBlocks); +EfiConvertPointer (0x0, (VOID **) >FvbInstance); + } EfiConvertPointer (0x0, (VOID **) >PlatformLangCodes); EfiConvertPointer (0x0, (VOID **) >LangCodes); EfiConvertPointer (0x0, (VOID **) >PlatformLang); Thanks Ard. I integrated it into the patch 10 of V4. Repo: g...@github.com:lzeng14/edk2.git Branch: MergedVariableDriver_EmuNvMode_V4 Julien, could you help take a try? Thanks, Star Press any key to continue... [Security] 3rd party image[0] can be loaded after EndOfDxe: MemoryMapped(0x2,0x67789000,0x68DF1200). InstallProtocolInterface: 5B1B31A1-9562-11D2-8E3F-00A0C969723B 7AB2B040 Loading driver at 0x00065783000 EntryPoint=0x00066878664 Loading driver at 0x00065783000 EntryPoint=0x00066878664 InstallProtocolInterface: BC62157E-3E33-4FEC-9920-2D3B36D750DF 7AA54B18 ProtectUefiImageCommon - 0x7AB2B040 - 0x65783000 - 0x02006000 SetUefiImageMemoryAttributes - 0x65783000 - 0x1000 (0x4008) SetUefiImageMemoryAttributes - 0x65784000 - 0x011CD000 (0x00020008) SetUefiImageMemoryAttributes - 0x66951000 - 0x00E38000 (0x4008) EFI stub: Booting Linux Kernel... EFI stub: Using DTB from configuration table EFI stub: Exiting boot services and installing virtual address map... XenBus: Set state to 5 XenBus: Set state to 5, done XenPvBlk: waiting backend state 5, current: 4 XenStore: Watch event 7B036698 XenBus: Set state to 6 XenBus: Set state to 6, done XenPvBlk: waiting backend state 6, current: 5 XenStore: Watch event 7B036698 XenBus: Set state to 1 XenBus: Set state to 1, done Xen GrantTable, removing 38003 Xen GrantTable, removing 38002 Xen GrantTable, removing 38001 Xen GrantTable, removing 38000 SetUefiImageMemoryAttributes - 0x7BE6 - 0x0004 (0x0008) SetUefiImageMemoryAttributes - 0x78AE - 0x0005 (0x0008) SetUefiImageMemoryAttributes - 0x78A9 - 0x0004 (0x0008) SetUefiImageMemoryAttributes - 0x789F - 0x0004 (0x0008) SetUefiImageMemoryAttributes - 0x7895 - 0x0004 (0x0008) SetUefiImageMemoryAttributes - 0x788B - 0x0004 (0x0008) Synchronous Exception at 0x7BE70698 PC 0x7BE70698 (0x7BE6+0x00010698) [ 0] RuntimeDxe.dll PC 0x78AFECB0 (0x78AE+0x0001ECB0) [ 1] VariableRuntimeDxe.dll PC 0x78AF0AC8 (0x78AE+0x00010AC8) [ 1] VariableRuntimeDxe.dll
Re: [edk2] [PATCH] SD/eMMC : Fix Command Argument for SD/eMMC R/W operation.
Hi Jun, Haojian, Please review the patch. Thanks, Meenakshi > -Original Message- > From: Leif Lindholm > Sent: Thursday, January 17, 2019 4:54 PM > To: Meenakshi Aggarwal > Cc: ard.biesheu...@linaro.org; edk2-devel@lists.01.org; Jun Nie > ; Haojian Zhuang > Subject: Re: [PATCH] SD/eMMC : Fix Command Argument for SD/eMMC R/W > operation. > > Jun, Haojian - any comments? > > On Wed, Jan 16, 2019 at 06:51:36PM +0530, Meenakshi Aggarwal wrote: > > Issue : SD read failure for high capacity cards e.g. 64 GB i Reason : > > Command argument value exceeds 32 bit for block number 0x3787FFF and > > cant be fit into 32 bit wide SD host controller register. > > > > Fix : > > AccessMode bits [29:30] of OCR is a valid definition to calculate data > > address for eMMC cards. > > > > For SD cards, data address is calculated on the basis of card capacity > > status bit[30] of OCR. > > > > Contributed-under: TianoCore Contribution Agreement 1.1 > > Signed-off-by: Meenakshi Aggarwal > > --- > > EmbeddedPkg/Universal/MmcDxe/MmcBlockIo.c | 19 ++- > > 1 file changed, 14 insertions(+), 5 deletions(-) > > > > diff --git a/EmbeddedPkg/Universal/MmcDxe/MmcBlockIo.c > > b/EmbeddedPkg/Universal/MmcDxe/MmcBlockIo.c > > index a2b9232..625a59e 100644 > > --- a/EmbeddedPkg/Universal/MmcDxe/MmcBlockIo.c > > +++ b/EmbeddedPkg/Universal/MmcDxe/MmcBlockIo.c > > @@ -148,12 +148,21 @@ MmcTransferBlock ( > >MmcHostInstance = MMC_HOST_INSTANCE_FROM_BLOCK_IO_THIS (This); > >MmcHost = MmcHostInstance->MmcHost; > > > > - //Set command argument based on the card access mode (Byte mode or > > Block mode) > > - if ((MmcHostInstance->CardInfo.OCRData.AccessMode & > MMC_OCR_ACCESS_MASK) == > > - MMC_OCR_ACCESS_SECTOR) { > > -CmdArg = Lba; > > + if (MmcHostInstance->CardInfo.CardType != EMMC_CARD) { > > +//Set command argument based on the card capacity (SDSC or SDXC/SDHC) > > +if (MmcHostInstance->CardInfo.OCRData.AccessMode & BIT1) { > > + CmdArg = Lba; > > +} else { > > + CmdArg = Lba * This->Media->BlockSize; > > +} > >} else { > > -CmdArg = Lba * This->Media->BlockSize; > > +//Set command argument based on the card access mode (Byte mode or > Block mode) > > +if ((MmcHostInstance->CardInfo.OCRData.AccessMode & > MMC_OCR_ACCESS_MASK) == > > +MMC_OCR_ACCESS_SECTOR) { > > + CmdArg = Lba; > > +} else { > > + CmdArg = Lba * This->Media->BlockSize; > > +} > >} > > > >Status = MmcHost->SendCommand (MmcHost, Cmd, CmdArg); > > -- > > 1.9.1 > > ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH 1/4] CryptoPkg/SmmCryptLib: permit use by MM_STANDALONE modules
Yes. No objections from me. Sorry for missed the previous mail. Thanks, Ting -Original Message- From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org] Sent: Monday, January 21, 2019 8:41 PM To: Gao, Liming Cc: Wang, Jian J ; Ye, Ting ; edk2-devel@lists.01.org; Kinney, Michael D ; Wei, Gang ; Zhang, Chao B ; Yao, Jiewen ; Wu, Hao A ; Zeng, Star ; Achin Gupta ; Jagadeesh Ujja Subject: Re: [PATCH 1/4] CryptoPkg/SmmCryptLib: permit use by MM_STANDALONE modules On Mon, 21 Jan 2019 at 13:40, Gao, Liming wrote: > > Ard: > Wang, Jian is the reviewer of CryptoPkg. I think that his review-by is > enough. > OK, thanks! > Thanks > Liming > > -Original Message- > > From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org] > > Sent: Monday, January 21, 2019 8:36 PM > > To: Wang, Jian J ; Ye, Ting > > > > Cc: edk2-devel@lists.01.org; Kinney, Michael D > > ; Gao, Liming ; > > Wei, Gang ; Zhang, Chao B > > ; Yao, Jiewen ; Wu, > > Hao A ; Zeng, Star ; Achin > > Gupta ; Jagadeesh Ujja > > Subject: Re: [PATCH 1/4] CryptoPkg/SmmCryptLib: permit use by > > MM_STANDALONE modules > > > > On Fri, 18 Jan 2019 at 12:12, Ard Biesheuvel > > wrote: > > > > > > On Fri, 18 Jan 2019 at 08:08, Wang, Jian J wrote: > > > > > > > > > > > > > > > > Reviewed-by: Jian J Wang > > > > > > > > > > Ting, do you have any objections to this patch? > > > > > > > Ping? > > > > > > > > > > > > > > > -Original Message- > > > > > From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org] > > > > > Sent: Thursday, January 17, 2019 5:22 AM > > > > > To: edk2-devel@lists.01.org > > > > > Cc: Ard Biesheuvel ; Kinney, > > > > > Michael D ; Gao, Liming > > > > > ; Ye, Ting ; Wei, > > > > > Gang ; Wang, Jian J > > > > > ; Zhang, Chao B > > > > > ; Yao, Jiewen ; > > > > > Wu, Hao A ; Zeng, Star > > > > > ; Achin Gupta ; > > > > > Jagadeesh Ujja > > > > > Subject: [PATCH 1/4] CryptoPkg/SmmCryptLib: permit use by > > > > > MM_STANDALONE modules > > > > > > > > > > Permit SmmCryptLib to be used by MM_STANDALONE modules > > > > > > > > > > Contributed-under: TianoCore Contribution Agreement 1.1 > > > > > Signed-off-by: Ard Biesheuvel > > > > > --- > > > > > CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf | 2 +- > > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > > > diff --git a/CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf > > > > > b/CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf > > > > > index c34699cd62bf..a681fe2f36b8 100644 > > > > > --- a/CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf > > > > > +++ b/CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf > > > > > @@ -30,7 +30,7 @@ [Defines] > > > > >MODULE_TYPE= DXE_SMM_DRIVER > > > > >VERSION_STRING = 1.0 > > > > >PI_SPECIFICATION_VERSION = 0x0001000A > > > > > - LIBRARY_CLASS = BaseCryptLib|DXE_SMM_DRIVER > > > > > SMM_CORE > > > > > + LIBRARY_CLASS = BaseCryptLib|DXE_SMM_DRIVER > > > > > SMM_CORE > > > > > MM_STANDALONE > > > > > > > > > > # > > > > > # The following information is for reference only and not > > > > > required by the build tools. > > > > > -- > > > > > 2.17.1 > > > > ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [platforms: PATCH v2 4/4] Marvell/Armada7k8k: Read DRAM settings from ARM-TF
From: Grzegorz Jaszczyk The memory controller registers are marked as secure in the latest ARM-TF for Armada SoCs. It is available however get the DRAM information via SiP services in the EL3, so use it instead of accessing the registers directly. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Marcin Wojtas --- Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLib.inf | 3 + Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.h | 25 Silicon/Marvell/Include/Library/MvSmc.h | 1 + Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.c | 60 ++-- 4 files changed, 22 insertions(+), 67 deletions(-) diff --git a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLib.inf b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLib.inf index e888566..0c7f320 100644 --- a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLib.inf +++ b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLib.inf @@ -41,12 +41,15 @@ [Packages] ArmPkg/ArmPkg.dec ArmPlatformPkg/ArmPlatformPkg.dec + EmbeddedPkg/EmbeddedPkg.dec MdeModulePkg/MdeModulePkg.dec MdePkg/MdePkg.dec Silicon/Marvell/Marvell.dec [LibraryClasses] + ArmadaSoCDescLib ArmLib + ArmSmcLib DebugLib MemoryAllocationLib MppLib diff --git a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.h b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.h index cc30e4a..8101cf3 100644 --- a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.h +++ b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.h @@ -46,28 +46,3 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. (MmioRead32 (CCU_MC_RCR_REG) & REMAP_SIZE_MASK) + SIZE_1MB #define DRAM_REMAP_TARGET \ (MmioRead32 (CCU_MC_RTBR_REG) << TARGET_BASE_OFFS) - -#define DRAM_CH0_MMAP_LOW_REG(cs) (0xf0020200 + (cs) * 0x8) -#define DRAM_CS_VALID_ENABLED_MASK 0x1 -#define DRAM_AREA_LENGTH_OFFS 16 -#define DRAM_AREA_LENGTH_MASK (0x1f << DRAM_AREA_LENGTH_OFFS) -#define DRAM_START_ADDRESS_L_OFFS 23 -#define DRAM_START_ADDRESS_L_MASK (0x1ff << DRAM_START_ADDRESS_L_OFFS) -#define DRAM_CH0_MMAP_HIGH_REG(cs) (0xf0020204 + (cs) * 0x8) -#define DRAM_START_ADDR_HTOL_OFFS 32 - -#define DRAM_MAX_CS_NUM 8 - -#define DRAM_CS_ENABLED(Cs) \ - (MmioRead32 (DRAM_CH0_MMAP_LOW_REG (Cs)) & DRAM_CS_VALID_ENABLED_MASK) -#define GET_DRAM_REGION_BASE(Cs) \ - ((UINT64)MmioRead32 (DRAM_CH0_MMAP_HIGH_REG ((Cs))) << \ - DRAM_START_ADDR_HTOL_OFFS) | \ - (MmioRead32 (DRAM_CH0_MMAP_LOW_REG (Cs)) & DRAM_START_ADDRESS_L_MASK); -#define GET_DRAM_REGION_SIZE_CODE(Cs) \ - (MmioRead32 (DRAM_CH0_MMAP_LOW_REG ((Cs))) & \ - DRAM_AREA_LENGTH_MASK) >> DRAM_AREA_LENGTH_OFFS -#define DRAM_REGION_SIZE_EVEN(C)(((C) >= 7) && ((C) <= 26)) -#define GET_DRAM_REGION_SIZE_EVEN(C)((UINT64)1 << ((C) + 16)) -#define DRAM_REGION_SIZE_ODD(C) ((C) <= 4) -#define GET_DRAM_REGION_SIZE_ODD(C) ((UINT64)0x1800 << (C)) diff --git a/Silicon/Marvell/Include/Library/MvSmc.h b/Silicon/Marvell/Include/Library/MvSmc.h index 2d1542a..0c90f11 100644 --- a/Silicon/Marvell/Include/Library/MvSmc.h +++ b/Silicon/Marvell/Include/Library/MvSmc.h @@ -19,5 +19,6 @@ #define MV_SMC_ID_COMPHY_POWER_ON 0x8201 #define MV_SMC_ID_COMPHY_POWER_OFF0x8202 #define MV_SMC_ID_COMPHY_PLL_LOCK 0x8203 +#define MV_SMC_ID_DRAM_SIZE 0x8210 #endif //__MV_SMC_H__ diff --git a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.c b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.c index 2a4f5ad..8517deb 100644 --- a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.c +++ b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.c @@ -32,12 +32,18 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. ***/ -#include +#include + +#include + +#include #include +#include #include #include #include #include +#include #include "Armada7k8kLibMem.h" @@ -57,49 +63,19 @@ GetDramSize ( IN OUT UINT64 *MemSize ) { - UINT64 BaseAddr; - UINT8 RegionCode; - UINT8 Cs; - - *MemSize = 0; - - for (Cs = 0; Cs < DRAM_MAX_CS_NUM; Cs++) { - -/* Exit loop on first disabled DRAM CS */ -if (!DRAM_CS_ENABLED (Cs)) { - break; -} - -/* - * Sanity check for base address of next DRAM block. - * Only continuous space will be used. - */ -BaseAddr = GET_DRAM_REGION_BASE (Cs); -if (BaseAddr != *MemSize) { - DEBUG ((DEBUG_ERROR, -"%a: DRAM blocks are not contiguous, limit size to 0x%llx\n", -__FUNCTION__, -*MemSize)); - return
[edk2] [platforms: PATCH v2 3/4] Marvell/Library: ArmadaSoCDescLib: Add North Bridge description
From: Grzegorz Jaszczyk For upcomming patch there is need to get AP806 base, provide required getter function for it. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Marcin Wojtas --- Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.h | 6 Silicon/Marvell/Include/Library/ArmadaSoCDescLib.h | 28 Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.c | 34 3 files changed, 68 insertions(+) diff --git a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.h b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.h index bfc8639..c2d7933 100644 --- a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.h +++ b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.h @@ -22,9 +22,15 @@ // Common macros // #define MV_SOC_CP_BASE(Cp) (0xF200 + ((Cp) * 0x200)) +#define MV_SOC_AP806_BASE0xF000 #define MV_SOC_AP806_COUNT 1 // +// Armada7k8k default North Bridge index +// +#define ARMADA7K8K_AP806_INDEX 0 + +// // Platform description of AHCI controllers // #define MV_SOC_AHCI_BASE(Cp) (MV_SOC_CP_BASE (Cp) + 0x54) diff --git a/Silicon/Marvell/Include/Library/ArmadaSoCDescLib.h b/Silicon/Marvell/Include/Library/ArmadaSoCDescLib.h index 26b075a..fc17c3a 100644 --- a/Silicon/Marvell/Include/Library/ArmadaSoCDescLib.h +++ b/Silicon/Marvell/Include/Library/ArmadaSoCDescLib.h @@ -20,6 +20,34 @@ #include // +// North Bridge description +// + +/** + +Routine Description: + + Get base address of the SoC North Bridge. + +Arguments: + + ApBase - Base address of the North Bridge. + ApIndex - Index of the North Bridge. + +Returns: + + EFI_SUCCESS - Proper base address is returned. + EFI_INVALID_PARAMETER - The index is out of range. + +**/ +EFI_STATUS +EFIAPI +ArmadaSoCAp8xxBaseGet ( + IN OUT EFI_PHYSICAL_ADDRESS *ApBase, + IN UINTN ApIndex + ); + +// // ComPhy SoC description // typedef struct { diff --git a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.c b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.c index 5b72c20..584f445 100644 --- a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.c +++ b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.c @@ -28,6 +28,40 @@ #include "Armada7k8kSoCDescLib.h" +/** + +Routine Description: + + Get base address of the SoC North Bridge. + +Arguments: + + ApBase - Base address of the North Bridge. + ApIndex - Index of the North Bridge. + +Returns: + + EFI_SUCCESS - Proper base address is returned. + EFI_INVALID_PARAMETER - The index is out of range. + +**/ +EFI_STATUS +EFIAPI +ArmadaSoCAp8xxBaseGet ( + IN OUT EFI_PHYSICAL_ADDRESS *ApBase, + IN UINTN ApIndex + ) +{ + if (ApIndex != ARMADA7K8K_AP806_INDEX) { +DEBUG ((DEBUG_ERROR, "%a: Only one AP806 in A7K/A8K SoC\n", __FUNCTION__)); +return EFI_INVALID_PARAMETER; + } + + *ApBase = MV_SOC_AP806_BASE; + + return EFI_SUCCESS; +} + EFI_STATUS EFIAPI ArmadaSoCDescComPhyGet ( -- 2.7.4 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [platforms: PATCH v2 2/4] Marvell/Library: Introduce common header for the SMC ID's
Marvell firmware allows to use SiP services other than for ComPhy handling. In order to avoid spreading the SMC ID's definitions across many files, introduce common header for that purpose. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Marcin Wojtas --- Silicon/Marvell/Include/Library/MvSmc.h | 23 Silicon/Marvell/Library/ComPhyLib/ComPhySipSvc.h | 8 +++ Silicon/Marvell/Library/ComPhyLib/ComPhyCp110.c | 14 ++-- 3 files changed, 33 insertions(+), 12 deletions(-) create mode 100644 Silicon/Marvell/Include/Library/MvSmc.h diff --git a/Silicon/Marvell/Include/Library/MvSmc.h b/Silicon/Marvell/Include/Library/MvSmc.h new file mode 100644 index 000..2d1542a --- /dev/null +++ b/Silicon/Marvell/Include/Library/MvSmc.h @@ -0,0 +1,23 @@ +/** +* +* Copyright (C) 2019, Marvell International Ltd. and its affiliates. +* +* This program and the accompanying materials are licensed and made available +* under the terms and conditions of the BSD License which accompanies this +* distribution. The full text of the license may be found at +* http://opensource.org/licenses/bsd-license.php +* +* THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, +* WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. +* +**/ + +#ifndef __MV_SMC_H__ +#define __MV_SMC_H__ + +/* Marvell SiP services SMC ID's */ +#define MV_SMC_ID_COMPHY_POWER_ON 0x8201 +#define MV_SMC_ID_COMPHY_POWER_OFF0x8202 +#define MV_SMC_ID_COMPHY_PLL_LOCK 0x8203 + +#endif //__MV_SMC_H__ diff --git a/Silicon/Marvell/Library/ComPhyLib/ComPhySipSvc.h b/Silicon/Marvell/Library/ComPhyLib/ComPhySipSvc.h index d156af6..f6ac65b 100644 --- a/Silicon/Marvell/Library/ComPhyLib/ComPhySipSvc.h +++ b/Silicon/Marvell/Library/ComPhyLib/ComPhySipSvc.h @@ -35,16 +35,14 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #ifndef __COMPHY_SIP_SVC_H__ #define __COMPHY_SIP_SVC_H__ +#include + /* * All values in this file are defined externally and used * for the SerDes configuration via SiP services. */ -/* Firmware related definitions used for SMC calls */ -#define MV_SIP_COMPHY_POWER_ON 0x8201 -#define MV_SIP_COMPHY_POWER_OFF 0x8202 -#define MV_SIP_COMPHY_PLL_LOCK 0x8203 - +/* Helper macros for passing ComPhy parameters to the EL3 */ #define COMPHY_FW_MODE_FORMAT(mode) (mode << 12) #define COMPHY_FW_FORMAT(mode, idx, speeds) \ ((mode << 12) | (idx << 8) | (speeds << 2)) diff --git a/Silicon/Marvell/Library/ComPhyLib/ComPhyCp110.c b/Silicon/Marvell/Library/ComPhyLib/ComPhyCp110.c index 2abb006..4f85676 100755 --- a/Silicon/Marvell/Library/ComPhyLib/ComPhyCp110.c +++ b/Silicon/Marvell/Library/ComPhyLib/ComPhyCp110.c @@ -163,7 +163,7 @@ ComPhySataPowerUp ( ComPhySataMacPowerDown (Desc[ChipId].SoC->AhciBaseAddress); - Status = ComPhySmc (MV_SIP_COMPHY_POWER_ON, + Status = ComPhySmc (MV_SMC_ID_COMPHY_POWER_ON, ComPhyBase, Lane, COMPHY_FW_FORMAT (COMPHY_SATA_MODE, @@ -175,7 +175,7 @@ ComPhySataPowerUp ( ComPhySataPhyPowerUp (Desc[ChipId].SoC->AhciBaseAddress); - Status = ComPhySmc (MV_SIP_COMPHY_PLL_LOCK, + Status = ComPhySmc (MV_SMC_ID_COMPHY_PLL_LOCK, ComPhyBase, Lane, COMPHY_FW_FORMAT (COMPHY_SATA_MODE, @@ -234,7 +234,7 @@ ComPhyCp110Init ( case COMPHY_TYPE_PCIE1: case COMPHY_TYPE_PCIE2: case COMPHY_TYPE_PCIE3: - Status = ComPhySmc (MV_SIP_COMPHY_POWER_ON, + Status = ComPhySmc (MV_SMC_ID_COMPHY_POWER_ON, PtrChipCfg->ComPhyBaseAddr, Lane, COMPHY_FW_PCIE_FORMAT (PcieWidth, @@ -269,7 +269,7 @@ ComPhyCp110Init ( break; case COMPHY_TYPE_USB3_HOST0: case COMPHY_TYPE_USB3_HOST1: - Status = ComPhySmc (MV_SIP_COMPHY_POWER_ON, + Status = ComPhySmc (MV_SMC_ID_COMPHY_POWER_ON, PtrChipCfg->ComPhyBaseAddr, Lane, COMPHY_FW_MODE_FORMAT (COMPHY_USB3H_MODE)); @@ -278,7 +278,7 @@ ComPhyCp110Init ( case COMPHY_TYPE_SGMII1: case COMPHY_TYPE_SGMII2: case COMPHY_TYPE_SGMII3: - Status = ComPhySmc (MV_SIP_COMPHY_POWER_ON, + Status = ComPhySmc (MV_SMC_ID_COMPHY_POWER_ON, PtrChipCfg->ComPhyBaseAddr, Lane, COMPHY_FW_FORMAT (COMPHY_SGMII_MODE, @@ -286,7 +286,7 @@ ComPhyCp110Init ( PtrComPhyMap->Speed)); break; case COMPHY_TYPE_SFI: - Status = ComPhySmc (MV_SIP_COMPHY_POWER_ON, + Status = ComPhySmc (MV_SMC_ID_COMPHY_POWER_ON, PtrChipCfg->ComPhyBaseAddr, Lane, COMPHY_FW_FORMAT (COMPHY_SFI_MODE, @@ -295,7 +295,7 @@ ComPhyCp110Init ( break; case COMPHY_TYPE_RXAUI0: case COMPHY_TYPE_RXAUI1: - Status = ComPhySmc
[edk2] [platforms: PATCH v2 1/4] Marvell/Armada7k8k: Shift PEI stack base
Recent changes in the ARM-TF configure its runtime serices region as protected, hence the hitherto PEI stack base address (0x41F) violated it. In order to fix this, extend the region which is non-accessible by the OS to cover both the ARM-TF (0x400 - 0x420) and OPTEE (0x440 - 0x540) within a single area (0x400 - 0x540). Set the PEI stack base address between both images (0x43F). Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Marcin Wojtas --- Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc index eafcd6e..c8c597f 100644 --- a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc +++ b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc @@ -376,12 +376,12 @@ gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize|36 - gArmPlatformTokenSpaceGuid.PcdCPUCoresStackBase|0x41F + gArmPlatformTokenSpaceGuid.PcdCPUCoresStackBase|0x43F gArmPlatformTokenSpaceGuid.PcdCPUCorePrimaryStackSize|0x1 # Secure region reservation gMarvellTokenSpaceGuid.PcdSecureRegionBase|0x400 - gMarvellTokenSpaceGuid.PcdSecureRegionSize|0x020 + gMarvellTokenSpaceGuid.PcdSecureRegionSize|0x140 # TRNG gMarvellTokenSpaceGuid.PcdEip76TrngBaseAddress|0xF276 -- 2.7.4 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [platforms: PATCH v2 0/4] Armada7k8k memory handling update
Hi, The second version of the patchset introduces the new common header for Marvell SMC ID's, and answers other remarks pointed out in v1 review. The details can be found in the changelog below. Patches are available in the github: https://github.com/MarvellEmbeddedProcessors/edk2-open-platform/commits/dram-upstream-r20190122 I'm looking forward to the comments and remarks. Best regards, Marcin Changelog: v1 -> v2: * 1/4 - Improve commit log - mention single area size and new PEI stack base * 2/4 (new patch) - Add common header for Marvell SMC ID's * 3/4 - Add function description comment - Define and use ARMADA7K8K_AP806_INDEX - Change function argument to EFI_PHYSICAL_ADDRESS * 4/4 - Move new SMC ID to MvSmc.h - Include ArmadaSoCDescLib.h directly (instead indirectly via BoardDesc.h) - Remove ARMADA7K8K_AP806_INDEX macro Grzegorz Jaszczyk (2): Marvell/Library: ArmadaSoCDescLib: Add North Bridge description Marvell/Armada7k8k: Read DRAM settings from ARM-TF Marcin Wojtas (2): Marvell/Armada7k8k: Shift PEI stack base Marvell/Library: Introduce common header for the SMC ID's Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc | 4 +- Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLib.inf | 3 + Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.h | 25 Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.h | 6 ++ Silicon/Marvell/Include/Library/ArmadaSoCDescLib.h | 28 + Silicon/Marvell/Include/Library/MvSmc.h | 24 Silicon/Marvell/Library/ComPhyLib/ComPhySipSvc.h | 8 +-- Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.c | 60 ++-- Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.c | 34 +++ Silicon/Marvell/Library/ComPhyLib/ComPhyCp110.c | 14 ++--- 10 files changed, 125 insertions(+), 81 deletions(-) create mode 100644 Silicon/Marvell/Include/Library/MvSmc.h -- 2.7.4 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH] UefiCpuPkg/MtrrLib: Fix a bug that may wrongly set memory <1MB to UC
Reviewed-by: Chasel Chiu > -Original Message- > From: Ni, Ray > Sent: Monday, January 21, 2019 11:17 PM > To: edk2-devel@lists.01.org > Cc: Ni, Ray ; Dong, Eric ; Chiu, Chasel > > Subject: [PATCH] UefiCpuPkg/MtrrLib: Fix a bug that may wrongly set memory > <1MB to UC > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1481 > > Today's MtrrLib contains a bug, for example: > when the original cache setting is WB for [0xF_, 0xF_8000) and, a new > request to set [0xF_, 0xF_4000) to WP, the cache setting for [0xF_4000, > 0xF_8000) is reset to UC. > > The reason is when MtrrLibSetBelow1MBMemoryAttribute() is called the > WorkingFixedSettings doesn't contain the actual MSR value stored in hardware, > but when writing the fixed MTRRs, the code logic assumes WorkingFixedSettings > contains the actual MSR value. > > The new fix is to change MtrrLibSetBelow1MBMemoryAttribute() to calculate > the correct ClearMasks[] and OrMasks[], and use them directly when writing the > fixed MTRRs. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Ray Ni > Cc: Eric Dong > Cc: Chasel Chiu > --- > UefiCpuPkg/Library/MtrrLib/MtrrLib.c | 59 +--- > 1 file changed, 18 insertions(+), 41 deletions(-) > > diff --git a/UefiCpuPkg/Library/MtrrLib/MtrrLib.c > b/UefiCpuPkg/Library/MtrrLib/MtrrLib.c > index 086f7ad8f0..2cf7d092e8 100644 > --- a/UefiCpuPkg/Library/MtrrLib/MtrrLib.c > +++ b/UefiCpuPkg/Library/MtrrLib/MtrrLib.c > @@ -5,7 +5,7 @@ > Most of services in this library instance are suggested to be invoked by > BSP > only, > except for MtrrSetAllMtrrs() which is used to sync BSP's MTRR setting to > APs. > > - Copyright (c) 2008 - 2018, Intel Corporation. All rights reserved. > + Copyright (c) 2008 - 2019, Intel Corporation. All rights > + reserved. >This program and the accompanying materials >are licensed and made available under the terms and conditions of the BSD > License >which accompanies this distribution. The full text of the license may be > found > at @@ -2099,8 +2099,8 @@ MtrrLibSetMemoryRanges ( >Set the below-1MB memory attribute to fixed MTRR buffer. >Modified flag array indicates which fixed MTRR is modified. > > - @param [in, out] FixedSettings Fixed MTRR buffer. > - @param [out] Modified Flag array indicating which MTRR is > modified. > + @param [in, out] ClearMasksThe bits to clear in the fixed MTRR MSR. > + @param [in, out] OrMasks The bits to set in the fixed MTRR MSR. >@param [in] BaseAddress Base address. >@param [in] LengthLength. >@param [in] Type Memory type. > @@ -2111,8 +2111,8 @@ MtrrLibSetMemoryRanges ( **/ RETURN_STATUS > MtrrLibSetBelow1MBMemoryAttribute ( > - IN OUT MTRR_FIXED_SETTINGS *FixedSettings, > - OUT BOOLEAN*Modified, > + IN OUT UINT64 *ClearMasks, > + IN OUT UINT64 *OrMasks, >IN PHYSICAL_ADDRESSBaseAddress, >IN UINT64 Length, >IN MTRR_MEMORY_CACHE_TYPE Type > @@ -2122,36 +2122,17 @@ MtrrLibSetBelow1MBMemoryAttribute ( >UINT32MsrIndex; >UINT64ClearMask; >UINT64OrMask; > - UINT64ClearMasks[ARRAY_SIZE (mMtrrLibFixedMtrrTable)]; > - UINT64OrMasks[ARRAY_SIZE (mMtrrLibFixedMtrrTable)]; > - BOOLEAN LocalModified[ARRAY_SIZE > (mMtrrLibFixedMtrrTable)]; > >ASSERT (BaseAddress < BASE_1MB); > > - SetMem (LocalModified, sizeof (LocalModified), FALSE); > - > - // > - // (Value & ~0 | 0) still equals to (Value) > - // > - SetMem (ClearMasks, sizeof (ClearMasks), 0); > - SetMem (OrMasks, sizeof (OrMasks), 0); > - >MsrIndex = (UINT32)-1; >while ((BaseAddress < BASE_1MB) && (Length != 0)) { > Status = MtrrLibProgramFixedMtrr (Type, , , > , , ); > if (RETURN_ERROR (Status)) { >return Status; > } > -ClearMasks[MsrIndex]= ClearMask; > -OrMasks[MsrIndex] = OrMask; > -Modified[MsrIndex] = TRUE; > -LocalModified[MsrIndex] = TRUE; > - } > - > - for (MsrIndex = 0; MsrIndex < ARRAY_SIZE (mMtrrLibFixedMtrrTable); > MsrIndex++) { > -if (LocalModified[MsrIndex]) { > - FixedSettings->Mtrr[MsrIndex] = (FixedSettings->Mtrr[MsrIndex] & > ~ClearMasks[MsrIndex]) | OrMasks[MsrIndex]; > -} > +ClearMasks[MsrIndex] = ClearMasks[MsrIndex] | ClearMask; > +OrMasks[MsrIndex]= (OrMasks[MsrIndex] & ~ClearMask) | OrMask; >} >return RETURN_SUCCESS; > } > @@ -2213,8 +2194,8 @@ MtrrSetMemoryAttributesInMtrrSettings ( >MTRR_MEMORY_RANGE WorkingVariableMtrr[ARRAY_SIZE > (MtrrSetting->Variables.Mtrr)]; >BOOLEAN VariableSettingModified[ARRAY_SIZE > (MtrrSetting->Variables.Mtrr)]; > > - BOOLEAN FixedSettingsModified[ARRAY_SIZE >
Re: [edk2] [PATCH v2 1/3] MdeModulePkg/Dhcp4Dxe: Remove unnecessary NULL pointer check.
On 1/21/19 10:21 PM, Laszlo Ersek wrote: > Hi Phil, > > On 01/21/19 14:26, Philippe Mathieu-Daudé wrote: >> Hi, >> >> On 1/21/19 1:53 PM, Gao, Liming wrote: >>> Thanks Ard and Laszlo. For the minor change in single patch, the patch may >>> be sent separately with the clear subject. Or, the patch set can be sent >>> again. >> >> Since it is hard to follow technical discussion when top-posted (see >> https://www.caliburn.nl/topposting.html) without scrolling and sometime >> loosing context, can we gently suggest bottom-posting in edk2-devel >> etiquette? (No offence, this is a humble suggestion from a not very >> active reviewer to a highly active contributor, but this might ease the >> on-list review workflow). > > top vs. bottom posting have been mentioned multiple times on edk2-devel; > it's just a fact that most corporate email environments don't support > bottom posting at all. > > I'm unhappy about it (obviously), but it's an uphill battle. Sometimes > the poster would actually *like* to bottom post, but the tooling (which > may not be their own choice) gets in their way. > > I suggest always scrolling to the bottom, or at least until you see a > signature. Oh, I was not aware of that, it makes now sense (there is a common pattern in companies/location with top-posts). Sorry for the noise, at least I tried ;) Regards, Phil. > > Thanks, > Laszlo > ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH V2 10/15] ArmVirtXen: Use merged variable driver for emulated NV mode
On Mon, 21 Jan 2019 at 22:22, Ard Biesheuvel wrote: > > On Mon, 21 Jan 2019 at 22:15, Laszlo Ersek wrote: > > > > Hi Julien, > > > > On 01/21/19 14:36, Julien Grall wrote: > > > Hi, > > > > > > On 21/01/2019 10:46, Zeng, Star wrote: > > >> On 2019/1/18 2:59, Julien Grall wrote: > > >> I saw the discussion at > > >> https://lists.01.org/pipermail/edk2-devel/2019-January/035405.html. > > >> Fortunately, > > >> it has been fixed. > > >> So I did rebase for the code. > > >> Repo: g...@github.com:lzeng14/edk2.git > > >> Branch: MergedVariableDriver_EmuNvMode_V3_rebased > > > > > > I was about to ask a branch as there were conflicts in the rebase. > > > Thank you for providing the branch! > > > > > >> > > >> If you can help have a quick test, that will be very helpful. :) > > > > > > With your series applied, EDK2 is crashing while the Linux EFI stub > > > is running. See the log below. > > > > > > My knowledge of EDK2 is quite limited, so I am not entirely where to > > > look at. I am happy to help debugging if you provide guidance. > > > > Let's focus on the following excerpts: > > > > > Synchronous Exception at 0x7BE70698 > > > PC 0x7BE70698 (0x7BE6+0x00010698) [ 0] RuntimeDxe.dll > > > PC 0x78AFECB0 (0x78AE+0x0001ECB0) [ 1] VariableRuntimeDxe.dll > > > PC 0x78AF0AC8 (0x78AE+0x00010AC8) [ 1] VariableRuntimeDxe.dll > > > PC 0x7BE7094C (0x7BE6+0x0001094C) [ 2] RuntimeDxe.dll > > > > and > > > > > [ 0] > > > /home/julien/works/edk2/Build/ArmVirtXen-AARCH64/DEBUG_GCC49/AARCH64/MdeModulePkg/Core/RuntimeDxe/RuntimeDxe/DEBUG/RuntimeDxe.dll > > > [ 1] > > > /home/julien/works/edk2/Build/ArmVirtXen-AARCH64/DEBUG_GCC49/AARCH64/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe/DEBUG/VariableRuntimeDxe.dll > > > [ 2] > > > /home/julien/works/edk2/Build/ArmVirtXen-AARCH64/DEBUG_GCC49/AARCH64/MdeModulePkg/Core/RuntimeDxe/RuntimeDxe/DEBUG/RuntimeDxe.dll > > > > Please run "objdump -S" on the DLL files listed in [0] through [2]. > > > > In the listings, please try to locate the neighborhood of the relative > > offsets that are listed in the stack dump (such as 0x10698 and 0x1094C > > in "RuntimeDxe.dll", and 0x1ECB0 and 0x10AC8 in "VariableRuntimeDxe.dll"). > > > > The disassembly should be intermixed with C source code, and four stack > > frames with C language snippets should help us establish a rudimentary > > call chain. > > > > (CC'ing Ard as well.) > > > > I already did the same. The crash is in a call to EfiConvertPointer () > from VariableClassAddressChangeEvent (), which dereferences > mVariableModuleGlobal->FvbInstance, which may be NULL in the EMU use > case. > > It is very surprising that this only happens on ARM, though ... Never mind. Another bug masked by the 'memory at address 0x0' issue. ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH V2 10/15] ArmVirtXen: Use merged variable driver for emulated NV mode
On Mon, 21 Jan 2019 at 22:15, Laszlo Ersek wrote: > > Hi Julien, > > On 01/21/19 14:36, Julien Grall wrote: > > Hi, > > > > On 21/01/2019 10:46, Zeng, Star wrote: > >> On 2019/1/18 2:59, Julien Grall wrote: > >> I saw the discussion at > >> https://lists.01.org/pipermail/edk2-devel/2019-January/035405.html. > >> Fortunately, > >> it has been fixed. > >> So I did rebase for the code. > >> Repo: g...@github.com:lzeng14/edk2.git > >> Branch: MergedVariableDriver_EmuNvMode_V3_rebased > > > > I was about to ask a branch as there were conflicts in the rebase. > > Thank you for providing the branch! > > > >> > >> If you can help have a quick test, that will be very helpful. :) > > > > With your series applied, EDK2 is crashing while the Linux EFI stub > > is running. See the log below. > > > > My knowledge of EDK2 is quite limited, so I am not entirely where to > > look at. I am happy to help debugging if you provide guidance. > > Let's focus on the following excerpts: > > > Synchronous Exception at 0x7BE70698 > > PC 0x7BE70698 (0x7BE6+0x00010698) [ 0] RuntimeDxe.dll > > PC 0x78AFECB0 (0x78AE+0x0001ECB0) [ 1] VariableRuntimeDxe.dll > > PC 0x78AF0AC8 (0x78AE+0x00010AC8) [ 1] VariableRuntimeDxe.dll > > PC 0x7BE7094C (0x7BE6+0x0001094C) [ 2] RuntimeDxe.dll > > and > > > [ 0] > > /home/julien/works/edk2/Build/ArmVirtXen-AARCH64/DEBUG_GCC49/AARCH64/MdeModulePkg/Core/RuntimeDxe/RuntimeDxe/DEBUG/RuntimeDxe.dll > > [ 1] > > /home/julien/works/edk2/Build/ArmVirtXen-AARCH64/DEBUG_GCC49/AARCH64/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe/DEBUG/VariableRuntimeDxe.dll > > [ 2] > > /home/julien/works/edk2/Build/ArmVirtXen-AARCH64/DEBUG_GCC49/AARCH64/MdeModulePkg/Core/RuntimeDxe/RuntimeDxe/DEBUG/RuntimeDxe.dll > > Please run "objdump -S" on the DLL files listed in [0] through [2]. > > In the listings, please try to locate the neighborhood of the relative > offsets that are listed in the stack dump (such as 0x10698 and 0x1094C > in "RuntimeDxe.dll", and 0x1ECB0 and 0x10AC8 in "VariableRuntimeDxe.dll"). > > The disassembly should be intermixed with C source code, and four stack > frames with C language snippets should help us establish a rudimentary > call chain. > > (CC'ing Ard as well.) > I already did the same. The crash is in a call to EfiConvertPointer () from VariableClassAddressChangeEvent (), which dereferences mVariableModuleGlobal->FvbInstance, which may be NULL in the EMU use case. It is very surprising that this only happens on ARM, though ... ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v2 1/3] MdeModulePkg/Dhcp4Dxe: Remove unnecessary NULL pointer check.
Hi Phil, On 01/21/19 14:26, Philippe Mathieu-Daudé wrote: > Hi, > > On 1/21/19 1:53 PM, Gao, Liming wrote: >> Thanks Ard and Laszlo. For the minor change in single patch, the patch may >> be sent separately with the clear subject. Or, the patch set can be sent >> again. > > Since it is hard to follow technical discussion when top-posted (see > https://www.caliburn.nl/topposting.html) without scrolling and sometime > loosing context, can we gently suggest bottom-posting in edk2-devel > etiquette? (No offence, this is a humble suggestion from a not very > active reviewer to a highly active contributor, but this might ease the > on-list review workflow). top vs. bottom posting have been mentioned multiple times on edk2-devel; it's just a fact that most corporate email environments don't support bottom posting at all. I'm unhappy about it (obviously), but it's an uphill battle. Sometimes the poster would actually *like* to bottom post, but the tooling (which may not be their own choice) gets in their way. I suggest always scrolling to the bottom, or at least until you see a signature. Thanks, Laszlo ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH V2 10/15] ArmVirtXen: Use merged variable driver for emulated NV mode
Hi Julien, On 01/21/19 14:36, Julien Grall wrote: > Hi, > > On 21/01/2019 10:46, Zeng, Star wrote: >> On 2019/1/18 2:59, Julien Grall wrote: >> I saw the discussion at >> https://lists.01.org/pipermail/edk2-devel/2019-January/035405.html. >> Fortunately, >> it has been fixed. >> So I did rebase for the code. >> Repo: g...@github.com:lzeng14/edk2.git >> Branch: MergedVariableDriver_EmuNvMode_V3_rebased > > I was about to ask a branch as there were conflicts in the rebase. > Thank you for providing the branch! > >> >> If you can help have a quick test, that will be very helpful. :) > > With your series applied, EDK2 is crashing while the Linux EFI stub > is running. See the log below. > > My knowledge of EDK2 is quite limited, so I am not entirely where to > look at. I am happy to help debugging if you provide guidance. Let's focus on the following excerpts: > Synchronous Exception at 0x7BE70698 > PC 0x7BE70698 (0x7BE6+0x00010698) [ 0] RuntimeDxe.dll > PC 0x78AFECB0 (0x78AE+0x0001ECB0) [ 1] VariableRuntimeDxe.dll > PC 0x78AF0AC8 (0x78AE+0x00010AC8) [ 1] VariableRuntimeDxe.dll > PC 0x7BE7094C (0x7BE6+0x0001094C) [ 2] RuntimeDxe.dll and > [ 0] > /home/julien/works/edk2/Build/ArmVirtXen-AARCH64/DEBUG_GCC49/AARCH64/MdeModulePkg/Core/RuntimeDxe/RuntimeDxe/DEBUG/RuntimeDxe.dll > [ 1] > /home/julien/works/edk2/Build/ArmVirtXen-AARCH64/DEBUG_GCC49/AARCH64/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe/DEBUG/VariableRuntimeDxe.dll > [ 2] > /home/julien/works/edk2/Build/ArmVirtXen-AARCH64/DEBUG_GCC49/AARCH64/MdeModulePkg/Core/RuntimeDxe/RuntimeDxe/DEBUG/RuntimeDxe.dll Please run "objdump -S" on the DLL files listed in [0] through [2]. In the listings, please try to locate the neighborhood of the relative offsets that are listed in the stack dump (such as 0x10698 and 0x1094C in "RuntimeDxe.dll", and 0x1ECB0 and 0x10AC8 in "VariableRuntimeDxe.dll"). The disassembly should be intermixed with C source code, and four stack frames with C language snippets should help us establish a rudimentary call chain. (CC'ing Ard as well.) Thanks Laszlo ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH V2 10/15] ArmVirtXen: Use merged variable driver for emulated NV mode
On Mon, 21 Jan 2019 at 14:36, Julien Grall wrote: > > Hi, > > On 21/01/2019 10:46, Zeng, Star wrote: > > On 2019/1/18 2:59, Julien Grall wrote: > > I saw the discussion at > > https://lists.01.org/pipermail/edk2-devel/2019-January/035405.html. > > Fortunately, > > it has been fixed. > > So I did rebase for the code. > > Repo: g...@github.com:lzeng14/edk2.git > > Branch: MergedVariableDriver_EmuNvMode_V3_rebased > > I was about to ask a branch as there were conflicts in the rebase. > Thank you for providing the branch! > > > > > If you can help have a quick test, that will be very helpful. :) > > With your series applied, EDK2 is crashing while the Linux EFI stub > is running. See the log below. > > My knowledge of EDK2 is quite limited, so I am not entirely where to > look at. I am happy to help debugging if you provide guidance. > Hi Julien, Could you try the patch below please? diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c index a8bb9cf25ebd..adaf6ccb48b0 100644 --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c @@ -236,14 +236,16 @@ VariableClassAddressChangeEvent ( { UINTN Index; - EfiConvertPointer (0x0, (VOID **) >FvbInstance->GetBlockSize); - EfiConvertPointer (0x0, (VOID **) >FvbInstance->GetPhysicalAddress); - EfiConvertPointer (0x0, (VOID **) >FvbInstance->GetAttributes); - EfiConvertPointer (0x0, (VOID **) >FvbInstance->SetAttributes); - EfiConvertPointer (0x0, (VOID **) >FvbInstance->Read); - EfiConvertPointer (0x0, (VOID **) >FvbInstance->Write); - EfiConvertPointer (0x0, (VOID **) >FvbInstance->EraseBlocks); - EfiConvertPointer (0x0, (VOID **) >FvbInstance); + if (mVariableModuleGlobal->FvbInstance != NULL) { +EfiConvertPointer (0x0, (VOID **) >FvbInstance->GetBlockSize); +EfiConvertPointer (0x0, (VOID **) >FvbInstance->GetPhysicalAddress); +EfiConvertPointer (0x0, (VOID **) >FvbInstance->GetAttributes); +EfiConvertPointer (0x0, (VOID **) >FvbInstance->SetAttributes); +EfiConvertPointer (0x0, (VOID **) >FvbInstance->Read); +EfiConvertPointer (0x0, (VOID **) >FvbInstance->Write); +EfiConvertPointer (0x0, (VOID **) >FvbInstance->EraseBlocks); +EfiConvertPointer (0x0, (VOID **) >FvbInstance); + } EfiConvertPointer (0x0, (VOID **) >PlatformLangCodes); EfiConvertPointer (0x0, (VOID **) >LangCodes); EfiConvertPointer (0x0, (VOID **) >PlatformLang); > Press any key to continue... > > [Security] 3rd party image[0] can be loaded after EndOfDxe: > MemoryMapped(0x2,0x67789000,0x68DF1200). > > InstallProtocolInterface: 5B1B31A1-9562-11D2-8E3F-00A0C969723B 7AB2B040 > > Loading driver at 0x00065783000 EntryPoint=0x00066878664 > > Loading driver at 0x00065783000 EntryPoint=0x00066878664 > > InstallProtocolInterface: BC62157E-3E33-4FEC-9920-2D3B36D750DF 7AA54B18 > > ProtectUefiImageCommon - 0x7AB2B040 > > - 0x65783000 - 0x02006000 > > SetUefiImageMemoryAttributes - 0x65783000 - 0x1000 > (0x4008) > > SetUefiImageMemoryAttributes - 0x65784000 - 0x011CD000 > (0x00020008) > > SetUefiImageMemoryAttributes - 0x66951000 - 0x00E38000 > (0x4008) > > EFI stub: Booting Linux Kernel... > > EFI stub: Using DTB from configuration table > > EFI stub: Exiting boot services and installing virtual address map... > > XenBus: Set state to 5 > > XenBus: Set state to 5, done > > XenPvBlk: waiting backend state 5, current: 4 > > XenStore: Watch event 7B036698 > > XenBus: Set state to 6 > > XenBus: Set state to 6, done > > XenPvBlk: waiting backend state 6, current: 5 > > XenStore: Watch event 7B036698 > > XenBus: Set state to 1 > > XenBus: Set state to 1, done > > Xen GrantTable, removing 38003 > > Xen GrantTable, removing 38002 > > Xen GrantTable, removing 38001 > > Xen GrantTable, removing 38000 > > SetUefiImageMemoryAttributes - 0x7BE6 - 0x0004 > (0x0008) > > SetUefiImageMemoryAttributes - 0x78AE - 0x0005 > (0x0008) > > SetUefiImageMemoryAttributes - 0x78A9 - 0x0004 > (0x0008) > > SetUefiImageMemoryAttributes - 0x789F - 0x0004 > (0x0008) > > SetUefiImageMemoryAttributes - 0x7895 - 0x0004 > (0x0008) > > SetUefiImageMemoryAttributes - 0x788B - 0x0004 > (0x0008) > > > > > > Synchronous Exception at 0x7BE70698 > > PC 0x7BE70698 (0x7BE6+0x00010698) [ 0] RuntimeDxe.dll > > PC 0x78AFECB0 (0x78AE+0x0001ECB0) [ 1] VariableRuntimeDxe.dll > > PC 0x78AF0AC8 (0x78AE+0x00010AC8) [ 1] VariableRuntimeDxe.dll > > PC 0x7BE7094C (0x7BE6+0x0001094C) [ 2] RuntimeDxe.dll > > PC 0x6687E014 > > PC 0x6687C348 > > PC 0x66878680 > > PC
Re: [edk2] [PATCH edk2-platforms 0/7] Silicon/SynQuacer: implement SMM based secure boot
On Thu, 17 Jan 2019 at 12:14, Leif Lindholm wrote: > > On Fri, Jan 04, 2019 at 03:43:29PM +0100, Ard Biesheuvel wrote: > > Wire up the various pieces so that the authenticated variable store > > runs entirely in standalone MM context residing in a secure partition. > > > > This primarily involves refactoring the platform's NOR flash driver so > > we can build a version that can work in the standalone MM context. > > Beyond that, it is just a matter of enabling all the boilerplate in > > the .DSC and .FDF files. > > > > Note that the resulting standalone MM firmware volume needs to be > > wrapped in a FIP, which is not part of the build sequence. > > > > Cc: Leif Lindholm > > Cc: Masahisa Kojima > > > > Ard Biesheuvel (7): > > Silicon/SynQuacer/Fip006Dxe: drop block I/O and disk I/O routines > > Silicon/SynQuacer/Fip006Dxe: factor out DXE specific pieces > > Silicon/SynQuacer/Fip006Dxe: implement standalone MM variant > > Silicon/SynQuacer/Fip006Dxe: use proper accessor for unaligned access > > Platform/DeveloperBox: create shared .DSC include file > > Platform/DeveloperBox: add .DSC/.FDF description of MM components > > Platform/DeveloperBox: add MM based UEFI secure boot support > > For the patches I haven't commented on individually: > Reviewed-by: Leif Lindholm > Thanks Series pushed as 9b725b6ebb39..d571b43f8741 > > .../Socionext/DeveloperBox/DeveloperBox.dsc | 304 +--- > > .../DeveloperBox/DeveloperBox.dsc.inc | 315 > > .../Socionext/DeveloperBox/DeveloperBox.fdf | 13 + > > .../Socionext/DeveloperBox/DeveloperBoxMm.dsc | 103 ++ > > .../Socionext/DeveloperBox/DeveloperBoxMm.fdf | 161 ++ > > .../SynQuacer/Drivers/Fip006Dxe/Fip006Dxe.inf |9 +- > > .../Drivers/Fip006Dxe/Fip006StandaloneMm.inf | 71 + > > .../SynQuacer/Drivers/Fip006Dxe/NorFlash.c| 1006 + > > .../Fip006Dxe/{NorFlashDxe.h => NorFlash.h} | 93 +- > > .../Drivers/Fip006Dxe/NorFlashBlockIoDxe.c| 138 -- > > .../SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.c | 1341 ++--- > > .../{NorFlashFvbDxe.c => NorFlashFvb.c} | 197 +-- > > .../SynQuacer/Drivers/Fip006Dxe/NorFlashSmm.c | 182 +++ > > 13 files changed, 2076 insertions(+), 1857 deletions(-) > > create mode 100644 Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc > > create mode 100644 Platform/Socionext/DeveloperBox/DeveloperBoxMm.dsc > > create mode 100644 Platform/Socionext/DeveloperBox/DeveloperBoxMm.fdf > > create mode 100644 > > Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/Fip006StandaloneMm.inf > > create mode 100644 Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlash.c > > rename Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/{NorFlashDxe.h => > > NorFlash.h} (85%) > > delete mode 100644 > > Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashBlockIoDxe.c > > rename Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/{NorFlashFvbDxe.c => > > NorFlashFvb.c} (76%) > > create mode 100644 > > Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashSmm.c > > > > -- > > 2.17.1 > > ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH edk2-platforms 6/7] Platform/DeveloperBox: add .DSC/.FDF description of MM components
On Mon, Jan 21, 2019 at 05:57:20PM +0100, Ard Biesheuvel wrote: > On Thu, 17 Jan 2019 at 13:18, Ard Biesheuvel > wrote: > > > > On Thu, 17 Jan 2019 at 13:08, Leif Lindholm > > wrote: > > > > > > On Thu, Jan 17, 2019 at 12:10:01PM +0100, Ard Biesheuvel wrote: > > > > > > > > > > > > > > > > > > # > > > > > > @@ -294,8 +295,10 @@ [PcdsFixedAtBuild.common] > > > > > > !endif > > > > > > > > > > > > gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareRevision|$(BUILD_NUMBER) > > > > > > > > > > > > - gArmTokenSpaceGuid.PcdMmBufferBase|0xFFC0 > > > > > > - gArmTokenSpaceGuid.PcdMmBufferSize|0x0020 > > > > > > + gEfiSecurityPkgTokenSpaceGuid.PcdUserPhysicalPresence|TRUE > > > > > > > > > > So, I can see why you add this hard-wired for the purpose of testing. > > > > > But please, add a *very* conspicuous, and strongly worded, comment > > > > > statement preceding it. > > > > > > > > Well, I was talking to Peter about this the other day: according to > > > > the spec, this setting should only matter before exit boot services, > > > > and since this platform only supports serial and GOP consoles, one > > > > could argue that only a physically present user could interact with it > > > > before that time. > > > > > > But that also makes the Pcd pointless. > > > > > > > The obvious way of implementing this non-trivially on this platform is > > > > to use a DIP switch, but that requires you to open the case to > > > > enroll/delete the platform key. Perhaps that does not matter, and it > > > > would in fact produce a less dangerous reference implementation. > > > > > > I would be totally OK with that. > > > I would also be totally OK with a DynamicPcd settable through the UI > > > (which is what most machines I come across have). > > > > > > > That won't work for this implementation: the state of dynamic PCDs > > does not propagate into the MM world (nor should it), and so no MM > > driver implementing the dynamic PCD protocol exists. > > > > > But I would also be cool with a sufficiently evil "here be dragons" > > > statement, pointing out that we don't care that much > > > *on*this*specific*system* because the NOR isn't actually hw protected > > > anyway, and this implementation is all about exercising the software > > > stack.. > > > > > > > I'll go with that for the time being. > > I'll add the following: > > diff --git a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc > b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc > index f191edcb78dd..1ac8c8f95722 100644 > --- a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc > +++ b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc > @@ -301,6 +301,13 @@ > !endif >gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareRevision|$(BUILD_NUMBER) > > + # > + # NOTE: this platform is not fully secure (the NOR flash is mapped > non-secure) > + # and so the MM based secure boot implementation it provides should > be treated > + # as a reference only. For this reason, it does not make a lot of sense to > + # implement an elaborate PlatformSecureLib implementation that can assert > + # physical presence, and instead, we'll stick with the default PCD based > one. > + # >gEfiSecurityPkgTokenSpaceGuid.PcdUserPhysicalPresence|TRUE > >gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000 Totally happy with that. Reviewed-by: Leif Lindholm ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH edk2-platforms 6/7] Platform/DeveloperBox: add .DSC/.FDF description of MM components
On Thu, 17 Jan 2019 at 13:18, Ard Biesheuvel wrote: > > On Thu, 17 Jan 2019 at 13:08, Leif Lindholm wrote: > > > > On Thu, Jan 17, 2019 at 12:10:01PM +0100, Ard Biesheuvel wrote: > > > > > > > > > > > > > > > # > > > > > @@ -294,8 +295,10 @@ [PcdsFixedAtBuild.common] > > > > > !endif > > > > >gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareRevision|$(BUILD_NUMBER) > > > > > > > > > > - gArmTokenSpaceGuid.PcdMmBufferBase|0xFFC0 > > > > > - gArmTokenSpaceGuid.PcdMmBufferSize|0x0020 > > > > > + gEfiSecurityPkgTokenSpaceGuid.PcdUserPhysicalPresence|TRUE > > > > > > > > So, I can see why you add this hard-wired for the purpose of testing. > > > > But please, add a *very* conspicuous, and strongly worded, comment > > > > statement preceding it. > > > > > > Well, I was talking to Peter about this the other day: according to > > > the spec, this setting should only matter before exit boot services, > > > and since this platform only supports serial and GOP consoles, one > > > could argue that only a physically present user could interact with it > > > before that time. > > > > But that also makes the Pcd pointless. > > > > > The obvious way of implementing this non-trivially on this platform is > > > to use a DIP switch, but that requires you to open the case to > > > enroll/delete the platform key. Perhaps that does not matter, and it > > > would in fact produce a less dangerous reference implementation. > > > > I would be totally OK with that. > > I would also be totally OK with a DynamicPcd settable through the UI > > (which is what most machines I come across have). > > > > That won't work for this implementation: the state of dynamic PCDs > does not propagate into the MM world (nor should it), and so no MM > driver implementing the dynamic PCD protocol exists. > > > But I would also be cool with a sufficiently evil "here be dragons" > > statement, pointing out that we don't care that much > > *on*this*specific*system* because the NOR isn't actually hw protected > > anyway, and this implementation is all about exercising the software > > stack.. > > > > I'll go with that for the time being. I'll add the following: diff --git a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc index f191edcb78dd..1ac8c8f95722 100644 --- a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc +++ b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc @@ -301,6 +301,13 @@ !endif gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareRevision|$(BUILD_NUMBER) + # + # NOTE: this platform is not fully secure (the NOR flash is mapped non-secure) + # and so the MM based secure boot implementation it provides should be treated + # as a reference only. For this reason, it does not make a lot of sense to + # implement an elaborate PlatformSecureLib implementation that can assert + # physical presence, and instead, we'll stick with the default PCD based one. + # gEfiSecurityPkgTokenSpaceGuid.PcdUserPhysicalPresence|TRUE gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH edk2-platforms 2/7] Silicon/SynQuacer/Fip006Dxe: factor out DXE specific pieces
On Mon, Jan 21, 2019 at 05:47:10PM +0100, Ard Biesheuvel wrote: > On Mon, 21 Jan 2019 at 17:46, Leif Lindholm wrote: > > > > On Mon, Jan 21, 2019 at 05:16:09PM +0100, Ard Biesheuvel wrote: > > > > > > diff --git > > > > > > a/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.h > > > > > > b/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlash.h > > > > > > similarity index 88% > > > > > > rename from > > > > > > Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.h > > > > > > rename to Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlash.h > > > > > > index 20e74b0320ce..61b8e6a08fa0 100644 > > > > > > --- a/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.h > > > > > > +++ b/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlash.h > > > > > > @@ -27,11 +27,9 @@ > > > > > > #include > > > > > > > > > > > > #include > > > > > > -#include > > > > > > #include > > > > > > +#include > > > > > > > > > > Why add this include? > > > > > I'm not going to ask to move out the existing headers not actually > > > > > used by this file, but could we avoid adding new ones? > > > > > > > > > > https://edk2-docs.gitbooks.io/edk-ii-c-coding-standards-specification/content/v/release/2.20/5_source_files/53_include_files.html#534-include-files-may-include-only-those-headers-that-it-directly-depends-upon > > > > > is actually a rule I agree with. > > > > > > > > > > But also, aren't all of the users of this file already manually > > > > > including this one? > > > > > > > > > > > > > Fair enough. I will drop the include here, and add it to whichever > > > > source file requires it afterwards. > > > > > > > > > > That include turns out to be entirely redundant, so I will just drop it. > > > > Works for me. > > > > Wonderful. Can I take that as an ack? Whoops. Yes. Acked-by: Leif Lindholm > > > > > > #include > > > > > > -#include > > > > > > -#include > > > > > > > > > > > > #include "Fip006Reg.h" > > > > > > > > > > > > > > > > > diff --git > > > > > > a/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.c > > > > > > b/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.c > > > > > > index e52ab52d8cf7..6c07799b22d8 100644 > > > > > > --- a/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.c > > > > > > +++ b/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.c > > > > > > @@ -15,15 +15,16 @@ > > > > > > **/ > > > > > > > > > > > > #include > > > > > > +#include > > > > > > +#include > > > > > > #include > > > > > > > > > > At least this one does. > > > > > > > > > > > #include > > > > > > #include > > > > > > #include > > > > > > #include > > > > > > +#include > > > > > > > > > > > > -#include "NorFlashDxe.h" > > > > > > - > > > > > > -STATIC EFI_EVENT mNorFlashVirtualAddrChangeEvent; > > > > > > +#include "NorFlash.h" ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH] ArmPkg: delete unused ArmTrustZoneSmc.h
On Mon, Jan 21, 2019 at 02:29:17PM +0100, Philippe Mathieu-Daudé wrote: > On 1/21/19 2:07 PM, Ard Biesheuvel wrote: > > On Mon, 21 Jan 2019 at 14:07, Leif Lindholm > > wrote: > >> > >> ArmPkg/Include/IndustryStandard/ArmTrustZoneSmc.h contains definitions > >> contradicting the SMC Calling Convention (ARM DEN0028B). > >> > >> It also has no users in public trees. So delete before it can cause > >> damage. > >> > >> Contributed-under: TianoCore Contribution Agreement 1.1 > >> Signed-off-by: Leif Lindholm > > > > Reviewed-by: Ard Biesheuvel > > Reviewed-by: Philippe Mathieu-Daudé Thanks both. Pushed as 8f470eb476. / Leif ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH edk2-platforms 2/7] Silicon/SynQuacer/Fip006Dxe: factor out DXE specific pieces
On Mon, 21 Jan 2019 at 17:46, Leif Lindholm wrote: > > On Mon, Jan 21, 2019 at 05:16:09PM +0100, Ard Biesheuvel wrote: > > > > > diff --git > > > > > a/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.h > > > > > b/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlash.h > > > > > similarity index 88% > > > > > rename from > > > > > Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.h > > > > > rename to Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlash.h > > > > > index 20e74b0320ce..61b8e6a08fa0 100644 > > > > > --- a/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.h > > > > > +++ b/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlash.h > > > > > @@ -27,11 +27,9 @@ > > > > > #include > > > > > > > > > > #include > > > > > -#include > > > > > #include > > > > > +#include > > > > > > > > Why add this include? > > > > I'm not going to ask to move out the existing headers not actually > > > > used by this file, but could we avoid adding new ones? > > > > > > > > https://edk2-docs.gitbooks.io/edk-ii-c-coding-standards-specification/content/v/release/2.20/5_source_files/53_include_files.html#534-include-files-may-include-only-those-headers-that-it-directly-depends-upon > > > > is actually a rule I agree with. > > > > > > > > But also, aren't all of the users of this file already manually > > > > including this one? > > > > > > > > > > Fair enough. I will drop the include here, and add it to whichever > > > source file requires it afterwards. > > > > > > > That include turns out to be entirely redundant, so I will just drop it. > > Works for me. > Wonderful. Can I take that as an ack? > > > > > #include > > > > > -#include > > > > > -#include > > > > > > > > > > #include "Fip006Reg.h" > > > > > > > > > > > > > > diff --git > > > > > a/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.c > > > > > b/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.c > > > > > index e52ab52d8cf7..6c07799b22d8 100644 > > > > > --- a/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.c > > > > > +++ b/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.c > > > > > @@ -15,15 +15,16 @@ > > > > > **/ > > > > > > > > > > #include > > > > > +#include > > > > > +#include > > > > > #include > > > > > > > > At least this one does. > > > > > > > > > #include > > > > > #include > > > > > #include > > > > > #include > > > > > +#include > > > > > > > > > > -#include "NorFlashDxe.h" > > > > > - > > > > > -STATIC EFI_EVENT mNorFlashVirtualAddrChangeEvent; > > > > > +#include "NorFlash.h" ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH edk2-platforms 2/7] Silicon/SynQuacer/Fip006Dxe: factor out DXE specific pieces
On Mon, Jan 21, 2019 at 05:16:09PM +0100, Ard Biesheuvel wrote: > > > > diff --git > > > > a/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.h > > > > b/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlash.h > > > > similarity index 88% > > > > rename from Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.h > > > > rename to Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlash.h > > > > index 20e74b0320ce..61b8e6a08fa0 100644 > > > > --- a/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.h > > > > +++ b/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlash.h > > > > @@ -27,11 +27,9 @@ > > > > #include > > > > > > > > #include > > > > -#include > > > > #include > > > > +#include > > > > > > Why add this include? > > > I'm not going to ask to move out the existing headers not actually > > > used by this file, but could we avoid adding new ones? > > > > > > https://edk2-docs.gitbooks.io/edk-ii-c-coding-standards-specification/content/v/release/2.20/5_source_files/53_include_files.html#534-include-files-may-include-only-those-headers-that-it-directly-depends-upon > > > is actually a rule I agree with. > > > > > > But also, aren't all of the users of this file already manually > > > including this one? > > > > > > > Fair enough. I will drop the include here, and add it to whichever > > source file requires it afterwards. > > > > That include turns out to be entirely redundant, so I will just drop it. Works for me. > > > > #include > > > > -#include > > > > -#include > > > > > > > > #include "Fip006Reg.h" > > > > > > > > > > > diff --git > > > > a/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.c > > > > b/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.c > > > > index e52ab52d8cf7..6c07799b22d8 100644 > > > > --- a/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.c > > > > +++ b/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.c > > > > @@ -15,15 +15,16 @@ > > > > **/ > > > > > > > > #include > > > > +#include > > > > +#include > > > > #include > > > > > > At least this one does. > > > > > > > #include > > > > #include > > > > #include > > > > #include > > > > +#include > > > > > > > > -#include "NorFlashDxe.h" > > > > - > > > > -STATIC EFI_EVENT mNorFlashVirtualAddrChangeEvent; > > > > +#include "NorFlash.h" ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH edk2-platforms 2/7] Silicon/SynQuacer/Fip006Dxe: factor out DXE specific pieces
On Thu, 17 Jan 2019 at 12:27, Ard Biesheuvel wrote: > > On Thu, 17 Jan 2019 at 11:10, Leif Lindholm wrote: > > > > On Fri, Jan 04, 2019 at 03:43:31PM +0100, Ard Biesheuvel wrote: > > > In preparation of creating a SMM version of the FIP006 NOR flash > > > driver, refactor the existing pieces into a core driver, the FVB > > > methods and the DXE instantiation code. > > > > > > Contributed-under: TianoCore Contribution Agreement 1.1 > > > Signed-off-by: Ard Biesheuvel > > > > I only have one nitpicky question on this patch: > > > > > --- > > > Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/Fip006Dxe.inf > > > |6 +- > > > Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlash.c > > > | 1006 + > > > Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/{NorFlashDxe.h => > > > NorFlash.h} | 52 +- > > > Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.c > > > | 1150 +++- > > > Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/{NorFlashFvbDxe.c => > > > NorFlashFvb.c} | 161 +-- > > > 5 files changed, 1194 insertions(+), 1181 deletions(-) > > > > > > > > diff --git a/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.h > > > b/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlash.h > > > similarity index 88% > > > rename from Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.h > > > rename to Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlash.h > > > index 20e74b0320ce..61b8e6a08fa0 100644 > > > --- a/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.h > > > +++ b/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlash.h > > > @@ -27,11 +27,9 @@ > > > #include > > > > > > #include > > > -#include > > > #include > > > +#include > > > > Why add this include? > > I'm not going to ask to move out the existing headers not actually > > used by this file, but could we avoid adding new ones? > > > > https://edk2-docs.gitbooks.io/edk-ii-c-coding-standards-specification/content/v/release/2.20/5_source_files/53_include_files.html#534-include-files-may-include-only-those-headers-that-it-directly-depends-upon > > is actually a rule I agree with. > > > > But also, aren't all of the users of this file already manually > > including this one? > > > > Fair enough. I will drop the include here, and add it to whichever > source file requires it afterwards. > That include turns out to be entirely redundant, so I will just drop it. > > > #include > > > -#include > > > -#include > > > > > > #include "Fip006Reg.h" > > > > > > > > diff --git a/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.c > > > b/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.c > > > index e52ab52d8cf7..6c07799b22d8 100644 > > > --- a/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.c > > > +++ b/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.c > > > @@ -15,15 +15,16 @@ > > > **/ > > > > > > #include > > > +#include > > > +#include > > > #include > > > > At least this one does. > > > > > #include > > > #include > > > #include > > > #include > > > +#include > > > > > > -#include "NorFlashDxe.h" > > > - > > > -STATIC EFI_EVENT mNorFlashVirtualAddrChangeEvent; > > > +#include "NorFlash.h" ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [platforms: PATCH 3/3] Marvell/Armada7k8k: Read DRAM settings from ARM-TF
On Mon, Jan 21, 2019 at 04:47:31PM +0100, Marcin Wojtas wrote: > > > -#define DRAM_REGION_SIZE_EVEN(C)(((C) >= 7) && ((C) <= 26)) > > > -#define GET_DRAM_REGION_SIZE_EVEN(C)((UINT64)1 << ((C) + 16)) > > > -#define DRAM_REGION_SIZE_ODD(C) ((C) <= 4) > > > -#define GET_DRAM_REGION_SIZE_ODD(C) ((UINT64)0x1800 << (C)) > > > +/* Firmware related definition used for SMC calls */ > > > +#define MV_SIP_DRAM_SIZE0x8210 > > > > Hmm... > > This would end us up with having Marvell SMC calls spread across > > multiple files. Could you insert an intermediate patch which breaks out > > the ones from Silicon/Marvell/Library/ComPhyLib/ComPhySipSvc.h into a > > separate (MarvellSmc.h?) include file? > > How about MvSmc.h ? I try to use 'Mv' prefix consistently, especially > when adding new code. Sure. > > > > If you could further add _SMC_ID to the #defines (like in edk2 > > ArmPkg/Include/IndustryStandard/ArmStdSmc.h), that would make me very > > happy. (I'd be happy for you to drop _SIP, or keep it either side of > > the addition, as per your preference - we don't seem to have precedent > > here yet.) > > > > How about below: > #define MV_SMC_ID_COMPHY_POWER_ON 0x8201 > #define MV_SMC_ID_COMPHY_POWER_OFF 0x8202 > #define MV_SMC_ID_COMPHY_PLL_LOCK 0x8203 > ? Yeah, that works. Regards, Leif > Thanks, > Marcin > > > > Regards, > > > > Leif > > > > > diff --git > > > a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.c > > > b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.c > > > index 2a4f5ad..62e8467 100644 > > > --- a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.c > > > +++ b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.c > > > @@ -33,11 +33,14 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH > > > DAMAGE. > > > > > > ***/ > > > > > > #include > > > +#include > > > #include > > > +#include > > > #include > > > #include > > > #include > > > #include > > > +#include > > > > > > #include "Armada7k8kLibMem.h" > > > > > > @@ -57,49 +60,19 @@ GetDramSize ( > > >IN OUT UINT64 *MemSize > > >) > > > { > > > - UINT64 BaseAddr; > > > - UINT8 RegionCode; > > > - UINT8 Cs; > > > - > > > - *MemSize = 0; > > > - > > > - for (Cs = 0; Cs < DRAM_MAX_CS_NUM; Cs++) { > > > - > > > -/* Exit loop on first disabled DRAM CS */ > > > -if (!DRAM_CS_ENABLED (Cs)) { > > > - break; > > > -} > > > - > > > -/* > > > - * Sanity check for base address of next DRAM block. > > > - * Only continuous space will be used. > > > - */ > > > -BaseAddr = GET_DRAM_REGION_BASE (Cs); > > > -if (BaseAddr != *MemSize) { > > > - DEBUG ((DEBUG_ERROR, > > > -"%a: DRAM blocks are not contiguous, limit size to 0x%llx\n", > > > -__FUNCTION__, > > > -*MemSize)); > > > - return EFI_SUCCESS; > > > -} > > > - > > > -/* Decode area length for current CS from register value */ > > > -RegionCode = GET_DRAM_REGION_SIZE_CODE (Cs); > > > - > > > -if (DRAM_REGION_SIZE_EVEN (RegionCode)) { > > > - *MemSize += GET_DRAM_REGION_SIZE_EVEN (RegionCode); > > > -} else if (DRAM_REGION_SIZE_ODD (RegionCode)) { > > > - *MemSize += GET_DRAM_REGION_SIZE_ODD (RegionCode); > > > -} else { > > > - DEBUG ((DEBUG_ERROR, > > > -"%a: Invalid memory region code (0x%x) for CS#%d\n", > > > -__FUNCTION__, > > > -RegionCode, > > > -Cs)); > > > - return EFI_INVALID_PARAMETER; > > > -} > > > + ARM_SMC_ARGS SmcRegs = {0}; > > > + EFI_STATUS Status; > > > + > > > + SmcRegs.Arg0 = MV_SIP_DRAM_SIZE; > > > + Status = ArmadaSoCAp8xxBaseGet (, ARMADA7K8K_AP806_INDEX); > > > + if (EFI_ERROR (Status)) { > > > +return Status; > > >} > > > > > > + ArmCallSmc (); > > > + > > > + *MemSize = SmcRegs.Arg0; > > > + > > >return EFI_SUCCESS; > > > } > > > > > > -- > > > 2.7.4 > > > ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [platforms: PATCH 3/3] Marvell/Armada7k8k: Read DRAM settings from ARM-TF
Hi Leif, pon., 21 sty 2019 o 12:51 Leif Lindholm napisał(a): > > On Mon, Jan 21, 2019 at 11:52:11AM +0100, Marcin Wojtas wrote: > > From: Grzegorz Jaszczyk > > > > The memory controller registers are marked as secure in the latest > > ARM-TF for Armada SoCs. It is available however get the DRAM > > information via SiP services in the EL3, so use it instead > > of accessing the registers directly. > > > > Contributed-under: TianoCore Contribution Agreement 1.1 > > Signed-off-by: Marcin Wojtas > > --- > > Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLib.inf | 3 ++ > > Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.h | 27 > > ++ > > Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.c | 55 > > +--- > > 3 files changed, 21 insertions(+), 64 deletions(-) > > > > diff --git > > a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLib.inf > > b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLib.inf > > index e888566..0c7f320 100644 > > --- a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLib.inf > > +++ b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLib.inf > > @@ -41,12 +41,15 @@ > > [Packages] > >ArmPkg/ArmPkg.dec > >ArmPlatformPkg/ArmPlatformPkg.dec > > + EmbeddedPkg/EmbeddedPkg.dec > >MdeModulePkg/MdeModulePkg.dec > >MdePkg/MdePkg.dec > >Silicon/Marvell/Marvell.dec > > > > [LibraryClasses] > > + ArmadaSoCDescLib > >ArmLib > > + ArmSmcLib > >DebugLib > >MemoryAllocationLib > >MppLib > > diff --git > > a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.h > > b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.h > > index cc30e4a..8a46df6 100644 > > --- a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.h > > +++ b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.h > > @@ -47,27 +47,8 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH > > DAMAGE. > > #define DRAM_REMAP_TARGET \ > >(MmioRead32 (CCU_MC_RTBR_REG) << TARGET_BASE_OFFS) > > > > -#define DRAM_CH0_MMAP_LOW_REG(cs) (0xf0020200 + (cs) * 0x8) > > -#define DRAM_CS_VALID_ENABLED_MASK 0x1 > > -#define DRAM_AREA_LENGTH_OFFS 16 > > -#define DRAM_AREA_LENGTH_MASK (0x1f << DRAM_AREA_LENGTH_OFFS) > > -#define DRAM_START_ADDRESS_L_OFFS 23 > > -#define DRAM_START_ADDRESS_L_MASK (0x1ff << > > DRAM_START_ADDRESS_L_OFFS) > > -#define DRAM_CH0_MMAP_HIGH_REG(cs) (0xf0020204 + (cs) * 0x8) > > -#define DRAM_START_ADDR_HTOL_OFFS 32 > > +/* Armada7k8k North Bridge index */ > > +#define ARMADA7K8K_AP806_INDEX 0 > > > > -#define DRAM_MAX_CS_NUM 8 > > - > > -#define DRAM_CS_ENABLED(Cs) \ > > - (MmioRead32 (DRAM_CH0_MMAP_LOW_REG (Cs)) & > > DRAM_CS_VALID_ENABLED_MASK) > > -#define GET_DRAM_REGION_BASE(Cs) \ > > - ((UINT64)MmioRead32 (DRAM_CH0_MMAP_HIGH_REG ((Cs))) << \ > > - DRAM_START_ADDR_HTOL_OFFS) | \ > > - (MmioRead32 (DRAM_CH0_MMAP_LOW_REG (Cs)) & > > DRAM_START_ADDRESS_L_MASK); > > -#define GET_DRAM_REGION_SIZE_CODE(Cs) \ > > - (MmioRead32 (DRAM_CH0_MMAP_LOW_REG ((Cs))) & \ > > - DRAM_AREA_LENGTH_MASK) >> DRAM_AREA_LENGTH_OFFS > > -#define DRAM_REGION_SIZE_EVEN(C)(((C) >= 7) && ((C) <= 26)) > > -#define GET_DRAM_REGION_SIZE_EVEN(C)((UINT64)1 << ((C) + 16)) > > -#define DRAM_REGION_SIZE_ODD(C) ((C) <= 4) > > -#define GET_DRAM_REGION_SIZE_ODD(C) ((UINT64)0x1800 << (C)) > > +/* Firmware related definition used for SMC calls */ > > +#define MV_SIP_DRAM_SIZE0x8210 > > Hmm... > This would end us up with having Marvell SMC calls spread across > multiple files. Could you insert an intermediate patch which breaks out > the ones from Silicon/Marvell/Library/ComPhyLib/ComPhySipSvc.h into a > separate (MarvellSmc.h?) include file? How about MvSmc.h ? I try to use 'Mv' prefix consistently, especially when adding new code. > > If you could further add _SMC_ID to the #defines (like in edk2 > ArmPkg/Include/IndustryStandard/ArmStdSmc.h), that would make me very > happy. (I'd be happy for you to drop _SIP, or keep it either side of > the addition, as per your preference - we don't seem to have precedent > here yet.) > How about below: #define MV_SMC_ID_COMPHY_POWER_ON 0x8201 #define MV_SMC_ID_COMPHY_POWER_OFF 0x8202 #define MV_SMC_ID_COMPHY_PLL_LOCK 0x8203 ? Thanks, Marcin > Regards, > > Leif > > > diff --git > > a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.c > > b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.c > > index 2a4f5ad..62e8467 100644 > > --- a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.c > > +++ b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.c > > @@ -33,11 +33,14 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
Re: [edk2] [platforms: PATCH 2/3] Marvell/Library: ArmadaSoCDescLib: Add North Bridge description
Hi Leif, pon., 21 sty 2019 o 12:32 Leif Lindholm napisał(a): > > On Mon, Jan 21, 2019 at 11:52:10AM +0100, Marcin Wojtas wrote: > > From: Grzegorz Jaszczyk > > > > For upcomming patch there is need to get AP806 base, provide required > > getter function for it. > > > > Contributed-under: TianoCore Contribution Agreement 1.1 > > Signed-off-by: Marcin Wojtas > > --- > > > > Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.h > > | 1 + > > Silicon/Marvell/Include/Library/ArmadaSoCDescLib.h > > | 10 ++ > > > > Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.c > > | 17 + > > 3 files changed, 28 insertions(+) > > > > diff --git > > a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.h > > > > b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.h > > index bfc8639..6caee6c 100644 > > --- > > a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.h > > +++ > > b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.h > > @@ -22,6 +22,7 @@ > > // Common macros > > // > > #define MV_SOC_CP_BASE(Cp) (0xF200 + ((Cp) * 0x200)) > > +#define MV_SOC_AP806_BASE0xF000 > > #define MV_SOC_AP806_COUNT 1 > > > > // > > diff --git a/Silicon/Marvell/Include/Library/ArmadaSoCDescLib.h > > b/Silicon/Marvell/Include/Library/ArmadaSoCDescLib.h > > index 26b075a..7aec9be 100644 > > --- a/Silicon/Marvell/Include/Library/ArmadaSoCDescLib.h > > +++ b/Silicon/Marvell/Include/Library/ArmadaSoCDescLib.h > > @@ -20,6 +20,16 @@ > > #include > > > > // > > +// North Bridge description > > +// > > +EFI_STATUS > > +EFIAPI > > +ArmadaSoCAp8xxBaseGet ( > > + IN OUT UINT64 *ApBase, > > + IN UINTNApIndex > > + ); > > + > > +// > > // ComPhy SoC description > > // > > typedef struct { > > diff --git > > a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.c > > > > b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.c > > index 5b72c20..089ac2d 100644 > > --- > > a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.c > > +++ > > b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.c > > @@ -30,6 +30,23 @@ > > > > As I said, I was going to get stricter about adding function > description comments - please add one here. (And clone it to > ArmadaSoCDescLib.h, for the obious place to go look for a template > when implementing a variant for a new platform.) Sure, I will add it. > > > EFI_STATUS > > EFIAPI > > +ArmadaSoCAp8xxBaseGet ( > > + IN OUT UINT64 *ApBase, > > + IN UINTNApIndex > > + ) > > +{ > > + if (ApIndex != 0) { > > This test should be using ARMADA7K8K_AP806_INDEX, and that definition > should be added to this patch. > Ok. Thanks, Marcin > > +DEBUG ((DEBUG_ERROR, "%a: Only one AP806 in A7K/A8K SoC\n", > > __FUNCTION__)); > > +return EFI_INVALID_PARAMETER; > > + } > > + > > + *ApBase = MV_SOC_AP806_BASE; > > + > > + return EFI_SUCCESS; > > +} > > + > > +EFI_STATUS > > +EFIAPI > > ArmadaSoCDescComPhyGet ( > >IN OUT MV_SOC_COMPHY_DESC **ComPhyDesc, > >IN OUT UINTN*DescCount > > -- > > 2.7.4 > > ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [platforms: PATCH 1/3] Marvell: Armada7k8k: Shift PEI stack base
On Mon, Jan 21, 2019 at 04:34:39PM +0100, Marcin Wojtas wrote: > Hi Leif, > > > pon., 21 sty 2019 o 12:26 Leif Lindholm napisał(a): > > > > On Mon, Jan 21, 2019 at 11:52:09AM +0100, Marcin Wojtas wrote: > > > Recent changes in the ARM-TF configure its runtime serices region > > > as protected, hence the hitherto PEI stack base address (0x41F) > > > violated it. > > > > > > In order to fix this, extend the region which is non-accessible > > > by the OS to cover both the ARM-TF (0x400 - 0x420) and OPTEE > > > (0x440 - 0x540) within a single area and set the PEI stack > > > > What is the single area? > > The single region is set to: > 0x400 - 0x540 > > PEI stack base is shifted right below the OPTEE region, i.e. to: > 0x43F > > Do you wish to add above to the commit message as well? Yes please. Best Regards, Leif > Best regards, > Marcin > > > > > > base address between both images. > > > > > > Contributed-under: TianoCore Contribution Agreement 1.1 > > > Signed-off-by: Marcin Wojtas > > > --- > > > Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc | 4 ++-- > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > diff --git a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc > > > b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc > > > index eafcd6e..c8c597f 100644 > > > --- a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc > > > +++ b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc > > > @@ -376,12 +376,12 @@ > > > > > >gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize|36 > > > > > > - gArmPlatformTokenSpaceGuid.PcdCPUCoresStackBase|0x41F > > > + gArmPlatformTokenSpaceGuid.PcdCPUCoresStackBase|0x43F > > >gArmPlatformTokenSpaceGuid.PcdCPUCorePrimaryStackSize|0x1 > > > > > ># Secure region reservation > > >gMarvellTokenSpaceGuid.PcdSecureRegionBase|0x400 > > > - gMarvellTokenSpaceGuid.PcdSecureRegionSize|0x020 > > > + gMarvellTokenSpaceGuid.PcdSecureRegionSize|0x140 > > > > > ># TRNG > > >gMarvellTokenSpaceGuid.PcdEip76TrngBaseAddress|0xF276 > > > -- > > > 2.7.4 > > > ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [platforms: PATCH 1/3] Marvell: Armada7k8k: Shift PEI stack base
Hi Leif, pon., 21 sty 2019 o 12:26 Leif Lindholm napisał(a): > > On Mon, Jan 21, 2019 at 11:52:09AM +0100, Marcin Wojtas wrote: > > Recent changes in the ARM-TF configure its runtime serices region > > as protected, hence the hitherto PEI stack base address (0x41F) > > violated it. > > > > In order to fix this, extend the region which is non-accessible > > by the OS to cover both the ARM-TF (0x400 - 0x420) and OPTEE > > (0x440 - 0x540) within a single area and set the PEI stack > > What is the single area? The single region is set to: 0x400 - 0x540 PEI stack base is shifted right below the OPTEE region, i.e. to: 0x43F Do you wish to add above to the commit message as well? Best regards, Marcin > > > base address between both images. > > > > Contributed-under: TianoCore Contribution Agreement 1.1 > > Signed-off-by: Marcin Wojtas > > --- > > Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc > > b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc > > index eafcd6e..c8c597f 100644 > > --- a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc > > +++ b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc > > @@ -376,12 +376,12 @@ > > > >gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize|36 > > > > - gArmPlatformTokenSpaceGuid.PcdCPUCoresStackBase|0x41F > > + gArmPlatformTokenSpaceGuid.PcdCPUCoresStackBase|0x43F > >gArmPlatformTokenSpaceGuid.PcdCPUCorePrimaryStackSize|0x1 > > > ># Secure region reservation > >gMarvellTokenSpaceGuid.PcdSecureRegionBase|0x400 > > - gMarvellTokenSpaceGuid.PcdSecureRegionSize|0x020 > > + gMarvellTokenSpaceGuid.PcdSecureRegionSize|0x140 > > > ># TRNG > >gMarvellTokenSpaceGuid.PcdEip76TrngBaseAddress|0xF276 > > -- > > 2.7.4 > > ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH] UefiCpuPkg/MtrrLib: Fix a bug that may wrongly set memory <1MB to UC
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1481 Today's MtrrLib contains a bug, for example: when the original cache setting is WB for [0xF_, 0xF_8000) and, a new request to set [0xF_, 0xF_4000) to WP, the cache setting for [0xF_4000, 0xF_8000) is reset to UC. The reason is when MtrrLibSetBelow1MBMemoryAttribute() is called the WorkingFixedSettings doesn't contain the actual MSR value stored in hardware, but when writing the fixed MTRRs, the code logic assumes WorkingFixedSettings contains the actual MSR value. The new fix is to change MtrrLibSetBelow1MBMemoryAttribute() to calculate the correct ClearMasks[] and OrMasks[], and use them directly when writing the fixed MTRRs. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Ray Ni Cc: Eric Dong Cc: Chasel Chiu --- UefiCpuPkg/Library/MtrrLib/MtrrLib.c | 59 +--- 1 file changed, 18 insertions(+), 41 deletions(-) diff --git a/UefiCpuPkg/Library/MtrrLib/MtrrLib.c b/UefiCpuPkg/Library/MtrrLib/MtrrLib.c index 086f7ad8f0..2cf7d092e8 100644 --- a/UefiCpuPkg/Library/MtrrLib/MtrrLib.c +++ b/UefiCpuPkg/Library/MtrrLib/MtrrLib.c @@ -5,7 +5,7 @@ Most of services in this library instance are suggested to be invoked by BSP only, except for MtrrSetAllMtrrs() which is used to sync BSP's MTRR setting to APs. - Copyright (c) 2008 - 2018, Intel Corporation. All rights reserved. + Copyright (c) 2008 - 2019, Intel Corporation. All rights reserved. This program and the accompanying materials are licensed and made available under the terms and conditions of the BSD License which accompanies this distribution. The full text of the license may be found at @@ -2099,8 +2099,8 @@ MtrrLibSetMemoryRanges ( Set the below-1MB memory attribute to fixed MTRR buffer. Modified flag array indicates which fixed MTRR is modified. - @param [in, out] FixedSettings Fixed MTRR buffer. - @param [out] Modified Flag array indicating which MTRR is modified. + @param [in, out] ClearMasksThe bits to clear in the fixed MTRR MSR. + @param [in, out] OrMasks The bits to set in the fixed MTRR MSR. @param [in] BaseAddress Base address. @param [in] LengthLength. @param [in] Type Memory type. @@ -2111,8 +2111,8 @@ MtrrLibSetMemoryRanges ( **/ RETURN_STATUS MtrrLibSetBelow1MBMemoryAttribute ( - IN OUT MTRR_FIXED_SETTINGS *FixedSettings, - OUT BOOLEAN*Modified, + IN OUT UINT64 *ClearMasks, + IN OUT UINT64 *OrMasks, IN PHYSICAL_ADDRESSBaseAddress, IN UINT64 Length, IN MTRR_MEMORY_CACHE_TYPE Type @@ -2122,36 +2122,17 @@ MtrrLibSetBelow1MBMemoryAttribute ( UINT32MsrIndex; UINT64ClearMask; UINT64OrMask; - UINT64ClearMasks[ARRAY_SIZE (mMtrrLibFixedMtrrTable)]; - UINT64OrMasks[ARRAY_SIZE (mMtrrLibFixedMtrrTable)]; - BOOLEAN LocalModified[ARRAY_SIZE (mMtrrLibFixedMtrrTable)]; ASSERT (BaseAddress < BASE_1MB); - SetMem (LocalModified, sizeof (LocalModified), FALSE); - - // - // (Value & ~0 | 0) still equals to (Value) - // - SetMem (ClearMasks, sizeof (ClearMasks), 0); - SetMem (OrMasks, sizeof (OrMasks), 0); - MsrIndex = (UINT32)-1; while ((BaseAddress < BASE_1MB) && (Length != 0)) { Status = MtrrLibProgramFixedMtrr (Type, , , , , ); if (RETURN_ERROR (Status)) { return Status; } -ClearMasks[MsrIndex]= ClearMask; -OrMasks[MsrIndex] = OrMask; -Modified[MsrIndex] = TRUE; -LocalModified[MsrIndex] = TRUE; - } - - for (MsrIndex = 0; MsrIndex < ARRAY_SIZE (mMtrrLibFixedMtrrTable); MsrIndex++) { -if (LocalModified[MsrIndex]) { - FixedSettings->Mtrr[MsrIndex] = (FixedSettings->Mtrr[MsrIndex] & ~ClearMasks[MsrIndex]) | OrMasks[MsrIndex]; -} +ClearMasks[MsrIndex] = ClearMasks[MsrIndex] | ClearMask; +OrMasks[MsrIndex]= (OrMasks[MsrIndex] & ~ClearMask) | OrMask; } return RETURN_SUCCESS; } @@ -2213,8 +2194,8 @@ MtrrSetMemoryAttributesInMtrrSettings ( MTRR_MEMORY_RANGE WorkingVariableMtrr[ARRAY_SIZE (MtrrSetting->Variables.Mtrr)]; BOOLEAN VariableSettingModified[ARRAY_SIZE (MtrrSetting->Variables.Mtrr)]; - BOOLEAN FixedSettingsModified[ARRAY_SIZE (mMtrrLibFixedMtrrTable)]; - MTRR_FIXED_SETTINGS WorkingFixedSettings; + UINT64ClearMasks[ARRAY_SIZE (mMtrrLibFixedMtrrTable)]; + UINT64OrMasks[ARRAY_SIZE (mMtrrLibFixedMtrrTable)]; MTRR_CONTEXT MtrrContext; BOOLEAN MtrrContextValid; @@ -2226,10 +2207,6 @@ MtrrSetMemoryAttributesInMtrrSettings ( // TRUE indicating the accordingly Variable setting needs modificaiton in OriginalVariableMtrr. // SetMem (VariableSettingModified,
Re: [edk2] [PATCH v2 00/11] StandaloneMmPkg: assorted fixes and improvements
On Fri, 18 Jan 2019 at 16:42, Yao, Jiewen wrote: > > Thanks to remind me. > I took a look at all patches. > > I skipped all ARM specific patches, and reviewed rest of them. > All my feedback is provided in the individual patch. > > Thank you > Yao Jiewen > > > -Original Message- > > From: Achin Gupta [mailto:achin.gu...@arm.com] > > Sent: Friday, January 18, 2019 1:27 AM > > To: Ard Biesheuvel ; edk2-devel@lists.01.org; > > Yao, Jiewen > > Cc: Supreeth Venkatesh ; Leif Lindholm > > ; Jagadeesh Ujja ; > > Thomas Abraham ; Sami Mujawar > > ; nd > > Subject: Re: [PATCH v2 00/11] StandaloneMmPkg: assorted fixes and > > improvements > > > > Hi Ard, > > > > For all the patches... > > > > Reviewed-by: Achin Gupta > > > > Jiewen. There are changes to the generic Standalone MM code in this series. > > Do you want to have a look as well? > > Thanks all I pushed most of this series 380148b685b7 StandaloneMmPkg: add HobLib implementation for MM_STANDALONE modules 66dde0c7516b StandaloneMmPkg: add MM_STANDALONE MemoryAllocationLib implementation 2cc186178bfd StandaloneMmPkg/StandaloneMmCoreHobLib: restrict to MM_CORE_STANDALONE c8102727edb0 StandaloneMmPkg/StandaloneMmCpu: fix typo Standlone -> Standalone 41915a19a772 StandaloneMmPkg/StandaloneMmCoreEntryPoint: use %a modifier for ASCII strings d2f438bf6a51 StandaloneMmPkg/StandaloneMmCoreEntryPoint: remove bogus ASSERT_EFI_ERROR()s 77746e70807d StandaloneMmPkg/StandaloneMmPeCoffExtraActionLib: ignore runtime attribute 877013d0a58f StandaloneMmPkg/Core/Dispatcher: don't copy dispatched image twice 4b28452d9863 StandaloneMmPkg/StandaloneMmCoreEntryPoint: permit the use of TE images The remaining two patches will be included in the next batch of StandaloneMmPkg work. ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH 0/4] Various Packages: add MM_STANDALONE support
On Mon, 21 Jan 2019 at 13:49, Gao, Liming wrote: > > Ard: > The patches created by you recently is to support the standalone MM > authenticated variable stack. This is likely a new feature. > > Could you help submit BZ for this new feature or reuse the existing BZ? > And, update > https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Release-Planning > to include it for edk2-stable201903 tag. > I will add a bugzilla entry for the remaining standalone MM work, but could someone please add StandaloneMmPkg to the bugzilla packages list? Thanks. ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH V2 10/15] ArmVirtXen: Use merged variable driver for emulated NV mode
Hi, On 21/01/2019 10:46, Zeng, Star wrote: > On 2019/1/18 2:59, Julien Grall wrote: > I saw the discussion at > https://lists.01.org/pipermail/edk2-devel/2019-January/035405.html. > Fortunately, > it has been fixed. > So I did rebase for the code. > Repo: g...@github.com:lzeng14/edk2.git > Branch: MergedVariableDriver_EmuNvMode_V3_rebased I was about to ask a branch as there were conflicts in the rebase. Thank you for providing the branch! > > If you can help have a quick test, that will be very helpful. :) With your series applied, EDK2 is crashing while the Linux EFI stub is running. See the log below. My knowledge of EDK2 is quite limited, so I am not entirely where to look at. I am happy to help debugging if you provide guidance. Press any key to continue... [Security] 3rd party image[0] can be loaded after EndOfDxe: MemoryMapped(0x2,0x67789000,0x68DF1200). InstallProtocolInterface: 5B1B31A1-9562-11D2-8E3F-00A0C969723B 7AB2B040 Loading driver at 0x00065783000 EntryPoint=0x00066878664 Loading driver at 0x00065783000 EntryPoint=0x00066878664 InstallProtocolInterface: BC62157E-3E33-4FEC-9920-2D3B36D750DF 7AA54B18 ProtectUefiImageCommon - 0x7AB2B040 - 0x65783000 - 0x02006000 SetUefiImageMemoryAttributes - 0x65783000 - 0x1000 (0x4008) SetUefiImageMemoryAttributes - 0x65784000 - 0x011CD000 (0x00020008) SetUefiImageMemoryAttributes - 0x66951000 - 0x00E38000 (0x4008) EFI stub: Booting Linux Kernel... EFI stub: Using DTB from configuration table EFI stub: Exiting boot services and installing virtual address map... XenBus: Set state to 5 XenBus: Set state to 5, done XenPvBlk: waiting backend state 5, current: 4 XenStore: Watch event 7B036698 XenBus: Set state to 6 XenBus: Set state to 6, done XenPvBlk: waiting backend state 6, current: 5 XenStore: Watch event 7B036698 XenBus: Set state to 1 XenBus: Set state to 1, done Xen GrantTable, removing 38003 Xen GrantTable, removing 38002 Xen GrantTable, removing 38001 Xen GrantTable, removing 38000 SetUefiImageMemoryAttributes - 0x7BE6 - 0x0004 (0x0008) SetUefiImageMemoryAttributes - 0x78AE - 0x0005 (0x0008) SetUefiImageMemoryAttributes - 0x78A9 - 0x0004 (0x0008) SetUefiImageMemoryAttributes - 0x789F - 0x0004 (0x0008) SetUefiImageMemoryAttributes - 0x7895 - 0x0004 (0x0008) SetUefiImageMemoryAttributes - 0x788B - 0x0004 (0x0008) Synchronous Exception at 0x7BE70698 PC 0x7BE70698 (0x7BE6+0x00010698) [ 0] RuntimeDxe.dll PC 0x78AFECB0 (0x78AE+0x0001ECB0) [ 1] VariableRuntimeDxe.dll PC 0x78AF0AC8 (0x78AE+0x00010AC8) [ 1] VariableRuntimeDxe.dll PC 0x7BE7094C (0x7BE6+0x0001094C) [ 2] RuntimeDxe.dll PC 0x6687E014 PC 0x6687C348 PC 0x66878680 PC 0x7F492BF4 (0x7F48C000+0x6BF4) [ 3] DxeCore.dll PC 0x783E6724 PC 0x783E6A38 PC 0x784BF888 PC 0x784605F8 PC 0x784602A4 PC 0x7845C608 PC 0x7845C838 PC 0x7845C914 PC 0x7845C974 PC 0x784DFB48 PC 0x786922D0 PC 0x7F492BF4 (0x7F48C000+0x6BF4) [ 3] DxeCore.dll PC 0x7875B27C PC 0x7877A2D0 PC 0x7F492BF4 (0x7F48C000+0x6BF4) [ 3] DxeCore.dll PC 0x7BD1912C (0x7BD0A000+0xF12C) [ 4] BdsDxe.dll PC 0x7BD0BFBC (0x7BD0A000+0x1FBC) [ 4] BdsDxe.dll PC 0x7BD0D5C8 (0x7BD0A000+0x35C8) [ 4] BdsDxe.dll PC 0x7F48E564 (0x7F48C000+0x2564) [ 5] DxeCore.dll PC 0x7F48D598 (0x7F48C000+0x1598) [ 5] DxeCore.dll PC 0x7F48D024 (0x7F48C000+0x1024) [ 5] DxeCore.dll PC 0x400895C8 PC 0x400897AC PC 0x40082B4C PC 0x40082C24 [ 0] /home/julien/works/edk2/Build/ArmVirtXen-AARCH64/DEBUG_GCC49/AARCH64/MdeModulePkg/Core/RuntimeDxe/RuntimeDxe/DEBUG/RuntimeDxe.dll [ 1] /home/julien/works/edk2/Build/ArmVirtXen-AARCH64/DEBUG_GCC49/AARCH64/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe/DEBUG/VariableRuntimeDxe.dll [ 2] /home/julien/works/edk2/Build/ArmVirtXen-AARCH64/DEBUG_GCC49/AARCH64/MdeModulePkg/Core/RuntimeDxe/RuntimeDxe/DEBUG/RuntimeDxe.dll [ 3] /home/julien/works/edk2/Build/ArmVirtXen-AARCH64/DEBUG_GCC49/AARCH64/MdeModulePkg/Core/Dxe/DxeMain/DEBUG/DxeCore.dll [ 4] /home/julien/works/edk2/Build/ArmVirtXen-AARCH64/DEBUG_GCC49/AARCH64/MdeModulePkg/Universal/BdsDxe/BdsDxe/DEBUG/BdsDxe.dll [ 5] /home/julien/works/edk2/Build/ArmVirtXen-AARCH64/DEBUG_GCC49/AARCH64/MdeModulePkg/Core/Dxe/DxeMain/DEBUG/DxeCore.dll X0 0x0018 X1 0x0018 X2 0x7BE7066C X3 0x7BFDD998 X4 0x7BE707D8 X5 0x0004 X6 0x X7 0x7B036E18 X8
Re: [edk2] [PATCH] ArmPkg: delete unused ArmTrustZoneSmc.h
On 1/21/19 2:07 PM, Ard Biesheuvel wrote: > On Mon, 21 Jan 2019 at 14:07, Leif Lindholm wrote: >> >> ArmPkg/Include/IndustryStandard/ArmTrustZoneSmc.h contains definitions >> contradicting the SMC Calling Convention (ARM DEN0028B). >> >> It also has no users in public trees. So delete before it can cause >> damage. >> >> Contributed-under: TianoCore Contribution Agreement 1.1 >> Signed-off-by: Leif Lindholm > > Reviewed-by: Ard Biesheuvel Reviewed-by: Philippe Mathieu-Daudé > >> --- >> ArmPkg/Include/IndustryStandard/ArmTrustZoneSmc.h | 161 >> -- >> 1 file changed, 161 deletions(-) >> delete mode 100644 ArmPkg/Include/IndustryStandard/ArmTrustZoneSmc.h >> >> diff --git a/ArmPkg/Include/IndustryStandard/ArmTrustZoneSmc.h >> b/ArmPkg/Include/IndustryStandard/ArmTrustZoneSmc.h >> deleted file mode 100644 >> index 71b4327ebf..00 >> --- a/ArmPkg/Include/IndustryStandard/ArmTrustZoneSmc.h >> +++ /dev/null >> @@ -1,161 +0,0 @@ >> -/** @file >> -* >> -* Copyright (c) 2012-2013, ARM Limited. All rights reserved. >> -* >> -* This program and the accompanying materials >> -* are licensed and made available under the terms and conditions of the >> BSD License >> -* which accompanies this distribution. The full text of the license may >> be found at >> -* http://opensource.org/licenses/bsd-license.php >> -* >> -* THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, >> -* WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR >> IMPLIED. >> -* >> -**/ >> - >> -#ifndef __ARM_TRUSTZONE_SMC_H__ >> -#define __ARM_TRUSTZONE_SMC_H__ >> - >> -#define ARM_TRUSTZONE_UID_4LETTERID 0x1 >> -#define ARM_TRUSTZONE_UID_MD5 0x2 >> - >> -#define ARM_TRUSTZONE_ARM_UID 0x40524d48 // "ARMH" >> - >> -#define IS_ARM_TRUSTZONE_SUPPORTED_SMC(Rx,Region) (((UINTN)(Rx) >= >> (UINTN)ARM_TRUSTZONE_##Region##_SMC_ID_START) && ((UINTN)(Rx) <= >> (UINTN)ARM_TRUSTZONE_##Region##_SMC_ID_END)) >> - >> -#define IS_ARM_TRUSTZONE_DEPRECIATED_SMC(Rx)((UINTN)(Rx) <= >> (UINTN)ARM_TRUSTZONE_DEPRECIATED_SMC_ID_END) >> -#define IS_ARM_TRUSTZONE_TRUSTED_OS_SMC(Rx) >> IS_ARM_TRUSTZONE_SUPPORTED_SMC(Rx,TRUSTED_OS) >> -#define IS_ARM_TRUSTZONE_ARM_FAST_SMC(Rx) >> IS_ARM_TRUSTZONE_SUPPORTED_SMC(Rx,ARM_FAST) >> -#define IS_ARM_TRUSTZONE_SIP_FAST_SMC(Rx) >> IS_ARM_TRUSTZONE_SUPPORTED_SMC(Rx,SIP_FAST) >> -#define IS_ARM_TRUSTZONE_ODM_FAST_SMC(Rx) >> IS_ARM_TRUSTZONE_SUPPORTED_SMC(Rx,ODM_FAST) >> -#define IS_ARM_TRUSTZONE_OEM_FAST_SMC(Rx) >> IS_ARM_TRUSTZONE_SUPPORTED_SMC(Rx,OEM_FAST) >> -#define IS_ARM_TRUSTZONE_TRUSTED_USER_FAST_SMC(Rx) >> IS_ARM_TRUSTZONE_SUPPORTED_SMC(Rx,TRUSTED_USER_FAST) >> -#define IS_ARM_TRUSTZONE_TRUSTED_OS_FAST_SMC(Rx) >> IS_ARM_TRUSTZONE_SUPPORTED_SMC(Rx,TRUSTED_OS_FAST) >> - >> -#define IS_ARM_TRUSTZONE_SUPPORTED_SMC_ID_PRESENCE(Rx,Region) ((Rx) == >> ARM_TRUSTZONE_##Region##_SMC_ID_PRESENCE) >> -#define IS_ARM_TRUSTZONE_SUPPORTED_SMC_ID_UID(Rx,Region) (((Rx) == >> ARM_TRUSTZONE_##Region##_SMC_ID_UID) || \ >> - ((Rx) == >> ARM_TRUSTZONE_##Region##_SMC_ID_UID+1) || \ >> - ((Rx) == >> ARM_TRUSTZONE_##Region##_SMC_ID_UID+2) || \ >> - ((Rx) == >> ARM_TRUSTZONE_##Region##_SMC_ID_UID+3) || \ >> - ((Rx) == >> ARM_TRUSTZONE_##Region##_SMC_ID_UID+4)) >> -#define IS_ARM_TRUSTZONE_SUPPORTED_SMC_ID_REVISION(Rx,Region) (((Rx) == >> ARM_TRUSTZONE_##Region##_SMC_ID_REVISION) || \ >> - ((Rx) == >> ARM_TRUSTZONE_##Region##_SMC_ID_REVISION+1)) >> -#define IS_ARM_TRUSTZONE_SUPPORTED_SMC_ID_RPC(Rx,Region) (((Rx) >= >> ARM_TRUSTZONE_##Region##_SMC_ID_RPC_START) && \ >> - ((Rx) <= >> ARM_TRUSTZONE_##Region##_SMC_ID_RPC_END)) >> - >> -#define ARM_TRUSTZONE_SUPPORTED_SMC_ID_UID_INDEX(Rx,Region) ((Rx) - >> ARM_TRUSTZONE_##Region##_SMC_ID_UID) >> -#define ARM_TRUSTZONE_SUPPORTED_SMC_ID_REVISION_INDEX(Rx,Region) ((Rx) - >> ARM_TRUSTZONE_##Region##_SMC_ID_REVISION) >> -#define ARM_TRUSTZONE_SUPPORTED_SMC_ID_RPC_INDEX(Rx,Region) ((Rx) - >> ARM_TRUSTZONE_##Region##_SMC_ID_RPC_START) >> - >> -#define ARM_TRUSTZONE_TRUSTED_OS_SMC_ID_RPC_INDEX(Rx) >> ARM_TRUSTZONE_SUPPORTED_SMC_ID_RPC_INDEX(Rx,TRUSTED_OS) >> - >> -#define IS_ARM_TRUSTZONE_ARM_FAST_SMC_ID_PRESENCE(Rx) >> IS_ARM_TRUSTZONE_SUPPORTED_SMC_ID_PRESENCE(Rx,ARM_FAST) >> -#define IS_ARM_TRUSTZONE_ARM_FAST_SMC_ID_UID(Rx) >> IS_ARM_TRUSTZONE_SUPPORTED_SMC_ID_UID(Rx,ARM_FAST) >> -#define IS_ARM_TRUSTZONE_ARM_FAST_SMC_ID_REVISION(Rx) >>
Re: [edk2] [PATCH v2 1/3] MdeModulePkg/Dhcp4Dxe: Remove unnecessary NULL pointer check.
Hi, On 1/21/19 1:53 PM, Gao, Liming wrote: > Thanks Ard and Laszlo. For the minor change in single patch, the patch may be > sent separately with the clear subject. Or, the patch set can be sent again. Since it is hard to follow technical discussion when top-posted (see https://www.caliburn.nl/topposting.html) without scrolling and sometime loosing context, can we gently suggest bottom-posting in edk2-devel etiquette? (No offence, this is a humble suggestion from a not very active reviewer to a highly active contributor, but this might ease the on-list review workflow). >> -Original Message- >> From: Laszlo Ersek [mailto:ler...@redhat.com] >> Sent: Saturday, January 19, 2019 9:17 AM >> To: Ard Biesheuvel ; Gao, Liming >> >> Cc: Wu, Jiaxin ; Fu, Siyuan ; Wu, >> Hao A ; edk2-devel@lists.01.org; Ye, >> Ting >> Subject: Re: [PATCH v2 1/3] MdeModulePkg/Dhcp4Dxe: Remove unnecessary NULL >> pointer check. >> >> On 01/18/19 12:09, Ard Biesheuvel wrote: >>> On Fri, 18 Jan 2019 at 06:38, Gao, Liming wrote: This is my idea to avoid the duplicated mail. I also include Ard and Laszlo to collect the feedback on how to handle the partial update in >> the patchset. >>> >>> Laszlo may disagree with me, but I think that it is not always >>> necessary to resend the entire series when only a single patch >>> changes. It does depend on the situation, though: if it is a trivial >>> patch in a more complicated series then it might make little sense. In >>> other case, just resending the whole thing is probably better. >> >> I think resending one patch can be acceptable, but the subject line >> (patch nr) and the threading have to be correct. Also, I don't think >> this approach scales beyond exactly one patch-update; it's easy to lose >> track of version numbers etc. So "use sparingly" I guess? :) For a 3 patches series, I wouldn't worry resending the whole series... The 'git backport-diff' tool is very powerful to resume differencies between 2 series, in particular when the project evolved between versions of a series (simplest example: a rebase): https://github.com/codyprime/git-scripts/blob/master/git-backport-diff#L27 Sadly I can't find a distribution handy package that provides it, so it has to be installed manually. Regards, Phil. > -Original Message- > From: Wu, Jiaxin > Sent: Friday, January 18, 2019 1:32 PM > To: Fu, Siyuan ; Wu, Hao A ; > edk2-devel@lists.01.org > Cc: Ye, Ting ; Gao, Liming > Subject: RE: [PATCH v2 1/3] MdeModulePkg/Dhcp4Dxe: Remove unnecessary > NULL pointer check. > > Just confirmed with Liming, we don't need to seed the full series patches > if only one is updated. > > Thanks, > jiaxin > >> -Original Message- >> From: Fu, Siyuan >> Sent: Friday, January 18, 2019 1:29 PM >> To: Wu, Hao A ; Wu, Jiaxin ; >> edk2-devel@lists.01.org >> Cc: Ye, Ting ; Gao, Liming >> Subject: RE: [PATCH v2 1/3] MdeModulePkg/Dhcp4Dxe: Remove >> unnecessary NULL pointer check. >> >> Hi, Jiaxin >> >> Yes the full patch series is needed for a v2 version. >> >> And also, why you removed the "(Instance->Token != NULL)" check in the if >> condition? >> >> BestRegards >> Fu Siyuan >> >> >>> -Original Message- >>> From: Wu, Hao A >>> Sent: Friday, January 18, 2019 1:22 PM >>> To: Wu, Jiaxin ; edk2-devel@lists.01.org >>> Cc: Ye, Ting ; Fu, Siyuan ; Gao, >>> Liming >>> Subject: RE: [PATCH v2 1/3] MdeModulePkg/Dhcp4Dxe: Remove >> unnecessary NULL >>> pointer check. >>> >>> Hi Jiaxin, >>> >>> A comment that is not related with the content of the patch itself: >>> Please help to send the full patch series when a new version is needed. >>> >>> Best Regards, >>> Hao Wu >>> -Original Message- From: Wu, Jiaxin Sent: Friday, January 18, 2019 1:16 PM To: edk2-devel@lists.01.org Cc: Ye, Ting; Fu, Siyuan; Wu, Hao A; Gao, Liming; Wu, Jiaxin Subject: [PATCH v2 1/3] MdeModulePkg/Dhcp4Dxe: Remove >> unnecessary NULL pointer check. v2: The DHCP Instance might be destroyed in PxeDhcpDone. So, we need safe-delete. REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1469 Since the value of Instance is retrieved from the list Entry, it can't be the NULL pointer, so just remove the unnecessary check. Cc: Ye Ting Cc: Fu Siyuan Cc: Wu Hao A Cc: Gao Liming Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Wu Jiaxin --- MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Io.c | 11 - >> -- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git
Re: [edk2] [PATCH] ArmPkg: delete unused ArmTrustZoneSmc.h
On Mon, 21 Jan 2019 at 14:07, Leif Lindholm wrote: > > ArmPkg/Include/IndustryStandard/ArmTrustZoneSmc.h contains definitions > contradicting the SMC Calling Convention (ARM DEN0028B). > > It also has no users in public trees. So delete before it can cause > damage. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Leif Lindholm Reviewed-by: Ard Biesheuvel > --- > ArmPkg/Include/IndustryStandard/ArmTrustZoneSmc.h | 161 > -- > 1 file changed, 161 deletions(-) > delete mode 100644 ArmPkg/Include/IndustryStandard/ArmTrustZoneSmc.h > > diff --git a/ArmPkg/Include/IndustryStandard/ArmTrustZoneSmc.h > b/ArmPkg/Include/IndustryStandard/ArmTrustZoneSmc.h > deleted file mode 100644 > index 71b4327ebf..00 > --- a/ArmPkg/Include/IndustryStandard/ArmTrustZoneSmc.h > +++ /dev/null > @@ -1,161 +0,0 @@ > -/** @file > -* > -* Copyright (c) 2012-2013, ARM Limited. All rights reserved. > -* > -* This program and the accompanying materials > -* are licensed and made available under the terms and conditions of the BSD > License > -* which accompanies this distribution. The full text of the license may be > found at > -* http://opensource.org/licenses/bsd-license.php > -* > -* THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, > -* WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR > IMPLIED. > -* > -**/ > - > -#ifndef __ARM_TRUSTZONE_SMC_H__ > -#define __ARM_TRUSTZONE_SMC_H__ > - > -#define ARM_TRUSTZONE_UID_4LETTERID 0x1 > -#define ARM_TRUSTZONE_UID_MD5 0x2 > - > -#define ARM_TRUSTZONE_ARM_UID 0x40524d48 // "ARMH" > - > -#define IS_ARM_TRUSTZONE_SUPPORTED_SMC(Rx,Region) (((UINTN)(Rx) >= > (UINTN)ARM_TRUSTZONE_##Region##_SMC_ID_START) && ((UINTN)(Rx) <= > (UINTN)ARM_TRUSTZONE_##Region##_SMC_ID_END)) > - > -#define IS_ARM_TRUSTZONE_DEPRECIATED_SMC(Rx)((UINTN)(Rx) <= > (UINTN)ARM_TRUSTZONE_DEPRECIATED_SMC_ID_END) > -#define IS_ARM_TRUSTZONE_TRUSTED_OS_SMC(Rx) > IS_ARM_TRUSTZONE_SUPPORTED_SMC(Rx,TRUSTED_OS) > -#define IS_ARM_TRUSTZONE_ARM_FAST_SMC(Rx) > IS_ARM_TRUSTZONE_SUPPORTED_SMC(Rx,ARM_FAST) > -#define IS_ARM_TRUSTZONE_SIP_FAST_SMC(Rx) > IS_ARM_TRUSTZONE_SUPPORTED_SMC(Rx,SIP_FAST) > -#define IS_ARM_TRUSTZONE_ODM_FAST_SMC(Rx) > IS_ARM_TRUSTZONE_SUPPORTED_SMC(Rx,ODM_FAST) > -#define IS_ARM_TRUSTZONE_OEM_FAST_SMC(Rx) > IS_ARM_TRUSTZONE_SUPPORTED_SMC(Rx,OEM_FAST) > -#define IS_ARM_TRUSTZONE_TRUSTED_USER_FAST_SMC(Rx) > IS_ARM_TRUSTZONE_SUPPORTED_SMC(Rx,TRUSTED_USER_FAST) > -#define IS_ARM_TRUSTZONE_TRUSTED_OS_FAST_SMC(Rx) > IS_ARM_TRUSTZONE_SUPPORTED_SMC(Rx,TRUSTED_OS_FAST) > - > -#define IS_ARM_TRUSTZONE_SUPPORTED_SMC_ID_PRESENCE(Rx,Region) ((Rx) == > ARM_TRUSTZONE_##Region##_SMC_ID_PRESENCE) > -#define IS_ARM_TRUSTZONE_SUPPORTED_SMC_ID_UID(Rx,Region) (((Rx) == > ARM_TRUSTZONE_##Region##_SMC_ID_UID) || \ > - ((Rx) == > ARM_TRUSTZONE_##Region##_SMC_ID_UID+1) || \ > - ((Rx) == > ARM_TRUSTZONE_##Region##_SMC_ID_UID+2) || \ > - ((Rx) == > ARM_TRUSTZONE_##Region##_SMC_ID_UID+3) || \ > - ((Rx) == > ARM_TRUSTZONE_##Region##_SMC_ID_UID+4)) > -#define IS_ARM_TRUSTZONE_SUPPORTED_SMC_ID_REVISION(Rx,Region) (((Rx) == > ARM_TRUSTZONE_##Region##_SMC_ID_REVISION) || \ > - ((Rx) == > ARM_TRUSTZONE_##Region##_SMC_ID_REVISION+1)) > -#define IS_ARM_TRUSTZONE_SUPPORTED_SMC_ID_RPC(Rx,Region) (((Rx) >= > ARM_TRUSTZONE_##Region##_SMC_ID_RPC_START) && \ > - ((Rx) <= > ARM_TRUSTZONE_##Region##_SMC_ID_RPC_END)) > - > -#define ARM_TRUSTZONE_SUPPORTED_SMC_ID_UID_INDEX(Rx,Region) ((Rx) - > ARM_TRUSTZONE_##Region##_SMC_ID_UID) > -#define ARM_TRUSTZONE_SUPPORTED_SMC_ID_REVISION_INDEX(Rx,Region) ((Rx) - > ARM_TRUSTZONE_##Region##_SMC_ID_REVISION) > -#define ARM_TRUSTZONE_SUPPORTED_SMC_ID_RPC_INDEX(Rx,Region) ((Rx) - > ARM_TRUSTZONE_##Region##_SMC_ID_RPC_START) > - > -#define ARM_TRUSTZONE_TRUSTED_OS_SMC_ID_RPC_INDEX(Rx) > ARM_TRUSTZONE_SUPPORTED_SMC_ID_RPC_INDEX(Rx,TRUSTED_OS) > - > -#define IS_ARM_TRUSTZONE_ARM_FAST_SMC_ID_PRESENCE(Rx) > IS_ARM_TRUSTZONE_SUPPORTED_SMC_ID_PRESENCE(Rx,ARM_FAST) > -#define IS_ARM_TRUSTZONE_ARM_FAST_SMC_ID_UID(Rx) > IS_ARM_TRUSTZONE_SUPPORTED_SMC_ID_UID(Rx,ARM_FAST) > -#define IS_ARM_TRUSTZONE_ARM_FAST_SMC_ID_REVISION(Rx) > IS_ARM_TRUSTZONE_SUPPORTED_SMC_ID_REVISION(Rx,ARM_FAST) > -#define IS_ARM_TRUSTZONE_ARM_FAST_SMC_ID_RPC(Rx) > IS_ARM_TRUSTZONE_SUPPORTED_SMC_ID_RPC(Rx,ARM_FAST) > -#define ARM_TRUSTZONE_ARM_FAST_SMC_ID_UID_INDEX(Rx)
[edk2] [PATCH] ArmPkg: delete unused ArmTrustZoneSmc.h
ArmPkg/Include/IndustryStandard/ArmTrustZoneSmc.h contains definitions contradicting the SMC Calling Convention (ARM DEN0028B). It also has no users in public trees. So delete before it can cause damage. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Leif Lindholm --- ArmPkg/Include/IndustryStandard/ArmTrustZoneSmc.h | 161 -- 1 file changed, 161 deletions(-) delete mode 100644 ArmPkg/Include/IndustryStandard/ArmTrustZoneSmc.h diff --git a/ArmPkg/Include/IndustryStandard/ArmTrustZoneSmc.h b/ArmPkg/Include/IndustryStandard/ArmTrustZoneSmc.h deleted file mode 100644 index 71b4327ebf..00 --- a/ArmPkg/Include/IndustryStandard/ArmTrustZoneSmc.h +++ /dev/null @@ -1,161 +0,0 @@ -/** @file -* -* Copyright (c) 2012-2013, ARM Limited. All rights reserved. -* -* This program and the accompanying materials -* are licensed and made available under the terms and conditions of the BSD License -* which accompanies this distribution. The full text of the license may be found at -* http://opensource.org/licenses/bsd-license.php -* -* THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, -* WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. -* -**/ - -#ifndef __ARM_TRUSTZONE_SMC_H__ -#define __ARM_TRUSTZONE_SMC_H__ - -#define ARM_TRUSTZONE_UID_4LETTERID 0x1 -#define ARM_TRUSTZONE_UID_MD5 0x2 - -#define ARM_TRUSTZONE_ARM_UID 0x40524d48 // "ARMH" - -#define IS_ARM_TRUSTZONE_SUPPORTED_SMC(Rx,Region) (((UINTN)(Rx) >= (UINTN)ARM_TRUSTZONE_##Region##_SMC_ID_START) && ((UINTN)(Rx) <= (UINTN)ARM_TRUSTZONE_##Region##_SMC_ID_END)) - -#define IS_ARM_TRUSTZONE_DEPRECIATED_SMC(Rx)((UINTN)(Rx) <= (UINTN)ARM_TRUSTZONE_DEPRECIATED_SMC_ID_END) -#define IS_ARM_TRUSTZONE_TRUSTED_OS_SMC(Rx) IS_ARM_TRUSTZONE_SUPPORTED_SMC(Rx,TRUSTED_OS) -#define IS_ARM_TRUSTZONE_ARM_FAST_SMC(Rx) IS_ARM_TRUSTZONE_SUPPORTED_SMC(Rx,ARM_FAST) -#define IS_ARM_TRUSTZONE_SIP_FAST_SMC(Rx) IS_ARM_TRUSTZONE_SUPPORTED_SMC(Rx,SIP_FAST) -#define IS_ARM_TRUSTZONE_ODM_FAST_SMC(Rx) IS_ARM_TRUSTZONE_SUPPORTED_SMC(Rx,ODM_FAST) -#define IS_ARM_TRUSTZONE_OEM_FAST_SMC(Rx) IS_ARM_TRUSTZONE_SUPPORTED_SMC(Rx,OEM_FAST) -#define IS_ARM_TRUSTZONE_TRUSTED_USER_FAST_SMC(Rx) IS_ARM_TRUSTZONE_SUPPORTED_SMC(Rx,TRUSTED_USER_FAST) -#define IS_ARM_TRUSTZONE_TRUSTED_OS_FAST_SMC(Rx) IS_ARM_TRUSTZONE_SUPPORTED_SMC(Rx,TRUSTED_OS_FAST) - -#define IS_ARM_TRUSTZONE_SUPPORTED_SMC_ID_PRESENCE(Rx,Region) ((Rx) == ARM_TRUSTZONE_##Region##_SMC_ID_PRESENCE) -#define IS_ARM_TRUSTZONE_SUPPORTED_SMC_ID_UID(Rx,Region) (((Rx) == ARM_TRUSTZONE_##Region##_SMC_ID_UID) || \ - ((Rx) == ARM_TRUSTZONE_##Region##_SMC_ID_UID+1) || \ - ((Rx) == ARM_TRUSTZONE_##Region##_SMC_ID_UID+2) || \ - ((Rx) == ARM_TRUSTZONE_##Region##_SMC_ID_UID+3) || \ - ((Rx) == ARM_TRUSTZONE_##Region##_SMC_ID_UID+4)) -#define IS_ARM_TRUSTZONE_SUPPORTED_SMC_ID_REVISION(Rx,Region) (((Rx) == ARM_TRUSTZONE_##Region##_SMC_ID_REVISION) || \ - ((Rx) == ARM_TRUSTZONE_##Region##_SMC_ID_REVISION+1)) -#define IS_ARM_TRUSTZONE_SUPPORTED_SMC_ID_RPC(Rx,Region) (((Rx) >= ARM_TRUSTZONE_##Region##_SMC_ID_RPC_START) && \ - ((Rx) <= ARM_TRUSTZONE_##Region##_SMC_ID_RPC_END)) - -#define ARM_TRUSTZONE_SUPPORTED_SMC_ID_UID_INDEX(Rx,Region) ((Rx) - ARM_TRUSTZONE_##Region##_SMC_ID_UID) -#define ARM_TRUSTZONE_SUPPORTED_SMC_ID_REVISION_INDEX(Rx,Region) ((Rx) - ARM_TRUSTZONE_##Region##_SMC_ID_REVISION) -#define ARM_TRUSTZONE_SUPPORTED_SMC_ID_RPC_INDEX(Rx,Region) ((Rx) - ARM_TRUSTZONE_##Region##_SMC_ID_RPC_START) - -#define ARM_TRUSTZONE_TRUSTED_OS_SMC_ID_RPC_INDEX(Rx) ARM_TRUSTZONE_SUPPORTED_SMC_ID_RPC_INDEX(Rx,TRUSTED_OS) - -#define IS_ARM_TRUSTZONE_ARM_FAST_SMC_ID_PRESENCE(Rx) IS_ARM_TRUSTZONE_SUPPORTED_SMC_ID_PRESENCE(Rx,ARM_FAST) -#define IS_ARM_TRUSTZONE_ARM_FAST_SMC_ID_UID(Rx) IS_ARM_TRUSTZONE_SUPPORTED_SMC_ID_UID(Rx,ARM_FAST) -#define IS_ARM_TRUSTZONE_ARM_FAST_SMC_ID_REVISION(Rx) IS_ARM_TRUSTZONE_SUPPORTED_SMC_ID_REVISION(Rx,ARM_FAST) -#define IS_ARM_TRUSTZONE_ARM_FAST_SMC_ID_RPC(Rx) IS_ARM_TRUSTZONE_SUPPORTED_SMC_ID_RPC(Rx,ARM_FAST) -#define ARM_TRUSTZONE_ARM_FAST_SMC_ID_UID_INDEX(Rx) ARM_TRUSTZONE_SUPPORTED_SMC_ID_UID_INDEX(Rx,ARM_FAST) -#define ARM_TRUSTZONE_ARM_FAST_SMC_ID_REVISION_INDEX(Rx) ARM_TRUSTZONE_SUPPORTED_SMC_ID_REVISION_INDEX(Rx,ARM_FAST) -#define ARM_TRUSTZONE_ARM_FAST_SMC_ID_RPC_INDEX(Rx) ARM_TRUSTZONE_SUPPORTED_SMC_ID_RPC_INDEX(Rx,ARM_FAST) -
Re: [edk2] [PATCH v2 1/3] MdeModulePkg/Dhcp4Dxe: Remove unnecessary NULL pointer check.
Thanks Ard and Laszlo. For the minor change in single patch, the patch may be sent separately with the clear subject. Or, the patch set can be sent again. Thanks Liming > -Original Message- > From: Laszlo Ersek [mailto:ler...@redhat.com] > Sent: Saturday, January 19, 2019 9:17 AM > To: Ard Biesheuvel ; Gao, Liming > > Cc: Wu, Jiaxin ; Fu, Siyuan ; Wu, > Hao A ; edk2-devel@lists.01.org; Ye, > Ting > Subject: Re: [PATCH v2 1/3] MdeModulePkg/Dhcp4Dxe: Remove unnecessary NULL > pointer check. > > On 01/18/19 12:09, Ard Biesheuvel wrote: > > On Fri, 18 Jan 2019 at 06:38, Gao, Liming wrote: > >> > >> This is my idea to avoid the duplicated mail. I also include Ard and > >> Laszlo to collect the feedback on how to handle the partial update in > the patchset. > >> > > > > Laszlo may disagree with me, but I think that it is not always > > necessary to resend the entire series when only a single patch > > changes. It does depend on the situation, though: if it is a trivial > > patch in a more complicated series then it might make little sense. In > > other case, just resending the whole thing is probably better. > > I think resending one patch can be acceptable, but the subject line > (patch nr) and the threading have to be correct. Also, I don't think > this approach scales beyond exactly one patch-update; it's easy to lose > track of version numbers etc. So "use sparingly" I guess? :) > > Thanks > Laszlo > > > > > > >>> -Original Message- > >>> From: Wu, Jiaxin > >>> Sent: Friday, January 18, 2019 1:32 PM > >>> To: Fu, Siyuan ; Wu, Hao A ; > >>> edk2-devel@lists.01.org > >>> Cc: Ye, Ting ; Gao, Liming > >>> Subject: RE: [PATCH v2 1/3] MdeModulePkg/Dhcp4Dxe: Remove unnecessary > >>> NULL pointer check. > >>> > >>> Just confirmed with Liming, we don't need to seed the full series patches > >>> if only one is updated. > >>> > >>> Thanks, > >>> jiaxin > >>> > -Original Message- > From: Fu, Siyuan > Sent: Friday, January 18, 2019 1:29 PM > To: Wu, Hao A ; Wu, Jiaxin ; > edk2-devel@lists.01.org > Cc: Ye, Ting ; Gao, Liming > Subject: RE: [PATCH v2 1/3] MdeModulePkg/Dhcp4Dxe: Remove > unnecessary NULL pointer check. > > Hi, Jiaxin > > Yes the full patch series is needed for a v2 version. > > And also, why you removed the "(Instance->Token != NULL)" check in the if > condition? > > BestRegards > Fu Siyuan > > > > -Original Message- > > From: Wu, Hao A > > Sent: Friday, January 18, 2019 1:22 PM > > To: Wu, Jiaxin ; edk2-devel@lists.01.org > > Cc: Ye, Ting ; Fu, Siyuan ; Gao, > > Liming > > Subject: RE: [PATCH v2 1/3] MdeModulePkg/Dhcp4Dxe: Remove > unnecessary NULL > > pointer check. > > > > Hi Jiaxin, > > > > A comment that is not related with the content of the patch itself: > > Please help to send the full patch series when a new version is needed. > > > > Best Regards, > > Hao Wu > > > >> -Original Message- > >> From: Wu, Jiaxin > >> Sent: Friday, January 18, 2019 1:16 PM > >> To: edk2-devel@lists.01.org > >> Cc: Ye, Ting; Fu, Siyuan; Wu, Hao A; Gao, Liming; Wu, Jiaxin > >> Subject: [PATCH v2 1/3] MdeModulePkg/Dhcp4Dxe: Remove > unnecessary > >> NULL pointer check. > >> > >> v2: The DHCP Instance might be destroyed in PxeDhcpDone. So, > >> we need safe-delete. > >> > >> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1469 > >> > >> Since the value of Instance is retrieved from the list Entry, > >> it can't be the NULL pointer, so just remove the unnecessary > >> check. > >> > >> Cc: Ye Ting > >> Cc: Fu Siyuan > >> Cc: Wu Hao A > >> Cc: Gao Liming > >> Contributed-under: TianoCore Contribution Agreement 1.1 > >> Signed-off-by: Wu Jiaxin > >> --- > >> MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Io.c | 11 - > -- > >> 1 file changed, 4 insertions(+), 7 deletions(-) > >> > >> diff --git a/MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Io.c > >> b/MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Io.c > >> index 98a22a77b4..780f8b4224 100644 > >> --- a/MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Io.c > >> +++ b/MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Io.c > >> @@ -1,9 +1,9 @@ > >> /** @file > >>EFI DHCP protocol implementation. > >> > >> -Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved. > >> +Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved. > >> This program and the accompanying materials > >> are licensed and made available under the terms and conditions of the > BSD > >> License > >> which accompanies this distribution. The full text of the license > >> may be > >> found at > >> http://opensource.org/licenses/bsd-license.php > >> >
Re: [edk2] [PATCH 0/4] Various Packages: add MM_STANDALONE support
Ard: The patches created by you recently is to support the standalone MM authenticated variable stack. This is likely a new feature. Could you help submit BZ for this new feature or reuse the existing BZ? And, update https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Release-Planning to include it for edk2-stable201903 tag. Thanks Liming > -Original Message- > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ard > Biesheuvel > Sent: Monday, January 21, 2019 8:44 PM > To: edk2-devel@lists.01.org > Cc: Ye, Ting ; Gao, Liming ; Wu, Hao > A ; Yao, Jiewen > ; Zeng, Star ; Kinney, Michael D > ; Zhang, Chao B > > Subject: Re: [edk2] [PATCH 0/4] Various Packages: add MM_STANDALONE support > > On Wed, 16 Jan 2019 at 22:22, Ard Biesheuvel > wrote: > > > > Add MM_STANDALONE to the list of permitted module types of various > > libraries that are required to build the standalone MM authenticated > > variable stack. > > > > In some cases, this requires the MODULE_TYPE to be modified to BASE, > > given that the constructor prototype is different between DXE/UEFI > > and MM_STANDALONE drivers. > > > > Cc: Michael D Kinney > > Cc: Liming Gao > > Cc: Ting Ye > > Cc: Gang Wei > > Cc: Jian Wang > > Cc: Chao Zhang > > Cc: Jiewen Yao > > Cc: Hao Wu > > Cc: Star Zeng > > Cc: Achin Gupta > > Cc: Jagadeesh Ujja > > > > Ard Biesheuvel (4): > > CryptoPkg/SmmCryptLib: permit use by MM_STANDALONE modules > > SecurityPkg/PlatformSecureLibNull: permit use by MM_STANDALONE modules > > MdeModulePkg/VarCheckLib: permit use by MM_STANDALONE modules > > MdePkg/UefiDevicePathLib: permit use by MM_STANDALONE modules > > > > Series pushed as 5c5ca9f1fbf8..f634e32db5b6 > > Thanks all! > ___ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH 0/4] Various Packages: add MM_STANDALONE support
On Wed, 16 Jan 2019 at 22:22, Ard Biesheuvel wrote: > > Add MM_STANDALONE to the list of permitted module types of various > libraries that are required to build the standalone MM authenticated > variable stack. > > In some cases, this requires the MODULE_TYPE to be modified to BASE, > given that the constructor prototype is different between DXE/UEFI > and MM_STANDALONE drivers. > > Cc: Michael D Kinney > Cc: Liming Gao > Cc: Ting Ye > Cc: Gang Wei > Cc: Jian Wang > Cc: Chao Zhang > Cc: Jiewen Yao > Cc: Hao Wu > Cc: Star Zeng > Cc: Achin Gupta > Cc: Jagadeesh Ujja > > Ard Biesheuvel (4): > CryptoPkg/SmmCryptLib: permit use by MM_STANDALONE modules > SecurityPkg/PlatformSecureLibNull: permit use by MM_STANDALONE modules > MdeModulePkg/VarCheckLib: permit use by MM_STANDALONE modules > MdePkg/UefiDevicePathLib: permit use by MM_STANDALONE modules > Series pushed as 5c5ca9f1fbf8..f634e32db5b6 Thanks all! ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH 1/4] CryptoPkg/SmmCryptLib: permit use by MM_STANDALONE modules
On Mon, 21 Jan 2019 at 13:40, Gao, Liming wrote: > > Ard: > Wang, Jian is the reviewer of CryptoPkg. I think that his review-by is > enough. > OK, thanks! > Thanks > Liming > > -Original Message- > > From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org] > > Sent: Monday, January 21, 2019 8:36 PM > > To: Wang, Jian J ; Ye, Ting > > Cc: edk2-devel@lists.01.org; Kinney, Michael D > > ; Gao, Liming ; Wei, Gang > > ; Zhang, Chao B ; Yao, Jiewen > > ; Wu, Hao A > > ; Zeng, Star ; Achin Gupta > > ; Jagadeesh Ujja > > > > Subject: Re: [PATCH 1/4] CryptoPkg/SmmCryptLib: permit use by MM_STANDALONE > > modules > > > > On Fri, 18 Jan 2019 at 12:12, Ard Biesheuvel > > wrote: > > > > > > On Fri, 18 Jan 2019 at 08:08, Wang, Jian J wrote: > > > > > > > > > > > > > > > > Reviewed-by: Jian J Wang > > > > > > > > > > Ting, do you have any objections to this patch? > > > > > > > Ping? > > > > > > > > > > > > > > > -Original Message- > > > > > From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org] > > > > > Sent: Thursday, January 17, 2019 5:22 AM > > > > > To: edk2-devel@lists.01.org > > > > > Cc: Ard Biesheuvel ; Kinney, Michael D > > > > > ; Gao, Liming ; Ye, > > > > > Ting > > > > > ; Wei, Gang ; Wang, Jian J > > > > > ; Zhang, Chao B ; Yao, > > > > > Jiewen ; Wu, Hao A ; Zeng, > > > > > Star > > > > > ; Achin Gupta ; Jagadeesh > > > > > Ujja > > > > > > > > > > Subject: [PATCH 1/4] CryptoPkg/SmmCryptLib: permit use by > > > > > MM_STANDALONE > > > > > modules > > > > > > > > > > Permit SmmCryptLib to be used by MM_STANDALONE modules > > > > > > > > > > Contributed-under: TianoCore Contribution Agreement 1.1 > > > > > Signed-off-by: Ard Biesheuvel > > > > > --- > > > > > CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf | 2 +- > > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > > > diff --git a/CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf > > > > > b/CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf > > > > > index c34699cd62bf..a681fe2f36b8 100644 > > > > > --- a/CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf > > > > > +++ b/CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf > > > > > @@ -30,7 +30,7 @@ [Defines] > > > > >MODULE_TYPE= DXE_SMM_DRIVER > > > > >VERSION_STRING = 1.0 > > > > >PI_SPECIFICATION_VERSION = 0x0001000A > > > > > - LIBRARY_CLASS = BaseCryptLib|DXE_SMM_DRIVER > > > > > SMM_CORE > > > > > + LIBRARY_CLASS = BaseCryptLib|DXE_SMM_DRIVER > > > > > SMM_CORE > > > > > MM_STANDALONE > > > > > > > > > > # > > > > > # The following information is for reference only and not required > > > > > by the build > > > > > tools. > > > > > -- > > > > > 2.17.1 > > > > ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH 1/4] CryptoPkg/SmmCryptLib: permit use by MM_STANDALONE modules
Ard: Wang, Jian is the reviewer of CryptoPkg. I think that his review-by is enough. Thanks Liming > -Original Message- > From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org] > Sent: Monday, January 21, 2019 8:36 PM > To: Wang, Jian J ; Ye, Ting > Cc: edk2-devel@lists.01.org; Kinney, Michael D ; > Gao, Liming ; Wei, Gang > ; Zhang, Chao B ; Yao, Jiewen > ; Wu, Hao A > ; Zeng, Star ; Achin Gupta > ; Jagadeesh Ujja > > Subject: Re: [PATCH 1/4] CryptoPkg/SmmCryptLib: permit use by MM_STANDALONE > modules > > On Fri, 18 Jan 2019 at 12:12, Ard Biesheuvel > wrote: > > > > On Fri, 18 Jan 2019 at 08:08, Wang, Jian J wrote: > > > > > > > > > > > > Reviewed-by: Jian J Wang > > > > > > > Ting, do you have any objections to this patch? > > > > Ping? > > > > > > > > > > -Original Message- > > > > From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org] > > > > Sent: Thursday, January 17, 2019 5:22 AM > > > > To: edk2-devel@lists.01.org > > > > Cc: Ard Biesheuvel ; Kinney, Michael D > > > > ; Gao, Liming ; Ye, > > > > Ting > > > > ; Wei, Gang ; Wang, Jian J > > > > ; Zhang, Chao B ; Yao, > > > > Jiewen ; Wu, Hao A ; Zeng, > > > > Star > > > > ; Achin Gupta ; Jagadeesh Ujja > > > > > > > > Subject: [PATCH 1/4] CryptoPkg/SmmCryptLib: permit use by MM_STANDALONE > > > > modules > > > > > > > > Permit SmmCryptLib to be used by MM_STANDALONE modules > > > > > > > > Contributed-under: TianoCore Contribution Agreement 1.1 > > > > Signed-off-by: Ard Biesheuvel > > > > --- > > > > CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf | 2 +- > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > diff --git a/CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf > > > > b/CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf > > > > index c34699cd62bf..a681fe2f36b8 100644 > > > > --- a/CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf > > > > +++ b/CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf > > > > @@ -30,7 +30,7 @@ [Defines] > > > >MODULE_TYPE= DXE_SMM_DRIVER > > > >VERSION_STRING = 1.0 > > > >PI_SPECIFICATION_VERSION = 0x0001000A > > > > - LIBRARY_CLASS = BaseCryptLib|DXE_SMM_DRIVER SMM_CORE > > > > + LIBRARY_CLASS = BaseCryptLib|DXE_SMM_DRIVER SMM_CORE > > > > MM_STANDALONE > > > > > > > > # > > > > # The following information is for reference only and not required by > > > > the build > > > > tools. > > > > -- > > > > 2.17.1 > > > ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH 4/4] MdePkg/UefiDevicePathLib: permit use by MM_STANDALONE modules
Reviewed-by: Liming Gao > -Original Message- > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ard > Biesheuvel > Sent: Friday, January 18, 2019 7:07 PM > To: edk2-devel@lists.01.org > Cc: Ye, Ting ; Gao, Liming ; Wu, Hao > A ; Yao, Jiewen > ; Zeng, Star ; Kinney, Michael D > ; Zhang, Chao B > > Subject: Re: [edk2] [PATCH 4/4] MdePkg/UefiDevicePathLib: permit use by > MM_STANDALONE modules > > On Wed, 16 Jan 2019 at 22:22, Ard Biesheuvel > wrote: > > > > Add MM_STANDALONE to the list of module types that are permitted to > > link to this library. > > > > Contributed-under: TianoCore Contribution Agreement 1.1 > > Signed-off-by: Ard Biesheuvel > > --- > > MdePkg/Library/UefiDevicePathLib/UefiDevicePathLib.inf | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/MdePkg/Library/UefiDevicePathLib/UefiDevicePathLib.inf > > b/MdePkg/Library/UefiDevicePathLib/UefiDevicePathLib.inf > > index d5f7bfa6af39..89ee87e15d0e 100644 > > --- a/MdePkg/Library/UefiDevicePathLib/UefiDevicePathLib.inf > > +++ b/MdePkg/Library/UefiDevicePathLib/UefiDevicePathLib.inf > > @@ -22,7 +22,7 @@ [Defines] > >FILE_GUID = 91c1677a-e57f-4191-8b8e-eb7711a716e0 > >MODULE_TYPE= UEFI_DRIVER > >VERSION_STRING = 1.0 > > - LIBRARY_CLASS = DevicePathLib|DXE_CORE DXE_DRIVER > > DXE_RUNTIME_DRIVER DXE_SMM_DRIVER > UEFI_APPLICATION UEFI_DRIVER SMM_CORE > > + LIBRARY_CLASS = DevicePathLib|DXE_CORE DXE_DRIVER > > DXE_RUNTIME_DRIVER DXE_SMM_DRIVER > UEFI_APPLICATION UEFI_DRIVER SMM_CORE MM_STANDALONE > > > > > > Liming, Mike: do you have any objections? > ___ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH 1/4] CryptoPkg/SmmCryptLib: permit use by MM_STANDALONE modules
On Fri, 18 Jan 2019 at 12:12, Ard Biesheuvel wrote: > > On Fri, 18 Jan 2019 at 08:08, Wang, Jian J wrote: > > > > > > > > Reviewed-by: Jian J Wang > > > > Ting, do you have any objections to this patch? > Ping? > > > > > -Original Message- > > > From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org] > > > Sent: Thursday, January 17, 2019 5:22 AM > > > To: edk2-devel@lists.01.org > > > Cc: Ard Biesheuvel ; Kinney, Michael D > > > ; Gao, Liming ; Ye, Ting > > > ; Wei, Gang ; Wang, Jian J > > > ; Zhang, Chao B ; Yao, > > > Jiewen ; Wu, Hao A ; Zeng, Star > > > ; Achin Gupta ; Jagadeesh Ujja > > > > > > Subject: [PATCH 1/4] CryptoPkg/SmmCryptLib: permit use by MM_STANDALONE > > > modules > > > > > > Permit SmmCryptLib to be used by MM_STANDALONE modules > > > > > > Contributed-under: TianoCore Contribution Agreement 1.1 > > > Signed-off-by: Ard Biesheuvel > > > --- > > > CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf > > > b/CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf > > > index c34699cd62bf..a681fe2f36b8 100644 > > > --- a/CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf > > > +++ b/CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf > > > @@ -30,7 +30,7 @@ [Defines] > > >MODULE_TYPE= DXE_SMM_DRIVER > > >VERSION_STRING = 1.0 > > >PI_SPECIFICATION_VERSION = 0x0001000A > > > - LIBRARY_CLASS = BaseCryptLib|DXE_SMM_DRIVER SMM_CORE > > > + LIBRARY_CLASS = BaseCryptLib|DXE_SMM_DRIVER SMM_CORE > > > MM_STANDALONE > > > > > > # > > > # The following information is for reference only and not required by > > > the build > > > tools. > > > -- > > > 2.17.1 > > ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [platforms: PATCH 3/3] Marvell/Armada7k8k: Read DRAM settings from ARM-TF
On Mon, Jan 21, 2019 at 11:52:11AM +0100, Marcin Wojtas wrote: > From: Grzegorz Jaszczyk > > The memory controller registers are marked as secure in the latest > ARM-TF for Armada SoCs. It is available however get the DRAM > information via SiP services in the EL3, so use it instead > of accessing the registers directly. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Marcin Wojtas > --- > Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLib.inf | 3 ++ > Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.h | 27 > ++ > Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.c | 55 > +--- > 3 files changed, 21 insertions(+), 64 deletions(-) > > diff --git > a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLib.inf > b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLib.inf > index e888566..0c7f320 100644 > --- a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLib.inf > +++ b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLib.inf > @@ -41,12 +41,15 @@ > [Packages] >ArmPkg/ArmPkg.dec >ArmPlatformPkg/ArmPlatformPkg.dec > + EmbeddedPkg/EmbeddedPkg.dec >MdeModulePkg/MdeModulePkg.dec >MdePkg/MdePkg.dec >Silicon/Marvell/Marvell.dec > > [LibraryClasses] > + ArmadaSoCDescLib >ArmLib > + ArmSmcLib >DebugLib >MemoryAllocationLib >MppLib > diff --git > a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.h > b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.h > index cc30e4a..8a46df6 100644 > --- a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.h > +++ b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.h > @@ -47,27 +47,8 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH > DAMAGE. > #define DRAM_REMAP_TARGET \ >(MmioRead32 (CCU_MC_RTBR_REG) << TARGET_BASE_OFFS) > > -#define DRAM_CH0_MMAP_LOW_REG(cs) (0xf0020200 + (cs) * 0x8) > -#define DRAM_CS_VALID_ENABLED_MASK 0x1 > -#define DRAM_AREA_LENGTH_OFFS 16 > -#define DRAM_AREA_LENGTH_MASK (0x1f << DRAM_AREA_LENGTH_OFFS) > -#define DRAM_START_ADDRESS_L_OFFS 23 > -#define DRAM_START_ADDRESS_L_MASK (0x1ff << DRAM_START_ADDRESS_L_OFFS) > -#define DRAM_CH0_MMAP_HIGH_REG(cs) (0xf0020204 + (cs) * 0x8) > -#define DRAM_START_ADDR_HTOL_OFFS 32 > +/* Armada7k8k North Bridge index */ > +#define ARMADA7K8K_AP806_INDEX 0 > > -#define DRAM_MAX_CS_NUM 8 > - > -#define DRAM_CS_ENABLED(Cs) \ > - (MmioRead32 (DRAM_CH0_MMAP_LOW_REG (Cs)) & > DRAM_CS_VALID_ENABLED_MASK) > -#define GET_DRAM_REGION_BASE(Cs) \ > - ((UINT64)MmioRead32 (DRAM_CH0_MMAP_HIGH_REG ((Cs))) << \ > - DRAM_START_ADDR_HTOL_OFFS) | \ > - (MmioRead32 (DRAM_CH0_MMAP_LOW_REG (Cs)) & > DRAM_START_ADDRESS_L_MASK); > -#define GET_DRAM_REGION_SIZE_CODE(Cs) \ > - (MmioRead32 (DRAM_CH0_MMAP_LOW_REG ((Cs))) & \ > - DRAM_AREA_LENGTH_MASK) >> DRAM_AREA_LENGTH_OFFS > -#define DRAM_REGION_SIZE_EVEN(C)(((C) >= 7) && ((C) <= 26)) > -#define GET_DRAM_REGION_SIZE_EVEN(C)((UINT64)1 << ((C) + 16)) > -#define DRAM_REGION_SIZE_ODD(C) ((C) <= 4) > -#define GET_DRAM_REGION_SIZE_ODD(C) ((UINT64)0x1800 << (C)) > +/* Firmware related definition used for SMC calls */ > +#define MV_SIP_DRAM_SIZE0x8210 Hmm... This would end us up with having Marvell SMC calls spread across multiple files. Could you insert an intermediate patch which breaks out the ones from Silicon/Marvell/Library/ComPhyLib/ComPhySipSvc.h into a separate (MarvellSmc.h?) include file? If you could further add _SMC_ID to the #defines (like in edk2 ArmPkg/Include/IndustryStandard/ArmStdSmc.h), that would make me very happy. (I'd be happy for you to drop _SIP, or keep it either side of the addition, as per your preference - we don't seem to have precedent here yet.) Regards, Leif > diff --git > a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.c > b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.c > index 2a4f5ad..62e8467 100644 > --- a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.c > +++ b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.c > @@ -33,11 +33,14 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH > DAMAGE. > > ***/ > > #include > +#include > #include > +#include > #include > #include > #include > #include > +#include > > #include "Armada7k8kLibMem.h" > > @@ -57,49 +60,19 @@ GetDramSize ( >IN OUT UINT64 *MemSize >) > { > - UINT64 BaseAddr; > - UINT8 RegionCode; > - UINT8 Cs; > - > - *MemSize = 0; > - > - for (Cs = 0; Cs < DRAM_MAX_CS_NUM; Cs++) { > - > -/* Exit loop on first disabled DRAM CS */ > -if
Re: [edk2] [platforms: PATCH 2/3] Marvell/Library: ArmadaSoCDescLib: Add North Bridge description
On Mon, Jan 21, 2019 at 11:52:10AM +0100, Marcin Wojtas wrote: > From: Grzegorz Jaszczyk > > For upcomming patch there is need to get AP806 base, provide required > getter function for it. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Marcin Wojtas > --- > > Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.h > | 1 + > Silicon/Marvell/Include/Library/ArmadaSoCDescLib.h > | 10 ++ > > Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.c > | 17 + > 3 files changed, 28 insertions(+) > > diff --git > a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.h > > b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.h > index bfc8639..6caee6c 100644 > --- > a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.h > +++ > b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.h > @@ -22,6 +22,7 @@ > // Common macros > // > #define MV_SOC_CP_BASE(Cp) (0xF200 + ((Cp) * 0x200)) > +#define MV_SOC_AP806_BASE0xF000 > #define MV_SOC_AP806_COUNT 1 > > // > diff --git a/Silicon/Marvell/Include/Library/ArmadaSoCDescLib.h > b/Silicon/Marvell/Include/Library/ArmadaSoCDescLib.h > index 26b075a..7aec9be 100644 > --- a/Silicon/Marvell/Include/Library/ArmadaSoCDescLib.h > +++ b/Silicon/Marvell/Include/Library/ArmadaSoCDescLib.h > @@ -20,6 +20,16 @@ > #include > > // > +// North Bridge description > +// > +EFI_STATUS > +EFIAPI > +ArmadaSoCAp8xxBaseGet ( > + IN OUT UINT64 *ApBase, > + IN UINTNApIndex > + ); > + > +// > // ComPhy SoC description > // > typedef struct { > diff --git > a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.c > > b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.c > index 5b72c20..089ac2d 100644 > --- > a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.c > +++ > b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.c > @@ -30,6 +30,23 @@ > As I said, I was going to get stricter about adding function description comments - please add one here. (And clone it to ArmadaSoCDescLib.h, for the obious place to go look for a template when implementing a variant for a new platform.) > EFI_STATUS > EFIAPI > +ArmadaSoCAp8xxBaseGet ( > + IN OUT UINT64 *ApBase, > + IN UINTNApIndex > + ) > +{ > + if (ApIndex != 0) { This test should be using ARMADA7K8K_AP806_INDEX, and that definition should be added to this patch. > +DEBUG ((DEBUG_ERROR, "%a: Only one AP806 in A7K/A8K SoC\n", > __FUNCTION__)); > +return EFI_INVALID_PARAMETER; > + } > + > + *ApBase = MV_SOC_AP806_BASE; > + > + return EFI_SUCCESS; > +} > + > +EFI_STATUS > +EFIAPI > ArmadaSoCDescComPhyGet ( >IN OUT MV_SOC_COMPHY_DESC **ComPhyDesc, >IN OUT UINTN*DescCount > -- > 2.7.4 > ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [platforms: PATCH 3/3] Marvell/Armada7k8k: Read DRAM settings from ARM-TF
From: Grzegorz Jaszczyk The memory controller registers are marked as secure in the latest ARM-TF for Armada SoCs. It is available however get the DRAM information via SiP services in the EL3, so use it instead of accessing the registers directly. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Marcin Wojtas --- Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLib.inf | 3 ++ Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.h | 27 ++ Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.c | 55 +--- 3 files changed, 21 insertions(+), 64 deletions(-) diff --git a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLib.inf b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLib.inf index e888566..0c7f320 100644 --- a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLib.inf +++ b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLib.inf @@ -41,12 +41,15 @@ [Packages] ArmPkg/ArmPkg.dec ArmPlatformPkg/ArmPlatformPkg.dec + EmbeddedPkg/EmbeddedPkg.dec MdeModulePkg/MdeModulePkg.dec MdePkg/MdePkg.dec Silicon/Marvell/Marvell.dec [LibraryClasses] + ArmadaSoCDescLib ArmLib + ArmSmcLib DebugLib MemoryAllocationLib MppLib diff --git a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.h b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.h index cc30e4a..8a46df6 100644 --- a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.h +++ b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.h @@ -47,27 +47,8 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #define DRAM_REMAP_TARGET \ (MmioRead32 (CCU_MC_RTBR_REG) << TARGET_BASE_OFFS) -#define DRAM_CH0_MMAP_LOW_REG(cs) (0xf0020200 + (cs) * 0x8) -#define DRAM_CS_VALID_ENABLED_MASK 0x1 -#define DRAM_AREA_LENGTH_OFFS 16 -#define DRAM_AREA_LENGTH_MASK (0x1f << DRAM_AREA_LENGTH_OFFS) -#define DRAM_START_ADDRESS_L_OFFS 23 -#define DRAM_START_ADDRESS_L_MASK (0x1ff << DRAM_START_ADDRESS_L_OFFS) -#define DRAM_CH0_MMAP_HIGH_REG(cs) (0xf0020204 + (cs) * 0x8) -#define DRAM_START_ADDR_HTOL_OFFS 32 +/* Armada7k8k North Bridge index */ +#define ARMADA7K8K_AP806_INDEX 0 -#define DRAM_MAX_CS_NUM 8 - -#define DRAM_CS_ENABLED(Cs) \ - (MmioRead32 (DRAM_CH0_MMAP_LOW_REG (Cs)) & DRAM_CS_VALID_ENABLED_MASK) -#define GET_DRAM_REGION_BASE(Cs) \ - ((UINT64)MmioRead32 (DRAM_CH0_MMAP_HIGH_REG ((Cs))) << \ - DRAM_START_ADDR_HTOL_OFFS) | \ - (MmioRead32 (DRAM_CH0_MMAP_LOW_REG (Cs)) & DRAM_START_ADDRESS_L_MASK); -#define GET_DRAM_REGION_SIZE_CODE(Cs) \ - (MmioRead32 (DRAM_CH0_MMAP_LOW_REG ((Cs))) & \ - DRAM_AREA_LENGTH_MASK) >> DRAM_AREA_LENGTH_OFFS -#define DRAM_REGION_SIZE_EVEN(C)(((C) >= 7) && ((C) <= 26)) -#define GET_DRAM_REGION_SIZE_EVEN(C)((UINT64)1 << ((C) + 16)) -#define DRAM_REGION_SIZE_ODD(C) ((C) <= 4) -#define GET_DRAM_REGION_SIZE_ODD(C) ((UINT64)0x1800 << (C)) +/* Firmware related definition used for SMC calls */ +#define MV_SIP_DRAM_SIZE0x8210 diff --git a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.c b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.c index 2a4f5ad..62e8467 100644 --- a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.c +++ b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.c @@ -33,11 +33,14 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. ***/ #include +#include #include +#include #include #include #include #include +#include #include "Armada7k8kLibMem.h" @@ -57,49 +60,19 @@ GetDramSize ( IN OUT UINT64 *MemSize ) { - UINT64 BaseAddr; - UINT8 RegionCode; - UINT8 Cs; - - *MemSize = 0; - - for (Cs = 0; Cs < DRAM_MAX_CS_NUM; Cs++) { - -/* Exit loop on first disabled DRAM CS */ -if (!DRAM_CS_ENABLED (Cs)) { - break; -} - -/* - * Sanity check for base address of next DRAM block. - * Only continuous space will be used. - */ -BaseAddr = GET_DRAM_REGION_BASE (Cs); -if (BaseAddr != *MemSize) { - DEBUG ((DEBUG_ERROR, -"%a: DRAM blocks are not contiguous, limit size to 0x%llx\n", -__FUNCTION__, -*MemSize)); - return EFI_SUCCESS; -} - -/* Decode area length for current CS from register value */ -RegionCode = GET_DRAM_REGION_SIZE_CODE (Cs); - -if (DRAM_REGION_SIZE_EVEN (RegionCode)) { - *MemSize += GET_DRAM_REGION_SIZE_EVEN (RegionCode); -} else if (DRAM_REGION_SIZE_ODD (RegionCode)) { - *MemSize += GET_DRAM_REGION_SIZE_ODD (RegionCode); -} else { - DEBUG ((DEBUG_ERROR, -"%a: Invalid memory region code (0x%x) for CS#%d\n", -
Re: [edk2] [platforms: PATCH 1/3] Marvell: Armada7k8k: Shift PEI stack base
On Mon, Jan 21, 2019 at 11:52:09AM +0100, Marcin Wojtas wrote: > Recent changes in the ARM-TF configure its runtime serices region > as protected, hence the hitherto PEI stack base address (0x41F) > violated it. > > In order to fix this, extend the region which is non-accessible > by the OS to cover both the ARM-TF (0x400 - 0x420) and OPTEE > (0x440 - 0x540) within a single area and set the PEI stack What is the single area? > base address between both images. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Marcin Wojtas > --- > Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc > b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc > index eafcd6e..c8c597f 100644 > --- a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc > +++ b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc > @@ -376,12 +376,12 @@ > >gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize|36 > > - gArmPlatformTokenSpaceGuid.PcdCPUCoresStackBase|0x41F > + gArmPlatformTokenSpaceGuid.PcdCPUCoresStackBase|0x43F >gArmPlatformTokenSpaceGuid.PcdCPUCorePrimaryStackSize|0x1 > ># Secure region reservation >gMarvellTokenSpaceGuid.PcdSecureRegionBase|0x400 > - gMarvellTokenSpaceGuid.PcdSecureRegionSize|0x020 > + gMarvellTokenSpaceGuid.PcdSecureRegionSize|0x140 > ># TRNG >gMarvellTokenSpaceGuid.PcdEip76TrngBaseAddress|0xF276 > -- > 2.7.4 > ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [platforms: PATCH 2/3] Marvell/Library: ArmadaSoCDescLib: Add North Bridge description
From: Grzegorz Jaszczyk For upcomming patch there is need to get AP806 base, provide required getter function for it. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Marcin Wojtas --- Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.h | 1 + Silicon/Marvell/Include/Library/ArmadaSoCDescLib.h | 10 ++ Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.c | 17 + 3 files changed, 28 insertions(+) diff --git a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.h b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.h index bfc8639..6caee6c 100644 --- a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.h +++ b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.h @@ -22,6 +22,7 @@ // Common macros // #define MV_SOC_CP_BASE(Cp) (0xF200 + ((Cp) * 0x200)) +#define MV_SOC_AP806_BASE0xF000 #define MV_SOC_AP806_COUNT 1 // diff --git a/Silicon/Marvell/Include/Library/ArmadaSoCDescLib.h b/Silicon/Marvell/Include/Library/ArmadaSoCDescLib.h index 26b075a..7aec9be 100644 --- a/Silicon/Marvell/Include/Library/ArmadaSoCDescLib.h +++ b/Silicon/Marvell/Include/Library/ArmadaSoCDescLib.h @@ -20,6 +20,16 @@ #include // +// North Bridge description +// +EFI_STATUS +EFIAPI +ArmadaSoCAp8xxBaseGet ( + IN OUT UINT64 *ApBase, + IN UINTNApIndex + ); + +// // ComPhy SoC description // typedef struct { diff --git a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.c b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.c index 5b72c20..089ac2d 100644 --- a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.c +++ b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.c @@ -30,6 +30,23 @@ EFI_STATUS EFIAPI +ArmadaSoCAp8xxBaseGet ( + IN OUT UINT64 *ApBase, + IN UINTNApIndex + ) +{ + if (ApIndex != 0) { +DEBUG ((DEBUG_ERROR, "%a: Only one AP806 in A7K/A8K SoC\n", __FUNCTION__)); +return EFI_INVALID_PARAMETER; + } + + *ApBase = MV_SOC_AP806_BASE; + + return EFI_SUCCESS; +} + +EFI_STATUS +EFIAPI ArmadaSoCDescComPhyGet ( IN OUT MV_SOC_COMPHY_DESC **ComPhyDesc, IN OUT UINTN*DescCount -- 2.7.4 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [platforms: PATCH 1/3] Marvell: Armada7k8k: Shift PEI stack base
Recent changes in the ARM-TF configure its runtime serices region as protected, hence the hitherto PEI stack base address (0x41F) violated it. In order to fix this, extend the region which is non-accessible by the OS to cover both the ARM-TF (0x400 - 0x420) and OPTEE (0x440 - 0x540) within a single area and set the PEI stack base address between both images. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Marcin Wojtas --- Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc index eafcd6e..c8c597f 100644 --- a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc +++ b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc @@ -376,12 +376,12 @@ gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize|36 - gArmPlatformTokenSpaceGuid.PcdCPUCoresStackBase|0x41F + gArmPlatformTokenSpaceGuid.PcdCPUCoresStackBase|0x43F gArmPlatformTokenSpaceGuid.PcdCPUCorePrimaryStackSize|0x1 # Secure region reservation gMarvellTokenSpaceGuid.PcdSecureRegionBase|0x400 - gMarvellTokenSpaceGuid.PcdSecureRegionSize|0x020 + gMarvellTokenSpaceGuid.PcdSecureRegionSize|0x140 # TRNG gMarvellTokenSpaceGuid.PcdEip76TrngBaseAddress|0xF276 -- 2.7.4 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [platforms: PATCH 0/3] Armada7k8k memory handling update
Hi, This short patchset adjust to the latest Marvell v18.12 ARM-TF sources. They change DRAM configuration registers to be secure, as well as extend the region, which is non-accessible by OS. The patches avoid the issues by shifitng PEI stack base and configuring the memory map, using SiP services. Patches are available in the github: https://github.com/MarvellEmbeddedProcessors/edk2-open-platform/commits/dram-upstream-r20190121 I'm looking forward to the comments and remarks. Best regards, Marcin Grzegorz Jaszczyk (2): Marvell/Library: ArmadaSoCDescLib: Add North Bridge description Marvell/Armada7k8k: Read DRAM settings from ARM-TF Marcin Wojtas (1): Marvell: Armada7k8k: Shift PEI stack base Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc | 4 +- Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLib.inf | 3 ++ Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.h | 27 ++ Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.h | 1 + Silicon/Marvell/Include/Library/ArmadaSoCDescLib.h | 10 Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.c | 55 +--- Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.c | 17 ++ 7 files changed, 51 insertions(+), 66 deletions(-) -- 2.7.4 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH V2 10/15] ArmVirtXen: Use merged variable driver for emulated NV mode
Hi Julien, On 2019/1/18 2:59, Julien Grall wrote: Hi Star, On 17/01/2019 01:26, Zeng, Star wrote: On 2019/1/16 22:26, Julien Grall wrote: Hi Laszlo, On 15/01/2019 09:37, Laszlo Ersek wrote: On 01/14/19 16:19, Star Zeng wrote: REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1323 Merge EmuVariable and Real variable driver. The real variable driver has been updated to support emulated variable NV mode and the EmuVariableRuntimeDxe will be removed later, so use merged variable driver for emulated NV mode. Cc: Laszlo Ersek Cc: Ard Biesheuvel Cc: Julien Grall Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Star Zeng --- ArmVirtPkg/ArmVirtXen.dsc | 9 +++-- ArmVirtPkg/ArmVirtXen.fdf | 4 ++-- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/ArmVirtPkg/ArmVirtXen.dsc b/ArmVirtPkg/ArmVirtXen.dsc index a29d8a4ae717..db85fb3402d0 100644 --- a/ArmVirtPkg/ArmVirtXen.dsc +++ b/ArmVirtPkg/ArmVirtXen.dsc @@ -1,7 +1,7 @@ # # Copyright (c) 2011-2015, ARM Limited. All rights reserved. # Copyright (c) 2014, Linaro Limited. All rights reserved. -# Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved. +# Copyright (c) 2015 - 2019, Intel Corporation. All rights reserved. # # This program and the accompanying materials # are licensed and made available under the terms and conditions of the BSD License @@ -101,6 +101,11 @@ [PcdsFixedAtBuild.common] # Set terminal type to TtyTerm, the value encoded is EFI_TTY_TERM_GUID gArmVirtTokenSpaceGuid.PcdTerminalTypeGuidBuffer|{0x80, 0x6d, 0x91, 0x7d, 0xb1, 0x5b, 0x8c, 0x45, 0xa4, 0x8f, 0xe2, 0x5f, 0xdd, 0x51, 0xef, 0x94} + # + # Make VariableRuntimeDxe work at emulated non-volatile variable mode. + # + gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvModeEnable|TRUE + [PcdsPatchableInModule.common] # # This will be overridden in the code @@ -172,7 +177,7 @@ [Components.common] MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleRuntimeDxe.inf - MdeModulePkg/Universal/Variable/EmuRuntimeDxe/EmuVariableRuntimeDxe.inf + MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf MdeModulePkg/Universal/MonotonicCounterRuntimeDxe/MonotonicCounterRuntimeDxe.inf MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystemRuntimeDxe.inf diff --git a/ArmVirtPkg/ArmVirtXen.fdf b/ArmVirtPkg/ArmVirtXen.fdf index 50e670254d52..5655c0df2926 100644 --- a/ArmVirtPkg/ArmVirtXen.fdf +++ b/ArmVirtPkg/ArmVirtXen.fdf @@ -1,7 +1,7 @@ # # Copyright (c) 2011-2015, ARM Limited. All rights reserved. # Copyright (c) 2014, Linaro Limited. All rights reserved. -# Copyright (c) 2015 - 2017, Intel Corporation. All rights reserved. +# Copyright (c) 2015 - 2019, Intel Corporation. All rights reserved. # # This program and the accompanying materials # are licensed and made available under the terms and conditions of the BSD License @@ -137,7 +137,7 @@ [FV.FvMain] INF MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf INF MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleRuntimeDxe.inf - INF MdeModulePkg/Universal/Variable/EmuRuntimeDxe/EmuVariableRuntimeDxe.inf + INF MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf INF MdeModulePkg/Universal/MonotonicCounterRuntimeDxe/MonotonicCounterRuntimeDxe.inf INF MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystemRuntimeDxe.inf Reviewed-by: Laszlo Ersek Julien, can you please regression test this series on Xen? The repo URL and the branch name are in the blurb. I will have a try and let you know the result. Thank you, Julien. I think you can try based on V3 series although there is no essential difference between V2 and V3. :) Repo: g...@github.com:lzeng14/edk2.git Branch: MergedVariableDriver_EmuNvMode_V3 Thank you for the link. I am having trouble to get newer UEFI (without your patch) running in a Xen guest. I have an old binary working. I am not entirely sure why, I will let you know when I can test your series. I saw the discussion at https://lists.01.org/pipermail/edk2-devel/2019-January/035405.html. Fortunately, it has been fixed. So I did rebase for the code. Repo: g...@github.com:lzeng14/edk2.git Branch: MergedVariableDriver_EmuNvMode_V3_rebased If you can help have a quick test, that will be very helpful. :) Thanks, Star Cheers, ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] Unable to boot Linux with master EDK2
Hi Dandan, On 21/01/2019 02:23, Bi, Dandan wrote: I have committed the fix patch. https://github.com/tianocore/edk2/commit/eb76b76218d5bac867414e2ff6dd09c6e7c700dd Please trying the latest EDK2 master. I have tried EDK2 master and got a guest booting with UEFI. Thank you for merging it! Cheers, -- Julien Grall ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel