[edk2-devel] [PATCH v2 3/4] Platform/RaspberryPi: Add entry for user fan control

[Jeremy Linton]

Add a menu item that allows the user to enable GPIO based
fan control via SSDT. This should only be seen/enabled on RPI4
because that is what its been tested with. As of this commit
its currently limited to only operating on a single GPIO pin (19).

Given GPIO pin current limitations its likely that a bit of
additional circuitry is required to drive a fan, and the GPIO
high/low signal can only be used as a enable/disable signal. A
search for "rpi npn gpio fan" or similar should turn up some
hits for how to do this simply.

It appears there are a couple boards (fan SHIM) which operate this
way, and probably should have custom menu items/SSDT edits as
people acquire the boards and test them.

Cc: Leif Lindholm 
Cc: Pete Batard 
Cc: Andrei Warkentin 
Cc: Ard Biesheuvel 
Cc: Samer El-Haj-Mahmoud 
Signed-off-by: Jeremy Linton 
Reviewed-by: Pete Batard <@pbatard>
---
 Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c | 78 ++
 .../RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf|  3 +
 .../RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni |  6 ++
 .../RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr | 18 +
 Platform/RaspberryPi/Include/ConfigVars.h  |  4 ++
 Platform/RaspberryPi/RPi3/RPi3.dsc |  5 ++
 Platform/RaspberryPi/RPi4/RPi4.dsc |  8 +++
 Platform/RaspberryPi/RaspberryPi.dec   |  1 +
 8 files changed, 123 insertions(+)

diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c 
b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
index af54136ade..24e65eeb5e 100644
--- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
@@ -15,6 +15,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -22,6 +23,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -246,6 +248,14 @@ SetupVariables (
 ASSERT_EFI_ERROR (Status);
   }
 
+  Size = sizeof (UINT32);
+  Status = gRT->GetVariable (L"FanOnGpio",
+  ,
+  NULL, , );
+  if (EFI_ERROR (Status)) {
+PcdSet32 (PcdFanOnGpio, PcdGet32 (PcdFanOnGpio));
+  }
+
   Size = sizeof(AssetTagVar);
 
   Status = gRT->GetVariable(L"AssetTag",
@@ -368,6 +378,7 @@ ApplyVariables (
   UINT32 CpuClock = PcdGet32 (PcdCpuClock);
   UINT32 CustomCpuClock = PcdGet32 (PcdCustomCpuClock);
   UINT32 Rate = 0;
+  UINT32 FanOnGpio = PcdGet32 (PcdFanOnGpio);
 
   switch (CpuClock) {
   case CHIPSET_CPU_CLOCK_LOW:
@@ -565,8 +576,71 @@ ApplyVariables (
 GpioPinFuncSet (23, GPIO_FSEL_INPUT);
 GpioPinFuncSet (24, GPIO_FSEL_INPUT);
   }
+
+  if (FanOnGpio) {
+DEBUG ((DEBUG_INFO, "Fan enabled on GPIO %d\n", FanOnGpio));
+GpioPinFuncSet (FanOnGpio, GPIO_FSEL_OUTPUT);
+  }
 }
 
+#define SSDT_PATTERN_LEN 6
+
+EFI_STATUS
+FindInstallSsdt (UINT64 OemTableId, CHAR8 *Name, UINT8 Value)
+{
+  EFI_ACPI_TABLE_PROTOCOL *AcpiTable;
+  UINTN   Index;
+  EFI_ACPI_DESCRIPTION_HEADER *Ssdt;
+  UINTN   SsdtSize;
+  EFI_STATUS  Status;
+  UINTN   TableKey;
+  CHAR8   Pattern[SSDT_PATTERN_LEN];
+
+
+  Status = gBS->LocateProtocol (, NULL,
+(VOID **));
+  if (EFI_ERROR (Status)) {
+return Status;
+  }
+
+  for (Index = 0; !EFI_ERROR (Status); Index++) {
+Status = GetSectionFromFv (, EFI_SECTION_RAW, Index,
+   (VOID **), );
+if (Ssdt->OemTableId == OemTableId)
+  break;
+SsdtSize = 0;
+  }
+
+  if (SsdtSize > 0) {
+/*
+ * Do a single 8 bit NameOp variable replacement these are of the
+ * form 08  SIZE VAL, where SIZE is 0A=byte, 0B=word, 0C=dword
+ * and  is the name and VAL is the value
+*/
+Pattern[0] = 0x08;//NameOp
+Pattern[1] = Name[0]; //Name
+Pattern[2] = Name[1];
+Pattern[3] = Name[2];
+Pattern[4] = Name[3];
+Pattern[5] = 0x0A;//Size
+
+for (Index = 0; Index < (SsdtSize - SSDT_PATTERN_LEN); Index++) {
+  if (CompareMem ((UINT8*)Ssdt + Index, Pattern, SSDT_PATTERN_LEN) == 0) {
+*((UINT8 *)Ssdt + Index + SSDT_PATTERN_LEN) = Value;
+break;
+  }
+}
+
+Status = AcpiTable->InstallAcpiTable (AcpiTable, Ssdt, SsdtSize,
+  );
+if (EFI_ERROR (Status)) {
+  DEBUG ((DEBUG_WARN, "%a: failed to install SSDT table %r\n",
+  __FUNCTION__, Status));
+}
+  }
+
+  return Status;
+}
 
 EFI_STATUS
 EFIAPI
@@ -620,6 +694,10 @@ ConfigInitialize (
   PcdGet32 (PcdSystemTableMode) == SYSTEM_TABLE_MODE_BOTH) {
  Status = LocateAndInstallAcpiFromFv ();
  ASSERT_EFI_ERROR (Status);
+ if (PcdGet32 (PcdFanOnGpio)) {
+   FindInstallSsdt (SIGNATURE_64 ('R', 'P', 'I', 'T', 'H', 'F', 'A', 'N'), 
+"GIOP", (UINT8)PcdGet32 (PcdFanOnGpio));
+ }
   }
 
   Status = gBS->CreateEventEx 

[edk2-devel] [PATCH v2 1/4] Platform/RaspberryPi4: Add a basic thermal zone

[Jeremy Linton]

Rather than exporting the temp sensor or mailbox
in ACPI land we can wrap them in AML and use the default
ACPI drivers provided by the OS. This enables the use of
"sensors" in linux to report the SOC temp.

This commit also adds a basic passive cooling ACPI thermalzone
with trip points for passive cooling (throttling) handled
by the vc firmware, hibernate and critical shutdown. The
vc apparently kicks in at ~80C, so the hibernate and critical
set points are set at +5 and +10 of that. In the future
CPPC should be able to monitor the thermal throttling.

Cc: Leif Lindholm 
Cc: Pete Batard 
Cc: Andrei Warkentin 
Cc: Ard Biesheuvel 
Cc: Samer El-Haj-Mahmoud 
Signed-off-by: Jeremy Linton 
Reviewed-by: Pete Batard <@pbatard>
---
 Platform/RaspberryPi/AcpiTables/Dsdt.asl   | 31 ++
 .../Bcm27xx/Include/IndustryStandard/Bcm2711.h |  2 ++
 2 files changed, 33 insertions(+)

diff --git a/Platform/RaspberryPi/AcpiTables/Dsdt.asl 
b/Platform/RaspberryPi/AcpiTables/Dsdt.asl
index 353af2d876..73067aefd2 100644
--- a/Platform/RaspberryPi/AcpiTables/Dsdt.asl
+++ b/Platform/RaspberryPi/AcpiTables/Dsdt.asl
@@ -252,6 +252,37 @@ DefinitionBlock ("Dsdt.aml", "DSDT", 5, "RPIFDN", "RPI", 2)
 }
   })
 }
+
+// Define a simple thermal zone. The idea here is we compute the SOC temp
+// via a register we can read, and give it to the OS. This enables basic
+// reports from the "sensors" utility, and the OS can then poll and take
+// actions if that temp exceeds any of the given thresholds.
+Device (EC0)
+{
+  Name (_HID, EISAID ("PNP0C06"))
+  Name (_CCA, 0x0)
+
+  // all temps in are tenths of K (aka 2732 is the min temps in Linux (aka 
0C))
+  ThermalZone (TZ0) {
+Method (_TMP, 0, Serialized) {
+  OperationRegion (TEMS, SystemMemory, THERM_SENSOR, 0x8)
+  Field (TEMS, DWordAcc, NoLock, Preserve) {
+TMPS, 32
+  }
+  return (((419949 - ((TMPS & 0x3ff) * 487)) / 100) + 2732);
+}
+Method (_SCP, 3) { }   // receive cooling policy from OS
+
+Method (_CRT) { Return (3632) }// (90C) Critical temp point 
(immediate power-off)
+Method (_HOT) { Return (3582) }// (85C) HOT state where OS should 
hibernate
+Method (_PSV) { Return (3532) }// (80C) Passive cooling (CPU 
throttling) trip point
+
+// SSDT inserts _AC0/_AL0 @60C here, if a FAN is configured
+
+Name (_TZP, 10)   //The OSPM must poll this device 
every 1 seconds
+Name (_PSL, Package () { \_SB_.CPU0, \_SB_.CPU1, \_SB_.CPU2, 
\_SB_.CPU3 })
+  }
+}
 #endif
 
   }
diff --git a/Silicon/Broadcom/Bcm27xx/Include/IndustryStandard/Bcm2711.h 
b/Silicon/Broadcom/Bcm27xx/Include/IndustryStandard/Bcm2711.h
index e9c81cafa1..86906b2438 100644
--- a/Silicon/Broadcom/Bcm27xx/Include/IndustryStandard/Bcm2711.h
+++ b/Silicon/Broadcom/Bcm27xx/Include/IndustryStandard/Bcm2711.h
@@ -86,4 +86,6 @@
 #define GENET_BASE_ADDRESS FixedPcdGet64 (PcdBcmGenetRegistersAddress)
 #define GENET_LENGTH   0x0001
 
+#define THERM_SENSOR   0xfd5d2200
+
 #endif /* BCM2711_H__ */
-- 
2.13.7


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#64481): https://edk2.groups.io/g/devel/message/64481
Mute This Topic: https://groups.io/mt/76302316/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v2 0/4] Platform/RasberryPi: Thermal zone

[Jeremy Linton]

This set creates a basic thermal zone, which reads the
SOC temp via a direct register read in AML. It also
adds an active cooling policy using a GPIO pin for fan
control that can optionally be enabled/disabled on either
GPIO18 (commercial fan shim board) or GPIO19 by the
user from the BDS.

it should now be possible when booted in ACPI mode to:

# sensors
acpitz-acpi-0
Adapter: ACPI interface
temp1:+57.6C  (crit = +90.0C)

and the fan state, if enabled may be read with:

/sys/bus/acpi/devices/PNP0C06:00/PNP0C0B:00/physical_node/thermal_cooling/cur_state

while the kernel will automatically cycle the fan if the SoC
temp exceeds 60C.


v1->v2:
  Add choice of GPIO pins to the BDS menu
  Correct whitespace/etc issues from v1 review
  Add an additional cleanup patch for contextual whitespace issues

Jeremy Linton (4):
  Platform/RaspberryPi4: Add a basic thermal zone
  Platform/RaspberryPi4: Create ACPI fan object
  Platform/RaspberryPi: Add entry for user fan control
  Platform/RaspberryPi: Trivial whitespace cleanup

 Platform/RaspberryPi/AcpiTables/Dsdt.asl   |  31 ++
 Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c | 104 ++---
 .../RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf|   3 +
 .../RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni |   6 ++
 .../RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr |  18 
 .../RaspberryPi/Drivers/ConfigDxe/SsdtThermal.asl  |  76 +++
 Platform/RaspberryPi/Include/ConfigVars.h  |   4 +
 Platform/RaspberryPi/RPi3/RPi3.dsc |   5 +
 Platform/RaspberryPi/RPi4/RPi4.dsc |   8 ++
 Platform/RaspberryPi/RaspberryPi.dec   |   1 +
 .../Bcm27xx/Include/IndustryStandard/Bcm2711.h |   2 +
 11 files changed, 245 insertions(+), 13 deletions(-)
 create mode 100644 Platform/RaspberryPi/Drivers/ConfigDxe/SsdtThermal.asl

-- 
2.13.7


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#64480): https://edk2.groups.io/g/devel/message/64480
Mute This Topic: https://groups.io/mt/76302314/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v2 2/4] Platform/RaspberryPi4: Create ACPI fan object

[Jeremy Linton]

Now that we have a thermal zone we can add active cooling
by specifying active cooling points (_ACx) which can
be tied to fan objects that turn fans on/off using GPIO
pins.

Cc: Leif Lindholm 
Cc: Pete Batard 
Cc: Andrei Warkentin 
Cc: Ard Biesheuvel 
Cc: Samer El-Haj-Mahmoud 
Signed-off-by: Jeremy Linton 
Reviewed-by: Pete Batard <@pbatard>
---
 .../RaspberryPi/Drivers/ConfigDxe/SsdtThermal.asl  | 76 ++
 1 file changed, 76 insertions(+)
 create mode 100644 Platform/RaspberryPi/Drivers/ConfigDxe/SsdtThermal.asl

diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/SsdtThermal.asl 
b/Platform/RaspberryPi/Drivers/ConfigDxe/SsdtThermal.asl
new file mode 100644
index 00..b26e9a07a8
--- /dev/null
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/SsdtThermal.asl
@@ -0,0 +1,76 @@
+/** @file
+ *
+ *  Secondary System Description Table (SSDT) for active (fan) cooling
+ *
+ *  Copyright (c) 2020, Arm Ltd. All rights reserved.
+ *
+ *  SPDX-License-Identifier: BSD-2-Clause-Patent
+ *
+ **/
+
+#include 
+#include 
+#include 
+
+#include 
+
+DefinitionBlock (__FILE__, "SSDT", 5, "RPIFDN", "RPITHFAN", 2)
+{
+  External (\_SB_.EC0, DeviceObj)
+  External (\_SB_.EC0.TZ0, DeviceObj)
+
+  Scope (\_SB_.EC0)
+  {
+// Define a NameOp we will modify during InstallTable
+Name (GIOP, 2) //08 47 49 4f 50 0a 02 (value must be >1)
+// Describe a fan
+PowerResource (PFAN, 0, 0) {
+  OperationRegion (GPIO, SystemMemory, GPIO_BASE_ADDRESS, 0x1000)
+  Field (GPIO, DWordAcc, NoLock, Preserve) {
+Offset (0x1C),
+GPS0, 32,
+GPS1, 32,
+RES1, 32,
+GPC0, 32,
+GPC1, 32,
+RES2, 32,
+GPL1, 32,
+GPL2, 32
+  }
+  // We are hitting a GPIO pin to on/off a fan.
+  // This assumes that UEFI has programmed the
+  // direction as OUT. Given the current limitations
+  // on the GPIO pins, its recommended to use
+  // the GPIO to switch a larger voltage/current
+  // for the fan rather than driving it directly.
+  Method (_STA) {
+if (GPL1 & (1 << GIOP)) {
+  Return (1) // present and enabled
+}
+Return (0)
+  }
+  Method (_ON)  {//turn fan on
+Store (1 << GIOP, GPS0)
+  }
+  Method (_OFF) {//turn fan off
+Store (1 << GIOP, GPC0)
+  }
+}
+Device (FAN) {
+  // Note, not currently an ACPIv4 fan
+  // the latter adds speed control/detection
+  // but in the case of linux needs FIF, FPS, FSL, and FST
+  Name (_HID, EISAID ("PNP0C0B"))
+  Name (_PR0, Package () { PFAN })
+}
+  }
+
+  // merge in an active cooling point.
+  Scope (\_SB_.EC0.TZ0)
+  {
+Method (_AC0) { Return (3332) }// (60C) active cooling trip point,
+   // if this is lower than PSV then we
+   // prefer active cooling
+Name (_AL0, Package () { \_SB_.EC0.FAN }) // the fan used for AC0 above
+  }
+}
-- 
2.13.7


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#64482): https://edk2.groups.io/g/devel/message/64482
Mute This Topic: https://groups.io/mt/76302318/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v2 4/4] Platform/RaspberryPi: Trivial whitespace cleanup

[Jeremy Linton]

Pete's review pointed out some whitespace issues in the
context of a previous patch. Since there are a number of
similar errors in the file lets fix them separately.

Signed-off-by: Jeremy Linton 
---
 Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c | 26 +++---
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c 
b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
index 24e65eeb5e..4a47160c42 100644
--- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
@@ -209,9 +209,9 @@ SetupVariables (
   }
 
   Size = sizeof (UINT32);
