Re: [edk2] [PATCH V2 10/15] ArmVirtXen: Use merged variable driver for emulated NV mode

2019-01-21 Thread Zeng, Star

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.

2019-01-21 Thread Meenakshi Aggarwal
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

2019-01-21 Thread Ye, Ting
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

2019-01-21 Thread Marcin Wojtas
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

2019-01-21 Thread Marcin Wojtas
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

2019-01-21 Thread Marcin Wojtas
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

2019-01-21 Thread Marcin Wojtas
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

2019-01-21 Thread Marcin Wojtas
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

2019-01-21 Thread Chiu, Chasel


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.

2019-01-21 Thread Philippe Mathieu-Daudé
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

2019-01-21 Thread Ard Biesheuvel
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

2019-01-21 Thread Ard Biesheuvel
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.

2019-01-21 Thread Laszlo Ersek
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

2019-01-21 Thread Laszlo Ersek
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

2019-01-21 Thread Ard Biesheuvel
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

2019-01-21 Thread Ard Biesheuvel
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

2019-01-21 Thread Leif Lindholm
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

2019-01-21 Thread Ard Biesheuvel
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

2019-01-21 Thread Leif Lindholm
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

2019-01-21 Thread Leif Lindholm
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

2019-01-21 Thread Ard Biesheuvel
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

2019-01-21 Thread Leif Lindholm
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

2019-01-21 Thread Ard Biesheuvel
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

2019-01-21 Thread Leif Lindholm
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

2019-01-21 Thread Marcin Wojtas
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

2019-01-21 Thread Marcin Wojtas
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

2019-01-21 Thread Leif Lindholm
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

2019-01-21 Thread Marcin Wojtas
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

2019-01-21 Thread Ruiyu Ni
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

2019-01-21 Thread Ard Biesheuvel
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

2019-01-21 Thread Ard Biesheuvel
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

2019-01-21 Thread Julien Grall
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

2019-01-21 Thread Philippe Mathieu-Daudé
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.

2019-01-21 Thread Philippe Mathieu-Daudé
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

2019-01-21 Thread Ard Biesheuvel
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

2019-01-21 Thread Leif Lindholm
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.

2019-01-21 Thread Gao, Liming
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

2019-01-21 Thread Gao, Liming
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

2019-01-21 Thread Ard Biesheuvel
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

2019-01-21 Thread Ard Biesheuvel
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

2019-01-21 Thread Gao, Liming
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

2019-01-21 Thread Gao, Liming
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

2019-01-21 Thread Ard Biesheuvel
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

2019-01-21 Thread Leif Lindholm
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

2019-01-21 Thread Leif Lindholm
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

2019-01-21 Thread Marcin Wojtas
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

2019-01-21 Thread Leif Lindholm
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

2019-01-21 Thread Marcin Wojtas
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

2019-01-21 Thread Marcin Wojtas
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

2019-01-21 Thread Marcin Wojtas
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

2019-01-21 Thread Zeng, Star

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

2019-01-21 Thread Julien Grall

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