Re: [edk2] [Patch] SecurityPkg: Fix TPM device compatibility issue

[Ni, Ruiyu]

Does it mean the current master code is good enough?

Sent from small device

> 在 2018年11月10日，上午1:23，Kinney, Michael D  写道：
> 
> Laszlo,
> 
> Given that the compatibility issue has been present
> for several months, I would prefer this change be
> deferred until after the stable tag.
> 
> We can document this as a known issue in the release
> page for the stable tag and point to the commit after
> the stable tag that resolves the compatibility issue
> so platforms that are impacted can choose to cherry-pick
> the one additional change.
> 
> Thanks,
> 
> Mike
> 
>> -Original Message-
>> From: edk2-devel [mailto:edk2-devel-
>> boun...@lists.01.org] On Behalf Of Laszlo Ersek
>> Sent: Friday, November 9, 2018 8:22 AM
>> To: Zhang, Chao B ; Leif
>> Lindholm 
>> Cc: Kinney, Michael D ;
>> edk2-devel@lists.01.org; Yao, Jiewen
>> 
>> Subject: Re: [edk2] [Patch] SecurityPkg: Fix TPM device
>> compatibility issue
>> 
>>> On 11/09/18 15:46, Zhang, Chao B wrote:
>>> Hi Leif:
>>>   The NTC1310 can work well with previous EDK2
>> stable version (UDK2018).
>> 
>> That's not correct. The last EDK2 stable version is not
>> UDK2018. The
>> last stable EDK2 version is "edk2-stable201808", and
>> the regression
>> (commit f15cb995bb38, according to this patch) is
>> already contained in
>> "edk2-stable201808".
>> 
>>> Interface Cache is a new feature introduced after
>> UDK2018.
>>> So far as we see, it causes NTC1310 fail to work.
>> 
>> That may be the case, but it's not a feature (and
>> resultant regression)
>> introduced in this development cycle.
>> 
>> Personally I'm still undecided.
>> 
>> - From an end-user perspective, this is certainly a
>> "bugfix"; given that
>> the NTC1310 hardware (which is the real culprit) cannot
>> be fixed at all.
>> 
>> - In a technical edk2 sense, this is a feature-let
>> however (with code
>> duplication at that) that works around a broken device
>> that already
>> doesn't work with "edk2-stable201808".
>> 
>> 
>> I'm *slightly* leaning against this change. If the end-
>> user regression
>> had really been painful, this workaround would have
>> been implemented
>> very soon after "edk2-stable201808", not in a last
>> minute rush before
>> "edk2-stable201811".
>> 
>> I'd like to hear Mike's and Andrew's opinion too.
>> 
>> Thanks,
>> Laszlo
>> 
>>> 
>>> From: edk2-devel [mailto:edk2-devel-
>> boun...@lists.01.org] On Behalf Of Leif Lindholm
>>> Sent: Friday, November 9, 2018 7:13 PM
>>> To: Laszlo Ersek 
>>> Cc: Kinney, Michael D ;
>> edk2-devel@lists.01.org; Yao, Jiewen
>> ; Zhang, Chao B
>> 
>>> Subject: Re: [edk2] [Patch] SecurityPkg: Fix TPM
>> device compatibility issue
>>> 
>>> On Fri, Nov 09, 2018 at 09:04:46AM +0100, Laszlo
>> Ersek wrote:
 On 11/09/18 07:02, Zhang, Chao B wrote:
> Issue Statement:
> TPM InterfaceId cache feature is introduced by
>> f15cb995bb3880b77e15afe6facd3da05e599a17. It follows
>> TCG PTP spec 1.3
> to improve TPM transmission performance and also
>> addresses defects in some TPM2.0 devices. But some
>> other TPM devices like
> NTC1310 SPI TPM is found function abnormally with
>> this feature, causing extra device compatibility issue.
> 
> Solution:
> Add a policy indicator in PcdActiveTpmInterfaceType
>> to disable TPM interface ID cache to support those
>> existing TPM devices
> 
> Contributed-under: TianoCore Contribution Agreement
>> 1.1
> Signed-off-by: Zhang, Chao B
>> mailto:chao.b.zh...@intel.com>>
> Cc: Andrew Fish
>> mailto:af...@apple.com>>
> Cc: Laszlo Ersek
>> mailto:ler...@redhat.com>>
> Cc: Leif Lindholm
>> mailto:leif.lindholm@linaro.o
>> rg>>
> Cc: Michael D Kinney
>> mailto:michael.d.kinney@int
>> el.com>>
> Cc: Yao Jiewen
>> mailto:jiewen@intel.com>>
> ---
> SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c |
>> 23 +++-
> SecurityPkg/SecurityPkg.dec |
>> 3 +-
> SecurityPkg/SecurityPkg.uni |
>> 3 +-
> SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigImpl.c |
>> 49 +
> SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c   |
>> 42 +
> 5 files changed, 117 insertions(+), 3 deletions(-)
 
 I'll let others review this patch for technical
>> merit.
 
 However, I'm really undecided whether this patch
>> qualifies for being
 pushed during the hard feature freeze. Comments
>> welcome.
>>> 
>>> Unless the current behaviour causes an absolutely
>> horrendous security
>>> hole, I don't see how this qualifies for pushing
>> during hard freeze.
>>> 
>>> According to its description, this is about
>> supporting (non-compliant)
>>> devices that have never worked with EDK2. And the
>> support it updates
>>> went in on 25 June. So there does not appear to be
>> any urgency.
>>> 
>>> Once it does go in, I would also appreciate some
>> simplification via
>>> macros to cut down some of those very long lines, but
>> then I'm not the
>>> maintainer of this package.

Re: [edk2] [Patch 0/2] Separate semaphore container.

[Dong, Eric]

Hi Stewards:
 
Since this is a bug fix, and the risk for this release is small. I plan to push 
this serial changes before edk2-stable201811 tag.

If you have any concern, please raise here. 

Thanks,
Eric

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Eric
> Dong
> Sent: Thursday, November 8, 2018 10:58 AM
> To: edk2-devel@lists.01.org
> Cc: Ni, Ruiyu ; Laszlo Ersek 
> Subject: [edk2] [Patch 0/2] Separate semaphore container.
> 
> In current implementation, core level semaphore use same container with
> package level semaphore. This design will let the core level semaphore not
> works as expected in below case:
> 1. Feature A has CPU_FEATURE_CORE_BEFORE dependence with Feature B.
> 2. Feature C has CPU_FEATURE_PACKAGE_AFTER dependence with Feature B.
> in this case an core level semaphore will be add between A and B, and an
> package level semaphore will be add between B and C.
> 
> For a CPU has one package, two cores and 4 threads. Execute like below:
> 
>   Thread 1  Thread 2. Thread 4
> ReleaseSemaph(1,2)  -|
> WaitForSemaph(1(2)) -|<---These two are Core Semaph
>   ReleaseSemaph(1,2) -|
>   WaitForSemaph(2)   -| <---  Core Semaph
> 
> ReleaseSemaph (1,2,3,4) -|
> WaitForSemaph (1(4))-| <  Package Semaph
> 
>   ReleaseSemaph(3,4)
>   WaitForSemaph(4(2)) <- Core Semaph
> 
> In above case, for thread 4, when it executes a core semaphore, i will found
> WaitForSemaph(4(2)) is met because Thread 1 has execute a package
> semaphore and ReleaseSemaph(4) for it before. This is not an expect behavior.
> Thread 4 should wait for thread 3 to do this.
> 
> Fix this issue by separate the semaphore container for core level and package
> level.
> 
> Cc: Laszlo Ersek 
> Cc: Ruiyu Ni 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Eric Dong 
> 
> Eric Dong (2):
>   UefiCpuPkg/RegisterCpuFeaturesLib: Separate semaphore container.
>   UefiCpuPkg/PiSmmCpuDxeSmm: Separate semaphore container.
> 
>  .../RegisterCpuFeaturesLib/CpuFeaturesInitialize.c  |  9 ++---
>  .../RegisterCpuFeaturesLib/RegisterCpuFeatures.h|  7 ---
>  UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c   | 21 
> ++---
>  3 files changed, 24 insertions(+), 13 deletions(-)
> 
> --
> 2.15.0.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] UefiCpuPkg/RegisterCpuFeaturesLib: Adjust Order.

[Dong, Eric]

Hi Stewards:
 
Since this is a bug fix, and the risk for this release is small. I plan to push 
this change before edk2-stable201811 tag.

If you have any concern, please raise here. 

Thanks,
Eric

> -Original Message-
> From: Ni, Ruiyu
> Sent: Friday, November 9, 2018 10:55 PM
> To: Dong, Eric ; edk2-devel@lists.01.org
> Cc: Laszlo Ersek 
> Subject: RE: [edk2] [Patch] UefiCpuPkg/RegisterCpuFeaturesLib: Adjust Order.
> 
> Eric,
> Thanks for fixing the new found issues.
> 
> Reviewed-by: Ruiyu Ni 
> 
> > -Original Message-
> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> > Eric Dong
> > Sent: Friday, November 9, 2018 1:21 PM
> > To: edk2-devel@lists.01.org
> > Cc: Ni, Ruiyu ; Laszlo Ersek 
> > Subject: [edk2] [Patch] UefiCpuPkg/RegisterCpuFeaturesLib: Adjust Order.
> >
> > V2 changes:
> > V1 change has regression which caused by change feature order.
> > V2 changes logic to detect dependence not only for the neighborhood
> > features. It need to check all features in the list.
> >
> > V1 Changes:
> > In current code logic, only adjust feature position if current CPU
> > feature position not follow the request order. Just like Feature A
> > need to be executed before feature B, but current feature A registers
> > after feature B. So code will adjust the position for feature A, move
> > it to just before feature B. If the position already met the
> > requirement, code will not adjust the position.
> >
> > This logic has issue when met all below cases:
> > 1. feature A has core or package level dependence with feature B.
> > 2. feature A is register before feature B.
> > 3. Also exist other features exist between feature A and B.
> >
> > Root cause is driver ignores the dependence for this case, so threads
> > may execute not follow the dependence order.
> >
> > Fix this issue by change code logic to adjust feature position for CPU
> > features which has dependence relationship.
> >
> > Cc: Laszlo Ersek 
> > Cc: Ruiyu Ni 
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Eric Dong 
> > ---
> >  .../RegisterCpuFeaturesLib/CpuFeaturesInitialize.c |  71 
> >  .../RegisterCpuFeaturesLib/RegisterCpuFeatures.h   |  16 +++
> >  .../RegisterCpuFeaturesLib.c   | 122 
> > +
> >  3 files changed, 189 insertions(+), 20 deletions(-)
> >
> > diff --git
> > a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
> > b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
> > index 4bed0ce3a4..69e2c04daf 100644
> > ---
> > a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
> > +++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.
> > +++ c
> > @@ -534,6 +534,28 @@ DumpRegisterTableOnProcessor (
> >}
> >  }
> >
> > +/**
> > +  Get the biggest dependence type.
> > +  PackageDepType > CoreDepType > ThreadDepType > NoneDepType.
> > +
> > +  @param[in]  BeforeDep   Before dependence type.
> > +  @param[in]  AfterDepAfter dependence type.
> > +  @param[in]  NoneNeibBeforeDep   Before dependence type for not
> > neighborhood features.
> > +  @param[in]  NoneNeibAfterDepAfter dependence type for not
> > neighborhood features.
> > +
> > +  @retval  Return the biggest dependence type.
> > +**/
> > +CPU_FEATURE_DEPENDENCE_TYPE
> > +BiggestDep (
> > +  IN CPU_FEATURE_DEPENDENCE_TYPE  BeforeDep,
> > +  IN CPU_FEATURE_DEPENDENCE_TYPE  AfterDep,
> > +  IN CPU_FEATURE_DEPENDENCE_TYPE  NoneNeibBeforeDep,
> > +  IN CPU_FEATURE_DEPENDENCE_TYPE  NoneNeibAfterDep
> > +  )
> > +{
> > +  return MAX(MAX (MAX(BeforeDep, AfterDep), NoneNeibBeforeDep),
> > NoneNeibAfterDep);
> > +}
> > +
> >  /**
> >Analysis register CPU features on each processor and save CPU
> > setting in CPU register table.
> >
> > @@ -558,6 +580,8 @@ AnalysisProcessorFeatures (
> >BOOLEAN  Success;
> >CPU_FEATURE_DEPENDENCE_TYPE  BeforeDep;
> >CPU_FEATURE_DEPENDENCE_TYPE  AfterDep;
> > +  CPU_FEATURE_DEPENDENCE_TYPE  NoneNeibBeforeDep;
> > +  CPU_FEATURE_DEPENDENCE_TYPE  NoneNeibAfterDep;
> >
> >CpuFeaturesData = GetCpuFeaturesData ();
> >CpuFeaturesData->CapabilityPcd = AllocatePool (CpuFeaturesData-
> > >BitMaskSize);
> > @@ -634,14 +658,9 @@ AnalysisProcessorFeatures (
> >  //
> >  CpuInfo = >InitOrder[ProcessorNumber].CpuInfo;
> >  Entry = GetFirstNode (>OrderList);
> > -NextEntry = Entry->ForwardLink;
> >  while (!IsNull (>OrderList, Entry)) {
> >CpuFeatureInOrder = CPU_FEATURE_ENTRY_FROM_LINK (Entry);
> > -  if (!IsNull (>OrderList, NextEntry)) {
> > -NextCpuFeatureInOrder = CPU_FEATURE_ENTRY_FROM_LINK
> (NextEntry);
> > -  } else {
> > -NextCpuFeatureInOrder = NULL;
> > -  }
> > +
> >Success = FALSE;
> >if (IsBitMaskMatch (CpuFeatureInOrder->FeatureMask,
> > CpuFeaturesData-
> > >SettingPcd)) {
> >  

Re: [edk2] [RFC] TPM non-MMIO Access

[Yao, Jiewen]

Hi Eugene
Complete agree.


1)  please ignore TpmCommLib. It is deprecated. :)


2)  we did notice there is non-MMIO TPM device before and abstract the 
device access via Tpm2DeviceLib and Tpm12DeviceLib library class. The 
Tpm2DeviceLibDTpm and Tpm12DeviceLibDTpm are the library instance for MMIO TIS 
access.

We do have other instance such as 
QuarkPlatformPkg\Library\Tpm12DeviceLibAtmelI2c or 
QuarkPlatformPkg\Library\Tpm12DeviceLibInfineonI2c.


3)  Tcg2Config is also a TPM device oriented driver. It is optional.
You may want to take a look at QuarkPlatformPkg\Quark.dsc to see different TPM 
device is chosen.

  #
  # TPM 1.2 Hardware.  Options are [NONE, LPC, ATMEL_I2C, INFINEON_I2C]
  #
  DEFINE TPM_12_HARDWARE  = NONE

Thank you
Yao Jiewen

From: Cohen, Eugene [mailto:eug...@hp.com]
Sent: Saturday, November 10, 2018 6:38 AM
To: edk2-devel@lists.01.org; Zhang, Chao B ; Yao, 
Jiewen 
Cc: Bin, Sung-Uk (빈성욱) ; Chae, Matthew 
Subject: [RFC] TPM non-MMIO Access

Dear SecurityPkg maintainers,

We are trying to use the SecurityPkg TPM support (Tcg2Config, 
Tpm2DeviceLibDTpm, etc) and see that access to the TPM are using the MMIO 
routines such as MmioRead8().

Not all platforms support memory-mapped access to the TPM so we would like to 
propose that we create a library to abstract the TPM access, called the 
TpmIoLib.  One instance of the library would provide the Mmio (TpmIoLibMmio) 
method but others can use protocols like SPI (TpmIoLibSpi).

Before starting this work I wanted to check if you agree with the approach of 
replacing the Mmio calls with new TpmIoLib ones.  We can upstream the library 
header and mmio library instance (or you could do the work if you think it 
would be easier for you).

I counted up the number of instances of unique MMIO calls in the TPM libraries 
and got

Library/TpmCommLib/TisPc.c:7
Library/Tpm2DeviceLibDTpm/Tpm2Tis.c:13
Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c:29
Library/Tpm12DeviceLibDTpm/Tpm12Tis.c:16

Please let me know if this works for you and how you'd like to structure the 
change.

Thanks,

Eugene




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

Re: [edk2] [PATCH 0/4] OvmfPkg: revert some untimely pushed VMW SVGA reverts

[yuchenlin via edk2-devel]

On 2018-11-10 03:44, Laszlo Ersek wrote:

Repo:   https://github.com/lersek/edk2.git
Branch: revert_revert_bz_1319

Not much to say here, I've written it up in
.

One way to verify this series quickly is:


$ git diff 62ea70e31285..revert_revert_bz_1319 -- OvmfPkg/
[nothing]


Thanks, and sorry about the churn
Laszlo

Cc: Anthony Perard 
Cc: Ard Biesheuvel 
Cc: Jordan Justen 
Cc: Julien Grall 
Cc: Philippe Mathieu-Daudé 
Cc: yuchenlin 

Laszlo Ersek (4):
  Reapply "OvmfPkg: VMWare SVGA display device register definitions"
  Reapply "OvmfPkg/QemuVideoDxe: Helper functions for unaligned port
I/O."
  Reapply "OvmfPkg/QemuVideoDxe: VMWare SVGA device support"
  Reapply "OvmfPkg/QemuVideoDxe: list "UnalignedIoInternal.h" in the 
INF

file"

 OvmfPkg/Include/IndustryStandard/VmwareSvga.h | 104 +
 OvmfPkg/QemuVideoDxe/Driver.c | 135 -
 OvmfPkg/QemuVideoDxe/Gop.c|  65 +++-
 OvmfPkg/QemuVideoDxe/Initialize.c | 157 


 OvmfPkg/QemuVideoDxe/Qemu.h   |  29 
 OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf |   7 +
 OvmfPkg/QemuVideoDxe/UnalignedIoGcc.c |  70 +
 OvmfPkg/QemuVideoDxe/UnalignedIoIcc.c |  80 ++
 OvmfPkg/QemuVideoDxe/UnalignedIoInternal.h|  59 
 OvmfPkg/QemuVideoDxe/UnalignedIoMsc.c |  78 ++
 OvmfPkg/QemuVideoDxe/UnalignedIoUnsupported.c |  66 
 11 files changed, 843 insertions(+), 7 deletions(-)
 create mode 100644 OvmfPkg/Include/IndustryStandard/VmwareSvga.h
 create mode 100644 OvmfPkg/QemuVideoDxe/UnalignedIoGcc.c
 create mode 100644 OvmfPkg/QemuVideoDxe/UnalignedIoIcc.c
 create mode 100644 OvmfPkg/QemuVideoDxe/UnalignedIoInternal.h
 create mode 100644 OvmfPkg/QemuVideoDxe/UnalignedIoMsc.c
 create mode 100644 OvmfPkg/QemuVideoDxe/UnalignedIoUnsupported.c


Thank you for preventing an regression on stable QEMU.

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

Re: [edk2] [RFC] TPM non-MMIO Access

[Yao, Jiewen]

Hi Eugene
The TpmIoLib proposal is similar to the existing TpmDeviceLib.
We have I2C TPM instance as example. You may create Tpm12DeviceLibXXXSpi.

Please let us know if there is any gap to support your non-MMIO device.

Thank you
Yao Jiewen


From: Yao, Jiewen
Sent: Saturday, November 10, 2018 7:12 AM
To: 'Cohen, Eugene' ; edk2-devel@lists.01.org; Zhang, Chao B 

Cc: Bin, Sung-Uk (빈성욱) ; Chae, Matthew 
Subject: RE: [RFC] TPM non-MMIO Access

Hi Eugene
Complete agree.


1)  please ignore TpmCommLib. It is deprecated. :)


2)  we did notice there is non-MMIO TPM device before and abstract the 
device access via Tpm2DeviceLib and Tpm12DeviceLib library class. The 
Tpm2DeviceLibDTpm and Tpm12DeviceLibDTpm are the library instance for MMIO TIS 
access.

We do have other instance such as 
QuarkPlatformPkg\Library\Tpm12DeviceLibAtmelI2c or 
QuarkPlatformPkg\Library\Tpm12DeviceLibInfineonI2c.


3)  Tcg2Config is also a TPM device oriented driver. It is optional.
You may want to take a look at QuarkPlatformPkg\Quark.dsc to see different TPM 
device is chosen.

  #
  # TPM 1.2 Hardware.  Options are [NONE, LPC, ATMEL_I2C, INFINEON_I2C]
  #
  DEFINE TPM_12_HARDWARE  = NONE

Thank you
Yao Jiewen

From: Cohen, Eugene [mailto:eug...@hp.com]
Sent: Saturday, November 10, 2018 6:38 AM
To: edk2-devel@lists.01.org; Zhang, Chao B 
mailto:chao.b.zh...@intel.com>>; Yao, Jiewen 
mailto:jiewen@intel.com>>
Cc: Bin, Sung-Uk (빈성욱) mailto:sunguk-...@hp.com>>; Chae, 
Matthew mailto:matthew.c...@hp.com>>
Subject: [RFC] TPM non-MMIO Access

Dear SecurityPkg maintainers,

We are trying to use the SecurityPkg TPM support (Tcg2Config, 
Tpm2DeviceLibDTpm, etc) and see that access to the TPM are using the MMIO 
routines such as MmioRead8().

Not all platforms support memory-mapped access to the TPM so we would like to 
propose that we create a library to abstract the TPM access, called the 
TpmIoLib.  One instance of the library would provide the Mmio (TpmIoLibMmio) 
method but others can use protocols like SPI (TpmIoLibSpi).

Before starting this work I wanted to check if you agree with the approach of 
replacing the Mmio calls with new TpmIoLib ones.  We can upstream the library 
header and mmio library instance (or you could do the work if you think it 
would be easier for you).

I counted up the number of instances of unique MMIO calls in the TPM libraries 
and got

Library/TpmCommLib/TisPc.c:7
Library/Tpm2DeviceLibDTpm/Tpm2Tis.c:13
Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c:29
Library/Tpm12DeviceLibDTpm/Tpm12Tis.c:16

Please let me know if this works for you and how you'd like to structure the 
change.

Thanks,

Eugene




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

[edk2] [platforms: PATCH v4 6/7] Marvell/Drivers: MvBoardDesc: Extend information for SdMmc

[Marcin Wojtas]

From: Tomasz Michalec 

Extend MvBoardDescSdMmcGet function to fill MV_BOARD_SDMMC_DESC
with Xenon specific info obtained from ArmadaBoardDescLib.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marcin Wojtas 
---
 Silicon/Marvell/Drivers/BoardDesc/MvBoardDescDxe.inf |  1 +
 Silicon/Marvell/Drivers/BoardDesc/MvBoardDescDxe.c   | 24 +---
 2 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/Silicon/Marvell/Drivers/BoardDesc/MvBoardDescDxe.inf 
b/Silicon/Marvell/Drivers/BoardDesc/MvBoardDescDxe.inf
index 41f72d6..0b93948 100644
--- a/Silicon/Marvell/Drivers/BoardDesc/MvBoardDescDxe.inf
+++ b/Silicon/Marvell/Drivers/BoardDesc/MvBoardDescDxe.inf
@@ -47,6 +47,7 @@
   Silicon/Marvell/Marvell.dec
 
 [LibraryClasses]
+  ArmadaBoardDescLib
   ArmadaSoCDescLib
   DebugLib
   MemoryAllocationLib
diff --git a/Silicon/Marvell/Drivers/BoardDesc/MvBoardDescDxe.c 
b/Silicon/Marvell/Drivers/BoardDesc/MvBoardDescDxe.c
index 39dc06c..f71bfc4 100644
--- a/Silicon/Marvell/Drivers/BoardDesc/MvBoardDescDxe.c
+++ b/Silicon/Marvell/Drivers/BoardDesc/MvBoardDescDxe.c
@@ -270,6 +270,7 @@ MvBoardDescSdMmcGet (
 {
   UINT8 *SdMmcDeviceEnabled;
   UINTN SdMmcCount, SdMmcDeviceTableSize, SdMmcIndex, Index;
+  UINTN SdMmcDevCount;
   MV_BOARD_SDMMC_DESC *BoardDesc;
   MV_SOC_SDMMC_DESC *SoCDesc;
   EFI_STATUS Status;
@@ -280,6 +281,13 @@ MvBoardDescSdMmcGet (
 return Status;
   }
 
+  /* Get per-board configuration of the controllers */
+  Status = ArmadaBoardDescSdMmcGet (, );
+  if (EFI_ERROR (Status)) {
+DEBUG ((DEBUG_ERROR, "%a: ArmadaBoardDescSdMmcGet filed\n", __FUNCTION__));
+return Status;
+  }
+
   /*
* Obtain table with enabled SDMMC controllers
* which is represented as an array of UINT8 values
@@ -294,18 +302,12 @@ MvBoardDescSdMmcGet (
   SdMmcDeviceTableSize = PcdGetSize (PcdPciESdhci);
 
   /* Check if PCD with SDMMC controllers is correctly defined */
