[edk2] [PATCH 0/2] ARM Dynamic Configuration

2018-06-14 Thread Chandni Cherukuri
On SGI platforms, the trusted firmware executes prior to the SEC
phase. It supplies to the SEC phase a pointer to a HW CONFIG fdt
in the x1 which contains platform specific information such as part
number and config number of the SGI platform. 

The HW CONFIG FDT would look as,
/dts-v1/;
/ {
/* compatible string */
compatible = "arm,sgi-isys3";

/* platform ID node */
system-id {
 platform-id = <0x03000783>;
}
};

In the very first step of the assembly code which executes in SEC phase
the fdt pointer is stored in a global variable from the register. A PPI 
is created during the SEC phase which stores the global variable.

During PEI phase, a Platform ID PEIM is installed which accessess the PPI
and retrieves the platform information using FDT helper functions and a
PlatformId HOB is created and populated with the information.
 
During DXE phase the drivers can access the Platform ID HOB to get the
platform information and perform platform specific functions based on the
platform.

Chandni Cherukuri (2):
  Platform/ARM/Sgi: Install a Platform ID HOB
  Platform/ARM/Sgi: Pick ACPI tables to install based on platform ID

 .../ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c   | 25 +-
 .../ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.inf |  2 +
 Platform/ARM/SgiPkg/Include/SgiPlatform.h  | 18 +
 .../SgiPkg/Library/PlatformLib/AArch64/Helper.S|  7 ++
 .../ARM/SgiPkg/Library/PlatformLib/PlatformLib.c   | 10 +++
 .../ARM/SgiPkg/Library/PlatformLib/PlatformLib.inf |  3 +
 .../Library/SgiPlatformPeiLib/SgiPlatformPeiLib.c  | 92 ++
 .../SgiPlatformPeiLib/SgiPlatformPeiLib.inf| 40 ++
 Platform/ARM/SgiPkg/SgiPlatform.dec|  5 ++
 Platform/ARM/SgiPkg/SgiPlatform.dsc|  1 +
 Platform/ARM/SgiPkg/SgiPlatform.fdf|  1 +
 11 files changed, 203 insertions(+), 1 deletion(-)
 create mode 100644 
Platform/ARM/SgiPkg/Library/SgiPlatformPeiLib/SgiPlatformPeiLib.c
 create mode 100644 
Platform/ARM/SgiPkg/Library/SgiPlatformPeiLib/SgiPlatformPeiLib.inf

-- 
2.7.4

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


[edk2] [PATCH 2/2] Platform/ARM/Sgi: Pick ACPI tables to install based on platform ID

2018-06-14 Thread Chandni Cherukuri
Use the platform ID and config ID values from the platform ID
HOB to choose the right set of ACPI tables to be installed.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Chandni Cherukuri 
---
 .../ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c   | 33 ++
 .../ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.inf |  2 ++
 Platform/ARM/SgiPkg/Include/SgiPlatform.h  | 24 ++--
 Platform/ARM/SgiPkg/SgiPlatform.dsc|  1 +
 Platform/ARM/SgiPkg/SgiPlatform.fdf|  1 +
 5 files changed, 53 insertions(+), 8 deletions(-)

diff --git a/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c 
b/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c
index edaae5b..d0d5808 100644
--- a/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c
+++ b/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c
@@ -14,6 +14,8 @@
 
 #include 
 #include 