-  Status = gRT->GetVariable(L"CustomCpuClock",
-,
-NULL, , );
+  Status = gRT->GetVariable (L"CustomCpuClock",
+ ,
+ NULL, , );
   if (EFI_ERROR (Status)) {
 Status = PcdSet32S (PcdCustomCpuClock, PcdGet32 (PcdCustomCpuClock));
 ASSERT_EFI_ERROR (Status);
@@ -256,9 +256,9 @@ SetupVariables (
 PcdSet32 (PcdFanOnGpio, PcdGet32 (PcdFanOnGpio));
   }
 
-  Size = sizeof(AssetTagVar);
+  Size = sizeof (AssetTagVar);
 
-  Status = gRT->GetVariable(L"AssetTag",
+  Status = gRT->GetVariable (L"AssetTag",
   ,
   NULL, , AssetTagVar);
 
@@ -267,7 +267,7 @@ SetupVariables (
 L"AssetTag",
 ,
 EFI_VARIABLE_NON_VOLATILE | 
EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS,
-sizeof(AssetTagVar),
+sizeof (AssetTagVar),
 AssetTagVar
 );
   }
@@ -433,9 +433,9 @@ ApplyVariables (
  * spaces. SystemMemorySizeBelow4GB tracks the maximum memory below 4GB
  * line, factoring in the limit imposed by the SoC register range.
  */
-SystemMemorySizeBelow4GB = MIN(SystemMemorySize, 4UL * SIZE_1GB);
-SystemMemorySizeBelow4GB = MIN(SystemMemorySizeBelow4GB, 
BCM2836_SOC_REGISTERS);
-SystemMemorySizeBelow4GB = MIN(SystemMemorySizeBelow4GB, 
BCM2711_SOC_REGISTERS);
+SystemMemorySizeBelow4GB = MIN (SystemMemorySize, 4UL * SIZE_1GB);
+SystemMemorySizeBelow4GB = MIN (SystemMemorySizeBelow4GB, 
BCM2836_SOC_REGISTERS);
+SystemMemorySizeBelow4GB = MIN (SystemMemorySizeBelow4GB, 
BCM2711_SOC_REGISTERS);
 
 ASSERT (SystemMemorySizeBelow4GB > 3UL * SIZE_1GB);
 
@@ -528,14 +528,14 @@ ApplyVariables (
   /*
* SD card pins go to Arasan.
*/
-  MmioWrite32((GPIO_BASE_ADDRESS + 0xD0),
-  MmioRead32(GPIO_BASE_ADDRESS + 0xD0) | 0x2);
+  MmioWrite32 ((GPIO_BASE_ADDRESS + 0xD0),
+  MmioRead32 (GPIO_BASE_ADDRESS + 0xD0) | 0x2);
 } else {
   /*
* SD card pins back to eMMC2.
*/
-  MmioWrite32((GPIO_BASE_ADDRESS + 0xD0),
-  MmioRead32(GPIO_BASE_ADDRESS + 0xD0) & ~0x2);
+  MmioWrite32 ((GPIO_BASE_ADDRESS + 0xD0),
+  MmioRead32 (GPIO_BASE_ADDRESS + 0xD0) & ~0x2);
   /*
* WiFi back to Arasan.
*/
-- 
2.13.7


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#64484): https://edk2.groups.io/g/devel/message/64484
Mute This Topic: https://groups.io/mt/76302321/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v1] NetworkPkg/UefiPxeBcDxe: Fix PXE_BOOT_SERVERS usage in boot info parse flow

[Siyuan, Fu]

Reviewed-by: Siyuan Fu 

> -Original Message-
> From: Maciej Rabeda 
> Sent: 2020年8月20日 0:54
> To: devel@edk2.groups.io
> Cc: Wu, Jiaxin ; Fu, Siyuan 
> Subject: [PATCH v1] NetworkPkg/UefiPxeBcDxe: Fix PXE_BOOT_SERVERS usage
> in boot info parse flow
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2876
> 
> According to PXE 2.1 spec, DHCP option 43, tag 6 (PXE_DISCOVERY_CONTROL),
> bit 3 specifies whether PXE client should use/accept TFTP servers defined
> within PXE_BOOT_SERVERS list (tag 8). This bit was not being taken into
> account when choosing boot server IP in PxeBcDhcp4BootInfo().
> 
> Cc: Jiaxin Wu 
> Cc: Siyuan Fu 
> Signed-off-by: Maciej Rabeda 
> ---
>  NetworkPkg/UefiPxeBcDxe/PxeBcBoot.c | 10 +++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/NetworkPkg/UefiPxeBcDxe/PxeBcBoot.c
> b/NetworkPkg/UefiPxeBcDxe/PxeBcBoot.c
> index d062a526077b..ed9bca0f7de3 100644
> --- a/NetworkPkg/UefiPxeBcDxe/PxeBcBoot.c
> +++ b/NetworkPkg/UefiPxeBcDxe/PxeBcBoot.c
> @@ -499,12 +499,16 @@ PxeBcDhcp4BootInfo (
> 
> 
>//
> 
>// Parse the boot server address.
> 
> -  // If prompt/discover is disabled, get the first boot server from the boot
> servers list.
> 
> -  // Otherwise, parse the boot server Ipv4 address from next server address 
> field
> in DHCP header.
> 
> +  // If prompt/discover is disabled, server list should be used and is 
> present
> (DHCP option 43),
> 
> +  // get the first boot server from the boot servers list.
> 
> +  // Otherwise, parse the boot server IPv4 address from next server address
> field in DHCP header.
> 
>// If all these fields are not available, use option 54 instead.
> 
>//
> 
>VendorOpt = >VendorOpt;
> 
> -  if (IS_DISABLE_PROMPT_MENU (VendorOpt->DiscoverCtrl) &&
> IS_VALID_BOOT_SERVERS (VendorOpt->BitMap)) {
> 
> +  if (IS_DISABLE_PROMPT_MENU (VendorOpt->DiscoverCtrl) &&
> 
> +  IS_VALID_BOOT_SERVERS (VendorOpt->BitMap) &&
> 
> +  IS_ENABLE_USE_SERVER_LIST (VendorOpt->DiscoverCtrl))
> 
> +  {
> 
>  Entry = VendorOpt->BootSvr;
> 
>  if (VendorOpt->BootSvrLen >= sizeof (PXEBC_BOOT_SVR_ENTRY) && Entry-
> >IpCnt > 0) {
> 
>CopyMem (
> 
> --
> 2.24.0.windows.2


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#64479): https://edk2.groups.io/g/devel/message/64479
Mute This Topic: https://groups.io/mt/76290910/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v3 0/6] Extend Last Attempt Status Usage

[Michael D Kinney]

Hi Michael,

A couple a couple general questions:

1) I see the entire range from 0x2000-0x3FFF is reserved for FmpDeviceLib.  If 
we
   every add more device/platform specific libs for FMP, there are no ranges 
available.
   Should we limit the FmpDeviceLib to a smaller range to support future 
expansion?

   Also, the style of LastAttemptStatus.h with extra defines for the length of 
   each range is hard to read, and I do not think there are any consumers of the
   length defines from this public include file.  Since there are really only 3
   defined ranges, couldn't this be simplified to min/max defines for each range
   for a total of 6 #defines.  I do not expect ranges (once defined) to change 
in
   length, and the most that might happen in the future is adding new ranges for
   new lib classes in the unused ranges.

2) This series makes non-backwards compatible changes to some of the lib 
classes.
   I agree this is the cleanest way to add support for the vendor specific 
   last attempt status.  It does mean that existing implementations will have 
   to update their lib implementations to be compatible with this new version.
   I would be slightly cleaner to introduce new APIs with support for the
   vendor specific last attempt status codes.  Then update all libs to produce
   both the existing APIs and the new APIs (The old APIs can call the new APIs).
   Then update FmpDxe to use the new APIs.  This would be 3 patch series.
   If FmpDxe never calls the old APIs, then we could (at a future date)
   delete the old APIs from the lib class and the lib implementations could
   remove the old API that calls the new API.

3) The following APIs in the Null implementations have OUT params.
   Should these OUT params be set to an expected value?

 CheckFmpDependency()
 FmpDeviceGetImage()
 FmpDeviceSetImage()

Thanks,

Mike
   

> -Original Message-
> From: michael.kuba...@outlook.com 
> Sent: Tuesday, August 18, 2020 2:16 PM
> To: devel@edk2.groups.io
> Cc: Gao, Liming ; Kinney, Michael D 
> ; Jiang, Guomin ; Xu,
> Wei6 ; Liu, Zhiguang 
> Subject: [PATCH v3 0/6] Extend Last Attempt Status Usage
> 
> From: Michael Kubacki 
> 
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2802
> 
> This patch series adds more granularity to Last Attempt Status
> codes reported during FMP check image and set image operations
> that greatly improve precision of the status codes.
> 
> The unsuccessful vendor range (0x1000 - 0x4000) was introduced
> in UEFI Specification 2.8. At a high-level, two subranges are
> defined within that range in this patch series:
>   1. The FMP Reserved range - reserved for components implemented
>  in FmpDevicePkg.
>   2. The FMP Device Library Reserved range - reserved for
>  FmpDeviceLib instance-specific usage.
> 
> The ranges are described in a public header file LastAttemptStatus.h
> while the specific codes used within FmpDevicePkg implementation
> are defined in a private header file FmpLastAttemptStatus.h.
> 
> FmpDeviceLib instances should use the range definition from the
> public header file to define Last Attempt Status codes local to
> their library instance.
> 
> Of note, there's multiple approaches to assigning private status
> codes in the FMP Reserved range. For example, individual components
> could define their last attempt status codes locally with the
> range allocated to the component defined in a package-wide private
> header file. However, one goal of the granularity being introduced
> is to provide straightforward traceability to an error source.
> 
> For that reason, it was chosen to define a constant set of codes at
> the package level in FmpLastAttemptStatus.h. For example, if a new
> FmpDependencyLib instance is added, it would not be able to reassign
> status code values in the pre-existing FMP Dependency range; it
> would reuse codes for the same error source and be able to add new
> codes onto the range for its usage. I wanted to highlight this for
> any feedback.
> 
> V3 changes:
>   1. Enhanced range definitions in LastAttemptStatus.h with more
>  completeness providing length, min, and max values.
>   2. Moved the actual Last Attempt Status code assignments to a
>  private header file PrivateInclude/FmpLastAttemptStatus.h.
>   3. Changed the value of
>  LAST_ATTEMPT_STATUS_ERROR_UNSUCCESSFUL_VENDOR_RANGE_MAX
>  to 0x3FFF instead of 0x4000 even though 0x4000 is defined in
>  the UEFI specification. The length is 0x4000 but the max
>  allowed value should be 0x3FFF. This change was made now to
>  prevent implementation compatibility issues in the future.
>   4. Included "DEVICE" in the following macro name to clearly
>  associate it with the FmpDeviceLib library class:
>  LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_ERROR_xxx
>   5. Included a map to help the reader better visualize the range
>  definitions in LastAttemptStatus.h.
>   6. Included additional documentation describing the enum in
>  

Re: [edk2-devel] Propose on enabling TLSv1.3

[Zhiguang Liu]

Hi Matthew,
Sorry for the slow response.
Please allow me some time to verify this change.

Anyone who is interested in the topic are welcomed to give comments, thanks

Thanks
Zhiguang

From: Huang, Matthew (HPS SW) 
Sent: Thursday, August 20, 2020 7:16 AM
To: devel@edk2.groups.io; Huang, Matthew (HPS SW) ; 
Liu, Zhiguang 
Cc: Wei, Kent (HPS SW) ; Lin, Derek (HPS SW) 
; Wang, Nickle (HPS SW) ; Wang, Sunny 
(HPS SW) 
Subject: 回覆: [edk2-devel] Propose on enabling TLSv1.3

Hi Zhiguang:

Any comments on these patches?

Matthew.

寄件者: devel@edk2.groups.io 
mailto:devel@edk2.groups.io>> 代理 Huang, Matthew (HPS SW)
寄件日期: Wednesday, August 12, 2020 7:13 PM
收件者: devel@edk2.groups.io; Huang, Matthew (HPS SW) 
mailto:chao-jui.hu...@hpe.com>>; 
zhiguang@intel.com
副本: Wei, Kent (HPS SW) mailto:kent@hpe.com>>; Lin, Derek 
(HPS SW) mailto:derek.l...@hpe.com>>; Wang, Nickle (HPS SW) 
mailto:nickle.w...@hpe.com>>; Wang, Sunny (HPS SW) 
mailto:sunnyw...@hpe.com>>
主旨: 回覆: [edk2-devel] Propose on enabling TLSv1.3

Hi Zhiguang:

Please refer to the attached ‘tlsv13.patch’ based on tianocore/edk2@be01087e07.

As I mentioned, ‘process_files.pl’ is processed with ActivePerl 5.28 Build  
(64-bit) and MSYS2 MinGW 64-bit, log is attached as ‘process_openssl.txt’.

The problems are still the same, current OpenSSL has two problems:


  1.  It will not ignore disabled TLSv1.3 cipher suites, which results in all 
the TLSv1.3 cipher suites defined in TlsCipherMappingTable will be published no 
matter what the actual value is in 
gEdkiiHttpTlsCipherListGuid.HttpTlsCipherList.
  2.  SSL_set_ciphersuites cannot handle non-TLSv1.3 ciphers, which results in 
the function fails to set any ciphersuite if there are TLSv1.2 ciphers in the 
‘CipherString’ argument.

They are minor ones, but would’ve caused the whole flow acts weird. Those two 
problems are more or less solved or discussed in the OpenSSL scene, but not 
included in EDK2 yet. If anyone wants to test TLSv1.3, attachment 
‘openssl.patch’ is suggested to be applied for a more reasonable outcome.

Regards,
Matthew.
寄件者: devel@edk2.groups.io 
mailto:devel@edk2.groups.io>> 代理 Huang, Matthew (HPS SW)
寄件日期: Monday, August 10, 2020 12:26 PM
收件者: devel@edk2.groups.io; 
zhiguang@intel.com
副本: Wei, Kent (HPS SW) mailto:kent@hpe.com>>; Lin, Derek 
(HPS SW) mailto:derek.l...@hpe.com>>; Wang, Nickle (HPS SW) 
mailto:nickle.w...@hpe.com>>; Wang, Sunny (HPS SW) 
mailto:sunnyw...@hpe.com>>
主旨: Re: [edk2-devel] Propose on enabling TLSv1.3

Hi Zhiguang:

Sure, I love to. But I’m new to the scene, please give me some time to figure 
out how to share the snippet properly, thanks.

Regards,
Matthew.
From: devel@edk2.groups.io 
mailto:devel@edk2.groups.io>> On Behalf Of Zhiguang Liu
Sent: Monday, August 10, 2020 11:00 AM
To: devel@edk2.groups.io; Huang, Matthew (HPS SW) 
mailto:chao-jui.hu...@hpe.com>>
Cc: Wei, Kent (HPS SW) mailto:kent@hpe.com>>; Lin, Derek 
(HPS SW) mailto:derek.l...@hpe.com>>; Wang, Nickle (HPS SW) 
mailto:nickle.w...@hpe.com>>; Wang, Sunny (HPS SW) 
mailto:sunnyw...@hpe.com>>
Subject: Re: [edk2-devel] Propose on enabling TLSv1.3

Hi Matthew,
Can you share the code about implementing tls 1.3 to the community?
We can discuss the problems according to the code.
Thanks
Zhiguang

From: devel@edk2.groups.io 
mailto:devel@edk2.groups.io>> On Behalf Of Huang, Matthew 
(HPS SW)
Sent: Monday, August 3, 2020 1:55 PM
To: devel@edk2.groups.io
Cc: Wei, Kent (HPS SW) mailto:kent@hpe.com>>; Lin, Derek 
(HPS SW) mailto:derek.l...@hpe.com>>; Wang, Nickle (HPS SW) 
mailto:nickle.w...@hpe.com>>; Wang, Sunny (HPS SW) 
mailto:sunnyw...@hpe.com>>
Subject: [edk2-devel] Propose on enabling TLSv1.3

Hi:

It’s Matthew from HPE UEFI team. There is no TLSv1.3 support under current EDK2 
releases, and I’m working on enabling TLSv1.3 under UEFI and the result looks 
promising. OpenSSL have already made RFC8446 happens in late 2018, the 
submodule we’re having on the master branch is more than enough to make the 
whole thing work.

There are several problems needed to be addressed:'

1. OpenSslLib needs a reconfiguration with “no-ec” option on in 
process_files.pl, and no off the shelf Perl built with native Windows command 
prompt could’ve processed the file correctly. But I’ve managed to remove the 
blockage using Perl MSYS2 build under Windows without any error. Since this is 
only a one-timer, I don’t think that would’ve caused too much of a trouble. The 
produced opensslconf.h seems correct, and this is all we need.

2. There are some policies issues caused by OpenSSL, OpenSSL explicitly 
describes that SSL_set_cipher_list is for TLS version 1.2 and lower, 
SSL_set_ciphersuites is for TLSv1.3, 

Re: [edk2-devel] [PATCH v8 5/5] CryptoPkg: OpensslLib: Use RngLib to generate entropy in rand_pool

[Yao, Jiewen]

Reviewed-by: Jiewen Yao 



> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Matthew
> Carlson
> Sent: Thursday, August 20, 2020 3:37 AM
> To: devel@edk2.groups.io
> Cc: Ard Biesheuvel ; Yao, Jiewen
> ; Wang, Jian J ; Lu, XiaoyuX
> ; Yao, Jiewen ; Matthew
> Carlson 
> Subject: [edk2-devel] [PATCH v8 5/5] CryptoPkg: OpensslLib: Use RngLib to
> generate entropy in rand_pool
> 
> From: Matthew Carlson 
> 
> Ref: https://github.com/tianocore/edk2/pull/845
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1871
> 
> Changes OpenSSL to no longer depend on TimerLib and instead use RngLib.
> This allows platforms to decide for themsevles what sort of entropy source
> they provide to OpenSSL and TlsLib.
> 
> Cc: Ard Biesheuvel 
> Cc: Jiewen Yao 
> Cc: Jian J Wang 
> Cc: Xiaoyu Lu 
> 
> Acked-by: Ard Biesheuvel 
> Reviewed-by: Jiewen Yao 
> Signed-off-by: Matthew Carlson 
> ---
>  CryptoPkg/Library/OpensslLib/rand_pool.c   | 265 +---
>  CryptoPkg/Library/OpensslLib/rand_pool_noise.c |  29 ---
>  CryptoPkg/Library/OpensslLib/rand_pool_noise_tsc.c |  43 
>  CryptoPkg/CryptoPkg.dsc|   1 +
>  CryptoPkg/Library/OpensslLib/OpensslLib.inf|  15 +-
>  CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf  |  15 +-
>  CryptoPkg/Library/OpensslLib/rand_pool_noise.h |  29 ---
>  7 files changed, 63 insertions(+), 334 deletions(-)
> 
> diff --git a/CryptoPkg/Library/OpensslLib/rand_pool.c
> b/CryptoPkg/Library/OpensslLib/rand_pool.c
> index 9e0179b03490..490b9e2f4692 100644
> --- a/CryptoPkg/Library/OpensslLib/rand_pool.c
> +++ b/CryptoPkg/Library/OpensslLib/rand_pool.c
> @@ -2,8 +2,8 @@
>OpenSSL_1_1_1b doesn't implement rand_pool_* functions for UEFI.
> 
>The file implement these functions.
> 
> 
> 
> -Copyright (c) 2019, Intel Corporation. All rights reserved.
> 
> -SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> +  Copyright (c) 2019, Intel Corporation. All rights reserved.
> 
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> 
> 
>  **/
> 
> 
> 
> @@ -11,53 +11,18 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>  #include 
> 
> 
> 
>  #include 
> 
> -#include 
> 
> -
> 
> -#include "rand_pool_noise.h"
> 
> -
> 
> -/**
> 
> -  Get some randomness from low-order bits of GetPerformanceCounter results.
> 
> -  And combine them to the 64-bit value
> 
> -
> 
> -  @param[out] RandBuffer pointer to store the 64-bit random value.
> 
> -
> 
> -  @retval TRUERandom number generated successfully.
> 
> -  @retval FALSE   Failed to generate.
> 
> -**/
> 
> -STATIC
> 
> -BOOLEAN
> 
> -EFIAPI
> 
> -GetRandNoise64FromPerformanceCounter(
> 
> -  OUT UINT64  *Rand
> 
> -  )
> 
> -{
> 
> -  UINT32 Index;
> 
> -  UINT32 *RandPtr;
> 
> -
> 
> -  if (NULL == Rand) {
> 
> -return FALSE;
> 
> -  }
> 
> -
> 
> -  RandPtr = (UINT32 *) Rand;
> 
> -
> 
> -  for (Index = 0; Index < 2; Index ++) {
> 
> -*RandPtr = (UINT32) (GetPerformanceCounter () & 0xFF);
> 
> -MicroSecondDelay (10);
> 
> -RandPtr++;
> 
> -  }
> 
> -
> 
> -  return TRUE;
> 
> -}
> 
> +#include 
> 
> 
> 
>  /**
> 
>Calls RandomNumber64 to fill
> 
>a buffer of arbitrary size with random bytes.
> 
> +  This is a shim layer to RngLib.
> 
> 
> 
>@param[in]   LengthSize of the buffer, in bytes,  to fill with.
> 
>@param[out]  RandBufferPointer to the buffer to store the random 
> result.
> 
> 
> 
> -  @retval EFI_SUCCESSRandom bytes generation succeeded.
> 
> -  @retval EFI_NOT_READY  Failed to request random bytes.
> 
> +  @retval TRUERandom bytes generation succeeded.
> 
> +  @retval FALSE   Failed to request random bytes.
> 
> 
> 
>  **/
> 
>  STATIC
> 
> @@ -65,7 +30,7 @@ BOOLEAN
>  EFIAPI
> 
>  RandGetBytes (
> 
>IN UINTN Length,
> 
> -  OUT UINT8*RandBuffer
> 
> +  OUT UINT8   *RandBuffer
> 
>)
> 
>  {
> 
>BOOLEAN Ret;
> 
> @@ -73,17 +38,17 @@ RandGetBytes (
> 
> 
>Ret = FALSE;
> 
> 
> 
> +  if (RandBuffer == NULL) {
> 
> +DEBUG((DEBUG_ERROR, "[OPENSSL_RAND_POOL] NULL RandBuffer. No
> random numbers are generated and your system is not secure\n"));
> 
> +ASSERT (RandBuffer != NULL); // Since we can't generate random numbers,
> we should assert. Otherwise we will just blow up later.
> 
> +return Ret;
> 
> +  }
> 
> +
> 
> +
> 
>while (Length > 0) {
> 
> -//
> 
> -// Get random noise from platform.
> 
> -// If it failed, fallback to PerformanceCounter
> 
> -// If you really care about security, you must override
> 
> -// GetRandomNoise64FromPlatform.
> 
> -//
> 
> -Ret = GetRandomNoise64 ();
> 
> -if (Ret == FALSE) {
> 
> -  Ret = GetRandNoise64FromPerformanceCounter ();
> 
> -}
> 
> +// Use RngLib to get random number
> 
> +Ret = GetRandomNumber64 ();
> 
> +
> 
>  if (!Ret) {
> 
>return Ret;
> 
>  }
> 
> @@ -91,7 +56,8 @@ RandGetBytes (
>*((UINT64*) RandBuffer) = 

回覆: [edk2-devel] Propose on enabling TLSv1.3

[Huang, Matthew (HPS SW)]

Hi Zhiguang:

Any comments on these patches?

Matthew.

寄件者: devel@edk2.groups.io  代理 Huang, Matthew (HPS SW)
寄件日期: Wednesday, August 12, 2020 7:13 PM
收件者: devel@edk2.groups.io; Huang, Matthew (HPS SW) ; 
zhiguang@intel.com
副本: Wei, Kent (HPS SW) ; Lin, Derek (HPS SW) 
; Wang, Nickle (HPS SW) ; Wang, Sunny 
(HPS SW) 
主旨: 回覆: [edk2-devel] Propose on enabling TLSv1.3

Hi Zhiguang:

Please refer to the attached ‘tlsv13.patch’ based on tianocore/edk2@be01087e07.

As I mentioned, ‘process_files.pl’ is processed with ActivePerl 5.28 Build  
(64-bit) and MSYS2 MinGW 64-bit, log is attached as ‘process_openssl.txt’.

The problems are still the same, current OpenSSL has two problems:


  1.  It will not ignore disabled TLSv1.3 cipher suites, which results in all 
the TLSv1.3 cipher suites defined in TlsCipherMappingTable will be published no 
matter what the actual value is in 
gEdkiiHttpTlsCipherListGuid.HttpTlsCipherList.
  2.  SSL_set_ciphersuites cannot handle non-TLSv1.3 ciphers, which results in 
the function fails to set any ciphersuite if there are TLSv1.2 ciphers in the 
‘CipherString’ argument.

They are minor ones, but would’ve caused the whole flow acts weird. Those two 
problems are more or less solved or discussed in the OpenSSL scene, but not 
included in EDK2 yet. If anyone wants to test TLSv1.3, attachment 
‘openssl.patch’ is suggested to be applied for a more reasonable outcome.

Regards,
Matthew.
寄件者: devel@edk2.groups.io 
mailto:devel@edk2.groups.io>> 代理 Huang, Matthew (HPS SW)
寄件日期: Monday, August 10, 2020 12:26 PM
收件者: devel@edk2.groups.io; 
zhiguang@intel.com
副本: Wei, Kent (HPS SW) mailto:kent@hpe.com>>; Lin, Derek 
(HPS SW) mailto:derek.l...@hpe.com>>; Wang, Nickle (HPS SW) 
mailto:nickle.w...@hpe.com>>; Wang, Sunny (HPS SW) 
mailto:sunnyw...@hpe.com>>
主旨: Re: [edk2-devel] Propose on enabling TLSv1.3

Hi Zhiguang:

Sure, I love to. But I’m new to the scene, please give me some time to figure 
out how to share the snippet properly, thanks.

Regards,
Matthew.
From: devel@edk2.groups.io 
mailto:devel@edk2.groups.io>> On Behalf Of Zhiguang Liu
Sent: Monday, August 10, 2020 11:00 AM
To: devel@edk2.groups.io; Huang, Matthew (HPS SW) 
mailto:chao-jui.hu...@hpe.com>>
Cc: Wei, Kent (HPS SW) mailto:kent@hpe.com>>; Lin, Derek 
(HPS SW) mailto:derek.l...@hpe.com>>; Wang, Nickle (HPS SW) 
mailto:nickle.w...@hpe.com>>; Wang, Sunny (HPS SW) 
mailto:sunnyw...@hpe.com>>
Subject: Re: [edk2-devel] Propose on enabling TLSv1.3

Hi Matthew,
Can you share the code about implementing tls 1.3 to the community?
We can discuss the problems according to the code.
Thanks
Zhiguang

From: devel@edk2.groups.io 
mailto:devel@edk2.groups.io>> On Behalf Of Huang, Matthew 
(HPS SW)
Sent: Monday, August 3, 2020 1:55 PM
To: devel@edk2.groups.io
Cc: Wei, Kent (HPS SW) mailto:kent@hpe.com>>; Lin, Derek 
(HPS SW) mailto:derek.l...@hpe.com>>; Wang, Nickle (HPS SW) 
mailto:nickle.w...@hpe.com>>; Wang, Sunny (HPS SW) 
mailto:sunnyw...@hpe.com>>
Subject: [edk2-devel] Propose on enabling TLSv1.3

Hi:

It’s Matthew from HPE UEFI team. There is no TLSv1.3 support under current EDK2 
releases, and I’m working on enabling TLSv1.3 under UEFI and the result looks 
promising. OpenSSL have already made RFC8446 happens in late 2018, the 
submodule we’re having on the master branch is more than enough to make the 
whole thing work.

There are several problems needed to be addressed:'

1. OpenSslLib needs a reconfiguration with “no-ec” option on in 
process_files.pl, and no off the shelf Perl built with native Windows command 
prompt could’ve processed the file correctly. But I’ve managed to remove the 
blockage using Perl MSYS2 build under Windows without any error. Since this is 
only a one-timer, I don’t think that would’ve caused too much of a trouble. The 
produced opensslconf.h seems correct, and this is all we need.

2. There are some policies issues caused by OpenSSL, OpenSSL explicitly 
describes that SSL_set_cipher_list is for TLS version 1.2 and lower, 
SSL_set_ciphersuites is for TLSv1.3, but these function are tangled to each 
other and the behavior is not equally fair. In current revision EDK2 included 
in the OpenSSL submodule, SSL_set_cipher_list can parse v1.3 cipher suites but 
will not apply them, meanwhile SSL_set_ciphersuites cannot support any cipher 
lower than v1.3. This will cause a problem that when user applies auto 
versioning, TLSv1.3 will not be applied even if v1.3 is enabled except setting 
an empty list using SSL_set_cipher_list.

3. Apart from point 2., SSL_set_ciphersuites in current revision EDK2 included 
in the OpenSSL submodule, cannot exclude ciphersuites that user disabled, so 
every cipher suites will be in the list for server to

But I browsed all OpenSSL github PRs or 

Re: [edk2-devel] EDK2 Specs PDF links don't work.

[Michael D Kinney]

Tim,

Agreed…the wiki pages have not been updated for the change.

Sorry for the confusion.

I will evaluate PDF and other eBook versions.

Mike

From: devel@edk2.groups.io  On Behalf Of Tim Lewis
Sent: Wednesday, August 19, 2020 1:59 PM
To: Kinney, Michael D ; devel@edk2.groups.io
Subject: Re: [edk2-devel] EDK2 Specs PDF links don't work.

Mike –

I miss the ability to search easily off-line. What really confused me is that 
all of the links for PDF still exist.

Tim

From: Kinney, Michael D 
mailto:michael.d.kin...@intel.com>>
Sent: Wednesday, August 19, 2020 1:08 PM
To: devel@edk2.groups.io; 
tim.le...@insyde.com; Kinney, Michael D 
mailto:michael.d.kin...@intel.com>>
Subject: RE: [edk2-devel] EDK2 Specs PDF links don't work.

Hi Tim,

We lost the PDF feature when we moved to the new GitBook and the old one was 
retired.

If offline PDFs are a must have feature, we have additional work we can do to 
re-enable.

Mike

From: devel@edk2.groups.io 
mailto:devel@edk2.groups.io>> On Behalf Of Tim Lewis
Sent: Wednesday, August 19, 2020 11:59 AM
To: devel@edk2.groups.io
Subject: [edk2-devel] EDK2 Specs PDF links don't work.

Sorry, I’m new to GitBook, but the PDF links for the specs are not working. It 
just brings me into the middle of the GitBook HTML pages. I like to have an 
off-line reference and PDFs worked well in the past. I see the link points to a 
PDF URL, but the result is still not PDF. I’d prefer not to have to become

Any help is appreciated.

Thanks,

Tim


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#64474): https://edk2.groups.io/g/devel/message/64474
Mute This Topic: https://groups.io/mt/76293464/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] EDK2 Specs PDF links don't work.

[Tim Lewis]

Mike –

 

I miss the ability to search easily off-line. What really confused me is that 
all of the links for PDF still exist. 

 

Tim

 

From: Kinney, Michael D  
Sent: Wednesday, August 19, 2020 1:08 PM
To: devel@edk2.groups.io; tim.le...@insyde.com; Kinney, Michael D 

Subject: RE: [edk2-devel] EDK2 Specs PDF links don't work.

 

Hi Tim,

 

We lost the PDF feature when we moved to the new GitBook and the old one was 
retired.

 

If offline PDFs are a must have feature, we have additional work we can do to 
re-enable.

 

Mike

 

From: devel@edk2.groups.io   mailto:devel@edk2.groups.io> > On Behalf Of Tim Lewis
Sent: Wednesday, August 19, 2020 11:59 AM
To: devel@edk2.groups.io  
Subject: [edk2-devel] EDK2 Specs PDF links don't work.

 

Sorry, I’m new to GitBook, but the PDF links for the specs are not working. It 
just brings me into the middle of the GitBook HTML pages. I like to have an 
off-line reference and PDFs worked well in the past. I see the link points to a 
PDF URL, but the result is still not PDF. I’d prefer not to have to become

 

Any help is appreciated.

 

Thanks,

 

Tim




-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#64473): https://edk2.groups.io/g/devel/message/64473
Mute This Topic: https://groups.io/mt/76293464/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] EDK2 Specs PDF links don't work.

[Kirkendall, Garrett]

I'll second the request for an offline option

Garrett Kirkendall
SMTS Firmware Engineer | CTE
7171 Southwest Parkway, Austin, TX 78735 USA
AMD   facebook  |  amd.com

From: devel@edk2.groups.io  On Behalf Of Michael D Kinney 
via groups.io
Sent: Wednesday, August 19, 2020 3:08 PM
To: devel@edk2.groups.io; tim.le...@insyde.com; Kinney, Michael D 

Subject: Re: [edk2-devel] EDK2 Specs PDF links don't work.

[CAUTION: External Email]
Hi Tim,

We lost the PDF feature when we moved to the new GitBook and the old one was 
retired.

If offline PDFs are a must have feature, we have additional work we can do to 
re-enable.

Mike

From: devel@edk2.groups.io 
mailto:devel@edk2.groups.io>> On Behalf Of Tim Lewis
Sent: Wednesday, August 19, 2020 11:59 AM
To: devel@edk2.groups.io
Subject: [edk2-devel] EDK2 Specs PDF links don't work.

Sorry, I’m new to GitBook, but the PDF links for the specs are not working. It 
just brings me into the middle of the GitBook HTML pages. I like to have an 
off-line reference and PDFs worked well in the past. I see the link points to a 
PDF URL, but the result is still not PDF. I’d prefer not to have to become

Any help is appreciated.

Thanks,

Tim


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#64472): https://edk2.groups.io/g/devel/message/64472
Mute This Topic: https://groups.io/mt/76293464/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] EDK2 Specs PDF links don't work.

[Michael D Kinney]

Hi Tim,

We lost the PDF feature when we moved to the new GitBook and the old one was 
retired.

If offline PDFs are a must have feature, we have additional work we can do to 
re-enable.

Mike

From: devel@edk2.groups.io  On Behalf Of Tim Lewis
Sent: Wednesday, August 19, 2020 11:59 AM
To: devel@edk2.groups.io
Subject: [edk2-devel] EDK2 Specs PDF links don't work.

Sorry, I’m new to GitBook, but the PDF links for the specs are not working. It 
just brings me into the middle of the GitBook HTML pages. I like to have an 
off-line reference and PDFs worked well in the past. I see the link points to a 
PDF URL, but the result is still not PDF. I’d prefer not to have to become

Any help is appreciated.

Thanks,

Tim


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#64471): https://edk2.groups.io/g/devel/message/64471
Mute This Topic: https://groups.io/mt/76293464/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v8 1/5] MdePkg: TimerRngLib: Added RngLib that uses TimerLib

[Matthew Carlson]

From: Matthew Carlson 

Added a new RngLib that provides random numbers from the TimerLib
using the performance counter. This is meant to be used for OpenSSL
to replicate past behavior. This should not be used in production as
a real source of entropy.

Ref: https://github.com/tianocore/edk2/pull/845
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1871

Cc: Ard Biesheuvel 
Cc: Michael D Kinney 
Cc: Liming Gao 
Cc: Zhiguang Liu 
Signed-off-by: Matthew Carlson 
---
 MdePkg/Library/BaseRngLibTimerLib/RngLibTimer.c  | 191 

 MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.inf |  36 
 MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.uni |  15 ++
 MdePkg/MdePkg.dsc|   3 +-
 4 files changed, 244 insertions(+), 1 deletion(-)

diff --git a/MdePkg/Library/BaseRngLibTimerLib/RngLibTimer.c 
b/MdePkg/Library/BaseRngLibTimerLib/RngLibTimer.c
new file mode 100644
index ..c72aa335823d
--- /dev/null
+++ b/MdePkg/Library/BaseRngLibTimerLib/RngLibTimer.c
@@ -0,0 +1,191 @@
+/** @file
+  BaseRng Library that uses the TimerLib to provide reasonably random numbers.
+  Do not use this on a production system.
+
+  Copyright (c) Microsoft Corporation.
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#include 
+#include 
+#include 
+#include 
+
+/**
+ * Using the TimerLib GetPerformanceCounterProperties() we delay
+ * for enough time for the PerformanceCounter to increment.
+ * Depending on your system
+ *
+ * If the return value from GetPerformanceCounterProperties (TimerLib)
+ * is zero, this function will not delay and attempt to assert.
+ */
+STATIC
+UINT32
+CalculateMinimumDecentDelayInMicroseconds (
+  VOID
+  )
+{
+  UINT64 StartValue;
+  UINT64 EndValue;
+  UINT64 CounterHz;
+  UINT64 MinumumDelayInMicroSeconds;
+
+  // Get the counter properties
+  CounterHz = GetPerformanceCounterProperties (, );
+  // Make sure we won't divide by zero
+  if (CounterHz == 0) {
+ASSERT(CounterHz != 0); // Assert so the developer knows something is wrong
+return;
+  }
+  // Calculate the minimum delay based on 1.5 microseconds divided by the 
hertz.
+  // We calculate the length of a cycle (1/CounterHz) and multiply it by 1.5 
microseconds
+  // This ensures that the performance counter has increased by at least one
+  return (UINT32)(MAX(DivU64x64Remainder(150 / CounterHz, NULL), 1));
+}
+
+
+/**
+  Generates a 16-bit random number.
+
+  if Rand is NULL, then ASSERT().
+
+  @param[out] Rand Buffer pointer to store the 16-bit random value.
+
+  @retval TRUE Random number generated successfully.
+  @retval FALSEFailed to generate the random number.
+
+**/
+BOOLEAN
+EFIAPI
+GetRandomNumber16 (
+  OUT UINT16*Rand
+  )
+{
+  UINT32  Index;
+  UINT8  *RandPtr;
+  UINT32  DelayInMicroSeconds;
+
+  ASSERT (Rand != NULL);
+
+  if (Rand == NULL) {
+return FALSE;
+  }
+  DelayInMicroSeconds = CalculateMinimumDecentDelayInMicroseconds ();
+  RandPtr = (UINT8*)Rand;
+  // Get 2 bytes of random ish data
+  for (Index = 0; Index < 2; Index ++) {
+*RandPtr = (UINT8)(GetPerformanceCounter () & 0xFF);
+// Delay to give the performance counter a chance to change
+MicroSecondDelay (DelayInMicroSeconds);
+RandPtr++;
+  }
+  return TRUE;
+}
+
+/**
+  Generates a 32-bit random number.
+
+  if Rand is NULL, then ASSERT().
+
+  @param[out] Rand Buffer pointer to store the 32-bit random value.
+
+  @retval TRUE Random number generated successfully.
+  @retval FALSEFailed to generate the random number.
+
+**/
+BOOLEAN
+EFIAPI
+GetRandomNumber32 (
+  OUT UINT32*Rand
+  )
+{
+  UINT32  Index;
+  UINT8  *RandPtr;
+  UINT32  DelayInMicroSeconds;
+
+  ASSERT (Rand != NULL);
+
+  if (NULL == Rand) {
+return FALSE;
+  }
+
+  RandPtr = (UINT8 *) Rand;
+  DelayInMicroSeconds = CalculateMinimumDecentDelayInMicroseconds ();
+  // Get 4 bytes of random ish data
+  for (Index = 0; Index < 4; Index ++) {
+*RandPtr = (UINT8) (GetPerformanceCounter () & 0xFF);
+// Delay to give the performance counter a chance to change
+MicroSecondDelay (DelayInMicroSeconds);
+RandPtr++;
+  }
+  return TRUE;
+}
+
+/**
+  Generates a 64-bit random number.
+
+  if Rand is NULL, then ASSERT().
+
+  @param[out] Rand Buffer pointer to store the 64-bit random value.
+
+  @retval TRUE Random number generated successfully.
+  @retval FALSEFailed to generate the random number.
+
+**/
+BOOLEAN
+EFIAPI
+GetRandomNumber64 (
+  OUT UINT64*Rand
+  )
+{
+  UINT32  Index;
+  UINT8  *RandPtr;
+  UINT32  DelayInMicroSeconds;
+
+  ASSERT (Rand != NULL);
+
+  if (NULL == Rand) {
+return FALSE;
+  }
+
+  RandPtr = (UINT8 *) Rand;
+  DelayInMicroSeconds = CalculateMinimumDecentDelayInMicroseconds ();
+  // Get 8 bytes of random ish data
+  for (Index = 0; Index < 8; Index ++) {
+*RandPtr = (UINT8)(GetPerformanceCounter () & 

[edk2-devel] [PATCH v8 2/5] MdePkg: BaseRngLibDxe: Add RngLib that uses RngDxe

[Matthew Carlson]

From: Matthew Carlson 

This adds a RngLib that uses the RngProtocol to provide randomness.
This means that the RngLib is meant to be used with DXE_DRIVERS.

Ref: https://github.com/tianocore/edk2/pull/845
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1871

Cc: Ard Biesheuvel 
Cc: Michael D Kinney 
Cc: Liming Gao 
Cc: Zhiguang Liu 
Signed-off-by: Matthew Carlson 
---
 MdePkg/Library/DxeRngLib/DxeRngLib.c   | 206 
 MdePkg/Library/DxeRngLib/DxeRngLib.inf |  38 
 MdePkg/Library/DxeRngLib/DxeRngLib.uni |  15 ++
 MdePkg/MdePkg.dsc  |   4 +-
 4 files changed, 262 insertions(+), 1 deletion(-)

diff --git a/MdePkg/Library/DxeRngLib/DxeRngLib.c 
b/MdePkg/Library/DxeRngLib/DxeRngLib.c
new file mode 100644
index ..0bd6585357b5
--- /dev/null
+++ b/MdePkg/Library/DxeRngLib/DxeRngLib.c
@@ -0,0 +1,206 @@
+/** @file
+ Provides an implementation of the library class RngLib that uses the Rng 
protocol.
+
+ Copyright (c) Microsoft Corporation. All rights reserved.
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/**
+Routine Description:
+
+Generates a random number via the NIST
+800-9A algorithm.  Refer to
+http://csrc.nist.gov/groups/STM/cavp/documents/drbg/DRBGVS.pdf
+for more information.
+
+Arguments:
+
+Buffer  -- Buffer to receive the random number.
+BufferSize  -- Number of bytes in Buffer.
+
+Return Value:
+
+EFI_SUCCESS or underlying failure code.
+
+**/
+STATIC
+EFI_STATUS
+GenerateRandomNumberViaNist800Algorithm (
+  OUT UINT8 *Buffer,
+  IN  UINTN  BufferSize
+  )
+{
+  EFI_STATUSStatus;
+  EFI_RNG_PROTOCOL *RngProtocol;
+
+  RngProtocol = NULL;
+
+  if (Buffer == NULL) {
+  DEBUG((DEBUG_ERROR, "[%a] Buffer == NULL.\n", __FUNCTION__));
+  return EFI_INVALID_PARAMETER;
+  }
+
+  Status = gBS->LocateProtocol (, NULL, (VOID 
**));
+  if (EFI_ERROR (Status) || RngProtocol == NULL) {
+  DEBUG((DEBUG_ERROR, "%a: Could not locate RNG prototocol, Status = 
%r\n", __FUNCTION__, Status));
+  return Status;
+  }
+
+  Status = RngProtocol->GetRNG (RngProtocol, 
, BufferSize, Buffer);
+  DEBUG((DEBUG_INFO, "%a: GetRNG algorithm CTR-256 - Status = %r\n", 
__FUNCTION__, Status));
+  if(!EFI_ERROR(Status)) {
+return Status;
+  }
+
+  Status = RngProtocol->GetRNG (RngProtocol, 
, BufferSize, Buffer);
+  DEBUG((DEBUG_INFO, "%a: GetRNG algorithm HMAC-256 - Status = %r\n", 
__FUNCTION__, Status));
+  if(!EFI_ERROR(Status)) {
+return Status;
+  }
+
+  Status = RngProtocol->GetRNG (RngProtocol, 
, BufferSize, Buffer);
+  DEBUG((DEBUG_INFO, "%a: GetRNG algorithm Hash-256 - Status = %r\n", 
__FUNCTION__, Status));
+  if (!EFI_ERROR(Status)) {
+return Status;
+  }
+  // If all the other methods have failed, use the default method from the 
RngProtocol
+  Status = RngProtocol->GetRNG (RngProtocol, NULL, BufferSize, Buffer);
+  DEBUG((DEBUG_INFO, "%a: GetRNG algorithm Hash-256 - Status = %r\n", 
__FUNCTION__, Status));
+  if (!EFI_ERROR(Status)) {
+return Status;
+  }
+  // If we get to this point, we have failed
+  DEBUG((DEBUG_ERROR, "%a: GetRNG() failed, staus = %r\n", __FUNCTION__, 
Status));
+
+  return Status;
+}// GenerateRandomNumberViaNist800Algorithm()
+
+
+/**
+  Generates a 16-bit random number.
+
+  if Rand is NULL, return FALSE.
+
+  @param[out] Rand Buffer pointer to store the 16-bit random value.
+
+  @retval TRUE Random number generated successfully.
+  @retval FALSEFailed to generate the random number.
+
+**/
+BOOLEAN
+EFIAPI
+GetRandomNumber16 (
+  OUT UINT16*Rand
+  )
+{
+  EFI_STATUS Status;
+
+  if (Rand == NULL)
+  {
+return FALSE;
+  }
+
+  Status = GenerateRandomNumberViaNist800Algorithm ((UINT8 *)Rand, 2);
+  if (EFI_ERROR (Status))
+  {
+return FALSE;
+  }
+  return TRUE;
+}
+
+/**
+  Generates a 32-bit random number.
+
+  if Rand is NULL, return FALSE.
+
+  @param[out] Rand Buffer pointer to store the 32-bit random value.
+
+  @retval TRUE Random number generated successfully.
+  @retval FALSEFailed to generate the random number.
+
+**/
+BOOLEAN
+EFIAPI
+GetRandomNumber32 (
+  OUT UINT32*Rand
+  )
+{
+  EFI_STATUS Status;
+
+  if (Rand == NULL) {
+return FALSE;
+  }
+
+  Status = GenerateRandomNumberViaNist800Algorithm ((UINT8 *)Rand, 4);
+  if (EFI_ERROR (Status)) {
+return FALSE;
+  }
+  return TRUE;
+}
+
+/**
+  Generates a 64-bit random number.
+
+  if Rand is NULL, return FALSE.
+
+  @param[out] Rand Buffer pointer to store the 64-bit random value.
+
+  @retval TRUE Random number generated successfully.
+  @retval FALSEFailed to generate the random number.
+
+**/
+BOOLEAN
+EFIAPI
+GetRandomNumber64 (
+  OUT UINT64*Rand
+  )
+{
+  EFI_STATUS Status;
+
+  if (Rand == NULL)
+  {
+return FALSE;
+  }
+
+  Status = GenerateRandomNumberViaNist800Algorithm ((UINT8 

[edk2-devel] [PATCH v8 5/5] CryptoPkg: OpensslLib: Use RngLib to generate entropy in rand_pool

[Matthew Carlson]

From: Matthew Carlson 

Ref: https://github.com/tianocore/edk2/pull/845
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1871

Changes OpenSSL to no longer depend on TimerLib and instead use RngLib.
This allows platforms to decide for themsevles what sort of entropy source
they provide to OpenSSL and TlsLib.

Cc: Ard Biesheuvel 
Cc: Jiewen Yao 
Cc: Jian J Wang 
Cc: Xiaoyu Lu 

Acked-by: Ard Biesheuvel 
Reviewed-by: Jiewen Yao 
Signed-off-by: Matthew Carlson 
---
 CryptoPkg/Library/OpensslLib/rand_pool.c   | 265 +---
 CryptoPkg/Library/OpensslLib/rand_pool_noise.c |  29 ---
 CryptoPkg/Library/OpensslLib/rand_pool_noise_tsc.c |  43 
 CryptoPkg/CryptoPkg.dsc|   1 +
 CryptoPkg/Library/OpensslLib/OpensslLib.inf|  15 +-
 CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf  |  15 +-
 CryptoPkg/Library/OpensslLib/rand_pool_noise.h |  29 ---
 7 files changed, 63 insertions(+), 334 deletions(-)

diff --git a/CryptoPkg/Library/OpensslLib/rand_pool.c 
b/CryptoPkg/Library/OpensslLib/rand_pool.c
index 9e0179b03490..490b9e2f4692 100644
--- a/CryptoPkg/Library/OpensslLib/rand_pool.c
+++ b/CryptoPkg/Library/OpensslLib/rand_pool.c
@@ -2,8 +2,8 @@
   OpenSSL_1_1_1b doesn't implement rand_pool_* functions for UEFI.
   The file implement these functions.
 
-Copyright (c) 2019, Intel Corporation. All rights reserved.
-SPDX-License-Identifier: BSD-2-Clause-Patent
+  Copyright (c) 2019, Intel Corporation. All rights reserved.
+  SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
 
@@ -11,53 +11,18 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #include 
 
 #include 
-#include 
-
-#include "rand_pool_noise.h"
-
-/**
-  Get some randomness from low-order bits of GetPerformanceCounter results.
-  And combine them to the 64-bit value
-
-  @param[out] RandBuffer pointer to store the 64-bit random value.
-
-  @retval TRUERandom number generated successfully.
-  @retval FALSE   Failed to generate.
-**/
-STATIC
-BOOLEAN
-EFIAPI
-GetRandNoise64FromPerformanceCounter(
-  OUT UINT64  *Rand
-  )
-{
-  UINT32 Index;
-  UINT32 *RandPtr;
-
-  if (NULL == Rand) {
-return FALSE;
-  }
-
-  RandPtr = (UINT32 *) Rand;
-
-  for (Index = 0; Index < 2; Index ++) {
-*RandPtr = (UINT32) (GetPerformanceCounter () & 0xFF);
-MicroSecondDelay (10);
-RandPtr++;
-  }
-
-  return TRUE;
-}
+#include 
 
 /**
   Calls RandomNumber64 to fill
   a buffer of arbitrary size with random bytes.
+  This is a shim layer to RngLib.
 
   @param[in]   LengthSize of the buffer, in bytes,  to fill with.
   @param[out]  RandBufferPointer to the buffer to store the random result.
 
-  @retval EFI_SUCCESSRandom bytes generation succeeded.
-  @retval EFI_NOT_READY  Failed to request random bytes.
+  @retval TRUERandom bytes generation succeeded.
+  @retval FALSE   Failed to request random bytes.
 
 **/
 STATIC
@@ -65,7 +30,7 @@ BOOLEAN
 EFIAPI
 RandGetBytes (
   IN UINTN Length,
-  OUT UINT8*RandBuffer
+  OUT UINT8   *RandBuffer
   )
 {
   BOOLEAN Ret;
@@ -73,17 +38,17 @@ RandGetBytes (
 
   Ret = FALSE;
 
+  if (RandBuffer == NULL) {
+DEBUG((DEBUG_ERROR, "[OPENSSL_RAND_POOL] NULL RandBuffer. No random 
numbers are generated and your system is not secure\n"));
+ASSERT (RandBuffer != NULL); // Since we can't generate random numbers, we 
should assert. Otherwise we will just blow up later.
+return Ret;
+  }
+
+
   while (Length > 0) {
-//
-// Get random noise from platform.
-// If it failed, fallback to PerformanceCounter
-// If you really care about security, you must override
-// GetRandomNoise64FromPlatform.
-//
-Ret = GetRandomNoise64 ();
-if (Ret == FALSE) {
-  Ret = GetRandNoise64FromPerformanceCounter ();
-}
+// Use RngLib to get random number
+Ret = GetRandomNumber64 ();
+
 if (!Ret) {
   return Ret;
 }
@@ -91,7 +56,8 @@ RandGetBytes (
   *((UINT64*) RandBuffer) = TempRand;
   RandBuffer += sizeof (UINT64);
   Length -= sizeof (TempRand);
-} else {
+}
+else {
   CopyMem (RandBuffer, , Length);
   Length = 0;
 }
@@ -100,125 +66,6 @@ RandGetBytes (
   return Ret;
 }
 
-/**
-  Creates a 128bit random value that is fully forward and backward prediction 
resistant,
-  suitable for seeding a NIST SP800-90 Compliant.
-  This function takes multiple random numbers from PerformanceCounter to 
ensure reseeding
-  and performs AES-CBC-MAC over the data to compute the seed value.
-
-  @param[out]  SeedBufferPointer to a 128bit buffer to store the random 
seed.
-
-  @retval TRUERandom seed generation succeeded.
-  @retval FALSE  Failed to request random bytes.
-
-**/
-STATIC
-BOOLEAN
-EFIAPI
-RandGetSeed128 (
-  OUT UINT8*SeedBuffer
-  )
-{
-  BOOLEAN Ret;
-  UINT8   RandByte[16];
-  UINT8   Key[16];
-  UINT8   Ffv[16];
-  UINT8   Xored[16];
-  UINT32  Index;
-  

[edk2-devel] [PATCH v8 4/5] ArmVirtPkg: Add RngLib based on TimerLib for CryptoPkg

[Matthew Carlson]

From: Matthew Carlson 

Updates the DSC for the ArmVirtPkg platform to add a RngLib that uses the
TimerLib. This is due to a later change that adds TimerLib as a dependency
for OpenSSL. The TimerLib based RngLib mimics the behavior of OpenSSL
previously and it is recommended to switch to a better source of
entropy than the system's performance counter.

Ref: https://github.com/tianocore/edk2/pull/845
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1871

Cc: Laszlo Ersek 
Cc: Ard Biesheuvel 
Cc: Leif Lindholm 

Reviewed-by: Laszlo Ersek 
Signed-off-by: Matthew Carlson 
---
 ArmVirtPkg/ArmVirt.dsc.inc | 1 +
 1 file changed, 1 insertion(+)

diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
index cf44fc73890b..cb3845d2bd37 100644
--- a/ArmVirtPkg/ArmVirt.dsc.inc
+++ b/ArmVirtPkg/ArmVirt.dsc.inc
@@ -160,6 +160,7 @@
   OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf
 !endif
   BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
+  RngLib|MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.inf
 
   #
   # Secure Boot dependencies
-- 
2.28.0.windows.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#64469): https://edk2.groups.io/g/devel/message/64469
Mute This Topic: https://groups.io/mt/76294217/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v8 3/5] OvmfPkg: Add RngLib based on TimerLib for Crypto

[Matthew Carlson]

From: Matthew Carlson 

Updates the DSC's for Ovmf based platforms to add a RngLib that uses the
TimerLib. This is due to a later change that adds TimerLib as a dependency
for OpenSSL. The TimerLib based RngLib mimics the behavior of OpenSSL
previously and it is recommended to switch to a better source of
entropy than the system's performance counter.

Ref: https://github.com/tianocore/edk2/pull/845
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1871

Cc: Jordan Justen 
Cc: Laszlo Ersek 
Cc: Ard Biesheuvel 
Cc: Anthony Perard 
Cc: Julien Grall 

Reviewed-by: Laszlo Ersek 
Signed-off-by: Matthew Carlson 
---
 OvmfPkg/Bhyve/BhyvePkgX64.dsc | 1 +
 OvmfPkg/OvmfPkgIa32.dsc   | 1 +
 OvmfPkg/OvmfPkgIa32X64.dsc| 1 +
 OvmfPkg/OvmfPkgX64.dsc| 1 +
 OvmfPkg/OvmfXen.dsc   | 1 +
 5 files changed, 5 insertions(+)

diff --git a/OvmfPkg/Bhyve/BhyvePkgX64.dsc b/OvmfPkg/Bhyve/BhyvePkgX64.dsc
index 99e214619be0..0bf1acbc8dc8 100644
--- a/OvmfPkg/Bhyve/BhyvePkgX64.dsc
+++ b/OvmfPkg/Bhyve/BhyvePkgX64.dsc
@@ -185,6 +185,7 @@
 !else
   OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf
 !endif
+  RngLib|MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.inf
 
 !if $(SECURE_BOOT_ENABLE) == TRUE
   
PlatformSecureLib|OvmfPkg/Bhyve/Library/PlatformSecureLib/PlatformSecureLib.inf
diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
index 133a9a93c071..fa18adeb5c5a 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -189,6 +189,7 @@
 !else
   OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf
 !endif
+  RngLib|MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.inf
 
 !if $(SECURE_BOOT_ENABLE) == TRUE
   PlatformSecureLib|OvmfPkg/Library/PlatformSecureLib/PlatformSecureLib.inf
diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index 338c38db29b5..7456a154168d 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -193,6 +193,7 @@
 !else
   OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf
 !endif
+  RngLib|MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.inf
 
 !if $(SECURE_BOOT_ENABLE) == TRUE
   PlatformSecureLib|OvmfPkg/Library/PlatformSecureLib/PlatformSecureLib.inf
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index b80710fbdca4..5bda143fd14d 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -193,6 +193,7 @@
 !else
   OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf
 !endif
+  RngLib|MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.inf
 
 !if $(SECURE_BOOT_ENABLE) == TRUE
   PlatformSecureLib|OvmfPkg/Library/PlatformSecureLib/PlatformSecureLib.inf
diff --git a/OvmfPkg/OvmfXen.dsc b/OvmfPkg/OvmfXen.dsc
index 37b63a874067..e562abd7175d 100644
--- a/OvmfPkg/OvmfXen.dsc
+++ b/OvmfPkg/OvmfXen.dsc
@@ -179,6 +179,7 @@
 !else
   OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf
 !endif
+  RngLib|MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.inf
 
   
AuthVariableLib|MdeModulePkg/Library/AuthVariableLibNull/AuthVariableLibNull.inf
   VarCheckLib|MdeModulePkg/Library/VarCheckLib/VarCheckLib.inf
-- 
2.28.0.windows.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#64468): https://edk2.groups.io/g/devel/message/64468
Mute This Topic: https://groups.io/mt/76294215/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v8 0/5] Use RngLib instead of TimerLib for OpensslLib

[Matthew Carlson]

From: Matthew Carlson 

Hello all,

This patch contains a fix for Bugzilla 1871.
There's been a good bit of community discussion around the topic,
so below follows a general overview of the discussion and what this patch does.

This is the seventh iteration of this patch series, focused on code style and a
few functions being renamed to comply with style.

Back in Devel message#40590 (https://edk2.groups.io/g/devel/message/40590)
around the patch series that updates OpenSSL to 1.1.1b, a comment was made
that suggested that platforms be in charge of the entropy/randomness that
is provided to OpenSSL as currently the entropry source seems to be a
hand-rolled random number generator that uses the PerformanceCounter from
TimerLib. This causes OpenSSL to depend on TimerLib, which is often platform
specific. In addition to being a potentially weaker source of randomness,
this also poses a challenge to compile BaseCryptLibOnProtocol with a platform-
agnostic version of TimerLib that works universally.

The solution here is to allow platform to specify their source of entropy in
addition to providing two new RngLibs: one that uses the TimerLib as well as
one that uses RngProtocol to provide randomness. Then the decision to use
RDRAND or other entropy sources is up to the platform. Mixing various entropy
sources is the onus of the platform. It has been suggested on Devel#40590 and
BZ#1871 that there should be mixing of the PerformanceCounter and RDRAND using
something similar to the yarrow alogirthm that FreeBSD uses for example. This
patch series doesn't offer an RngLib that offers that sort of mixing as the
ultimate source of random is defined by the platform.

This patch series offers three benefits:
1. Dependency reduction: Removes the need for a platform specific timer
library.  We publish a single binary used on numerous platforms for
crypto and the introduced timer lib dependency caused issues because we
could not fulfill our platform needs with one library instance.

2. Code maintenance: Removing this additional code and leveraging an existing
library within Edk2 means less code to maintain.

3. Platform defined quality: A platform can choose which instance to use and
the implications of that instance.

This patch series seeks to address five seperate issues.
  1) Use RngLib interface to generate random entropy in rand_pool
  2) Remove dependency on TimerLib in OpensslLib
  3) Add a new version of RngLib implemented by TimerLib
  4) Add a new version of RngLib implemented by EFI_RNG_PROTOCOL
  5) Add RngLib to platforms in EDK2 such as ArmVirtPkg and OvmfPkg

Since this changes the dependencies of OpenSSL, this has the potential of being
a breaking change for platforms in edk2-platforms. The easiest solution is just
to use the RngLib that uses the TimerLib as this closely mimics the behavior of
OpenSSL prior to this patch series. There is also a null version of RngLib for
CI environments that need this change
(https://edk2.groups.io/g/devel/message/50432). Though it should be pointed out
that in CI environments, the null version of BaseCryptLib or OpenSSL should be
used.

In addition, it has been suggested that
1) Add AsmRdSeed to BaseLib.
2) Update BaseRngLib to use AsmRdSeed() for the random number,
if RdSeed is supported (CPUID BIT18)

However, this is largely out of scope for this particular patch series and
will likely need to be in a follow-up series later.

It is my understanding that the OpenSSL code uses the values provided as a
randomness pool rather than a seed or random numbers itself, so the
requirements for randomness are not quite as stringent as other applications.

For the ArmVirtPkg and OvmfPkg platforms, the patch series here just adds in
the TimerLib based RngLib as that is similar to the functionality of before.
It is added as a common library so any custom RngLib defined in the DSC
should take precedence over the TimerLibRngLib.

Ref: https://github.com/tianocore/edk2/pull/845
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1871

Cc: Ard Biesheuvel 
Cc: Anthony Perard 
Cc: Jiewen Yao 
Cc: Jian J Wang 
Cc: Julien Grall 
Cc: Jordan Justen 
Cc: Laszlo Ersek 
Cc: Liming Gao 
Cc: Leif Lindholm 
Cc: Michael D Kinney 
Cc: Xiaoyu Lu 
Cc: Zhiguang Liu 
Cc: Sean Brogan 

Signed-off-by: Matthew Carlson 

Matthew Carlson (5):
  MdePkg: TimerRngLib: Added RngLib that uses TimerLib
  MdePkg: BaseRngLibDxe: Add RngLib that uses RngDxe
  OvmfPkg: Add RngLib based on TimerLib for Crypto
  ArmVirtPkg: Add RngLib based on TimerLib for CryptoPkg
  CryptoPkg: OpensslLib: Use RngLib to generate entropy in rand_pool

 CryptoPkg/Library/OpensslLib/rand_pool.c | 265 
+---
 CryptoPkg/Library/OpensslLib/rand_pool_noise.c   |  29 ---
 CryptoPkg/Library/OpensslLib/rand_pool_noise_tsc.c   |  43 
 MdePkg/Library/BaseRngLibTimerLib/RngLibTimer.c  | 191 ++
 MdePkg/Library/DxeRngLib/DxeRngLib.c | 206 +++
 

Re: [edk2-devel] [PATCH v1] NetworkPkg/UefiPxeBcDxe: Fix PXE_BOOT_SERVERS usage in boot info parse flow

[Laszlo Ersek]

On 08/19/20 20:13, Laszlo Ersek wrote:
> On 08/19/20 18:53, Maciej Rabeda wrote:
>> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2876
>>
>> According to PXE 2.1 spec, DHCP option 43, tag 6 (PXE_DISCOVERY_CONTROL),
>> bit 3 specifies whether PXE client should use/accept TFTP servers defined
>> within PXE_BOOT_SERVERS list (tag 8). This bit was not being taken into
>> account when choosing boot server IP in PxeBcDhcp4BootInfo().
>>
>> Cc: Jiaxin Wu 
>> Cc: Siyuan Fu 
>> Signed-off-by: Maciej Rabeda 
>> ---
>>  NetworkPkg/UefiPxeBcDxe/PxeBcBoot.c | 10 +++---
>>  1 file changed, 7 insertions(+), 3 deletions(-)
>>
>> diff --git a/NetworkPkg/UefiPxeBcDxe/PxeBcBoot.c 
>> b/NetworkPkg/UefiPxeBcDxe/PxeBcBoot.c
>> index d062a526077b..ed9bca0f7de3 100644
>> --- a/NetworkPkg/UefiPxeBcDxe/PxeBcBoot.c
>> +++ b/NetworkPkg/UefiPxeBcDxe/PxeBcBoot.c
>> @@ -499,12 +499,16 @@ PxeBcDhcp4BootInfo (
>>  
>>//
>>// Parse the boot server address.
>> -  // If prompt/discover is disabled, get the first boot server from the 
>> boot servers list.
>> -  // Otherwise, parse the boot server Ipv4 address from next server address 
>> field in DHCP header.
>> +  // If prompt/discover is disabled, server list should be used and is 
>> present (DHCP option 43),
>> +  // get the first boot server from the boot servers list.
>> +  // Otherwise, parse the boot server IPv4 address from next server address 
>> field in DHCP header.
>>// If all these fields are not available, use option 54 instead.
>>//
>>VendorOpt = >VendorOpt;
>> -  if (IS_DISABLE_PROMPT_MENU (VendorOpt->DiscoverCtrl) && 
>> IS_VALID_BOOT_SERVERS (VendorOpt->BitMap)) {
>> +  if (IS_DISABLE_PROMPT_MENU (VendorOpt->DiscoverCtrl) &&
>> +  IS_VALID_BOOT_SERVERS (VendorOpt->BitMap) &&
>> +  IS_ENABLE_USE_SERVER_LIST (VendorOpt->DiscoverCtrl))
>> +  {
>>  Entry = VendorOpt->BootSvr;
>>  if (VendorOpt->BootSvrLen >= sizeof (PXEBC_BOOT_SVR_ENTRY) && 
>> Entry->IpCnt > 0) {
>>CopyMem (
>>
> 
> I'm still undecided whether option#43 / tag#6 / bit#3 being clear means
> we should *ignore* PXE_BOOT_SERVERS (tag#8), but I'm willing to defer to
> you on that. So, I can give a cautious
> 
> Reviewed-by: Laszlo Ersek 
> 
> for this patch.
> 
> Regarding the feature freeze -- in theory, this is a bugfix. However,
> before we merge it, it would be really nice to get feedback from the
> original reporter (CC'd).
> 
> I also intend to regression-test this patch, I'll report back.

I've repeated my usual PXEv4 boot tests (using OVMF IA32X64 and
ArmVirtQemu AARCH64 guests, covering shim + grub + vmlinuz + initrd);
nothing seems to have regressed.

Regression-tested-by: Laszlo Ersek 

Thanks
Laszlo


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#64464): https://edk2.groups.io/g/devel/message/64464
Mute This Topic: https://groups.io/mt/76290910/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] EDK2 Specs PDF links don't work.

[Tim Lewis]

Sorry, I’m new to GitBook, but the PDF links for the specs are not working. It 
just brings me into the middle of the GitBook HTML pages. I like to have an 
off-line reference and PDFs worked well in the past. I see the link points to a 
PDF URL, but the result is still not PDF. I’d prefer not to have to become

 

Any help is appreciated.

 

Thanks,

 

Tim


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#64463): https://edk2.groups.io/g/devel/message/64463
Mute This Topic: https://groups.io/mt/76293464/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v1] NetworkPkg/UefiPxeBcDxe: Fix PXE_BOOT_SERVERS usage in boot info parse flow

[Michael Brown]

On 19/08/2020 19:13, Laszlo Ersek wrote:

I'm still undecided whether option#43 / tag#6 / bit#3 being clear means
we should *ignore* PXE_BOOT_SERVERS (tag#8), but I'm willing to defer to
you on that. So, I can give a cautious

Reviewed-by: Laszlo Ersek 

for this patch.


FWIW, iPXE's equivalent logic (based on a combination of what the PXE 
spec says and what the Intel reference PXE implementation actually does, 
which is not necessarily the same thing) is to *ignore* PXE_BOOT_SERVERS 
if a DHCP filename is available and option 43 tag 6 bit 3 is *set*.


Michael

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#64462): https://edk2.groups.io/g/devel/message/64462
Mute This Topic: https://groups.io/mt/76290910/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v1] NetworkPkg/UefiPxeBcDxe: Fix PXE_BOOT_SERVERS usage in boot info parse flow

[Laszlo Ersek]

On 08/19/20 18:53, Maciej Rabeda wrote:
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2876
> 
> According to PXE 2.1 spec, DHCP option 43, tag 6 (PXE_DISCOVERY_CONTROL),
> bit 3 specifies whether PXE client should use/accept TFTP servers defined
> within PXE_BOOT_SERVERS list (tag 8). This bit was not being taken into
> account when choosing boot server IP in PxeBcDhcp4BootInfo().
> 
> Cc: Jiaxin Wu 
> Cc: Siyuan Fu 
> Signed-off-by: Maciej Rabeda 
> ---
>  NetworkPkg/UefiPxeBcDxe/PxeBcBoot.c | 10 +++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/NetworkPkg/UefiPxeBcDxe/PxeBcBoot.c 
> b/NetworkPkg/UefiPxeBcDxe/PxeBcBoot.c
> index d062a526077b..ed9bca0f7de3 100644
> --- a/NetworkPkg/UefiPxeBcDxe/PxeBcBoot.c
> +++ b/NetworkPkg/UefiPxeBcDxe/PxeBcBoot.c
> @@ -499,12 +499,16 @@ PxeBcDhcp4BootInfo (
>  
>//
>// Parse the boot server address.
> -  // If prompt/discover is disabled, get the first boot server from the boot 
> servers list.
> -  // Otherwise, parse the boot server Ipv4 address from next server address 
> field in DHCP header.
> +  // If prompt/discover is disabled, server list should be used and is 
> present (DHCP option 43),
> +  // get the first boot server from the boot servers list.
> +  // Otherwise, parse the boot server IPv4 address from next server address 
> field in DHCP header.
>// If all these fields are not available, use option 54 instead.
>//
>VendorOpt = >VendorOpt;
> -  if (IS_DISABLE_PROMPT_MENU (VendorOpt->DiscoverCtrl) && 
> IS_VALID_BOOT_SERVERS (VendorOpt->BitMap)) {
> +  if (IS_DISABLE_PROMPT_MENU (VendorOpt->DiscoverCtrl) &&
> +  IS_VALID_BOOT_SERVERS (VendorOpt->BitMap) &&
> +  IS_ENABLE_USE_SERVER_LIST (VendorOpt->DiscoverCtrl))
> +  {
>  Entry = VendorOpt->BootSvr;
>  if (VendorOpt->BootSvrLen >= sizeof (PXEBC_BOOT_SVR_ENTRY) && 
> Entry->IpCnt > 0) {
>CopyMem (
> 

I'm still undecided whether option#43 / tag#6 / bit#3 being clear means
we should *ignore* PXE_BOOT_SERVERS (tag#8), but I'm willing to defer to
you on that. So, I can give a cautious

Reviewed-by: Laszlo Ersek 

for this patch.

Regarding the feature freeze -- in theory, this is a bugfix. However,
before we merge it, it would be really nice to get feedback from the
original reporter (CC'd).

I also intend to regression-test this patch, I'll report back.

Thanks,
Laszlo


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#64461): https://edk2.groups.io/g/devel/message/64461
Mute This Topic: https://groups.io/mt/76290910/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v5 1/1] ShellPkg/DynamicCommand: add HttpDynamicCommand

[Vladimir Olovyannikov via groups.io]

> -Original Message-
> From: Rabeda, Maciej 
> Sent: Wednesday, August 19, 2020 10:44 AM
> To: Laszlo Ersek ; Vladimir Olovyannikov
> ; Gao, Zhichao
> ; devel@edk2.groups.io
> Cc: Samer El-Haj-Mahmoud ; Wu, Jiaxin
> ; Fu, Siyuan ; Ni, Ray
> ; Gao, Liming ; Nd
> 
> Subject: Re: [PATCH v5 1/1] ShellPkg/DynamicCommand: add
> HttpDynamicCommand
>
> @Laszlo,
>
> As for type casting, I don't have strong opinions.
> Space after typecast (for me) always seemed to be visually more consistent
> with the rest of the code.
> In my regular projects written in C-based languages, I would do it the way
> you are describing.
>
> @Vladimir
> I went through the rest of the code.
> There are more coding standard problems, but in terms of HTTP usage the
> code seems to be OK.
OK, great, thanks. I am reviewing it and will submit the patch shortly.
Thanks to Laszlo, I hope to be able to run CI locally and detect hidden
issues.
Indeed, I have to switch between Linux and UEFI code, so I missed several
code style issues.
>
> Thanks,
> Maciej
>
> PS. Rabeda is my last name :)
Oops, thanks for letting me know, Maciej.
It is just a bit unusual to have the last name first :)

Thank you,
Vladimir
>
> On 19-Aug-20 11:47, Laszlo Ersek wrote:
> > On 08/18/20 20:33, Vladimir Olovyannikov wrote:
> >>> -Original Message-
> >>> From: Rabeda, Maciej 
> >>> Sent: Tuesday, August 18, 2020 9:54 AM
> >>> To: Vladimir Olovyannikov ;
> >>> Laszlo Ersek ; Gao, Zhichao
> >>> ; devel@edk2.groups.io
> >>> Cc: Samer El-Haj-Mahmoud ; Wu,
> Jiaxin
> >>> ; Fu, Siyuan ; Ni, Ray
> >>> ; Gao, Liming ; Nd
> >>> 
> >>> Subject: Re: [PATCH v5 1/1] ShellPkg/DynamicCommand: add
> >>> HttpDynamicCommand
> >>>
> >>> Hi Vladimir,
> >>>
> >>> I am inprog of going through the patch. There are some coding
> >>> standard violations.
> >>> For reference:
> >>> https://edk2-docs.gitbook.io/edk-ii-c-coding-standards-specification
> >>> /
> > [...]
> >
> >> OK, I will look through all files again, probably something slipped
> >> from my attention as I work with Linux code as well.
> > Now that we have ECC checking enabled in CI, submitting a personal
> > pull request could help. Additionally, the CI workload should be
> > possible to run locally, according to ".pytool/Readme.md".
> >
> > (I'm planning to learn how to run that locally myself.)
> >
> >>> HttpApp.c:
> >>> Line 48: Space after type cast.
> >> Sorry, I don't understand here. You mean, Something =
> >> (CAST)SomethingElse should actually be "Something = (CAST)
> SomethingElse?
> >> I don't see this say in the Tftp.c:
> >> if (AsciiStrnCmp ((CHAR8 *)Option->OptionStr, "tsize", 5) == 0) ...
> >> or in TftpApp.c
> >> Status = (EFI_STATUS)RunTftp (ImageHandle, SystemTable); Am I missing
> >> anything?
> >
> > In core edk2 packages, casts are frequently written like this:
> >
> >(TYPE) Expression
> >
> > (Importantly, Expression itself is not parenthesized.)
> >
> > In my opinion, this is a bad practice. That's because the typecast has
> > one of the strongest operator bindings in the C language, and visually
> > distancing (TYPE) from "Expression" suggests the opposite -- it's
> > counter-intuitive.
> >
> > I strongly prefer
> >
> >(TYPE)Expression
> >
> > but other maintainers may have a different preference.
> >
> > Thanks
> > Laszlo
> >

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#64459): https://edk2.groups.io/g/devel/message/64459
Mute This Topic: https://groups.io/mt/75826968/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v2 2/2] CryptoPkg/OpensslLib: Commit the auto-generated assembly files for X64

[Laszlo Ersek]

On 08/19/20 17:37, Andrew Fish via groups.io wrote:
> 
> 
>> On Aug 18, 2020, at 2:22 PM, Zurcher, Christopher J 
>>  wrote:
>>
>> Per the added header comment in process_files.pl:
>>
>> # Due to the script wrapping required to process the OpenSSL
>> # configuration data, each native architecture must be processed
>> # individually by the maintainer (in addition to the standard version):
>> #   ./process_files.pl
> 
> Side note: process_file.pl is missing a copyright statement. 

True :(

Andrew, could you please report a BZ about it?

Thanks!
Laszlo


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#64460): https://edk2.groups.io/g/devel/message/64460
Mute This Topic: https://groups.io/mt/75978613/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] running CI locally

[Laszlo Ersek]

On 08/19/20 18:22, Sean Brogan wrote:
> Laszlo/Mike,
> 
> This is the joy of distributed repositories.
> Remember edk2 ci is actually using edk2-pytool-extensions and
> edk2-pytool-library.  Documentation is in those projects.
> 
> https://github.com/tianocore/edk2-pytool-extensions/tree/master/docs
> https://github.com/tianocore/edk2-pytool-library/tree/master/docs
> 
> I won't say they are great and I hope someday relatively soon we can
> talk about an edk2 static site generator that can include docs from
> multiple repositories (much like https://microsoft.github.io/mu/) as I
> think documentation on edk2 is a weak spot.
> 
> Regarding Mono and nuget.  It is one of those things we wish was
> different as we have found it very inconsistent across different
> distributions of linux.  But we do have docs here.
> https://github.com/tianocore/edk2-pytool-extensions/blob/master/docs/usability/using_linux.md
> 
> 
> And more specifically here:
> https://github.com/tianocore/edk2-pytool-extensions/blob/master/docs/usability/using_extdep.md#a-note-on-nuget-on-linux
> 
> 
> 
> Finally getting to why you don't run the compile.
> Core CI (stuart_ci_build)  is a plugin runner.  Compile test is just one
> of those tests.  OvmfPkg is a platform and thus didn't opt into core ci
> compile testing.  In the table here i tried to document that OvmfPkg
> didn't compile using core ci and that a user should look at the readme.
> https://github.com/tianocore/edk2/blob/master/.pytool/Readme.md#basic-status
> 
> 
> So for OvmfPkg we enabled what we call platform ci (stuart_build).  I
> think the write up here is pretty complete (although i see it has no
> mention of mono either).
> https://github.com/tianocore/edk2/tree/master/OvmfPkg/PlatformCI
> 
> One final note.  Yes the logging to console is by default very brief.
> This is by design as it is easier to quickly look and see what test
> failed and then use the log file (as mike mentioned) to find the root
> cause.
> 
> Hope that helps and it is great to see people using it.  Feedback is
> much appreciated.

The documentation looks *awesome*, and it's entirely my fault that I
couldn't find it. I apologize.

I'll report back with more results.

Thanks!
Laszlo


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#64458): https://edk2.groups.io/g/devel/message/64458
Mute This Topic: https://groups.io/mt/76285051/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] running CI locally

[Laszlo Ersek]

Hi Mike,

On 08/19/20 17:29, Kinney, Michael D wrote:
> Hi Laszlo,
> 
> Thank you for the feedback.  I agree there are some documentation updates 
> required.
> 
> The spell check requirements are documented here:
> 
> https://github.com/tianocore/edk2/blob/master/.pytool/Readme.md#spell-checking---cspell
> 
> You do need to install nodejs and cspell.
> 
> * Install nodejs from https://nodejs.org/en/
> * Install cspell
>   1. Open cmd prompt with access to node and npm
>   2. Run `npm install -g cspell`

npm is a security hole one can drive a truck through :)

Additionally, I just realized that "nuget" downloads native iasl and
nasm binaries ("BaseTools/Bin/iasl_extdep/Linux-x86/iasl",
"BaseTools/Bin/mu_nasm_extdep/Linux-x86-64/nasm").

This basically requires me to trust two package repositories (npmjs.org,
nuget.org) that, well, I don't trust.

This is *not* criticism of the CI system; it only means that running it
locally on my laptop needs more work from me. In particular, it requires
spinning up a VM that I don't use for anything else.

Given the typing / clicking / scripting necessary for this, and the
network-originated updates for the "in VM" CI system, I wonder if
running CI locally actually saves me any time.

One thing it would certainly give me is "confidentiality", as I wouldn't
have to push my branch to github (for a personal CI build PR) before I
were ready to submit the set for real.

I think I'll set up a new libvirt/QEMU/KVM domain for this, and report
back with more results once I have them.

> I agree that the verbosity of the output is very brief.  There is a more 
> complete
> log that is always produced in the build output directory in 
> Build/CI_BUILDLOG.txt.
> Can you look at that file and see if it provide a more detailed reason for the
> failure?

Yes, I'll check that.

Thanks!
Laszlo


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#64457): https://edk2.groups.io/g/devel/message/64457
Mute This Topic: https://groups.io/mt/76285051/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v5 1/1] ShellPkg/DynamicCommand: add HttpDynamicCommand

[Maciej Rabeda]

@Laszlo,

As for type casting, I don't have strong opinions.
Space after typecast (for me) always seemed to be visually more 
consistent with the rest of the code.
In my regular projects written in C-based languages, I would do it the 
way you are describing.


@Vladimir
I went through the rest of the code.
There are more coding standard problems, but in terms of HTTP usage the 
code seems to be OK.


Thanks,
Maciej

PS. Rabeda is my last name :)

On 19-Aug-20 11:47, Laszlo Ersek wrote:

On 08/18/20 20:33, Vladimir Olovyannikov wrote:

-Original Message-
From: Rabeda, Maciej 
Sent: Tuesday, August 18, 2020 9:54 AM
To: Vladimir Olovyannikov ; Laszlo
Ersek ; Gao, Zhichao ;
devel@edk2.groups.io
Cc: Samer El-Haj-Mahmoud ; Wu, Jiaxin
; Fu, Siyuan ; Ni, Ray
; Gao, Liming ; Nd

Subject: Re: [PATCH v5 1/1] ShellPkg/DynamicCommand: add
HttpDynamicCommand

Hi Vladimir,

I am inprog of going through the patch. There are some coding standard
violations.
For reference:
https://edk2-docs.gitbook.io/edk-ii-c-coding-standards-specification/

[...]


OK, I will look through all files again, probably something slipped from my
attention as I work with Linux code as well.

Now that we have ECC checking enabled in CI, submitting a personal pull
request could help. Additionally, the CI workload should be possible to
run locally, according to ".pytool/Readme.md".

(I'm planning to learn how to run that locally myself.)


HttpApp.c:
Line 48: Space after type cast.

Sorry, I don't understand here. You mean, Something = (CAST)SomethingElse
should actually be "Something = (CAST) SomethingElse?
I don't see this say in the Tftp.c:
if (AsciiStrnCmp ((CHAR8 *)Option->OptionStr, "tsize", 5) == 0) ...
or in TftpApp.c
Status = (EFI_STATUS)RunTftp (ImageHandle, SystemTable);
Am I missing anything?


In core edk2 packages, casts are frequently written like this:

   (TYPE) Expression

(Importantly, Expression itself is not parenthesized.)

In my opinion, this is a bad practice. That's because the typecast has
one of the strongest operator bindings in the C language, and visually
distancing (TYPE) from "Expression" suggests the opposite -- it's
counter-intuitive.

I strongly prefer

   (TYPE)Expression

but other maintainers may have a different preference.

Thanks
Laszlo




-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#64456): https://edk2.groups.io/g/devel/message/64456
Mute This Topic: https://groups.io/mt/75826968/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v2 2/2] CryptoPkg/OpensslLib: Commit the auto-generated assembly files for X64

[Laszlo Ersek]

On 08/19/20 18:08, Kinney, Michael D wrote:
> Laszlo,
> 
> The current CryptoPkg DCS file with use of the CRYPTO_SERVICES define is 
> cumbersome.
> 
>   #
>   # Flavor of PEI, DXE, SMM modules to build.
>   # Must be one of ALL, NONE, MIN_PEI, MIN_DXE_MIN_SMM.
>   # Default is ALL that is used for package build verification.
>   #   PACKAGE - Package verification build of all components.  Null
>   # versions of libraries are used to minimize build 
> times.
>   #   ALL - Build PEIM, DXE, and SMM drivers.  Protocols and PPIs
>   # publish all services.
>   #   NONE- Build PEIM, DXE, and SMM drivers.  Protocols and PPIs
>   # publish no services.  Used to verify compiler/linker
>   # optimizations are working correctly.
>   #   MIN_PEI - Build PEIM with PPI that publishes minimum required
>   # services.
>   #   MIN_DXE_MIN_SMM - Build DXE and SMM drivers with Protocols that publish
>   # minimum required services.
>   #
>   DEFINE CRYPTO_SERVICES = PACKAGE
> 
> There is a known limitation for using structured PCDs in a module scope and 
> that limitation is what resulted in the use of this define.  Bob Feng
> has provided a BaseTools patch that attempts to address this limitation.
> 
> https://edk2.groups.io/g/devel/message/63906
> 
> This patch is functional, but has one open issue around the PCD report.  Once
> this issue is resolved we will be able to specify structured PCD field values
> in the scope of a single module.  I have a branch that simplifies the DSC and
> allows all flavors of the crypto modules to be built in a single invocation
> of the build command.  There is more cleanup of the DSC possible, but I
> wanted to share a quick test case for Bob's patch.
> 
> 
> https://github.com/mdkinney/edk2/tree/Bug_xxx_CryptoPkg_UseModuleScopedPcds
> 
> This feature supports both the generation of standard flavors of the crypto
> modules that a platform could consume as a pre-built binary and also allows
> a platform to choose their own profile by specifying the specific crypto APIs
> needed in PEI, DXE, SMM when building crypto modules from sources.

Yes, this resolves my first problem entirely!

Thanks!
Laszlo

>   
> Best regards,
> 
> Mike
> 
>> -Original Message-
>> From: devel@edk2.groups.io  On Behalf Of Laszlo Ersek
>> Sent: Wednesday, August 19, 2020 3:44 AM
>> To: devel@edk2.groups.io; spbro...@outlook.com; Kinney, Michael D 
>> ; Wang, Jian J
>> ; Zurcher, Christopher J 
>> ; Yao, Jiewen 
>> Cc: Lu, XiaoyuX ; Ard Biesheuvel 
>> 
>> Subject: Re: [edk2-devel] [PATCH v2 2/2] CryptoPkg/OpensslLib: Commit the 
>> auto-generated assembly files for X64
>>
>> Hi,
>>
>> On 08/18/20 23:33, Sean wrote:
>>> Mike,
>>>
>>> I am not technically a basetool maintainer but as an active user/dev in
>>> basetools, i would be opposed to bringing in perl as an edk2 dependency.
>>> Also introducing another language is counter to the goal of aligning on
>>> python and improving the python used within edk2.  From my perspective
>>> the openssl config case isn't strong enough to counter the above goal.
>>> In fact as you know we are trying to change the paradigm for
>>> Crypto/OpenSSL with the Crypto Driver
>>> (https://github.com/tianocore/edk2/tree/master/CryptoPkg/Driver) and
>>> BaseCryptLibOnProtocolPpi
>>> (https://github.com/tianocore/edk2/tree/master/CryptoPkg/Library/BaseCryptLibOnProtocolPpi)
>>> work so that everyday development doesn't need to compile openssl in
>>> their edk2 builds.
>>
>> Here I'd only like to comment on this one aspect (= build OpenSSL as a
>> statically linked library vs. as a crypto service driver / PEIM).
>>
>> Recently I tried to evaluate the crypto driver for OVMF. I started with
>> the PEI phase. The configuration interface (the PCD) is baroque, *BUT*
>> that is a direct consequence of OpenSSL offering a huge range of
>> interfaces. So no complaints about the config interface.
>>
>> I also reviewed the CryptoPkg.dsc "pre-sets" (i.e., CRYPTO_SERVICES
>> being one of PACKAGE / ALL / NONE / MIN_PEI / MIN_DXE_MIN_SMM). I had
>> two problems:
>>
>> - These pre-sets are supremely suitable for a platform that is composed
>> of multiple build runs; that is, build the crypto PEIM, build the DXE /
>> SMM protocol drivers, package up the resultant binaries, and *then*
>> build the actual platforms (which will then include the crypto service
>> drivers in *binary* form). On the other hand, the pre-sets are not
>> useful to a platform that is supposed to be built in a single-shot.
>> Importantly, I'm not saying that the pre-sets are *detrimental* to such
>> platforms -- they aren't. It's just that the pre-sets target a different
>> use case.
>>
>> - The other problem I had was the one that we had discussed when the
>> crypto service driver was being introduced. Namely, selecting the
>> OpenSSL interfaces (interface 

Re: [edk2-devel] [edk2-announce] Re: Soft Feature Freeze starts now for edk2-stable202008

[Laszlo Ersek]

On 08/19/20 18:20, Gao, Liming wrote:
> 
>> -Original Message-
>> From: Chang, Abner (HPS SW/FW Technologist) 
>> Sent: Wednesday, August 19, 2020 10:59 PM
>> To: Laszlo Ersek ; Leif Lindholm 
>> Cc: devel@edk2.groups.io; Gao, Liming ; 
>> annou...@edk2.groups.io; af...@apple.com; Kinney, Michael D
>> 
>> Subject: RE: [edk2-announce] Re: Soft Feature Freeze starts now for 
>> edk2-stable202008
>>
>>
>>
>>> -Original Message-
>>> From: Laszlo Ersek [mailto:ler...@redhat.com]
>>> Sent: Wednesday, August 19, 2020 10:30 PM
>>> To: Chang, Abner (HPS SW/FW Technologist) ; Leif
>>> Lindholm 
>>> Cc: devel@edk2.groups.io; liming.gao ;
>>> annou...@edk2.groups.io; af...@apple.com; Kinney, Michael D
>>> 
>>> Subject: Re: [edk2-announce] Re: Soft Feature Freeze starts now for edk2-
>>> stable202008
>>>
>>> On 08/19/20 15:34, Chang, Abner (HPS SW/FW Technologist) wrote:


> -Original Message-
> From: Laszlo Ersek [mailto:ler...@redhat.com]
> Sent: Wednesday, August 19, 2020 9:19 PM
> To: Leif Lindholm ; Chang, Abner (HPS SW/FW
> Technologist) 
> Cc: devel@edk2.groups.io; liming.gao ;
> annou...@edk2.groups.io; af...@apple.com; Kinney, Michael D
> 
> Subject: Re: [edk2-announce] Re: Soft Feature Freeze starts now for
> edk2-
> stable202008
>
> On 08/19/20 13:48, Leif Lindholm wrote:
>> (Slightly trimmed recipient list due to different patch being
>> discussed.)
>>
>> So, I can't make this call, because I'm the one who messed up.
>>
>> This patch does exactly what I had requested Abner to do some time
>> back (off-list, unfortunately), and I was *convinced* I gave it an
>> R-b as soon as it hit my inbox - until Abner nudged me about it 
>> yesterday.
>>
>> The patch in question is
>> https://edk2.groups.io/g/devel/topic/76021725
>
> My understanding is:
>
> (1) there is an external project that consumes the FDT library in
> EmbeddedPkg, meaning the lib class header
> "EmbeddedPkg/Include/libfdt.h"
> and the lib instance "EmbeddedPkg/Library/FdtLib/FdtLib.inf",
 [Chang, Abner] Yes
>
> (2) the lib class header pulls in "fdt.h" and "libfdt_env.h",
 [Chang, Abner] yes
>
> (3) the external project is not edk2-platforms,
 [Chang, Abner] yes
>
> (4) the external project wants -- for some strange reason -- edk2's
> "libfdt_env.h" to provide an strncmp() function (or function-like
> macro), with that particular stncmp() implementation not being needed
> in either edk2- platforms or edk2 itself,
 [Chang, Abner] yes, at least so far
>
> (5) the patch for adding said strncmp() was posted on Aug 6th (at
> least when viewed from my time zone), i.e., before the SFF,
 [Chang, Abner] Yes
>
> (6) it was reviewed 12 days later (within the SFF)
 [Chang, Abner] yes.
>
> If my understanding is correct, then I don't see how this patch could
> be considered a bugfix -- even as a feature addition, it seems hardly
> justified to me --, and there would have been ~8 days before the SFF to
>>> review it.
>
> I think we should postpone the patch until after the stable tag.
 This patch is important because the edk2-stable202008 would be the stable
>>> tag (if this patch is accepted) for booting RISC-V platform to Linux kernel 
>>> with
>>> EFI Runtime service on either real platform and QEMU. We can publish this
>>> information in RISC-V community which is considered as a valuable milestone
>>> for RISC-V edk2 port.
>>>
>>> Let's move out the dates for the stable tag then, by one week:
>>>
>>> - let the SFF start on 2020-08-21
>>> - let the HFF start on 2020-08-28
>>> - let's release edk2-stable202008 on 2020-09-04
>>>
>>> Release slips are permitted and there have been examples.
>> Appreciate for this.
> 
> Previous stable tags (202005/202002/201905) extended SFF or HFF period to let 
> the critical bug fix catch the stable tag. 
> Today case is like a new feature. According to Abner comment, it is important 
> to RISC-V community.
> If SFF start time is changed, more features may catch this stable, such as 
> VariableLock feature.

Yes, exactly.

We must apply the same deadline to all features. If one feature is
important enough for delaying the SFF, then patch review for other
pending features is resumed as well.

>>
>>>
>>> What doesn't make sense is making rules and then breaking them
>>> opportunistically, whenever they're uncomfortable. If that's a frequent
>>> occurrence, we should pick different rules, or -- again -- if this is a very
>>> important patch, we should delay the release for it.
> 
> I suggest to define the process on delay the release. The initial process is 
> specified here.
> The requestor provides the justification why his patch needs to catch the 
> stable tag after SFF phase. 
> All stewards make the decision whether delay the stable tag to include his 
> 

[edk2-devel] [PATCH v1] NetworkPkg/UefiPxeBcDxe: Fix PXE_BOOT_SERVERS usage in boot info parse flow

[Maciej Rabeda]

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2876

According to PXE 2.1 spec, DHCP option 43, tag 6 (PXE_DISCOVERY_CONTROL),
bit 3 specifies whether PXE client should use/accept TFTP servers defined
within PXE_BOOT_SERVERS list (tag 8). This bit was not being taken into
account when choosing boot server IP in PxeBcDhcp4BootInfo().

Cc: Jiaxin Wu 
Cc: Siyuan Fu 
Signed-off-by: Maciej Rabeda 
---
 NetworkPkg/UefiPxeBcDxe/PxeBcBoot.c | 10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/NetworkPkg/UefiPxeBcDxe/PxeBcBoot.c 
b/NetworkPkg/UefiPxeBcDxe/PxeBcBoot.c
index d062a526077b..ed9bca0f7de3 100644
--- a/NetworkPkg/UefiPxeBcDxe/PxeBcBoot.c
+++ b/NetworkPkg/UefiPxeBcDxe/PxeBcBoot.c
@@ -499,12 +499,16 @@ PxeBcDhcp4BootInfo (
 
   //
   // Parse the boot server address.
-  // If prompt/discover is disabled, get the first boot server from the boot 
servers list.
-  // Otherwise, parse the boot server Ipv4 address from next server address 
field in DHCP header.
+  // If prompt/discover is disabled, server list should be used and is present 
(DHCP option 43),
+  // get the first boot server from the boot servers list.
+  // Otherwise, parse the boot server IPv4 address from next server address 
field in DHCP header.
   // If all these fields are not available, use option 54 instead.
   //
   VendorOpt = >VendorOpt;
-  if (IS_DISABLE_PROMPT_MENU (VendorOpt->DiscoverCtrl) && 
IS_VALID_BOOT_SERVERS (VendorOpt->BitMap)) {
+  if (IS_DISABLE_PROMPT_MENU (VendorOpt->DiscoverCtrl) &&
+  IS_VALID_BOOT_SERVERS (VendorOpt->BitMap) &&
+  IS_ENABLE_USE_SERVER_LIST (VendorOpt->DiscoverCtrl))
+  {
 Entry = VendorOpt->BootSvr;
 if (VendorOpt->BootSvrLen >= sizeof (PXEBC_BOOT_SVR_ENTRY) && Entry->IpCnt 
> 0) {
   CopyMem (
-- 
2.24.0.windows.2


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#64453): https://edk2.groups.io/g/devel/message/64453
Mute This Topic: https://groups.io/mt/76290910/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] running CI locally

[Sean]

Laszlo/Mike,

This is the joy of distributed repositories.
Remember edk2 ci is actually using edk2-pytool-extensions and 
edk2-pytool-library.  Documentation is in those projects.


https://github.com/tianocore/edk2-pytool-extensions/tree/master/docs
https://github.com/tianocore/edk2-pytool-library/tree/master/docs

I won't say they are great and I hope someday relatively soon we can 
talk about an edk2 static site generator that can include docs from 
multiple repositories (much like https://microsoft.github.io/mu/) as I 
think documentation on edk2 is a weak spot.


Regarding Mono and nuget.  It is one of those things we wish was 
different as we have found it very inconsistent across different 
distributions of linux.  But we do have docs here.

https://github.com/tianocore/edk2-pytool-extensions/blob/master/docs/usability/using_linux.md

And more specifically here:
https://github.com/tianocore/edk2-pytool-extensions/blob/master/docs/usability/using_extdep.md#a-note-on-nuget-on-linux


Finally getting to why you don't run the compile.
Core CI (stuart_ci_build)  is a plugin runner.  Compile test is just one 
of those tests.  OvmfPkg is a platform and thus didn't opt into core ci 
compile testing.  In the table here i tried to document that OvmfPkg 
didn't compile using core ci and that a user should look at the readme.
https://github.com/tianocore/edk2/blob/master/.pytool/Readme.md#basic-status 



So for OvmfPkg we enabled what we call platform ci (stuart_build).  I 
think the write up here is pretty complete (although i see it has no 
mention of mono either). 
https://github.com/tianocore/edk2/tree/master/OvmfPkg/PlatformCI


One final note.  Yes the logging to console is by default very brief. 
This is by design as it is easier to quickly look and see what test 
failed and then use the log file (as mike mentioned) to find the root 
cause.


Hope that helps and it is great to see people using it.  Feedback is 
much appreciated.


Thanks
Sean

On 8/19/2020 8:29 AM, Michael D Kinney wrote:

Hi Laszlo,

Thank you for the feedback.  I agree there are some documentation updates 
required.

The spell check requirements are documented here:

https://github.com/tianocore/edk2/blob/master/.pytool/Readme.md#spell-checking---cspell

You do need to install nodejs and cspell.

* Install nodejs from https://nodejs.org/en/
* Install cspell
   1. Open cmd prompt with access to node and npm
   2. Run `npm install -g cspell`

I agree that the verbosity of the output is very brief.  There is a more 
complete
log that is always produced in the build output directory in 
Build/CI_BUILDLOG.txt.
Can you look at that file and see if it provide a more detailed reason for the
failure?

Thanks,

Mike



-Original Message-
From: devel@edk2.groups.io  On Behalf Of Laszlo Ersek
Sent: Wednesday, August 19, 2020 5:27 AM
To: edk2-devel-groups-io ; Sean Brogan 
; Bret Barkelew

Cc: Vladimir Olovyannikov ; Rebecca Cran 
; Tom Lendacky

Subject: [edk2-devel] running CI locally

Hi!

I'd like to test CI locally. I'm going through ".pytool/Readme.md" with
the tree checked out at 7e6f150b6902 (= current HEAD). I'm doing this in
a RHEL8 VM, with a python3 virtual environment set up / entered.


* My first note is that the command

   pip install --upgrade pip-requirements.txt

under "Prerequisets", has a small typo; it should be

   pip install --upgrade -r pip-requirements.txt

(the "-r" option is missing).

(

After adding "-r", the following components are now installed in my
virtual env:

- edk2-pytool-library: 0.10.12
- edk2-pytool-extensions: 0.13.9
- antlr4-python3-runtime: 4.7.1
- pyyaml: 5.3.1

Stating this because it might matter for the rest of my email.

)


* Second, when I run the following command:

   stuart_update -c .pytool/CISettings.py TOOL_CHAIN_TAG=GCC5

I get the following warnings:


WARNING - [SDE] Failed to fetch NugetDependecy: mu_nasm@2.14.02: [Nuget] We 
failed to install this version 2.14.02 of mu_nasm
WARNING - [SDE] Failed to fetch NugetDependecy: iasl@20190215.0.0: [Nuget] We 
failed to install this version 20190215.0.0 of iasl


(repeated one more time:)


WARNING - [SDE] Failed to fetch NugetDependecy: mu_nasm@2.14.02: [Nuget] We 
failed to install this version 2.14.02 of mu_nasm
WARNING - [SDE] Failed to fetch NugetDependecy: iasl@20190215.0.0: [Nuget] We 
failed to install this version 20190215.0.0 of iasl


and then finally:


ERROR - We were unable to successfully update 2 dependencies in environment
ERROR - Error


The virtual machine has NASM installed (2.13.03-2.el8) and IASL too
(acpica-tools-20180629-3.el8).

Where do the NASM and IASL version requirements (2.14.02 and
20190215.0.0, respectively) come from?

Hm... After a git-grep for those version numbers, I find:

- BaseTools/Bin/nasm_ext_dep.yaml
- BaseTools/Bin/iasl_ext_dep.yaml

I was about to say that these version requirements are too strict: for
example, "BaseTools/Conf/tools_def.template" requires "NASM 2.10 or
later for use with 

Re: [edk2-devel] [edk2-announce] Re: Soft Feature Freeze starts now for edk2-stable202008

[Liming Gao]

> -Original Message-
> From: Chang, Abner (HPS SW/FW Technologist) 
> Sent: Wednesday, August 19, 2020 10:59 PM
> To: Laszlo Ersek ; Leif Lindholm 
> Cc: devel@edk2.groups.io; Gao, Liming ; 
> annou...@edk2.groups.io; af...@apple.com; Kinney, Michael D
> 
> Subject: RE: [edk2-announce] Re: Soft Feature Freeze starts now for 
> edk2-stable202008
> 
> 
> 
> > -Original Message-
> > From: Laszlo Ersek [mailto:ler...@redhat.com]
> > Sent: Wednesday, August 19, 2020 10:30 PM
> > To: Chang, Abner (HPS SW/FW Technologist) ; Leif
> > Lindholm 
> > Cc: devel@edk2.groups.io; liming.gao ;
> > annou...@edk2.groups.io; af...@apple.com; Kinney, Michael D
> > 
> > Subject: Re: [edk2-announce] Re: Soft Feature Freeze starts now for edk2-
> > stable202008
> >
> > On 08/19/20 15:34, Chang, Abner (HPS SW/FW Technologist) wrote:
> > >
> > >
> > >> -Original Message-
> > >> From: Laszlo Ersek [mailto:ler...@redhat.com]
> > >> Sent: Wednesday, August 19, 2020 9:19 PM
> > >> To: Leif Lindholm ; Chang, Abner (HPS SW/FW
> > >> Technologist) 
> > >> Cc: devel@edk2.groups.io; liming.gao ;
> > >> annou...@edk2.groups.io; af...@apple.com; Kinney, Michael D
> > >> 
> > >> Subject: Re: [edk2-announce] Re: Soft Feature Freeze starts now for
> > >> edk2-
> > >> stable202008
> > >>
> > >> On 08/19/20 13:48, Leif Lindholm wrote:
> > >>> (Slightly trimmed recipient list due to different patch being
> > >>> discussed.)
> > >>>
> > >>> So, I can't make this call, because I'm the one who messed up.
> > >>>
> > >>> This patch does exactly what I had requested Abner to do some time
> > >>> back (off-list, unfortunately), and I was *convinced* I gave it an
> > >>> R-b as soon as it hit my inbox - until Abner nudged me about it 
> > >>> yesterday.
> > >>>
> > >>> The patch in question is
> > >>> https://edk2.groups.io/g/devel/topic/76021725
> > >>
> > >> My understanding is:
> > >>
> > >> (1) there is an external project that consumes the FDT library in
> > >> EmbeddedPkg, meaning the lib class header
> > >> "EmbeddedPkg/Include/libfdt.h"
> > >> and the lib instance "EmbeddedPkg/Library/FdtLib/FdtLib.inf",
> > > [Chang, Abner] Yes
> > >>
> > >> (2) the lib class header pulls in "fdt.h" and "libfdt_env.h",
> > > [Chang, Abner] yes
> > >>
> > >> (3) the external project is not edk2-platforms,
> > > [Chang, Abner] yes
> > >>
> > >> (4) the external project wants -- for some strange reason -- edk2's
> > >> "libfdt_env.h" to provide an strncmp() function (or function-like
> > >> macro), with that particular stncmp() implementation not being needed
> > >> in either edk2- platforms or edk2 itself,
> > > [Chang, Abner] yes, at least so far
> > >>
> > >> (5) the patch for adding said strncmp() was posted on Aug 6th (at
> > >> least when viewed from my time zone), i.e., before the SFF,
> > > [Chang, Abner] Yes
> > >>
> > >> (6) it was reviewed 12 days later (within the SFF)
> > > [Chang, Abner] yes.
> > >>
> > >> If my understanding is correct, then I don't see how this patch could
> > >> be considered a bugfix -- even as a feature addition, it seems hardly
> > >> justified to me --, and there would have been ~8 days before the SFF to
> > review it.
> > >>
> > >> I think we should postpone the patch until after the stable tag.
> > > This patch is important because the edk2-stable202008 would be the stable
> > tag (if this patch is accepted) for booting RISC-V platform to Linux kernel 
> > with
> > EFI Runtime service on either real platform and QEMU. We can publish this
> > information in RISC-V community which is considered as a valuable milestone
> > for RISC-V edk2 port.
> >
> > Let's move out the dates for the stable tag then, by one week:
> >
> > - let the SFF start on 2020-08-21
> > - let the HFF start on 2020-08-28
> > - let's release edk2-stable202008 on 2020-09-04
> >
> > Release slips are permitted and there have been examples.
> Appreciate for this.

Previous stable tags (202005/202002/201905) extended SFF or HFF period to let 
the critical bug fix catch the stable tag. 
Today case is like a new feature. According to Abner comment, it is important 
to RISC-V community.
If SFF start time is changed, more features may catch this stable, such as 
VariableLock feature.
> 
> >
> > What doesn't make sense is making rules and then breaking them
> > opportunistically, whenever they're uncomfortable. If that's a frequent
> > occurrence, we should pick different rules, or -- again -- if this is a very
> > important patch, we should delay the release for it.

I suggest to define the process on delay the release. The initial process is 
specified here.
The requestor provides the justification why his patch needs to catch the 
stable tag after SFF phase. 
All stewards make the decision whether delay the stable tag to include his 
patch.  
If stewards agree to delay the stable tag, and there is no objection from the 
community, 
the release maintainer will update edk2 release planning with new date. 
If stewards rejects to this 

Re: [edk2-devel] [PATCH v2 2/2] CryptoPkg/OpensslLib: Commit the auto-generated assembly files for X64

[Michael D Kinney]

Laszlo,

The current CryptoPkg DCS file with use of the CRYPTO_SERVICES define is 
cumbersome.

  #
  # Flavor of PEI, DXE, SMM modules to build.
  # Must be one of ALL, NONE, MIN_PEI, MIN_DXE_MIN_SMM.
  # Default is ALL that is used for package build verification.
  #   PACKAGE - Package verification build of all components.  Null
  # versions of libraries are used to minimize build times.
  #   ALL - Build PEIM, DXE, and SMM drivers.  Protocols and PPIs
  # publish all services.
  #   NONE- Build PEIM, DXE, and SMM drivers.  Protocols and PPIs
  # publish no services.  Used to verify compiler/linker
  # optimizations are working correctly.
  #   MIN_PEI - Build PEIM with PPI that publishes minimum required
  # services.
  #   MIN_DXE_MIN_SMM - Build DXE and SMM drivers with Protocols that publish
  # minimum required services.
  #
  DEFINE CRYPTO_SERVICES = PACKAGE

There is a known limitation for using structured PCDs in a module scope and 
that limitation is what resulted in the use of this define.  Bob Feng
has provided a BaseTools patch that attempts to address this limitation.

https://edk2.groups.io/g/devel/message/63906

This patch is functional, but has one open issue around the PCD report.  Once
this issue is resolved we will be able to specify structured PCD field values
in the scope of a single module.  I have a branch that simplifies the DSC and
allows all flavors of the crypto modules to be built in a single invocation
of the build command.  There is more cleanup of the DSC possible, but I
wanted to share a quick test case for Bob's patch.

https://github.com/mdkinney/edk2/tree/Bug_xxx_CryptoPkg_UseModuleScopedPcds

This feature supports both the generation of standard flavors of the crypto
modules that a platform could consume as a pre-built binary and also allows
a platform to choose their own profile by specifying the specific crypto APIs
needed in PEI, DXE, SMM when building crypto modules from sources.
  
Best regards,

Mike

> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Laszlo Ersek
> Sent: Wednesday, August 19, 2020 3:44 AM
> To: devel@edk2.groups.io; spbro...@outlook.com; Kinney, Michael D 
> ; Wang, Jian J
> ; Zurcher, Christopher J 
> ; Yao, Jiewen 
> Cc: Lu, XiaoyuX ; Ard Biesheuvel 
> 
> Subject: Re: [edk2-devel] [PATCH v2 2/2] CryptoPkg/OpensslLib: Commit the 
> auto-generated assembly files for X64
> 
> Hi,
> 
> On 08/18/20 23:33, Sean wrote:
> > Mike,
> >
> > I am not technically a basetool maintainer but as an active user/dev in
> > basetools, i would be opposed to bringing in perl as an edk2 dependency.
> > Also introducing another language is counter to the goal of aligning on
> > python and improving the python used within edk2.  From my perspective
> > the openssl config case isn't strong enough to counter the above goal.
> > In fact as you know we are trying to change the paradigm for
> > Crypto/OpenSSL with the Crypto Driver
> > (https://github.com/tianocore/edk2/tree/master/CryptoPkg/Driver) and
> > BaseCryptLibOnProtocolPpi
> > (https://github.com/tianocore/edk2/tree/master/CryptoPkg/Library/BaseCryptLibOnProtocolPpi)
> > work so that everyday development doesn't need to compile openssl in
> > their edk2 builds.
> 
> Here I'd only like to comment on this one aspect (= build OpenSSL as a
> statically linked library vs. as a crypto service driver / PEIM).
> 
> Recently I tried to evaluate the crypto driver for OVMF. I started with
> the PEI phase. The configuration interface (the PCD) is baroque, *BUT*
> that is a direct consequence of OpenSSL offering a huge range of
> interfaces. So no complaints about the config interface.
> 
> I also reviewed the CryptoPkg.dsc "pre-sets" (i.e., CRYPTO_SERVICES
> being one of PACKAGE / ALL / NONE / MIN_PEI / MIN_DXE_MIN_SMM). I had
> two problems:
> 
> - These pre-sets are supremely suitable for a platform that is composed
> of multiple build runs; that is, build the crypto PEIM, build the DXE /
> SMM protocol drivers, package up the resultant binaries, and *then*
> build the actual platforms (which will then include the crypto service
> drivers in *binary* form). On the other hand, the pre-sets are not
> useful to a platform that is supposed to be built in a single-shot.
> Importantly, I'm not saying that the pre-sets are *detrimental* to such
> platforms -- they aren't. It's just that the pre-sets target a different
> use case.
> 
> - The other problem I had was the one that we had discussed when the
> crypto service driver was being introduced. Namely, selecting the
> OpenSSL interfaces (interface families) that the platform actually consumes.
> 
> Now, I carefully tracked down the modules in OVMF that needed crypto
> support, by *not* resolving SmmCryptLib, RuntimeCryptLib, TlsLib in
> general [LibraryClasses] sections in the OVMF DSC 

Re: [edk2-devel] running CI locally

[Michael D Kinney]

Hi Laszlo,

Thank you for the feedback.  I agree there are some documentation updates 
required.

The spell check requirements are documented here:

https://github.com/tianocore/edk2/blob/master/.pytool/Readme.md#spell-checking---cspell

You do need to install nodejs and cspell.

* Install nodejs from https://nodejs.org/en/
* Install cspell
  1. Open cmd prompt with access to node and npm
  2. Run `npm install -g cspell`

I agree that the verbosity of the output is very brief.  There is a more 
complete
log that is always produced in the build output directory in 
Build/CI_BUILDLOG.txt.
Can you look at that file and see if it provide a more detailed reason for the
failure?

Thanks,

Mike


> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Laszlo Ersek
> Sent: Wednesday, August 19, 2020 5:27 AM
> To: edk2-devel-groups-io ; Sean Brogan 
> ; Bret Barkelew
> 
> Cc: Vladimir Olovyannikov ; Rebecca Cran 
> ; Tom Lendacky
> 
> Subject: [edk2-devel] running CI locally
> 
> Hi!
> 
> I'd like to test CI locally. I'm going through ".pytool/Readme.md" with
> the tree checked out at 7e6f150b6902 (= current HEAD). I'm doing this in
> a RHEL8 VM, with a python3 virtual environment set up / entered.
> 
> 
> * My first note is that the command
> 
>   pip install --upgrade pip-requirements.txt
> 
> under "Prerequisets", has a small typo; it should be
> 
>   pip install --upgrade -r pip-requirements.txt
> 
> (the "-r" option is missing).
> 
> (
> 
> After adding "-r", the following components are now installed in my
> virtual env:
> 
> - edk2-pytool-library: 0.10.12
> - edk2-pytool-extensions: 0.13.9
> - antlr4-python3-runtime: 4.7.1
> - pyyaml: 5.3.1
> 
> Stating this because it might matter for the rest of my email.
> 
> )
> 
> 
> * Second, when I run the following command:
> 
>   stuart_update -c .pytool/CISettings.py TOOL_CHAIN_TAG=GCC5
> 
> I get the following warnings:
> 
> > WARNING - [SDE] Failed to fetch NugetDependecy: mu_nasm@2.14.02: [Nuget] We 
> > failed to install this version 2.14.02 of mu_nasm
> > WARNING - [SDE] Failed to fetch NugetDependecy: iasl@20190215.0.0: [Nuget] 
> > We failed to install this version 20190215.0.0 of iasl
> 
> (repeated one more time:)
> 
> > WARNING - [SDE] Failed to fetch NugetDependecy: mu_nasm@2.14.02: [Nuget] We 
> > failed to install this version 2.14.02 of mu_nasm
> > WARNING - [SDE] Failed to fetch NugetDependecy: iasl@20190215.0.0: [Nuget] 
> > We failed to install this version 20190215.0.0 of iasl
> 
> and then finally:
> 
> > ERROR - We were unable to successfully update 2 dependencies in environment
> > ERROR - Error
> 
> The virtual machine has NASM installed (2.13.03-2.el8) and IASL too
> (acpica-tools-20180629-3.el8).
> 
> Where do the NASM and IASL version requirements (2.14.02 and
> 20190215.0.0, respectively) come from?
> 
> Hm... After a git-grep for those version numbers, I find:
> 
> - BaseTools/Bin/nasm_ext_dep.yaml
> - BaseTools/Bin/iasl_ext_dep.yaml
> 
> I was about to say that these version requirements are too strict: for
> example, "BaseTools/Conf/tools_def.template" requires "NASM 2.10 or
> later for use with the GCC toolchain family". What I have installed
> satisfies that, and so CI shouldn't require anything more recent.
> *However*, both of the above YAML files have very helpful comments, so I
> understand these high versions are downloaded afresh, and only for the
> CI run.
> 
> And so my question becomes: why do the "nuget" downloads fail for me
> (because, presumably, they work fine in the central CI env on github /
> Azure); and how can I fix the issue if it pops up again?
> 
> I've checked "nuget.org" in my browser, and it has:
> 
> - https://www.nuget.org/packages/mu_nasm/ -->  2.14.2
> - https://www.nuget.org/packages/iasl/ -->  20190215.0.0
> 
> ... On a hunch, I've attempted adding the "--verbose" option to the
> "stuart_update" command; this is the output (excerpt):
> 
> > DEBUG - Verify 'mu_nasm' returning 'False'.
> > DEBUG - Verify 'iasl' returning 'False'.
> > DEBUG - Creating 4 threads for the SDE update
> > UpdatingDEBUG - Verify 'mu_nasm' returning 'False'.
> > DEBUG - Verify 'gcc_aarch64_linux' returning 'True'.
> > DEBUG - Cleaning dependency directory for 'mu_nasm'...
> > DEBUG - Verify 'gcc_arm_linux' returning 'True'.
> > INFO - Cmd to run is: mono 
> > /root/py3venv/lib/python3.6/site-packages/edk2toolext/bin/NuGet.exe locals 
> > global-packages -list
> > DEBUG - Verify 'iasl' returning 'False'.
> > INFO - 
> > DEBUG - Cleaning dependency directory for 'iasl'...
> > DEBUG - Verify 'gcc_riscv64_unknown' returning 'True'.
> > INFO - --Cmd Output Starting---
> > INFO - Cmd to run is: mono 
> > /root/py3venv/lib/python3.6/site-packages/edk2toolext/bin/NuGet.exe locals 
> > global-packages -list
> > INFO - 
> > INFO - 
> > INFO - 

Re: [edk2-devel] question about UnitTest Framework

[Michael D Kinney]

Hi,

I would also like to add that UEFI SCTs and Unit Testing are different scopes 
of testing.

The UEFI SCTs are system level conformance tests that require and entire 
platform FW image to execute on a platform and they focus on measuring if a 
conformant set of UEFI interfaces are present.  The UEFI SCTs need to run 
correctly no matter what the underlying implementation is, so these are black 
box tests and do not have a large number of test cases against each API.  We 
can only implement test cases based on the API definitions in the UEFI 
Specifications.

The UnitTestFrameworkPkg provide both host and target based testing 
environment, and the scope of a unit test is a single C/ASM function, and that 
single function can either be public APIs or internal worker functions.  It 
supports both black box and white box testing.  The white box testing approach 
allows all code paths to be exercised with different input data patterns to 
verify that a function works as expected for all inputs.  The host based tests 
run as an application and do not require a full platform FW image.  This allows 
the unit test to test code paths that may be very rare in full system testing 
or require HW to be broken to be seen in full system testing.  This means the 
UnitTestFrameworkPkg has the potential to provide much greater code coverage 
than UEFI SCTs.

Thanks,

Mike

From: devel@edk2.groups.io  On Behalf Of Bret Barkelew 
via groups.io
Sent: Tuesday, August 18, 2020 11:00 PM
To: devel@edk2.groups.io; tiger...@zhaoxin.com
Subject: Re: [edk2-devel] question about UnitTest Framework

Correct. It’s possible that we can have some sort of test reunion at some 
point, but SCT is largely for interfaces and spec compliance, and the UnitTest 
Framework is for implementation business logic, CI, and code coverage.

- Bret

From: Tiger Liu(BJ-RD) via groups.io
Sent: Tuesday, August 18, 2020 10:17 PM
To: devel@edk2.groups.io; Tiger 
Liu(BJ-RD)
Subject: [外部] Re: [edk2-devel] question about UnitTest Framework

Hi,
After studying previous discuss mail, UnitTest Framework is work for Continuous 
Integration mechanism.

Code writer will provide some unit test functions, maybe they could also do 
code coverage test.

Thanks
-邮件原件-
发件人: devel@edk2.groups.io 
mailto:devel@edk2.groups.io>> 代表 Tiger Liu(BJ-RD)
发送时间: 2020年8月13日 16:03
收件人: devel@edk2.groups.io
主题: [edk2-devel] question about UnitTest Framework

Hi, Experts:
I have a question about UnitTest Framework.

UEFI Code has included some test infrastructure, such as:
PI-SCT / SCT / FWTS etc.

So, why we introduce a new UnitTest Framework?

Is it mainly for Code Coverage test?

Thanks


保密声明：
本邮件含有保密或专有信息，仅供指定收件人使用。严禁对本邮件或其内容做任何未经授权的查阅、使用、复制或转发。
CONFIDENTIAL NOTE:
This email contains confidential or legally privileged information and is for 
the sole use of its intended recipient. Any unauthorized review, use, copying 
or forwarding of this email or the content of this email is strictly prohibited.





保密声明：
本邮件含有保密或专有信息，仅供指定收件人使用。严禁对本邮件或其内容做任何未经授权的查阅、使用、复制或转发。
CONFIDENTIAL NOTE:
This email contains confidential or legally privileged information and is for 
the sole use of its intended recipient. Any unauthorized review, use, copying 
or forwarding of this email or the content of this email is strictly prohibited.



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#64446): https://edk2.groups.io/g/devel/message/64446
Mute This Topic: https://groups.io/mt/76163664/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [edk2-announce] Re: Soft Feature Freeze starts now for edk2-stable202008

[Abner Chang]

> -Original Message-
> From: Laszlo Ersek [mailto:ler...@redhat.com]
> Sent: Wednesday, August 19, 2020 10:30 PM
> To: Chang, Abner (HPS SW/FW Technologist) ; Leif
> Lindholm 
> Cc: devel@edk2.groups.io; liming.gao ;
> annou...@edk2.groups.io; af...@apple.com; Kinney, Michael D
> 
> Subject: Re: [edk2-announce] Re: Soft Feature Freeze starts now for edk2-
> stable202008
> 
> On 08/19/20 15:34, Chang, Abner (HPS SW/FW Technologist) wrote:
> >
> >
> >> -Original Message-
> >> From: Laszlo Ersek [mailto:ler...@redhat.com]
> >> Sent: Wednesday, August 19, 2020 9:19 PM
> >> To: Leif Lindholm ; Chang, Abner (HPS SW/FW
> >> Technologist) 
> >> Cc: devel@edk2.groups.io; liming.gao ;
> >> annou...@edk2.groups.io; af...@apple.com; Kinney, Michael D
> >> 
> >> Subject: Re: [edk2-announce] Re: Soft Feature Freeze starts now for
> >> edk2-
> >> stable202008
> >>
> >> On 08/19/20 13:48, Leif Lindholm wrote:
> >>> (Slightly trimmed recipient list due to different patch being
> >>> discussed.)
> >>>
> >>> So, I can't make this call, because I'm the one who messed up.
> >>>
> >>> This patch does exactly what I had requested Abner to do some time
> >>> back (off-list, unfortunately), and I was *convinced* I gave it an
> >>> R-b as soon as it hit my inbox - until Abner nudged me about it yesterday.
> >>>
> >>> The patch in question is
> >>> https://edk2.groups.io/g/devel/topic/76021725
> >>
> >> My understanding is:
> >>
> >> (1) there is an external project that consumes the FDT library in
> >> EmbeddedPkg, meaning the lib class header
> >> "EmbeddedPkg/Include/libfdt.h"
> >> and the lib instance "EmbeddedPkg/Library/FdtLib/FdtLib.inf",
> > [Chang, Abner] Yes
> >>
> >> (2) the lib class header pulls in "fdt.h" and "libfdt_env.h",
> > [Chang, Abner] yes
> >>
> >> (3) the external project is not edk2-platforms,
> > [Chang, Abner] yes
> >>
> >> (4) the external project wants -- for some strange reason -- edk2's
> >> "libfdt_env.h" to provide an strncmp() function (or function-like
> >> macro), with that particular stncmp() implementation not being needed
> >> in either edk2- platforms or edk2 itself,
> > [Chang, Abner] yes, at least so far
> >>
> >> (5) the patch for adding said strncmp() was posted on Aug 6th (at
> >> least when viewed from my time zone), i.e., before the SFF,
> > [Chang, Abner] Yes
> >>
> >> (6) it was reviewed 12 days later (within the SFF)
> > [Chang, Abner] yes.
> >>
> >> If my understanding is correct, then I don't see how this patch could
> >> be considered a bugfix -- even as a feature addition, it seems hardly
> >> justified to me --, and there would have been ~8 days before the SFF to
> review it.
> >>
> >> I think we should postpone the patch until after the stable tag.
> > This patch is important because the edk2-stable202008 would be the stable
> tag (if this patch is accepted) for booting RISC-V platform to Linux kernel 
> with
> EFI Runtime service on either real platform and QEMU. We can publish this
> information in RISC-V community which is considered as a valuable milestone
> for RISC-V edk2 port.
> 
> Let's move out the dates for the stable tag then, by one week:
> 
> - let the SFF start on 2020-08-21
> - let the HFF start on 2020-08-28
> - let's release edk2-stable202008 on 2020-09-04
> 
> Release slips are permitted and there have been examples.
Appreciate for this.

> 
> What doesn't make sense is making rules and then breaking them
> opportunistically, whenever they're uncomfortable. If that's a frequent
> occurrence, we should pick different rules, or -- again -- if this is a very
> important patch, we should delay the release for it.
> 
> BTW what about reverting the OpenSBI change? You could still call
> sbi_strncmp() -- rather than strncmp() -- in the "helper" lib.
I replied this in another thread.
> 
> Thanks
> Laszlo


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#64445): https://edk2.groups.io/g/devel/message/64445
Mute This Topic: https://groups.io/mt/76284301/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [edk2-announce] Re: Soft Feature Freeze starts now for edk2-stable202008

[Abner Chang]

> -Original Message-
> From: Laszlo Ersek [mailto:ler...@redhat.com]
> Sent: Wednesday, August 19, 2020 9:37 PM
> To: Leif Lindholm ; Chang, Abner (HPS SW/FW
> Technologist) 
> Cc: devel@edk2.groups.io; liming.gao ;
> annou...@edk2.groups.io; af...@apple.com; Kinney, Michael D
> 
> Subject: Re: [edk2-announce] Re: Soft Feature Freeze starts now for edk2-
> stable202008
> 
> On 08/19/20 15:19, Laszlo Ersek wrote:
> > On 08/19/20 13:48, Leif Lindholm wrote:
> >> (Slightly trimmed recipient list due to different patch being
> >> discussed.)
> >>
> >> So, I can't make this call, because I'm the one who messed up.
> >>
> >> This patch does exactly what I had requested Abner to do some time
> >> back (off-list, unfortunately), and I was *convinced* I gave it an
> >> R-b as soon as it hit my inbox - until Abner nudged me about it yesterday.
> >>
> >> The patch in question is
> >> https://edk2.groups.io/g/devel/topic/76021725
> >
> > My understanding is:
> >
> > (1) there is an external project that consumes the FDT library in
> > EmbeddedPkg, meaning the lib class header
> "EmbeddedPkg/Include/libfdt.h"
> > and the lib instance "EmbeddedPkg/Library/FdtLib/FdtLib.inf",
> >
> > (2) the lib class header pulls in "fdt.h" and "libfdt_env.h",
> >
> > (3) the external project is not edk2-platforms,
> >
> > (4) the external project wants -- for some strange reason -- edk2's
> > "libfdt_env.h" to provide an strncmp() function (or function-like
> > macro), with that particular stncmp() implementation not being needed
> > in either edk2-platforms or edk2 itself,
> >
> > (5) the patch for adding said strncmp() was posted on Aug 6th (at
> > least when viewed from my time zone), i.e., before the SFF,
> >
> > (6) it was reviewed 12 days later (within the SFF)
> >
> > If my understanding is correct, then I don't see how this patch could
> > be considered a bugfix -- even as a feature addition, it seems hardly
> > justified to me --, and there would have been ~8 days before the SFF
> > to review it.
> >
> > I think we should postpone the patch until after the stable tag.
> 
> Regarding OpenSBI, as of commit 9d56961b2314, the sbi_strncmp() function
> has not been removed, and it seems that sbi_strncmp() is still used / called 
> in
> at least some build configurations (where the
> 
> lib/utils/libfdt/libfdt_env.h:#define strncmp   sbi_strncmp
> 
> definition is supposed to be in effect).
> 
> This means that the codebase can not rid itself of sbi_strncmp().
The code base doesn't have to get rid of using sbi_strncmp function (the code 
base defines bunch of sbi_strxxx functions) which may be used in other non-fdt 
related OpenSBI code and it is out of fdt library scope.

> 
> To me this indicates that OpenSBI commit 2cfd2fc90488 ("lib: utils: Use
> strncmp in fdt_parse_hart_id()", 2020-07-29) was wrong. It should have
> replaced sbi_strcmp() with sbi_strncmp(), not strncmp().
We don’t use libfdt_env.h from OpenSBI, we use libfdt_env.h from EmbeddedPkg 
which is impossible to define a macro to replace "sbi_strncmp" with  
AsciiStrnCmp in Edk2 libfdt_env.h. We definitely can include sbi_string.h 
somewhere in EDK2 RISC-V header file (this is the temp solution I use now  
before edk2 upstream has this fix), so it won't pop up build error. But this 
temp solution looks ugly and specifically. Furthermore, OpenSBI fdt helper lib 
is sort of a partial fdtlib code, use C API keeps the consistency with fdtlib 
makes sense to me.
> 
> After all, the size limit seems to be the motivation for the entire change --
> put a size limit on the string comparison in fdt_parse_hart_id(). Commit
> 2cfd2fc90488 did two things at the same
> time: replace the size-unbounded comparison with the size-bounded one,
> *and* switch to the standard C function name from the self-contained
> function. The former goal looks justifiable, the latter I don't understand
Actually the major motivation is not for the bounded comparison, that is to use 
C API and leverage edk2 libfdt_env.h.
> 
> Now: I realize that, going back to edk2 commit fa4a70633868
> ("EmbeddedPkg: Added support to libfdt", 2012-09-27), we already have a
> bunch of wrappers in "EmbeddedPkg/Include/libfdt_env.h". So I guess
> adding "one more" is not inconsistent with those -- even though the lib
> instance within edk2 doesn't need the new function.
> 
> But it's still a micro-feature whose review should have completed before the
> SFF.
> 
> Thanks
> Laszlo


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#6): https://edk2.groups.io/g/devel/message/6
Mute This Topic: https://groups.io/mt/76284301/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH] SgiPkg: Add chip count PCD to build information file for rdn1edgex2

[Vijayenthiran Subramaniam]

PcdChipCount constant is used to define the maximum number chips
included in the multi-chip configuration. Add this constant to the
RDN1EdgeX2 ACPI tables build information (INF) file to allow this
constant to be used in the RDN1EdgeX2 ACPI tables.

Signed-off-by: Vijayenthiran Subramaniam 
---
 Platform/ARM/SgiPkg/AcpiTables/RdN1EdgeX2AcpiTables.inf | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Platform/ARM/SgiPkg/AcpiTables/RdN1EdgeX2AcpiTables.inf 
b/Platform/ARM/SgiPkg/AcpiTables/RdN1EdgeX2AcpiTables.inf
index 974d9db543..d44f02ab0c 100644
--- a/Platform/ARM/SgiPkg/AcpiTables/RdN1EdgeX2AcpiTables.inf
+++ b/Platform/ARM/SgiPkg/AcpiTables/RdN1EdgeX2AcpiTables.inf
@@ -45,6 +45,8 @@
   gArmSgiTokenSpaceGuid.PcdDramBlock2Base
   gArmSgiTokenSpaceGuid.PcdDramBlock2Size
 
+  gArmSgiTokenSpaceGuid.PcdChipCount
+
   gArmTokenSpaceGuid.PcdArmArchTimerSecIntrNum
   gArmTokenSpaceGuid.PcdArmArchTimerIntrNum
   gArmTokenSpaceGuid.PcdArmArchTimerHypIntrNum
-- 
2.17.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#64443): https://edk2.groups.io/g/devel/message/64443
Mute This Topic: https://groups.io/mt/76288010/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 1/1] UefiCpuPkg/MpInitLib: Always initialize the DoDecrement variable

[Liming Gao]

Laszlo:

> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Laszlo Ersek
> Sent: Wednesday, August 19, 2020 5:24 PM
> To: Tom Lendacky ; devel@edk2.groups.io
> Cc: Gao, Liming ; Dong, Eric ; Ni, 
> Ray ; Kumar, Rahul1
> 
> Subject: Re: [edk2-devel] [PATCH 1/1] UefiCpuPkg/MpInitLib: Always initialize 
> the DoDecrement variable
> 
> On 08/18/20 15:10, Tom Lendacky wrote:
> > From: Tom Lendacky 
> >
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2901
> >
> > The DoDecrement variable in ApWakeupFunction () wasn't always being
> > initialized. Update the code to always fully initialize it.
> >
> > Cc: Eric Dong 
> > Cc: Ray Ni 
> > Cc: Laszlo Ersek 
> > Cc: Rahul Kumar 
> > Signed-off-by: Tom Lendacky 
> > ---
> >  UefiCpuPkg/Library/MpInitLib/MpLib.c | 4 +---
> >  1 file changed, 1 insertion(+), 3 deletions(-)
> >
> > diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c 
> > b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> > index 90416c81b616..e24bdc64f930 100644
> > --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
> > +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> > @@ -885,9 +885,7 @@ ApWakeupFunction (
> >UINT64Status;
> >BOOLEAN   DoDecrement;
> >
> > -  if (CpuMpData->InitFlag == ApInitConfig) {
> > -DoDecrement = TRUE;
> > -  }
> > +  DoDecrement = (CpuMpData->InitFlag == ApInitConfig) ? TRUE : 
> > FALSE;
> >
> >while (TRUE) {
> >  Msr.GhcbPhysicalAddress = AsmReadMsr64 (MSR_SEV_ES_GHCB);
> >
> 
> Not that I want to obsess about style, but
> 
>   (condition) ? TRUE : FALSE
> 
> is an anti-patter that's similar to
> 
>   (condition) == TRUE
> 
> Instead, I suggest:
> 
>   DoDecrement = (BOOLEAN)(CpuMpData->InitFlag == ApInitConfig);
> 
> (The (BOOLEAN) cast is necessary, or at least used to be necessary,
> becasue the == operator returns "int" (INT32), but BOOLEAN (i.e., the
> type of "DoDecrement") is UINT8 -- and some VS toolchains perceive (or
> used to perceive) this implicit conversion as a "potential loss of
> precision". That warning is of course bogus, as the == operator only
> produces 0 or 1, each of which values fits comfortably into a UINT8. But
> still the explicit (BOOLEAN) cast is how we suppress the warning.)

I agree this style is simpler than before. 
> 
> Different question: who's supposed to merge (v2 of) this? Per
> "Maintainers.txt", it should be Eric or Ray; OTOH, maybe the fix is
> urgent (build failure with CLANGPDB) and anyone with push access could
> qualify.
This fix needs to catch this stable tag. Once the package maintainer reviews 
it, I will merge it. 

Thanks
Liming
> 
> Thanks,
> Laszlo
> 
> 
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#64442): https://edk2.groups.io/g/devel/message/64442
Mute This Topic: https://groups.io/mt/76264245/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH edk2-platforms 5/7] SbsaQemu: AcpiDxe: Create SSDT table at runtime

[Tanmay Jagdale]

- Add support to create SSDT table at runtime. Since SSDT
  table is a data table, added a few helper macros to create
  the AML entries.
- Also added a function to calculate the length of Packages.

Signed-off-by: Tanmay Jagdale 
---
 .../Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c | 144 ++
 .../Include/IndustryStandard/SbsaQemuAcpi.h   |  29 
 2 files changed, 173 insertions(+)

diff --git a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c 
b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
index 569cda8b6474..d90ce0c2a718 100644
--- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
+++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
@@ -9,6 +9,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -202,6 +203,144 @@ AddMadtTable (
   return Status;
 }
 
+/*
+ * Function to calculate the PkgLength field in ACPI tables
+ */
+STATIC
+UINT32
+SetPkgLength (
+  IN UINT8  *TablePtr,
+  IN UINT32 Length
+)
+{
+  UINT8  ByteCount;
+  UINT8  *PkgLeadByte = TablePtr;
+
+  if (Length < 64) {
+*TablePtr = Length;
+return 1;
+  }
+
+  // Set the LSB of Length in PkgLeadByte and advance Length
+  *PkgLeadByte = Length & 0xF;
+  Length = Length >> 4;
+
+  while (Length) {
+TablePtr++;
+*TablePtr = (Length & 0xFF);
+Length = (Length >> 8);
+  }
+
+  // Calculate the number of bytes the Length field uses
+  // and set the ByteCount field in PkgLeadByte.
+  ByteCount = (TablePtr - PkgLeadByte) & 0xF;
+  *PkgLeadByte |= (ByteCount << 6);
+
+  return ByteCount + 1;
+}
+
+/*
+ * A function that adds SSDT ACPI table.
+ */
+EFI_STATUS
+AddSsdtTable (
+  IN EFI_ACPI_TABLE_PROTOCOL   *AcpiTable
+  )
+{
+  EFI_STATUSStatus;
+  UINTN TableHandle;
+  UINT32TableSize;
+  EFI_PHYSICAL_ADDRESS  PageAddress;
+  UINT8 *New;
+  UINT32CpuId;
+  UINT32Offset;
+  UINT8 ScopeOpName[] =  SBSAQEMU_ACPI_SCOPE_NAME;
+  UINT32NumCores = PcdGet32 (PcdCoreCount);
+
+  EFI_ACPI_DESCRIPTION_HEADER Header =
+SBSAQEMU_ACPI_HEADER (
+  EFI_ACPI_6_0_SECONDARY_SYSTEM_DESCRIPTION_TABLE_SIGNATURE,
+  EFI_ACPI_DESCRIPTION_HEADER,
+  EFI_ACPI_6_0_SECONDARY_SYSTEM_DESCRIPTION_TABLE_REVISION);
+
+  SBSAQEMU_ACPI_CPU_DEVICE CpuDevice = {
+{ AML_EXT_OP, AML_EXT_DEVICE_OP }, /* Device () */
+SBSAQEMU_ACPI_CPU_DEV_LEN, /* Length */
+SBSAQEMU_ACPI_CPU_DEV_NAME,/* Device Name "C000" */
+SBSAQEMU_ACPI_CPU_HID, /* Name (HID, "ACPI0007") */
+SBSAQEMU_ACPI_CPU_UID, /* Name (UID, 0) */
+  };
+
+  // Calculate the new table size based on the number of cores
+  TableSize = sizeof (EFI_ACPI_DESCRIPTION_HEADER) +
+  SBSAQEMU_ACPI_SCOPE_OP_MAX_LENGTH + sizeof (ScopeOpName) +
+ (sizeof (CpuDevice) * NumCores);
+
+  Status = gBS->AllocatePages (
+  AllocateAnyPages,
+  EfiACPIReclaimMemory,
+  EFI_SIZE_TO_PAGES (TableSize),
+  
+  );
+  if (EFI_ERROR(Status)) {
+DEBUG((EFI_D_ERROR, "Failed to allocate pages for SSDT table\n"));
+return EFI_OUT_OF_RESOURCES;
+  }
+
+  New = (UINT8 *)(UINTN) PageAddress;
+  ZeroMem (New, TableSize);
+
+  // Add the ACPI Description table header
+  CopyMem (New, , sizeof (EFI_ACPI_DESCRIPTION_HEADER));
+  ((EFI_ACPI_DESCRIPTION_HEADER*) New)->Length = TableSize;
+  New += sizeof (EFI_ACPI_DESCRIPTION_HEADER);
+
+  // Insert the top level ScopeOp
+  *New = AML_SCOPE_OP;
+  New++;
+  Offset = SetPkgLength (New,
+ (TableSize - sizeof (EFI_ACPI_DESCRIPTION_HEADER) - 1));
+  New += Offset;
+  CopyMem (New, , sizeof (ScopeOpName));
+  New += sizeof (ScopeOpName);
+
+  // Add new Device structures for the Cores
+  for (CpuId = 0; CpuId < NumCores; CpuId++) {
+SBSAQEMU_ACPI_CPU_DEVICE *CpuDevicePtr;
+UINT8 CpuIdByte1, CpuIdByte2, CpuIdByte3;
+
+CopyMem (New, , sizeof (SBSAQEMU_ACPI_CPU_DEVICE));
+CpuDevicePtr = (SBSAQEMU_ACPI_CPU_DEVICE *) New;
+
+CpuIdByte1 = CpuId & 0xF;
+CpuIdByte2 = (CpuId >> 4) & 0xF;
+CpuIdByte3 = (CpuId >> 8) & 0xF;
+
+CpuDevicePtr->dev_name[1] = SBSAQEMU_ACPI_ITOA(CpuIdByte3);
+CpuDevicePtr->dev_name[2] = SBSAQEMU_ACPI_ITOA(CpuIdByte2);
+CpuDevicePtr->dev_name[3] = SBSAQEMU_ACPI_ITOA(CpuIdByte1);
+
+CpuDevicePtr->uid[6] = CpuIdByte1 | CpuIdByte2;
+CpuDevicePtr->uid[7] = CpuIdByte3;
+New += sizeof (SBSAQEMU_ACPI_CPU_DEVICE);
+  }
+
+  // Perform Checksum
+  AcpiPlatformChecksum ((UINT8*) PageAddress, TableSize);
+
+  Status = AcpiTable->InstallAcpiTable (
+AcpiTable,
+(EFI_ACPI_COMMON_HEADER *)PageAddress,
+TableSize,
+
+);
+  if (EFI_ERROR(Status)) {
+DEBUG((EFI_D_ERROR, "Failed to install SSDT table\n"));
+  

[edk2-devel] [PATCH edk2-platforms 6/7] SbsaQemu: AcpiDxe: Create PPTT table at runtime

[Tanmay Jagdale]

Add support to create Processor Properties Topology Table at
runtime. The cache topology of each CPU is as follows:

  CPU N
 
 |      |
 |  | L1-I |  | L1-D |  |
 |  | 32KB |  | 32KB |  |
 |      |
 |  --  |
 |  |L2 512KB|  |
 |  --  |
 

Signed-off-by: Tanmay Jagdale 
---
 .../Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c | 111 
 .../Include/IndustryStandard/SbsaQemuAcpi.h   | 124 ++
 2 files changed, 235 insertions(+)

diff --git a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c 
b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
index d90ce0c2a718..8527b976ee33 100644
--- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
+++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
@@ -9,6 +9,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -341,6 +342,111 @@ AddSsdtTable (
   return Status;
 }
 
+/*
+ * A function that adds the SSDT ACPI table.
+ */
+EFI_STATUS
+AddPpttTable (
+  IN EFI_ACPI_TABLE_PROTOCOL   *AcpiTable
+  )
+{
+  EFI_STATUSStatus;
+  UINTN TableHandle;
+  UINT32TableSize;
+  EFI_PHYSICAL_ADDRESS  PageAddress;
+  UINT8 *New;
+  UINT32CpuId;
+  UINT32NumCores = PcdGet32 (PcdCoreCount);
+
+  EFI_ACPI_6_3_PPTT_STRUCTURE_CACHE L1DCache = 
SBSAQEMU_ACPI_PPTT_L1_D_CACHE_STRUCT;
+  EFI_ACPI_6_3_PPTT_STRUCTURE_CACHE L1ICache = 
SBSAQEMU_ACPI_PPTT_L1_I_CACHE_STRUCT;
+  EFI_ACPI_6_3_PPTT_STRUCTURE_CACHE L2Cache = 
SBSAQEMU_ACPI_PPTT_L2_CACHE_STRUCT;
+
+  EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR Cluster = 
SBSAQEMU_ACPI_PPTT_CLUSTER_STRUCT;
+  EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR Core = SBSAQEMU_ACPI_PPTT_CORE_STRUCT;
+
+  EFI_ACPI_DESCRIPTION_HEADER Header =
+SBSAQEMU_ACPI_HEADER (
+  EFI_ACPI_6_3_PROCESSOR_PROPERTIES_TOPOLOGY_TABLE_STRUCTURE_SIGNATURE,
+  EFI_ACPI_DESCRIPTION_HEADER,
+  EFI_ACPI_6_3_PROCESSOR_PROPERTIES_TOPOLOGY_TABLE_REVISION);
+
+  TableSize = sizeof (EFI_ACPI_DESCRIPTION_HEADER) +
+sizeof (EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR) +
+(sizeof (EFI_ACPI_6_3_PPTT_STRUCTURE_CACHE) * 3) +
+(sizeof (EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR) * NumCores) +
+(sizeof (UINT32) * 2 * NumCores);
+
+  Status = gBS->AllocatePages (
+  AllocateAnyPages,
+  EfiACPIReclaimMemory,
+  EFI_SIZE_TO_PAGES (TableSize),
+  
+  );
+  if (EFI_ERROR(Status)) {
+DEBUG((EFI_D_ERROR, "Failed to allocate pages for PPTT table\n"));
+return EFI_OUT_OF_RESOURCES;
+  }
+
+  New = (UINT8 *)(UINTN) PageAddress;
+  ZeroMem (New, TableSize);
+
+  // Add the ACPI Description table header
+  CopyMem (New, , sizeof (EFI_ACPI_DESCRIPTION_HEADER));
+  ((EFI_ACPI_DESCRIPTION_HEADER*) New)->Length = TableSize;
+  New += sizeof (EFI_ACPI_DESCRIPTION_HEADER);
+
+  // Add the Cluster PPTT structure
+  CopyMem (New, , sizeof (EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR));
+  New += sizeof (EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR);
+
+  // Add L1 D Cache structure
+  CopyMem (New, , sizeof (EFI_ACPI_6_3_PPTT_STRUCTURE_CACHE));
+  ((EFI_ACPI_6_3_PPTT_STRUCTURE_CACHE*) New)->NextLevelOfCache = 
L2_CACHE_INDEX;
+  New += sizeof (EFI_ACPI_6_3_PPTT_STRUCTURE_CACHE);
+
+  // Add L1 I Cache structure
+  CopyMem (New, , sizeof (EFI_ACPI_6_3_PPTT_STRUCTURE_CACHE));
+  ((EFI_ACPI_6_3_PPTT_STRUCTURE_CACHE*) New)->NextLevelOfCache = 
L2_CACHE_INDEX;
+  New += sizeof (EFI_ACPI_6_3_PPTT_STRUCTURE_CACHE);
+
+  // Add L2 Cache structure
+  CopyMem (New, , sizeof (EFI_ACPI_6_3_PPTT_STRUCTURE_CACHE));
+  ((EFI_ACPI_6_3_PPTT_STRUCTURE_CACHE*) New)->NextLevelOfCache = 0; /* L2 is 
LLC */
+  New += sizeof (EFI_ACPI_6_3_PPTT_STRUCTURE_CACHE);
+
+  for (CpuId = 0; CpuId < NumCores; CpuId++) {
+EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR *CorePtr;
+UINT32*PrivateResourcePtr;
+
+CopyMem (New, , sizeof (EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR));
+CorePtr = (EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR *) New;
+CorePtr->Parent = CLUSTER_INDEX;
+CorePtr->AcpiProcessorId = CpuId;
+New += sizeof (EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR);
+
+PrivateResourcePtr = (UINT32 *) New;
+PrivateResourcePtr[0] = L1_D_CACHE_INDEX;
+PrivateResourcePtr[1] = L1_I_CACHE_INDEX;
+New += (2 * sizeof (UINT32));
+  }
+
+  // Perform Checksum
+  AcpiPlatformChecksum ((UINT8*) PageAddress, TableSize);
+
+  Status = AcpiTable->InstallAcpiTable (
+AcpiTable,
+(EFI_ACPI_COMMON_HEADER *)PageAddress,
+TableSize,
+
+);
+  if (EFI_ERROR(Status)) {
+DEBUG((EFI_D_ERROR, "Failed to install PPTT 

[edk2-devel] [PATCH edk2-platforms 7/7] SbsaQemu: AcpiTables: Add DBG2 Table

[Tanmay Jagdale]

Signed-off-by: Tanmay Jagdale 
---
 .../Qemu/SbsaQemu/AcpiTables/AcpiTables.inf   |  1 +
 Silicon/Qemu/SbsaQemu/AcpiTables/Dbg2.aslc| 68 +++
 2 files changed, 69 insertions(+)
 create mode 100644 Silicon/Qemu/SbsaQemu/AcpiTables/Dbg2.aslc

diff --git a/Silicon/Qemu/SbsaQemu/AcpiTables/AcpiTables.inf 
b/Silicon/Qemu/SbsaQemu/AcpiTables/AcpiTables.inf
index 0b5017ce81c5..cf6628c9e491 100644
--- a/Silicon/Qemu/SbsaQemu/AcpiTables/AcpiTables.inf
+++ b/Silicon/Qemu/SbsaQemu/AcpiTables/AcpiTables.inf
@@ -17,6 +17,7 @@ [Defines]
 
 [Sources]
   Dsdt.asl
+  Dbg2.aslc
   Fadt.aslc
   Gtdt.aslc
   Spcr.aslc
diff --git a/Silicon/Qemu/SbsaQemu/AcpiTables/Dbg2.aslc 
b/Silicon/Qemu/SbsaQemu/AcpiTables/Dbg2.aslc
new file mode 100644
index ..801b05b59a42
--- /dev/null
+++ b/Silicon/Qemu/SbsaQemu/AcpiTables/Dbg2.aslc
@@ -0,0 +1,68 @@
+/** @file
+*  Debug Port Table (DBG2)
+*
+*  Copyright (c) 2020 Linaro Ltd. All rights reserved.
+*
+*  SPDX-License-Identifier: BSD-2-Clause-Patent
+*
+**/
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#pragma pack(1)
+
+#define SBSAQEMU_UART_STR { '\\', '_', 'S', 'B', '.', 'C', 'O', 'M', '0', 0x00 
}
+
+typedef struct {
+  EFI_ACPI_DBG2_DEBUG_DEVICE_INFORMATION_STRUCT Dbg2Device;
+  EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTUREBaseAddressRegister;
+  UINT32AddressSize;
+  UINT8 NameSpaceString[10];
+} DBG2_DEBUG_DEVICE_INFORMATION;
+
+typedef struct {
+  EFI_ACPI_DEBUG_PORT_2_DESCRIPTION_TABLE   Description;
+  DBG2_DEBUG_DEVICE_INFORMATION Dbg2DeviceInfo;
+} DBG2_TABLE;
+
+
+STATIC DBG2_TABLE Dbg2 = {
+  {
+SBSAQEMU_ACPI_HEADER (
+  EFI_ACPI_6_3_DEBUG_PORT_2_TABLE_SIGNATURE,
+  DBG2_TABLE,
+  EFI_ACPI_DBG2_DEBUG_DEVICE_INFORMATION_STRUCT_REVISION
+),
+OFFSET_OF (DBG2_TABLE, Dbg2DeviceInfo),
+1  /* NumberOfDebugPorts */
+  },
+  {
+{
+  EFI_ACPI_DBG2_DEBUG_DEVICE_INFORMATION_STRUCT_REVISION,
+  sizeof (DBG2_DEBUG_DEVICE_INFORMATION),
+  1,   /* NumberofGenericAddressRegisters 
*/
+  10,  /* NameSpaceStringLength */
+  OFFSET_OF (DBG2_DEBUG_DEVICE_INFORMATION, NameSpaceString),
+  0,   /* OemDataLength */
+  0,   /* OemDataOffset */
+  EFI_ACPI_DBG2_PORT_TYPE_SERIAL,
+  EFI_ACPI_DBG2_PORT_SUBTYPE_SERIAL_ARM_PL011_UART,
+  {EFI_ACPI_RESERVED_BYTE, EFI_ACPI_RESERVED_BYTE},
+  OFFSET_OF (DBG2_DEBUG_DEVICE_INFORMATION, BaseAddressRegister),
+  OFFSET_OF (DBG2_DEBUG_DEVICE_INFORMATION, AddressSize)
+},
+ARM_GAS32 (SBSAQEMU_UART0_BASE), /* BaseAddressRegister */
+0x1000,  /* AddressSize */
+SBSAQEMU_UART_STR,   /* NameSpaceString */
+  }
+};
+
+#pragma pack()
+
+// Reference the table being generated to prevent the optimizer from removing
+// the data structure from the executable
+VOID* CONST ReferenceAcpiTable = 
-- 
2.28.0


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#64441): https://edk2.groups.io/g/devel/message/64441
Mute This Topic: https://groups.io/mt/76287472/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH edk2-platforms 2/7] SbsaQemu: AcpiTables: Add PCI support and MCFG Table

[Tanmay Jagdale]

Add PCI related entries to DSDT table along with the routing
entries. Also add the MCFG table.

Co-authored-by: Graeme Gregory 
Signed-off-by: Tanmay Jagdale 
---
 .../Qemu/SbsaQemu/AcpiTables/AcpiTables.inf   |   1 +
 Silicon/Qemu/SbsaQemu/AcpiTables/Dsdt.asl | 316 ++
 Silicon/Qemu/SbsaQemu/AcpiTables/Mcfg.aslc|  43 +++
 3 files changed, 360 insertions(+)
 create mode 100644 Silicon/Qemu/SbsaQemu/AcpiTables/Mcfg.aslc

diff --git a/Silicon/Qemu/SbsaQemu/AcpiTables/AcpiTables.inf 
b/Silicon/Qemu/SbsaQemu/AcpiTables/AcpiTables.inf
index ee524895524e..0b5017ce81c5 100644
--- a/Silicon/Qemu/SbsaQemu/AcpiTables/AcpiTables.inf
+++ b/Silicon/Qemu/SbsaQemu/AcpiTables/AcpiTables.inf
@@ -20,6 +20,7 @@ [Sources]
   Fadt.aslc
   Gtdt.aslc
   Spcr.aslc
+  Mcfg.aslc
 
 [Packages]
   ArmPlatformPkg/ArmPlatformPkg.dec
diff --git a/Silicon/Qemu/SbsaQemu/AcpiTables/Dsdt.asl 
b/Silicon/Qemu/SbsaQemu/AcpiTables/Dsdt.asl
index 85339d4559d3..2d3d4a2ddedc 100644
--- a/Silicon/Qemu/SbsaQemu/AcpiTables/Dsdt.asl
+++ b/Silicon/Qemu/SbsaQemu/AcpiTables/Dsdt.asl
@@ -8,6 +8,23 @@
 
 #include 
 
+#define LINK_DEVICE(Uid, LinkName, Irq)
\
+Device (LinkName) {
\
+Name (_HID, EISAID("PNP0C0F")) 
\
+Name (_UID, Uid)   
\
+Name (_PRS, ResourceTemplate() {   
\
+Interrupt (ResourceProducer, Level, ActiveHigh, Exclusive) { 
Irq } \
+}) 
\
+Method (_CRS, 0) { Return (_PRS) } 
\
+Method (_SRS, 1) { }   
\
+Method (_DIS) { }  
\
+}
+
+#define PRT_ENTRY(Address, Pin, Link)  
\
+Package (4) {  
\
+Address, Pin, Link, Zero   
\
+  }
+
 DefinitionBlock ("DsdtTable.aml", "DSDT", 1, "LINARO", "SBSAQEMU",
  FixedPcdGet32 (PcdAcpiDefaultOemRevision)) {
   Scope (_SB) {
@@ -129,5 +146,304 @@ DefinitionBlock ("DsdtTable.aml", "DSDT", 1, "LINARO", 
"SBSAQEMU",
 } // USB0_RHUB
 } // USB0
 
+Device (PCI0)
+{
+  Name (_HID, EISAID ("PNP0A08")) // PCI Express Root Bridge
+  Name (_CID, EISAID ("PNP0A03")) // Compatible PCI Root Bridge
+  Name (_SEG, Zero) // PCI Segment Group number
+  Name (_BBN, Zero) // PCI Base Bus Number
+  Name (_ADR, Zero)
+  Name (_UID, "PCI0")
+  Name (_CCA, One)// Initially mark the PCI coherent (for JunoR1)
+
+  Method (_CBA, 0, NotSerialized) {
+  return (0xf000)
+  }
+
+  LINK_DEVICE(0, GSI0, 0x23)
+  LINK_DEVICE(1, GSI1, 0x24)
+  LINK_DEVICE(2, GSI2, 0x25)
+  LINK_DEVICE(3, GSI3, 0x26)
+
+  Name (_PRT, Package ()  // _PRT: PCI Routing Table
+  {
+PRT_ENTRY(0x, 0, GSI0),
+PRT_ENTRY(0x, 0, GSI1),
+PRT_ENTRY(0x, 0, GSI2),
+PRT_ENTRY(0x, 0, GSI3),
+
+PRT_ENTRY(0x0001, 0, GSI1),
+PRT_ENTRY(0x0001, 1, GSI2),
+PRT_ENTRY(0x0001, 2, GSI3),
+PRT_ENTRY(0x0001, 3, GSI0),
+
+PRT_ENTRY(0x0002, 0, GSI2),
+PRT_ENTRY(0x0002, 1, GSI3),
+PRT_ENTRY(0x0002, 2, GSI0),
+PRT_ENTRY(0x0002, 3, GSI1),
+
+PRT_ENTRY(0x0003, 0, GSI3),
+PRT_ENTRY(0x0003, 1, GSI0),
+PRT_ENTRY(0x0003, 2, GSI1),
+PRT_ENTRY(0x0003, 3, GSI2),
+
+PRT_ENTRY(0x0004, 0, GSI0),
+PRT_ENTRY(0x0004, 1, GSI1),
+PRT_ENTRY(0x0004, 2, GSI2),
+PRT_ENTRY(0x0004, 3, GSI3),
+
+PRT_ENTRY(0x0005, 0, GSI1),
+PRT_ENTRY(0x0005, 1, GSI2),
+PRT_ENTRY(0x0005, 2, GSI3),
+PRT_ENTRY(0x0005, 3, GSI0),
+
+PRT_ENTRY(0x0006, 0, GSI2),
+PRT_ENTRY(0x0006, 1, GSI3),
+PRT_ENTRY(0x0006, 2, GSI0),
+PRT_ENTRY(0x0006, 3, GSI1),
+
+PRT_ENTRY(0x0007, 0, GSI3),
+PRT_ENTRY(0x0007, 1, GSI0),
+PRT_ENTRY(0x0007, 2, GSI1),
+PRT_ENTRY(0x0007, 3, GSI2),
+
+PRT_ENTRY(0x0008, 0, GSI0),
+PRT_ENTRY(0x0008, 1, GSI1),
+PRT_ENTRY(0x0008, 2, GSI2),
+PRT_ENTRY(0x0008, 3, GSI3),
+
+PRT_ENTRY(0x0009, 0, GSI1),
+PRT_ENTRY(0x0009, 1, GSI2),
+PRT_ENTRY(0x0009, 2, GSI3),
+PRT_ENTRY(0x0009, 3, GSI0),
+
+PRT_ENTRY(0x000A, 0, GSI2),
+PRT_ENTRY(0x000A, 1, GSI3),
+PRT_ENTRY(0x000A, 2, GSI0),
+PRT_ENTRY(0x000A, 3, GSI1),

[edk2-devel] [PATCH edk2-platforms 4/7] SbsaQemu: AcpiDxe: Create MADT table at runtime

[Tanmay Jagdale]

- Add support to create MADT table at runtime.
- Included a macro for GIC Redistributor structure initialisation.

Signed-off-by: Tanmay Jagdale 
---
 .../Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c | 159 ++
 .../SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf   |  20 ++-
 .../Include/IndustryStandard/SbsaQemuAcpi.h   |  15 ++
 3 files changed, 193 insertions(+), 1 deletion(-)

diff --git a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c 
b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
index 09e5ba432a59..569cda8b6474 100644
--- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
+++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
@@ -6,11 +6,19 @@
 *  SPDX-License-Identifier: BSD-2-Clause-Patent
 *
 **/
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
 #include 
+#include 
 #include 
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -62,6 +70,138 @@ CountCpusFromFdt (
   ASSERT_RETURN_ERROR (PcdStatus);
 }
 
+/*
+ * A Function to Compute the ACPI Table Checksum
+ */
+VOID
+AcpiPlatformChecksum (
+  IN UINT8  *Buffer,
+  IN UINTN  Size
+  )
+{
+  UINTN ChecksumOffset;
+
+  ChecksumOffset = OFFSET_OF (EFI_ACPI_DESCRIPTION_HEADER, Checksum);
+
+  // Set checksum to 0 first
+  Buffer[ChecksumOffset] = 0;
+
+  // Update checksum value
+  Buffer[ChecksumOffset] = CalculateCheckSum8(Buffer, Size);
+}
+
+/*
+ * A function that add the MADT ACPI table.
+  IN EFI_ACPI_COMMON_HEADER*CurrentTable
+ */
+EFI_STATUS
+AddMadtTable (
+  IN EFI_ACPI_TABLE_PROTOCOL   *AcpiTable
+  )
+{
+  EFI_STATUSStatus;
+  UINTN TableHandle;
+  UINT32TableSize;
+  EFI_PHYSICAL_ADDRESS  PageAddress;
+  UINT8 *New;
+  UINT32NumCores;
+
+  // Initialize MADT ACPI Header
+  EFI_ACPI_6_0_MULTIPLE_APIC_DESCRIPTION_TABLE_HEADER Header = {
+ SBSAQEMU_ACPI_HEADER 
(EFI_ACPI_6_0_MULTIPLE_APIC_DESCRIPTION_TABLE_SIGNATURE,
+   EFI_ACPI_6_0_MULTIPLE_APIC_DESCRIPTION_TABLE_HEADER,
+   
EFI_ACPI_6_0_MULTIPLE_APIC_DESCRIPTION_TABLE_REVISION),
+  0, 0 };
+
+  // Initialize GICC Structure
+  EFI_ACPI_6_0_GIC_STRUCTURE Gicc = EFI_ACPI_6_0_GICC_STRUCTURE_INIT (
+0, /* GicID */
+0, /* AcpiCpuUid */
+0, /* Mpidr */
+EFI_ACPI_6_0_GIC_ENABLED,  /* Flags */
+SBSAQEMU_MADT_GIC_PMU_IRQ, /* PMU Irq */
+FixedPcdGet32 (PcdGicDistributorBase), /* PhysicalBaseAddress */
+SBSAQEMU_MADT_GIC_VBASE,   /* GicVBase */
+SBSAQEMU_MADT_GIC_HBASE,   /* GicHBase */
+25,/* GsivId */
+0, /* GicRBase */
+0  /* Efficiency */
+);
+
+  // Initialize GIC Distributor Structure
+  EFI_ACPI_6_0_GIC_DISTRIBUTOR_STRUCTURE Gicd =
+EFI_ACPI_6_0_GIC_DISTRIBUTOR_INIT (
+  0,
+  FixedPcdGet32 (PcdGicDistributorBase),
+  0,
+  3 /* GicVersion */
+);
+
+ // Initialize GIC Redistributor Structure
+  EFI_ACPI_6_0_GICR_STRUCTURE Gicr = SBSAQEMU_MADT_GICR_INIT();
+
+  // Get CoreCount which was determined eariler after parsing device tree
+  NumCores = PcdGet32 (PcdCoreCount);
+
+  // Calculate the new table size based on the number of cores
+  TableSize = sizeof (EFI_ACPI_6_0_MULTIPLE_APIC_DESCRIPTION_TABLE_HEADER) +
+   (sizeof (EFI_ACPI_6_0_GIC_STRUCTURE) * NumCores) +
+   sizeof (EFI_ACPI_6_0_GIC_DISTRIBUTOR_STRUCTURE) +
+   sizeof (EFI_ACPI_6_0_GICR_STRUCTURE);
+
+  Status = gBS->AllocatePages (
+  AllocateAnyPages,
+  EfiACPIReclaimMemory,
+  EFI_SIZE_TO_PAGES (TableSize),
+  
+  );
+  if (EFI_ERROR(Status)) {
+DEBUG ((EFI_D_ERROR, "Failed to allocate pages for MADT table\n"));
+return EFI_OUT_OF_RESOURCES;
+  }
+
+  New = (UINT8 *)(UINTN) PageAddress;
+  ZeroMem (New, TableSize);
+
+  // Add the  ACPI Description table header
+  CopyMem (New, , sizeof 
(EFI_ACPI_6_0_MULTIPLE_APIC_DESCRIPTION_TABLE_HEADER));
+  ((EFI_ACPI_DESCRIPTION_HEADER*) New)->Length = TableSize;
+  New += sizeof (EFI_ACPI_6_0_MULTIPLE_APIC_DESCRIPTION_TABLE_HEADER);
+
+  // Add new GICC structures for the Cores
+  for (NumCores = 0; NumCores < PcdGet32 (PcdCoreCount); NumCores++) {
+EFI_ACPI_6_0_GIC_STRUCTURE *GiccPtr;
+
+CopyMem (New, , sizeof (EFI_ACPI_6_0_GIC_STRUCTURE));
+GiccPtr = (EFI_ACPI_6_0_GIC_STRUCTURE *) New;
+GiccPtr->AcpiProcessorUid = NumCores;
+GiccPtr->MPIDR = NumCores;
+New += sizeof (EFI_ACPI_6_0_GIC_STRUCTURE);
+  }
+
+  // GIC Distributor Structure
+  CopyMem (New, , sizeof (EFI_ACPI_6_0_GIC_DISTRIBUTOR_STRUCTURE));
+  New += sizeof (EFI_ACPI_6_0_GIC_DISTRIBUTOR_STRUCTURE);
+
+  

Re: [edk2-devel] [edk2-announce] Re: Soft Feature Freeze starts now for edk2-stable202008

[Laszlo Ersek]

On 08/19/20 15:34, Chang, Abner (HPS SW/FW Technologist) wrote:
> 
> 
>> -Original Message-
>> From: Laszlo Ersek [mailto:ler...@redhat.com]
>> Sent: Wednesday, August 19, 2020 9:19 PM
>> To: Leif Lindholm ; Chang, Abner (HPS SW/FW
>> Technologist) 
>> Cc: devel@edk2.groups.io; liming.gao ;
>> annou...@edk2.groups.io; af...@apple.com; Kinney, Michael D
>> 
>> Subject: Re: [edk2-announce] Re: Soft Feature Freeze starts now for edk2-
>> stable202008
>>
>> On 08/19/20 13:48, Leif Lindholm wrote:
>>> (Slightly trimmed recipient list due to different patch being
>>> discussed.)
>>>
>>> So, I can't make this call, because I'm the one who messed up.
>>>
>>> This patch does exactly what I had requested Abner to do some time
>>> back (off-list, unfortunately), and I was *convinced* I gave it an R-b
>>> as soon as it hit my inbox - until Abner nudged me about it yesterday.
>>>
>>> The patch in question is
>>> https://edk2.groups.io/g/devel/topic/76021725
>>
>> My understanding is:
>>
>> (1) there is an external project that consumes the FDT library in
>> EmbeddedPkg, meaning the lib class header
>> "EmbeddedPkg/Include/libfdt.h"
>> and the lib instance "EmbeddedPkg/Library/FdtLib/FdtLib.inf",
> [Chang, Abner] Yes
>>
>> (2) the lib class header pulls in "fdt.h" and "libfdt_env.h",
> [Chang, Abner] yes
>>
>> (3) the external project is not edk2-platforms,
> [Chang, Abner] yes
>>
>> (4) the external project wants -- for some strange reason -- edk2's
>> "libfdt_env.h" to provide an strncmp() function (or function-like macro), 
>> with
>> that particular stncmp() implementation not being needed in either edk2-
>> platforms or edk2 itself,
> [Chang, Abner] yes, at least so far
>>
>> (5) the patch for adding said strncmp() was posted on Aug 6th (at least when
>> viewed from my time zone), i.e., before the SFF,
> [Chang, Abner] Yes
>>
>> (6) it was reviewed 12 days later (within the SFF)
> [Chang, Abner] yes.
>>
>> If my understanding is correct, then I don't see how this patch could be
>> considered a bugfix -- even as a feature addition, it seems hardly justified 
>> to
>> me --, and there would have been ~8 days before the SFF to review it.
>>
>> I think we should postpone the patch until after the stable tag.
> This patch is important because the edk2-stable202008 would be the stable tag 
> (if this patch is accepted) for booting RISC-V platform to Linux kernel with 
> EFI Runtime service on either real platform and QEMU. We can publish this 
> information in RISC-V community which is considered as a valuable milestone 
> for RISC-V edk2 port.

Let's move out the dates for the stable tag then, by one week:

- let the SFF start on 2020-08-21
- let the HFF start on 2020-08-28
- let's release edk2-stable202008 on 2020-09-04

Release slips are permitted and there have been examples.

What doesn't make sense is making rules and then breaking them
opportunistically, whenever they're uncomfortable. If that's a frequent
occurrence, we should pick different rules, or -- again -- if this is a
very important patch, we should delay the release for it.

BTW what about reverting the OpenSBI change? You could still call
sbi_strncmp() -- rather than strncmp() -- in the "helper" lib.

Thanks
Laszlo


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#64433): https://edk2.groups.io/g/devel/message/64433
Mute This Topic: https://groups.io/mt/76284301/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH edk2-platforms 3/7] SbsaQemu: Add new ACPI driver and FDT parser to count CPUs

[Tanmay Jagdale]

- Add a new ACPI driver for the SbsaQemu platform which would
  handle any modifications needed for the ACPI tables.

- Move FdtLib from LibraryClasses.common.PEIM to LibraryClasses.common
  so that SbsaQemuAcpiDxe driver can use the device tree APIs

- Since the core count is controlled by Qemu, move the PcdCoreCount
  from PcdsFixedAtBuild to PcdsDynamic.

- Add a parser function in this driver which parses the FDT created
  by Qemu to determine the number of CPUs and hence update the
  PcdCoreCount variable.

Signed-off-by: Tanmay Jagdale 
---
 Platform/Qemu/SbsaQemu/SbsaQemu.dsc   |  6 +-
 Platform/Qemu/SbsaQemu/SbsaQemu.fdf   |  1 +
 Silicon/Qemu/SbsaQemu/Acpi.dsc.inc|  1 +
 .../Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c | 76 +++
 .../SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf   | 49 
 Silicon/Qemu/SbsaQemu/SbsaQemu.dec|  4 +
 6 files changed, 135 insertions(+), 2 deletions(-)
 create mode 100644 
Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
 create mode 100644 
Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf

diff --git a/Platform/Qemu/SbsaQemu/SbsaQemu.dsc 
b/Platform/Qemu/SbsaQemu/SbsaQemu.dsc
index 4739443cae93..d42b9cd4de49 100644
--- a/Platform/Qemu/SbsaQemu/SbsaQemu.dsc
+++ b/Platform/Qemu/SbsaQemu/SbsaQemu.dsc
@@ -75,6 +75,7 @@ [LibraryClasses.common]
   ShellLib|ShellPkg/Library/UefiShellLib/UefiShellLib.inf
   FileHandleLib|MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.inf
 
+  FdtLib|EmbeddedPkg/Library/FdtLib/FdtLib.inf
   UefiRuntimeLib|MdePkg/Library/UefiRuntimeLib/UefiRuntimeLib.inf
   
OrderedCollectionLib|MdePkg/Library/BaseOrderedCollectionRedBlackTreeLib/BaseOrderedCollectionRedBlackTreeLib.inf
 
@@ -217,7 +218,6 @@ [LibraryClasses.common.PEIM]
 
   
PeiServicesTablePointerLib|ArmPkg/Library/PeiServicesTablePointerLib/PeiServicesTablePointerLib.inf
 
-  FdtLib|EmbeddedPkg/Library/FdtLib/FdtLib.inf
   ArmPlatformLib|Silicon/Qemu/SbsaQemu/Library/SbsaQemuLib/SbsaQemuLib.inf
 
 [LibraryClasses.common.DXE_CORE]
@@ -376,7 +376,6 @@ [PcdsFixedAtBuild.common]
   #
   gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|TRUE
 
-  gArmPlatformTokenSpaceGuid.PcdCoreCount|1
   gArmTokenSpaceGuid.PcdVFPEnabled|1
 
   # System Memory Base -- fixed
@@ -477,6 +476,9 @@ [PcdsFixedAtBuild.common]
 [PcdsDynamicDefault.common]
   gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|3
 
+  # Core and Cluster Count
+  gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdCoreCount|1
+  gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdClusterCount|1
 
   # System Memory Size -- 128 MB initially, actual size will be fetched from DT
   # TODO as no DT will be used we should pass this by some other method
diff --git a/Platform/Qemu/SbsaQemu/SbsaQemu.fdf 
b/Platform/Qemu/SbsaQemu/SbsaQemu.fdf
index 4526eaaa02c5..3bcf0bf0040a 100644
--- a/Platform/Qemu/SbsaQemu/SbsaQemu.fdf
+++ b/Platform/Qemu/SbsaQemu/SbsaQemu.fdf
@@ -232,6 +232,7 @@ [FV.FvMain]
   #
   INF MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableDxe.inf
   INF MdeModulePkg/Universal/Acpi/AcpiPlatformDxe/AcpiPlatformDxe.inf
+  INF Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf
   INF RuleOverride = ACPITABLE Silicon/Qemu/SbsaQemu/AcpiTables/AcpiTables.inf
   INF 
MdeModulePkg/Universal/Acpi/BootGraphicsResourceTableDxe/BootGraphicsResourceTableDxe.inf
 
diff --git a/Silicon/Qemu/SbsaQemu/Acpi.dsc.inc 
b/Silicon/Qemu/SbsaQemu/Acpi.dsc.inc
index c4a8d7a27b78..593670383750 100644
--- a/Silicon/Qemu/SbsaQemu/Acpi.dsc.inc
+++ b/Silicon/Qemu/SbsaQemu/Acpi.dsc.inc
@@ -33,3 +33,4 @@ [Components.common]
   MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableDxe.inf
   MdeModulePkg/Universal/Acpi/AcpiPlatformDxe/AcpiPlatformDxe.inf
   Silicon/Qemu/SbsaQemu/AcpiTables/AcpiTables.inf
+  Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf
diff --git a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c 
b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
new file mode 100644
index ..09e5ba432a59
--- /dev/null
+++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
@@ -0,0 +1,76 @@
+/** @file
+*  This file is an ACPI driver for the Qemu SBSA platform.
+*
+*  Copyright (c) 2020, Linaro Ltd. All rights reserved.
+*
+*  SPDX-License-Identifier: BSD-2-Clause-Patent
+*
+**/
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/*
+ * A function that walks through the Device Tree created
+ * by Qemu and counts the number of CPUs present in it.
+ */
+STATIC
+VOID
+CountCpusFromFdt (
+  VOID
+)
+{
+  VOID   *DeviceTreeBase;
+  INT32  Node, Prev;
+  RETURN_STATUS  PcdStatus;
+  INT32  CpuNode;
+  INT32  CpuCount;
+
+  DeviceTreeBase = (VOID *)(UINTN)PcdGet64 (PcdDeviceTreeBaseAddress);
+  ASSERT (DeviceTreeBase != NULL);
+
+  // Make sure we have a valid device tree blob
+  ASSERT (fdt_check_header (DeviceTreeBase) == 0);
+
+  CpuNode = fdt_path_offset 

[edk2-devel] [PATCH edk2-platforms 0/7] Add ACPI tables support for SbsaQemu

[Tanmay Jagdale]

This patch series adds ACPI tables support for the SbsaQemu platform.
We are using a pseudo static approach to create the ACPI tables.

The ACPI tables namely DBG2, DSDT, MCFG, SPCR, GTDT are created in a
static way at compile time because they hold a fixed configuration
and there are no changes at runtime.

The MADT, SSDT and PPTT tables are dependant on the number of CPUs and
hence they are created at runtime based on the number of CPUs the user
has requested.

Tanmay Jagdale (7):
  SbsaQemu: Initial support for static ACPI tables
  SbsaQemu: AcpiTables: Add PCI support and MCFG Table
  SbsaQemu: Add new ACPI driver and FDT parser to count CPUs
  SbsaQemu: AcpiDxe: Create MADT table at runtime
  SbsaQemu: AcpiDxe: Create SSDT table at runtime
  SbsaQemu: AcpiDxe: Create PPTT table at runtime
  SbsaQemu: AcpiTables: Add DBG2 Table

 Platform/Qemu/SbsaQemu/SbsaQemu.dsc   |  12 +-
 Platform/Qemu/SbsaQemu/SbsaQemu.fdf   |   9 +
 Silicon/Qemu/SbsaQemu/Acpi.dsc.inc|  36 ++
 .../Qemu/SbsaQemu/AcpiTables/AcpiTables.inf   |  47 ++
 Silicon/Qemu/SbsaQemu/AcpiTables/Dbg2.aslc|  68 +++
 Silicon/Qemu/SbsaQemu/AcpiTables/Dsdt.asl | 449 
 Silicon/Qemu/SbsaQemu/AcpiTables/Fadt.aslc|  80 +++
 Silicon/Qemu/SbsaQemu/AcpiTables/Gtdt.aslc|  67 +++
 Silicon/Qemu/SbsaQemu/AcpiTables/Mcfg.aslc|  43 ++
 Silicon/Qemu/SbsaQemu/AcpiTables/Spcr.aslc|  53 ++
 .../Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c | 490 ++
 .../SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf   |  67 +++
 .../Include/IndustryStandard/SbsaQemuAcpi.h   | 199 +++
 Silicon/Qemu/SbsaQemu/SbsaQemu.dec|   8 +-
 14 files changed, 1624 insertions(+), 4 deletions(-)
 create mode 100644 Silicon/Qemu/SbsaQemu/Acpi.dsc.inc
 create mode 100644 Silicon/Qemu/SbsaQemu/AcpiTables/AcpiTables.inf
 create mode 100644 Silicon/Qemu/SbsaQemu/AcpiTables/Dbg2.aslc
 create mode 100644 Silicon/Qemu/SbsaQemu/AcpiTables/Dsdt.asl
 create mode 100644 Silicon/Qemu/SbsaQemu/AcpiTables/Fadt.aslc
 create mode 100644 Silicon/Qemu/SbsaQemu/AcpiTables/Gtdt.aslc
 create mode 100644 Silicon/Qemu/SbsaQemu/AcpiTables/Mcfg.aslc
 create mode 100644 Silicon/Qemu/SbsaQemu/AcpiTables/Spcr.aslc
 create mode 100644 
Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
 create mode 100644 
Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf
 create mode 100644 
Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuAcpi.h

-- 
2.28.0


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#64434): https://edk2.groups.io/g/devel/message/64434
Mute This Topic: https://groups.io/mt/76287421/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH edk2-platforms 1/7] SbsaQemu: Initial support for static ACPI tables

[Tanmay Jagdale]

- Add the following ACPI tables for SbsaQemu platform
DSDT, FADT, GTDT, SPCR
- Created an Include directory to hold common header files.
- Also included the Acpiview shell utility.

Co-authored-by: Graeme Gregory 
Co-authored-by: Jonathan Cameron 
Signed-off-by: Tanmay Jagdale 
---
 Platform/Qemu/SbsaQemu/SbsaQemu.dsc   |   6 +
 Platform/Qemu/SbsaQemu/SbsaQemu.fdf   |   8 ++
 Silicon/Qemu/SbsaQemu/Acpi.dsc.inc|  35 +
 .../Qemu/SbsaQemu/AcpiTables/AcpiTables.inf   |  45 ++
 Silicon/Qemu/SbsaQemu/AcpiTables/Dsdt.asl | 133 ++
 Silicon/Qemu/SbsaQemu/AcpiTables/Fadt.aslc|  80 +++
 Silicon/Qemu/SbsaQemu/AcpiTables/Gtdt.aslc|  67 +
 Silicon/Qemu/SbsaQemu/AcpiTables/Spcr.aslc|  53 +++
 .../Include/IndustryStandard/SbsaQemuAcpi.h   |  31 
 Silicon/Qemu/SbsaQemu/SbsaQemu.dec|   4 +-
 10 files changed, 460 insertions(+), 2 deletions(-)
 create mode 100644 Silicon/Qemu/SbsaQemu/Acpi.dsc.inc
 create mode 100644 Silicon/Qemu/SbsaQemu/AcpiTables/AcpiTables.inf
 create mode 100644 Silicon/Qemu/SbsaQemu/AcpiTables/Dsdt.asl
 create mode 100644 Silicon/Qemu/SbsaQemu/AcpiTables/Fadt.aslc
 create mode 100644 Silicon/Qemu/SbsaQemu/AcpiTables/Gtdt.aslc
 create mode 100644 Silicon/Qemu/SbsaQemu/AcpiTables/Spcr.aslc
 create mode 100644 
Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuAcpi.h

diff --git a/Platform/Qemu/SbsaQemu/SbsaQemu.dsc 
b/Platform/Qemu/SbsaQemu/SbsaQemu.dsc
index 4db3ab465163..4739443cae93 100644
--- a/Platform/Qemu/SbsaQemu/SbsaQemu.dsc
+++ b/Platform/Qemu/SbsaQemu/SbsaQemu.dsc
@@ -516,6 +516,7 @@ [Components.common]
   ShellPkg/Application/Shell/Shell.inf {
 
   
ShellCommandLib|ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.inf
+  
NULL|ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.inf
   
NULL|ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2CommandsLib.inf
   
NULL|ShellPkg/Library/UefiShellLevel1CommandsLib/UefiShellLevel1CommandsLib.inf
   
NULL|ShellPkg/Library/UefiShellLevel3CommandsLib/UefiShellLevel3CommandsLib.inf
@@ -675,3 +676,8 @@ [Components.common]
   MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBusDxe.inf
   MdeModulePkg/Bus/Usb/UsbKbDxe/UsbKbDxe.inf
   MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassStorageDxe.inf
+
+  #
+  # ACPI Support
+!include Silicon/Qemu/SbsaQemu/Acpi.dsc.inc
+  
MdeModulePkg/Universal/Acpi/BootGraphicsResourceTableDxe/BootGraphicsResourceTableDxe.inf
diff --git a/Platform/Qemu/SbsaQemu/SbsaQemu.fdf 
b/Platform/Qemu/SbsaQemu/SbsaQemu.fdf
index be7c78acebfd..4526eaaa02c5 100644
--- a/Platform/Qemu/SbsaQemu/SbsaQemu.fdf
+++ b/Platform/Qemu/SbsaQemu/SbsaQemu.fdf
@@ -227,6 +227,14 @@ [FV.FvMain]
   INF MdeModulePkg/Bus/Scsi/ScsiBusDxe/ScsiBusDxe.inf
   INF MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDiskDxe.inf
 
+  #
+  # ACPI support
+  #
+  INF MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableDxe.inf
+  INF MdeModulePkg/Universal/Acpi/AcpiPlatformDxe/AcpiPlatformDxe.inf
+  INF RuleOverride = ACPITABLE Silicon/Qemu/SbsaQemu/AcpiTables/AcpiTables.inf
+  INF 
MdeModulePkg/Universal/Acpi/BootGraphicsResourceTableDxe/BootGraphicsResourceTableDxe.inf
+
   #
   # PCI support
   #
diff --git a/Silicon/Qemu/SbsaQemu/Acpi.dsc.inc 
b/Silicon/Qemu/SbsaQemu/Acpi.dsc.inc
new file mode 100644
index ..c4a8d7a27b78
--- /dev/null
+++ b/Silicon/Qemu/SbsaQemu/Acpi.dsc.inc
@@ -0,0 +1,35 @@
+#
+#  Copyright (c) 2020, Linaro Limited. All rights reserved.
+#
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+
+
+#
+# Pcd Section - list of all EDK II PCD Entries defined by this Platform
+#
+
+
+[PcdsFeatureFlag]
+  gEfiMdeModulePkgTokenSpaceGuid.PcdInstallAcpiSdtProtocol|TRUE
+
+[PcdsFixedAtBuild.common]
+  gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultOemId|"LINARO"
+  gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultOemTableId|0x554D455141534253 
#SBSAQEMU
+  gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultOemRevision|0x20200810
+  gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultCreatorId|0x4f524e4c #LNRO
+  gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultCreatorRevision|1
+
+
+#
+# Components Section - list of all EDK II Modules needed by this Platform
+#
+
+
+[Components.common]
+  #
+  # ACPI support
+  #
+  MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableDxe.inf
+  MdeModulePkg/Universal/Acpi/AcpiPlatformDxe/AcpiPlatformDxe.inf
+  Silicon/Qemu/SbsaQemu/AcpiTables/AcpiTables.inf
diff --git a/Silicon/Qemu/SbsaQemu/AcpiTables/AcpiTables.inf 
b/Silicon/Qemu/SbsaQemu/AcpiTables/AcpiTables.inf
new file mode 100644
index ..ee524895524e
--- /dev/null
+++ 

Re: [edk2-devel] [edk2-announce] Re: Soft Feature Freeze starts now for edk2-stable202008

[Laszlo Ersek]

On 08/19/20 15:19, Laszlo Ersek wrote:
> On 08/19/20 13:48, Leif Lindholm wrote:
>> (Slightly trimmed recipient list due to different patch being discussed.)
>>
>> So, I can't make this call, because I'm the one who messed up.
>>
>> This patch does exactly what I had requested Abner to do some time
>> back (off-list, unfortunately), and I was *convinced* I gave it an R-b
>> as soon as it hit my inbox - until Abner nudged me about it yesterday.
>>
>> The patch in question is
>> https://edk2.groups.io/g/devel/topic/76021725
> 
> My understanding is:
> 
> (1) there is an external project that consumes the FDT library in
> EmbeddedPkg, meaning the lib class header "EmbeddedPkg/Include/libfdt.h"
> and the lib instance "EmbeddedPkg/Library/FdtLib/FdtLib.inf",
> 
> (2) the lib class header pulls in "fdt.h" and "libfdt_env.h",
> 
> (3) the external project is not edk2-platforms,
> 
> (4) the external project wants -- for some strange reason -- edk2's
> "libfdt_env.h" to provide an strncmp() function (or function-like
> macro), with that particular stncmp() implementation not being needed in
> either edk2-platforms or edk2 itself,
> 
> (5) the patch for adding said strncmp() was posted on Aug 6th (at least
> when viewed from my time zone), i.e., before the SFF,
> 
> (6) it was reviewed 12 days later (within the SFF)
> 
> If my understanding is correct, then I don't see how this patch could be
> considered a bugfix -- even as a feature addition, it seems hardly
> justified to me --, and there would have been ~8 days before the SFF to
> review it.
> 
> I think we should postpone the patch until after the stable tag.

Regarding OpenSBI, as of commit 9d56961b2314, the sbi_strncmp() function
has not been removed, and it seems that sbi_strncmp() is still used /
called in at least some build configurations (where the

lib/utils/libfdt/libfdt_env.h:#define strncmp   sbi_strncmp

definition is supposed to be in effect).

This means that the codebase can not rid itself of sbi_strncmp().

To me this indicates that OpenSBI commit 2cfd2fc90488 ("lib: utils: Use
strncmp in fdt_parse_hart_id()", 2020-07-29) was wrong. It should have
replaced sbi_strcmp() with sbi_strncmp(), not strncmp().

After all, the size limit seems to be the motivation for the entire
change -- put a size limit on the string comparison in
fdt_parse_hart_id(). Commit 2cfd2fc90488 did two things at the same
time: replace the size-unbounded comparison with the size-bounded one,
*and* switch to the standard C function name from the self-contained
function. The former goal looks justifiable, the latter I don't understand.

Now: I realize that, going back to edk2 commit fa4a70633868
("EmbeddedPkg: Added support to libfdt", 2012-09-27), we already have a
bunch of wrappers in "EmbeddedPkg/Include/libfdt_env.h". So I guess
adding "one more" is not inconsistent with those -- even though the lib
instance within edk2 doesn't need the new function.

But it's still a micro-feature whose review should have completed before
the SFF.

Thanks
Laszlo


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#64432): https://edk2.groups.io/g/devel/message/64432
Mute This Topic: https://groups.io/mt/76284301/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [edk2-announce] Re: Soft Feature Freeze starts now for edk2-stable202008

[Abner Chang]

> -Original Message-
> From: Laszlo Ersek [mailto:ler...@redhat.com]
> Sent: Wednesday, August 19, 2020 9:19 PM
> To: Leif Lindholm ; Chang, Abner (HPS SW/FW
> Technologist) 
> Cc: devel@edk2.groups.io; liming.gao ;
> annou...@edk2.groups.io; af...@apple.com; Kinney, Michael D
> 
> Subject: Re: [edk2-announce] Re: Soft Feature Freeze starts now for edk2-
> stable202008
> 
> On 08/19/20 13:48, Leif Lindholm wrote:
> > (Slightly trimmed recipient list due to different patch being
> > discussed.)
> >
> > So, I can't make this call, because I'm the one who messed up.
> >
> > This patch does exactly what I had requested Abner to do some time
> > back (off-list, unfortunately), and I was *convinced* I gave it an R-b
> > as soon as it hit my inbox - until Abner nudged me about it yesterday.
> >
> > The patch in question is
> > https://edk2.groups.io/g/devel/topic/76021725
> 
> My understanding is:
> 
> (1) there is an external project that consumes the FDT library in
> EmbeddedPkg, meaning the lib class header
> "EmbeddedPkg/Include/libfdt.h"
> and the lib instance "EmbeddedPkg/Library/FdtLib/FdtLib.inf",
[Chang, Abner] Yes
> 
> (2) the lib class header pulls in "fdt.h" and "libfdt_env.h",
[Chang, Abner] yes
> 
> (3) the external project is not edk2-platforms,
[Chang, Abner] yes
> 
> (4) the external project wants -- for some strange reason -- edk2's
> "libfdt_env.h" to provide an strncmp() function (or function-like macro), with
> that particular stncmp() implementation not being needed in either edk2-
> platforms or edk2 itself,
[Chang, Abner] yes, at least so far
> 
> (5) the patch for adding said strncmp() was posted on Aug 6th (at least when
> viewed from my time zone), i.e., before the SFF,
[Chang, Abner] Yes
> 
> (6) it was reviewed 12 days later (within the SFF)
[Chang, Abner] yes.
> 
> If my understanding is correct, then I don't see how this patch could be
> considered a bugfix -- even as a feature addition, it seems hardly justified 
> to
> me --, and there would have been ~8 days before the SFF to review it.
> 
> I think we should postpone the patch until after the stable tag.
This patch is important because the edk2-stable202008 would be the stable tag 
(if this patch is accepted) for booting RISC-V platform to Linux kernel with 
EFI Runtime service on either real platform and QEMU. We can publish this 
information in RISC-V community which is considered as a valuable milestone for 
RISC-V edk2 port.

> 
> Thanks
> Laszlo
> 
> 
> > On Wed, Aug 19, 2020 at 11:29:52 +, Chang, Abner (HPS SW/FW
> Technologist) wrote:
> >> Hmm.. I had a one commit (which is not the feature) just reviewed by
> >> Leif but not pushing yet. Is that possible to push before the Hard
> >> Feature freeze and also be included in 202008 stable tag? I guess it
> >> gets the chance according to the article of SoftFeatureFreeze on Wiki
> >> page.
> >>
> >> Patch attached FYR
> >>
> >> Abner
> >>
> >>> -Original Message-
> >>> From: annou...@edk2.groups.io [mailto:annou...@edk2.groups.io]
> On
> >>> Behalf Of Laszlo Ersek
> >>> Sent: Wednesday, August 19, 2020 5:59 PM
> >>> To: Bret Barkelew ;
> >>> devel@edk2.groups.io; liming.gao ;
> >>> annou...@edk2.groups.io
> >>> Cc: Leif Lindholm ; af...@apple.com; Kinney,
> >>> Michael D ; Guptha, Soumya K
> >>> 
> >>> Subject: Re: [edk2-announce] [EXTERNAL] Re: Soft Feature Freeze
> >>> starts now for edk2-stable202008
> >>>
> >>> On 08/18/20 17:10, Bret Barkelew wrote:
>  I agree with the process and withdraw my request, replacing it
>  instead with
> >>> a disapproving head shake and deep sigh.
> >>>
> >>> We as a community definitely deserve your disapproval, as our review
> >>> response times have been abysmal. :(
> >>>
>  I’ll go back to pushing on this after the tag.
> >>>
> >>> Thanks for your persistence!
> >>> Laszlo
> >>>
> >>>
> >>> 
> >>
> >
> >


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#64431): https://edk2.groups.io/g/devel/message/64431
Mute This Topic: https://groups.io/mt/76284301/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [edk2-announce] Re: Soft Feature Freeze starts now for edk2-stable202008

[Abner Chang]

...and I didn’t aware that announcement of Soft Feature freeze because 
[edk2-annnounce] never went to my inbox and end up with I missed it, I should 
set the rule for it. :(

> -Original Message-
> From: Leif Lindholm [mailto:l...@nuviainc.com]
> Sent: Wednesday, August 19, 2020 7:49 PM
> To: Chang, Abner (HPS SW/FW Technologist) 
> Cc: Laszlo Ersek ; devel@edk2.groups.io; liming.gao
> ; annou...@edk2.groups.io; af...@apple.com;
> Kinney, Michael D 
> Subject: Re: [edk2-announce] Re: Soft Feature Freeze starts now for edk2-
> stable202008
> 
> (Slightly trimmed recipient list due to different patch being discussed.)
> 
> So, I can't make this call, because I'm the one who messed up.
> 
> This patch does exactly what I had requested Abner to do some time back
> (off-list, unfortunately), and I was *convinced* I gave it an R-b as soon as 
> it
> hit my inbox - until Abner nudged me about it yesterday.
> 
> The patch in question is
> INVALID URI REMOVED
> 3A__edk2.groups.io_g_devel_topic_76021725=DwIDaQ=C5b8zRQO1mi
> GmBeVZ2LFWg=_SN6FZBN4Vgi4Ulkskz6qU3NYRO03nHp9P7Z5q59A3E=
> aIPVzq25kdyKl8jdl_OJcSuym6rNT7xSGICexWHc8UY=vVeUUfgQDUnjWzqh
> YpCiNqNHDScByiwJzFiUZ4kx4TU=
> 
> /
> Leif
> 
> On Wed, Aug 19, 2020 at 11:29:52 +, Chang, Abner (HPS SW/FW
> Technologist) wrote:
> > Hmm.. I had a one commit (which is not the feature) just reviewed by
> > Leif but not pushing yet. Is that possible to push before the Hard
> > Feature freeze and also be included in 202008 stable tag? I guess it
> > gets the chance according to the article of SoftFeatureFreeze on Wiki
> > page.
> >
> > Patch attached FYR
> >
> > Abner
> >
> > > -Original Message-
> > > From: annou...@edk2.groups.io [mailto:annou...@edk2.groups.io] On
> > > Behalf Of Laszlo Ersek
> > > Sent: Wednesday, August 19, 2020 5:59 PM
> > > To: Bret Barkelew ;
> > > devel@edk2.groups.io; liming.gao ;
> > > annou...@edk2.groups.io
> > > Cc: Leif Lindholm ; af...@apple.com; Kinney,
> > > Michael D ; Guptha, Soumya K
> > > 
> > > Subject: Re: [edk2-announce] [EXTERNAL] Re: Soft Feature Freeze
> > > starts now for edk2-stable202008
> > >
> > > On 08/18/20 17:10, Bret Barkelew wrote:
> > > > I agree with the process and withdraw my request, replacing it
> > > > instead with
> > > a disapproving head shake and deep sigh.
> > >
> > > We as a community definitely deserve your disapproval, as our review
> > > response times have been abysmal. :(
> > >
> > > > I’ll go back to pushing on this after the tag.
> > >
> > > Thanks for your persistence!
> > > Laszlo
> > >
> > >
> > > 
> >
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#64430): https://edk2.groups.io/g/devel/message/64430
Mute This Topic: https://groups.io/mt/76284301/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [edk2-announce] Re: Soft Feature Freeze starts now for edk2-stable202008

[Laszlo Ersek]

On 08/19/20 13:48, Leif Lindholm wrote:
> (Slightly trimmed recipient list due to different patch being discussed.)
> 
> So, I can't make this call, because I'm the one who messed up.
> 
> This patch does exactly what I had requested Abner to do some time
> back (off-list, unfortunately), and I was *convinced* I gave it an R-b
> as soon as it hit my inbox - until Abner nudged me about it yesterday.
> 
> The patch in question is
> https://edk2.groups.io/g/devel/topic/76021725

My understanding is:

(1) there is an external project that consumes the FDT library in
EmbeddedPkg, meaning the lib class header "EmbeddedPkg/Include/libfdt.h"
and the lib instance "EmbeddedPkg/Library/FdtLib/FdtLib.inf",

(2) the lib class header pulls in "fdt.h" and "libfdt_env.h",

(3) the external project is not edk2-platforms,

(4) the external project wants -- for some strange reason -- edk2's
"libfdt_env.h" to provide an strncmp() function (or function-like
macro), with that particular stncmp() implementation not being needed in
either edk2-platforms or edk2 itself,

(5) the patch for adding said strncmp() was posted on Aug 6th (at least
when viewed from my time zone), i.e., before the SFF,

(6) it was reviewed 12 days later (within the SFF)

If my understanding is correct, then I don't see how this patch could be
considered a bugfix -- even as a feature addition, it seems hardly
justified to me --, and there would have been ~8 days before the SFF to
review it.

I think we should postpone the patch until after the stable tag.

Thanks
Laszlo


> On Wed, Aug 19, 2020 at 11:29:52 +, Chang, Abner (HPS SW/FW Technologist) 
> wrote:
>> Hmm.. I had a one commit (which is not the feature) just reviewed by
>> Leif but not pushing yet. Is that possible to push before the Hard
>> Feature freeze and also be included in 202008 stable tag? I guess it
>> gets the chance according to the article of SoftFeatureFreeze on
>> Wiki page.
>>
>> Patch attached FYR
>>
>> Abner
>>
>>> -Original Message-
>>> From: annou...@edk2.groups.io [mailto:annou...@edk2.groups.io] On
>>> Behalf Of Laszlo Ersek
>>> Sent: Wednesday, August 19, 2020 5:59 PM
>>> To: Bret Barkelew ; devel@edk2.groups.io;
>>> liming.gao ; annou...@edk2.groups.io
>>> Cc: Leif Lindholm ; af...@apple.com; Kinney, Michael D
>>> ; Guptha, Soumya K
>>> 
>>> Subject: Re: [edk2-announce] [EXTERNAL] Re: Soft Feature Freeze starts
>>> now for edk2-stable202008
>>>
>>> On 08/18/20 17:10, Bret Barkelew wrote:
 I agree with the process and withdraw my request, replacing it instead with
>>> a disapproving head shake and deep sigh.
>>>
>>> We as a community definitely deserve your disapproval, as our review
>>> response times have been abysmal. :(
>>>
 I’ll go back to pushing on this after the tag.
>>>
>>> Thanks for your persistence!
>>> Laszlo
>>>
>>>
>>> 
>>
> 
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#64429): https://edk2.groups.io/g/devel/message/64429
Mute This Topic: https://groups.io/mt/76284301/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] running CI locally

[Laszlo Ersek]

Hi!

I'd like to test CI locally. I'm going through ".pytool/Readme.md" with
the tree checked out at 7e6f150b6902 (= current HEAD). I'm doing this in
a RHEL8 VM, with a python3 virtual environment set up / entered.


* My first note is that the command

  pip install --upgrade pip-requirements.txt

under "Prerequisets", has a small typo; it should be

  pip install --upgrade -r pip-requirements.txt

(the "-r" option is missing).

(

After adding "-r", the following components are now installed in my
virtual env:

- edk2-pytool-library: 0.10.12
- edk2-pytool-extensions: 0.13.9
- antlr4-python3-runtime: 4.7.1
- pyyaml: 5.3.1

Stating this because it might matter for the rest of my email.

)


* Second, when I run the following command:

  stuart_update -c .pytool/CISettings.py TOOL_CHAIN_TAG=GCC5

I get the following warnings:

> WARNING - [SDE] Failed to fetch NugetDependecy: mu_nasm@2.14.02: [Nuget] We 
> failed to install this version 2.14.02 of mu_nasm
> WARNING - [SDE] Failed to fetch NugetDependecy: iasl@20190215.0.0: [Nuget] We 
> failed to install this version 20190215.0.0 of iasl

(repeated one more time:)

> WARNING - [SDE] Failed to fetch NugetDependecy: mu_nasm@2.14.02: [Nuget] We 
> failed to install this version 2.14.02 of mu_nasm
> WARNING - [SDE] Failed to fetch NugetDependecy: iasl@20190215.0.0: [Nuget] We 
> failed to install this version 20190215.0.0 of iasl

and then finally:

> ERROR - We were unable to successfully update 2 dependencies in environment
> ERROR - Error

The virtual machine has NASM installed (2.13.03-2.el8) and IASL too
(acpica-tools-20180629-3.el8).

Where do the NASM and IASL version requirements (2.14.02 and
20190215.0.0, respectively) come from?

Hm... After a git-grep for those version numbers, I find:

- BaseTools/Bin/nasm_ext_dep.yaml
- BaseTools/Bin/iasl_ext_dep.yaml

I was about to say that these version requirements are too strict: for
example, "BaseTools/Conf/tools_def.template" requires "NASM 2.10 or
later for use with the GCC toolchain family". What I have installed
satisfies that, and so CI shouldn't require anything more recent.
*However*, both of the above YAML files have very helpful comments, so I
understand these high versions are downloaded afresh, and only for the
CI run.

And so my question becomes: why do the "nuget" downloads fail for me
(because, presumably, they work fine in the central CI env on github /
Azure); and how can I fix the issue if it pops up again?

I've checked "nuget.org" in my browser, and it has:

- https://www.nuget.org/packages/mu_nasm/ -->  2.14.2
- https://www.nuget.org/packages/iasl/ -->  20190215.0.0

... On a hunch, I've attempted adding the "--verbose" option to the
"stuart_update" command; this is the output (excerpt):

> DEBUG - Verify 'mu_nasm' returning 'False'.
> DEBUG - Verify 'iasl' returning 'False'.
> DEBUG - Creating 4 threads for the SDE update
> UpdatingDEBUG - Verify 'mu_nasm' returning 'False'.
> DEBUG - Verify 'gcc_aarch64_linux' returning 'True'.
> DEBUG - Cleaning dependency directory for 'mu_nasm'...
> DEBUG - Verify 'gcc_arm_linux' returning 'True'.
> INFO - Cmd to run is: mono 
> /root/py3venv/lib/python3.6/site-packages/edk2toolext/bin/NuGet.exe locals 
> global-packages -list
> DEBUG - Verify 'iasl' returning 'False'.
> INFO - 
> DEBUG - Cleaning dependency directory for 'iasl'...
> DEBUG - Verify 'gcc_riscv64_unknown' returning 'True'.
> INFO - --Cmd Output Starting---
> INFO - Cmd to run is: mono 
> /root/py3venv/lib/python3.6/site-packages/edk2toolext/bin/NuGet.exe locals 
> global-packages -list
> INFO - 
> INFO - 
> INFO - --Cmd Output Starting---
> INFO - 
> INFO - /bin/sh: mono: command not found
> INFO - /bin/sh: mono: command not found
> INFO - 
> INFO - --Cmd Output Finished---
> INFO - - Running Time (mm:ss): 00:00 --
> INFO - --- Return Code: 0x007f 
> INFO - 
> INFO - Nuget was unable to provide global packages cache location.
> INFO - Cmd to run is: mono 
> /root/py3venv/lib/python3.6/site-packages/edk2toolext/bin/NuGet.exe install 
> mu_nasm -Source https://api.nuget.org/v3/index.json -ExcludeVersion 
> -NonInteractive -Version 2.14.02 -Verbosity detailed -OutputDirectory 
> "/root/src/rhel8/edk2/BaseTools/Bin/mu_nasm_extdep_temp"
> INFO - 
> INFO - --Cmd Output Starting---
> INFO - 
> INFO - 
> INFO - --Cmd Output Finished---
> INFO - - Running Time (mm:ss): 00:00 --
> INFO - --- 

Re: [edk2-devel] [edk2-announce] Re: Soft Feature Freeze starts now for edk2-stable202008

[Leif Lindholm]

(Slightly trimmed recipient list due to different patch being discussed.)

So, I can't make this call, because I'm the one who messed up.

This patch does exactly what I had requested Abner to do some time
back (off-list, unfortunately), and I was *convinced* I gave it an R-b
as soon as it hit my inbox - until Abner nudged me about it yesterday.

The patch in question is
https://edk2.groups.io/g/devel/topic/76021725

/
Leif

On Wed, Aug 19, 2020 at 11:29:52 +, Chang, Abner (HPS SW/FW Technologist) 
wrote:
> Hmm.. I had a one commit (which is not the feature) just reviewed by
> Leif but not pushing yet. Is that possible to push before the Hard
> Feature freeze and also be included in 202008 stable tag? I guess it
> gets the chance according to the article of SoftFeatureFreeze on
> Wiki page.
>
> Patch attached FYR
> 
> Abner
> 
> > -Original Message-
> > From: annou...@edk2.groups.io [mailto:annou...@edk2.groups.io] On
> > Behalf Of Laszlo Ersek
> > Sent: Wednesday, August 19, 2020 5:59 PM
> > To: Bret Barkelew ; devel@edk2.groups.io;
> > liming.gao ; annou...@edk2.groups.io
> > Cc: Leif Lindholm ; af...@apple.com; Kinney, Michael D
> > ; Guptha, Soumya K
> > 
> > Subject: Re: [edk2-announce] [EXTERNAL] Re: Soft Feature Freeze starts
> > now for edk2-stable202008
> > 
> > On 08/18/20 17:10, Bret Barkelew wrote:
> > > I agree with the process and withdraw my request, replacing it instead 
> > > with
> > a disapproving head shake and deep sigh.
> > 
> > We as a community definitely deserve your disapproval, as our review
> > response times have been abysmal. :(
> > 
> > > I’ll go back to pushing on this after the tag.
> > 
> > Thanks for your persistence!
> > Laszlo
> > 
> > 
> > 
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#64427): https://edk2.groups.io/g/devel/message/64427
Mute This Topic: https://groups.io/mt/76284301/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v2 2/2] CryptoPkg/OpensslLib: Commit the auto-generated assembly files for X64

[Laszlo Ersek]

Hi,

On 08/18/20 23:33, Sean wrote:
> Mike,
> 
> I am not technically a basetool maintainer but as an active user/dev in
> basetools, i would be opposed to bringing in perl as an edk2 dependency.
> Also introducing another language is counter to the goal of aligning on
> python and improving the python used within edk2.  From my perspective
> the openssl config case isn't strong enough to counter the above goal.
> In fact as you know we are trying to change the paradigm for
> Crypto/OpenSSL with the Crypto Driver
> (https://github.com/tianocore/edk2/tree/master/CryptoPkg/Driver) and
> BaseCryptLibOnProtocolPpi
> (https://github.com/tianocore/edk2/tree/master/CryptoPkg/Library/BaseCryptLibOnProtocolPpi)
> work so that everyday development doesn't need to compile openssl in
> their edk2 builds.

Here I'd only like to comment on this one aspect (= build OpenSSL as a
statically linked library vs. as a crypto service driver / PEIM).

Recently I tried to evaluate the crypto driver for OVMF. I started with
the PEI phase. The configuration interface (the PCD) is baroque, *BUT*
that is a direct consequence of OpenSSL offering a huge range of
interfaces. So no complaints about the config interface.

I also reviewed the CryptoPkg.dsc "pre-sets" (i.e., CRYPTO_SERVICES
being one of PACKAGE / ALL / NONE / MIN_PEI / MIN_DXE_MIN_SMM). I had
two problems:

- These pre-sets are supremely suitable for a platform that is composed
of multiple build runs; that is, build the crypto PEIM, build the DXE /
SMM protocol drivers, package up the resultant binaries, and *then*
build the actual platforms (which will then include the crypto service
drivers in *binary* form). On the other hand, the pre-sets are not
useful to a platform that is supposed to be built in a single-shot.
Importantly, I'm not saying that the pre-sets are *detrimental* to such
platforms -- they aren't. It's just that the pre-sets target a different
use case.

- The other problem I had was the one that we had discussed when the
crypto service driver was being introduced. Namely, selecting the
OpenSSL interfaces (interface families) that the platform actually consumes.

Now, I carefully tracked down the modules in OVMF that needed crypto
support, by *not* resolving SmmCryptLib, RuntimeCryptLib, TlsLib in
general [LibraryClasses] sections in the OVMF DSC files. Then I re-added
those lib-class resolutions as module-scoped  overrides
to the actual modules that needed them.

However, I didn't know how to even *begin* evaluating the specific "API
needs" of the modules identified thusly. On a Windows or Linux OS, when
you have a dynamically linked executable, and it doesn't find a symbol
in a shared library, you get a nice error message, and the application
doesn't start. On the other hand, if a crypto protocol call fails in SMM
because we missed a feature bit in the config PCD, the results are
somewhat less user-friendly.

The expression "minimum required services" in CryptoPkg.dsc seems
relevant, but it didn't convince me that it would cover everything
needed by -- for example -- VariableSmm, VariableRuntimeDxe, and TlsDxe.

So, given that I couldn't construct a "tight profile", I started my
investigation (for OVMF's PEI phase) by including the crypto service
PEIM with *all* interfaces enabled.

This would be restricted to "TPM_ENABLE", because only that is when
OVMF's PEI phase needs crypto -- due to including the various TPM1 and
TPM2 PEIMs.

So basically this check would replace the statically linked -- and
accordingly trimmed! -- "thick" OpenSSL library copies in the TPM1/PTM2
PEIMs, with the thin wrapper lib
(BaseCryptLibOnProtocolPpi/PeiCryptLib.inf) *plus* the full-blown crypto
service PEIM.

The result was a *violent* size explosion in PEIFV; at least in the
NOOPT build. Before:

> PEIFV [64%Full] 917504 total, 592456 used, 325048 free

after:

>   the required fv image size 0x132968 exceeds the set fv image size
> 0xe

The PEIFV footprint more than doubled, from 592,456 bytes to 1,255,784
bytes.

I gave up there. Until the "crypto profile" construction is not
automated for platforms, somehow, I don't know how I could maintain OVMF
consuming the crypto service PEIMs/drivers.

(I wonder if we should maintain a "required crypto services" bitmap for
each individual PEIM / DXE / SMM driver inside edk2. And then, when a
platform includes any one such PEIM or driver, they'd know to "OR" the
bitmap for that particular module into their platform PCD setting.)

> So I support leaving it as is which means if you have to change
> something in openssl config you deal with it and a special one off.

(OK, I guess I can comment on this too, after all.)

I agree.

While perl is readily available on Linux build hosts, I remember:
- accommodating the python3 BaseTools requirements on my RHEL7 laptop,
- (almost) bumping the NASM version so we could compile the VMGEXIT
instruction for IA32,
- the python virtual environment discussions for running CI locally.

So I agree that 

Re: [edk2-devel] [EXTERNAL] Re: Soft Feature Freeze starts now for edk2-stable202008

[Laszlo Ersek]

On 08/18/20 17:10, Bret Barkelew wrote:
> I agree with the process and withdraw my request, replacing it instead with a 
> disapproving head shake and deep sigh.

We as a community definitely deserve your disapproval, as our review
response times have been abysmal. :(

> I’ll go back to pushing on this after the tag.

Thanks for your persistence!
Laszlo


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#64424): https://edk2.groups.io/g/devel/message/64424
Mute This Topic: https://groups.io/mt/76253625/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v5 1/1] ShellPkg/DynamicCommand: add HttpDynamicCommand

[Laszlo Ersek]

On 08/18/20 20:33, Vladimir Olovyannikov wrote:
>> -Original Message-
>> From: Rabeda, Maciej 
>> Sent: Tuesday, August 18, 2020 9:54 AM
>> To: Vladimir Olovyannikov ; Laszlo
>> Ersek ; Gao, Zhichao ;
>> devel@edk2.groups.io
>> Cc: Samer El-Haj-Mahmoud ; Wu, Jiaxin
>> ; Fu, Siyuan ; Ni, Ray
>> ; Gao, Liming ; Nd
>> 
>> Subject: Re: [PATCH v5 1/1] ShellPkg/DynamicCommand: add
>> HttpDynamicCommand
>>
>> Hi Vladimir,
>>
>> I am inprog of going through the patch. There are some coding standard
>> violations.
>> For reference:
>> https://edk2-docs.gitbook.io/edk-ii-c-coding-standards-specification/

[...]

> OK, I will look through all files again, probably something slipped from my
> attention as I work with Linux code as well.

Now that we have ECC checking enabled in CI, submitting a personal pull
request could help. Additionally, the CI workload should be possible to
run locally, according to ".pytool/Readme.md".

(I'm planning to learn how to run that locally myself.)

> 
>> HttpApp.c:
>> Line 48: Space after type cast.
> Sorry, I don't understand here. You mean, Something = (CAST)SomethingElse
> should actually be "Something = (CAST) SomethingElse?
> I don't see this say in the Tftp.c:
> if (AsciiStrnCmp ((CHAR8 *)Option->OptionStr, "tsize", 5) == 0) ...
> or in TftpApp.c
> Status = (EFI_STATUS)RunTftp (ImageHandle, SystemTable);
> Am I missing anything?


In core edk2 packages, casts are frequently written like this:

  (TYPE) Expression

(Importantly, Expression itself is not parenthesized.)

In my opinion, this is a bad practice. That's because the typecast has
one of the strongest operator bindings in the C language, and visually
distancing (TYPE) from "Expression" suggests the opposite -- it's
counter-intuitive.

I strongly prefer

  (TYPE)Expression

but other maintainers may have a different preference.

Thanks
Laszlo


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#64423): https://edk2.groups.io/g/devel/message/64423
Mute This Topic: https://groups.io/mt/75826968/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH EDK2 v2 1/1] SecurityPkg/DxeImageVerificationLib:Enhanced verification of Offset

[Laszlo Ersek]

On 08/18/20 19:29, Bret Barkelew wrote:
> Jiewen,
> 
> I don’t completely agree with your second point. If the underlying issue is 
> already out of embargo, and we’ve just failed to fix it, that’s not a new 
> issue. I would want to see fixes to the fixes fast-tracked (or at least 
> heavily prioritized), rather than re-entering a full embargo period.

I had the same thought at first -- in the end I guess "it depends". If
we realize that the patch failed to fix the original issue (or a less
obvious issue of the same issue), then I agree the "cat is out of the
bag"; the patch already exposed the vulnerability, so waiting more would
be counter-productive.

OTOH if the fix ends up helping humans recognize a *different*
vulnerability, or even *introduces* a new vulnerability, then that does
seem to deserve an embargo.

I think it depends on the extent of the information that the first patch
exposes. Unfortunately, that "extent" may be vague.

Laszlo

> From: Yao, Jiewen via groups.io
> Sent: Tuesday, August 18, 2020 6:12 AM
> To: devel@edk2.groups.io; 
> ler...@redhat.com
> Cc: xiewen...@huawei.com; Wang, Jian 
> J; 
> huangmin...@huawei.com; 
> songdongku...@huawei.com; Marvin 
> Häuser; Vitaly 
> Cheptsov
> Subject: [EXTERNAL] Re: [edk2-devel] [PATCH EDK2 v2 1/1] 
> SecurityPkg/DxeImageVerificationLib:Enhanced verification of Offset
> 
> Hi Laszlo
> I agree with on your most points.
> It is all about *return of investment* or *risk control*. Like we cannot 
> pursue 100% security because we will bankrupt ourselves if so.
> 
> Here I just raise my concern.
> 1) If we alway allow developers’ low quality patch without test, the overall 
> quality will become lower and lower. Personally I have no confidence to help 
> catch all those issues. You are the role model on code review. But not all 
> people review the code like you. We need both expertise and time for code 
> review.
> 
> 2) I purposely separate security fix from non security one, because the 
> process is different. The embargo might be 6 months. What if we found the 
> security patch does not fix the problem after embargo? Unlike we send one 
> more patch tomorrow, we need wait for another 6 months...
> 
> thank you!
> Yao, Jiewen
> 
> 
>> 在 2020年8月18日，下午6:17，Laszlo Ersek  写道：
>>
>> Hi Jiewen,
>>
>> (+Marvin, +Vitaly)
>>
>> On 08/18/20 01:23, Yao, Jiewen wrote:
 -Original Message-
 From: devel@edk2.groups.io  On Behalf Of Laszlo
 Ersek
 Sent: Tuesday, August 18, 2020 12:53 AM
 To: Yao, Jiewen ; devel@edk2.groups.io;
 xiewen...@huawei.com; Wang, Jian J 
 Cc: huangmin...@huawei.com; songdongku...@huawei.com
 Subject: Re: [edk2-devel] [PATCH EDK2 v2 1/1]
 SecurityPkg/DxeImageVerificationLib:Enhanced verification of Offset
>>
>> [...]
>>
>>> However, I do think the producer is mandatory for a fix or at least a
>>> security fix.
>>> The owner to fix the issue should guarantee the patch is good.
>>> The owner shall never rely on the code reviewer to figure out if the
>>> patch is good and complete.
>>>
>>> I have some bad experience that bug owner just wrote a patch and tried
>>> to fix a problem, without any test.
>>> And it happened passed code review from someone who does not well
>>> understand the problem, but give rb based upon the time pressure.
>>> Later, the fix was approved to be useless.
>>>
>>> In my memory, at least 3 cases were security fix. They are found, just
>>> because they are sensitive, more people took a look later.
>>>It was simple. It was one-line change.
>>>   But it has not test, and it was wrong.
>>> "It was ridiculous" -- commented by the people who find the so-called
>>> security fix does not fix the issue.
>>
>> Just because sloppy/rushed reviews exist, and just because reviewers
>> operate under time pressure, we should not automatically reject security
>> fixes that come without a reproducer.
>>
>> Some organizations do develop reproducers, but they never share them
>> publicly (for fear of abuse by others).
>>
>> But more importantly, in an open development project, a developer could
>> have time and expertise to contribute a fix, but not to create a
>> reproducer.
>>
>> - If we make contributing harder, fewer people will upstream their
>>  fixes.
>>
>> - If we make contributing harder, then contributions that do make it to
>>  the tree will be of higher quality.
>>
>> Both statements ring true to me -- so it's a tradeoff.
>>
>> (By "we", I mean the edk2 community.)
>>
 Additionally, the exact statement that the bug report does make,
 namely

  it's possible to overflow Offset back to 0 causing an endless loop

 is wrong (as far as I can tell anyway). It is not "OffSet" that can

Re: [edk2-devel] [PATCH EDK2 v2 1/1] SecurityPkg/DxeImageVerificationLib:Enhanced verification of Offset

[Laszlo Ersek]

On 08/18/20 17:18, Mathews, John wrote:
> I dug up the original report details.  This was noted as a concern during a 
> source code inspection.  There was no demonstration of how it might be 
> triggered.
> 
> " There is an integer overflow vulnerability in the 
> DxeImageVerificationHandler function when
> parsing the PE files attribute certificate table. In cases where 
> WinCertificate->dwLength is
> sufficiently large, it's possible to overflow Offset back to 0 causing an 
> endless loop."
> 
> The recommendation was to add stricter checking of "Offset" and the embedded 
> length fields of certificate data
> before using them.

Thanks for checking!

Laszlo

> 
> 
> 
> -Original Message-
> From: Laszlo Ersek  
> Sent: Tuesday, August 18, 2020 1:59 AM
> To: Wang, Jian J ; devel@edk2.groups.io; Yao, Jiewen 
> ; xiewen...@huawei.com
> Cc: huangmin...@huawei.com; songdongku...@huawei.com; Mathews, John 
> 
> Subject: Re: [edk2-devel] [PATCH EDK2 v2 1/1] 
> SecurityPkg/DxeImageVerificationLib:Enhanced verification of Offset
> 
> On 08/18/20 04:10, Wang, Jian J wrote:
>> Laszlo,
>>
>> My apologies for the slow response. I'm not the original reporter but 
>> just the BZ submitter. And I didn't do deep analysis to this issue. 
>> The issues was reported from one internal team. Add John in loop to see if 
>> he knows more about it or not.
>>
>> My superficial understanding on such issue is that, if there's 
>> "potential" issue in theory and hard to reproduce, it's still worthy 
>> of using an alternative way to replace the original implementation 
>> with no "potential" issue at all. Maybe we don't have to prove old way is 
>> something wrong but must prove that the new way is really safe.
> 
> I agree, thanks.
> 
> It would be nice to hear more from the internal team about the originally 
> reported (even if hard-to-trigger) issue.
> 
> Thanks!
> Laszlo
> 
>>
>> Regards,
>> Jian
>>
>>> -Original Message-
>>> From: devel@edk2.groups.io  On Behalf Of Laszlo 
>>> Ersek
>>> Sent: Tuesday, August 18, 2020 12:53 AM
>>> To: Yao, Jiewen ; devel@edk2.groups.io; 
>>> xiewen...@huawei.com; Wang, Jian J 
>>> Cc: huangmin...@huawei.com; songdongku...@huawei.com
>>> Subject: Re: [edk2-devel] [PATCH EDK2 v2 1/1] 
>>> SecurityPkg/DxeImageVerificationLib:Enhanced verification of Offset
>>>
>>> Hi Jiewen,
>>>
>>> On 08/14/20 10:53, Yao, Jiewen wrote:
> To Jiewen,
> Sorry, I don't have environment to reproduce the issue.

 Please help me understand, if you don’t have environment to 
 reproduce the
>>> issue, how do you guarantee that your patch does fix the problem and 
>>> we don’t have any other vulnerabilities?
>>>
>>> The original bug report in
>>>  is seriously 
>>> lacking. It does not go into detail about the alleged integer overflow.
>>> It does not quote the code, does not explain the control flow, does 
>>> not identify the exact edk2 commit at which the vulnerability exists.
>>>
>>> The bug report also does not offer a reproducer.
>>>
>>> Additionally, the exact statement that the bug report does make, 
>>> namely
>>>
>>>   it's possible to overflow Offset back to 0 causing an endless loop
>>>
>>> is wrong (as far as I can tell anyway). It is not "OffSet" that can 
>>> be overflowed to zero, but the *addend* that is added to OffSet can 
>>> be overflowed to zero. Therefore the infinite loop will arise because 
>>> OffSet remains stuck at its present value, and not because OffSet 
>>> will be re-set to zero.
>>>
>>> For the reasons, we can only speculate as to what the actual problem 
>>> is, unless Jian decides to join the discussion and clarifies what he 
>>> had in mind originally.
>>>
>>> My understanding (or even "reconstruction") of the vulnerability is 
>>> described above, and in the patches that I proposed.
>>>
>>> We can write a patch based on code analysis. It's possible to 
>>> identify integer overflows based on code analysis, and it's possible 
>>> to verify the correctness of fixes by code review. Obviously testing 
>>> is always good, but many times, constructing reproducers for such 
>>> issues that were found by code review, is difficult and time 
>>> consuming. We can say that we don't fix vulnerabilities without 
>>> reproducers, or we can say that we make an effort to fix them even if 
>>> all we have is code analysis (and not a reproducer).
>>>
>>> So the above paragraph concerns "correctness". Regarding 
>>> "completeness", I guarantee you that this patch does not fix *all* 
>>> problems related to PE parsing. (See the other BZ tickets.) It does 
>>> fix *one* issue with PE parsing. We can say that we try to fix such 
>>> issues gradually (give different CVE numbers to different issues, and 
>>> address them one at a time), or we can say that we rewrite PE parsing from 
>>> the ground up.
>>> (BTW: I have seriously attempted that in the past, and I gave up, 
>>> because the PE format is FUBAR.)
>>>
>>> 

Re: [edk2-devel] [PATCH 1/1] UefiCpuPkg/MpInitLib: Always initialize the DoDecrement variable

[Laszlo Ersek]

On 08/18/20 15:10, Tom Lendacky wrote:
> From: Tom Lendacky 
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2901
> 
> The DoDecrement variable in ApWakeupFunction () wasn't always being
> initialized. Update the code to always fully initialize it.
> 
> Cc: Eric Dong 
> Cc: Ray Ni 
> Cc: Laszlo Ersek 
> Cc: Rahul Kumar 
> Signed-off-by: Tom Lendacky 
> ---
>  UefiCpuPkg/Library/MpInitLib/MpLib.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c 
> b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> index 90416c81b616..e24bdc64f930 100644
> --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
> +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> @@ -885,9 +885,7 @@ ApWakeupFunction (
>UINT64Status;
>BOOLEAN   DoDecrement;
>  
> -  if (CpuMpData->InitFlag == ApInitConfig) {
> -DoDecrement = TRUE;
> -  }
> +  DoDecrement = (CpuMpData->InitFlag == ApInitConfig) ? TRUE : FALSE;
>  
>while (TRUE) {
>  Msr.GhcbPhysicalAddress = AsmReadMsr64 (MSR_SEV_ES_GHCB);
> 

Not that I want to obsess about style, but

  (condition) ? TRUE : FALSE

is an anti-patter that's similar to

  (condition) == TRUE

Instead, I suggest:

  DoDecrement = (BOOLEAN)(CpuMpData->InitFlag == ApInitConfig);

(The (BOOLEAN) cast is necessary, or at least used to be necessary,
becasue the == operator returns "int" (INT32), but BOOLEAN (i.e., the
type of "DoDecrement") is UINT8 -- and some VS toolchains perceive (or
used to perceive) this implicit conversion as a "potential loss of
precision". That warning is of course bogus, as the == operator only
produces 0 or 1, each of which values fits comfortably into a UINT8. But
still the explicit (BOOLEAN) cast is how we suppress the warning.)

Different question: who's supposed to merge (v2 of) this? Per
"Maintainers.txt", it should be Eric or Ray; OTOH, maybe the fix is
urgent (build failure with CLANGPDB) and anyone with push access could
qualify.

Thanks,
Laszlo


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#64420): https://edk2.groups.io/g/devel/message/64420
Mute This Topic: https://groups.io/mt/76264245/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v4 0/2] Enable EDKII CI support for DynamicTablesPkg

[Liming Gao]

Merged 
a048af3c9073e4b8108e6cf920bbb35574059639..5a6d764e1d073d28e8f398289ccb5592bf9a72ba

Thanks
Liming
-Original Message-
From: devel@edk2.groups.io  On Behalf Of Leif Lindholm
Sent: 2020年8月18日 19:34
To: Gao, Liming 
Cc: Sami Mujawar ; devel@edk2.groups.io; 
bret.barke...@microsoft.com; sean.bro...@microsoft.com; Kinney, Michael D 
; af...@apple.com; Laszlo Ersek 
; Alexei Fedorov ; Ard Biesheuvel 
; Matteo Carlini ; Ben Adderson 
; nd 
Subject: Re: [edk2-devel] [PATCH v4 0/2] Enable EDKII CI support for 
DynamicTablesPkg

No objections from me.

On Mon, Aug 17, 2020 at 06:37:02 +, Gao, Liming wrote:
> Sami:
>   I think this change is OK. And, Shenglei and Alexei gave reviewed-by before 
> soft feature freeze. 
> 
> Mike, Leif, Andrew and Laszlo:
>   Have you any comments to merge this patch to edk2-stable202008 stable tag. 
> 
> Thanks
> Liming
> -Original Message-
> From: Sami Mujawar 
> Sent: 2020年8月14日 21:24
> To: Gao, Liming ; devel@edk2.groups.io; 
> bret.barke...@microsoft.com; sean.bro...@microsoft.com; Kinney, 
> Michael D 
> Cc: Alexei Fedorov ; Sami Mujawar 
> ; Ard Biesheuvel ; 
> Matteo Carlini ; Ben Adderson 
> ; nd 
> Subject: RE: [PATCH v4 0/2] Enable EDKII CI support for 
> DynamicTablesPkg
> 
> Hi All,
> 
> Is there anything else needed before this patch series can be merged?
> If possible, we would like this feature enabled in the edk2-stable202008.
> 
> Regards,
> 
> Sami Mujawar
> 
> -Original Message-
> From: Sami Mujawar 
> Sent: 07 August 2020 06:30 PM
> To: devel@edk2.groups.io
> Cc: Sami Mujawar ; Alexei Fedorov 
> ; sean.bro...@microsoft.com; 
> liming@intel.com; michael.d.kin...@intel.com; 
> bret.barke...@microsoft.com; Ard Biesheuvel ; 
> Matteo Carlini ; Laura Moretta 
> ; nd 
> Subject: [PATCH v4 0/2] Enable EDKII CI support for DynamicTablesPkg
> 
> The TianoCore EDKII project has introduced a Core CI infrastructure using 
> TianoCore EDKII Tools PIP modules.
> 
> The v2 patch series at https://edk2.groups.io/g/devel/message/63259
> adds support for building DynamicTablesPkg using the EKDII Core CI.
> 
> Splitting the v2 patch series into 2 separate series. This v4 series contains 
> the patches for enabling Core CI for DynamicTablesPkg in .pytools and 
> .azurepipelines i.e. the last two patches from the v2 series.
> 
> The v3 series containing the patches for DynamicTablesPkg has already been 
> merged in edk2 master at:
> - 
> https://github.com/tianocore/edk2/commit/2d0c42fdf2cf1855b0a042ef82d84
> 8c7716adefe
> - 
> https://github.com/tianocore/edk2/commit/e3f8605a23ebe9c54ae2b17819d00
> e185069667d Ref (v3 series): 
> https://edk2.groups.io/g/devel/topic/patch_v3_0_2_add_edkii_ci/7605235
> 1
> 
> There is no code change other than splitting the v2 series.
> 
> The changes for this v4 series can be seen at:
> https://github.com/samimujawar/edk2/tree/839_dynamictablespkg_ci_v4
> 
> Sami Mujawar (2):
>   .pytool: CI Settings to support DynamicTablesPkg
>   .azurepipelines: Add DynamicTablesPkg to CI matrix
> 
>  .azurepipelines/templates/pr-gate-build-job.yml | 3 ++-
>  .pytool/CISettings.py   | 2 ++
>  .pytool/Readme.md   | 2 +-
>  3 files changed, 5 insertions(+), 2 deletions(-) 
> 
> --
> 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
> 




-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#64419): https://edk2.groups.io/g/devel/message/64419
Mute This Topic: https://groups.io/mt/76053995/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 1/1] UefiCpuPkg/MpInitLib: Always initialize the DoDecrement variable

[Liming Gao]

Tested-by: Liming Gao 
Reviewed-by: Liming Gao 

-Original Message-
From: Tom Lendacky  
Sent: 2020年8月18日 21:10
To: devel@edk2.groups.io
Cc: Gao, Liming ; Dong, Eric ; Ni, 
Ray ; Laszlo Ersek ; Kumar, Rahul1 

Subject: [PATCH 1/1] UefiCpuPkg/MpInitLib: Always initialize the DoDecrement 
variable

From: Tom Lendacky 

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2901

The DoDecrement variable in ApWakeupFunction () wasn't always being 
initialized. Update the code to always fully initialize it.

Cc: Eric Dong 
Cc: Ray Ni 
Cc: Laszlo Ersek 
Cc: Rahul Kumar 
Signed-off-by: Tom Lendacky 
---
 UefiCpuPkg/Library/MpInitLib/MpLib.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c 
b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index 90416c81b616..e24bdc64f930 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -885,9 +885,7 @@ ApWakeupFunction (
   UINT64Status;
   BOOLEAN   DoDecrement;
 
-  if (CpuMpData->InitFlag == ApInitConfig) {
-DoDecrement = TRUE;
-  }
+  DoDecrement = (CpuMpData->InitFlag == ApInitConfig) ? TRUE : 
+ FALSE;
 
   while (TRUE) {
 Msr.GhcbPhysicalAddress = AsmReadMsr64 (MSR_SEV_ES_GHCB);
--
2.28.0


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#64418): https://edk2.groups.io/g/devel/message/64418
Mute This Topic: https://groups.io/mt/76264245/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-