-  if (SdMmcDeviceTableSize > SdMmcCount) {
+  if ((SdMmcDeviceTableSize > SdMmcCount) ||
+  (SdMmcDeviceTableSize < SdMmcDevCount)) {
 DEBUG ((DEBUG_ERROR, "%a: Wrong PcdPciESdhci format\n", __FUNCTION__));
 return EFI_INVALID_PARAMETER;
   }
 
-  /* Allocate and fill board description */
-  BoardDesc = AllocateZeroPool (SdMmcDeviceTableSize * sizeof 
(MV_BOARD_SDMMC_DESC));
-  if (BoardDesc == NULL) {
-DEBUG ((DEBUG_ERROR, "%a: Cannot allocate memory\n", __FUNCTION__));
-return EFI_OUT_OF_RESOURCES;
-  }
-
   SdMmcIndex = 0;
   for (Index = 0; Index < SdMmcDeviceTableSize; Index++) {
 if (!SdMmcDeviceEnabled[Index]) {
@@ -313,6 +315,12 @@ MvBoardDescSdMmcGet (
   continue;
 }
 
+if (SdMmcIndex >= SdMmcDevCount) {
+  DEBUG ((DEBUG_ERROR,
+"%a: More enabled devices than returned by ArmadaBoardDescSdMmcGet\n",
+__FUNCTION__));
+  return EFI_INVALID_PARAMETER;
+}
 BoardDesc[SdMmcIndex].SoC = [Index];
 SdMmcIndex++;
   }
-- 
2.7.4

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

[edk2] [platforms: PATCH v4 5/7] Marvell/Armada80x0Db: Introduce board description library

[Marcin Wojtas]

From: Tomasz Michalec 

This patch implements ArmadaBoarDescLib library for
Armada8040 Development Board and add to it ArmadaBoardDescSdMmcGet
function with description of connected Xenon host controllers.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marcin Wojtas 
---
 Platform/Marvell/Armada80x0Db/Armada80x0Db.dsc 
 |  3 +
 
Platform/Marvell/Armada80x0Db/Armada80x0DbBoardDescLib/Armada80x0DbBoardDescLib.inf
 | 34 ++
 
Platform/Marvell/Armada80x0Db/Armada80x0DbBoardDescLib/Armada80x0DbBoardDescLib.c
   | 66 
 3 files changed, 103 insertions(+)
 create mode 100644 
Platform/Marvell/Armada80x0Db/Armada80x0DbBoardDescLib/Armada80x0DbBoardDescLib.inf
 create mode 100644 
Platform/Marvell/Armada80x0Db/Armada80x0DbBoardDescLib/Armada80x0DbBoardDescLib.c

diff --git a/Platform/Marvell/Armada80x0Db/Armada80x0Db.dsc 
b/Platform/Marvell/Armada80x0Db/Armada80x0Db.dsc
index 92e2dc8..42f7bd3 100644
--- a/Platform/Marvell/Armada80x0Db/Armada80x0Db.dsc
+++ b/Platform/Marvell/Armada80x0Db/Armada80x0Db.dsc
@@ -54,6 +54,9 @@
 [Components.AARCH64]
   Silicon/Marvell/Armada7k8k/AcpiTables/Armada80x0Db.inf
 
+[LibraryClasses.common]
+  
ArmadaBoardDescLib|Platform/Marvell/Armada80x0Db/Armada80x0DbBoardDescLib/Armada80x0DbBoardDescLib.inf
+
 

 #
 # Pcd Section - list of all EDK II PCD Entries defined by this Platform
diff --git 
a/Platform/Marvell/Armada80x0Db/Armada80x0DbBoardDescLib/Armada80x0DbBoardDescLib.inf
 
b/Platform/Marvell/Armada80x0Db/Armada80x0DbBoardDescLib/Armada80x0DbBoardDescLib.inf
new file mode 100644
index 000..2d39d96
--- /dev/null
+++ 
b/Platform/Marvell/Armada80x0Db/Armada80x0DbBoardDescLib/Armada80x0DbBoardDescLib.inf
@@ -0,0 +1,34 @@
+## @file
+#
+#  Copyright (C) 2018, Marvell International Ltd. and its affiliates
+#
+#  This program and the accompanying materials are licensed and made available
+#  under the terms and conditions of the BSD License which accompanies this
+#  distribution. The full text of the license may be found at
+#  http://opensource.org/licenses/bsd-license.php
+#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR
+#  IMPLIED.
+#
+#
+##
+
+[Defines]
+  INF_VERSION= 0x0001001A
+  BASE_NAME  = Armada80x0DbBoardDescLib
+  FILE_GUID  = fee9e874-1481-4b4f-9882-966bd0d1310f
+  MODULE_TYPE= BASE
+  VERSION_STRING = 1.0
+  LIBRARY_CLASS  = ArmadaBoardDescLib
+
+[Sources]
+  Armada80x0DbBoardDescLib.c
+
+[Packages]
+  MdeModulePkg/MdeModulePkg.dec
+  MdePkg/MdePkg.dec
+  Silicon/Marvell/Marvell.dec
+
+[LibraryClasses]
+  DebugLib
+  IoLib
diff --git 
a/Platform/Marvell/Armada80x0Db/Armada80x0DbBoardDescLib/Armada80x0DbBoardDescLib.c
 
b/Platform/Marvell/Armada80x0Db/Armada80x0DbBoardDescLib/Armada80x0DbBoardDescLib.c
new file mode 100644
index 000..00d696d
--- /dev/null
+++ 
b/Platform/Marvell/Armada80x0Db/Armada80x0DbBoardDescLib/Armada80x0DbBoardDescLib.c
@@ -0,0 +1,66 @@
+/**
+*
+*  Copyright (C) 2018, Marvell International Ltd. and its affiliates.
+*
+*  This program and the accompanying materials are licensed and made available
+*  under the terms and conditions of the BSD License which accompanies this
+*  distribution. The full text of the license may be found at
+*  http://opensource.org/licenses/bsd-license.php
+*
+*  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+*  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
IMPLIED.
+*
+**/
+
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+//
+// Order of devices in SdMmcDescTemplate has to be in par with ArmadaSoCDescLib
+//
+STATIC
+MV_BOARD_SDMMC_DESC mSdMmcDescTemplate[] = {
+  { /* eMMC 0xF06E */
+0, /* SOC will be filled by MvBoardDescDxe */
+0, /* SdMmcDevCount will be filled by MvBoardDescDxe */
+TRUE,  /* Xenon1v8Enabled */
+TRUE,  /* Xenon8BitBusEnabled */
+TRUE,  /* XenonSlowModeEnabled */
+0x40,  /* XenonTuningStepDivisor */
+EmbeddedSlot /* SlotType */
+  },
+  { /* SD/MMC 0xF278 */
+0, /* SOC will be filled by MvBoardDescDxe */
+0, /* SdMmcDevCount will be filled by MvBoardDescDxe */
+FALSE, /* Xenon1v8Enabled */
+FALSE, /* Xenon8BitBusEnabled */
+FALSE, /* XenonSlowModeEnabled */
+0x19,  /* XenonTuningStepDivisor */
+EmbeddedSlot /* SlotType */
+  }
+};
+
+EFI_STATUS
+EFIAPI
+ArmadaBoardDescSdMmcGet (
+  IN OUT UINTN   *SdMmcDevCount,
+  IN OUT MV_BOARD_SDMMC_DESC **SdMmcDesc
+  )
+{
+  *SdMmcDevCount = ARRAY_SIZE (mSdMmcDescTemplate);
+
+  *SdMmcDesc = AllocateCopyPool (sizeof (mSdMmcDescTemplate),
+ );
+  if (*SdMmcDesc == NULL) {
+DEBUG ((DEBUG_ERROR, 

[edk2] [platforms: PATCH v4 4/7] Marvell/Armada70x0Db: Introduce board description library

[Marcin Wojtas]

From: Tomasz Michalec 

This patch implements ArmadaBoarDescLib library for
Armada7040 Development Board and add to it ArmadaBoardDescSdMmcGet
function with description of connected Xenon host controllers.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marcin Wojtas 
---
 Platform/Marvell/Armada70x0Db/Armada70x0Db.dsc 
 |  3 +
 
Platform/Marvell/Armada70x0Db/Armada70x0DbBoardDescLib/Armada70x0DbBoardDescLib.inf
 | 34 ++
 
Platform/Marvell/Armada70x0Db/Armada70x0DbBoardDescLib/Armada70x0DbBoardDescLib.c
   | 66 
 3 files changed, 103 insertions(+)
 create mode 100644 
Platform/Marvell/Armada70x0Db/Armada70x0DbBoardDescLib/Armada70x0DbBoardDescLib.inf
 create mode 100644 
Platform/Marvell/Armada70x0Db/Armada70x0DbBoardDescLib/Armada70x0DbBoardDescLib.c

diff --git a/Platform/Marvell/Armada70x0Db/Armada70x0Db.dsc 
b/Platform/Marvell/Armada70x0Db/Armada70x0Db.dsc
index e0bf447..a935f36 100644
--- a/Platform/Marvell/Armada70x0Db/Armada70x0Db.dsc
+++ b/Platform/Marvell/Armada70x0Db/Armada70x0Db.dsc
@@ -54,6 +54,9 @@
 [Components.AARCH64]
   Silicon/Marvell/Armada7k8k/AcpiTables/Armada70x0Db.inf
 
+[LibraryClasses.common]
+  
ArmadaBoardDescLib|Platform/Marvell/Armada70x0Db/Armada70x0DbBoardDescLib/Armada70x0DbBoardDescLib.inf
+
 

 #
 # Pcd Section - list of all EDK II PCD Entries defined by this Platform
diff --git 
a/Platform/Marvell/Armada70x0Db/Armada70x0DbBoardDescLib/Armada70x0DbBoardDescLib.inf
 
b/Platform/Marvell/Armada70x0Db/Armada70x0DbBoardDescLib/Armada70x0DbBoardDescLib.inf
new file mode 100644
index 000..b26f55b
--- /dev/null
+++ 
b/Platform/Marvell/Armada70x0Db/Armada70x0DbBoardDescLib/Armada70x0DbBoardDescLib.inf
@@ -0,0 +1,34 @@
+## @file
+#
+#  Copyright (C) 2018, Marvell International Ltd. and its affiliates
+#
+#  This program and the accompanying materials are licensed and made available
+#  under the terms and conditions of the BSD License which accompanies this
+#  distribution. The full text of the license may be found at
+#  http://opensource.org/licenses/bsd-license.php
+#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR
+#  IMPLIED.
+#
+#
+##
+
+[Defines]
+  INF_VERSION= 0x0001001A
+  BASE_NAME  = Armada70x0DbBoardDescLib
+  FILE_GUID  = 3164c8d9-19d4-4ad6-8196-cea094b1ddf1
+  MODULE_TYPE= BASE
+  VERSION_STRING = 1.0
+  LIBRARY_CLASS  = ArmadaBoardDescLib
+
+[Sources]
+  Armada70x0DbBoardDescLib.c
+
+[Packages]
+  MdeModulePkg/MdeModulePkg.dec
+  MdePkg/MdePkg.dec
+  Silicon/Marvell/Marvell.dec
+
+[LibraryClasses]
+  DebugLib
+  IoLib
diff --git 
a/Platform/Marvell/Armada70x0Db/Armada70x0DbBoardDescLib/Armada70x0DbBoardDescLib.c
 
b/Platform/Marvell/Armada70x0Db/Armada70x0DbBoardDescLib/Armada70x0DbBoardDescLib.c
new file mode 100644
index 000..dd5e3a0
--- /dev/null
+++ 
b/Platform/Marvell/Armada70x0Db/Armada70x0DbBoardDescLib/Armada70x0DbBoardDescLib.c
@@ -0,0 +1,66 @@
+/**
+*
+*  Copyright (C) 2018, Marvell International Ltd. and its affiliates.
+*
+*  This program and the accompanying materials are licensed and made available
+*  under the terms and conditions of the BSD License which accompanies this
+*  distribution. The full text of the license may be found at
+*  http://opensource.org/licenses/bsd-license.php
+*
+*  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+*  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
IMPLIED.
+*
+**/
+
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+//
+// Order of devices in SdMmcDescTemplate has to be in par with ArmadaSoCDescLib
+//
+STATIC
+MV_BOARD_SDMMC_DESC mSdMmcDescTemplate[] = {
+  { /* eMMC 0xF06E */
+0, /* SOC will be filled by MvBoardDescDxe */
+0, /* SdMmcDevCount will be filled by MvBoardDescDxe */
+FALSE, /* Xenon1v8Enabled */
+FALSE, /* Xenon8BitBusEnabled */
+TRUE,  /* XenonSlowModeEnabled */
+0x40,  /* XenonTuningStepDivisor */
+EmbeddedSlot /* SlotType */
+  },
+  { /* SD/MMC 0xF278 */
+0, /* SOC will be filled by MvBoardDescDxe */
+0, /* SdMmcDevCount will be filled by MvBoardDescDxe */
+FALSE, /* Xenon1v8Enabled */
+FALSE, /* Xenon8BitBusEnabled */
+FALSE, /* XenonSlowModeEnabled */
+0x19,  /* XenonTuningStepDivisor */
+EmbeddedSlot /* SlotType */
+  }
+};
+
+EFI_STATUS
+EFIAPI
+ArmadaBoardDescSdMmcGet (
+  IN OUT UINTN   *SdMmcDevCount,
+  IN OUT MV_BOARD_SDMMC_DESC **SdMmcDesc
+  )
+{
+  *SdMmcDevCount = ARRAY_SIZE (mSdMmcDescTemplate);
+
+  *SdMmcDesc = AllocateCopyPool (sizeof (mSdMmcDescTemplate),
+ );
+  if (*SdMmcDesc == NULL) {
+DEBUG ((DEBUG_ERROR, 

[edk2] [platforms: PATCH v4 3/7] SolidRun/Armada80x0McBin: Introduce board description library

[Marcin Wojtas]

From: Tomasz Michalec 

This patch implements ArmadaBoarDescLib library for
Armada80x0McBin comunity board and add to it ArmadaBoardDescSdMmcGet
function with description of connected Xenon host controllers.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marcin Wojtas 
---
 Platform/SolidRun/Armada80x0McBin/Armada80x0McBin.dsc  
   |  3 +
 
Platform/SolidRun/Armada80x0McBin/Armada80x0McBinBoardDescLib/Armada80x0McBinBoardDescLib.inf
 | 34 ++
 
Platform/SolidRun/Armada80x0McBin/Armada80x0McBinBoardDescLib/Armada80x0McBinBoardDescLib.c
   | 66 
 3 files changed, 103 insertions(+)
 create mode 100644 
Platform/SolidRun/Armada80x0McBin/Armada80x0McBinBoardDescLib/Armada80x0McBinBoardDescLib.inf
 create mode 100644 
Platform/SolidRun/Armada80x0McBin/Armada80x0McBinBoardDescLib/Armada80x0McBinBoardDescLib.c

diff --git a/Platform/SolidRun/Armada80x0McBin/Armada80x0McBin.dsc 
b/Platform/SolidRun/Armada80x0McBin/Armada80x0McBin.dsc
index 52e2b9b..077224d 100644
--- a/Platform/SolidRun/Armada80x0McBin/Armada80x0McBin.dsc
+++ b/Platform/SolidRun/Armada80x0McBin/Armada80x0McBin.dsc
@@ -55,6 +55,9 @@
 [Components.AARCH64]
   Silicon/Marvell/Armada7k8k/AcpiTables/Armada80x0McBin.inf
 
+[LibraryClasses.common]
+  
ArmadaBoardDescLib|Platform/SolidRun/Armada80x0McBin/Armada80x0McBinBoardDescLib/Armada80x0McBinBoardDescLib.inf
+
 

 #
 # Pcd Section - list of all EDK II PCD Entries defined by this Platform
diff --git 
a/Platform/SolidRun/Armada80x0McBin/Armada80x0McBinBoardDescLib/Armada80x0McBinBoardDescLib.inf
 
b/Platform/SolidRun/Armada80x0McBin/Armada80x0McBinBoardDescLib/Armada80x0McBinBoardDescLib.inf
new file mode 100644
index 000..63a4f66
--- /dev/null
+++ 
b/Platform/SolidRun/Armada80x0McBin/Armada80x0McBinBoardDescLib/Armada80x0McBinBoardDescLib.inf
@@ -0,0 +1,34 @@
+## @file
+#
+#  Copyright (C) 2018, Marvell International Ltd. and its affiliates
+#
+#  This program and the accompanying materials are licensed and made available
+#  under the terms and conditions of the BSD License which accompanies this
+#  distribution. The full text of the license may be found at
+#  http://opensource.org/licenses/bsd-license.php
+#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR
+#  IMPLIED.
+#
+#
+##
+
+[Defines]
+  INF_VERSION= 0x0001001A
+  BASE_NAME  = ArmadaMcBinBoardDescLib
+  FILE_GUID  = 8208558f-5f33-46e2-b5c5-43354384389e
+  MODULE_TYPE= BASE
+  VERSION_STRING = 1.0
+  LIBRARY_CLASS  = ArmadaBoardDescLib
+
+[Sources]
+  Armada80x0McBinBoardDescLib.c
+
+[Packages]
+  MdeModulePkg/MdeModulePkg.dec
+  MdePkg/MdePkg.dec
+  Silicon/Marvell/Marvell.dec
+
+[LibraryClasses]
+  DebugLib
+  IoLib
diff --git 
a/Platform/SolidRun/Armada80x0McBin/Armada80x0McBinBoardDescLib/Armada80x0McBinBoardDescLib.c
 
b/Platform/SolidRun/Armada80x0McBin/Armada80x0McBinBoardDescLib/Armada80x0McBinBoardDescLib.c
new file mode 100644
index 000..9e38ce0
--- /dev/null
+++ 
b/Platform/SolidRun/Armada80x0McBin/Armada80x0McBinBoardDescLib/Armada80x0McBinBoardDescLib.c
@@ -0,0 +1,66 @@
+/**
+*
+*  Copyright (C) 2018, Marvell International Ltd. and its affiliates.
+*
+*  This program and the accompanying materials are licensed and made available
+*  under the terms and conditions of the BSD License which accompanies this
+*  distribution. The full text of the license may be found at
+*  http://opensource.org/licenses/bsd-license.php
+*
+*  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+*  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
IMPLIED.
+*
+**/
+
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+//
+// Order of devices in SdMmcDescTemplate has to be in par with ArmadaSoCDescLib
+//
+STATIC
+MV_BOARD_SDMMC_DESC mMcBinSdMmcDescTemplate[] = {
+  { /* eMMC 0xF06E */
+0, /* SOC will be filled by MvBoardDescDxe */
+0, /* SdMmcDevCount will be filled by MvBoardDescDxe */
+FALSE, /* Xenon1v8Enabled */
+TRUE,  /* Xenon8BitBusEnabled */
+TRUE,  /* XenonSlowModeEnabled */
+0x40,  /* XenonTuningStepDivisor */
+EmbeddedSlot /* SlotType */
+  },
+  { /* SD/MMC 0xF278 */
+0, /* SOC will be filled by MvBoardDescDxe */
+0, /* SdMmcDevCount will be filled by MvBoardDescDxe */
+FALSE, /* Xenon1v8Enabled */
+FALSE, /* Xenon8BitBusEnabled */
+FALSE, /* XenonSlowModeEnabled */
+0x19,  /* XenonTuningStepDivisor */
+EmbeddedSlot /* SlotType */
+  }
+};
+
+EFI_STATUS
+EFIAPI
+ArmadaBoardDescSdMmcGet (
+  IN OUT UINTN   *SdMmcDevCount,
+  IN OUT MV_BOARD_SDMMC_DESC **SdMmcDesc
+  )
+{
+  *SdMmcDevCount = ARRAY_SIZE 

[edk2] [platforms: PATCH v4 2/7] Marvell/Library: ArmadaBoardDescLib: Extend SDMMC information

[Marcin Wojtas]

From: Tomasz Michalec 

Added fields specific for Xenon host controller and declaration
of ArmadaBoardDescSdMmcGet function.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marcin Wojtas 
---
 Silicon/Marvell/Include/Library/ArmadaBoardDescLib.h | 22 +++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/Silicon/Marvell/Include/Library/ArmadaBoardDescLib.h 
b/Silicon/Marvell/Include/Library/ArmadaBoardDescLib.h
index ee8e06e..8e29a68 100644
--- a/Silicon/Marvell/Include/Library/ArmadaBoardDescLib.h
+++ b/Silicon/Marvell/Include/Library/ArmadaBoardDescLib.h
@@ -55,9 +55,21 @@ typedef struct {
 //
 // SDMMC devices per-board description
 //
+typedef enum {
+  RemovableSlot,
+  EmbeddedSlot,
+  SharedBusSlot,
+  UnknownSlot
+} MV_SDMMC_SLOT_TYPE;
+
 typedef struct {
   MV_SOC_SDMMC_DESC *SoC;
-  UINTN  SdMmcDevCount;
+  UINTNSdMmcDevCount;
+  BOOLEAN  Xenon1v8Enabled;
+  BOOLEAN  Xenon8BitBusEnabled;
+  BOOLEAN  XenonSlowModeEnabled;
+  UINT8XenonTuningStepDivisor;
+  MV_SDMMC_SLOT_TYPE SlotType;
 } MV_BOARD_SDMMC_DESC;
 
 //
@@ -84,4 +96,12 @@ typedef struct {
   UINTN UtmiDevCount;
   UINTN UtmiPortType;
 } MV_BOARD_UTMI_DESC;
+
+EFI_STATUS
+EFIAPI
+ArmadaBoardDescSdMmcGet (
+  IN OUT UINTN   *SdMmcDevCount,
+  IN OUT MV_BOARD_SDMMC_DESC **SdMmcDesc
+  );
+
 #endif /* __ARMADA_SOC_DESC_LIB_H__ */
-- 
2.7.4

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

[edk2] [platforms: PATCH v4 1/7] Silicon/SynQuacer/PlatformDxe: adjust to updated SdMmcOverride

[Marcin Wojtas]

The newest changes in the SdMmcOverride protocol added additional
arguments to the NotifyPhase and Capability routines. Update
according places in the Synquacer Emmc driver.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marcin Wojtas 
---
 Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Emmc.c | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Emmc.c 
b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Emmc.c
index e0987c9..47f5ccc 100644
--- a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Emmc.c
+++ b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Emmc.c
@@ -72,6 +72,8 @@ STATIC VOID *mEventRegistration;
   @param[in]  ControllerHandle  The EFI_HANDLE of the controller.
   @param[in]  Slot  The 0 based slot index.
   @param[in,out]  SdMmcHcSlotCapability The SDHCI capability structure.
+  @param[in,out]  BaseClkFreq   The base clock frequency value that
+optionally can be updated.
 
   @retval EFI_SUCCESS   The override function completed successfully.
   @retval EFI_NOT_FOUND The specified controller or slot does not 
exist.
@@ -84,7 +86,8 @@ EFIAPI
 SynQuacerSdMmcCapability (
   IN  EFI_HANDLE  ControllerHandle,
   IN  UINT8   Slot,
-  IN  OUT VOID*SdMmcHcSlotCapability
+  IN OUT  VOID*SdMmcHcSlotCapability,
+  IN OUT  UINT32  *BaseClkFreq
   )
 {
   UINT64 Capability;
@@ -117,6 +120,7 @@ SynQuacerSdMmcCapability (
   @param[in]  PhaseType The type of operation and whether the
 hook is invoked right before (pre) or
 right after (post)
+  @param[in,out]  PhaseData The pointer to a phase-specific data.
 
   @retval EFI_SUCCESS   The override function completed successfully.
   @retval EFI_NOT_FOUND The specified controller or slot does not 
exist.
@@ -129,7 +133,8 @@ EFIAPI
 SynQuacerSdMmcNotifyPhase (
   IN  EFI_HANDLE  ControllerHandle,
   IN  UINT8   Slot,
-  IN  EDKII_SD_MMC_PHASE_TYPE PhaseType
+  IN  EDKII_SD_MMC_PHASE_TYPE PhaseType,
+  IN OUT  VOID   *PhaseData
   )
 {
   if (ControllerHandle != mSdMmcControllerHandle) {
-- 
2.7.4

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

[edk2] [platforms: PATCH v4 0/7] Armada7k8k Xenon driver rework

[Marcin Wojtas]

Hi,

The fourth version of the patchset removes dependency on
the internal MdeModulePkg header and reworks Capability
callback handling. Details can be found in the changelog below.

Patches are available in the github:
https://github.com/MarvellEmbeddedProcessors/edk2-open-platform/commits/xenon-upstream-r20181109

Generic driver patches with fixes and extended SdMmcOverride protocol:
https://github.com/MarvellEmbeddedProcessors/edk2-open-platform/commits/sdmmc-override-upstream-r20181109

I'm looking forward to the comments and remarks.

Best regards,
Marcin

Changelog:
v3->v4:
* 1/7:
  - add Ard's RB

* 2/7:
  - use local enum definition for SlotType in order not to include MdeModulePkg
private header

* 7/7:
  - rework capability handling, without using the structure defined in the
MdeModulePkg header

v2->v3
* 1/7:
  - rename NotifyPhase parameter to PhaseData

* 7/7:
  - rename NotifyPhase parameter to PhaseData
  - update UHS_MODE_SEL only for HS200/HS400
 in XenonSdMmcHcUhsSignaling
  - use local macros for standard SDHC registers in order not to
include private MdeModulePkg header

v1 -> v2
* 1/7 and 7/7 - adjust to modified SdMmcOverride
  NotifyPhase and Capability routines


Marcin Wojtas (1):
  Silicon/SynQuacer/PlatformDxe: adjust to updated SdMmcOverride

Tomasz Michalec (6):
  Marvell/Library: ArmadaBoardDescLib: Extend SDMMC information
  SolidRun/Armada80x0McBin: Introduce board description library
  Marvell/Armada70x0Db: Introduce board description library
  Marvell/Armada80x0Db: Introduce board description library
  Marvell/Drivers: MvBoardDesc: Extend information for SdMmc
  Marvell/Drivers: XenonDxe: Switch to use generic SdMmcPciHcDxe

 Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc  
   |3 +-
 Platform/Marvell/Armada70x0Db/Armada70x0Db.dsc 
   |3 +
 Platform/Marvell/Armada80x0Db/Armada80x0Db.dsc 
   |3 +
 Platform/SolidRun/Armada80x0McBin/Armada80x0McBin.dsc  
   |3 +
 Silicon/Marvell/Armada7k8k/Armada7k8k.fdf  
   |3 +-
 
Platform/Marvell/Armada70x0Db/Armada70x0DbBoardDescLib/Armada70x0DbBoardDescLib.inf
   |   34 +
 
Platform/Marvell/Armada80x0Db/Armada80x0DbBoardDescLib/Armada80x0DbBoardDescLib.inf
   |   34 +
 
Platform/SolidRun/Armada80x0McBin/Armada80x0McBinBoardDescLib/Armada80x0McBinBoardDescLib.inf
 |   34 +
 Silicon/Marvell/Drivers/BoardDesc/MvBoardDescDxe.inf   
   |1 +
 Silicon/Marvell/Drivers/SdMmc/XenonDxe/{SdMmcPciHcDxe.inf => XenonDxe.inf} 
   |   33 +-
 Silicon/Marvell/Drivers/SdMmc/XenonDxe/SdMmcPciHcDxe.h 
   |  791 
 Silicon/Marvell/Drivers/SdMmc/XenonDxe/SdMmcPciHci.h   
   |  550 --
 Silicon/Marvell/Drivers/SdMmc/XenonDxe/XenonPciHci.h   
   |  151 ++
 Silicon/Marvell/Drivers/SdMmc/XenonDxe/XenonSdMmcOverride.h
   |   53 +
 Silicon/Marvell/Drivers/SdMmc/XenonDxe/XenonSdhci.h
   |  131 +-
 Silicon/Marvell/Include/Library/ArmadaBoardDescLib.h   
   |   22 +-
 
Platform/Marvell/Armada70x0Db/Armada70x0DbBoardDescLib/Armada70x0DbBoardDescLib.c
 |   66 +
 
Platform/Marvell/Armada80x0Db/Armada80x0DbBoardDescLib/Armada80x0DbBoardDescLib.c
 |   66 +
 
Platform/SolidRun/Armada80x0McBin/Armada80x0McBinBoardDescLib/Armada80x0McBinBoardDescLib.c
   |   66 +
 Silicon/Marvell/Drivers/BoardDesc/MvBoardDescDxe.c 
   |   24 +-
 Silicon/Marvell/Drivers/SdMmc/XenonDxe/ComponentName.c 
   |  211 ---
 Silicon/Marvell/Drivers/SdMmc/XenonDxe/EmmcDevice.c
   | 1164 
 Silicon/Marvell/Drivers/SdMmc/XenonDxe/SdDevice.c  
   | 1190 
 Silicon/Marvell/Drivers/SdMmc/XenonDxe/SdMmcPciHcDxe.c 
   | 1320 --
 Silicon/Marvell/Drivers/SdMmc/XenonDxe/SdMmcPciHci.c   
   | 1928 
 Silicon/Marvell/Drivers/SdMmc/XenonDxe/XenonPciHci.c   
   |  321 
 Silicon/Marvell/Drivers/SdMmc/XenonDxe/XenonSdMmcOverride.c
   |  432 +
 Silicon/Marvell/Drivers/SdMmc/XenonDxe/XenonSdhci.c
   |  408 +++--
 Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Emmc.c 
   |9 +-
 Silicon/Marvell/Drivers/SdMmc/XenonDxe/SdMmcPciHcDxe.uni   
   |   23 -
 

[edk2] [PATCH v4 4/4] MdeModulePkg/SdMmcPciHcDxe: Allow overriding base clock frequency

[Marcin Wojtas]

Some SdMmc host controllers are run by clocks with different
frequency than it is reflected in Capabilities Register 1.
It is allowed by SDHCI specification ver. 4.2 - if BaseClkFreq
field value of the Capability Register 1 is zero, the clock
frequency must be obtained via another method.

Because the bitfield is only 8 bits wide, a maximum value
that could be obtained from hardware is 255MHz.
In case the actual frequency exceeds 255MHz, the 8-bit BaseClkFreq
member of SD_MMC_HC_SLOT_CAP structure occurs to be not sufficient
to be used for setting the clock speed in SdMmcHcClockSupply
function.

This patch adds new UINT32 array ('BaseClkFreq[]') to
SD_MMC_HC_PRIVATE_DATA structure for specifying
the input clock speed for each slot of the host controller.
All routines that are used for clock configuration are
updated accordingly.

This patch also adds new IN OUT BaseClockFreq field
in the Capability callback of the SdMmcOverride,
protocol which allows to update BaseClkFreq value.

The patch reuses original commit from edk2-platforms:
20f6f144d3a8 ("Marvell/Drivers: XenonDxe: Allow overriding base clock
frequency")

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marcin Wojtas 
Reviewed-by: Ard Biesheuvel 
---
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h |  6 +
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h   |  8 +++
 MdeModulePkg/Include/Protocol/SdMmcOverride.h  |  7 --
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c|  4 ++--
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c  |  4 ++--
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c | 13 ++-
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c   | 23 ++--
 7 files changed, 43 insertions(+), 22 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h 
b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
index c683600..8c1a589 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
@@ -118,6 +118,12 @@ typedef struct {
   UINT64  MaxCurrent[SD_MMC_HC_MAX_SLOT];
 
   UINT32  ControllerVersion;
+
+  //
+  // Some controllers may require to override base clock frequency
+  // value stored in Capabilities Register 1.
+  //
+  UINT32  BaseClkFreq[SD_MMC_HC_MAX_SLOT];
 } SD_MMC_HC_PRIVATE_DATA;
 
 #define SD_MMC_HC_TRB_SIG SIGNATURE_32 ('T', 'R', 'B', 'T')
diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h 
b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
index b47b0df..dd45cbd 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
@@ -423,7 +423,7 @@ SdMmcHcStopClock (
   @param[in] PciIo  The PCI IO protocol instance.
   @param[in] Slot   The slot number of the SD card to send the command 
to.
   @param[in] ClockFreq  The max clock frequency to be set. The unit is KHz.
-  @param[in] Capability The capability of the slot.
+  @param[in] BaseClkFreqThe base clock frequency of host controller in MHz.
 
   @retval EFI_SUCCESS   The clock is supplied successfully.
   @retval OthersThe clock isn't supplied successfully.
@@ -434,7 +434,7 @@ SdMmcHcClockSupply (
   IN EFI_PCI_IO_PROTOCOL*PciIo,
   IN UINT8  Slot,
   IN UINT64 ClockFreq,
-  IN SD_MMC_HC_SLOT_CAP Capability
+  IN UINT32 BaseClkFreq
   );
 
 /**
@@ -482,7 +482,7 @@ SdMmcHcSetBusWidth (
 
   @param[in] PciIo  The PCI IO protocol instance.
   @param[in] Slot   The slot number of the SD card to send the command 
to.
-  @param[in] Capability The capability of the slot.
+  @param[in] BaseClkFreqThe base clock frequency of host controller in MHz.
 
   @retval EFI_SUCCESS   The clock is supplied successfully.
   @retval OthersThe clock isn't supplied successfully.
@@ -492,7 +492,7 @@ EFI_STATUS
 SdMmcHcInitClockFreq (
   IN EFI_PCI_IO_PROTOCOL*PciIo,
   IN UINT8  Slot,
-  IN SD_MMC_HC_SLOT_CAP Capability
+  IN UINT32 BaseClkFreq
   );
 
 /**
diff --git a/MdeModulePkg/Include/Protocol/SdMmcOverride.h 
b/MdeModulePkg/Include/Protocol/SdMmcOverride.h
index 6160b5b..0aaf258 100644
--- a/MdeModulePkg/Include/Protocol/SdMmcOverride.h
+++ b/MdeModulePkg/Include/Protocol/SdMmcOverride.h
@@ -22,7 +22,7 @@
 #define EDKII_SD_MMC_OVERRIDE_PROTOCOL_GUID \
   { 0xeaf9e3c1, 0xc9cd, 0x46db, { 0xa5, 0xe5, 0x5a, 0x12, 0x4c, 0x83, 0x23, 
0x23 } }
 
-#define EDKII_SD_MMC_OVERRIDE_PROTOCOL_VERSION0x1
+#define EDKII_SD_MMC_OVERRIDE_PROTOCOL_VERSION0x2
 
 typedef struct _EDKII_SD_MMC_OVERRIDE EDKII_SD_MMC_OVERRIDE;
 
@@ -58,6 +58,8 @@ typedef enum {
   @param[in]  ControllerHandle  The EFI_HANDLE of the controller.
   @param[in]  Slot  The 0 based slot index.
   @param[in,out]  

[edk2] [PATCH v4 3/4] MdeModulePkg/SdMmcPciHcDxe: Add SwitchClockFreqPost to SdMmcOverride

[Marcin Wojtas]

From: Tomasz Michalec 

Some SD Host Controlers need to do additional opperations after clock
frequency switch.

This patch add new callback type to NotifyPhase of the SdMmcOverride
protocol. It is called after SdMmcHcClockSupply.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marcin Wojtas 
---
 MdeModulePkg/Include/Protocol/SdMmcOverride.h   |  1 +
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c | 31 +---
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c   | 18 
 3 files changed, 46 insertions(+), 4 deletions(-)

diff --git a/MdeModulePkg/Include/Protocol/SdMmcOverride.h 
b/MdeModulePkg/Include/Protocol/SdMmcOverride.h
index f948bef..6160b5b 100644
--- a/MdeModulePkg/Include/Protocol/SdMmcOverride.h
+++ b/MdeModulePkg/Include/Protocol/SdMmcOverride.h
@@ -48,6 +48,7 @@ typedef enum {
   EdkiiSdMmcInitHostPre,
   EdkiiSdMmcInitHostPost,
   EdkiiSdMmcUhsSignaling,
+  EdkiiSdMmcSwitchClockFreqPost,
 } EDKII_SD_MMC_PHASE_TYPE;
 
 /**
diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c 
b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
index db4e8a1..b75a9bb 100755
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
@@ -651,6 +651,7 @@ EmmcSwitchBusWidth (
   @param[in] Slot   The slot number of the SD card to send the command 
to.
   @param[in] RcaThe relative device address to be assigned.
   @param[in] HsTiming   The value to be written to HS_TIMING field of 
EXT_CSD register.
+  @param[in] Timing The bus mode timing indicator.
   @param[in] ClockFreq  The max clock frequency to be set, the unit is MHz.
 
   @retval EFI_SUCCESS   The operation is done correctly.
@@ -664,6 +665,7 @@ EmmcSwitchClockFreq (
   IN UINT8  Slot,
   IN UINT16 Rca,
   IN UINT8  HsTiming,
+  IN SD_MMC_BUS_MODETiming,
   IN UINT32 ClockFreq
   )
 {
@@ -706,6 +708,27 @@ EmmcSwitchClockFreq (
   // Convert the clock freq unit from MHz to KHz.
   //
   Status = SdMmcHcClockSupply (PciIo, Slot, ClockFreq * 1000, 
Private->Capability[Slot]);
+  if (EFI_ERROR (Status)) {
+return Status;
+  }
+
+  if (mOverride != NULL && mOverride->NotifyPhase != NULL) {
+Status = mOverride->NotifyPhase (
+  Private->ControllerHandle,
+  Slot,
+  EdkiiSdMmcSwitchClockFreqPost,
+  
+  );
+if (EFI_ERROR (Status)) {
+  DEBUG ((
+DEBUG_ERROR,
+"%a: SD/MMC switch clock freq post notifier callback failed - %r\n",
+__FUNCTION__,
+Status
+));
+  return Status;
+}
+  }
 
   return Status;
 }
@@ -775,7 +798,7 @@ EmmcSwitchToHighSpeed (
   }
 
   HsTiming = 1;
-  Status = EmmcSwitchClockFreq (PciIo, PassThru, Slot, Rca, HsTiming, 
ClockFreq);
+  Status = EmmcSwitchClockFreq (PciIo, PassThru, Slot, Rca, HsTiming, Timing, 
ClockFreq);
 
   return Status;
 }
@@ -863,7 +886,7 @@ EmmcSwitchToHS200 (
   Status = SdMmcHcOrMmio (PciIo, Slot, SD_MMC_HC_CLOCK_CTRL, sizeof 
(ClockCtrl), );
 
   HsTiming = 2;
-  Status = EmmcSwitchClockFreq (PciIo, PassThru, Slot, Rca, HsTiming, 
ClockFreq);
+  Status = EmmcSwitchClockFreq (PciIo, PassThru, Slot, Rca, HsTiming, Timing, 
ClockFreq);
   if (EFI_ERROR (Status)) {
 return Status;
   }
@@ -913,7 +936,7 @@ EmmcSwitchToHS400 (
   // Set to Hight Speed timing and set the clock frequency to a value less 
than 52MHz.
   //
   HsTiming = 1;
-  Status = EmmcSwitchClockFreq (PciIo, PassThru, Slot, Rca, HsTiming, 52);
+  Status = EmmcSwitchClockFreq (PciIo, PassThru, Slot, Rca, HsTiming, 
SdMmcMmcHsSdr, 52);
   if (EFI_ERROR (Status)) {
 return Status;
   }
@@ -933,7 +956,7 @@ EmmcSwitchToHS400 (
   }
 
   HsTiming = 3;
-  Status = EmmcSwitchClockFreq (PciIo, PassThru, Slot, Rca, HsTiming, 
ClockFreq);
+  Status = EmmcSwitchClockFreq (PciIo, PassThru, Slot, Rca, HsTiming, Timing, 
ClockFreq);
 
   return Status;
 }
diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c 
b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
index 55b3564..32fd416 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
@@ -869,6 +869,24 @@ SdCardSetBusMode (
 return Status;
   }
 
+  if (mOverride != NULL && mOverride->NotifyPhase != NULL) {
+Status = mOverride->NotifyPhase (
+  Private->ControllerHandle,
+  Slot,
+  EdkiiSdMmcSwitchClockFreqPost,
+  
+  );
+if (EFI_ERROR (Status)) {
+  DEBUG ((
+DEBUG_ERROR,
+"%a: SD/MMC switch clock freq post notifier callback failed - %r\n",
+__FUNCTION__,
+Status
+));
+  return Status;
+}
+  }
+
   if ((AccessMode == 3) 

[edk2] [PATCH v4 2/4] MdeModulePkg/SdMmcPciHcDxe: Add UhsSignaling to SdMmcOverride protocol

[Marcin Wojtas]

From: Tomasz Michalec 

Some SD Host Controllers use different values in Host Control 2 Register
to select UHS Mode. This patch adds a new UhsSignaling type routine to
the NotifyPhase of the SdMmcOverride protocol.

UHS signaling configuration is moved to a common, default routine
(SdMmcHcUhsSignaling). After it is executed, the protocol producer
can override the values if needed.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marcin Wojtas 
---
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h | 34 
 MdeModulePkg/Include/Protocol/SdMmcOverride.h| 17 
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c  | 86 ---
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c| 15 ++--
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c | 89 
 5 files changed, 181 insertions(+), 60 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h 
b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
index 7e3f588..b47b0df 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
@@ -63,6 +63,21 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 #define SD_MMC_HC_CTRL_VER0xFE
 
 //
+// SD Host Controller bits to HOST_CTRL2 register
+//
+#define SD_MMC_HC_CTRL_UHS_MASK   0x0007
+#define SD_MMC_HC_CTRL_UHS_SDR12  0x
+#define SD_MMC_HC_CTRL_UHS_SDR25  0x0001
+#define SD_MMC_HC_CTRL_UHS_SDR50  0x0002
+#define SD_MMC_HC_CTRL_UHS_SDR104 0x0003
+#define SD_MMC_HC_CTRL_UHS_DDR50  0x0004
+#define SD_MMC_HC_CTRL_MMC_LEGACY 0x
+#define SD_MMC_HC_CTRL_MMC_HS_SDR 0x0001
+#define SD_MMC_HC_CTRL_MMC_HS_DDR 0x0004
+#define SD_MMC_HC_CTRL_MMC_HS200  0x0003
+#define SD_MMC_HC_CTRL_MMC_HS400  0x0005
+
+//
 // The transfer modes supported by SD Host Controller
 // Simplified Spec 3.0 Table 1-2
 //
@@ -518,4 +533,23 @@ SdMmcHcInitTimeoutCtrl (
   IN UINT8  Slot
   );
 
+/**
+  Set SD Host Controller control 2 registry according to selected speed.
+
+  @param[in] ControllerHandle The handle of the controller.
+  @param[in] PciIoThe PCI IO protocol instance.
+  @param[in] Slot The slot number of the SD card to send the 
command to.
+  @param[in] Timing   The timing to select.
+
+  @retval EFI_SUCCESS The timing is set successfully.
+  @retval Others  The timing isn't set successfully.
+**/
+EFI_STATUS
+SdMmcHcUhsSignaling (
+  IN EFI_HANDLE ControllerHandle,
+  IN EFI_PCI_IO_PROTOCOL*PciIo,
+  IN UINT8  Slot,
+  IN SD_MMC_BUS_MODETiming
+  );
+
 #endif
diff --git a/MdeModulePkg/Include/Protocol/SdMmcOverride.h 
b/MdeModulePkg/Include/Protocol/SdMmcOverride.h
index 8a7669e..f948bef 100644
--- a/MdeModulePkg/Include/Protocol/SdMmcOverride.h
+++ b/MdeModulePkg/Include/Protocol/SdMmcOverride.h
@@ -26,11 +26,28 @@
 
 typedef struct _EDKII_SD_MMC_OVERRIDE EDKII_SD_MMC_OVERRIDE;
 
+//
+// Bus timing modes
+//
+typedef enum {
+  SdMmcUhsSdr12,
+  SdMmcUhsSdr25,
+  SdMmcUhsSdr50,
+  SdMmcUhsSdr104,
+  SdMmcUhsDdr50,
+  SdMmcMmcLegacy,
+  SdMmcMmcHsSdr,
+  SdMmcMmcHsDdr,
+  SdMmcMmcHs200,
+  SdMmcMmcHs400,
+} SD_MMC_BUS_MODE;
+
 typedef enum {
   EdkiiSdMmcResetPre,
   EdkiiSdMmcResetPost,
   EdkiiSdMmcInitHostPre,
   EdkiiSdMmcInitHostPost,
+  EdkiiSdMmcUhsSignaling,
 } EDKII_SD_MMC_PHASE_TYPE;
 
 /**
diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c 
b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
index c5fd214..db4e8a1 100755
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
@@ -740,10 +740,13 @@ EmmcSwitchToHighSpeed (
   IN UINT8  BusWidth
   )
 {
-  EFI_STATUS  Status;
-  UINT8   HsTiming;
-  UINT8   HostCtrl1;
-  UINT8   HostCtrl2;
+  EFI_STATUS  Status;
+  UINT8   HsTiming;
+  UINT8   HostCtrl1;
+  SD_MMC_BUS_MODE Timing;
+  SD_MMC_HC_PRIVATE_DATA  *Private;
+
+  Private = SD_MMC_HC_PRIVATE_FROM_THIS (PassThru);
 
   Status = EmmcSwitchBusWidth (PciIo, PassThru, Slot, Rca, IsDdr, BusWidth);
   if (EFI_ERROR (Status)) {
@@ -758,25 +761,15 @@ EmmcSwitchToHighSpeed (
 return Status;
   }
 
-  //
-  // Clean UHS Mode Select field of Host Control 2 reigster before update
-  //
-  HostCtrl2 = (UINT8)~0x7;
-  Status = SdMmcHcAndMmio (PciIo, Slot, SD_MMC_HC_HOST_CTRL2, sizeof 
(HostCtrl2), );
-  if (EFI_ERROR (Status)) {
-return Status;
-  }
-  //
-  // Set UHS Mode Select field of Host Control 2 reigster to SDR12/25/50
-  //
   if (IsDdr) {
-HostCtrl2 = BIT2;
+Timing = SdMmcMmcHsDdr;
   } else if (ClockFreq == 52) {
-HostCtrl2 = BIT0;
+Timing = SdMmcMmcHsSdr;
   } else {
-HostCtrl2 = 0;
+Timing = SdMmcMmcLegacy;
   }
-  Status = SdMmcHcOrMmio (PciIo, Slot, SD_MMC_HC_HOST_CTRL2, sizeof 

[edk2] [PATCH v4 1/4] MdeModulePkg/SdMmcPciHcDxe: Add an optional parameter in NotifyPhase

[Marcin Wojtas]

In order to ensure bigger flexibility in the NotifyPhase
routine of the SdMmcOverride protocol, enable using an
optional phase-specific data. This will allow to exchange
more information between the protocol producer driver
and SdMmcPciHcDxe in the newly added callbacks.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marcin Wojtas 
Reviewed-by: Ard Biesheuvel 
---
 MdeModulePkg/Include/Protocol/SdMmcOverride.h|  4 +++-
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c | 12 
 2 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/MdeModulePkg/Include/Protocol/SdMmcOverride.h 
b/MdeModulePkg/Include/Protocol/SdMmcOverride.h
index 0766252..8a7669e 100644
--- a/MdeModulePkg/Include/Protocol/SdMmcOverride.h
+++ b/MdeModulePkg/Include/Protocol/SdMmcOverride.h
@@ -63,6 +63,7 @@ EFI_STATUS
   @param[in]  PhaseType The type of operation and whether the
 hook is invoked right before (pre) or
 right after (post)
+  @param[in,out]  PhaseData The pointer to a phase-specific data.
 
   @retval EFI_SUCCESS   The override function completed successfully.
   @retval EFI_NOT_FOUND The specified controller or slot does not 
exist.
@@ -74,7 +75,8 @@ EFI_STATUS
 (EFIAPI * EDKII_SD_MMC_NOTIFY_PHASE) (
   IN  EFI_HANDLE  ControllerHandle,
   IN  UINT8   Slot,
-  IN  EDKII_SD_MMC_PHASE_TYPE PhaseType
+  IN  EDKII_SD_MMC_PHASE_TYPE PhaseType,
+  IN OUT  VOID   *PhaseData
   );
 
 struct _EDKII_SD_MMC_OVERRIDE {
diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c 
b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
index bedc968..923c55b 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
@@ -444,7 +444,8 @@ SdMmcHcReset (
 Status = mOverride->NotifyPhase (
   Private->ControllerHandle,
   Slot,
-  EdkiiSdMmcResetPre);
+  EdkiiSdMmcResetPre,
+  NULL);
 if (EFI_ERROR (Status)) {
   DEBUG ((DEBUG_WARN,
 "%a: SD/MMC pre reset notifier callback failed - %r\n",
@@ -494,7 +495,8 @@ SdMmcHcReset (
 Status = mOverride->NotifyPhase (
   Private->ControllerHandle,
   Slot,
-  EdkiiSdMmcResetPost);
+  EdkiiSdMmcResetPost,
+  NULL);
 if (EFI_ERROR (Status)) {
   DEBUG ((DEBUG_WARN,
 "%a: SD/MMC post reset notifier callback failed - %r\n",
@@ -1088,7 +1090,8 @@ SdMmcHcInitHost (
 Status = mOverride->NotifyPhase (
   Private->ControllerHandle,
   Slot,
-  EdkiiSdMmcInitHostPre);
+  EdkiiSdMmcInitHostPre,
+  NULL);
 if (EFI_ERROR (Status)) {
   DEBUG ((DEBUG_WARN,
 "%a: SD/MMC pre init notifier callback failed - %r\n",
@@ -1123,7 +1126,8 @@ SdMmcHcInitHost (
 Status = mOverride->NotifyPhase (
   Private->ControllerHandle,
   Slot,
-  EdkiiSdMmcInitHostPost);
+  EdkiiSdMmcInitHostPost,
+  NULL);
 if (EFI_ERROR (Status)) {
   DEBUG ((DEBUG_WARN,
 "%a: SD/MMC post init notifier callback failed - %r\n",
-- 
2.7.4

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

[edk2] [PATCH v4 0/4] SdMmcOverride extension

[Marcin Wojtas]

Hi,

Although I could've waited for Hao's remarks, I think it may
be better if he takes a look at much cleaner code, which
addresses v3 review comments.
The newest version of the patchset cleans-up significantly
patches 2&3 by removing code duplication and other minor
improvements.

Patches are available in the github:
https://github.com/MarvellEmbeddedProcessors/edk2-open-platform/commits/sdmmc-override-upstream-r20181109

Please note that extending SdMmcOverride protocol was impacting
so far the only user of it (Synquacer controller). In paralel
edk2-platforms patchset, a patch can be found:
("Silicon/SynQuacer/PlatformDxe: adjust to updated SdMmcOverride")
which adjust to the new API.
https://github.com/MarvellEmbeddedProcessors/edk2-open-platform/commits/xenon-upstream-r20181109

I'm looking forward to the comments and remarks.

Best regards,
Marcin

Changelog:
v3->v4
* 2/4:
  - avoid duplication by calling SdMmcOverride callback in SdMmcHcUhsSignaling

* 3/4:
  - avoid duplication by calling SdMmcOverride callback in EmmcSwitchClockFreq

* 4/4:
  - add Ard's RB

v2->v3
* 1/4:
  - rename new parameter to PhaseData
  - add Ard's RB

* 2/4:
  - s/Controler/Controller/
  - remove all references to MMC_SDR_50 mode
  - rename and reorder MMC bus modes
  - rename enum: s/SD_MMC_UHS_TIMING/SD_MMC_BUS_MODE/
and move it to protocol header in order to drop including private one
  - fix if condition in EmmcSwitchToHighSpeed
  - call SdMmcHcUhsSignaling unconditionally before SdMmcOverride
callback, so that protocol producer can optionally modify only quirky
timing mode values.

*4/4
  - bump protocol version to 2
  - remove redundant assert from SdMmcPciHcDriverBindingStart
(BaseClkFreq is already checked in SdMmcHcInitClockFreq)
  - update comment in SdMmcHcInitClockFreq
  - restore original DumpCapabilityReg and append 

v1 -> v2
* Rebase onto newest master
* 1/4 [new patch] - preparation for extending NotifyPhase
* 2/4 - UhsSignaling as a part of NotifyPhase
* 3/4 - SwitchClockFreqPost as a part of NotifyPhase
* 4/4 - Allow updating BaseClkFreq via Capability instead of the
independent callback.


Marcin Wojtas (2):
  MdeModulePkg/SdMmcPciHcDxe: Add an optional parameter in NotifyPhase
  MdeModulePkg/SdMmcPciHcDxe: Allow overriding base clock frequency

Tomasz Michalec (2):
  MdeModulePkg/SdMmcPciHcDxe: Add UhsSignaling to SdMmcOverride protocol
  MdeModulePkg/SdMmcPciHcDxe: Add SwitchClockFreqPost to SdMmcOverride

 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h |   6 +
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h   |  42 ++-
 MdeModulePkg/Include/Protocol/SdMmcOverride.h  |  29 -
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c| 121 ++-
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c  |  35 --
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c |  13 +-
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c   | 124 +---
 7 files changed, 280 insertions(+), 90 deletions(-)

-- 
2.7.4

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

[edk2] [RFC] TPM non-MMIO Access

[Cohen, Eugene]

Dear SecurityPkg maintainers,

We are trying to use the SecurityPkg TPM support (Tcg2Config, 
Tpm2DeviceLibDTpm, etc) and see that access to the TPM are using the MMIO 
routines such as MmioRead8().

Not all platforms support memory-mapped access to the TPM so we would like to 
propose that we create a library to abstract the TPM access, called the 
TpmIoLib.  One instance of the library would provide the Mmio (TpmIoLibMmio) 
method but others can use protocols like SPI (TpmIoLibSpi).

Before starting this work I wanted to check if you agree with the approach of 
replacing the Mmio calls with new TpmIoLib ones.  We can upstream the library 
header and mmio library instance (or you could do the work if you think it 
would be easier for you).

I counted up the number of instances of unique MMIO calls in the TPM libraries 
and got

Library/TpmCommLib/TisPc.c:7
Library/Tpm2DeviceLibDTpm/Tpm2Tis.c:13
Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c:29
Library/Tpm12DeviceLibDTpm/Tpm12Tis.c:16

Please let me know if this works for you and how you'd like to structure the 
change.

Thanks,

Eugene




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

Re: [edk2] [PATCH] ArmPkg/ArmScmiDxe: Add clock enable function

[Sami Mujawar]

-Original Message-
From: Leif Lindholm  
Sent: 09 November 2018 06:49 PM
To: Jeff Brasen 
Cc: edk2-devel@lists.01.org; Ard Biesheuvel ; Girish 
Pathak ; Sami Mujawar 
Subject: Re: [PATCH] ArmPkg/ArmScmiDxe: Add clock enable function

On Fri, Nov 09, 2018 at 06:35:02PM +, Jeff Brasen wrote:
> >   1.  Add new ArmScmiClock2Protocol.h + guid
> >   2.  Add new guid and document that you have to use that guid for the 
> > enable function
> >   3.  Add new guid under old name and have the old guid installed on
> >   the handle as well, this would keep things fairly clean but
> >   would have a guid that wouldn't map to a protocol in header
> >   form.
> >   4.  Just change the protocol without changing the guid, this has
> >   the reverse issue of the change I made (except errors can
> >   result in a crash) (don't really like this option)
> 
> I think my (quite puritan) preference would be 5. Add another guid, 
> describe it in the same .h file.
> 
> For example, see (among several others) 
> EFI_FIRMWARE_VOLUME_BLOCK_PROTOCOL/EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL
> in MdePkg/Include/Protocol/FirmwareVolumeBlock.h.
> (This may be what you mean by 2?)
> 
> [JB] Yes this was what I was thinking with #2

OK, cool, then we're on the same page.

[SAMI] I think this would be the right approach.

> It's a bit of a sledgehammer, but it is a well known and common 
> pattern in edk2.
> 
> However, if we do this, I would prefer to take the opportunity to add 
> any new functions not already implemented at the same time. Do you 
> know if we have other missing calls?
> 
> Now, this _isn't_ a protocol described by any external specification, 
> so we don't need to be quite as rigid as for public interfaces.
> Basically, there shouldn't be (non-debug/non-devtool) dynamic 
> applications making use of this protocol in the first place. (If we 
> think there should be, we need to document this GUID and protocol in a 
> spec somewhere.) So in reality, I think 4 would also be a valid thing 
> to do.
> 
> But I do want feedback from the original author.
> 
> [JB] Sounds good, I think Girish is out of office until 11/21

Yeah. I'd take feedback from Sami as well :) But both me and Ard will be at 
plumbers next week, and we are expecting the stable tag to happen then as well 
- so that is basically lost anyway.

Regards,

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

Re: [edk2] [PATCH 0/4] OvmfPkg: revert some untimely pushed VMW SVGA reverts

[Ard Biesheuvel]

> On 9 Nov 2018, at 20:44, Laszlo Ersek  wrote:
> 
> Repo:   https://github.com/lersek/edk2.git
> Branch: revert_revert_bz_1319
> 
> Not much to say here, I've written it up in
> .
> 
> One way to verify this series quickly is:
> 
>> $ git diff 62ea70e31285..revert_revert_bz_1319 -- OvmfPkg/
>> [nothing]
> 
> Thanks, and sorry about the churn
> Laszlo
> 
> Cc: Anthony Perard 
> Cc: Ard Biesheuvel 
> Cc: Jordan Justen 
> Cc: Julien Grall 
> Cc: Philippe Mathieu-Daudé 
> Cc: yuchenlin 
> 
> Laszlo Ersek (4):
>  Reapply "OvmfPkg: VMWare SVGA display device register definitions"
>  Reapply "OvmfPkg/QemuVideoDxe: Helper functions for unaligned port
>I/O."
>  Reapply "OvmfPkg/QemuVideoDxe: VMWare SVGA device support"
>  Reapply "OvmfPkg/QemuVideoDxe: list "UnalignedIoInternal.h" in the INF
>file"
> 
> OvmfPkg/Include/IndustryStandard/VmwareSvga.h | 104 +
> OvmfPkg/QemuVideoDxe/Driver.c | 135 -
> OvmfPkg/QemuVideoDxe/Gop.c|  65 +++-
> OvmfPkg/QemuVideoDxe/Initialize.c | 157 
> OvmfPkg/QemuVideoDxe/Qemu.h   |  29 
> OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf |   7 +
> OvmfPkg/QemuVideoDxe/UnalignedIoGcc.c |  70 +
> OvmfPkg/QemuVideoDxe/UnalignedIoIcc.c |  80 ++
> OvmfPkg/QemuVideoDxe/UnalignedIoInternal.h|  59 
> OvmfPkg/QemuVideoDxe/UnalignedIoMsc.c |  78 ++
> OvmfPkg/QemuVideoDxe/UnalignedIoUnsupported.c |  66 
> 11 files changed, 843 insertions(+), 7 deletions(-)
> create mode 100644 OvmfPkg/Include/IndustryStandard/VmwareSvga.h
> create mode 100644 OvmfPkg/QemuVideoDxe/UnalignedIoGcc.c
> create mode 100644 OvmfPkg/QemuVideoDxe/UnalignedIoIcc.c
> create mode 100644 OvmfPkg/QemuVideoDxe/UnalignedIoInternal.h
> create mode 100644 OvmfPkg/QemuVideoDxe/UnalignedIoMsc.c
> create mode 100644 OvmfPkg/QemuVideoDxe/UnalignedIoUnsupported.c
> 

Acked-by: Ard Biesheuvel 


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

Re: [edk2] [patch] MdeModulePkg/DisplayEngine: Remove useless NULL ptr check for NewPos

[Laszlo Ersek]

On 11/09/18 18:49, Kinney, Michael D wrote:
> Laszlo,
> 
> I added EDK2|Documentation|Wiki Pages
> 
> I also updated the sort order so Wiki Pages is
> first, followed by maintained GitBook documents
> sorted alphabetically, with "Other Document" last.

Thanks, Mike! I've filed
, and assigned it
to Stephano.

Stephano, I hope that's alright with you -- with that assignment I
attempted to express that the topic should be brought to the community.

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

Re: [edk2] [PATCH 0/4] OvmfPkg: revert some untimely pushed VMW SVGA reverts

[Philippe Mathieu-Daudé]

On 9/11/18 20:44, Laszlo Ersek wrote:

Repo:   https://github.com/lersek/edk2.git
Branch: revert_revert_bz_1319

Not much to say here, I've written it up in
.

One way to verify this series quickly is:


$ git diff 62ea70e31285..revert_revert_bz_1319 -- OvmfPkg/
[nothing]


Thanks, and sorry about the churn
Laszlo

Cc: Anthony Perard 
Cc: Ard Biesheuvel 
Cc: Jordan Justen 
Cc: Julien Grall 
Cc: Philippe Mathieu-Daudé 
Cc: yuchenlin 

Laszlo Ersek (4):
   Reapply "OvmfPkg: VMWare SVGA display device register definitions"
   Reapply "OvmfPkg/QemuVideoDxe: Helper functions for unaligned port
 I/O."
   Reapply "OvmfPkg/QemuVideoDxe: VMWare SVGA device support"
   Reapply "OvmfPkg/QemuVideoDxe: list "UnalignedIoInternal.h" in the INF
 file"


Series:
Reviewed-by: Philippe Mathieu-Daudé 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH 0/4] OvmfPkg: revert some untimely pushed VMW SVGA reverts

[Laszlo Ersek]

Repo:   https://github.com/lersek/edk2.git
Branch: revert_revert_bz_1319

Not much to say here, I've written it up in
.

One way to verify this series quickly is:

> $ git diff 62ea70e31285..revert_revert_bz_1319 -- OvmfPkg/
> [nothing]

Thanks, and sorry about the churn
Laszlo

Cc: Anthony Perard 
Cc: Ard Biesheuvel 
Cc: Jordan Justen 
Cc: Julien Grall 
Cc: Philippe Mathieu-Daudé 
Cc: yuchenlin 

Laszlo Ersek (4):
  Reapply "OvmfPkg: VMWare SVGA display device register definitions"
  Reapply "OvmfPkg/QemuVideoDxe: Helper functions for unaligned port
I/O."
  Reapply "OvmfPkg/QemuVideoDxe: VMWare SVGA device support"
  Reapply "OvmfPkg/QemuVideoDxe: list "UnalignedIoInternal.h" in the INF
file"

 OvmfPkg/Include/IndustryStandard/VmwareSvga.h | 104 +
 OvmfPkg/QemuVideoDxe/Driver.c | 135 -
 OvmfPkg/QemuVideoDxe/Gop.c|  65 +++-
 OvmfPkg/QemuVideoDxe/Initialize.c | 157 
 OvmfPkg/QemuVideoDxe/Qemu.h   |  29 
 OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf |   7 +
 OvmfPkg/QemuVideoDxe/UnalignedIoGcc.c |  70 +
 OvmfPkg/QemuVideoDxe/UnalignedIoIcc.c |  80 ++
 OvmfPkg/QemuVideoDxe/UnalignedIoInternal.h|  59 
 OvmfPkg/QemuVideoDxe/UnalignedIoMsc.c |  78 ++
 OvmfPkg/QemuVideoDxe/UnalignedIoUnsupported.c |  66 
 11 files changed, 843 insertions(+), 7 deletions(-)
 create mode 100644 OvmfPkg/Include/IndustryStandard/VmwareSvga.h
 create mode 100644 OvmfPkg/QemuVideoDxe/UnalignedIoGcc.c
 create mode 100644 OvmfPkg/QemuVideoDxe/UnalignedIoIcc.c
 create mode 100644 OvmfPkg/QemuVideoDxe/UnalignedIoInternal.h
 create mode 100644 OvmfPkg/QemuVideoDxe/UnalignedIoMsc.c
 create mode 100644 OvmfPkg/QemuVideoDxe/UnalignedIoUnsupported.c

-- 
2.19.1.3.g30247aa5d201

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

[edk2] [PATCH 1/4] Reapply "OvmfPkg: VMWare SVGA display device register definitions"

[Laszlo Ersek]

This reverts commit 328409ce8de7f318ee9c929b64302bd361cd1dbd, reapplying
9bcca53fe466cdff397578328d9d87d257aba493.

Note that the commit now being reverted is technically correct; the only
reason we're reverting it is because it should not have been pushed past
the Soft Feature Freeze for the edk2-stable201811 tag.

Cc: Anthony Perard 
Cc: Ard Biesheuvel 
Cc: Jordan Justen 
Cc: Julien Grall 
Cc: Philippe Mathieu-Daudé 
Cc: yuchenlin 
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1319
Signed-off-by: Laszlo Ersek 
---
 OvmfPkg/Include/IndustryStandard/VmwareSvga.h | 104 
 1 file changed, 104 insertions(+)

diff --git a/OvmfPkg/Include/IndustryStandard/VmwareSvga.h 
b/OvmfPkg/Include/IndustryStandard/VmwareSvga.h
new file mode 100644
index ..693d44bab6c3
--- /dev/null
+++ b/OvmfPkg/Include/IndustryStandard/VmwareSvga.h
@@ -0,0 +1,104 @@
+/** @file
+
+  Macro and enum definitions of a subset of port numbers, register identifiers
+  and values required for driving the VMWare SVGA virtual display adapter,
+  also implemented by Qemu.
+
+  This file's contents was extracted from file lib/vmware/svga_reg.h in commit
+  329dd537456f93a806841ec8a8213aed11395def of VMWare's vmware-svga repository:
+  git://git.code.sf.net/p/vmware-svga/git
+
+
+  Copyright 1998-2009 VMware, Inc.  All rights reserved.
+  Portions Copyright 2017 Phil Dennis-Jordan 
+
+  Permission is hereby granted, free of charge, to any person
+  obtaining a copy of this software and associated documentation
+  files (the "Software"), to deal in the Software without
+  restriction, including without limitation the rights to use, copy,
+  modify, merge, publish, distribute, sublicense, and/or sell copies
+  of the Software, and to permit persons to whom the Software is
+  furnished to do so, subject to the following conditions:
+
+  The above copyright notice and this permission notice shall be
+  included in all copies or substantial portions of the Software.
+
+  THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+  EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+  MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+  NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
+  BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
+  ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
+  CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+  SOFTWARE.
+
+**/
+
+#ifndef _VMWARE_SVGA_H_
+#define _VMWARE_SVGA_H_
+
+#include 
+
+//
+// IDs for recognising the device
+//
+#define VMWARE_PCI_VENDOR_ID_VMWARE0x15AD
+#define VMWARE_PCI_DEVICE_ID_VMWARE_SVGA2  0x0405
+
+//
+// I/O port BAR offsets for register selection and read/write.
+//
+// The register index is written to the 32-bit index port, followed by a 32-bit
+// read or write on the value port to read or set that register's contents.
+//
+#define VMWARE_SVGA_INDEX_PORT 0x0
+#define VMWARE_SVGA_VALUE_PORT 0x1
+
+//
+// Some of the device's register indices for basic framebuffer functionality.
+//
+typedef enum {
+  VmwareSvgaRegId = 0,
+  VmwareSvgaRegEnable = 1,
+  VmwareSvgaRegWidth = 2,
+  VmwareSvgaRegHeight = 3,
+  VmwareSvgaRegMaxWidth = 4,
+  VmwareSvgaRegMaxHeight = 5,
+
+  VmwareSvgaRegBitsPerPixel = 7,
+
+  VmwareSvgaRegRedMask = 9,
+  VmwareSvgaRegGreenMask = 10,
+  VmwareSvgaRegBlueMask = 11,
+  VmwareSvgaRegBytesPerLine = 12,
+
+  VmwareSvgaRegFbOffset = 14,
+
+  VmwareSvgaRegFbSize = 16,
+  VmwareSvgaRegCapabilities = 17,
+
+  VmwareSvgaRegHostBitsPerPixel = 28,
+} VMWARE_SVGA_REGISTER;
+
+//
+// Values used with VmwareSvgaRegId for sanity-checking the device and getting
+// its version.
+//
+#define VMWARE_SVGA_MAGIC  0x90U
+#define VMWARE_SVGA_MAKE_ID(ver)   (VMWARE_SVGA_MAGIC << 8 | (ver))
+
+#define VMWARE_SVGA_VERSION_2  2
+#define VMWARE_SVGA_ID_2   VMWARE_SVGA_MAKE_ID (VMWARE_SVGA_VERSION_2)
+
+#define VMWARE_SVGA_VERSION_1  1
+#define VMWARE_SVGA_ID_1   VMWARE_SVGA_MAKE_ID (VMWARE_SVGA_VERSION_1)
+
+#define VMWARE_SVGA_VERSION_0  0
+#define VMWARE_SVGA_ID_0   VMWARE_SVGA_MAKE_ID (VMWARE_SVGA_VERSION_0)
+
+//
+// One of the capability bits advertised by VmwareSvgaRegCapabilities.
+//
+#define VMWARE_SVGA_CAP_8BIT_EMULATION BIT8
+
+#endif
-- 
2.19.1.3.g30247aa5d201


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

[edk2] [PATCH 3/4] Reapply "OvmfPkg/QemuVideoDxe: VMWare SVGA device support"

[Laszlo Ersek]

This reverts commit 98856a724c2acdc0094220d4de615a557dad0f88, reapplying
c137d95081690d4877fbeb5f1856972e84ac32f2.

Note that the commit now being reverted is technically correct; the only
reason we're reverting it is because it should not have been pushed past
the Soft Feature Freeze for the edk2-stable201811 tag.

Cc: Anthony Perard 
Cc: Ard Biesheuvel 
Cc: Jordan Justen 
Cc: Julien Grall 
Cc: Philippe Mathieu-Daudé 
Cc: yuchenlin 
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1319
Signed-off-by: Laszlo Ersek 
---
 OvmfPkg/QemuVideoDxe/Qemu.h   |  29 
 OvmfPkg/QemuVideoDxe/Driver.c | 135 -
 OvmfPkg/QemuVideoDxe/Gop.c|  65 +++-
 OvmfPkg/QemuVideoDxe/Initialize.c | 157 
 4 files changed, 379 insertions(+), 7 deletions(-)

diff --git a/OvmfPkg/QemuVideoDxe/Qemu.h b/OvmfPkg/QemuVideoDxe/Qemu.h
index d7da761705a1..bc49f867a4ff 100644
--- a/OvmfPkg/QemuVideoDxe/Qemu.h
+++ b/OvmfPkg/QemuVideoDxe/Qemu.h
@@ -92,6 +92,7 @@ typedef enum {
   QEMU_VIDEO_CIRRUS_5446,
   QEMU_VIDEO_BOCHS,
   QEMU_VIDEO_BOCHS_MMIO,
+  QEMU_VIDEO_VMWARE_SVGA,
 } QEMU_VIDEO_VARIANT;
 
 typedef struct {
@@ -116,10 +117,13 @@ typedef struct {
   //
   UINTN MaxMode;
   QEMU_VIDEO_MODE_DATA  *ModeData;
+  EFI_GRAPHICS_OUTPUT_MODE_INFORMATION  *VmwareSvgaModeInfo;
 
   QEMU_VIDEO_VARIANTVariant;
   FRAME_BUFFER_CONFIGURE*FrameBufferBltConfigure;
   UINTN FrameBufferBltConfigureSize;
+  UINT8 FrameBufferVramBarIndex;
+  UINT16VmwareSvgaBasePort;
 } QEMU_VIDEO_PRIVATE_DATA;
 
 ///
@@ -503,9 +507,34 @@ QemuVideoBochsModeSetup (
   BOOLEAN  IsQxl
   );
 
+EFI_STATUS
+QemuVideoVmwareSvgaModeSetup (
+  QEMU_VIDEO_PRIVATE_DATA *Private
+  );
+
 VOID
 InstallVbeShim (
   IN CONST CHAR16 *CardName,
   IN EFI_PHYSICAL_ADDRESS FrameBufferBase
   );
+
+VOID
+VmwareSvgaWrite (
+  QEMU_VIDEO_PRIVATE_DATA *Private,
+  UINT16  Register,
+  UINT32  Value
+  );
+
+UINT32
+VmwareSvgaRead (
+  QEMU_VIDEO_PRIVATE_DATA *Private,
+  UINT16  Register
+  );
+
+VOID
+InitializeVmwareSvgaGraphicsMode (
+  QEMU_VIDEO_PRIVATE_DATA  *Private,
+  QEMU_VIDEO_BOCHS_MODES   *ModeData
+  );
+
 #endif
diff --git a/OvmfPkg/QemuVideoDxe/Driver.c b/OvmfPkg/QemuVideoDxe/Driver.c
index 2304afd1e686..73eb2cad0584 100644
--- a/OvmfPkg/QemuVideoDxe/Driver.c
+++ b/OvmfPkg/QemuVideoDxe/Driver.c
@@ -14,8 +14,10 @@
 
 **/
 
-#include "Qemu.h"
+#include 
 #include 
+#include "Qemu.h"
+#include "UnalignedIoInternal.h"
 
 EFI_DRIVER_BINDING_PROTOCOL gQemuVideoDriverBinding = {
   QemuVideoControllerDriverSupported,
@@ -69,6 +71,12 @@ QEMU_VIDEO_CARD gQemuVideoCardList[] = {
 0x1050,
 QEMU_VIDEO_BOCHS_MMIO,
 L"QEMU VirtIO VGA"
+},{
+PCI_CLASS_DISPLAY_VGA,
+VMWARE_PCI_VENDOR_ID_VMWARE,
+VMWARE_PCI_DEVICE_ID_VMWARE_SVGA2,
+QEMU_VIDEO_VMWARE_SVGA,
+L"QEMU VMWare SVGA"
 },{
 0 /* end of list */
 }
@@ -256,6 +264,7 @@ QemuVideoControllerDriverStart (
 goto ClosePciIo;
   }
   Private->Variant = Card->Variant;
+  Private->FrameBufferVramBarIndex = PCI_BAR_IDX0;
 
   //
   // IsQxl is based on the detected Card->Variant, which at a later point might
@@ -330,6 +339,58 @@ QemuVideoControllerDriverStart (
 }
   }
 
+  //
+  // Check if accessing Vmware SVGA interface works
+  //
+  if (Private->Variant == QEMU_VIDEO_VMWARE_SVGA) {
+EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *IoDesc;
+UINT32TargetId;
+UINT32SvgaIdRead;
+
+IoDesc = NULL;
+Status = Private->PciIo->GetBarAttributes (
+   Private->PciIo,
+   PCI_BAR_IDX0,
+   NULL,
+   (VOID**) 
+   );
+if (EFI_ERROR (Status) ||
+IoDesc->ResType != ACPI_ADDRESS_SPACE_TYPE_IO ||
+IoDesc->AddrRangeMin > MAX_UINT16 + 1 - (VMWARE_SVGA_VALUE_PORT + 4)) {
+  if (IoDesc != NULL) {
+FreePool (IoDesc);
+  }
+  Status = EFI_DEVICE_ERROR;
+  goto RestoreAttributes;
+}
+Private->VmwareSvgaBasePort = (UINT16) IoDesc->AddrRangeMin;
+FreePool (IoDesc);
+
+TargetId = VMWARE_SVGA_ID_2;
+while (TRUE) {
+  VmwareSvgaWrite (Private, VmwareSvgaRegId, TargetId);
+  SvgaIdRead = VmwareSvgaRead (Private, VmwareSvgaRegId);
+  if ((SvgaIdRead == TargetId) || (TargetId <= VMWARE_SVGA_ID_0)) {
+break;
+  }
+  TargetId--;
+}
+
+if (SvgaIdRead != TargetId) {
+  DEBUG ((
+DEBUG_ERROR,
+"QemuVideo: QEMU_VIDEO_VMWARE_SVGA ID mismatch "
+"(got 0x%x, base address 0x%x)\n",
+SvgaIdRead,
+Private->VmwareSvgaBasePort
+));
+  Status = 

[edk2] [PATCH 4/4] Reapply "OvmfPkg/QemuVideoDxe: list "UnalignedIoInternal.h" in the INF file"

[Laszlo Ersek]

This reverts commit e038bde2679bbd200086c25ab43090ad3b8b25a3, reapplying
b2959e9f1a57279506ca46d56bc424fd7fa6b62a.

Note that the commit now being reverted is technically correct; the only
reason we're reverting it is because it should not have been pushed past
the Soft Feature Freeze for the edk2-stable201811 tag.

Cc: Anthony Perard 
Cc: Ard Biesheuvel 
Cc: Jordan Justen 
Cc: Julien Grall 
Cc: Philippe Mathieu-Daudé 
Cc: yuchenlin 
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1319
Signed-off-by: Laszlo Ersek 
---
 OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf | 1 +
 1 file changed, 1 insertion(+)

diff --git a/OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf 
b/OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf
index 895e6b8dbd4d..a04ed537dd27 100644
--- a/OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf
+++ b/OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf
@@ -39,6 +39,7 @@ [Sources.common]
   Gop.c
   Initialize.c
   Qemu.h
+  UnalignedIoInternal.h
 
 [Sources.Ia32, Sources.X64]
   UnalignedIoGcc.c| GCC
-- 
2.19.1.3.g30247aa5d201

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

[edk2] [PATCH 2/4] Reapply "OvmfPkg/QemuVideoDxe: Helper functions for unaligned port I/O."

[Laszlo Ersek]

This reverts commit 438ada5aa5a1174940795678c2dae07cde8f3869,
reapplying 05a5379458725234de8a05780fcb5da2c12680e4.

Note that the commit now being reverted is technically correct; the only
reason we're reverting it is because it should not have been pushed past
the Soft Feature Freeze for the edk2-stable201811 tag.

Cc: Anthony Perard 
Cc: Ard Biesheuvel 
Cc: Jordan Justen 
Cc: Julien Grall 
Cc: Philippe Mathieu-Daudé 
Cc: yuchenlin 
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1319
Signed-off-by: Laszlo Ersek 
---
 OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf |  6 ++
 OvmfPkg/QemuVideoDxe/UnalignedIoInternal.h| 59 +++
 OvmfPkg/QemuVideoDxe/UnalignedIoGcc.c | 70 +
 OvmfPkg/QemuVideoDxe/UnalignedIoIcc.c | 80 
 OvmfPkg/QemuVideoDxe/UnalignedIoMsc.c | 78 +++
 OvmfPkg/QemuVideoDxe/UnalignedIoUnsupported.c | 66 
 6 files changed, 359 insertions(+)

diff --git a/OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf 
b/OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf
index e5575622dc61..895e6b8dbd4d 100644
--- a/OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf
+++ b/OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf
@@ -41,9 +41,15 @@ [Sources.common]
   Qemu.h
 
 [Sources.Ia32, Sources.X64]
+  UnalignedIoGcc.c| GCC
+  UnalignedIoIcc.c| INTEL
+  UnalignedIoMsc.c| MSFT
   VbeShim.c
   VbeShim.h
 
+[Sources.EBC]
+  UnalignedIoUnsupported.c
+
 [Packages]
   MdePkg/MdePkg.dec
   MdeModulePkg/MdeModulePkg.dec
diff --git a/OvmfPkg/QemuVideoDxe/UnalignedIoInternal.h 
b/OvmfPkg/QemuVideoDxe/UnalignedIoInternal.h
new file mode 100644
index ..234de6c21bd1
--- /dev/null
+++ b/OvmfPkg/QemuVideoDxe/UnalignedIoInternal.h
@@ -0,0 +1,59 @@
+/** @file
+  Unaligned port I/O, with implementations for various x86 compilers and a
+  dummy for platforms which do not support unaligned port I/O.
+
+  Copyright (c) 2017, Phil Dennis-Jordan.
+  This program and the accompanying materials are licensed and made available
+  under the terms and conditions of the BSD License which accompanies this
+  distribution.  The full text of the license may be found at
+  http://opensource.org/licenses/bsd-license.php.
+
+  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#ifndef _UNALIGNED_IO_INTERNAL_H_
+#define _UNALIGNED_IO_INTERNAL_H_
+
+/**
+  Performs a 32-bit write to the specified, possibly unaligned I/O-type 
address.
+
+  Writes the 32-bit I/O port specified by Port with the value specified by 
Value
+  and returns Value. This function must guarantee that all I/O read and write
+  operations are serialized.
+
+  If 32-bit unaligned I/O port operations are not supported, then ASSERT().
+
+  @param[in]  Port   I/O port address
+  @param[in]  Value  32-bit word to write
+
+  @return The value written to the I/O port.
+
+**/
+UINT32
+UnalignedIoWrite32 (
+  IN  UINTN Port,
+  IN  UINT32Value
+  );
+
+/**
+  Reads a 32-bit word from the specified, possibly unaligned I/O-type address.
+
+  Reads the 32-bit I/O port specified by Port. The 32-bit read value is
+  returned. This function must guarantee that all I/O read and write operations
+  are serialized.
+
+  If 32-bit unaligned I/O port operations are not supported, then ASSERT().
+
+  @param[in]  Port  The I/O port to read.
+
+  @return The value read.
+
+**/
+UINT32
+UnalignedIoRead32 (
+  IN  UINTN Port
+  );
+
+#endif
diff --git a/OvmfPkg/QemuVideoDxe/UnalignedIoGcc.c 
b/OvmfPkg/QemuVideoDxe/UnalignedIoGcc.c
new file mode 100644
index ..105d55d3b903
--- /dev/null
+++ b/OvmfPkg/QemuVideoDxe/UnalignedIoGcc.c
@@ -0,0 +1,70 @@
+/** @file
+  Unaligned Port I/O. This file has compiler specifics for GCC as there is no
+  ANSI C standard for doing IO.
+
+  Based on IoLibGcc.c.
+
+  Copyright (c) 2006 - 2010, Intel Corporation. All rights reserved.
+  This program and the accompanying materials are licensed and made available
+  under the terms and conditions of the BSD License which accompanies this
+  distribution.  The full text of the license may be found at
+  http://opensource.org/licenses/bsd-license.php.
+
+  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+
+#include "UnalignedIoInternal.h"
+
+/**
+  Performs a 32-bit write to the specified, possibly unaligned I/O-type
+  address.
+
+  Writes the 32-bit I/O port specified by Port with the value specified by
+  Value and returns Value. This function must guarantee that all I/O read and
+  write operations are serialized.
+
+  If 32-bit unaligned I/O port operations are not supported, then ASSERT().
+
+  @param[in]  Port   I/O port address
+  @param[in]  Value  32-bit word to write
+
+  @return The value written to the I/O port.
+
+**/
+UINT32

Re: [edk2] [urgent] Soft Feature Freeze has started since Nov.1 for dk2-stable201811

[Laszlo Ersek]

On 11/09/18 20:08, Phil Dennis-Jordan wrote:
> Hi Laszlo,
> 
> On Fri, Nov 9, 2018 at 5:47 PM Laszlo Ersek  wrote:
>>
>> + Yuchenlin, + Gerd, + both Phils
>>
>> On 11/07/18 20:13, Laszlo Ersek wrote:
>>
> For example, I reviewed and pushed 4 patches yesterday (on 2018-Nov-06):
>
>   1  e038bde2679b Revert "OvmfPkg/QemuVideoDxe: list 
> "UnalignedIoInternal.h" in the INF file"
>   2  98856a724c2a Revert "OvmfPkg/QemuVideoDxe: VMWare SVGA device 
> support"
>   3  438ada5aa5a1 Revert "OvmfPkg/QemuVideoDxe: Helper functions for 
> unaligned port I/O."
>   4  328409ce8de7 Revert "OvmfPkg: VMWare SVGA display device register 
> definitions"
>
> which are the first four patches (out of five) from the following
> series:
>
>   [edk2] [PATCH v2 0/5] OvmfPkg: simply use the Bochs interface for vmsvga
>
> These reverts are arguably not bugfixes; they are preparation for
> re-implementing a feature from scratch (the last patch in that series).
> Thus, had I known we were already in the Soft Feature Freeze, I wouldn't
> have pushed them, because the review was not complete before the soft
> freeze start.
>
> But I had just returned from a week (or more) of PTO, there was no
> announcement on the list yet, and I didn't remember the wiki page.
>
> (In the technical sense, the reverts are not disruptive, luckily; they
> remove code that is dead anyway.)
>>
>> I've given this more thought.
>>
>> The reverts indeed remove dead code, but the code in question is dead
>> *only* on QEMU v2.10+. On QEMU v2.9 and earlier, the code is not dead.
>>
>> (See the original discussion in the thread "[edk2] [PATCH] OvmfPkg:
>> initialize bochs when initializing vmsvga".)
>>
>> This means that, with only the first four patches applied from the
>> series (= the reverts), and with the fifth patch (= the clean
>> re-implementation of the feature) postponed, people running
>> edk2-stable201811 on *old* -- v2.9 or older -- QEMU, with VMW SVGA, will
>> suffer a regression.
>>
>> So that leaves me with a question. Should I revert the first four
>> patches now, for edk2-stable201811? (I.e., revert the reverts -->
>> re-instate the incorrect VMW SVGA driver impl, that happens to work on
>> v2.9 and earlier.)
>>
>> ... Note that upstream QEMU no longer supports (= maintains stable
>> branches) for v2.9 and earlier releases. The QEMU homepage
>>  currently advertizes:
>> - 3.1.0-rc0
>> - 3.0.0
>> - 2.12.1
>> - 2.11.2
>>
>> Personally I'm leaning towards keeping the reverts for
>> edk2-stable201811. (v2.9 is really old, and the VMW SVGA device model is
>> virtually unused.) For my professional integrity though, I must ask this
>> question publicly.
> 
> I suspect the number of users relying on this feature is small, and I
> have my doubts that this group are running an unpatched snapshot of
> the master branch, let alone a stable OVMF release. But I don't have
> data for this, only anecdotal evidence. I personally won't be losing
> much sleep over it assuming we fix it for ~all versions of Qemu soon
> after.

Thanks Phil -- that sounds justified and (for me) convenient too. But, I
want to be precise and correct about this. I didn't push the patches out
of carelessness (I had returned from a stretch of PTO, and there hadn't
been a timely announcement on edk2-devel about the soft freeze -- and I
don't think I could be expected to remember the schedule wiki page from
a distance of multiple months).

But, *why* I messed up doesn't matter; only the fact that I did, does.
So, I've filed 
now, and I'll soon post the revert-revert series.

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

Re: [edk2] [urgent] Soft Feature Freeze has started since Nov.1 for dk2-stable201811

[Phil Dennis-Jordan]

Hi Laszlo,

On Fri, Nov 9, 2018 at 5:47 PM Laszlo Ersek  wrote:
>
> + Yuchenlin, + Gerd, + both Phils
>
> On 11/07/18 20:13, Laszlo Ersek wrote:
>
> >>> For example, I reviewed and pushed 4 patches yesterday (on 2018-Nov-06):
> >>>
> >>>   1  e038bde2679b Revert "OvmfPkg/QemuVideoDxe: list 
> >>> "UnalignedIoInternal.h" in the INF file"
> >>>   2  98856a724c2a Revert "OvmfPkg/QemuVideoDxe: VMWare SVGA device 
> >>> support"
> >>>   3  438ada5aa5a1 Revert "OvmfPkg/QemuVideoDxe: Helper functions for 
> >>> unaligned port I/O."
> >>>   4  328409ce8de7 Revert "OvmfPkg: VMWare SVGA display device register 
> >>> definitions"
> >>>
> >>> which are the first four patches (out of five) from the following
> >>> series:
> >>>
> >>>   [edk2] [PATCH v2 0/5] OvmfPkg: simply use the Bochs interface for vmsvga
> >>>
> >>> These reverts are arguably not bugfixes; they are preparation for
> >>> re-implementing a feature from scratch (the last patch in that series).
> >>> Thus, had I known we were already in the Soft Feature Freeze, I wouldn't
> >>> have pushed them, because the review was not complete before the soft
> >>> freeze start.
> >>>
> >>> But I had just returned from a week (or more) of PTO, there was no
> >>> announcement on the list yet, and I didn't remember the wiki page.
> >>>
> >>> (In the technical sense, the reverts are not disruptive, luckily; they
> >>> remove code that is dead anyway.)
>
> I've given this more thought.
>
> The reverts indeed remove dead code, but the code in question is dead
> *only* on QEMU v2.10+. On QEMU v2.9 and earlier, the code is not dead.
>
> (See the original discussion in the thread "[edk2] [PATCH] OvmfPkg:
> initialize bochs when initializing vmsvga".)
>
> This means that, with only the first four patches applied from the
> series (= the reverts), and with the fifth patch (= the clean
> re-implementation of the feature) postponed, people running
> edk2-stable201811 on *old* -- v2.9 or older -- QEMU, with VMW SVGA, will
> suffer a regression.
>
> So that leaves me with a question. Should I revert the first four
> patches now, for edk2-stable201811? (I.e., revert the reverts -->
> re-instate the incorrect VMW SVGA driver impl, that happens to work on
> v2.9 and earlier.)
>
> ... Note that upstream QEMU no longer supports (= maintains stable
> branches) for v2.9 and earlier releases. The QEMU homepage
>  currently advertizes:
> - 3.1.0-rc0
> - 3.0.0
> - 2.12.1
> - 2.11.2
>
> Personally I'm leaning towards keeping the reverts for
> edk2-stable201811. (v2.9 is really old, and the VMW SVGA device model is
> virtually unused.) For my professional integrity though, I must ask this
> question publicly.

I suspect the number of users relying on this feature is small, and I
have my doubts that this group are running an unpatched snapshot of
the master branch, let alone a stable OVMF release. But I don't have
data for this, only anecdotal evidence. I personally won't be losing
much sleep over it assuming we fix it for ~all versions of Qemu soon
after.

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

Re: [edk2] [PATCH] ArmPkg/ArmScmiDxe: Add clock enable function

[Leif Lindholm]

On Fri, Nov 09, 2018 at 06:35:02PM +, Jeff Brasen wrote:
> >   1.  Add new ArmScmiClock2Protocol.h + guid
> >   2.  Add new guid and document that you have to use that guid for the 
> > enable function
> >   3.  Add new guid under old name and have the old guid installed on
> >   the handle as well, this would keep things fairly clean but
> >   would have a guid that wouldn't map to a protocol in header
> >   form.
> >   4.  Just change the protocol without changing the guid, this has
> >   the reverse issue of the change I made (except errors can
> >   result in a crash) (don't really like this option)
> 
> I think my (quite puritan) preference would be
> 5. Add another guid, describe it in the same .h file.
> 
> For example, see (among several others)
> EFI_FIRMWARE_VOLUME_BLOCK_PROTOCOL/EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL
> in MdePkg/Include/Protocol/FirmwareVolumeBlock.h.
> (This may be what you mean by 2?)
> 
> [JB] Yes this was what I was thinking with #2

OK, cool, then we're on the same page.

> It's a bit of a sledgehammer, but it is a well known and common
> pattern in edk2.
> 
> However, if we do this, I would prefer to take the opportunity to add
> any new functions not already implemented at the same time. Do you
> know if we have other missing calls?
> 
> Now, this _isn't_ a protocol described by any external specification,
> so we don't need to be quite as rigid as for public interfaces.
> Basically, there shouldn't be (non-debug/non-devtool) dynamic
> applications making use of this protocol in the first place. (If we
> think there should be, we need to document this GUID and protocol in a
> spec somewhere.)
> So in reality, I think 4 would also be a valid thing to do.
> 
> But I do want feedback from the original author.
> 
> [JB] Sounds good, I think Girish is out of office until 11/21

Yeah. I'd take feedback from Sami as well :)
But both me and Ard will be at plumbers next week, and we are
expecting the stable tag to happen then as well - so that is basically
lost anyway.

Regards,

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

Re: [edk2] [PATCH] ArmPkg/ArmScmiDxe: Add clock enable function

[Leif Lindholm]

On Fri, Nov 09, 2018 at 05:03:53PM +, Jeff Brasen wrote:
> 
> From: Leif Lindholm 
> Sent: Friday, November 9, 2018 7:09 AM
> To: Jeff Brasen
> Cc: edk2-devel@lists.01.org; Ard Biesheuvel; Grish Pathak; Sami Mujawar
> Subject: Re: [PATCH] ArmPkg/ArmScmiDxe: Add clock enable function
> 
> Hi Jeff,
> 
> On Thu, Nov 08, 2018 at 10:50:44AM -0700, Jeff Brasen wrote:
> > Add function to allow enabling and disabling of the clock using the SCMI
> > interface. Update the protocol GUID as the protocol interface has
> > changed.
> 
> Changing a protocol GUID for an interface update feels a bit
> un-idiomatic for tianocore. (Generally, a new version of the protocol
> is added.)
> 
> My main concern is that I can't see how removing the ability to
> discover the protocol by the old GUID could ever have a desirable
> outcome.
> 
> Girish, Sami, what's your take?
> 
> [JB] I was trying to prevent new dynamic apps from crashing on old platforms. 
> I figured this was ok as this is a fairly new non-specification based 
> protocol. I do see the concern with this though. Of the following what is 
> your preference?
> 
> 
>   1.  Add new ArmScmiClock2Protocol.h + guid
>   2.  Add new guid and document that you have to use that guid for the enable 
> function
>   3.  Add new guid under old name and have the old guid installed on
>   the handle as well, this would keep things fairly clean but
>   would have a guid that wouldn't map to a protocol in header
>   form.
>   4.  Just change the protocol without changing the guid, this has
>   the reverse issue of the change I made (except errors can
>   result in a crash) (don't really like this option)

I think my (quite puritan) preference would be
5. Add another guid, describe it in the same .h file.

For example, see (among several others)
EFI_FIRMWARE_VOLUME_BLOCK_PROTOCOL/EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL
in MdePkg/Include/Protocol/FirmwareVolumeBlock.h.
(This may be what you mean by 2?)

It's a bit of a sledgehammer, but it is a well known and common
pattern in edk2.

However, if we do this, I would prefer to take the opportunity to add
any new functions not already implemented at the same time. Do you
know if we have other missing calls?

Now, this _isn't_ a protocol described by any external specification,
so we don't need to be quite as rigid as for public interfaces.
Basically, there shouldn't be (non-debug/non-devtool) dynamic
applications making use of this protocol in the first place. (If we
think there should be, we need to document this GUID and protocol in a
spec somewhere.)
So in reality, I think 4 would also be a valid thing to do.

But I do want feedback from the original author.

Regards,

Leif

> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Jeff Brasen 
> > Cc: Ard Biesheuvel 
> > Cc: Leif Lindholm 
> > Cc: Grish Pathak 
> > ---
> >  ArmPkg/ArmPkg.dec  |  2 +-
> >  .../ArmScmiDxe/ArmScmiClockProtocolPrivate.h   |  7 +++
> >  ArmPkg/Drivers/ArmScmiDxe/ScmiClockProtocol.c  | 50 
> > +-
> >  ArmPkg/Include/Protocol/ArmScmiClockProtocol.h | 21 -
> >  4 files changed, 77 insertions(+), 3 deletions(-)
> 
> Could you make sure you follow
> https://github.com/tianocore/tianocore.github.io/wiki/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers#contrib-23
> when generating patches (or let os know if you are, and we have more
> git breakage)?
> 
> /
> Leif
> 
> > diff --git a/ArmPkg/ArmPkg.dec b/ArmPkg/ArmPkg.dec
> > index 84e57a0..a7b55a1 100644
> > --- a/ArmPkg/ArmPkg.dec
> > +++ b/ArmPkg/ArmPkg.dec
> > @@ -57,7 +57,7 @@
> >
> >## Arm System Control and Management Interface(SCMI) Clock management 
> > protocol
> >## ArmPkg/Include/Protocol/ArmScmiClockProtocol.h
> > -  gArmScmiClockProtocolGuid = { 0x91ce67a8, 0xe0aa, 0x4012, { 0xb9, 0x9f, 
> > 0xb6, 0xfc, 0xf3, 0x4, 0x8e, 0xaa } }
> > +  gArmScmiClockProtocolGuid = { 0xb8d8caf2, 0x9e94, 0x462c, { 0xa8, 0x34, 
> > 0x6c, 0x99, 0xfc, 0x05, 0xef, 0xcf } }
> >
> >## Arm System Control and Management Interface(SCMI) Clock management 
> > protocol
> >## ArmPkg/Include/Protocol/ArmScmiPerformanceProtocol.h
> > diff --git a/ArmPkg/Drivers/ArmScmiDxe/ArmScmiClockProtocolPrivate.h 
> > b/ArmPkg/Drivers/ArmScmiDxe/ArmScmiClockProtocolPrivate.h
> > index 0d1ec6f..c135bac 100644
> > --- a/ArmPkg/Drivers/ArmScmiDxe/ArmScmiClockProtocolPrivate.h
> > +++ b/ArmPkg/Drivers/ArmScmiDxe/ArmScmiClockProtocolPrivate.h
> > @@ -59,6 +59,13 @@ typedef struct {
> >CLOCK_RATE_DWORD Rate;
> >  } CLOCK_RATE_SET_ATTRIBUTES;
> >
> > +
> > +// Message parameters for CLOCK_CONFIG_SET command.
> > +typedef struct {
> > +  UINT32 ClockId;
> > +  UINT32 Attributes;
> > +} CLOCK_CONFIG_SET_ATTRIBUTES;
> > +
> >  //  if ClockAttr Bit[0] is set then clock device is enabled.
> >  #define CLOCK_ENABLE_MASK 0x1
> >  #define CLOCK_ENABLED(ClockAttr)  

Re: [edk2] [urgent] Soft Feature Freeze has started since Nov.1 for dk2-stable201811

[Leif Lindholm]

Hi Laszlo,

On Fri, Nov 09, 2018 at 05:46:47PM +0100, Laszlo Ersek wrote:
> + Yuchenlin, + Gerd, + both Phils
> 
> On 11/07/18 20:13, Laszlo Ersek wrote:
> 
> >>> For example, I reviewed and pushed 4 patches yesterday (on 2018-Nov-06):
> >>>
> >>>   1  e038bde2679b Revert "OvmfPkg/QemuVideoDxe: list 
> >>> "UnalignedIoInternal.h" in the INF file"
> >>>   2  98856a724c2a Revert "OvmfPkg/QemuVideoDxe: VMWare SVGA device 
> >>> support"
> >>>   3  438ada5aa5a1 Revert "OvmfPkg/QemuVideoDxe: Helper functions for 
> >>> unaligned port I/O."
> >>>   4  328409ce8de7 Revert "OvmfPkg: VMWare SVGA display device register 
> >>> definitions"
> >>>
> >>> which are the first four patches (out of five) from the following
> >>> series:
> >>>
> >>>   [edk2] [PATCH v2 0/5] OvmfPkg: simply use the Bochs interface for vmsvga
> >>>
> >>> These reverts are arguably not bugfixes; they are preparation for
> >>> re-implementing a feature from scratch (the last patch in that series).
> >>> Thus, had I known we were already in the Soft Feature Freeze, I wouldn't
> >>> have pushed them, because the review was not complete before the soft
> >>> freeze start.
> >>>
> >>> But I had just returned from a week (or more) of PTO, there was no
> >>> announcement on the list yet, and I didn't remember the wiki page.
> >>>
> >>> (In the technical sense, the reverts are not disruptive, luckily; they
> >>> remove code that is dead anyway.)
> 
> I've given this more thought.
> 
> The reverts indeed remove dead code, but the code in question is dead
> *only* on QEMU v2.10+. On QEMU v2.9 and earlier, the code is not dead.
> 
> (See the original discussion in the thread "[edk2] [PATCH] OvmfPkg:
> initialize bochs when initializing vmsvga".)
> 
> This means that, with only the first four patches applied from the
> series (= the reverts), and with the fifth patch (= the clean
> re-implementation of the feature) postponed, people running
> edk2-stable201811 on *old* -- v2.9 or older -- QEMU, with VMW SVGA, will
> suffer a regression.
> 
> So that leaves me with a question. Should I revert the first four
> patches now, for edk2-stable201811? (I.e., revert the reverts -->
> re-instate the incorrect VMW SVGA driver impl, that happens to work on
> v2.9 and earlier.)
> 
> ... Note that upstream QEMU no longer supports (= maintains stable
> branches) for v2.9 and earlier releases. The QEMU homepage
>  currently advertizes:
> - 3.1.0-rc0
> - 3.0.0
> - 2.12.1
> - 2.11.2
> 
> Personally I'm leaning towards keeping the reverts for
> edk2-stable201811. (v2.9 is really old, and the VMW SVGA device model is
> virtually unused.) For my professional integrity though, I must ask this
> question publicly.

Well, I don't really have a horse in this race, but since you've
directed the email at me (at least that's what mailman makes it look
like :) I'll state my opinion:

My leaning would be towards reverting the reverts.
My workstation runs Debian Stretch, and the QEMU included there is
2.8.1. So a current "stable" distribution would be affected ...

... for people who run bleeding edge EDK2 on stable-distro-provided
QEMU. Which is why it's only leaning.

There is no such thing as too many reverts.

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

Re: [edk2] [PATCH edk2-staging 00/19] IntelUndiPkg/GigUndiDxe: build fixes for AARCH64/ARM/GCC

[Ard Biesheuvel]

On 9 November 2018 at 18:54, Knop, Ryszard  wrote:
> Hey Ard, I'm from the Preboot team responsible for this component. We're too 
> busy to review and merge this at the moment, so this'll have to stay on the 
> list for now, but we'll come back to it in a few weeks once we've finished 
> some of our current tasks.
> Thanks, Richard.
>

Thank you. No rush.

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ard 
> Biesheuvel
> Sent: Wednesday, November 7, 2018 0:04
> To: Kinney, Michael D 
> Cc: Jin, Eric ; edk2-devel@lists.01.org; Kacperski, Kamil 
> ; Orlowski, Pawel 
> Subject: Re: [edk2] [PATCH edk2-staging 00/19] IntelUndiPkg/GigUndiDxe: build 
> fixes for AARCH64/ARM/GCC
>
> On 6 November 2018 at 23:10, Kinney, Michael D  
> wrote:
>> Hi Ard,
>>
>> Can you please add CC lines to the commit message for the developers
>> that have contributed to the edk2-staging/Intel_UNDI branch?
>>
>> This would include:
>>
>> Cc: Maciej Rabeda 
>
> Maciej was already on cc. I hope he can forward the emails to his colleagues, 
> if they can't find them in the archives.
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
> 
>
> Intel Technology Poland sp. z o.o.
> ul. Slowackiego 173 | 80-298 Gdansk | Sad Rejonowy Gdansk Polnoc | VII 
> Wydzial Gospodarczy Krajowego Rejestru Sadowego - KRS 101882 | NIP 
> 957-07-52-316 | Kapital zakladowy 200.000 PLN.
>
> Ta wiadomosc wraz z zalacznikami jest przeznaczona dla okreslonego adresata i 
> moze zawierac informacje poufne. W razie przypadkowego otrzymania tej 
> wiadomosci, prosimy o powiadomienie nadawcy oraz trwale jej usuniecie; 
> jakiekolwiek
> przegladanie lub rozpowszechnianie jest zabronione.
> This e-mail and any attachments may contain confidential material for the 
> sole use of the intended recipient(s). If you are not the intended recipient, 
> please contact the sender and delete all copies; any review or distribution by
> others is strictly prohibited.
>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH edk2-staging 00/19] IntelUndiPkg/GigUndiDxe: build fixes for AARCH64/ARM/GCC

[Knop, Ryszard]

Hey Ard, I'm from the Preboot team responsible for this component. We're too 
busy to review and merge this at the moment, so this'll have to stay on the 
list for now, but we'll come back to it in a few weeks once we've finished some 
of our current tasks.
Thanks, Richard.

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ard 
Biesheuvel
Sent: Wednesday, November 7, 2018 0:04
To: Kinney, Michael D 
Cc: Jin, Eric ; edk2-devel@lists.01.org; Kacperski, Kamil 
; Orlowski, Pawel 
Subject: Re: [edk2] [PATCH edk2-staging 00/19] IntelUndiPkg/GigUndiDxe: build 
fixes for AARCH64/ARM/GCC

On 6 November 2018 at 23:10, Kinney, Michael D  
wrote:
> Hi Ard,
>
> Can you please add CC lines to the commit message for the developers 
> that have contributed to the edk2-staging/Intel_UNDI branch?
>
> This would include:
>
> Cc: Maciej Rabeda 

Maciej was already on cc. I hope he can forward the emails to his colleagues, 
if they can't find them in the archives.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Intel Technology Poland sp. z o.o.
ul. Slowackiego 173 | 80-298 Gdansk | Sad Rejonowy Gdansk Polnoc | VII Wydzial 
Gospodarczy Krajowego Rejestru Sadowego - KRS 101882 | NIP 957-07-52-316 | 
Kapital zakladowy 200.000 PLN.

Ta wiadomosc wraz z zalacznikami jest przeznaczona dla okreslonego adresata i 
moze zawierac informacje poufne. W razie przypadkowego otrzymania tej 
wiadomosci, prosimy o powiadomienie nadawcy oraz trwale jej usuniecie; 
jakiekolwiek
przegladanie lub rozpowszechnianie jest zabronione.
This e-mail and any attachments may contain confidential material for the sole 
use of the intended recipient(s). If you are not the intended recipient, please 
contact the sender and delete all copies; any review or distribution by
others is strictly prohibited.

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

Re: [edk2] [patch] MdeModulePkg/DisplayEngine: Remove useless NULL ptr check for NewPos

[Kinney, Michael D]

Laszlo,

I added EDK2|Documentation|Wiki Pages

I also updated the sort order so Wiki Pages is
first, followed by maintained GitBook documents
sorted alphabetically, with "Other Document" last.

Thanks,

Mike

> -Original Message-
> From: Laszlo Ersek [mailto:ler...@redhat.com]
> Sent: Friday, November 9, 2018 9:24 AM
> To: Kinney, Michael D ;
> Leif Lindholm ; Gao, Liming
> ; Cetola, Stephano
> 
> Cc: edk2-devel@lists.01.org
> Subject: Re: [edk2] [patch] MdeModulePkg/DisplayEngine:
> Remove useless NULL ptr check for NewPos
> 
> On 11/09/18 18:14, Kinney, Michael D wrote:
> > Hi Laszlo,
> >
> > There is already a "Documentation" component under
> > EDK2 in BZ.  Can we use that and identify the target
> > of the documentation change is in a Wiki page?
> 
> Absolutely.
> 
> > When you select "Documentation" there is a drop down
> > for "Tianocore documents" that lists the documents
> > published through GitBook along with a catch all
> called
> > "Other Document".  Is this the drop down you would
> like
> > to see "Wiki" added?
> 
> I was undecided. I recalled that earlier there used to
> be a "TianoCore
> Website" component somewhere, but now I couldn't find
> it. Also, I
> considered both options we're discussing now (i.e.,
> EDK2|Wiki, vs.
> EDK2|Documentation|Wiki). I couldn't really decide.
> "EDK2|Documentation|Xxx" seemed to suggest serious
> specifications that
> we publish "in print".
> 
> I think I'd be fine with either option (EDK2|Wiki vs.
> EDK2|Documentation|Wiki); whichever is easier to
> implement on the
> Bugzilla side. :)
> 
> Thank you!
> Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [patch] MdeModulePkg/DisplayEngine: Remove useless NULL ptr check for NewPos

[Laszlo Ersek]

On 11/09/18 18:14, Kinney, Michael D wrote:
> Hi Laszlo,
> 
> There is already a "Documentation" component under
> EDK2 in BZ.  Can we use that and identify the target
> of the documentation change is in a Wiki page?

Absolutely.

> When you select "Documentation" there is a drop down
> for "Tianocore documents" that lists the documents
> published through GitBook along with a catch all called
> "Other Document".  Is this the drop down you would like
> to see "Wiki" added?

I was undecided. I recalled that earlier there used to be a "TianoCore
Website" component somewhere, but now I couldn't find it. Also, I
considered both options we're discussing now (i.e., EDK2|Wiki, vs.
EDK2|Documentation|Wiki). I couldn't really decide.
"EDK2|Documentation|Xxx" seemed to suggest serious specifications that
we publish "in print".

I think I'd be fine with either option (EDK2|Wiki vs.
EDK2|Documentation|Wiki); whichever is easier to implement on the
Bugzilla side. :)

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

Re: [edk2] [edk2-wiki PATCH] release planning: announce the soft and hard feature freezes

[Laszlo Ersek]

On 11/09/18 18:07, Kinney, Michael D wrote:
> Perhaps we should update Maintainers.txt with some lines
> that identify the developers that are currently assigned
> to the release/release planning role.  That way, the Wiki
> can reference Maintainters.txt and we can update
> Maintainers.txt as the assignments change over time.

This is a great idea!

Recently, Liming posted a patch:

Maintainers.txt: Update EDK II Releases to EDK-II-Release-Planning wiki

it updates the "EDK II Releases" section in Maintainers.txt. The patch
hasn't been pushed yet.

Liming, can you submit a v2 of that patch, such that it adds you with an
"M" role for the  "EDK II Releases" section? Then, I could update the
wiki patch using a reference to Maintainers.txt.

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

[edk2] [PATCH] MdeModulePkg/SdDxe: Fix potential NULL pointer access

[Jeff Brasen]

SdReadWrite can be called with a NULL Token for synchronous operations.
Add guard for DEBUG print to only print event pointer with Token is not
NULL.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jeff Brasen 
---
 MdeModulePkg/Bus/Sd/SdDxe/SdBlockIo.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/MdeModulePkg/Bus/Sd/SdDxe/SdBlockIo.c 
b/MdeModulePkg/Bus/Sd/SdDxe/SdBlockIo.c
index b8d115a..920e3cd 100644
--- a/MdeModulePkg/Bus/Sd/SdDxe/SdBlockIo.c
+++ b/MdeModulePkg/Bus/Sd/SdDxe/SdBlockIo.c
@@ -670,8 +670,11 @@ SdReadWrite (
 if (EFI_ERROR (Status)) {
   return Status;
 }
-DEBUG ((DEBUG_BLKIO, "Sd%a(): Lba 0x%x BlkNo 0x%x Event %p with %r\n", 
IsRead ? "Read" : "Write", Lba, BlockNum, Token->Event, Status));
-
+if (Token != NULL) {
+  DEBUG ((DEBUG_BLKIO, "Sd%a(): Lba 0x%x BlkNo 0x%x Event %p with %r\n", 
IsRead ? "Read" : "Write", Lba, BlockNum, Token->Event, Status));
+} else {
+  DEBUG ((DEBUG_BLKIO, "Sd%a(): Lba 0x%x BlkNo 0x%x with %r\n", IsRead ? 
"Read" : "Write", Lba, BlockNum, Status));
+}
 Lba   += BlockNum;
 Buffer = (UINT8*)Buffer + BufferSize;
 Remaining -= BlockNum;
-- 
2.7.4

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

Re: [edk2] [patch] MdeModulePkg/DisplayEngine: Remove useless NULL ptr check for NewPos

[Kinney, Michael D]

Hi Laszlo,

There is already a "Documentation" component under
EDK2 in BZ.  Can we use that and identify the target
of the documentation change is in a Wiki page?

When you select "Documentation" there is a drop down
for "Tianocore documents" that lists the documents
published through GitBook along with a catch all called
"Other Document".  Is this the drop down you would like
to see "Wiki" added?

Thanks,

Mike

> -Original Message-
> From: Laszlo Ersek [mailto:ler...@redhat.com]
> Sent: Friday, November 9, 2018 8:33 AM
> To: Leif Lindholm ; Gao,
> Liming ; Kinney, Michael D
> ; Cetola, Stephano
> 
> Cc: edk2-devel@lists.01.org
> Subject: Re: [edk2] [patch] MdeModulePkg/DisplayEngine:
> Remove useless NULL ptr check for NewPos
> 
> On 11/09/18 17:01, Leif Lindholm wrote:
> > On Fri, Nov 09, 2018 at 03:32:03PM +, Gao, Liming
> wrote:
> >> Laszlo and Leif:
> >>   I suggest to continue to update wiki page with
> more
> >>   information. If so, we can avoid such case again.
> >
> > Agreed, we need to be able to interpret what the
> process says
> > identically.
> 
> Making the wiki real precise on this question requires
> dedicated work. I
> was OK to simply send a patch about the announcements,
> but this topic
> requires more thinking, more discussion, and well, more
> tracking.
> 
> Mike: would it be proper to create a "Wiki" Component
> under the EDK2
> product in the bugzilla?
> 
> If so, I would suggest filing a BZ against that (new)
> component, and
> then we should discuss the freeze scopes in one of the
> next community
> meetings.
> 
> Thanks
> Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [edk2-wiki PATCH] release planning: announce the soft and hard feature freezes

[Kinney, Michael D]

Perhaps we should update Maintainers.txt with some lines
that identify the developers that are currently assigned
to the release/release planning role.  That way, the Wiki
can reference Maintainters.txt and we can update
Maintainers.txt as the assignments change over time.

Thanks,

Mike

> -Original Message-
> From: Laszlo Ersek [mailto:ler...@redhat.com]
> Sent: Friday, November 9, 2018 8:27 AM
> To: Gao, Liming ; Leif Lindholm
> 
> Cc: edk2-devel@lists.01.org; Andrew Fish
> ; Richardson, Brian
> ; Kinney, Michael D
> ; Cetola, Stephano
> 
> Subject: Re: [edk2-wiki PATCH] release planning:
> announce the soft and hard feature freezes
> 
> On 11/09/18 16:09, Gao, Liming wrote:
> > Laszlo:
> >  Can ';' be removed from below sentence?
> >
> > an announcement email is sent to the edk2-devel
> mailing list; by default by Liming Gao.
> >
> > ==>
> >
> > an announcement email is sent to the edk2-devel
> mailing list by default by Liming Gao.
> 
> Hmm I don't really like that, to me it makes the
> sentence a lot harder
> to read. How about this simplified version instead
> (i.e., drop "by
> default", and the semicolon):
> 
> "an announcement email is sent to the edk2-devel
> mailing list by Liming Gao"
> 
> ?
> 
> Thanks!
> Laszlo
> 
> 
> 
> >
> > Thanks
> > Liming
> >> -Original Message-
> >> From: Leif Lindholm
> [mailto:leif.lindh...@linaro.org]
> >> Sent: Friday, November 9, 2018 6:05 PM
> >> To: Laszlo Ersek 
> >> Cc: edk2-devel@lists.01.org; Andrew Fish
> ; Richardson, Brian
> ; Gao, Liming
> >> ; Kinney, Michael D
> ; Cetola, Stephano
> 
> >> Subject: Re: [edk2-wiki PATCH] release planning:
> announce the soft and hard feature freezes
> >>
> >> On Thu, Nov 08, 2018 at 06:28:07PM +0100, Laszlo
> Ersek wrote:
> >>> Add a paragraph to each of the SoftFeatureFreeze
> and HardFeatureFreeze
> >>> articles about the respective announcements on
> edk2-devel.
> >>>
> >>> Cc: Andrew Fish 
> >>> Cc: Brian Richardson 
> >>> Cc: Leif Lindholm 
> >>> Cc: Liming Gao 
> >>> Cc: Michael Kinney 
> >>> Cc: Stephano Cetola 
> >>> Contributed-under: TianoCore Contribution Agreement
> 1.1
> >>> Signed-off-by: Laszlo Ersek 
> >>
> >> Reviewed-by: Leif Lindholm
> 
> >>
> >>> ---
> >>>
> >>> Notes:
> >>> Check out the rendered pages in my clone of the
> wiki:
> >>>
> >>>
> https://github.com/lersek/edk2/wiki/SoftFeatureFreeze
> >>>
> https://github.com/lersek/edk2/wiki/HardFeatureFreeze
> >>>
> >>>  HardFeatureFreeze.md | 8 
> >>>  SoftFeatureFreeze.md | 8 
> >>>  2 files changed, 16 insertions(+)
> >>>
> >>> diff --git a/HardFeatureFreeze.md
> b/HardFeatureFreeze.md
> >>> index 01288dd03f71..e08f4c047e8d 100644
> >>> --- a/HardFeatureFreeze.md
> >>> +++ b/HardFeatureFreeze.md
> >>> @@ -4,3 +4,11 @@ tag](EDK-II#stable-tags).
> >>>
> >>>  (This definition is modeled after QEMU's [hard
> feature
> >>>
> freeze](https://wiki.qemu.org/Planning/HardFeatureFreez
> e)).
> >>> +
> >>> +# Announcing the hard feature freeze
> >>> +
> >>> +The proposed schedule for the active development
> cycle is shown in the [EDK II
> >>> +Release Planning](EDK-II-Release-Planning)
> article. Shortly before the hard
> >>> +feature freeze, an announcement email is sent to
> the
> >>> +[edk2-
> devel](https://lists.01.org/mailman/listinfo/edk2-
> devel) mailing list; by
> >>> +default by [Liming
> Gao](https://github.com/lgao4/).
> >>> diff --git a/SoftFeatureFreeze.md
> b/SoftFeatureFreeze.md
> >>> index 9dc7d9c19369..e33dd80ccbee 100644
> >>> --- a/SoftFeatureFreeze.md
> >>> +++ b/SoftFeatureFreeze.md
> >>> @@ -40,3 +40,11 @@ communicate with the maintainer
> about their intentions. It also helps if you:
> >>>
> >>>  (This definition is modeled after QEMU's [soft
> feature
> >>>
> freeze](https://wiki.qemu.org/Planning/SoftFeatureFreez
> e).)
> >>> +
> >>> +# Announcing the soft feature freeze
> >>> +
> >>> +The proposed schedule for the active development
> cycle is shown in the [EDK II
> >>> +Release Planning](EDK-II-Release-Planning)
> article. Shortly before the soft
> >>> +feature freeze, an announcement email is sent to
> the
> >>> +[edk2-
> devel](https://lists.01.org/mailman/listinfo/edk2-
> devel) mailing list; by
> >>> +default by [Liming
> Gao](https://github.com/lgao4/).
> >>> --
> >>> 2.19.1.3.g30247aa5d201
> >>>

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

Re: [edk2] [PATCH] ArmPkg/ArmScmiDxe: Add clock enable function

[Jeff Brasen]

From: Leif Lindholm 
Sent: Friday, November 9, 2018 7:09 AM
To: Jeff Brasen
Cc: edk2-devel@lists.01.org; Ard Biesheuvel; Grish Pathak; Sami Mujawar
Subject: Re: [PATCH] ArmPkg/ArmScmiDxe: Add clock enable function

Hi Jeff,

On Thu, Nov 08, 2018 at 10:50:44AM -0700, Jeff Brasen wrote:
> Add function to allow enabling and disabling of the clock using the SCMI
> interface. Update the protocol GUID as the protocol interface has
> changed.

Changing a protocol GUID for an interface update feels a bit
un-idiomatic for tianocore. (Generally, a new version of the protocol
is added.)

My main concern is that I can't see how removing the ability to
discover the protocol by the old GUID could ever have a desirable
outcome.

Girish, Sami, what's your take?

[JB] I was trying to prevent new dynamic apps from crashing on old platforms. I 
figured this was ok as this is a fairly new non-specification based protocol. I 
do see the concern with this though. Of the following what is your preference?


  1.  Add new ArmScmiClock2Protocol.h + guid
  2.  Add new guid and document that you have to use that guid for the enable 
function
  3.  Add new guid under old name and have the old guid installed on the handle 
as well, this would keep things fairly clean but would have a guid that 
wouldn't map to a protocol in header form.
  4.  Just change the protocol without changing the guid, this has the reverse 
issue of the change I made (except errors can result in a crash) (don't really 
like this option)

> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Jeff Brasen 
> Cc: Ard Biesheuvel 
> Cc: Leif Lindholm 
> Cc: Grish Pathak 
> ---
>  ArmPkg/ArmPkg.dec  |  2 +-
>  .../ArmScmiDxe/ArmScmiClockProtocolPrivate.h   |  7 +++
>  ArmPkg/Drivers/ArmScmiDxe/ScmiClockProtocol.c  | 50 
> +-
>  ArmPkg/Include/Protocol/ArmScmiClockProtocol.h | 21 -
>  4 files changed, 77 insertions(+), 3 deletions(-)

Could you make sure you follow
https://github.com/tianocore/tianocore.github.io/wiki/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers#contrib-23
when generating patches (or let os know if you are, and we have more
git breakage)?

/
Leif

> diff --git a/ArmPkg/ArmPkg.dec b/ArmPkg/ArmPkg.dec
> index 84e57a0..a7b55a1 100644
> --- a/ArmPkg/ArmPkg.dec
> +++ b/ArmPkg/ArmPkg.dec
> @@ -57,7 +57,7 @@
>
>## Arm System Control and Management Interface(SCMI) Clock management 
> protocol
>## ArmPkg/Include/Protocol/ArmScmiClockProtocol.h
> -  gArmScmiClockProtocolGuid = { 0x91ce67a8, 0xe0aa, 0x4012, { 0xb9, 0x9f, 
> 0xb6, 0xfc, 0xf3, 0x4, 0x8e, 0xaa } }
> +  gArmScmiClockProtocolGuid = { 0xb8d8caf2, 0x9e94, 0x462c, { 0xa8, 0x34, 
> 0x6c, 0x99, 0xfc, 0x05, 0xef, 0xcf } }
>
>## Arm System Control and Management Interface(SCMI) Clock management 
> protocol
>## ArmPkg/Include/Protocol/ArmScmiPerformanceProtocol.h
> diff --git a/ArmPkg/Drivers/ArmScmiDxe/ArmScmiClockProtocolPrivate.h 
> b/ArmPkg/Drivers/ArmScmiDxe/ArmScmiClockProtocolPrivate.h
> index 0d1ec6f..c135bac 100644
> --- a/ArmPkg/Drivers/ArmScmiDxe/ArmScmiClockProtocolPrivate.h
> +++ b/ArmPkg/Drivers/ArmScmiDxe/ArmScmiClockProtocolPrivate.h
> @@ -59,6 +59,13 @@ typedef struct {
>CLOCK_RATE_DWORD Rate;
>  } CLOCK_RATE_SET_ATTRIBUTES;
>
> +
> +// Message parameters for CLOCK_CONFIG_SET command.
> +typedef struct {
> +  UINT32 ClockId;
> +  UINT32 Attributes;
> +} CLOCK_CONFIG_SET_ATTRIBUTES;
> +
>  //  if ClockAttr Bit[0] is set then clock device is enabled.
>  #define CLOCK_ENABLE_MASK 0x1
>  #define CLOCK_ENABLED(ClockAttr)  ((ClockAttr & CLOCK_ENABLE_MASK) == 1)
> diff --git a/ArmPkg/Drivers/ArmScmiDxe/ScmiClockProtocol.c 
> b/ArmPkg/Drivers/ArmScmiDxe/ScmiClockProtocol.c
> index 64d2afa..493eb45 100644
> --- a/ArmPkg/Drivers/ArmScmiDxe/ScmiClockProtocol.c
> +++ b/ArmPkg/Drivers/ArmScmiDxe/ScmiClockProtocol.c
> @@ -388,6 +388,53 @@ ClockRateSet (
>return Status;
>  }
>
> +/** Enable/Disable specified clock.
> +
> +  @param[in]  ThisA Pointer to SCMI_CLOCK_PROTOCOL Instance.
> +  @param[in]  ClockId Identifier for the clock device.
> +  @param[in]  Enable  TRUE to enable, FALSE to disable.
> +
> +  @retval EFI_SUCCESS  Clock enable/disable successful.
> +  @retval EFI_DEVICE_ERROR SCP returns an SCMI error.
> +  @retval !(EFI_SUCCESS)   Other errors.
> +**/
> +STATIC
> +EFI_STATUS
> +ClockEnable (
> +  IN SCMI_CLOCK_PROTOCOL  *This,
> +  IN UINT32   ClockId,
> +  IN BOOLEAN  Enable
> +  )
> +{
> +  EFI_STATUS  Status;
> +  CLOCK_CONFIG_SET_ATTRIBUTES *ClockConfigSetAttributes;
> +  SCMI_COMMANDCmd;
> +  UINT32  PayloadLength;
> +
> +  Status = ScmiCommandGetPayload ((UINT32**));
> +  if (EFI_ERROR (Status)) {
> +return Status;
> +  }
> +
> +  // Fill arguments for clock protocol command.
> +  

Re: [edk2] [Patch] BaseTools: Optimize string concatenation

[Kinney, Michael D]

Liming,

If we can support both Python2 and Python3 equally,
then I agree that these types of performance enhancements
can be added to edk2/master after the stable tag.

Let's make sure we enter a BZ for each performance
improvement and as Leif has asked, put evidence of the
performance improvement in the BZ.

Mike

> -Original Message-
> From: Gao, Liming
> Sent: Friday, November 9, 2018 6:17 AM
> To: Kinney, Michael D ;
> Feng, Bob C ; edk2-
> de...@lists.01.org
> Cc: Carsey, Jaben 
> Subject: RE: [edk2] [Patch] BaseTools: Optimize string
> concatenation
> 
> Mike:
>   This patch bases on edk2 master with Python27.
> Seemly, most people are not aware that Python3
> migration
> (https://bugzilla.tianocore.org/show_bug.cgi?id=55) is
> added for next edk2-stable201903 tag planning. In BZ
> 55, I propose to keep Python2 and Python3 both, and new
> feature and patches are created based on Python3. I
> will send the mail to collect the feedback on this
> approach.
> 
> Thanks
> Liming
> > -Original Message-
> > From: Kinney, Michael D
> > Sent: Friday, November 9, 2018 12:41 AM
> > To: Feng, Bob C ; edk2-
> de...@lists.01.org; Kinney, Michael D
> 
> > Cc: Carsey, Jaben ; Gao,
> Liming 
> > Subject: RE: [edk2] [Patch] BaseTools: Optimize
> string concatenation
> >
> > Bob,
> >
> > Is this for edk2/master or for the Python 3
> conversion in the
> > edk2-staging branch?  If it is for the edk-staging
> branch, then
> > the Subject is not correct.
> >
> > Thanks,
> >
> > Mike
> >
> > > -Original Message-
> > > From: edk2-devel [mailto:edk2-devel-
> > > boun...@lists.01.org] On Behalf Of BobCF
> > > Sent: Thursday, November 8, 2018 2:16 AM
> > > To: edk2-devel@lists.01.org
> > > Cc: Carsey, Jaben ; Gao,
> Liming
> > > 
> > > Subject: [edk2] [Patch] BaseTools: Optimize string
> > > concatenation
> > >
> > > https://bugzilla.tianocore.org/show_bug.cgi?id=1288
> > >
> > > This patch is one of build tool performance
> improvement
> > > series patches.
> > >
> > > This patch is going to use join function instead of
> > > string += string2 statement.
> > >
> > > Current code use string += string2 in a loop to
> combine
> > > a string. while creating a string list in a loop
> and
> > > using
> > > "".join(stringlist) after the loop will be much
> faster.
> > >
> > > Contributed-under: TianoCore Contribution Agreement
> 1.1
> > > Signed-off-by: BobCF 
> > > Cc: Liming Gao 
> > > Cc: Jaben Carsey 
> > > ---
> > >  BaseTools/Source/Python/AutoGen/StrGather.py  | 39
> > > +--
> > >  BaseTools/Source/Python/Common/Misc.py| 21
> > > +-
> > >  .../Source/Python/Workspace/InfBuildData.py   |  4
> +-
> > >  .../Python/Workspace/WorkspaceCommon.py   | 11
> ++-
> > > ---
> > >  4 files changed, 44 insertions(+), 31 deletions(-)
> > >
> > > diff --git
> > > a/BaseTools/Source/Python/AutoGen/StrGather.py
> > > b/BaseTools/Source/Python/AutoGen/StrGather.py
> > > index 361d499076..d34a9e9447 100644
> > > --- a/BaseTools/Source/Python/AutoGen/StrGather.py
> > > +++ b/BaseTools/Source/Python/AutoGen/StrGather.py
> > > @@ -135,11 +135,11 @@ def AscToHexList(Ascii):
> > >  # @param UniGenCFlag  UniString is generated
> into
> > > AutoGen C file when it is set to True
> > >  #
> > >  # @retval Str:   A string of .h file
> content
> > >  #
> > >  def CreateHFileContent(BaseName, UniObjectClass,
> > > IsCompatibleMode, UniGenCFlag):
> > > -Str = ''
> > > +Str = []
> > >  ValueStartPtr = 60
> > >  Line = COMMENT_DEFINE_STR + ' ' +
> > > LANGUAGE_NAME_STRING_NAME + ' ' * (ValueStartPtr -
> > > len(DEFINE_STR + LANGUAGE_NAME_STRING_NAME)) +
> > > DecToHexStr(0, 4) + COMMENT_NOT_REFERENCED
> > >  Str = WriteLine(Str, Line)
> > >  Line = COMMENT_DEFINE_STR + ' ' +
> > > PRINTABLE_LANGUAGE_NAME_STRING_NAME + ' ' *
> > > (ValueStartPtr - len(DEFINE_STR +
> > > PRINTABLE_LANGUAGE_NAME_STRING_NAME)) +
> DecToHexStr(1,
> > > 4) + COMMENT_NOT_REFERENCED
> > >  Str = WriteLine(Str, Line)
> > > @@ -164,16 +164,16 @@ def
> CreateHFileContent(BaseName,
> > > UniObjectClass, IsCompatibleMode, UniGenCFlag):
> > >  Line = COMMENT_DEFINE_STR + '
> ' +
> > > Name + ' ' + DecToHexStr(Token, 4) +
> > > COMMENT_NOT_REFERENCED
> > >  else:
> > >  Line = COMMENT_DEFINE_STR + '
> ' +
> > > Name + ' ' * (ValueStartPtr - len(DEFINE_STR +
> Name)) +
> > > DecToHexStr(Token, 4) + COMMENT_NOT_REFERENCED
> > >  UnusedStr = WriteLine(UnusedStr,
> Line)
> > >
> > > -Str = ''.join([Str, UnusedStr])
> > > +Str.extend( UnusedStr)
> > >
> > >  Str = WriteLine(Str, '')
> > >  if IsCompatibleMode or UniGenCFlag:
> > >  Str = WriteLine(Str, 'extern unsigned char
> ' +
> > > BaseName + 'Strings[];')
> > > -return Str
> > > +return "".join(Str)
> > >
> > >  ## Create a complete .h file
> > >  #
> > >  # Create a complet .h file with file header and
> file
> > > content
> > >  #
> > > 

[edk2] [urgent] Soft Feature Freeze has started since Nov.1 for dk2-stable201811

[Laszlo Ersek]

+ Yuchenlin, + Gerd, + both Phils

On 11/07/18 20:13, Laszlo Ersek wrote:

>>> For example, I reviewed and pushed 4 patches yesterday (on 2018-Nov-06):
>>>
>>>   1  e038bde2679b Revert "OvmfPkg/QemuVideoDxe: list 
>>> "UnalignedIoInternal.h" in the INF file"
>>>   2  98856a724c2a Revert "OvmfPkg/QemuVideoDxe: VMWare SVGA device support"
>>>   3  438ada5aa5a1 Revert "OvmfPkg/QemuVideoDxe: Helper functions for 
>>> unaligned port I/O."
>>>   4  328409ce8de7 Revert "OvmfPkg: VMWare SVGA display device register 
>>> definitions"
>>>
>>> which are the first four patches (out of five) from the following
>>> series:
>>>
>>>   [edk2] [PATCH v2 0/5] OvmfPkg: simply use the Bochs interface for vmsvga
>>>
>>> These reverts are arguably not bugfixes; they are preparation for
>>> re-implementing a feature from scratch (the last patch in that series).
>>> Thus, had I known we were already in the Soft Feature Freeze, I wouldn't
>>> have pushed them, because the review was not complete before the soft
>>> freeze start.
>>>
>>> But I had just returned from a week (or more) of PTO, there was no
>>> announcement on the list yet, and I didn't remember the wiki page.
>>>
>>> (In the technical sense, the reverts are not disruptive, luckily; they
>>> remove code that is dead anyway.)

I've given this more thought.

The reverts indeed remove dead code, but the code in question is dead
*only* on QEMU v2.10+. On QEMU v2.9 and earlier, the code is not dead.

(See the original discussion in the thread "[edk2] [PATCH] OvmfPkg:
initialize bochs when initializing vmsvga".)

This means that, with only the first four patches applied from the
series (= the reverts), and with the fifth patch (= the clean
re-implementation of the feature) postponed, people running
edk2-stable201811 on *old* -- v2.9 or older -- QEMU, with VMW SVGA, will
suffer a regression.

So that leaves me with a question. Should I revert the first four
patches now, for edk2-stable201811? (I.e., revert the reverts -->
re-instate the incorrect VMW SVGA driver impl, that happens to work on
v2.9 and earlier.)

... Note that upstream QEMU no longer supports (= maintains stable
branches) for v2.9 and earlier releases. The QEMU homepage
 currently advertizes:
- 3.1.0-rc0
- 3.0.0
- 2.12.1
- 2.11.2

Personally I'm leaning towards keeping the reverts for
edk2-stable201811. (v2.9 is really old, and the VMW SVGA device model is
virtually unused.) For my professional integrity though, I must ask this
question publicly.

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

Re: [edk2] [patch] MdeModulePkg/DisplayEngine: Remove useless NULL ptr check for NewPos

[Laszlo Ersek]

On 11/09/18 17:01, Leif Lindholm wrote:
> On Fri, Nov 09, 2018 at 03:32:03PM +, Gao, Liming wrote:
>> Laszlo and Leif:
>>   I suggest to continue to update wiki page with more
>>   information. If so, we can avoid such case again.
> 
> Agreed, we need to be able to interpret what the process says
> identically.

Making the wiki real precise on this question requires dedicated work. I
was OK to simply send a patch about the announcements, but this topic
requires more thinking, more discussion, and well, more tracking.

Mike: would it be proper to create a "Wiki" Component under the EDK2
product in the bugzilla?

If so, I would suggest filing a BZ against that (new) component, and
then we should discuss the freeze scopes in one of the next community
meetings.

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

Re: [edk2] [PATCH v2 edk2-platforms 1/5] Platform/ARM/Sgi: Adapt to changes in system-id DT node.

[Leif Lindholm]

On Mon, Nov 05, 2018 at 02:56:55PM +0530, Chandni Cherukuri wrote:
> The 'system-id' node of HW_CONFIG device tree has been updated to have
> a new property 'config-id' to hold the platform configuration value.
> Prior to this, configuration ID value was represented by the the upper
> four bits of the 'platform ID' property value but it now has a seperate
> property of its own in the 'system-id' node. So adapt to these changes
> in the 'system-id' node.

This gets a bit hairy semantically:
- PlatformId used to be the contents of node "platform-id"
- PlatformId is now the union of the lower 28 bits of node
  "platform-id" and the 4 bottom bits of new node "config-id",
- The result is still called PlatformId.

Is there some better way of describing this? Should the function name
change?

> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Chandni Cherukuri 
> ---
>  .../SgiPkg/Library/SgiPlatformPei/SgiPlatformPeim.c   | 19 
> ++-

Can you ensure you generate patches in accordance with the
instructions at
https://github.com/tianocore/tianocore.github.io/wiki/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers#contrib-23
?

>  1 file changed, 18 insertions(+), 1 deletion(-)
> 
> diff --git a/Platform/ARM/SgiPkg/Library/SgiPlatformPei/SgiPlatformPeim.c 
> b/Platform/ARM/SgiPkg/Library/SgiPlatformPei/SgiPlatformPeim.c
> index 081d329..8b6c71b 100644
> --- a/Platform/ARM/SgiPkg/Library/SgiPlatformPei/SgiPlatformPeim.c
> +++ b/Platform/ARM/SgiPkg/Library/SgiPlatformPei/SgiPlatformPeim.c
> @@ -42,6 +42,8 @@ GetSgiPlatformId (

Since this is changing what the return value of the function means,
can the function description comment be updated as well?

>CONST VOID*HwCfgDtBlob;
>SGI_HW_CONFIG_INFO_PPI*HwConfigInfoPpi;
>EFI_STATUSStatus;
> +  UINT32PlatformId;
> +  UINT32ConfigId;
>  
>Status = PeiServicesLocatePpi (, 0, NULL,
>   (VOID**));
> @@ -69,7 +71,22 @@ GetSgiPlatformId (
>  return 0;
>}
>  
> -  return fdt32_to_cpu (*Property);
> +  PlatformId = fdt32_to_cpu (*Property);
> +
> +  Property = fdt_getprop (HwCfgDtBlob, Offset, "config-id", NULL);
> +  if (Property == NULL) {
> +DEBUG ((DEBUG_ERROR, "Config Id is NULL\n"));
> +return 0;
> +  }
> +
> +  ConfigId = fdt32_to_cpu (*Property);
> +
> +  // Concatenate the value of config ID into the platform ID value with
> +  // config ID occupying the upper 4 bits of platform ID.
> +  ConfigId = ConfigId & SGI_CONFIG_MASK;

We are here silently discarding 28 bits of data that need to be set to
0 for the below operation to make sense.
I think an ASSERT might be in order.

I also note that we've neither looked at nor cleared the top four bits
of PlatformId.

> +  PlatformId = PlatformId | (ConfigId << SGI_CONFIG_SHIFT);
> +
> +  return PlatformId;

I would be happier with just the return statement - reusing the
PlatformId variable does not simplify anything.

/
Leif.

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

Re: [edk2] [Patch] SecurityPkg: Fix TPM device compatibility issue

[Laszlo Ersek]

meta:

On 11/09/18 16:55, Leif Lindholm wrote:
> On Fri, Nov 09, 2018 at 02:46:40PM +, Zhang, Chao B wrote:
>> Hi Leif:
>>The NTC1310 can work well with previous EDK2 stable version
>>(UDK2018). Interface Cache is a new feature introduced after
>>UDK2018.
>> So far as we see, it causes NTC1310 fail to work.
> 
> OK, in that case I am willing to change my opinion from "no" to "I
> would prefer not".
> 
> Again, this patch went into the tree on 25 June. It was part of
> edk2-stable201808, but we only see a report during the hard freeze
> period preceding edk2-stable201811.

(I didn't mean to "steal" this argument -- I'm honestly seeing this
email only now, after sending my email a minute ago.)

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

Re: [edk2] [Patch] SecurityPkg: Fix TPM device compatibility issue

[Laszlo Ersek]

On 11/09/18 15:46, Zhang, Chao B wrote:
> Hi Leif:
>The NTC1310 can work well with previous EDK2 stable version (UDK2018).

That's not correct. The last EDK2 stable version is not UDK2018. The
last stable EDK2 version is "edk2-stable201808", and the regression
(commit f15cb995bb38, according to this patch) is already contained in
"edk2-stable201808".

> Interface Cache is a new feature introduced after UDK2018.
> So far as we see, it causes NTC1310 fail to work.

That may be the case, but it's not a feature (and resultant regression)
introduced in this development cycle.

Personally I'm still undecided.

- From an end-user perspective, this is certainly a "bugfix"; given that
the NTC1310 hardware (which is the real culprit) cannot be fixed at all.

- In a technical edk2 sense, this is a feature-let however (with code
duplication at that) that works around a broken device that already
doesn't work with "edk2-stable201808".


I'm *slightly* leaning against this change. If the end-user regression
had really been painful, this workaround would have been implemented
very soon after "edk2-stable201808", not in a last minute rush before
"edk2-stable201811".

I'd like to hear Mike's and Andrew's opinion too.

Thanks,
Laszlo

> 
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Leif 
> Lindholm
> Sent: Friday, November 9, 2018 7:13 PM
> To: Laszlo Ersek 
> Cc: Kinney, Michael D ; edk2-devel@lists.01.org; 
> Yao, Jiewen ; Zhang, Chao B 
> Subject: Re: [edk2] [Patch] SecurityPkg: Fix TPM device compatibility issue
> 
> On Fri, Nov 09, 2018 at 09:04:46AM +0100, Laszlo Ersek wrote:
>> On 11/09/18 07:02, Zhang, Chao B wrote:
>>> Issue Statement:
>>> TPM InterfaceId cache feature is introduced by 
>>> f15cb995bb3880b77e15afe6facd3da05e599a17. It follows TCG PTP spec 1.3
>>> to improve TPM transmission performance and also addresses defects in some 
>>> TPM2.0 devices. But some other TPM devices like
>>> NTC1310 SPI TPM is found function abnormally with this feature, causing 
>>> extra device compatibility issue.
>>>
>>> Solution:
>>> Add a policy indicator in PcdActiveTpmInterfaceType to disable TPM 
>>> interface ID cache to support those existing TPM devices
>>>
>>> Contributed-under: TianoCore Contribution Agreement 1.1
>>> Signed-off-by: Zhang, Chao B 
>>> mailto:chao.b.zh...@intel.com>>
>>> Cc: Andrew Fish mailto:af...@apple.com>>
>>> Cc: Laszlo Ersek mailto:ler...@redhat.com>>
>>> Cc: Leif Lindholm 
>>> mailto:leif.lindh...@linaro.org>>
>>> Cc: Michael D Kinney 
>>> mailto:michael.d.kin...@intel.com>>
>>> Cc: Yao Jiewen mailto:jiewen@intel.com>>
>>> ---
>>>  SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c | 23 +++-
>>>  SecurityPkg/SecurityPkg.dec |  3 +-
>>>  SecurityPkg/SecurityPkg.uni |  3 +-
>>>  SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigImpl.c | 49 
>>> +
>>>  SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c   | 42 +
>>>  5 files changed, 117 insertions(+), 3 deletions(-)
>>
>> I'll let others review this patch for technical merit.
>>
>> However, I'm really undecided whether this patch qualifies for being
>> pushed during the hard feature freeze. Comments welcome.
> 
> Unless the current behaviour causes an absolutely horrendous security
> hole, I don't see how this qualifies for pushing during hard freeze.
> 
> According to its description, this is about supporting (non-compliant)
> devices that have never worked with EDK2. And the support it updates
> went in on 25 June. So there does not appear to be any urgency.
> 
> Once it does go in, I would also appreciate some simplification via
> macros to cut down some of those very long lines, but then I'm not the
> maintainer of this package.
> 
> Regards,
> 
> Leif
> ___
> 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] MdeModulePkg/DisplayEngine: Remove useless NULL ptr check for NewPos

[Leif Lindholm]

On Fri, Nov 09, 2018 at 03:32:03PM +, Gao, Liming wrote:
> Laszlo and Leif:
>   I suggest to continue to update wiki page with more
>   information. If so, we can avoid such case again.

Agreed, we need to be able to interpret what the process says
identically.

>   For this change, it has no real functionality impact.

But this is my point: we should not be making judgement calls this
late in the process. If I look at that patch, sure, it looks fine to
me. I still don't want it going in during freeze, because I don't
_know_ it has no real functionality impact. I may be missing
something. And even if I am not missing anything, the reshuffle may
still be sufficient to change compiler behaviour, exposing a
previously undetected bug.

>   If you think roll back is better than keep it, I am OK.

That would be my preference. I have zero objection to it going back in
immediately after the stable tag is made.

Regards,

Leif

> Thanks
> Liming
> > -Original Message-
> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> > Laszlo Ersek
> > Sent: Friday, November 9, 2018 7:02 PM
> > To: Leif Lindholm ; Gao, Liming 
> > 
> > Cc: Kinney, Michael D ; edk2-devel@lists.01.org
> > Subject: Re: [edk2] [patch] MdeModulePkg/DisplayEngine: Remove useless NULL 
> > ptr check for NewPos
> > 
> > On 11/09/18 11:49, Leif Lindholm wrote:
> > > On Fri, Nov 09, 2018 at 07:56:07AM +0100, Ard Biesheuvel wrote:
> > >> On 9 November 2018 at 01:19, Gao, Liming  wrote:
> > >>> Ard:
> > >>>   This is a small fix. And, this patch is sent before the hard
> > >>>   freeze. It is the low risk for this release. So, I push it.
> > >>
> > >> OK, fair enough.
> > >
> > > I don't agree actually.
> > >
> > > https://github.com/tianocore/tianocore.github.io/wiki/HardFeatureFreeze
> > > specifies clearly that only bug fixes are permitted in during hard
> > > freeze. Maybe we could document that a bit more explicitly, but this
> > > patch was no bugfix. It should not have gone in.
> > >
> > > By my interpretation, it would not even fulfill the requirements for
> > > https://github.com/lersek/edk2/wiki/SoftFeatureFreeze:
> > > "By the date of the soft feature freeze, developers must have sent
> > > their patches to the mailing list and received positive maintainer
> > > reviews."
> > > Soft feature freeze was 1 November. The patch was sent out 7 November.
> > > It received reviews 8 November (after the start of the hard freeze).
> > >
> > > The point of these freezes is that sometimes patches are wrong. And
> > > sometimes patches that look correct, are not correct. If we start
> > > making exceptions because "oh, it's trivial", that means we get these
> > > patches into the tree with much reduced time for anyone to catch any
> > > adverse effects before we make the stable tag. And at that point, the
> > > stable tag no longer has value.
> > >
> > > (I am much more flexible on the topic of updating documentation, like
> > > Maintainers.txt, but even there we must be very careful.)
> > 
> > I haven't been following this specific patch, but now it does not look
> > like a bugfix to me. Without applying the patch, there is no bug
> > actually, functional or performance. The subject says, "Remove useless ...".
> > 
> > Optimizations, simplifications, refactorings, features, and so on, are
> > not bugfixes. They should not go in after the hard freeze. Even after
> > the soft freeze, they should only go in if the only remaining step is
> > the push (i.e. they should be ready for pushing before the soft freeze,
> > sufficiently reviewed.).
> > 
> > Thanks
> > Laszlo
> > 
> >  -Original Message-
> >  From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> >  Sent: Friday, November 09, 2018 2:25 AM
> >  To: Zeng, Star 
> >  Cc: Bi, Dandan ; edk2-devel@lists.01.org; Wu, Hao 
> >  A
> >  ; Dong, Eric ; Gao, Liming
> >  
> >  Subject: Re: [edk2] [patch] MdeModulePkg/DisplayEngine: Remove useless
> >  NULL ptr check for NewPos
> > 
> >  On 8 November 2018 at 02:09, Zeng, Star  wrote:
> > > Reviewed-by: Star Zeng 
> > >
> > > -Original Message-
> > > From: Bi, Dandan
> > > Sent: Wednesday, November 7, 2018 10:53 PM
> > > To: edk2-devel@lists.01.org
> > > Cc: Gao, Liming ; Dong, Eric 
> > > ;
> >  Zeng, Star ; Wu, Hao A 
> > > Subject: [patch] MdeModulePkg/DisplayEngine: Remove useless NULL ptr
> >  check for NewPos
> > >
> > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1306
> > >
> > > In function UiDisplayMenu, the NewPos ptr which used to point to the
> >  highlight menu entry. It will always point to the menu entry which 
> >  need to be
> >  highlighted or the gMenuOption menu if the highlight menu is not found.
> > > So we can remove the NULL ptr check for NewPos in this function.
> > > And add the ASSERT code to avoid if any false positive reports of NULL
> >  pointer 

Re: [edk2] [Patch] SecurityPkg: Fix TPM device compatibility issue

[Leif Lindholm]

On Fri, Nov 09, 2018 at 02:46:40PM +, Zhang, Chao B wrote:
> Hi Leif:
>The NTC1310 can work well with previous EDK2 stable version
>(UDK2018). Interface Cache is a new feature introduced after
>UDK2018.
> So far as we see, it causes NTC1310 fail to work.

OK, in that case I am willing to change my opinion from "no" to "I
would prefer not".

Again, this patch went into the tree on 25 June. It was part of
edk2-stable201808, but we only see a report during the hard freeze
period preceding edk2-stable201811.

My concern is this:
This patch _will_ affect platforms other than the ones displaying the
erroneous behaviour, and all affected platforms can manually apply
this patch until it shows up in edk2-stable201902.
Anyone not working against the stable tags can have it available in
master a week from now.

If the package maintainers do consider this a release critical bug,
it needs a bugzilla entry (referenced in the commit message). The
style issues should also be addressed.

Regards,

Leif

> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Leif 
> Lindholm
> Sent: Friday, November 9, 2018 7:13 PM
> To: Laszlo Ersek 
> Cc: Kinney, Michael D ; edk2-devel@lists.01.org; 
> Yao, Jiewen ; Zhang, Chao B 
> Subject: Re: [edk2] [Patch] SecurityPkg: Fix TPM device compatibility issue
> 
> On Fri, Nov 09, 2018 at 09:04:46AM +0100, Laszlo Ersek wrote:
> > On 11/09/18 07:02, Zhang, Chao B wrote:
> > > Issue Statement:
> > > TPM InterfaceId cache feature is introduced by 
> > > f15cb995bb3880b77e15afe6facd3da05e599a17. It follows TCG PTP spec 1.3
> > > to improve TPM transmission performance and also addresses defects in 
> > > some TPM2.0 devices. But some other TPM devices like
> > > NTC1310 SPI TPM is found function abnormally with this feature, causing 
> > > extra device compatibility issue.
> > >
> > > Solution:
> > > Add a policy indicator in PcdActiveTpmInterfaceType to disable TPM 
> > > interface ID cache to support those existing TPM devices
> > >
> > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > Signed-off-by: Zhang, Chao B 
> > > mailto:chao.b.zh...@intel.com>>
> > > Cc: Andrew Fish mailto:af...@apple.com>>
> > > Cc: Laszlo Ersek mailto:ler...@redhat.com>>
> > > Cc: Leif Lindholm 
> > > mailto:leif.lindh...@linaro.org>>
> > > Cc: Michael D Kinney 
> > > mailto:michael.d.kin...@intel.com>>
> > > Cc: Yao Jiewen mailto:jiewen@intel.com>>
> > > ---
> > >  SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c | 23 +++-
> > >  SecurityPkg/SecurityPkg.dec |  3 +-
> > >  SecurityPkg/SecurityPkg.uni |  3 +-
> > >  SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigImpl.c | 49 
> > > +
> > >  SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c   | 42 
> > > +
> > >  5 files changed, 117 insertions(+), 3 deletions(-)
> >
> > I'll let others review this patch for technical merit.
> >
> > However, I'm really undecided whether this patch qualifies for being
> > pushed during the hard feature freeze. Comments welcome.
> 
> Unless the current behaviour causes an absolutely horrendous security
> hole, I don't see how this qualifies for pushing during hard freeze.
> 
> According to its description, this is about supporting (non-compliant)
> devices that have never worked with EDK2. And the support it updates
> went in on 25 June. So there does not appear to be any urgency.
> 
> Once it does go in, I would also appreciate some simplification via
> macros to cut down some of those very long lines, but then I'm not the
> maintainer of this package.
> 
> Regards,
> 
> Leif
> ___
> 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] MdeModulePkg/DisplayEngine: Remove useless NULL ptr check for NewPos

[Gao, Liming]

Laszlo and Leif:
  I suggest to continue to update wiki page with more information. If so, we 
can avoid such case again. 

  For this change, it has no real functionality impact. If you think roll back 
is better than keep it, I am OK. 

Thanks
Liming
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Laszlo 
> Ersek
> Sent: Friday, November 9, 2018 7:02 PM
> To: Leif Lindholm ; Gao, Liming 
> 
> Cc: Kinney, Michael D ; edk2-devel@lists.01.org
> Subject: Re: [edk2] [patch] MdeModulePkg/DisplayEngine: Remove useless NULL 
> ptr check for NewPos
> 
> On 11/09/18 11:49, Leif Lindholm wrote:
> > On Fri, Nov 09, 2018 at 07:56:07AM +0100, Ard Biesheuvel wrote:
> >> On 9 November 2018 at 01:19, Gao, Liming  wrote:
> >>> Ard:
> >>>   This is a small fix. And, this patch is sent before the hard
> >>>   freeze. It is the low risk for this release. So, I push it.
> >>
> >> OK, fair enough.
> >
> > I don't agree actually.
> >
> > https://github.com/tianocore/tianocore.github.io/wiki/HardFeatureFreeze
> > specifies clearly that only bug fixes are permitted in during hard
> > freeze. Maybe we could document that a bit more explicitly, but this
> > patch was no bugfix. It should not have gone in.
> >
> > By my interpretation, it would not even fulfill the requirements for
> > https://github.com/lersek/edk2/wiki/SoftFeatureFreeze:
> > "By the date of the soft feature freeze, developers must have sent
> > their patches to the mailing list and received positive maintainer
> > reviews."
> > Soft feature freeze was 1 November. The patch was sent out 7 November.
> > It received reviews 8 November (after the start of the hard freeze).
> >
> > The point of these freezes is that sometimes patches are wrong. And
> > sometimes patches that look correct, are not correct. If we start
> > making exceptions because "oh, it's trivial", that means we get these
> > patches into the tree with much reduced time for anyone to catch any
> > adverse effects before we make the stable tag. And at that point, the
> > stable tag no longer has value.
> >
> > (I am much more flexible on the topic of updating documentation, like
> > Maintainers.txt, but even there we must be very careful.)
> 
> I haven't been following this specific patch, but now it does not look
> like a bugfix to me. Without applying the patch, there is no bug
> actually, functional or performance. The subject says, "Remove useless ...".
> 
> Optimizations, simplifications, refactorings, features, and so on, are
> not bugfixes. They should not go in after the hard freeze. Even after
> the soft freeze, they should only go in if the only remaining step is
> the push (i.e. they should be ready for pushing before the soft freeze,
> sufficiently reviewed.).
> 
> Thanks
> Laszlo
> 
>  -Original Message-
>  From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
>  Sent: Friday, November 09, 2018 2:25 AM
>  To: Zeng, Star 
>  Cc: Bi, Dandan ; edk2-devel@lists.01.org; Wu, Hao A
>  ; Dong, Eric ; Gao, Liming
>  
>  Subject: Re: [edk2] [patch] MdeModulePkg/DisplayEngine: Remove useless
>  NULL ptr check for NewPos
> 
>  On 8 November 2018 at 02:09, Zeng, Star  wrote:
> > Reviewed-by: Star Zeng 
> >
> > -Original Message-
> > From: Bi, Dandan
> > Sent: Wednesday, November 7, 2018 10:53 PM
> > To: edk2-devel@lists.01.org
> > Cc: Gao, Liming ; Dong, Eric 
> > ;
>  Zeng, Star ; Wu, Hao A 
> > Subject: [patch] MdeModulePkg/DisplayEngine: Remove useless NULL ptr
>  check for NewPos
> >
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1306
> >
> > In function UiDisplayMenu, the NewPos ptr which used to point to the
>  highlight menu entry. It will always point to the menu entry which need 
>  to be
>  highlighted or the gMenuOption menu if the highlight menu is not found.
> > So we can remove the NULL ptr check for NewPos in this function.
> > And add the ASSERT code to avoid if any false positive reports of NULL
>  pointer dereference issue raised from static analysis.
> >
> > Cc: Liming Gao 
> > Cc: Eric Dong 
> > Cc: Star Zeng 
> > Cc: Hao Wu 
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Dandan Bi 
> 
>  Why was this patch merged today? Surely, this doesn't meet the hard
>  freeze requirements ?
> 
> > ---
> >  MdeModulePkg/Universal/DisplayEngineDxe/FormDisplay.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/MdeModulePkg/Universal/DisplayEngineDxe/FormDisplay.c
>  b/MdeModulePkg/Universal/DisplayEngineDxe/FormDisplay.c
> > index 7390f954b6..44f087fe01 100644
> > --- a/MdeModulePkg/Universal/DisplayEngineDxe/FormDisplay.c
> > +++ b/MdeModulePkg/Universal/DisplayEngineDxe/FormDisplay.c
> > @@ -2880,10 +2880,11 @@ UiDisplayMenu (
> >

Re: [edk2] [Patch V2] BaseTools: Replace the sqlite database with list

[Carsey, Jaben]

This is much easier to review! Thanks!

I am good with the code change.

I wonder: If we do not have the DB does it take longer if I do build, clean, 
build than it did before?

> -Original Message-
> From: Feng, Bob C
> Sent: Friday, November 09, 2018 12:42 AM
> To: edk2-devel@lists.01.org
> Cc: Gao, Liming ; Carsey, Jaben
> 
> Subject: [Patch V2] BaseTools: Replace the sqlite database with list
> Importance: High
> 
> https://bugzilla.tianocore.org/show_bug.cgi?id=1288
> 
> [V2]
> Optimize this patch so that it can be easy to review.
> This patch is just apply the change to original files while
> not create new similar files.
> 
> [V1]
> This patch is one of build tool performance improvement
> series patches.
> 
> This patch is going to use python list to store the parser data
> instead of using sqlite database.
> 
> The replacement solution is as below:
> 
> SQL insert: list.append()
> SQL select: list comprehension. for example:
> Select * from table where field = “something”
> ->
> [ item for item in table if item[3] == “something”]
> 
> SQL update: python map function. for example:
> Update table set field1=newvalue where filed2 = “something”.
> -> map(lambda x: x[1] = newvalue,
>[item for item in table if item[2] == “something”])
> 
> SQL delete: list comprehension.
> 
> With this change, We can save the time of interpreting SQL statement
> and the time of write database to file system
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: BobCF 
> Cc: Liming Gao 
> Cc: Jaben Carsey 
> ---
>  BaseTools/Source/Python/GenFds/GenFds.py  |   3 +-
>  .../Source/Python/Workspace/MetaDataTable.py  |  93 ++-
>  .../Source/Python/Workspace/MetaFileParser.py |   5 +-
>  .../Source/Python/Workspace/MetaFileTable.py  | 248 ++
>  .../Python/Workspace/WorkspaceDatabase.py | 131 +
>  BaseTools/Source/Python/build/build.py|  27 +-
>  6 files changed, 186 insertions(+), 321 deletions(-)
> 
> diff --git a/BaseTools/Source/Python/GenFds/GenFds.py
> b/BaseTools/Source/Python/GenFds/GenFds.py
> index 0c8091b798..4fd96706af 100644
> --- a/BaseTools/Source/Python/GenFds/GenFds.py
> +++ b/BaseTools/Source/Python/GenFds/GenFds.py
> @@ -222,12 +222,11 @@ def main():
>  if "TOOL_CHAIN_TAG" not in GlobalData.gGlobalDefines:
>  GlobalData.gGlobalDefines['TOOL_CHAIN_TAG'] =
> GenFdsGlobalVariable.ToolChainTag
> 
>  """call Workspace build create database"""
>  GlobalData.gDatabasePath =
> os.path.normpath(os.path.join(ConfDirectoryPath,
> GlobalData.gDatabasePath))
> -BuildWorkSpace = WorkspaceDatabase(GlobalData.gDatabasePath)
> -BuildWorkSpace.InitDatabase()
> +BuildWorkSpace = WorkspaceDatabase()
> 
>  #
>  # Get files real name in workspace dir
>  #
>  GlobalData.gAllFiles = DirCache(Workspace)
> diff --git a/BaseTools/Source/Python/Workspace/MetaDataTable.py
> b/BaseTools/Source/Python/Workspace/MetaDataTable.py
> index bd751eadfb..8becddbe08 100644
> --- a/BaseTools/Source/Python/Workspace/MetaDataTable.py
> +++ b/BaseTools/Source/Python/Workspace/MetaDataTable.py
> @@ -37,84 +37,58 @@ class Table(object):
>  _COLUMN_ = ''
>  _ID_STEP_ = 1
>  _ID_MAX_ = 0x8000
>  _DUMMY_ = 0
> 
> -def __init__(self, Cursor, Name='', IdBase=0, Temporary=False):
> -self.Cur = Cursor
> +def __init__(self, Db, Name='', IdBase=0, Temporary=False):
> +self.Db = Db
>  self.Table = Name
>  self.IdBase = int(IdBase)
>  self.ID = int(IdBase)
>  self.Temporary = Temporary
> +self.Contents = []
> 
>  def __str__(self):
>  return self.Table
> 
>  ## Create table
>  #
>  # Create a table
>  #
>  def Create(self, NewTable=True):
> -if NewTable:
> -self.Drop()
> -
> -if self.Temporary:
> -SqlCommand = """create temp table IF NOT EXISTS %s (%s)""" %
> (self.Table, self._COLUMN_)
> -else:
> -SqlCommand = """create table IF NOT EXISTS %s (%s)""" % 
> (self.Table,
> self._COLUMN_)
> -EdkLogger.debug(EdkLogger.DEBUG_8, SqlCommand)
> -self.Cur.execute(SqlCommand)
> +self.Db.CreateEmptyTable(self.Table)
>  self.ID = self.GetId()
> 
>  ## Insert table
>  #
>  # Insert a record into a table
>  #
>  def Insert(self, *Args):
>  self.ID = self.ID + self._ID_STEP_
>  if self.ID >= (self.IdBase + self._ID_MAX_):
>  self.ID = self.IdBase + self._ID_STEP_
> -Values = ", ".join(str(Arg) for Arg in Args)
> -SqlCommand = "insert into %s values(%s, %s)" % (self.Table, self.ID,
> Values)
> -EdkLogger.debug(EdkLogger.DEBUG_5, SqlCommand)
> -self.Cur.execute(SqlCommand)
> -return self.ID
> +row = [self.ID]
> +row.extend(Args)
> +self.Contents.append(row)
> 
> -## Query table
> -#
> -

Re: [edk2] [PATCH] BaseTools: Enable Pcd Array support.

[Carsey, Jaben]

How concerned with spaces in the DSC lines are we?  The following looks very 
sensitive to spaces and could be replaced with a regular expression that would 
be more flexible.

if "{CODE(" not in PcdValue:

aprox:
if re.match('[{]\w*CODE\w*[(]', PcdValue)

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> BobCF
> Sent: Wednesday, November 07, 2018 1:19 AM
> To: edk2-devel@lists.01.org
> Cc: Gao, Liming 
> Subject: [edk2] [PATCH] BaseTools: Enable Pcd Array support.
> 
> From: "bob.c.f...@intel.com" 
> 
> https://bugzilla.tianocore.org/show_bug.cgi?id=1292
> 
> This patch is going to enable Array data type for PCD.
> 
> 1. Support Pcd ARRAY as Structure PCD type
>including basic datatype array and structure array.
>For example:
>gStructuredPcdPkgTokenSpaceGuid.PcdTest|{0x0}|TEST[10]|0x00010080
> 
> gStructuredPcdPkgTokenSpaceGuid.PcdTest2|{0x0}|UINT8[10]|0x00010081
> 2. Support C CODE style value initialization in DEC/DSC.
>For example:
> gStructuredPcdPkgTokenSpaceGuid.PcdTest|{CODE({
>   {0, {0, 0, 0, 0,  0, 0, 0}},
>   {0, {0, 0, 0, 0,  0, 0, 0}},
>   {0, {0, 0, 0, 0,  0, 0, 0}},
>   {0, {0, 0, 0, 0,  0, 0, 0}},
>   {0, {0, 0, 0, 0,  0, 0, 0}},
>   {0, {0, 0, 0, 0,  0, 0, 0}},
>   {0, {0, 0, 0, 0,  0, 0, 0}},
>   {0, {0, 0, 0, 0,  0, 0, 0}},
>   {0, {0, 0, 0, 0,  0, 0, 0}},
>   {0, {0, 0, 0, 0,  0, 0, 0}},
> })}
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Bob Feng 
> Cc: Liming Gao 
> ---
>  BaseTools/Source/Python/Common/Expression.py  | 396 +
> -
>  BaseTools/Source/Python/Common/Misc.py|   2 +
>  .../Python/Workspace/BuildClassObject.py  |  85 +++-
>  .../Source/Python/Workspace/DecBuildData.py   |  29 +-
>  .../Source/Python/Workspace/DscBuildData.py   | 384 +++--
>  .../Source/Python/Workspace/MetaFileParser.py |  86 +++-
>  6 files changed, 644 insertions(+), 338 deletions(-)
> 
> diff --git a/BaseTools/Source/Python/Common/Expression.py
> b/BaseTools/Source/Python/Common/Expression.py
> index a21ab5daa7..f6e245be70 100644
> --- a/BaseTools/Source/Python/Common/Expression.py
> +++ b/BaseTools/Source/Python/Common/Expression.py
> @@ -820,221 +820,219 @@ class ValueExpressionEx(ValueExpression):
> 
>  def __call__(self, RealValue=False, Depth=0):
>  PcdValue = self.PcdValue
> -try:
> -PcdValue = ValueExpression.__call__(self, RealValue, Depth)
> -if self.PcdType == TAB_VOID and (PcdValue.startswith("'") or
> PcdValue.startswith("L'")):
> -PcdValue, Size = ParseFieldValue(PcdValue)
> -PcdValueList = []
> -for I in range(Size):
> -PcdValueList.append('0x%02X'%(PcdValue & 0xff))
> -PcdValue = PcdValue >> 8
> -PcdValue = '{' + ','.join(PcdValueList) + '}'
> -elif self.PcdType in TAB_PCD_NUMERIC_TYPES and
> (PcdValue.startswith("'") or \
> -  PcdValue.startswith('"') or PcdValue.startswith("L'") 
> or
> PcdValue.startswith('L"') or PcdValue.startswith('{')):
> -raise BadExpression
> -except WrnExpression as Value:
> -PcdValue = Value.result
> -except BadExpression as Value:
> -if self.PcdType in TAB_PCD_NUMERIC_TYPES:
> -PcdValue = PcdValue.strip()
> -if PcdValue.startswith('{') and PcdValue.endswith('}'):
> -PcdValue = SplitPcdValueString(PcdValue[1:-1])
> -if ERR_STRING_CMP.split(':')[0] in Value.message:
> -raise BadExpression("Type: %s, Value: %s, %s" % 
> (self.PcdType,
> PcdValue, Value))
> -if isinstance(PcdValue, type([])):
> -TmpValue = 0
> -Size = 0
> -ValueType = ''
> -for Item in PcdValue:
> -Item = Item.strip()
> -if Item.startswith(TAB_UINT8):
> -ItemSize = 1
> -ValueType = TAB_UINT8
> -elif Item.startswith(TAB_UINT16):
> -ItemSize = 2
> -ValueType = TAB_UINT16
> -elif Item.startswith(TAB_UINT32):
> -ItemSize = 4
> -ValueType = TAB_UINT32
> -elif Item.startswith(TAB_UINT64):
> -ItemSize = 8
> -ValueType = TAB_UINT64
> -elif Item[0] in {'"', "'", 'L'}:
> -ItemSize = 0
> -ValueType = TAB_VOID
> -else:
> -ItemSize = 0
> -ValueType = TAB_UINT8
> -Item = ValueExpressionEx(Item, ValueType, 
> self._Symb)(True)
> -
> - 

Re: [edk2] [edk2-wiki PATCH] release planning: announce the soft and hard feature freezes

[Gao, Liming]

Laszlo:
 Can ';' be removed from below sentence? 

an announcement email is sent to the edk2-devel mailing list; by default by 
Liming Gao.

==>

an announcement email is sent to the edk2-devel mailing list by default by 
Liming Gao.

Thanks
Liming
> -Original Message-
> From: Leif Lindholm [mailto:leif.lindh...@linaro.org]
> Sent: Friday, November 9, 2018 6:05 PM
> To: Laszlo Ersek 
> Cc: edk2-devel@lists.01.org; Andrew Fish ; Richardson, Brian 
> ; Gao, Liming
> ; Kinney, Michael D ; 
> Cetola, Stephano 
> Subject: Re: [edk2-wiki PATCH] release planning: announce the soft and hard 
> feature freezes
> 
> On Thu, Nov 08, 2018 at 06:28:07PM +0100, Laszlo Ersek wrote:
> > Add a paragraph to each of the SoftFeatureFreeze and HardFeatureFreeze
> > articles about the respective announcements on edk2-devel.
> >
> > Cc: Andrew Fish 
> > Cc: Brian Richardson 
> > Cc: Leif Lindholm 
> > Cc: Liming Gao 
> > Cc: Michael Kinney 
> > Cc: Stephano Cetola 
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Laszlo Ersek 
> 
> Reviewed-by: Leif Lindholm 
> 
> > ---
> >
> > Notes:
> > Check out the rendered pages in my clone of the wiki:
> >
> > https://github.com/lersek/edk2/wiki/SoftFeatureFreeze
> > https://github.com/lersek/edk2/wiki/HardFeatureFreeze
> >
> >  HardFeatureFreeze.md | 8 
> >  SoftFeatureFreeze.md | 8 
> >  2 files changed, 16 insertions(+)
> >
> > diff --git a/HardFeatureFreeze.md b/HardFeatureFreeze.md
> > index 01288dd03f71..e08f4c047e8d 100644
> > --- a/HardFeatureFreeze.md
> > +++ b/HardFeatureFreeze.md
> > @@ -4,3 +4,11 @@ tag](EDK-II#stable-tags).
> >
> >  (This definition is modeled after QEMU's [hard feature
> >  freeze](https://wiki.qemu.org/Planning/HardFeatureFreeze)).
> > +
> > +# Announcing the hard feature freeze
> > +
> > +The proposed schedule for the active development cycle is shown in the 
> > [EDK II
> > +Release Planning](EDK-II-Release-Planning) article. Shortly before the hard
> > +feature freeze, an announcement email is sent to the
> > +[edk2-devel](https://lists.01.org/mailman/listinfo/edk2-devel) mailing 
> > list; by
> > +default by [Liming Gao](https://github.com/lgao4/).
> > diff --git a/SoftFeatureFreeze.md b/SoftFeatureFreeze.md
> > index 9dc7d9c19369..e33dd80ccbee 100644
> > --- a/SoftFeatureFreeze.md
> > +++ b/SoftFeatureFreeze.md
> > @@ -40,3 +40,11 @@ communicate with the maintainer about their intentions. 
> > It also helps if you:
> >
> >  (This definition is modeled after QEMU's [soft feature
> >  freeze](https://wiki.qemu.org/Planning/SoftFeatureFreeze).)
> > +
> > +# Announcing the soft feature freeze
> > +
> > +The proposed schedule for the active development cycle is shown in the 
> > [EDK II
> > +Release Planning](EDK-II-Release-Planning) article. Shortly before the soft
> > +feature freeze, an announcement email is sent to the
> > +[edk2-devel](https://lists.01.org/mailman/listinfo/edk2-devel) mailing 
> > list; by
> > +default by [Liming Gao](https://github.com/lgao4/).
> > --
> > 2.19.1.3.g30247aa5d201
> >
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [Patch] UefiCpuPkg/RegisterCpuFeaturesLib: Adjust Order.

[Ni, Ruiyu]

Eric,
Thanks for fixing the new found issues.

Reviewed-by: Ruiyu Ni 

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Eric
> Dong
> Sent: Friday, November 9, 2018 1:21 PM
> To: edk2-devel@lists.01.org
> Cc: Ni, Ruiyu ; Laszlo Ersek 
> Subject: [edk2] [Patch] UefiCpuPkg/RegisterCpuFeaturesLib: Adjust Order.
> 
> V2 changes:
> V1 change has regression which caused by change feature order.
> V2 changes logic to detect dependence not only for the
> neighborhood features. It need to check all features in the list.
> 
> V1 Changes:
> In current code logic, only adjust feature position if current
> CPU feature position not follow the request order. Just like
> Feature A need to be executed before feature B, but current
> feature A registers after feature B. So code will adjust the
> position for feature A, move it to just before feature B. If
> the position already met the requirement, code will not adjust
> the position.
> 
> This logic has issue when met all below cases:
> 1. feature A has core or package level dependence with feature B.
> 2. feature A is register before feature B.
> 3. Also exist other features exist between feature A and B.
> 
> Root cause is driver ignores the dependence for this case, so
> threads may execute not follow the dependence order.
> 
> Fix this issue by change code logic to adjust feature position
> for CPU features which has dependence relationship.
> 
> Cc: Laszlo Ersek 
> Cc: Ruiyu Ni 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Eric Dong 
> ---
>  .../RegisterCpuFeaturesLib/CpuFeaturesInitialize.c |  71 
>  .../RegisterCpuFeaturesLib/RegisterCpuFeatures.h   |  16 +++
>  .../RegisterCpuFeaturesLib.c   | 122 
> +
>  3 files changed, 189 insertions(+), 20 deletions(-)
> 
> diff --git a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
> b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
> index 4bed0ce3a4..69e2c04daf 100644
> --- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
> +++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
> @@ -534,6 +534,28 @@ DumpRegisterTableOnProcessor (
>}
>  }
> 
> +/**
> +  Get the biggest dependence type.
> +  PackageDepType > CoreDepType > ThreadDepType > NoneDepType.
> +
> +  @param[in]  BeforeDep   Before dependence type.
> +  @param[in]  AfterDepAfter dependence type.
> +  @param[in]  NoneNeibBeforeDep   Before dependence type for not
> neighborhood features.
> +  @param[in]  NoneNeibAfterDepAfter dependence type for not
> neighborhood features.
> +
> +  @retval  Return the biggest dependence type.
> +**/
> +CPU_FEATURE_DEPENDENCE_TYPE
> +BiggestDep (
> +  IN CPU_FEATURE_DEPENDENCE_TYPE  BeforeDep,
> +  IN CPU_FEATURE_DEPENDENCE_TYPE  AfterDep,
> +  IN CPU_FEATURE_DEPENDENCE_TYPE  NoneNeibBeforeDep,
> +  IN CPU_FEATURE_DEPENDENCE_TYPE  NoneNeibAfterDep
> +  )
> +{
> +  return MAX(MAX (MAX(BeforeDep, AfterDep), NoneNeibBeforeDep),
> NoneNeibAfterDep);
> +}
> +
>  /**
>Analysis register CPU features on each processor and save CPU setting in 
> CPU
> register table.
> 
> @@ -558,6 +580,8 @@ AnalysisProcessorFeatures (
>BOOLEAN  Success;
>CPU_FEATURE_DEPENDENCE_TYPE  BeforeDep;
>CPU_FEATURE_DEPENDENCE_TYPE  AfterDep;
> +  CPU_FEATURE_DEPENDENCE_TYPE  NoneNeibBeforeDep;
> +  CPU_FEATURE_DEPENDENCE_TYPE  NoneNeibAfterDep;
> 
>CpuFeaturesData = GetCpuFeaturesData ();
>CpuFeaturesData->CapabilityPcd = AllocatePool (CpuFeaturesData-
> >BitMaskSize);
> @@ -634,14 +658,9 @@ AnalysisProcessorFeatures (
>  //
>  CpuInfo = >InitOrder[ProcessorNumber].CpuInfo;
>  Entry = GetFirstNode (>OrderList);
> -NextEntry = Entry->ForwardLink;
>  while (!IsNull (>OrderList, Entry)) {
>CpuFeatureInOrder = CPU_FEATURE_ENTRY_FROM_LINK (Entry);
> -  if (!IsNull (>OrderList, NextEntry)) {
> -NextCpuFeatureInOrder = CPU_FEATURE_ENTRY_FROM_LINK (NextEntry);
> -  } else {
> -NextCpuFeatureInOrder = NULL;
> -  }
> +
>Success = FALSE;
>if (IsBitMaskMatch (CpuFeatureInOrder->FeatureMask, CpuFeaturesData-
> >SettingPcd)) {
>  Status = CpuFeatureInOrder->InitializeFunc (ProcessorNumber, CpuInfo,
> CpuFeatureInOrder->ConfigData, TRUE);
> @@ -674,31 +693,43 @@ AnalysisProcessorFeatures (
>}
> 
>if (Success) {
> -//
> -// If feature has dependence with the next feature (ONLY care
> core/package dependency).
> -// and feature initialize succeed, add sync semaphere here.
> -//
> -if (NextCpuFeatureInOrder != NULL) {
> +NextEntry = Entry->ForwardLink;
> +if (!IsNull (>OrderList, NextEntry)) {
> +  NextCpuFeatureInOrder = CPU_FEATURE_ENTRY_FROM_LINK
> (NextEntry);
> +
> +  //
> +  // If feature has 

Re: [edk2] [Patch] SecurityPkg: Fix TPM device compatibility issue

[Zhang, Chao B]

Hi Leif:
   The NTC1310 can work well with previous EDK2 stable version (UDK2018). 
Interface Cache is a new feature introduced after UDK2018.
So far as we see, it causes NTC1310 fail to work.

From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Leif 
Lindholm
Sent: Friday, November 9, 2018 7:13 PM
To: Laszlo Ersek 
Cc: Kinney, Michael D ; edk2-devel@lists.01.org; 
Yao, Jiewen ; Zhang, Chao B 
Subject: Re: [edk2] [Patch] SecurityPkg: Fix TPM device compatibility issue

On Fri, Nov 09, 2018 at 09:04:46AM +0100, Laszlo Ersek wrote:
> On 11/09/18 07:02, Zhang, Chao B wrote:
> > Issue Statement:
> > TPM InterfaceId cache feature is introduced by 
> > f15cb995bb3880b77e15afe6facd3da05e599a17. It follows TCG PTP spec 1.3
> > to improve TPM transmission performance and also addresses defects in some 
> > TPM2.0 devices. But some other TPM devices like
> > NTC1310 SPI TPM is found function abnormally with this feature, causing 
> > extra device compatibility issue.
> >
> > Solution:
> > Add a policy indicator in PcdActiveTpmInterfaceType to disable TPM 
> > interface ID cache to support those existing TPM devices
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Zhang, Chao B 
> > mailto:chao.b.zh...@intel.com>>
> > Cc: Andrew Fish mailto:af...@apple.com>>
> > Cc: Laszlo Ersek mailto:ler...@redhat.com>>
> > Cc: Leif Lindholm 
> > mailto:leif.lindh...@linaro.org>>
> > Cc: Michael D Kinney 
> > mailto:michael.d.kin...@intel.com>>
> > Cc: Yao Jiewen mailto:jiewen@intel.com>>
> > ---
> >  SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c | 23 +++-
> >  SecurityPkg/SecurityPkg.dec |  3 +-
> >  SecurityPkg/SecurityPkg.uni |  3 +-
> >  SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigImpl.c | 49 
> > +
> >  SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c   | 42 +
> >  5 files changed, 117 insertions(+), 3 deletions(-)
>
> I'll let others review this patch for technical merit.
>
> However, I'm really undecided whether this patch qualifies for being
> pushed during the hard feature freeze. Comments welcome.

Unless the current behaviour causes an absolutely horrendous security
hole, I don't see how this qualifies for pushing during hard freeze.

According to its description, this is about supporting (non-compliant)
devices that have never worked with EDK2. And the support it updates
went in on 25 June. So there does not appear to be any urgency.

Once it does go in, I would also appreciate some simplification via
macros to cut down some of those very long lines, but then I'm not the
maintainer of this package.

Regards,

Leif
___
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] SecurityPkg: Fix TPM device compatibility issue

[Zhang, Chao B]

Hi All:
  Let me introduce more background.
   We enabled  Interface type Cache feature because it complies with TCG PTP 
1.03 spec (also earlier PTP 00.43) and reduces traffic to communicate with TPM.
It by chance addresses defect within some TPM2.0 device that frequent touching 
InterfaceID register could cause permanent damage.
   But in our recent test, we found other device compatibility issue. Using 
interface cache and skipping touching real hardware will cause NTC 1310 TPM 2.0 
malfunction.
In conclusion, I think our Interface Type Cache feature is the right direction, 
but with the intention to keep device compatibility, we still need to expose 
enable/disable configuration.


From: Laszlo Ersek [mailto:ler...@redhat.com]
Sent: Friday, November 9, 2018 4:05 PM
To: Zhang, Chao B ; edk2-devel@lists.01.org
Cc: Andrew Fish ; Leif Lindholm ; 
Kinney, Michael D ; Yao, Jiewen 

Subject: Re: [Patch] SecurityPkg: Fix TPM device compatibility issue

On 11/09/18 07:02, Zhang, Chao B wrote:
> Issue Statement:
> TPM InterfaceId cache feature is introduced by 
> f15cb995bb3880b77e15afe6facd3da05e599a17. It follows TCG PTP spec 1.3
> to improve TPM transmission performance and also addresses defects in some 
> TPM2.0 devices. But some other TPM devices like
> NTC1310 SPI TPM is found function abnormally with this feature, causing extra 
> device compatibility issue.
>
> Solution:
> Add a policy indicator in PcdActiveTpmInterfaceType to disable TPM interface 
> ID cache to support those existing TPM devices
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Zhang, Chao B 
> mailto:chao.b.zh...@intel.com>>
> Cc: Andrew Fish mailto:af...@apple.com>>
> Cc: Laszlo Ersek mailto:ler...@redhat.com>>
> Cc: Leif Lindholm mailto:leif.lindh...@linaro.org>>
> Cc: Michael D Kinney 
> mailto:michael.d.kin...@intel.com>>
> Cc: Yao Jiewen mailto:jiewen@intel.com>>
> ---
>  SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c | 23 +++-
>  SecurityPkg/SecurityPkg.dec |  3 +-
>  SecurityPkg/SecurityPkg.uni |  3 +-
>  SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigImpl.c | 49 
> +
>  SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c   | 42 +
>  5 files changed, 117 insertions(+), 3 deletions(-)

I'll let others review this patch for technical merit.

However, I'm really undecided whether this patch qualifies for being
pushed during the hard feature freeze. Comments welcome.

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

[edk2] Edk2 2019 Q1 stable tag planning is added into EDK-II-Release-Planning wiki

[Gao, Liming]

Hi, all
  Next edk2 stable tag planning has been added into EDK-II-Release-Planning 
wiki 
https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Release-Planning. 
Please review the proposed schedule. The proposed features are not finalized. 
If you know any feature will be ready, please propose to add it into the 
feature scope.
edk2-stable201903 tag planning
Proposed Schedule
Date

Description

2018-11-15

Beginning of development

2019-02-22

Soft Feature 
Freeze

2019-03-01

Hard Feature 
Freeze

2019-03-08

Release

Proposed Features

  *   Python 3 migration
  *   Add MSFT toolchain support to 
BaseStackCheckLib
  *   BaseTool Suggestions for improving building 
performance
  *   Delete IPv4 only TCP/iSCSI/PXE drivers in 
MdeModulePkg
  *   Remove EdkShellPkg from 
edk2/master
  *   Remove EdkShellBinPkg from 
edk2/master
  *   BaseTools: Support Array and C code style initialization in Structure 
PCD
  *   TBD Bugzilla List

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

Re: [edk2] [Patch] BaseTools: Optimize string concatenation

[Gao, Liming]

Mike:
  This patch bases on edk2 master with Python27. Seemly, most people are not 
aware that Python3 migration 
(https://bugzilla.tianocore.org/show_bug.cgi?id=55) is added for next 
edk2-stable201903 tag planning. In BZ 55, I propose to keep Python2 and Python3 
both, and new feature and patches are created based on Python3. I will send the 
mail to collect the feedback on this approach. 

Thanks
Liming
> -Original Message-
> From: Kinney, Michael D
> Sent: Friday, November 9, 2018 12:41 AM
> To: Feng, Bob C ; edk2-devel@lists.01.org; Kinney, 
> Michael D 
> Cc: Carsey, Jaben ; Gao, Liming 
> Subject: RE: [edk2] [Patch] BaseTools: Optimize string concatenation
> 
> Bob,
> 
> Is this for edk2/master or for the Python 3 conversion in the
> edk2-staging branch?  If it is for the edk-staging branch, then
> the Subject is not correct.
> 
> Thanks,
> 
> Mike
> 
> > -Original Message-
> > From: edk2-devel [mailto:edk2-devel-
> > boun...@lists.01.org] On Behalf Of BobCF
> > Sent: Thursday, November 8, 2018 2:16 AM
> > To: edk2-devel@lists.01.org
> > Cc: Carsey, Jaben ; Gao, Liming
> > 
> > Subject: [edk2] [Patch] BaseTools: Optimize string
> > concatenation
> >
> > https://bugzilla.tianocore.org/show_bug.cgi?id=1288
> >
> > This patch is one of build tool performance improvement
> > series patches.
> >
> > This patch is going to use join function instead of
> > string += string2 statement.
> >
> > Current code use string += string2 in a loop to combine
> > a string. while creating a string list in a loop and
> > using
> > "".join(stringlist) after the loop will be much faster.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: BobCF 
> > Cc: Liming Gao 
> > Cc: Jaben Carsey 
> > ---
> >  BaseTools/Source/Python/AutoGen/StrGather.py  | 39
> > +--
> >  BaseTools/Source/Python/Common/Misc.py| 21
> > +-
> >  .../Source/Python/Workspace/InfBuildData.py   |  4 +-
> >  .../Python/Workspace/WorkspaceCommon.py   | 11 ++-
> > ---
> >  4 files changed, 44 insertions(+), 31 deletions(-)
> >
> > diff --git
> > a/BaseTools/Source/Python/AutoGen/StrGather.py
> > b/BaseTools/Source/Python/AutoGen/StrGather.py
> > index 361d499076..d34a9e9447 100644
> > --- a/BaseTools/Source/Python/AutoGen/StrGather.py
> > +++ b/BaseTools/Source/Python/AutoGen/StrGather.py
> > @@ -135,11 +135,11 @@ def AscToHexList(Ascii):
> >  # @param UniGenCFlag  UniString is generated into
> > AutoGen C file when it is set to True
> >  #
> >  # @retval Str:   A string of .h file content
> >  #
> >  def CreateHFileContent(BaseName, UniObjectClass,
> > IsCompatibleMode, UniGenCFlag):
> > -Str = ''
> > +Str = []
> >  ValueStartPtr = 60
> >  Line = COMMENT_DEFINE_STR + ' ' +
> > LANGUAGE_NAME_STRING_NAME + ' ' * (ValueStartPtr -
> > len(DEFINE_STR + LANGUAGE_NAME_STRING_NAME)) +
> > DecToHexStr(0, 4) + COMMENT_NOT_REFERENCED
> >  Str = WriteLine(Str, Line)
> >  Line = COMMENT_DEFINE_STR + ' ' +
> > PRINTABLE_LANGUAGE_NAME_STRING_NAME + ' ' *
> > (ValueStartPtr - len(DEFINE_STR +
> > PRINTABLE_LANGUAGE_NAME_STRING_NAME)) + DecToHexStr(1,
> > 4) + COMMENT_NOT_REFERENCED
> >  Str = WriteLine(Str, Line)
> > @@ -164,16 +164,16 @@ def CreateHFileContent(BaseName,
> > UniObjectClass, IsCompatibleMode, UniGenCFlag):
> >  Line = COMMENT_DEFINE_STR + ' ' +
> > Name + ' ' + DecToHexStr(Token, 4) +
> > COMMENT_NOT_REFERENCED
> >  else:
> >  Line = COMMENT_DEFINE_STR + ' ' +
> > Name + ' ' * (ValueStartPtr - len(DEFINE_STR + Name)) +
> > DecToHexStr(Token, 4) + COMMENT_NOT_REFERENCED
> >  UnusedStr = WriteLine(UnusedStr, Line)
> >
> > -Str = ''.join([Str, UnusedStr])
> > +Str.extend( UnusedStr)
> >
> >  Str = WriteLine(Str, '')
> >  if IsCompatibleMode or UniGenCFlag:
> >  Str = WriteLine(Str, 'extern unsigned char ' +
> > BaseName + 'Strings[];')
> > -return Str
> > +return "".join(Str)
> >
> >  ## Create a complete .h file
> >  #
> >  # Create a complet .h file with file header and file
> > content
> >  #
> > @@ -185,11 +185,11 @@ def CreateHFileContent(BaseName,
> > UniObjectClass, IsCompatibleMode, UniGenCFlag):
> >  # @retval Str:   A string of complete .h file
> >  #
> >  def CreateHFile(BaseName, UniObjectClass,
> > IsCompatibleMode, UniGenCFlag):
> >  HFile = WriteLine('', CreateHFileContent(BaseName,
> > UniObjectClass, IsCompatibleMode, UniGenCFlag))
> >
> > -return HFile
> > +return "".join(HFile)
> >
> >  ## Create a buffer to store all items in an array
> >  #
> >  # @param BinBuffer   Buffer to contain Binary data.
> >  # @param Array:  The array need to be formatted
> > @@ -209,11 +209,11 @@ def CreateBinBuffer(BinBuffer,
> > Array):
> >  #
> >  def CreateArrayItem(Array, Width = 16):
> >  MaxLength = Width
> >  Index = 0
> >  Line = '  '
> > -ArrayItem = ''
> > +ArrayItem = []
> >
> >  

Re: [edk2] [PATCH] ArmPkg/ArmScmiDxe: Add clock enable function

[Leif Lindholm]

Hi Jeff,

On Thu, Nov 08, 2018 at 10:50:44AM -0700, Jeff Brasen wrote:
> Add function to allow enabling and disabling of the clock using the SCMI
> interface. Update the protocol GUID as the protocol interface has
> changed.

Changing a protocol GUID for an interface update feels a bit
un-idiomatic for tianocore. (Generally, a new version of the protocol
is added.)

My main concern is that I can't see how removing the ability to
discover the protocol by the old GUID could ever have a desirable
outcome.

Girish, Sami, what's your take?

> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Jeff Brasen 
> Cc: Ard Biesheuvel 
> Cc: Leif Lindholm 
> Cc: Grish Pathak 
> ---
>  ArmPkg/ArmPkg.dec  |  2 +-
>  .../ArmScmiDxe/ArmScmiClockProtocolPrivate.h   |  7 +++
>  ArmPkg/Drivers/ArmScmiDxe/ScmiClockProtocol.c  | 50 
> +-
>  ArmPkg/Include/Protocol/ArmScmiClockProtocol.h | 21 -
>  4 files changed, 77 insertions(+), 3 deletions(-)

Could you make sure you follow
https://github.com/tianocore/tianocore.github.io/wiki/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers#contrib-23
when generating patches (or let os know if you are, and we have more
git breakage)?

/
Leif

> diff --git a/ArmPkg/ArmPkg.dec b/ArmPkg/ArmPkg.dec
> index 84e57a0..a7b55a1 100644
> --- a/ArmPkg/ArmPkg.dec
> +++ b/ArmPkg/ArmPkg.dec
> @@ -57,7 +57,7 @@
>  
>## Arm System Control and Management Interface(SCMI) Clock management 
> protocol
>## ArmPkg/Include/Protocol/ArmScmiClockProtocol.h
> -  gArmScmiClockProtocolGuid = { 0x91ce67a8, 0xe0aa, 0x4012, { 0xb9, 0x9f, 
> 0xb6, 0xfc, 0xf3, 0x4, 0x8e, 0xaa } }
> +  gArmScmiClockProtocolGuid = { 0xb8d8caf2, 0x9e94, 0x462c, { 0xa8, 0x34, 
> 0x6c, 0x99, 0xfc, 0x05, 0xef, 0xcf } }
>  
>## Arm System Control and Management Interface(SCMI) Clock management 
> protocol
>## ArmPkg/Include/Protocol/ArmScmiPerformanceProtocol.h
> diff --git a/ArmPkg/Drivers/ArmScmiDxe/ArmScmiClockProtocolPrivate.h 
> b/ArmPkg/Drivers/ArmScmiDxe/ArmScmiClockProtocolPrivate.h
> index 0d1ec6f..c135bac 100644
> --- a/ArmPkg/Drivers/ArmScmiDxe/ArmScmiClockProtocolPrivate.h
> +++ b/ArmPkg/Drivers/ArmScmiDxe/ArmScmiClockProtocolPrivate.h
> @@ -59,6 +59,13 @@ typedef struct {
>CLOCK_RATE_DWORD Rate;
>  } CLOCK_RATE_SET_ATTRIBUTES;
>  
> +
> +// Message parameters for CLOCK_CONFIG_SET command.
> +typedef struct {
> +  UINT32 ClockId;
> +  UINT32 Attributes;
> +} CLOCK_CONFIG_SET_ATTRIBUTES;
> +
>  //  if ClockAttr Bit[0] is set then clock device is enabled.
>  #define CLOCK_ENABLE_MASK 0x1
>  #define CLOCK_ENABLED(ClockAttr)  ((ClockAttr & CLOCK_ENABLE_MASK) == 1)
> diff --git a/ArmPkg/Drivers/ArmScmiDxe/ScmiClockProtocol.c 
> b/ArmPkg/Drivers/ArmScmiDxe/ScmiClockProtocol.c
> index 64d2afa..493eb45 100644
> --- a/ArmPkg/Drivers/ArmScmiDxe/ScmiClockProtocol.c
> +++ b/ArmPkg/Drivers/ArmScmiDxe/ScmiClockProtocol.c
> @@ -388,6 +388,53 @@ ClockRateSet (
>return Status;
>  }
>  
> +/** Enable/Disable specified clock.
> +
> +  @param[in]  ThisA Pointer to SCMI_CLOCK_PROTOCOL Instance.
> +  @param[in]  ClockId Identifier for the clock device.
> +  @param[in]  Enable  TRUE to enable, FALSE to disable.
> +
> +  @retval EFI_SUCCESS  Clock enable/disable successful.
> +  @retval EFI_DEVICE_ERROR SCP returns an SCMI error.
> +  @retval !(EFI_SUCCESS)   Other errors.
> +**/
> +STATIC
> +EFI_STATUS
> +ClockEnable (
> +  IN SCMI_CLOCK_PROTOCOL  *This,
> +  IN UINT32   ClockId,
> +  IN BOOLEAN  Enable
> +  )
> +{
> +  EFI_STATUS  Status;
> +  CLOCK_CONFIG_SET_ATTRIBUTES *ClockConfigSetAttributes;
> +  SCMI_COMMANDCmd;
> +  UINT32  PayloadLength;
> +
> +  Status = ScmiCommandGetPayload ((UINT32**));
> +  if (EFI_ERROR (Status)) {
> +return Status;
> +  }
> +
> +  // Fill arguments for clock protocol command.
> +  ClockConfigSetAttributes->ClockId= ClockId;
> +  ClockConfigSetAttributes->Attributes = Enable ? BIT0 : 0;
> +
> +  Cmd.ProtocolId = SCMI_PROTOCOL_ID_CLOCK;
> +  Cmd.MessageId  = SCMI_MESSAGE_ID_CLOCK_CONFIG_SET;
> +
> +  PayloadLength = sizeof (CLOCK_CONFIG_SET_ATTRIBUTES);
> +
> +  // Execute and wait for response on a SCMI channel.
> +  Status = ScmiCommandExecute (
> + ,
> + ,
> + NULL
> + );
> +
> +  return Status;
> +}
> +
>  // Instance of the SCMI clock management protocol.
>  STATIC CONST SCMI_CLOCK_PROTOCOL ScmiClockProtocol = {
>ClockGetVersion,
> @@ -395,7 +442,8 @@ STATIC CONST SCMI_CLOCK_PROTOCOL ScmiClockProtocol = {
>ClockGetClockAttributes,
>ClockDescribeRates,
>ClockRateGet,
> -  ClockRateSet
> +  ClockRateSet,
> +  ClockEnable
>   };
>  
>  /** Initialize clock management protocol and install protocol on a given 
> handle.
> diff --git a/ArmPkg/Include/Protocol/ArmScmiClockProtocol.h 
> 

Re: [edk2] [PATCH v2 2/2] Edk2Platforms: Replace MdeModulePkg PXE/iSCSI/TCP with NetworkPkg drivers.

[Thomas Abraham]

On Wed, Nov 7, 2018 at 1:55 PM Leif Lindholm  wrote:
>
> Hi Fu Siyan,
>
> On Wed, Nov 07, 2018 at 08:12:55AM +, Fu, Siyuan wrote:
> > Hi, Leif and Ard
> >
> > I just realized that you may not be CCed in this email. I resent these 
> > patches a few minutes ago, my Git Bash send-email reports it added you to 
> > CC receiver, but the Outlook received email still doesn't have your name in 
> > CC list.
> >
> > Do you have any comments for this v2 patch of the edk2platforms?
> >
> >
> > BestRegards
> > Fu Siyuan
> >
> > > -Original Message-
> > > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Fu
> > > Siyuan
> > > Sent: Monday, November 5, 2018 9:33 AM
> > > To: edk2-devel@lists.01.org
> > > Cc: Kinney, Michael D 
> > > Subject: [edk2] [PATCH v2 2/2] Edk2Platforms: Replace MdeModulePkg
> > > PXE/iSCSI/TCP with NetworkPkg drivers.
> > >
> > > V2:
> > > Additional fixups required for NetworkPkg migration
>
> Revision history like this belongs in the cover letter.
>
> > > The PXE/iSCSI/TCP drivers in MdeModulePkg are going to be deprecated. All
> > > platform DSC/FDF files should be updated to use the dual-stack drivers in
> > > NetworkPkg.
> > >
> > > The NetworkPkg driver have all the functionality compared with
> > > MdeModulePkg
> > > one, with more bug fixes and new feature added. While its image size will
> > > be a little bigger because it contains both IPv4 and IPv6 stack support,
> > > so it may cause build error in a platform if the flash space is very 
> > > tight.
> > > Basically, this patch won't cause any other problem if build could pass.
> > >
> > > I haven't built all the updated platform because the repo ReadMe doesn't
> > > provide a method to build them on Windows Environment, so I would very
> > > appreciate if anybody can help to test the build or tell me how to build
> > > it on Windows.
>
> And comments like the paragraph above belong in the cover letter or
> below the ---
> If you are OK with me deleting these bits before committing:
> Reviewed-by: Leif Lindholm 
>
> /
> Leif
>
> > >
> > > Cc: Ard Biesheuvel 
> > > Cc: Leif Lindholm 
> > > Cc: Michael D Kinney 
> > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > Signed-off-by: Fu Siyuan 
> > > Signed-off-by: Leif Lindholm 
> > > ---
> > >  Platform/AMD/OverdriveBoard/OverdriveBoard.dsc  |  6 +++---
> > >  Platform/AMD/OverdriveBoard/OverdriveBoard.fdf  |  6 +++---
> > >  Platform/ARM/SgiPkg/SgiPlatform.fdf |  6 +++---
> > >  Platform/ARM/VExpressPkg/ArmVExpress-CTA15-A7.dsc   |  1 +
> > >  Platform/ARM/VExpressPkg/ArmVExpress-networking.fdf.inc |  6 +++---
> > >  Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc| 13 +++-
> > > -
> > >  Platform/Comcast/RDKQemu/RDKQemu.dsc| 10 +++-
> > > --
> > >  Platform/Hisilicon/D03/D03.dsc  |  4 ++--
> > >  Platform/Hisilicon/D03/D03.fdf  |  4 ++--
> > >  Platform/Hisilicon/D05/D05.dsc  | 11 +++-
> > > ---
> > >  Platform/Hisilicon/D05/D05.fdf  |  9 +++-
> > > -
> > >  Platform/Hisilicon/D06/D06.dsc  | 11 +++-
> > > ---
> > >  Platform/Hisilicon/D06/D06.fdf  |  9 +++-
> > > -
> > >  Platform/Hisilicon/HiKey/HiKey.dsc  |  4 ++--
> > >  Platform/Hisilicon/HiKey/HiKey.fdf  |  4 ++--
> > >  Platform/Hisilicon/HiKey960/HiKey960.dsc|  4 ++--
> > >  Platform/Hisilicon/HiKey960/HiKey960.fdf|  4 ++--
> > >  Platform/LeMaker/CelloBoard/CelloBoard.dsc  | 11
> > > ---
> > >  Platform/LeMaker/CelloBoard/CelloBoard.fdf  |  6 +++---
> > >  Platform/SoftIron/Overdrive1000Board/Overdrive1000Board.dsc |  6 +++---
> > >  Platform/SoftIron/Overdrive1000Board/Overdrive1000Board.fdf |  6 +++---
> > >  Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc   |  4 ++--
> > >  Silicon/Marvell/Armada7k8k/Armada7k8k.fdf   |  4 ++--
> > >  23 files changed, 68 insertions(+), 81 deletions(-)
> > >

[...]

> > >#
> > ># Core Info
> > > diff --git a/Platform/ARM/SgiPkg/SgiPlatform.fdf
> > > b/Platform/ARM/SgiPkg/SgiPlatform.fdf
> > > index fd87563246..c9129841d7 100644
> > > --- a/Platform/ARM/SgiPkg/SgiPlatform.fdf
> > > +++ b/Platform/ARM/SgiPkg/SgiPlatform.fdf
> > > @@ -185,10 +185,10 @@ READ_LOCK_STATUS   = TRUE
> > >INF MdeModulePkg/Universal/Network/MnpDxe/MnpDxe.inf
> > >INF MdeModulePkg/Universal/Network/VlanConfigDxe/VlanConfigDxe.inf
> > >INF MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Dxe.inf
> > > -  INF MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Dxe.inf
> > >INF MdeModulePkg/Universal/Network/Udp4Dxe/Udp4Dxe.inf
> > > -  INF MdeModulePkg/Universal/Network/UefiPxeBcDxe/UefiPxeBcDxe.inf
> > > -  INF 

Re: [edk2] [PATCH v2 7/7] ArmPkg: Extra action to update permissions for S-ELO MM Image

[Achin Gupta]

Hi Ard,

Just a polite poke that Sughosh had posted the patches as I had described below
here [1] & here [2]. Please let us know what you think.

cheers,
Achin

[1] https://lists.01.org/pipermail/edk2-devel/2018-October/031377.html
[2] https://lists.01.org/pipermail/edk2-devel/2018-October/031384.html

On Wed, Oct 24, 2018 at 08:05:22AM -0300, Ard Biesheuvel wrote:
> On 24 October 2018 at 05:22, Achin Gupta  wrote:
> > Hi Ard,
> >
> > Please see CIL..
> >
> 
> FYI I will be on leave until 5th of November, so I will get to this
> once I get back.
> 
> -- 
> Ard.
> 
> > On Fri, Aug 24, 2018 at 03:55:29PM +0100, Ard Biesheuvel wrote:
> >> On 21 August 2018 at 07:50, Sughosh Ganu  wrote:
> >> > hi Ard,
> >> >
> >> > On Tue July 23, 2018 at 11:03PM +0530, Supreeth Venkatesh wrote:
> >> >>
> >> >> On Sat, 2018-07-21 at 20:06 +0900, Ard Biesheuvel wrote:
> >> >> > On 20 July 2018 at 21:38, Sughosh Ganu  wrote:
> >> >> > >
> >> >> > > From: Achin Gupta 
> >> >> > >
> >> >> > > The Standalone MM drivers runs in S-EL0 in AArch64 on ARM Standard
> >> >> > > Platforms and is deployed during SEC phase. The memory allocated to
> >> >> > > the Standalone MM drivers should be marked as RO+X.
> >> >> > >
> >> >> > > During PE/COFF Image section parsing, this patch implements extra
> >> >> > > action "UpdatePeCoffPermissions" to request the privileged firmware
> >> >> > > in
> >> >> > > EL3 to update the permissions.
> >> >> > >
> >> >> > > Contributed-under: TianoCore Contribution Agreement 1.1
> >> >> > > Signed-off-by: Sughosh Ganu 
> >> >> > Apologies for bringing this up only now, but I don't think I was ever
> >> >> > cc'ed on these patches.
> >> >> >
> >> >> Apologies if you have missed it. But I am pretty sure it was part of
> >> >> earlier large patch-set on which you and leif were copied, as it was
> >> >> part of ArmPkg.
> >> >> >
> >> >> > We are relying on a debug hook in the PE/COFF loader to ensure that
> >> >> > we
> >> >> > don't end up with memory that is both writable and executable in the
> >> >> > secure world. Do we really think that is a good idea?
> >> >> >
> >> >> > (I know this code was derived from a proof of concept that I did
> >> >> > years
> >> >> > ago, but that was just a PoC)
> >> >> I think we need a little bit more details on what is your suggestion?
> >> >>
> >> >> A little bit background here: This code runs in S-EL0 and Request gets
> >> >> sent to secure world SPM to ensure that the region permissions are
> >> >> updated correctly via the "ArmMmuStandaloneMmCoreLib" SVC -
> >> >> ARM_SVC_ID_SP_SET_MEM_ATTRIBUTES_AARCH64.
> >> >>
> >> >> DebugPeCoffExtraActionLib is just used to extract image region
> >> >> information, but the region permission
> >> >> update request is sent to secure world for validation.
> >> >>
> >> >> With the above explanation, can you provide an insight into what was
> >> >> your thinking?
> >> >> Do you want us to create a separate library and call it
> >> >> as PeCoffExtraActionLib to avoid the "Debug" word though it is a hook
> >> >> to PeCoffExtraActionLib in MdePkg or do we want to create this library
> >> >> in a separate package (may be in MdePkg?) or something totally
> >> >> different.
> >> >
> >> > Supreeth had replied to your comments on the patch. Can you please
> >> > check this. If you feel that this needs to be implemented differently,
> >> > can you please suggest it to us. Thanks.
> >> >
> >>
> >> My point is that such a fundamental action that needs to occur while
> >> loading the PE/COFF image should not be hooked into the loader this
> >> way.
> >
> > Based upon our discussion at the Linaro Connect, we investigated leveraging 
> > the
> > DXE Image Protection support [1] in Standalone MM (StMM). Amongst other
> > challenges, there is a chicken and egg problem. I will try and explain.
> >
> > DXE Memory protection has dependencies that cannot be fulfilled in StMM. A
> > non-exhaustive list is:
> >
> > 1. Dependency on CPU_ARCH protocol
> > 2. Dependency on Loaded Image patch protocol
> > 3. Dependency on Boot services
> >
> > There is an inherent assumption that this support will never be used in
> > SMM. Furthermore, in StMM, permissions are changed when the StMM drivers are
> > first dispatched. A dependency on a driver to change the permissions is the
> > chicken and egg. So we need a library.
> >
> > One option is to introduce a memory protection library in StMM i.e. a 
> > library
> > interface like StandaloneMmImageProtect(). This function will be called from
> > generic code after the PE-COFF loader has loaded and relocated the StMM 
> > driver
> > image. However, this support is not required on x86. They will have to 
> > include a
> > NULL library implementation. This would be in addition to the NULL
> > PeCoffExtraActionLib they already include through MdePkg.dsc.
> >
> > I am hesitant to take this approach in the absence of a requirement on x86. 
> > At
> > the same time, the current approach of leveraging the 
> > 

Re: [edk2] [PATCH v2 edk2-platforms] Silicon/SynQuacerPciCpuIo2Dxe: fix PCIe I/O translation

[Leif Lindholm]

On Fri, Nov 09, 2018 at 08:58:48AM +0100, Ard Biesheuvel wrote:
> Commit 9dd8190e4995 ("Silicon/SynQuacer: tweak PCI I/O windows for
> ACPI/Linux support") updated the min/max/offset definitions for the
> PCIe I/O resource windows on SynQuacer, and updated the read path of
> the platform's EfiCpuIo2 protocol implementation, but failed to update
> the write path as well, resulting in spurious errors if when attempting
> to write to PCIe I/O ports on PCIe RC #1, which uses translation for the
> I/O BAR window.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 

Much neater, thank you.
Reviewed-by: Leif Lindholm 

> ---
> v2: use helper function and temp vars
> 
>  
> Silicon/Socionext/SynQuacer/Drivers/SynQuacerPciCpuIo2Dxe/SynQuacerPciCpuIo2Dxe.c
>  | 62 
>  1 file changed, 37 insertions(+), 26 deletions(-)
> 
> diff --git 
> a/Silicon/Socionext/SynQuacer/Drivers/SynQuacerPciCpuIo2Dxe/SynQuacerPciCpuIo2Dxe.c
>  
> b/Silicon/Socionext/SynQuacer/Drivers/SynQuacerPciCpuIo2Dxe/SynQuacerPciCpuIo2Dxe.c
> index 736b20cd5129..049657231cab 100644
> --- 
> a/Silicon/Socionext/SynQuacer/Drivers/SynQuacerPciCpuIo2Dxe/SynQuacerPciCpuIo2Dxe.c
> +++ 
> b/Silicon/Socionext/SynQuacer/Drivers/SynQuacerPciCpuIo2Dxe/SynQuacerPciCpuIo2Dxe.c
> @@ -354,6 +354,37 @@ CpuMemoryServiceWrite (
>return EFI_SUCCESS;
>  }
>  
> +STATIC
> +EFI_STATUS
> +TranslateIoAddress (
> +  IN  OUT UINT64 *Address
> +  )
> +{
> +  UINT64 Start;
> +  UINT64 End;
> +  UINT64 Shift;
> +
> +  Start = SYNQUACER_PCI_SEG0_PORTIO_MIN + SYNQUACER_PCI_SEG0_PORTIO_OFFSET;
> +  End   = SYNQUACER_PCI_SEG0_PORTIO_MAX + SYNQUACER_PCI_SEG0_PORTIO_OFFSET;
> +  Shift = SYNQUACER_PCI_SEG0_PORTIO_MEMBASE - 
> SYNQUACER_PCI_SEG0_PORTIO_OFFSET;
> +
> +  if (*Address >= Start && *Address <= End) {
> +*Address += Shift;
> +return EFI_SUCCESS;
> +  }
> +
> +  Start = SYNQUACER_PCI_SEG1_PORTIO_MIN + SYNQUACER_PCI_SEG1_PORTIO_OFFSET;
> +  End   = SYNQUACER_PCI_SEG1_PORTIO_MAX + SYNQUACER_PCI_SEG1_PORTIO_OFFSET;
> +  Shift = SYNQUACER_PCI_SEG1_PORTIO_MEMBASE - 
> SYNQUACER_PCI_SEG1_PORTIO_OFFSET;
> +
> +  if (*Address >= Start && *Address <= End) {
> +*Address += Shift;
> +return EFI_SUCCESS;
> +  }
> +  ASSERT (FALSE);
> +  return EFI_INVALID_PARAMETER;
> +}
> +
>  /**
>Reads I/O registers.
>  
> @@ -415,22 +445,9 @@ CpuIoServiceRead (
>  return Status;
>}
>  
> -  if ((Address >= (SYNQUACER_PCI_SEG0_PORTIO_MIN +
> -   SYNQUACER_PCI_SEG0_PORTIO_OFFSET)) &&
> -  (Address <= (SYNQUACER_PCI_SEG0_PORTIO_MAX +
> -   SYNQUACER_PCI_SEG0_PORTIO_OFFSET))) {
> -Address += SYNQUACER_PCI_SEG0_PORTIO_MEMBASE -
> -   SYNQUACER_PCI_SEG0_PORTIO_OFFSET;
> -  } else if ((Address >= (SYNQUACER_PCI_SEG1_PORTIO_MIN +
> -  SYNQUACER_PCI_SEG1_PORTIO_OFFSET)) &&
> - (Address <= (SYNQUACER_PCI_SEG1_PORTIO_MAX +
> -  SYNQUACER_PCI_SEG1_PORTIO_OFFSET))) {
> -Address += SYNQUACER_PCI_SEG1_PORTIO_MEMBASE -
> -   SYNQUACER_PCI_SEG1_PORTIO_OFFSET;
> -
> -  } else {
> -ASSERT (FALSE);
> -return EFI_INVALID_PARAMETER;
> +  Status = TranslateIoAddress ();
> +  if (EFI_ERROR (Status)) {
> +return Status;
>}
>  
>//
> @@ -518,16 +535,9 @@ CpuIoServiceWrite (
>  return Status;
>}
>  
> -  if ((Address >= SYNQUACER_PCI_SEG0_PORTIO_MIN) &&
> -  (Address <= SYNQUACER_PCI_SEG0_PORTIO_MAX)) {
> -Address += SYNQUACER_PCI_SEG0_PORTIO_MEMBASE;
> -  } else if ((Address >= SYNQUACER_PCI_SEG1_PORTIO_MIN) &&
> - (Address <= SYNQUACER_PCI_SEG1_PORTIO_MAX)) {
> -Address += SYNQUACER_PCI_SEG1_PORTIO_MEMBASE;
> -
> -  } else {
> -ASSERT (FALSE);
> -return EFI_INVALID_PARAMETER;
> +  Status = TranslateIoAddress ();
> +  if (EFI_ERROR (Status)) {
> +return Status;
>}
>  
>//
> -- 
> 2.19.1
> 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [Patch] BaseTools: Optimize string concatenation

[Leif Lindholm]

Hi Bob,

On Fri, Nov 09, 2018 at 03:25:04AM +, Feng, Bob C wrote:
> Yes. I should show the data.
> 
> My unites scripts is as below. The parameter lines is a string list which 
> size is 43395. The test result is
> 
> ''.join(String list) time:  0.042262
> String += String time  :  3.822699
> 
> def TestPlus(lines):
> str_target = ""
> 
> for line in lines:
> str_target += line
> 
> return str_target
> 
> def TestJoin(lines):
> str_target = []
> 
> for line in lines:
> str_target.append(line)
> 
> return "".join(str_target)
> 
> def CompareStrCat():
> lines = GetStrings()
> print (len(lines))
> 
> begin = time.perf_counter()
> for _ in range(10):
> TestJoin(lines)
> end = time.perf_counter() - begin
> print ("''.join(String list) time: %f" % end)
> 
> begin = time.perf_counter()
> for _ in range(10):
> TestPlus(lines)
> end = time.perf_counter() - begin
> print ("String += String time: %f" % end)
> 
> For build OvmfX64, it's not very effective, it saves 2~3 second in
> Parse/AutoGen phase, because OvmfX64 is relatively simple. It does
> not enable much features such as Multiple SKU and structure PCD by
> default and there is no big size Autogen.c/Autogen.h/Makefile
> generated either. but for the complex platform, this patch will be
> much effective. The unites above simulates a real case that there is
> a 43395 lines of Autogen.c generated.

I beg to differ. Shaving 2-3 seconds off the autogen phase is a
substantial improvement.

However, on my wimpy 24-core Cortex-A53 system, the effect is not
noticeable (fluctuates between 1:56-1:58 whether with or without this
patch).

And even on my x86 workstation, I see no measurable difference (12-13s).
What is the hardware you are benchmarking on?

It does not appear to be detrimental to performance on any of my
platforms, but I would like to see some more measurements, and I would
like to see that logged with some more detail in bugzilla.

Regards,

Leif

> Since this patch mostly effect the Parser/AutoGen phase, I just use
> "build genmake" to show the improvement data.
> The final result for clean build is:
> Current code:  17 seconds
> After patch:  15 seconds
> 
> Details:
> Current data:
> 
> d:\edk2 (master -> origin)
> λ build genmake -p OvmfPkg\OvmfPkgIa32X64.dsc -a IA32 -a X64 -t VS2015x86
> Build environment: Windows-10-10.0.10240
> Build start time: 10:12:32, Nov.09 2018
> 
> WORKSPACE= d:\edk2
> ECP_SOURCE   = d:\edk2\edkcompatibilitypkg
> EDK_SOURCE   = d:\edk2\edkcompatibilitypkg
> EFI_SOURCE   = d:\edk2\edkcompatibilitypkg
> EDK_TOOLS_PATH   = d:\edk2\basetools
> EDK_TOOLS_BIN= d:\edk2\basetools\bin\win32
> CONF_PATH= d:\edk2\conf
> 
> Architecture(s)  = IA32 X64
> Build target = DEBUG
> Toolchain= VS2015x86
> 
> Active Platform  = d:\edk2\OvmfPkg\OvmfPkgIa32X64.dsc
> Flash Image Definition   = d:\edk2\OvmfPkg\OvmfPkgIa32X64.fdf
> 
> Processing meta-data ... done!
> Generating code . done!
> Generating makefile . done!
> Generating code .. done!
> Generating makefile .. done!
> 
> - Done -
> Build end time: 10:12:49, Nov.09 2018
> Build total time: 00:00:17
> 
> After applying this patch:
> 
> d:\edk2 (master -> origin)
> λ build genmake -p OvmfPkg\OvmfPkgIa32X64.dsc -a IA32 -a X64 -
> Build environment: Windows-10-10.0.10240  
> Build start time: 10:11:41, Nov.09 2018   
>   
> WORKSPACE= d:\edk2
> ECP_SOURCE   = d:\edk2\edkcompatibilitypkg
> EDK_SOURCE   = d:\edk2\edkcompatibilitypkg
> EFI_SOURCE   = d:\edk2\edkcompatibilitypkg
> EDK_TOOLS_PATH   = d:\edk2\basetools  
> EDK_TOOLS_BIN= d:\edk2\basetools\bin\win32
> CONF_PATH= d:\edk2\conf   
>   
>   
> Architecture(s)  = IA32 X64   
> Build target = DEBUG  
> Toolchain= VS2015x86  
>   
> Active Platform  = d:\edk2\OvmfPkg\OvmfPkgIa32X64.dsc 
> Flash Image Definition   = d:\edk2\OvmfPkg\OvmfPkgIa32X64.fdf 
>   
> Processing meta-data . done!  
> Generating code . done!   
> Generating makefile . done!   
> Generating code .. done!  
> Generating makefile .. done!  
>  

Re: [edk2] [Patch] SecurityPkg: Fix TPM device compatibility issue

[Leif Lindholm]

On Fri, Nov 09, 2018 at 09:04:46AM +0100, Laszlo Ersek wrote:
> On 11/09/18 07:02, Zhang, Chao B wrote:
> > Issue Statement:
> > TPM InterfaceId cache feature is introduced by 
> > f15cb995bb3880b77e15afe6facd3da05e599a17. It follows TCG PTP spec 1.3
> > to improve TPM transmission performance and also addresses defects in some 
> > TPM2.0 devices. But some other TPM devices like
> > NTC1310 SPI TPM is found function abnormally with this feature, causing 
> > extra device compatibility issue.
> > 
> > Solution:
> > Add a policy indicator in PcdActiveTpmInterfaceType to disable TPM 
> > interface ID cache to support those existing TPM devices
> > 
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Zhang, Chao B 
> > Cc: Andrew Fish 
> > Cc: Laszlo Ersek 
> > Cc: Leif Lindholm 
> > Cc: Michael D Kinney 
> > Cc: Yao Jiewen 
> > ---
> >  SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c | 23 +++-
> >  SecurityPkg/SecurityPkg.dec |  3 +-
> >  SecurityPkg/SecurityPkg.uni |  3 +-
> >  SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigImpl.c | 49 
> > +
> >  SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c   | 42 +
> >  5 files changed, 117 insertions(+), 3 deletions(-)
> 
> I'll let others review this patch for technical merit.
> 
> However, I'm really undecided whether this patch qualifies for being
> pushed during the hard feature freeze. Comments welcome.

Unless the current behaviour causes an absolutely horrendous security
hole, I don't see how this qualifies for pushing during hard freeze.

According to its description, this is about supporting (non-compliant)
devices that have never worked with EDK2. And the support it updates
went in on 25 June. So there does not appear to be any urgency.

Once it does go in, I would also appreciate some simplification via
macros to cut down some of those very long lines, but then I'm not the
maintainer of this package.

Regards,

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

Re: [edk2] [patch] MdeModulePkg/DisplayEngine: Remove useless NULL ptr check for NewPos

[Laszlo Ersek]

On 11/09/18 11:49, Leif Lindholm wrote:
> On Fri, Nov 09, 2018 at 07:56:07AM +0100, Ard Biesheuvel wrote:
>> On 9 November 2018 at 01:19, Gao, Liming  wrote:
>>> Ard:
>>>   This is a small fix. And, this patch is sent before the hard
>>>   freeze. It is the low risk for this release. So, I push it.
>>
>> OK, fair enough.
> 
> I don't agree actually.
> 
> https://github.com/tianocore/tianocore.github.io/wiki/HardFeatureFreeze
> specifies clearly that only bug fixes are permitted in during hard
> freeze. Maybe we could document that a bit more explicitly, but this
> patch was no bugfix. It should not have gone in.
> 
> By my interpretation, it would not even fulfill the requirements for
> https://github.com/lersek/edk2/wiki/SoftFeatureFreeze:
> "By the date of the soft feature freeze, developers must have sent
> their patches to the mailing list and received positive maintainer
> reviews."
> Soft feature freeze was 1 November. The patch was sent out 7 November.
> It received reviews 8 November (after the start of the hard freeze).
> 
> The point of these freezes is that sometimes patches are wrong. And
> sometimes patches that look correct, are not correct. If we start
> making exceptions because "oh, it's trivial", that means we get these
> patches into the tree with much reduced time for anyone to catch any
> adverse effects before we make the stable tag. And at that point, the
> stable tag no longer has value.
> 
> (I am much more flexible on the topic of updating documentation, like
> Maintainers.txt, but even there we must be very careful.)

I haven't been following this specific patch, but now it does not look
like a bugfix to me. Without applying the patch, there is no bug
actually, functional or performance. The subject says, "Remove useless ...".

Optimizations, simplifications, refactorings, features, and so on, are
not bugfixes. They should not go in after the hard freeze. Even after
the soft freeze, they should only go in if the only remaining step is
the push (i.e. they should be ready for pushing before the soft freeze,
sufficiently reviewed.).

Thanks
Laszlo

 -Original Message-
 From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
 Sent: Friday, November 09, 2018 2:25 AM
 To: Zeng, Star 
 Cc: Bi, Dandan ; edk2-devel@lists.01.org; Wu, Hao A
 ; Dong, Eric ; Gao, Liming
 
 Subject: Re: [edk2] [patch] MdeModulePkg/DisplayEngine: Remove useless
 NULL ptr check for NewPos

 On 8 November 2018 at 02:09, Zeng, Star  wrote:
> Reviewed-by: Star Zeng 
>
> -Original Message-
> From: Bi, Dandan
> Sent: Wednesday, November 7, 2018 10:53 PM
> To: edk2-devel@lists.01.org
> Cc: Gao, Liming ; Dong, Eric ;
 Zeng, Star ; Wu, Hao A 
> Subject: [patch] MdeModulePkg/DisplayEngine: Remove useless NULL ptr
 check for NewPos
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1306
>
> In function UiDisplayMenu, the NewPos ptr which used to point to the
 highlight menu entry. It will always point to the menu entry which need to 
 be
 highlighted or the gMenuOption menu if the highlight menu is not found.
> So we can remove the NULL ptr check for NewPos in this function.
> And add the ASSERT code to avoid if any false positive reports of NULL
 pointer dereference issue raised from static analysis.
>
> Cc: Liming Gao 
> Cc: Eric Dong 
> Cc: Star Zeng 
> Cc: Hao Wu 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Dandan Bi 

 Why was this patch merged today? Surely, this doesn't meet the hard
 freeze requirements ?

> ---
>  MdeModulePkg/Universal/DisplayEngineDxe/FormDisplay.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/MdeModulePkg/Universal/DisplayEngineDxe/FormDisplay.c
 b/MdeModulePkg/Universal/DisplayEngineDxe/FormDisplay.c
> index 7390f954b6..44f087fe01 100644
> --- a/MdeModulePkg/Universal/DisplayEngineDxe/FormDisplay.c
> +++ b/MdeModulePkg/Universal/DisplayEngineDxe/FormDisplay.c
> @@ -2880,10 +2880,11 @@ UiDisplayMenu (
>// MenuOption is set to NULL in Repaint
>// NewPos: Current menu option that need to hilight
>//
>ControlFlag = CfUpdateHelpString;
>
> +  ASSERT (NewPos != NULL);
>UpdateHighlightMenuInfo(NewPos, TopOfScreen, SkipValue);
>
>if (SkipHighLight) {
>  SkipHighLight = FALSE;
>  MenuOption= SavedMenuOption;
> @@ -2908,11 +2909,11 @@ UiDisplayMenu (
>  Temp2 = SkipValue;
>} else {
>  Temp2 = 0;
>}
>
> -  if (NewPos != NULL && (MenuOption == NULL || NewPos !=
 >Link)) {
> +  if (MenuOption == NULL || NewPos != >Link) {
>  if (MenuOption != NULL) {
>//
>// Remove the old 

Re: [edk2] [patch] MdeModulePkg/DisplayEngine: Remove useless NULL ptr check for NewPos

[Leif Lindholm]

On Fri, Nov 09, 2018 at 07:56:07AM +0100, Ard Biesheuvel wrote:
> On 9 November 2018 at 01:19, Gao, Liming  wrote:
> > Ard:
> >   This is a small fix. And, this patch is sent before the hard
> >   freeze. It is the low risk for this release. So, I push it.
> 
> OK, fair enough.

I don't agree actually.

https://github.com/tianocore/tianocore.github.io/wiki/HardFeatureFreeze
specifies clearly that only bug fixes are permitted in during hard
freeze. Maybe we could document that a bit more explicitly, but this
patch was no bugfix. It should not have gone in.

By my interpretation, it would not even fulfill the requirements for
https://github.com/lersek/edk2/wiki/SoftFeatureFreeze:
"By the date of the soft feature freeze, developers must have sent
their patches to the mailing list and received positive maintainer
reviews."
Soft feature freeze was 1 November. The patch was sent out 7 November.
It received reviews 8 November (after the start of the hard freeze).

The point of these freezes is that sometimes patches are wrong. And
sometimes patches that look correct, are not correct. If we start
making exceptions because "oh, it's trivial", that means we get these
patches into the tree with much reduced time for anyone to catch any
adverse effects before we make the stable tag. And at that point, the
stable tag no longer has value.

(I am much more flexible on the topic of updating documentation, like
Maintainers.txt, but even there we must be very careful.)

Regards,

Leif

> >>-Original Message-
> >>From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> >>Sent: Friday, November 09, 2018 2:25 AM
> >>To: Zeng, Star 
> >>Cc: Bi, Dandan ; edk2-devel@lists.01.org; Wu, Hao A
> >>; Dong, Eric ; Gao, Liming
> >>
> >>Subject: Re: [edk2] [patch] MdeModulePkg/DisplayEngine: Remove useless
> >>NULL ptr check for NewPos
> >>
> >>On 8 November 2018 at 02:09, Zeng, Star  wrote:
> >>> Reviewed-by: Star Zeng 
> >>>
> >>> -Original Message-
> >>> From: Bi, Dandan
> >>> Sent: Wednesday, November 7, 2018 10:53 PM
> >>> To: edk2-devel@lists.01.org
> >>> Cc: Gao, Liming ; Dong, Eric ;
> >>Zeng, Star ; Wu, Hao A 
> >>> Subject: [patch] MdeModulePkg/DisplayEngine: Remove useless NULL ptr
> >>check for NewPos
> >>>
> >>> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1306
> >>>
> >>> In function UiDisplayMenu, the NewPos ptr which used to point to the
> >>highlight menu entry. It will always point to the menu entry which need to 
> >>be
> >>highlighted or the gMenuOption menu if the highlight menu is not found.
> >>> So we can remove the NULL ptr check for NewPos in this function.
> >>> And add the ASSERT code to avoid if any false positive reports of NULL
> >>pointer dereference issue raised from static analysis.
> >>>
> >>> Cc: Liming Gao 
> >>> Cc: Eric Dong 
> >>> Cc: Star Zeng 
> >>> Cc: Hao Wu 
> >>> Contributed-under: TianoCore Contribution Agreement 1.1
> >>> Signed-off-by: Dandan Bi 
> >>
> >>Why was this patch merged today? Surely, this doesn't meet the hard
> >>freeze requirements ?
> >>
> >>> ---
> >>>  MdeModulePkg/Universal/DisplayEngineDxe/FormDisplay.c | 3 ++-
> >>>  1 file changed, 2 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/MdeModulePkg/Universal/DisplayEngineDxe/FormDisplay.c
> >>b/MdeModulePkg/Universal/DisplayEngineDxe/FormDisplay.c
> >>> index 7390f954b6..44f087fe01 100644
> >>> --- a/MdeModulePkg/Universal/DisplayEngineDxe/FormDisplay.c
> >>> +++ b/MdeModulePkg/Universal/DisplayEngineDxe/FormDisplay.c
> >>> @@ -2880,10 +2880,11 @@ UiDisplayMenu (
> >>>// MenuOption is set to NULL in Repaint
> >>>// NewPos: Current menu option that need to hilight
> >>>//
> >>>ControlFlag = CfUpdateHelpString;
> >>>
> >>> +  ASSERT (NewPos != NULL);
> >>>UpdateHighlightMenuInfo(NewPos, TopOfScreen, SkipValue);
> >>>
> >>>if (SkipHighLight) {
> >>>  SkipHighLight = FALSE;
> >>>  MenuOption= SavedMenuOption;
> >>> @@ -2908,11 +2909,11 @@ UiDisplayMenu (
> >>>  Temp2 = SkipValue;
> >>>} else {
> >>>  Temp2 = 0;
> >>>}
> >>>
> >>> -  if (NewPos != NULL && (MenuOption == NULL || NewPos !=
> >>>Link)) {
> >>> +  if (MenuOption == NULL || NewPos != >Link) {
> >>>  if (MenuOption != NULL) {
> >>>//
> >>>// Remove the old highlight menu.
> >>>//
> >>>Status = DisplayOneMenu (MenuOption,
> >>> --
> >>> 2.18.0.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] [edk2-wiki PATCH] release planning: announce the soft and hard feature freezes

[Leif Lindholm]

On Thu, Nov 08, 2018 at 06:28:07PM +0100, Laszlo Ersek wrote:
> Add a paragraph to each of the SoftFeatureFreeze and HardFeatureFreeze
> articles about the respective announcements on edk2-devel.
> 
> Cc: Andrew Fish 
> Cc: Brian Richardson 
> Cc: Leif Lindholm 
> Cc: Liming Gao 
> Cc: Michael Kinney 
> Cc: Stephano Cetola 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Laszlo Ersek 

Reviewed-by: Leif Lindholm 

> ---
> 
> Notes:
> Check out the rendered pages in my clone of the wiki:
> 
> https://github.com/lersek/edk2/wiki/SoftFeatureFreeze
> https://github.com/lersek/edk2/wiki/HardFeatureFreeze
> 
>  HardFeatureFreeze.md | 8 
>  SoftFeatureFreeze.md | 8 
>  2 files changed, 16 insertions(+)
> 
> diff --git a/HardFeatureFreeze.md b/HardFeatureFreeze.md
> index 01288dd03f71..e08f4c047e8d 100644
> --- a/HardFeatureFreeze.md
> +++ b/HardFeatureFreeze.md
> @@ -4,3 +4,11 @@ tag](EDK-II#stable-tags).
>  
>  (This definition is modeled after QEMU's [hard feature
>  freeze](https://wiki.qemu.org/Planning/HardFeatureFreeze)).
> +
> +# Announcing the hard feature freeze
> +
> +The proposed schedule for the active development cycle is shown in the [EDK 
> II
> +Release Planning](EDK-II-Release-Planning) article. Shortly before the hard
> +feature freeze, an announcement email is sent to the
> +[edk2-devel](https://lists.01.org/mailman/listinfo/edk2-devel) mailing list; 
> by
> +default by [Liming Gao](https://github.com/lgao4/).
> diff --git a/SoftFeatureFreeze.md b/SoftFeatureFreeze.md
> index 9dc7d9c19369..e33dd80ccbee 100644
> --- a/SoftFeatureFreeze.md
> +++ b/SoftFeatureFreeze.md
> @@ -40,3 +40,11 @@ communicate with the maintainer about their intentions. It 
> also helps if you:
>  
>  (This definition is modeled after QEMU's [soft feature
>  freeze](https://wiki.qemu.org/Planning/SoftFeatureFreeze).)
> +
> +# Announcing the soft feature freeze
> +
> +The proposed schedule for the active development cycle is shown in the [EDK 
> II
> +Release Planning](EDK-II-Release-Planning) article. Shortly before the soft
> +feature freeze, an announcement email is sent to the
> +[edk2-devel](https://lists.01.org/mailman/listinfo/edk2-devel) mailing list; 
> by
> +default by [Liming Gao](https://github.com/lgao4/).
> -- 
> 2.19.1.3.g30247aa5d201
> 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [Patch][edk2-platforms/devel-IntelAtomProcessorE3900] Change BIOS Version.

[zwei4]

From: Bright 

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Sun, Zailiang 
CC: David Wei 
---
 Vlv2TbltDevicePkg/BiosId.env | 2 +-
 Vlv2TbltDevicePkg/BiosIdD.env| 2 +-
 Vlv2TbltDevicePkg/BiosIdR.env| 2 +-
 Vlv2TbltDevicePkg/BiosIdx64D.env | 2 +-
 Vlv2TbltDevicePkg/BiosIdx64R.env | 2 +-
 5 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/Vlv2TbltDevicePkg/BiosId.env b/Vlv2TbltDevicePkg/BiosId.env
index 35314db780..dfdcb34f41 100644
--- a/Vlv2TbltDevicePkg/BiosId.env
+++ b/Vlv2TbltDevicePkg/BiosId.env
@@ -24,6 +24,6 @@
 BOARD_ID  = MNW2MAX
 BOARD_REV = 1
 BUILD_TYPE= D
-VERSION_MAJOR = 0099
+VERSION_MAJOR = 0100
 VERSION_MINOR = 01
 BOARD_EXT = X64
diff --git a/Vlv2TbltDevicePkg/BiosIdD.env b/Vlv2TbltDevicePkg/BiosIdD.env
index 0e8f680a69..e3bd0053b6 100644
--- a/Vlv2TbltDevicePkg/BiosIdD.env
+++ b/Vlv2TbltDevicePkg/BiosIdD.env
@@ -35,5 +35,5 @@ OEM_ID= I32
 BUILD_TYPE= D
 
 BOARD_ID = BLAKCRB
-VERSION_MAJOR = 0099
+VERSION_MAJOR = 0100
 VERSION_MINOR = 01
diff --git a/Vlv2TbltDevicePkg/BiosIdR.env b/Vlv2TbltDevicePkg/BiosIdR.env
index 1ba47cbc05..4629c44d14 100644
--- a/Vlv2TbltDevicePkg/BiosIdR.env
+++ b/Vlv2TbltDevicePkg/BiosIdR.env
@@ -35,5 +35,5 @@ OEM_ID= I32
 BUILD_TYPE= R
 
 BOARD_ID = BLAKCRB
-VERSION_MAJOR = 0099
+VERSION_MAJOR = 0100
 VERSION_MINOR = 01
diff --git a/Vlv2TbltDevicePkg/BiosIdx64D.env b/Vlv2TbltDevicePkg/BiosIdx64D.env
index ba9ce76f63..902820b6c3 100644
--- a/Vlv2TbltDevicePkg/BiosIdx64D.env
+++ b/Vlv2TbltDevicePkg/BiosIdx64D.env
@@ -25,6 +25,6 @@ BOARD_REV = 1
 OEM_ID= X64
 BUILD_TYPE= D
 
-VERSION_MAJOR = 0099
+VERSION_MAJOR = 0100
 VERSION_MINOR = 01
 BOARD_ID = BBAYCRB 
diff --git a/Vlv2TbltDevicePkg/BiosIdx64R.env b/Vlv2TbltDevicePkg/BiosIdx64R.env
index 4ccb2a0583..7d06cf5bae 100644
--- a/Vlv2TbltDevicePkg/BiosIdx64R.env
+++ b/Vlv2TbltDevicePkg/BiosIdx64R.env
@@ -25,6 +25,6 @@ BOARD_REV = 1
 OEM_ID= X64
 BUILD_TYPE= R
 
-VERSION_MAJOR = 0099
+VERSION_MAJOR = 0100
 VERSION_MINOR = 01
 BOARD_ID = BBAYCRB 
-- 
2.19.1.windows.1

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

[edk2] [Patch V2] BaseTools: Replace the sqlite database with list

[BobCF]

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

[V2]
Optimize this patch so that it can be easy to review.
This patch is just apply the change to original files while
not create new similar files.

[V1]
This patch is one of build tool performance improvement
series patches.

This patch is going to use python list to store the parser data
instead of using sqlite database.

The replacement solution is as below:

SQL insert: list.append()
SQL select: list comprehension. for example:
Select * from table where field = “something”
->
[ item for item in table if item[3] == “something”]

SQL update: python map function. for example:
Update table set field1=newvalue where filed2 = “something”.
-> map(lambda x: x[1] = newvalue,
   [item for item in table if item[2] == “something”])

SQL delete: list comprehension.

With this change, We can save the time of interpreting SQL statement
and the time of write database to file system

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: BobCF 
Cc: Liming Gao 
Cc: Jaben Carsey 
---
 BaseTools/Source/Python/GenFds/GenFds.py  |   3 +-
 .../Source/Python/Workspace/MetaDataTable.py  |  93 ++-
 .../Source/Python/Workspace/MetaFileParser.py |   5 +-
 .../Source/Python/Workspace/MetaFileTable.py  | 248 ++
 .../Python/Workspace/WorkspaceDatabase.py | 131 +
 BaseTools/Source/Python/build/build.py|  27 +-
 6 files changed, 186 insertions(+), 321 deletions(-)

diff --git a/BaseTools/Source/Python/GenFds/GenFds.py 
b/BaseTools/Source/Python/GenFds/GenFds.py
index 0c8091b798..4fd96706af 100644
--- a/BaseTools/Source/Python/GenFds/GenFds.py
+++ b/BaseTools/Source/Python/GenFds/GenFds.py
@@ -222,12 +222,11 @@ def main():
 if "TOOL_CHAIN_TAG" not in GlobalData.gGlobalDefines:
 GlobalData.gGlobalDefines['TOOL_CHAIN_TAG'] = 
GenFdsGlobalVariable.ToolChainTag
 
 """call Workspace build create database"""
 GlobalData.gDatabasePath = 
os.path.normpath(os.path.join(ConfDirectoryPath, GlobalData.gDatabasePath))
-BuildWorkSpace = WorkspaceDatabase(GlobalData.gDatabasePath)
-BuildWorkSpace.InitDatabase()
+BuildWorkSpace = WorkspaceDatabase()
 
 #
 # Get files real name in workspace dir
 #
 GlobalData.gAllFiles = DirCache(Workspace)
diff --git a/BaseTools/Source/Python/Workspace/MetaDataTable.py 
b/BaseTools/Source/Python/Workspace/MetaDataTable.py
index bd751eadfb..8becddbe08 100644
--- a/BaseTools/Source/Python/Workspace/MetaDataTable.py
+++ b/BaseTools/Source/Python/Workspace/MetaDataTable.py
@@ -37,84 +37,58 @@ class Table(object):
 _COLUMN_ = ''
 _ID_STEP_ = 1
 _ID_MAX_ = 0x8000
 _DUMMY_ = 0
 
-def __init__(self, Cursor, Name='', IdBase=0, Temporary=False):
-self.Cur = Cursor
+def __init__(self, Db, Name='', IdBase=0, Temporary=False):
+self.Db = Db
 self.Table = Name
 self.IdBase = int(IdBase)
 self.ID = int(IdBase)
 self.Temporary = Temporary
+self.Contents = []
 
 def __str__(self):
 return self.Table
 
 ## Create table
 #
 # Create a table
 #
 def Create(self, NewTable=True):
-if NewTable:
-self.Drop()
-
-if self.Temporary:
-SqlCommand = """create temp table IF NOT EXISTS %s (%s)""" % 
(self.Table, self._COLUMN_)
-else:
-SqlCommand = """create table IF NOT EXISTS %s (%s)""" % 
(self.Table, self._COLUMN_)
-EdkLogger.debug(EdkLogger.DEBUG_8, SqlCommand)
-self.Cur.execute(SqlCommand)
+self.Db.CreateEmptyTable(self.Table)
 self.ID = self.GetId()
 
 ## Insert table
 #
 # Insert a record into a table
 #
 def Insert(self, *Args):
 self.ID = self.ID + self._ID_STEP_
 if self.ID >= (self.IdBase + self._ID_MAX_):
 self.ID = self.IdBase + self._ID_STEP_
-Values = ", ".join(str(Arg) for Arg in Args)
-SqlCommand = "insert into %s values(%s, %s)" % (self.Table, self.ID, 
Values)
-EdkLogger.debug(EdkLogger.DEBUG_5, SqlCommand)
-self.Cur.execute(SqlCommand)
-return self.ID
+row = [self.ID]
+row.extend(Args)
+self.Contents.append(row)
 
-## Query table
-#
-# Query all records of the table
-#
-def Query(self):
-SqlCommand = """select * from %s""" % self.Table
-self.Cur.execute(SqlCommand)
-for Rs in self.Cur:
-EdkLogger.verbose(str(Rs))
-TotalCount = self.GetId()
+return self.ID
 
-## Drop a table
-#
-# Drop the table
-#
-def Drop(self):
-SqlCommand = """drop table IF EXISTS %s""" % self.Table
-self.Cur.execute(SqlCommand)
 
 ## Get count
 #
 # Get a count of all records of the table
 #
 # @retval Count:  Total count of all records
 #
 def GetCount(self):
-SqlCommand = """select count(ID) from 

Re: [edk2] [Patch 2/2] UefiCpuPkg/PiSmmCpuDxeSmm: Separate semaphore container.

[Ni, Ruiyu]

Eric,
Similar comments suggestion to this patch.
Reviewed-by: Ruiyu Ni 

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

Re: [edk2] [Patch 1/2] UefiCpuPkg/RegisterCpuFeaturesLib: Separate semaphore container.

[Ni, Ruiyu]

Eric,
Patch is good.
Reviewed-by: Ruiyu Ni 

Some modification to commit message to help understanding the changes.
Please rewording if you think some of them is not proper.

You can push the changes if no concerns about the new commit message.

-

In current implementation, core and package level sync uses same 
semaphores. Sharing the semaphore may cause wrong execution order.

For example:
1. Feature A has CPU_FEATURE_CORE_BEFORE dependency with Feature B.
2. Feature C has CPU_FEATURE_PACKAGE_AFTER dependency with Feature B.
The expected feature initialization order is A B C:
A  (Core Depends) > B  (Package Depends) > C

For a CPU has 1 package, 2 cores and 4 threads. The feature 
initialization order may like below:


  Thread#1 Thread#2Thread#3   Thread#4
  [A.Init] [A.Init]   [A.Init]
Release(S1, S2)Release(S1, S2)Release(S3, S4)
Wait(S1) * 2   Wait(S2) * 2<- Core sync
  [B.Init] [B.Init]
Release (S1,S2,S3,S4)
Wait (S1) * 4  <-- Package sync
  Wait(S4 * 2) <- Core sync
  [B.Init]

In above case, for thread#4, when it syncs in core level, Wait(S4) * 2 
isn't blocked and [B.Init] runs. But [A.Init] hasn't run in thread#3. 
It's wrong! Thread#4 should execute [B.Init] after thread#3 executes 
[A.Init] because B core level depends on A.


The reason is of the wrong execution order is that S4 is released in 
thread#1 by calling Release (S1, S2, S3, S4) and in thread #4 by calling 
Release (S3, S4).


To fix this issue, core level sync and package level sync should use 
separate semaphores.


In above example, the S4 released in Release (S1, S2, S3, S4) should not 
be the same semaphore as that in Release (S3, S4).


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

Re: [edk2] [Patch] SecurityPkg: Fix TPM device compatibility issue

[Laszlo Ersek]

On 11/09/18 07:02, Zhang, Chao B wrote:
> Issue Statement:
> TPM InterfaceId cache feature is introduced by 
> f15cb995bb3880b77e15afe6facd3da05e599a17. It follows TCG PTP spec 1.3
> to improve TPM transmission performance and also addresses defects in some 
> TPM2.0 devices. But some other TPM devices like
> NTC1310 SPI TPM is found function abnormally with this feature, causing extra 
> device compatibility issue.
> 
> Solution:
> Add a policy indicator in PcdActiveTpmInterfaceType to disable TPM interface 
> ID cache to support those existing TPM devices
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Zhang, Chao B 
> Cc: Andrew Fish 
> Cc: Laszlo Ersek 
> Cc: Leif Lindholm 
> Cc: Michael D Kinney 
> Cc: Yao Jiewen 
> ---
>  SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c | 23 +++-
>  SecurityPkg/SecurityPkg.dec |  3 +-
>  SecurityPkg/SecurityPkg.uni |  3 +-
>  SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigImpl.c | 49 
> +
>  SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c   | 42 +
>  5 files changed, 117 insertions(+), 3 deletions(-)

I'll let others review this patch for technical merit.

However, I'm really undecided whether this patch qualifies for being
pushed during the hard feature freeze. Comments welcome.

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

Re: [edk2] [patch] MdePkg: Fix incorrect check for DisplayOnly text format in AcpiEx

[Laszlo Ersek]

On 11/09/18 04:34, Bi, Dandan wrote:
> Hi Stewards and package maintainers:
> 
> Since this is a clear bug.  And the risk for this release is small.
> So I plan to push this patch before edk2-stable201811 tag is created.

I agree, this is a bugfix, not a feature patch.

Thanks,
Laszlo

> If you have any concern, please raise here. 
> 
> 
> Thanks,
> Dandan
> 
>> -Original Message-
>> From: Gao, Liming
>> Sent: Thursday, November 8, 2018 9:56 PM
>> To: Bi, Dandan ; edk2-devel@lists.01.org
>> Cc: Ni, Ruiyu ; Kinney, Michael D
>> 
>> Subject: RE: [patch] MdePkg: Fix incorrect check for DisplayOnly text format
>> in AcpiEx
>>
>> Reviewed-by: Liming Gao 
>>
>>> -Original Message-
>>> From: Bi, Dandan
>>> Sent: Thursday, November 8, 2018 9:50 PM
>>> To: edk2-devel@lists.01.org
>>> Cc: Ni, Ruiyu ; Kinney, Michael D
>>> ; Gao, Liming 
>>> Subject: [patch] MdePkg: Fix incorrect check for DisplayOnly text
>>> format in AcpiEx
>>>
>>> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1312
>>>
>>> Text format for AcpiEx device path in UEFI Spec:
>>> AcpiEx(HID,CID,UID,HIDSTR,CIDSTR,UIDSTR)
>>> AcpiEx(HID|HIDSTR,(CID|CIDSTR,UID|UIDSTR))(Display Only)
>>>
>>> When convert device path to text for ACPI device path, current code
>>> check AllowShortcuts parameter to convert the device path to
>>> DisplayOnly text format(shorter text
>>> representation) by mistake.
>>> It should check DisplayOnly parameter.
>>>
>>> This commit is to fix this issue.
>>>
>>> Cc: Ruiyu Ni 
>>> Cc: Michael D Kinney 
>>> Cc: Liming Gao 
>>> Contributed-under: TianoCore Contribution Agreement 1.1
>>> Signed-off-by: Dandan Bi 
>>> ---
>>>  MdePkg/Library/UefiDevicePathLib/DevicePathToText.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/MdePkg/Library/UefiDevicePathLib/DevicePathToText.c
>>> b/MdePkg/Library/UefiDevicePathLib/DevicePathToText.c
>>> index cdcdb3623a..97d279eeb2 100644
>>> --- a/MdePkg/Library/UefiDevicePathLib/DevicePathToText.c
>>> +++ b/MdePkg/Library/UefiDevicePathLib/DevicePathToText.c
>>> @@ -495,11 +495,11 @@ DevPathToTextAcpiEx (
>>>  CIDText,
>>>  UIDStr
>>> );
>>>  }
>>>} else {
>>> -if (AllowShortcuts) {
>>> +if (DisplayOnly) {
>>>//
>>>// display only
>>>//
>>>if (AcpiEx->HID == 0) {
>>>  UefiDevicePathLibCatPrint (Str, L"AcpiEx(%a,", HIDStr);
>>> --
>>> 2.18.0.windows.1
> 

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

Re: [edk2] [Patch] Maintainers.txt: Update EDK II Releases to EDK-II-Release-Planning wiki

[Laszlo Ersek]

On 11/09/18 01:26, Gao, Liming wrote:
> Thanks for your review. I would like to push this change for this stable tag 
> edk2-stable201811. It has no impact for the release. Are you OK for it?

Sure; it is not a feature patch set.

Thanks!
Laszlo

>> -Original Message-
>> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
>> Laszlo Ersek
>> Sent: Friday, November 09, 2018 1:45 AM
>> To: Leif Lindholm ; Gao, Liming
>> 
>> Cc: Kinney, Michael D ; edk2-
>> de...@lists.01.org
>> Subject: Re: [edk2] [Patch] Maintainers.txt: Update EDK II Releases to 
>> EDK-II-
>> Release-Planning wiki
>>
>> On 11/08/18 18:13, Leif Lindholm wrote:
>>> I'm generally happy with this if the rest of you are?
>>>
>>> Reviewed-by: Leif Lindholm 
>>>
>>> /
>>> Leif
>>>
>>> On Thu, Nov 08, 2018 at 10:49:34PM +0800, Liming Gao wrote:
 Contributed-under: TianoCore Contribution Agreement 1.1
 Signed-off-by: Liming Gao 
 ---
  Maintainers.txt | 4 +---
  1 file changed, 1 insertion(+), 3 deletions(-)

 diff --git a/Maintainers.txt b/Maintainers.txt
 index 6c9156169a..fc183d6477 100644
 --- a/Maintainers.txt
 +++ b/Maintainers.txt
 @@ -51,9 +51,7 @@ W:
>> https://github.com/tianocore/tianocore.github.io/wiki/Security

  EDK II Releases:
  
 -UDK2014
 -W: http://www.tianocore.org/udk2014/
 -S: Supported
 +W: https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-
>> Release-Planning

  EDK II Packages:
  
>>
>> Reviewed-by: Laszlo Ersek 
>>
>> 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