date:20180605

Re: [edk2] How to Interpret ReadKeyStrokeEX Data

2018-06-05 Thread Rothman, Michael A

Jim,

I think the problem you're seeing is that the USB keyboard driver you're using 
is downrev and needs to be updated.

If you look at 
https://github.com/tianocore/edk2/commit/dd190645eb43424706eb1709d0032c69a1935d9f
 there was a fix checked in to address exactly the issue you're running into. 
It's basically a symptom of running a new shell without a correspondingly 
updated keyboard driver. The new shell in effect exposed a latent bug.

Hope that helps.

Thanks,
Mike Rothman 
(迈克 罗斯曼 / माइकल रोथ्मेन् / Михаил Ротман / משה רוטמן)
רועה עיקרי של חתולים

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Rothman, 
Michael A
Sent: Monday, June 4, 2018 10:31 AM
To: jim.dai...@dell.com; Ni, Ruiyu 
Cc: Carsey, Jaben ; edk2-devel@lists.01.org; 
fel...@mail.ru
Subject: Re: [edk2] How to Interpret ReadKeyStrokeEX Data

Since I'm largely the person who might be to blame for the language and intent 
here and I’ll focus on the spec-related item.  See my comments below.



Thanks,

Mike Rothman

(迈克 罗斯曼 / माइकल रोथ्मेन् / Михаил Ротман / משה רוטמן)

רועה עיקרי של חתולים





-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
jim.dai...@dell.com
Sent: Friday, June 1, 2018 11:27 AM
To: Ni, Ruiyu 
Cc: Carsey, Jaben ; edk2-devel@lists.01.org; 
fel...@mail.ru
Subject: [edk2] How to Interpret ReadKeyStrokeEX Data



(Subject changed)



I guess this is a question of UEFI spec interpretation.  In the Console I/O 
Protocol description of Version 2.5 of the spec (page 456), it says:



If the Scan Code is set to 0x00 then the Unicode character is

valid and should be used.



To me that clearly says it all.  The shift modifier is a don't care when the 
scan code is zero.  And, this change in the shell code seems to be a violation 
of that statement.



However, I also see some confusing (and grammatically incorrect) text in the 
description of the ReadKeyStrokeEx() function of the simple text in protocol 
that I am guessing is related to this change (*emphasis* mine):



When interpreting the data from this function, it should be

noted that if a class of printable characters that are normally

*adjusted* by shift modifiers (e.g. Shift Key + "f" key) would

be presented solely as a KeyData.Key.UnicodeChar without the

associated shift state.



What I think the spec is trying to say here is that for A-Z and a-z, the 
consumer does NOT need to look at the shift state to tell "A" from "a", for 
example, because the Unicode character will be either "A" or "a" as appropriate.



n  No, it is any key that would create a displayable character that adjusts 
based on the shift state. Realize that the users of 
ReadKeyStroke/ReadKeyStrokeEx fall back to a common denominator of Scan Code or 
Unicode Character. So if someone types a shift 4, the underlying keyboard 
layout that the keyboard driver controls would dictate how that gets 
translated. On my keyboard in the US it turns into a “$” symbol, while someone 
in Europe may very well have a software-defined keyboard layout which 
translates the same keystroke to a “€” symbol. That of course applies to the 
many characters you specified (A-F, a-f) and many others.

n  The text above is intended to imply what it says, “a class of printable 
characters … would be presented solely as a KeyData.Key.UnicodeChar without the 
associated shift state. This makes consumers of both the Ex and Non-Ex variant 
of ReadKeyStroke able to use the same logic to test for any shiftable 
characters by simply looking at the Unicode value. I’d note the shift and 
toggle states (which are only available on Ex) are there not so much for 
interpreting the translated key but to maximize flexibility associated with 
keyboard mapping as a UEFI application.



I think saying this is unnecessary simply because the earlier statement (If the 
Scan Code is set to 0x00 then the Unicode character is valid and should be 
used.) covers this case.



Further, I believe this text applies only to the A-Z keys because their 
corresponding characters are *adjusted* (to upper case) when the shift key is 
pressed. That is, if you adjust "blue" to "BLUE", you have two different 
appearances, but only one meaning.



However, a "3" is not *adjusted* to a "#"; they are totally different 
characters with different meanings altogether. No C pre-processor would be 
happy seeing: "3ifdef SYMBOL".



In any case, I see nothing gained by ignoring keys having a zero scan code and 
a valid Unicode character; the spec says that "the Unicode character is valid 
and should be used".



Regards,

Jim



> -Original Message-

> From: Ni, Ruiyu [mailto:ruiyu...@intel.com]

> Sent: Thursday, May 31, 2018 7:19 PM

> To: Dailey, Jim

> Cc: Carsey, Jaben; fel...@mail.ru; 
> edk2-devel@lists.01.org

> Subject: RE: [edk2] [PATCH] ShellPkg/[hex]edit: use

Re: [edk2] [patch 2/2] ShellPkg/UefiShellAcpiViewCommandLib: Fix ECC issues

2018-06-05 Thread Bi, Dandan

Hi Alexei,

Current in the Edk2 implementation, the guard macros in the include header 
files start with underscore and end with underscore.  And the number of 
underscore usually used here is one or two.

And the check tool (ECC) also follow above rule to do the check. So it will 
report error for the ACPIPARSER_H_ 
So do you agree to keep the changes in the patch or update it to_ ACPIPARSER_H_ 
for alignment consideration firstly?

Then there is another topic, we need to make the Check Tool align with CSS, we 
may enhance the check tool or update the Spec.

Thanks,
Dandan
-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Alexei 
Fedorov
Sent: Tuesday, June 05, 2018 5:13 PM
To: Bi, Dandan ; edk2-devel@lists.01.org
Cc: Carsey, Jaben ; Ni, Ruiyu 
Subject: Re: [edk2] [patch 2/2] ShellPkg/UefiShellAcpiViewCommandLib: Fix ECC 
issues

Hi Dandan Bi,


Your patch contains a number of modifications like the one below:

-#ifndef ACPIPARSER_H_
-#define ACPIPARSER_H_
+#ifndef __ACPIPARSER_H__
+#define __ACPIPARSER_H__


which violate CCS

"3.3.2 Include Files"

"4.3.5.4 The names of guard macros shall end with an underscore character."

and the given examples.


Alexei


From: edk2-devel  on behalf of Dandan Bi 

Sent: 05 June 2018 09:33
To: edk2-devel@lists.01.org
Cc: Jaben Carsey; Ruiyu Ni
Subject: [edk2] [patch 2/2] ShellPkg/UefiShellAcpiViewCommandLib: Fix ECC issues

1. Separate variable definition and initialization.
2. Make the variable naming following Edk2 rule.
Naming convention of local variable:
a.First character should be upper case.
b.Must contain lower case characters.
c.No white space characters.

Cc: Sami Mujawar 
Cc: Evan Lloyd 
Cc: Sami Mujawar 
Cc: Evan Lloyd 
Cc: Jaben Carsey 
Cc: Ruiyu Ni 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Dandan Bi 
---
 .../UefiShellAcpiViewCommandLib/AcpiParser.c   | 44 ++--
 .../UefiShellAcpiViewCommandLib/AcpiParser.h   |  6 +--
 .../UefiShellAcpiViewCommandLib/AcpiTableParser.c  | 50 +--  
.../UefiShellAcpiViewCommandLib/AcpiTableParser.h  |  6 +--  
.../Library/UefiShellAcpiViewCommandLib/AcpiView.c | 58 ++  
.../Library/UefiShellAcpiViewCommandLib/AcpiView.h | 16 +++---
 .../Parsers/Dbg2/Dbg2Parser.c  |  5 +-
 .../Parsers/Gtdt/GtdtParser.c  |  5 +-
 .../Parsers/Iort/IortParser.c  | 26 +-
 .../Parsers/Madt/MadtParser.c  |  4 +-
 .../Parsers/Rsdp/RsdpParser.c  | 10 +++-
 .../Parsers/Slit/SlitParser.c  | 44 
 .../Parsers/Spcr/SpcrParser.c  | 10 +++-
 .../Parsers/Srat/SratParser.c  | 21 +---
 .../UefiShellAcpiViewCommandLib.c  |  5 +-
 .../UefiShellAcpiViewCommandLib.h  |  6 +--
 .../UefiShellAcpiViewCommandLib.inf|  3 ++
 17 files changed, 190 insertions(+), 129 deletions(-)

diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c 
b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
index 088575d0144..6d3bc451acd 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
@@ -19,10 +19,19 @@

 STATIC UINT32   gIndent;
 STATIC UINT32   mTableErrorCount;
 STATIC UINT32   mTableWarningCount;