+#include 
+#include 
 
 EFI_STATUS
 InitVirtioBlockIo (
@@ -28,20 +30,41 @@ ArmSgiPkgEntryPoint (
   )
 {
   EFI_STATUS  Status;
+  VOID*PlatformIdHob;
+  SGI_PLATFORM_DESCRIPTOR *HobData;
+  UINT32  ConfigId;
+  UINT32  PartNum;
 
-  Status = LocateAndInstallAcpiFromFv ();
-  if (EFI_ERROR (Status)) {
-DEBUG ((DEBUG_ERROR, "%a: Failed to install ACPI tables\n", __FUNCTION__));
-return Status;
+  PlatformIdHob = GetFirstGuidHob ();
+  if (PlatformIdHob == NULL) {
+DEBUG ((DEBUG_ERROR, "Platform ID Hob is NULL\n"));
+return EFI_INVALID_PARAMETER;
+  }
+
+  HobData = (SGI_PLATFORM_DESCRIPTOR *)(GET_GUID_HOB_DATA (PlatformIdHob));
+
+  PartNum = (HobData->PlatformId & SGI_PART_NUM_MASK);
+  ConfigId = ((HobData->PlatformId >> SGI_CONFIG_SHIFT) & SGI_CONFIG_MASK);
+
+  if ((PartNum == SGI575_PART_NUM) && (ConfigId == SGI575_CONF_NUM)) {
+Status = LocateAndInstallAcpiFromFv ();
+if (EFI_ERROR (Status)) {
+  DEBUG ((DEBUG_ERROR, "%a: Failed to install ACPI tables\n", 
__FUNCTION__));
+  return Status;
+}
+  } else {
+DEBUG ((DEBUG_ERROR, "PlatformDxe: Unsupported Platform Id\n"));
+return EFI_UNSUPPORTED;
   }
 
-  Status = EFI_REQUEST_UNLOAD_IMAGE;
   if (FeaturePcdGet (PcdVirtioSupported)) {
 Status = InitVirtioBlockIo (ImageHandle);
 if (EFI_ERROR (Status)) {
   DEBUG ((DEBUG_ERROR, "%a: Failed to install Virtio Block device\n",
 __FUNCTION__));
 }
+  } else {
+Status = EFI_REQUEST_UNLOAD_IMAGE;
   }
 
   return Status;
diff --git a/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.inf 
b/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.inf
index 51ad22f..b6b8209 100644
--- a/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.inf
+++ b/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.inf
@@ -30,10 +30,12 @@
 
 [LibraryClasses]
   AcpiLib
+  HobLib
   UefiDriverEntryPoint
   VirtioMmioDeviceLib
 
 [Guids]
+  gArmSgiPlatformIdDescriptorGuid
   gSgi575AcpiTablesiFileGuid
 
 [FeaturePcd]
diff --git a/Platform/ARM/SgiPkg/Include/SgiPlatform.h 
b/Platform/ARM/SgiPkg/Include/SgiPlatform.h
index 00ca7e9..756954c 100644
--- a/Platform/ARM/SgiPkg/Include/SgiPlatform.h
+++ b/Platform/ARM/SgiPkg/Include/SgiPlatform.h
@@ -12,8 +12,8 @@
 *
 **/
 
-#ifndef __SGI_PLATFORM_H__
-#define __SGI_PLATFORM_H__
+#ifndef SGI_PLATFORM_H_
+#define SGI_PLATFORM_H_
 
 
/***
 // Platform Memory Map
@@ -68,4 +68,22 @@
 #define SGI_SYSPH_SYS_REG_FLASH   0x4C
 #define SGI_SYSPH_SYS_REG_FLASH_RWEN  0x1
 
-#endif // __SGI_PLATFORM_H__
+// SGI575_VERSION values
+#define SGI575_CONF_NUM   0x3
+#define SGI575_PART_NUM   0x783
+
+#define SGI_CONFIG_MASK   0x0F
+#define SGI_CONFIG_SHIFT  0x1C
+#define SGI_PART_NUM_MASK 0xFFF
+
+//HwConfig DT structure^M
+typedef struct {
+  UINT64  *HwConfigDtAddr;
+} SGI_HW_CONFIG_INFO_PPI;
+
+// ARM platform description data.
+typedef struct {
+  UINTN  PlatformId;
+} SGI_PLATFORM_DESCRIPTOR;
+
+#endif // SGI_PLATFORM_H_
diff --git a/Platform/ARM/SgiPkg/SgiPlatform.dsc 
b/Platform/ARM/SgiPkg/SgiPlatform.dsc
index a56175e..f571897 100644
--- a/Platform/ARM/SgiPkg/SgiPlatform.dsc
+++ b/Platform/ARM/SgiPkg/SgiPlatform.dsc
@@ -199,6 +199,7 @@
 
   
NULL|MdeModulePkg/Library/LzmaCustomDecompressLib/LzmaCustomDecompressLib.inf
   }
+  Platform/ARM/SgiPkg/Library/SgiPlatformPeiLib/SgiPlatformPeiLib.inf
 
   #
   # DXE
diff --git a/Platform/ARM/SgiPkg/SgiPlatform.fdf 
b/Platform/ARM/SgiPkg/SgiPlatform.fdf
index 17cdf48..05203a8 100644
--- a/Platform/ARM/SgiPkg/SgiPlatform.fdf
+++ b/Platform/ARM/SgiPkg/SgiPlatform.fdf
@@ -220,6 +220,7 @@ READ_LOCK_STATUS   = TRUE
   INF MdeModulePkg/Universal/FaultTolerantWritePei/FaultTolerantWritePei.inf
   INF MdeModulePkg/Universal/PCD/Pei/Pcd.inf
   INF 

[edk2] [PATCH 1/2] Platform/ARM/Sgi: Install a Platform ID HOB

2018-06-14 Thread Chandni Cherukuri
On SGI platforms, the trusted firmware executes prior to
the SEC phase. It supplies to the SEC phase a pointer to
a fdt, that contains platform specific information such as
part number and config number of the SGI platform. The
platform code that executes during the SEC phase creates a
PPI that allows access to other PEIMs to obtain a copy of the
fdt pointer.

Further, during the PEI phase, a Platform ID PEIM installs a
Platform ID HOB. The Platform ID HOB can be used by DXE
drivers to identify the platform it is executing on.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Chandni Cherukuri 
---
 .../SgiPkg/Library/PlatformLib/AArch64/Helper.S|  15 ++-
 .../ARM/SgiPkg/Library/PlatformLib/PlatformLib.c   |  10 ++
 .../ARM/SgiPkg/Library/PlatformLib/PlatformLib.inf |   3 +
 .../Library/SgiPlatformPeiLib/SgiPlatformPeiLib.c  | 108 +
 .../SgiPlatformPeiLib/SgiPlatformPeiLib.inf|  40 
 Platform/ARM/SgiPkg/SgiPlatform.dec|   5 +
 6 files changed, 177 insertions(+), 4 deletions(-)
 create mode 100644 
Platform/ARM/SgiPkg/Library/SgiPlatformPeiLib/SgiPlatformPeiLib.c
 create mode 100644 
Platform/ARM/SgiPkg/Library/SgiPlatformPeiLib/SgiPlatformPeiLib.inf

diff --git a/Platform/ARM/SgiPkg/Library/PlatformLib/AArch64/Helper.S 
b/Platform/ARM/SgiPkg/Library/PlatformLib/AArch64/Helper.S
index dab6c77..80b93ea 100644
--- a/Platform/ARM/SgiPkg/Library/PlatformLib/AArch64/Helper.S
+++ b/Platform/ARM/SgiPkg/Library/PlatformLib/AArch64/Helper.S
@@ -22,15 +22,22 @@ GCC_ASM_EXPORT(ArmPlatformPeiBootAction)
 GCC_ASM_EXPORT(ArmPlatformGetCorePosition)
 GCC_ASM_EXPORT(ArmPlatformGetPrimaryCoreMpId)
 GCC_ASM_EXPORT(ArmPlatformIsPrimaryCore)
+GCC_ASM_EXPORT(HwConfigDtBlob)
+
+HwConfigDtBlob: .long 0
 
 //
 // First platform specific function to be called in the PEI phase
 //
-// This function is actually the first function called by the PrePi
-// or PrePeiCore modules. It allows to retrieve arguments passed to
-// the UEFI firmware through the CPU registers.
-//
+// The trusted firmware passed the hw config DT blob in x1 register.
+// Keep a copy of it in a global variable.
+//VOID
+//ArmPlatformPeiBootAction (
+//  VOID
+//  );
 ASM_PFX(ArmPlatformPeiBootAction):
+  ldr  x10, =HwConfigDtBlob
+  str  x1, [x10]
   ret
 
 //UINTN
diff --git a/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.c 
b/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.c
index ea3201a..54860e0 100644
--- a/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.c
+++ b/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.c
@@ -15,6 +15,10 @@
 #include 
 #include 
 #include 
+#include 
+
+extern UINT64 HwConfigDtBlob;
+STATIC SGI_HW_CONFIG_INFO_PPI mHwConfigDtInfoPpi;
 
 STATIC ARM_CORE_INFO mCoreInfoTable[] = {
   {
@@ -36,6 +40,7 @@ ArmPlatformInitialize (
   IN  UINTN MpId
   )
 {
+  mHwConfigDtInfoPpi.HwConfigDtAddr = 
   return RETURN_SUCCESS;
 }
 
@@ -59,6 +64,11 @@ EFI_PEI_PPI_DESCRIPTOR gPlatformPpiTable[] = {
 EFI_PEI_PPI_DESCRIPTOR_PPI,
 ,
 
+  },
+  {
+EFI_PEI_PPI_DESCRIPTOR_PPI,
+,
+
   }
 };
 
diff --git a/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.inf 
b/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.inf
index 42e14d5..5d4bf72 100644
--- a/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.inf
+++ b/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.inf
@@ -61,7 +61,10 @@
   gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress
 
 [Guids]
+  gArmSgiPlatformIdDescriptorGuid
   gEfiHobListGuid  ## CONSUMES  ## SystemTable
+  gFdtTableGuid
 
 [Ppis]
   gArmMpCoreInfoPpiGuid
+  gHwConfigDtInfoPpiGuid
diff --git a/Platform/ARM/SgiPkg/Library/SgiPlatformPeiLib/SgiPlatformPeiLib.c 
b/Platform/ARM/SgiPkg/Library/SgiPlatformPeiLib/SgiPlatformPeiLib.c
new file mode 100644
index 000..f6446e6
--- /dev/null
+++ b/Platform/ARM/SgiPkg/Library/SgiPlatformPeiLib/SgiPlatformPeiLib.c
@@ -0,0 +1,108 @@
+/** @file
+*
+*  Copyright (c) 2018, 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.
+*
+**/
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/**
+
+  This function returns the Platform ID of the platform
+
+  This functions locates the HwConfig PPI and gets the base address of HW 
CONFIG
+  DT from which the platform ID is obtained using FDT helper functions
+
+  @return returns the platform ID on success else returns 0 on error
+
+**/
+
+STATIC
+UINT32
+GetSgiPlatformId (
+ VOID
+)
+{
+  CONST UINT32  *Property;
+  INT32 Offset;
+  CONST 

Re: [edk2] [Patch] Vlv2TbltDevicePkg: Set SMM Stack size to 16 KB

2018-06-14 Thread Wei, David
Reviewed-by: David Wei  

Thanks,
David  Wei

Intel SSG/STO/UEFI BIOS 


-Original Message-
From: Kinney, Michael D 
Sent: Tuesday, June 12, 2018 2:05 AM
To: edk2-devel@lists.01.org
Cc: Kinney, Michael D ; Wei, David 
; Guo, Mang 
Subject: [Patch] Vlv2TbltDevicePkg: Set SMM Stack size to 16 KB

From: Michael D Kinney 

Stack overflows were observed at the default SMM stack
size of 8 KB.  Increase stack size to 16 KB to prevent
SMM stack overflows.

Cc: David Wei 
Cc: Mang Guo 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Michael D Kinney 
---
 Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc | 5 +
 Vlv2TbltDevicePkg/PlatformPkgIA32.dsc   | 5 +
 Vlv2TbltDevicePkg/PlatformPkgX64.dsc| 5 +
 3 files changed, 15 insertions(+)

diff --git a/Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc 
b/Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc
index b6741257e7..af845ed19f 100644
--- a/Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc
+++ b/Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc
@@ -701,6 +701,11 @@ [PcdsFixedAtBuild.common]
   gEfiSourceLevelDebugPkgTokenSpaceGuid.PcdDebugLoadImageMethod|2
 !endif
 
+  #
+  # Set SMM stack size to 16 KB.
+  #
+  gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmStackSize|0x4000
+
 [PcdsFixedAtBuild.IA32.PEIM, PcdsFixedAtBuild.IA32.PEI_CORE, 
PcdsFixedAtBuild.IA32.SEC]
 !if $(TARGET) == RELEASE
   gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0x0
diff --git a/Vlv2TbltDevicePkg/PlatformPkgIA32.dsc 
b/Vlv2TbltDevicePkg/PlatformPkgIA32.dsc
index bd276f0643..44a6fcab18 100644
--- a/Vlv2TbltDevicePkg/PlatformPkgIA32.dsc
+++ b/Vlv2TbltDevicePkg/PlatformPkgIA32.dsc
@@ -701,6 +701,11 @@ [PcdsFixedAtBuild.common]
   gEfiSourceLevelDebugPkgTokenSpaceGuid.PcdDebugLoadImageMethod|2
 !endif
 
+  #
+  # Set SMM stack size to 16 KB.
+  #
+  gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmStackSize|0x4000
+
 [PcdsFixedAtBuild.IA32.PEIM, PcdsFixedAtBuild.IA32.PEI_CORE, 
PcdsFixedAtBuild.IA32.SEC]
 !if $(TARGET) == RELEASE
   gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0x0
diff --git a/Vlv2TbltDevicePkg/PlatformPkgX64.dsc 
b/Vlv2TbltDevicePkg/PlatformPkgX64.dsc
index 042a35b2b7..0217a2818f 100644
--- a/Vlv2TbltDevicePkg/PlatformPkgX64.dsc
+++ b/Vlv2TbltDevicePkg/PlatformPkgX64.dsc
@@ -701,6 +701,11 @@ [PcdsFixedAtBuild.common]
   gEfiSourceLevelDebugPkgTokenSpaceGuid.PcdDebugLoadImageMethod|2
 !endif
 
+  #
+  # Set SMM stack size to 16 KB.
+  #
+  gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmStackSize|0x4000
+
 [PcdsFixedAtBuild.IA32]
 !if $(TARGET) == RELEASE
   gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0x0
-- 
2.14.2.windows.3

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] Creating EFI System Partition

2018-06-14 Thread Andrew Fish



> On Jun 14, 2018, at 5:35 PM, Robinson, Herbie  
> wrote:
> 
> I have been tasked with implementing EFI boot in our VOS operating system (in 
> a panic, nobody bothered to tell us ahead of time that legacy boot was being 
> dropped from the BIOS on our latest hardware).  I think we have a pretty good 
> handle on writing the bootloader, but I can't find any solid information on 
> creating an empty EFI System Partition bearing the correct flavor of FAT32 to 
> put the bootloader in.  I need to end up with a binary image file (which I 
> can process into an object module and embed into our OS code for initializing 
> disks).
> 

Herbie,

It is "bog standard" FAT3", see Microsoft Extensible Firmware Initiative FAT32 
File System Specification 


The OS loader is just an EFI Executable in file. So any old FAT32 tools will 
do. 

So are you looking for EFI tools to do GPT partitioning and FAT32 file system 
creation? 

Thanks,

Andrew Fish

> I found a thread on submitting a "GPT" EFI shell application on this list, 
> but it seems to have trailed off to nowhere about two years ago.
> 
> Did that end up somewhere that I haven't found?
> 
> Herbie Robinson
> Software Architect
> Stratus Technologies | www.stratus.com
> 5 Mill and Main Place, Suite 500 | Maynard, MA 01754
> T: +1-978-461-7531 | E: herbie.robin...@stratus.com
> [Stratus Technologies]
> 
> ___
> 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 v2] SourceLevelDebugPkg/DebugCommunicationLibUsb: Add endpoint configuration.

2018-06-14 Thread Wu, Hao A
Hi Marvin,

Actually, the device we own has the IN/OUT endpoints with 0x82 and 0x01
respectively.

I double checked the value returned in the USB Debug Device Descriptor for the
device I own, and the values are correct, matching the hard-code configuration
in current code.

I think we can directly use the endpoints value from the descriptor, so please
help to work a series without the PCDs, thanks in advance.

Best Regards,
Hao Wu

> -Original Message-
> From: Marvin Häuser [mailto:marvin.haeu...@outlook.com]
> Sent: Thursday, June 14, 2018 11:46 PM
> To: edk2-devel@lists.01.org; Wu, Hao A
> Cc: Ni, Ruiyu
> Subject: RE: [PATCH v2] SourceLevelDebugPkg/DebugCommunicationLibUsb:
> Add endpoint configuration.
> 
> Hey Hao,
> 
> I don't require hardcoded endpoint information, but I have only one debug
> device to test with in the first place.
> The static endpoint option was added in case there are Debug Devices around
> which report incorrect information.
> I assumed that was part of the reason the endpoints were hardcoded in the 
> first
> place.
> 
> Do you want me to submit a V2 without the PCDs?
> 
> Thanks,
> Marvin.
> 
> > -Original Message-
> > From: Wu, Hao A 
> > Sent: Thursday, June 14, 2018 9:03 AM
> > To: Marvin Häuser ; edk2-
> > de...@lists.01.org
> > Cc: Ni, Ruiyu 
> > Subject: RE: [PATCH v2] SourceLevelDebugPkg/DebugCommunicationLibUsb:
> > Add endpoint configuration.
> >
> > Hi Marvin,
> >
> > One thing to confirm with you. For your using case, do you need to hard-
> > code the IN/OUT endpoints for EHCI debug device?
> >
> > If not, I think we can just remove the hard-code endpoints settings in the
> > current code. And use the endpoints returned from the USB Device
> > Descriptor.
> > By doing so, I think we can avoid adding those 2 PCDs.
> >
> > Please let me know your thoughts on this. Thanks in advance.
> >
> > Best Regards,
> > Hao Wu
> >
> > > -Original Message-
> > > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> > > Marvin Häuser
> > > Sent: Thursday, June 14, 2018 3:58 AM
> > > To: edk2-devel@lists.01.org
> > > Cc: Wu, Hao A
> > > Subject: [edk2] [PATCH v2]
> > SourceLevelDebugPkg/DebugCommunicationLibUsb:
> > > Add endpoint configuration.
> > >
> > > Currently, DebugCommunicationLibUsb uses the hardcoded endpoints
> > 0x82
> > > and 0x01 to communicate with the EHCI Debug Device. These, however,
> > > are not standardized and may vary across different hardware.
> > > To solve this problem, two PCDs have been introduced to configure the
> > > in and out endpoints of the EHCI Debug Device. These may be set to 0
> > > to retrieve the endpoints from the USB Device Descriptor directly.
> > > To ensure maximum compatibility, the PCD defaults have been set to the
> > > former hardcoded values.
> > >
> > > V2:
> > >   - Store endpoint data in the USB Debug Port handle sturcture.
> > >
> > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > Signed-off-by: Marvin Haeuser 
> > > ---
> > >
> > >
> > SourceLevelDebugPkg/Library/DebugCommunicationLibUsb/DebugCommun
> > ica
> > > tionLibUsb.c   | 32 ++--
> > >
> > >
> > SourceLevelDebugPkg/Library/DebugCommunicationLibUsb/DebugCommun
> > ica
> > > tionLibUsb.inf |  6 +++-
> > >  SourceLevelDebugPkg/SourceLevelDebugPkg.dec  
> > >  |
> 12
> > > 
> > >  3 files changed, 47 insertions(+), 3 deletions(-)
> > >
> > > diff --git
> > >
> > a/SourceLevelDebugPkg/Library/DebugCommunicationLibUsb/DebugComm
> > uni
> > > cationLibUsb.c
> > >
> > b/SourceLevelDebugPkg/Library/DebugCommunicationLibUsb/DebugComm
> > uni
> > > cationLibUsb.c
> > > index d996f80f59e3..a9a9ea07a39b 100644
> > > ---
> > >
> > a/SourceLevelDebugPkg/Library/DebugCommunicationLibUsb/DebugComm
> > uni
> > > cationLibUsb.c
> > > +++
> > >
> > b/SourceLevelDebugPkg/Library/DebugCommunicationLibUsb/DebugComm
> > uni
> > > cationLibUsb.c
> > > @@ -132,6 +132,14 @@ typedef struct _USB_DEBUG_PORT_HANDLE{
> > >//
> > >UINT32   EhciMemoryBase;
> > >//
> > > +  // The usb debug device In endpoint.
> > > +  //
> > > +  UINT8InEndpoint;
> > > +  //
> > > +  // The usb debug device Out endpoint.
> > > +  //
> > > +  UINT8OutEndpoint;
> > > +  //
> > >// The Bulk In endpoint toggle bit.
> > >//
> > >UINT8BulkInToggle;
> > > @@ -603,6 +611,8 @@ InitializeUsbDebugHardware (
> > >UINT32*UsbHCSParam;
> > >UINT8 DebugPortNumber;
> > >UINT8 Length;
> > > +  UINT8 InEndpoint;
> > > +  UINT8 OutEndpoint;
> > >
> > >UsbDebugPortRegister = (USB_DEBUG_PORT_REGISTER
> > *)((UINTN)Handle-
> > > >UsbDebugPortMemoryBase + Handle->DebugPortOffset);
> > >UsbHCSParam = (UINT32 *)((UINTN)Handle->EhciMemoryBase + 0x04);
> > @@
> > > -722,6 +732,24 @@ InitializeUsbDebugHardware (
> > >return RETURN_DEVICE_ERROR;
> > >  

[edk2] Creating EFI System Partition

2018-06-14 Thread Robinson, Herbie
I have been tasked with implementing EFI boot in our VOS operating system (in a 
panic, nobody bothered to tell us ahead of time that legacy boot was being 
dropped from the BIOS on our latest hardware).  I think we have a pretty good 
handle on writing the bootloader, but I can't find any solid information on 
creating an empty EFI System Partition bearing the correct flavor of FAT32 to 
put the bootloader in.  I need to end up with a binary image file (which I can 
process into an object module and embed into our OS code for initializing 
disks).

I found a thread on submitting a "GPT" EFI shell application on this list, but 
it seems to have trailed off to nowhere about two years ago.

Did that end up somewhere that I haven't found?

Herbie Robinson
Software Architect
Stratus Technologies | www.stratus.com
5 Mill and Main Place, Suite 500 | Maynard, MA 01754
T: +1-978-461-7531 | E: herbie.robin...@stratus.com
[Stratus Technologies]

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [OvmfPkg] Secure Boot issues

2018-06-14 Thread Laszlo Ersek
On 06/13/18 23:25, Philipp Deppenwiese wrote:
> We are getting the error after the Windows 10 installation.
> 
> Windows boots up for few seconds and then crashes.

I've installed a brand new libvirt domain from the installer ISO
discussed below, reusing the virtual hardware config from my previous
Windows 10 domain -- after enabling SB with EnrollDefaultKeys.efi as
very first step.

I've installed a bunch of virtio drivers in the guest. As I mentioned
earlier, Windows would permit the installation of those drivers from the
Fedora virtio-win ISO, but it would also block those drivers from being
launched:

"Windows cannot verify the digital signature for the drivers required
for this device. A recent hardware or software change might have
installed a file that is signed incorrectly or damaged, or that might be
malicious software from an unknown source. (Code 52)"

Using the WHQL'd RHEL variants, the virtio drivers work fine. I also
installed Firefox in the guest, Cygwin (for having an openssh server),
and the QEMU guest agent.

I haven't experienced any issues with the guest, beyond the usual
"Windows Update hogs resources beyond recognition" (once I installed the
virtio-net driver).

Thanks,
Laszlo

> On 13.06.2018 23:18, Laszlo Ersek wrote:
>> On 06/13/18 21:41, Philipp Deppenwiese wrote:
>>> Hey Laszlo,
>>>
>>> It's free for trial and available under:
>>>
>>> https://www.microsoft.com/de-de/evalcenter/evaluate-windows-10-enterprise
>> I tried the following ISO:
>>
>> af9a46ddd2a88ea01e9d3a52f56cf48e6e9d989e5a35f6e88d68be48dccfcb8d
>> 14393.0.160715-1616.rs1_release_cliententerprise_s_eval_x64fre_en-us.iso
>>
>> I simply took my Windows 10 libvirt domain that I used yesterday for
>> testing, and booted it off of the above ISO, rather than the virtual
>> hard disk. I didn't go through with the entire installation, just until
>> it asked for the virtio-win driver CD (so that it could continue reading
>> the virtio-scsi Windows installer ISO, post-EFI), at which point I
>> forced off the VM. Nonetheless, until that point, everything seemed to
>> work fine. Did you encounter the crash after that point, or before it?
>>
>> Thanks
>> Laszlo
>>
>>> On 13.06.2018 21:21, Laszlo Ersek wrote:
 On 06/13/18 15:45, Philipp Deppenwiese wrote:
> Hey Laszlo,
>
> We are using VirtualBox as virtualization solution and
> don't load guest drivers. But we had the same issue with
> the current Qemu version as well.
>
> Can you try to test your setup with the latest Windows 10 LTSB ?
> That would help us to understand if that's a general EDK2
> issue or just our problem.
 My testing yesterday was supposed to cover the "latest in Windows 10
 LTSB" -- I had indeed installed the OS earlier from a Windows 10
 Enterprise N 2015 LTSB ISO, but yesterday that long-term guest of mine
 pulled down all pending updates.

 Is that not what you mean? Can you give me an ISO filename and a SHA1
 checksum then? I could try looking it up in MSDN. (It's not guaranteed
 that my subscription will allow me access to it though.)

 Thanks
 Laszlo
> 
> ___
> 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] UefiCpuPkg/LocalApicLib: Exclude second SendIpi sequence on AMD processors.

2018-06-14 Thread Laszlo Ersek
On 06/13/18 22:11, Leo Duran wrote:
> On AMD processors the second SendIpi in the SendInitSipiSipi and
> SendInitSipiSipiAllExcludingSelf routines is not required, and may cause
> undesired side-effects during MP initialization.
> 
> This patch leverages the StandardSignatureIsAuthenticAMD check to exclude
> the second SendIpi and its associated MicroSecondDelay (200).
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Leo Duran 
> Cc: Jordan Justen 
> Cc: Jeff Fan 
> Cc: Liming Gao 
> ---
>  UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c | 12 
>  UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c | 12 
>  2 files changed, 16 insertions(+), 8 deletions(-)
> 
> diff --git a/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c 
> b/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c
> index b0b7e32..6e80536 100644
> --- a/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c
> +++ b/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c
> @@ -554,8 +554,10 @@ SendInitSipiSipi (
>IcrLow.Bits.DeliveryMode = LOCAL_APIC_DELIVERY_MODE_STARTUP;
>IcrLow.Bits.Level = 1;
>SendIpi (IcrLow.Uint32, ApicId);
> -  MicroSecondDelay (200);
> -  SendIpi (IcrLow.Uint32, ApicId);
> +  if (!StandardSignatureIsAuthenticAMD()) {
> +MicroSecondDelay (200);
> +SendIpi (IcrLow.Uint32, ApicId);
> +  }
>  }
>  
>  /**
> @@ -588,8 +590,10 @@ SendInitSipiSipiAllExcludingSelf (
>IcrLow.Bits.Level = 1;
>IcrLow.Bits.DestinationShorthand = 
> LOCAL_APIC_DESTINATION_SHORTHAND_ALL_EXCLUDING_SELF;
>SendIpi (IcrLow.Uint32, 0);
> -  MicroSecondDelay (200);
> -  SendIpi (IcrLow.Uint32, 0);
> +  if (!StandardSignatureIsAuthenticAMD()) {
> +MicroSecondDelay (200);
> +SendIpi (IcrLow.Uint32, 0);
> +  }
>  }
>  
>  /**
> diff --git a/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c 
> b/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c
> index 1f4dcf7..5d82836 100644
> --- a/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c
> +++ b/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c
> @@ -649,8 +649,10 @@ SendInitSipiSipi (
>IcrLow.Bits.DeliveryMode = LOCAL_APIC_DELIVERY_MODE_STARTUP;
>IcrLow.Bits.Level = 1;
>SendIpi (IcrLow.Uint32, ApicId);
> -  MicroSecondDelay (200);
> -  SendIpi (IcrLow.Uint32, ApicId);
> +  if (!StandardSignatureIsAuthenticAMD()) {
> +MicroSecondDelay (200);
> +SendIpi (IcrLow.Uint32, ApicId);
> +  }
>  }
>  
>  /**
> @@ -683,8 +685,10 @@ SendInitSipiSipiAllExcludingSelf (
>IcrLow.Bits.Level = 1;
>IcrLow.Bits.DestinationShorthand = 
> LOCAL_APIC_DESTINATION_SHORTHAND_ALL_EXCLUDING_SELF;
>SendIpi (IcrLow.Uint32, 0);
> -  MicroSecondDelay (200);
> -  SendIpi (IcrLow.Uint32, 0);
> +  if (!StandardSignatureIsAuthenticAMD()) {
> +MicroSecondDelay (200);
> +SendIpi (IcrLow.Uint32, 0);
> +  }
>  }
>  
>  /**
> 

Given all the feedback (thanks all for that!), I'm fine with the patch.

Reviewed-by: Laszlo Ersek 

I'm only a reviewer for UefiCpuPkg, not a maintainer, so I can't go
ahead and commit the patch just yet. Anyway, I do suggest a small coding
style improvement (which we can do ourselves before we push the patch):
there should be a space character between
"StandardSignatureIsAuthenticAMD" and "()".

Thanks
Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [PATCH] UefiCpuPkg/LocalApicLib: Exclude second SendIpi sequence on AMD processors.

2018-06-14 Thread Brijesh Singh




On 06/14/2018 10:00 AM, Laszlo Ersek wrote:

On 06/14/18 16:52, Andrew Fish wrote:




On Jun 14, 2018, at 7:08 AM, Duran, Leo  wrote:




-Original Message-
From: Laszlo Ersek mailto:ler...@redhat.com>>
Sent: Wednesday, June 13, 2018 3:50 PM
To: Duran, Leo mailto:leo.du...@amd.com>>; 
edk2-devel@lists.01.org 
Cc: Jordan Justen mailto:jordan.l.jus...@intel.com>>; Jeff Fan
mailto:jeff@intel.com>>; Liming Gao mailto:liming@intel.com>>; Singh, Brijesh
mailto:brijesh.si...@amd.com>>; Paolo Bonzini 
mailto:pbonz...@redhat.com>>; Igor
Mammedov mailto:imamm...@redhat.com>>
Subject: Re: [edk2] [PATCH] UefiCpuPkg/LocalApicLib: Exclude second
SendIpi sequence on AMD processors.

Hello Leo,

On 06/13/18 22:11, Leo Duran wrote:

On AMD processors the second SendIpi in the SendInitSipiSipi and
SendInitSipiSipiAllExcludingSelf routines is not required, and may
cause undesired side-effects during MP initialization.

This patch leverages the StandardSignatureIsAuthenticAMD check to
exclude the second SendIpi and its associated MicroSecondDelay (200).


QEMU and KVM emulate some AMD processors too; of particular interest is
the recent EPYC addition, I believe (for SME/SEV, minimally).

Did you check whether the StandardSignatureIsAuthenticAMD() check
applies to those QEMU VCPU models, and if so, whether omitting the second
Startup IPI interferes with *V*CPU startup in OVMF guests? (In
multiprocessing modules, such as CpuMpPei, CpuDxe, and
PiSmmCpuDxeSmm.)

Adding Brijesh, Paolo and Igor.

Thanks!
Laszlo


Hi Lazlo,

My understanding is that hypervisors simply ignore the second SIPI, so a single 
(or double) SIPI should be fine.
In any event, I'm checking with Brijesh on your specific question.



My understanding is the 2nd SIPI was for an Intel processor bug in the mid 
1990's and it has not been required since. People are just scared to change it 
since all the Operating Systems have been historically validated against INT 
SIPI SIPI.

One of my co-works removed our extra SIPI, not knowing the history, and 
everything worked. Well we booted a little faster without the extra SIPI.

If people still have the compatibility concern can we make the 2nd SIPI 
configurable via a PCD. But given the StandardSignatureIsAuthenticAMD() data 
point we should default the 2nd SIPI to off and move the world forward? What do 
folks think?


Given that I asked Brijesh to comment too, I'd like to see his feedback
as well (or Leo to forward Brijesh's findings); then I'm OK with
removing the second SIPI.




I did a quick test with and without SEV enabled, QEMU seems to be 
working fine. As Leo pointed out that the second SIPI is not required on 
AMD processor.


-Brijesh
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [PATCH v2] SourceLevelDebugPkg/DebugCommunicationLibUsb: Add endpoint configuration.

2018-06-14 Thread Marvin Häuser
Hey Hao,

I don't require hardcoded endpoint information, but I have only one debug 
device to test with in the first place.
The static endpoint option was added in case there are Debug Devices around 
which report incorrect information.
I assumed that was part of the reason the endpoints were hardcoded in the first 
place.

Do you want me to submit a V2 without the PCDs?

Thanks,
Marvin.

> -Original Message-
> From: Wu, Hao A 
> Sent: Thursday, June 14, 2018 9:03 AM
> To: Marvin Häuser ; edk2-
> de...@lists.01.org
> Cc: Ni, Ruiyu 
> Subject: RE: [PATCH v2] SourceLevelDebugPkg/DebugCommunicationLibUsb:
> Add endpoint configuration.
> 
> Hi Marvin,
> 
> One thing to confirm with you. For your using case, do you need to hard-
> code the IN/OUT endpoints for EHCI debug device?
> 
> If not, I think we can just remove the hard-code endpoints settings in the
> current code. And use the endpoints returned from the USB Device
> Descriptor.
> By doing so, I think we can avoid adding those 2 PCDs.
> 
> Please let me know your thoughts on this. Thanks in advance.
> 
> Best Regards,
> Hao Wu
> 
> > -Original Message-
> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> > Marvin Häuser
> > Sent: Thursday, June 14, 2018 3:58 AM
> > To: edk2-devel@lists.01.org
> > Cc: Wu, Hao A
> > Subject: [edk2] [PATCH v2]
> SourceLevelDebugPkg/DebugCommunicationLibUsb:
> > Add endpoint configuration.
> >
> > Currently, DebugCommunicationLibUsb uses the hardcoded endpoints
> 0x82
> > and 0x01 to communicate with the EHCI Debug Device. These, however,
> > are not standardized and may vary across different hardware.
> > To solve this problem, two PCDs have been introduced to configure the
> > in and out endpoints of the EHCI Debug Device. These may be set to 0
> > to retrieve the endpoints from the USB Device Descriptor directly.
> > To ensure maximum compatibility, the PCD defaults have been set to the
> > former hardcoded values.
> >
> > V2:
> >   - Store endpoint data in the USB Debug Port handle sturcture.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Marvin Haeuser 
> > ---
> >
> >
> SourceLevelDebugPkg/Library/DebugCommunicationLibUsb/DebugCommun
> ica
> > tionLibUsb.c   | 32 ++--
> >
> >
> SourceLevelDebugPkg/Library/DebugCommunicationLibUsb/DebugCommun
> ica
> > tionLibUsb.inf |  6 +++-
> >  SourceLevelDebugPkg/SourceLevelDebugPkg.dec
> >| 12
> > 
> >  3 files changed, 47 insertions(+), 3 deletions(-)
> >
> > diff --git
> >
> a/SourceLevelDebugPkg/Library/DebugCommunicationLibUsb/DebugComm
> uni
> > cationLibUsb.c
> >
> b/SourceLevelDebugPkg/Library/DebugCommunicationLibUsb/DebugComm
> uni
> > cationLibUsb.c
> > index d996f80f59e3..a9a9ea07a39b 100644
> > ---
> >
> a/SourceLevelDebugPkg/Library/DebugCommunicationLibUsb/DebugComm
> uni
> > cationLibUsb.c
> > +++
> >
> b/SourceLevelDebugPkg/Library/DebugCommunicationLibUsb/DebugComm
> uni
> > cationLibUsb.c
> > @@ -132,6 +132,14 @@ typedef struct _USB_DEBUG_PORT_HANDLE{
> >//
> >UINT32   EhciMemoryBase;
> >//
> > +  // The usb debug device In endpoint.
> > +  //
> > +  UINT8InEndpoint;
> > +  //
> > +  // The usb debug device Out endpoint.
> > +  //
> > +  UINT8OutEndpoint;
> > +  //
> >// The Bulk In endpoint toggle bit.
> >//
> >UINT8BulkInToggle;
> > @@ -603,6 +611,8 @@ InitializeUsbDebugHardware (
> >UINT32*UsbHCSParam;
> >UINT8 DebugPortNumber;
> >UINT8 Length;
> > +  UINT8 InEndpoint;
> > +  UINT8 OutEndpoint;
> >
> >UsbDebugPortRegister = (USB_DEBUG_PORT_REGISTER
> *)((UINTN)Handle-
> > >UsbDebugPortMemoryBase + Handle->DebugPortOffset);
> >UsbHCSParam = (UINT32 *)((UINTN)Handle->EhciMemoryBase + 0x04);
> @@
> > -722,6 +732,24 @@ InitializeUsbDebugHardware (
> >return RETURN_DEVICE_ERROR;
> >  }
> >
> > +//
> > +// Determine the usb debug device endpoints.
> > +//
> > +InEndpoint = PcdGet8 (PcdUsbDebugPortInEndpoint);
> > +
> > +if (InEndpoint == 0) {
> > +  InEndpoint = UsbDebugPortDescriptor.DebugInEndpoint;
> > +}
> > +
> > +OutEndpoint = PcdGet8 (PcdUsbDebugPortOutEndpoint);
> > +
> > +if (OutEndpoint == 0) {
> > +  OutEndpoint = UsbDebugPortDescriptor.DebugOutEndpoint;
> > +}
> > +
> > +Handle->InEndpoint  = InEndpoint;
> > +Handle->OutEndpoint = OutEndpoint;
> > +
> >  //
> >  // enable the usb debug feature.
> >  //
> > @@ -879,7 +907,7 @@ DebugPortWriteBuffer (
> >Sent = (UINT8)(NumberOfBytes - Total);
> >  }
> >
> > -Status = UsbDebugPortOut(UsbDebugPortRegister, Buffer + Total, Sent,
> > OUTPUT_PID, 0x7F, 0x01, UsbDebugPortHandle->BulkOutToggle);
> > +Status = UsbDebugPortOut(UsbDebugPortRegister, Buffer + Total,
> > + Sent,
> > OUTPUT_PID, 0x7F, 

Re: [edk2] [PATCH v1] MdeModulePkg: Enable SATA Controller PCI mem space

2018-06-14 Thread Evan Lloyd
Reviewed-by: Evan Lloyd 

> -Original Message-
> From: Sami Mujawar 
> Sent: 14 June 2018 12:38
> To: edk2-devel@lists.01.org
> Cc: star.z...@intel.com; eric.d...@intel.com; ruiyu...@intel.com;
> ard.biesheu...@linaro.org; leif.lindh...@linaro.org; Matteo Carlini
> ; Stephanie Hughes-Fitt  f...@arm.com>; Evan Lloyd ; Thomas Abraham
> ; nd 
> Subject: [PATCH v1] MdeModulePkg: Enable SATA Controller PCI mem space
> 
> The SATA controller driver crashes while accessing the PCI memory, as the
> PCI memory space is not enabled.
> 
> Enable the PCI memory space access to prevent the SATA Controller driver
> from crashing.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Sami Mujawar 
> ---
> The changes can be seen at
> https://github.com/samimujawar/edk2/tree/284_sata_controler_pci_mem_f
> ix_v1
> 
> Notes:
> v1:
> - Fix SATA Controller driver crash[SAMI]
> 
>  MdeModulePkg/Bus/Pci/SataControllerDxe/SataController.c | 80
> +++-
> MdeModulePkg/Bus/Pci/SataControllerDxe/SataController.h |  7 ++
>  2 files changed, 86 insertions(+), 1 deletion(-)
> 
> diff --git a/MdeModulePkg/Bus/Pci/SataControllerDxe/SataController.c
> b/MdeModulePkg/Bus/Pci/SataControllerDxe/SataController.c
> index
> a6d55c15571728eb3fd572003f383ba7c86635ae..21cc101d693f5adfd9d43f0c2
> 1a096eb59ba73b1 100644
> --- a/MdeModulePkg/Bus/Pci/SataControllerDxe/SataController.c
> +++ b/MdeModulePkg/Bus/Pci/SataControllerDxe/SataController.c
> @@ -2,6 +2,7 @@
>This driver module produces IDE_CONTROLLER_INIT protocol for Sata
> Controllers.
> 
>Copyright (c) 2011 - 2016, Intel Corporation. All rights reserved.
> +  Copyright (c) 2018, ARM Ltd. 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 @@ -364,6 +365,7 @@ SataControllerStart (
>EFI_SATA_CONTROLLER_PRIVATE_DATA  *Private;
>UINT32Data32;
>UINTN TotalCount;
> +  UINT64PciAttributes;
> 
>DEBUG ((EFI_D_INFO, "SataControllerStart start\n"));
> 
> @@ -406,6 +408,61 @@ SataControllerStart (
>Private->IdeInit.CalculateMode  = IdeInitCalculateMode;
>Private->IdeInit.SetTiming  = IdeInitSetTiming;
>Private->IdeInit.EnumAll= SATA_ENUMER_ALL;
> +  Private->PciAttributesChanged   = FALSE;
> +
> +  // Save original PCI attributes
> +  Status = PciIo->Attributes (
> +PciIo,
> +EfiPciIoAttributeOperationGet,
> +0,
> +>OriginalPciAttributes
> +);
> +  if (EFI_ERROR (Status)) {
> +  goto Done;
> +  }
> +
> +  DEBUG ((
> +EFI_D_INFO,
> +"PCI Attributes = 0x%llx\n",
> +Private->OriginalPciAttributes
> +));
> +
> +  if ((Private->OriginalPciAttributes & EFI_PCI_IO_ATTRIBUTE_MEMORY) == 0)
> {
> +Status = PciIo->Attributes (
> +  PciIo,
> +  EfiPciIoAttributeOperationSupported,
> +  0,
> +  
> +  );
> +if (EFI_ERROR (Status)) {
> +  goto Done;
> +}
> +
> +DEBUG ((EFI_D_INFO, "Supported PCI Attributes = 0x%llx\n",
> + PciAttributes));
> +
> +if ((PciAttributes & EFI_PCI_IO_ATTRIBUTE_MEMORY) == 0) {
> +  DEBUG ((
> +EFI_D_ERROR,
> +"Error: EFI_PCI_IO_ATTRIBUTE_MEMORY not supported\n"
> +));
> +  Status = EFI_UNSUPPORTED;
> +  goto Done;
> +}
> +
> +PciAttributes = Private->OriginalPciAttributes |
> + EFI_PCI_IO_ATTRIBUTE_MEMORY;
> +
> +DEBUG ((EFI_D_INFO, "Enable PCI Attributes = 0x%llx\n", PciAttributes));
> +Status = PciIo->Attributes (
> +  PciIo,
> +  EfiPciIoAttributeOperationEnable,
> +  PciAttributes,
> +  NULL
> +  );
> +if (EFI_ERROR (Status)) {
> +  goto Done;
> +}
> +Private->PciAttributesChanged = TRUE;  }
> 
>Status = PciIo->Pci.Read (
>  PciIo,
> @@ -414,7 +471,10 @@ SataControllerStart (
>  sizeof (PciData.Hdr.ClassCode),
>  PciData.Hdr.ClassCode
>  );
> -  ASSERT_EFI_ERROR (Status);
> +  if (EFI_ERROR (Status)) {
> +ASSERT (FALSE);
> +goto Done;
> +  }
> 
>if (IS_PCI_IDE ()) {
>  Private->IdeInit.ChannelCount = IDE_MAX_CHANNEL; @@ -481,6 +541,15
> @@ Done:
>if (Private->IdentifyValid != NULL) {
>  FreePool (Private->IdentifyValid);
>}
> +  if (Private->PciAttributesChanged) {
> +// Restore original PCI attributes
> +PciIo->Attributes (
> + PciIo,
> + EfiPciIoAttributeOperationSet,
> + 

Re: [edk2] [PATCH] UefiCpuPkg/LocalApicLib: Exclude second SendIpi sequence on AMD processors.

2018-06-14 Thread Laszlo Ersek
On 06/14/18 16:52, Andrew Fish wrote:
> 
> 
>> On Jun 14, 2018, at 7:08 AM, Duran, Leo  wrote:
>>
>>
>>
>>> -Original Message-
>>> From: Laszlo Ersek mailto:ler...@redhat.com>>
>>> Sent: Wednesday, June 13, 2018 3:50 PM
>>> To: Duran, Leo mailto:leo.du...@amd.com>>; 
>>> edk2-devel@lists.01.org 
>>> Cc: Jordan Justen >> >; Jeff Fan
>>> mailto:jeff@intel.com>>; Liming Gao 
>>> mailto:liming@intel.com>>; Singh, Brijesh
>>> mailto:brijesh.si...@amd.com>>; Paolo Bonzini 
>>> mailto:pbonz...@redhat.com>>; Igor
>>> Mammedov mailto:imamm...@redhat.com>>
>>> Subject: Re: [edk2] [PATCH] UefiCpuPkg/LocalApicLib: Exclude second
>>> SendIpi sequence on AMD processors.
>>>
>>> Hello Leo,
>>>
>>> On 06/13/18 22:11, Leo Duran wrote:
 On AMD processors the second SendIpi in the SendInitSipiSipi and
 SendInitSipiSipiAllExcludingSelf routines is not required, and may
 cause undesired side-effects during MP initialization.

 This patch leverages the StandardSignatureIsAuthenticAMD check to
 exclude the second SendIpi and its associated MicroSecondDelay (200).
>>>
>>> QEMU and KVM emulate some AMD processors too; of particular interest is
>>> the recent EPYC addition, I believe (for SME/SEV, minimally).
>>>
>>> Did you check whether the StandardSignatureIsAuthenticAMD() check
>>> applies to those QEMU VCPU models, and if so, whether omitting the second
>>> Startup IPI interferes with *V*CPU startup in OVMF guests? (In
>>> multiprocessing modules, such as CpuMpPei, CpuDxe, and
>>> PiSmmCpuDxeSmm.)
>>>
>>> Adding Brijesh, Paolo and Igor.
>>>
>>> Thanks!
>>> Laszlo
>>
>> Hi Lazlo,
>>
>> My understanding is that hypervisors simply ignore the second SIPI, so a 
>> single (or double) SIPI should be fine.
>> In any event, I'm checking with Brijesh on your specific question.
>>
> 
> My understanding is the 2nd SIPI was for an Intel processor bug in the mid 
> 1990's and it has not been required since. People are just scared to change 
> it since all the Operating Systems have been historically validated against 
> INT SIPI SIPI. 
> 
> One of my co-works removed our extra SIPI, not knowing the history, and 
> everything worked. Well we booted a little faster without the extra SIPI. 
> 
> If people still have the compatibility concern can we make the 2nd SIPI 
> configurable via a PCD. But given the StandardSignatureIsAuthenticAMD() data 
> point we should default the 2nd SIPI to off and move the world forward? What 
> do folks think?

Given that I asked Brijesh to comment too, I'd like to see his feedback
as well (or Leo to forward Brijesh's findings); then I'm OK with
removing the second SIPI.

Thanks
Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [PATCH] UefiCpuPkg/LocalApicLib: Exclude second SendIpi sequence on AMD processors.

2018-06-14 Thread Andrew Fish



> On Jun 14, 2018, at 7:08 AM, Duran, Leo  wrote:
> 
> 
> 
>> -Original Message-
>> From: Laszlo Ersek mailto:ler...@redhat.com>>
>> Sent: Wednesday, June 13, 2018 3:50 PM
>> To: Duran, Leo mailto:leo.du...@amd.com>>; 
>> edk2-devel@lists.01.org 
>> Cc: Jordan Justen > >; Jeff Fan
>> mailto:jeff@intel.com>>; Liming Gao 
>> mailto:liming@intel.com>>; Singh, Brijesh
>> mailto:brijesh.si...@amd.com>>; Paolo Bonzini 
>> mailto:pbonz...@redhat.com>>; Igor
>> Mammedov mailto:imamm...@redhat.com>>
>> Subject: Re: [edk2] [PATCH] UefiCpuPkg/LocalApicLib: Exclude second
>> SendIpi sequence on AMD processors.
>> 
>> Hello Leo,
>> 
>> On 06/13/18 22:11, Leo Duran wrote:
>>> On AMD processors the second SendIpi in the SendInitSipiSipi and
>>> SendInitSipiSipiAllExcludingSelf routines is not required, and may
>>> cause undesired side-effects during MP initialization.
>>> 
>>> This patch leverages the StandardSignatureIsAuthenticAMD check to
>>> exclude the second SendIpi and its associated MicroSecondDelay (200).
>> 
>> QEMU and KVM emulate some AMD processors too; of particular interest is
>> the recent EPYC addition, I believe (for SME/SEV, minimally).
>> 
>> Did you check whether the StandardSignatureIsAuthenticAMD() check
>> applies to those QEMU VCPU models, and if so, whether omitting the second
>> Startup IPI interferes with *V*CPU startup in OVMF guests? (In
>> multiprocessing modules, such as CpuMpPei, CpuDxe, and
>> PiSmmCpuDxeSmm.)
>> 
>> Adding Brijesh, Paolo and Igor.
>> 
>> Thanks!
>> Laszlo
> 
> Hi Lazlo,
> 
> My understanding is that hypervisors simply ignore the second SIPI, so a 
> single (or double) SIPI should be fine.
> In any event, I'm checking with Brijesh on your specific question.
> 

My understanding is the 2nd SIPI was for an Intel processor bug in the mid 
1990's and it has not been required since. People are just scared to change it 
since all the Operating Systems have been historically validated against INT 
SIPI SIPI. 

One of my co-works removed our extra SIPI, not knowing the history, and 
everything worked. Well we booted a little faster without the extra SIPI. 

If people still have the compatibility concern can we make the 2nd SIPI 
configurable via a PCD. But given the StandardSignatureIsAuthenticAMD() data 
point we should default the 2nd SIPI to off and move the world forward? What do 
folks think?

Thanks,

Andrew Fish

PS If I'm remembering correctly Mark Doran (of UEFI Fame) help lead the MP Spec 
back in the 1990s that 1st documented the INIT SIPI SIPI so we could get some 
more history from him if needed. 

> Leo.
> 
>> 
>>> 
>>> Contributed-under: TianoCore Contribution Agreement 1.1
>>> Signed-off-by: Leo Duran 
>>> Cc: Jordan Justen 
>>> Cc: Jeff Fan 
>>> Cc: Liming Gao 
>>> ---
>>> UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c | 12 
>>> UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c | 12
>>> 
>>> 2 files changed, 16 insertions(+), 8 deletions(-)
>>> 
>>> diff --git a/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c
>>> b/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c
>>> index b0b7e32..6e80536 100644
>>> --- a/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c
>>> +++ b/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c
>>> @@ -554,8 +554,10 @@ SendInitSipiSipi (
>>>   IcrLow.Bits.DeliveryMode = LOCAL_APIC_DELIVERY_MODE_STARTUP;
>>>   IcrLow.Bits.Level = 1;
>>>   SendIpi (IcrLow.Uint32, ApicId);
>>> -  MicroSecondDelay (200);
>>> -  SendIpi (IcrLow.Uint32, ApicId);
>>> +  if (!StandardSignatureIsAuthenticAMD()) {
>>> +MicroSecondDelay (200);
>>> +SendIpi (IcrLow.Uint32, ApicId);
>>> +  }
>>> }
>>> 
>>> /**
>>> @@ -588,8 +590,10 @@ SendInitSipiSipiAllExcludingSelf (
>>>   IcrLow.Bits.Level = 1;
>>>   IcrLow.Bits.DestinationShorthand =
>> LOCAL_APIC_DESTINATION_SHORTHAND_ALL_EXCLUDING_SELF;
>>>   SendIpi (IcrLow.Uint32, 0);
>>> -  MicroSecondDelay (200);
>>> -  SendIpi (IcrLow.Uint32, 0);
>>> +  if (!StandardSignatureIsAuthenticAMD()) {
>>> +MicroSecondDelay (200);
>>> +SendIpi (IcrLow.Uint32, 0);
>>> +  }
>>> }
>>> 
>>> /**
>>> diff --git
>>> a/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c
>>> b/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c
>>> index 1f4dcf7..5d82836 100644
>>> --- a/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c
>>> +++ b/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c
>>> @@ -649,8 +649,10 @@ SendInitSipiSipi (
>>>   IcrLow.Bits.DeliveryMode = LOCAL_APIC_DELIVERY_MODE_STARTUP;
>>>   IcrLow.Bits.Level = 1;
>>>   SendIpi (IcrLow.Uint32, ApicId);
>>> -  MicroSecondDelay (200);
>>> -  SendIpi (IcrLow.Uint32, ApicId);
>>> +  if (!StandardSignatureIsAuthenticAMD()) {
>>> +MicroSecondDelay (200);
>>> +SendIpi (IcrLow.Uint32, ApicId);
>>> +  }
>>> }
>>> 
>>> /**
>>> @@ -683,8 +685,10 @@ SendInitSipiSipiAllExcludingSelf (
>>>   IcrLow.Bits.Level = 1;
>>>   

Re: [edk2] [PATCH] UefiCpuPkg/LocalApicLib: Exclude second SendIpi sequence on AMD processors.

2018-06-14 Thread Duran, Leo



> -Original Message-
> From: Laszlo Ersek 
> Sent: Wednesday, June 13, 2018 3:50 PM
> To: Duran, Leo ; edk2-devel@lists.01.org
> Cc: Jordan Justen ; Jeff Fan
> ; Liming Gao ; Singh, Brijesh
> ; Paolo Bonzini ; Igor
> Mammedov 
> Subject: Re: [edk2] [PATCH] UefiCpuPkg/LocalApicLib: Exclude second
> SendIpi sequence on AMD processors.
> 
> Hello Leo,
> 
> On 06/13/18 22:11, Leo Duran wrote:
> > On AMD processors the second SendIpi in the SendInitSipiSipi and
> > SendInitSipiSipiAllExcludingSelf routines is not required, and may
> > cause undesired side-effects during MP initialization.
> >
> > This patch leverages the StandardSignatureIsAuthenticAMD check to
> > exclude the second SendIpi and its associated MicroSecondDelay (200).
> 
> QEMU and KVM emulate some AMD processors too; of particular interest is
> the recent EPYC addition, I believe (for SME/SEV, minimally).
> 
> Did you check whether the StandardSignatureIsAuthenticAMD() check
> applies to those QEMU VCPU models, and if so, whether omitting the second
> Startup IPI interferes with *V*CPU startup in OVMF guests? (In
> multiprocessing modules, such as CpuMpPei, CpuDxe, and
> PiSmmCpuDxeSmm.)
> 
> Adding Brijesh, Paolo and Igor.
> 
> Thanks!
> Laszlo

Hi Lazlo,

My understanding is that hypervisors simply ignore the second SIPI, so a single 
(or double) SIPI should be fine.
In any event, I'm checking with Brijesh on your specific question.

Leo.

> 
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Leo Duran 
> > Cc: Jordan Justen 
> > Cc: Jeff Fan 
> > Cc: Liming Gao 
> > ---
> >  UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c | 12 
> > 
> >  UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c | 12
> > 
> >  2 files changed, 16 insertions(+), 8 deletions(-)
> >
> > diff --git a/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c
> > b/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c
> > index b0b7e32..6e80536 100644
> > --- a/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c
> > +++ b/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c
> > @@ -554,8 +554,10 @@ SendInitSipiSipi (
> >IcrLow.Bits.DeliveryMode = LOCAL_APIC_DELIVERY_MODE_STARTUP;
> >IcrLow.Bits.Level = 1;
> >SendIpi (IcrLow.Uint32, ApicId);
> > -  MicroSecondDelay (200);
> > -  SendIpi (IcrLow.Uint32, ApicId);
> > +  if (!StandardSignatureIsAuthenticAMD()) {
> > +MicroSecondDelay (200);
> > +SendIpi (IcrLow.Uint32, ApicId);
> > +  }
> >  }
> >
> >  /**
> > @@ -588,8 +590,10 @@ SendInitSipiSipiAllExcludingSelf (
> >IcrLow.Bits.Level = 1;
> >IcrLow.Bits.DestinationShorthand =
> LOCAL_APIC_DESTINATION_SHORTHAND_ALL_EXCLUDING_SELF;
> >SendIpi (IcrLow.Uint32, 0);
> > -  MicroSecondDelay (200);
> > -  SendIpi (IcrLow.Uint32, 0);
> > +  if (!StandardSignatureIsAuthenticAMD()) {
> > +MicroSecondDelay (200);
> > +SendIpi (IcrLow.Uint32, 0);
> > +  }
> >  }
> >
> >  /**
> > diff --git
> > a/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c
> > b/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c
> > index 1f4dcf7..5d82836 100644
> > --- a/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c
> > +++ b/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c
> > @@ -649,8 +649,10 @@ SendInitSipiSipi (
> >IcrLow.Bits.DeliveryMode = LOCAL_APIC_DELIVERY_MODE_STARTUP;
> >IcrLow.Bits.Level = 1;
> >SendIpi (IcrLow.Uint32, ApicId);
> > -  MicroSecondDelay (200);
> > -  SendIpi (IcrLow.Uint32, ApicId);
> > +  if (!StandardSignatureIsAuthenticAMD()) {
> > +MicroSecondDelay (200);
> > +SendIpi (IcrLow.Uint32, ApicId);
> > +  }
> >  }
> >
> >  /**
> > @@ -683,8 +685,10 @@ SendInitSipiSipiAllExcludingSelf (
> >IcrLow.Bits.Level = 1;
> >IcrLow.Bits.DestinationShorthand =
> LOCAL_APIC_DESTINATION_SHORTHAND_ALL_EXCLUDING_SELF;
> >SendIpi (IcrLow.Uint32, 0);
> > -  MicroSecondDelay (200);
> > -  SendIpi (IcrLow.Uint32, 0);
> > +  if (!StandardSignatureIsAuthenticAMD()) {
> > +MicroSecondDelay (200);
> > +SendIpi (IcrLow.Uint32, 0);
> > +  }
> >  }
> >
> >  /**
> >

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [PATCH v1] MdeModulePkg: Enable SATA Controller PCI mem space

2018-06-14 Thread Thomas Abraham
On Thu, Jun 14, 2018 at 5:08 PM, Sami Mujawar  wrote:
> The SATA controller driver crashes while accessing the
> PCI memory, as the PCI memory space is not enabled.
>
> Enable the PCI memory space access to prevent the SATA
> Controller driver from crashing.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Sami Mujawar 
> ---
> The changes can be seen at 
> https://github.com/samimujawar/edk2/tree/284_sata_controler_pci_mem_fix_v1
>
> Notes:
> v1:
> - Fix SATA Controller driver crash[SAMI]
>
>  MdeModulePkg/Bus/Pci/SataControllerDxe/SataController.c | 80 
> +++-
>  MdeModulePkg/Bus/Pci/SataControllerDxe/SataController.h |  7 ++
>  2 files changed, 86 insertions(+), 1 deletion(-)
>
> diff --git a/MdeModulePkg/Bus/Pci/SataControllerDxe/SataController.c 
> b/MdeModulePkg/Bus/Pci/SataControllerDxe/SataController.c
> index 
> a6d55c15571728eb3fd572003f383ba7c86635ae..21cc101d693f5adfd9d43f0c21a096eb59ba73b1
>  100644
> --- a/MdeModulePkg/Bus/Pci/SataControllerDxe/SataController.c
> +++ b/MdeModulePkg/Bus/Pci/SataControllerDxe/SataController.c
> @@ -2,6 +2,7 @@
>This driver module produces IDE_CONTROLLER_INIT protocol for Sata 
> Controllers.
>
>Copyright (c) 2011 - 2016, Intel Corporation. All rights reserved.
> +  Copyright (c) 2018, ARM Ltd. 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
> @@ -364,6 +365,7 @@ SataControllerStart (
>EFI_SATA_CONTROLLER_PRIVATE_DATA  *Private;
>UINT32Data32;
>UINTN TotalCount;
> +  UINT64PciAttributes;
>
>DEBUG ((EFI_D_INFO, "SataControllerStart start\n"));
>
> @@ -406,6 +408,61 @@ SataControllerStart (
>Private->IdeInit.CalculateMode  = IdeInitCalculateMode;
>Private->IdeInit.SetTiming  = IdeInitSetTiming;
>Private->IdeInit.EnumAll= SATA_ENUMER_ALL;
> +  Private->PciAttributesChanged   = FALSE;
> +
> +  // Save original PCI attributes
> +  Status = PciIo->Attributes (
> +PciIo,
> +EfiPciIoAttributeOperationGet,
> +0,
> +>OriginalPciAttributes
> +);
> +  if (EFI_ERROR (Status)) {
> +  goto Done;
> +  }
> +
> +  DEBUG ((
> +EFI_D_INFO,
> +"PCI Attributes = 0x%llx\n",
> +Private->OriginalPciAttributes
> +));
> +
> +  if ((Private->OriginalPciAttributes & EFI_PCI_IO_ATTRIBUTE_MEMORY) == 0) {
> +Status = PciIo->Attributes (
> +  PciIo,
> +  EfiPciIoAttributeOperationSupported,
> +  0,
> +  
> +  );
> +if (EFI_ERROR (Status)) {
> +  goto Done;
> +}
> +
> +DEBUG ((EFI_D_INFO, "Supported PCI Attributes = 0x%llx\n", 
> PciAttributes));
> +
> +if ((PciAttributes & EFI_PCI_IO_ATTRIBUTE_MEMORY) == 0) {
> +  DEBUG ((
> +EFI_D_ERROR,
> +"Error: EFI_PCI_IO_ATTRIBUTE_MEMORY not supported\n"
> +));
> +  Status = EFI_UNSUPPORTED;
> +  goto Done;
> +}
> +
> +PciAttributes = Private->OriginalPciAttributes | 
> EFI_PCI_IO_ATTRIBUTE_MEMORY;
> +
> +DEBUG ((EFI_D_INFO, "Enable PCI Attributes = 0x%llx\n", PciAttributes));
> +Status = PciIo->Attributes (
> +  PciIo,
> +  EfiPciIoAttributeOperationEnable,
> +  PciAttributes,
> +  NULL
> +  );
> +if (EFI_ERROR (Status)) {
> +  goto Done;
> +}
> +Private->PciAttributesChanged = TRUE;
> +  }
>
>Status = PciIo->Pci.Read (
>  PciIo,
> @@ -414,7 +471,10 @@ SataControllerStart (
>  sizeof (PciData.Hdr.ClassCode),
>  PciData.Hdr.ClassCode
>  );
> -  ASSERT_EFI_ERROR (Status);
> +  if (EFI_ERROR (Status)) {
> +ASSERT (FALSE);
> +goto Done;
> +  }
>
>if (IS_PCI_IDE ()) {
>  Private->IdeInit.ChannelCount = IDE_MAX_CHANNEL;
> @@ -481,6 +541,15 @@ Done:
>if (Private->IdentifyValid != NULL) {
>  FreePool (Private->IdentifyValid);
>}
> +  if (Private->PciAttributesChanged) {
> +// Restore original PCI attributes
> +PciIo->Attributes (
> + PciIo,
> + EfiPciIoAttributeOperationSet,
> + Private->OriginalPciAttributes,
> + NULL
> + );
> +  }
>FreePool (Private);
>  }
>}
> @@ -556,6 +625,15 @@ SataControllerStop (
>  if (Private->IdentifyValid != NULL) {
>FreePool (Private->IdentifyValid);
>  }
> +if (Private->PciAttributesChanged) {
> +  // Restore original PCI attributes
> +  

[edk2] [PATCH v1] MdeModulePkg: Enable SATA Controller PCI mem space

2018-06-14 Thread Sami Mujawar
The SATA controller driver crashes while accessing the
PCI memory, as the PCI memory space is not enabled.

Enable the PCI memory space access to prevent the SATA
Controller driver from crashing.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Sami Mujawar 
---
The changes can be seen at 
https://github.com/samimujawar/edk2/tree/284_sata_controler_pci_mem_fix_v1

Notes:
v1:
- Fix SATA Controller driver crash[SAMI]

 MdeModulePkg/Bus/Pci/SataControllerDxe/SataController.c | 80 
+++-
 MdeModulePkg/Bus/Pci/SataControllerDxe/SataController.h |  7 ++
 2 files changed, 86 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Bus/Pci/SataControllerDxe/SataController.c 
b/MdeModulePkg/Bus/Pci/SataControllerDxe/SataController.c
index 
a6d55c15571728eb3fd572003f383ba7c86635ae..21cc101d693f5adfd9d43f0c21a096eb59ba73b1
 100644
--- a/MdeModulePkg/Bus/Pci/SataControllerDxe/SataController.c
+++ b/MdeModulePkg/Bus/Pci/SataControllerDxe/SataController.c
@@ -2,6 +2,7 @@
   This driver module produces IDE_CONTROLLER_INIT protocol for Sata 
Controllers.
 
   Copyright (c) 2011 - 2016, Intel Corporation. All rights reserved.
+  Copyright (c) 2018, ARM Ltd. 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
@@ -364,6 +365,7 @@ SataControllerStart (
   EFI_SATA_CONTROLLER_PRIVATE_DATA  *Private;
   UINT32Data32;
   UINTN TotalCount;
+  UINT64PciAttributes;
 
   DEBUG ((EFI_D_INFO, "SataControllerStart start\n"));
 
@@ -406,6 +408,61 @@ SataControllerStart (
   Private->IdeInit.CalculateMode  = IdeInitCalculateMode;
   Private->IdeInit.SetTiming  = IdeInitSetTiming;
   Private->IdeInit.EnumAll= SATA_ENUMER_ALL;
+  Private->PciAttributesChanged   = FALSE;
+
+  // Save original PCI attributes
+  Status = PciIo->Attributes (
+PciIo,
+EfiPciIoAttributeOperationGet,
+0,
+>OriginalPciAttributes
+);
+  if (EFI_ERROR (Status)) {
+  goto Done;
+  }
+
+  DEBUG ((
+EFI_D_INFO,
+"PCI Attributes = 0x%llx\n",
+Private->OriginalPciAttributes
+));
+
+  if ((Private->OriginalPciAttributes & EFI_PCI_IO_ATTRIBUTE_MEMORY) == 0) {
+Status = PciIo->Attributes (
+  PciIo,
+  EfiPciIoAttributeOperationSupported,
+  0,
+  
+  );
+if (EFI_ERROR (Status)) {
+  goto Done;
+}
+
+DEBUG ((EFI_D_INFO, "Supported PCI Attributes = 0x%llx\n", PciAttributes));
+
+if ((PciAttributes & EFI_PCI_IO_ATTRIBUTE_MEMORY) == 0) {
+  DEBUG ((
+EFI_D_ERROR,
+"Error: EFI_PCI_IO_ATTRIBUTE_MEMORY not supported\n"
+));
+  Status = EFI_UNSUPPORTED;
+  goto Done;
+}
+
+PciAttributes = Private->OriginalPciAttributes | 
EFI_PCI_IO_ATTRIBUTE_MEMORY;
+
+DEBUG ((EFI_D_INFO, "Enable PCI Attributes = 0x%llx\n", PciAttributes));
+Status = PciIo->Attributes (
+  PciIo,
+  EfiPciIoAttributeOperationEnable,
+  PciAttributes,
+  NULL
+  );
+if (EFI_ERROR (Status)) {
+  goto Done;
+}
+Private->PciAttributesChanged = TRUE;
+  }
 
   Status = PciIo->Pci.Read (
 PciIo,
@@ -414,7 +471,10 @@ SataControllerStart (
 sizeof (PciData.Hdr.ClassCode),
 PciData.Hdr.ClassCode
 );
-  ASSERT_EFI_ERROR (Status);
+  if (EFI_ERROR (Status)) {
+ASSERT (FALSE);
+goto Done;
+  }
 
   if (IS_PCI_IDE ()) {
 Private->IdeInit.ChannelCount = IDE_MAX_CHANNEL;
@@ -481,6 +541,15 @@ Done:
   if (Private->IdentifyValid != NULL) {
 FreePool (Private->IdentifyValid);
   }
+  if (Private->PciAttributesChanged) {
+// Restore original PCI attributes
+PciIo->Attributes (
+ PciIo,
+ EfiPciIoAttributeOperationSet,
+ Private->OriginalPciAttributes,
+ NULL
+ );
+  }
   FreePool (Private);
 }
   }
@@ -556,6 +625,15 @@ SataControllerStop (
 if (Private->IdentifyValid != NULL) {
   FreePool (Private->IdentifyValid);
 }
+if (Private->PciAttributesChanged) {
+  // Restore original PCI attributes
+  Private->PciIo->Attributes (
+Private->PciIo,
+EfiPciIoAttributeOperationSet,
+Private->OriginalPciAttributes,
+NULL
+);
+}
 FreePool (Private);
   }
 
diff --git 

Re: [edk2] [PATCH v3 2/4] OvmfPkg: add QemuRamfbDxe

2018-06-14 Thread Laszlo Ersek
On 06/13/18 22:30, Ard Biesheuvel wrote:
> On 13 June 2018 at 21:15, Laszlo Ersek  wrote:
>> On 06/13/18 21:03, Laszlo Ersek wrote:
>>> On 06/13/18 20:20, Laszlo Ersek wrote:
>>>
 * testing on aarch64/KVM:

 The graphics output looks great as long as I'm in the UEFI shell / the
 setup TUI / the grub menu. However, once I boot a Fedora 28 Server
 ISO, and the graphical Anacona Welcome screen appears, I get the exact
 same display corruption as before, with QemuVideoDxe + the VGA device
 model. I don't have the slightest idea why that is the case, but it's
 very visible, if I quickly move the thick blue line cursor around the
 language and keyboard layout selection lists. It's visible to the
 naked eye how dirty memory is gradually flushed to the display.
>>
>> Small (or not so small) technical correction: the problem is that the
>> guest OS writes directly to DRAM, but QEMU reads from the CPU cache. So
>> I guess the gradual process that is visible to the naked eye is not
>> "flushing" but "invalidation"; i.e. as the CPU caches become invalid,
>> QEMU is gradually forced to reach out to DRAM and finally see what the
>> guest OS put there. I guess.
>>
>> More below:
>>
>>>
>>> Wait, I see "efifb" has a parameter called "nowc", and it disables
>>> write-combining, according to "Documentation/fb/efifb.txt".
>>>
>>> Looking at the source code ("drivers/video/fbdev/efifb.c"), "nowc"
>>> decides between:
>>>
  if (nowc)
  info->screen_base = ioremap(efifb_fix.smem_start, 
 efifb_fix.smem_len);
  else
  info->screen_base = ioremap_wc(efifb_fix.smem_start, 
 efifb_fix.smem_len);
>>>
>>> Am I right to think that *both* of these ioremap() variants map the
>>> phsyical address range as uncache-able? ("nowc" defaults to "false", and
>>> I didn't specify "nowc" at all on the kernel command line.)
>>>
>>> Quoting "arch/arm/include/asm/io.h":
>>>
  * Function Memory type CacheabilityCache hint
  * ioremap()Device  n/a n/a
  * ioremap_nocache()Device  n/a n/a
  * ioremap_cache()  Normal  Writeback   Read allocate
  * ioremap_wc() Normal  Non-cacheable   n/a
  * ioremap_wt() Normal  Non-cacheable   n/a
>>>
>>> ioremap() implies "device memory" (by definition uncacheable only),
>>> while ioremap_wc() is normal memory, but UC. Sigh.
>>>
>>> The include file "arch/arm64/include/asm/io.h" doesn't have such helpful
>>> comments, but it does declare ioremap_cache() at least:
>>>
 extern void __iomem *ioremap_cache(phys_addr_t phys_addr, size_t size);
>>>
>>> Now, I *guess* if I rebuilt the efifb driver to use ioremap_cache() --
>>> dependent on a new module parameter or some such --, the Linux guest
>>> would start working as expected. Unfortunately, the Linux guest is
>>> already pretty happy with virtio-gpu-pci; the question is how the
>>> Windows guest would map the EFI framebuffer!
>>>
>>> Unfortunately, I cannot test ARM64 Windows guests ATM.
>>>
>>> So... If the consensus is that the edk2 code simply cannot get better
>>> than this, and everything else is up to the guest OS(es), then I'm 100%
>>> willing to push this version (with my minimal updates squashed).
>>
>> I've just remembered that Drew drew our attention earlier to:
>>
>>   [PATCH 0/4] KVM/arm64: Cache maintenance relaxations
>>   http://mid.mail-archive.com/20180517103548.5622-1-marc.zyngier@arm.com
>>
>> I believe this means that, on an ARMv8.4 host, the ramfb device model
>> will automatically work, in all guest OSes. If that's the case, I
>> suggest we merge this series.
>>
> 
> I am not sure how I managed to confuse myself into thinking that ramfb
> would solve the coherency issue on ARM, but unfortunately, the
> observed behavior is exactly as expected. The framebuffer is still
> located in RAM that is mapped cacheable by the host and non-cacheable
> by the guest.
> 
> There is a notable difference though: this time, the efifb framebuffer
> is identifiable as ordinary [although reserved] memory by the guest,
> and so I think it is reasonable for the efifb driver to check whether
> the memory is covered by a region in the UEFI memory map that has the
> EFI_MEMORY_WB attribute, in which case it is permitted to use a
> cacheable mapping.
> 

OK. Thank you for pointing this out!

So, with the testing and related discussion up-thread, for the series:

Tested-by: Laszlo Ersek 

Pushed as commit range e03a460f061a..c64688f36a8b.

In addition, I've now closed
.

Thank you very much Gerd for this work!
Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


[edk2] [EDK2][UEFI_Shell_Specification_2.2]the fs-dirs define may be not accurate.

2018-06-14 Thread krishnaLee
Hi,
  Recently,I want to exactly know what is filepath/filename,I come across this 
specification,
Follow information come from UEFI_Shell_2.2.pdf,


3.7 File Names
fs-path  := [fs-map-name] [fs-divider][fs-dirs][fs-name]
fs-map-name  := identifier :
fs-divider:= \ | /
fs-dirs := fs-dir | 
 fs-dirs fs-dir
fs-dir := fs-name fs-divider
fs-name   := fs-file-name .fs-file-ext
fs-file-name := one or more ASCII characters, excluding * ? < > \ / ” : )


#I want to say follow  example is a fs-dirs,relatively;
fo\foo\   


#but follow example is also a fs-dirs,absolutely;
FS0:\fo\foo\


#so the define should be:
fs-dirs-absolute := fs-map-name fs-divider | fs-map-name fs-divider 
fs-dirs-relative
fs-dirs-relative   := fs-dir | 
  fs-dirs-relative fs-dir
fs-dir  := fs-name fs-divider


#is that right?


thanks
by krishna
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [PATCH] UefiCpuPkg/LocalApicLib: Exclude second SendIpi sequence on AMD processors.

2018-06-14 Thread Paolo Bonzini
On 14/06/2018 07:39, Ni, Ruiyu wrote:
> 
> 
> Thanks/Ray
> 
>> -Original Message-
>> From: edk2-devel  On Behalf Of Paolo
>> Bonzini
>> Sent: Thursday, June 14, 2018 4:52 AM
>> To: Laszlo Ersek ; Leo Duran ;
>> edk2-devel@lists.01.org
>> Cc: Justen, Jordan L ; Brijesh Singh
>> ; Jeff Fan ; Gao, Liming
>> 
>> Subject: Re: [edk2] [PATCH] UefiCpuPkg/LocalApicLib: Exclude second
>> SendIpi sequence on AMD processors.
>>
>> On 13/06/2018 22:49, Laszlo Ersek wrote:
>>> Hello Leo,
>>>
>>> On 06/13/18 22:11, Leo Duran wrote:
 On AMD processors the second SendIpi in the SendInitSipiSipi and
 SendInitSipiSipiAllExcludingSelf routines is not required, and may
 cause undesired side-effects during MP initialization.

 This patch leverages the StandardSignatureIsAuthenticAMD check to
 exclude the second SendIpi and its associated MicroSecondDelay (200).
>>>
>>> QEMU and KVM emulate some AMD processors too; of particular interest
>>> is the recent EPYC addition, I believe (for SME/SEV, minimally).
>>>
>>> Did you check whether the StandardSignatureIsAuthenticAMD() check
>>> applies to those QEMU VCPU models, and if so, whether omitting the
>>> second Startup IPI interferes with *V*CPU startup in OVMF guests? (In
>>> multiprocessing modules, such as CpuMpPei, CpuDxe, and
>>> PiSmmCpuDxeSmm.)
>>>
>>> Adding Brijesh, Paolo and Igor.
>>
>> Actually I would be surprised if any recent processor needs the 
>> INIT-SIPI-SIPI
>> (though I'm not sure what undesired side effects it might trigger).
> 
> Why the recent processors don't need INIT-SIPI-SIPI?
> I thought it is the only way to wake up processors when it's halted (HLT).

INIT-SIPI should be enough.  The second SIPI is just in case the first
one was missed.

Paolo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [Patch] FDF spec: Add the syntax to describe structure pcd usage

2018-06-14 Thread Zhu, Yonghong
Reviewed-by: Yonghong Zhu  

Best Regards,
Zhu Yonghong


-Original Message-
From: Gao, Liming 
Sent: Tuesday, June 05, 2018 3:33 PM
To: edk2-devel@lists.01.org
Cc: Zhu, Yonghong 
Subject: [Patch] FDF spec: Add the syntax to describe structure pcd usage

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Liming Gao 
Cc: Yonghong Zhu 
---
 2_fdf_design_discussion/21_processing_overview.md   | 1 +
 2_fdf_design_discussion/22_flash_description_file_format.md | 6 +++---
 2_fdf_design_discussion/24_[fd]_sections.md | 5 +++--
 3_edk_ii_fdf_file_format/32_fdf_definition.md   | 2 ++
 3_edk_ii_fdf_file_format/35_[fd]_sections.md| 8 
 3_edk_ii_fdf_file_format/36_[fv]_sections.md| 2 +-
 3_edk_ii_fdf_file_format/37_[capsule]_sections.md   | 2 +-
 7 files changed, 15 insertions(+), 11 deletions(-)

diff --git a/2_fdf_design_discussion/21_processing_overview.md 
b/2_fdf_design_discussion/21_processing_overview.md
index 7a045dd..68d70f0 100644
--- a/2_fdf_design_discussion/21_processing_overview.md
+++ b/2_fdf_design_discussion/21_processing_overview.md
@@ -108,6 +108,7 @@ FDF file.
 The PCDs used in the FDF file must be specified as:
 
 `PcdTokenSpaceGuidCName.PcdCName`
+or `PcdTokenSpaceGuidCName.PcdCName.PcdFieldName`
 
 ### 2.1.2 Precedence of PCD Values
 
diff --git a/2_fdf_design_discussion/22_flash_description_file_format.md 
b/2_fdf_design_discussion/22_flash_description_file_format.md
index 368ce32..f266460 100644
--- a/2_fdf_design_discussion/22_flash_description_file_format.md
+++ b/2_fdf_design_discussion/22_flash_description_file_format.md
@@ -492,9 +492,9 @@ Unique PCDs are identified using the format to identify the 
named PCD:
 
 `PcdTokenSpaceGuidCName.PcdCName`
 
-The PCD's Name (`PcdName`) is defined as PCD Token Space Guid C name and the 
-PCD C name - separated by a period "." character. PCD C names are used in C 
-code and must follow the C variable name rules.
+The PCD's Name (`PcdName`) is defined as PCD Token Space Guid C name, 
+the PCD C name and the optional field name - separated by a period "." 
character.
+PCD C names are used in C code and must follow the C variable name rules.
 
 A PCD's values are positional with in the FDF file, and may be set by either  
the automatic setting grammar defined in this specification, or through `SET` 
diff --git a/2_fdf_design_discussion/24_[fd]_sections.md 
b/2_fdf_design_discussion/24_[fd]_sections.md
index 04053a0..e532041 100644
--- a/2_fdf_design_discussion/24_[fd]_sections.md
+++ b/2_fdf_design_discussion/24_[fd]_sections.md
@@ -145,10 +145,11 @@ region, so the `SET` statement can occur anywhere within 
an FD section.
 
 `SET PcdName = VALUE`
 
-Additionally, a PCD Name is made up of two parts, separated by a period "."
-character. The format for a `PcdName` is:
+Additionally, a PCD Name is made up of two parts or three parts, 
+separated by a period "." character. The format for a `PcdName` is:
 
 `PcdTokenSpaceGuidCName.PcdCName`
+or `PcdTokenSpaceGuidCName.PcdCName.PcdFieldName`
 
 The following is an example of the `SET` statement:
 
diff --git a/3_edk_ii_fdf_file_format/32_fdf_definition.md 
b/3_edk_ii_fdf_file_format/32_fdf_definition.md
index 1379db4..6506a3d 100644
--- a/3_edk_ii_fdf_file_format/32_fdf_definition.md
+++ b/3_edk_ii_fdf_file_format/32_fdf_definition.md
@@ -174,9 +174,11 @@ The following are common definitions used by multiple 
section types.
  ::= {} {}
 ::= (A-Z)(A-Z0-9_)*
  ::= "$("  ")"
+ ::=  "."  "." 
   ::=  "." 
  ::= 
   ::= 
+::= 
::= "PCD("  ")"
 ::= {"0x"} {"0X"} (\x0 - \xFF)
::= "0x"} {"0X"} (\x0 - \x)
diff --git a/3_edk_ii_fdf_file_format/35_[fd]_sections.md 
b/3_edk_ii_fdf_file_format/35_[fd]_sections.md
index 6c87ebd..f0003e7 100644
--- a/3_edk_ii_fdf_file_format/35_[fd]_sections.md
+++ b/3_edk_ii_fdf_file_format/35_[fd]_sections.md
@@ -62,11 +62,11 @@ Conditional statements may be used anywhere within this 
section.
 "Size"   [] 
 "ErasePolarity"  {"0"} {"1"} 
+
-   ::=  
+   ::=  {} {}
   ::=  "BlockSize"   []

[ "NumBlocks"   ]
-::=  "SET"
+::=  "SET" {} {}   

 ::= {} {} {} {}
{} {} {}
  ::= 
@@ -86,8 +86,8 @@ Conditional statements may be used anywhere within this 
section.
   ::=   ".inf" [ ]
::= {"RELOCS_STRIPPED"} {"RELOCS_RETAINED"}
 ::=  "CAPSULE"  UiCapsuleName 
-::= 
-  ::= 
+::= {} {}
+  ::= {} {}
::=  "FV"   
  ::=  "FILE"   
  ::=  "DATA" 
diff --git a/3_edk_ii_fdf_file_format/36_[fv]_sections.md 
b/3_edk_ii_fdf_file_format/36_[fv]_sections.md
index b4f292a..633b4a7 100644
--- 

Re: [edk2] [Patch] BaseTools Conf: Update tools_def and build_rule to remove IPF setting

2018-06-14 Thread Zhu, Yonghong
Reviewed-by: Yonghong Zhu  

Best Regards,
Zhu Yonghong

-Original Message-
From: Gao, Liming 
Sent: Wednesday, June 13, 2018 2:47 PM
To: edk2-devel@lists.01.org
Cc: Zhu, Yonghong 
Subject: [Patch] BaseTools Conf: Update tools_def and build_rule to remove IPF 
setting

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Liming Gao 
Cc: Yonghong Zhu 
---
 BaseTools/Conf/build_rule.template |  40 --
 BaseTools/Conf/tools_def.template  | 773 -
 2 files changed, 813 deletions(-)

diff --git a/BaseTools/Conf/build_rule.template 
b/BaseTools/Conf/build_rule.template
index 671d378..b2667c2 100755
--- a/BaseTools/Conf/build_rule.template
+++ b/BaseTools/Conf/build_rule.template
@@ -137,30 +137,6 @@
 
 "$(CC)" $(CC_FLAGS) -o ${dst} $(INC) ${src}
 
-[C-Code-File.COMMON.IPF]
-
-?.c
-?.C
-?.cc
-?.CC
-?.cpp
-?.Cpp
-?.CPP
-
-
-$(MAKE_FILE)
-
-
-$(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.obj
-
-
-"$(CC)" /Fo${dst} $(CC_FLAGS) $(INC) ${src}
-
-
-# For RVCTCYGWIN CC_FLAGS must be first to work around pathing issues
-"$(CC)" $(CC_FLAGS) -c -o ${dst} $(INC) ${src}
-"$(SYMRENAME)" $(SYMRENAME_FLAGS) ${dst}
-
 
[C-Code-File.BASE.AARCH64,C-Code-File.SEC.AARCH64,C-Code-File.PEI_CORE.AARCH64,C-Code-File.PEIM.AARCH64,C-Code-File.BASE.ARM,C-Code-File.SEC.ARM,C-Code-File.PEI_CORE.ARM,C-Code-File.PEIM.ARM]
 
 ?.c
@@ -251,22 +227,6 @@
 Trim --trim-long --source-code -o ${d_path}(+)${s_base}.iii 
${d_path}(+)${s_base}.i
 "$(NASM)" -I${s_path}(+) $(NASM_FLAGS) -o $dst 
${d_path}(+)${s_base}.iii
 
-[Assembly-Code-File.COMMON.IPF]
-
-?.s
-
-
-$(MAKE_FILE)
-
-
-$(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.obj
-
-
-"$(PP)" $(PP_FLAGS) $(INC) ${src} > ${d_path}(+)${s_base}.i
-Trim --source-code -o ${d_path}(+)${s_base}.iii ${d_path}(+)${s_base}.i
-# For RVCTCYGWIN ASM_FLAGS must be first to work around pathing issues
-"$(ASM)" $(ASM_FLAGS) -o ${dst} ${d_path}(+)${s_base}.iii
-
 [Device-Tree-Source-File]
 
 ?.dts
diff --git a/BaseTools/Conf/tools_def.template 
b/BaseTools/Conf/tools_def.template
index 7e9c915..474eb2b 100755
--- a/BaseTools/Conf/tools_def.template
+++ b/BaseTools/Conf/tools_def.template
@@ -164,7 +164,6 @@ DEFINE ELFGCC_BIN   = /usr/bin
 # Option 1: Hard coded full path to compiler suite
 DEFINE UNIXGCC_IA32_PETOOLS_PREFIX = 
/opt/tiano/i386-tiano-pe/i386-tiano-pe/bin/
 DEFINE UNIXGCC_X64_PETOOLS_PREFIX  = 
/opt/tiano/x86_64-pc-mingw64/x86_64-pc-mingw64/bin/
-DEFINE UNIXGCC_IPF_PETOOLS_PREFIX  = /opt/tiano/ia64-pc-elf/ia64-pc-elf/bin/
 #
 # Option 2: Use an environment variable
 #DEFINE UNIXGCC_IA32_PETOOLS_PREFIX = ENV(IA32_PETOOLS_PREFIX)
@@ -187,7 +186,6 @@ DEFINE UNIXGCC_IPF_PETOOLS_PREFIX  = 
/opt/tiano/ia64-pc-elf/ia64-pc-elf/bin/
 DEFINE CYGWIN_BIN  = c:/cygwin/bin
 DEFINE CYGWIN_BINIA32  = 
c:/cygwin/opt/tiano/i386-tiano-pe/i386-tiano-pe/bin/
 DEFINE CYGWIN_BINX64   = 
c:/cygwin/opt/tiano/x86_64-pc-mingw64/x86_64-pc-mingw64/bin/
-DEFINE CYGWIN_BINIPF   = c:/cygwin/opt/tiano/gcc/ipf/bin/ia64-pc-elf-
 
 DEFINE GCC44_IA32_PREFIX   = ENV(GCC44_BIN)
 DEFINE GCC44_X64_PREFIX= ENV(GCC44_BIN)
@@ -453,7 +451,6 @@ DEFINE DTC_BIN = ENV(DTC_PREFIX)dtc
 #   https://acpica.org/downloads
 #   MYTOOLS -win32-  Requires:
 # Microsoft Visual Studio 2008 for IA32/X64
-# Microsoft Windows Server 2003 Driver Development 
Kit (Microsoft WINDDK) version 3790.1830 for IPF
 #Optional:
 # Required to build EBC drivers:
 #   Intel(r) Compiler for Efi Byte Code (Intel(r) 
EBC Compiler)
@@ -1044,34 +1041,6 @@ RELEASE_VS2005_X64_DLINK_FLAGS  = /NOLOGO /NODEFAULTLIB 
/IGNORE:4001 /IGNORE:425
 NOOPT_VS2005_X64_DLINK_FLAGS= /NOLOGO /NODEFAULTLIB /IGNORE:4001 /OPT:REF 
/OPT:ICF=10 /MAP /ALIGN:32 /SECTION:.xdata,D /SECTION:.pdata,D /Machine:X64 
/LTCG /DLL /ENTRY:$(IMAGE_ENTRY_POINT) /SUBSYSTEM:EFI_BOOT_SERVICE_DRIVER 
/SAFESEH:NO /BASE:0 /DRIVER /DEBUG
 
 ##
-# IPF definitions
-##
-*_VS2005_IPF_*_DLL = DEF(VS2005_DLL)
-
-*_VS2005_IPF_PP_PATH   = DEF(VS2005_BIN64)\cl.exe
-*_VS2005_IPF_APP_PATH  = DEF(VS2005_BIN64)\cl.exe
-*_VS2005_IPF_VFRPP_PATH= DEF(VS2005_BIN64)\cl.exe
-*_VS2005_IPF_CC_PATH   = DEF(VS2005_BIN64)\cl.exe
-*_VS2005_IPF_ASM_PATH  = DEF(VS2005_BIN64)\ias.exe
-*_VS2005_IPF_SLINK_PATH= DEF(VS2005_BIN64)\lib.exe
-*_VS2005_IPF_DLINK_PATH= DEF(VS2005_BIN64)\link.exe
-*_VS2005_IPF_ASLCC_PATH= DEF(VS2005_BIN64)\cl.exe
-*_VS2005_IPF_ASLPP_PATH= DEF(VS2005_BIN64)\cl.exe
-*_VS2005_IPF_ASLDLINK_PATH = DEF(VS2005_BIN64)\link.exe
-
-  

[edk2] [Patch] INF Spec: FixedAtBuild (VOID*) PCD use in the [DEPEX] section

2018-06-14 Thread Yonghong Zhu
Fixes: https://bugzilla.tianocore.org/show_bug.cgi?id=444
Cc: Liming Gao 
Cc: Michael Kinney 
Cc: Kevin W Shaw 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Yonghong Zhu 
---
 3_edk_ii_inf_file_format/314_[depex]_sections.md | 20 
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/3_edk_ii_inf_file_format/314_[depex]_sections.md 
b/3_edk_ii_inf_file_format/314_[depex]_sections.md
index e325f06..942bcf9 100644
--- a/3_edk_ii_inf_file_format/314_[depex]_sections.md
+++ b/3_edk_ii_inf_file_format/314_[depex]_sections.md
@@ -1,9 +1,9 @@
 

Re: [edk2] [PATCH v2] SourceLevelDebugPkg/DebugCommunicationLibUsb: Add endpoint configuration.

2018-06-14 Thread Wu, Hao A
Hi Marvin,

One thing to confirm with you. For your using case, do you need to hard-code
the IN/OUT endpoints for EHCI debug device?

If not, I think we can just remove the hard-code endpoints settings in the
current code. And use the endpoints returned from the USB Device Descriptor.
By doing so, I think we can avoid adding those 2 PCDs.

Please let me know your thoughts on this. Thanks in advance.

Best Regards,
Hao Wu

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Marvin Häuser
> Sent: Thursday, June 14, 2018 3:58 AM
> To: edk2-devel@lists.01.org
> Cc: Wu, Hao A
> Subject: [edk2] [PATCH v2] SourceLevelDebugPkg/DebugCommunicationLibUsb:
> Add endpoint configuration.
> 
> Currently, DebugCommunicationLibUsb uses the hardcoded endpoints 0x82
> and 0x01 to communicate with the EHCI Debug Device. These, however,
> are not standardized and may vary across different hardware.
> To solve this problem, two PCDs have been introduced to configure the
> in and out endpoints of the EHCI Debug Device. These may be set to 0
> to retrieve the endpoints from the USB Device Descriptor directly.
> To ensure maximum compatibility, the PCD defaults have been set to
> the former hardcoded values.
> 
> V2:
>   - Store endpoint data in the USB Debug Port handle sturcture.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Marvin Haeuser 
> ---
> 
> SourceLevelDebugPkg/Library/DebugCommunicationLibUsb/DebugCommunica
> tionLibUsb.c   | 32 ++--
> 
> SourceLevelDebugPkg/Library/DebugCommunicationLibUsb/DebugCommunica
> tionLibUsb.inf |  6 +++-
>  SourceLevelDebugPkg/SourceLevelDebugPkg.dec  
>  | 12
> 
>  3 files changed, 47 insertions(+), 3 deletions(-)
> 
> diff --git
> a/SourceLevelDebugPkg/Library/DebugCommunicationLibUsb/DebugCommuni
> cationLibUsb.c
> b/SourceLevelDebugPkg/Library/DebugCommunicationLibUsb/DebugCommuni
> cationLibUsb.c
> index d996f80f59e3..a9a9ea07a39b 100644
> ---
> a/SourceLevelDebugPkg/Library/DebugCommunicationLibUsb/DebugCommuni
> cationLibUsb.c
> +++
> b/SourceLevelDebugPkg/Library/DebugCommunicationLibUsb/DebugCommuni
> cationLibUsb.c
> @@ -132,6 +132,14 @@ typedef struct _USB_DEBUG_PORT_HANDLE{
>//
>UINT32   EhciMemoryBase;
>//
> +  // The usb debug device In endpoint.
> +  //
> +  UINT8InEndpoint;
> +  //
> +  // The usb debug device Out endpoint.
> +  //
> +  UINT8OutEndpoint;
> +  //
>// The Bulk In endpoint toggle bit.
>//
>UINT8BulkInToggle;
> @@ -603,6 +611,8 @@ InitializeUsbDebugHardware (
>UINT32*UsbHCSParam;
>UINT8 DebugPortNumber;
>UINT8 Length;
> +  UINT8 InEndpoint;
> +  UINT8 OutEndpoint;
> 
>UsbDebugPortRegister = (USB_DEBUG_PORT_REGISTER *)((UINTN)Handle-
> >UsbDebugPortMemoryBase + Handle->DebugPortOffset);
>UsbHCSParam = (UINT32 *)((UINTN)Handle->EhciMemoryBase + 0x04);
> @@ -722,6 +732,24 @@ InitializeUsbDebugHardware (
>return RETURN_DEVICE_ERROR;
>  }
> 
> +//
> +// Determine the usb debug device endpoints.
> +//
> +InEndpoint = PcdGet8 (PcdUsbDebugPortInEndpoint);
> +
> +if (InEndpoint == 0) {
> +  InEndpoint = UsbDebugPortDescriptor.DebugInEndpoint;
> +}
> +
> +OutEndpoint = PcdGet8 (PcdUsbDebugPortOutEndpoint);
> +
> +if (OutEndpoint == 0) {
> +  OutEndpoint = UsbDebugPortDescriptor.DebugOutEndpoint;
> +}
> +
> +Handle->InEndpoint  = InEndpoint;
> +Handle->OutEndpoint = OutEndpoint;
> +
>  //
>  // enable the usb debug feature.
>  //
> @@ -879,7 +907,7 @@ DebugPortWriteBuffer (
>Sent = (UINT8)(NumberOfBytes - Total);
>  }
> 
> -Status = UsbDebugPortOut(UsbDebugPortRegister, Buffer + Total, Sent,
> OUTPUT_PID, 0x7F, 0x01, UsbDebugPortHandle->BulkOutToggle);
> +Status = UsbDebugPortOut(UsbDebugPortRegister, Buffer + Total, Sent,
> OUTPUT_PID, 0x7F, UsbDebugPortHandle->OutEndpoint, UsbDebugPortHandle-
> >BulkOutToggle);
> 
>  if (RETURN_ERROR(Status)) {
>return Total;
> @@ -959,7 +987,7 @@ DebugPortPollBuffer (
>  UsbDebugPortRegister->SendPid  = DATA1_PID;
>}
>UsbDebugPortRegister->UsbAddress  = 0x7F;
> -  UsbDebugPortRegister->UsbEndPoint = 0x82 & 0x0F;
> +  UsbDebugPortRegister->UsbEndPoint = UsbDebugPortHandle->InEndpoint &
> 0x0F;
> 
>//
>// Clearing W/R bit to indicate it's a READ operation
> diff --git
> a/SourceLevelDebugPkg/Library/DebugCommunicationLibUsb/DebugCommuni
> cationLibUsb.inf
> b/SourceLevelDebugPkg/Library/DebugCommunicationLibUsb/DebugCommuni
> cationLibUsb.inf
> index 028b04afbf00..eb55e5ee0f63 100644
> ---
> a/SourceLevelDebugPkg/Library/DebugCommunicationLibUsb/DebugCommuni
> cationLibUsb.inf
> +++
> b/SourceLevelDebugPkg/Library/DebugCommunicationLibUsb/DebugCommuni
> cationLibUsb.inf
> @@ -43,9 +43,13 @@ [Pcd]
>

[edk2] [Patch][edk2-platforms/devel-MinnowBoardMax-UDK2017] Enabled HTTPS boot

2018-06-14 Thread Guo, Mang
Added HTTPS boot support on MinnowBoard Max. This feature is controlled by 
NETWORK_TLS_ENABLE and the default value is FALSE.

Cc: zwei4 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Guo Mang 
---
 Vlv2TbltDevicePkg/PlatformPkg.fdf   | 9 +++--
 Vlv2TbltDevicePkg/PlatformPkgConfig.dsc | 3 ++-
 Vlv2TbltDevicePkg/PlatformPkgGcc.fdf| 7 ++-
 Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc | 5 +
 Vlv2TbltDevicePkg/PlatformPkgIA32.dsc   | 6 ++
 Vlv2TbltDevicePkg/PlatformPkgX64.dsc| 6 ++
 6 files changed, 32 insertions(+), 4 deletions(-)

diff --git a/Vlv2TbltDevicePkg/PlatformPkg.fdf 
b/Vlv2TbltDevicePkg/PlatformPkg.fdf
index 84bc7db..444670b 100644
--- a/Vlv2TbltDevicePkg/PlatformPkg.fdf
+++ b/Vlv2TbltDevicePkg/PlatformPkg.fdf
@@ -732,12 +732,17 @@ FILE FREEFORM = 878AC2CC-5343-46F2-B563-51F89DAF56BA {
   INF  MdeModulePkg/Universal/Network/Udp4Dxe/Udp4Dxe.inf
   INF  MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Dxe.inf
   INF  MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Dxe.inf
-!if $(HTTP_BOOT_SUPPORT) == TRUE
+  !if $(HTTP_BOOT_SUPPORT) == TRUE
   INF NetworkPkg\HttpDxe\HttpDxe.inf
   INF NetworkPkg\HttpBootDxe\HttpBootDxe.inf
   INF NetworkPkg\HttpUtilitiesDxe\HttpUtilitiesDxe.inf
   INF NetworkPkg\DnsDxe\DnsDxe.inf
-!endif
+  !if $(NETWORK_TLS_ENABLE) == TRUE
+INF  NetworkPkg/TlsDxe/TlsDxe.inf
+INF  NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxe.inf
+  !endif
+  INF RuleOverride = DRIVER_ACPITABLE 
MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskDxe.inf
+  !endif
   !if $(NETWORK_IP6_ENABLE) == TRUE
   INF  NetworkPkg/Ip6Dxe/Ip6Dxe.inf
   INF  NetworkPkg/Dhcp6Dxe/Dhcp6Dxe.inf
diff --git a/Vlv2TbltDevicePkg/PlatformPkgConfig.dsc 
b/Vlv2TbltDevicePkg/PlatformPkgConfig.dsc
index ad1ed4d..a73f881 100644
--- a/Vlv2TbltDevicePkg/PlatformPkgConfig.dsc
+++ b/Vlv2TbltDevicePkg/PlatformPkgConfig.dsc
@@ -93,4 +93,5 @@ DEFINE ESRT_ENABLE   = TRUE
 #
  DEFINE SOURCE_DEBUG_ENABLE = FALSE
 
-DEFINE HTTP_BOOT_SUPPORT = FALSE
\ No newline at end of file
+DEFINE HTTP_BOOT_SUPPORT = FALSE
+DEFINE NETWORK_TLS_ENABLE = FALSE
\ No newline at end of file
diff --git a/Vlv2TbltDevicePkg/PlatformPkgGcc.fdf 
b/Vlv2TbltDevicePkg/PlatformPkgGcc.fdf
index 2c1a283..404a87a 100644
--- a/Vlv2TbltDevicePkg/PlatformPkgGcc.fdf
+++ b/Vlv2TbltDevicePkg/PlatformPkgGcc.fdf
@@ -690,7 +690,12 @@ FILE FREEFORM = 878AC2CC-5343-46F2-B563-51F89DAF56BA {
   INF NetworkPkg/HttpBootDxe/HttpBootDxe.inf
   INF NetworkPkg/HttpUtilitiesDxe/HttpUtilitiesDxe.inf
   INF NetworkPkg/DnsDxe/DnsDxe.inf
-!endif
+  !if $(NETWORK_TLS_ENABLE) == TRUE
+INF  NetworkPkg/TlsDxe/TlsDxe.inf
+INF  NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxe.inf
+  !endif
+  INF RuleOverride = DRIVER_ACPITABLE 
MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskDxe.inf
+  !endif
   !if $(NETWORK_IP6_ENABLE) == TRUE
   INF  NetworkPkg/Ip6Dxe/Ip6Dxe.inf
   INF  NetworkPkg/Dhcp6Dxe/Dhcp6Dxe.inf
diff --git a/Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc 
b/Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc
index d837563..8aaff47 100644
--- a/Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc
+++ b/Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc
@@ -111,6 +111,7 @@
   TcgPpVendorLib|SecurityPkg/Library/TcgPpVendorLibNull/TcgPpVendorLibNull.inf
   
Tcg2PpVendorLib|SecurityPkg/Library/Tcg2PpVendorLibNull/Tcg2PpVendorLibNull.inf
   
Tpm2DeviceLib|SecurityPkg/Library/Tpm2DeviceLibRouter/Tpm2DeviceLibRouterDxe.inf
+  TlsLib|CryptoPkg/Library/TlsLib/TlsLib.inf
 !endif
 
   #
@@ -1597,6 +1598,10 @@ 
$(PLATFORM_BINARY_PACKAGE)/$(DXE_ARCHITECTURE)$(TARGET)/IA32/fTPMInitPeim.inf
 NetworkPkg/HttpBootDxe/HttpBootDxe.inf
 NetworkPkg/HttpUtilitiesDxe/HttpUtilitiesDxe.inf
 NetworkPkg/DnsDxe/DnsDxe.inf
+!if $(NETWORK_TLS_ENABLE) == TRUE
+  NetworkPkg/TlsDxe/TlsDxe.inf
+  NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxe.inf
+!endif
 !endif
 MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Dxe.inf
 MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Dxe.inf
diff --git a/Vlv2TbltDevicePkg/PlatformPkgIA32.dsc 
b/Vlv2TbltDevicePkg/PlatformPkgIA32.dsc
index 15e0b81..06a0a93 100644
--- a/Vlv2TbltDevicePkg/PlatformPkgIA32.dsc
+++ b/Vlv2TbltDevicePkg/PlatformPkgIA32.dsc
@@ -111,6 +111,7 @@
   TcgPpVendorLib|SecurityPkg/Library/TcgPpVendorLibNull/TcgPpVendorLibNull.inf
   
Tcg2PpVendorLib|SecurityPkg/Library/Tcg2PpVendorLibNull/Tcg2PpVendorLibNull.inf
   
Tpm2DeviceLib|SecurityPkg/Library/Tpm2DeviceLibRouter/Tpm2DeviceLibRouterDxe.inf
+  TlsLib|CryptoPkg/Library/TlsLib/TlsLib.inf
 !endif
   
   #
@@ -1609,6 +1610,11 @@ 
$(PLATFORM_BINARY_PACKAGE)/$(DXE_ARCHITECTURE)$(TARGET)/IA32/fTPMInitPeim.inf
 NetworkPkg\HttpBootDxe\HttpBootDxe.inf
 NetworkPkg\HttpUtilitiesDxe\HttpUtilitiesDxe.inf
 NetworkPkg\DnsDxe\DnsDxe.inf
+!if $(NETWORK_TLS_ENABLE) == TRUE
+  NetworkPkg/TlsDxe/TlsDxe.inf
+  NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxe.inf
+!endif
+MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskDxe.inf
 !endif
 

Re: [edk2] [Patch] BaseTools: remove including Base.h if the module type is not BASE

2018-06-14 Thread Gao, Liming
Reviewed-by: Liming Gao 

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> Yonghong Zhu
> Sent: Tuesday, June 12, 2018 2:40 PM
> To: edk2-devel@lists.01.org
> Subject: [edk2] [Patch] BaseTools: remove including Base.h if the module type 
> is not BASE
> 
> According the module type to include the header file.
> 
> Fixes: https://bugzilla.tianocore.org/show_bug.cgi?id=867
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Yonghong Zhu 
> ---
>  BaseTools/Source/Python/AutoGen/GenC.py | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/BaseTools/Source/Python/AutoGen/GenC.py 
> b/BaseTools/Source/Python/AutoGen/GenC.py
> index 1be27d2..19ed196 100644
> --- a/BaseTools/Source/Python/AutoGen/GenC.py
> +++ b/BaseTools/Source/Python/AutoGen/GenC.py
> @@ -2011,13 +2011,11 @@ def CreateHeaderCode(Info, AutoGenC, AutoGenH):
>  # header file Prologue
>  
> AutoGenH.Append(gAutoGenHPrologueString.Replace({'File':'AUTOGENH','Guid':Info.Guid.replace('-','_')}))
>  AutoGenH.Append(gAutoGenHCppPrologueString)
>  if Info.AutoGenVersion >= 0x00010005:
>  # header files includes
> -AutoGenH.Append("#include <%s>\n" % gBasicHeaderFile)
> -if Info.ModuleType in gModuleTypeHeaderFile \
> -   and gModuleTypeHeaderFile[Info.ModuleType][0] != gBasicHeaderFile:
> +if Info.ModuleType in gModuleTypeHeaderFile:
>  AutoGenH.Append("#include <%s>\n" % 
> gModuleTypeHeaderFile[Info.ModuleType][0])
>  #
>  # if either PcdLib in [LibraryClasses] sections or there exist Pcd 
> section, add PcdLib.h
>  # As if modules only uses FixedPcd, then PcdLib is not needed in 
> [LibraryClasses] section.
>  #
> --
> 2.6.1.windows.1
> 
> ___
> 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