date:20190227

Re: [edk2] [PATCH 2/2] UefiCpuPkg/Microcode: Add verification logic before parsing payload

2019-02-27 Thread Ni, Ray




> -Original Message-
> From: edk2-devel  On Behalf Of Chen A
> Chen
> Sent: Thursday, February 28, 2019 2:03 PM
> To: edk2-devel@lists.01.org
> Cc: Dong, Eric 
> Subject: [edk2] [PATCH 2/2] UefiCpuPkg/Microcode: Add verification logic
> before parsing payload
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1020
> 
> The function MicrocodeDetect may receive untrusted input. Add verification
> logic to check Microcode Payload Header.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Chen A Chen 
> Cc: Ray Ni 
> Cc: Eric Dong 
> ---
>  UefiCpuPkg/Library/MpInitLib/Microcode.c | 190
> +--
>  1 file changed, 127 insertions(+), 63 deletions(-)
> 
> diff --git a/UefiCpuPkg/Library/MpInitLib/Microcode.c
> b/UefiCpuPkg/Library/MpInitLib/Microcode.c
> index 5f9ae22794..1d5e964791 100644
> --- a/UefiCpuPkg/Library/MpInitLib/Microcode.c
> +++ b/UefiCpuPkg/Library/MpInitLib/Microcode.c
> @@ -32,6 +32,69 @@ GetCurrentMicrocodeSignature (
>return BiosSignIdMsr.Bits.MicrocodeUpdateSignature;
>  }
> 
> +/**
> +  Verify Microcode.
> +
> +  @param[in]  MicrocodeHeader   Point to the Microcode payload buffer.
> +
> +  @retval TRUE  Valid payload
> +  @retval FALSE Invalid payload
> +**/
> +BOOLEAN
> +VerifyMicrocodeHeader (


1. "Verify" cannot tell what the return value stands for. Use 
"IsMicrocodeHeaderValid"?

> +  IN CPU_MICROCODE_HEADER *MicrocodeHeader
> +  )
> +{
> +  UINTN   TotalSize;
> +  UINTN   DataSize;
> +
> +  if (MicrocodeHeader == NULL) {
> +return FALSE;
> +  }
2. Can we use assert here?

> +
> +  ///
> +  /// Verify Version
> +  ///
> +  if (MicrocodeHeader->HeaderVersion != 0x1) {
> +return FALSE;
> +  }
> +  if (MicrocodeHeader->LoaderRevision != 0x1) {
> +return FALSE;
> +  }
> +
> +  ///
> +  /// Verify TotalSize
> +  ///
> +  if (MicrocodeHeader->DataSize == 0) {
> +TotalSize = 2048;
> +  } else {
> +TotalSize = MicrocodeHeader->TotalSize;
> +  }
> +  if (TotalSize <= sizeof (CPU_MICROCODE_HEADER)) {
> +return FALSE;
> +  }

3. Can you please add more comments here to describe the alignment requirement?

> +  if ((TotalSize & (SIZE_1KB - 1)) != 0) {
> +return FALSE;
> +  }
> +
> +  ///
> +  /// Verify DataSize
> +  ///
> +  if (MicrocodeHeader->DataSize == 0) {
> +DataSize = 2048 - sizeof (CPU_MICROCODE_HEADER);
> +  } else {
> +DataSize = MicrocodeHeader->DataSize;
> +  }
> +  if (DataSize > TotalSize - sizeof (CPU_MICROCODE_HEADER)) {

4. if MIcrocodeHeader->DataSize is 0, seems the if-check is redundant.

> +return FALSE;
> +  }

5. Can you please add more comments here to describe the alignment requirement?
> +  if ((DataSize & 0x3) != 0) {
> +return FALSE;
> +  }
> +
> +  return TRUE;
> +}
> +
>  /**
>Detect whether specified processor can find matching microcode patch and
> load it.
> 
> @@ -166,6 +229,18 @@ MicrocodeDetect (
>  //
>  CorrectMicrocode = FALSE;
> 
> +if (!VerifyMicrocodeHeader (MicrocodeEntryPoint)) {
> +  //
> +  // It is the padding data between the microcode patches for microcode
> patches alignment.
> +  // Because the microcode patch is the multiple of 1-KByte, the padding
> data should not
> +  // exist if the microcode patch alignment value is not larger than 
> 1-KByte.
> So, the microcode
> +  // alignment value should be larger than 1-KByte. We could skip
> SIZE_1KB padding data to
> +  // find the next possible microcode patch header.
> +  //
> +  MicrocodeEntryPoint = (CPU_MICROCODE_HEADER *) (((UINTN)
> MicrocodeEntryPoint) + SIZE_1KB);
> +  continue;
> +}
> +
>  //
>  // Save an in-complete CheckSum32 from CheckSum Part1 for common
> parts.
>  //
> @@ -184,90 +259,79 @@ MicrocodeDetect (
>  InCompleteCheckSum32 -= MicrocodeEntryPoint->ProcessorFlags;
>  InCompleteCheckSum32 -= MicrocodeEntryPoint->Checksum;
> 
> -if (MicrocodeEntryPoint->HeaderVersion == 0x1) {
> +//
> +// It is the microcode header. It is not the padding data between
> microcode patches
> +// because the padding data should not include 0x0001 and it should
> be the repeated
> +// byte format (like 0xXYXYXYXY).
> +//
> +if (MicrocodeEntryPoint->ProcessorSignature.Uint32 == Eax.Uint32 &&
> +MicrocodeEntryPoint->UpdateRevision > LatestRevision &&
> +(MicrocodeEntryPoint->ProcessorFlags & (1 << PlatformId))
> +) {
>//
> -  // It is the microcode header. It is not the padding data between
> microcode patches
> -  // because the padding data should not include 0x0001 and it should
> be the repeated
> -  // byte format (like 0xXYXYXYXY).
> +  // Calculate CheckSum Part1.
>//
> -  if (MicrocodeEntryPoint->ProcessorSignature.Uint32 == Eax.Uint32 &&
> -  MicrocodeEntryPoint->UpdateRevision > LatestRevision &&
> -

Re: [edk2] [PATCH 1/2] UefiCpuPkg/Microcode: Fix InComplete CheckSum32 issue

2019-02-27 Thread Ni, Ray

Reviewed-by: Ray Ni 

> -Original Message-
> From: edk2-devel  On Behalf Of Chen A
> Chen
> Sent: Thursday, February 28, 2019 2:03 PM
> To: edk2-devel@lists.01.org
> Cc: Dong, Eric 
> Subject: [edk2] [PATCH 1/2] UefiCpuPkg/Microcode: Fix InComplete
> CheckSum32 issue
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1020
> 
> The Microcode region indicated by MicrocodePatchAddress PCD may contain
> more than one Microcode entry. We should save InCompleteCheckSum32
> value for each payload. Move the logic for calculate InCompleteCheckSum32
> from the outsize of the do-while loop to the inside.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Chen A Chen 
> Cc: Ray Ni 
> Cc: Eric Dong 
> ---
>  UefiCpuPkg/Library/MpInitLib/Microcode.c | 37 -
> ---
>  1 file changed, 19 insertions(+), 18 deletions(-)
> 
> diff --git a/UefiCpuPkg/Library/MpInitLib/Microcode.c
> b/UefiCpuPkg/Library/MpInitLib/Microcode.c
> index e1f661d6b1..5f9ae22794 100644
> --- a/UefiCpuPkg/Library/MpInitLib/Microcode.c
> +++ b/UefiCpuPkg/Library/MpInitLib/Microcode.c
> @@ -159,30 +159,31 @@ MicrocodeDetect (
>MicrocodeEnd = (UINTN) (CpuMpData->MicrocodePatchAddress +
> CpuMpData->MicrocodePatchRegionSize);
>MicrocodeEntryPoint = (CPU_MICROCODE_HEADER *) (UINTN)
> CpuMpData->MicrocodePatchAddress;
> 
> -  //
> -  // Save an in-complete CheckSum32 from CheckSum Part1 for common
> parts.
> -  //
> -  if (MicrocodeEntryPoint->DataSize == 0) {
> -InCompleteCheckSum32 = CalculateSum32 (
> - (UINT32 *) MicrocodeEntryPoint,
> - sizeof (CPU_MICROCODE_HEADER) + 2000
> - );
> -  } else {
> -InCompleteCheckSum32 = CalculateSum32 (
> - (UINT32 *) MicrocodeEntryPoint,
> - sizeof (CPU_MICROCODE_HEADER) + 
> MicrocodeEntryPoint-
> >DataSize
> - );
> -  }
> -  InCompleteCheckSum32 -= MicrocodeEntryPoint-
> >ProcessorSignature.Uint32;
> -  InCompleteCheckSum32 -= MicrocodeEntryPoint->ProcessorFlags;
> -  InCompleteCheckSum32 -= MicrocodeEntryPoint->Checksum;
> -
>do {
>  //
>  // Check if the microcode is for the Cpu and the version is newer
>  // and the update can be processed on the platform
>  //
>  CorrectMicrocode = FALSE;
> +
> +//
> +// Save an in-complete CheckSum32 from CheckSum Part1 for common
> parts.
> +//
> +if (MicrocodeEntryPoint->DataSize == 0) {
> +  InCompleteCheckSum32 = CalculateSum32 (
> +   (UINT32 *) MicrocodeEntryPoint,
> +   sizeof (CPU_MICROCODE_HEADER) + 2000
> +   );
> +} else {
> +  InCompleteCheckSum32 = CalculateSum32 (
> +   (UINT32 *) MicrocodeEntryPoint,
> +   sizeof (CPU_MICROCODE_HEADER) + 
> MicrocodeEntryPoint-
> >DataSize
> +   );
> +}
> +InCompleteCheckSum32 -= MicrocodeEntryPoint-
> >ProcessorSignature.Uint32;
> +InCompleteCheckSum32 -= MicrocodeEntryPoint->ProcessorFlags;
> +InCompleteCheckSum32 -= MicrocodeEntryPoint->Checksum;
> +
>  if (MicrocodeEntryPoint->HeaderVersion == 0x1) {
>//
>// It is the microcode header. It is not the padding data between
> microcode patches
> --
> 2.16.2.windows.1
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH 2/2] UefiCpuPkg/Microcode: Add verification logic before parsing payload

2019-02-27 Thread Chen A Chen

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

The function MicrocodeDetect may receive untrusted input. Add verification
logic to check Microcode Payload Header.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Chen A Chen 
Cc: Ray Ni 
Cc: Eric Dong 
---
 UefiCpuPkg/Library/MpInitLib/Microcode.c | 190 +--
 1 file changed, 127 insertions(+), 63 deletions(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/Microcode.c 
b/UefiCpuPkg/Library/MpInitLib/Microcode.c
index 5f9ae22794..1d5e964791 100644
--- a/UefiCpuPkg/Library/MpInitLib/Microcode.c
+++ b/UefiCpuPkg/Library/MpInitLib/Microcode.c
@@ -32,6 +32,69 @@ GetCurrentMicrocodeSignature (
   return BiosSignIdMsr.Bits.MicrocodeUpdateSignature;
 }
 
+/**
+  Verify Microcode.
+
+  @param[in]  MicrocodeHeader   Point to the Microcode payload buffer.
+
+  @retval TRUE  Valid payload
+  @retval FALSE Invalid payload
+**/
+BOOLEAN
+VerifyMicrocodeHeader (
+  IN CPU_MICROCODE_HEADER *MicrocodeHeader
+  )
+{
+  UINTN   TotalSize;
+  UINTN   DataSize;
+
+  if (MicrocodeHeader == NULL) {
+return FALSE;
+  }
+
+  ///
+  /// Verify Version
+  ///
+  if (MicrocodeHeader->HeaderVersion != 0x1) {
+return FALSE;
+  }
+  if (MicrocodeHeader->LoaderRevision != 0x1) {
+return FALSE;
+  }
+
+  ///
+  /// Verify TotalSize
+  ///
+  if (MicrocodeHeader->DataSize == 0) {
+TotalSize = 2048;
+  } else {
+TotalSize = MicrocodeHeader->TotalSize;
+  }
+  if (TotalSize <= sizeof (CPU_MICROCODE_HEADER)) {
+return FALSE;
+  }
+  if ((TotalSize & (SIZE_1KB - 1)) != 0) {
+return FALSE;
+  }
+
+  ///
+  /// Verify DataSize
+  ///
+  if (MicrocodeHeader->DataSize == 0) {
+DataSize = 2048 - sizeof (CPU_MICROCODE_HEADER);
+  } else {
+DataSize = MicrocodeHeader->DataSize;
+  }
+  if (DataSize > TotalSize - sizeof (CPU_MICROCODE_HEADER)) {
+return FALSE;
+  }
+  if ((DataSize & 0x3) != 0) {
+return FALSE;
+  }
+
+  return TRUE;
+}
+
 /**
   Detect whether specified processor can find matching microcode patch and 
load it.
 
@@ -166,6 +229,18 @@ MicrocodeDetect (
 //
 CorrectMicrocode = FALSE;
 
+if (!VerifyMicrocodeHeader (MicrocodeEntryPoint)) {
+  //
+  // It is the padding data between the microcode patches for microcode 
patches alignment.
+  // Because the microcode patch is the multiple of 1-KByte, the padding 
data should not
+  // exist if the microcode patch alignment value is not larger than 
1-KByte. So, the microcode
+  // alignment value should be larger than 1-KByte. We could skip SIZE_1KB 
padding data to
+  // find the next possible microcode patch header.
+  //
+  MicrocodeEntryPoint = (CPU_MICROCODE_HEADER *) (((UINTN) 
MicrocodeEntryPoint) + SIZE_1KB);
+  continue;
+}
+
 //
 // Save an in-complete CheckSum32 from CheckSum Part1 for common parts.
 //
@@ -184,90 +259,79 @@ MicrocodeDetect (
 InCompleteCheckSum32 -= MicrocodeEntryPoint->ProcessorFlags;
 InCompleteCheckSum32 -= MicrocodeEntryPoint->Checksum;
 
-if (MicrocodeEntryPoint->HeaderVersion == 0x1) {
+//
+// It is the microcode header. It is not the padding data between 
microcode patches
+// because the padding data should not include 0x0001 and it should be 
the repeated
+// byte format (like 0xXYXYXYXY).
+//
+if (MicrocodeEntryPoint->ProcessorSignature.Uint32 == Eax.Uint32 &&
+MicrocodeEntryPoint->UpdateRevision > LatestRevision &&
+(MicrocodeEntryPoint->ProcessorFlags & (1 << PlatformId))
+) {
   //
-  // It is the microcode header. It is not the padding data between 
microcode patches
-  // because the padding data should not include 0x0001 and it should 
be the repeated
-  // byte format (like 0xXYXYXYXY).
+  // Calculate CheckSum Part1.
   //
-  if (MicrocodeEntryPoint->ProcessorSignature.Uint32 == Eax.Uint32 &&
-  MicrocodeEntryPoint->UpdateRevision > LatestRevision &&
-  (MicrocodeEntryPoint->ProcessorFlags & (1 << PlatformId))
-  ) {
+  CheckSum32 = InCompleteCheckSum32;
+  CheckSum32 += MicrocodeEntryPoint->ProcessorSignature.Uint32;
+  CheckSum32 += MicrocodeEntryPoint->ProcessorFlags;
+  CheckSum32 += MicrocodeEntryPoint->Checksum;
+  if (CheckSum32 == 0) {
+CorrectMicrocode = TRUE;
+ProcessorFlags = MicrocodeEntryPoint->ProcessorFlags;
+  }
+} else if ((MicrocodeEntryPoint->DataSize != 0) &&
+   (MicrocodeEntryPoint->UpdateRevision > LatestRevision)) {
+  ExtendedTableLength = MicrocodeEntryPoint->TotalSize - 
(MicrocodeEntryPoint->DataSize +
+  sizeof (CPU_MICROCODE_HEADER));
+  if (ExtendedTableLength != 0) {
 //
-// Calculate CheckSum Part1.
+// Extended Table exist, check if the CPU in support list
 //
-CheckSum32

[edk2] [PATCH 1/2] UefiCpuPkg/Microcode: Fix InComplete CheckSum32 issue

2019-02-27 Thread Chen A Chen

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

The Microcode region indicated by MicrocodePatchAddress PCD may contain
more than one Microcode entry. We should save InCompleteCheckSum32 value
for each payload. Move the logic for calculate InCompleteCheckSum32 from
the outsize of the do-while loop to the inside.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Chen A Chen 
Cc: Ray Ni 
Cc: Eric Dong 
---
 UefiCpuPkg/Library/MpInitLib/Microcode.c | 37 
 1 file changed, 19 insertions(+), 18 deletions(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/Microcode.c 
b/UefiCpuPkg/Library/MpInitLib/Microcode.c
index e1f661d6b1..5f9ae22794 100644
--- a/UefiCpuPkg/Library/MpInitLib/Microcode.c
+++ b/UefiCpuPkg/Library/MpInitLib/Microcode.c
@@ -159,30 +159,31 @@ MicrocodeDetect (
   MicrocodeEnd = (UINTN) (CpuMpData->MicrocodePatchAddress + 
CpuMpData->MicrocodePatchRegionSize);
   MicrocodeEntryPoint = (CPU_MICROCODE_HEADER *) (UINTN) 
CpuMpData->MicrocodePatchAddress;
 
-  //
-  // Save an in-complete CheckSum32 from CheckSum Part1 for common parts.
-  //
-  if (MicrocodeEntryPoint->DataSize == 0) {
-InCompleteCheckSum32 = CalculateSum32 (
- (UINT32 *) MicrocodeEntryPoint,
- sizeof (CPU_MICROCODE_HEADER) + 2000
- );
-  } else {
-InCompleteCheckSum32 = CalculateSum32 (
- (UINT32 *) MicrocodeEntryPoint,
- sizeof (CPU_MICROCODE_HEADER) + 
MicrocodeEntryPoint->DataSize
- );
-  }
-  InCompleteCheckSum32 -= MicrocodeEntryPoint->ProcessorSignature.Uint32;
-  InCompleteCheckSum32 -= MicrocodeEntryPoint->ProcessorFlags;
-  InCompleteCheckSum32 -= MicrocodeEntryPoint->Checksum;
-
   do {
 //
 // Check if the microcode is for the Cpu and the version is newer
 // and the update can be processed on the platform
 //
 CorrectMicrocode = FALSE;
+
+//
+// Save an in-complete CheckSum32 from CheckSum Part1 for common parts.
+//
+if (MicrocodeEntryPoint->DataSize == 0) {
+  InCompleteCheckSum32 = CalculateSum32 (
+   (UINT32 *) MicrocodeEntryPoint,
+   sizeof (CPU_MICROCODE_HEADER) + 2000
+   );
+} else {
+  InCompleteCheckSum32 = CalculateSum32 (
+   (UINT32 *) MicrocodeEntryPoint,
+   sizeof (CPU_MICROCODE_HEADER) + 
MicrocodeEntryPoint->DataSize
+   );
+}
+InCompleteCheckSum32 -= MicrocodeEntryPoint->ProcessorSignature.Uint32;
+InCompleteCheckSum32 -= MicrocodeEntryPoint->ProcessorFlags;
+InCompleteCheckSum32 -= MicrocodeEntryPoint->Checksum;
+
 if (MicrocodeEntryPoint->HeaderVersion == 0x1) {
   //
   // It is the microcode header. It is not the padding data between 
microcode patches
-- 
2.16.2.windows.1

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

[edk2] [PATCH 0/2] Fix Microcode failure issue

2019-02-27 Thread Chen A Chen

The first patch is used to fix InComplete CheckSum32 issue.
Add a verification logic to check Microcode header before parsing payload.

Chen A Chen (2):
  UefiCpuPkg/Microcode: Fix InComplete CheckSum32 issue
  UefiCpuPkg/Microcode: Add verification logic before parsing payload

 UefiCpuPkg/Library/MpInitLib/Microcode.c | 227 ---
 1 file changed, 146 insertions(+), 81 deletions(-)

-- 
2.16.2.windows.1

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

Re: [edk2] Self-replicating image

2019-02-27 Thread Kinney, Michael D

Hi Tomas,

I have posted a branch in edk2-staging with the proposed
design changes to support multiple controller and a functional
example in OVMF using multiple E1000 adapters.

https://github.com/tianocore/edk2-staging/tree/Bug_1525_FmpDevicePkg_MultipleControllers

QEMU has a few limitations, so some aspects are simulated,
but you should be able to use the example as a template to
try your use case.

The E1000 UEFI PCI device driver that uses FmpDxe as a
Library is located at the following location.

OvmfPkg/E1000Fmp

It uses the E1000 specific FmpDeviceLib instance at:

OvmfPkg/Feature/Capsule/Library/FmpDeviceLibE1000

I have added device specific DSC files for each FMP driver
that are included from the OVMF DSC file.  The one for
E1000 is at:

OvmfPkg/Feature/Capsule/FmpE1000.dsc

QEMU does not support update of the PCI Option ROM 
mapped to a PCI device.  So I simulate the update of a
FW storage device using a 32-byte UEFI Variable that
is unique for each adapter.

Best regards,

Mike

> -Original Message-
> From: Tomas Pilar (tpilar)
> [mailto:tpi...@solarflare.com]
> Sent: Monday, February 11, 2019 2:37 AM
> To: Kinney, Michael D ;
> Andrew Fish ; edk2-devel@lists.01.org
> Subject: Re: [edk2] Self-replicating image
> 
> Hi Mike,
> 
> Sorry to say I have not yet filed a BZ. Would you like
> me to, or are you happy doing it?
> 
> Cheers,
> Tom
> 
> On 08/02/2019 17:59, Kinney, Michael D wrote:
> > Hi Thomas,
> >
> > I have been looking into this topic of multiple
> controllers this
> > week.  I have some prototype code that I think
> resolves the issues
> > but needs a bit more work on some corner cases.
> >
> > I am using the PCI Option ROM use case to evaluate
> the changes.
> > PCI Option ROM can use Bus Specific Driver Override
> Protocol so
> > the driver from the option ROM manages the PCI
> controller the option
> > ROM is attached.  PCI Option ROMs can also use the
> Driver Family
> > Override Protocol so one of the PCI Option ROMs can
> manage a group
> > of PCI controllers.
> >
> > It is also possible for an FMP driver for integrated
> devices to
> > manage multiple integrated devices if there is more
> than one of
> > the same device with FW storage.  The multiple
> controller use case
> > is not limited to busses like PCI.
> >
> > The current version of the FmpDeviceLib is optimized
> for an FMP
> > instance that manages a single FW storage device.  If
> an FmpDeviceLib
> > is intended to manage multiple FW storage devices,
> then a few
> > extra services in the FmpDeviceLib are required.
> >
> > The concept is to extend the FmpDeviceLib with a
> couple extra
> > APIs that add support for Stop()/Unload() operations
> and to
> > to set the context for the current FmpDeviceLib
> actions.
> >
> > Have you entered a BZ for this issue yet?  I can
> start adding
> > details in the BZ and post some proposed changes
> soon.
> >
> > Best regards,
> >
> > Mike
> >
> >> -Original Message-
> >> From: edk2-devel [mailto:edk2-devel-
> >> boun...@lists.01.org] On Behalf Of Andrew Fish via
> >> edk2-devel
> >> Sent: Friday, February 8, 2019 9:16 AM
> >> To: Tomas Pilar (tpilar) 
> >> Cc: edk2-devel@lists.01.org
> >> Subject: Re: [edk2] Self-replicating image
> >>
> >>
> >>
> >>> On Feb 8, 2019, at 6:42 AM, Tomas Pilar (tpilar)
> >>  wrote:
> >>> Hi,
> >>>
> >>> I am currently pondering the most elegant way to
> >> implement capsule update for our devices that would
> >> work in the presence of multiple devices in the
> host.
> >>> Capsule allows embedding a driver that is executed
> >> prior to the update, which is very handy. Crypto
> >> library is quite large and would not fit into an
> >> OptionROM, so being able to supply FMP driver in the
> >> capsule is great.
> >>> However, if only one instance of the driver loads,
> >> the FMP upstream is currently written to support
> only
> >> one device per instance. So I wonder if there is a
> >> easy, neat way for my image to replicate on
> >> DriverBinding so that I end up with one instance per
> >> device.
> >>
> >> Tom,
> >>
> >> The usually model in EFI is to have one driver
> handle
> >> multiple things.
> >>
> >>> It looks like I should be able to do it with gBS-
> >>> LoadImage() and passing information about currently
> >> loaded image though I might have to CopyMem() the
> image
> >> itself to new location.
> >> gBS->LoadImage() will load and relocate the image to
> a
> >> malloced address of the correct memory type for the
> >> image. The inputs are just the source of the image,
> so
> >> no need to CopyMem() anything. gBS->StartImage()
> calls
> >> the entry point.
> >>
> >> Thanks,
> >>
> >> Andrew Fish
> >>
> >>> Thoughts?
> >>>
> >>> Cheers,
> >>> Tom
> >>> ___
> >>> edk2-devel mailing list
> >>> edk2-devel@lists.01.org
> >>> https://lists.01.org/mailman/listinfo/edk2-devel
> >> ___
> >> edk2-devel mailing list
> >>

Re: [edk2] [PATCH] MdeModulePkg/SdMmcPciHcDxe: Fix DMA on SDHC v3 64-bit systems

2019-02-27 Thread Wu, Hao A

Loop Ashish in.
Some comments below.

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Cohen, Eugene
> Sent: Wednesday, February 27, 2019 6:59 PM
> To: edk2-devel@lists.01.org; Wu, Hao A
> Subject: [edk2] [PATCH] MdeModulePkg/SdMmcPciHcDxe: Fix DMA on SDHC
> v3 64-bit systems
> 
> The SdMmcPciHcDriverBindingStart function was checking two
> different capability bits in determining whether 64-bit DMA
> modes were supported, one mode is defined in the SDHC version
> 3 specification (using 96-bit descriptors) and another is
> defined in the SDHC version 4 specification (using 128-bit
> descriptors).  Since the currently implementation of 64-bit
> ADMA2 only supports the SDHC version 4 implementation it is
> incorrect to check the V3 64-bit capability bit since this
> will activate V4 ADMA2 on V3 controllers.

I remember the commit b5547b9ce97e80c3127682a2a5d4b9bd14af353e from Ashish
only handles the controllers with version greater or equal to 4.00.

And the ADMA2 (96-bit Descriptor) mode for V3 controllers is selected by
setting the 'DMA Select' filed in the Host Control 1 Register to 11b. But
the currently behavior of the driver is setting the field to 10b, which I
think will not switch to the ADMA2 (96-bit Descriptor) mode for V3.

Maybe there is something I miss here. Could you help to provide some more
detail on the issue you met? Thanks.

Best Regards,
Hao Wu

> 
> Cc: Hao Wu 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Eugene Cohen 
> ---
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> index b474f8d..5bc91c5 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> @@ -666,8 +666,7 @@ SdMmcPciHcDriverBindingStart (
>  // If any of the slots does not support 64b system bus
>  // do not enable 64b DMA in the PCI layer.
>  //
> -if (Private->Capability[Slot].SysBus64V3 == 0 &&
> -Private->Capability[Slot].SysBus64V4 == 0) {
> +if (Private->Capability[Slot].SysBus64V4 == 0) {
>Support64BitDma = FALSE;
>  }
> 
> --
> 2.7.4
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [staging/Bug_1525_FmpDevicePkg_MultipleControllers] Create new branch

2019-02-27 Thread Kinney, Michael D

# FmpDevicePkg Multiple Controllers

This branch is used to evaluate methods to update the FmpDevicePkg to support
drivers that manage multiple controllers and also provide a firmware update
capability to each managed controller.  The primary use case is the update of
a PCI Option ROM when multiple adapters are present.  This branch addresses
the following TianoCore Bugzilla issues:

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

Branch owner(s):

Michael D Kinney 

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

Re: [edk2] [edk2-announce] Soft Feature Freeze starts today for edk2-stable201903

2019-02-27 Thread Gao, Liming

Laszlo:

>-Original Message-
>From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
>Laszlo Ersek
>Sent: Thursday, February 28, 2019 3:54 AM
>To: Gao, Liming 
>Cc: edk2-devel@lists.01.org
>Subject: Re: [edk2] [edk2-announce] Soft Feature Freeze starts today for
>edk2-stable201903
>
>On 02/27/19 14:16, Gao, Liming wrote:
>> Laszlo:
>>
>> Thanks
>> Liming
>>> -Original Message-
>>> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
>Laszlo Ersek
>>> Sent: Tuesday, February 26, 2019 7:55 PM
>>> To: Gao, Liming 
>>> Cc: edk2-devel@lists.01.org
>>> Subject: Re: [edk2] [edk2-announce] Soft Feature Freeze starts today for
>edk2-stable201903
>>>
>>> On 02/26/19 04:30, Gao, Liming wrote:
 Hi, all
   Two features (Add SMM CET support and Add WiFi Connection Manager)
>get Acked-By or Reviewed-by near the soft feature freeze
>>> date. CET is X86 specific feature. WiFi connection manager is the
>standalone feature. Their impact should be small. So, I suggest to include
>>> them in this stable tag edk2-stable201903. If you have different opinion,
>please raise. If no objection, I will push them late this week.

 Add SMM CET support https://lists.01.org/pipermail/edk2-devel/2019-
>February/037128.html
 NetworkPkg: Add WiFi Connection Manager to NetworkPkg
>https://lists.01.org/pipermail/edk2-devel/2019-February/037137.html
>>>
>>> I think the required feedback tags arrived on time, but just barely. (At
>>> least in my time zone, CET = UTC+01:00.)
>>>
>>> - For Jiewen's "[edk2] [PATCH V3 0/4] Add SMM CET support":
>>>   - Ray's R-b appeared at 02/22/19 15:29
>>>   - My Regression-tested-by appeared at 02/22/19 22:41
>>>
>>> - For Wang Fan's "[edk2] [Patch V2] NetworkPkg: Add WiFi Connection
>>>   Manager to NetworkPkg":
>>>   - Jiaxin's R-b appeared at 02/22/19 08:56.
>>>
>>> I'm OK if both features are pushed.
>>>
>> Thanks.
I push these two features. 
CET feature: 
84110bbe4bb3a346514b9bb12eadb7586bca7dfd..3eb69b081c683f9d825930d0c511e43c0485e5d2
Wifi Conncetion: 90b24889f9ced53c18b73266d507e45fbd94fab0

>>
>>>
>>> However, I do have a suggestion for the announcement email, for the
>>> future. Given that we've already encountered three cases in this cycle
>>> where the feedback tags are on the boundary, I suggest to include an
>>> exact timestamp (in the UTC zone) in each announcement. I never
>expected
>>> that this accuracy would be necessary, but apparently it is.
>>>
>> I suggest to list UTC zone in Proposed Schedule of EDK II Release Planning
>wiki page as below. I prefer to use PST (UST -8), because edk2-devel Archives
>(https://lists.01.org/pipermail/edk2-devel/) uses PST time. Then, we can base
>on the archive mail to check each patch.
>>
>> Date (UTC -8)Description
>> 2019-03-08 Beginning of development
>> 2019-02-22 Soft Feature Freeze
>> 2019-03-01 Hard Feature Freeze
>> 2019-03-08 Release
>
>I think PST (UTC-8) works fine as well, as long as we state it clearly.
>Given the time zone, the timestamp is unique, and can be translated to
>other time zones.
>
>I do recommend that we specify "midnight" or 00:00:00 as well, however,
>if that is our intent. (I'm fine with other hours too, such as "noon"
>etc, but it should be specific down to the second. We should be able to
>look at any email on the list and determine if it was before or after.
>"Equal" should still be accepted.)
>
I update wiki page 
https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Release-Planning 
with UTC zone. 
I think the nature time of day is the begin time of one day. 00:00:00 should be 
common sense. We may not highlight it. 

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

[edk2] [Patch] Document: Update the INF spec about [Depex] section

2019-02-27 Thread Feng, Bob C

 ::= {"BEFORE"} {"AFTER"}  should be
 ::= [{"BEFORE"} {"AFTER"}]

The "BEFORE" or "AFTER" is optional key words in current implementation.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Bob Feng 
Cc: Liming Gao 
---
 3_edk_ii_inf_file_format/314_[depex]_sections.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/3_edk_ii_inf_file_format/314_[depex]_sections.md 
b/3_edk_ii_inf_file_format/314_[depex]_sections.md
index 942bcf9..3c0820a 100644
--- a/3_edk_ii_inf_file_format/314_[depex]_sections.md
+++ b/3_edk_ii_inf_file_format/314_[depex]_sections.md
@@ -179,11 +179,11 @@ and VOID* datum type, and the size of the PCD must be 16 
bytes.
  ::= {} {} {}
::= *
  ["END" ]
  ::= {} {}
::= 
-  ::= {"BEFORE"} {"AFTER"}  []
+  ::= [{"BEFORE"} {"AFTER"}]  []
   ::= {} {}
 ::= "PUSH"  []
 ::= "SOR"  []
::= {} {}
::= {"TRUE"} {"FALSE"} {} {} []
-- 
2.20.1.windows.1

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

[edk2] [Patch] Document: Update Dsc spec to all empty value for HIIPcd

2019-02-27 Thread Feng, Bob C

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

Update Dsc spec to all empty value for HIIPcd.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Bob Feng 
Cc: Liming Gao 
---
 2_dsc_overview/29_pcd_sections.md| 4 ++--
 3_edk_ii_dsc_file_format/310_pcd_sections.md | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/2_dsc_overview/29_pcd_sections.md 
b/2_dsc_overview/29_pcd_sections.md
index 791125f..d84d2f4 100644
--- a/2_dsc_overview/29_pcd_sections.md
+++ b/2_dsc_overview/29_pcd_sections.md
@@ -230,11 +230,11 @@ example:
 `[PcdsDynamicHii.common.Sku1]`
 
 While the format for content of this section is as follows, note that the
 backslash character is used here to indicate the continuation of the line:
 
-`PcdTokenSpaceGuidCName.PcdCName|VariableName|VariableGuid|VariableOffset[|HiiDefaultValue[|HiiAttrubte]]`
+`PcdTokenSpaceGuidCName.PcdCName|VariableName|VariableGuid|VariableOffset[|[HiiDefaultValue][|HiiAttrubte]]`
 
 For VOID* PCDs, the HiiDefaultValue will be a pointer; specifying the optional
 HiiDefaultValue has no meaning.
 
 The optional HII Attribute entry is a comma separated list of attributes as
@@ -340,11 +340,11 @@ Specifying a `SKUID` for an HII PCD selection is 
optional, for example:
 `[PcdsDynamicExHii.common.Sku1]`
 
 While the format for content of this section is as follows, note that the
 backslash character is used here to indicate the continuation of the line:
 
-`PcdTokenSpaceGuidCName.PcdCName|VariableName|VariableGuid|VariableOffset[|HiiDefaultValue]`
+`PcdTokenSpaceGuidCName.PcdCName|VariableName|VariableGuid|VariableOffset[|[HiiDefaultValue]]`
 
 The optional HII Attribute entry is a comma separated list of attributes as
 described in Table 9 HII Attributes.
 
 **Note:** The VariableName field in the HII format PCD entry must not be an 
empty string.
diff --git a/3_edk_ii_dsc_file_format/310_pcd_sections.md 
b/3_edk_ii_dsc_file_format/310_pcd_sections.md
index f9f1359..f982d60 100644
--- a/3_edk_ii_dsc_file_format/310_pcd_sections.md
+++ b/3_edk_ii_dsc_file_format/310_pcd_sections.md
@@ -503,11 +503,11 @@ sections of the DSC file.
  ::= {} {} [ ]
::= 
   ::= {} {}
  ::= []
::= 
- ::=   [ ]
+ ::=  [] [ ]
  ::= 
::= if (pcddatumtype == "BOOLEAN"):
{} {}
  elif (pcddatumtype == "UINT8"):
{} {}
@@ -721,11 +721,11 @@ sections of the DSC file.
  ::= {} {} [ ]
::= 
   ::= {} {}
  ::= []
::= 
- ::=   [ ]
+ ::=  [] [ ]
  ::= 
::= if (pcddatumtype == "BOOLEAN"):
{} {}
  elif (pcddatumtype == "UINT8"):
{} {}
-- 
2.20.1.windows.1

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

Re: [edk2] [Patch] Revert "BaseTools:BaseTools supports to the driver combination."

2019-02-27 Thread Feng, Bob C

Reviewed-by: Bob Feng 

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Liming 
Gao
Sent: Monday, February 25, 2019 8:15 AM
To: edk2-devel@lists.01.org
Subject: [edk2] [Patch] Revert "BaseTools:BaseTools supports to the driver 
combination."

This reverts commit 838bc257bae3f9fc6723f41f3980f6cfbedb77e5.
After further evaluation, there are the unclear behavior in for the driver 
combination feature. To not impact Q1 stable tag, remove it first.
1. If the drivers to be combined have the different PCD or library instance
   setting, build should not combine them and report build break. But this
   commit doesn't consider this case.
2. When start the sub driver fail, continue to start other sub driver. This
   behavior is required to be clarifed in build spec. 
3. Unload the sub driver when the combined driver start fail. This case need 
   to call the sub driver unload function for the driver start fail only.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Liming Gao 
---
 BaseTools/Source/Python/AutoGen/GenC.py| 32 --
 .../Source/Python/Workspace/WorkspaceCommon.py |  8 --
 2 files changed, 5 insertions(+), 35 deletions(-)

diff --git a/BaseTools/Source/Python/AutoGen/GenC.py 
b/BaseTools/Source/Python/AutoGen/GenC.py
index 0f1b3bb9a3..a922464f56 100644
--- a/BaseTools/Source/Python/AutoGen/GenC.py
+++ b/BaseTools/Source/Python/AutoGen/GenC.py
@@ -1457,25 +1457,10 @@ def CreateLibraryDestructorCode(Info, AutoGenC, 
AutoGenH):
 def CreateModuleEntryPointCode(Info, AutoGenC, AutoGenH):
 if Info.IsLibrary or Info.ModuleType in [SUP_MODULE_USER_DEFINED, 
SUP_MODULE_SEC]:
 return
-ModuleEntryPointList = []
-for Lib in Info.DependentLibraryList:
-if len(Lib.ModuleEntryPointList) > 0:
-if Lib.ModuleType == Info.ModuleType:
-ModuleEntryPointList = ModuleEntryPointList + 
Lib.ModuleEntryPointList
-else:
-EdkLogger.error(
-"build",
-PREBUILD_ERROR,
-"Driver's ModuleType must be consistent [%s]"%(str(Lib)),
-File=str(Info.PlatformInfo),
-ExtraData="consumed by [%s]" % str(Info.MetaFile)
-)
-ModuleEntryPointList = ModuleEntryPointList + 
Info.Module.ModuleEntryPointList
-
 #
 # Module Entry Points
 #
-NumEntryPoints = len(ModuleEntryPointList)
+NumEntryPoints = len(Info.Module.ModuleEntryPointList)
 if 'PI_SPECIFICATION_VERSION' in Info.Module.Specification:
 PiSpecVersion = Info.Module.Specification['PI_SPECIFICATION_VERSION']
 else:
@@ -1485,7 +1470,7 @@ def CreateModuleEntryPointCode(Info, AutoGenC, AutoGenH):
 else:
 UefiSpecVersion = '0x'
 Dict = {
-'Function'   :   ModuleEntryPointList,
+'Function'   :   Info.Module.ModuleEntryPointList,
 'PiSpecVersion'  :   PiSpecVersion + 'U',
 'UefiSpecVersion':   UefiSpecVersion + 'U'
 }
@@ -1498,7 +1483,7 @@ def CreateModuleEntryPointCode(Info, AutoGenC, AutoGenH):
   AUTOGEN_ERROR,
   '%s must have exactly one entry point' % Info.ModuleType,
   File=str(Info),
-  ExtraData= ", ".join(ModuleEntryPointList)
+  ExtraData= ", 
+ ".join(Info.Module.ModuleEntryPointList)
   )
 if Info.ModuleType == SUP_MODULE_PEI_CORE:
 AutoGenC.Append(gPeiCoreEntryPointString.Replace(Dict))
@@ -1552,18 +1537,11 @@ def CreateModuleEntryPointCode(Info, AutoGenC, 
AutoGenH):
 def CreateModuleUnloadImageCode(Info, AutoGenC, AutoGenH):
 if Info.IsLibrary or Info.ModuleType in [SUP_MODULE_USER_DEFINED, 
SUP_MODULE_BASE, SUP_MODULE_SEC]:
 return
-
-ModuleUnloadImageList = []
-for Lib in Info.DependentLibraryList:
-if len(Lib.ModuleUnloadImageList) > 0:
-ModuleUnloadImageList = ModuleUnloadImageList + 
Lib.ModuleUnloadImageList
-ModuleUnloadImageList = ModuleUnloadImageList + 
Info.Module.ModuleUnloadImageList
-
 #
 # Unload Image Handlers
 #
-NumUnloadImage = len(ModuleUnloadImageList)
-Dict = {'Count':str(NumUnloadImage) + 'U', 
'Function':ModuleUnloadImageList}
+NumUnloadImage = len(Info.Module.ModuleUnloadImageList)
+Dict = {'Count':str(NumUnloadImage) + 'U', 
+ 'Function':Info.Module.ModuleUnloadImageList}
 if NumUnloadImage < 2:
 AutoGenC.Append(gUefiUnloadImageString[NumUnloadImage].Replace(Dict))
 else:
diff --git a/BaseTools/Source/Python/Workspace/WorkspaceCommon.py 
b/BaseTools/Source/Python/Workspace/WorkspaceCommon.py
index 22abda8743..b79280bc2e 100644
--- a/BaseTools/Source/Python/Workspace/WorkspaceCommon.py
+++ b/BaseTools/Source/Python/Workspace/WorkspaceCommon.py
@@ -20,8 +20,6 @@ from Workspace.BuildClassObject import StructurePcd  from 
Common.BuildToolError import

[edk2] [Patch V2] BaseTools: Add python3-distutils Ubuntu package checking

2019-02-27 Thread Feng, Bob C

https://bugzilla.tianocore.org/show_bug.cgi?id=1509
V2:
Remove OS/Package specific words. Print the error info which
is from python error message.

Add python3-distutils Ubuntu package checking.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Bob Feng 
Cc: Liming Gao 
---
 BaseTools/Tests/RunTests.py | 12 
 1 file changed, 12 insertions(+)

diff --git a/BaseTools/Tests/RunTests.py b/BaseTools/Tests/RunTests.py
index 0dd65632d0..f6d3f43653 100644
--- a/BaseTools/Tests/RunTests.py
+++ b/BaseTools/Tests/RunTests.py
@@ -17,10 +17,22 @@
 #
 import os
 import sys
 import unittest
 
+distutils_exist = True
+try:
+import distutils.util
+except:
+distutils_exist = False
+
+if not distutils_exist:
+print("""
+Python report "No module named 'distutils.uitl'"
+""")
+sys.exit(-1)
+
 import TestTools
 
 def GetCTestSuite():
 import CToolsTests
 return CToolsTests.TheTestSuite()
-- 
2.20.1.windows.1

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

Re: [edk2] [PATCH v2 0/2] MdeModulePkg: Resolve buffer cross boundary access in Ramdisk

2019-02-27 Thread Gao, Liming

I update 
https://github.com/tianocore/tianocore.github.io/wiki/Commit-Message-Format 
with CVE example. Please check it. 

>-Original Message-
>From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
>Laszlo Ersek
>Sent: Thursday, February 28, 2019 3:31 AM
>To: Gao, Liming ; Wu, Hao A ;
>edk2-devel@lists.01.org
>Cc: Zeng, Star 
>Subject: Re: [edk2] [PATCH v2 0/2] MdeModulePkg: Resolve buffer cross
>boundary access in Ramdisk
>
>On 02/27/19 13:49, Gao, Liming wrote:
>> Laszlo:
>>   I add my comments.
>>
>> Thanks
>> Liming
>>> -Original Message-
>>> From: Laszlo Ersek [mailto:ler...@redhat.com]
>>> Sent: Wednesday, February 27, 2019 4:58 PM
>>> To: Wu, Hao A ; Gao, Liming
>; edk2-devel@lists.01.org
>>> Cc: Zeng, Star 
>>> Subject: Re: [edk2] [PATCH v2 0/2] MdeModulePkg: Resolve buffer cross
>boundary access in Ramdisk
>>>
>>> On 02/27/19 07:56, Wu, Hao A wrote:
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf
>Of
> Laszlo Ersek
> Sent: Tuesday, February 26, 2019 7:45 PM
> To: Wu, Hao A; edk2-devel@lists.01.org
> Cc: Zeng, Star
> Subject: Re: [edk2] [PATCH v2 0/2] MdeModulePkg: Resolve buffer
>cross
> boundary access in Ramdisk
>
> On 02/26/19 08:45, Hao Wu wrote:
>> V2 changes:
>>
>> Correct CC list information.
>>
>>
>> V1 history:
>>
>> The series will resolve a buffer cross boundary access issue during the
>> use of RAM disks. It is the mitigation for issue CVE-2018-12180.
>>
>> Cc: Jian J Wang 
>> Cc: Ray Ni 
>> Cc: Star Zeng 
>>
>> Hao Wu (2):
>>   MdeModulePkg/PartitionDxe: Ensure blocksize can hold MBR (CVE
>FIX)
>>   MdeModulePkg/RamDiskDxe: Ramdisk size be multiple of BlkSize
>(CVE
> FIX)
>>
>>  MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskImpl.h |  6
>+++---
>>  MdeModulePkg/Universal/Disk/PartitionDxe/Gpt.c   |  9
>-
>>  MdeModulePkg/Universal/Disk/PartitionDxe/Mbr.c   |  9
>-
>>  MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskBlockIo.c  | 20
> ++--
>>  MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskProtocol.c |  5
>+++--
>>  5 files changed, 36 insertions(+), 13 deletions(-)
>>
>
> Please put the exact CVE numbers in the subject lines.

 Hello Laszlo and Liming,

 I totally agree the commit subject line should include the CVE number.
 But I have one feedback that, if the commit is for a CVE fix, is it
 possible to exempt the commit subject from 71 characters limit?
>>>
>>> In my opinion, that is absolutely the case.
>>>
 I found it can be hard to summary the commit with the Package/Module
>plus
 the CVE number information.
>>>
>>> I agree, it is hard. But, IMO, in this case, the precise CVE reference
>>> takes priority.
>>>
>> For this case, I suggest to allow subject line length to be bigger, such as 
>> 120
>character.
>> I will update wiki
>https://github.com/tianocore/tianocore.github.io/wiki/Commit-Message-
>Format for CVE commit message format.
>> For example: Pkg-Module: Brief-single-line-summary (CVE-Year-Number)
>
>Thanks for that!
>Laszlo
>___
>edk2-devel mailing list
>edk2-devel@lists.01.org
>https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] Maintainers.txt: clarify Reviewer requirements and responsibilities

2019-02-27 Thread Andrew Fish via edk2-devel

Reviewed-by: Andrew Fish 

> On Feb 27, 2019, at 5:12 PM, Gao, Liming  wrote:
> 
> Reviewed-by: Liming Gao 
> 
>> -Original Message-
>> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
>> Laszlo Ersek
>> Sent: Thursday, February 28, 2019 5:22 AM
>> To: edk2-devel-01 
>> Cc: Gao, Liming ; Kinney, Michael D
>> 
>> Subject: [edk2] [PATCH] Maintainers.txt: clarify Reviewer requirements and
>> responsibilities
>> 
>> The current language for "Package Reviewer" only vaguely hints that
>> Package Reviewers should be able to provide guidance and directions.
>> Make this more obvious.
>> 
>> Cc: Andrew Fish 
>> Cc: Ard Biesheuvel 
>> Cc: Leif Lindholm 
>> Cc: Liming Gao 
>> Cc: Michael D Kinney 
>> Cc: Philippe Mathieu-Daude 
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Laszlo Ersek 
>> ---
>> 
>> Notes:
>>   - this is clearly a political patch; feel free to disagree
>>   - I'm proposing this for the next development cycle
>> 
>> Maintainers.txt | 5 -
>> 1 file changed, 4 insertions(+), 1 deletion(-)
>> 
>> diff --git a/Maintainers.txt b/Maintainers.txt
>> index 7772926b2fb1..ed090aeb4bdb 100644
>> --- a/Maintainers.txt
>> +++ b/Maintainers.txt
>> @@ -20,7 +20,10 @@ Descriptions of section entries:
>>  M: Package Maintainer: Cc address for patches and questions. Responsible
>> for reviewing and pushing package changes to source control.
>>  R: Package Reviewer: Cc address for patches and questions. Reviewers help
>> - maintainers review code, but don't have push access.
>> + maintainers review code, but don't have push access. A designated
>> Package
>> + Reviewer is reasonably familiar with the Package (or some modules
>> + thereof), and/or provides testing or regression testing for the Package
>> + (or some modules thereof), in certain platforms and environments.
>>  W: Web-page with status/info
>>  T: SCM tree type and location.  Type is one of: git, svn.
>>  S: Status, one of the following:
>> --
>> 2.19.1.3.g30247aa5d201
>> 
>> ___
>> edk2-devel mailing list
>> edk2-devel@lists.01.org
>> https://lists.01.org/mailman/listinfo/edk2-devel
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel

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

Re: [edk2] [PATCH] Maintainers.txt: clarify Reviewer requirements and responsibilities

2019-02-27 Thread Gao, Liming

Reviewed-by: Liming Gao 

>-Original Message-
>From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
>Laszlo Ersek
>Sent: Thursday, February 28, 2019 5:22 AM
>To: edk2-devel-01 
>Cc: Gao, Liming ; Kinney, Michael D
>
>Subject: [edk2] [PATCH] Maintainers.txt: clarify Reviewer requirements and
>responsibilities
>
>The current language for "Package Reviewer" only vaguely hints that
>Package Reviewers should be able to provide guidance and directions.
>Make this more obvious.
>
>Cc: Andrew Fish 
>Cc: Ard Biesheuvel 
>Cc: Leif Lindholm 
>Cc: Liming Gao 
>Cc: Michael D Kinney 
>Cc: Philippe Mathieu-Daude 
>Contributed-under: TianoCore Contribution Agreement 1.1
>Signed-off-by: Laszlo Ersek 
>---
>
>Notes:
>- this is clearly a political patch; feel free to disagree
>- I'm proposing this for the next development cycle
>
> Maintainers.txt | 5 -
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
>diff --git a/Maintainers.txt b/Maintainers.txt
>index 7772926b2fb1..ed090aeb4bdb 100644
>--- a/Maintainers.txt
>+++ b/Maintainers.txt
>@@ -20,7 +20,10 @@ Descriptions of section entries:
>   M: Package Maintainer: Cc address for patches and questions. Responsible
>  for reviewing and pushing package changes to source control.
>   R: Package Reviewer: Cc address for patches and questions. Reviewers help
>- maintainers review code, but don't have push access.
>+ maintainers review code, but don't have push access. A designated
>Package
>+ Reviewer is reasonably familiar with the Package (or some modules
>+ thereof), and/or provides testing or regression testing for the Package
>+ (or some modules thereof), in certain platforms and environments.
>   W: Web-page with status/info
>   T: SCM tree type and location.  Type is one of: git, svn.
>   S: Status, one of the following:
>--
>2.19.1.3.g30247aa5d201
>
>___
>edk2-devel mailing list
>edk2-devel@lists.01.org
>https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH] Maintainers.txt: clarify Reviewer requirements and responsibilities

2019-02-27 Thread Laszlo Ersek

The current language for "Package Reviewer" only vaguely hints that
Package Reviewers should be able to provide guidance and directions.
Make this more obvious.

Cc: Andrew Fish 
Cc: Ard Biesheuvel 
Cc: Leif Lindholm 
Cc: Liming Gao 
Cc: Michael D Kinney 
Cc: Philippe Mathieu-Daude 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek 
---

Notes:
- this is clearly a political patch; feel free to disagree
- I'm proposing this for the next development cycle

 Maintainers.txt | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/Maintainers.txt b/Maintainers.txt
index 7772926b2fb1..ed090aeb4bdb 100644
--- a/Maintainers.txt
+++ b/Maintainers.txt
@@ -20,7 +20,10 @@ Descriptions of section entries:
   M: Package Maintainer: Cc address for patches and questions. Responsible
  for reviewing and pushing package changes to source control.
   R: Package Reviewer: Cc address for patches and questions. Reviewers help
- maintainers review code, but don't have push access.
+ maintainers review code, but don't have push access. A designated Package
+ Reviewer is reasonably familiar with the Package (or some modules
+ thereof), and/or provides testing or regression testing for the Package
+ (or some modules thereof), in certain platforms and environments.
   W: Web-page with status/info
   T: SCM tree type and location.  Type is one of: git, svn.
   S: Status, one of the following:
-- 
2.19.1.3.g30247aa5d201

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

Re: [edk2] Hang when calling ExitBootServices on IA32 firmware v1.0 on MinnowBoard Turbot

2019-02-27 Thread Telemann Pioneer

Hi Rebecca,



MinnowBoard Turbot Dual Ethernet runs with UDK2018



https://github.com/MinnowWare/UDK2018-MinnowBoard



That BIOS starts a 64Bit Windows10, including S3 and resume…



Phil




From: edk2-devel  on behalf of Rebecca Cran 
via edk2-devel 
Sent: Wednesday, February 27, 2019 12:00:45 AM
To: edk2-devel@lists.01.org
Subject: Re: [edk2] Hang when calling ExitBootServices on IA32 firmware v1.0 on 
MinnowBoard Turbot

On 2/25/19 5:08 PM, Rebecca Cran via edk2-devel wrote:
> I've been trying to test a boot loader on my MinnowBoard Turbot board.
> It's running the latest 1.0 firmware from firmware.intel.com, and I'm
> seeing a hang at the point when gBS->ExitBootServices is called.


I did more debugging using OVMF and found that the i386 boot loader code
was trying to call printf (which allocates memory) after
ExitBootServices. And that booting an i386 kernel isn't actually
supported. So there's not a problem with the MinnowBoard firmware.


--
Rebecca Cran

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

Re: [edk2] [PATCH v1 0/6] Revert DynamicTablesPkg: Framework updates and fixes

2019-02-27 Thread Laszlo Ersek

On 02/27/19 14:44, Sami Mujawar wrote:
> Hi Laszlo,
> 
> The DynamicTablesPkg won't be in use until the corresponding edk2-platform 
> patches for Dynamic Tables are merged (at which point it would be a major 
> feature addition).
> I think for now, I will hold on until the stable release is tagged.


Thank you for the info; that works fine too, in my opinion.

Laszlo

> -Original Message-
> From: Laszlo Ersek  
> Sent: 26 February 2019 11:06 AM
> To: Sami Mujawar ; edk2-devel@lists.01.org
> Cc: Stephanie Hughes-Fitt ; Alexei Fedorov 
> ; nd 
> Subject: Re: [edk2] [PATCH v1 0/6] Revert DynamicTablesPkg: Framework updates 
> and fixes
> 
> On 02/26/19 12:01, Laszlo Ersek wrote:
>> On 02/26/19 09:44, Sami Mujawar wrote:
>>> Reverting this patch series as Soft Feature Freeze for
>>> edk2-stable201903 started on 22 Feb 2019.
>>>
>>> Cc: Laszlo Ersek 
>>> Cc: Alexei Fedorov 
>>> Contributed-under: TianoCore Contribution Agreement 1.1
>>> Signed-off-by: Sami Mujawar 
>>>
>>> The changes can be seen at 
>>> https://github.com/samimujawar/edk2/tree/Revert_473_dynamic_tables_fr
>>> amework_v1
>>>
>>> Sami Mujawar (6):
>>>   Revert "DynamicTablesPkg: Minor updates and fix typos"
>>>   Revert "DynamicTablesPkg: Remove GIC Distributor Id field"
>>>   Revert "DynamicTablesPkg: DGB2: Update DBG2_DEBUG_PORT_DDI"
>>>   Revert "DynamicTablesPkg: Add OEM Info"
>>>   Revert "DynamicTablesPkg: Rename enum used for ID Mapping"
>>>   Revert "DynamicTablesPkg: Fix protocol section"
>>>
>>>  
>>> DynamicTablesPkg/Drivers/DynamicTableFactoryDxe/DynamicTableFactoryDxe.inf 
>>> |  7 +-  
>>> DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.inf 
>>> |  7 +-
>>>  DynamicTablesPkg/Include/ArmNameSpaceObjects.h 
>>> | 73 +---
>>>  DynamicTablesPkg/Include/Library/TableHelperLib.h  
>>> |  4 +-
>>>  DynamicTablesPkg/Include/StandardNameSpaceObjects.h
>>> | 18 -
>>>  DynamicTablesPkg/Library/Acpi/Arm/AcpiDbg2LibArm/Dbg2Generator.c   
>>> |  7 +-
>>>  DynamicTablesPkg/Library/Acpi/Arm/AcpiFadtLibArm/FadtGenerator.c   
>>> |  2 +-
>>>  DynamicTablesPkg/Library/Acpi/Arm/AcpiGtdtLibArm/GtdtGenerator.c   
>>> |  2 +-
>>>  DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.c   
>>> |  8 +--
>>>  DynamicTablesPkg/Library/Acpi/Arm/AcpiMadtLibArm/MadtGenerator.c   
>>> |  6 +-
>>>  DynamicTablesPkg/Library/Acpi/Arm/AcpiMcfgLibArm/McfgGenerator.c   
>>> |  2 +-
>>>  DynamicTablesPkg/Library/Acpi/Arm/AcpiSpcrLibArm/SpcrGenerator.c   
>>> |  2 +-
>>>  DynamicTablesPkg/Library/Common/TableHelperLib/TableHelper.c   
>>> | 26 ++-
>>>  13 files changed, 48 insertions(+), 116 deletions(-)
>>>
>>
>> Thank you. Sorry about the inconvenience.
>>
>> Acked-by: Laszlo Ersek 
>>
>> Laszlo
>>
> 
> Note: if you have small individual patches that cleanly qualify as bugfixes, 
> especially for features introduced during this development cycle (since the 
> last table tag), those should be acceptable even during the hard feature 
> freeze.
> 
> https://github.com/tianocore/tianocore.github.io/wiki/HardFeatureFreeze
> 
> So if you have fixes like that (possibly a subset of the present patch set), 
> I certainly encourage you to repost those. I'm not familiar with 
> DynamicTablesPkg, and so I can't evaluate this question myself, on a 
> patch-by-patch basis. It's also possible that you'll have to split out parts 
> of larger patches (refactor them) so that only the strict-sense fixes can be 
> posted and applied.
> 
> Thanks!
> Laszlo
> 

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

Re: [edk2] [edk2-announce] Soft Feature Freeze starts today for edk2-stable201903

2019-02-27 Thread Laszlo Ersek

On 02/27/19 14:16, Gao, Liming wrote:
> Laszlo:
> 
> Thanks
> Liming
>> -Original Message-
>> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
>> Laszlo Ersek
>> Sent: Tuesday, February 26, 2019 7:55 PM
>> To: Gao, Liming 
>> Cc: edk2-devel@lists.01.org
>> Subject: Re: [edk2] [edk2-announce] Soft Feature Freeze starts today for 
>> edk2-stable201903
>>
>> On 02/26/19 04:30, Gao, Liming wrote:
>>> Hi, all
>>>   Two features (Add SMM CET support and Add WiFi Connection Manager) get 
>>> Acked-By or Reviewed-by near the soft feature freeze
>> date. CET is X86 specific feature. WiFi connection manager is the standalone 
>> feature. Their impact should be small. So, I suggest to include
>> them in this stable tag edk2-stable201903. If you have different opinion, 
>> please raise. If no objection, I will push them late this week.
>>>
>>> Add SMM CET support 
>>> https://lists.01.org/pipermail/edk2-devel/2019-February/037128.html
>>> NetworkPkg: Add WiFi Connection Manager to NetworkPkg 
>>> https://lists.01.org/pipermail/edk2-devel/2019-February/037137.html
>>
>> I think the required feedback tags arrived on time, but just barely. (At
>> least in my time zone, CET = UTC+01:00.)
>>
>> - For Jiewen's "[edk2] [PATCH V3 0/4] Add SMM CET support":
>>   - Ray's R-b appeared at 02/22/19 15:29
>>   - My Regression-tested-by appeared at 02/22/19 22:41
>>
>> - For Wang Fan's "[edk2] [Patch V2] NetworkPkg: Add WiFi Connection
>>   Manager to NetworkPkg":
>>   - Jiaxin's R-b appeared at 02/22/19 08:56.
>>
>> I'm OK if both features are pushed.
>>
> Thanks.
> 
>>
>> However, I do have a suggestion for the announcement email, for the
>> future. Given that we've already encountered three cases in this cycle
>> where the feedback tags are on the boundary, I suggest to include an
>> exact timestamp (in the UTC zone) in each announcement. I never expected
>> that this accuracy would be necessary, but apparently it is.
>>
> I suggest to list UTC zone in Proposed Schedule of EDK II Release Planning 
> wiki page as below. I prefer to use PST (UST -8), because edk2-devel Archives 
> (https://lists.01.org/pipermail/edk2-devel/) uses PST time. Then, we can base 
> on the archive mail to check each patch. 
> 
> Date (UTC -8) Description
> 2019-03-08 Beginning of development
> 2019-02-22 Soft Feature Freeze
> 2019-03-01 Hard Feature Freeze
> 2019-03-08 Release

I think PST (UTC-8) works fine as well, as long as we state it clearly.
Given the time zone, the timestamp is unique, and can be translated to
other time zones.

I do recommend that we specify "midnight" or 00:00:00 as well, however,
if that is our intent. (I'm fine with other hours too, such as "noon"
etc, but it should be specific down to the second. We should be able to
look at any email on the list and determine if it was before or after.
"Equal" should still be accepted.)

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

Re: [edk2] [PATCH v2 0/2] MdeModulePkg: Resolve buffer cross boundary access in Ramdisk

2019-02-27 Thread Laszlo Ersek

On 02/27/19 13:49, Gao, Liming wrote:
> Laszlo:
>   I add my comments. 
> 
> Thanks
> Liming
>> -Original Message-
>> From: Laszlo Ersek [mailto:ler...@redhat.com]
>> Sent: Wednesday, February 27, 2019 4:58 PM
>> To: Wu, Hao A ; Gao, Liming ; 
>> edk2-devel@lists.01.org
>> Cc: Zeng, Star 
>> Subject: Re: [edk2] [PATCH v2 0/2] MdeModulePkg: Resolve buffer cross 
>> boundary access in Ramdisk
>>
>> On 02/27/19 07:56, Wu, Hao A wrote:
 -Original Message-
 From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
 Laszlo Ersek
 Sent: Tuesday, February 26, 2019 7:45 PM
 To: Wu, Hao A; edk2-devel@lists.01.org
 Cc: Zeng, Star
 Subject: Re: [edk2] [PATCH v2 0/2] MdeModulePkg: Resolve buffer cross
 boundary access in Ramdisk

 On 02/26/19 08:45, Hao Wu wrote:
> V2 changes:
>
> Correct CC list information.
>
>
> V1 history:
>
> The series will resolve a buffer cross boundary access issue during the
> use of RAM disks. It is the mitigation for issue CVE-2018-12180.
>
> Cc: Jian J Wang 
> Cc: Ray Ni 
> Cc: Star Zeng 
>
> Hao Wu (2):
>   MdeModulePkg/PartitionDxe: Ensure blocksize can hold MBR (CVE FIX)
>   MdeModulePkg/RamDiskDxe: Ramdisk size be multiple of BlkSize (CVE
 FIX)
>
>  MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskImpl.h |  6 +++---
>  MdeModulePkg/Universal/Disk/PartitionDxe/Gpt.c   |  9 -
>  MdeModulePkg/Universal/Disk/PartitionDxe/Mbr.c   |  9 -
>  MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskBlockIo.c  | 20
 ++--
>  MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskProtocol.c |  5 +++--
>  5 files changed, 36 insertions(+), 13 deletions(-)
>

 Please put the exact CVE numbers in the subject lines.
>>>
>>> Hello Laszlo and Liming,
>>>
>>> I totally agree the commit subject line should include the CVE number.
>>> But I have one feedback that, if the commit is for a CVE fix, is it
>>> possible to exempt the commit subject from 71 characters limit?
>>
>> In my opinion, that is absolutely the case.
>>
>>> I found it can be hard to summary the commit with the Package/Module plus
>>> the CVE number information.
>>
>> I agree, it is hard. But, IMO, in this case, the precise CVE reference
>> takes priority.
>>
> For this case, I suggest to allow subject line length to be bigger, such as 
> 120 character.
> I will update wiki 
> https://github.com/tianocore/tianocore.github.io/wiki/Commit-Message-Format 
> for CVE commit message format. 
> For example: Pkg-Module: Brief-single-line-summary (CVE-Year-Number)

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

Re: [edk2] [PATCH v1 0/6] Revert DynamicTablesPkg: Framework updates and fixes

2019-02-27 Thread Sami Mujawar

Hi Laszlo,

The DynamicTablesPkg won't be in use until the corresponding edk2-platform 
patches for Dynamic Tables are merged (at which point it would be a major 
feature addition).
I think for now, I will hold on until the stable release is tagged.

Regards,

Sami Mujawar
-Original Message-
From: Laszlo Ersek  
Sent: 26 February 2019 11:06 AM
To: Sami Mujawar ; edk2-devel@lists.01.org
Cc: Stephanie Hughes-Fitt ; Alexei Fedorov 
; nd 
Subject: Re: [edk2] [PATCH v1 0/6] Revert DynamicTablesPkg: Framework updates 
and fixes

On 02/26/19 12:01, Laszlo Ersek wrote:
> On 02/26/19 09:44, Sami Mujawar wrote:
>> Reverting this patch series as Soft Feature Freeze for
>> edk2-stable201903 started on 22 Feb 2019.
>>
>> Cc: Laszlo Ersek 
>> Cc: Alexei Fedorov 
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Sami Mujawar 
>>
>> The changes can be seen at 
>> https://github.com/samimujawar/edk2/tree/Revert_473_dynamic_tables_fr
>> amework_v1
>>
>> Sami Mujawar (6):
>>   Revert "DynamicTablesPkg: Minor updates and fix typos"
>>   Revert "DynamicTablesPkg: Remove GIC Distributor Id field"
>>   Revert "DynamicTablesPkg: DGB2: Update DBG2_DEBUG_PORT_DDI"
>>   Revert "DynamicTablesPkg: Add OEM Info"
>>   Revert "DynamicTablesPkg: Rename enum used for ID Mapping"
>>   Revert "DynamicTablesPkg: Fix protocol section"
>>
>>  
>> DynamicTablesPkg/Drivers/DynamicTableFactoryDxe/DynamicTableFactoryDxe.inf | 
>>  7 +-  
>> DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.inf | 
>>  7 +-
>>  DynamicTablesPkg/Include/ArmNameSpaceObjects.h 
>> | 73 +---
>>  DynamicTablesPkg/Include/Library/TableHelperLib.h  
>> |  4 +-
>>  DynamicTablesPkg/Include/StandardNameSpaceObjects.h
>> | 18 -
>>  DynamicTablesPkg/Library/Acpi/Arm/AcpiDbg2LibArm/Dbg2Generator.c   
>> |  7 +-
>>  DynamicTablesPkg/Library/Acpi/Arm/AcpiFadtLibArm/FadtGenerator.c   
>> |  2 +-
>>  DynamicTablesPkg/Library/Acpi/Arm/AcpiGtdtLibArm/GtdtGenerator.c   
>> |  2 +-
>>  DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.c   
>> |  8 +--
>>  DynamicTablesPkg/Library/Acpi/Arm/AcpiMadtLibArm/MadtGenerator.c   
>> |  6 +-
>>  DynamicTablesPkg/Library/Acpi/Arm/AcpiMcfgLibArm/McfgGenerator.c   
>> |  2 +-
>>  DynamicTablesPkg/Library/Acpi/Arm/AcpiSpcrLibArm/SpcrGenerator.c   
>> |  2 +-
>>  DynamicTablesPkg/Library/Common/TableHelperLib/TableHelper.c   
>> | 26 ++-
>>  13 files changed, 48 insertions(+), 116 deletions(-)
>>
> 
> Thank you. Sorry about the inconvenience.
> 
> Acked-by: Laszlo Ersek 
> 
> Laszlo
> 

Note: if you have small individual patches that cleanly qualify as bugfixes, 
especially for features introduced during this development cycle (since the 
last table tag), those should be acceptable even during the hard feature freeze.

https://github.com/tianocore/tianocore.github.io/wiki/HardFeatureFreeze

So if you have fixes like that (possibly a subset of the present patch set), I 
certainly encourage you to repost those. I'm not familiar with 
DynamicTablesPkg, and so I can't evaluate this question myself, on a 
patch-by-patch basis. It's also possible that you'll have to split out parts of 
larger patches (refactor them) so that only the strict-sense fixes can be 
posted and applied.

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

Re: [edk2] [edk2-announce] Soft Feature Freeze starts today for edk2-stable201903

2019-02-27 Thread Gao, Liming

Laszlo:

Thanks
Liming
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Laszlo 
> Ersek
> Sent: Tuesday, February 26, 2019 7:55 PM
> To: Gao, Liming 
> Cc: edk2-devel@lists.01.org
> Subject: Re: [edk2] [edk2-announce] Soft Feature Freeze starts today for 
> edk2-stable201903
> 
> On 02/26/19 04:30, Gao, Liming wrote:
> > Hi, all
> >   Two features (Add SMM CET support and Add WiFi Connection Manager) get 
> > Acked-By or Reviewed-by near the soft feature freeze
> date. CET is X86 specific feature. WiFi connection manager is the standalone 
> feature. Their impact should be small. So, I suggest to include
> them in this stable tag edk2-stable201903. If you have different opinion, 
> please raise. If no objection, I will push them late this week.
> >
> > Add SMM CET support 
> > https://lists.01.org/pipermail/edk2-devel/2019-February/037128.html
> > NetworkPkg: Add WiFi Connection Manager to NetworkPkg 
> > https://lists.01.org/pipermail/edk2-devel/2019-February/037137.html
> 
> I think the required feedback tags arrived on time, but just barely. (At
> least in my time zone, CET = UTC+01:00.)
> 
> - For Jiewen's "[edk2] [PATCH V3 0/4] Add SMM CET support":
>   - Ray's R-b appeared at 02/22/19 15:29
>   - My Regression-tested-by appeared at 02/22/19 22:41
> 
> - For Wang Fan's "[edk2] [Patch V2] NetworkPkg: Add WiFi Connection
>   Manager to NetworkPkg":
>   - Jiaxin's R-b appeared at 02/22/19 08:56.
> 
> I'm OK if both features are pushed.
> 
Thanks.

> 
> However, I do have a suggestion for the announcement email, for the
> future. Given that we've already encountered three cases in this cycle
> where the feedback tags are on the boundary, I suggest to include an
> exact timestamp (in the UTC zone) in each announcement. I never expected
> that this accuracy would be necessary, but apparently it is.
> 
I suggest to list UTC zone in Proposed Schedule of EDK II Release Planning wiki 
page as below. I prefer to use PST (UST -8), because edk2-devel Archives 
(https://lists.01.org/pipermail/edk2-devel/) uses PST time. Then, we can base 
on the archive mail to check each patch. 

Date (UTC -8)   Description
2019-03-08 Beginning of development
2019-02-22 Soft Feature Freeze
2019-03-01 Hard Feature Freeze
2019-03-08 Release

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

Re: [edk2] [PATCH v2 0/2] MdeModulePkg: Resolve buffer cross boundary access in Ramdisk

2019-02-27 Thread Gao, Liming

Laszlo:
  I add my comments. 

Thanks
Liming
> -Original Message-
> From: Laszlo Ersek [mailto:ler...@redhat.com]
> Sent: Wednesday, February 27, 2019 4:58 PM
> To: Wu, Hao A ; Gao, Liming ; 
> edk2-devel@lists.01.org
> Cc: Zeng, Star 
> Subject: Re: [edk2] [PATCH v2 0/2] MdeModulePkg: Resolve buffer cross 
> boundary access in Ramdisk
> 
> On 02/27/19 07:56, Wu, Hao A wrote:
> >> -Original Message-
> >> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> >> Laszlo Ersek
> >> Sent: Tuesday, February 26, 2019 7:45 PM
> >> To: Wu, Hao A; edk2-devel@lists.01.org
> >> Cc: Zeng, Star
> >> Subject: Re: [edk2] [PATCH v2 0/2] MdeModulePkg: Resolve buffer cross
> >> boundary access in Ramdisk
> >>
> >> On 02/26/19 08:45, Hao Wu wrote:
> >>> V2 changes:
> >>>
> >>> Correct CC list information.
> >>>
> >>>
> >>> V1 history:
> >>>
> >>> The series will resolve a buffer cross boundary access issue during the
> >>> use of RAM disks. It is the mitigation for issue CVE-2018-12180.
> >>>
> >>> Cc: Jian J Wang 
> >>> Cc: Ray Ni 
> >>> Cc: Star Zeng 
> >>>
> >>> Hao Wu (2):
> >>>   MdeModulePkg/PartitionDxe: Ensure blocksize can hold MBR (CVE FIX)
> >>>   MdeModulePkg/RamDiskDxe: Ramdisk size be multiple of BlkSize (CVE
> >> FIX)
> >>>
> >>>  MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskImpl.h |  6 +++---
> >>>  MdeModulePkg/Universal/Disk/PartitionDxe/Gpt.c   |  9 -
> >>>  MdeModulePkg/Universal/Disk/PartitionDxe/Mbr.c   |  9 -
> >>>  MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskBlockIo.c  | 20
> >> ++--
> >>>  MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskProtocol.c |  5 +++--
> >>>  5 files changed, 36 insertions(+), 13 deletions(-)
> >>>
> >>
> >> Please put the exact CVE numbers in the subject lines.
> >
> > Hello Laszlo and Liming,
> >
> > I totally agree the commit subject line should include the CVE number.
> > But I have one feedback that, if the commit is for a CVE fix, is it
> > possible to exempt the commit subject from 71 characters limit?
> 
> In my opinion, that is absolutely the case.
> 
> > I found it can be hard to summary the commit with the Package/Module plus
> > the CVE number information.
> 
> I agree, it is hard. But, IMO, in this case, the precise CVE reference
> takes priority.
> 
For this case, I suggest to allow subject line length to be bigger, such as 120 
character.
I will update wiki 
https://github.com/tianocore/tianocore.github.io/wiki/Commit-Message-Format for 
CVE commit message format. 
For example: Pkg-Module: Brief-single-line-summary (CVE-Year-Number)

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

Re: [edk2] [Patch] BaseTools: Add python3-distutils Ubuntu package checking

2019-02-27 Thread Laszlo Ersek

On 02/27/19 09:52, Feng, Bob C wrote:
> Thanks for comments. I think the print message is not good. It's based on 
> Ubutun OS. It's not right. 
> 
> I think the import error need to be caught and then print some messages, 
> otherwise the build tool will break and print the call stack which is not 
> friendly to user.

I agree. OS or package name specific references should not be printed,
but a user-friendly, concise message about the missing Python feature
should be.

As soon as I see a Python (or Java, for that matter) stack dump dropped
on me, my first instinct is to close the terminal window. Obviously I
won't do that most of the time, but it takes effort to start deciphering
the stack dump. Give me a usable one-liner error message, and
*optionally* a stack dump to dig down.

Given that the situation can be remedied by a google search ("what
package on my system provides this Python feature") plus a package
installation, the stack dump is entirely irrelevant in this case.

Thanks
Laszlo


> -Original Message-
> From: Leif Lindholm [mailto:leif.lindh...@linaro.org] 
> Sent: Wednesday, February 27, 2019 4:26 PM
> To: Feng, Bob C 
> Cc: Ard Biesheuvel ; edk2-devel@lists.01.org; Gao, 
> Liming 
> Subject: Re: [edk2] [Patch] BaseTools: Add python3-distutils Ubuntu package 
> checking
> 
> On Wed, Feb 27, 2019 at 09:07:49AM +0100, Ard Biesheuvel wrote:
>> On Tue, 26 Feb 2019 at 02:05, Feng, Bob C  wrote:
>>>
>>> https://bugzilla.tianocore.org/show_bug.cgi?id=1509
>>>
>>> Add python3-distutils Ubuntu package checking.
>>>
>>
>> Hi Bob,
>>
>> This assumes that all Linux systems are Ubuntu based, which is not 
>> true. The apt tool is specific to Debian/Ubuntu, Fedora/Redhat and 
>> Suse all use something else.
>>
>> In general, I don't think we should validate the Python environment to 
>> this extent, since we cannot fix the problem for the user anyway, only 
>> flag it, and since python explodes rather loudly in this case, I think 
>> we should be able to leave it up to developers that are savvy enough 
>> to build EDK2 to also find the python distutils package for their 
>> platform.
>>
>> Note that that doesn't mean we shouldn't document this, and not just 
>> for Ubuntu. But I think putting it in the script is overkill.
> 
> Yes, I agree
> 
> It is also worth noting that python3-distutils is the current debian/ubuntu 
> package name. So if we *do* print a message...
> 
>>> Contributed-under: TianoCore Contribution Agreement 1.1
>>> Signed-off-by: Bob Feng 
>>> Cc: Liming Gao 
>>> ---
>>>  BaseTools/Tests/RunTests.py | 14 ++
>>>  1 file changed, 14 insertions(+)
>>>
>>> diff --git a/BaseTools/Tests/RunTests.py 
>>> b/BaseTools/Tests/RunTests.py index 0dd65632d0..64778db981 100644
>>> --- a/BaseTools/Tests/RunTests.py
>>> +++ b/BaseTools/Tests/RunTests.py
>>> @@ -17,10 +17,24 @@
>>>  #
>>>  import os
>>>  import sys
>>>  import unittest
>>>
>>> +distutils_exist = True
>>> +try:
>>> +import distutils.util
>>> +except:
>>> +distutils_exist = False
>>> +
>>> +if not distutils_exist:
>>> +print("""
>>> +python3-distutil packages is missing. Please install it with the following 
>>> command:
> 
> ... printing "missing python distutils package" and possibly python version 
> would be more reliable.
> 
> But as Ard points out - this is effectively what python itself will say.
> 
> /
> Leif
> 
>>> +
>>> +bash$ sudo apt-get install python3-distutil
>>> +""")
>>> +sys.exit(-1)
>>> +
>>>  import TestTools
>>>
>>>  def GetCTestSuite():
>>>  import CToolsTests
>>>  return CToolsTests.TheTestSuite()
>>> --
>>> 2.20.1.windows.1
>>>
>>> ___
>>> edk2-devel mailing list
>>> edk2-devel@lists.01.org
>>> https://lists.01.org/mailman/listinfo/edk2-devel
>> ___
>> edk2-devel mailing list
>> edk2-devel@lists.01.org
>> https://lists.01.org/mailman/listinfo/edk2-devel
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
> 

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

Re: [edk2] [PATCH] Maintainers.txt: Register myself as OvmfPkg/ArmVirtPkg reviewer

2019-02-27 Thread Philippe Mathieu-Daudé

Hi,

On 2/21/19 10:13 PM, Philippe Mathieu-Daude wrote:
> Part of my assignment is to review OVMF and ArmVirt patches,
> having my address listed will help me to catch the related
> patches.

I had a misunderstanding regarding how EDK2 treats a 'R:' tag, as I
understood it as similar to Linux/QEMU, which is simply "reviewers
should be CCed on patches".
Since EDK2 R: tag seems somehow different, but the description is not
explicit:

  R: Package Reviewer: Cc address for patches and questions. Reviewers
 help maintainers review code, but don't have push access.

Can someone improve the documentation?

Meanwhile please disregard this inappropriate patch.

Thanks,

Phil.

> 
> Signed-off-by: Philippe Mathieu-Daude 
> ---
>  Maintainers.txt | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Maintainers.txt b/Maintainers.txt
> index 7772926b2f..3c17038fab 100644
> --- a/Maintainers.txt
> +++ b/Maintainers.txt
> @@ -82,6 +82,7 @@ M: Laszlo Ersek 
>  M: Ard Biesheuvel 
>  R: Julien Grall 
> (Xen modules)
> +R: Philippe Mathieu-Daudé 
>  
>  BaseTools
>  W: https://github.com/tianocore/tianocore.github.io/wiki/BaseTools
> @@ -225,6 +226,7 @@ R: Marc-André Lureau 
> (TPM2 modules)
>  R: Stefan Berger 
> (TPM2 modules)
> +R: Philippe Mathieu-Daudé 
>  S: Maintained
>  
>  PcAtChipsetPkg
> 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH] MdeModulePkg/SdMmcPciHcDxe: Fix DMA on SDHC v3 64-bit systems

2019-02-27 Thread Cohen, Eugene

The SdMmcPciHcDriverBindingStart function was checking two
different capability bits in determining whether 64-bit DMA
modes were supported, one mode is defined in the SDHC version
3 specification (using 96-bit descriptors) and another is
defined in the SDHC version 4 specification (using 128-bit
descriptors).  Since the currently implementation of 64-bit
ADMA2 only supports the SDHC version 4 implementation it is
incorrect to check the V3 64-bit capability bit since this
will activate V4 ADMA2 on V3 controllers.

Cc: Hao Wu 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eugene Cohen 
---
MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c 
b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
index b474f8d..5bc91c5 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
@@ -666,8 +666,7 @@ SdMmcPciHcDriverBindingStart (
 // If any of the slots does not support 64b system bus
 // do not enable 64b DMA in the PCI layer.
 //
-if (Private->Capability[Slot].SysBus64V3 == 0 &&
-Private->Capability[Slot].SysBus64V4 == 0) {
+if (Private->Capability[Slot].SysBus64V4 == 0) {
   Support64BitDma = FALSE;
 }

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

Re: [edk2] [Patch] BaseTools: Add python3-distutils Ubuntu package checking

2019-02-27 Thread Leif Lindholm

On Wed, Feb 27, 2019 at 08:52:08AM +, Feng, Bob C wrote:
> Thanks for comments. I think the print message is not good. It's based on 
> Ubutun OS. It's not right. 
> 
> I think the import error need to be caught and then print some
> messages, otherwise the build tool will break and print the call
> stack which is not friendly to user.

I agree printing the call stack is not useful for import errors.
Is there no way to suppress that for basic environment issues?

Surely a "failed to import..." message would also be printed?

Regards,

Leif

> Thanks,
> Bob 
> 
> -Original Message-
> From: Leif Lindholm [mailto:leif.lindh...@linaro.org] 
> Sent: Wednesday, February 27, 2019 4:26 PM
> To: Feng, Bob C 
> Cc: Ard Biesheuvel ; edk2-devel@lists.01.org; Gao, 
> Liming 
> Subject: Re: [edk2] [Patch] BaseTools: Add python3-distutils Ubuntu package 
> checking
> 
> On Wed, Feb 27, 2019 at 09:07:49AM +0100, Ard Biesheuvel wrote:
> > On Tue, 26 Feb 2019 at 02:05, Feng, Bob C  wrote:
> > >
> > > https://bugzilla.tianocore.org/show_bug.cgi?id=1509
> > >
> > > Add python3-distutils Ubuntu package checking.
> > >
> > 
> > Hi Bob,
> > 
> > This assumes that all Linux systems are Ubuntu based, which is not 
> > true. The apt tool is specific to Debian/Ubuntu, Fedora/Redhat and 
> > Suse all use something else.
> > 
> > In general, I don't think we should validate the Python environment to 
> > this extent, since we cannot fix the problem for the user anyway, only 
> > flag it, and since python explodes rather loudly in this case, I think 
> > we should be able to leave it up to developers that are savvy enough 
> > to build EDK2 to also find the python distutils package for their 
> > platform.
> > 
> > Note that that doesn't mean we shouldn't document this, and not just 
> > for Ubuntu. But I think putting it in the script is overkill.
> 
> Yes, I agree
> 
> It is also worth noting that python3-distutils is the current debian/ubuntu 
> package name. So if we *do* print a message...
> 
> > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > Signed-off-by: Bob Feng 
> > > Cc: Liming Gao 
> > > ---
> > >  BaseTools/Tests/RunTests.py | 14 ++
> > >  1 file changed, 14 insertions(+)
> > >
> > > diff --git a/BaseTools/Tests/RunTests.py 
> > > b/BaseTools/Tests/RunTests.py index 0dd65632d0..64778db981 100644
> > > --- a/BaseTools/Tests/RunTests.py
> > > +++ b/BaseTools/Tests/RunTests.py
> > > @@ -17,10 +17,24 @@
> > >  #
> > >  import os
> > >  import sys
> > >  import unittest
> > >
> > > +distutils_exist = True
> > > +try:
> > > +import distutils.util
> > > +except:
> > > +distutils_exist = False
> > > +
> > > +if not distutils_exist:
> > > +print("""
> > > +python3-distutil packages is missing. Please install it with the 
> > > following command:
> 
> ... printing "missing python distutils package" and possibly python version 
> would be more reliable.
> 
> But as Ard points out - this is effectively what python itself will say.
> 
> /
> Leif
> 
> > > +
> > > +bash$ sudo apt-get install python3-distutil
> > > +""")
> > > +sys.exit(-1)
> > > +
> > >  import TestTools
> > >
> > >  def GetCTestSuite():
> > >  import CToolsTests
> > >  return CToolsTests.TheTestSuite()
> > > --
> > > 2.20.1.windows.1
> > >
> > > ___
> > > edk2-devel mailing list
> > > edk2-devel@lists.01.org
> > > https://lists.01.org/mailman/listinfo/edk2-devel
> > ___
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v2 0/2] MdeModulePkg: Resolve buffer cross boundary access in Ramdisk

2019-02-27 Thread Laszlo Ersek

On 02/27/19 07:56, Wu, Hao A wrote:
>> -Original Message-
>> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
>> Laszlo Ersek
>> Sent: Tuesday, February 26, 2019 7:45 PM
>> To: Wu, Hao A; edk2-devel@lists.01.org
>> Cc: Zeng, Star
>> Subject: Re: [edk2] [PATCH v2 0/2] MdeModulePkg: Resolve buffer cross
>> boundary access in Ramdisk
>>
>> On 02/26/19 08:45, Hao Wu wrote:
>>> V2 changes:
>>>
>>> Correct CC list information.
>>>
>>>
>>> V1 history:
>>>
>>> The series will resolve a buffer cross boundary access issue during the
>>> use of RAM disks. It is the mitigation for issue CVE-2018-12180.
>>>
>>> Cc: Jian J Wang 
>>> Cc: Ray Ni 
>>> Cc: Star Zeng 
>>>
>>> Hao Wu (2):
>>>   MdeModulePkg/PartitionDxe: Ensure blocksize can hold MBR (CVE FIX)
>>>   MdeModulePkg/RamDiskDxe: Ramdisk size be multiple of BlkSize (CVE
>> FIX)
>>>
>>>  MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskImpl.h |  6 +++---
>>>  MdeModulePkg/Universal/Disk/PartitionDxe/Gpt.c   |  9 -
>>>  MdeModulePkg/Universal/Disk/PartitionDxe/Mbr.c   |  9 -
>>>  MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskBlockIo.c  | 20
>> ++--
>>>  MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskProtocol.c |  5 +++--
>>>  5 files changed, 36 insertions(+), 13 deletions(-)
>>>
>>
>> Please put the exact CVE numbers in the subject lines.
> 
> Hello Laszlo and Liming,
> 
> I totally agree the commit subject line should include the CVE number.
> But I have one feedback that, if the commit is for a CVE fix, is it
> possible to exempt the commit subject from 71 characters limit?

In my opinion, that is absolutely the case.

> I found it can be hard to summary the commit with the Package/Module plus
> the CVE number information.

I agree, it is hard. But, IMO, in this case, the precise CVE reference
takes priority.

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

Re: [edk2] [Patch] BaseTools: Add python3-distutils Ubuntu package checking

2019-02-27 Thread Feng, Bob C

Thanks for comments. I think the print message is not good. It's based on 
Ubutun OS. It's not right. 

I think the import error need to be caught and then print some messages, 
otherwise the build tool will break and print the call stack which is not 
friendly to user.

Thanks,
Bob 

-Original Message-
From: Leif Lindholm [mailto:leif.lindh...@linaro.org] 
Sent: Wednesday, February 27, 2019 4:26 PM
To: Feng, Bob C 
Cc: Ard Biesheuvel ; edk2-devel@lists.01.org; Gao, 
Liming 
Subject: Re: [edk2] [Patch] BaseTools: Add python3-distutils Ubuntu package 
checking

On Wed, Feb 27, 2019 at 09:07:49AM +0100, Ard Biesheuvel wrote:
> On Tue, 26 Feb 2019 at 02:05, Feng, Bob C  wrote:
> >
> > https://bugzilla.tianocore.org/show_bug.cgi?id=1509
> >
> > Add python3-distutils Ubuntu package checking.
> >
> 
> Hi Bob,
> 
> This assumes that all Linux systems are Ubuntu based, which is not 
> true. The apt tool is specific to Debian/Ubuntu, Fedora/Redhat and 
> Suse all use something else.
> 
> In general, I don't think we should validate the Python environment to 
> this extent, since we cannot fix the problem for the user anyway, only 
> flag it, and since python explodes rather loudly in this case, I think 
> we should be able to leave it up to developers that are savvy enough 
> to build EDK2 to also find the python distutils package for their 
> platform.
> 
> Note that that doesn't mean we shouldn't document this, and not just 
> for Ubuntu. But I think putting it in the script is overkill.

Yes, I agree

It is also worth noting that python3-distutils is the current debian/ubuntu 
package name. So if we *do* print a message...

> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Bob Feng 
> > Cc: Liming Gao 
> > ---
> >  BaseTools/Tests/RunTests.py | 14 ++
> >  1 file changed, 14 insertions(+)
> >
> > diff --git a/BaseTools/Tests/RunTests.py 
> > b/BaseTools/Tests/RunTests.py index 0dd65632d0..64778db981 100644
> > --- a/BaseTools/Tests/RunTests.py
> > +++ b/BaseTools/Tests/RunTests.py
> > @@ -17,10 +17,24 @@
> >  #
> >  import os
> >  import sys
> >  import unittest
> >
> > +distutils_exist = True
> > +try:
> > +import distutils.util
> > +except:
> > +distutils_exist = False
> > +
> > +if not distutils_exist:
> > +print("""
> > +python3-distutil packages is missing. Please install it with the following 
> > command:

... printing "missing python distutils package" and possibly python version 
would be more reliable.

But as Ard points out - this is effectively what python itself will say.

/
Leif

> > +
> > +bash$ sudo apt-get install python3-distutil
> > +""")
> > +sys.exit(-1)
> > +
> >  import TestTools
> >
> >  def GetCTestSuite():
> >  import CToolsTests
> >  return CToolsTests.TheTestSuite()
> > --
> > 2.20.1.windows.1
> >
> > ___
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [Patch] BaseTools: Add python3-distutils Ubuntu package checking

2019-02-27 Thread Leif Lindholm

On Wed, Feb 27, 2019 at 09:07:49AM +0100, Ard Biesheuvel wrote:
> On Tue, 26 Feb 2019 at 02:05, Feng, Bob C  wrote:
> >
> > https://bugzilla.tianocore.org/show_bug.cgi?id=1509
> >
> > Add python3-distutils Ubuntu package checking.
> >
> 
> Hi Bob,
> 
> This assumes that all Linux systems are Ubuntu based, which is not
> true. The apt tool is specific to Debian/Ubuntu, Fedora/Redhat and
> Suse all use something else.
> 
> In general, I don't think we should validate the Python environment to
> this extent, since we cannot fix the problem for the user anyway, only
> flag it, and since python explodes rather loudly in this case, I think
> we should be able to leave it up to developers that are savvy enough
> to build EDK2 to also find the python distutils package for their
> platform.
> 
> Note that that doesn't mean we shouldn't document this, and not just
> for Ubuntu. But I think putting it in the script is overkill.

Yes, I agree

It is also worth noting that python3-distutils is the current
debian/ubuntu package name. So if we *do* print a message...

> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Bob Feng 
> > Cc: Liming Gao 
> > ---
> >  BaseTools/Tests/RunTests.py | 14 ++
> >  1 file changed, 14 insertions(+)
> >
> > diff --git a/BaseTools/Tests/RunTests.py b/BaseTools/Tests/RunTests.py
> > index 0dd65632d0..64778db981 100644
> > --- a/BaseTools/Tests/RunTests.py
> > +++ b/BaseTools/Tests/RunTests.py
> > @@ -17,10 +17,24 @@
> >  #
> >  import os
> >  import sys
> >  import unittest
> >
> > +distutils_exist = True
> > +try:
> > +import distutils.util
> > +except:
> > +distutils_exist = False
> > +
> > +if not distutils_exist:
> > +print("""
> > +python3-distutil packages is missing. Please install it with the following 
> > command:

... printing "missing python distutils package" and possibly python
version would be more reliable.

But as Ard points out - this is effectively what python itself will
say.

/
Leif

> > +
> > +bash$ sudo apt-get install python3-distutil
> > +""")
> > +sys.exit(-1)
> > +
> >  import TestTools
> >
> >  def GetCTestSuite():
> >  import CToolsTests
> >  return CToolsTests.TheTestSuite()
> > --
> > 2.20.1.windows.1
> >
> > ___
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [Patch] BaseTools: Add python3-distutils Ubuntu package checking

2019-02-27 Thread Ard Biesheuvel

On Tue, 26 Feb 2019 at 02:05, Feng, Bob C  wrote:
>
> https://bugzilla.tianocore.org/show_bug.cgi?id=1509
>
> Add python3-distutils Ubuntu package checking.
>

Hi Bob,

This assumes that all Linux systems are Ubuntu based, which is not
true. The apt tool is specific to Debian/Ubuntu, Fedora/Redhat and
Suse all use something else.

In general, I don't think we should validate the Python environment to
this extent, since we cannot fix the problem for the user anyway, only
flag it, and since python explodes rather loudly in this case, I think
we should be able to leave it up to developers that are savvy enough
to build EDK2 to also find the python distutils package for their
platform.

Note that that doesn't mean we shouldn't document this, and not just
for Ubuntu. But I think putting it in the script is overkill.

> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Bob Feng 
> Cc: Liming Gao 
> ---
>  BaseTools/Tests/RunTests.py | 14 ++
>  1 file changed, 14 insertions(+)
>
> diff --git a/BaseTools/Tests/RunTests.py b/BaseTools/Tests/RunTests.py
> index 0dd65632d0..64778db981 100644
> --- a/BaseTools/Tests/RunTests.py
> +++ b/BaseTools/Tests/RunTests.py
> @@ -17,10 +17,24 @@
>  #
>  import os
>  import sys
>  import unittest
>
> +distutils_exist = True
> +try:
> +import distutils.util
> +except:
> +distutils_exist = False
> +
> +if not distutils_exist:
> +print("""
> +python3-distutil packages is missing. Please install it with the following 
> command:
> +
> +bash$ sudo apt-get install python3-distutil
> +""")
> +sys.exit(-1)
> +
>  import TestTools
>
>  def GetCTestSuite():
>  import CToolsTests
>  return CToolsTests.TheTestSuite()
> --
> 2.20.1.windows.1
>
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 2/2] UefiCpuPkg/Microcode: Add verification logic before parsing payload

Re: [edk2] [PATCH 1/2] UefiCpuPkg/Microcode: Fix InComplete CheckSum32 issue

[edk2] [PATCH 2/2] UefiCpuPkg/Microcode: Add verification logic before parsing payload

[edk2] [PATCH 1/2] UefiCpuPkg/Microcode: Fix InComplete CheckSum32 issue

[edk2] [PATCH 0/2] Fix Microcode failure issue

Re: [edk2] Self-replicating image

Re: [edk2] [PATCH] MdeModulePkg/SdMmcPciHcDxe: Fix DMA on SDHC v3 64-bit systems

[edk2] [staging/Bug_1525_FmpDevicePkg_MultipleControllers] Create new branch

Re: [edk2] [edk2-announce] Soft Feature Freeze starts today for edk2-stable201903

[edk2] [Patch] Document: Update the INF spec about [Depex] section

[edk2] [Patch] Document: Update Dsc spec to all empty value for HIIPcd

Re: [edk2] [Patch] Revert "BaseTools:BaseTools supports to the driver combination."

[edk2] [Patch V2] BaseTools: Add python3-distutils Ubuntu package checking

Re: [edk2] [PATCH v2 0/2] MdeModulePkg: Resolve buffer cross boundary access in Ramdisk

Re: [edk2] [PATCH] Maintainers.txt: clarify Reviewer requirements and responsibilities

Re: [edk2] [PATCH] Maintainers.txt: clarify Reviewer requirements and responsibilities

[edk2] [PATCH] Maintainers.txt: clarify Reviewer requirements and responsibilities

Re: [edk2] Hang when calling ExitBootServices on IA32 firmware v1.0 on MinnowBoard Turbot

Re: [edk2] [PATCH v1 0/6] Revert DynamicTablesPkg: Framework updates and fixes

Re: [edk2] [edk2-announce] Soft Feature Freeze starts today for edk2-stable201903

Re: [edk2] [PATCH v2 0/2] MdeModulePkg: Resolve buffer cross boundary access in Ramdisk

Re: [edk2] [PATCH v1 0/6] Revert DynamicTablesPkg: Framework updates and fixes

Re: [edk2] [edk2-announce] Soft Feature Freeze starts today for edk2-stable201903

Re: [edk2] [PATCH v2 0/2] MdeModulePkg: Resolve buffer cross boundary access in Ramdisk

Re: [edk2] [Patch] BaseTools: Add python3-distutils Ubuntu package checking

Re: [edk2] [PATCH] Maintainers.txt: Register myself as OvmfPkg/ArmVirtPkg reviewer

[edk2] [PATCH] MdeModulePkg/SdMmcPciHcDxe: Fix DMA on SDHC v3 64-bit systems

Re: [edk2] [Patch] BaseTools: Add python3-distutils Ubuntu package checking

Re: [edk2] [PATCH v2 0/2] MdeModulePkg: Resolve buffer cross boundary access in Ramdisk

Re: [edk2] [Patch] BaseTools: Add python3-distutils Ubuntu package checking

Re: [edk2] [Patch] BaseTools: Add python3-distutils Ubuntu package checking

Re: [edk2] [Patch] BaseTools: Add python3-distutils Ubuntu package checking

32 matches

Site Navigation

Mail list logo

Footer information