date:20181001

[edk2] [PATCH] BaseTools/edksetup.sh: Handle the return value from grep

2018-10-01 Thread Gary Lin

When SetupPython3() parses the possible python binary paths, it uses
`grep "[[:digit:]]$"` to filter the python versions in the file name.
Since grep would return 1 when the last character of the file name is
not a digit, OvmfPkg/build.sh would fail immediately when the function
is handling some cases, e.g. "python3:".

This commit adds `|| true` to the grep command to make sure the command
never returns 1 in any condition.

[NOTE: For the python3 branch]

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Gary Lin 
Cc: Liming Gao 
Cc: Yonghong Zhu 
---
 edksetup.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/edksetup.sh b/edksetup.sh
index 5ff3be19fb2f..d4e577e60781 100755
--- a/edksetup.sh
+++ b/edksetup.sh
@@ -115,7 +115,7 @@ function SetupPython3()
 {
   for python in $(whereis python3)
   do
-python=$(echo $python | grep "[[:digit:]]$")
+python=$(echo $python | grep "[[:digit:]]$" || true)
 python_version=${python##*python}
 if [ -z "${python_version}" ];then
   continue
-- 
2.18.0

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

[edk2] GenFds broken in latest basetools-win32

2018-10-01 Thread Kurt Kennett

"genfds" seems broken in latest tiano-win32

Can anybody else repo this:

C:\repo\tiano-win32>genfds
Traceback (most recent call last):
  File "C:\Python27\lib\site-packages\cx_Freeze\initscripts\Console.py", line 
27, in 
  File "GenFds\GenFds.py", line 24, in 
ValueError: Attempted relative import in non-package

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

Re: [edk2] [PATCH 5/5] MdePkg/BaseSynchronizationLib GCC: simplify IA32 InternalSyncCompareExchange64()

2018-10-01 Thread Laszlo Ersek

On 10/01/18 20:27, Philippe Mathieu-Daudé wrote:
> On 30/09/2018 00:23, Laszlo Ersek wrote:
>> The IA32 variant of InternalSyncCompareExchange64() is correct, but we can
>> simplify it. We don't need to load the lower 32 bits of ExchangeValue into
>> EBX in two steps (first into a general register, then into EBX); we can
>> ask GCC to populate EBX like that itself.
>>
>> Cc: Liming Gao 
>> Cc: Michael D Kinney 
>> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1208
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Laszlo Ersek 
> 
> Reviewed-by: Philippe Mathieu-Daudé 
> 
>> ---
>>  MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c | 5 +
>>  1 file changed, 1 insertion(+), 4 deletions(-)
>>
>> diff --git a/MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c 
>> b/MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c
>> index 44188e265af2..af39bdeb516c 100644
>> --- a/MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c
>> +++ b/MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c
>> @@ -193,14 +193,11 @@ InternalSyncCompareExchange64 (
>>)
>>  {
>>__asm__ __volatile__ (
>> -"push%%ebx  \n\t"
>> -"movl%2,%%ebx   \n\t"
>>  "lock   \n\t"
>>  "cmpxchg8b   (%1)   \n\t"
>> -"pop %%ebx  \n\t"
>>  : "+A"  (CompareValue)// %0
>>  : "S"   (Value),  // %1
>> -  "r"   ((UINT32) ExchangeValue), // %2
>> +  "b"   ((UINT32) ExchangeValue), // %2
>>"c"   ((UINT32) (ExchangeValue >> 32))  // %3
>>  : "memory",
>>"cc"
>>

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

Re: [edk2] [PATCH 5/5] MdePkg/BaseSynchronizationLib GCC: simplify IA32 InternalSyncCompareExchange64()

2018-10-01 Thread Philippe Mathieu-Daudé

On 30/09/2018 00:23, Laszlo Ersek wrote:
> The IA32 variant of InternalSyncCompareExchange64() is correct, but we can
> simplify it. We don't need to load the lower 32 bits of ExchangeValue into
> EBX in two steps (first into a general register, then into EBX); we can
> ask GCC to populate EBX like that itself.
> 
> Cc: Liming Gao 
> Cc: Michael D Kinney 
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1208
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Laszlo Ersek 

Reviewed-by: Philippe Mathieu-Daudé 

> ---
>  MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c | 5 +
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c 
> b/MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c
> index 44188e265af2..af39bdeb516c 100644
> --- a/MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c
> +++ b/MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c
> @@ -193,14 +193,11 @@ InternalSyncCompareExchange64 (
>)
>  {
>__asm__ __volatile__ (
> -"push%%ebx  \n\t"
> -"movl%2,%%ebx   \n\t"
>  "lock   \n\t"
>  "cmpxchg8b   (%1)   \n\t"
> -"pop %%ebx  \n\t"
>  : "+A"  (CompareValue)// %0
>  : "S"   (Value),  // %1
> -  "r"   ((UINT32) ExchangeValue), // %2
> +  "b"   ((UINT32) ExchangeValue), // %2
>"c"   ((UINT32) (ExchangeValue >> 32))  // %3
>  : "memory",
>"cc"
> 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] MdeModulePkg:disable wraning to pass gcc4.8 build

2018-10-01 Thread Laszlo Ersek

Wait a sec:

On 10/01/18 19:48, Laszlo Ersek wrote:

> However, this (edk2) commit message seems to suggest that the warnings
> are only present with gcc-4.8 (and no later gcc releases). This in turn
> suggests that the warnings are spurious (presumably, later gcc releases
> recognize that none of the affected variables are actually read without
> a prior initialization or assignment.)
> 
> Hence, upstream Oniguruma might argue that we should fix our compiler --
> they might consider gcc-4.8 too old to care -- rather than litter their
> code with superfluous assignments, just to pacify gcc-4.8.

[...]

> So, I think I agree with this patch, but the commit message should be a
> *lot* more detailed. (Basically, include the analysis from above.)

In fact, another improvement is possible: if the issue only affects
gcc-4.8, then:

>> On 29/09/18 02:55, Dongao Guo wrote:
>>> Change-Id: I782962e4994a8edf14beb7ede8b1aabe233dc3a8
>>> Contributed-under: TianoCore Contribution Agreement 1.1
>>> ---
>>>  MdeModulePkg/Universal/RegularExpressionDxe/RegularExpressionDxe.inf | 3 
>>> +++
>>>  1 file changed, 3 insertions(+)
>>>
>>> diff --git 
>>> a/MdeModulePkg/Universal/RegularExpressionDxe/RegularExpressionDxe.inf 
>>> b/MdeModulePkg/Universal/RegularExpressionDxe/RegularExpressionDxe.inf
>>> index 16e91bd..07bc02e 100644
>>> --- a/MdeModulePkg/Universal/RegularExpressionDxe/RegularExpressionDxe.inf
>>> +++ b/MdeModulePkg/Universal/RegularExpressionDxe/RegularExpressionDxe.inf
>>> @@ -109,3 +109,6 @@
>>>
>>># Oniguruma: error: variable 'fp' set but not used
>>>GCC:*_*_*_CC_FLAGS = -Wno-error=unused-but-set-variable
>>> +
>>> +  # Oniguruma: tag_end in parse_callout_of_name
>>> +  GCC:*_*_*_CC_FLAGS = -Wno-error=maybe-uninitialized

this toolchain glob pattern is too general. It should be:

  GCC:*_GCC48_*_CC_FLAGS

(or maybe spell it out for all GCC versions up to and including 4.8,
from the earliest we support, 4.4.)

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

Re: [edk2] [PATCH] MdeModulePkg:disable wraning to pass gcc4.8 build

2018-10-01 Thread Laszlo Ersek

On 10/01/18 11:23, Tomas Pilar (tpilar) wrote:
> Wouldn't it be better to fix the warnings in code rather than
> silencing them?

The same idea occurred to me, but it really depends on upstream
Oniguruma. The upstream project's issue tracker on github seems quite
active:

  https://github.com/kkos/oniguruma/issues

so we could reasonably file a bug report there (and/or submit a pull
request).

However, this (edk2) commit message seems to suggest that the warnings
are only present with gcc-4.8 (and no later gcc releases). This in turn
suggests that the warnings are spurious (presumably, later gcc releases
recognize that none of the affected variables are actually read without
a prior initialization or assignment.)

Hence, upstream Oniguruma might argue that we should fix our compiler --
they might consider gcc-4.8 too old to care -- rather than litter their
code with superfluous assignments, just to pacify gcc-4.8.

(Obviously, if any one of those warnings is valid, then we *must* fix it
in upstream Oniguruma first, and backport the fix.)

... For reference, I've now built RegularExpressionDxe as follows (using
gcc-4.8.5-28.el7_5.1.x86_64):

  build -t GCC48 -b DEBUG -a X64 \
-p MdeModulePkg/MdeModulePkg.dsc \
-m MdeModulePkg/Universal/RegularExpressionDxe/RegularExpressionDxe.inf

and I get the following 3 "maybe-uninitialized" warnings (currently:
errors):

(1)

> MdeModulePkg/Universal/RegularExpressionDxe/Oniguruma/regcomp.c: In
> function 'compile_length_tree':
> MdeModulePkg/Universal/RegularExpressionDxe/Oniguruma/regcomp.c:1516:7:
> warning: 'len' may be used uninitialized in this function
> [-Wmaybe-uninitialized]
>int len;
>^

I think this is an invalid warning; the type of the controlling
expression (node->type) is enum GimmickType, and the case labels cover
all values of the enum. An assert(0) could be added, I guess, but again,
upstream Oniguruma would be justified to reject the idea.

(2)

> MdeModulePkg/Universal/RegularExpressionDxe/Oniguruma/regparse.c: In
> function 'parse_callout_args.isra.10.constprop.30':
> MdeModulePkg/Universal/RegularExpressionDxe/Oniguruma/regparse.c:6753:25:
> warning: 'rl' may be used uninitialized in this function
> [-Wmaybe-uninitialized]
>vals[n].l = rl;
>  ^

This warning is invalid, given:

  6749if (cn > 0) {
  6750  long rl;
  6751  r = parse_long(enc, buf, bufend, 1, LONG_MAX, );
  6752  if (r == ONIG_NORMAL) {
  6753vals[n].l = rl;

Because parse_long() only returns ONIG_NORMAL after it sets (*rl).

(3)

> MdeModulePkg/Universal/RegularExpressionDxe/Oniguruma/regparse.c: In
> function 'parse_callout_of_name.constprop.29':
> MdeModulePkg/Universal/RegularExpressionDxe/Oniguruma/regparse.c:6861:38:
> warning: 'tag_end' may be used uninitialized in this function
> [-Wmaybe-uninitialized]
>  if (! is_allowed_callout_tag_name(enc, tag_start, tag_end))
  ^

This warning is also invalid, given:

  6852if (c == '[') {
  6853  if (PEND) return ONIGERR_END_PATTERN_IN_GROUP;
  6854  tag_start = p;
  6855  while (! PEND) {
  6856if (PEND) return ONIGERR_END_PATTERN_IN_GROUP;
  6857tag_end = p;
  6858PFETCH_S(c);
  6859if (c == ']') break;
  6860  }
  6861  if (! is_allowed_callout_tag_name(enc, tag_start, tag_end))
  6862return ONIGERR_INVALID_CALLOUT_TAG_NAME;
  6863

To see that, first we should note:

#define PEND (p < end ?  0 : 1)

therefore PEND doesn't change if neither "p" nor "end" change.

Second, when we reach line 6855 (the "while") for the very first time,
(!PEND) is certainly true (i.e., PEND is false), because otherwise we
would have bailed at line 6853. Therefore we reach line 6857, and assign
"tag_end". Regardless of whether we iterate zero or more *additional*
times around the loop, "tag_end" will have been set, whenever we reach
line 6861.

So, I think I agree with this patch, but the commit message should be a
*lot* more detailed. (Basically, include the analysis from above.)

Thanks
Laszlo

> On 29/09/18 02:55, Dongao Guo wrote:
>> Change-Id: I782962e4994a8edf14beb7ede8b1aabe233dc3a8
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> ---
>>  MdeModulePkg/Universal/RegularExpressionDxe/RegularExpressionDxe.inf | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git 
>> a/MdeModulePkg/Universal/RegularExpressionDxe/RegularExpressionDxe.inf 
>> b/MdeModulePkg/Universal/RegularExpressionDxe/RegularExpressionDxe.inf
>> index 16e91bd..07bc02e 100644
>> --- a/MdeModulePkg/Universal/RegularExpressionDxe/RegularExpressionDxe.inf
>> +++ b/MdeModulePkg/Universal/RegularExpressionDxe/RegularExpressionDxe.inf
>> @@ -109,3 +109,6 @@
>>
>># Oniguruma: error: variable 'fp' set but not used
>>GCC:*_*_*_CC_FLAGS = -Wno-error=unused-but-set-variable
>> +
>> +  # Oniguruma: tag_end in parse_callout_of_name

Re: [edk2] [PATCH v1 1/1] OvmfPkg/PlatformPei: clear CPU caches

2018-10-01 Thread Laszlo Ersek

On 10/01/18 13:45, marcandre.lur...@redhat.com wrote:
> From: Marc-André Lureau 
> 
> The TCG "Platform Reset Attack Mitigation Specification" requires to
> clear the processor caches when the MOR bit is set at boot time.
> 
> According to Paolo Bonzini, clearing the CPU cache takes only a few
> hundred clock cycles, so it can be done unconditionally.
> 
> Flush the cache on all logical processors, thanks to
> EFI_PEI_MP_SERVICES_PPI, calling WBINVD "Write Back and Invalidate
> Cache" x86 instruction.

(1) Please update this paragraph (and the code, of course) as suggested
by Michael. (I guess I should have known better, and suggested
WriteBackInvalidateDataCache() myself. While in this particular case,
there's not a big difference, because this code is X86 only, I agree
it's better to call the more abstract interface.)

> 
> Cc: Jordan Justen 
> Cc: Laszlo Ersek 
> Cc: Ard Biesheuvel 
> Cc: Anthony Perard 
> Cc: Julien Grall 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Marc-André Lureau 
> ---
>  OvmfPkg/PlatformPei/PlatformPei.inf |   1 +
>  OvmfPkg/PlatformPei/Platform.h  |   5 +
>  OvmfPkg/PlatformPei/ClearCache.c| 110 
>  OvmfPkg/PlatformPei/Platform.c  |   1 +
>  4 files changed, 117 insertions(+)
> 
> diff --git a/OvmfPkg/PlatformPei/PlatformPei.inf 
> b/OvmfPkg/PlatformPei/PlatformPei.inf
> index 9c5ad9961c4a..9c9a95fb3fe5 100644
> --- a/OvmfPkg/PlatformPei/PlatformPei.inf
> +++ b/OvmfPkg/PlatformPei/PlatformPei.inf
> @@ -30,6 +30,7 @@
>  
>  [Sources]
>AmdSev.c
> +  ClearCache.c
>Cmos.c
>Cmos.h
>FeatureControl.c
> diff --git a/OvmfPkg/PlatformPei/Platform.h b/OvmfPkg/PlatformPei/Platform.h
> index f942e61bb4f9..b12a5c1f5f78 100644
> --- a/OvmfPkg/PlatformPei/Platform.h
> +++ b/OvmfPkg/PlatformPei/Platform.h
> @@ -83,6 +83,11 @@ InstallFeatureControlCallback (
>VOID
>);
>  
> +VOID
> +InstallClearCacheCallback (
> +  VOID
> +  );
> +
>  EFI_STATUS
>  InitializeXen (
>VOID
> diff --git a/OvmfPkg/PlatformPei/ClearCache.c 
> b/OvmfPkg/PlatformPei/ClearCache.c
> new file mode 100644
> index ..a1fff8446d13
> --- /dev/null
> +++ b/OvmfPkg/PlatformPei/ClearCache.c
> @@ -0,0 +1,110 @@
> +/**@file
> +  Install a callback to clear cache on all processors.

(2) Please fuse the first two paragraphs of the commit message into a
short additional sentence here, such as:

  This is for conformance with the TCG "Platform Reset Attack Mitigation
  Specification". Because clearing the CPU caches at boot doesn't impact
  performance significantly, do it unconditionally, for simplicity's
  sake.

> +
> +  Copyright (C) 2018, Red Hat, Inc.
> +
> +  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 "Platform.h"
> +
> +/**
> +  All APs execute this function in parallel. The BSP executes the function
> +  separately.
> +
> +  @param[in,out] WorkSpace  Pointer to the input/output argument workspace
> +shared by all processors.
> +**/

(3) Please prepend a short sentence to the top of the comment block,
stating what the callback does.

> +STATIC
> +VOID
> +EFIAPI
> +ClearCache (
> +  IN OUT VOID *WorkSpace
> +  )
> +{
> +  AsmWbinvd ();

(4) This is where Mike's comment applies.

> +}
> +
> +/**
> +  Notification function called when EFI_PEI_MP_SERVICES_PPI becomes 
> available.
> +
> +  @param[in] PeiServices  Indirect reference to the PEI Services Table.
> +  @param[in] NotifyDescriptor Address of the notification descriptor data
> +  structure.
> +  @param[in] Ppi  Address of the PPI that was installed.
> +
> +  @return  Status of the notification. The status code returned from this
> +   function is ignored.
> +**/
> +STATIC
> +EFI_STATUS
> +EFIAPI
> +OnMpServicesAvailable (

(5) Technically this is correct, but we should preferably avoid a
function name collision even for debug log purposes, between this file,
and "OvmfPkg/PlatformPei/FeatureControl.c".

We already disambiguate the log message quite well, because we log
gEfiCallerBaseName, not just __FUNCTION__; however, in this case,
gEfiCallerBaseName will no longer be unique. So please rename the
function (include ClearCache somehow in the name).

> +  IN EFI_PEI_SERVICES   **PeiServices,
> +  IN EFI_PEI_NOTIFY_DESCRIPTOR  *NotifyDescriptor,
> +  IN VOID   *Ppi
> +  )
> +{
> +  EFI_PEI_MP_SERVICES_PPI *MpServices;
> +  EFI_STATUS  Status;
> +
> +  DEBUG ((DEBUG_INFO, "%a: %a\n", gEfiCallerBaseName,

Re: [edk2] [PATCH v1 1/1] OvmfPkg/PlatformPei: clear CPU caches

2018-10-01 Thread Kinney, Michael D

Hi,

Could you use a service from the CacheMaintenanceLib instead?

/**
  Writes back and invalidates the entire data cache in cache coherency domain
  of the calling CPU.

  Writes back and invalidates the entire data cache in cache coherency domain
  of the calling CPU. This function guarantees that all dirty cache lines are
  written back to system memory, and also invalidates all the data cache lines
  in the cache coherency domain of the calling CPU.

**/
VOID
EFIAPI
WriteBackInvalidateDataCache (
  VOID
  )

Thanks,

Mike

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-
> boun...@lists.01.org] On Behalf Of
> marcandre.lur...@redhat.com
> Sent: Monday, October 1, 2018 4:45 AM
> To: edk2-devel@lists.01.org
> Cc: Justen, Jordan L ;
> Anthony Perard ; Laszlo
> Ersek 
> Subject: [edk2] [PATCH v1 1/1] OvmfPkg/PlatformPei:
> clear CPU caches
> 
> From: Marc-André Lureau 
> 
> The TCG "Platform Reset Attack Mitigation
> Specification" requires to
> clear the processor caches when the MOR bit is set at
> boot time.
> 
> According to Paolo Bonzini, clearing the CPU cache
> takes only a few
> hundred clock cycles, so it can be done
> unconditionally.
> 
> Flush the cache on all logical processors, thanks to
> EFI_PEI_MP_SERVICES_PPI, calling WBINVD "Write Back and
> Invalidate
> Cache" x86 instruction.
> 
> Cc: Jordan Justen 
> Cc: Laszlo Ersek 
> Cc: Ard Biesheuvel 
> Cc: Anthony Perard 
> Cc: Julien Grall 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Marc-André Lureau
> 
> ---
>  OvmfPkg/PlatformPei/PlatformPei.inf |   1 +
>  OvmfPkg/PlatformPei/Platform.h  |   5 +
>  OvmfPkg/PlatformPei/ClearCache.c| 110
> 
>  OvmfPkg/PlatformPei/Platform.c  |   1 +
>  4 files changed, 117 insertions(+)
> 
> diff --git a/OvmfPkg/PlatformPei/PlatformPei.inf
> b/OvmfPkg/PlatformPei/PlatformPei.inf
> index 9c5ad9961c4a..9c9a95fb3fe5 100644
> --- a/OvmfPkg/PlatformPei/PlatformPei.inf
> +++ b/OvmfPkg/PlatformPei/PlatformPei.inf
> @@ -30,6 +30,7 @@
> 
>  [Sources]
>AmdSev.c
> +  ClearCache.c
>Cmos.c
>Cmos.h
>FeatureControl.c
> diff --git a/OvmfPkg/PlatformPei/Platform.h
> b/OvmfPkg/PlatformPei/Platform.h
> index f942e61bb4f9..b12a5c1f5f78 100644
> --- a/OvmfPkg/PlatformPei/Platform.h
> +++ b/OvmfPkg/PlatformPei/Platform.h
> @@ -83,6 +83,11 @@ InstallFeatureControlCallback (
>VOID
>);
> 
> +VOID
> +InstallClearCacheCallback (
> +  VOID
> +  );
> +
>  EFI_STATUS
>  InitializeXen (
>VOID
> diff --git a/OvmfPkg/PlatformPei/ClearCache.c
> b/OvmfPkg/PlatformPei/ClearCache.c
> new file mode 100644
> index ..a1fff8446d13
> --- /dev/null
> +++ b/OvmfPkg/PlatformPei/ClearCache.c
> @@ -0,0 +1,110 @@
> +/**@file
> +  Install a callback to clear cache on all processors.
> +
> +  Copyright (C) 2018, Red Hat, Inc.
> +
> +  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 "Platform.h"
> +
> +/**
> +  All APs execute this function in parallel. The BSP
> executes the function
> +  separately.
> +
> +  @param[in,out] WorkSpace  Pointer to the
> input/output argument workspace
> +shared by all processors.
> +**/
> +STATIC
> +VOID
> +EFIAPI
> +ClearCache (
> +  IN OUT VOID *WorkSpace
> +  )
> +{
> +  AsmWbinvd ();
> +}
> +
> +/**
> +  Notification function called when
> EFI_PEI_MP_SERVICES_PPI becomes available.
> +
> +  @param[in] PeiServices  Indirect reference to
> the PEI Services Table.
> +  @param[in] NotifyDescriptor Address of the
> notification descriptor data
> +  structure.
> +  @param[in] Ppi  Address of the PPI that
> was installed.
> +
> +  @return  Status of the notification. The status code
> returned from this
> +   function is ignored.
> +**/
> +STATIC
> +EFI_STATUS
> +EFIAPI
> +OnMpServicesAvailable (
> +  IN EFI_PEI_SERVICES   **PeiServices,
> +  IN EFI_PEI_NOTIFY_DESCRIPTOR  *NotifyDescriptor,
> +  IN VOID   *Ppi
> +  )
> +{
> +  EFI_PEI_MP_SERVICES_PPI *MpServices;
> +  EFI_STATUS  Status;
> +
> +  DEBUG ((DEBUG_INFO, "%a: %a\n", gEfiCallerBaseName,
> __FUNCTION__));
> +
> +  //
> +  // Clear cache on all the APs in parallel.
> +  //
> +  MpServices = Ppi;
> +  Status = MpServices->StartupAllAPs (
> + (CONST EFI_PEI_SERVICES
> **)PeiServices,
> + MpServices,
> + ClearCache,  //
> Procedure
> + FALSE,   //
> SingleThread
> +

Re: [edk2] [Patch] BaseTools Python: Migrate Python27 to Python36

2018-10-01 Thread Kinney, Michael D

Gary,

Please send the patch to edk2-devel.

Yonghong may not be available much this week,
so you may not see a response form him till
next week.

Thanks,

Mike

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-
> boun...@lists.01.org] On Behalf Of Gary Lin
> Sent: Monday, October 1, 2018 3:24 AM
> To: Zhu, Yonghong 
> Cc: Sun, Yanyan ; edk2-
> de...@lists.01.org; Chen, Hesheng
> 
> Subject: Re: [edk2] [Patch] BaseTools Python: Migrate
> Python27 to Python36
> 
> On Wed, Sep 26, 2018 at 01:02:05PM +, Zhu, Yonghong
> wrote:
> > Hi All,
> >
> Hi Yonghong,
> 
> > Now we are working on bugzilla
> 55
> (https://bugzilla.tianocore.org/show_bug.cgi?id=55)  to
> migrate BaseTools python tool from Python27 to
> Python36.  After this migration, the BaseTools will
> only support Python 3. And for now, user need to
> install a Python version >= 3.6.
> > Here is the link: https://github.com/yzhu52/edk2.git
> Branch: Python3_V2
> > In this branch, after run edksetup script file, tool
> auto detect the Python tool that version >= 3.6,  tool
> would report error if it can't find the python that >=
> 3.6.
> >
> I'm testing the branch and found a minor bug in
> edksetup.sh that caused
> OvmfPkg/build.sh failed to build. I wrote a patch and
> it works for me.
> Where should I send the patch? to the edk2-devel
> mailinglist directly?
> or a pull request in github?
> 
> Thanks,
> 
> Gary Lin
> 
> > Current we still in doing some validation for this
> migration, and not finish the update for UPT, ECC, EOT,
> Tests those tools and scripts.
> > We already did following on this branch:
> >
> > 1.   Remove the "from __future__ import" items
> >
> > 2.   Update the xrange to range
> >
> > 3.   Update long to int
> >
> > 4.   Use input instead of raw_input
> >
> > 5.   Not use iteritems(), but use items()
> directly
> >
> > 6.   Remove the super() function argument
> >
> > 7.   Remove the itertools usage
> >
> > 8.   Fix some open file's read and write function
> >
> > 9.   Hand the bytes and str type difference
> >
> > 10.   Do some list and iterator update
> >
> > 11.   Change the division operation in the expression
> >
> > 12.   Use '\n' but not os.linesep
> >
> > 13.   Update the BinWrappers script
> >
> > 14.   Update edksetup script file
> >
> >
> > Your suggestions is highly appreciated.
> >
> > Best Regards,
> > Zhu Yonghong
> >
> > ___
> > 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] ShellPkg ShellLib.h: Fix wrong parameter name for ShellOpenFileByName

2018-10-01 Thread Carsey, Jaben

I think we need to update description. I guess that OpenFileByName may search 
CWD and or PATH, but DevicePath is absolute and therefore will not..

> -Original Message-
> From: Zeng, Star
> Sent: Saturday, September 29, 2018 6:20 PM
> To: Ni, Ruiyu ; edk2-devel@lists.01.org
> Cc: Carsey, Jaben ; Zeng, Star
> 
> Subject: RE: [PATCH] ShellPkg ShellLib.h: Fix wrong parameter name for
> ShellOpenFileByName
> Importance: High
> 
> Ray,
> 
> Thanks.
> 
> I did not check the detail. But at least, one of definition and implementation
> needs to be updated. Maybe the description need to be enhanced also as
> you described.
> 
> ShellLib.h:
> 
> EFI_STATUS
> EFIAPI
> ShellOpenFileByDevicePath(
>   IN OUT EFI_DEVICE_PATH_PROTOCOL **FilePath,
>   OUT SHELL_FILE_HANDLE   *FileHandle,
>   IN UINT64   OpenMode,
>   IN UINT64   Attributes
>   );
> 
> EFI_STATUS
> EFIAPI
> ShellOpenFileByName(
>   IN CONST CHAR16   *FilePath,
>   OUT SHELL_FILE_HANDLE *FileHandle,
>   IN UINT64 OpenMode,
>   IN UINT64 Attributes
>   );
> 
> 
> UefiShellLib.c:
> 
> EFI_STATUS
> EFIAPI
> ShellOpenFileByDevicePath(
>   IN OUT EFI_DEVICE_PATH_PROTOCOL **FilePath,
>   OUT SHELL_FILE_HANDLE   *FileHandle,
>   IN UINT64   OpenMode,
>   IN UINT64   Attributes
>   )
> 
> EFI_STATUS
> EFIAPI
> ShellOpenFileByName(
>   IN CONST CHAR16   *FileName,
>   OUT SHELL_FILE_HANDLE *FileHandle,
>   IN UINT64 OpenMode,
>   IN UINT64 Attributes
>   )
> 
> Star
> -Original Message-
> From: Ni, Ruiyu
> Sent: Sunday, September 30, 2018 9:12 AM
> To: Zeng, Star ; edk2-devel@lists.01.org
> Cc: Carsey, Jaben 
> Subject: RE: [PATCH] ShellPkg ShellLib.h: Fix wrong parameter name for
> ShellOpenFileByName
> 
> Star,
> Per my understanding, FilePath means the full path pointing to the file,
> FileName means the very last part of the full path.
> E.g.: "fs0:\a\b\c\d.txt" is a FilePath, "d.txt" is a FileName.
> 
> So I think only specifying FileName is not enough to identify a file.
> The parameter name should be fine.
> 
> > -Original Message-
> > From: Zeng, Star
> > Sent: Saturday, September 29, 2018 5:43 PM
> > To: edk2-devel@lists.01.org
> > Cc: Zeng, Star ; Ni, Ruiyu ;
> > Carsey, Jaben 
> > Subject: [PATCH] ShellPkg ShellLib.h: Fix wrong parameter name for
> > ShellOpenFileByName
> >
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1221
> >
> > The parameter name FilePath should be FileName.
> >
> > I am trying to write an application for my own use and want to use
> > this interface, but confused by the parameter name.
> >
> > Interesting, the implementation in UefiShellLib.c is correct.
> >
> > Cc: Ruiyu Ni 
> > Cc: Jaben Carsey 
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Star Zeng 
> > ---
> >  ShellPkg/Include/Library/ShellLib.h | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/ShellPkg/Include/Library/ShellLib.h
> > b/ShellPkg/Include/Library/ShellLib.h
> > index 92fddc50f5dd..d6a5319285dd 100644
> > --- a/ShellPkg/Include/Library/ShellLib.h
> > +++ b/ShellPkg/Include/Library/ShellLib.h
> > @@ -126,7 +126,7 @@ ShellOpenFileByDevicePath(
> >otherwise, the Filehandle is NULL. Attributes is valid only for
> >EFI_FILE_MODE_CREATE.
> >
> > -  @param[in] FilePath   The pointer to file name.
> > +  @param[in] FileName   The pointer to file name.
> >@param[out] FileHandleThe pointer to the file handle.
> >@param[in] OpenMode   The mode to open the file with.
> >@param[in] Attributes The file's file attributes.
> > @@ -151,7 +151,7 @@ ShellOpenFileByDevicePath(  EFI_STATUS  EFIAPI
> > ShellOpenFileByName(
> > -  IN CONST CHAR16   *FilePath,
> > +  IN CONST CHAR16   *FileName,
> >OUT SHELL_FILE_HANDLE *FileHandle,
> >IN UINT64 OpenMode,
> >IN UINT64 Attributes
> > --
> > 2.7.0.windows.1

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

[edk2] [PATCH v1 1/1] OvmfPkg/PlatformPei: clear CPU caches

2018-10-01 Thread marcandre . lureau

From: Marc-André Lureau 

The TCG "Platform Reset Attack Mitigation Specification" requires to
clear the processor caches when the MOR bit is set at boot time.

According to Paolo Bonzini, clearing the CPU cache takes only a few
hundred clock cycles, so it can be done unconditionally.

Flush the cache on all logical processors, thanks to
EFI_PEI_MP_SERVICES_PPI, calling WBINVD "Write Back and Invalidate
Cache" x86 instruction.

Cc: Jordan Justen 
Cc: Laszlo Ersek 
Cc: Ard Biesheuvel 
Cc: Anthony Perard 
Cc: Julien Grall 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marc-André Lureau 
---
 OvmfPkg/PlatformPei/PlatformPei.inf |   1 +
 OvmfPkg/PlatformPei/Platform.h  |   5 +
 OvmfPkg/PlatformPei/ClearCache.c| 110 
 OvmfPkg/PlatformPei/Platform.c  |   1 +
 4 files changed, 117 insertions(+)

diff --git a/OvmfPkg/PlatformPei/PlatformPei.inf 
b/OvmfPkg/PlatformPei/PlatformPei.inf
index 9c5ad9961c4a..9c9a95fb3fe5 100644
--- a/OvmfPkg/PlatformPei/PlatformPei.inf
+++ b/OvmfPkg/PlatformPei/PlatformPei.inf
@@ -30,6 +30,7 @@
 
 [Sources]
   AmdSev.c
+  ClearCache.c
   Cmos.c
   Cmos.h
   FeatureControl.c
diff --git a/OvmfPkg/PlatformPei/Platform.h b/OvmfPkg/PlatformPei/Platform.h
index f942e61bb4f9..b12a5c1f5f78 100644
--- a/OvmfPkg/PlatformPei/Platform.h
+++ b/OvmfPkg/PlatformPei/Platform.h
@@ -83,6 +83,11 @@ InstallFeatureControlCallback (
   VOID
   );
 
+VOID
+InstallClearCacheCallback (
+  VOID
+  );
+
 EFI_STATUS
 InitializeXen (
   VOID
diff --git a/OvmfPkg/PlatformPei/ClearCache.c b/OvmfPkg/PlatformPei/ClearCache.c
new file mode 100644
index ..a1fff8446d13
--- /dev/null
+++ b/OvmfPkg/PlatformPei/ClearCache.c
@@ -0,0 +1,110 @@
+/**@file
+  Install a callback to clear cache on all processors.
+
+  Copyright (C) 2018, Red Hat, Inc.
+
+  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 "Platform.h"
+
+/**
+  All APs execute this function in parallel. The BSP executes the function
+  separately.
+
+  @param[in,out] WorkSpace  Pointer to the input/output argument workspace
+shared by all processors.
+**/
+STATIC
+VOID
+EFIAPI
+ClearCache (
+  IN OUT VOID *WorkSpace
+  )
+{
+  AsmWbinvd ();
+}
+
+/**
+  Notification function called when EFI_PEI_MP_SERVICES_PPI becomes available.
+
+  @param[in] PeiServices  Indirect reference to the PEI Services Table.
+  @param[in] NotifyDescriptor Address of the notification descriptor data
+  structure.
+  @param[in] Ppi  Address of the PPI that was installed.
+
+  @return  Status of the notification. The status code returned from this
+   function is ignored.
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+OnMpServicesAvailable (
+  IN EFI_PEI_SERVICES   **PeiServices,
+  IN EFI_PEI_NOTIFY_DESCRIPTOR  *NotifyDescriptor,
+  IN VOID   *Ppi
+  )
+{
+  EFI_PEI_MP_SERVICES_PPI *MpServices;
+  EFI_STATUS  Status;
+
+  DEBUG ((DEBUG_INFO, "%a: %a\n", gEfiCallerBaseName, __FUNCTION__));
+
+  //
+  // Clear cache on all the APs in parallel.
+  //
+  MpServices = Ppi;
+  Status = MpServices->StartupAllAPs (
+ (CONST EFI_PEI_SERVICES **)PeiServices,
+ MpServices,
+ ClearCache,  // Procedure
+ FALSE,   // SingleThread
+ 0,   // TimeoutInMicroSeconds: inf.
+ NULL // ProcedureArgument
+ );
+  if (EFI_ERROR (Status) && Status != EFI_NOT_STARTED) {
+DEBUG ((DEBUG_ERROR, "%a: StartupAllAps(): %r\n", __FUNCTION__, Status));
+return Status;
+  }
+
+  //
+  // Now clear cache on the BSP too.
+  //
+  ClearCache (NULL);
+  return EFI_SUCCESS;
+}
+
+//
+// Notification object for registering the callback, for when
+// EFI_PEI_MP_SERVICES_PPI becomes available.
+//
+STATIC CONST EFI_PEI_NOTIFY_DESCRIPTOR mMpServicesNotify = {
+  EFI_PEI_PPI_DESCRIPTOR_NOTIFY_CALLBACK | // Flags
+  EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST,
+  ,   // Guid
+  OnMpServicesAvailable// Notify
+};
+
+VOID
+InstallClearCacheCallback (
+  VOID
+  )
+{
+  EFI_STATUS   Status;
+
+  Status = PeiServicesNotifyPpi ();
+  if (EFI_ERROR (Status)) {
+DEBUG ((DEBUG_ERROR, "%a: failed to set up MP Services callback: %r\n",
+  __FUNCTION__, Status));
+  }
+}
diff --git a/OvmfPkg/PlatformPei/Platform.c b/OvmfPkg/PlatformPei/Platform.c
index 5a78668126b4..22139a64cbf4 100644
---

Re: [edk2] [PATCH 4/5] MdePkg/BaseSynchronizationLib GCC: fix X64 InternalSyncCompareExchange64()

2018-10-01 Thread Philippe Mathieu-Daudé

On 30/09/2018 00:23, Laszlo Ersek wrote:
> (This patch is identical to the X64 half of the last one, except for the
> InternalSyncCompareExchange32() -> InternalSyncCompareExchange64() and
> "cmpxchgl" -> "cmpxchgq" replacements.)
> 
> The CMPXCHG instruction has the following operands:
> - AX (implicit, CompareValue):input and output
> - destination operand (*Value):   input and output
> - source operand (ExchangeValue): input
> 
> The X64 version of InternalSyncCompareExchange64() attempts to mark both
> CompareValue and (*Value) as input/output, but it doesn't use the
> appropriate constraints for either operand.
> 
> Fix these issues. Furthermore, prefer the short "+" constraint for I/O
> operands over the  constraint that can be applied
> to the input instances of I/O operands.
> 
> Cc: Liming Gao 
> Cc: Michael D Kinney 
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1208
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Laszlo Ersek 

Reviewed-by: Philippe Mathieu-Daudé 

> ---
>  MdePkg/Library/BaseSynchronizationLib/X64/GccInline.c | 10 --
>  1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/MdePkg/Library/BaseSynchronizationLib/X64/GccInline.c 
> b/MdePkg/Library/BaseSynchronizationLib/X64/GccInline.c
> index a85cf0265c8b..edb904c00704 100644
> --- a/MdePkg/Library/BaseSynchronizationLib/X64/GccInline.c
> +++ b/MdePkg/Library/BaseSynchronizationLib/X64/GccInline.c
> @@ -194,12 +194,10 @@ InternalSyncCompareExchange64 (
>  {
>__asm__ __volatile__ (
>  "lock \n\t"
> -"cmpxchgq%3, %1   \n\t"
> -: "=a" (CompareValue),  // %0
> -  "=m" (*Value) // %1
> -: "a"  (CompareValue),  // %2
> -  "r"  (ExchangeValue), // %3
> -  "m"  (*Value) // %4
> +"cmpxchgq%2, %1   \n\t"
> +: "+a" (CompareValue),  // %0
> +  "+m" (*Value) // %1
> +: "r"  (ExchangeValue)  // %2
>  : "memory",
>"cc"
>  );
> 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 3/5] MdePkg/BaseSynchronizationLib GCC: fix InternalSyncCompareExchange32()

2018-10-01 Thread Philippe Mathieu-Daudé

On 30/09/2018 00:23, Laszlo Ersek wrote:
> (This patch is identical to the last one, except for the
> InternalSyncCompareExchange16() -> InternalSyncCompareExchange32() and
> "cmpxchgw" -> "cmpxchgl" replacements.)
> 
> The CMPXCHG instruction has the following operands:
> - AX (implicit, CompareValue):input and output
> - destination operand (*Value):   input and output
> - source operand (ExchangeValue): input
> 
> The IA32 version of InternalSyncCompareExchange32() correctly marks
> CompareValue as input/output, but it marks (*Value) only as input.
> 
> The X64 version of InternalSyncCompareExchange32() attempts to mark both
> CompareValue and (*Value) as input/output, but it doesn't use the
> appropriate constraints for either operand.
> 
> Fix these issues. Furthermore, prefer the short "+" constraint for I/O
> operands over the  constraint that can be applied
> to the input instances of I/O operands.
> 
> Cc: Liming Gao 
> Cc: Michael D Kinney 
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1208
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Laszlo Ersek 

Reviewed-by: Philippe Mathieu-Daudé 

> ---
>  MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c |  9 -
>  MdePkg/Library/BaseSynchronizationLib/X64/GccInline.c  | 10 --
>  2 files changed, 8 insertions(+), 11 deletions(-)
> 
> diff --git a/MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c 
> b/MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c
> index bd98a5aebfe7..44188e265af2 100644
> --- a/MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c
> +++ b/MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c
> @@ -155,11 +155,10 @@ InternalSyncCompareExchange32 (
>  {
>__asm__ __volatile__ (
>  "lock \n\t"
> -"cmpxchgl%1, %2   \n\t"
> -: "=a" (CompareValue)   // %0
> -: "q"  (ExchangeValue), // %1
> -  "m"  (*Value),// %2
> -  "0"  (CompareValue)   // %3
> +"cmpxchgl%2, %1   \n\t"
> +: "+a" (CompareValue),  // %0
> +  "+m" (*Value) // %1
> +: "q"  (ExchangeValue)  // %2
>  : "memory",
>"cc"
>  );
> diff --git a/MdePkg/Library/BaseSynchronizationLib/X64/GccInline.c 
> b/MdePkg/Library/BaseSynchronizationLib/X64/GccInline.c
> index 4338fb8e65e8..a85cf0265c8b 100644
> --- a/MdePkg/Library/BaseSynchronizationLib/X64/GccInline.c
> +++ b/MdePkg/Library/BaseSynchronizationLib/X64/GccInline.c
> @@ -155,12 +155,10 @@ InternalSyncCompareExchange32 (
>  {
>__asm__ __volatile__ (
>  "lock \n\t"
> -"cmpxchgl%3, %1   \n\t"
> -: "=a" (CompareValue),  // %0
> -  "=m" (*Value) // %1
> -: "a"  (CompareValue),  // %2
> -  "r"  (ExchangeValue), // %3
> -  "m"  (*Value) // %4
> +"cmpxchgl%2, %1   \n\t"
> +: "+a" (CompareValue),  // %0
> +  "+m" (*Value) // %1
> +: "r"  (ExchangeValue)  // %2
>  : "memory",
>"cc"
>  );
> 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 2/5] MdePkg/BaseSynchronizationLib GCC: fix InternalSyncCompareExchange16()

2018-10-01 Thread Philippe Mathieu-Daudé

On 30/09/2018 00:23, Laszlo Ersek wrote:
> The CMPXCHG instruction has the following operands:
> - AX (implicit, CompareValue):input and output
> - destination operand (*Value):   input and output
> - source operand (ExchangeValue): input
> 
> The IA32 version of InternalSyncCompareExchange16() correctly marks
> CompareValue as input/output, but it marks (*Value) only as input.
> 
> The X64 version of InternalSyncCompareExchange16() attempts to mark both
> CompareValue and (*Value) as input/output, but it doesn't use the
> appropriate constraints for either operand.
> 
> Fix these issues. Furthermore, prefer the short "+" constraint for I/O
> operands over the  constraint that can be applied
> to the input instances of I/O operands.
> 
> Cc: Liming Gao 
> Cc: Michael D Kinney 
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1208
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Laszlo Ersek 

Reviewed-by: Philippe Mathieu-Daudé 

> ---
>  MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c |  9 -
>  MdePkg/Library/BaseSynchronizationLib/X64/GccInline.c  | 10 --
>  2 files changed, 8 insertions(+), 11 deletions(-)
> 
> diff --git a/MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c 
> b/MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c
> index 1976720ac636..bd98a5aebfe7 100644
> --- a/MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c
> +++ b/MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c
> @@ -115,11 +115,10 @@ InternalSyncCompareExchange16 (
>  {
>__asm__ __volatile__ (
>  "lock \n\t"
> -"cmpxchgw%1, %2   \n\t"
> -: "=a" (CompareValue)   // %0
> -: "q"  (ExchangeValue), // %1
> -  "m"  (*Value),// %2
> -  "0"  (CompareValue)   // %3
> +"cmpxchgw%2, %1   \n\t"
> +: "+a" (CompareValue),  // %0
> +  "+m" (*Value) // %1
> +: "q"  (ExchangeValue)  // %2
>  : "memory",
>"cc"
>  );
> diff --git a/MdePkg/Library/BaseSynchronizationLib/X64/GccInline.c 
> b/MdePkg/Library/BaseSynchronizationLib/X64/GccInline.c
> index 0212798d7a27..4338fb8e65e8 100644
> --- a/MdePkg/Library/BaseSynchronizationLib/X64/GccInline.c
> +++ b/MdePkg/Library/BaseSynchronizationLib/X64/GccInline.c
> @@ -115,12 +115,10 @@ InternalSyncCompareExchange16 (
>  {
>__asm__ __volatile__ (
>  "lock \n\t"
> -"cmpxchgw%3, %1   \n\t"
> -: "=a" (CompareValue),  // %0
> -  "=m" (*Value) // %1
> -: "a"  (CompareValue),  // %2
> -  "r"  (ExchangeValue), // %3
> -  "m"  (*Value) // %4
> +"cmpxchgw%2, %1   \n\t"
> +: "+a" (CompareValue),  // %0
> +  "+m" (*Value) // %1
> +: "r"  (ExchangeValue)  // %2
>  : "memory",
>"cc"
>  );
> 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [Patch] BaseTools Python: Migrate Python27 to Python36

2018-10-01 Thread Gary Lin

On Wed, Sep 26, 2018 at 01:02:05PM +, Zhu, Yonghong wrote:
> Hi All,
> 
Hi Yonghong,

> Now we are working on bugzilla 
> 55 
> (https://bugzilla.tianocore.org/show_bug.cgi?id=55)  to migrate BaseTools 
> python tool from Python27 to Python36.  After this migration, the BaseTools 
> will only support Python 3. And for now, user need to install a Python 
> version >= 3.6.
> Here is the link: https://github.com/yzhu52/edk2.git   Branch: Python3_V2
> In this branch, after run edksetup script file, tool auto detect the Python 
> tool that version >= 3.6,  tool would report error if it can't find the 
> python that >= 3.6.
> 
I'm testing the branch and found a minor bug in edksetup.sh that caused
OvmfPkg/build.sh failed to build. I wrote a patch and it works for me.
Where should I send the patch? to the edk2-devel mailinglist directly?
or a pull request in github?

Thanks,

Gary Lin

> Current we still in doing some validation for this migration, and not finish 
> the update for UPT, ECC, EOT, Tests those tools and scripts.
> We already did following on this branch:
> 
> 1.   Remove the "from __future__ import" items
> 
> 2.   Update the xrange to range
> 
> 3.   Update long to int
> 
> 4.   Use input instead of raw_input
> 
> 5.   Not use iteritems(), but use items() directly
> 
> 6.   Remove the super() function argument
> 
> 7.   Remove the itertools usage
> 
> 8.   Fix some open file's read and write function
> 
> 9.   Hand the bytes and str type difference
> 
> 10.   Do some list and iterator update
> 
> 11.   Change the division operation in the expression
> 
> 12.   Use '\n' but not os.linesep
> 
> 13.   Update the BinWrappers script
> 
> 14.   Update edksetup script file
> 
> 
> Your suggestions is highly appreciated.
> 
> Best Regards,
> Zhu Yonghong
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
> 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 1/5] MdePkg/BaseSynchronizationLib GCC: fix whitespace and comments

2018-10-01 Thread Philippe Mathieu-Daudé

On 30/09/2018 00:23, Laszlo Ersek wrote:
> The "GccInline.c" files have some inconsistent whitespace, and missing (or
> incorrect) operand comments. Fix and unify them.
> 
> This patch doesn't change behavior.
> 
> Cc: Liming Gao 
> Cc: Michael D Kinney 
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1208
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Laszlo Ersek 

Reviewed-by: Philippe Mathieu-Daudé 

> ---
>  MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c | 35 ++---
>  MdePkg/Library/BaseSynchronizationLib/X64/GccInline.c  | 53 
> +---
>  2 files changed, 39 insertions(+), 49 deletions(-)
> 
> diff --git a/MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c 
> b/MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c
> index fa2be7f4b35c..1976720ac636 100644
> --- a/MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c
> +++ b/MdePkg/Library/BaseSynchronizationLib/Ia32/GccInline.c
> @@ -39,7 +39,7 @@ InternalSyncIncrement (
>  "movl$1, %%eax  \n\t"
>  "lock   \n\t"
>  "xadd%%eax, %1  \n\t"
> -"inc %%eax  "
> +"inc %%eax  \n\t"
>  : "=a" (Result),  // %0
>"+m" (*Value)   // %1
>  : // no inputs that aren't also outputs
> @@ -48,7 +48,6 @@ InternalSyncIncrement (
>  );
>  
>return Result;
> -
>  }
>  
>  
> @@ -76,10 +75,10 @@ InternalSyncDecrement (
>  "movl$-1, %%eax  \n\t"
>  "lock\n\t"
>  "xadd%%eax, %1   \n\t"
> -"dec %%eax  "
> -: "=a" (Result),  // %0
> -  "+m" (*Value)   // %1
> -: // no inputs that aren't also outputs
> +"dec %%eax   \n\t"
> +: "=a" (Result),   // %0
> +  "+m" (*Value)// %1
> +:  // no inputs that aren't also outputs
>  : "memory",
>"cc"
>  );
> @@ -87,6 +86,7 @@ InternalSyncDecrement (
>return Result;
>  }
>  
> +
>  /**
>Performs an atomic compare exchange operation on a 16-bit unsigned integer.
>  
> @@ -113,15 +113,13 @@ InternalSyncCompareExchange16 (
>IN  UINT16ExchangeValue
>)
>  {
> -
>__asm__ __volatile__ (
> -" \n\t"
>  "lock \n\t"
>  "cmpxchgw%1, %2   \n\t"
> -: "=a" (CompareValue)
> -: "q"  (ExchangeValue),
> -  "m"  (*Value),
> -  "0"  (CompareValue)
> +: "=a" (CompareValue)   // %0
> +: "q"  (ExchangeValue), // %1
> +  "m"  (*Value),// %2
> +  "0"  (CompareValue)   // %3
>  : "memory",
>"cc"
>  );
> @@ -129,6 +127,7 @@ InternalSyncCompareExchange16 (
>return CompareValue;
>  }
>  
> +
>  /**
>Performs an atomic compare exchange operation on a 32-bit unsigned integer.
>  
> @@ -155,15 +154,13 @@ InternalSyncCompareExchange32 (
>IN  UINT32ExchangeValue
>)
>  {
> -
>__asm__ __volatile__ (
> -" \n\t"
>  "lock \n\t"
>  "cmpxchgl%1, %2   \n\t"
> -: "=a" (CompareValue) // %0
> -: "q"  (ExchangeValue),   // %1
> -  "m"  (*Value),  // %2
> -  "0"  (CompareValue) // %4
> +: "=a" (CompareValue)   // %0
> +: "q"  (ExchangeValue), // %1
> +  "m"  (*Value),// %2
> +  "0"  (CompareValue)   // %3
>  : "memory",
>"cc"
>  );
> @@ -171,6 +168,7 @@ InternalSyncCompareExchange32 (
>return CompareValue;
>  }
>  
> +
>  /**
>Performs an atomic compare exchange operation on a 64-bit unsigned integer.
>  
> @@ -197,7 +195,6 @@ InternalSyncCompareExchange64 (
>)
>  {
>__asm__ __volatile__ (
> -"   \n\t"
>  "push%%ebx  \n\t"
>  "movl%2,%%ebx   \n\t"
>  "lock   \n\t"
> diff --git a/MdePkg/Library/BaseSynchronizationLib/X64/GccInline.c 
> b/MdePkg/Library/BaseSynchronizationLib/X64/GccInline.c
> index ab7efe23c4db..0212798d7a27 100644
> --- a/MdePkg/Library/BaseSynchronizationLib/X64/GccInline.c
> +++ b/MdePkg/Library/BaseSynchronizationLib/X64/GccInline.c
> @@ -39,7 +39,7 @@ InternalSyncIncrement (
>  "movl$1, %%eax  \n\t"
>  "lock   \n\t"
>  "xadd%%eax, %1  \n\t"
> -"inc %%eax  "
> +"inc %%eax  \n\t"
>  : "=a" (Result),  // %0
>"+m" (*Value)   // %1
>  : // no inputs that aren't also outputs
> @@ -75,10 +75,10 @@ InternalSyncDecrement (
>  "movl$-1, %%eax  \n\t"
>  "lock\n\t"
>  "xadd%%eax, %1   \n\t"
> -"dec %%eax  "
> -: "=a" (Result),  // %0
> -  "+m" (*Value)   // %1
> -: // no inputs that aren't also outputs
> +"dec %%eax   \n\t"
> +

Re: [edk2] [PATCH] MdeModulePkg:disable wraning to pass gcc4.8 build

2018-10-01 Thread Tomas Pilar (tpilar)

Wouldn't it be better to fix the warnings in code rather than silencing them?

Tom

On 29/09/18 02:55, Dongao Guo wrote:
> Change-Id: I782962e4994a8edf14beb7ede8b1aabe233dc3a8
> Contributed-under: TianoCore Contribution Agreement 1.1
> ---
>  MdeModulePkg/Universal/RegularExpressionDxe/RegularExpressionDxe.inf | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git 
> a/MdeModulePkg/Universal/RegularExpressionDxe/RegularExpressionDxe.inf 
> b/MdeModulePkg/Universal/RegularExpressionDxe/RegularExpressionDxe.inf
> index 16e91bd..07bc02e 100644
> --- a/MdeModulePkg/Universal/RegularExpressionDxe/RegularExpressionDxe.inf
> +++ b/MdeModulePkg/Universal/RegularExpressionDxe/RegularExpressionDxe.inf
> @@ -109,3 +109,6 @@
>  
># Oniguruma: error: variable 'fp' set but not used
>GCC:*_*_*_CC_FLAGS = -Wno-error=unused-but-set-variable
> +
> +  # Oniguruma: tag_end in parse_callout_of_name
> +  GCC:*_*_*_CC_FLAGS = -Wno-error=maybe-uninitialized

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

[edk2] [PATCH] BaseTools/edksetup.sh: Handle the return value from grep

[edk2] GenFds broken in latest basetools-win32

Re: [edk2] [PATCH 5/5] MdePkg/BaseSynchronizationLib GCC: simplify IA32 InternalSyncCompareExchange64()

Re: [edk2] [PATCH 5/5] MdePkg/BaseSynchronizationLib GCC: simplify IA32 InternalSyncCompareExchange64()

Re: [edk2] [PATCH] MdeModulePkg:disable wraning to pass gcc4.8 build

Re: [edk2] [PATCH] MdeModulePkg:disable wraning to pass gcc4.8 build

Re: [edk2] [PATCH v1 1/1] OvmfPkg/PlatformPei: clear CPU caches

Re: [edk2] [PATCH v1 1/1] OvmfPkg/PlatformPei: clear CPU caches

Re: [edk2] [Patch] BaseTools Python: Migrate Python27 to Python36

Re: [edk2] [PATCH] ShellPkg ShellLib.h: Fix wrong parameter name for ShellOpenFileByName

[edk2] [PATCH v1 1/1] OvmfPkg/PlatformPei: clear CPU caches

Re: [edk2] [PATCH 4/5] MdePkg/BaseSynchronizationLib GCC: fix X64 InternalSyncCompareExchange64()

Re: [edk2] [PATCH 3/5] MdePkg/BaseSynchronizationLib GCC: fix InternalSyncCompareExchange32()

Re: [edk2] [PATCH 2/5] MdePkg/BaseSynchronizationLib GCC: fix InternalSyncCompareExchange16()

Re: [edk2] [Patch] BaseTools Python: Migrate Python27 to Python36

Re: [edk2] [PATCH 1/5] MdePkg/BaseSynchronizationLib GCC: fix whitespace and comments

Re: [edk2] [PATCH] MdeModulePkg:disable wraning to pass gcc4.8 build

17 matches

Site Navigation

Mail list logo

Footer information