+STATIC ACPI_DESCRIPTION_HEADER_INFO AcpiHdrInfo;
+
+/**
+  An ACPI_PARSER array describing the ACPI header.
+**/
+STATIC CONST ACPI_PARSER AcpiHeaderParser[] = {
+  PARSE_ACPI_HEADER ()
+};
+
 /**
   This function resets the ACPI table error counter to Zero.
 **/
 VOID
 ResetErrorCount (
@@ -112,14 +121,17 @@ VerifyChecksum (
   IN BOOLEAN Log,
   IN UINT8*  Ptr,
   IN UINT32  Length
   )
 {
-  UINTN ByteCount = 0;
-  UINT8 Checksum = 0;
+  UINTN ByteCount;
+  UINT8 Checksum;
   UINTN OriginalAttribute;

+  ByteCount = 0;
+  Checksum = 0;
+
   while (ByteCount < Length) {
 Checksum += *(Ptr++);
 ByteCount++;
   }

@@ -164,15 +176,18 @@ EFIAPI
 DumpRaw (
   IN UINT8* Ptr,
   IN UINT32 Length
   )
 {
-  UINTN ByteCount = 0;
+  UINTN ByteCount;
   UINTN PartLineChars;
-  UINTN AsciiBufferIndex = 0;
+  UINTN AsciiBufferIndex;
   CHAR8 AsciiBuffer[17];

+  ByteCount = 0;
+  AsciiBufferIndex = 0;
+
   Print (L"Address  : 0x%p\n", Ptr);
   Print (L"Length   : %d\n", Length);

   while (ByteCount < Length) {
 if ((ByteCount & 0x0F) == 0) {
@@ -275,11 +290,14 @@ DumpUint64 (
   )
 {
   // Some fields are not aligned and this causes alignment faults
   // on ARM platforms if the compiler generates LDRD instructions.
   // Perform word access so that LDRD instructions are not generated.
-  UINT64 Val = *(UINT32*)(Ptr + sizeof (UINT32));
+  UINT64 Val;
+
+  Val = *(UINT32*)(Ptr + sizeof (UINT32));
+
   Val <<= 32;
   Val |= *(UINT32*)Ptr;

   Print (Format, Val);
 }
@@ -454,17 +472,20 @@

Re: [edk2] [PATCH] MdeModulePkg/Library/BaseSerialPortLib16550: Ensure FIFO Polled Mode

2018-06-05 Thread Duran, Leo

Hi Star,

I came across a 16550 model (simulation) which required clearing IER, and it 
seems that's allowed in the 16650 spec, as noted here:
http://www.ti.com/lit/ds/symlink/pc16550d.pdf

8.4.2 FIFO Polled Mode Operation
With FCR0=1 resetting IER0, IER1, IER2, IER3 or all to zero puts the UART in 
the FIFO Polled Mode of operation.

Thanks,
Leo.

> -Original Message-
> From: Zeng, Star [mailto:star.z...@intel.com]
> Sent: Tuesday, June 05, 2018 7:43 PM
> To: Duran, Leo ; Dong, Eric 
> Cc: edk2-devel@lists.01.org; Zeng, Star 
> Subject: RE: [edk2] [PATCH] MdeModulePkg/Library/BaseSerialPortLib16550:
> Ensure FIFO Polled Mode
> 
> It will be better to have the information that may could be added into the
> commit message.
> 
> 1. Did you meet real issue without this patch?
> 2. what is the default value of IER in your case?
> 
> 
> Thanks,
> Star
> -Original Message-
> From: Duran, Leo [mailto:leo.du...@amd.com]
> Sent: Wednesday, June 6, 2018 5:21 AM
> To: Zeng, Star ; Dong, Eric 
> Cc: edk2-devel@lists.01.org
> Subject: RE: [edk2] [PATCH] MdeModulePkg/Library/BaseSerialPortLib16550:
> Ensure FIFO Polled Mode
> 
> Any updates on this patch?
> 
> Do you require to know my "default value of IER"?
> 
> Thanks,
> Leo.
> 
> -Original Message-
> From: edk2-devel  On Behalf Of Duran,
> Leo
> Sent: Friday, May 25, 2018 8:38 AM
> To: Zeng, Star ; edk2-devel@lists.01.org
> Cc: Dong, Eric ; Zeng, Star 
> Subject: Re: [edk2] [PATCH] MdeModulePkg/Library/BaseSerialPortLib16550:
> Ensure FIFO Polled Mode
> 
> Don''t have access to test platform at this time.
> But will report IER value if I,m able to.
> 
> Leo
> 
> Get Outlook for iOS
> 
> From: Zeng, Star 
> Sent: Friday, May 25, 2018 6:13:16 AM
> To: Duran, Leo; edk2-devel@lists.01.org
> Cc: Dong, Eric; Zeng, Star
> Subject: RE: [edk2] [PATCH] MdeModulePkg/Library/BaseSerialPortLib16550:
> Ensure FIFO Polled Mode
> 
> Reviewed-by: Star Zeng 
> 
> Just a little curious about
> 1. Did you meet real issue without this patch?
> 2. what is the default value of IER in your case?
> 
> 
> Thanks,
> Star
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Leo Duran
> Sent: Friday, May 25, 2018 3:08 AM
> To: edk2-devel@lists.01.org
> Cc: Dong, Eric ; Zeng, Star 
> Subject: [edk2] [PATCH] MdeModulePkg/Library/BaseSerialPortLib16550:
> Ensure FIFO Polled Mode
> 
> Put the UART in FIFO Polled Mode by clearing IER after setting FCR.
> Also, add comments to show DLAB state for registers 0 and 1.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Leo Duran 
> Cc: Star Zeng 
> CC: Eric Dong 
> ---
>  .../BaseSerialPortLib16550/BaseSerialPortLib16550.c  | 16 --
> --
>  1 file changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git
> a/MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.c
> b/MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.c
> index 0ccac96..6532c4d 100644
> ---
> a/MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.c
> +++
> b/MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550
> +++ .c
> @@ -3,6 +3,8 @@
> 
>(C) Copyright 2014 Hewlett-Packard Development Company, L.P.
>Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.
> +  Copyright (c) 2018, AMD Incorporated. 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 @@ -30,10 +32,11 @@  //  // 16550 UART register offsets and
> bitfields  //
> -#define R_UART_RXBUF  0
> -#define R_UART_TXBUF  0
> -#define R_UART_BAUD_LOW   0
> -#define R_UART_BAUD_HIGH  1
> +#define R_UART_RXBUF  0   // LCR_DLAB = 0
> +#define R_UART_TXBUF  0   // LCR_DLAB = 0
> +#define R_UART_BAUD_LOW   0   // LCR_DLAB = 1
> +#define R_UART_BAUD_HIGH  1   // LCR_DLAB = 1
> +#define R_UART_IER1   // LCR_DLAB = 0
>  #define R_UART_FCR2
>  #define   B_UART_FCR_FIFOEBIT0
>  #define   B_UART_FCR_FIFO64   BIT5
> @@ -554,6 +557,11 @@ SerialPortInitialize (
>SerialPortWriteRegister (SerialRegisterBase, R_UART_FCR,
> (UINT8)(PcdGet8 (PcdSerialFifoControl) & (B_UART_FCR_FIFOE |
> B_UART_FCR_FIFO64)));
> 
>//
> +  // Set FIFO Polled Mode by clearing IER after setting FCR  //
> + SerialPortWriteRegister (SerialRegisterBase, R_UART_IER, 0x00);
> +
> +  //
>// Put Modem Control Register(MCR) into its reset state of 0x00.
>//
>SerialPortWriteRegister (SerialRegisterBase, R_UART_MCR, 0x00);
> --
> 2.7.4
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
> ___
> edk2-devel

Re: [edk2] [Patch] BaseTools: Sort PCD by token space first then by PcdCName

2018-06-05 Thread Zhu, Yonghong

Reviewed-by: Yonghong Zhu  

Best Regards,
Zhu Yonghong


-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Yonghong 
Zhu
Sent: Tuesday, June 05, 2018 1:31 PM
To: edk2-devel@lists.01.org
Cc: Feng, YunhuaX ; Gao, Liming 
Subject: [edk2] [Patch] BaseTools: Sort PCD by token space first then by 
PcdCName

From: Yunhua Feng 

Sort PCD by token space first, then by PcdCName in the build report.

Cc: Liming Gao 
Cc: Yonghong Zhu 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Yunhua Feng 
---
 BaseTools/Source/Python/build/BuildReport.py | 330 ++-
 1 file changed, 167 insertions(+), 163 deletions(-)

diff --git a/BaseTools/Source/Python/build/BuildReport.py 
b/BaseTools/Source/Python/build/BuildReport.py
index 3c495a6..ad05c4a 100644
--- a/BaseTools/Source/Python/build/BuildReport.py
+++ b/BaseTools/Source/Python/build/BuildReport.py
@@ -925,194 +925,198 @@ class PcdReport(object):
 # For module PCD sub-section
 #
 FileWrite(File, gSubSectionStart)
 FileWrite(File, TAB_BRG_PCD)
 FileWrite(File, gSubSectionSep)
-
+AllPcdDict = {}
 for Key in PcdDict:
+AllPcdDict[Key] = {}
+for Type in PcdDict[Key]:
+for Pcd in PcdDict[Key][Type]:
+AllPcdDict[Key][(Pcd.TokenCName, Type)] = Pcd
+for Key in sorted(AllPcdDict):
 #
 # Group PCD by their token space GUID C Name
 #
 First = True
-for Type in PcdDict[Key]:
+for PcdTokenCName, Type in sorted(AllPcdDict[Key]):
 #
 # Group PCD by their usage type
 #
+Pcd = AllPcdDict[Key][(PcdTokenCName, Type)]
 TypeName, DecType = gPcdTypeMap.get(Type, ("", Type))
-for Pcd in PcdDict[Key][Type]:
-PcdTokenCName = Pcd.TokenCName
-MixedPcdFlag = False
-if GlobalData.MixedPcd:
-for PcdKey in GlobalData.MixedPcd:
-if (Pcd.TokenCName, Pcd.TokenSpaceGuidCName) in 
GlobalData.MixedPcd[PcdKey]:
-PcdTokenCName = PcdKey[0]
-MixedPcdFlag = True
-if MixedPcdFlag and not ModulePcdSet:
-continue
-#
-# Get PCD default value and their override relationship
-#
-DecDefaultValue = self.DecPcdDefault.get((Pcd.TokenCName, 
Pcd.TokenSpaceGuidCName, DecType))
-DscDefaultValue = self.DscPcdDefault.get((Pcd.TokenCName, 
Pcd.TokenSpaceGuidCName))
-DscDefaultValBak = DscDefaultValue
-DscDefaultValue = self.FdfPcdSet.get((Pcd.TokenCName, 
Key), DscDefaultValue)
-if DscDefaultValue != DscDefaultValBak:
-try:
-DscDefaultValue = 
ValueExpressionEx(DscDefaultValue, Pcd.DatumType, self._GuidDict)(True)
-except BadExpression, DscDefaultValue:
-EdkLogger.error('BuildReport', FORMAT_INVALID, 
"PCD Value: %s, Type: %s" %(DscDefaultValue, Pcd.DatumType))
-
-InfDefaultValue = None
-
-PcdValue = DecDefaultValue
-if DscDefaultValue:
-PcdValue = DscDefaultValue
-if ModulePcdSet is not None:
-if (Pcd.TokenCName, Pcd.TokenSpaceGuidCName, Type) not 
in ModulePcdSet:
-continue
-InfDefault, PcdValue = ModulePcdSet[Pcd.TokenCName, 
Pcd.TokenSpaceGuidCName, Type]
-Pcd.DefaultValue = PcdValue
-if InfDefault == "":
-InfDefault = None
-
-BuildOptionMatch = False
-if GlobalData.BuildOptionPcd:
-for pcd in GlobalData.BuildOptionPcd:
-if (Pcd.TokenSpaceGuidCName, Pcd.TokenCName) == 
(pcd[0], pcd[1]):
-if pcd[2]:
-continue
-PcdValue = pcd[3]
-Pcd.DefaultValue = PcdValue
-BuildOptionMatch = True
-break
+MixedPcdFlag = False
+if GlobalData.MixedPcd:
+for PcdKey in GlobalData.MixedPcd:
+if (Pcd.TokenCName, Pcd.TokenSpaceGuidCName) in 
GlobalData.MixedPcd[PcdKey]:
+PcdTokenCName = PcdKey[0]
+MixedPcdFlag = True
+if MixedPcdFlag and not

Re: [edk2] [PATCH] MdeModulePkg/Library/BaseSerialPortLib16550: Ensure FIFO Polled Mode

2018-06-05 Thread Zeng, Star

It will be better to have the information that may could be added into the 
commit message.

1. Did you meet real issue without this patch?
2. what is the default value of IER in your case?


Thanks,
Star
-Original Message-
From: Duran, Leo [mailto:leo.du...@amd.com] 
Sent: Wednesday, June 6, 2018 5:21 AM
To: Zeng, Star ; Dong, Eric 
Cc: edk2-devel@lists.01.org
Subject: RE: [edk2] [PATCH] MdeModulePkg/Library/BaseSerialPortLib16550: Ensure 
FIFO Polled Mode

Any updates on this patch?

Do you require to know my "default value of IER"?

Thanks,
Leo.

-Original Message-
From: edk2-devel  On Behalf Of Duran, Leo
Sent: Friday, May 25, 2018 8:38 AM
To: Zeng, Star ; edk2-devel@lists.01.org
Cc: Dong, Eric ; Zeng, Star 
Subject: Re: [edk2] [PATCH] MdeModulePkg/Library/BaseSerialPortLib16550: Ensure 
FIFO Polled Mode

Don''t have access to test platform at this time.
But will report IER value if I,m able to.

Leo

Get Outlook for iOS 
From: Zeng, Star 
Sent: Friday, May 25, 2018 6:13:16 AM
To: Duran, Leo; edk2-devel@lists.01.org
Cc: Dong, Eric; Zeng, Star
Subject: RE: [edk2] [PATCH] MdeModulePkg/Library/BaseSerialPortLib16550: Ensure 
FIFO Polled Mode

Reviewed-by: Star Zeng 

Just a little curious about
1. Did you meet real issue without this patch?
2. what is the default value of IER in your case?


Thanks,
Star
-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Leo Duran
Sent: Friday, May 25, 2018 3:08 AM
To: edk2-devel@lists.01.org
Cc: Dong, Eric ; Zeng, Star 
Subject: [edk2] [PATCH] MdeModulePkg/Library/BaseSerialPortLib16550: Ensure 
FIFO Polled Mode

Put the UART in FIFO Polled Mode by clearing IER after setting FCR.
Also, add comments to show DLAB state for registers 0 and 1.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Leo Duran 
Cc: Star Zeng 
CC: Eric Dong 
---
 .../BaseSerialPortLib16550/BaseSerialPortLib16550.c  | 16 
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git 
a/MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.c 
b/MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.c
index 0ccac96..6532c4d 100644
--- a/MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.c
+++ b/MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550
+++ .c
@@ -3,6 +3,8 @@

   (C) Copyright 2014 Hewlett-Packard Development Company, L.P.
   Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.
+  Copyright (c) 2018, AMD Incorporated. 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 @@ -30,10 +32,11 @@  //  // 16550 UART register offsets and bitfields  
//
-#define R_UART_RXBUF  0
-#define R_UART_TXBUF  0
-#define R_UART_BAUD_LOW   0
-#define R_UART_BAUD_HIGH  1
+#define R_UART_RXBUF  0   // LCR_DLAB = 0
+#define R_UART_TXBUF  0   // LCR_DLAB = 0
+#define R_UART_BAUD_LOW   0   // LCR_DLAB = 1
+#define R_UART_BAUD_HIGH  1   // LCR_DLAB = 1
+#define R_UART_IER1   // LCR_DLAB = 0
 #define R_UART_FCR2
 #define   B_UART_FCR_FIFOEBIT0
 #define   B_UART_FCR_FIFO64   BIT5
@@ -554,6 +557,11 @@ SerialPortInitialize (
   SerialPortWriteRegister (SerialRegisterBase, R_UART_FCR, (UINT8)(PcdGet8 
(PcdSerialFifoControl) & (B_UART_FCR_FIFOE | B_UART_FCR_FIFO64)));

   //
+  // Set FIFO Polled Mode by clearing IER after setting FCR  // 
+ SerialPortWriteRegister (SerialRegisterBase, R_UART_IER, 0x00);
+
+  //
   // Put Modem Control Register(MCR) into its reset state of 0x00.
   //
   SerialPortWriteRegister (SerialRegisterBase, R_UART_MCR, 0x00);
--
2.7.4

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

Re: [edk2] [PATCH 1/2] ArmPlatformPkg: PL011 Dynamic clock freq Support

2018-06-05 Thread Udit Kumar



> -Original Message-
> From: Alexei Fedorov [mailto:alexei.fedo...@arm.com]
> Sent: Tuesday, June 5, 2018 6:01 PM
> To: Udit Kumar ; edk2-devel@lists.01.org;
> ard.biesheu...@linaro.org; leif.lindh...@linaro.org
> Subject: RE: [edk2] [PATCH 1/2] ArmPlatformPkg: PL011 Dynamic clock freq
> Support
> 
> Please see my comment in-lined
> 
> > -Original Message-
> > From: edk2-devel  On Behalf Of Udit
> > Kumar
> > Sent: 05 June 2018 00:36
> > To: edk2-devel@lists.01.org; ard.biesheu...@linaro.org;
> > leif.lindh...@linaro.org
> > Subject: [edk2] [PATCH 1/2] ArmPlatformPkg: PL011 Dynamic clock freq
> > Support
> >
> > Some platform support dynamic clocking, Which is controlled by some
> > jumper setting or hardware registers.
> > Result of that PCD PL011UartClkInHz needs to be updated for frequency
> change.
> > This patch implements support for dynamic frequency for
> > PL011 uart.
> > This patch implement NULL lib for such platform where Pcd clock
> > frequency to
> > PL011 can change
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Udit Kumar 
> > ---
> >  .../Include/Library/ArmPlatformClockLib.h  | 32 
> > 
> >  .../ArmPlatformClockLib.inf| 33 
> > 
> >  .../ArmPlatformClockLibNull.c  | 35 
> > ++
> >  3 files changed, 100 insertions(+)
> >  create mode 100644
> > ArmPlatformPkg/Include/Library/ArmPlatformClockLib.h
> >  create mode 100644
> > ArmPlatformPkg/Library/ArmPlatformClockLibNull/ArmPlatformClockLib.inf
> >  create mode 100644
> > ArmPlatformPkg/Library/ArmPlatformClockLibNull/ArmPlatformClockLibNull
> > .c
> >
> > diff --git a/ArmPlatformPkg/Include/Library/ArmPlatformClockLib.h
> > b/ArmPlatformPkg/Include/Library/ArmPlatformClockLib.h
> > new file mode 100644
> > index 000..f9d1425
> > --- /dev/null
> > +++ b/ArmPlatformPkg/Include/Library/ArmPlatformClockLib.h
> > @@ -0,0 +1,32 @@
> > +/** @file
> > +*
> > +*  Copyright 2018 NXP
> > +*
> > +*  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
> > +*
> > +https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fope
> > +nsource.org%2Flicenses%2Fbsd-
> license.php=02%7C01%7Cudit.kumar%40
> >
> +nxp.com%7Cdd365496e05b46d7be4508d5cae0286e%7C686ea1d3bc2b4c6fa9
> 2cd99c
> >
> +5c301635%7C0%7C1%7C636637986464241388=rmypRtJplfoFQHAftihQ
> tfqHQ
> > +IcLO0Xele3IKdab6fM%3D=0
> > +*
> > +*  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 _ARMPLATFORMCLOCKLIB_H_
> > +#define _ARMPLATFORMCLOCKLIB_H_
> > +
> > +
> > +/**
> > +  Return frequency of PL011.
> > +
> > +  If this function return 0 then fixed value in Pcd will be used
> 
> Why cannot this function just return FixedPcdGet32 (PL011UartClkInHz) if
> dynamic clocking is not supported?

Ok, agreed with Ard's comment too 
Thanks
 
> > +
> > +  @return Return frequency of PL011
> > +
> > +**/
> > +UINT32
> > +ArmPlatformGetPL011ClockFreq (
> > +  VOID
> > +  );
> > +
> > +#endif
> > diff --git
> > a/ArmPlatformPkg/Library/ArmPlatformClockLibNull/ArmPlatformClockLib.i
> > nf
> > b/ArmPlatformPkg/Library/ArmPlatformClockLibNull/ArmPlatformClockLib.i
> > nf
> > new file mode 100644
> > index 000..b708ad3
> > --- /dev/null
> > +++ b/ArmPlatformPkg/Library/ArmPlatformClockLibNull/ArmPlatformClockL
> > +++ ib
> > +++ .inf
> > @@ -0,0 +1,33 @@
> > +#/* @file
> > +#  Copyright 2018 NXP
> > +#
> > +#  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 #
> > +https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fope
> > +nsource.org%2Flicenses%2Fbsd-
> license.php=02%7C01%7Cudit.kumar%40
> >
> +nxp.com%7Cdd365496e05b46d7be4508d5cae0286e%7C686ea1d3bc2b4c6fa9
> 2cd99c
> >
> +5c301635%7C0%7C1%7C636637986464241388=rmypRtJplfoFQHAftihQ
> tfqHQ
> > +IcLO0Xele3IKdab6fM%3D=0
> > +#
> > +#  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= 0x0001000A
> > +  BASE_NAME  = ArmPlatformClockLibNull
> > +  FILE_GUID  = af8fef24-afbb-472a-b8b7-13101a79703c
> > +  MODULE_TYPE= BASE
> > +  VERSION_STRING = 1.0
> > +  LIBRARY_CLASS  = ArmPlatformClockLib
> > +
> > +[Packages]
> > +  MdePkg/MdePkg.dec
> > +  MdeModulePkg/MdeModulePkg.dec
> > +  ArmPkg/ArmPkg.dec

Re: [edk2] [PATCH 1/2] ArmPlatformPkg: PL011 Dynamic clock freq Support

2018-06-05 Thread Udit Kumar




> -Original Message-
> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> Sent: Tuesday, June 5, 2018 5:56 PM
> To: Udit Kumar 
> Cc: edk2-devel@lists.01.org; Leif Lindholm 
> Subject: Re: [edk2][PATCH 1/2] ArmPlatformPkg: PL011 Dynamic clock freq
> Support
> 
> On 5 June 2018 at 14:23, Ard Biesheuvel  wrote:
> > On 5 June 2018 at 01:35, Udit Kumar  wrote:
> >> Some platform support dynamic clocking, Which is controlled by some
> >> jumper setting or hardware registers.
> >> Result of that PCD PL011UartClkInHz needs to be updated for frequency
> >> change.
> >> This patch implements support for dynamic frequency for
> >> PL011 uart.
> >> This patch implement NULL lib for such platform where Pcd clock
> >> frequency to PL011 can change
> >>
> >> Contributed-under: TianoCore Contribution Agreement 1.1
> >> Signed-off-by: Udit Kumar 
> >
> > I'm not crazy about this. How exactly are you reading the frequency in
> > your case?
> >
> >> ---
> >>  .../Include/Library/ArmPlatformClockLib.h  | 32 
> >> 
> >>  .../ArmPlatformClockLib.inf| 33 
> >> 
> >>  .../ArmPlatformClockLibNull.c  | 35 
> >> ++
> >>  3 files changed, 100 insertions(+)
> >>  create mode 100644
> >> ArmPlatformPkg/Include/Library/ArmPlatformClockLib.h
> >>  create mode 100644
> >> ArmPlatformPkg/Library/ArmPlatformClockLibNull/ArmPlatformClockLib.in
> >> f  create mode 100644
> >> ArmPlatformPkg/Library/ArmPlatformClockLibNull/ArmPlatformClockLibNul
> >> l.c
> >>
> >
> > ArmPlatformClockLib doesn't make sense, since this is specific to PL011, no?
> >
> >
> >> diff --git a/ArmPlatformPkg/Include/Library/ArmPlatformClockLib.h
> >> b/ArmPlatformPkg/Include/Library/ArmPlatformClockLib.h
> >> new file mode 100644
> >> index 000..f9d1425
> >> --- /dev/null
> >> +++ b/ArmPlatformPkg/Include/Library/ArmPlatformClockLib.h
> >
> > Please add new library classes to the package .dec file
> >
> >> @@ -0,0 +1,32 @@
> >> +/** @file
> >> +*
> >> +*  Copyright 2018 NXP
> >> +*
> >> +*  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
> >> +*
> >> +https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fop
> >> +ensource.org%2Flicenses%2Fbsd-
> license.php=02%7C01%7Cudit.kumar%
> >>
> +40nxp.com%7C1293bf5f16dd456e1a4708d5cadf750d%7C686ea1d3bc2b4c6fa
> 92cd
> >>
> +99c5c301635%7C0%7C0%7C636637983450007594=1qT2ZTJAYabHBvcQ
> kNXnP
> >> +PPQgM038f7hSYUa7r2Hhvo%3D=0
> >> +*
> >> +*  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 _ARMPLATFORMCLOCKLIB_H_
> >> +#define _ARMPLATFORMCLOCKLIB_H_
> >> +
> >> +
> >> +/**
> >> +  Return frequency of PL011.
> >> +
> >> +  If this function return 0 then fixed value in Pcd will be used
> >> +
> >> +  @return Return frequency of PL011
> >> +
> >> +**/
> >> +UINT32
> >> +ArmPlatformGetPL011ClockFreq (
> >> +  VOID
> >> +  );
> >> +
> >> +#endif
> >> diff --git
> >> a/ArmPlatformPkg/Library/ArmPlatformClockLibNull/ArmPlatformClockLib.
> >> inf
> >> b/ArmPlatformPkg/Library/ArmPlatformClockLibNull/ArmPlatformClockLib.
> >> inf
> >> new file mode 100644
> >> index 000..b708ad3
> >> --- /dev/null
> >> +++ b/ArmPlatformPkg/Library/ArmPlatformClockLibNull/ArmPlatformClock
> >> +++ Lib.inf
> >> @@ -0,0 +1,33 @@
> >> +#/* @file
> >> +#  Copyright 2018 NXP
> >> +#
> >> +#  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 #
> >> +https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fop
> >> +ensource.org%2Flicenses%2Fbsd-
> license.php=02%7C01%7Cudit.kumar%
> >>
> +40nxp.com%7C1293bf5f16dd456e1a4708d5cadf750d%7C686ea1d3bc2b4c6fa
> 92cd
> >>
> +99c5c301635%7C0%7C0%7C636637983450007594=1qT2ZTJAYabHBvcQ
> kNXnP
> >> +PPQgM038f7hSYUa7r2Hhvo%3D=0
> >> +#
> >> +#  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= 0x0001000A
> >> +  BASE_NAME  = ArmPlatformClockLibNull
> 
> I forgot: the Null suffix is inappropriate here. Please invent a name that 
> reflects
> the fixed, constant nature of the clock frequency.

Agree, with return of Pcd value, NULL make no sense 
Will update in v2 

Thanks 
 
> >> +  FILE_GUID  = af8fef24-afbb-472a-b8b7-13101a79703c
> >> +  MODULE_TYPE= BASE
> >> +  VERSION_STRING = 1.0
> >>

Re: [edk2] [PATCH 1/2] ArmPlatformPkg: PL011 Dynamic clock freq Support

2018-06-05 Thread Udit Kumar




> -Original Message-
> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> Sent: Tuesday, June 5, 2018 5:54 PM
> To: Udit Kumar 
> Cc: edk2-devel@lists.01.org; Leif Lindholm 
> Subject: Re: [edk2][PATCH 1/2] ArmPlatformPkg: PL011 Dynamic clock freq
> Support
> 
> On 5 June 2018 at 01:35, Udit Kumar  wrote:
> > Some platform support dynamic clocking, Which is controlled by some
> > jumper setting or hardware registers.
> > Result of that PCD PL011UartClkInHz needs to be updated for frequency
> > change.
> > This patch implements support for dynamic frequency for
> > PL011 uart.
> > This patch implement NULL lib for such platform where Pcd clock
> > frequency to PL011 can change
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Udit Kumar 
> 
> I'm not crazy about this. How exactly are you reading the frequency in your
> case?

On our SOC, we have sysclk, which is going to system PLL. System PLL generate 
the
clocks for different IPs. 
Sysclk can be 66MHz to 100Mhz, the value of sysclk is read from FPGA register.
To get IP clock, we need to read system PLL multiplier, Which is derived from 
a programmable hardware configuration called RCW (Reset configuration Word)   

 
> > ---
> >  .../Include/Library/ArmPlatformClockLib.h  | 32
> 
> >  .../ArmPlatformClockLib.inf| 33 
> > 
> >  .../ArmPlatformClockLibNull.c  | 35
> ++
> >  3 files changed, 100 insertions(+)
> >  create mode 100644
> > ArmPlatformPkg/Include/Library/ArmPlatformClockLib.h
> >  create mode 100644
> >
> ArmPlatformPkg/Library/ArmPlatformClockLibNull/ArmPlatformClockLib.inf
> >  create mode 100644
> >
> ArmPlatformPkg/Library/ArmPlatformClockLibNull/ArmPlatformClockLibNull
> > .c
> >
> 
> ArmPlatformClockLib doesn't make sense, since this is specific to PL011, no?

Ok, I will rename it 
 
> 
> > diff --git a/ArmPlatformPkg/Include/Library/ArmPlatformClockLib.h
> > b/ArmPlatformPkg/Include/Library/ArmPlatformClockLib.h
> > new file mode 100644
> > index 000..f9d1425
> > --- /dev/null
> > +++ b/ArmPlatformPkg/Include/Library/ArmPlatformClockLib.h
> 
> Please add new library classes to the package .dec file

Ok 
 
> > @@ -0,0 +1,32 @@
> > +/** @file
> > +*
> > +*  Copyright 2018 NXP
> > +*
> > +*  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
> > +*
> >
> +https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fop
> e
> > +nsource.org%2Flicenses%2Fbsd-
> license.php=02%7C01%7Cudit.kumar%40
> >
> +nxp.com%7Cad5930949e7843640e4e08d5cadf3192%7C686ea1d3bc2b4
> c6fa92cd99c
> >
> +5c301635%7C0%7C0%7C636637982321509959=EjBUXzY%2F0DcM
> OZ22tlOlSK0
> > +tZqANTqrgS%2BtgzLXLxrY%3D=0
> > +*
> > +*  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 _ARMPLATFORMCLOCKLIB_H_
> > +#define _ARMPLATFORMCLOCKLIB_H_
> > +
> > +
> > +/**
> > +  Return frequency of PL011.
> > +
> > +  If this function return 0 then fixed value in Pcd will be used
> > +
> > +  @return Return frequency of PL011
> > +
> > +**/
> > +UINT32
> > +ArmPlatformGetPL011ClockFreq (
> > +  VOID
> > +  );
> > +
> > +#endif
> > diff --git
> >
> a/ArmPlatformPkg/Library/ArmPlatformClockLibNull/ArmPlatformClockLib.i
> > nf
> >
> b/ArmPlatformPkg/Library/ArmPlatformClockLibNull/ArmPlatformClockLib.i
> > nf
> > new file mode 100644
> > index 000..b708ad3
> > --- /dev/null
> > +++
> b/ArmPlatformPkg/Library/ArmPlatformClockLibNull/ArmPlatformClockL
> > +++ ib.inf
> > @@ -0,0 +1,33 @@
> > +#/* @file
> > +#  Copyright 2018 NXP
> > +#
> > +#  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 #
> >
> +https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fop
> e
> > +nsource.org%2Flicenses%2Fbsd-
> license.php=02%7C01%7Cudit.kumar%40
> >
> +nxp.com%7Cad5930949e7843640e4e08d5cadf3192%7C686ea1d3bc2b4
> c6fa92cd99c
> >
> +5c301635%7C0%7C0%7C636637982321509959=EjBUXzY%2F0DcM
> OZ22tlOlSK0
> > +tZqANTqrgS%2BtgzLXLxrY%3D=0
> > +#
> > +#  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= 0x0001000A
> > +  BASE_NAME  = ArmPlatformClockLibNull
> > +  FILE_GUID  = af8fef24-afbb-472a-b8b7-13101a79703c
> > +  MODULE_TYPE= BASE
> > +  VERSION_STRING = 1.0
> > +

Re: [edk2] TPM 2.0 ACPI tableTPM2 table event log

2018-06-05 Thread Yao, Jiewen

Yes, if you want this feature, you may file a Bugzilla to track it. 
https://bugzilla.tianocore.org/

Thank you
Yao Jiewen

From: Lin, Derek (HPS UEFI Dev) [mailto:derek.l...@hpe.com]
Sent: Tuesday, June 5, 2018 2:55 AM
To: Zhang, Chao B ; Yao, Jiewen 
Cc: edk2-devel@lists.01.org; Spottswood, Jason ; Lin, 
Derek (HPS UEFI Dev) 
Subject: TPM 2.0 ACPI tableTPM2 table event log

Hi Chao and Jiewen,

There are new fields in TPM2 ACPI table for getting TCG event log under OS.

https://trustedcomputinggroup.org/wp-content/uploads/TCG_ACPIGeneralSpecification_v1.20_r8.pdf
Page 11.
Log Area Minimum Length (LAML)
Log Area Start Address (LASA)

I remember they are not existed in older TCG ACPI spec.

Do you have plan to implement it?

Thanks,
Derek

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

Re: [edk2] [PATCH v8 edk-platforms 0/2] add platform boot options

2018-06-05 Thread Leif Lindholm

Thanks Haoijian,

Series Reviewed-by: Leif Lindholm 

Pushed as 9bb89f3323..da3e09e28d.

On Mon, Jun 04, 2018 at 01:44:36PM +0800, Haojian Zhuang wrote:
> Changelog:
> v8:
>   * Rebase for virtual keyboard driver.
> v7:
>   * Fix memory leakage on DevicePath.
> v6:
>   * Remove redundant definition -- "GRUB_FILE_NAME".
> v5:
>   * Avoid to merge device path and grub's file path in driver.
> Merge them directly in DSC file.
>   * Avoid duplicated code to create boot options.
>   * Use goto to handle error condition.
> v4:
>   * Add BootCount parameter in the interface.
> v3:
>   * Move in initilization of boot entry.
>   * Update the name of interface in platform boot manager protocol.
> v2:
>   * Update with platform boot manager protocol.
> 
> Haojian Zhuang (2):
>   Platform/HiKey960: register predefined boot options
>   Platform/HiKey: create 4 boot options
> 
>  Platform/Hisilicon/HiKey/HiKey.dec|  10 +-
>  Platform/Hisilicon/{HiKey/HiKey.dec => HiKey960/HiKey960.dec} |  19 ++-
>  Platform/Hisilicon/HiKey/HiKey.dsc|   7 +
>  Platform/Hisilicon/HiKey960/HiKey960.dsc  |   6 +
>  Platform/Hisilicon/HiKey/HiKeyDxe/HiKeyDxe.inf|  11 ++
>  Platform/Hisilicon/HiKey960/HiKey960Dxe/HiKey960Dxe.inf   |  11 ++
>  Platform/Hisilicon/HiKey/HiKeyDxe/HiKeyDxe.h  |   2 +
>  Platform/Hisilicon/HiKey960/HiKey960Dxe/HiKey960Dxe.h |  20 +--
>  Platform/Hisilicon/HiKey/HiKeyDxe/HiKeyDxe.c  | 164 
> +++
>  Platform/Hisilicon/HiKey960/HiKey960Dxe/HiKey960Dxe.c | 166 
> 
>  10 files changed, 384 insertions(+), 32 deletions(-)
>  copy Platform/Hisilicon/{HiKey/HiKey.dec => HiKey960/HiKey960.dec} (54%)
> 
> Cc: Laszlo Ersek 
> Cc: Leif Lindholm 
> Cc: Ard Biesheuvel 
> -- 
> 2.7.4
> 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v4 0/2] add platform boot manager protocol

2018-06-05 Thread Leif Lindholm

On Mon, Apr 23, 2018 at 10:15:41PM +0200, Laszlo Ersek wrote:
> On 04/23/18 08:21, Haojian Zhuang wrote:
> > Changelog:
> > v4:
> >   * Add BootCount parameter in the interface.
> >   * Clean the logic on boot options according to Laszlo's comment.
> > v3:
> >   * Update the name of interface.
> >   * Move the initialization into platform driver.
> >   * Fix comment style.
> >   * Fix minor issues with comments.
> > v2:
> >   * Avoid to use hardcoding value. Create boot options by functions.
> > 
> > Haojian Zhuang (2):
> >   EmbeddedPkg: add platform boot manager protocol
> >   ArmPkg/PlatformBootManagerLib: load platform boot options
> > 
> >  ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c | 103 
> > +
> >  .../PlatformBootManagerLib.inf |   2 +
> >  EmbeddedPkg/EmbeddedPkg.dec|   1 +
> >  EmbeddedPkg/Include/Protocol/PlatformBootManager.h |  86 +
> >  4 files changed, 192 insertions(+)
> >  create mode 100644 EmbeddedPkg/Include/Protocol/PlatformBootManager.h
> > 
> 
> (CC Leif, Ard)
> 
> For the series:
> Reviewed-by: Laszlo Ersek 

Thanks for the review Lazslo.

Reviewed-by: Leif Lindholm 

Series pushed as 76022b02e8..1b6e7633ca.

I will add that the bits that go into EmbeddedPkg here do so for no
other reason than "we don't have a better place for them, and they're
certainly not ARM-specific". The case for a generic UtilityPkg is growing.

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

Re: [edk2] [PATCH] ArmVirtPkg: implement KVM safe IoLib instance

2018-06-05 Thread Laszlo Ersek

On 06/05/18 13:05, Ard Biesheuvel wrote:
> KVM on ARM refuses to decode load/store instructions used to perform
> I/O to emulated devices, and instead relies on the exception syndrome
> information to describe the operand register, access size, etc.
> This is only possible for instructions that have a single input/output
> register (as opposed to ones that increment the offset register, or
> load/store pair instructions, etc). Otherwise, QEMU crashes with the
> following error
> 
>   error: kvm run failed Function not implemented
>   R00=01010101 R01=0008 R02=0048 R03=08000820
>   R04=0120 R05=7faaa0e0 R06=7faaa0dc R07=7faaa0e8
>   R08=7faaa0ec R09=7faaa088 R10=00ff R11=0080
>   R12=ff00 R13=7fccfe08 R14=7faa835f R15=7faa887c
>   PSR=81f3 N--- T svc32
>   QEMU: Terminated
> 
> and KVM produces a warning such as the following in the kernel log
> 
>   kvm [17646]: load/store instruction decoding not implemented
> 
> The IoLib implementation provided by MdePkg/Library/BaseIoLibIntrinsic
> is based on C code, and when LTO is in effect, the MMIO accesses could
> be merged with, e.g., manipulations of the loop counter, producing
> opcodes that KVM does not support for emulated MMIO.
> 
> So instead, let's reimplement IoLib in a KVM safe manner.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 
> ---
> Yet another approach for the KVM MMIO emulation issue. Note that this one
> (as well as the MdePkg) affect both AArch64 and ARM. This is deliberate,
> given that there is no reason AArch64 should be immune to this: we simply
> haven't triggered the issue yet.
> 
>  ArmVirtPkg/Library/ArmVirtIoLib/AArch64/ArmVirtMmio.S |  164 ++
>  ArmVirtPkg/Library/ArmVirtIoLib/Arm/ArmVirtMmio.S |  154 ++
>  ArmVirtPkg/Library/ArmVirtIoLib/Arm/ArmVirtMmio.asm   |  165 ++
>  ArmVirtPkg/Library/ArmVirtIoLib/ArmVirtIoLib.c|  589 +
>  ArmVirtPkg/Library/ArmVirtIoLib/ArmVirtIoLib.h|  188 ++
>  ArmVirtPkg/Library/ArmVirtIoLib/ArmVirtIoLib.inf  |   49 +
>  ArmVirtPkg/Library/ArmVirtIoLib/IoHighLevel.c | 2358 
> 
>  ArmVirtPkg/Library/ArmVirtIoLib/IoLibMmioBuffer.c |  413 
>  8 files changed, 4080 insertions(+)

(1) I believe the edk2 nomenclature for library instances would suggest
we call this "BaseIoLibArmVirt" or "BaseIoLibKvm". However, I notice a
great many of our library instances under ArmVirtPkg/Library don't
follow that already, whereas we have several instances starting with
"ArmVirt*". So I value the consistency here; OK.

(2) I've compared some files:

(2a) BaseIoLibIntrinsic.inf <-> ArmVirtIoLib.inf:

- blurb updated,
- copyright notice updated,
- text rewrapped,
- INF_VERSION good,
- BASE_NAME good,
- FILE_GUID unique,
- VALID_ARCHITECTURES good,
- [Sources] etc good

OK.

(2b) IoLibMmioBuffer.c <-> IoLibMmioBuffer.c:

- copyright notice updated,
- text rewrapped,
- some trailing whitespace stripped,
- ArmVirtIoLib.h included rather than BaseIoLibIntrinsicInternal.h,

OK.

(2c) IoHighLevel.c <-> IoHighLevel.c:

- same as (2b),
- plus the reference to "IoLib instances" in the blurb has been updated
  to "MdePkg IoLib instances".

OK.

(2d) BaseIoLibIntrinsicInternal.h <-> ArmVirtIoLib.h:

- kept the common includes (see (2b) and (2c) #includes)
- added function declarations for the assembly functions

OK.

(2e) IoLibArm.c <-> ArmVirtIoLib.c:

This replaces the volatile pointer de-references with calls to the
assembly functions, which seems fine. However, some ASSERT()s about
alignment appear removed (in "read" functions only); is that
intentional? See MmioRead16(), MmioRead32().

Curiously, MmioRead64() preserves the ASSERT().

Otherwise, OK.

(3) New files (the assembly files): Question: why does ARM provide DMB
vs. DSB instructions? Answer: so that ARM experts can have a good
discussion about them every time they show up in new code. :P

(I mean, what chance do mere mortals have to get them right?...)

(4) When we added SEV customizations to IoLib (see
"BaseIoLibIntrinsicSev.inf" / commit b6d11d7c4678), we added those to
MdePkg. Can you please *briefly* investigate whether similar to commit
b6d11d7c4678 would be possible here?

I.e., introduce a new INF file called BaseIoLibIntrinsicArmVirt.inf, and
add "IoLibArmVirt.c" alongside the preexistent "IoLibArm.c". We already
have subdirs for Ia32 and X64 *.nasm, we could create Arm and AArch64
dirs as siblings. This way we could reuse "IoLibMmioBuffer.c" and
"IoHighLevel.c", perhaps?

Anyway I'm not trying to slow down this development by suggesting we add
it to MdePkg. So, if it ends up being too complex for MdePkg, I'm fine
with this patch, Package-wise.

In summary:
- Are the alignment ASSERT() removals in MmioRead16() and MmioRead32()
intentional?
- Can we make one *quick* attempt to put this into MdePkg, if you think
that's feasible?

Thanks!
Laszlo
___
edk2-devel mailing list

[edk2] [Patch] BaseTools: Display both Hex and integer value format of PCD value

2018-06-05 Thread Yonghong Zhu

From: Yunhua Feng 

If the PCD's datum type is UINT8, UINT16, UINT32 or UINT64, then in
the report will display both hexadecimal format and integer format
of PCD value.

Cc: Liming Gao 
Cc: Yonghong Zhu 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Yunhua Feng 
---
 BaseTools/Source/Python/build/BuildReport.py | 40 
 1 file changed, 40 insertions(+)

diff --git a/BaseTools/Source/Python/build/BuildReport.py 
b/BaseTools/Source/Python/build/BuildReport.py
index ad05c4a..61ea645 100644
--- a/BaseTools/Source/Python/build/BuildReport.py
+++ b/BaseTools/Source/Python/build/BuildReport.py
@@ -1138,29 +1138,44 @@ class PcdReport(object):
 if IsByteArray:
 FileWrite(File, '%*s = %s' % (self.MaxLen + 19, 'DSC 
DEFAULT', "{"))
 for Array in ArrayList:
 FileWrite(File, '%s' % (Array))
 else:
+if Pcd.DatumType in TAB_PCD_CLEAN_NUMERIC_TYPES:
+if Value.startswith(('0x', '0X')):
+Value = '{} ({:d})'.format(Value, int(Value, 0))
+else:
+Value = "0x{:X} ({})".format(int(Value, 0), Value)
 FileWrite(File, '%*s = %s' % (self.MaxLen + 19, 'DSC 
DEFAULT', Value))
 if not InfMatch and InfDefaultValue is not None:
 Value = InfDefaultValue.strip()
 IsByteArray, ArrayList = ByteArrayForamt(Value)
 if IsByteArray:
 FileWrite(File, '%*s = %s' % (self.MaxLen + 19, 'INF 
DEFAULT', "{"))
 for Array in ArrayList:
 FileWrite(File, '%s' % (Array))
 else:
+if Pcd.DatumType in TAB_PCD_CLEAN_NUMERIC_TYPES:
+if Value.startswith(('0x', '0X')):
+Value = '{} ({:d})'.format(Value, int(Value, 0))
+else:
+Value = "0x{:X} ({})".format(int(Value, 0), Value)
 FileWrite(File, '%*s = %s' % (self.MaxLen + 19, 'INF 
DEFAULT', Value))
 
 if not DecMatch and DecDefaultValue is not None:
 Value = DecDefaultValue.strip()
 IsByteArray, ArrayList = ByteArrayForamt(Value)
 if IsByteArray:
 FileWrite(File, '%*s = %s' % (self.MaxLen + 19, 'DEC 
DEFAULT', "{"))
 for Array in ArrayList:
 FileWrite(File, '%s' % (Array))
 else:
+if Pcd.DatumType in TAB_PCD_CLEAN_NUMERIC_TYPES:
+if Value.startswith(('0x', '0X')):
+Value = '{} ({:d})'.format(Value, int(Value, 0))
+else:
+Value = "0x{:X} ({})".format(int(Value, 0), Value)
 FileWrite(File, '%*s = %s' % (self.MaxLen + 19, 'DEC 
DEFAULT', Value))
 if IsStructure:
 self.PrintStructureInfo(File, Pcd.DefaultValues)
 if DecMatch and IsStructure:
 self.PrintStructureInfo(File, Pcd.DefaultValues)
@@ -1172,10 +1187,15 @@ class PcdReport(object):
 if IsByteArray:
 FileWrite(File, ' %-*s   : %6s %10s = %s' % (self.MaxLen, Flag 
+ ' ' + PcdTokenCName, TypeName, '(' + Pcd.DatumType + ')', '{'))
 for Array in ArrayList:
 FileWrite(File, '%s' % (Array))
 else:
+if Pcd.DatumType in TAB_PCD_CLEAN_NUMERIC_TYPES:
+if Value.startswith(('0x','0X')):
+Value = '{} ({:d})'.format(Value, int(Value, 0))
+else:
+Value = "0x{:X} ({})".format(int(Value, 0), Value)
 FileWrite(File, ' %-*s   : %6s %10s = %s' % (self.MaxLen, Flag 
+ ' ' + PcdTokenCName, TypeName, '(' + Pcd.DatumType + ')', Value))
 if IsStructure:
 OverrideValues = Pcd.SkuOverrideValues
 if OverrideValues:
 Keys = OverrideValues.keys()
@@ -1208,10 +1228,15 @@ class PcdReport(object):
 else:
 FileWrite(File, ' %-*s   : %6s %10s 
%10s %10s = %s' % (self.MaxLen, Flag + ' ' + PcdTokenCName, TypeName, '(' + 
Pcd.DatumType + ')', '(' + SkuIdName + ')', '(' + DefaultStore + ')', '{'))
 for Array in ArrayList:
 FileWrite(File, '%s' % (Array))
 else:
+if Pcd.DatumType in 
TAB_PCD_CLEAN_NUMERIC_TYPES:
+if Value.startswith(('0x', '0X')):
+Value = '{} ({:d})'.format(Value, 
int(Value, 0))
+else:
+Value = "0x{:X} 
({})".format(int(Value, 0), Value)
 if

Re: [edk2] [PATCH 1/2] ArmPlatformPkg: PL011 Dynamic clock freq Support

2018-06-05 Thread Alexei Fedorov

Please see my comment in-lined

> -Original Message-
> From: edk2-devel  On Behalf Of Udit Kumar
> Sent: 05 June 2018 00:36
> To: edk2-devel@lists.01.org; ard.biesheu...@linaro.org;
> leif.lindh...@linaro.org
> Subject: [edk2] [PATCH 1/2] ArmPlatformPkg: PL011 Dynamic clock freq Support
>
> Some platform support dynamic clocking, Which is controlled by some jumper
> setting or hardware registers.
> Result of that PCD PL011UartClkInHz needs to be updated for frequency change.
> This patch implements support for dynamic frequency for
> PL011 uart.
> This patch implement NULL lib for such platform where Pcd clock frequency to
> PL011 can change
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Udit Kumar 
> ---
>  .../Include/Library/ArmPlatformClockLib.h  | 32 
>  .../ArmPlatformClockLib.inf| 33 
>  .../ArmPlatformClockLibNull.c  | 35 
> ++
>  3 files changed, 100 insertions(+)
>  create mode 100644 ArmPlatformPkg/Include/Library/ArmPlatformClockLib.h
>  create mode 100644
> ArmPlatformPkg/Library/ArmPlatformClockLibNull/ArmPlatformClockLib.inf
>  create mode 100644
> ArmPlatformPkg/Library/ArmPlatformClockLibNull/ArmPlatformClockLibNull.c
>
> diff --git a/ArmPlatformPkg/Include/Library/ArmPlatformClockLib.h
> b/ArmPlatformPkg/Include/Library/ArmPlatformClockLib.h
> new file mode 100644
> index 000..f9d1425
> --- /dev/null
> +++ b/ArmPlatformPkg/Include/Library/ArmPlatformClockLib.h
> @@ -0,0 +1,32 @@
> +/** @file
> +*
> +*  Copyright 2018 NXP
> +*
> +*  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 _ARMPLATFORMCLOCKLIB_H_
> +#define _ARMPLATFORMCLOCKLIB_H_
> +
> +
> +/**
> +  Return frequency of PL011.
> +
> +  If this function return 0 then fixed value in Pcd will be used

Why cannot this function just return FixedPcdGet32 (PL011UartClkInHz) if 
dynamic clocking is not supported?

> +
> +  @return Return frequency of PL011
> +
> +**/
> +UINT32
> +ArmPlatformGetPL011ClockFreq (
> +  VOID
> +  );
> +
> +#endif
> diff --git
> a/ArmPlatformPkg/Library/ArmPlatformClockLibNull/ArmPlatformClockLib.inf
> b/ArmPlatformPkg/Library/ArmPlatformClockLibNull/ArmPlatformClockLib.inf
> new file mode 100644
> index 000..b708ad3
> --- /dev/null
> +++ b/ArmPlatformPkg/Library/ArmPlatformClockLibNull/ArmPlatformClockLib
> +++ .inf
> @@ -0,0 +1,33 @@
> +#/* @file
> +#  Copyright 2018 NXP
> +#
> +#  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= 0x0001000A
> +  BASE_NAME  = ArmPlatformClockLibNull
> +  FILE_GUID  = af8fef24-afbb-472a-b8b7-13101a79703c
> +  MODULE_TYPE= BASE
> +  VERSION_STRING = 1.0
> +  LIBRARY_CLASS  = ArmPlatformClockLib
> +
> +[Packages]
> +  MdePkg/MdePkg.dec
> +  MdeModulePkg/MdeModulePkg.dec
> +  ArmPkg/ArmPkg.dec
> +  ArmPlatformPkg/ArmPlatformPkg.dec
> +
> +[LibraryClasses]
> +  ArmLib
> +  DebugLib
> +
> +[Sources.common]
> +  ArmPlatformClockLibNull.c
> diff --git
> a/ArmPlatformPkg/Library/ArmPlatformClockLibNull/ArmPlatformClockLibNull.c
> b/ArmPlatformPkg/Library/ArmPlatformClockLibNull/ArmPlatformClockLibNull.c
> new file mode 100644
> index 000..28eaa63
> --- /dev/null
> +++ b/ArmPlatformPkg/Library/ArmPlatformClockLibNull/ArmPlatformClockLib
> +++ Null.c
> @@ -0,0 +1,35 @@
> +/** @file
> +*
> +*  Copyright 2018 NXP
> +*
> +*  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 
> +
> +
> +
> +/**
> +  Return clock in for PL011 Uart IP.
> +**/
> +UINT32
> +ArmPlatformGetPL011ClockFreq (
> +  VOID
> +  )
> +{
> +  // Implement platform specific code
> +  //

Re: [edk2] [PATCH 2/2] ArmPlatformPkg: Include ArmPlatformClock Lib

2018-06-05 Thread Ard Biesheuvel

On 5 June 2018 at 01:35, Udit Kumar  wrote:
> This patch includes, ArmPlatformClock in PL011 lib.
>
> In case of NULL implemenation of Clock Lib, Pcd
> value will be used for PL011 frequency.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Udit Kumar 
> ---
>  ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.c   | 7 +--
>  ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.inf | 1 +
>  2 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.c 
> b/ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.c
> index 6aa8063..40fa50a 100644
> --- a/ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.c
> +++ b/ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.c
> @@ -17,11 +17,14 @@
>
>  #include 
>
> +#include 
>  #include 
>  #include 
>  #include 
>  #include 
>
> +
> +

Spurious whitespace changes

>  /** Initialise the serial device hardware with default settings.
>
>@retval RETURN_SUCCESSThe serial device was initialised.
> @@ -48,7 +51,7 @@ SerialPortInitialize (
>
>return PL011UartInitializePort (
> (UINTN)FixedPcdGet64 (PcdSerialRegisterBase),
> -   FixedPcdGet32 (PL011UartClkInHz),
> +   ArmPlatformGetPL011ClockFreq() ? ArmPlatformGetPL011ClockFreq() : 
> FixedPcdGet32 (PL011UartClkInHz),

Get rid of this conditional please

> ,
> ,
> ,
> @@ -156,7 +159,7 @@ SerialPortSetAttributes (
>  {
>return PL011UartInitializePort (
> (UINTN)FixedPcdGet64 (PcdSerialRegisterBase),
> -   FixedPcdGet32 (PL011UartClkInHz),
> +   ArmPlatformGetPL011ClockFreq() ? ArmPlatformGetPL011ClockFreq() : 
> FixedPcdGet32 (PL011UartClkInHz),
> BaudRate,
> ReceiveFifoDepth,
> Parity,
> diff --git a/ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.inf 
> b/ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.inf
> index 3683e06..9820811 100644
> --- a/ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.inf
> +++ b/ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.inf
> @@ -26,6 +26,7 @@
>PL011SerialPortLib.c
>
>  [LibraryClasses]
> +  ArmPlatformClockLib
>PL011UartLib
>PcdLib
>
> --
> 1.9.1
>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 1/2] ArmPlatformPkg: PL011 Dynamic clock freq Support

2018-06-05 Thread Ard Biesheuvel

On 5 June 2018 at 01:35, Udit Kumar  wrote:
> Some platform support dynamic clocking, Which is controlled
> by some jumper setting or hardware registers.
> Result of that PCD PL011UartClkInHz needs to be updated for
> frequency change.
> This patch implements support for dynamic frequency for
> PL011 uart.
> This patch implement NULL lib for such platform where
> Pcd clock frequency to PL011 can change
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Udit Kumar 

I'm not crazy about this. How exactly are you reading the frequency in
your case?

> ---
>  .../Include/Library/ArmPlatformClockLib.h  | 32 
>  .../ArmPlatformClockLib.inf| 33 
>  .../ArmPlatformClockLibNull.c  | 35 
> ++
>  3 files changed, 100 insertions(+)
>  create mode 100644 ArmPlatformPkg/Include/Library/ArmPlatformClockLib.h
>  create mode 100644 
> ArmPlatformPkg/Library/ArmPlatformClockLibNull/ArmPlatformClockLib.inf
>  create mode 100644 
> ArmPlatformPkg/Library/ArmPlatformClockLibNull/ArmPlatformClockLibNull.c
>

ArmPlatformClockLib doesn't make sense, since this is specific to PL011, no?


> diff --git a/ArmPlatformPkg/Include/Library/ArmPlatformClockLib.h 
> b/ArmPlatformPkg/Include/Library/ArmPlatformClockLib.h
> new file mode 100644
> index 000..f9d1425
> --- /dev/null
> +++ b/ArmPlatformPkg/Include/Library/ArmPlatformClockLib.h

Please add new library classes to the package .dec file

> @@ -0,0 +1,32 @@
> +/** @file
> +*
> +*  Copyright 2018 NXP
> +*
> +*  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 _ARMPLATFORMCLOCKLIB_H_
> +#define _ARMPLATFORMCLOCKLIB_H_
> +
> +
> +/**
> +  Return frequency of PL011.
> +
> +  If this function return 0 then fixed value in Pcd will be used
> +
> +  @return Return frequency of PL011
> +
> +**/
> +UINT32
> +ArmPlatformGetPL011ClockFreq (
> +  VOID
> +  );
> +
> +#endif
> diff --git 
> a/ArmPlatformPkg/Library/ArmPlatformClockLibNull/ArmPlatformClockLib.inf 
> b/ArmPlatformPkg/Library/ArmPlatformClockLibNull/ArmPlatformClockLib.inf
> new file mode 100644
> index 000..b708ad3
> --- /dev/null
> +++ b/ArmPlatformPkg/Library/ArmPlatformClockLibNull/ArmPlatformClockLib.inf
> @@ -0,0 +1,33 @@
> +#/* @file
> +#  Copyright 2018 NXP
> +#
> +#  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= 0x0001000A
> +  BASE_NAME  = ArmPlatformClockLibNull
> +  FILE_GUID  = af8fef24-afbb-472a-b8b7-13101a79703c
> +  MODULE_TYPE= BASE
> +  VERSION_STRING = 1.0
> +  LIBRARY_CLASS  = ArmPlatformClockLib
> +
> +[Packages]
> +  MdePkg/MdePkg.dec
> +  MdeModulePkg/MdeModulePkg.dec
> +  ArmPkg/ArmPkg.dec
> +  ArmPlatformPkg/ArmPlatformPkg.dec
> +
> +[LibraryClasses]
> +  ArmLib
> +  DebugLib
> +
> +[Sources.common]
> +  ArmPlatformClockLibNull.c
> diff --git 
> a/ArmPlatformPkg/Library/ArmPlatformClockLibNull/ArmPlatformClockLibNull.c 
> b/ArmPlatformPkg/Library/ArmPlatformClockLibNull/ArmPlatformClockLibNull.c
> new file mode 100644
> index 000..28eaa63
> --- /dev/null
> +++ b/ArmPlatformPkg/Library/ArmPlatformClockLibNull/ArmPlatformClockLibNull.c
> @@ -0,0 +1,35 @@
> +/** @file
> +*
> +*  Copyright 2018 NXP
> +*
> +*  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 
> +
> +
> +
> +/**
> +  Return clock in for PL011 Uart IP.
> +**/
> +UINT32
> +ArmPlatformGetPL011ClockFreq (
> +  VOID
> +  )
> +{
> +  // Implement platform specific code
> +  // and return PL011 freq
> +
> +  // Some platform supports dynamic clocking,
> +  // This function needs to be implemented on platforms which

[edk2] [Patch] Build spec: Add a Common PCD rules section for build report

2018-06-05 Thread Yonghong Zhu

1. Add a Common PCD rules section to introduce 1)the *B, *F, *P, *M's
meaning, 2) the PCD display order, 3) both display hex value and
integer for UINT* type PCD.
2. Update some items to match with code, eg: UEFI Spec Version
3. Update some typo, eg: Fv Name.

Cc: Liming Gao 
Cc: Michael Kinney 
Cc: Kevin W Shaw 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Yonghong Zhu 
---
 .../132_sample_launch_steps_nt32_platform.md   |   4 +-
 13_build_reports/133_output.md |   9 +-
 13_build_reports/134_platform_summary.md   |  58 ++-
 13_build_reports/135_mixed_pcd_section.md  |   8 +-
 13_build_reports/136_global_pcd_section.md |  67 ++---
 13_build_reports/137_fd_section.md |  18 ++--
 13_build_reports/138_module_section.md | 107 +++--
 13_build_reports/README.md |   4 +-
 8 files changed, 122 insertions(+), 153 deletions(-)

diff --git a/13_build_reports/132_sample_launch_steps_nt32_platform.md 
b/13_build_reports/132_sample_launch_steps_nt32_platform.md
index 099dc07..7c73a5d 100644
--- a/13_build_reports/132_sample_launch_steps_nt32_platform.md
+++ b/13_build_reports/132_sample_launch_steps_nt32_platform.md
@@ -1,9 +1,9 @@
 
 >==<
 ```
 
-### 13.4.2 PCDs not used
+### 13.4.3 PCDs not used
 
 If a PCD defined in DSC or FDF file, but the PCD is not used in a conditional
 directive statement and not used by any module, the not used PCD section is
 generated. This is optional section.
 
@@ -126,17 +139,10 @@ report. Only the final value is displayed.
 
 The first line is required:
 
 `[*P|*F|*B] :  () [()][()] 
= `
 
-* `*P` means the Pcd's value was obtained from the DSC file
-* `*F` means the PCD's value was obtained from the FDF file.
-* `*B` means the PCD's value set by a build option.
-* If no `*P`, `*F` or `*B` is shown, the PCD's value comes from DEC file. If 
the
-  value obtained from either a build option, the DSC or FDF is the same as the
-  value in the DEC, then `*B`, `*P` or `*F` will not be shown in the report.
-
 **Note: ** If the Pcd is a Structure PCD, `` is the Struct Name.
 
 Additional lines may be displayed showing default values when the value is not 
a
 default value.
 
@@ -154,10 +160,10 @@ PCDs not used by modules or in conditional directives
 PCD statements
 >--<
 gMyTokenSpaceGuid
 *P SmmEnable   : FEATURE (BOOLEAN) = 0x0
  DEC DEFAULT = 0x1
-*B UsbEnable   : FIXED   (UNIT32) = 0x1
- DEC DEFAULT = 0x0
+*B UsbEnable   : FIXED   (UNIT32) = 0x1 (1)
+ DEC DEFAULT = 0x0 (0)
 <-->
 >==<
 ```
diff --git a/13_build_reports/135_mixed_pcd_section.md 
b/13_build_reports/135_mixed_pcd_section.md
index b09f46c..654f157 100644
--- a/13_build_reports/135_mixed_pcd_section.md
+++ b/13_build_reports/135_mixed_pcd_section.md
@@ -1,9 +1,9 @@
 
 
 ## 13.5 Mixed PCD Section
 
-There is an optional sub-section that, when present, lists the PCDs in the
-platform that use multiple access methods. This sub-section is only present if
+There is an optional section that, when present, lists the PCDs in the
+platform that use multiple access methods. This section is only present if
 there are Binary modules included in the platform build and the binary module
 uses a different PCD access method than other modules in the same platform
 build.
 
-The sub-section header is:
+The section header is:
 
 ```
 >===<
 The following PCDs use different access methods:
 
=
diff --git a/13_build_reports/136_global_pcd_section.md 
b/13_build_reports/136_global_pcd_section.md
index 2a5b682..64f800c 100644
--- a/13_build_reports/136_global_pcd_section.md
+++ b/13_build_reports/136_global_pcd_section.md
@@ -29,19 +29,18 @@
 
 -->
 
 ## 13.6 Global PCD Section
 
-This section contains the information for all PCDs whose values are the same
-for all modules in a platform. The content of global PCD sub-section is grouped
-by token space:
+This section contains the information for all PCDs that used for all modules in
+a platform. The content of global PCD section is grouped by token space:
 
 ```
-gEfiNt32PkgTokenSpaceGuid
+gEfiMdeModulePkgTokenSpaceGuid
 ...
 ...
-gEfiMdeModulePkgTokenSpaceGuid
+gEfiNt32PkgTokenSpaceGuid
 ...
 ...
 ...
 ```
 
@@ -54,29 +53,22 @@ Each global PCD item contains one or more lines:
 
 The first line is required:
 
 `[*P|*F|*B] :  () [()][()] 
= `
 
-*

Re: [edk2] [PATCH edk2-platforms v1] Platform/ARM: Declare FVP Generic Timer Frame #1 as Non-secure

2018-06-05 Thread Leif Lindholm

On Mon, Jun 04, 2018 at 06:44:33PM +0530, Thomas Abraham wrote:
> On Fri, Jun 1, 2018 at 3:04 AM, Leif Lindholm  
> wrote:
> > On Wed, May 23, 2018 at 02:51:34PM +0100, AlexeiFedorov wrote:
> >> From: Alexei Fedorov 
> >>
> >> Programming Reference for Base FVPs describes 2 Generic Memory-mapped
> >> Timer frames with Non-secure access permitted to frame #1.
> >> However ACPI GTDT lists both timer frames #0 and #1 with
> >> Secure Timer flag being set in GTx Common Flags field:
> >>  #define FVP_GTX_COMMON_FLAGS
> >>(GTX_TIMER_SAVE_CONTEXT | GTX_TIMER_SECURE)
> >> Declaring both timer frames as Secure prevents OS running
> >> in Non-secure state from accessing Generic Timer frame #1.
> >>
> >> This patch fixes the above issue by removal of FVP_GTX_COMMON_FLAGS
> >> and adding two FVP_GTX_COMMON_FLAGS_S and FVP_GTX_COMMON_FLAGS_NS
> >> definitions used for Secure frame #0 and Non-secure frame #1
> >> accordingly.
> >>
> >> Contributed-under: TianoCore Contribution Agreement 1.1
> >> Signed-off-by: Alexei Fedorov 
> >
> > The patch looks good to me, so if Thomas agrees:
> > Reviewed-by: Leif Lindholm 
> >
> > Thomas?
> 
> Verified that frame #1 can be marked as accessible from non-secure mode.
> 
> Reviewed-by: Thomas Abraham 

Thanks!

Pushed as 9bb89f3323.

/
Leif

> >
> > /
> > Leif
> >
> >> ---
> >> All the changes can be reviewed at:
> >> https://github.com/AlexeiFedorov/edk2-platforms/tree/262_gtdt_timer_frame_ns_v1
> >>
> >> Notes:
> >> v1:
> >> - remove FVP_GTX_COMMON_FLAGS [Alexei]
> >> - define FVP_GTX_COMMON_FLAGS_S and FVP_GTX_COMMON_FLAGS_NS   [Alexei]
> >> - use FVP_GTX_COMMON_FLAGS_S for timer frame #0 and   [Alexei]
> >>   FVP_GTX_COMMON_FLAGS_NS for frame #1
> >>
> >>  Platform/ARM/VExpressPkg/AcpiTables/Gtdt.aslc | 9 +
> >>  1 file changed, 5 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/Platform/ARM/VExpressPkg/AcpiTables/Gtdt.aslc 
> >> b/Platform/ARM/VExpressPkg/AcpiTables/Gtdt.aslc
> >> index 
> >> 1cb4b498300cf1a08514835677154eace1dd1803..7a0b2b686ec849a8385ac5793e5d5855b9ba83ca
> >>  100644
> >> --- a/Platform/ARM/VExpressPkg/AcpiTables/Gtdt.aslc
> >> +++ b/Platform/ARM/VExpressPkg/AcpiTables/Gtdt.aslc
> >> @@ -1,7 +1,7 @@
> >>  /** @file
> >>  *  Generic Timer Description Table (GTDT)
> >>  *
> >> -*  Copyright (c) 2012 - 2017, ARM Limited. All rights reserved.
> >> +*  Copyright (c) 2012 - 2018, ARM Limited. All rights reserved.
> >>  *  Copyright (c) 2016, Linaro Ltd. All rights reserved
> >>  *
> >>  *  This program and the accompanying materials
> >> @@ -61,7 +61,8 @@
> >>  #define GTX_TIMER_SAVE_CONTEXT  
> >> EFI_ACPI_6_1_GTDT_GT_BLOCK_COMMON_FLAG_ALWAYS_ON_CAPABILITY
> >>  #define GTX_TIMER_LOSE_CONTEXT  0
> >>
> >> -#define FVP_GTX_COMMON_FLAGS(GTX_TIMER_SAVE_CONTEXT | 
> >> GTX_TIMER_SECURE)
> >> +#define FVP_GTX_COMMON_FLAGS_S  (GTX_TIMER_SAVE_CONTEXT | 
> >> GTX_TIMER_SECURE)
> >> +#define FVP_GTX_COMMON_FLAGS_NS (GTX_TIMER_SAVE_CONTEXT | 
> >> GTX_TIMER_NON_SECURE)
> >>
> >>  #define FVP_SBSA_WATCHDOG_REFRESH_BASE 0x2a45
> >>  #define FVP_SBSA_WATCHDOG_CONTROL_BASE 0x2a44
> >> @@ -136,7 +137,7 @@ FVP_GENERIC_TIMER_DESCRIPTION_TABLES Gtdt = {
> >>FVP_GTX_TIMER_FLAGS,  // UINT32 
> >> GTxPhysicalTimerFlags
> >>0,// UINT32 
> >> GTxVirtualTimerGSIV
> >>0,// UINT32 
> >> GTxVirtualTimerFlags
> >> -  FVP_GTX_COMMON_FLAGS  // UINT32 
> >> GTxCommonFlags
> >> +  FVP_GTX_COMMON_FLAGS_S// UINT32 
> >> GTxCommonFlags
> >>  },
> >>  {
> >>1,// UINT8 
> >> GTFrameNumber
> >> @@ -149,7 +150,7 @@ FVP_GENERIC_TIMER_DESCRIPTION_TABLES Gtdt = {
> >>FVP_GTX_TIMER_FLAGS,  // UINT32 
> >> GTxPhysicalTimerFlags
> >>0,// UINT32 
> >> GTxVirtualTimerGSIV
> >>0,// UINT32 
> >> GTxVirtualTimerFlags
> >> -  FVP_GTX_COMMON_FLAGS  // UINT32 
> >> GTxCommonFlags
> >> +  FVP_GTX_COMMON_FLAGS_NS   // UINT32 
> >> GTxCommonFlags
> >>  }
> >>},
> >>  #if (FVP_WATCHDOG_COUNT != 0)
> >> --
> >> 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
> >>
> >>
> > ___
> > 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] MdeModulePkg/Bus/Sd/EmmcDxe: Too verbose debug print on read

2018-06-05 Thread Ard Biesheuvel

On 5 June 2018 at 13:45,   wrote:
> Hi,
>
> My team is developing a board booting Linux from an on-board eMMC,
> and we find that EmmcReadWrite() debug print is too verbose for INFO level.
>
> // 
> https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.c#L904
> DEBUG ((EFI_D_INFO, "Emmc%a(): Part %d Lba ...
>
> Is this an intended level?
> I wonder if the level should be `DEBUG_BLKIO` instead.
>

I agree. I noticed the same, when loading GRUB, kernel and initrd from
eMMC, a significant amount of time is spent in these debug prints
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [platforms PATCH v4 5/5] Marvell/Armada7k8k: Wire up capsule support

2018-06-05 Thread Marcin Wojtas

2018-06-05 13:59 GMT+02:00 Leif Lindholm :
> On Mon, Jun 04, 2018 at 09:13:45PM +0200, Ard Biesheuvel wrote:
>> On 4 June 2018 at 21:08, Leif Lindholm  wrote:
>> > On Mon, Jun 04, 2018 at 08:53:53PM +0200, Marcin Wojtas wrote:
>> >> 2018-06-04 19:46 GMT+02:00 Leif Lindholm :
>> >> >> diff --git a/Silicon/Marvell/Armada7k8k/Armada7k8kCapsule.fdf 
>> >> >> b/Silicon/Marvell/Armada7k8k/Armada7k8kCapsule.fdf
>> >> >> new file mode 100644
>> >> >> index 000..3fe165f
>> >> >> --- /dev/null
>> >> >> +++ b/Silicon/Marvell/Armada7k8k/Armada7k8kCapsule.fdf
>> >> >> @@ -0,0 +1,70 @@
>> >> >> +#
>> >> >> +#  Copyright (C) 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.
>> >> >> +#
>> >> >> +
>> >> >> +[FD.Armada_Capsule]
>> >> >> +BaseAddress   = 0x #|gArmTokenSpaceGuid.PcdFdBaseAddress  # 
>> >> >> The base address of the Firmware in NOR Flash.
>> >> >> +Size  = 0x0040 #|gArmTokenSpaceGuid.PcdFdSize # 
>> >> >> The size in bytes of the FLASH Device
>> >> >> +ErasePolarity = 1
>> >> >> +
>> >> >> +0x|0x0001
>> >> >> +FILE = 
>> >> >> $(WORKSPACE)/$(OUTPUT_DIRECTORY)/$(TARGET)_$(TOOL_CHAIN_TAG)/FV/SYSTEMFIRMWAREDESCRIPTOR.Fv
>> >> >> +
>> >> >> +0x0010|0x0030
>> >> >> +FILE = $(FIRMWARE_IMAGE_PATH)
>> >> >
>> >> > OK, so using it not as a pointer to a directory, this doesn't lose any
>> >> > flexibility compared to a PREFIX. But a _PATH variable is something I
>> >> > expect to be pointing to a directory.
>> >> >
>> >> > I'd be happy without the _PATH suffix - can we drop it?
>> >>
>> >> Sure, I'm fine with that - should I resend?
>> >
>> > Nah, I can fold it in.
>> > If Ard's OK with this one?
>> >
>>
>> All fine with the series now
>>
>> Reviewed-by: Ard Biesheuvel 
>
> Thanks. I think it would be worth considering enabling the capsule
> build on the platforms by default (certainly for MacchiatoBIN once
> that gets submitted). But for now:
>
> Reviewed-by: Leif Lindholm 
> Pushed as 0923068a48..d1d8e5e095.
>

Great, thanks!

Best regards,
Marcin

> /
> Leif
>
>> > Regards,
>> >
>> > Leif
>> >
>> >> Thanks,
>> >> Marcin
>> >>
>> >> > /
>> >> > Leif
>> >> >
>> >> >> +
>> >> >> +[FV.SystemFirmwareUpdateCargo]
>> >> >> +FvAlignment= 8
>> >> >> +ERASE_POLARITY = 1
>> >> >> +MEMORY_MAPPED  = TRUE
>> >> >> +STICKY_WRITE   = TRUE
>> >> >> +LOCK_CAP   = TRUE
>> >> >> +LOCK_STATUS= TRUE
>> >> >> +WRITE_DISABLED_CAP = TRUE
>> >> >> +WRITE_ENABLED_CAP  = TRUE
>> >> >> +WRITE_STATUS   = TRUE
>> >> >> +WRITE_LOCK_CAP = TRUE
>> >> >> +WRITE_LOCK_STATUS  = TRUE
>> >> >> +READ_DISABLED_CAP  = TRUE
>> >> >> +READ_ENABLED_CAP   = TRUE
>> >> >> +READ_STATUS= TRUE
>> >> >> +READ_LOCK_CAP  = TRUE
>> >> >> +READ_LOCK_STATUS   = TRUE
>> >> >> +
>> >> >> +  FILE RAW = b3890e02-c46b-4970-9536-57787a9e06c7 { # 
>> >> >> PcdEdkiiSystemFirmwareFileGuid
>> >> >> + FD = Armada_Capsule
>> >> >> +  }
>> >> >> +
>> >> >> +  FILE RAW = ce57b167-b0e4-41e8-a897-5f4feb781d40 { # 
>> >> >> gEdkiiSystemFmpCapsuleDriverFvFileGuid
>> >> >> +
>> >> >> $(WORKSPACE)/$(OUTPUT_DIRECTORY)/$(TARGET)_$(TOOL_CHAIN_TAG)/FV/CAPSULEDISPATCHFV.Fv
>> >> >> +  }
>> >> >> +
>> >> >> +  FILE RAW = 812136D3-4D3A-433A-9418-29BB9BF78F6E { # 
>> >> >> gEdkiiSystemFmpCapsuleConfigFileGuid
>> >> >> +
>> >> >> Silicon/Marvell/Armada7k8k/Feature/Capsule/SystemFirmwareUpdateConfig/SystemFirmwareUpdateConfig.ini
>> >> >> +  }
>> >> >> +
>> >> >> +[FmpPayload.FmpPayloadSystemFirmwarePkcs7]
>> >> >> +IMAGE_HEADER_INIT_VERSION = 0x02
>> >> >> +IMAGE_TYPE_ID = 757fc475-6b22-4482-868e-ded286f30940 # 
>> >> >> PcdSystemFmpCapsuleImageTypeIdGuid
>> >> >> +IMAGE_INDEX   = 0x1
>> >> >> +HARDWARE_INSTANCE = 0x0
>> >> >> +MONOTONIC_COUNT   = 0x1
>> >> >> +CERTIFICATE_GUID  = 4AAFD29D-68DF-49EE-8AA9-347D375665A7 # 
>> >> >> PKCS7
>> >> >> +
>> >> >> +  FV = SystemFirmwareUpdateCargo
>> >> >> +
>> >> >> +[Capsule.MvFirmwareUpdateCapsuleFmpPkcs7]
>> >> >> +CAPSULE_GUID= 6dcbd5ed-e82d-4c44-bda1-7194199ad92a # 
>> >> >> gEfiFmpCapsuleGuid
>> >> >> +CAPSULE_HEADER_SIZE = 0x20
>> >> >> +CAPSULE_HEADER_INIT_VERSION = 0x1
>> >> >> +
>> >> >> +  FMP_PAYLOAD = FmpPayloadSystemFirmwarePkcs7
>> >> >> +
>> >> >> --
>> >> >> 2.7.4
>> >> >>
___
edk2-devel mailing list
edk2-devel@lists.01.org

[edk2] OSFC 2018

2018-06-05 Thread Philipp Deppenwiese

Dear Ladies and Gentlemen,

We invite you to the Open Source Firmware Conference 2018
( www.osfc.io ) which takes place at the 12th - 15th September
in Erlangen, Germany.

The OSFC 2018 is the first conference focusing exclusively on Open
Source firmware.
Therefore, our mission is to provide an appropriate platform to bring
together as many Open Source projects,
hardware manufacturers and developers as possible.
In order to collaborate, share knowledge and push the Open Source
firmware development.

Get in touch with community of coreboot, LinuxBoot, Tianocore, U-Boot
and be part of our amazing firmware conference!

Tickets are available: https://osfc.io/tickets
CFP is available: https://easychair.org/cfp/osfc2018
Follow us on Twitter: osfc_io


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

Re: [edk2] [platforms PATCH v4 5/5] Marvell/Armada7k8k: Wire up capsule support

2018-06-05 Thread Leif Lindholm

On Mon, Jun 04, 2018 at 09:13:45PM +0200, Ard Biesheuvel wrote:
> On 4 June 2018 at 21:08, Leif Lindholm  wrote:
> > On Mon, Jun 04, 2018 at 08:53:53PM +0200, Marcin Wojtas wrote:
> >> 2018-06-04 19:46 GMT+02:00 Leif Lindholm :
> >> >> diff --git a/Silicon/Marvell/Armada7k8k/Armada7k8kCapsule.fdf 
> >> >> b/Silicon/Marvell/Armada7k8k/Armada7k8kCapsule.fdf
> >> >> new file mode 100644
> >> >> index 000..3fe165f
> >> >> --- /dev/null
> >> >> +++ b/Silicon/Marvell/Armada7k8k/Armada7k8kCapsule.fdf
> >> >> @@ -0,0 +1,70 @@
> >> >> +#
> >> >> +#  Copyright (C) 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.
> >> >> +#
> >> >> +
> >> >> +[FD.Armada_Capsule]
> >> >> +BaseAddress   = 0x #|gArmTokenSpaceGuid.PcdFdBaseAddress  # 
> >> >> The base address of the Firmware in NOR Flash.
> >> >> +Size  = 0x0040 #|gArmTokenSpaceGuid.PcdFdSize # 
> >> >> The size in bytes of the FLASH Device
> >> >> +ErasePolarity = 1
> >> >> +
> >> >> +0x|0x0001
> >> >> +FILE = 
> >> >> $(WORKSPACE)/$(OUTPUT_DIRECTORY)/$(TARGET)_$(TOOL_CHAIN_TAG)/FV/SYSTEMFIRMWAREDESCRIPTOR.Fv
> >> >> +
> >> >> +0x0010|0x0030
> >> >> +FILE = $(FIRMWARE_IMAGE_PATH)
> >> >
> >> > OK, so using it not as a pointer to a directory, this doesn't lose any
> >> > flexibility compared to a PREFIX. But a _PATH variable is something I
> >> > expect to be pointing to a directory.
> >> >
> >> > I'd be happy without the _PATH suffix - can we drop it?
> >>
> >> Sure, I'm fine with that - should I resend?
> >
> > Nah, I can fold it in.
> > If Ard's OK with this one?
> >
> 
> All fine with the series now
> 
> Reviewed-by: Ard Biesheuvel 

Thanks. I think it would be worth considering enabling the capsule
build on the platforms by default (certainly for MacchiatoBIN once
that gets submitted). But for now:

Reviewed-by: Leif Lindholm 
Pushed as 0923068a48..d1d8e5e095.

/
Leif

> > Regards,
> >
> > Leif
> >
> >> Thanks,
> >> Marcin
> >>
> >> > /
> >> > Leif
> >> >
> >> >> +
> >> >> +[FV.SystemFirmwareUpdateCargo]
> >> >> +FvAlignment= 8
> >> >> +ERASE_POLARITY = 1
> >> >> +MEMORY_MAPPED  = TRUE
> >> >> +STICKY_WRITE   = TRUE
> >> >> +LOCK_CAP   = TRUE
> >> >> +LOCK_STATUS= TRUE
> >> >> +WRITE_DISABLED_CAP = TRUE
> >> >> +WRITE_ENABLED_CAP  = TRUE
> >> >> +WRITE_STATUS   = TRUE
> >> >> +WRITE_LOCK_CAP = TRUE
> >> >> +WRITE_LOCK_STATUS  = TRUE
> >> >> +READ_DISABLED_CAP  = TRUE
> >> >> +READ_ENABLED_CAP   = TRUE
> >> >> +READ_STATUS= TRUE
> >> >> +READ_LOCK_CAP  = TRUE
> >> >> +READ_LOCK_STATUS   = TRUE
> >> >> +
> >> >> +  FILE RAW = b3890e02-c46b-4970-9536-57787a9e06c7 { # 
> >> >> PcdEdkiiSystemFirmwareFileGuid
> >> >> + FD = Armada_Capsule
> >> >> +  }
> >> >> +
> >> >> +  FILE RAW = ce57b167-b0e4-41e8-a897-5f4feb781d40 { # 
> >> >> gEdkiiSystemFmpCapsuleDriverFvFileGuid
> >> >> +
> >> >> $(WORKSPACE)/$(OUTPUT_DIRECTORY)/$(TARGET)_$(TOOL_CHAIN_TAG)/FV/CAPSULEDISPATCHFV.Fv
> >> >> +  }
> >> >> +
> >> >> +  FILE RAW = 812136D3-4D3A-433A-9418-29BB9BF78F6E { # 
> >> >> gEdkiiSystemFmpCapsuleConfigFileGuid
> >> >> +
> >> >> Silicon/Marvell/Armada7k8k/Feature/Capsule/SystemFirmwareUpdateConfig/SystemFirmwareUpdateConfig.ini
> >> >> +  }
> >> >> +
> >> >> +[FmpPayload.FmpPayloadSystemFirmwarePkcs7]
> >> >> +IMAGE_HEADER_INIT_VERSION = 0x02
> >> >> +IMAGE_TYPE_ID = 757fc475-6b22-4482-868e-ded286f30940 # 
> >> >> PcdSystemFmpCapsuleImageTypeIdGuid
> >> >> +IMAGE_INDEX   = 0x1
> >> >> +HARDWARE_INSTANCE = 0x0
> >> >> +MONOTONIC_COUNT   = 0x1
> >> >> +CERTIFICATE_GUID  = 4AAFD29D-68DF-49EE-8AA9-347D375665A7 # 
> >> >> PKCS7
> >> >> +
> >> >> +  FV = SystemFirmwareUpdateCargo
> >> >> +
> >> >> +[Capsule.MvFirmwareUpdateCapsuleFmpPkcs7]
> >> >> +CAPSULE_GUID= 6dcbd5ed-e82d-4c44-bda1-7194199ad92a # 
> >> >> gEfiFmpCapsuleGuid
> >> >> +CAPSULE_HEADER_SIZE = 0x20
> >> >> +CAPSULE_HEADER_INIT_VERSION = 0x1
> >> >> +
> >> >> +  FMP_PAYLOAD = FmpPayloadSystemFirmwarePkcs7
> >> >> +
> >> >> --
> >> >> 2.7.4
> >> >>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] MdeModulePkg/Bus/Sd/EmmcDxe: Too verbose debug print on read

2018-06-05 Thread methavanitpong.pipat

Hi,

My team is developing a board booting Linux from an on-board eMMC,
and we find that EmmcReadWrite() debug print is too verbose for INFO level. 

// 
https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.c#L904
DEBUG ((EFI_D_INFO, "Emmc%a(): Part %d Lba ...

Is this an intended level?
I wonder if the level should be `DEBUG_BLKIO` instead. 


Thanks,
--
Pipat Methavanitpong
Software Developer, S-Project 3
Socionext Inc.

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

[edk2] [edk2-platforms][PATCH] Platform: Add ArmPlatformClockNULL Lib

2018-06-05 Thread Udit Kumar

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Meenakshi Aggarwal 
---
 Platform/AMD/OverdriveBoard/OverdriveBoard.dsc   | 1 +
 Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc | 1 +
 Platform/Hisilicon/D05/D05.dsc   | 2 ++
 Platform/Hisilicon/HiKey/HiKey.dsc   | 1 +
 Platform/Hisilicon/HiKey960/HiKey960.dsc | 1 +
 Platform/LeMaker/CelloBoard/CelloBoard.dsc   | 1 +
 Platform/Socionext/DeveloperBox/DeveloperBox.dsc | 1 +
 Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc | 1 +
 Platform/SoftIron/Overdrive1000Board/Overdrive1000Board.dsc  | 1 +
 9 files changed, 10 insertions(+)

diff --git a/Platform/AMD/OverdriveBoard/OverdriveBoard.dsc 
b/Platform/AMD/OverdriveBoard/OverdriveBoard.dsc
index 5e564f6..7d9c062 100644
--- a/Platform/AMD/OverdriveBoard/OverdriveBoard.dsc
+++ b/Platform/AMD/OverdriveBoard/OverdriveBoard.dsc
@@ -106,6 +106,7 @@ DEFINE DO_CAPSULE   = FALSE
   UefiScsiLib|MdePkg/Library/UefiScsiLib/UefiScsiLib.inf
 
   # ARM PL011 UART Driver
+  
ArmPlatformClockLib|ArmPlatformPkg/Library/ArmPlatformClockLibNull/ArmPlatformClockLib.inf
   PL011UartLib|ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.inf
   
SerialPortLib|ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.inf
 
diff --git a/Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc 
b/Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc
index 8a43d31..f6f68e2 100644
--- a/Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc
+++ b/Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc
@@ -87,6 +87,7 @@
   
RealTimeClockLib|ArmPlatformPkg/Library/PL031RealTimeClockLib/PL031RealTimeClockLib.inf
   TimeBaseLib|EmbeddedPkg/Library/TimeBaseLib/TimeBaseLib.inf
   # ARM PL011 UART Driver
+  
ArmPlatformClockLib|ArmPlatformPkg/Library/ArmPlatformClockLibNull/ArmPlatformClockLib.inf
   PL011UartLib|ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.inf
   
SerialPortLib|ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.inf
   TimerLib|ArmPkg/Library/ArmArchTimerLib/ArmArchTimerLib.inf
diff --git a/Platform/Hisilicon/D05/D05.dsc b/Platform/Hisilicon/D05/D05.dsc
index b6e1a9d..83e99a8 100644
--- a/Platform/Hisilicon/D05/D05.dsc
+++ b/Platform/Hisilicon/D05/D05.dsc
@@ -96,6 +96,7 @@
   UefiUsbLib|MdePkg/Library/UefiUsbLib/UefiUsbLib.inf
 
   LpcLib|Silicon/Hisilicon/Hi1610/Library/LpcLib/LpcLib.inf
+  
ArmPlatformClockLib|ArmPlatformPkg/Library/ArmPlatformClockLibNull/ArmPlatformClockLib.inf
   
SerialPortLib|ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.inf
 
 [LibraryClasses.common.SEC]
@@ -104,6 +105,7 @@
 
 [LibraryClasses.common.DXE_RUNTIME_DRIVER]
   I2CLib|Silicon/Hisilicon/Library/I2CLib/I2CLibRuntime.inf
+  
ArmPlatformClockLib|ArmPlatformPkg/Library/ArmPlatformClockLibNull/ArmPlatformClockLib.inf
   
SerialPortLib|ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.inf
 
 [BuildOptions]
diff --git a/Platform/Hisilicon/HiKey/HiKey.dsc 
b/Platform/Hisilicon/HiKey/HiKey.dsc
index 83dd68a..8bee33f 100644
--- a/Platform/Hisilicon/HiKey/HiKey.dsc
+++ b/Platform/Hisilicon/HiKey/HiKey.dsc
@@ -46,6 +46,7 @@
   DxeServicesLib|MdePkg/Library/DxeServicesLib/DxeServicesLib.inf
   BootLogoLib|MdeModulePkg/Library/BootLogoLib/BootLogoLib.inf
 
+  
ArmPlatformClockLib|ArmPlatformPkg/Library/ArmPlatformClockLibNull/ArmPlatformClockLib.inf
   
SerialPortLib|ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.inf
   
RealTimeClockLib|ArmPlatformPkg/Library/PL031RealTimeClockLib/PL031RealTimeClockLib.inf
   TimeBaseLib|EmbeddedPkg/Library/TimeBaseLib/TimeBaseLib.inf
diff --git a/Platform/Hisilicon/HiKey960/HiKey960.dsc 
b/Platform/Hisilicon/HiKey960/HiKey960.dsc
index 79e6875..9b1e213 100644
--- a/Platform/Hisilicon/HiKey960/HiKey960.dsc
+++ b/Platform/Hisilicon/HiKey960/HiKey960.dsc
@@ -45,6 +45,7 @@
   DxeServicesLib|MdePkg/Library/DxeServicesLib/DxeServicesLib.inf
   BootLogoLib|MdeModulePkg/Library/BootLogoLib/BootLogoLib.inf
 
+  
ArmPlatformClockLib|ArmPlatformPkg/Library/ArmPlatformClockLibNull/ArmPlatformClockLib.inf
   
SerialPortLib|ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.inf
   
RealTimeClockLib|ArmPlatformPkg/Library/PL031RealTimeClockLib/PL031RealTimeClockLib.inf
   TimeBaseLib|EmbeddedPkg/Library/TimeBaseLib/TimeBaseLib.inf
diff --git a/Platform/LeMaker/CelloBoard/CelloBoard.dsc 
b/Platform/LeMaker/CelloBoard/CelloBoard.dsc
index 7cd166d..e6857e7 100644
--- a/Platform/LeMaker/CelloBoard/CelloBoard.dsc
+++ b/Platform/LeMaker/CelloBoard/CelloBoard.dsc
@@ -102,6 +102,7 @@ DEFINE DO_FLASHER   = FALSE
   UefiScsiLib|MdePkg/Library/UefiScsiLib/UefiScsiLib.inf
 
   # ARM PL011 UART Driver
+  
ArmPlatformClockLib|ArmPlatformPkg/Library/ArmPlatformClockLibNull/ArmPlatformClockLib.inf
   PL011UartLib|ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.inf
   
SerialPortLib|ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.inf
 
diff --git

[edk2] [PATCH 1/2] ArmPlatformPkg: PL011 Dynamic clock freq Support

2018-06-05 Thread Udit Kumar

Some platform support dynamic clocking, Which is controlled
by some jumper setting or hardware registers.
Result of that PCD PL011UartClkInHz needs to be updated for
frequency change.
This patch implements support for dynamic frequency for
PL011 uart.
This patch implement NULL lib for such platform where
Pcd clock frequency to PL011 can change

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Udit Kumar 
---
 .../Include/Library/ArmPlatformClockLib.h  | 32 
 .../ArmPlatformClockLib.inf| 33 
 .../ArmPlatformClockLibNull.c  | 35 ++
 3 files changed, 100 insertions(+)
 create mode 100644 ArmPlatformPkg/Include/Library/ArmPlatformClockLib.h
 create mode 100644 
ArmPlatformPkg/Library/ArmPlatformClockLibNull/ArmPlatformClockLib.inf
 create mode 100644 
ArmPlatformPkg/Library/ArmPlatformClockLibNull/ArmPlatformClockLibNull.c

diff --git a/ArmPlatformPkg/Include/Library/ArmPlatformClockLib.h 
b/ArmPlatformPkg/Include/Library/ArmPlatformClockLib.h
new file mode 100644
index 000..f9d1425
--- /dev/null
+++ b/ArmPlatformPkg/Include/Library/ArmPlatformClockLib.h
@@ -0,0 +1,32 @@
+/** @file
+*
+*  Copyright 2018 NXP
+*
+*  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 _ARMPLATFORMCLOCKLIB_H_
+#define _ARMPLATFORMCLOCKLIB_H_
+
+
+/**
+  Return frequency of PL011.
+
+  If this function return 0 then fixed value in Pcd will be used
+
+  @return Return frequency of PL011
+
+**/
+UINT32
+ArmPlatformGetPL011ClockFreq (
+  VOID
+  );
+
+#endif
diff --git 
a/ArmPlatformPkg/Library/ArmPlatformClockLibNull/ArmPlatformClockLib.inf 
b/ArmPlatformPkg/Library/ArmPlatformClockLibNull/ArmPlatformClockLib.inf
new file mode 100644
index 000..b708ad3
--- /dev/null
+++ b/ArmPlatformPkg/Library/ArmPlatformClockLibNull/ArmPlatformClockLib.inf
@@ -0,0 +1,33 @@
+#/* @file
+#  Copyright 2018 NXP
+#
+#  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= 0x0001000A
+  BASE_NAME  = ArmPlatformClockLibNull
+  FILE_GUID  = af8fef24-afbb-472a-b8b7-13101a79703c
+  MODULE_TYPE= BASE
+  VERSION_STRING = 1.0
+  LIBRARY_CLASS  = ArmPlatformClockLib
+
+[Packages]
+  MdePkg/MdePkg.dec
+  MdeModulePkg/MdeModulePkg.dec
+  ArmPkg/ArmPkg.dec
+  ArmPlatformPkg/ArmPlatformPkg.dec
+
+[LibraryClasses]
+  ArmLib
+  DebugLib
+
+[Sources.common]
+  ArmPlatformClockLibNull.c
diff --git 
a/ArmPlatformPkg/Library/ArmPlatformClockLibNull/ArmPlatformClockLibNull.c 
b/ArmPlatformPkg/Library/ArmPlatformClockLibNull/ArmPlatformClockLibNull.c
new file mode 100644
index 000..28eaa63
--- /dev/null
+++ b/ArmPlatformPkg/Library/ArmPlatformClockLibNull/ArmPlatformClockLibNull.c
@@ -0,0 +1,35 @@
+/** @file
+*
+*  Copyright 2018 NXP
+*
+*  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 
+
+
+
+/**
+  Return clock in for PL011 Uart IP.
+**/
+UINT32
+ArmPlatformGetPL011ClockFreq (
+  VOID
+  )
+{
+  // Implement platform specific code
+  // and return PL011 freq
+
+  // Some platform supports dynamic clocking,
+  // This function needs to be implemented on platforms which supports
+  // dynamic clocking to avoid re-building of UEFI firmware for PL011
+  // clock change
+  return 0;
+}
-- 
1.9.1

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

[edk2] [PATCH 2/2] ArmPlatformPkg: Include ArmPlatformClock Lib

2018-06-05 Thread Udit Kumar

This patch includes, ArmPlatformClock in PL011 lib.

In case of NULL implemenation of Clock Lib, Pcd
value will be used for PL011 frequency.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Udit Kumar 
---
 ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.c   | 7 +--
 ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.inf | 1 +
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.c 
b/ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.c
index 6aa8063..40fa50a 100644
--- a/ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.c
+++ b/ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.c
@@ -17,11 +17,14 @@
 
 #include 
 
+#include 
 #include 
 #include 
 #include 
 #include 
 
+
+
 /** Initialise the serial device hardware with default settings.
 
   @retval RETURN_SUCCESSThe serial device was initialised.
@@ -48,7 +51,7 @@ SerialPortInitialize (
 
   return PL011UartInitializePort (
(UINTN)FixedPcdGet64 (PcdSerialRegisterBase),
-   FixedPcdGet32 (PL011UartClkInHz),
+   ArmPlatformGetPL011ClockFreq() ? ArmPlatformGetPL011ClockFreq() : 
FixedPcdGet32 (PL011UartClkInHz),
,
,
,
@@ -156,7 +159,7 @@ SerialPortSetAttributes (
 {
   return PL011UartInitializePort (
(UINTN)FixedPcdGet64 (PcdSerialRegisterBase),
-   FixedPcdGet32 (PL011UartClkInHz),
+   ArmPlatformGetPL011ClockFreq() ? ArmPlatformGetPL011ClockFreq() : 
FixedPcdGet32 (PL011UartClkInHz),
BaudRate,
ReceiveFifoDepth,
Parity,
diff --git a/ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.inf 
b/ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.inf
index 3683e06..9820811 100644
--- a/ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.inf
+++ b/ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.inf
@@ -26,6 +26,7 @@
   PL011SerialPortLib.c
 
 [LibraryClasses]
+  ArmPlatformClockLib
   PL011UartLib
   PcdLib
 
-- 
1.9.1

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

Re: [edk2] [PATCH] ArmVirtPkg: implement KVM safe IoLib instance

2018-06-05 Thread Leif Lindholm

On Tue, Jun 05, 2018 at 01:05:43PM +0200, Ard Biesheuvel wrote:
> KVM on ARM refuses to decode load/store instructions used to perform
> I/O to emulated devices, and instead relies on the exception syndrome
> information to describe the operand register, access size, etc.
> This is only possible for instructions that have a single input/output
> register (as opposed to ones that increment the offset register, or
> load/store pair instructions, etc). Otherwise, QEMU crashes with the
> following error
> 
>   error: kvm run failed Function not implemented
>   R00=01010101 R01=0008 R02=0048 R03=08000820
>   R04=0120 R05=7faaa0e0 R06=7faaa0dc R07=7faaa0e8
>   R08=7faaa0ec R09=7faaa088 R10=00ff R11=0080
>   R12=ff00 R13=7fccfe08 R14=7faa835f R15=7faa887c
>   PSR=81f3 N--- T svc32
>   QEMU: Terminated
> 
> and KVM produces a warning such as the following in the kernel log
> 
>   kvm [17646]: load/store instruction decoding not implemented
> 
> The IoLib implementation provided by MdePkg/Library/BaseIoLibIntrinsic
> is based on C code, and when LTO is in effect, the MMIO accesses could
> be merged with, e.g., manipulations of the loop counter, producing
> opcodes that KVM does not support for emulated MMIO.
> 
> So instead, let's reimplement IoLib in a KVM safe manner.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 
> ---
> Yet another approach for the KVM MMIO emulation issue. Note that this one
> (as well as the MdePkg) affect both AArch64 and ARM. This is deliberate,
> given that there is no reason AArch64 should be immune to this: we simply
> haven't triggered the issue yet.

I'm happier with this version (at the same time as I'm sad we're
splitting this out).

Reviewed-by: Leif Lindholm 

>  ArmVirtPkg/Library/ArmVirtIoLib/AArch64/ArmVirtMmio.S |  164 ++
>  ArmVirtPkg/Library/ArmVirtIoLib/Arm/ArmVirtMmio.S |  154 ++
>  ArmVirtPkg/Library/ArmVirtIoLib/Arm/ArmVirtMmio.asm   |  165 ++
>  ArmVirtPkg/Library/ArmVirtIoLib/ArmVirtIoLib.c|  589 +
>  ArmVirtPkg/Library/ArmVirtIoLib/ArmVirtIoLib.h|  188 ++
>  ArmVirtPkg/Library/ArmVirtIoLib/ArmVirtIoLib.inf  |   49 +
>  ArmVirtPkg/Library/ArmVirtIoLib/IoHighLevel.c | 2358 
> 
>  ArmVirtPkg/Library/ArmVirtIoLib/IoLibMmioBuffer.c |  413 
>  8 files changed, 4080 insertions(+)
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH] ArmVirtPkg: implement KVM safe IoLib instance

2018-06-05 Thread Ard Biesheuvel

KVM on ARM refuses to decode load/store instructions used to perform
I/O to emulated devices, and instead relies on the exception syndrome
information to describe the operand register, access size, etc.
This is only possible for instructions that have a single input/output
register (as opposed to ones that increment the offset register, or
load/store pair instructions, etc). Otherwise, QEMU crashes with the
following error

  error: kvm run failed Function not implemented
  R00=01010101 R01=0008 R02=0048 R03=08000820
  R04=0120 R05=7faaa0e0 R06=7faaa0dc R07=7faaa0e8
  R08=7faaa0ec R09=7faaa088 R10=00ff R11=0080
  R12=ff00 R13=7fccfe08 R14=7faa835f R15=7faa887c
  PSR=81f3 N--- T svc32
  QEMU: Terminated

and KVM produces a warning such as the following in the kernel log

  kvm [17646]: load/store instruction decoding not implemented

The IoLib implementation provided by MdePkg/Library/BaseIoLibIntrinsic
is based on C code, and when LTO is in effect, the MMIO accesses could
be merged with, e.g., manipulations of the loop counter, producing
opcodes that KVM does not support for emulated MMIO.

So instead, let's reimplement IoLib in a KVM safe manner.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
---
Yet another approach for the KVM MMIO emulation issue. Note that this one
(as well as the MdePkg) affect both AArch64 and ARM. This is deliberate,
given that there is no reason AArch64 should be immune to this: we simply
haven't triggered the issue yet.

 ArmVirtPkg/Library/ArmVirtIoLib/AArch64/ArmVirtMmio.S |  164 ++
 ArmVirtPkg/Library/ArmVirtIoLib/Arm/ArmVirtMmio.S |  154 ++
 ArmVirtPkg/Library/ArmVirtIoLib/Arm/ArmVirtMmio.asm   |  165 ++
 ArmVirtPkg/Library/ArmVirtIoLib/ArmVirtIoLib.c|  589 +
 ArmVirtPkg/Library/ArmVirtIoLib/ArmVirtIoLib.h|  188 ++
 ArmVirtPkg/Library/ArmVirtIoLib/ArmVirtIoLib.inf  |   49 +
 ArmVirtPkg/Library/ArmVirtIoLib/IoHighLevel.c | 2358 

 ArmVirtPkg/Library/ArmVirtIoLib/IoLibMmioBuffer.c |  413 
 8 files changed, 4080 insertions(+)

diff --git a/ArmVirtPkg/Library/ArmVirtIoLib/AArch64/ArmVirtMmio.S 
b/ArmVirtPkg/Library/ArmVirtIoLib/AArch64/ArmVirtMmio.S
new file mode 100644
index ..47be68a3e783
--- /dev/null
+++ b/ArmVirtPkg/Library/ArmVirtIoLib/AArch64/ArmVirtMmio.S
@@ -0,0 +1,164 @@
+#
+#  Copyright (c) 2014-2018, Linaro Limited. All rights reserved.
+#
+#  This program and the accompanying materials are licensed and made available
+#  under the terms and conditions of the BSD License which accompanies this
+#  distribution.  The full text of the license may be found at
+#  http://opensource.org/licenses/bsd-license.php
+#
+#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
IMPLIED.
+#
+#
+
+.text
+.align 3
+
+GCC_ASM_EXPORT(MmioRead8Internal)
+GCC_ASM_EXPORT(MmioWrite8Internal)
+GCC_ASM_EXPORT(MmioRead16Internal)
+GCC_ASM_EXPORT(MmioWrite16Internal)
+GCC_ASM_EXPORT(MmioRead32Internal)
+GCC_ASM_EXPORT(MmioWrite32Internal)
+GCC_ASM_EXPORT(MmioRead64Internal)
+GCC_ASM_EXPORT(MmioWrite64Internal)
+
+//
+//  Reads an 8-bit MMIO register.
+//
+//  Reads the 8-bit MMIO register specified by Address. The 8-bit read value is
+//  returned. This function must guarantee that all MMIO read and write
+//  operations are serialized.
+//
+//  If 8-bit MMIO register operations are not supported, then ASSERT().
+//
+//  @param  Address The MMIO register to read.
+//
+//  @return The value read.
+//
+ASM_PFX(MmioRead8Internal):
+  ldrbw0, [x0]
+  dmb ld
+  ret
+
+//
+//  Writes an 8-bit MMIO register.
+//
+//  Writes the 8-bit MMIO register specified by Address with the value 
specified
+//  by Value and returns Value. This function must guarantee that all MMIO read
+//  and write operations are serialized.
+//
+//  If 8-bit MMIO register operations are not supported, then ASSERT().
+//
+//  @param  Address The MMIO register to write.
+//  @param  Value   The value to write to the MMIO register.
+//
+ASM_PFX(MmioWrite8Internal):
+  dmb st
+  strbw1, [x0]
+  ret
+
+//
+//  Reads a 16-bit MMIO register.
+//
+//  Reads the 16-bit MMIO register specified by Address. The 16-bit read value 
is
+//  returned. This function must guarantee that all MMIO read and write
+//  operations are serialized.
+//
+//  If 16-bit MMIO register operations are not supported, then ASSERT().
+//
+//  @param  Address The MMIO register to read.
+//
+//  @return The value read.
+//
+ASM_PFX(MmioRead16Internal):
+  ldrhw0, [x0]
+  dmb ld
+  ret
+
+//
+//  Writes a 16-bit MMIO register.
+//
+//  Writes the 16-bit MMIO register specified by Address with the value 
specified
+//  by Value and returns Value. This function must guarantee that all MMIO read
+//  and write operations are serialized.
+//
+//  If 16-bit MMIO register operations are not supported, then

Re: [edk2] [RFC PATCH] MdePkg/BaseIoLibIntrinsic ARM AARCH64: avoid C code for MMIO access

2018-06-05 Thread Ard Biesheuvel

On 5 June 2018 at 10:39, Leif Lindholm  wrote:
> On Tue, Jun 05, 2018 at 09:30:04AM +0200, Ard Biesheuvel wrote:
>> Even though MMIO shares the address space with ordinary memory, the
>> accesses involved are *not* ordinary memory accesses, and so it is
>> a bad idea to let the compiler generate them using pointer dereferences.
>>
>> Instead, introduce a set of internal accessors implemented in assembler,
>> and call those from the various Mmio[Read|Write]XX () implementations.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Ard Biesheuvel 
>> ---
>> Open coding the MMIO accesses is a good idea in general, but it also works
>> around an issue that affects EDK2 running under KVM on ARM or AARCH64, where
>> LTO code generation results in MMIO accesses involving instructions that 
>> cannot
>> be emulated by the hypervisor.
>
> I think there are arguments for and against this solution instead of
> disabling LTO.
>
> My main argument against this version is that it makes a
> re-unification of the (needlessly) different pure C implementations
> more tedious/questionable.
>
> It _is_ possible that the introduction of LTO makes the described
> semantics of the functions currently incorrect for ARM*, at which
> point the above reservation is irrelevant.
>
> However, the DSBs seem like a bit of a sledge hammer - all that's
> being mandated is serialization? Why would DMB not suffice?
> And if we make that change ... I would sort of prefer to see the
> barriers added as a separate patch, with a clear commit message -
> because that is not just part of the refactoring.
>

Points taken.

Perhaps it is more appropriate to create a special ArmVirtPkg IoLib
implementation for the time being, and defer changes like this one
until we have more data points to consider.


>>  MdePkg/Library/BaseIoLibIntrinsic/AArch64/IoLibMmio.S| 164 
>> ++
>>  MdePkg/Library/BaseIoLibIntrinsic/Arm/IoLibMmio.S| 164 
>> ++
>>  MdePkg/Library/BaseIoLibIntrinsic/Arm/IoLibMmio.asm  | 165 
>> ++
>>  MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf |   9 +-
>>  MdePkg/Library/BaseIoLibIntrinsic/IoLibArm.c |  36 ++--
>>  MdePkg/Library/BaseIoLibIntrinsic/IoLibArm.h | 179 
>> 
>>  6 files changed, 692 insertions(+), 25 deletions(-)
>>
>> diff --git a/MdePkg/Library/BaseIoLibIntrinsic/AArch64/IoLibMmio.S 
>> b/MdePkg/Library/BaseIoLibIntrinsic/AArch64/IoLibMmio.S
>> new file mode 100644
>> index ..ac96df602f7a
>> --- /dev/null
>> +++ b/MdePkg/Library/BaseIoLibIntrinsic/AArch64/IoLibMmio.S
>> @@ -0,0 +1,164 @@
>> +#
>> +#  Copyright (c) 2014-2018, Linaro Limited. All rights reserved.
>> +#
>> +#  This program and the accompanying materials are licensed and made 
>> available
>> +#  under the terms and conditions of the BSD License which accompanies this
>> +#  distribution.  The full text of the license may be found at
>> +#  http://opensource.org/licenses/bsd-license.php
>> +#
>> +#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
>> +#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
>> IMPLIED.
>> +#
>> +#
>> +
>> +.text
>> +.align 3
>> +
>> +GCC_ASM_EXPORT(MmioRead8Internal)
>> +GCC_ASM_EXPORT(MmioWrite8Internal)
>> +GCC_ASM_EXPORT(MmioRead16Internal)
>> +GCC_ASM_EXPORT(MmioWrite16Internal)
>> +GCC_ASM_EXPORT(MmioRead32Internal)
>> +GCC_ASM_EXPORT(MmioWrite32Internal)
>> +GCC_ASM_EXPORT(MmioRead64Internal)
>> +GCC_ASM_EXPORT(MmioWrite64Internal)
>> +
>> +//
>> +//  Reads an 8-bit MMIO register.
>> +//
>> +//  Reads the 8-bit MMIO register specified by Address. The 8-bit read 
>> value is
>> +//  returned. This function must guarantee that all MMIO read and write
>> +//  operations are serialized.
>> +//
>> +//  If 8-bit MMIO register operations are not supported, then ASSERT().
>> +//
>> +//  @param  Address The MMIO register to read.
>> +//
>> +//  @return The value read.
>> +//
>> +ASM_PFX(MmioRead8Internal):
>> +  ldrbw0, [x0]
>> +  dsb ld
>> +  ret
>> +
>> +//
>> +//  Writes an 8-bit MMIO register.
>> +//
>> +//  Writes the 8-bit MMIO register specified by Address with the value 
>> specified
>> +//  by Value and returns Value. This function must guarantee that all MMIO 
>> read
>> +//  and write operations are serialized.
>> +//
>> +//  If 8-bit MMIO register operations are not supported, then ASSERT().
>> +//
>> +//  @param  Address The MMIO register to write.
>> +//  @param  Value   The value to write to the MMIO register.
>> +//
>> +ASM_PFX(MmioWrite8Internal):
>> +  dsb st
>> +  strbw1, [x0]
>> +  ret
>> +
>> +//
>> +//  Reads a 16-bit MMIO register.
>> +//
>> +//  Reads the 16-bit MMIO register specified by Address. The 16-bit read 
>> value is
>> +//  returned. This function must guarantee that all MMIO read and write
>> +//  operations are serialized.
>> +//
>> +//  If 16-bit MMIO register operations are not supported,

[edk2] TPM 2.0 ACPI tableTPM2 table event log

2018-06-05 Thread Lin, Derek (HPS UEFI Dev)

Hi Chao and Jiewen,

There are new fields in TPM2 ACPI table for getting TCG event log under OS.

https://trustedcomputinggroup.org/wp-content/uploads/TCG_ACPIGeneralSpecification_v1.20_r8.pdf
Page 11.
Log Area Minimum Length (LAML)
Log Area Start Address (LASA)

I remember they are not existed in older TCG ACPI spec.

Do you have plan to implement it?

Thanks,
Derek

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

Re: [edk2] [patch 2/2] ShellPkg/UefiShellAcpiViewCommandLib: Fix ECC issues

2018-06-05 Thread Alexei Fedorov

Hi Dandan Bi,


Your patch contains a number of modifications like the one below:

-#ifndef ACPIPARSER_H_
-#define ACPIPARSER_H_
+#ifndef __ACPIPARSER_H__
+#define __ACPIPARSER_H__


which violate CCS

"3.3.2 Include Files"

"4.3.5.4 The names of guard macros shall end with an underscore character."

and the given examples.


Alexei


From: edk2-devel  on behalf of Dandan Bi 

Sent: 05 June 2018 09:33
To: edk2-devel@lists.01.org
Cc: Jaben Carsey; Ruiyu Ni
Subject: [edk2] [patch 2/2] ShellPkg/UefiShellAcpiViewCommandLib: Fix ECC issues

1. Separate variable definition and initialization.
2. Make the variable naming following Edk2 rule.
Naming convention of local variable:
a.First character should be upper case.
b.Must contain lower case characters.
c.No white space characters.

Cc: Sami Mujawar 
Cc: Evan Lloyd 
Cc: Sami Mujawar 
Cc: Evan Lloyd 
Cc: Jaben Carsey 
Cc: Ruiyu Ni 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Dandan Bi 
---
 .../UefiShellAcpiViewCommandLib/AcpiParser.c   | 44 ++--
 .../UefiShellAcpiViewCommandLib/AcpiParser.h   |  6 +--
 .../UefiShellAcpiViewCommandLib/AcpiTableParser.c  | 50 +--
 .../UefiShellAcpiViewCommandLib/AcpiTableParser.h  |  6 +--
 .../Library/UefiShellAcpiViewCommandLib/AcpiView.c | 58 ++
 .../Library/UefiShellAcpiViewCommandLib/AcpiView.h | 16 +++---
 .../Parsers/Dbg2/Dbg2Parser.c  |  5 +-
 .../Parsers/Gtdt/GtdtParser.c  |  5 +-
 .../Parsers/Iort/IortParser.c  | 26 +-
 .../Parsers/Madt/MadtParser.c  |  4 +-
 .../Parsers/Rsdp/RsdpParser.c  | 10 +++-
 .../Parsers/Slit/SlitParser.c  | 44 
 .../Parsers/Spcr/SpcrParser.c  | 10 +++-
 .../Parsers/Srat/SratParser.c  | 21 +---
 .../UefiShellAcpiViewCommandLib.c  |  5 +-
 .../UefiShellAcpiViewCommandLib.h  |  6 +--
 .../UefiShellAcpiViewCommandLib.inf|  3 ++
 17 files changed, 190 insertions(+), 129 deletions(-)

diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c 
b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
index 088575d0144..6d3bc451acd 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
@@ -19,10 +19,19 @@

 STATIC UINT32   gIndent;
 STATIC UINT32   mTableErrorCount;
 STATIC UINT32   mTableWarningCount;

+STATIC ACPI_DESCRIPTION_HEADER_INFO AcpiHdrInfo;
+
+/**
+  An ACPI_PARSER array describing the ACPI header.
+**/
+STATIC CONST ACPI_PARSER AcpiHeaderParser[] = {
+  PARSE_ACPI_HEADER ()
+};
+
 /**
   This function resets the ACPI table error counter to Zero.
 **/
 VOID
 ResetErrorCount (
@@ -112,14 +121,17 @@ VerifyChecksum (
   IN BOOLEAN Log,
   IN UINT8*  Ptr,
   IN UINT32  Length
   )
 {
-  UINTN ByteCount = 0;
-  UINT8 Checksum = 0;
+  UINTN ByteCount;
+  UINT8 Checksum;
   UINTN OriginalAttribute;

+  ByteCount = 0;
+  Checksum = 0;
+
   while (ByteCount < Length) {
 Checksum += *(Ptr++);
 ByteCount++;
   }

@@ -164,15 +176,18 @@ EFIAPI
 DumpRaw (
   IN UINT8* Ptr,
   IN UINT32 Length
   )
 {
-  UINTN ByteCount = 0;
+  UINTN ByteCount;
   UINTN PartLineChars;
-  UINTN AsciiBufferIndex = 0;
+  UINTN AsciiBufferIndex;
   CHAR8 AsciiBuffer[17];

+  ByteCount = 0;
+  AsciiBufferIndex = 0;
+
   Print (L"Address  : 0x%p\n", Ptr);
   Print (L"Length   : %d\n", Length);

   while (ByteCount < Length) {
 if ((ByteCount & 0x0F) == 0) {
@@ -275,11 +290,14 @@ DumpUint64 (
   )
 {
   // Some fields are not aligned and this causes alignment faults
   // on ARM platforms if the compiler generates LDRD instructions.
   // Perform word access so that LDRD instructions are not generated.
-  UINT64 Val = *(UINT32*)(Ptr + sizeof (UINT32));
+  UINT64 Val;
+
+  Val = *(UINT32*)(Ptr + sizeof (UINT32));
+
   Val <<= 32;
   Val |= *(UINT32*)Ptr;

   Print (Format, Val);
 }
@@ -454,17 +472,20 @@ ParseAcpi (
   IN CONST ACPI_PARSER* Parser,
   IN UINT32 ParserItems
 )
 {
   UINT32  Index;
-  UINT32  Offset = 0;
+  UINT32  Offset;
+  BOOLEAN HighLight;
+
+  Offset = 0;

   // Increment the Indent
   gIndent += Indent;

   if (Trace && (AsciiName != NULL)){
-BOOLEAN HighLight = GetColourHighlighting ();
+HighLight = GetColourHighlighting ();
 UINTN   OriginalAttribute;

 if (HighLight) {
   OriginalAttribute = gST->ConOut->Mode->Attribute;
   gST->ConOut->SetAttribute (
@@ -618,15 +639,10 @@ UINT32
 EFIAPI
 DumpAcpiHeader (
   IN UINT8* Ptr
   )
 {
-  ACPI_DESCRIPTION_HEADER_INFO AcpiHdrInfo;
-  ACPI_PARSER AcpiHeaderParser[] = {
-PARSE_ACPI_HEADER ()
-  };
-
   return ParseAcpi (
TRUE,
0,
"ACPI Table Header",
Ptr,
@@ -656,14 +672,10 @@ ParseAcpiHeader (
   OUT CONST UINT32** Length,

Re: [edk2] reg: ISCSI Aborted attempt entry in IBFT Table

2018-06-05 Thread Omkar K

Hello Ting,

1. We did not enable MPIO.
2. in IScsiStart(), at this point

//
// Select the first login session. Abort others.
//
if (Private->Session == NULL) {
Private->Session = Session;
BootSelected = AttemptConfigData->AttemptConfigIndex;
//
// Don't validate other attempt in multipath mode if one is success.
//
if (mPrivate->EnableMpio) {
  break;
}
} else {
IScsiSessionAbort (Session);
FreePool (Session);
}

other than one attempt per Nic, other sessions are aborted. Still, all the 
attempts are published in IBFT.
We can observe the issue when different targets are configured on one NIC where 
all the attempts are published in IBFT.
But, the issue disappeared when the aborted attempts are not published in IBFT.

Thanks,
Omkar

-Original Message-
From: Ye, Ting [mailto:ting...@intel.com] 
Sent: Monday, June 04, 2018 2:26 PM
To: Sivaraman Nainar; edk2-devel@lists.01.org
Cc: Omkar K; Madhan B. Santharam
Subject: RE: reg: ISCSI Aborted attempt entry in IBFT Table

Hi Siva,

Per design, the iSCSI multipath I/O will publish all configured attempts to 
IBFT, no matter the connection is success or fail currently.
Did you enable the MPIO when you configure the attempts? 

I am not clear what do you mean "aborted attempt".

Thanks,
Ting

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
Sivaraman Nainar
Sent: Thursday, May 31, 2018 8:18 PM
To: edk2-devel@lists.01.org
Cc: Omkar K ; Madhan B. Santharam 
Subject: [edk2] reg: ISCSI Aborted attempt entry in IBFT Table

Hello all:
Here is the issue which requires clarification.
Issue Synopsis:
When there are more than one iSCSI target configured and Ibft table published 
with the connected and aborted attempt details, all the targets are not seen in 
ESXi and SLES OS. But in Windows it can see the targets connected.
Use case:
Target 1: IP : 192.xx.xx.31 Target 2 : IP : 192.xx.xx.1
NIC 1 configured with attempts Target 1 & Target 2 NIC 2 configured with 
attempts Target 1 Connection

Login

Ibft

Block Device in UEFI Shell

SLES / Esx OS

Windows

NIC1 Target1

Success

Published

Mounted

Target Seen



NIC1 Target2

Success

Published

Mounted

Target Seen



NIC1 Target1
NIC1 Target2
NIC2 Target1

Individual Login success

Published for all attempts (3)

NIC1 Target 1 NIC2 Target 1

None Seen

NIC1 Target 1 NIC2 Target 1


When the attempts which are login success but Aborted by ISCSI Driver are 
present in ibft table SLES and ESX not able to see the targets during 
Installation.
If the ISCSI Driver not adding the ibft entry for the aborted attempts then the 
targets are seen in Esx and SLES.
So it requires clarification that If the driver need to SKIP adding the aborted 
attempt entry to ibft or its OS responsibility to handle the invalid entries in 
ibft.
-Siva
___
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] [RFC PATCH] MdePkg/BaseIoLibIntrinsic ARM AARCH64: avoid C code for MMIO access

2018-06-05 Thread Bhupesh Sharma

On Tue, Jun 5, 2018 at 1:00 PM, Ard Biesheuvel
 wrote:
> Even though MMIO shares the address space with ordinary memory, the
> accesses involved are *not* ordinary memory accesses, and so it is
> a bad idea to let the compiler generate them using pointer dereferences.
>
> Instead, introduce a set of internal accessors implemented in assembler,
> and call those from the various Mmio[Read|Write]XX () implementations.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 
> ---
> Open coding the MMIO accesses is a good idea in general, but it also works
> around an issue that affects EDK2 running under KVM on ARM or AARCH64, where
> LTO code generation results in MMIO accesses involving instructions that 
> cannot
> be emulated by the hypervisor.
>
>  MdePkg/Library/BaseIoLibIntrinsic/AArch64/IoLibMmio.S| 164 
> ++
>  MdePkg/Library/BaseIoLibIntrinsic/Arm/IoLibMmio.S| 164 
> ++
>  MdePkg/Library/BaseIoLibIntrinsic/Arm/IoLibMmio.asm  | 165 
> ++
>  MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf |   9 +-
>  MdePkg/Library/BaseIoLibIntrinsic/IoLibArm.c |  36 ++--
>  MdePkg/Library/BaseIoLibIntrinsic/IoLibArm.h | 179 
> 
>  6 files changed, 692 insertions(+), 25 deletions(-)
>
> diff --git a/MdePkg/Library/BaseIoLibIntrinsic/AArch64/IoLibMmio.S 
> b/MdePkg/Library/BaseIoLibIntrinsic/AArch64/IoLibMmio.S
> new file mode 100644
> index ..ac96df602f7a
> --- /dev/null
> +++ b/MdePkg/Library/BaseIoLibIntrinsic/AArch64/IoLibMmio.S
> @@ -0,0 +1,164 @@
> +#
> +#  Copyright (c) 2014-2018, Linaro Limited. All rights reserved.
> +#
> +#  This program and the accompanying materials are licensed and made 
> available
> +#  under the terms and conditions of the BSD License which accompanies this
> +#  distribution.  The full text of the license may be found at
> +#  http://opensource.org/licenses/bsd-license.php
> +#
> +#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
> IMPLIED.
> +#
> +#
> +
> +.text
> +.align 3
> +
> +GCC_ASM_EXPORT(MmioRead8Internal)
> +GCC_ASM_EXPORT(MmioWrite8Internal)
> +GCC_ASM_EXPORT(MmioRead16Internal)
> +GCC_ASM_EXPORT(MmioWrite16Internal)
> +GCC_ASM_EXPORT(MmioRead32Internal)
> +GCC_ASM_EXPORT(MmioWrite32Internal)
> +GCC_ASM_EXPORT(MmioRead64Internal)
> +GCC_ASM_EXPORT(MmioWrite64Internal)
> +
> +//
> +//  Reads an 8-bit MMIO register.
> +//
> +//  Reads the 8-bit MMIO register specified by Address. The 8-bit read value 
> is
> +//  returned. This function must guarantee that all MMIO read and write
> +//  operations are serialized.
> +//
> +//  If 8-bit MMIO register operations are not supported, then ASSERT().
> +//
> +//  @param  Address The MMIO register to read.
> +//
> +//  @return The value read.
> +//
> +ASM_PFX(MmioRead8Internal):
> +  ldrbw0, [x0]
> +  dsb ld

I am generally worried about sprinkling of the 'dsb' that the patch
here (and below) does, without adding comments.

Please note that with current (and possible future) ARM core erratas
and SoC specific erratas (from Si vendors) assumptions about the
serialization sequence can be tricky at times in terms of MMIO
accesses and dsb usages.

I would suggest that we document the 'dsb' usage here and _warn_ users
to add to/fix the sequence in case they are working on a 'broken' Si
with known erratas (which they normally get around with by fixing and
using inhouse versions of gcc which fix the sequencing issue for them
while generating the relevant code).

Regards,
Bhupesh


> +  ret
> +
> +//
> +//  Writes an 8-bit MMIO register.
> +//
> +//  Writes the 8-bit MMIO register specified by Address with the value 
> specified
> +//  by Value and returns Value. This function must guarantee that all MMIO 
> read
> +//  and write operations are serialized.
> +//
> +//  If 8-bit MMIO register operations are not supported, then ASSERT().
> +//
> +//  @param  Address The MMIO register to write.
> +//  @param  Value   The value to write to the MMIO register.
> +//
> +ASM_PFX(MmioWrite8Internal):
> +  dsb st
> +  strbw1, [x0]
> +  ret
> +
> +//
> +//  Reads a 16-bit MMIO register.
> +//
> +//  Reads the 16-bit MMIO register specified by Address. The 16-bit read 
> value is
> +//  returned. This function must guarantee that all MMIO read and write
> +//  operations are serialized.
> +//
> +//  If 16-bit MMIO register operations are not supported, then ASSERT().
> +//
> +//  @param  Address The MMIO register to read.
> +//
> +//  @return The value read.
> +//
> +ASM_PFX(MmioRead16Internal):
> +  ldrhw0, [x0]
> +  dsb ld
> +  ret
> +
> +//
> +//  Writes a 16-bit MMIO register.
> +//
> +//  Writes the 16-bit MMIO register specified by Address with the value 
> specified
> +//  by Value and returns Value. This function must guarantee that all MMIO 
> read
> +//  and

Re: [edk2] [RFC PATCH] MdePkg/BaseIoLibIntrinsic ARM AARCH64: avoid C code for MMIO access

2018-06-05 Thread Laszlo Ersek

On 06/05/18 09:30, Ard Biesheuvel wrote:
> Even though MMIO shares the address space with ordinary memory, the
> accesses involved are *not* ordinary memory accesses, and so it is
> a bad idea to let the compiler generate them using pointer dereferences.

I think I slightly disagree with the wording here. Using
volatile-qualified pointers (implying compiler barriers), possibly
combined with CPU barriers (wrapped into C functions), seems valid in
both practice and on the ISO C level. "volatile" does not guarantee
atomicity, but an IoLib instance can still assume that, if it is tied to
a specific toolchain, and the toolchain guarantees atomicity. Anyway:

> 
> Instead, introduce a set of internal accessors implemented in assembler,
> and call those from the various Mmio[Read|Write]XX () implementations.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 
> ---
> Open coding the MMIO accesses is a good idea in general, but it also works
> around an issue that affects EDK2 running under KVM on ARM or AARCH64, where
> LTO code generation results in MMIO accesses involving instructions that 
> cannot
> be emulated by the hypervisor.

edk2 code (and developers!) are generally conscious about this issue,
and call the IoLib APIs carefully. Thus, I believe that, with this
patch, we can address the problem in edk2 "for good" -- we can trust
that all such accesses will go through a few "choke points", and we can
add the work-arounds to the "choke points". (This is exactly what I
believe we aren't certain about with regard to the Linux kernel.)

Therefore I welcome the patch! I just think the commit message should
name the KVM workaround as the primary reason. The C code is not wrong
(knowing the compiler), and the generated assembly is also not wrong on
the bare metal.

I don't insist though that you update the commit message; I prefer
working code to documentation that's polished to my taste. :)

Acked-by: Laszlo Ersek 

Thank you!
Laszlo

> 
>  MdePkg/Library/BaseIoLibIntrinsic/AArch64/IoLibMmio.S| 164 
> ++
>  MdePkg/Library/BaseIoLibIntrinsic/Arm/IoLibMmio.S| 164 
> ++
>  MdePkg/Library/BaseIoLibIntrinsic/Arm/IoLibMmio.asm  | 165 
> ++
>  MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf |   9 +-
>  MdePkg/Library/BaseIoLibIntrinsic/IoLibArm.c |  36 ++--
>  MdePkg/Library/BaseIoLibIntrinsic/IoLibArm.h | 179 
> 
>  6 files changed, 692 insertions(+), 25 deletions(-)
> 
> diff --git a/MdePkg/Library/BaseIoLibIntrinsic/AArch64/IoLibMmio.S 
> b/MdePkg/Library/BaseIoLibIntrinsic/AArch64/IoLibMmio.S
> new file mode 100644
> index ..ac96df602f7a
> --- /dev/null
> +++ b/MdePkg/Library/BaseIoLibIntrinsic/AArch64/IoLibMmio.S
> @@ -0,0 +1,164 @@
> +#
> +#  Copyright (c) 2014-2018, Linaro Limited. All rights reserved.
> +#
> +#  This program and the accompanying materials are licensed and made 
> available
> +#  under the terms and conditions of the BSD License which accompanies this
> +#  distribution.  The full text of the license may be found at
> +#  http://opensource.org/licenses/bsd-license.php
> +#
> +#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
> IMPLIED.
> +#
> +#
> +
> +.text
> +.align 3
> +
> +GCC_ASM_EXPORT(MmioRead8Internal)
> +GCC_ASM_EXPORT(MmioWrite8Internal)
> +GCC_ASM_EXPORT(MmioRead16Internal)
> +GCC_ASM_EXPORT(MmioWrite16Internal)
> +GCC_ASM_EXPORT(MmioRead32Internal)
> +GCC_ASM_EXPORT(MmioWrite32Internal)
> +GCC_ASM_EXPORT(MmioRead64Internal)
> +GCC_ASM_EXPORT(MmioWrite64Internal)
> +
> +//
> +//  Reads an 8-bit MMIO register.
> +//
> +//  Reads the 8-bit MMIO register specified by Address. The 8-bit read value 
> is
> +//  returned. This function must guarantee that all MMIO read and write
> +//  operations are serialized.
> +//
> +//  If 8-bit MMIO register operations are not supported, then ASSERT().
> +//
> +//  @param  Address The MMIO register to read.
> +//
> +//  @return The value read.
> +//
> +ASM_PFX(MmioRead8Internal):
> +  ldrbw0, [x0]
> +  dsb ld
> +  ret
> +
> +//
> +//  Writes an 8-bit MMIO register.
> +//
> +//  Writes the 8-bit MMIO register specified by Address with the value 
> specified
> +//  by Value and returns Value. This function must guarantee that all MMIO 
> read
> +//  and write operations are serialized.
> +//
> +//  If 8-bit MMIO register operations are not supported, then ASSERT().
> +//
> +//  @param  Address The MMIO register to write.
> +//  @param  Value   The value to write to the MMIO register.
> +//
> +ASM_PFX(MmioWrite8Internal):
> +  dsb st
> +  strbw1, [x0]
> +  ret
> +
> +//
> +//  Reads a 16-bit MMIO register.
> +//
> +//  Reads the 16-bit MMIO register specified by Address. The 16-bit read 
> value is
> +//  returned. This function must guarantee that all MMIO read and write

Re: [edk2] [RFC PATCH] MdePkg/BaseIoLibIntrinsic ARM AARCH64: avoid C code for MMIO access

2018-06-05 Thread Leif Lindholm

On Tue, Jun 05, 2018 at 09:30:04AM +0200, Ard Biesheuvel wrote:
> Even though MMIO shares the address space with ordinary memory, the
> accesses involved are *not* ordinary memory accesses, and so it is
> a bad idea to let the compiler generate them using pointer dereferences.
> 
> Instead, introduce a set of internal accessors implemented in assembler,
> and call those from the various Mmio[Read|Write]XX () implementations.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 
> ---
> Open coding the MMIO accesses is a good idea in general, but it also works
> around an issue that affects EDK2 running under KVM on ARM or AARCH64, where
> LTO code generation results in MMIO accesses involving instructions that 
> cannot
> be emulated by the hypervisor.

I think there are arguments for and against this solution instead of
disabling LTO.

My main argument against this version is that it makes a
re-unification of the (needlessly) different pure C implementations
more tedious/questionable.

It _is_ possible that the introduction of LTO makes the described
semantics of the functions currently incorrect for ARM*, at which
point the above reservation is irrelevant.

However, the DSBs seem like a bit of a sledge hammer - all that's
being mandated is serialization? Why would DMB not suffice?
And if we make that change ... I would sort of prefer to see the
barriers added as a separate patch, with a clear commit message -
because that is not just part of the refactoring.

/
Leif

>  MdePkg/Library/BaseIoLibIntrinsic/AArch64/IoLibMmio.S| 164 
> ++
>  MdePkg/Library/BaseIoLibIntrinsic/Arm/IoLibMmio.S| 164 
> ++
>  MdePkg/Library/BaseIoLibIntrinsic/Arm/IoLibMmio.asm  | 165 
> ++
>  MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf |   9 +-
>  MdePkg/Library/BaseIoLibIntrinsic/IoLibArm.c |  36 ++--
>  MdePkg/Library/BaseIoLibIntrinsic/IoLibArm.h | 179 
> 
>  6 files changed, 692 insertions(+), 25 deletions(-)
> 
> diff --git a/MdePkg/Library/BaseIoLibIntrinsic/AArch64/IoLibMmio.S 
> b/MdePkg/Library/BaseIoLibIntrinsic/AArch64/IoLibMmio.S
> new file mode 100644
> index ..ac96df602f7a
> --- /dev/null
> +++ b/MdePkg/Library/BaseIoLibIntrinsic/AArch64/IoLibMmio.S
> @@ -0,0 +1,164 @@
> +#
> +#  Copyright (c) 2014-2018, Linaro Limited. All rights reserved.
> +#
> +#  This program and the accompanying materials are licensed and made 
> available
> +#  under the terms and conditions of the BSD License which accompanies this
> +#  distribution.  The full text of the license may be found at
> +#  http://opensource.org/licenses/bsd-license.php
> +#
> +#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
> IMPLIED.
> +#
> +#
> +
> +.text
> +.align 3
> +
> +GCC_ASM_EXPORT(MmioRead8Internal)
> +GCC_ASM_EXPORT(MmioWrite8Internal)
> +GCC_ASM_EXPORT(MmioRead16Internal)
> +GCC_ASM_EXPORT(MmioWrite16Internal)
> +GCC_ASM_EXPORT(MmioRead32Internal)
> +GCC_ASM_EXPORT(MmioWrite32Internal)
> +GCC_ASM_EXPORT(MmioRead64Internal)
> +GCC_ASM_EXPORT(MmioWrite64Internal)
> +
> +//
> +//  Reads an 8-bit MMIO register.
> +//
> +//  Reads the 8-bit MMIO register specified by Address. The 8-bit read value 
> is
> +//  returned. This function must guarantee that all MMIO read and write
> +//  operations are serialized.
> +//
> +//  If 8-bit MMIO register operations are not supported, then ASSERT().
> +//
> +//  @param  Address The MMIO register to read.
> +//
> +//  @return The value read.
> +//
> +ASM_PFX(MmioRead8Internal):
> +  ldrbw0, [x0]
> +  dsb ld
> +  ret
> +
> +//
> +//  Writes an 8-bit MMIO register.
> +//
> +//  Writes the 8-bit MMIO register specified by Address with the value 
> specified
> +//  by Value and returns Value. This function must guarantee that all MMIO 
> read
> +//  and write operations are serialized.
> +//
> +//  If 8-bit MMIO register operations are not supported, then ASSERT().
> +//
> +//  @param  Address The MMIO register to write.
> +//  @param  Value   The value to write to the MMIO register.
> +//
> +ASM_PFX(MmioWrite8Internal):
> +  dsb st
> +  strbw1, [x0]
> +  ret
> +
> +//
> +//  Reads a 16-bit MMIO register.
> +//
> +//  Reads the 16-bit MMIO register specified by Address. The 16-bit read 
> value is
> +//  returned. This function must guarantee that all MMIO read and write
> +//  operations are serialized.
> +//
> +//  If 16-bit MMIO register operations are not supported, then ASSERT().
> +//
> +//  @param  Address The MMIO register to read.
> +//
> +//  @return The value read.
> +//
> +ASM_PFX(MmioRead16Internal):
> +  ldrhw0, [x0]
> +  dsb ld
> +  ret
> +
> +//
> +//  Writes a 16-bit MMIO register.
> +//
> +//  Writes the 16-bit MMIO register specified by Address with the value 
> specified
> +//  by Value and returns Value.

Re: [edk2] [patch] MdeModulePkg/DisplayUpdateProgressLib: Fix ECC issues

2018-06-05 Thread Zeng, Star

Reviewed-by: Star Zeng 

-Original Message-
From: Bi, Dandan 
Sent: Tuesday, June 5, 2018 4:36 PM
To: edk2-devel@lists.01.org
Cc: Zeng, Star ; Dong, Eric 
Subject: [patch] MdeModulePkg/DisplayUpdateProgressLib: Fix ECC issues

Make the comment align with Edk2 coding style.

Cc: Star Zeng 
Cc: Eric Dong 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Dandan Bi 
---
 .../DisplayUpdateProgressLibGraphics.c  | 6 +++---
 .../DisplayUpdateProgressLibText/DisplayUpdateProgressLibText.c | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git 
a/MdeModulePkg/Library/DisplayUpdateProgressLibGraphics/DisplayUpdateProgressLibGraphics.c
 
b/MdeModulePkg/Library/DisplayUpdateProgressLibGraphics/DisplayUpdateProgressLibGraphics.c
index 007522cea08..2c916105136 100644
--- 
a/MdeModulePkg/Library/DisplayUpdateProgressLibGraphics/DisplayUpdateProgressLibGraphics.c
+++ 
b/MdeModulePkg/Library/DisplayUpdateProgressLibGraphics/DisplayUpdateProgressLibGraphics.c
@@ -1,6 +1,6 @@
-/**  @file
+/** @file
   Provides services to display completion progress of a firmware update on a
   graphical console that supports the Graphics Output Protocol.
 
   Copyright (c) 2016, Microsoft Corporation. All rights reserved.
   Copyright (c) 2018, Intel Corporation. All rights reserved.
@@ -116,17 +116,17 @@ const EFI_GRAPHICS_OUTPUT_BLT_PIXEL_UNION  
mProgressBarDefaultColor = {
 // Set to TRUE if a valid Graphics Output Protocol is found and the progress
 // bar fits under the boot logo using the current graphics mode.
 //
 BOOLEAN mGraphicsGood = FALSE;
 
-/*
+/**
   Internal function used to find the bounds of the white logo (on black or
   red background).
 
   These bounds are then computed to find the block size, 0%, 100%, etc.
 
-*/
+**/
 VOID
 FindDim (
VOID
   )
 {
diff --git 
a/MdeModulePkg/Library/DisplayUpdateProgressLibText/DisplayUpdateProgressLibText.c
 
b/MdeModulePkg/Library/DisplayUpdateProgressLibText/DisplayUpdateProgressLibText.c
index 7aca8b89d01..960ccc015fe 100644
--- 
a/MdeModulePkg/Library/DisplayUpdateProgressLibText/DisplayUpdateProgressLibText.c
+++ 
b/MdeModulePkg/Library/DisplayUpdateProgressLibText/DisplayUpdateProgressLibText.c
@@ -1,6 +1,6 @@
-/**  @file
+/** @file
   Provides services to display completion progress of a firmware update on a
   text console.
 
   Copyright (c) 2016, Microsoft Corporation. All rights reserved.
   Copyright (c) 2018, Intel Corporation. All rights reserved.
-- 
2.14.3.windows.1

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

[edk2] [patch 2/2] ShellPkg/UefiShellAcpiViewCommandLib: Fix ECC issues

2018-06-05 Thread Dandan Bi

1. Separate variable definition and initialization.
2. Make the variable naming following Edk2 rule.
Naming convention of local variable:
a.First character should be upper case.
b.Must contain lower case characters.
c.No white space characters.

Cc: Sami Mujawar 
Cc: Evan Lloyd 
Cc: Sami Mujawar 
Cc: Evan Lloyd 
Cc: Jaben Carsey 
Cc: Ruiyu Ni 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Dandan Bi 
---
 .../UefiShellAcpiViewCommandLib/AcpiParser.c   | 44 ++--
 .../UefiShellAcpiViewCommandLib/AcpiParser.h   |  6 +--
 .../UefiShellAcpiViewCommandLib/AcpiTableParser.c  | 50 +--
 .../UefiShellAcpiViewCommandLib/AcpiTableParser.h  |  6 +--
 .../Library/UefiShellAcpiViewCommandLib/AcpiView.c | 58 ++
 .../Library/UefiShellAcpiViewCommandLib/AcpiView.h | 16 +++---
 .../Parsers/Dbg2/Dbg2Parser.c  |  5 +-
 .../Parsers/Gtdt/GtdtParser.c  |  5 +-
 .../Parsers/Iort/IortParser.c  | 26 +-
 .../Parsers/Madt/MadtParser.c  |  4 +-
 .../Parsers/Rsdp/RsdpParser.c  | 10 +++-
 .../Parsers/Slit/SlitParser.c  | 44 
 .../Parsers/Spcr/SpcrParser.c  | 10 +++-
 .../Parsers/Srat/SratParser.c  | 21 +---
 .../UefiShellAcpiViewCommandLib.c  |  5 +-
 .../UefiShellAcpiViewCommandLib.h  |  6 +--
 .../UefiShellAcpiViewCommandLib.inf|  3 ++
 17 files changed, 190 insertions(+), 129 deletions(-)

diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c 
b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
index 088575d0144..6d3bc451acd 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
@@ -19,10 +19,19 @@
 
 STATIC UINT32   gIndent;
 STATIC UINT32   mTableErrorCount;
 STATIC UINT32   mTableWarningCount;
 
+STATIC ACPI_DESCRIPTION_HEADER_INFO AcpiHdrInfo;
+
+/**
+  An ACPI_PARSER array describing the ACPI header.
+**/
+STATIC CONST ACPI_PARSER AcpiHeaderParser[] = {
+  PARSE_ACPI_HEADER ()
+};
+
 /**
   This function resets the ACPI table error counter to Zero.
 **/
 VOID
 ResetErrorCount (
@@ -112,14 +121,17 @@ VerifyChecksum (
   IN BOOLEAN Log,
   IN UINT8*  Ptr,
   IN UINT32  Length
   )
 {
-  UINTN ByteCount = 0;
-  UINT8 Checksum = 0;
+  UINTN ByteCount;
+  UINT8 Checksum;
   UINTN OriginalAttribute;
 
+  ByteCount = 0;
+  Checksum = 0;
+
   while (ByteCount < Length) {
 Checksum += *(Ptr++);
 ByteCount++;
   }
 
@@ -164,15 +176,18 @@ EFIAPI
 DumpRaw (
   IN UINT8* Ptr,
   IN UINT32 Length
   )
 {
-  UINTN ByteCount = 0;
+  UINTN ByteCount;
   UINTN PartLineChars;
-  UINTN AsciiBufferIndex = 0;
+  UINTN AsciiBufferIndex;
   CHAR8 AsciiBuffer[17];
 
+  ByteCount = 0;
+  AsciiBufferIndex = 0;
+
   Print (L"Address  : 0x%p\n", Ptr);
   Print (L"Length   : %d\n", Length);
 
   while (ByteCount < Length) {
 if ((ByteCount & 0x0F) == 0) {
@@ -275,11 +290,14 @@ DumpUint64 (
   )
 {
   // Some fields are not aligned and this causes alignment faults
   // on ARM platforms if the compiler generates LDRD instructions.
   // Perform word access so that LDRD instructions are not generated.
-  UINT64 Val = *(UINT32*)(Ptr + sizeof (UINT32));
+  UINT64 Val;
+
+  Val = *(UINT32*)(Ptr + sizeof (UINT32));
+
   Val <<= 32;
   Val |= *(UINT32*)Ptr;
 
   Print (Format, Val);
 }
@@ -454,17 +472,20 @@ ParseAcpi (
   IN CONST ACPI_PARSER* Parser,
   IN UINT32 ParserItems
 )
 {
   UINT32  Index;
-  UINT32  Offset = 0;
+  UINT32  Offset;
+  BOOLEAN HighLight;
+
+  Offset = 0;
 
   // Increment the Indent
   gIndent += Indent;
 
   if (Trace && (AsciiName != NULL)){
-BOOLEAN HighLight = GetColourHighlighting ();
+HighLight = GetColourHighlighting ();
 UINTN   OriginalAttribute;
 
 if (HighLight) {
   OriginalAttribute = gST->ConOut->Mode->Attribute;
   gST->ConOut->SetAttribute (
@@ -618,15 +639,10 @@ UINT32
 EFIAPI
 DumpAcpiHeader (
   IN UINT8* Ptr
   )
 {
-  ACPI_DESCRIPTION_HEADER_INFO AcpiHdrInfo;
-  ACPI_PARSER AcpiHeaderParser[] = {
-PARSE_ACPI_HEADER ()
-  };
-
   return ParseAcpi (
TRUE,
0,
"ACPI Table Header",
Ptr,
@@ -656,14 +672,10 @@ ParseAcpiHeader (
   OUT CONST UINT32** Length,
   OUT CONST UINT8**  Revision
   )
 {
   UINT32BytesParsed;
-  ACPI_DESCRIPTION_HEADER_INFO  AcpiHdrInfo;
-  ACPI_PARSER AcpiHeaderParser[] = {
-PARSE_ACPI_HEADER ()
-  };
 
   BytesParsed = ParseAcpi (
   FALSE,
   0,
   NULL,
diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h 
b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h
index ecf7dae9038..cea8857bc08 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h
+++

[edk2] [patch 0/2] ShellPkg/UefiShellAcpiViewCommandLib: Fix ECC issues

2018-06-05 Thread Dandan Bi

ECC tool report some coding style issue in UefiShellAcpiViewCommandLib.
This patch series is to clean these issues.

Cc: Sami Mujawar 
Cc: Evan Lloyd 
Cc: Sami Mujawar 
Cc: Evan Lloyd 
Cc: Jaben Carsey 
Cc: Ruiyu Ni 

Dandan Bi (2):
  ShellPkg/UefiShellAcpiViewCommandLib: Fix ECC issues
  ShellPkg/UefiShellAcpiViewCommandLib: Fix ECC issues

 .../UefiShellAcpiViewCommandLib/AcpiParser.c   | 174 +---
 .../UefiShellAcpiViewCommandLib/AcpiParser.h   | 229 -
 .../UefiShellAcpiViewCommandLib/AcpiTableParser.c  |  77 +++
 .../UefiShellAcpiViewCommandLib/AcpiTableParser.h  |  45 ++--
 .../Library/UefiShellAcpiViewCommandLib/AcpiView.c | 113 ++
 .../Library/UefiShellAcpiViewCommandLib/AcpiView.h |  75 ---
 .../Parsers/Bgrt/BgrtParser.c  |  12 +-
 .../Parsers/Dbg2/Dbg2Parser.c  |  34 +--
 .../Parsers/Dsdt/DsdtParser.c  |   7 +-
 .../Parsers/Fadt/FadtParser.c  |  68 --
 .../Parsers/Gtdt/GtdtParser.c  |  57 +++--
 .../Parsers/Iort/IortParser.c  | 159 --
 .../Parsers/Madt/MadtParser.c  |  67 +++---
 .../Parsers/Mcfg/McfgParser.c  |  18 +-
 .../Parsers/Rsdp/RsdpParser.c  |  42 ++--
 .../Parsers/Slit/SlitParser.c  |  61 +++---
 .../Parsers/Spcr/SpcrParser.c  |  42 ++--
 .../Parsers/Srat/SratParser.c  |  82 +---
 .../Parsers/Ssdt/SsdtParser.c  |   7 +-
 .../Parsers/Xsdt/XsdtParser.c  |  11 +-
 .../UefiShellAcpiViewCommandLib.c  |  18 +-
 .../UefiShellAcpiViewCommandLib.h  |  10 +-
 .../UefiShellAcpiViewCommandLib.inf|   3 +
 23 files changed, 835 insertions(+), 576 deletions(-)

-- 
2.14.3.windows.1

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

[edk2] [patch 1/2] ShellPkg/UefiShellAcpiViewCommandLib: Fix ECC issues

2018-06-05 Thread Dandan Bi

Make the function comments follow EDK2 coding style.

Cc: Sami Mujawar 
Cc: Evan Lloyd 
Cc: Sami Mujawar 
Cc: Evan Lloyd 
Cc: Jaben Carsey 
Cc: Ruiyu Ni 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Dandan Bi 
---
 .../UefiShellAcpiViewCommandLib/AcpiParser.c   | 130 ++--
 .../UefiShellAcpiViewCommandLib/AcpiParser.h   | 223 -
 .../UefiShellAcpiViewCommandLib/AcpiTableParser.c  |  27 ++-
 .../UefiShellAcpiViewCommandLib/AcpiTableParser.h  |  39 ++--
 .../Library/UefiShellAcpiViewCommandLib/AcpiView.c |  55 +++--
 .../Library/UefiShellAcpiViewCommandLib/AcpiView.h |  59 +++---
 .../Parsers/Bgrt/BgrtParser.c  |  12 +-
 .../Parsers/Dbg2/Dbg2Parser.c  |  29 +--
 .../Parsers/Dsdt/DsdtParser.c  |   7 +-
 .../Parsers/Fadt/FadtParser.c  |  68 +--
 .../Parsers/Gtdt/GtdtParser.c  |  52 +++--
 .../Parsers/Iort/IortParser.c  | 133 +++-
 .../Parsers/Madt/MadtParser.c  |  63 +++---
 .../Parsers/Mcfg/McfgParser.c  |  18 +-
 .../Parsers/Rsdp/RsdpParser.c  |  32 +--
 .../Parsers/Slit/SlitParser.c  |  17 +-
 .../Parsers/Spcr/SpcrParser.c  |  32 +--
 .../Parsers/Srat/SratParser.c  |  61 +++---
 .../Parsers/Ssdt/SsdtParser.c  |   7 +-
 .../Parsers/Xsdt/XsdtParser.c  |  11 +-
 .../UefiShellAcpiViewCommandLib.c  |  13 +-
 .../UefiShellAcpiViewCommandLib.h  |   4 +-
 22 files changed, 645 insertions(+), 447 deletions(-)

diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c 
b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
index 318f386fda1..088575d0144 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
@@ -1,6 +1,6 @@
-/**
+/** @file
   ACPI parser
 
   Copyright (c) 2016 - 2018, ARM Limited. All rights reserved.
   This program and the accompanying materials
   are licensed and made available under the terms and conditions of the BSD 
License
@@ -19,88 +19,95 @@
 
 STATIC UINT32   gIndent;
 STATIC UINT32   mTableErrorCount;
 STATIC UINT32   mTableWarningCount;
 
-/** This function resets the ACPI table error counter to Zero.
-*/
+/**
+  This function resets the ACPI table error counter to Zero.
+**/
 VOID
 ResetErrorCount (
   VOID
   )
 {
   mTableErrorCount = 0;
 }
 
-/** This function returns the ACPI table error count.
+/**
+  This function returns the ACPI table error count.
 
   @retval Returns the count of errors detected in the ACPI tables.
-*/
+**/
 UINT32
 GetErrorCount (
   VOID
   )
 {
   return mTableErrorCount;
 }
 
-/** This function resets the ACPI table warning counter to Zero.
-*/
+/**
+  This function resets the ACPI table warning counter to Zero.
+**/
 VOID
 ResetWarningCount (
   VOID
   )
 {
   mTableWarningCount = 0;
 }
 
-/** This function returns the ACPI table warning count.
+/**
+  This function returns the ACPI table warning count.
 
   @retval Returns the count of warning detected in the ACPI tables.
-*/
+**/
 UINT32
 GetWarningCount (
   VOID
   )
 {
   return mTableWarningCount;
 }
 
-/** This function increments the ACPI table error counter.
-*/
+/**
+  This function increments the ACPI table error counter.
+**/
 VOID
 EFIAPI
 IncrementErrorCount (
   VOID
   )
 {
   mTableErrorCount++;
 }
 
-/** This function increments the ACPI table warning counter.
-*/
+/**
+  This function increments the ACPI table warning counter.
+**/
 VOID
 EFIAPI
 IncrementWarningCount (
   VOID
   )
 {
   mTableWarningCount++;
 }
 
-/** This function verifies the ACPI table checksum.
+/**
+  This function verifies the ACPI table checksum.
 
   This function verifies the checksum for the ACPI table and optionally
   prints the status.
 
   @param [in] Log If TRUE log the status of the checksum.
   @param [in] Ptr Pointer to the start of the table buffer.
   @param [in] Length  The length of the buffer.
 
   @retval TRUEThe checksum is OK.
   @retval FALSE   The checksum failed.
-*/
+**/
 BOOLEAN
 EFIAPI
 VerifyChecksum (
   IN BOOLEAN Log,
   IN UINT8*  Ptr,
@@ -144,15 +151,16 @@ VerifyChecksum (
   }
 
   return (Checksum == 0);
 }
 
-/** This function performs a raw data dump of the ACPI table.
+/**
+  This function performs a raw data dump of the ACPI table.
 
   @param [in] Ptr Pointer to the start of the table buffer.
   @param [in] Length  The length of the buffer.
-*/
+**/
 VOID
 EFIAPI
 DumpRaw (
   IN UINT8* Ptr,
   IN UINT32 Length
@@ -203,64 +211,64 @@ DumpRaw (
   // Print ASCII data for the final line.
   AsciiBuffer[AsciiBufferIndex] = '\0';
   Print (L"  %a", AsciiBuffer);
 }
 
-/** This function traces 1 byte of data as specified in the
-format string.
+/**
+  This function traces 1 byte

[edk2] [patch 1/3] ShellPkg/Dp: make sure memory is freed before exit

2018-06-05 Thread Dandan Bi

Run dp command now:
Firstly it will get performance records from FPDT and then
parse the DP command. And if encounter invalid parameters,
it will exit directly. Thus the performance records got before
are invalid. And what's worse is that the memory allocated in
getting performance records phase is not freed.

This patch update the code to parse the command firstly and
then get the performance records. And make sure that all the
clean work has been done before exiting.

Cc: Liming Gao 
Cc: Ruiyu Ni 
Cc: Jaben Carsey 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Dandan Bi 
---
 ShellPkg/DynamicCommand/DpDynamicCommand/Dp.c | 70 ++-
 1 file changed, 37 insertions(+), 33 deletions(-)

diff --git a/ShellPkg/DynamicCommand/DpDynamicCommand/Dp.c 
b/ShellPkg/DynamicCommand/DpDynamicCommand/Dp.c
index aa9c2cdf7a8..fe85937f557 100644
--- a/ShellPkg/DynamicCommand/DpDynamicCommand/Dp.c
+++ b/ShellPkg/DynamicCommand/DpDynamicCommand/Dp.c
@@ -390,11 +390,11 @@ BuildCachedGuidHandleTable (
   }
   if (HandleBuffer != NULL) {
 FreePool (HandleBuffer);
 HandleBuffer = NULL;
   }
-  return Status;
+  return EFI_SUCCESS;
 }
 
 /**
   Get Measurement form Fpdt records.
 
@@ -729,39 +729,10 @@ RunDp (
   // initialize the shell lib (we must be in non-auto-init...)
   //
   Status = ShellInitialize();
   ASSERT_EFI_ERROR(Status);
 
-  //
-  // DP dump performance data by parsing FPDT table in ACPI table.
-  // Folloing 3 steps are to get the measurement form the FPDT table.
-  //
-
-  //
-  //1. Get FPDT from ACPI table.
-  //
-  Status = GetBootPerformanceTable ();
-  if (EFI_ERROR(Status)) {
-return Status;
-  }
-
-  //
-  //2. Cache the ModuleGuid and hanlde mapping table.
-  //
-  Status = BuildCachedGuidHandleTable();
-  if (EFI_ERROR (Status)) {
-return Status;
-  }
-
-  //
-  //3. Build the measurement array form the FPDT records.
-  //
-  Status = BuildMeasurementList ();
-  if (EFI_ERROR(Status)) {
-return Status;
-  }
-
   //
   // Process Command Line arguments
   //
   Status = ShellCommandLineParse (ParamList, , NULL, TRUE);
   if (EFI_ERROR(Status)) {
@@ -809,10 +780,42 @@ RunDp (
 #if PROFILING_IMPLEMENTED
 ProfileMode = TRUE;
 #endif  // PROFILING_IMPLEMENTED
   }
 
+  //
+  // DP dump performance data by parsing FPDT table in ACPI table.
+  // Folloing 3 steps are to get the measurement form the FPDT table.
+  //
+
+  //
+  //1. Get FPDT from ACPI table.
+  //
+  Status = GetBootPerformanceTable ();
+  if (EFI_ERROR (Status)) {
+ShellStatus = Status;
+goto Done;
+  }
+
+  //
+  //2. Cache the ModuleGuid and hanlde mapping table.
+  //
+  Status = BuildCachedGuidHandleTable();
+  if (EFI_ERROR (Status)) {
+ShellStatus = Status;
+goto Done;
+  }
+
+  //
+  //3. Build the measurement array form the FPDT records.
+  //
+  Status = BuildMeasurementList ();
+  if (EFI_ERROR (Status)) {
+ShellStatus = SHELL_OUT_OF_RESOURCES;
+goto Done;
+  }
+
   //
   // Initialize the pre-defined cumulative data.
   //
   InitCumulativeData ();
 
@@ -821,21 +824,22 @@ RunDp (
   //
   CustomCumulativeToken = ShellCommandLineGetValue (ParamPackage, L"-c");
   if (CustomCumulativeToken != NULL) {
 CustomCumulativeData = AllocateZeroPool (sizeof (PERF_CUM_DATA));
 if (CustomCumulativeData == NULL) {
-  return SHELL_OUT_OF_RESOURCES;
+  ShellStatus = SHELL_OUT_OF_RESOURCES;
+  goto Done;
 }
 CustomCumulativeData->MinDur = PERF_MAXDUR;
 CustomCumulativeData->MaxDur = 0;
 CustomCumulativeData->Count  = 0;
 CustomCumulativeData->Duration = 0;
 NameSize = StrLen (CustomCumulativeToken) + 1;
 CustomCumulativeData->Name   = AllocateZeroPool (NameSize);
 if (CustomCumulativeData->Name == NULL) {
-  FreePool (CustomCumulativeData);
-  return SHELL_OUT_OF_RESOURCES;
+  ShellStatus = SHELL_OUT_OF_RESOURCES;
+  goto Done;
 }
 UnicodeStrToAsciiStrS (CustomCumulativeToken, CustomCumulativeData->Name, 
NameSize);
   }
 
   //
-- 
2.14.3.windows.1

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

[edk2] [patch 3/3] ShellPkg/Dp: Make the help info align with code

2018-06-05 Thread Dandan Bi

Remove -T, -P, -h flags in the help info of DP to
align with current code implementation.

Cc: Liming Gao 
Cc: Ruiyu Ni 
Cc: Jaben Carsey 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Dandan Bi 
---
 ShellPkg/DynamicCommand/DpDynamicCommand/Dp.uni | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/ShellPkg/DynamicCommand/DpDynamicCommand/Dp.uni 
b/ShellPkg/DynamicCommand/DpDynamicCommand/Dp.uni
index c7eb0fbd71e..ede59069b79 100644
--- a/ShellPkg/DynamicCommand/DpDynamicCommand/Dp.uni
+++ b/ShellPkg/DynamicCommand/DpDynamicCommand/Dp.uni
@@ -96,21 +96,19 @@
 #string STR_GET_HELP_DP #language en-US ""
 ".TH dp 0 "Display performance metrics"\r\n"
 ".SH NAME\r\n"
 "Displays performance metrics that are stored in memory.\r\n"
 ".SH SYNOPSIS\r\n"
-"DP [-b] [-v] [-x] [-s | -A | -R] [-T] [-P] [-t value] [-n count] [-c 
[token]][-i] [-h | -?]\r\n"
+"DP [-b] [-v] [-x] [-s | -A | -R] [-t value] [-n count] [-c [token]][-i] 
[-?]\r\n"
 ".SH OPTIONS\r\n"
 " \r\n"
 "  -b   - Displays on multiple pages\r\n"
 "  -v   - Displays additional information\r\n"
 "  -x   - Prevents display of individual measurements for cumulative 
items\r\n"
 "  -s   - Displays summary information only\r\n"
 "  -A   - Displays all measurements in a list\r\n"
 "  -R   - Displays all measurements in raw format\r\n"
-"  -T   - Displays trace measurements only\r\n"
-"  -P   - Displays profile measurements only\r\n"
 "  -t VALUE - Sets display threshold to VALUE microseconds\r\n"
 "  -n COUNT - Limits display to COUNT lines in All and Raw modes\r\n"
 "  -i   - Displays identifier\r\n"
 "  -c TOKEN - Display pre-defined and custom cumulative data\r\n" 
 " Pre-defined cumulative token are:\r\n"
-- 
2.14.3.windows.1

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

[edk2] [patch 2/3] ShellPkg/Dp: Initialize summary date when run DP

2018-06-05 Thread Dandan Bi

Issue:
When run "dp -s" or ("dp -v") command in shell several times,
the summary reuslts are different each time.

The root cause is that the previous global data "SummaryData"
is not cleaned when the dp command is callled next time.
This patch initializes the global data "SummaryData"
when the dp dymanic command is called.

Cc: Liming Gao 
Cc: Ruiyu Ni 
Cc: Jaben Carsey 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Dandan Bi 
---
 ShellPkg/DynamicCommand/DpDynamicCommand/Dp.c | 23 +++
 1 file changed, 23 insertions(+)

diff --git a/ShellPkg/DynamicCommand/DpDynamicCommand/Dp.c 
b/ShellPkg/DynamicCommand/DpDynamicCommand/Dp.c
index fe85937f557..d8451dbf59f 100644
--- a/ShellPkg/DynamicCommand/DpDynamicCommand/Dp.c
+++ b/ShellPkg/DynamicCommand/DpDynamicCommand/Dp.c
@@ -672,10 +672,28 @@ InitCumulativeData (
 CumData[Index].MaxDur = 0;
 CumData[Index].Duration = 0;
   }
 }
 
+/**
+  Initialize the Summary data.
+
+**/
+VOID
+InitSummaryData (
+  VOID
+  )
+{
+  SummaryData.NumTrace  = 0;
+  SummaryData.NumProfile= 0 ;
+  SummaryData.NumIncomplete = 0;
+  SummaryData.NumSummary= 0;
+  SummaryData.NumHandles= 0;
+  SummaryData.NumPEIMs  = 0;
+  SummaryData.NumGlobal = 0;
+}
+
 /**
   Dump performance data.
   
   @param[in]  ImageHandle The image handle.
   @param[in]  SystemTable The system table.
@@ -817,10 +835,15 @@ RunDp (
   //
   // Initialize the pre-defined cumulative data.
   //
   InitCumulativeData ();
 
+  //
+  // Initialize the Summary data.
+  //
+  InitSummaryData ();
+
   //
   // Init the custom cumulative data.
   //
   CustomCumulativeToken = ShellCommandLineGetValue (ParamPackage, L"-c");
   if (CustomCumulativeToken != NULL) {
-- 
2.14.3.windows.1

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

[edk2] [PATCH] IntelSiliconPkg IntelVTdDxe: Fix incorrect code to clear VTd error

2018-06-05 Thread Star Zeng

According to VTd spec, Software writes the value read from this
field (F) to Clear it. But current code is using 0 to clear the
field, that is incorrect.

And R_FSTS_REG register value clearing should be not in the for loop.

Cc: Jiewen Yao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng 
---
 IntelSiliconPkg/Feature/VTd/IntelVTdDxe/VtdReg.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/VtdReg.c 
b/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/VtdReg.c
index 8dbc83fa2d67..e564d373c756 100644
--- a/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/VtdReg.c
+++ b/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/VtdReg.c
@@ -554,11 +554,13 @@ DumpVtdIfError (
   for (Index = 0; Index < (UINTN)CapReg.Bits.NFR + 1; Index++) {
 FrcdReg.Uint64[1] = MmioRead64 
(mVtdUnitInformation[Num].VtdUnitBaseAddress + ((CapReg.Bits.FRO * 16) + (Index 
* 16) + R_FRCD_REG + sizeof(UINT64)));
 if (FrcdReg.Bits.F != 0) {
-  FrcdReg.Bits.F = 0;
+  //
+  // Software writes the value read from this field (F) to Clear it.
+  //
   MmioWrite64 (mVtdUnitInformation[Num].VtdUnitBaseAddress + 
((CapReg.Bits.FRO * 16) + (Index * 16) + R_FRCD_REG + sizeof(UINT64)), 
FrcdReg.Uint64[1]);
 }
-MmioWrite32 (mVtdUnitInformation[Num].VtdUnitBaseAddress + R_FSTS_REG, 
MmioRead32 (mVtdUnitInformation[Num].VtdUnitBaseAddress + R_FSTS_REG));
   }
+  MmioWrite32 (mVtdUnitInformation[Num].VtdUnitBaseAddress + R_FSTS_REG, 
MmioRead32 (mVtdUnitInformation[Num].VtdUnitBaseAddress + R_FSTS_REG));
 }
   }
 }
-- 
2.7.0.windows.1

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

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

2018-06-05 Thread Liming Gao

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

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

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

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

[edk2] [RFC PATCH] MdePkg/BaseIoLibIntrinsic ARM AARCH64: avoid C code for MMIO access

2018-06-05 Thread Ard Biesheuvel

Even though MMIO shares the address space with ordinary memory, the
accesses involved are *not* ordinary memory accesses, and so it is
a bad idea to let the compiler generate them using pointer dereferences.

Instead, introduce a set of internal accessors implemented in assembler,
and call those from the various Mmio[Read|Write]XX () implementations.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
---
Open coding the MMIO accesses is a good idea in general, but it also works
around an issue that affects EDK2 running under KVM on ARM or AARCH64, where
LTO code generation results in MMIO accesses involving instructions that cannot
be emulated by the hypervisor.

 MdePkg/Library/BaseIoLibIntrinsic/AArch64/IoLibMmio.S| 164 
++
 MdePkg/Library/BaseIoLibIntrinsic/Arm/IoLibMmio.S| 164 
++
 MdePkg/Library/BaseIoLibIntrinsic/Arm/IoLibMmio.asm  | 165 
++
 MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf |   9 +-
 MdePkg/Library/BaseIoLibIntrinsic/IoLibArm.c |  36 ++--
 MdePkg/Library/BaseIoLibIntrinsic/IoLibArm.h | 179 

 6 files changed, 692 insertions(+), 25 deletions(-)

diff --git a/MdePkg/Library/BaseIoLibIntrinsic/AArch64/IoLibMmio.S 
b/MdePkg/Library/BaseIoLibIntrinsic/AArch64/IoLibMmio.S
new file mode 100644
index ..ac96df602f7a
--- /dev/null
+++ b/MdePkg/Library/BaseIoLibIntrinsic/AArch64/IoLibMmio.S
@@ -0,0 +1,164 @@
+#
+#  Copyright (c) 2014-2018, Linaro Limited. All rights reserved.
+#
+#  This program and the accompanying materials are licensed and made available
+#  under the terms and conditions of the BSD License which accompanies this
+#  distribution.  The full text of the license may be found at
+#  http://opensource.org/licenses/bsd-license.php
+#
+#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
IMPLIED.
+#
+#
+
+.text
+.align 3
+
+GCC_ASM_EXPORT(MmioRead8Internal)
+GCC_ASM_EXPORT(MmioWrite8Internal)
+GCC_ASM_EXPORT(MmioRead16Internal)
+GCC_ASM_EXPORT(MmioWrite16Internal)
+GCC_ASM_EXPORT(MmioRead32Internal)
+GCC_ASM_EXPORT(MmioWrite32Internal)
+GCC_ASM_EXPORT(MmioRead64Internal)
+GCC_ASM_EXPORT(MmioWrite64Internal)
+
+//
+//  Reads an 8-bit MMIO register.
+//
+//  Reads the 8-bit MMIO register specified by Address. The 8-bit read value is
+//  returned. This function must guarantee that all MMIO read and write
+//  operations are serialized.
+//
+//  If 8-bit MMIO register operations are not supported, then ASSERT().
+//
+//  @param  Address The MMIO register to read.
+//
+//  @return The value read.
+//
+ASM_PFX(MmioRead8Internal):
+  ldrbw0, [x0]
+  dsb ld
+  ret
+
+//
+//  Writes an 8-bit MMIO register.
+//
+//  Writes the 8-bit MMIO register specified by Address with the value 
specified
+//  by Value and returns Value. This function must guarantee that all MMIO read
+//  and write operations are serialized.
+//
+//  If 8-bit MMIO register operations are not supported, then ASSERT().
+//
+//  @param  Address The MMIO register to write.
+//  @param  Value   The value to write to the MMIO register.
+//
+ASM_PFX(MmioWrite8Internal):
+  dsb st
+  strbw1, [x0]
+  ret
+
+//
+//  Reads a 16-bit MMIO register.
+//
+//  Reads the 16-bit MMIO register specified by Address. The 16-bit read value 
is
+//  returned. This function must guarantee that all MMIO read and write
+//  operations are serialized.
+//
+//  If 16-bit MMIO register operations are not supported, then ASSERT().
+//
+//  @param  Address The MMIO register to read.
+//
+//  @return The value read.
+//
+ASM_PFX(MmioRead16Internal):
+  ldrhw0, [x0]
+  dsb ld
+  ret
+
+//
+//  Writes a 16-bit MMIO register.
+//
+//  Writes the 16-bit MMIO register specified by Address with the value 
specified
+//  by Value and returns Value. This function must guarantee that all MMIO read
+//  and write operations are serialized.
+//
+//  If 16-bit MMIO register operations are not supported, then ASSERT().
+//
+//  @param  Address The MMIO register to write.
+//  @param  Value   The value to write to the MMIO register.
+//
+ASM_PFX(MmioWrite16Internal):
+  dsb st
+  strhw1, [x0]
+  ret
+
+//
+//  Reads a 32-bit MMIO register.
+//
+//  Reads the 32-bit MMIO register specified by Address. The 32-bit read value 
is
+//  returned. This function must guarantee that all MMIO read and write
+//  operations are serialized.
+//
+//  If 32-bit MMIO register operations are not supported, then ASSERT().
+//
+//  @param  Address The MMIO register to read.
+//
+//  @return The value read.
+//
+ASM_PFX(MmioRead32Internal):
+  ldr w0, [x0]
+  dsb ld
+  ret
+
+//
+//  Writes a 32-bit MMIO register.
+//
+//  Writes the 32-bit MMIO register specified by Address with the value 
specified
+//  by Value and returns Value. This function must guarantee that all MMIO read
+//

45 matches

Mail list logo